[PATCH 5/5] arm/arm64: KVM: Initialize the vgic on-demand when injecting IRQs

[eric.auger@linaro.org (Eric Auger)]

On 12/11/2014 01:01 PM, Christoffer Dall wrote:
> On Wed, Dec 10, 2014 at 01:45:50PM +0100, Eric Auger wrote:
>> On 12/09/2014 04:44 PM, Christoffer Dall wrote:
>>> Userspace assumes that it can wire up IRQ injections after having
>>> created all VCPUs and after having created the VGIC, but potentially
>>> before starting the first VCPU.  This can currently lead to lost IRQs
>>> because the state of that IRQ injection is not stored anywhere and we
>>> don't return an error to userspace.
>>>
>>> We haven't seen this problem manifest itself yet, 
>> Actually we did with VFIO signaling setup before VGIC init!
>> presumably because
> 
> well, not with code in mainline
> 
>>> guests reset the devices on boot, but this could cause issues with
>>> migration and other non-standard startup configurations.
>>>
>>> Signed-off-by: Christoffer Dall <christoffer.dall@linaro.org>
>>> ---
>>>  virt/kvm/arm/vgic.c | 9 +++++++--
>>>  1 file changed, 7 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/virt/kvm/arm/vgic.c b/virt/kvm/arm/vgic.c
>>> index c98cc6b..feef015 100644
>>> --- a/virt/kvm/arm/vgic.c
>>> +++ b/virt/kvm/arm/vgic.c
>>> @@ -1693,8 +1693,13 @@ out:
>>>  int kvm_vgic_inject_irq(struct kvm *kvm, int cpuid, unsigned int irq_num,
>>>  			bool level)
>>>  {
>>> -	if (likely(vgic_ready(kvm)) &&
>>> -	    vgic_update_irq_pending(kvm, cpuid, irq_num, level))
>>> +	if (unlikely(!vgic_initialized(kvm))) {
>>> +		mutex_lock(&kvm->lock);
>>> +		vgic_init(kvm);
>>> +		mutex_unlock(&kvm->lock);
>>> +	}
>> I was previously encouraged to test the virtual interrupt controller
>> readiness when setting irqfd up(proposal made in
>> https://lkml.org/lkml/2014/12/3/601). I guess this becomes useless now,
>> correct? Reviewed-by on the whole series.
>>
> I think we should move to your userspace explicit init for all
> non-legacy userspace and only support gicv3 and vfio/irqfd stuff with
> userspace explicitly initializing the vgic.

Hi Christoffer,

The use case I have in mind still is VFIO+irqfd:
since we cannot preclude the user from ignoring the userspace explicit
init and setting up VFIO signaling+irqfd before vgic init, to me there
is a risk injection starts even before creation. Either we test
irqchip_in_kernel in kvm_vgic_inject_irq or we must have a test when
setting up irqfd as proposed in above patch.

Actually before being able to inject any virtual IRQ we weed even more:
if virtual IRQ settings were not yet defined by the guest we do not know
what to do with the IRQ. We must a least know whether it is level or
edge. Current irq_cfg bitmap might be insufficient to store the info
since it only has 2 states and by chance I use a level-sensitive IRQ and
my QEMU pieces pay attention to that sequencing. I guess the problem is
the same for user-space injection, isn't it?

Eric

> 
> Thanks for the review!
> 
> -Christoffer
>