On Tue, 7 May 2024 at 14:00, Sebastian Huber
<sebastian.hu...@embedded-brains.de> wrote:
>
> According to the GICv2 specification section 4.3.12, "Interrupt Processor
> Targets Registers, GICD_ITARGETSRn":
>
> "Any change to a CPU targets field value:
> [...]
> * Has an effect on any pending interrupts. This means:
> - adding a CPU interface to the target list of a pending interrupt makes
> that
> interrupt pending on that CPU interface
> - removing a CPU interface from the target list of a pending interrupt
> removes the pending state of that interrupt on that CPU interface."
>
> Signed-off-by: Sebastian Huber <sebastian.hu...@embedded-brains.de>
> ---
> hw/intc/arm_gic.c | 7 +++++++
> 1 file changed, 7 insertions(+)
>
> diff --git a/hw/intc/arm_gic.c b/hw/intc/arm_gic.c
> index 20b3f701e0..79aee56053 100644
> --- a/hw/intc/arm_gic.c
> +++ b/hw/intc/arm_gic.c
> @@ -1397,6 +1397,13 @@ static void gic_dist_writeb(void *opaque, hwaddr
> offset,
> value = ALL_CPU_MASK;
> }
> s->irq_target[irq] = value & ALL_CPU_MASK;
> + if (irq >= GIC_INTERNAL && s->irq_state[irq].pending) {
> + /*
> + * Changing the target of an interrupt that is currently
> + * pending updates the set of CPUs it is pending on.
> + */
> + GIC_DIST_SET_PENDING(irq, value);
Looking back at the 2021 thread this is the change I suggested then,
but I think I was wrong. This will set the pending bit for the new
specified set of targets, but it won't remove it from any CPUs that
previously were targeted and are not in the new target list (because
GIC_DIST_SET_PENDING does a logical OR into the pending field).
So I think what we want is
s->irq_state[irq].pending = value & ALL_CPU_MASK;
> + }
> }
> } else if (offset < 0xf00) {
> /* Interrupt Configuration. */
> --
thanks
-- PMM
