On Mon, Dec 3, 2012 at 9:21 AM, Marc Zyngier [off-list ref] wrote:

On 03/12/12 13:25, Will Deacon wrote:

quoted

On Sat, Nov 10, 2012 at 03:45:18PM +0000, Christoffer Dall wrote:

quoted

From: Marc Zyngier <redacted>

Plug the interrupt injection code. Interrupts can now be generated
from user space.

Signed-off-by: Marc Zyngier <redacted>
Signed-off-by: Christoffer Dall <redacted>
---
 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.

quoted

quoted

+
+            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