All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/3 v2] MSI-X enabling
@ 2009-02-11  8:08 Sheng Yang
  2009-02-11  8:08 ` [PATCH 1/3] KVM: Ioctls for init MSI-X entry Sheng Yang
                   ` (2 more replies)
  0 siblings, 3 replies; 23+ messages in thread
From: Sheng Yang @ 2009-02-11  8:08 UTC (permalink / raw)
  To: Avi Kivity; +Cc: kvm

Based on former patchset.

^ permalink raw reply	[flat|nested] 23+ messages in thread

* [PATCH 1/3] KVM: Ioctls for init MSI-X entry
  2009-02-11  8:08 [PATCH 0/3 v2] MSI-X enabling Sheng Yang
@ 2009-02-11  8:08 ` Sheng Yang
  2009-02-11 12:44   ` Avi Kivity
  2009-02-11  8:08 ` [PATCH 2/3] KVM: Add gsi_msg_pending_bitmap for MSI-X Sheng Yang
  2009-02-11  8:08 ` [PATCH 3/3] KVM: Enable MSI-X for KVM assigned device Sheng Yang
  2 siblings, 1 reply; 23+ messages in thread
From: Sheng Yang @ 2009-02-11  8:08 UTC (permalink / raw)
  To: Avi Kivity; +Cc: kvm, 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


^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [PATCH 2/3] KVM: Add gsi_msg_pending_bitmap for MSI-X
  2009-02-11  8:08 [PATCH 0/3 v2] MSI-X enabling Sheng Yang
  2009-02-11  8:08 ` [PATCH 1/3] KVM: Ioctls for init MSI-X entry Sheng Yang
@ 2009-02-11  8:08 ` Sheng Yang
  2009-02-12 19:51   ` Marcelo Tosatti
  2009-02-11  8:08 ` [PATCH 3/3] KVM: Enable MSI-X for KVM assigned device Sheng Yang
  2 siblings, 1 reply; 23+ messages in thread
From: Sheng Yang @ 2009-02-11  8:08 UTC (permalink / raw)
  To: Avi Kivity; +Cc: kvm, 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


^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [PATCH 3/3] KVM: Enable MSI-X for KVM assigned device
  2009-02-11  8:08 [PATCH 0/3 v2] MSI-X enabling Sheng Yang
  2009-02-11  8:08 ` [PATCH 1/3] KVM: Ioctls for init MSI-X entry Sheng Yang
  2009-02-11  8:08 ` [PATCH 2/3] KVM: Add gsi_msg_pending_bitmap for MSI-X Sheng Yang
@ 2009-02-11  8:08 ` Sheng Yang
  2009-02-11 12:48   ` Avi Kivity
  2009-02-12 20:40   ` Marcelo Tosatti
  2 siblings, 2 replies; 23+ messages in thread
From: Sheng Yang @ 2009-02-11  8:08 UTC (permalink / raw)
  To: Avi Kivity; +Cc: kvm, 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


^ permalink raw reply related	[flat|nested] 23+ messages in thread

* Re: [PATCH 1/3] KVM: Ioctls for init MSI-X entry
  2009-02-11  8:08 ` [PATCH 1/3] KVM: Ioctls for init MSI-X entry Sheng Yang
@ 2009-02-11 12:44   ` Avi Kivity
  2009-02-12  6:28     ` Sheng Yang
  0 siblings, 1 reply; 23+ messages in thread
From: Avi Kivity @ 2009-02-11 12:44 UTC (permalink / raw)
  To: Sheng Yang; +Cc: kvm

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


^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH 3/3] KVM: Enable MSI-X for KVM assigned device
  2009-02-11  8:08 ` [PATCH 3/3] KVM: Enable MSI-X for KVM assigned device Sheng Yang
@ 2009-02-11 12:48   ` Avi Kivity
  2009-02-12  6:03     ` Sheng Yang
  2009-02-12 20:40   ` Marcelo Tosatti
  1 sibling, 1 reply; 23+ messages in thread
From: Avi Kivity @ 2009-02-11 12:48 UTC (permalink / raw)
  To: Sheng Yang; +Cc: kvm

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


^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH 3/3] KVM: Enable MSI-X for KVM assigned device
  2009-02-11 12:48   ` Avi Kivity
@ 2009-02-12  6:03     ` Sheng Yang
  0 siblings, 0 replies; 23+ messages in thread
From: Sheng Yang @ 2009-02-12  6:03 UTC (permalink / raw)
  To: Avi Kivity; +Cc: kvm

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


^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH 1/3] KVM: Ioctls for init MSI-X entry
  2009-02-11 12:44   ` Avi Kivity
@ 2009-02-12  6:28     ` Sheng Yang
  2009-02-12 10:07       ` Sheng Yang
  0 siblings, 1 reply; 23+ messages in thread
From: Sheng Yang @ 2009-02-12  6:28 UTC (permalink / raw)
  To: Avi Kivity; +Cc: kvm

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

^ permalink raw reply	[flat|nested] 23+ messages in thread

* [PATCH 1/3] KVM: Ioctls for init MSI-X entry
  2009-02-12  6:28     ` Sheng Yang
