[PATCH v4 09/13] ARM: KVM: VGIC interrupt injection

[Christoffer Dall]

On Mon, Dec 3, 2012 at 9:21 AM, Marc Zyngier <marc.zyngier at arm.com> wrote:
> On 03/12/12 13:25, Will Deacon wrote:
>> On Sat, Nov 10, 2012 at 03:45:18PM +0000, Christoffer Dall wrote:
>>> From: Marc Zyngier <marc.zyngier at arm.com>
>>>
>>> Plug the interrupt injection code. Interrupts can now be generated
>>> from user space.
>>>
>>> Signed-off-by: Marc Zyngier <marc.zyngier at arm.com>
>>> Signed-off-by: Christoffer Dall <c.dall at virtualopensystems.com>
>>> ---
>>>  arch/arm/include/asm/kvm_vgic.h |    8 +++
>>>  arch/arm/kvm/arm.c              |   29 +++++++++++++
>>>  arch/arm/kvm/vgic.c             |   90 +++++++++++++++++++++++++++++++++++++++
>>>  3 files changed, 127 insertions(+)
>>>
>>> diff --git a/arch/arm/include/asm/kvm_vgic.h b/arch/arm/include/asm/kvm_vgic.h
>>> index 7229324..6e3d303 100644
>>> --- a/arch/arm/include/asm/kvm_vgic.h
>>> +++ b/arch/arm/include/asm/kvm_vgic.h
>>> @@ -241,6 +241,8 @@ struct kvm_exit_mmio;
>>>  int kvm_vgic_set_addr(struct kvm *kvm, unsigned long type, u64 addr);
>>>  void kvm_vgic_sync_to_cpu(struct kvm_vcpu *vcpu);
>>>  void kvm_vgic_sync_from_cpu(struct kvm_vcpu *vcpu);
>>> +int kvm_vgic_inject_irq(struct kvm *kvm, int cpuid, unsigned int irq_num,
>>> +                    bool level);
>>>  int kvm_vgic_vcpu_pending_irq(struct kvm_vcpu *vcpu);
>>>  bool vgic_handle_mmio(struct kvm_vcpu *vcpu, struct kvm_run *run,
>>>                    struct kvm_exit_mmio *mmio);
>>> @@ -271,6 +273,12 @@ static inline void kvm_vgic_vcpu_init(struct kvm_vcpu *vcpu) {}
>>>  static inline void kvm_vgic_sync_to_cpu(struct kvm_vcpu *vcpu) {}
>>>  static inline void kvm_vgic_sync_from_cpu(struct kvm_vcpu *vcpu) {}
>>>
>>> +static inline int kvm_vgic_inject_irq(struct kvm *kvm, int cpuid,
>>> +                                  const struct kvm_irq_level *irq)
>>> +{
>>> +    return 0;
>>> +}
>>> +
>>>  static inline int kvm_vgic_vcpu_pending_irq(struct kvm_vcpu *vcpu)
>>>  {
>>>      return 0;
>>> diff --git a/arch/arm/kvm/arm.c b/arch/arm/kvm/arm.c
>>> index 3ac1aab..f43da01 100644
>>> --- a/arch/arm/kvm/arm.c
>>> +++ b/arch/arm/kvm/arm.c
>>> @@ -764,10 +764,31 @@ int kvm_vm_ioctl_irq_line(struct kvm *kvm, struct kvm_irq_level *irq_level)
>>>
>>>      switch (irq_type) {
>>>      case KVM_ARM_IRQ_TYPE_CPU:
>>> +            if (irqchip_in_kernel(kvm))
>>> +                    return -ENXIO;
>>> +
>>>              if (irq_num > KVM_ARM_IRQ_CPU_FIQ)
>>>                      return -EINVAL;
>>>
>>>              return vcpu_interrupt_line(vcpu, irq_num, level);
>>> +#ifdef CONFIG_KVM_ARM_VGIC
>>> +    case KVM_ARM_IRQ_TYPE_PPI:
>>> +            if (!irqchip_in_kernel(kvm))
>>> +                    return -ENXIO;
>>> +
>>> +            if (irq_num < 16 || irq_num > 31)
>>> +                    return -EINVAL;
>>
>> It's our favourite two numbers again! :)
>
> I already fixed a number of them. Probably missed this one though.
>
>>> +
>>> +            return kvm_vgic_inject_irq(kvm, vcpu->vcpu_id, irq_num, level);
>>> +    case KVM_ARM_IRQ_TYPE_SPI:
>>> +            if (!irqchip_in_kernel(kvm))
>>> +                    return -ENXIO;
>>> +
>>> +            if (irq_num < 32 || irq_num > KVM_ARM_IRQ_GIC_MAX)
>>> +                    return -EINVAL;
>>> +
>>> +            return kvm_vgic_inject_irq(kvm, 0, irq_num, level);
>>> +#endif
>>>      }
>>>
>>>      return -EINVAL;
>>> @@ -849,6 +870,14 @@ long kvm_arch_vm_ioctl(struct file *filp,
>>>      void __user *argp = (void __user *)arg;
>>>
>>>      switch (ioctl) {
>>> +#ifdef CONFIG_KVM_ARM_VGIC
>>> +    case KVM_CREATE_IRQCHIP: {
>>> +            if (vgic_present)
>>> +                    return kvm_vgic_create(kvm);
>>> +            else
>>> +                    return -EINVAL;
>>
>> ENXIO? At least, that's what you use when setting the GIC addresses.
>
> -EINVAL seems to be one of the values other archs are using. -ENXIO is
> not one of them for KVM_CREATE_IRQCHIP. Doesn't mean they are right, but
> for the sake of keeping userspace happy, I'm not really inclined to
> change this.
>

We don't have user space code relying on this, and EINVAL is
misleading, so let's use ENXIO to be consistent with
SET_DEVICE_ADDRESS. No error values are specified in the API docs, so
we should use the most appropriate one.

You fix?

-Christoffer