[PATCH] KVM: Add KVM_CAP_IRQ_ARCH capability

[PATCH] KVM: Add KVM_CAP_IRQ_ARCH capability

[Paul Mackerras]

Setting this capability on a vcpu connects that vcpu to an interrupt
controller device.  The args[0] field of the argument kvm_enable_cap
struct specifies the overall architecture of the interrupt
controller.  The args[1] field specifies the CPU number for the vcpu
from the interrupt controller's point of view.

Signed-off-by: Paul Mackerras <paulus@samba.org>
---
 arch/powerpc/include/asm/kvm_host.h |    3 +++
 arch/powerpc/kvm/powerpc.c          |   29 +++++++++++++++++++++++++++++
 include/uapi/linux/kvm.h            |    1 +
 3 files changed, 33 insertions(+)

diff --git a/arch/powerpc/include/asm/kvm_host.h b/arch/powerpc/include/asm/kvm_host.h
index f4ba881..dd167e4 100644
--- a/arch/powerpc/include/asm/kvm_host.h
+++ b/arch/powerpc/include/asm/kvm_host.h
@@ -373,6 +373,9 @@ struct kvmppc_booke_debug_reg {
 struct kvm_vcpu_arch {
 	ulong host_stack;
 	u32 host_pid;
+
+	u32 intr_ctrler;
+
 #ifdef CONFIG_PPC_BOOK3S
 	struct kvmppc_slb slb[64];
 	int slb_max;		/* 1 + index of last valid entry in slb[] */
diff --git a/arch/powerpc/kvm/powerpc.c b/arch/powerpc/kvm/powerpc.c
index 934413c..891603e 100644
--- a/arch/powerpc/kvm/powerpc.c
+++ b/arch/powerpc/kvm/powerpc.c
@@ -459,6 +459,12 @@ void kvm_arch_vcpu_free(struct kvm_vcpu *vcpu)
 	hrtimer_cancel(&vcpu->arch.dec_timer);
 	tasklet_kill(&vcpu->arch.tasklet);
 
+	/* Release any per-vcpu irq controller state */
+	switch (vcpu->arch.intr_ctrler) {
+	default:
+		break;
+	}
+
 	kvmppc_remove_vcpu_debugfs(vcpu);
 	kvmppc_core_vcpu_free(vcpu);
 }
@@ -791,6 +797,29 @@ static int kvm_vcpu_ioctl_enable_cap(struct kvm_vcpu *vcpu,
 		break;
 	}
 #endif
+	case KVM_CAP_IRQ_ARCH:
+		r = -EBUSY;
+		mutex_lock(&vcpu->kvm->lock);
+		/* disallow changing once set */
+		if (!vcpu->arch.intr_ctrler) {
+			r = 0;
+			switch (cap->args[0]) {
+			case 0:		/* no interrupt controller */
+				break;
+			default:
+				r = -EINVAL;
+			}
+			if (!r) {
+				/*
+				 * Make sure any state set up by the interrupt
+				 * controller init routine is seen before this.
+				 */
+				smp_wmb();
+				vcpu->arch.intr_ctrler = cap->args[0];
+			}
+		}
+		mutex_unlock(&vcpu->kvm->lock);
+		break;
 	default:
 		r = -EINVAL;
 		break;
diff --git a/include/uapi/linux/kvm.h b/include/uapi/linux/kvm.h
index 1f348e0..d875d37 100644
--- a/include/uapi/linux/kvm.h
+++ b/include/uapi/linux/kvm.h
@@ -663,6 +663,7 @@ struct kvm_ppc_smmu_info {
 #define KVM_CAP_S390_CSS_SUPPORT 85
 #define KVM_CAP_PPC_EPR 86
 #define KVM_CAP_DEVICE_CTRL 87
+#define KVM_CAP_IRQ_ARCH 88
 
 #ifdef KVM_CAP_IRQ_ROUTING
 
-- 
1.7.10.4

Re: [PATCH] KVM: Add KVM_CAP_IRQ_ARCH capability

[Scott Wood]

On 03/13/2013 08:20:44 PM, Paul Mackerras wrote:
> Setting this capability on a vcpu connects that vcpu to an interrupt
> controller device.  The args[0] field of the argument kvm_enable_cap
> struct specifies the overall architecture of the interrupt
> controller.  The args[1] field specifies the CPU number for the vcpu
> from the interrupt controller's point of view.
> 
> Signed-off-by: Paul Mackerras <paulus@samba.org>
> ---
>  arch/powerpc/include/asm/kvm_host.h |    3 +++
>  arch/powerpc/kvm/powerpc.c          |   29  
> +++++++++++++++++++++++++++++
>  include/uapi/linux/kvm.h            |    1 +
>  3 files changed, 33 insertions(+)
> 
> diff --git a/arch/powerpc/include/asm/kvm_host.h  
> b/arch/powerpc/include/asm/kvm_host.h
> index f4ba881..dd167e4 100644
> --- a/arch/powerpc/include/asm/kvm_host.h
> +++ b/arch/powerpc/include/asm/kvm_host.h
> @@ -373,6 +373,9 @@ struct kvmppc_booke_debug_reg {
>  struct kvm_vcpu_arch {
>  	ulong host_stack;
>  	u32 host_pid;
> +
> +	u32 intr_ctrler;
> +

That abbreviation seems a bit awkward, and we should also have a
private-data pointer.

How about:

	u32 irq_arch;
	void *irq_priv;

>  #endif
> +	case KVM_CAP_IRQ_ARCH:
> +		r = -EBUSY;
> +		mutex_lock(&vcpu->kvm->lock);
> +		/* disallow changing once set */
> +		if (!vcpu->arch.intr_ctrler) {
> +			r = 0;
> +			switch (cap->args[0]) {
> +			case 0:		/* no interrupt controller */
> +				break;

s/0/KVM_IRQ_ARCH_NONE/

...at least so that this patch makes it clear where other type ids  
should
be defined.

> +			default:
> +				r = -EINVAL;
> +			}
> +			if (!r) {
> +				/*
> +				 * Make sure any state set up by the  
> interrupt
> +				 * controller init routine is seen  
> before this.
> +				 */
> +				smp_wmb();
> +				vcpu->arch.intr_ctrler = cap->args[0];
> +			}

Do we really need that wmb()?  We're in vcpu context, right?  If the  
vcpu
migrates to another host cpu, that involves rescheduling which already
has a sync.  If the interrupt controller code we call here modifies data
that will be seen from outside the vcpu, it's the responsibility of that
code to use whatever locks, barriers, etc.  are needed (and it's  
unlikely
that vcpu->arch.intr_ctrler will be the relevant thing that it needs to
order with).

This patch should also add a hook at vcpu destruction to call into the
irq code.

-Scott

Re: [PATCH] KVM: Add KVM_CAP_IRQ_ARCH capability

[Alexander Graf]

On 14.03.2013, at 19:20, Scott Wood wrote:

> On 03/13/2013 08:20:44 PM, Paul Mackerras wrote:
>> Setting this capability on a vcpu connects that vcpu to an interrupt
>> controller device.  The args[0] field of the argument kvm_enable_cap
>> struct specifies the overall architecture of the interrupt
>> controller.  The args[1] field specifies the CPU number for the vcpu
>> from the interrupt controller's point of view.
>> Signed-off-by: Paul Mackerras <paulus@samba.org>
>> ---
>> arch/powerpc/include/asm/kvm_host.h |    3 +++
>> arch/powerpc/kvm/powerpc.c          |   29 +++++++++++++++++++++++++++++
>> include/uapi/linux/kvm.h            |    1 +
>> 3 files changed, 33 insertions(+)
>> diff --git a/arch/powerpc/include/asm/kvm_host.h b/arch/powerpc/include/asm/kvm_host.h
>> index f4ba881..dd167e4 100644
>> --- a/arch/powerpc/include/asm/kvm_host.h
>> +++ b/arch/powerpc/include/asm/kvm_host.h
>> @@ -373,6 +373,9 @@ struct kvmppc_booke_debug_reg {
>> struct kvm_vcpu_arch {
>> 	ulong host_stack;
>> 	u32 host_pid;
>> +
>> +	u32 intr_ctrler;
>> +
> 
> That abbreviation seems a bit awkward, and we should also have a
> private-data pointer.
> 
> How about:
> 
> 	u32 irq_arch;

We also want int pic_fd, no?


Alex

Re: [PATCH] KVM: Add KVM_CAP_IRQ_ARCH capability

[Scott Wood]

On 03/14/2013 01:33:30 PM, Alexander Graf wrote:
> 
> On 14.03.2013, at 19:20, Scott Wood wrote:
> 
> > On 03/13/2013 08:20:44 PM, Paul Mackerras wrote:
> >> Setting this capability on a vcpu connects that vcpu to an  
> interrupt
> >> controller device.  The args[0] field of the argument  
> kvm_enable_cap
> >> struct specifies the overall architecture of the interrupt
> >> controller.  The args[1] field specifies the CPU number for the  
> vcpu
> >> from the interrupt controller's point of view.
> >> Signed-off-by: Paul Mackerras <paulus@samba.org>
> >> ---
> >> arch/powerpc/include/asm/kvm_host.h |    3 +++
> >> arch/powerpc/kvm/powerpc.c          |   29  
> +++++++++++++++++++++++++++++
> >> include/uapi/linux/kvm.h            |    1 +
> >> 3 files changed, 33 insertions(+)
> >> diff --git a/arch/powerpc/include/asm/kvm_host.h  
> b/arch/powerpc/include/asm/kvm_host.h
> >> index f4ba881..dd167e4 100644
> >> --- a/arch/powerpc/include/asm/kvm_host.h
> >> +++ b/arch/powerpc/include/asm/kvm_host.h
> >> @@ -373,6 +373,9 @@ struct kvmppc_booke_debug_reg {
> >> struct kvm_vcpu_arch {
> >> 	ulong host_stack;
> >> 	u32 host_pid;
> >> +
> >> +	u32 intr_ctrler;
> >> +
> >
> > That abbreviation seems a bit awkward, and we should also have a
> > private-data pointer.
> >
> > How about:
> >
> > 	u32 irq_arch;
> 
> We also want int pic_fd, no?

Not sure we really need that on the vcpu.  We'll need it on the vm  
unless we add it as an arg to the vcpu cap enable.

-Scott

Re: [PATCH] KVM: Add KVM_CAP_IRQ_ARCH capability

[Alexander Graf]

On 14.03.2013, at 19:35, Scott Wood wrote:

> On 03/14/2013 01:33:30 PM, Alexander Graf wrote:
>> On 14.03.2013, at 19:20, Scott Wood wrote:
>> > On 03/13/2013 08:20:44 PM, Paul Mackerras wrote:
>> >> Setting this capability on a vcpu connects that vcpu to an interrupt
>> >> controller device.  The args[0] field of the argument kvm_enable_cap
>> >> struct specifies the overall architecture of the interrupt
>> >> controller.  The args[1] field specifies the CPU number for the vcpu
>> >> from the interrupt controller's point of view.
>> >> Signed-off-by: Paul Mackerras <paulus@samba.org>
>> >> ---
>> >> arch/powerpc/include/asm/kvm_host.h |    3 +++
>> >> arch/powerpc/kvm/powerpc.c          |   29 +++++++++++++++++++++++++++++
>> >> include/uapi/linux/kvm.h            |    1 +
>> >> 3 files changed, 33 insertions(+)
>> >> diff --git a/arch/powerpc/include/asm/kvm_host.h b/arch/powerpc/include/asm/kvm_host.h
>> >> index f4ba881..dd167e4 100644
>> >> --- a/arch/powerpc/include/asm/kvm_host.h
>> >> +++ b/arch/powerpc/include/asm/kvm_host.h
>> >> @@ -373,6 +373,9 @@ struct kvmppc_booke_debug_reg {
>> >> struct kvm_vcpu_arch {
>> >> 	ulong host_stack;
>> >> 	u32 host_pid;
>> >> +
>> >> +	u32 intr_ctrler;
>> >> +
>> >
>> > That abbreviation seems a bit awkward, and we should also have a
>> > private-data pointer.
>> >
>> > How about:
>> >
>> > 	u32 irq_arch;
>> We also want int pic_fd, no?
> 
> Not sure we really need that on the vcpu.  We'll need it on the vm unless we add it as an arg to the vcpu cap enable.

I don't think we need anything vm global for the cpu <-> PIC connections. Also, if you want to deregister a CPU (hotplug remove), you probably want to tell the PIC that the CPU has gone.


Alex

Re: [PATCH] KVM: Add KVM_CAP_IRQ_ARCH capability

[Scott Wood]

On 03/14/2013 02:03:30 PM, Alexander Graf wrote:
> 
> On 14.03.2013, at 19:35, Scott Wood wrote:
> 
> > On 03/14/2013 01:33:30 PM, Alexander Graf wrote:
> >> On 14.03.2013, at 19:20, Scott Wood wrote:
> >> > On 03/13/2013 08:20:44 PM, Paul Mackerras wrote:
> >> >> Setting this capability on a vcpu connects that vcpu to an  
> interrupt
> >> >> controller device.  The args[0] field of the argument  
> kvm_enable_cap
> >> >> struct specifies the overall architecture of the interrupt
> >> >> controller.  The args[1] field specifies the CPU number for the  
> vcpu
> >> >> from the interrupt controller's point of view.
> >> >> Signed-off-by: Paul Mackerras <paulus@samba.org>
> >> >> ---
> >> >> arch/powerpc/include/asm/kvm_host.h |    3 +++
> >> >> arch/powerpc/kvm/powerpc.c          |   29  
> +++++++++++++++++++++++++++++
> >> >> include/uapi/linux/kvm.h            |    1 +
> >> >> 3 files changed, 33 insertions(+)
> >> >> diff --git a/arch/powerpc/include/asm/kvm_host.h  
> b/arch/powerpc/include/asm/kvm_host.h
> >> >> index f4ba881..dd167e4 100644
> >> >> --- a/arch/powerpc/include/asm/kvm_host.h
> >> >> +++ b/arch/powerpc/include/asm/kvm_host.h
> >> >> @@ -373,6 +373,9 @@ struct kvmppc_booke_debug_reg {
> >> >> struct kvm_vcpu_arch {
> >> >> 	ulong host_stack;
> >> >> 	u32 host_pid;
> >> >> +
> >> >> +	u32 intr_ctrler;
> >> >> +
> >> >
> >> > That abbreviation seems a bit awkward, and we should also have a
> >> > private-data pointer.
> >> >
> >> > How about:
> >> >
> >> > 	u32 irq_arch;
> >> We also want int pic_fd, no?
> >
> > Not sure we really need that on the vcpu.  We'll need it on the vm  
> unless we add it as an arg to the vcpu cap enable.
> 
> I don't think we need anything vm global for the cpu <-> PIC  
> connections.

The two components have to link up somehow.  If the vcpu cap enable  
ioctl doesn't specify it, then the device creation will have to stick a  
reference to itself somewhere in the vm.

> Also, if you want to deregister a CPU (hotplug remove), you probably  
> want to tell the PIC that the CPU has gone.

Yes, I mentioned that in my previous e-mail.

-Scott

Re: [PATCH] KVM: Add KVM_CAP_IRQ_ARCH capability

[Alexander Graf]

On 14.03.2013, at 20:10, Scott Wood wrote:

> On 03/14/2013 02:03:30 PM, Alexander Graf wrote:
>> On 14.03.2013, at 19:35, Scott Wood wrote:
>> > On 03/14/2013 01:33:30 PM, Alexander Graf wrote:
>> >> On 14.03.2013, at 19:20, Scott Wood wrote:
>> >> > On 03/13/2013 08:20:44 PM, Paul Mackerras wrote:
>> >> >> Setting this capability on a vcpu connects that vcpu to an interrupt
>> >> >> controller device.  The args[0] field of the argument kvm_enable_cap
>> >> >> struct specifies the overall architecture of the interrupt
>> >> >> controller.  The args[1] field specifies the CPU number for the vcpu
>> >> >> from the interrupt controller's point of view.
>> >> >> Signed-off-by: Paul Mackerras <paulus@samba.org>
>> >> >> ---
>> >> >> arch/powerpc/include/asm/kvm_host.h |    3 +++
>> >> >> arch/powerpc/kvm/powerpc.c          |   29 +++++++++++++++++++++++++++++
>> >> >> include/uapi/linux/kvm.h            |    1 +
>> >> >> 3 files changed, 33 insertions(+)
>> >> >> diff --git a/arch/powerpc/include/asm/kvm_host.h b/arch/powerpc/include/asm/kvm_host.h
>> >> >> index f4ba881..dd167e4 100644
>> >> >> --- a/arch/powerpc/include/asm/kvm_host.h
>> >> >> +++ b/arch/powerpc/include/asm/kvm_host.h
>> >> >> @@ -373,6 +373,9 @@ struct kvmppc_booke_debug_reg {
>> >> >> struct kvm_vcpu_arch {
>> >> >> 	ulong host_stack;
>> >> >> 	u32 host_pid;
>> >> >> +
>> >> >> +	u32 intr_ctrler;
>> >> >> +
>> >> >
>> >> > That abbreviation seems a bit awkward, and we should also have a
>> >> > private-data pointer.
>> >> >
>> >> > How about:
>> >> >
>> >> > 	u32 irq_arch;
>> >> We also want int pic_fd, no?
>> >
>> > Not sure we really need that on the vcpu.  We'll need it on the vm unless we add it as an arg to the vcpu cap enable.
>> I don't think we need anything vm global for the cpu <-> PIC connections.
> 
> The two components have to link up somehow.  If the vcpu cap enable ioctl doesn't specify it, then the device creation will have to stick a reference to itself somewhere in the vm.

Yes, and I think it's a mistake to do it in the vm. We should rather specify the PIC fd in the cap enable ioctl.

> 
>> Also, if you want to deregister a CPU (hotplug remove), you probably want to tell the PIC that the CPU has gone.
> 
> Yes, I mentioned that in my previous e-mail.

Yup :). No disagreement with you so far ;)


Alex

Re: [PATCH] KVM: Add KVM_CAP_IRQ_ARCH capability

[Paul Mackerras]

On Thu, Mar 14, 2013 at 08:03:30PM +0100, Alexander Graf wrote:
> 
> On 14.03.2013, at 19:35, Scott Wood wrote:
> 
> > On 03/14/2013 01:33:30 PM, Alexander Graf wrote:
> >> We also want int pic_fd, no?
> > 
> > Not sure we really need that on the vcpu.  We'll need it on the vm unless we add it as an arg to the vcpu cap enable.
> 
> I don't think we need anything vm global for the cpu <-> PIC connections. Also, if you want to deregister a CPU (hotplug remove), you probably want to tell the PIC that the CPU has gone.

I had kvm_arch_vcpu_free() calling a release function for the selected
PIC, which should be enough to let the PIC know the CPU has gone away,
I would think.

I agree we don't need anything vm global.  The only vm ioctl which
needs to care about interrupt controllers is KVM_IRQ_LINE, and the
approach I take in my patchset is just to call all of the compiled-in
interrupt controllers until one of them takes it.  That assumes that
each irq architecture adds its own private data pointer to kvm->arch
if it's compiled in, which is feasible provided there are only a few
architectures supported, and gives the advantages of strong typing.
I did it this way because with the irq architecture being specified
per vcpu, there is no overall vm global irq architecture.

Paul.

Re: [PATCH] KVM: Add KVM_CAP_IRQ_ARCH capability

[Scott Wood]

On 03/14/2013 05:28:03 PM, Paul Mackerras wrote:
> On Thu, Mar 14, 2013 at 08:03:30PM +0100, Alexander Graf wrote:
> >
> > On 14.03.2013, at 19:35, Scott Wood wrote:
> >
> > > On 03/14/2013 01:33:30 PM, Alexander Graf wrote:
> > >> We also want int pic_fd, no?
> > >
> > > Not sure we really need that on the vcpu.  We'll need it on the  
> vm unless we add it as an arg to the vcpu cap enable.
> >
> > I don't think we need anything vm global for the cpu <-> PIC  
> connections. Also, if you want to deregister a CPU (hotplug remove),  
> you probably want to tell the PIC that the CPU has gone.
> 
> I had kvm_arch_vcpu_free() calling a release function for the selected
> PIC, which should be enough to let the PIC know the CPU has gone away,
> I would think.

Ah, sorry, I missed that somehow.

> I agree we don't need anything vm global.  The only vm ioctl which
> needs to care about interrupt controllers is KVM_IRQ_LINE, and the
> approach I take in my patchset is just to call all of the compiled-in
> interrupt controllers until one of them takes it.

KVM_IRQ_LINE is (eventually) supposed to go through the IRQ routing  
layer that determines which irqchip it goes to.  Otherwise, how will we  
do things like generate MSIs through irqfd?  At least, for those of us  
who haven't already mapped MSIs to normal interrupts inside QEMU via  
hcalls. :-)

-Scott

Re: [PATCH] KVM: Add KVM_CAP_IRQ_ARCH capability

[Paul Mackerras]

On Thu, Mar 14, 2013 at 01:20:38PM -0500, Scott Wood wrote:
> On 03/13/2013 08:20:44 PM, Paul Mackerras wrote:

> >--- a/arch/powerpc/include/asm/kvm_host.h
> >+++ b/arch/powerpc/include/asm/kvm_host.h
> >@@ -373,6 +373,9 @@ struct kvmppc_booke_debug_reg {
> > struct kvm_vcpu_arch {
> > 	ulong host_stack;
> > 	u32 host_pid;
> >+
> >+	u32 intr_ctrler;
> >+
> 
> That abbreviation seems a bit awkward, and we should also have a
> private-data pointer.
> 
> How about:
> 
> 	u32 irq_arch;
> 	void *irq_priv;

Regarding the irq_priv - in my patchset the XICS code adds its own
private data pointer.  That has the advantage that it can be strongly
typed, and if it is non-NULL then I know it points to XICS data, not
the data for some other type of controller.  As long as we are only
going to have a small number of IRQ architectures then it's feasible
to allow each to have its own data pointer, and we get the advantages
of strong typing.

> >+			switch (cap->args[0]) {
> >+			case 0:		/* no interrupt controller */
> >+				break;
> 
> s/0/KVM_IRQ_ARCH_NONE/
> 
> ...at least so that this patch makes it clear where other type ids
> should
> be defined.

OK, whatever.

> >+			default:
> >+				r = -EINVAL;
> >+			}
> >+			if (!r) {
> >+				/*
> >+				 * Make sure any state set up by the interrupt
> >+				 * controller init routine is seen before this.
> >+				 */
> >+				smp_wmb();
> >+				vcpu->arch.intr_ctrler = cap->args[0];
> >+			}
> 
> Do we really need that wmb()?  We're in vcpu context, right?  If the
> vcpu
> migrates to another host cpu, that involves rescheduling which already
> has a sync.  If the interrupt controller code we call here modifies data
> that will be seen from outside the vcpu, it's the responsibility of that
> code to use whatever locks, barriers, etc.  are needed (and it's
> unlikely
> that vcpu->arch.intr_ctrler will be the relevant thing that it needs to
> order with).

OK, you're right, the generic KVM code serializes most vcpu ioctls,
including KVM_RUN and KVM_ENABLE_CAP, so the barrier isn't in fact
needed.

> This patch should also add a hook at vcpu destruction to call into the
> irq code.

You appear to have missed this hunk:

diff --git a/arch/powerpc/kvm/powerpc.c b/arch/powerpc/kvm/powerpc.c
index 934413c..891603e 100644
--- a/arch/powerpc/kvm/powerpc.c
+++ b/arch/powerpc/kvm/powerpc.c
@@ -459,6 +459,12 @@ void kvm_arch_vcpu_free(struct kvm_vcpu *vcpu)
 	hrtimer_cancel(&vcpu->arch.dec_timer);
 	tasklet_kill(&vcpu->arch.tasklet);
 
+	/* Release any per-vcpu irq controller state */
+	switch (vcpu->arch.intr_ctrler) {
+	default:
+		break;
+	}
+
 	kvmppc_remove_vcpu_debugfs(vcpu);
 	kvmppc_core_vcpu_free(vcpu);
 }

Paul.

Re: [PATCH] KVM: Add KVM_CAP_IRQ_ARCH capability

[Scott Wood]

On 03/14/2013 05:19:17 PM, Paul Mackerras wrote:
> On Thu, Mar 14, 2013 at 01:20:38PM -0500, Scott Wood wrote:
> > On 03/13/2013 08:20:44 PM, Paul Mackerras wrote:
> 
> > >--- a/arch/powerpc/include/asm/kvm_host.h
> > >+++ b/arch/powerpc/include/asm/kvm_host.h
> > >@@ -373,6 +373,9 @@ struct kvmppc_booke_debug_reg {
> > > struct kvm_vcpu_arch {
> > > 	ulong host_stack;
> > > 	u32 host_pid;
> > >+
> > >+	u32 intr_ctrler;
> > >+
> >
> > That abbreviation seems a bit awkward, and we should also have a
> > private-data pointer.
> >
> > How about:
> >
> > 	u32 irq_arch;
> > 	void *irq_priv;
> 
> Regarding the irq_priv - in my patchset the XICS code adds its own
> private data pointer.  That has the advantage that it can be strongly
> typed, and if it is non-NULL then I know it points to XICS data, not
> the data for some other type of controller.  As long as we are only
> going to have a small number of IRQ architectures then it's feasible
> to allow each to have its own data pointer, and we get the advantages
> of strong typing.

OK.

> > >+			switch (cap->args[0]) {
> > >+			case 0:		/* no interrupt controller */
> > >+				break;
> >
> > s/0/KVM_IRQ_ARCH_NONE/
> >
> > ...at least so that this patch makes it clear where other type ids
> > should
> > be defined.
> 
> OK, whatever.

Well, it wouldn't be good for MPIC to end up defined in one place and  
XICS in another, especially if adding a definition is how new IDs are  
allocated...

-Scott