[PATCH v3 09/11] KVM: arm/arm64: vgic: Prevent userspace injection of a mapped interrupt

[marc.zyngier@arm.com (Marc Zyngier)]

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@arm.com>
>> ---
>>  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?

Thanks,

	M.
-- 
Jazz is not dead. It just smells funny...