diff mbox

[v5,06/14] KVM: ARM: Inject IRQs and FIQs from userspace

Message ID 20130108183917.46302.55603.stgit@ubuntu (mailing list archive)
State New, archived
Headers show

Commit Message

Christoffer Dall Jan. 8, 2013, 6:39 p.m. UTC
From: Christoffer Dall <cdall@cs.columbia.edu>

All interrupt injection is now based on the VM ioctl KVM_IRQ_LINE.  This
works semantically well for the GIC as we in fact raise/lower a line on
a machine component (the gic).  The IOCTL uses the follwing struct.

struct kvm_irq_level {
	union {
		__u32 irq;     /* GSI */
		__s32 status;  /* not used for KVM_IRQ_LEVEL */
	};
	__u32 level;           /* 0 or 1 */
};

ARM can signal an interrupt either at the CPU level, or at the in-kernel irqchip
(GIC), and for in-kernel irqchip can tell the GIC to use PPIs designated for
specific cpus.  The irq field is interpreted like this:

  bits:  | 31 ... 24 | 23  ... 16 | 15    ...    0 |
  field: | irq_type  | vcpu_index |   irq_number   |

The irq_type field has the following values:
- irq_type[0]: out-of-kernel GIC: irq_number 0 is IRQ, irq_number 1 is FIQ
- irq_type[1]: in-kernel GIC: SPI, irq_number between 32 and 1019 (incl.)
               (the vcpu_index field is ignored)
- irq_type[2]: in-kernel GIC: PPI, irq_number between 16 and 31 (incl.)

The irq_number thus corresponds to the irq ID in as in the GICv2 specs.

This is documented in Documentation/kvm/api.txt.

Reviewed-by: Marcelo Tosatti <mtosatti@redhat.com>
Signed-off-by: Christoffer Dall <c.dall@virtualopensystems.com>
---
 Documentation/virtual/kvm/api.txt |   25 ++++++++++++--
 arch/arm/include/asm/kvm_arm.h    |    1 +
 arch/arm/include/uapi/asm/kvm.h   |   21 ++++++++++++
 arch/arm/kvm/arm.c                |   65 +++++++++++++++++++++++++++++++++++++
 arch/arm/kvm/trace.h              |   25 ++++++++++++++
 include/uapi/linux/kvm.h          |    1 +
 6 files changed, 134 insertions(+), 4 deletions(-)

Comments

Gleb Natapov Jan. 15, 2013, 9:56 a.m. UTC | #1
On Tue, Jan 08, 2013 at 01:39:17PM -0500, Christoffer Dall wrote:
> From: Christoffer Dall <cdall@cs.columbia.edu>
> 
> All interrupt injection is now based on the VM ioctl KVM_IRQ_LINE.  This
> works semantically well for the GIC as we in fact raise/lower a line on
> a machine component (the gic).  The IOCTL uses the follwing struct.
> 
> struct kvm_irq_level {
> 	union {
> 		__u32 irq;     /* GSI */
> 		__s32 status;  /* not used for KVM_IRQ_LEVEL */
> 	};
> 	__u32 level;           /* 0 or 1 */
> };
> 
> ARM can signal an interrupt either at the CPU level, or at the in-kernel irqchip
CPU level interrupt should use KVM_INTERRUPT instead.

> (GIC), and for in-kernel irqchip can tell the GIC to use PPIs designated for
> specific cpus.  The irq field is interpreted like this:
> 
Haven't read about GIC yet. Is PPI an interrupt that device can send
directly to a specific CPU? Can we model that with irq routing like we do
for MSI?

