[PATCH 0/3 v2] MSI-X enabling

[PATCH 0/3 v2] MSI-X enabling

[Sheng Yang]

Based on former patchset.

[PATCH 1/3] KVM: Ioctls for init MSI-X entry

[Sheng Yang]

Introduce KVM_SET_MSIX_NR and KVM_SET_MSIX_ENTRY two ioctls.

This two ioctls are used by userspace to specific guest device MSI-X entry
number and correlate MSI-X entry with GSI during the initialization stage.

MSI-X should be well initialzed before enabling.

Don't support change MSI-X entry number for now.

Signed-off-by: Sheng Yang <sheng@linux.intel.com>
---
 include/linux/kvm.h      |   14 +++++++
 include/linux/kvm_host.h |    3 +
 virt/kvm/kvm_main.c      |   96 ++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 113 insertions(+), 0 deletions(-)

diff --git a/include/linux/kvm.h b/include/linux/kvm.h
index c1425ab..5200768 100644
--- a/include/linux/kvm.h
+++ b/include/linux/kvm.h
@@ -474,6 +474,8 @@ struct kvm_irq_routing {
 #define KVM_ASSIGN_IRQ _IOR(KVMIO, 0x70, \
 			    struct kvm_assigned_irq)
 #define KVM_REINJECT_CONTROL      _IO(KVMIO, 0x71)
+#define KVM_SET_MSIX_NR _IOR(KVMIO, 0x72, struct kvm_assigned_msix_nr)
+#define KVM_SET_MSIX_ENTRY _IOR(KVMIO, 0x73, struct kvm_assigned_msix_entry)
 
 /*
  * ioctls for vcpu fds
@@ -594,4 +596,16 @@ struct kvm_assigned_irq {
 #define KVM_DEV_IRQ_ASSIGN_MSI_ACTION	KVM_DEV_IRQ_ASSIGN_ENABLE_MSI
 #define KVM_DEV_IRQ_ASSIGN_ENABLE_MSI	(1 << 0)
 
+struct kvm_assigned_msix_nr {
+	__u32 assigned_dev_id;
+	__u16 entry_nr;
+};
+
+struct kvm_assigned_msix_entry {
+	__u32 assigned_dev_id;
+	__u32 gsi;
+	__u16 entry;
+	__u16 pos;
+};
+
 #endif
diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h
index 14bd85d..a7d6123 100644
--- a/include/linux/kvm_host.h
+++ b/include/linux/kvm_host.h
@@ -325,9 +325,12 @@ struct kvm_assigned_dev_kernel {
 	int assigned_dev_id;
 	int host_busnr;
 	int host_devfn;
+	int entries_nr;
 	int host_irq;
 	bool host_irq_disabled;
+	struct msix_entry *host_msix_entries;
 	int guest_irq;
+	struct msix_entry *guest_msix_entries;
 #define KVM_ASSIGNED_DEV_GUEST_INTX	(1 << 0)
 #define KVM_ASSIGNED_DEV_GUEST_MSI	(1 << 1)
 #define KVM_ASSIGNED_DEV_HOST_INTX	(1 << 8)
diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
index 266bdaf..ea96690 100644
--- a/virt/kvm/kvm_main.c
+++ b/virt/kvm/kvm_main.c
@@ -1593,6 +1593,80 @@ static int kvm_vcpu_ioctl_set_sigmask(struct kvm_vcpu *vcpu, sigset_t *sigset)
 	return 0;
 }
 
+static int kvm_vm_ioctl_set_msix_nr(struct kvm *kvm,
+				    struct kvm_assigned_msix_nr *entry_nr)
+{
+	int r = 0;
+	struct kvm_assigned_dev_kernel *adev;
+
+	mutex_lock(&kvm->lock);
+
+	adev = kvm_find_assigned_dev(&kvm->arch.assigned_dev_head,
+				      entry_nr->assigned_dev_id);
+	if (!adev) {
+		r = -EINVAL;
+		goto msix_nr_out;
+	}
+
+	if (adev->entries_nr == 0) {
+		adev->entries_nr = entry_nr->entry_nr;
+		if (adev->entries_nr == 0)
+			goto msix_nr_out;
+
+		adev->host_msix_entries = kmalloc(sizeof(struct msix_entry) *
+						entry_nr->entry_nr,
+						GFP_KERNEL);
+		if (!adev->host_msix_entries) {
+			printk(KERN_ERR "no memory for host msix entries!\n");
+			r = -ENOMEM;
+			goto msix_nr_out;
+		}
+		adev->guest_msix_entries = kmalloc(sizeof(struct msix_entry) *
+						entry_nr->entry_nr,
+						GFP_KERNEL);
+		if (!adev->guest_msix_entries) {
+			printk(KERN_ERR "no memory for host msix entries!\n");
+			r = -ENOMEM;
+			goto msix_nr_out;
+		}
+	} else
+		printk(KERN_WARNING "kvm: not allow recall set msix nr!\n");
+msix_nr_out:
+	mutex_unlock(&kvm->lock);
+	return r;
+}
+
+static int kvm_vm_ioctl_set_msix_entry(struct kvm *kvm,
+				       struct kvm_assigned_msix_entry *entry)
+{
+	int r = 0;
+	struct kvm_assigned_dev_kernel *adev;
+
+	mutex_lock(&kvm->lock);
+
+	adev = kvm_find_assigned_dev(&kvm->arch.assigned_dev_head,
+				      entry->assigned_dev_id);
+
+	if (!adev) {
+		r = -EINVAL;
+		goto msix_entry_out;
+	}
+
+	if (entry->pos >= adev->entries_nr) {
+		printk(KERN_ERR "kvm: Too much entries for MSI-X!\n");
+		r = -ENOSPC;
+		goto msix_entry_out;
+	}
+
+	adev->guest_msix_entries[entry->pos].entry = entry->entry;
+	adev->guest_msix_entries[entry->pos].vector = entry->gsi;
+	adev->host_msix_entries[entry->pos].entry = entry->entry;
+msix_entry_out:
+	mutex_unlock(&kvm->lock);
+
+	return r;
+}
+
 static long kvm_vcpu_ioctl(struct file *filp,
 			   unsigned int ioctl, unsigned long arg)
 {
@@ -1917,7 +1991,29 @@ static long kvm_vm_ioctl(struct file *filp,
 		vfree(entries);
 		break;
 	}
+#ifdef KVM_CAP_DEVICE_MSIX
+	case KVM_SET_MSIX_NR: {
+		struct kvm_assigned_msix_nr entry_nr;
+		r = -EFAULT;
+		if (copy_from_user(&entry_nr, argp, sizeof entry_nr))
+			goto out;
+		r = kvm_vm_ioctl_set_msix_nr(kvm, &entry_nr);
+		if (r)
+			goto out;
+		break;
+	}
+	case KVM_SET_MSIX_ENTRY: {
+		struct kvm_assigned_msix_entry entry;
+		r = -EFAULT;
+		if (copy_from_user(&entry, argp, sizeof entry))
+			goto out;
+		r = kvm_vm_ioctl_set_msix_entry(kvm, &entry);
+		if (r)
+			goto out;
+		break;
+	}
 #endif
+#endif /* KVM_CAP_IRQ_ROUTING */
 	default:
 		r = kvm_arch_vm_ioctl(filp, ioctl, arg);
 	}
-- 
1.5.4.5

Re: [PATCH 1/3] KVM: Ioctls for init MSI-X entry

[Avi Kivity]

Sheng Yang wrote:
> Introduce KVM_SET_MSIX_NR and KVM_SET_MSIX_ENTRY two ioctls.
>
> This two ioctls are used by userspace to specific guest device MSI-X entry
> number and correlate MSI-X entry with GSI during the initialization stage.
>
> MSI-X should be well initialzed before enabling.
>
> Don't support change MSI-X entry number for now.
>
> Signed-off-by: Sheng Yang <sheng@linux.intel.com>
> ---
>  include/linux/kvm.h      |   14 +++++++
>  include/linux/kvm_host.h |    3 +
>  virt/kvm/kvm_main.c      |   96 ++++++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 113 insertions(+), 0 deletions(-)
>
> diff --git a/include/linux/kvm.h b/include/linux/kvm.h
> index c1425ab..5200768 100644
> --- a/include/linux/kvm.h
> +++ b/include/linux/kvm.h
> @@ -474,6 +474,8 @@ struct kvm_irq_routing {
>  #define KVM_ASSIGN_IRQ _IOR(KVMIO, 0x70, \
>  			    struct kvm_assigned_irq)
>  #define KVM_REINJECT_CONTROL      _IO(KVMIO, 0x71)
> +#define KVM_SET_MSIX_NR _IOR(KVMIO, 0x72, struct kvm_assigned_msix_nr)
> +#define KVM_SET_MSIX_ENTRY _IOR(KVMIO, 0x73, struct kvm_assigned_msix_entry)
>  
>  /*
>   * ioctls for vcpu fds
> @@ -594,4 +596,16 @@ struct kvm_assigned_irq {
>  #define KVM_DEV_IRQ_ASSIGN_MSI_ACTION	KVM_DEV_IRQ_ASSIGN_ENABLE_MSI
>  #define KVM_DEV_IRQ_ASSIGN_ENABLE_MSI	(1 << 0)
>  
> +struct kvm_assigned_msix_nr {
> +	__u32 assigned_dev_id;
> +	__u16 entry_nr;
> +};
> +
> +struct kvm_assigned_msix_entry {
> +	__u32 assigned_dev_id;
> +	__u32 gsi;
> +	__u16 entry;
> +	__u16 pos;
> +}

I'm guessing the intent here is "msi-x number 'pos' on the host assigned 
device will be injected to the guest as gsi 'gsi'"?  What's the meaning 
of 'entry' and 'pos'?

Both structures need better padding and some documentation.

-- 
error compiling committee.c: too many arguments to function

Re: [PATCH 1/3] KVM: Ioctls for init MSI-X entry

[Sheng Yang]

On Wednesday 11 February 2009 20:44:57 Avi Kivity wrote:
> Sheng Yang wrote:
> > Introduce KVM_SET_MSIX_NR and KVM_SET_MSIX_ENTRY two ioctls.
> >
> > This two ioctls are used by userspace to specific guest device MSI-X
> > entry number and correlate MSI-X entry with GSI during the initialization
> > stage.
> >
> > MSI-X should be well initialzed before enabling.
> >
> > Don't support change MSI-X entry number for now.
> >
> > Signed-off-by: Sheng Yang <sheng@linux.intel.com>
> > ---
> >  include/linux/kvm.h      |   14 +++++++
> >  include/linux/kvm_host.h |    3 +
> >  virt/kvm/kvm_main.c      |   96
> > ++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 113
> > insertions(+), 0 deletions(-)
> >
> > diff --git a/include/linux/kvm.h b/include/linux/kvm.h
> > index c1425ab..5200768 100644
> > --- a/include/linux/kvm.h
> > +++ b/include/linux/kvm.h
> > @@ -474,6 +474,8 @@ struct kvm_irq_routing {
> >  #define KVM_ASSIGN_IRQ _IOR(KVMIO, 0x70, \
> >  			    struct kvm_assigned_irq)
> >  #define KVM_REINJECT_CONTROL      _IO(KVMIO, 0x71)
> > +#define KVM_SET_MSIX_NR _IOR(KVMIO, 0x72, struct kvm_assigned_msix_nr)
> > +#define KVM_SET_MSIX_ENTRY _IOR(KVMIO, 0x73, struct
> > kvm_assigned_msix_entry)
> >
> >  /*
> >   * ioctls for vcpu fds
> > @@ -594,4 +596,16 @@ struct kvm_assigned_irq {
> >  #define KVM_DEV_IRQ_ASSIGN_MSI_ACTION	KVM_DEV_IRQ_ASSIGN_ENABLE_MSI
> >  #define KVM_DEV_IRQ_ASSIGN_ENABLE_MSI	(1 << 0)
> >
> > +struct kvm_assigned_msix_nr {
> > +	__u32 assigned_dev_id;
> > +	__u16 entry_nr;
> > +};
> > +
> > +struct kvm_assigned_msix_entry {
> > +	__u32 assigned_dev_id;
> > +	__u32 gsi;
> > +	__u16 entry;
> > +	__u16 pos;
> > +}
>
> I'm guessing the intent here is "msi-x number 'pos' on the host assigned
> device will be injected to the guest as gsi 'gsi'"?  What's the meaning
> of 'entry' and 'pos'?

Sorry for the ambiguous. "entry" is the index in device MSI-X table(including 
empty entry); "pos" is without the empty entry, and should be less than the 
total valid entry numbers that table have. "pos" is mostly a assistant here, I 
think it can be estimated.
>
> Both structures need better padding and some documentation.

Of course. Would update the patches.

-- 
regards
Yang, Sheng

[PATCH 1/3] KVM: Ioctls for init MSI-X entry

[Sheng Yang]

Introduce KVM_SET_MSIX_NR and KVM_SET_MSIX_ENTRY two ioctls.

This two ioctls are used by userspace to specific guest device MSI-X entry
number and correlate MSI-X entry with GSI during the initialization stage.

MSI-X should be well initialzed before enabling.

Don't support change MSI-X entry number for now.

Signed-off-by: Sheng Yang <sheng@linux.intel.com>
---
 include/linux/kvm.h      |   15 +++++++
 include/linux/kvm_host.h |    3 +
 virt/kvm/kvm_main.c      |  101 ++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 119 insertions(+), 0 deletions(-)

diff --git a/include/linux/kvm.h b/include/linux/kvm.h
index 2163b3d..c76ea44 100644
--- a/include/linux/kvm.h
+++ b/include/linux/kvm.h
@@ -475,6 +475,8 @@ struct kvm_irq_routing {
 #define KVM_ASSIGN_IRQ _IOR(KVMIO, 0x70, \
 			    struct kvm_assigned_irq)
 #define KVM_REINJECT_CONTROL      _IO(KVMIO, 0x71)
+#define KVM_SET_MSIX_NR _IOR(KVMIO, 0x72, struct kvm_assigned_msix_nr)
+#define KVM_SET_MSIX_ENTRY _IOR(KVMIO, 0x73, struct kvm_assigned_msix_entry)
 
 /*
  * ioctls for vcpu fds
@@ -595,4 +597,17 @@ struct kvm_assigned_irq {
 #define KVM_DEV_IRQ_ASSIGN_MSI_ACTION	KVM_DEV_IRQ_ASSIGN_ENABLE_MSI
 #define KVM_DEV_IRQ_ASSIGN_ENABLE_MSI	(1 << 0)
 
+struct kvm_assigned_msix_nr {
+	__u32 assigned_dev_id;
+	__u16 entry_nr;
+	__u16 padding;
+};
+
+struct kvm_assigned_msix_entry {
+	__u32 assigned_dev_id;
+	__u32 gsi;
+	__u16 entry; /* The index of entry in the MSI-X table */
+	__u16 padding[3];
+};
+
 #endif
diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h
index 7c7096d..ac4d63c 100644
--- a/include/linux/kvm_host.h
+++ b/include/linux/kvm_host.h
@@ -326,9 +326,12 @@ struct kvm_assigned_dev_kernel {
 	int assigned_dev_id;
 	int host_busnr;
 	int host_devfn;
+	int entries_nr;
 	int host_irq;
 	bool host_irq_disabled;
+	struct msix_entry *host_msix_entries;
 	int guest_irq;
+	struct msix_entry *guest_msix_entries;
 #define KVM_ASSIGNED_DEV_GUEST_INTX	(1 << 0)
 #define KVM_ASSIGNED_DEV_GUEST_MSI	(1 << 1)
 #define KVM_ASSIGNED_DEV_HOST_INTX	(1 << 8)
diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
index 266bdaf..f395160 100644
--- a/virt/kvm/kvm_main.c
+++ b/virt/kvm/kvm_main.c
@@ -1593,6 +1593,85 @@ static int kvm_vcpu_ioctl_set_sigmask(struct kvm_vcpu *vcpu, sigset_t *sigset)
 	return 0;
 }
 
+static int kvm_vm_ioctl_set_msix_nr(struct kvm *kvm,
+				    struct kvm_assigned_msix_nr *entry_nr)
+{
+	int r = 0;
+	struct kvm_assigned_dev_kernel *adev;
+
+	mutex_lock(&kvm->lock);
+
+	adev = kvm_find_assigned_dev(&kvm->arch.assigned_dev_head,
+				      entry_nr->assigned_dev_id);
+	if (!adev) {
+		r = -EINVAL;
+		goto msix_nr_out;
+	}
+
+	if (adev->entries_nr == 0) {
+		adev->entries_nr = entry_nr->entry_nr;
+		if (adev->entries_nr == 0)
+			goto msix_nr_out;
+
+		adev->host_msix_entries = kzalloc(sizeof(struct msix_entry) *
+						entry_nr->entry_nr,
+						GFP_KERNEL);
+		if (!adev->host_msix_entries) {
+			printk(KERN_ERR "no memory for host msix entries!\n");
+			r = -ENOMEM;
+			goto msix_nr_out;
+		}
+		adev->guest_msix_entries = kzalloc(sizeof(struct msix_entry) *
+						entry_nr->entry_nr,
+						GFP_KERNEL);
+		if (!adev->guest_msix_entries) {
+			printk(KERN_ERR "no memory for host msix entries!\n");
+			r = -ENOMEM;
+			goto msix_nr_out;
+		}
+	} else
+		printk(KERN_WARNING "kvm: not allow recall set msix nr!\n");
+msix_nr_out:
+	mutex_unlock(&kvm->lock);
+	return r;
+}
+
+static int kvm_vm_ioctl_set_msix_entry(struct kvm *kvm,
+				       struct kvm_assigned_msix_entry *entry)
+{
+	int r = 0, i;
+	struct kvm_assigned_dev_kernel *adev;
+
+	mutex_lock(&kvm->lock);
+
+	adev = kvm_find_assigned_dev(&kvm->arch.assigned_dev_head,
+				      entry->assigned_dev_id);
+
+	if (!adev) {
+		r = -EINVAL;
+		goto msix_entry_out;
+	}
+
+	for (i = 0; i < adev->entries_nr; i++)
+		if (adev->guest_msix_entries[i].vector == 0 ||
+		    adev->guest_msix_entries[i].entry == entry->entry) {
+			adev->guest_msix_entries[i].entry = entry->entry;
+			adev->guest_msix_entries[i].vector = entry->gsi;
+			adev->host_msix_entries[i].entry = entry->entry;
+			break;
+		}
+	if (i == adev->entries_nr) {
+		printk(KERN_ERR "kvm: Too much entries for MSI-X!\n");
+		r = -ENOSPC;
+		goto msix_entry_out;
+	}
+
+msix_entry_out:
+	mutex_unlock(&kvm->lock);
+
+	return r;
+}
+
 static long kvm_vcpu_ioctl(struct file *filp,
 			   unsigned int ioctl, unsigned long arg)
 {
@@ -1917,7 +1996,29 @@ static long kvm_vm_ioctl(struct file *filp,
 		vfree(entries);
 		break;
 	}
+#ifdef KVM_CAP_DEVICE_MSIX
+	case KVM_SET_MSIX_NR: {
+		struct kvm_assigned_msix_nr entry_nr;
+		r = -EFAULT;
+		if (copy_from_user(&entry_nr, argp, sizeof entry_nr))
+			goto out;
+		r = kvm_vm_ioctl_set_msix_nr(kvm, &entry_nr);
+		if (r)
+			goto out;
+		break;
+	}
+	case KVM_SET_MSIX_ENTRY: {
+		struct kvm_assigned_msix_entry entry;
+		r = -EFAULT;
+		if (copy_from_user(&entry, argp, sizeof entry))
+			goto out;
+		r = kvm_vm_ioctl_set_msix_entry(kvm, &entry);
+		if (r)
+			goto out;
+		break;
+	}
 #endif
+#endif /* KVM_CAP_IRQ_ROUTING */
 	default:
 		r = kvm_arch_vm_ioctl(filp, ioctl, arg);
 	}
-- 
1.5.4.5

Re: [PATCH 1/3] KVM: Ioctls for init MSI-X entry

[Marcelo Tosatti]

On Thu, Feb 12, 2009 at 06:07:48PM +0800, Sheng Yang wrote:
> Introduce KVM_SET_MSIX_NR and KVM_SET_MSIX_ENTRY two ioctls.
> 
> This two ioctls are used by userspace to specific guest device MSI-X entry
> number and correlate MSI-X entry with GSI during the initialization stage.
> 
> MSI-X should be well initialzed before enabling.
> 
> Don't support change MSI-X entry number for now.
> 
> Signed-off-by: Sheng Yang <sheng@linux.intel.com>

> +		adev->host_msix_entries = kzalloc(sizeof(struct msix_entry) *
> +						entry_nr->entry_nr,
> +						GFP_KERNEL);

> +		if (!adev->host_msix_entries) {

Please verify its within a sane limit. Like the msr entry ioctl code.
Also free host_msix_entries if guest_msix_entries allocation fails.

Otherwise seems OK.

[PATCH 1/3] KVM: Ioctls for init MSI-X entry

[Sheng Yang]

Introduce KVM_SET_MSIX_NR and KVM_SET_MSIX_ENTRY two ioctls.

This two ioctls are used by userspace to specific guest device MSI-X entry
number and correlate MSI-X entry with GSI during the initialization stage.

MSI-X should be well initialzed before enabling.

Don't support change MSI-X entry number for now.

Signed-off-by: Sheng Yang <sheng@linux.intel.com>
---
 include/linux/kvm.h      |   16 +++++++
 include/linux/kvm_host.h |    3 +
 virt/kvm/kvm_main.c      |  103 ++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 122 insertions(+), 0 deletions(-)

diff --git a/include/linux/kvm.h b/include/linux/kvm.h
index 2163b3d..a2dfbe0 100644
--- a/include/linux/kvm.h
+++ b/include/linux/kvm.h
@@ -475,6 +475,8 @@ struct kvm_irq_routing {
 #define KVM_ASSIGN_IRQ _IOR(KVMIO, 0x70, \
 			    struct kvm_assigned_irq)
 #define KVM_REINJECT_CONTROL      _IO(KVMIO, 0x71)
+#define KVM_SET_MSIX_NR _IOR(KVMIO, 0x72, struct kvm_assigned_msix_nr)
+#define KVM_SET_MSIX_ENTRY _IOR(KVMIO, 0x73, struct kvm_assigned_msix_entry)
 
 /*
  * ioctls for vcpu fds
@@ -595,4 +597,18 @@ struct kvm_assigned_irq {
 #define KVM_DEV_IRQ_ASSIGN_MSI_ACTION	KVM_DEV_IRQ_ASSIGN_ENABLE_MSI
 #define KVM_DEV_IRQ_ASSIGN_ENABLE_MSI	(1 << 0)
 
+struct kvm_assigned_msix_nr {
+	__u32 assigned_dev_id;
+	__u16 entry_nr;
+	__u16 padding;
+};
+
+#define KVM_MAX_MSIX_PER_DEV		512
+struct kvm_assigned_msix_entry {
+	__u32 assigned_dev_id;
+	__u32 gsi;
+	__u16 entry; /* The index of entry in the MSI-X table */
+	__u16 padding[3];
+};
+
 #endif
diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h
index 7c7096d..ac4d63c 100644
--- a/include/linux/kvm_host.h
+++ b/include/linux/kvm_host.h
@@ -326,9 +326,12 @@ struct kvm_assigned_dev_kernel {
 	int assigned_dev_id;
 	int host_busnr;
 	int host_devfn;
+	int entries_nr;
 	int host_irq;
 	bool host_irq_disabled;
+	struct msix_entry *host_msix_entries;
 	int guest_irq;
+	struct msix_entry *guest_msix_entries;
 #define KVM_ASSIGNED_DEV_GUEST_INTX	(1 << 0)
 #define KVM_ASSIGNED_DEV_GUEST_MSI	(1 << 1)
 #define KVM_ASSIGNED_DEV_HOST_INTX	(1 << 8)
diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
index 266bdaf..08d5372 100644
--- a/virt/kvm/kvm_main.c
+++ b/virt/kvm/kvm_main.c
@@ -1593,6 +1593,87 @@ static int kvm_vcpu_ioctl_set_sigmask(struct kvm_vcpu *vcpu, sigset_t *sigset)
 	return 0;
 }
 
+static int kvm_vm_ioctl_set_msix_nr(struct kvm *kvm,
+				    struct kvm_assigned_msix_nr *entry_nr)
+{
+	int r = 0;
+	struct kvm_assigned_dev_kernel *adev;
+
+	mutex_lock(&kvm->lock);
+
+	adev = kvm_find_assigned_dev(&kvm->arch.assigned_dev_head,
+				      entry_nr->assigned_dev_id);
+	if (!adev) {
+		r = -EINVAL;
+		goto msix_nr_out;
+	}
+
+	if (adev->entries_nr == 0) {
+		adev->entries_nr = entry_nr->entry_nr;
+		if (adev->entries_nr == 0 ||
+		    adev->entries_nr >= KVM_MAX_MSIX_PER_DEV)
+			goto msix_nr_out;
+
+		adev->host_msix_entries = kzalloc(sizeof(struct msix_entry) *
+						entry_nr->entry_nr,
+						GFP_KERNEL);
+		if (!adev->host_msix_entries) {
+			printk(KERN_ERR "no memory for host msix entries!\n");
+			r = -ENOMEM;
+			goto msix_nr_out;
+		}
+		adev->guest_msix_entries = kzalloc(sizeof(struct msix_entry) *
+						entry_nr->entry_nr,
+						GFP_KERNEL);
+		if (!adev->guest_msix_entries) {
+			printk(KERN_ERR "no memory for host msix entries!\n");
+			free(adev->host_msix_entries);
+			r = -ENOMEM;
+			goto msix_nr_out;
+		}
+	} else
+		printk(KERN_WARNING "kvm: not allow recall set msix nr!\n");
+msix_nr_out:
+	mutex_unlock(&kvm->lock);
+	return r;
+}
+
+static int kvm_vm_ioctl_set_msix_entry(struct kvm *kvm,
+				       struct kvm_assigned_msix_entry *entry)
+{
+	int r = 0, i;
+	struct kvm_assigned_dev_kernel *adev;
+
+	mutex_lock(&kvm->lock);
+
+	adev = kvm_find_assigned_dev(&kvm->arch.assigned_dev_head,
+				      entry->assigned_dev_id);
+
+	if (!adev) {
+		r = -EINVAL;
+		goto msix_entry_out;
+	}
+
+	for (i = 0; i < adev->entries_nr; i++)
+		if (adev->guest_msix_entries[i].vector == 0 ||
+		    adev->guest_msix_entries[i].entry == entry->entry) {
+			adev->guest_msix_entries[i].entry = entry->entry;
+			adev->guest_msix_entries[i].vector = entry->gsi;
+			adev->host_msix_entries[i].entry = entry->entry;
+			break;
+		}
+	if (i == adev->entries_nr) {
+		printk(KERN_ERR "kvm: Too much entries for MSI-X!\n");
+		r = -ENOSPC;
+		goto msix_entry_out;
+	}
+
+msix_entry_out:
+	mutex_unlock(&kvm->lock);
+
+	return r;
+}
+
 static long kvm_vcpu_ioctl(struct file *filp,
 			   unsigned int ioctl, unsigned long arg)
 {
@@ -1917,7 +1998,29 @@ static long kvm_vm_ioctl(struct file *filp,
 		vfree(entries);
 		break;
 	}
+#ifdef KVM_CAP_DEVICE_MSIX
+	case KVM_SET_MSIX_NR: {
+		struct kvm_assigned_msix_nr entry_nr;
+		r = -EFAULT;
+		if (copy_from_user(&entry_nr, argp, sizeof entry_nr))
+			goto out;
+		r = kvm_vm_ioctl_set_msix_nr(kvm, &entry_nr);
+		if (r)
+			goto out;
+		break;
+	}
+	case KVM_SET_MSIX_ENTRY: {
+		struct kvm_assigned_msix_entry entry;
+		r = -EFAULT;
+		if (copy_from_user(&entry, argp, sizeof entry))
+			goto out;
+		r = kvm_vm_ioctl_set_msix_entry(kvm, &entry);
+		if (r)
+			goto out;
+		break;
+	}
 #endif
+#endif /* KVM_CAP_IRQ_ROUTING */
 	default:
 		r = kvm_arch_vm_ioctl(filp, ioctl, arg);
 	}
-- 
1.5.4.5

[PATCH 1/3] KVM: Ioctls for init MSI-X entry

[Sheng Yang]

(oops... fix a typo)

Introduce KVM_SET_MSIX_NR and KVM_SET_MSIX_ENTRY two ioctls.

This two ioctls are used by userspace to specific guest device MSI-X entry
number and correlate MSI-X entry with GSI during the initialization stage.

MSI-X should be well initialzed before enabling.

Don't support change MSI-X entry number for now.

Signed-off-by: Sheng Yang <sheng@linux.intel.com>
---
 include/linux/kvm.h      |   16 +++++++
 include/linux/kvm_host.h |    3 +
 virt/kvm/kvm_main.c      |  103 ++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 122 insertions(+), 0 deletions(-)

diff --git a/include/linux/kvm.h b/include/linux/kvm.h
index 2163b3d..a2dfbe0 100644
--- a/include/linux/kvm.h
+++ b/include/linux/kvm.h
@@ -475,6 +475,8 @@ struct kvm_irq_routing {
 #define KVM_ASSIGN_IRQ _IOR(KVMIO, 0x70, \
 			    struct kvm_assigned_irq)
 #define KVM_REINJECT_CONTROL      _IO(KVMIO, 0x71)
+#define KVM_SET_MSIX_NR _IOR(KVMIO, 0x72, struct kvm_assigned_msix_nr)
+#define KVM_SET_MSIX_ENTRY _IOR(KVMIO, 0x73, struct kvm_assigned_msix_entry)
 
 /*
  * ioctls for vcpu fds
@@ -595,4 +597,18 @@ struct kvm_assigned_irq {
 #define KVM_DEV_IRQ_ASSIGN_MSI_ACTION	KVM_DEV_IRQ_ASSIGN_ENABLE_MSI
 #define KVM_DEV_IRQ_ASSIGN_ENABLE_MSI	(1 << 0)
 
+struct kvm_assigned_msix_nr {
+	__u32 assigned_dev_id;
+	__u16 entry_nr;
+	__u16 padding;
+};
+
+#define KVM_MAX_MSIX_PER_DEV		512
+struct kvm_assigned_msix_entry {
+	__u32 assigned_dev_id;
+	__u32 gsi;
+	__u16 entry; /* The index of entry in the MSI-X table */
+	__u16 padding[3];
+};
+
 #endif
diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h
index 7c7096d..ac4d63c 100644
--- a/include/linux/kvm_host.h
+++ b/include/linux/kvm_host.h
@@ -326,9 +326,12 @@ struct kvm_assigned_dev_kernel {
 	int assigned_dev_id;
 	int host_busnr;
 	int host_devfn;
+	int entries_nr;
 	int host_irq;
 	bool host_irq_disabled;
+	struct msix_entry *host_msix_entries;
 	int guest_irq;
+	struct msix_entry *guest_msix_entries;
 #define KVM_ASSIGNED_DEV_GUEST_INTX	(1 << 0)
 #define KVM_ASSIGNED_DEV_GUEST_MSI	(1 << 1)
 #define KVM_ASSIGNED_DEV_HOST_INTX	(1 << 8)
diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
index 266bdaf..bb4aa73 100644
--- a/virt/kvm/kvm_main.c
+++ b/virt/kvm/kvm_main.c
@@ -1593,6 +1593,87 @@ static int kvm_vcpu_ioctl_set_sigmask(struct kvm_vcpu *vcpu, sigset_t *sigset)
 	return 0;
 }
 
+static int kvm_vm_ioctl_set_msix_nr(struct kvm *kvm,
+				    struct kvm_assigned_msix_nr *entry_nr)
+{
+	int r = 0;
+	struct kvm_assigned_dev_kernel *adev;
+
+	mutex_lock(&kvm->lock);
+
+	adev = kvm_find_assigned_dev(&kvm->arch.assigned_dev_head,
+				      entry_nr->assigned_dev_id);
+	if (!adev) {
+		r = -EINVAL;
+		goto msix_nr_out;
+	}
+
+	if (adev->entries_nr == 0) {
+		adev->entries_nr = entry_nr->entry_nr;
+		if (adev->entries_nr == 0 ||
+		    adev->entries_nr >= KVM_MAX_MSIX_PER_DEV)
+			goto msix_nr_out;
+
+		adev->host_msix_entries = kzalloc(sizeof(struct msix_entry) *
+						entry_nr->entry_nr,
+						GFP_KERNEL);
+		if (!adev->host_msix_entries) {
+			printk(KERN_ERR "no memory for host msix entries!\n");
+			r = -ENOMEM;
+			goto msix_nr_out;
+		}
+		adev->guest_msix_entries = kzalloc(sizeof(struct msix_entry) *
+						entry_nr->entry_nr,
+						GFP_KERNEL);
+		if (!adev->guest_msix_entries) {
+			printk(KERN_ERR "no memory for host msix entries!\n");
+			kfree(adev->host_msix_entries);
+			r = -ENOMEM;
+			goto msix_nr_out;
+		}
+	} else
+		printk(KERN_WARNING "kvm: not allow recall set msix nr!\n");
+msix_nr_out:
+	mutex_unlock(&kvm->lock);
+	return r;
+}
+
+static int kvm_vm_ioctl_set_msix_entry(struct kvm *kvm,
+				       struct kvm_assigned_msix_entry *entry)
+{
+	int r = 0, i;
+	struct kvm_assigned_dev_kernel *adev;
+
+	mutex_lock(&kvm->lock);
+
+	adev = kvm_find_assigned_dev(&kvm->arch.assigned_dev_head,
+				      entry->assigned_dev_id);
+
+	if (!adev) {
+		r = -EINVAL;
+		goto msix_entry_out;
+	}
+
+	for (i = 0; i < adev->entries_nr; i++)
+		if (adev->guest_msix_entries[i].vector == 0 ||
+		    adev->guest_msix_entries[i].entry == entry->entry) {
+			adev->guest_msix_entries[i].entry = entry->entry;
+			adev->guest_msix_entries[i].vector = entry->gsi;
+			adev->host_msix_entries[i].entry = entry->entry;
+			break;
+		}
+	if (i == adev->entries_nr) {
+		printk(KERN_ERR "kvm: Too much entries for MSI-X!\n");
+		r = -ENOSPC;
+		goto msix_entry_out;
+	}
+
+msix_entry_out:
+	mutex_unlock(&kvm->lock);
+
+	return r;
+}
+
 static long kvm_vcpu_ioctl(struct file *filp,
 			   unsigned int ioctl, unsigned long arg)
 {
@@ -1917,7 +1998,29 @@ static long kvm_vm_ioctl(struct file *filp,
 		vfree(entries);
 		break;
 	}
+#ifdef KVM_CAP_DEVICE_MSIX
+	case KVM_SET_MSIX_NR: {
+		struct kvm_assigned_msix_nr entry_nr;
+		r = -EFAULT;
+		if (copy_from_user(&entry_nr, argp, sizeof entry_nr))
+			goto out;
+		r = kvm_vm_ioctl_set_msix_nr(kvm, &entry_nr);
+		if (r)
+			goto out;
+		break;
+	}
+	case KVM_SET_MSIX_ENTRY: {
+		struct kvm_assigned_msix_entry entry;
+		r = -EFAULT;
+		if (copy_from_user(&entry, argp, sizeof entry))
+			goto out;
+		r = kvm_vm_ioctl_set_msix_entry(kvm, &entry);
+		if (r)
+			goto out;
+		break;
+	}
 #endif
+#endif /* KVM_CAP_IRQ_ROUTING */
 	default:
 		r = kvm_arch_vm_ioctl(filp, ioctl, arg);
 	}
