[kvmarm] [Android-virt] [PATCH v2 06/11] ARM: KVM: VGIC interrupt injection

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

On 30/07/12 15:56, Antonios Motakis wrote:

Hi Antonio,

> By chance I noticed some minor issues when compiling the kernel with KVM, but your VGIC support disabled.
> 
> On Thu, Jul 5, 2012 at 5:28 PM, Marc Zyngier <marc.zyngier at arm.com<mailto:marc.zyngier at arm.com>> wrote:
> Plug the interrupt injection code. Interrupts can now be generated
> from user space.
> 
> Signed-off-by: Marc Zyngier <marc.zyngier at arm.com<mailto:marc.zyngier at arm.com>>
> ---
>  arch/arm/include/asm/kvm_vgic.h |    1 +
>  arch/arm/kvm/arm.c              |   43 ++++++++++++++++++++++++++++--
>  arch/arm/kvm/vgic.c             |   56 +++++++++++++++++++++++++++++++++++++++
>  3 files changed, 98 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/arm/include/asm/kvm_vgic.h b/arch/arm/include/asm/kvm_vgic.h
> index 481f4a9..48c27da 100644
> --- a/arch/arm/include/asm/kvm_vgic.h
> +++ b/arch/arm/include/asm/kvm_vgic.h
> @@ -220,6 +220,7 @@ struct kvm_exit_mmio;
>  #ifdef CONFIG_KVM_ARM_VGIC
>  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, const struct kvm_irq_level *irq);
>  int kvm_vgic_vcpu_pending_irq(struct kvm_vcpu *vcpu);
>  int vgic_handle_mmio(struct kvm_vcpu *vcpu, struct kvm_run *run,
>                      struct kvm_exit_mmio *mmio);
> diff --git a/arch/arm/kvm/arm.c b/arch/arm/kvm/arm.c
> index bdf0f86..7b27a4e 100644
> --- a/arch/arm/kvm/arm.c
> +++ b/arch/arm/kvm/arm.c
> @@ -654,6 +654,12 @@ int kvm_vm_ioctl_irq_line(struct kvm *kvm, struct kvm_irq_level *irq_level)
>         bool set;
>         int bit_nr;
> 
> +       if (irqchip_in_kernel(kvm)) {
> +               if (irq_level->irq < 32)
> +                       return -EINVAL;
> +               return kvm_vgic_inject_irq(kvm, 0, irq_level);
> +       }
> +
> 
> This will cause the compilation to fail. Should be "#ifdef"ed like everything else.

Yes, I know about this one, and it is now fixed in my latest tree.

>         vcpu_idx = irq_level->irq >> 1;
>         if (vcpu_idx >= KVM_MAX_VCPUS)
>                 return -EINVAL;
> @@ -696,7 +702,28 @@ int kvm_vm_ioctl_irq_line(struct kvm *kvm, struct kvm_irq_level *irq_level)
>  long kvm_arch_vcpu_ioctl(struct file *filp,
>                          unsigned int ioctl, unsigned long arg)
>  {
> -       return -EINVAL;
> +       struct kvm_vcpu *vcpu = filp->private_data;
> +       void __user *argp = (void __user *)arg;
> +
> +       switch (ioctl) {
> +#ifdef CONFIG_KVM_ARM_VGIC
> +       case KVM_IRQ_LINE: {
> +               struct kvm_irq_level irq_event;
> +
> +               if (copy_from_user(&irq_event, argp, sizeof irq_event))
> +                       return -EFAULT;
> +
> +               if (!irqchip_in_kernel(vcpu->kvm))
> +                       return -EINVAL;
> +
> +               if (irq_event.irq < 16 || irq_event.irq >= 32)
> +                       return -EINVAL;
> +               return kvm_vgic_inject_irq(vcpu->kvm, vcpu->vcpu_id, &irq_event);
> +       }
> +#endif
> +       default:
> +               return -EINVAL;
> +       }
>  }
> 
> 
> Will cause warnings for unused variables vcpu and argp.
> 
>  int kvm_vm_ioctl_get_dirty_log(struct kvm *kvm, struct kvm_dirty_log *log)
> @@ -707,7 +734,19 @@ int kvm_vm_ioctl_get_dirty_log(struct kvm *kvm, struct kvm_dirty_log *log)
>  long kvm_arch_vm_ioctl(struct file *filp,
>                        unsigned int ioctl, unsigned long arg)
>  {
> -       return -EINVAL;
> +       struct kvm *kvm = filp->private_data;
> +
> +       switch (ioctl) {
> +#ifdef CONFIG_KVM_ARM_VGIC
> +       case KVM_CREATE_IRQCHIP:
> +               if (vgic_present)
> +                       return kvm_vgic_init(kvm);
> +               else
> +                       return -EINVAL;
> +#endif
> +       default:
> +               return -EINVAL;
> +       }
>  }
> 
> 
> Ditto for variable kvm.

Good catch. I'll move them to a more local scope.

Thanks for the reports!

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