Hi Alex, On Fri, 21 May 2021 18:01:05 +0100, Alexandru Elisei <alexandru.elisei@xxxxxxx> wrote: > > Hi Marc, > > On 5/10/21 2:48 PM, Marc Zyngier wrote: > > The vGIC, as architected by ARM, allows a virtual interrupt to > > trigger the deactivation of a physical interrupt. This allows > > the following interrupt to be delivered without requiring an exit. > > If I got this right, the AIC doesn't implement/ignores the s/AIC/M1 CPU/ > ICH_LR_EL2.HW bit. Does it mean that the CPU IF behaves as if HW = > 0b0, meaning it asserts a maintenance interrupt on virtual interrupt > deactivation when ICH_LR_EL2.EOI = 0b1? I assume that's the case, > just double checking. Yes, that's what it looks like. > I am wondering what would happen if we come across an interrupt > controller where the CPU IF cannot assert a maintenance interrupt at > all and we rely on the EOI bit to take us out of the guest to > deactivate the HW interrupt. That'd be broken, and we wouldn't be able to support such an implementation, at least not in configuration such as CPU isolation. > I have to say that it looks a bit strange to start relying on the > maintenance interrupt to emulate interrupt deactivate for hardware > interrupts, but at the same timer allowing an interrupt controller > without a maintenance interrupt. We are not allowing a vGIC without a maintenance interrupt. We are actively mandating it. The M1 does have a working per-CPU maintenance interrupt. It just isn't wired into an interrupt controller, which means we can't mask it. But it is otherwise perfectly functional. > Other than that, this idea sounds like the best thing to do > considering the circumstances, I certainly can't think of anything > better. > > > > > However, some implementations have choosen not to implement this, > > meaning that we will need some unsavoury workarounds to deal with this. > > > > On detecting such a case, taint the kernel and spit a nastygram. > > We'll deal with this in later patches. > > > > Signed-off-by: Marc Zyngier <maz@xxxxxxxxxx> > > --- > > arch/arm64/kvm/vgic/vgic-init.c | 10 ++++++++++ > > include/kvm/arm_vgic.h | 3 +++ > > include/linux/irqchip/arm-vgic-info.h | 2 ++ > > 3 files changed, 15 insertions(+) > > > > diff --git a/arch/arm64/kvm/vgic/vgic-init.c b/arch/arm64/kvm/vgic/vgic-init.c > > index 9fd23f32aa54..5b06a9970a57 100644 > > --- a/arch/arm64/kvm/vgic/vgic-init.c > > +++ b/arch/arm64/kvm/vgic/vgic-init.c > > @@ -524,6 +524,16 @@ int kvm_vgic_hyp_init(void) > > if (!gic_kvm_info) > > return -ENODEV; > > > > + /* > > + * If we get one of these oddball non-GICs, taint the kernel, > > + * as we have no idea of how they *really* behave. > > + */ > > + if (gic_kvm_info->no_hw_deactivation) { > > + kvm_info("Non-architectural vgic, tainting kernel\n"); > > + add_taint(TAINT_CPU_OUT_OF_SPEC, LOCKDEP_STILL_OK); > > + kvm_vgic_global_state.no_hw_deactivation = true; > > + } > > IMO, since this means we're going to rely even more on the > maintenance interrupt (not just for software emulation of level > sensitive interrupts), I think there should be some sort of > dependency on having something that resembles a working maintenance > interrupt. But the timer interrupt is exactly a SW emulation of a level sensitive interrupt in this context. And the maintenance interrupt is still required to use the vGIC. Thanks, M. -- Without deviation from the norm, progress is not possible. _______________________________________________ kvmarm mailing list kvmarm@xxxxxxxxxxxxxxxxxxxxx https://lists.cs.columbia.edu/mailman/listinfo/kvmarm