diff mbox

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

Message ID 20121110154518.3061.69353.stgit@chazy-air (mailing list archive)
State New, archived
Headers show

Commit Message

Christoffer Dall Nov. 10, 2012, 3:45 p.m. UTC
From: Marc Zyngier <marc.zyngier@arm.com>

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

Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
Signed-off-by: Christoffer Dall <c.dall@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(+)

Comments

Will Deacon Dec. 3, 2012, 1:25 p.m. UTC | #1
On Sat, Nov 10, 2012 at 03:45:18PM +0000, Christoffer Dall wrote:
> From: Marc Zyngier <marc.zyngier@arm.com>
> 
> Plug the interrupt injection code. Interrupts can now be generated
> from user space.
> 
> Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
> Signed-off-by: Christoffer Dall <c.dall@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! :)

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

> +	}
> +#endif
>  	case KVM_SET_DEVICE_ADDRESS: {
>  		struct kvm_device_address dev_addr;
>  
> diff --git a/arch/arm/kvm/vgic.c b/arch/arm/kvm/vgic.c
> index dda5623..70040bb 100644
> --- a/arch/arm/kvm/vgic.c
> +++ b/arch/arm/kvm/vgic.c
> @@ -75,6 +75,7 @@
>  #define ACCESS_WRITE_MASK(x)	((x) & (3 << 1))
>  
>  static void vgic_update_state(struct kvm *kvm);
> +static void vgic_kick_vcpus(struct kvm *kvm);
>  static void vgic_dispatch_sgi(struct kvm_vcpu *vcpu, u32 reg);
>  
>  static inline int vgic_irq_is_edge(struct vgic_dist *dist, int irq)
> @@ -542,6 +543,9 @@ bool vgic_handle_mmio(struct kvm_vcpu *vcpu, struct kvm_run *run, struct kvm_exi
>  	kvm_prepare_mmio(run, mmio);
>  	kvm_handle_mmio_return(vcpu, run);
>  
> +	if (updated_state)
> +		vgic_kick_vcpus(vcpu->kvm);
> +
>  	return true;
>  }
>  
> @@ -867,6 +871,92 @@ int kvm_vgic_vcpu_pending_irq(struct kvm_vcpu *vcpu)
>  	return test_bit(vcpu->vcpu_id, &dist->irq_pending_on_cpu);
>  }
>  
> +static void vgic_kick_vcpus(struct kvm *kvm)
> +{
> +	struct kvm_vcpu *vcpu;
> +	int c;
> +
> +	/*
> +	 * We've injected an interrupt, time to find out who deserves
> +	 * a good kick...
> +	 */
> +	kvm_for_each_vcpu(c, vcpu, kvm) {
> +		if (kvm_vgic_vcpu_pending_irq(vcpu))
> +			kvm_vcpu_kick(vcpu);
> +	}
> +}
> +
> +static bool vgic_update_irq_state(struct kvm *kvm, int cpuid,
> +				  unsigned int irq_num, bool level)
> +{
> +	struct vgic_dist *dist = &kvm->arch.vgic;
> +	struct kvm_vcpu *vcpu;
> +	int is_edge, is_level, state;
> +	int enabled;
> +	bool ret = true;
> +
> +	spin_lock(&dist->lock);
> +
> +	is_edge = vgic_irq_is_edge(dist, irq_num);
> +	is_level = !is_edge;
> +	state = vgic_bitmap_get_irq_val(&dist->irq_state, cpuid, irq_num);
> +
> +	/*
> +	 * Only inject an interrupt if:
> +	 * - level triggered and we change level
> +	 * - edge triggered and we have a rising edge
> +	 */
> +	if ((is_level && !(state ^ level)) || (is_edge && (state || !level))) {
> +		ret = false;
> +		goto out;
> +	}

Eek, more of the edge/level combo. Can this be be restructured so that we
have vgic_update_{edge,level}_irq_state, which are called from here
appropriately?

Will
Marc Zyngier Dec. 3, 2012, 2:21 p.m. UTC | #2
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@arm.com>
>>
>> Plug the interrupt injection code. Interrupts can now be generated
>> from user space.
>>
>> Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
>> Signed-off-by: Christoffer Dall <c.dall@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.

Christoffer?

>> +	}
>> +#endif
>>  	case KVM_SET_DEVICE_ADDRESS: {
>>  		struct kvm_device_address dev_addr;
>>  
>> diff --git a/arch/arm/kvm/vgic.c b/arch/arm/kvm/vgic.c
>> index dda5623..70040bb 100644
>> --- a/arch/arm/kvm/vgic.c
>> +++ b/arch/arm/kvm/vgic.c
>> @@ -75,6 +75,7 @@
>>  #define ACCESS_WRITE_MASK(x)	((x) & (3 << 1))
>>  
>>  static void vgic_update_state(struct kvm *kvm);
>> +static void vgic_kick_vcpus(struct kvm *kvm);
>>  static void vgic_dispatch_sgi(struct kvm_vcpu *vcpu, u32 reg);
>>  
>>  static inline int vgic_irq_is_edge(struct vgic_dist *dist, int irq)
>> @@ -542,6 +543,9 @@ bool vgic_handle_mmio(struct kvm_vcpu *vcpu, struct kvm_run *run, struct kvm_exi
>>  	kvm_prepare_mmio(run, mmio);
>>  	kvm_handle_mmio_return(vcpu, run);
>>  
>> +	if (updated_state)
>> +		vgic_kick_vcpus(vcpu->kvm);
>> +
>>  	return true;
>>  }
>>  
>> @@ -867,6 +871,92 @@ int kvm_vgic_vcpu_pending_irq(struct kvm_vcpu *vcpu)
>>  	return test_bit(vcpu->vcpu_id, &dist->irq_pending_on_cpu);
>>  }
>>  
>> +static void vgic_kick_vcpus(struct kvm *kvm)
>> +{
>> +	struct kvm_vcpu *vcpu;
>> +	int c;
>> +
>> +	/*
>> +	 * We've injected an interrupt, time to find out who deserves
>> +	 * a good kick...
>> +	 */
>> +	kvm_for_each_vcpu(c, vcpu, kvm) {
>> +		if (kvm_vgic_vcpu_pending_irq(vcpu))
>> +			kvm_vcpu_kick(vcpu);
>> +	}
>> +}
>> +
>> +static bool vgic_update_irq_state(struct kvm *kvm, int cpuid,
>> +				  unsigned int irq_num, bool level)
>> +{
>> +	struct vgic_dist *dist = &kvm->arch.vgic;
>> +	struct kvm_vcpu *vcpu;
>> +	int is_edge, is_level, state;
>> +	int enabled;
>> +	bool ret = true;
>> +
>> +	spin_lock(&dist->lock);
>> +
>> +	is_edge = vgic_irq_is_edge(dist, irq_num);
>> +	is_level = !is_edge;
>> +	state = vgic_bitmap_get_irq_val(&dist->irq_state, cpuid, irq_num);
>> +
>> +	/*
>> +	 * Only inject an interrupt if:
>> +	 * - level triggered and we change level
>> +	 * - edge triggered and we have a rising edge
>> +	 */
>> +	if ((is_level && !(state ^ level)) || (is_edge && (state || !level))) {
>> +		ret = false;
>> +		goto out;
>> +	}
> 
> Eek, more of the edge/level combo. Can this be be restructured so that we
> have vgic_update_{edge,level}_irq_state, which are called from here
> appropriately?

I'll have a look.

Thanks,

	M.
Christoffer Dall Dec. 3, 2012, 2:58 p.m. UTC | #3
[...]

>>> +
>>> +static bool vgic_update_irq_state(struct kvm *kvm, int cpuid,
>>> +                              unsigned int irq_num, bool level)
>>> +{
>>> +    struct vgic_dist *dist = &kvm->arch.vgic;
>>> +    struct kvm_vcpu *vcpu;
>>> +    int is_edge, is_level, state;
>>> +    int enabled;
>>> +    bool ret = true;
>>> +
>>> +    spin_lock(&dist->lock);
>>> +
>>> +    is_edge = vgic_irq_is_edge(dist, irq_num);
>>> +    is_level = !is_edge;
>>> +    state = vgic_bitmap_get_irq_val(&dist->irq_state, cpuid, irq_num);
>>> +
>>> +    /*
>>> +     * Only inject an interrupt if:
>>> +     * - level triggered and we change level
>>> +     * - edge triggered and we have a rising edge
>>> +     */
>>> +    if ((is_level && !(state ^ level)) || (is_edge && (state || !level))) {
>>> +            ret = false;
>>> +            goto out;
>>> +    }
>>
>> Eek, more of the edge/level combo. Can this be be restructured so that we
>> have vgic_update_{edge,level}_irq_state, which are called from here
>> appropriately?
>
> I'll have a look.
>
oh, you're no fun anymore. That if statement is one of the funniest
pieces of this code.

-Christoffer
Christoffer Dall Dec. 3, 2012, 7:13 p.m. UTC | #4
On Mon, Dec 3, 2012 at 9:21 AM, Marc Zyngier <marc.zyngier@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@arm.com>
>>>
>>> Plug the interrupt injection code. Interrupts can now be generated
>>> from user space.
>>>
>>> Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
>>> Signed-off-by: Christoffer Dall <c.dall@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
Marc Zyngier Dec. 3, 2012, 7:22 p.m. UTC | #5
On 03/12/12 19:13, Christoffer Dall wrote:
> On Mon, Dec 3, 2012 at 9:21 AM, Marc Zyngier <marc.zyngier@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@arm.com>
>>>>
>>>> Plug the interrupt injection code. Interrupts can now be generated
>>>> from user space.
>>>>
>>>> Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
>>>> Signed-off-by: Christoffer Dall <c.dall@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?

Yes, I'll fix it as part the whole vgic series.

	M.
diff mbox

Patch

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;
+
+		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;
+	}
+#endif
 	case KVM_SET_DEVICE_ADDRESS: {
 		struct kvm_device_address dev_addr;
 
diff --git a/arch/arm/kvm/vgic.c b/arch/arm/kvm/vgic.c
index dda5623..70040bb 100644
--- a/arch/arm/kvm/vgic.c
+++ b/arch/arm/kvm/vgic.c
@@ -75,6 +75,7 @@ 
 #define ACCESS_WRITE_MASK(x)	((x) & (3 << 1))
 
 static void vgic_update_state(struct kvm *kvm);
+static void vgic_kick_vcpus(struct kvm *kvm);
 static void vgic_dispatch_sgi(struct kvm_vcpu *vcpu, u32 reg);
 
 static inline int vgic_irq_is_edge(struct vgic_dist *dist, int irq)
@@ -542,6 +543,9 @@  bool vgic_handle_mmio(struct kvm_vcpu *vcpu, struct kvm_run *run, struct kvm_exi
 	kvm_prepare_mmio(run, mmio);
 	kvm_handle_mmio_return(vcpu, run);
 
+	if (updated_state)
+		vgic_kick_vcpus(vcpu->kvm);
+
 	return true;
 }
 
@@ -867,6 +871,92 @@  int kvm_vgic_vcpu_pending_irq(struct kvm_vcpu *vcpu)
 	return test_bit(vcpu->vcpu_id, &dist->irq_pending_on_cpu);
 }
 
+static void vgic_kick_vcpus(struct kvm *kvm)
+{
+	struct kvm_vcpu *vcpu;
+	int c;
+
+	/*
+	 * We've injected an interrupt, time to find out who deserves
+	 * a good kick...
+	 */
+	kvm_for_each_vcpu(c, vcpu, kvm) {
+		if (kvm_vgic_vcpu_pending_irq(vcpu))
+			kvm_vcpu_kick(vcpu);
+	}
+}
+
+static bool vgic_update_irq_state(struct kvm *kvm, int cpuid,
+				  unsigned int irq_num, bool level)
+{
+	struct vgic_dist *dist = &kvm->arch.vgic;
+	struct kvm_vcpu *vcpu;
+	int is_edge, is_level, state;
+	int enabled;
+	bool ret = true;
+
+	spin_lock(&dist->lock);
+
+	is_edge = vgic_irq_is_edge(dist, irq_num);
+	is_level = !is_edge;
+	state = vgic_bitmap_get_irq_val(&dist->irq_state, cpuid, irq_num);
+
+	/*
+	 * Only inject an interrupt if:
+	 * - level triggered and we change level
+	 * - edge triggered and we have a rising edge
+	 */
+	if ((is_level && !(state ^ level)) || (is_edge && (state || !level))) {
+		ret = false;
+		goto out;
+	}
+
+	vgic_bitmap_set_irq_val(&dist->irq_state, cpuid, irq_num, level);
+
+	enabled = vgic_bitmap_get_irq_val(&dist->irq_enabled, cpuid, irq_num);
+
+	if (!enabled) {
+		ret = false;
+		goto out;
+	}
+
+	if (is_level && vgic_bitmap_get_irq_val(&dist->irq_active,
+						cpuid, irq_num)) {
+		/*
+		 * Level interrupt in progress, will be picked up
+		 * when EOId.
+		 */
+		ret = false;
+		goto out;
+	}
+
+	if (irq_num >= 32)
+		cpuid = dist->irq_spi_cpu[irq_num - 32];
+
+	kvm_debug("Inject IRQ%d level %d CPU%d\n", irq_num, level, cpuid);
+
+	vcpu = kvm_get_vcpu(kvm, cpuid);
+
+	if (level) {
+		set_bit(irq_num, vcpu->arch.vgic_cpu.pending);
+		set_bit(cpuid, &dist->irq_pending_on_cpu);
+	}
+
+out:
+	spin_unlock(&dist->lock);
+
+	return ret;
+}
+
+int kvm_vgic_inject_irq(struct kvm *kvm, int cpuid, unsigned int irq_num,
+			bool level)
+{
+	if (vgic_update_irq_state(kvm, cpuid, irq_num, level))
+		vgic_kick_vcpus(kvm);
+
+	return 0;
+}
+
 static bool vgic_ioaddr_overlap(struct kvm *kvm)
 {
 	phys_addr_t dist = kvm->arch.vgic.vgic_dist_base;