| Message ID | 20231218113305.2511480-4-peter.maydell@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | target/arm: Implement emulation of nested virtualization | expand |
On 12/18/23 22:32, Peter Maydell wrote: > The hypervisor can deliver (virtual) LPIs to a guest by setting up a > list register to have an intid which is an LPI. The GIC has to treat > these a little differently to standard interrupt IDs, because LPIs > have no Active state, and so the guest will only EOI them, it will > not also deactivate them. So icv_eoir_write() must do two things: > > * if the LPI ID is not in any list register, we drop the > priority but do not increment the EOI count > * if the LPI ID is in a list register, we immediately deactivate > it, regardless of the split-drop-and-deactivate control > > This can be seen in the VirtualWriteEOIR0() and VirtualWriteEOIR1() > pseudocode in the GICv3 architecture specification. > > Without this fix, potentially a hypervisor guest might stall because > LPIs get stuck in a bogus Active+Pending state. > > Cc: qemu-stable@nongnu.org > Signed-off-by: Peter Maydell <peter.maydell@linaro.org> > --- > Weirdly, I only saw this being a problem when the hypervisor guest > was an EL2-enabled one under my FEAT_NV/FEAT_NV2 implementation. > But there's nothing FEAT_NV specific about the bug. > --- > hw/intc/arm_gicv3_cpuif.c | 17 +++++++++++++---- > 1 file changed, 13 insertions(+), 4 deletions(-) Reviewed-by: Richard Henderson <richard.henderson@linaro.org> r~
diff --git a/hw/intc/arm_gicv3_cpuif.c b/hw/intc/arm_gicv3_cpuif.c index ab1a00508e6..258dee1b808 100644 --- a/hw/intc/arm_gicv3_cpuif.c +++ b/hw/intc/arm_gicv3_cpuif.c @@ -1434,16 +1434,25 @@ static void icv_eoir_write(CPUARMState *env, const ARMCPRegInfo *ri, idx = icv_find_active(cs, irq); if (idx < 0) { - /* No valid list register corresponding to EOI ID */ - icv_increment_eoicount(cs); + /* + * No valid list register corresponding to EOI ID; if this is a vLPI + * not in the list regs then do nothing; otherwise increment EOI count + */ + if (irq < GICV3_LPI_INTID_START) { + icv_increment_eoicount(cs); + } } else { uint64_t lr = cs->ich_lr_el2[idx]; int thisgrp = (lr & ICH_LR_EL2_GROUP) ? GICV3_G1NS : GICV3_G0; int lr_gprio = ich_lr_prio(lr) & icv_gprio_mask(cs, grp); if (thisgrp == grp && lr_gprio == dropprio) { - if (!icv_eoi_split(env, cs)) { - /* Priority drop and deactivate not split: deactivate irq now */ + if (!icv_eoi_split(env, cs) || irq >= GICV3_LPI_INTID_START) { + /* + * Priority drop and deactivate not split: deactivate irq now. + * LPIs always get their active state cleared immediately + * because no separate deactivate is expected. + */ icv_deactivate_irq(cs, idx); } }
The hypervisor can deliver (virtual) LPIs to a guest by setting up a list register to have an intid which is an LPI. The GIC has to treat these a little differently to standard interrupt IDs, because LPIs have no Active state, and so the guest will only EOI them, it will not also deactivate them. So icv_eoir_write() must do two things: * if the LPI ID is not in any list register, we drop the priority but do not increment the EOI count * if the LPI ID is in a list register, we immediately deactivate it, regardless of the split-drop-and-deactivate control This can be seen in the VirtualWriteEOIR0() and VirtualWriteEOIR1() pseudocode in the GICv3 architecture specification. Without this fix, potentially a hypervisor guest might stall because LPIs get stuck in a bogus Active+Pending state. Cc: qemu-stable@nongnu.org Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- Weirdly, I only saw this being a problem when the hypervisor guest was an EL2-enabled one under my FEAT_NV/FEAT_NV2 implementation. But there's nothing FEAT_NV specific about the bug. --- hw/intc/arm_gicv3_cpuif.c | 17 +++++++++++++---- 1 file changed, 13 insertions(+), 4 deletions(-)