>   bits:  | 31 ... 24 | 23  ... 16 | 15    ...    0 |
>   field: | irq_type  | vcpu_index |   irq_number   |
> 
> The irq_type field has the following values:
> - irq_type[0]: out-of-kernel GIC: irq_number 0 is IRQ, irq_number 1 is FIQ
> - irq_type[1]: in-kernel GIC: SPI, irq_number between 32 and 1019 (incl.)
>                (the vcpu_index field is ignored)
> - irq_type[2]: in-kernel GIC: PPI, irq_number between 16 and 31 (incl.)
> 
> The irq_number thus corresponds to the irq ID in as in the GICv2 specs.
> 
> This is documented in Documentation/kvm/api.txt.
> 
> Reviewed-by: Marcelo Tosatti <mtosatti@redhat.com>
> Signed-off-by: Christoffer Dall <c.dall@virtualopensystems.com>
> ---
>  Documentation/virtual/kvm/api.txt |   25 ++++++++++++--
>  arch/arm/include/asm/kvm_arm.h    |    1 +
>  arch/arm/include/uapi/asm/kvm.h   |   21 ++++++++++++
>  arch/arm/kvm/arm.c                |   65 +++++++++++++++++++++++++++++++++++++
>  arch/arm/kvm/trace.h              |   25 ++++++++++++++
>  include/uapi/linux/kvm.h          |    1 +
>  6 files changed, 134 insertions(+), 4 deletions(-)
> 
> diff --git a/Documentation/virtual/kvm/api.txt b/Documentation/virtual/kvm/api.txt
> index 4237c27..5050492 100644
> --- a/Documentation/virtual/kvm/api.txt
> +++ b/Documentation/virtual/kvm/api.txt
> @@ -615,15 +615,32 @@ created.
>  4.25 KVM_IRQ_LINE
>  
>  Capability: KVM_CAP_IRQCHIP
> -Architectures: x86, ia64
> +Architectures: x86, ia64, arm
>  Type: vm ioctl
>  Parameters: struct kvm_irq_level
>  Returns: 0 on success, -1 on error
>  
>  Sets the level of a GSI input to the interrupt controller model in the kernel.
> -Requires that an interrupt controller model has been previously created with
> -KVM_CREATE_IRQCHIP.  Note that edge-triggered interrupts require the level
> -to be set to 1 and then back to 0.
> +On some architectures it is required that an interrupt controller model has
> +been previously created with KVM_CREATE_IRQCHIP.  Note that edge-triggered
> +interrupts require the level to be set to 1 and then back to 0.
> +
> +ARM can signal an interrupt either at the CPU level, or at the in-kernel irqchip
> +(GIC), and for in-kernel irqchip can tell the GIC to use PPIs designated for
> +specific cpus.  The irq field is interpreted like this:
> +
> +  bits:  | 31 ... 24 | 23  ... 16 | 15    ...    0 |
> +  field: | irq_type  | vcpu_index |     irq_id     |
> +
> +The irq_type field has the following values:
> +- irq_type[0]: out-of-kernel GIC: irq_id 0 is IRQ, irq_id 1 is FIQ
> +- irq_type[1]: in-kernel GIC: SPI, irq_id between 32 and 1019 (incl.)
> +               (the vcpu_index field is ignored)
> +- irq_type[2]: in-kernel GIC: PPI, irq_id between 16 and 31 (incl.)
> +
> +(The irq_id field thus corresponds nicely to the IRQ ID in the ARM GIC specs)
> +
> +In both cases, level is used to raise/lower the line.
>  
>  struct kvm_irq_level {
>  	union {
> diff --git a/arch/arm/include/asm/kvm_arm.h b/arch/arm/include/asm/kvm_arm.h
> index 613afe2..fb22ee8 100644
> --- a/arch/arm/include/asm/kvm_arm.h
> +++ b/arch/arm/include/asm/kvm_arm.h
> @@ -68,6 +68,7 @@
>  #define HCR_GUEST_MASK (HCR_TSC | HCR_TSW | HCR_TWI | HCR_VM | HCR_BSU_IS | \
>  			HCR_FB | HCR_TAC | HCR_AMO | HCR_IMO | HCR_FMO | \
>  			HCR_SWIO | HCR_TIDCP)
> +#define HCR_VIRT_EXCP_MASK (HCR_VA | HCR_VI | HCR_VF)
>  
>  /* Hyp System Control Register (HSCTLR) bits */
>  #define HSCTLR_TE	(1 << 30)
> diff --git a/arch/arm/include/uapi/asm/kvm.h b/arch/arm/include/uapi/asm/kvm.h
> index c6298b1..4cf6d8f 100644
> --- a/arch/arm/include/uapi/asm/kvm.h
> +++ b/arch/arm/include/uapi/asm/kvm.h
> @@ -23,6 +23,7 @@
>  #include <asm/ptrace.h>
>  
>  #define __KVM_HAVE_GUEST_DEBUG
> +#define __KVM_HAVE_IRQ_LINE
>  
>  #define KVM_REG_SIZE(id)						\
>  	(1U << (((id) & KVM_REG_SIZE_MASK) >> KVM_REG_SIZE_SHIFT))
> @@ -103,4 +104,24 @@ struct kvm_arch_memory_slot {
>  #define KVM_REG_ARM_CORE		(0x0010 << KVM_REG_ARM_COPROC_SHIFT)
>  #define KVM_REG_ARM_CORE_REG(name)	(offsetof(struct kvm_regs, name) / 4)
>  
> +/* KVM_IRQ_LINE irq field index values */
> +#define KVM_ARM_IRQ_TYPE_SHIFT		24
> +#define KVM_ARM_IRQ_TYPE_MASK		0xff
> +#define KVM_ARM_IRQ_VCPU_SHIFT		16
> +#define KVM_ARM_IRQ_VCPU_MASK		0xff
> +#define KVM_ARM_IRQ_NUM_SHIFT		0
> +#define KVM_ARM_IRQ_NUM_MASK		0xffff
> +
> +/* irq_type field */
> +#define KVM_ARM_IRQ_TYPE_CPU		0
> +#define KVM_ARM_IRQ_TYPE_SPI		1
> +#define KVM_ARM_IRQ_TYPE_PPI		2
> +
> +/* out-of-kernel GIC cpu interrupt injection irq_number field */
> +#define KVM_ARM_IRQ_CPU_IRQ		0
> +#define KVM_ARM_IRQ_CPU_FIQ		1
> +
> +/* Highest supported SPI, from VGIC_NR_IRQS */
> +#define KVM_ARM_IRQ_GIC_MAX		127
> +
>  #endif /* __ARM_KVM_H__ */
> diff --git a/arch/arm/kvm/arm.c b/arch/arm/kvm/arm.c
> index ab82039..9b4566e 100644
> --- a/arch/arm/kvm/arm.c
> +++ b/arch/arm/kvm/arm.c
> @@ -24,6 +24,7 @@
>  #include <linux/fs.h>
>  #include <linux/mman.h>
>  #include <linux/sched.h>
> +#include <linux/kvm.h>
>  #include <trace/events/kvm.h>
>  
>  #define CREATE_TRACE_POINTS
> @@ -284,6 +285,7 @@ void kvm_arch_vcpu_uninit(struct kvm_vcpu *vcpu)
>  
>  void kvm_arch_vcpu_load(struct kvm_vcpu *vcpu, int cpu)
>  {
> +	vcpu->cpu = cpu;
>  }
>  
>  void kvm_arch_vcpu_put(struct kvm_vcpu *vcpu)
> @@ -324,6 +326,69 @@ int kvm_arch_vcpu_ioctl_run(struct kvm_vcpu *vcpu, struct kvm_run *run)
>  	return -EINVAL;
>  }
>  
> +static int vcpu_interrupt_line(struct kvm_vcpu *vcpu, int number, bool level)
> +{
> +	int bit_index;
> +	bool set;
> +	unsigned long *ptr;
> +
> +	if (number == KVM_ARM_IRQ_CPU_IRQ)
> +		bit_index = __ffs(HCR_VI);
> +	else /* KVM_ARM_IRQ_CPU_FIQ */
> +		bit_index = __ffs(HCR_VF);
> +
> +	ptr = (unsigned long *)&vcpu->arch.irq_lines;
> +	if (level)
> +		set = test_and_set_bit(bit_index, ptr);
> +	else
> +		set = test_and_clear_bit(bit_index, ptr);
> +
> +	/*
> +	 * If we didn't change anything, no need to wake up or kick other CPUs
> +	 */
> +	if (set == level)
> +		return 0;
> +
> +	/*
> +	 * The vcpu irq_lines field was updated, wake up sleeping VCPUs and
> +	 * trigger a world-switch round on the running physical CPU to set the
> +	 * virtual IRQ/FIQ fields in the HCR appropriately.
> +	 */
> +	kvm_vcpu_kick(vcpu);
> +
> +	return 0;
> +}
> +
> +int kvm_vm_ioctl_irq_line(struct kvm *kvm, struct kvm_irq_level *irq_level)
> +{
> +	u32 irq = irq_level->irq;
> +	unsigned int irq_type, vcpu_idx, irq_num;
> +	int nrcpus = atomic_read(&kvm->online_vcpus);
> +	struct kvm_vcpu *vcpu = NULL;
> +	bool level = irq_level->level;
> +
> +	irq_type = (irq >> KVM_ARM_IRQ_TYPE_SHIFT) & KVM_ARM_IRQ_TYPE_MASK;
> +	vcpu_idx = (irq >> KVM_ARM_IRQ_VCPU_SHIFT) & KVM_ARM_IRQ_VCPU_MASK;
> +	irq_num = (irq >> KVM_ARM_IRQ_NUM_SHIFT) & KVM_ARM_IRQ_NUM_MASK;
> +
> +	trace_kvm_irq_line(irq_type, vcpu_idx, irq_num, irq_level->level);
> +
> +	if (irq_type != KVM_ARM_IRQ_TYPE_CPU)
> +		return -EINVAL;
> +
> +	if (vcpu_idx >= nrcpus)
> +		return -EINVAL;
> +
> +	vcpu = kvm_get_vcpu(kvm, vcpu_idx);
> +	if (!vcpu)
> +		return -EINVAL;
> +
> +	if (irq_num > KVM_ARM_IRQ_CPU_FIQ)
> +		return -EINVAL;
> +
> +	return vcpu_interrupt_line(vcpu, irq_num, level);
> +}
> +
>  long kvm_arch_vcpu_ioctl(struct file *filp,
>  			 unsigned int ioctl, unsigned long arg)
>  {
> diff --git a/arch/arm/kvm/trace.h b/arch/arm/kvm/trace.h
> index 862b2cc..105d1f7 100644
> --- a/arch/arm/kvm/trace.h
> +++ b/arch/arm/kvm/trace.h
> @@ -39,6 +39,31 @@ TRACE_EVENT(kvm_exit,
>  	TP_printk("PC: 0x%08lx", __entry->vcpu_pc)
>  );
>  
> +TRACE_EVENT(kvm_irq_line,
> +	TP_PROTO(unsigned int type, int vcpu_idx, int irq_num, int level),
> +	TP_ARGS(type, vcpu_idx, irq_num, level),
> +
> +	TP_STRUCT__entry(
> +		__field(	unsigned int,	type		)
> +		__field(	int,		vcpu_idx	)
> +		__field(	int,		irq_num		)
> +		__field(	int,		level		)
> +	),
> +
> +	TP_fast_assign(
> +		__entry->type		= type;
> +		__entry->vcpu_idx	= vcpu_idx;
> +		__entry->irq_num	= irq_num;
> +		__entry->level		= level;
> +	),
> +
> +	TP_printk("Inject %s interrupt (%d), vcpu->idx: %d, num: %d, level: %d",
> +		  (__entry->type == KVM_ARM_IRQ_TYPE_CPU) ? "CPU" :
> +		  (__entry->type == KVM_ARM_IRQ_TYPE_PPI) ? "VGIC PPI" :
> +		  (__entry->type == KVM_ARM_IRQ_TYPE_SPI) ? "VGIC SPI" : "UNKNOWN",
> +		  __entry->type, __entry->vcpu_idx, __entry->irq_num, __entry->level)
> +);
> +
>  TRACE_EVENT(kvm_unmap_hva,
>  	TP_PROTO(unsigned long hva),
>  	TP_ARGS(hva),
> diff --git a/include/uapi/linux/kvm.h b/include/uapi/linux/kvm.h
> index 24978d5..dc63665 100644
> --- a/include/uapi/linux/kvm.h
> +++ b/include/uapi/linux/kvm.h
> @@ -115,6 +115,7 @@ struct kvm_irq_level {
>  	 * ACPI gsi notion of irq.
>  	 * For IA-64 (APIC model) IOAPIC0: irq 0-23; IOAPIC1: irq 24-47..
>  	 * For X86 (standard AT mode) PIC0/1: irq 0-15. IOAPIC0: 0-23..
> +	 * For ARM: See Documentation/virtual/kvm/api.txt
>  	 */
>  	union {
>  		__u32 irq;
> 
> --
> To unsubscribe from this list: send the line "unsubscribe kvm" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

--
			Gleb.
Peter Maydell Jan. 15, 2013, 12:15 p.m. UTC | #2
On 15 January 2013 09:56, Gleb Natapov <gleb@redhat.com> wrote:
> On Tue, Jan 08, 2013 at 01:39:17PM -0500, Christoffer Dall wrote:
>> From: Christoffer Dall <cdall@cs.columbia.edu>
>>
>> All interrupt injection is now based on the VM ioctl KVM_IRQ_LINE.  This
>> works semantically well for the GIC as we in fact raise/lower a line on
>> a machine component (the gic).  The IOCTL uses the follwing struct.
>>
>> struct kvm_irq_level {
>>       union {
>>               __u32 irq;     /* GSI */
>>               __s32 status;  /* not used for KVM_IRQ_LEVEL */
>>       };
>>       __u32 level;           /* 0 or 1 */
>> };
>>
>> ARM can signal an interrupt either at the CPU level, or at the in-kernel irqchip
> CPU level interrupt should use KVM_INTERRUPT instead.

No, that would be wrong. KVM_INTERRUPT is for interrupts which must be
delivered synchronously to the CPU. KVM_IRQ_LINE is for interrupts which
can be fed to the kernel asynchronously. It happens that on x86 "must be
delivered synchronously" and "not going to in kernel irqchip" are the same, but
this isn't true for other archs. For ARM all our interrupts can be fed
to the kernel
asynchronously, and so we use KVM_IRQ_LINE in all cases.

There was a big discussion thread about this on kvm and qemu-devel last
July (and we cleaned up some of the QEMU code to not smoosh together
all these different concepts under "do I have an irqchip or not?").

>> (GIC), and for in-kernel irqchip can tell the GIC to use PPIs designated for
>> specific cpus.  The irq field is interpreted like this:
>>
> Haven't read about GIC yet. Is PPI an interrupt that device can send
> directly to a specific CPU? Can we model that with irq routing like we do
> for MSI?

There is no routing involved -- you are raising a specific signal
line (which happens to result in prodding a particular CPU), that's all.

-- PMM
Gleb Natapov Jan. 15, 2013, 12:52 p.m. UTC | #3
On Tue, Jan 15, 2013 at 12:15:01PM +0000, Peter Maydell wrote:
> On 15 January 2013 09:56, Gleb Natapov <gleb@redhat.com> wrote:
> > On Tue, Jan 08, 2013 at 01:39:17PM -0500, Christoffer Dall wrote:
> >> From: Christoffer Dall <cdall@cs.columbia.edu>
> >>
> >> All interrupt injection is now based on the VM ioctl KVM_IRQ_LINE.  This
> >> works semantically well for the GIC as we in fact raise/lower a line on
> >> a machine component (the gic).  The IOCTL uses the follwing struct.
> >>
> >> struct kvm_irq_level {
> >>       union {
> >>               __u32 irq;     /* GSI */
> >>               __s32 status;  /* not used for KVM_IRQ_LEVEL */
> >>       };
> >>       __u32 level;           /* 0 or 1 */
> >> };
> >>
> >> ARM can signal an interrupt either at the CPU level, or at the in-kernel irqchip
> > CPU level interrupt should use KVM_INTERRUPT instead.
> 
> No, that would be wrong. KVM_INTERRUPT is for interrupts which must be
> delivered synchronously to the CPU. KVM_IRQ_LINE is for interrupts which
> can be fed to the kernel asynchronously. It happens that on x86 "must be
> delivered synchronously" and "not going to in kernel irqchip" are the same, but
> this isn't true for other archs. For ARM all our interrupts can be fed
> to the kernel
> asynchronously, and so we use KVM_IRQ_LINE in all cases.
> 
I do no quite understand what you mean by synchronously and
asynchronously. The difference between KVM_INTERRUPT and KVM_IRQ_LINE line
is that former is used when destination cpu is known to userspace later
is used when kernel code is involved in figuring out the destination. The
injections themselves are currently synchronous for both of them on x86
and ARM. i.e vcpu is kicked out from guest mode when interrupt need to
be injected into a guest and vcpu state is changed to inject interrupt
during next guest entry. In the near feature x86 will be able to inject
interrupt without kicking vcpu out from the guest mode does ARM plan to
do the same? For GIC interrupts or for IRQ/FIQ or for both?

> There was a big discussion thread about this on kvm and qemu-devel last
> July (and we cleaned up some of the QEMU code to not smoosh together
> all these different concepts under "do I have an irqchip or not?").
Do you have a pointer?

> 
> >> (GIC), and for in-kernel irqchip can tell the GIC to use PPIs designated for
> >> specific cpus.  The irq field is interpreted like this:
> >>
> > Haven't read about GIC yet. Is PPI an interrupt that device can send
> > directly to a specific CPU? Can we model that with irq routing like we do
> > for MSI?
> 
> There is no routing involved -- you are raising a specific signal
> line (which happens to result in prodding a particular CPU), that's all.
> 
We call it "irq routing", but it is not really a router. It just a
configuration to let KVM know how specific lines are wired. We abuse it
for MSI injection. So instead of encoding destination into kvm_irq_level
you configure "irq routing" entry with this information and get back a
cookie. You provide the cookie in kvm_irq_level->irq to KVM_IRQ_LEVEL
ioctl. This way you are not limited to 8 bit of cpuid for instance. This
is not efficient if "irq routing" is very dynamic though.
 
--
			Gleb.
Peter Maydell Jan. 15, 2013, 2:04 p.m. UTC | #4
On 15 January 2013 12:52, Gleb Natapov <gleb@redhat.com> wrote:
> On Tue, Jan 15, 2013 at 12:15:01PM +0000, Peter Maydell wrote:
>> On 15 January 2013 09:56, Gleb Natapov <gleb@redhat.com> wrote:
>> >> ARM can signal an interrupt either at the CPU level, or at the in-kernel irqchip
>> > CPU level interrupt should use KVM_INTERRUPT instead.
>>
>> No, that would be wrong. KVM_INTERRUPT is for interrupts which must be
>> delivered synchronously to the CPU. KVM_IRQ_LINE is for interrupts which
>> can be fed to the kernel asynchronously. It happens that on x86 "must be
>> delivered synchronously" and "not going to in kernel irqchip" are the same, but
>> this isn't true for other archs. For ARM all our interrupts can be fed
>> to the kernel asynchronously, and so we use KVM_IRQ_LINE in all
>> cases.

> I do no quite understand what you mean by synchronously and
> asynchronously.

Synchronously: the vcpu has to be stopped and userspace then
feeds in the interrupt to be taken when the guest is resumed.
Asynchronously: any old thread can tell the kernel there's an
interrupt, and the guest vcpu then deals with it when needed
(the vcpu thread may leave the guest but doesn't come out of
the host kernel to qemu).

> The difference between KVM_INTERRUPT and KVM_IRQ_LINE line
> is that former is used when destination cpu is known to userspace later
> is used when kernel code is involved in figuring out the destination.

This doesn't match up with Avi's explanation at all.

> The
> injections themselves are currently synchronous for both of them on x86
> and ARM. i.e vcpu is kicked out from guest mode when interrupt need to
> be injected into a guest and vcpu state is changed to inject interrupt
> during next guest entry. In the near feature x86 will be able to inject
> interrupt without kicking vcpu out from the guest mode does ARM plan to
> do the same? For GIC interrupts or for IRQ/FIQ or for both?
>
>> There was a big discussion thread about this on kvm and qemu-devel last
>> July (and we cleaned up some of the QEMU code to not smoosh together
>> all these different concepts under "do I have an irqchip or not?").
> Do you have a pointer?

  http://lists.gnu.org/archive/html/qemu-devel/2012-07/msg02460.html
and there was a later longer (but less clear) thread which included
this mail from Avi:
  http://lists.gnu.org/archive/html/qemu-devel/2012-07/msg02872.html
basically explaining that the reason for the weird synchronous
KVM_INTERRUPT API is that it's emulating a weird synchronous
hardware interface which is specific to x86. ARM doesn't have
a synchronous interface in the same way, so it's much more
straightforward to use KVM_IRQ_LINE.

-- PMM
Christoffer Dall Jan. 15, 2013, 2:40 p.m. UTC | #5
On Tue, Jan 15, 2013 at 9:04 AM, Peter Maydell <peter.maydell@linaro.org> wrote:
> On 15 January 2013 12:52, Gleb Natapov <gleb@redhat.com> wrote:
>> On Tue, Jan 15, 2013 at 12:15:01PM +0000, Peter Maydell wrote:
>>> On 15 January 2013 09:56, Gleb Natapov <gleb@redhat.com> wrote:
>>> >> ARM can signal an interrupt either at the CPU level, or at the in-kernel irqchip
>>> > CPU level interrupt should use KVM_INTERRUPT instead.
>>>
>>> No, that would be wrong. KVM_INTERRUPT is for interrupts which must be
>>> delivered synchronously to the CPU. KVM_IRQ_LINE is for interrupts which
>>> can be fed to the kernel asynchronously. It happens that on x86 "must be
>>> delivered synchronously" and "not going to in kernel irqchip" are the same, but
>>> this isn't true for other archs. For ARM all our interrupts can be fed
>>> to the kernel asynchronously, and so we use KVM_IRQ_LINE in all
>>> cases.
>
>> I do no quite understand what you mean by synchronously and
>> asynchronously.
>
> Synchronously: the vcpu has to be stopped and userspace then
> feeds in the interrupt to be taken when the guest is resumed.
> Asynchronously: any old thread can tell the kernel there's an
> interrupt, and the guest vcpu then deals with it when needed
> (the vcpu thread may leave the guest but doesn't come out of
> the host kernel to qemu).
>
>> The difference between KVM_INTERRUPT and KVM_IRQ_LINE line
>> is that former is used when destination cpu is known to userspace later
>> is used when kernel code is involved in figuring out the destination.
>
> This doesn't match up with Avi's explanation at all.
>
>> The
>> injections themselves are currently synchronous for both of them on x86
>> and ARM. i.e vcpu is kicked out from guest mode when interrupt need to
>> be injected into a guest and vcpu state is changed to inject interrupt
>> during next guest entry. In the near feature x86 will be able to inject
>> interrupt without kicking vcpu out from the guest mode does ARM plan to
>> do the same? For GIC interrupts or for IRQ/FIQ or for both?
>>
>>> There was a big discussion thread about this on kvm and qemu-devel last
>>> July (and we cleaned up some of the QEMU code to not smoosh together
>>> all these different concepts under "do I have an irqchip or not?").
>> Do you have a pointer?
>
>   http://lists.gnu.org/archive/html/qemu-devel/2012-07/msg02460.html
> and there was a later longer (but less clear) thread which included
> this mail from Avi:
>   http://lists.gnu.org/archive/html/qemu-devel/2012-07/msg02872.html
> basically explaining that the reason for the weird synchronous
> KVM_INTERRUPT API is that it's emulating a weird synchronous
> hardware interface which is specific to x86. ARM doesn't have
> a synchronous interface in the same way, so it's much more
> straightforward to use KVM_IRQ_LINE.
>
Also, this code has been reviewed numerous times by the KVM community
and as Peter points out has also been discussed in detail.

Could we please not change this API in the last second?

-Christoffer
Gleb Natapov Jan. 15, 2013, 3:17 p.m. UTC | #6
On Tue, Jan 15, 2013 at 02:04:47PM +0000, Peter Maydell wrote:
> On 15 January 2013 12:52, Gleb Natapov <gleb@redhat.com> wrote:
> > On Tue, Jan 15, 2013 at 12:15:01PM +0000, Peter Maydell wrote:
> >> On 15 January 2013 09:56, Gleb Natapov <gleb@redhat.com> wrote:
> >> >> ARM can signal an interrupt either at the CPU level, or at the in-kernel irqchip
> >> > CPU level interrupt should use KVM_INTERRUPT instead.
> >>
> >> No, that would be wrong. KVM_INTERRUPT is for interrupts which must be
> >> delivered synchronously to the CPU. KVM_IRQ_LINE is for interrupts which
> >> can be fed to the kernel asynchronously. It happens that on x86 "must be
> >> delivered synchronously" and "not going to in kernel irqchip" are the same, but
> >> this isn't true for other archs. For ARM all our interrupts can be fed
> >> to the kernel asynchronously, and so we use KVM_IRQ_LINE in all
> >> cases.
> 
> > I do no quite understand what you mean by synchronously and
> > asynchronously.
> 
> Synchronously: the vcpu has to be stopped and userspace then
> feeds in the interrupt to be taken when the guest is resumed.
> Asynchronously: any old thread can tell the kernel there's an
> interrupt, and the guest vcpu then deals with it when needed
> (the vcpu thread may leave the guest but doesn't come out of
> the host kernel to qemu).
> 
> > The difference between KVM_INTERRUPT and KVM_IRQ_LINE line
> > is that former is used when destination cpu is known to userspace later
> > is used when kernel code is involved in figuring out the destination.
> 
> This doesn't match up with Avi's explanation at all.
> 
> > The
> > injections themselves are currently synchronous for both of them on x86
> > and ARM. i.e vcpu is kicked out from guest mode when interrupt need to
> > be injected into a guest and vcpu state is changed to inject interrupt
> > during next guest entry. In the near feature x86 will be able to inject
> > interrupt without kicking vcpu out from the guest mode does ARM plan to
> > do the same? For GIC interrupts or for IRQ/FIQ or for both?
> >
> >> There was a big discussion thread about this on kvm and qemu-devel last
> >> July (and we cleaned up some of the QEMU code to not smoosh together
> >> all these different concepts under "do I have an irqchip or not?").
> > Do you have a pointer?
> 
>   http://lists.gnu.org/archive/html/qemu-devel/2012-07/msg02460.html
> and there was a later longer (but less clear) thread which included
> this mail from Avi:
>   http://lists.gnu.org/archive/html/qemu-devel/2012-07/msg02872.html
> basically explaining that the reason for the weird synchronous
> KVM_INTERRUPT API is that it's emulating a weird synchronous
> hardware interface which is specific to x86. ARM doesn't have
> a synchronous interface in the same way, so it's much more
> straightforward to use KVM_IRQ_LINE.
> 
OK. I see. So basically Avi saw KVM_INTERRUPT as an oddball interface
required only for APIC emulation in userspace. It is used for PIC also,
where this is not strictly needed, but this is for historical reasons
(KVM_IRQ_LINE was introduces late and it is GSI centric on x86).

Thank you for the pointer.

--
			Gleb.
Alexander Graf Jan. 15, 2013, 4:25 p.m. UTC | #7
On 01/15/2013 04:17 PM, Gleb Natapov wrote:
> On Tue, Jan 15, 2013 at 02:04:47PM +0000, Peter Maydell wrote:
>> On 15 January 2013 12:52, Gleb Natapov<gleb@redhat.com>  wrote:
>>> On Tue, Jan 15, 2013 at 12:15:01PM +0000, Peter Maydell wrote:
>>>> On 15 January 2013 09:56, Gleb Natapov<gleb@redhat.com>  wrote:
>>>>>> ARM can signal an interrupt either at the CPU level, or at the in-kernel irqchip
>>>>> CPU level interrupt should use KVM_INTERRUPT instead.
>>>> No, that would be wrong. KVM_INTERRUPT is for interrupts which must be
>>>> delivered synchronously to the CPU. KVM_IRQ_LINE is for interrupts which
>>>> can be fed to the kernel asynchronously. It happens that on x86 "must be
>>>> delivered synchronously" and "not going to in kernel irqchip" are the same, but
>>>> this isn't true for other archs. For ARM all our interrupts can be fed
>>>> to the kernel asynchronously, and so we use KVM_IRQ_LINE in all
>>>> cases.
>>> I do no quite understand what you mean by synchronously and
>>> asynchronously.
>> Synchronously: the vcpu has to be stopped and userspace then
>> feeds in the interrupt to be taken when the guest is resumed.
>> Asynchronously: any old thread can tell the kernel there's an
>> interrupt, and the guest vcpu then deals with it when needed
>> (the vcpu thread may leave the guest but doesn't come out of
>> the host kernel to qemu).
>>
>>> The difference between KVM_INTERRUPT and KVM_IRQ_LINE line
>>> is that former is used when destination cpu is known to userspace later
>>> is used when kernel code is involved in figuring out the destination.
>> This doesn't match up with Avi's explanation at all.
>>
>>> The
>>> injections themselves are currently synchronous for both of them on x86
>>> and ARM. i.e vcpu is kicked out from guest mode when interrupt need to
>>> be injected into a guest and vcpu state is changed to inject interrupt
>>> during next guest entry. In the near feature x86 will be able to inject
>>> interrupt without kicking vcpu out from the guest mode does ARM plan to
>>> do the same? For GIC interrupts or for IRQ/FIQ or for both?
>>>
>>>> There was a big discussion thread about this on kvm and qemu-devel last
>>>> July (and we cleaned up some of the QEMU code to not smoosh together
>>>> all these different concepts under "do I have an irqchip or not?").
>>> Do you have a pointer?
>>    http://lists.gnu.org/archive/html/qemu-devel/2012-07/msg02460.html
>> and there was a later longer (but less clear) thread which included
>> this mail from Avi:
>>    http://lists.gnu.org/archive/html/qemu-devel/2012-07/msg02872.html
>> basically explaining that the reason for the weird synchronous
>> KVM_INTERRUPT API is that it's emulating a weird synchronous
>> hardware interface which is specific to x86. ARM doesn't have
>> a synchronous interface in the same way, so it's much more
>> straightforward to use KVM_IRQ_LINE.
>>
> OK. I see. So basically Avi saw KVM_INTERRUPT as an oddball interface
> required only for APIC emulation in userspace. It is used for PIC also,
> where this is not strictly needed, but this is for historical reasons
> (KVM_IRQ_LINE was introduces late and it is GSI centric on x86).
>
> Thank you for the pointer.

Yeah, please keep in mind that KVM_INTERRUPT is not a unified interface 
either. In fact, it is asynchronous on PPC :). And it's called 
KVM_S390_INTERRUPT on s390 and also asynchronous. X86 is the oddball here.

But I don't care whether we call the ioctl to steer CPU interrupt pins 
KVM_INTERRUPT, KVM_S390_INTERRUPT or KVM_IRQ_LINE, as long as the code 
makes it obvious what is happening.


Alex
Gleb Natapov Jan. 16, 2013, 10:40 a.m. UTC | #8
On Tue, Jan 15, 2013 at 05:25:13PM +0100, Alexander Graf wrote:
> On 01/15/2013 04:17 PM, Gleb Natapov wrote:
> >On Tue, Jan 15, 2013 at 02:04:47PM +0000, Peter Maydell wrote:
> >>On 15 January 2013 12:52, Gleb Natapov<gleb@redhat.com>  wrote:
> >>>On Tue, Jan 15, 2013 at 12:15:01PM +0000, Peter Maydell wrote:
> >>>>On 15 January 2013 09:56, Gleb Natapov<gleb@redhat.com>  wrote:
> >>>>>>ARM can signal an interrupt either at the CPU level, or at the in-kernel irqchip
> >>>>>CPU level interrupt should use KVM_INTERRUPT instead.
> >>>>No, that would be wrong. KVM_INTERRUPT is for interrupts which must be
> >>>>delivered synchronously to the CPU. KVM_IRQ_LINE is for interrupts which
> >>>>can be fed to the kernel asynchronously. It happens that on x86 "must be
> >>>>delivered synchronously" and "not going to in kernel irqchip" are the same, but
> >>>>this isn't true for other archs. For ARM all our interrupts can be fed
> >>>>to the kernel asynchronously, and so we use KVM_IRQ_LINE in all
> >>>>cases.
> >>>I do no quite understand what you mean by synchronously and
> >>>asynchronously.
> >>Synchronously: the vcpu has to be stopped and userspace then
> >>feeds in the interrupt to be taken when the guest is resumed.
> >>Asynchronously: any old thread can tell the kernel there's an
> >>interrupt, and the guest vcpu then deals with it when needed
> >>(the vcpu thread may leave the guest but doesn't come out of
> >>the host kernel to qemu).
> >>
> >>>The difference between KVM_INTERRUPT and KVM_IRQ_LINE line
> >>>is that former is used when destination cpu is known to userspace later
> >>>is used when kernel code is involved in figuring out the destination.
> >>This doesn't match up with Avi's explanation at all.
> >>
> >>>The
> >>>injections themselves are currently synchronous for both of them on x86
> >>>and ARM. i.e vcpu is kicked out from guest mode when interrupt need to
> >>>be injected into a guest and vcpu state is changed to inject interrupt
> >>>during next guest entry. In the near feature x86 will be able to inject
> >>>interrupt without kicking vcpu out from the guest mode does ARM plan to
> >>>do the same? For GIC interrupts or for IRQ/FIQ or for both?
> >>>
> >>>>There was a big discussion thread about this on kvm and qemu-devel last
> >>>>July (and we cleaned up some of the QEMU code to not smoosh together
> >>>>all these different concepts under "do I have an irqchip or not?").
> >>>Do you have a pointer?
> >>   http://lists.gnu.org/archive/html/qemu-devel/2012-07/msg02460.html
> >>and there was a later longer (but less clear) thread which included
> >>this mail from Avi:
> >>   http://lists.gnu.org/archive/html/qemu-devel/2012-07/msg02872.html
> >>basically explaining that the reason for the weird synchronous
> >>KVM_INTERRUPT API is that it's emulating a weird synchronous
> >>hardware interface which is specific to x86. ARM doesn't have
> >>a synchronous interface in the same way, so it's much more
> >>straightforward to use KVM_IRQ_LINE.
> >>
> >OK. I see. So basically Avi saw KVM_INTERRUPT as an oddball interface
> >required only for APIC emulation in userspace. It is used for PIC also,
> >where this is not strictly needed, but this is for historical reasons
> >(KVM_IRQ_LINE was introduces late and it is GSI centric on x86).
> >
> >Thank you for the pointer.
> 
> Yeah, please keep in mind that KVM_INTERRUPT is not a unified
> interface either. In fact, it is asynchronous on PPC :). And it's
> called KVM_S390_INTERRUPT on s390 and also asynchronous. X86 is the
> oddball here.
> 
KVM_INTERRUPT needs vcpu fd to be issues. Usually such ioctls are
issued only by vcpu thread which makes them synchronous and vcpu_load()
synchronise them anyway if the rule is not met. And sure enough those
KVM_S390_INTERRUPT/KVM_INTERRUPT are special cased in kvm_vcpu_ioctl()
to not call vcpu_load(), sigh :(

There was an idea to change vcpu ioctls to kvm syscall which would have
made it impossible to use KVM_INTERRUPT asynchronously.


> But I don't care whether we call the ioctl to steer CPU interrupt
> pins KVM_INTERRUPT, KVM_S390_INTERRUPT or KVM_IRQ_LINE, as long as
> the code makes it obvious what is happening.
> 
Some consistency would be nice though. You do not always look at the
kernel code when you read userspace code and iothread calling KVM_INTERRUPT
would have made me suspicious.

--
			Gleb.
diff mbox

Patch

diff --git a/Documentation/virtual/kvm/api.txt b/Documentation/virtual/kvm/api.txt
index 4237c27..5050492 100644
--- a/Documentation/virtual/kvm/api.txt
+++ b/Documentation/virtual/kvm/api.txt
@@ -615,15 +615,32 @@  created.
 4.25 KVM_IRQ_LINE
 
 Capability: KVM_CAP_IRQCHIP
-Architectures: x86, ia64
+Architectures: x86, ia64, arm
 Type: vm ioctl
 Parameters: struct kvm_irq_level
 Returns: 0 on success, -1 on error
 
 Sets the level of a GSI input to the interrupt controller model in the kernel.
-Requires that an interrupt controller model has been previously created with
-KVM_CREATE_IRQCHIP.  Note that edge-triggered interrupts require the level
-to be set to 1 and then back to 0.
+On some architectures it is required that an interrupt controller model has
+been previously created with KVM_CREATE_IRQCHIP.  Note that edge-triggered
+interrupts require the level to be set to 1 and then back to 0.
+
+ARM can signal an interrupt either at the CPU level, or at the in-kernel irqchip
+(GIC), and for in-kernel irqchip can tell the GIC to use PPIs designated for
+specific cpus.  The irq field is interpreted like this:
+
+  bits:  | 31 ... 24 | 23  ... 16 | 15    ...    0 |
+  field: | irq_type  | vcpu_index |     irq_id     |
+
+The irq_type field has the following values:
+- irq_type[0]: out-of-kernel GIC: irq_id 0 is IRQ, irq_id 1 is FIQ
+- irq_type[1]: in-kernel GIC: SPI, irq_id between 32 and 1019 (incl.)
+               (the vcpu_index field is ignored)
+- irq_type[2]: in-kernel GIC: PPI, irq_id between 16 and 31 (incl.)
+
+(The irq_id field thus corresponds nicely to the IRQ ID in the ARM GIC specs)
+
+In both cases, level is used to raise/lower the line.
 
 struct kvm_irq_level {
 	union {
diff --git a/arch/arm/include/asm/kvm_arm.h b/arch/arm/include/asm/kvm_arm.h
index 613afe2..fb22ee8 100644
--- a/arch/arm/include/asm/kvm_arm.h
+++ b/arch/arm/include/asm/kvm_arm.h
@@ -68,6 +68,7 @@ 
 #define HCR_GUEST_MASK (HCR_TSC | HCR_TSW | HCR_TWI | HCR_VM | HCR_BSU_IS | \
 			HCR_FB | HCR_TAC | HCR_AMO | HCR_IMO | HCR_FMO | \
 			HCR_SWIO | HCR_TIDCP)
+#define HCR_VIRT_EXCP_MASK (HCR_VA | HCR_VI | HCR_VF)
 
 /* Hyp System Control Register (HSCTLR) bits */
 #define HSCTLR_TE	(1 << 30)
diff --git a/arch/arm/include/uapi/asm/kvm.h b/arch/arm/include/uapi/asm/kvm.h
index c6298b1..4cf6d8f 100644
--- a/arch/arm/include/uapi/asm/kvm.h
+++ b/arch/arm/include/uapi/asm/kvm.h
@@ -23,6 +23,7 @@ 
 #include <asm/ptrace.h>
 
 #define __KVM_HAVE_GUEST_DEBUG
+#define __KVM_HAVE_IRQ_LINE
 
 #define KVM_REG_SIZE(id)						\
 	(1U << (((id) & KVM_REG_SIZE_MASK) >> KVM_REG_SIZE_SHIFT))
@@ -103,4 +104,24 @@  struct kvm_arch_memory_slot {
 #define KVM_REG_ARM_CORE		(0x0010 << KVM_REG_ARM_COPROC_SHIFT)
 #define KVM_REG_ARM_CORE_REG(name)	(offsetof(struct kvm_regs, name) / 4)
 
+/* KVM_IRQ_LINE irq field index values */
+#define KVM_ARM_IRQ_TYPE_SHIFT		24
+#define KVM_ARM_IRQ_TYPE_MASK		0xff
+#define KVM_ARM_IRQ_VCPU_SHIFT		16
+#define KVM_ARM_IRQ_VCPU_MASK		0xff
+#define KVM_ARM_IRQ_NUM_SHIFT		0
+#define KVM_ARM_IRQ_NUM_MASK		0xffff
+
+/* irq_type field */
+#define KVM_ARM_IRQ_TYPE_CPU		0
+#define KVM_ARM_IRQ_TYPE_SPI		1
+#define KVM_ARM_IRQ_TYPE_PPI		2
+
+/* out-of-kernel GIC cpu interrupt injection irq_number field */
+#define KVM_ARM_IRQ_CPU_IRQ		0
+#define KVM_ARM_IRQ_CPU_FIQ		1
+
+/* Highest supported SPI, from VGIC_NR_IRQS */
+#define KVM_ARM_IRQ_GIC_MAX		127
+
 #endif /* __ARM_KVM_H__ */
diff --git a/arch/arm/kvm/arm.c b/arch/arm/kvm/arm.c
index ab82039..9b4566e 100644
--- a/arch/arm/kvm/arm.c
+++ b/arch/arm/kvm/arm.c
@@ -24,6 +24,7 @@ 
 #include <linux/fs.h>
 #include <linux/mman.h>
 #include <linux/sched.h>
+#include <linux/kvm.h>
 #include <trace/events/kvm.h>
 
 #define CREATE_TRACE_POINTS
@@ -284,6 +285,7 @@  void kvm_arch_vcpu_uninit(struct kvm_vcpu *vcpu)
 
 void kvm_arch_vcpu_load(struct kvm_vcpu *vcpu, int cpu)
 {
+	vcpu->cpu = cpu;
 }
 
 void kvm_arch_vcpu_put(struct kvm_vcpu *vcpu)
@@ -324,6 +326,69 @@  int kvm_arch_vcpu_ioctl_run(struct kvm_vcpu *vcpu, struct kvm_run *run)
 	return -EINVAL;
 }
 
+static int vcpu_interrupt_line(struct kvm_vcpu *vcpu, int number, bool level)
+{
+	int bit_index;
+	bool set;
+	unsigned long *ptr;
+
+	if (number == KVM_ARM_IRQ_CPU_IRQ)
+		bit_index = __ffs(HCR_VI);
+	else /* KVM_ARM_IRQ_CPU_FIQ */
+		bit_index = __ffs(HCR_VF);
+
+	ptr = (unsigned long *)&vcpu->arch.irq_lines;
+	if (level)
+		set = test_and_set_bit(bit_index, ptr);
+	else
+		set = test_and_clear_bit(bit_index, ptr);
+
+	/*
+	 * If we didn't change anything, no need to wake up or kick other CPUs
+	 */
+	if (set == level)
+		return 0;
+
+	/*
+	 * The vcpu irq_lines field was updated, wake up sleeping VCPUs and
+	 * trigger a world-switch round on the running physical CPU to set the
+	 * virtual IRQ/FIQ fields in the HCR appropriately.
+	 */
+	kvm_vcpu_kick(vcpu);
+
+	return 0;
+}
+
+int kvm_vm_ioctl_irq_line(struct kvm *kvm, struct kvm_irq_level *irq_level)
+{
+	u32 irq = irq_level->irq;
+	unsigned int irq_type, vcpu_idx, irq_num;
+	int nrcpus = atomic_read(&kvm->online_vcpus);
+	struct kvm_vcpu *vcpu = NULL;
+	bool level = irq_level->level;
+
+	irq_type = (irq >> KVM_ARM_IRQ_TYPE_SHIFT) & KVM_ARM_IRQ_TYPE_MASK;
+	vcpu_idx = (irq >> KVM_ARM_IRQ_VCPU_SHIFT) & KVM_ARM_IRQ_VCPU_MASK;
+	irq_num = (irq >> KVM_ARM_IRQ_NUM_SHIFT) & KVM_ARM_IRQ_NUM_MASK;
+
+	trace_kvm_irq_line(irq_type, vcpu_idx, irq_num, irq_level->level);
+
+	if (irq_type != KVM_ARM_IRQ_TYPE_CPU)
+		return -EINVAL;
+
+	if (vcpu_idx >= nrcpus)
+		return -EINVAL;
+
+	vcpu = kvm_get_vcpu(kvm, vcpu_idx);
+	if (!vcpu)
+		return -EINVAL;
+
+	if (irq_num > KVM_ARM_IRQ_CPU_FIQ)
+		return -EINVAL;
+
+	return vcpu_interrupt_line(vcpu, irq_num, level);
+}
+
 long kvm_arch_vcpu_ioctl(struct file *filp,
 			 unsigned int ioctl, unsigned long arg)
 {
diff --git a/arch/arm/kvm/trace.h b/arch/arm/kvm/trace.h
index 862b2cc..105d1f7 100644
--- a/arch/arm/kvm/trace.h
+++ b/arch/arm/kvm/trace.h
@@ -39,6 +39,31 @@  TRACE_EVENT(kvm_exit,
 	TP_printk("PC: 0x%08lx", __entry->vcpu_pc)
 );
 
+TRACE_EVENT(kvm_irq_line,
+	TP_PROTO(unsigned int type, int vcpu_idx, int irq_num, int level),
+	TP_ARGS(type, vcpu_idx, irq_num, level),
+
+	TP_STRUCT__entry(
+		__field(	unsigned int,	type		)
+		__field(	int,		vcpu_idx	)
+		__field(	int,		irq_num		)
+		__field(	int,		level		)
+	),
+
+	TP_fast_assign(
+		__entry->type		= type;
+		__entry->vcpu_idx	= vcpu_idx;
+		__entry->irq_num	= irq_num;
+		__entry->level		= level;
+	),
+
+	TP_printk("Inject %s interrupt (%d), vcpu->idx: %d, num: %d, level: %d",
+		  (__entry->type == KVM_ARM_IRQ_TYPE_CPU) ? "CPU" :
+		  (__entry->type == KVM_ARM_IRQ_TYPE_PPI) ? "VGIC PPI" :
+		  (__entry->type == KVM_ARM_IRQ_TYPE_SPI) ? "VGIC SPI" : "UNKNOWN",
+		  __entry->type, __entry->vcpu_idx, __entry->irq_num, __entry->level)
+);
+
 TRACE_EVENT(kvm_unmap_hva,
 	TP_PROTO(unsigned long hva),
 	TP_ARGS(hva),
diff --git a/include/uapi/linux/kvm.h b/include/uapi/linux/kvm.h
index 24978d5..dc63665 100644
--- a/include/uapi/linux/kvm.h
+++ b/include/uapi/linux/kvm.h
@@ -115,6 +115,7 @@  struct kvm_irq_level {
 	 * ACPI gsi notion of irq.
 	 * For IA-64 (APIC model) IOAPIC0: irq 0-23; IOAPIC1: irq 24-47..
 	 * For X86 (standard AT mode) PIC0/1: irq 0-15. IOAPIC0: 0-23..
+	 * For ARM: See Documentation/virtual/kvm/api.txt
 	 */
 	union {
 		__u32 irq;