@ 2009-02-12 10:07       ` Sheng Yang
  2009-02-12 18:46         ` Marcelo Tosatti
  0 siblings, 1 reply; 23+ messages in thread
From: Sheng Yang @ 2009-02-12 10:07 UTC (permalink / raw)
  To: Avi Kivity; +Cc: kvm, 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


^ permalink raw reply related	[flat|nested] 23+ messages in thread

* Re: [PATCH 1/3] KVM: Ioctls for init MSI-X entry
  2009-02-12 10:07       ` Sheng Yang
@ 2009-02-12 18:46         ` Marcelo Tosatti
  2009-02-16  3:18           ` Sheng Yang
  0 siblings, 1 reply; 23+ messages in thread
From: Marcelo Tosatti @ 2009-02-12 18:46 UTC (permalink / raw)
  To: Sheng Yang; +Cc: Avi Kivity, kvm

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.


^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH 2/3] KVM: Add gsi_msg_pending_bitmap for MSI-X
  2009-02-11  8:08 ` [PATCH 2/3] KVM: Add gsi_msg_pending_bitmap for MSI-X Sheng Yang
@ 2009-02-12 19:51   ` Marcelo Tosatti
  2009-02-13  3:37     ` Sheng Yang
  0 siblings, 1 reply; 23+ messages in thread
From: Marcelo Tosatti @ 2009-02-12 19:51 UTC (permalink / raw)
  To: Sheng Yang; +Cc: Avi Kivity, kvm

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?


^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH 3/3] KVM: Enable MSI-X for KVM assigned device
  2009-02-11  8:08 ` [PATCH 3/3] KVM: Enable MSI-X for KVM assigned device Sheng Yang
  2009-02-11 12:48   ` Avi Kivity
@ 2009-02-12 20:40   ` Marcelo Tosatti
  2009-02-13  5:29     ` Sheng Yang
  1 sibling, 1 reply; 23+ messages in thread
From: Marcelo Tosatti @ 2009-02-12 20:40 UTC (permalink / raw)
  To: Sheng Yang; +Cc: Avi Kivity, kvm

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

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH 2/3] KVM: Add gsi_msg_pending_bitmap for MSI-X
  2009-02-12 19:51   ` Marcelo Tosatti
@ 2009-02-13  3:37     ` Sheng Yang
  2009-02-13 17:10       ` Marcelo Tosatti
  2009-02-17 18:09       ` Avi Kivity
  0 siblings, 2 replies; 23+ messages in thread
From: Sheng Yang @ 2009-02-13  3:37 UTC (permalink / raw)
  To: Marcelo Tosatti; +Cc: Avi Kivity, kvm

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


^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH 3/3] KVM: Enable MSI-X for KVM assigned device
  2009-02-12 20:40   ` Marcelo Tosatti
@ 2009-02-13  5:29     ` Sheng Yang
  2009-02-13 17:13       ` Marcelo Tosatti
  0 siblings, 1 reply; 23+ messages in thread
From: Sheng Yang @ 2009-02-13  5:29 UTC (permalink / raw)
  To: Marcelo Tosatti; +Cc: Avi Kivity, kvm

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


^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH 2/3] KVM: Add gsi_msg_pending_bitmap for MSI-X
  2009-02-13  3:37     ` Sheng Yang
@ 2009-02-13 17:10       ` Marcelo Tosatti
  2009-02-17 18:09       ` Avi Kivity
  1 sibling, 0 replies; 23+ messages in thread
From: Marcelo Tosatti @ 2009-02-13 17:10 UTC (permalink / raw)
  To: Sheng Yang; +Cc: Avi Kivity, kvm

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.

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH 3/3] KVM: Enable MSI-X for KVM assigned device
  2009-02-13  5:29     ` Sheng Yang
@ 2009-02-13 17:13       ` Marcelo Tosatti
  0 siblings, 0 replies; 23+ messages in thread
From: Marcelo Tosatti @ 2009-02-13 17:13 UTC (permalink / raw)
  To: Sheng Yang; +Cc: Avi Kivity, kvm

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.


^ permalink raw reply	[flat|nested] 23+ messages in thread

* [PATCH 1/3] KVM: Ioctls for init MSI-X entry
  2009-02-12 18:46         ` Marcelo Tosatti
@ 2009-02-16  3:18           ` Sheng Yang
  2009-02-16  5:49             ` Sheng Yang
  0 siblings, 1 reply; 23+ messages in thread
From: Sheng Yang @ 2009-02-16  3:18 UTC (permalink / raw)
  To: Avi Kivity; +Cc: Marcelo Tosatti, kvm, 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


^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [PATCH 1/3] KVM: Ioctls for init MSI-X entry
  2009-02-16  3:18           ` Sheng Yang
