On 29 June 2018 at 14:29, Luc Michel <luc.mic...@greensocs.com> wrote:
> Implement write access to GICD_ISACTIVERn and GICD_ICACTIVERn registers
> in the GICv2. Those registers allow to set or clear the active state of
> an IRQ in the distributor.
>
> Signed-off-by: Luc Michel <luc.mic...@greensocs.com>
> ---
> hw/intc/arm_gic.c | 41 +++++++++++++++++++++++++++++++++++++++--
> 1 file changed, 39 insertions(+), 2 deletions(-)
>
> diff --git a/hw/intc/arm_gic.c b/hw/intc/arm_gic.c
> index ea0323f969..5755a4fb2c 100644
> --- a/hw/intc/arm_gic.c
> +++ b/hw/intc/arm_gic.c
> @@ -982,9 +982,46 @@ static void gic_dist_writeb(void *opaque, hwaddr offset,
> GIC_CLEAR_PENDING(irq + i, ALL_CPU_MASK);
> }
> }
> + } else if (offset < 0x380) {
> + /* Interrupt Set Active. */
These are read-only for the GICv1 and 11MPCore, so we need
to guard this code with a check on s->revision == 2:
if (s->revision != 2) {
goto bad_reg;
}
> + irq = (offset - 0x300) * 8 + GIC_BASE_IRQ;
> + if (irq >= s->num_irq) {
> + goto bad_reg;
> + }
> +
> + /* This register is banked per-cpu for PPIs */
> + int cm = irq < GIC_INTERNAL ? (1 << cpu) : ALL_CPU_MASK;
> +
> + for (i = 0; i < 8; i++) {
> + if (s->security_extn && !attrs.secure &&
> + !GIC_DIST_TEST_GROUP(irq + i, 1 << cpu)) {
> + continue; /* Ignore Non-secure access of Group0 IRQ */
> + }
> +
> + if (value & (1 << i)) {
> + GIC_DIST_SET_ACTIVE(irq + i, cm);
You don't introduce this macro until patch 2, so this patch
has to go after that, so that we still compile at all points
in the patch sequence.
> + }
> + }
> } else if (offset < 0x400) {
> - /* Interrupt Active. */
> - goto bad_reg;
> + /* Interrupt Clear Active. */
> + irq = (offset - 0x380) * 8 + GIC_BASE_IRQ;
> + if (irq >= s->num_irq) {
> + goto bad_reg;
> + }
These registers only exist for GICv2, so again a check
on s->revision is required.
You also need to implement reading of GICD_ICACTIVERn for
GICv2 -- the code in gic_dist_readb() only implements reads of
GICD_ISACTIVERn (which is all that GICv1 needed).
> +
> + /* This register is banked per-cpu for PPIs */
> + int cm = irq < GIC_INTERNAL ? (1 << cpu) : ALL_CPU_MASK;
> +
> + for (i = 0; i < 8; i++) {
> + if (s->security_extn && !attrs.secure &&
> + !GIC_DIST_TEST_GROUP(irq + i, 1 << cpu)) {
> + continue; /* Ignore Non-secure access of Group0 IRQ */
> + }
> +
> + if (value & (1 << i)) {
> + GIC_DIST_CLEAR_ACTIVE(irq + i, cm);
> + }
> + }
> } else if (offset < 0x800) {
> /* Interrupt Priority. */
> irq = (offset - 0x400) + GIC_BASE_IRQ;
> --
> 2.17.1
Looks good otherwise.
thanks
-- PMM
