[PATCH] arm/gicv3: update virtual irq state after IAR register read

[PATCH] arm/gicv3: update virtual irq state after IAR register read

[Jeff Kubascik]

The IAR0/IAR1 register is used to acknowledge an interrupt - a read of the
register activates the highest priority pending interrupt and provides its
interrupt ID. Activating an interrupt can change the CPU's virtual interrupt
state - this change makes sure the virtual irq state is updated.

Signed-off-by: Jeff Kubascik <jeff.kubascik@dornerworks.com>
---
Hello,

I am using the ARMv8 version of QEMU configured with the gicv3 interrupt
controller to run the Xen hypervisor with RTEMS as a guest VM (AArch32). I
have noticed that when Xen injects a virtual irq to the guest VM, the guest
gets stuck in the interrupt handler.

The guest's interrupt handler does the following in order:
  - Reads IAR1 to acknowledge the interrupt and get the INTID
  - Unmasks interrupts by clearing the I bit in the CPSR register
  - Dispatches the interrupt to the appropriate driver
  - Restores the I bit in the CPSR state
  - Writes the INTID to EOIR1 to drop priority and deactivate the interrupt

However, when the I bit was cleared, QEMU immediately raised the same
virtual interrupt again. This process repeated itself indefinitely.

The read of the IAR1 register should activate the interrupt and prevent it
from firing again. However, the gicv3 device code wasn't updating the
irq_line_state, so the CPU_INTERRUPT_VIRQ flag remained set. Therefore, when
I bit was cleared, the CPU immediately switched to the exception handler.

I have tested this patch, and it works for Xen with both an AArch64 Linux
guest and an AArch32 RTEMS guest.

Sincerely,
Jeff Kubascik
---
 hw/intc/arm_gicv3_cpuif.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/hw/intc/arm_gicv3_cpuif.c b/hw/intc/arm_gicv3_cpuif.c
index a254b0ce87..08e000e33c 100644
--- a/hw/intc/arm_gicv3_cpuif.c
+++ b/hw/intc/arm_gicv3_cpuif.c
@@ -664,6 +664,9 @@ static uint64_t icv_iar_read(CPUARMState *env, const ARMCPRegInfo *ri)
 
     trace_gicv3_icv_iar_read(ri->crm == 8 ? 0 : 1,
                              gicv3_redist_affid(cs), intid);
+
+    gicv3_cpuif_virt_update(cs);
+
     return intid;
 }
 
-- 
2.17.1

Re: [PATCH] arm/gicv3: update virtual irq state after IAR register read

[Philippe Mathieu-Daudé]

On 1/13/20 4:46 PM, Jeff Kubascik wrote:
> The IAR0/IAR1 register is used to acknowledge an interrupt - a read of the
> register activates the highest priority pending interrupt and provides its
> interrupt ID. Activating an interrupt can change the CPU's virtual interrupt
> state - this change makes sure the virtual irq state is updated.
> 
> Signed-off-by: Jeff Kubascik <jeff.kubascik@dornerworks.com>
> ---
> Hello,
> 
> I am using the ARMv8 version of QEMU configured with the gicv3 interrupt
> controller to run the Xen hypervisor with RTEMS as a guest VM (AArch32). I
> have noticed that when Xen injects a virtual irq to the guest VM, the guest
> gets stuck in the interrupt handler.
> 
> The guest's interrupt handler does the following in order:
>    - Reads IAR1 to acknowledge the interrupt and get the INTID
>    - Unmasks interrupts by clearing the I bit in the CPSR register
>    - Dispatches the interrupt to the appropriate driver
>    - Restores the I bit in the CPSR state
>    - Writes the INTID to EOIR1 to drop priority and deactivate the interrupt
> 
> However, when the I bit was cleared, QEMU immediately raised the same
> virtual interrupt again. This process repeated itself indefinitely.
> 
> The read of the IAR1 register should activate the interrupt and prevent it
> from firing again. However, the gicv3 device code wasn't updating the
> irq_line_state, so the CPU_INTERRUPT_VIRQ flag remained set. Therefore, when
> I bit was cleared, the CPU immediately switched to the exception handler.
> 
> I have tested this patch, and it works for Xen with both an AArch64 Linux
> guest and an AArch32 RTEMS guest.
> 
> Sincerely,
> Jeff Kubascik
> ---
>   hw/intc/arm_gicv3_cpuif.c | 3 +++
>   1 file changed, 3 insertions(+)
> 
> diff --git a/hw/intc/arm_gicv3_cpuif.c b/hw/intc/arm_gicv3_cpuif.c
> index a254b0ce87..08e000e33c 100644
> --- a/hw/intc/arm_gicv3_cpuif.c
> +++ b/hw/intc/arm_gicv3_cpuif.c
> @@ -664,6 +664,9 @@ static uint64_t icv_iar_read(CPUARMState *env, const ARMCPRegInfo *ri)
>   
>       trace_gicv3_icv_iar_read(ri->crm == 8 ? 0 : 1,
>                                gicv3_redist_affid(cs), intid);
> +
> +    gicv3_cpuif_virt_update(cs);
> +
>       return intid;
>   }
>   
> 

Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>

Re: [PATCH] arm/gicv3: update virtual irq state after IAR register read

[Peter Maydell]

On Mon, 13 Jan 2020 at 15:46, Jeff Kubascik
<jeff.kubascik@dornerworks.com> wrote:
>
> The IAR0/IAR1 register is used to acknowledge an interrupt - a read of the
> register activates the highest priority pending interrupt and provides its
> interrupt ID. Activating an interrupt can change the CPU's virtual interrupt
> state - this change makes sure the virtual irq state is updated.
>
> Signed-off-by: Jeff Kubascik <jeff.kubascik@dornerworks.com>
> ---



Applied to target-arm.next, thanks.

-- PMM