-- 
1.5.4.5

Re: [PATCH 1/3] KVM: Ioctls for init MSI-X entry

[Marcelo Tosatti]

On Mon, Feb 16, 2009 at 01:49:29PM +0800, Sheng Yang wrote:
> (oops... fix a typo)
> 
> Introduce KVM_SET_MSIX_NR and KVM_SET_MSIX_ENTRY two ioctls.
> 
> This two ioctls are used by userspace to specific guest device MSI-X entry
> number and correlate MSI-X entry with GSI during the initialization stage.
> 
> MSI-X should be well initialzed before enabling.
> 
> Don't support change MSI-X entry number for now.
> 
> Signed-off-by: Sheng Yang <sheng@linux.intel.com>
> ---
>  include/linux/kvm.h      |   16 +++++++
>  include/linux/kvm_host.h |    3 +
>  virt/kvm/kvm_main.c      |  103 ++++++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 122 insertions(+), 0 deletions(-)
> 
> diff --git a/include/linux/kvm.h b/include/linux/kvm.h
> index 2163b3d..a2dfbe0 100644
> --- a/include/linux/kvm.h
> +++ b/include/linux/kvm.h
> @@ -475,6 +475,8 @@ struct kvm_irq_routing {
>  #define KVM_ASSIGN_IRQ _IOR(KVMIO, 0x70, \
>  			    struct kvm_assigned_irq)
>  #define KVM_REINJECT_CONTROL      _IO(KVMIO, 0x71)
> +#define KVM_SET_MSIX_NR _IOR(KVMIO, 0x72, struct kvm_assigned_msix_nr)
> +#define KVM_SET_MSIX_ENTRY _IOR(KVMIO, 0x73, struct kvm_assigned_msix_entry)
>  
>  /*
>   * ioctls for vcpu fds
> @@ -595,4 +597,18 @@ struct kvm_assigned_irq {
>  #define KVM_DEV_IRQ_ASSIGN_MSI_ACTION	KVM_DEV_IRQ_ASSIGN_ENABLE_MSI
>  #define KVM_DEV_IRQ_ASSIGN_ENABLE_MSI	(1 << 0)
>  
> +struct kvm_assigned_msix_nr {
> +	__u32 assigned_dev_id;
> +	__u16 entry_nr;
> +	__u16 padding;
> +};
> +
> +#define KVM_MAX_MSIX_PER_DEV		512
> +struct kvm_assigned_msix_entry {
> +	__u32 assigned_dev_id;
> +	__u32 gsi;
> +	__u16 entry; /* The index of entry in the MSI-X table */
> +	__u16 padding[3];
> +};
> +
>  #endif
> diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h
> index 7c7096d..ac4d63c 100644
> --- a/include/linux/kvm_host.h
> +++ b/include/linux/kvm_host.h
> @@ -326,9 +326,12 @@ struct kvm_assigned_dev_kernel {
>  	int assigned_dev_id;
>  	int host_busnr;
>  	int host_devfn;
> +	int entries_nr;
>  	int host_irq;
>  	bool host_irq_disabled;
> +	struct msix_entry *host_msix_entries;
>  	int guest_irq;
> +	struct msix_entry *guest_msix_entries;
>  #define KVM_ASSIGNED_DEV_GUEST_INTX	(1 << 0)
>  #define KVM_ASSIGNED_DEV_GUEST_MSI	(1 << 1)
>  #define KVM_ASSIGNED_DEV_HOST_INTX	(1 << 8)
> diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
> index 266bdaf..bb4aa73 100644
> --- a/virt/kvm/kvm_main.c
> +++ b/virt/kvm/kvm_main.c
> @@ -1593,6 +1593,87 @@ static int kvm_vcpu_ioctl_set_sigmask(struct kvm_vcpu *vcpu, sigset_t *sigset)
>  	return 0;
>  }
>  
> +static int kvm_vm_ioctl_set_msix_nr(struct kvm *kvm,
> +				    struct kvm_assigned_msix_nr *entry_nr)
> +{
> +	int r = 0;
> +	struct kvm_assigned_dev_kernel *adev;
> +
> +	mutex_lock(&kvm->lock);
> +
> +	adev = kvm_find_assigned_dev(&kvm->arch.assigned_dev_head,
> +				      entry_nr->assigned_dev_id);
> +	if (!adev) {
> +		r = -EINVAL;
> +		goto msix_nr_out;
> +	}
> +
> +	if (adev->entries_nr == 0) {
> +		adev->entries_nr = entry_nr->entry_nr;
> +		if (adev->entries_nr == 0 ||

                                    <= 0 ? 

> +		    adev->entries_nr >= KVM_MAX_MSIX_PER_DEV)

Otherwise looks OK, thanks.

Re: [PATCH 1/3] KVM: Ioctls for init MSI-X entry

[Sheng Yang]

On Tuesday 17 February 2009 07:23:23 Marcelo Tosatti wrote:
> On Mon, Feb 16, 2009 at 01:49:29PM +0800, Sheng Yang wrote:
> > (oops... fix a typo)
> >
> > Introduce KVM_SET_MSIX_NR and KVM_SET_MSIX_ENTRY two ioctls.
> >
> > This two ioctls are used by userspace to specific guest device MSI-X
> > entry number and correlate MSI-X entry with GSI during the initialization
> > stage.
> >
> > MSI-X should be well initialzed before enabling.
> >
> > Don't support change MSI-X entry number for now.
> >
> > Signed-off-by: Sheng Yang <sheng@linux.intel.com>
> > ---
> >  include/linux/kvm.h      |   16 +++++++
> >  include/linux/kvm_host.h |    3 +
> >  virt/kvm/kvm_main.c      |  103
> > ++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 122
> > insertions(+), 0 deletions(-)
> >
> > diff --git a/include/linux/kvm.h b/include/linux/kvm.h
> > index 2163b3d..a2dfbe0 100644
> > --- a/include/linux/kvm.h
> > +++ b/include/linux/kvm.h
> > @@ -475,6 +475,8 @@ struct kvm_irq_routing {
> >  #define KVM_ASSIGN_IRQ _IOR(KVMIO, 0x70, \
> >  			    struct kvm_assigned_irq)
> >  #define KVM_REINJECT_CONTROL      _IO(KVMIO, 0x71)
> > +#define KVM_SET_MSIX_NR _IOR(KVMIO, 0x72, struct kvm_assigned_msix_nr)
> > +#define KVM_SET_MSIX_ENTRY _IOR(KVMIO, 0x73, struct
> > kvm_assigned_msix_entry)
> >
> >  /*
> >   * ioctls for vcpu fds
> > @@ -595,4 +597,18 @@ struct kvm_assigned_irq {
> >  #define KVM_DEV_IRQ_ASSIGN_MSI_ACTION	KVM_DEV_IRQ_ASSIGN_ENABLE_MSI
> >  #define KVM_DEV_IRQ_ASSIGN_ENABLE_MSI	(1 << 0)
> >
> > +struct kvm_assigned_msix_nr {
> > +	__u32 assigned_dev_id;
> > +	__u16 entry_nr;
> > +	__u16 padding;
> > +};
> > +
> > +#define KVM_MAX_MSIX_PER_DEV		512
> > +struct kvm_assigned_msix_entry {
> > +	__u32 assigned_dev_id;
> > +	__u32 gsi;
> > +	__u16 entry; /* The index of entry in the MSI-X table */
> > +	__u16 padding[3];
> > +};
> > +
> >  #endif
> > diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h
> > index 7c7096d..ac4d63c 100644
> > --- a/include/linux/kvm_host.h
> > +++ b/include/linux/kvm_host.h
> > @@ -326,9 +326,12 @@ struct kvm_assigned_dev_kernel {
> >  	int assigned_dev_id;
> >  	int host_busnr;
> >  	int host_devfn;
> > +	int entries_nr;
> >  	int host_irq;
> >  	bool host_irq_disabled;
> > +	struct msix_entry *host_msix_entries;
> >  	int guest_irq;
> > +	struct msix_entry *guest_msix_entries;
> >  #define KVM_ASSIGNED_DEV_GUEST_INTX	(1 << 0)
> >  #define KVM_ASSIGNED_DEV_GUEST_MSI	(1 << 1)
> >  #define KVM_ASSIGNED_DEV_HOST_INTX	(1 << 8)
> > diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
> > index 266bdaf..bb4aa73 100644
> > --- a/virt/kvm/kvm_main.c
> > +++ b/virt/kvm/kvm_main.c
> > @@ -1593,6 +1593,87 @@ static int kvm_vcpu_ioctl_set_sigmask(struct
> > kvm_vcpu *vcpu, sigset_t *sigset) return 0;
> >  }
> >
> > +static int kvm_vm_ioctl_set_msix_nr(struct kvm *kvm,
> > +				    struct kvm_assigned_msix_nr *entry_nr)
> > +{
> > +	int r = 0;
> > +	struct kvm_assigned_dev_kernel *adev;
> > +
> > +	mutex_lock(&kvm->lock);
> > +
> > +	adev = kvm_find_assigned_dev(&kvm->arch.assigned_dev_head,
> > +				      entry_nr->assigned_dev_id);
> > +	if (!adev) {
> > +		r = -EINVAL;
> > +		goto msix_nr_out;
> > +	}
> > +
> > +	if (adev->entries_nr == 0) {
> > +		adev->entries_nr = entry_nr->entry_nr;
> > +		if (adev->entries_nr == 0 ||
>
>                                     <= 0 ?

Oh, yeah, that's right. :)

I would change the type of adev->entries_nr to unsigned int.

-- 
regards
Yang, Sheng

>
> > +		    adev->entries_nr >= KVM_MAX_MSIX_PER_DEV)
>
> Otherwise looks OK, thanks.

[PATCH 1/3] KVM: Ioctls for init MSI-X entry

[Sheng Yang]

Introduce KVM_SET_MSIX_NR and KVM_SET_MSIX_ENTRY two ioctls.

This two ioctls are used by userspace to specific guest device MSI-X entry
number and correlate MSI-X entry with GSI during the initialization stage.

MSI-X should be well initialzed before enabling.

Don't support change MSI-X entry number for now.

Signed-off-by: Sheng Yang <sheng@linux.intel.com>
---
 include/linux/kvm.h      |   16 +++++++
 include/linux/kvm_host.h |    3 +
 virt/kvm/kvm_main.c      |  103 ++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 122 insertions(+), 0 deletions(-)

diff --git a/include/linux/kvm.h b/include/linux/kvm.h
index 2163b3d..a2dfbe0 100644
--- a/include/linux/kvm.h
+++ b/include/linux/kvm.h
@@ -475,6 +475,8 @@ struct kvm_irq_routing {
 #define KVM_ASSIGN_IRQ _IOR(KVMIO, 0x70, \
 			    struct kvm_assigned_irq)
 #define KVM_REINJECT_CONTROL      _IO(KVMIO, 0x71)
+#define KVM_SET_MSIX_NR _IOR(KVMIO, 0x72, struct kvm_assigned_msix_nr)
+#define KVM_SET_MSIX_ENTRY _IOR(KVMIO, 0x73, struct kvm_assigned_msix_entry)
 
 /*
  * ioctls for vcpu fds
@@ -595,4 +597,18 @@ struct kvm_assigned_irq {
 #define KVM_DEV_IRQ_ASSIGN_MSI_ACTION	KVM_DEV_IRQ_ASSIGN_ENABLE_MSI
 #define KVM_DEV_IRQ_ASSIGN_ENABLE_MSI	(1 << 0)
 
+struct kvm_assigned_msix_nr {
+	__u32 assigned_dev_id;
+	__u16 entry_nr;
+	__u16 padding;
+};
+
+#define KVM_MAX_MSIX_PER_DEV		512
+struct kvm_assigned_msix_entry {
+	__u32 assigned_dev_id;
+	__u32 gsi;
+	__u16 entry; /* The index of entry in the MSI-X table */
+	__u16 padding[3];
+};
+
 #endif
diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h
index 7c7096d..b105ada 100644
--- a/include/linux/kvm_host.h
+++ b/include/linux/kvm_host.h
@@ -326,9 +326,12 @@ struct kvm_assigned_dev_kernel {
 	int assigned_dev_id;
 	int host_busnr;
 	int host_devfn;
+	unsigned int entries_nr;
 	int host_irq;
 	bool host_irq_disabled;
+	struct msix_entry *host_msix_entries;
 	int guest_irq;
+	struct msix_entry *guest_msix_entries;
 #define KVM_ASSIGNED_DEV_GUEST_INTX	(1 << 0)
 #define KVM_ASSIGNED_DEV_GUEST_MSI	(1 << 1)
 #define KVM_ASSIGNED_DEV_HOST_INTX	(1 << 8)
diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
index 266bdaf..bb4aa73 100644
--- a/virt/kvm/kvm_main.c
+++ b/virt/kvm/kvm_main.c
@@ -1593,6 +1593,87 @@ static int kvm_vcpu_ioctl_set_sigmask(struct kvm_vcpu *vcpu, sigset_t *sigset)
 	return 0;
 }
 
+static int kvm_vm_ioctl_set_msix_nr(struct kvm *kvm,
+				    struct kvm_assigned_msix_nr *entry_nr)
+{
+	int r = 0;
+	struct kvm_assigned_dev_kernel *adev;
+
+	mutex_lock(&kvm->lock);
+
+	adev = kvm_find_assigned_dev(&kvm->arch.assigned_dev_head,
+				      entry_nr->assigned_dev_id);
+	if (!adev) {
+		r = -EINVAL;
+		goto msix_nr_out;
+	}
+
+	if (adev->entries_nr == 0) {
+		adev->entries_nr = entry_nr->entry_nr;
+		if (adev->entries_nr == 0 ||
+		    adev->entries_nr >= KVM_MAX_MSIX_PER_DEV)
+			goto msix_nr_out;
+
+		adev->host_msix_entries = kzalloc(sizeof(struct msix_entry) *
+						entry_nr->entry_nr,
+						GFP_KERNEL);
+		if (!adev->host_msix_entries) {
+			printk(KERN_ERR "no memory for host msix entries!\n");
+			r = -ENOMEM;
+			goto msix_nr_out;
+		}
+		adev->guest_msix_entries = kzalloc(sizeof(struct msix_entry) *
+						entry_nr->entry_nr,
+						GFP_KERNEL);
+		if (!adev->guest_msix_entries) {
+			printk(KERN_ERR "no memory for host msix entries!\n");
+			kfree(adev->host_msix_entries);
+			r = -ENOMEM;
+			goto msix_nr_out;
+		}
+	} else
+		printk(KERN_WARNING "kvm: not allow recall set msix nr!\n");
+msix_nr_out:
+	mutex_unlock(&kvm->lock);
+	return r;
+}
+
+static int kvm_vm_ioctl_set_msix_entry(struct kvm *kvm,
+				       struct kvm_assigned_msix_entry *entry)
+{
+	int r = 0, i;
+	struct kvm_assigned_dev_kernel *adev;
+
+	mutex_lock(&kvm->lock);
+
+	adev = kvm_find_assigned_dev(&kvm->arch.assigned_dev_head,
+				      entry->assigned_dev_id);
+
+	if (!adev) {
+		r = -EINVAL;
+		goto msix_entry_out;
+	}
+
+	for (i = 0; i < adev->entries_nr; i++)
+		if (adev->guest_msix_entries[i].vector == 0 ||
+		    adev->guest_msix_entries[i].entry == entry->entry) {
+			adev->guest_msix_entries[i].entry = entry->entry;
+			adev->guest_msix_entries[i].vector = entry->gsi;
+			adev->host_msix_entries[i].entry = entry->entry;
+			break;
+		}
+	if (i == adev->entries_nr) {
+		printk(KERN_ERR "kvm: Too much entries for MSI-X!\n");
+		r = -ENOSPC;
+		goto msix_entry_out;
+	}
+
+msix_entry_out:
+	mutex_unlock(&kvm->lock);
+
+	return r;
+}
+
 static long kvm_vcpu_ioctl(struct file *filp,
 			   unsigned int ioctl, unsigned long arg)
 {
@@ -1917,7 +1998,29 @@ static long kvm_vm_ioctl(struct file *filp,
 		vfree(entries);
 		break;
 	}
+#ifdef KVM_CAP_DEVICE_MSIX
+	case KVM_SET_MSIX_NR: {
+		struct kvm_assigned_msix_nr entry_nr;
+		r = -EFAULT;
+		if (copy_from_user(&entry_nr, argp, sizeof entry_nr))
+			goto out;
+		r = kvm_vm_ioctl_set_msix_nr(kvm, &entry_nr);
+		if (r)
+			goto out;
+		break;
+	}
+	case KVM_SET_MSIX_ENTRY: {
+		struct kvm_assigned_msix_entry entry;
+		r = -EFAULT;
+		if (copy_from_user(&entry, argp, sizeof entry))
+			goto out;
+		r = kvm_vm_ioctl_set_msix_entry(kvm, &entry);
+		if (r)
+			goto out;
+		break;
+	}
 #endif
+#endif /* KVM_CAP_IRQ_ROUTING */
 	default:
 		r = kvm_arch_vm_ioctl(filp, ioctl, arg);
 	}
-- 
1.5.4.5

[PATCH 2/3] KVM: Add gsi_msg_pending_bitmap for MSI-X

[Sheng Yang]

We have to handle more than one interrupt with one handler for MSI-X. So we
need a bitmap to track the triggered interrupts.

Signed-off-by: Sheng Yang <sheng@linux.intel.com>
---
 include/linux/kvm_host.h |    5 +-
 virt/kvm/kvm_main.c      |  103 ++++++++++++++++++++++++++++++++++++++++++++-
 2 files changed, 103 insertions(+), 5 deletions(-)

diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h
index a7d6123..c081867 100644
--- a/include/linux/kvm_host.h
+++ b/include/linux/kvm_host.h
@@ -144,6 +144,8 @@ struct kvm {
 #ifdef CONFIG_HAVE_KVM_IRQCHIP
 	struct list_head irq_routing; /* of kvm_kernel_irq_routing_entry */
 	struct hlist_head mask_notifier_list;
+#define KVM_MAX_IRQ_ROUTES 1024
+	DECLARE_BITMAP(irq_routes_pending_bitmap, KVM_MAX_IRQ_ROUTES);
 #endif
 
 #ifdef KVM_ARCH_WANT_MMU_NOTIFIER
@@ -335,6 +337,7 @@ struct kvm_assigned_dev_kernel {
 #define KVM_ASSIGNED_DEV_GUEST_MSI	(1 << 1)
 #define KVM_ASSIGNED_DEV_HOST_INTX	(1 << 8)
 #define KVM_ASSIGNED_DEV_HOST_MSI	(1 << 9)
+#define KVM_ASSIGNED_DEV_MSIX		((1 << 2) | (1 << 10))
 	unsigned long irq_requested_type;
 	int irq_source_id;
 	int flags;
@@ -502,8 +505,6 @@ static inline int mmu_notifier_retry(struct kvm_vcpu *vcpu, unsigned long mmu_se
 
 #ifdef CONFIG_HAVE_KVM_IRQCHIP
 
-#define KVM_MAX_IRQ_ROUTES 1024
-
 int kvm_setup_default_irq_routing(struct kvm *kvm);
 int kvm_set_irq_routing(struct kvm *kvm,
 			const struct kvm_irq_routing_entry *entries,
diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
index ea96690..961603f 100644
--- a/virt/kvm/kvm_main.c
+++ b/virt/kvm/kvm_main.c
@@ -95,25 +95,113 @@ static struct kvm_assigned_dev_kernel *kvm_find_assigned_dev(struct list_head *h
 	return NULL;
 }
 
+static int find_host_irq_from_gsi(struct kvm_assigned_dev_kernel *assigned_dev,
+				  u32 gsi)
+{
+	int i, entry, irq;
+	struct msix_entry *host_msix_entries, *guest_msix_entries;
+
+	host_msix_entries = assigned_dev->host_msix_entries;
+	guest_msix_entries = assigned_dev->guest_msix_entries;
+
+	entry = -1;
+	irq = 0;
+	for (i = 0; i < assigned_dev->entries_nr; i++)
+		if (gsi == (guest_msix_entries + i)->vector) {
+			entry = (guest_msix_entries + i)->entry;
+			break;
+		}
+	if (entry < 0) {
+		printk(KERN_WARNING "Fail to find correlated MSI-X entry!\n");
+		return 0;
+	}
+	for (i = 0; i < assigned_dev->entries_nr; i++)
+		if (entry == (host_msix_entries + i)->entry) {
+			irq = (host_msix_entries + i)->vector;
+			break;
+		}
+	if (irq == 0) {
+		printk(KERN_WARNING "Fail to find correlated MSI-X irq!\n");
+		return 0;
+	}
+
+	return irq;
+}
+
+static u32 find_gsi_from_host_irq(struct kvm_assigned_dev_kernel *assigned_dev,
+				  int irq)
+{
+	int i, entry;
+	u32 gsi;
+	struct msix_entry *host_msix_entries, *guest_msix_entries;
+
+	host_msix_entries = assigned_dev->host_msix_entries;
+	guest_msix_entries = assigned_dev->guest_msix_entries;
+
+	entry = -1;
+	gsi = 0;
+	for (i = 0; i < assigned_dev->entries_nr; i++)
+		if (irq == (host_msix_entries + i)->vector) {
+			entry = (host_msix_entries + i)->entry;
+			break;
+		}
+	if (entry < 0) {
+		printk(KERN_WARNING "Fail to find correlated MSI-X entry!\n");
+		return 0;
+	}
+	for (i = 0; i < assigned_dev->entries_nr; i++)
+		if (entry == (guest_msix_entries + i)->entry) {
+			gsi = (guest_msix_entries + i)->vector;
+			break;
+		}
+	if (gsi == 0) {
+		printk(KERN_WARNING "Fail to find correlated MSI-X gsi!\n");
+		return 0;
+	}
+
+	return gsi;
+}
+
 static void kvm_assigned_dev_interrupt_work_handler(struct work_struct *work)
 {
 	struct kvm_assigned_dev_kernel *assigned_dev;
+	struct kvm *kvm;
+	u32 gsi;
+	int irq;
 
 	assigned_dev = container_of(work, struct kvm_assigned_dev_kernel,
 				    interrupt_work);
+	kvm = assigned_dev->kvm;
 
 	/* This is taken to safely inject irq inside the guest. When
 	 * the interrupt injection (or the ioapic code) uses a
 	 * finer-grained lock, update this
 	 */
-	mutex_lock(&assigned_dev->kvm->lock);
-	kvm_set_irq(assigned_dev->kvm, assigned_dev->irq_source_id,
-		    assigned_dev->guest_irq, 1);
+	mutex_lock(&kvm->lock);
+handle_irq:
+	if (assigned_dev->irq_requested_type & KVM_ASSIGNED_DEV_MSIX) {
+		gsi = find_first_bit(kvm->irq_routes_pending_bitmap,
+				     KVM_MAX_IRQ_ROUTES);
+		BUG_ON(gsi >= KVM_MAX_IRQ_ROUTES);
+		clear_bit(gsi, kvm->irq_routes_pending_bitmap);
+	} else
+		gsi = assigned_dev->guest_irq;
+
+	kvm_set_irq(assigned_dev->kvm, assigned_dev->irq_source_id, gsi, 1);
 
 	if (assigned_dev->irq_requested_type & KVM_ASSIGNED_DEV_GUEST_MSI) {
 		enable_irq(assigned_dev->host_irq);
 		assigned_dev->host_irq_disabled = false;
+	} else if (assigned_dev->irq_requested_type & KVM_ASSIGNED_DEV_MSIX) {
+		irq = find_host_irq_from_gsi(assigned_dev, gsi);
+		enable_irq(irq);
+		assigned_dev->host_irq_disabled = false;
+		gsi = find_first_bit(kvm->irq_routes_pending_bitmap,
+				KVM_MAX_IRQ_ROUTES);
+		if (gsi < KVM_MAX_IRQ_ROUTES)
+			goto handle_irq;
 	}
+
 	mutex_unlock(&assigned_dev->kvm->lock);
 }
 
@@ -121,6 +209,15 @@ static irqreturn_t kvm_assigned_dev_intr(int irq, void *dev_id)
 {
 	struct kvm_assigned_dev_kernel *assigned_dev =
 		(struct kvm_assigned_dev_kernel *) dev_id;
+	struct kvm *kvm = assigned_dev->kvm;
+
+	if (assigned_dev->irq_requested_type == KVM_ASSIGNED_DEV_MSIX) {
+		u32 gsi;
+		gsi = find_gsi_from_host_irq(assigned_dev, irq);
+		if (gsi == 0)
+			return IRQ_HANDLED;
+		set_bit(gsi, kvm->irq_routes_pending_bitmap);
+	}
 
 	schedule_work(&assigned_dev->interrupt_work);
 
-- 
1.5.4.5

Re: [PATCH 2/3] KVM: Add gsi_msg_pending_bitmap for MSI-X

[Marcelo Tosatti]

Hi Sheng,

On Wed, Feb 11, 2009 at 04:08:50PM +0800, Sheng Yang wrote:
> We have to handle more than one interrupt with one handler for MSI-X. So we
> need a bitmap to track the triggered interrupts.
> 
> Signed-off-by: Sheng Yang <sheng@linux.intel.com>
> ---
>  include/linux/kvm_host.h |    5 +-
>  virt/kvm/kvm_main.c      |  103 ++++++++++++++++++++++++++++++++++++++++++++-
>  2 files changed, 103 insertions(+), 5 deletions(-)
> 

> @@ -335,6 +337,7 @@ struct kvm_assigned_dev_kernel {
>  #define KVM_ASSIGNED_DEV_GUEST_MSI	(1 << 1)
>  #define KVM_ASSIGNED_DEV_HOST_INTX	(1 << 8)
>  #define KVM_ASSIGNED_DEV_HOST_MSI	(1 << 9)
> +#define KVM_ASSIGNED_DEV_MSIX		((1 << 2) | (1 << 10))

Can you explain the usage of the two bits?

> diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
> index ea96690..961603f 100644
> --- a/virt/kvm/kvm_main.c
> +++ b/virt/kvm/kvm_main.c
> @@ -95,25 +95,113 @@ static struct kvm_assigned_dev_kernel *kvm_find_assigned_dev(struct list_head *h
>  	return NULL;
>  }
>  
> +static int find_host_irq_from_gsi(struct kvm_assigned_dev_kernel *assigned_dev,
> +				  u32 gsi)
> +{
> +	int i, entry, irq;
> +	struct msix_entry *host_msix_entries, *guest_msix_entries;
> +
> +	host_msix_entries = assigned_dev->host_msix_entries;
> +	guest_msix_entries = assigned_dev->guest_msix_entries;
> +
> +	entry = -1;
> +	irq = 0;
> +	for (i = 0; i < assigned_dev->entries_nr; i++)
> +		if (gsi == (guest_msix_entries + i)->vector) {
> +			entry = (guest_msix_entries + i)->entry;
> +			break;
> +		}
> +	if (entry < 0) {
> +		printk(KERN_WARNING "Fail to find correlated MSI-X entry!\n");
> +		return 0;
> +	}
> +	for (i = 0; i < assigned_dev->entries_nr; i++)
> +		if (entry == (host_msix_entries + i)->entry) {
> +			irq = (host_msix_entries + i)->vector;
> +			break;
> +		}
> +	if (irq == 0) {
> +		printk(KERN_WARNING "Fail to find correlated MSI-X irq!\n");
> +		return 0;
> +	}

Can drop the printk's (also from find_gsi_from_host_irq).

>  	struct kvm_assigned_dev_kernel *assigned_dev;
> +	struct kvm *kvm;
> +	u32 gsi;
> +	int irq;
>  
>  	assigned_dev = container_of(work, struct kvm_assigned_dev_kernel,
>  				    interrupt_work);
> +	kvm = assigned_dev->kvm;
>  
>  	/* This is taken to safely inject irq inside the guest. When
>  	 * the interrupt injection (or the ioapic code) uses a
>  	 * finer-grained lock, update this
>  	 */
> -	mutex_lock(&assigned_dev->kvm->lock);
> -	kvm_set_irq(assigned_dev->kvm, assigned_dev->irq_source_id,
> -		    assigned_dev->guest_irq, 1);
> +	mutex_lock(&kvm->lock);
> +handle_irq:
> +	if (assigned_dev->irq_requested_type & KVM_ASSIGNED_DEV_MSIX) {
> +		gsi = find_first_bit(kvm->irq_routes_pending_bitmap,
> +				     KVM_MAX_IRQ_ROUTES);
> +		BUG_ON(gsi >= KVM_MAX_IRQ_ROUTES);
> +		clear_bit(gsi, kvm->irq_routes_pending_bitmap);
> +	} else
> +		gsi = assigned_dev->guest_irq;
> +
> +	kvm_set_irq(assigned_dev->kvm, assigned_dev->irq_source_id, gsi, 1);
>  
>  	if (assigned_dev->irq_requested_type & KVM_ASSIGNED_DEV_GUEST_MSI) {
>  		enable_irq(assigned_dev->host_irq);
>  		assigned_dev->host_irq_disabled = false;
> +	} else if (assigned_dev->irq_requested_type & KVM_ASSIGNED_DEV_MSIX) {
> +		irq = find_host_irq_from_gsi(assigned_dev, gsi);

Check the return value?

> +		enable_irq(irq);

Do you guarantee that particular irq you're enable_irq'ing is not bogus?
Its has been passed from userspace after all.

In a later patch you can assign KVM_ASSIGNED_DEV_MSIX if the irqchip is
not in-kernel in assigned_device_update_msix:

+       adev->irq_requested_type |= KVM_ASSIGNED_DEV_MSIX;
+

Just trying to harden the code against bogosity elsewhere.

> +		assigned_dev->host_irq_disabled = false;
> +		gsi = find_first_bit(kvm->irq_routes_pending_bitmap,
> +				KVM_MAX_IRQ_ROUTES);
> +		if (gsi < KVM_MAX_IRQ_ROUTES)
> +			goto handle_irq;
>  	}
> +
>  	mutex_unlock(&assigned_dev->kvm->lock);
>  }
>  
> @@ -121,6 +209,15 @@ static irqreturn_t kvm_assigned_dev_intr(int irq, void *dev_id)
>  {
>  	struct kvm_assigned_dev_kernel *assigned_dev =
>  		(struct kvm_assigned_dev_kernel *) dev_id;
> +	struct kvm *kvm = assigned_dev->kvm;
> +
> +	if (assigned_dev->irq_requested_type == KVM_ASSIGNED_DEV_MSIX) {
> +		u32 gsi;
> +		gsi = find_gsi_from_host_irq(assigned_dev, irq);
> +		if (gsi == 0)
> +			return IRQ_HANDLED;

So you chose GSI == 0 as invalid because of x86 assumptions? Or is there
any other reason?

IRQ sharing in the host side is not supported correct?

Re: [PATCH 2/3] KVM: Add gsi_msg_pending_bitmap for MSI-X

[Sheng Yang]

On Friday 13 February 2009 03:51:18 Marcelo Tosatti wrote:
> Hi Sheng,
>
> On Wed, Feb 11, 2009 at 04:08:50PM +0800, Sheng Yang wrote:
> > We have to handle more than one interrupt with one handler for MSI-X. So
> > we need a bitmap to track the triggered interrupts.
> >
> > Signed-off-by: Sheng Yang <sheng@linux.intel.com>
> > ---
> >  include/linux/kvm_host.h |    5 +-
> >  virt/kvm/kvm_main.c      |  103
> > ++++++++++++++++++++++++++++++++++++++++++++- 2 files changed, 103
> > insertions(+), 5 deletions(-)
> >
> >
> > @@ -335,6 +337,7 @@ struct kvm_assigned_dev_kernel {
> >  #define KVM_ASSIGNED_DEV_GUEST_MSI	(1 << 1)
> >  #define KVM_ASSIGNED_DEV_HOST_INTX	(1 << 8)
> >  #define KVM_ASSIGNED_DEV_HOST_MSI	(1 << 9)
> > +#define KVM_ASSIGNED_DEV_MSIX		((1 << 2) | (1 << 10))
>
> Can you explain the usage of the two bits?

Um... Just to keep consistent with formers(one for guest and one for host), at 
cost of one bit.

> > diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
> > index ea96690..961603f 100644
> > --- a/virt/kvm/kvm_main.c
> > +++ b/virt/kvm/kvm_main.c
> > @@ -95,25 +95,113 @@ static struct kvm_assigned_dev_kernel
> > *kvm_find_assigned_dev(struct list_head *h return NULL;
> >  }
> >
> > +static int find_host_irq_from_gsi(struct kvm_assigned_dev_kernel
> > *assigned_dev, +				  u32 gsi)
> > +{
> > +	int i, entry, irq;
> > +	struct msix_entry *host_msix_entries, *guest_msix_entries;
> > +
> > +	host_msix_entries = assigned_dev->host_msix_entries;
> > +	guest_msix_entries = assigned_dev->guest_msix_entries;
> > +
> > +	entry = -1;
> > +	irq = 0;
> > +	for (i = 0; i < assigned_dev->entries_nr; i++)
> > +		if (gsi == (guest_msix_entries + i)->vector) {
> > +			entry = (guest_msix_entries + i)->entry;
> > +			break;
> > +		}
> > +	if (entry < 0) {
> > +		printk(KERN_WARNING "Fail to find correlated MSI-X entry!\n");
> > +		return 0;
> > +	}
> > +	for (i = 0; i < assigned_dev->entries_nr; i++)
> > +		if (entry == (host_msix_entries + i)->entry) {
> > +			irq = (host_msix_entries + i)->vector;
> > +			break;
> > +		}
> > +	if (irq == 0) {
> > +		printk(KERN_WARNING "Fail to find correlated MSI-X irq!\n");
> > +		return 0;
> > +	}
>
> Can drop the printk's (also from find_gsi_from_host_irq).

Not that confident. In fact, I often triggered this during debug... IIRC, 
userspace program shouldn't trigger this if kernel space works well. Maybe it 
can be changed to WARN_ON() or BUG_ON() later.
>
> >  	struct kvm_assigned_dev_kernel *assigned_dev;
> > +	struct kvm *kvm;
> > +	u32 gsi;
> > +	int irq;
> >
> >  	assigned_dev = container_of(work, struct kvm_assigned_dev_kernel,
> >  				    interrupt_work);
> > +	kvm = assigned_dev->kvm;
> >
> >  	/* This is taken to safely inject irq inside the guest. When
> >  	 * the interrupt injection (or the ioapic code) uses a
> >  	 * finer-grained lock, update this
> >  	 */
> > -	mutex_lock(&assigned_dev->kvm->lock);
> > -	kvm_set_irq(assigned_dev->kvm, assigned_dev->irq_source_id,
> > -		    assigned_dev->guest_irq, 1);
> > +	mutex_lock(&kvm->lock);
> > +handle_irq:
> > +	if (assigned_dev->irq_requested_type & KVM_ASSIGNED_DEV_MSIX) {
> > +		gsi = find_first_bit(kvm->irq_routes_pending_bitmap,
> > +				     KVM_MAX_IRQ_ROUTES);
> > +		BUG_ON(gsi >= KVM_MAX_IRQ_ROUTES);
> > +		clear_bit(gsi, kvm->irq_routes_pending_bitmap);
> > +	} else
> > +		gsi = assigned_dev->guest_irq;
> > +
> > +	kvm_set_irq(assigned_dev->kvm, assigned_dev->irq_source_id, gsi, 1);
> >
> >  	if (assigned_dev->irq_requested_type & KVM_ASSIGNED_DEV_GUEST_MSI) {
> >  		enable_irq(assigned_dev->host_irq);
> >  		assigned_dev->host_irq_disabled = false;
> > +	} else if (assigned_dev->irq_requested_type & KVM_ASSIGNED_DEV_MSIX) {
> > +		irq = find_host_irq_from_gsi(assigned_dev, gsi);
>
> Check the return value?

Yeah...
>
> > +		enable_irq(irq);
>
> Do you guarantee that particular irq you're enable_irq'ing is not bogus?
> Its has been passed from userspace after all.

It isn't passed from userspace. This one is filled by pci_enable_msix(), which 
should be OK. 
>
> In a later patch you can assign KVM_ASSIGNED_DEV_MSIX if the irqchip is
> not in-kernel in assigned_device_update_msix:
>
> +       adev->irq_requested_type |= KVM_ASSIGNED_DEV_MSIX;
> +
>
> Just trying to harden the code against bogosity elsewhere.

Yeah, of course. :)
>
> > +		assigned_dev->host_irq_disabled = false;
> > +		gsi = find_first_bit(kvm->irq_routes_pending_bitmap,
> > +				KVM_MAX_IRQ_ROUTES);
> > +		if (gsi < KVM_MAX_IRQ_ROUTES)
> > +			goto handle_irq;
> >  	}
> > +
> >  	mutex_unlock(&assigned_dev->kvm->lock);
> >  }
> >
> > @@ -121,6 +209,15 @@ static irqreturn_t kvm_assigned_dev_intr(int irq,
> > void *dev_id) {
> >  	struct kvm_assigned_dev_kernel *assigned_dev =
> >  		(struct kvm_assigned_dev_kernel *) dev_id;
> > +	struct kvm *kvm = assigned_dev->kvm;
> > +
> > +	if (assigned_dev->irq_requested_type == KVM_ASSIGNED_DEV_MSIX) {
> > +		u32 gsi;
> > +		gsi = find_gsi_from_host_irq(assigned_dev, irq);
> > +		if (gsi == 0)
> > +			return IRQ_HANDLED;
>
> So you chose GSI == 0 as invalid because of x86 assumptions? Or is there
> any other reason?

Yeah, it based on x86 and IA64 IRQ 0 can't be used by MSI-X. And only x86 
support MSI-X now(and IA64 would follow later).
>
> IRQ sharing in the host side is not supported correct?

Um? Yeah... And seems we won't support it forever...

-- 
regards
Yang, Sheng

Re: [PATCH 2/3] KVM: Add gsi_msg_pending_bitmap for MSI-X

[Marcelo Tosatti]

On Fri, Feb 13, 2009 at 11:37:45AM +0800, Sheng Yang wrote:
> > > +#define KVM_ASSIGNED_DEV_MSIX		((1 << 2) | (1 << 10))
> >
> > Can you explain the usage of the two bits?
> 
> Um... Just to keep consistent with formers(one for guest and one for host), at 
> cost of one bit.

OK

> > Can drop the printk's (also from find_gsi_from_host_irq).
> 
> Not that confident. In fact, I often triggered this during debug... IIRC, 
> userspace program shouldn't trigger this if kernel space works well. Maybe it 
> can be changed to WARN_ON() or BUG_ON() later.

OK

> > Check the return value?
> 
> Yeah...
> >
> > > +		enable_irq(irq);
> >
> > Do you guarantee that particular irq you're enable_irq'ing is not bogus?
> > Its has been passed from userspace after all.
> 
> It isn't passed from userspace. This one is filled by pci_enable_msix(), which 
> should be OK. 

Alright.

> > So you chose GSI == 0 as invalid because of x86 assumptions? Or is there
> > any other reason?
> 
> Yeah, it based on x86 and IA64 IRQ 0 can't be used by MSI-X. And only x86 
> support MSI-X now(and IA64 would follow later).

OK.

> >
> > IRQ sharing in the host side is not supported correct?
> 
> Um? Yeah... And seems we won't support it forever...

OK.

Re: [PATCH 2/3] KVM: Add gsi_msg_pending_bitmap for MSI-X

[Avi Kivity]

Sheng Yang wrote:
>>> +	if (assigned_dev->irq_requested_type == KVM_ASSIGNED_DEV_MSIX) {
>>> +		u32 gsi;
>>> +		gsi = find_gsi_from_host_irq(assigned_dev, irq);
>>> +		if (gsi == 0)
>>> +			return IRQ_HANDLED;
>>>       
>> So you chose GSI == 0 as invalid because of x86 assumptions? Or is there
>> any other reason?
>>     
>
> Yeah, it based on x86 and IA64 IRQ 0 can't be used by MSI-X. And only x86 
> support MSI-X now(and IA64 would follow later).
>   

gsi != irq... as used in kvm it is just a cookie and is never visible to 
the guest.  I'd prefer an illegal value here.


Also, please repost the entire patchset so I can be sure I apply the 
latest; too many versions floating around.

-- 
I have a truly marvellous patch that fixes the bug which this
signature is too narrow to contain.

Re: [PATCH 2/3] KVM: Add gsi_msg_pending_bitmap for MSI-X

[Sheng Yang]

On Wednesday 18 February 2009 02:09:49 Avi Kivity wrote:
> Sheng Yang wrote:
> >>> +	if (assigned_dev->irq_requested_type == KVM_ASSIGNED_DEV_MSIX) {
> >>> +		u32 gsi;
> >>> +		gsi = find_gsi_from_host_irq(assigned_dev, irq);
> >>> +		if (gsi == 0)
> >>> +			return IRQ_HANDLED;
> >>
> >> So you chose GSI == 0 as invalid because of x86 assumptions? Or is there
> >> any other reason?
> >
> > Yeah, it based on x86 and IA64 IRQ 0 can't be used by MSI-X. And only x86
> > support MSI-X now(and IA64 would follow later).
>
> gsi != irq... as used in kvm it is just a cookie and is never visible to
> the guest.  I'd prefer an illegal value here.
>
>
> Also, please repost the entire patchset so I can be sure I apply the
> latest; too many versions floating around.

OK. And please don't forget there is a patchset named "Optimize and unify 
IOAPIC/MSI delivery" before this one. :)

-- 
regards
Yang, Sheng

[PATCH 3/3] KVM: Enable MSI-X for KVM assigned device

[Sheng Yang]

This patch finally enable MSI-X.

What we need for MSI-X:
1. Intercept one page in MMIO region of device. So that we can get guest desired
MSI-X table and set up the real one. Now this have been done by guest, and
transfer to kernel using ioctl KVM_SET_MSIX_NR and KVM_SET_MSIX_ENTRY.

2. Information for incoming interrupt. Now one device can have more than one
interrupt, and they are all handled by one workqueue structure. So we need to
identify them. The previous patch enable gsi_msg_pending_bitmap get this done.

3. Mapping from host IRQ to guest gsi as well as guest gsi to real MSI/MSI-X
message address/data. We used same entry number for the host and guest here, so
that it's easy to find the correlated guest gsi.

What we lack for now:
1. The PCI spec said nothing can existed with MSI-X table in the same page of
MMIO region, except pending bits. The patch ignore pending bits as the first
step (so they are always 0 - no pending).

2. The PCI spec allowed to change MSI-X table dynamically. That means, the OS
can enable MSI-X, then mask one MSI-X entry, modify it, and unmask it. The patch
didn't support this, and Linux also don't work in this way.

3. The patch didn't implement MSI-X mask all and mask single entry. I would
implement the former in driver/pci/msi.c later. And for single entry, userspace
should have reposibility to handle it.

Signed-off-by: Sheng Yang <sheng@linux.intel.com>
---
 include/linux/kvm.h |    8 ++++
 virt/kvm/kvm_main.c |  107 ++++++++++++++++++++++++++++++++++++++++++++++++---
 2 files changed, 109 insertions(+), 6 deletions(-)

diff --git a/include/linux/kvm.h b/include/linux/kvm.h
index 5200768..fcd08da 100644
--- a/include/linux/kvm.h
+++ b/include/linux/kvm.h
@@ -439,6 +439,9 @@ struct kvm_irq_routing {
 };
 
 #endif
+#if defined(CONFIG_X86)
+#define KVM_CAP_DEVICE_MSIX 26
+#endif
 
 /*
  * ioctls for VM fds
@@ -596,6 +599,11 @@ struct kvm_assigned_irq {
 #define KVM_DEV_IRQ_ASSIGN_MSI_ACTION	KVM_DEV_IRQ_ASSIGN_ENABLE_MSI
 #define KVM_DEV_IRQ_ASSIGN_ENABLE_MSI	(1 << 0)
 
+#define KVM_DEV_IRQ_ASSIGN_MSIX_ACTION  (KVM_DEV_IRQ_ASSIGN_ENABLE_MSIX |\
+					KVM_DEV_IRQ_ASSIGN_MASK_MSIX)
+#define KVM_DEV_IRQ_ASSIGN_ENABLE_MSIX  (1 << 1)
+#define KVM_DEV_IRQ_ASSIGN_MASK_MSIX    (1 << 2)
+
 struct kvm_assigned_msix_nr {
 	__u32 assigned_dev_id;
 	__u16 entry_nr;
diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
index 961603f..c774027 100644
--- a/virt/kvm/kvm_main.c
+++ b/virt/kvm/kvm_main.c
@@ -281,13 +281,33 @@ static void kvm_free_assigned_irq(struct kvm *kvm,
 	 * now, the kvm state is still legal for probably we also have to wait
 	 * interrupt_work done.
 	 */
-	disable_irq_nosync(assigned_dev->host_irq);
-	cancel_work_sync(&assigned_dev->interrupt_work);
+	if (assigned_dev->irq_requested_type & KVM_ASSIGNED_DEV_MSIX) {
+		int i;
+		for (i = 0; i < assigned_dev->entries_nr; i++)
+			disable_irq_nosync(assigned_dev->
+					   host_msix_entries[i].vector);
+
+		cancel_work_sync(&assigned_dev->interrupt_work);
+
+		for (i = 0; i < assigned_dev->entries_nr; i++)
+			free_irq(assigned_dev->host_msix_entries[i].vector,
+				 (void *)assigned_dev);
+
+		assigned_dev->entries_nr = 0;
+		kfree(assigned_dev->host_msix_entries);
+		kfree(assigned_dev->guest_msix_entries);
+		pci_disable_msix(assigned_dev->dev);
+	} else {
+		/* Deal with MSI and INTx */
+		disable_irq_nosync(assigned_dev->host_irq);
+		cancel_work_sync(&assigned_dev->interrupt_work);
 
-	free_irq(assigned_dev->host_irq, (void *)assigned_dev);
+		free_irq(assigned_dev->host_irq, (void *)assigned_dev);
 
-	if (assigned_dev->irq_requested_type & KVM_ASSIGNED_DEV_HOST_MSI)
-		pci_disable_msi(assigned_dev->dev);
+		if (assigned_dev->irq_requested_type &
+				KVM_ASSIGNED_DEV_HOST_MSI)
+			pci_disable_msi(assigned_dev->dev);
+	}
 
 	assigned_dev->irq_requested_type = 0;
 }
@@ -416,6 +436,69 @@ static int assigned_device_update_msi(struct kvm *kvm,
 	adev->irq_requested_type |= KVM_ASSIGNED_DEV_HOST_MSI;
 	return 0;
 }
+
+static int assigned_device_update_msix(struct kvm *kvm,
+			struct kvm_assigned_dev_kernel *adev,
+			struct kvm_assigned_irq *airq)
+{
+	/* TODO Deal with KVM_DEV_IRQ_ASSIGNED_MASK_MSIX */
+	int i, r;
+
+	adev->ack_notifier.gsi = -1;
+
+	if (irqchip_in_kernel(kvm)) {
+		if (airq->flags & KVM_DEV_IRQ_ASSIGN_MASK_MSIX) {
+			printk(KERN_WARNING
+			       "kvm: unsupported mask MSI-X, flags 0x%x!\n",
+			       airq->flags);
+			return 0;
+		}
+
+		if (!(airq->flags & KVM_DEV_IRQ_ASSIGN_ENABLE_MSIX)) {
+			/* Guest disable MSI-X */
+			kvm_free_assigned_irq(kvm, adev);
+			if (msi2intx) {
+				pci_enable_msi(adev->dev);
+				if (adev->dev->msi_enabled)
+					return assigned_device_update_msi(kvm,
+							adev, airq);
+			}
+			return assigned_device_update_intx(kvm, adev, airq);
+		}
+
+		/* host_msix_entries and guest_msix_entries should have been
+		 * initialized */
+
+		if (adev->entries_nr == 0) {
+			printk(KERN_ERR
+			       "kvm: MSI-X entry not initialized!\n");
+			return -EFAULT;
+		}
+
+		kvm_free_assigned_irq(kvm, adev);
+
+		r = pci_enable_msix(adev->dev, adev->host_msix_entries,
+				    adev->entries_nr);
+		if (r) {
+			printk(KERN_ERR "Fail to enable MSI-X feature!\n");
+			return r;
+		}
+
+		for (i = 0; i < adev->entries_nr; i++) {
+			r = request_irq((adev->host_msix_entries + i)->vector,
+					kvm_assigned_dev_intr, 0,
+					"kvm_assigned_msix_device",
+					(void *)adev);
+			if (r)
+				return r;
+		}
+	}
+
+	adev->irq_requested_type |= KVM_ASSIGNED_DEV_MSIX;
+
+	return 0;
+}
+
 #endif
 
 static int kvm_vm_ioctl_assign_irq(struct kvm *kvm,
@@ -462,12 +545,24 @@ static int kvm_vm_ioctl_assign_irq(struct kvm *kvm,
 		}
 	}
 
-	if ((match->irq_requested_type & KVM_ASSIGNED_DEV_HOST_MSI) &&
+	if (match->irq_requested_type & KVM_ASSIGNED_DEV_MSIX)
+		current_flags |= KVM_DEV_IRQ_ASSIGN_ENABLE_MSIX;
+	else if ((match->irq_requested_type & KVM_ASSIGNED_DEV_HOST_MSI) &&
 		 (match->irq_requested_type & KVM_ASSIGNED_DEV_GUEST_MSI))
 		current_flags |= KVM_DEV_IRQ_ASSIGN_ENABLE_MSI;
 
 	changed_flags = assigned_irq->flags ^ current_flags;
 
+#ifdef CONFIG_X86
+	if (changed_flags & KVM_DEV_IRQ_ASSIGN_MSIX_ACTION) {
+		r = assigned_device_update_msix(kvm, match, assigned_irq);
+		if (r) {
+			printk(KERN_WARNING "kvm: failed to execute "
+					"MSI-X action!\n");
+			goto out_release;
+		}
+	} else
+#endif
 	if ((changed_flags & KVM_DEV_IRQ_ASSIGN_MSI_ACTION) ||
 	    (msi2intx && match->dev->msi_enabled)) {
 #ifdef CONFIG_X86
-- 
1.5.4.5

Re: [PATCH 3/3] KVM: Enable MSI-X for KVM assigned device

[Avi Kivity]

Sheng Yang wrote:
> This patch finally enable MSI-X.
>
> What we need for MSI-X:
> 1. Intercept one page in MMIO region of device. So that we can get guest desired
> MSI-X table and set up the real one. Now this have been done by guest, and
> transfer to kernel using ioctl KVM_SET_MSIX_NR and KVM_SET_MSIX_ENTRY.
>
>   

Looking at the patch, you're doing that in userspace.  Good.

> 2. The PCI spec allowed to change MSI-X table dynamically. That means, the OS
> can enable MSI-X, then mask one MSI-X entry, modify it, and unmask it. The patch
> didn't support this, and Linux also don't work in this way.
>   

What about Windows?


-- 
error compiling committee.c: too many arguments to function

Re: [PATCH 3/3] KVM: Enable MSI-X for KVM assigned device

[Sheng Yang]

On Wednesday 11 February 2009 20:48:55 Avi Kivity wrote:
> Sheng Yang wrote:
> > This patch finally enable MSI-X.
> >
> > What we need for MSI-X:
> > 1. Intercept one page in MMIO region of device. So that we can get guest
> > desired MSI-X table and set up the real one. Now this have been done by
> > guest, and transfer to kernel using ioctl KVM_SET_MSIX_NR and
> > KVM_SET_MSIX_ENTRY.
>
> Looking at the patch, you're doing that in userspace.  Good.
>
> > 2. The PCI spec allowed to change MSI-X table dynamically. That means,
> > the OS can enable MSI-X, then mask one MSI-X entry, modify it, and unmask
> > it. The patch didn't support this, and Linux also don't work in this way.
>
> What about Windows?

Not sure. Don't have Windows driver to test... And only Vista and later 
support MSI/MSI-X, not sure it would be so aggressive... We can add some debug 
if such condition happened.

-- 
regards
Yang, Sheng

Re: [PATCH 3/3] KVM: Enable MSI-X for KVM assigned device

[Marcelo Tosatti]

On Wed, Feb 11, 2009 at 04:08:51PM +0800, Sheng Yang wrote:
> This patch finally enable MSI-X.
> 
> What we need for MSI-X:
> 1. Intercept one page in MMIO region of device. So that we can get guest desired
> MSI-X table and set up the real one. Now this have been done by guest, and
> transfer to kernel using ioctl KVM_SET_MSIX_NR and KVM_SET_MSIX_ENTRY.
> 
> 2. Information for incoming interrupt. Now one device can have more than one
> interrupt, and they are all handled by one workqueue structure. So we need to
> identify them. The previous patch enable gsi_msg_pending_bitmap get this done.
> 
> 3. Mapping from host IRQ to guest gsi as well as guest gsi to real MSI/MSI-X
> message address/data. We used same entry number for the host and guest here, so
> that it's easy to find the correlated guest gsi.
> 
> What we lack for now:
> 1. The PCI spec said nothing can existed with MSI-X table in the same page of
> MMIO region, except pending bits. The patch ignore pending bits as the first
> step (so they are always 0 - no pending).
> 
> 2. The PCI spec allowed to change MSI-X table dynamically. That means, the OS
> can enable MSI-X, then mask one MSI-X entry, modify it, and unmask it. The patch
> didn't support this, and Linux also don't work in this way.

Have you thought about supporting this with the current ioctl interface?
Point is that once ioctl's are in and userspace code uses it, you can't
change. So:

- If userspace calls kvm_vm_ioctl_set_msix_entry twice don't you leak
  memory (that is a bug actually unless I'm mistaken)? So in the future
  kvm_vm_ioctl_set_msix_entry will deallocate the currently allocated
  guest/host entries and replace with userspace supplied entries?

- interrupt context can read the table while kvm_vm_ioctl_set_msix_entry
  is modifying it. So you either need to forbid more than one
  kvm_vm_ioctl_set_msix_entry call in the lifetime of a guest (which
  you can later allow when you support MSI table change), or handle 
  accesses from multiple contexes now. It seems forbidding is enough for
  the moment from what you said.

But in general looks good to me (not familiar with the internals of
pci / msi).

Re: [PATCH 3/3] KVM: Enable MSI-X for KVM assigned device

[Sheng Yang]

On Friday 13 February 2009 04:40:05 Marcelo Tosatti wrote:
> On Wed, Feb 11, 2009 at 04:08:51PM +0800, Sheng Yang wrote:
> > This patch finally enable MSI-X.
> >
> > What we need for MSI-X:
> > 1. Intercept one page in MMIO region of device. So that we can get guest
> > desired MSI-X table and set up the real one. Now this have been done by
> > guest, and transfer to kernel using ioctl KVM_SET_MSIX_NR and
> > KVM_SET_MSIX_ENTRY.
> >
> > 2. Information for incoming interrupt. Now one device can have more than
> > one interrupt, and they are all handled by one workqueue structure. So we
> > need to identify them. The previous patch enable gsi_msg_pending_bitmap
> > get this done.
> >
> > 3. Mapping from host IRQ to guest gsi as well as guest gsi to real
> > MSI/MSI-X message address/data. We used same entry number for the host
> > and guest here, so that it's easy to find the correlated guest gsi.
> >
> > What we lack for now:
> > 1. The PCI spec said nothing can existed with MSI-X table in the same
> > page of MMIO region, except pending bits. The patch ignore pending bits
> > as the first step (so they are always 0 - no pending).
> >
> > 2. The PCI spec allowed to change MSI-X table dynamically. That means,
> > the OS can enable MSI-X, then mask one MSI-X entry, modify it, and unmask
> > it. The patch didn't support this, and Linux also don't work in this way.
>
> Have you thought about supporting this with the current ioctl interface?
> Point is that once ioctl's are in and userspace code uses it, you can't
> change. So:
>
> - If userspace calls kvm_vm_ioctl_set_msix_entry twice don't you leak
>   memory (that is a bug actually unless I'm mistaken)? So in the future
>   kvm_vm_ioctl_set_msix_entry will deallocate the currently allocated
>   guest/host entries and replace with userspace supplied entries?
>

The allocation only happen once, at the second time it would report error in 
current code. But allocate/deallocate is also acceptable for future.

> - interrupt context can read the table while kvm_vm_ioctl_set_msix_entry
>   is modifying it. So you either need to forbid more than one
>   kvm_vm_ioctl_set_msix_entry call in the lifetime of a guest (which
>   you can later allow when you support MSI table change), or handle
>   accesses from multiple contexes now. It seems forbidding is enough for
>   the moment from what you said.

Yeah.

But for the modifying the MSI-X table, the most critical problem is, current 
Linux didn't support it IIRC. So I have to disable MSI-X then enable it again 
with new table, and it would result in lost interrupt.

So seems the most reasonable method is to modify pci_enable_msix() and related 
function's action to support this...

-- 
regards
Yang, Sheng

>
> But in general looks good to me (not familiar with the internals of
> pci / msi).

Re: [PATCH 3/3] KVM: Enable MSI-X for KVM assigned device

[Marcelo Tosatti]

On Fri, Feb 13, 2009 at 01:29:42PM +0800, Sheng Yang wrote:
> The allocation only happen once, at the second time it would report error in 
> current code. But allocate/deallocate is also acceptable for future.

Oops, it OK. 

> > - interrupt context can read the table while kvm_vm_ioctl_set_msix_entry
> >   is modifying it. So you either need to forbid more than one
> >   kvm_vm_ioctl_set_msix_entry call in the lifetime of a guest (which
> >   you can later allow when you support MSI table change), or handle
> >   accesses from multiple contexes now. It seems forbidding is enough for
> >   the moment from what you said.
> 
> Yeah.
> 
> But for the modifying the MSI-X table, the most critical problem is, current 
> Linux didn't support it IIRC. So I have to disable MSI-X then enable it again 
> with new table, and it would result in lost interrupt.
> 
> So seems the most reasonable method is to modify pci_enable_msix() and related 
> function's action to support this...

Alright, then just reply "[PATCH 1/3] KVM: Ioctls for init MSI-X entry"
with the minor comments reworked so Avi can apply.

Thanks.