@ 2009-02-16  5:49             ` Sheng Yang
  2009-02-16 23:23               ` Marcelo Tosatti
  0 siblings, 1 reply; 23+ messages in thread
From: Sheng Yang @ 2009-02-16  5:49 UTC (permalink / raw)
  To: Avi Kivity; +Cc: Marcelo Tosatti, kvm, 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


^ permalink raw reply related	[flat|nested] 23+ messages in thread

* Re: [PATCH 1/3] KVM: Ioctls for init MSI-X entry
  2009-02-16  5:49             ` Sheng Yang
@ 2009-02-16 23:23               ` Marcelo Tosatti
  2009-02-17  1:35                 ` Sheng Yang
  0 siblings, 1 reply; 23+ messages in thread
From: Marcelo Tosatti @ 2009-02-16 23:23 UTC (permalink / raw)
  To: Sheng Yang; +Cc: Avi Kivity, kvm

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.


^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH 1/3] KVM: Ioctls for init MSI-X entry
  2009-02-16 23:23               ` Marcelo Tosatti
@ 2009-02-17  1:35                 ` Sheng Yang
  2009-02-17  1:36                   ` Sheng Yang
  0 siblings, 1 reply; 23+ messages in thread
From: Sheng Yang @ 2009-02-17  1:35 UTC (permalink / raw)
  To: Marcelo Tosatti; +Cc: Avi Kivity, kvm

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.


^ permalink raw reply	[flat|nested] 23+ messages in thread

* [PATCH 1/3] KVM: Ioctls for init MSI-X entry
  2009-02-17  1:35                 ` Sheng Yang
@ 2009-02-17  1:36                   ` Sheng Yang
  0 siblings, 0 replies; 23+ messages in thread
From: Sheng Yang @ 2009-02-17  1:36 UTC (permalink / raw)
  To: Avi Kivity; +Cc: Marcelo Tosatti, kvm, 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


^ permalink raw reply related	[flat|nested] 23+ messages in thread

* Re: [PATCH 2/3] KVM: Add gsi_msg_pending_bitmap for MSI-X
  2009-02-13  3:37     ` Sheng Yang
  2009-02-13 17:10       ` Marcelo Tosatti
@ 2009-02-17 18:09       ` Avi Kivity
  2009-02-18  1:33         ` Sheng Yang
  1 sibling, 1 reply; 23+ messages in thread
From: Avi Kivity @ 2009-02-17 18:09 UTC (permalink / raw)
  To: Sheng Yang; +Cc: Marcelo Tosatti, kvm

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.


^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH 2/3] KVM: Add gsi_msg_pending_bitmap for MSI-X
  2009-02-17 18:09       ` Avi Kivity
@ 2009-02-18  1:33         ` Sheng Yang
  0 siblings, 0 replies; 23+ messages in thread
From: Sheng Yang @ 2009-02-18  1:33 UTC (permalink / raw)
  To: Avi Kivity; +Cc: Marcelo Tosatti, kvm

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

^ permalink raw reply	[flat|nested] 23+ messages in thread

end of thread, other threads:[~2009-02-18  1:33 UTC | newest]

Thread overview: 23+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2009-02-11  8:08 [PATCH 0/3 v2] MSI-X enabling Sheng Yang
2009-02-11  8:08 ` [PATCH 1/3] KVM: Ioctls for init MSI-X entry Sheng Yang
2009-02-11 12:44   ` Avi Kivity
2009-02-12  6:28     ` Sheng Yang
2009-02-12 10:07       ` Sheng Yang
2009-02-12 18:46         ` Marcelo Tosatti
2009-02-16  3:18           ` Sheng Yang
2009-02-16  5:49             ` Sheng Yang
2009-02-16 23:23               ` Marcelo Tosatti
2009-02-17  1:35                 ` Sheng Yang
2009-02-17  1:36                   ` Sheng Yang
2009-02-11  8:08 ` [PATCH 2/3] KVM: Add gsi_msg_pending_bitmap for MSI-X Sheng Yang
2009-02-12 19:51   ` Marcelo Tosatti
2009-02-13  3:37     ` Sheng Yang
2009-02-13 17:10       ` Marcelo Tosatti
2009-02-17 18:09       ` Avi Kivity
2009-02-18  1:33         ` Sheng Yang
2009-02-11  8:08 ` [PATCH 3/3] KVM: Enable MSI-X for KVM assigned device Sheng Yang
2009-02-11 12:48   ` Avi Kivity
2009-02-12  6:03     ` Sheng Yang
2009-02-12 20:40   ` Marcelo Tosatti
2009-02-13  5:29     ` Sheng Yang
2009-02-13 17:13       ` Marcelo Tosatti

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.