On Mon, Dec 3, 2012 at 9:21 AM, Marc Zyngier <marc.zyngier@xxxxxxx> 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@xxxxxxx>
>>>
>>> Plug the interrupt injection code. Interrupts can now be generated
>>> from user space.
>>>
>>> Signed-off-by: Marc Zyngier <marc.zyngier@xxxxxxx>
>>> Signed-off-by: Christoffer Dall <c.dall@xxxxxxxxxxxxxxxxxxxxxx>
>>> ---
>>> 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
--
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
