qemu-arm
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH] hw/intc/arm_gic: Fix set/clear pending of PPI/SPI


From: Peter Maydell
Subject: Re: [PATCH] hw/intc/arm_gic: Fix set/clear pending of PPI/SPI
Date: Mon, 26 Jul 2021 14:02:55 +0100

On Mon, 26 Jul 2021 at 09:06, Sebastian Huber
<sebastian.huber@embedded-brains.de> wrote:
> Ok, I will remove this part from the patch in v2. I probably didn't
> fully understand how the Qemu GICv2 emulation works. What I wanted to
> address is this behaviour (see GICv2 manual) when someone changes the
> GICD_ITARGETSR<n> (n > 1):
>
> "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.
>
> Note
>
> There is a small but finite time required for any change to take effect."

We do get this wrong, but none of your changes to the file actually
change this behaviour :-)

What we currently do for writes to GICD_ITARGETS<r> is:

        /* Interrupt CPU Target. RAZ/WI on uniprocessor GICs, with the
         * annoying exception of the 11MPCore's GIC.
         */
        if (s->num_cpu != 1 || s->revision == REV_11MPCORE) {
            irq = (offset - 0x800);
            if (irq >= s->num_irq) {
                goto bad_reg;
            }
            if (irq < 29 && s->revision == REV_11MPCORE) {
                value = 0;
            } else if (irq < GIC_INTERNAL) {
                value = ALL_CPU_MASK;
            }
            s->irq_target[irq] = value & ALL_CPU_MASK;
        }

which is to say that we update irq_target[] but we don't change the
status of any pending interrupt. We should add code here that
checks whether irq is pending on any core and if so marks it as
instead pending on the targets we just set up, something like

 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);
 }

thanks
-- PMM



reply via email to

[Prev in Thread] Current Thread [Next in Thread]