[PATCH v5 04/20] KVM: arm/arm64: Guard kvm_vgic_map_is_active against !vgic_initialized

[Christoffer Dall]

On Thu, Nov 16, 2017 at 12:29:48PM +0000, Andre Przywara wrote:
> Hi,
> 
> On 27/10/17 09:34, Christoffer Dall wrote:
> > If the vgic is not initialized, don't try to grab its spinlocks or
> > traverse its data structures.
> > 
> > This is important because we soon have to start considering the active
> > state of a virtual interrupts when doing vcpu_load, which may happen
> > early on before the vgic is initialized.
> 
> I understand this patch is on its way to Linus already, but I just found
> this by browsing for VGIC changes...
> 
> > Signed-off-by: Christoffer Dall <cdall at linaro.org>
> > Acked-by: Marc Zyngier <marc.zyngier at arm.com>
> > ---
> >  virt/kvm/arm/vgic/vgic.c | 3 +++
> >  1 file changed, 3 insertions(+)
> > 
> > diff --git a/virt/kvm/arm/vgic/vgic.c b/virt/kvm/arm/vgic/vgic.c
> > index fed717e..e1f7dbc 100644
> > --- a/virt/kvm/arm/vgic/vgic.c
> > +++ b/virt/kvm/arm/vgic/vgic.c
> > @@ -777,6 +777,9 @@ bool kvm_vgic_map_is_active(struct kvm_vcpu *vcpu, unsigned int virt_irq)
> >  	struct vgic_irq *irq = vgic_get_irq(vcpu->kvm, vcpu, virt_irq);
> 
> Isn't vgic_get_irq() already accessing VGIC data structures? In which
> case this assignment should be moved after the vgic_initialized() check
> below?

Theoretically, yes, but we're saved by two things:

First, this is only ever called on PPIs from the timer code, which are
always statically allocated as part of the VCPU.

Second, this isn't actually needed anymore after the irqchip in
userspace support, which partially reworked the timer init sequence
(done after the first version of these patches), so this patch could
actually have been entirely dropped, or reworked as you suggest.

Thanks,
-Christoffer