Hi Marc,
On 08/04/2015 06:44 PM, Marc Zyngier wrote:
> On 04/08/15 17:21, Eric Auger wrote:
>> Hi Marc,
>> On 07/24/2015 05:55 PM, Marc Zyngier wrote:
>>> Virtual interrupts mapped to a HW interrupt should only be triggered
>>> from inside the kernel. Otherwise, you could end up confusing the
>>> kernel (and the GIC's) state machine.
>>>
>>> Rearrange the injection path so that kvm_vgic_inject_irq is
>>> used for non-mapped interrupts, and kvm_vgic_inject_mapped_irq is
>>> used for mapped interrupts. The latter should only be called from
>>> inside the kernel (timer, VFIO).
>> nit: I would replace VFIO by irqfd.
>> VFIO just triggers the eventfd/irqfd. This is KVM/irqfd that injects the
>> virtual irq upon the irqfd signaling and he irqfd adaptation/ARM
>> currently is implemented in vgic.c
>
> Ah, thanks for reminding me of the right terminology, I tend to think of
> it as one big bag of nasty tricks... ;-)
>
> I'll update the commit message.
>
>>>
>>> Signed-off-by: Marc Zyngier <marc.zyngier@xxxxxxx>
>>> ---
>>> include/kvm/arm_vgic.h | 2 +
>>> virt/kvm/arm/vgic.c | 99 ++++++++++++++++++++++++++++++++++----------------
>>> 2 files changed, 70 insertions(+), 31 deletions(-)
>>>
>>> diff --git a/include/kvm/arm_vgic.h b/include/kvm/arm_vgic.h
>>> index 7306b4b..f6bfd79 100644
>>> --- a/include/kvm/arm_vgic.h
>>> +++ b/include/kvm/arm_vgic.h
>>> @@ -351,6 +351,8 @@ void kvm_vgic_flush_hwstate(struct kvm_vcpu *vcpu);
>>> void kvm_vgic_sync_hwstate(struct kvm_vcpu *vcpu);
>>> int kvm_vgic_inject_irq(struct kvm *kvm, int cpuid, unsigned int irq_num,
>>> bool level);
>>> +int kvm_vgic_inject_mapped_irq(struct kvm *kvm, int cpuid,
>>> + struct irq_phys_map *map, bool level);
>>> void vgic_v3_dispatch_sgi(struct kvm_vcpu *vcpu, u64 reg);
>>> int kvm_vgic_vcpu_pending_irq(struct kvm_vcpu *vcpu);
>>> int kvm_vgic_vcpu_active_irq(struct kvm_vcpu *vcpu);
>>> diff --git a/virt/kvm/arm/vgic.c b/virt/kvm/arm/vgic.c
>>> index 3f7b690..e40ef70 100644
>>> --- a/virt/kvm/arm/vgic.c
>>> +++ b/virt/kvm/arm/vgic.c
>>> @@ -1533,7 +1533,8 @@ static int vgic_validate_injection(struct kvm_vcpu *vcpu, int irq, int level)
>>> }
>>>
>>> static int vgic_update_irq_pending(struct kvm *kvm, int cpuid,
>>> - unsigned int irq_num, bool level)
>>> + struct irq_phys_map *map,
>>> + unsigned int irq_num, bool level)
>>> {
>> In vgic_update_irq_pending, I needed to modify the following line and
>> add the "&& !map".
>>
>> if (!vgic_validate_injection(vcpu, irq_num, level) && !map) {
>>
>> Without that, the level being not properly modeled for level sensitive
>> forwarded IRQs, the 2d injection fails.
>
> Ah! Is that because we never see the line being reset to zero, and the
> VGIC still sees the line as pending at the distributor level?
yes indeed
Eric
>
> Thanks,
>
> M.
>
--
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
