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@xxxxxxxxxx>
>>> ---
>>> 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
>
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
