[PATCH 0/11][v5] Enable MSI for KVM

[PATCH 0/11][v5] Enable MSI for KVM

[Sheng Yang]

Hi

Here is v5 for MSI on assigned devices, sorry for the delay. Due to the bad
weather, I got a badly cold recently, indeed feeling bad... :(

v4->v5

Addressed all the comments from Avi. I would work on the generic MSI solution
later as well. The most important one is a new modules paramter "msi2intx" can
disable MSI2INTx convert if necessary, which is prepare for someone ran into
trouble because of this.

The userspace patch would follow soon, I still need sometime to clean up current
device assignment userspace.

Comments are welcome!

Thanks!
--
regards
Yang, Sheng

[PATCH 01/11] KVM: Move ack notifier register and IRQ sourcd ID request

[Sheng Yang]

Signed-off-by: Sheng Yang <sheng@linux.intel.com>
---
 virt/kvm/kvm_main.c |   30 +++++++++++++++++++-----------
 1 files changed, 19 insertions(+), 11 deletions(-)

diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
index 4727c08..8966fd1 100644
--- a/virt/kvm/kvm_main.c
+++ b/virt/kvm/kvm_main.c
@@ -192,16 +192,31 @@ static int kvm_vm_ioctl_assign_irq(struct kvm *kvm,
 		return -EINVAL;
 	}
 
-	if (match->irq_requested) {
+	if (!match->irq_requested) {
+		INIT_WORK(&match->interrupt_work,
+				kvm_assigned_dev_interrupt_work_handler);
+		if (irqchip_in_kernel(kvm)) {
+			/* Register ack nofitier */
+			match->ack_notifier.gsi = -1;
+			match->ack_notifier.irq_acked =
+					kvm_assigned_dev_ack_irq;
+			kvm_register_irq_ack_notifier(kvm,
+					&match->ack_notifier);
+
+			/* Request IRQ source ID */
+			r = kvm_request_irq_source_id(kvm);
+			if (r < 0)
+				goto out_release;
+			else
+				match->irq_source_id = r;
+		}
+	} else {
 		match->guest_irq = assigned_irq->guest_irq;
 		match->ack_notifier.gsi = assigned_irq->guest_irq;
 		mutex_unlock(&kvm->lock);
 		return 0;
 	}
 
-	INIT_WORK(&match->interrupt_work,
-		  kvm_assigned_dev_interrupt_work_handler);
-
 	if (irqchip_in_kernel(kvm)) {
 		if (!capable(CAP_SYS_RAWIO)) {
 			r = -EPERM;
@@ -214,13 +229,6 @@ static int kvm_vm_ioctl_assign_irq(struct kvm *kvm,
 			match->host_irq = match->dev->irq;
 		match->guest_irq = assigned_irq->guest_irq;
 		match->ack_notifier.gsi = assigned_irq->guest_irq;
-		match->ack_notifier.irq_acked = kvm_assigned_dev_ack_irq;
-		kvm_register_irq_ack_notifier(kvm, &match->ack_notifier);
-		r = kvm_request_irq_source_id(kvm);
-		if (r < 0)
-			goto out_release;
-		else
-			match->irq_source_id = r;
 
 		/* Even though this is PCI, we don't want to use shared
 		 * interrupts. Sharing host devices with guest-assigned devices
-- 
1.5.4.5

[PATCH 02/11] KVM: Separate update irq to a single function

[Sheng Yang]

Signed-off-by: Sheng Yang <sheng@linux.intel.com>
---
 virt/kvm/kvm_main.c |   68 ++++++++++++++++++++++++++++----------------------
 1 files changed, 38 insertions(+), 30 deletions(-)

diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
index 8966fd1..ef2f03c 100644
--- a/virt/kvm/kvm_main.c
+++ b/virt/kvm/kvm_main.c
@@ -176,6 +176,41 @@ void kvm_free_all_assigned_devices(struct kvm *kvm)
 	}
 }
 
+static int assigned_device_update_intx(struct kvm *kvm,
+			struct kvm_assigned_dev_kernel *adev,
+			struct kvm_assigned_irq *airq)
+{
+	if (adev->irq_requested) {
+		adev->guest_irq = airq->guest_irq;
+		adev->ack_notifier.gsi = airq->guest_irq;
+		return 0;
+	}
+
+	if (irqchip_in_kernel(kvm)) {
+		if (!capable(CAP_SYS_RAWIO))
+			return -EPERM;
+
+		if (airq->host_irq)
+			adev->host_irq = airq->host_irq;
+		else
+			adev->host_irq = adev->dev->irq;
+		adev->guest_irq = airq->guest_irq;
+		adev->ack_notifier.gsi = airq->guest_irq;
+
+		/* Even though this is PCI, we don't want to use shared
+		 * interrupts. Sharing host devices with guest-assigned devices
+		 * on the same interrupt line is not a happy situation: there
+		 * are going to be long delays in accepting, acking, etc.
+		 */
+		if (request_irq(adev->host_irq, kvm_assigned_dev_intr,
+				0, "kvm_assigned_intx_device", (void *)adev))
+			return -EIO;
+	}
+
+	adev->irq_requested = true;
+	return 0;
+}
+
 static int kvm_vm_ioctl_assign_irq(struct kvm *kvm,
 				   struct kvm_assigned_irq
 				   *assigned_irq)
@@ -210,39 +245,12 @@ static int kvm_vm_ioctl_assign_irq(struct kvm *kvm,
 			else
 				match->irq_source_id = r;
 		}
-	} else {
-		match->guest_irq = assigned_irq->guest_irq;
-		match->ack_notifier.gsi = assigned_irq->guest_irq;
-		mutex_unlock(&kvm->lock);
-		return 0;
 	}
 
-	if (irqchip_in_kernel(kvm)) {
-		if (!capable(CAP_SYS_RAWIO)) {
-			r = -EPERM;
-			goto out_release;
-		}
-
-		if (assigned_irq->host_irq)
-			match->host_irq = assigned_irq->host_irq;
-		else
-			match->host_irq = match->dev->irq;
-		match->guest_irq = assigned_irq->guest_irq;
-		match->ack_notifier.gsi = assigned_irq->guest_irq;
-
-		/* Even though this is PCI, we don't want to use shared
-		 * interrupts. Sharing host devices with guest-assigned devices
-		 * on the same interrupt line is not a happy situation: there
-		 * are going to be long delays in accepting, acking, etc.
-		 */
-		if (request_irq(match->host_irq, kvm_assigned_dev_intr, 0,
-				"kvm_assigned_device", (void *)match)) {
-			r = -EIO;
-			goto out_release;
-		}
-	}
+	r = assigned_device_update_intx(kvm, match, assigned_irq);
+	if (r)
+		goto out_release;
 
-	match->irq_requested = true;
 	mutex_unlock(&kvm->lock);
 	return r;
 out_release:
-- 
1.5.4.5

[PATCH 03/11] KVM: Replace irq_requested with more generic irq_requested_type

[Sheng Yang]

Separate guest irq type and host irq type, for we can support guest using INTx
with host using MSI (but not opposite combination).

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

diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h
index 3a0fb77..c3d4b96 100644
--- a/include/linux/kvm_host.h
+++ b/include/linux/kvm_host.h
@@ -307,7 +307,9 @@ struct kvm_assigned_dev_kernel {
 	int host_devfn;
 	int host_irq;
 	int guest_irq;
-	int irq_requested;
+#define KVM_ASSIGNED_DEV_GUEST_INTX	(1 << 0)
+#define KVM_ASSIGNED_DEV_HOST_INTX	(1 << 8)
+	unsigned long irq_requested_type;
 	int irq_source_id;
 	struct pci_dev *dev;
 	struct kvm *kvm;
diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
index ef2f03c..638de47 100644
--- a/virt/kvm/kvm_main.c
+++ b/virt/kvm/kvm_main.c
@@ -140,7 +140,7 @@ static void kvm_free_assigned_device(struct kvm *kvm,
 				     struct kvm_assigned_dev_kernel
 				     *assigned_dev)
 {
-	if (irqchip_in_kernel(kvm) && assigned_dev->irq_requested)
+	if (irqchip_in_kernel(kvm) && assigned_dev->irq_requested_type)
 		free_irq(assigned_dev->host_irq, (void *)assigned_dev);
 
 	kvm_unregister_irq_ack_notifier(&assigned_dev->ack_notifier);
@@ -180,7 +180,7 @@ static int assigned_device_update_intx(struct kvm *kvm,
 			struct kvm_assigned_dev_kernel *adev,
 			struct kvm_assigned_irq *airq)
 {
-	if (adev->irq_requested) {
+	if (adev->irq_requested_type & KVM_ASSIGNED_DEV_GUEST_INTX) {
 		adev->guest_irq = airq->guest_irq;
 		adev->ack_notifier.gsi = airq->guest_irq;
 		return 0;
@@ -207,7 +207,8 @@ static int assigned_device_update_intx(struct kvm *kvm,
 			return -EIO;
 	}
 
-	adev->irq_requested = true;
+	adev->irq_requested_type = KVM_ASSIGNED_DEV_GUEST_INTX |
+				   KVM_ASSIGNED_DEV_HOST_INTX;
 	return 0;
 }
 
@@ -227,7 +228,7 @@ static int kvm_vm_ioctl_assign_irq(struct kvm *kvm,
 		return -EINVAL;
 	}
 
-	if (!match->irq_requested) {
+	if (!match->irq_requested_type) {
 		INIT_WORK(&match->interrupt_work,
 				kvm_assigned_dev_interrupt_work_handler);
 		if (irqchip_in_kernel(kvm)) {
-- 
1.5.4.5

[PATCH 04/11] KVM: Clean up assigned_device_update_irq

[Sheng Yang]

Signed-off-by: Sheng Yang <sheng@linux.intel.com>
---
 virt/kvm/kvm_main.c |   10 ++++------
 1 files changed, 4 insertions(+), 6 deletions(-)

diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
index 638de47..2089f8b 100644
--- a/virt/kvm/kvm_main.c
+++ b/virt/kvm/kvm_main.c
@@ -180,11 +180,11 @@ static int assigned_device_update_intx(struct kvm *kvm,
 			struct kvm_assigned_dev_kernel *adev,
 			struct kvm_assigned_irq *airq)
 {
-	if (adev->irq_requested_type & KVM_ASSIGNED_DEV_GUEST_INTX) {
-		adev->guest_irq = airq->guest_irq;
-		adev->ack_notifier.gsi = airq->guest_irq;
+	adev->guest_irq = airq->guest_irq;
+	adev->ack_notifier.gsi = airq->guest_irq;
+
+	if (adev->irq_requested_type & KVM_ASSIGNED_DEV_HOST_INTX)
 		return 0;
-	}
 
 	if (irqchip_in_kernel(kvm)) {
 		if (!capable(CAP_SYS_RAWIO))
@@ -194,8 +194,6 @@ static int assigned_device_update_intx(struct kvm *kvm,
 			adev->host_irq = airq->host_irq;
 		else
 			adev->host_irq = adev->dev->irq;
-		adev->guest_irq = airq->guest_irq;
-		adev->ack_notifier.gsi = airq->guest_irq;
 
 		/* Even though this is PCI, we don't want to use shared
 		 * interrupts. Sharing host devices with guest-assigned devices
-- 
1.5.4.5

[PATCH 05/11] KVM: Add fields for MSI device assignment

[Sheng Yang]

Prepared for kvm_arch_assigned_device_msi_dispatch().

Signed-off-by: Sheng Yang <sheng@linux.intel.com>
---
 include/linux/kvm.h      |    7 +++++++
 include/linux/kvm_host.h |    4 ++++
 2 files changed, 11 insertions(+), 0 deletions(-)

diff --git a/include/linux/kvm.h b/include/linux/kvm.h
index 44fd7fa..bb283c3 100644
--- a/include/linux/kvm.h
+++ b/include/linux/kvm.h
@@ -507,10 +507,17 @@ struct kvm_assigned_irq {
 	__u32 guest_irq;
 	__u32 flags;
 	union {
+		struct {
+			__u32 addr_lo;
+			__u32 addr_hi;
+			__u32 data;
+		} guest_msi;
 		__u32 reserved[12];
 	};
 };
 
 #define KVM_DEV_ASSIGN_ENABLE_IOMMU	(1 << 0)
 
+#define KVM_DEV_IRQ_ASSIGN_ENABLE_MSI	(1 << 0)
+
 #endif
diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h
index c3d4b96..8091a4d 100644
--- a/include/linux/kvm_host.h
+++ b/include/linux/kvm_host.h
@@ -16,6 +16,7 @@
 #include <linux/mm.h>
 #include <linux/preempt.h>
 #include <linux/marker.h>
+#include <linux/msi.h>
 #include <asm/signal.h>
 
 #include <linux/kvm.h>
@@ -307,8 +308,11 @@ struct kvm_assigned_dev_kernel {
 	int host_devfn;
 	int host_irq;
 	int guest_irq;
+	struct msi_msg guest_msi;
 #define KVM_ASSIGNED_DEV_GUEST_INTX	(1 << 0)
+#define KVM_ASSIGNED_DEV_GUEST_MSI	(1 << 1)
 #define KVM_ASSIGNED_DEV_HOST_INTX	(1 << 8)
+#define KVM_ASSIGNED_DEV_HOST_MSI	(1 << 9)
 	unsigned long irq_requested_type;
 	int irq_source_id;
 	struct pci_dev *dev;
-- 
1.5.4.5

[PATCH 06/11] KVM: Export ioapic_get_delivery_bitmask

[Sheng Yang]

It would be used for MSI in device assignment, for MSI dispatch.

Signed-off-by: Sheng Yang <sheng@linux.intel.com>
---
 virt/kvm/ioapic.c |    7 ++++---
 virt/kvm/ioapic.h |    2 ++
 2 files changed, 6 insertions(+), 3 deletions(-)

diff --git a/virt/kvm/ioapic.c b/virt/kvm/ioapic.c
index c8f939c..23b81cf 100644
--- a/virt/kvm/ioapic.c
+++ b/virt/kvm/ioapic.c
@@ -153,8 +153,8 @@ static void ioapic_inj_nmi(struct kvm_vcpu *vcpu)
 	kvm_vcpu_kick(vcpu);
 }
 
-static u32 ioapic_get_delivery_bitmask(struct kvm_ioapic *ioapic, u8 dest,
-				       u8 dest_mode)
+u32 kvm_ioapic_get_delivery_bitmask(struct kvm_ioapic *ioapic, u8 dest,
+				    u8 dest_mode)
 {
 	u32 mask = 0;
 	int i;
@@ -208,7 +208,8 @@ static int ioapic_deliver(struct kvm_ioapic *ioapic, int irq)
 		     "vector=%x trig_mode=%x\n",
 		     dest, dest_mode, delivery_mode, vector, trig_mode);
 
-	deliver_bitmask = ioapic_get_delivery_bitmask(ioapic, dest, dest_mode);
+	deliver_bitmask = kvm_ioapic_get_delivery_bitmask(ioapic, dest,
+							  dest_mode);
 	if (!deliver_bitmask) {
 		ioapic_debug("no target on destination\n");
 		return 0;
diff --git a/virt/kvm/ioapic.h b/virt/kvm/ioapic.h
index cd7ae76..49c9581 100644
--- a/virt/kvm/ioapic.h
+++ b/virt/kvm/ioapic.h
@@ -85,5 +85,7 @@ void kvm_ioapic_update_eoi(struct kvm *kvm, int vector, int trigger_mode);
 int kvm_ioapic_init(struct kvm *kvm);
 void kvm_ioapic_set_irq(struct kvm_ioapic *ioapic, int irq, int level);
 void kvm_ioapic_reset(struct kvm_ioapic *ioapic);
+u32 kvm_ioapic_get_delivery_bitmask(struct kvm_ioapic *ioapic, u8 dest,
+				u8 dest_mode);
 
 #endif
-- 
1.5.4.5

[PATCH 07/11] x86: Rename MSI macro name

[Sheng Yang]

Signed-off-by: Sheng Yang <sheng@linux.intel.com>
---
 arch/x86/include/asm/msidef.h |    4 ++--
 arch/x86/kernel/io_apic.c     |    4 ++--
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/arch/x86/include/asm/msidef.h b/arch/x86/include/asm/msidef.h
index 6706b30..988cb27 100644
--- a/arch/x86/include/asm/msidef.h
+++ b/arch/x86/include/asm/msidef.h
@@ -15,8 +15,8 @@
 					 MSI_DATA_VECTOR_MASK)
 
 #define MSI_DATA_DELIVERY_MODE_SHIFT	8
-#define  MSI_DATA_DELIVERY_FIXED	(0 << MSI_DATA_DELIVERY_MODE_SHIFT)
-#define  MSI_DATA_DELIVERY_LOWPRI	(1 << MSI_DATA_DELIVERY_MODE_SHIFT)
+#define  MSI_DATA_DELIVERY_FIXED_BIT	(0 << MSI_DATA_DELIVERY_MODE_SHIFT)
+#define  MSI_DATA_DELIVERY_LOWPRI_BIT	(1 << MSI_DATA_DELIVERY_MODE_SHIFT)
 
 #define MSI_DATA_LEVEL_SHIFT		14
 #define	 MSI_DATA_LEVEL_DEASSERT	(0 << MSI_DATA_LEVEL_SHIFT)
diff --git a/arch/x86/kernel/io_apic.c b/arch/x86/kernel/io_apic.c
index 7a3f202..9cf604d 100644
--- a/arch/x86/kernel/io_apic.c
+++ b/arch/x86/kernel/io_apic.c
@@ -3005,8 +3005,8 @@ static int msi_compose_msg(struct pci_dev *pdev, unsigned int irq, struct msi_ms
 			MSI_DATA_TRIGGER_EDGE |
 			MSI_DATA_LEVEL_ASSERT |
 			((INT_DELIVERY_MODE != dest_LowestPrio) ?
-				MSI_DATA_DELIVERY_FIXED:
-				MSI_DATA_DELIVERY_LOWPRI) |
+				MSI_DATA_DELIVERY_FIXED_BIT:
+				MSI_DATA_DELIVERY_LOWPRI_BIT) |
 			MSI_DATA_VECTOR(cfg->vector);
 	}
 	return err;
-- 
1.5.4.5

[PATCH 08/11] x86: Add MSI delivery mode value

[Sheng Yang]

Signed-off-by: Sheng Yang <sheng@linux.intel.com>
---
 arch/x86/include/asm/msidef.h |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/arch/x86/include/asm/msidef.h b/arch/x86/include/asm/msidef.h
index 988cb27..412f94e 100644
--- a/arch/x86/include/asm/msidef.h
+++ b/arch/x86/include/asm/msidef.h
@@ -17,6 +17,8 @@
 #define MSI_DATA_DELIVERY_MODE_SHIFT	8
 #define  MSI_DATA_DELIVERY_FIXED_BIT	(0 << MSI_DATA_DELIVERY_MODE_SHIFT)
 #define  MSI_DATA_DELIVERY_LOWPRI_BIT	(1 << MSI_DATA_DELIVERY_MODE_SHIFT)
+#define  MSI_DATA_DELIVERY_FIXED	0
+#define  MSI_DATA_DELIVERY_LOWPRI	1
 
 #define MSI_DATA_LEVEL_SHIFT		14
 #define	 MSI_DATA_LEVEL_DEASSERT	(0 << MSI_DATA_LEVEL_SHIFT)
-- 
1.5.4.5

[PATCH 09/11] KVM: Add assigned_device_msi_dispatch()

[Sheng Yang]

The function is used to dispatch MSI to lapic according to MSI message
address and message data.

Signed-off-by: Sheng Yang <sheng@linux.intel.com>
---
 virt/kvm/kvm_main.c |   54 +++++++++++++++++++++++++++++++++++++++++++++++++++
 1 files changed, 54 insertions(+), 0 deletions(-)

diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
index 2089f8b..c41488f 100644
--- a/virt/kvm/kvm_main.c
+++ b/virt/kvm/kvm_main.c
@@ -47,6 +47,10 @@
 #include <asm/uaccess.h>
 #include <asm/pgtable.h>
 
+#ifdef CONFIG_X86
+#include <asm/msidef.h>
+#endif
+
 #ifdef KVM_COALESCED_MMIO_PAGE_OFFSET
 #include "coalesced_mmio.h"
 #endif
@@ -78,6 +82,56 @@ static long kvm_vcpu_ioctl(struct file *file, unsigned int ioctl,
 bool kvm_rebooting;
 
 #ifdef KVM_CAP_DEVICE_ASSIGNMENT
+
+#ifdef CONFIG_X86
+static void assigned_device_msi_dispatch(struct kvm_assigned_dev_kernel *dev)
+{
+	int vcpu_id;
+	struct kvm_vcpu *vcpu;
+	struct kvm_ioapic *ioapic = ioapic_irqchip(dev->kvm);
+	int dest_id = (dev->guest_msi.address_lo & MSI_ADDR_DEST_ID_MASK)
+			>> MSI_ADDR_DEST_ID_SHIFT;
+	int vector = (dev->guest_msi.data & MSI_DATA_VECTOR_MASK)
+			>> MSI_DATA_VECTOR_SHIFT;
+	int dest_mode = test_bit(MSI_ADDR_DEST_MODE_SHIFT,
+				(unsigned long *)&dev->guest_msi.address_lo);
+	int trig_mode = test_bit(MSI_DATA_TRIGGER_SHIFT,
+				(unsigned long *)&dev->guest_msi.data);
+	int delivery_mode = test_bit(MSI_DATA_DELIVERY_MODE_SHIFT,
+				(unsigned long *)&dev->guest_msi.data);
+	u32 deliver_bitmask;
+
+	BUG_ON(!ioapic);
+
+	deliver_bitmask = kvm_ioapic_get_delivery_bitmask(ioapic,
+				dest_id, dest_mode);
+	switch (delivery_mode) {
+	case MSI_DATA_DELIVERY_LOWPRI:
+		vcpu = kvm_get_lowest_prio_vcpu(ioapic->kvm, vector,
+				deliver_bitmask);
+		if (vcpu != NULL)
+			kvm_apic_set_irq(vcpu, vector, trig_mode);
+		else
+			printk(KERN_INFO "kvm: null lowest priority vcpu!\n");
+		break;
+	case MSI_DATA_DELIVERY_FIXED:
+		for (vcpu_id = 0; deliver_bitmask != 0; vcpu_id++) {
+			if (!(deliver_bitmask & (1 << vcpu_id)))
+				continue;
+			deliver_bitmask &= ~(1 << vcpu_id);
+			vcpu = ioapic->kvm->vcpus[vcpu_id];
+			if (vcpu)
+				kvm_apic_set_irq(vcpu, vector, trig_mode);
+		}
+		break;
+	default:
+		printk(KERN_INFO "kvm: unsupported MSI delivery mode\n");
+	}
+}
+#else
+static void assigned_device_msi_dispatch(struct kvm_assigned_dev_kernel *dev) {}
+#endif
+
 static struct kvm_assigned_dev_kernel *kvm_find_assigned_dev(struct list_head *head,
 						      int assigned_dev_id)
 {
-- 
1.5.4.5

[PATCH 10/11] KVM: Enable MSI for device assignment

[Sheng Yang]

We enable guest MSI and host MSI support in this patch. The userspace want to
enable MSI should set KVM_DEV_IRQ_ASSIGN_ENABLE_MSI in the assigned_irq's flag.
Function would return -ENOTTY if can't enable MSI, userspace shouldn't set MSI
Enable bit when KVM_ASSIGN_IRQ return -ENOTTY with
KVM_DEV_IRQ_ASSIGN_ENABLE_MSI.

Userspace can tell the support of MSI device from #ifdef KVM_CAP_DEVICE_MSI.

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

diff --git a/include/linux/kvm.h b/include/linux/kvm.h
index bb283c3..e7dae05 100644
--- a/include/linux/kvm.h
+++ b/include/linux/kvm.h
@@ -392,6 +392,9 @@ struct kvm_trace_rec {
 #endif
 #define KVM_CAP_IOMMU 18
 #define KVM_CAP_NMI 19
+#if defined(CONFIG_X86)||defined(CONFIG_IA64)
+#define KVM_CAP_DEVICE_MSI 20
+#endif
 
 /*
  * ioctls for VM fds
diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
index c41488f..8e0b599 100644
--- a/virt/kvm/kvm_main.c
+++ b/virt/kvm/kvm_main.c
@@ -158,9 +158,15 @@ static void kvm_assigned_dev_interrupt_work_handler(struct work_struct *work)
 	 * 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);
+	if (assigned_dev->irq_requested_type & KVM_ASSIGNED_DEV_GUEST_INTX)
+		kvm_set_irq(assigned_dev->kvm,
+			    assigned_dev->irq_source_id,
+			    assigned_dev->guest_irq, 1);
+	else if (assigned_dev->irq_requested_type &
+				KVM_ASSIGNED_DEV_GUEST_MSI) {
+		assigned_device_msi_dispatch(assigned_dev);
+		enable_irq(assigned_dev->host_irq);
+	}
 	mutex_unlock(&assigned_dev->kvm->lock);
 	kvm_put_kvm(assigned_dev->kvm);
 }
@@ -196,6 +202,8 @@ static void kvm_free_assigned_device(struct kvm *kvm,
 {
 	if (irqchip_in_kernel(kvm) && assigned_dev->irq_requested_type)
 		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);
 
 	kvm_unregister_irq_ack_notifier(&assigned_dev->ack_notifier);
 	kvm_free_irq_source_id(kvm, assigned_dev->irq_source_id);
@@ -241,6 +249,11 @@ static int assigned_device_update_intx(struct kvm *kvm,
 		return 0;
 
 	if (irqchip_in_kernel(kvm)) {
+		if (adev->irq_requested_type & KVM_ASSIGNED_DEV_HOST_MSI) {
+			free_irq(adev->host_irq, (void *)kvm);
+			pci_disable_msi(adev->dev);
+		}
+
 		if (!capable(CAP_SYS_RAWIO))
 			return -EPERM;
 
@@ -264,6 +277,41 @@ static int assigned_device_update_intx(struct kvm *kvm,
 	return 0;
 }
 
+#ifdef CONFIG_X86
+static int assigned_device_update_msi(struct kvm *kvm,
+			struct kvm_assigned_dev_kernel *adev,
+			struct kvm_assigned_irq *airq)
+{
+	int r;
+
+	/* x86 don't care upper address of guest msi message addr */
+	adev->guest_msi.address_lo = airq->guest_msi.addr_lo;
+	adev->guest_msi.data = airq->guest_msi.data;
+	adev->ack_notifier.gsi = -1;
+
+	if (adev->irq_requested_type & KVM_ASSIGNED_DEV_HOST_MSI)
+		return 0;
+
+	if (irqchip_in_kernel(kvm)) {
+		if (adev->irq_requested_type & KVM_ASSIGNED_DEV_HOST_INTX)
+			free_irq(adev->host_irq, (void *)adev);
+
+		r = pci_enable_msi(adev->dev);
+		if (r)
+			return r;
+
+		adev->host_irq = adev->dev->irq;
+		if (request_irq(adev->host_irq, kvm_assigned_dev_intr, 0,
+				"kvm_assigned_msi_device", (void *)adev))
+			return -EIO;
+	}
+
+	adev->irq_requested_type = KVM_ASSIGNED_DEV_GUEST_MSI |
+				   KVM_ASSIGNED_DEV_HOST_MSI;
+	return 0;
+}
+#endif
+
 static int kvm_vm_ioctl_assign_irq(struct kvm *kvm,
 				   struct kvm_assigned_irq
 				   *assigned_irq)
@@ -300,9 +348,30 @@ static int kvm_vm_ioctl_assign_irq(struct kvm *kvm,
 		}
 	}
 
-	r = assigned_device_update_intx(kvm, match, assigned_irq);
-	if (r)
-		goto out_release;
+	if (assigned_irq->flags & KVM_DEV_IRQ_ASSIGN_ENABLE_MSI) {
+#ifdef CONFIG_X86
+		r = assigned_device_update_msi(kvm, match, assigned_irq);
+		if (r) {
+			printk(KERN_WARNING "kvm: failed to enable "
+					"MSI device!\n");
+			goto out_release;
+		}
+#else
+		r = -ENOTTY;
+#endif
+	} else if (assigned_irq->host_irq == 0 && match->dev->irq == 0) {
+		/* Host device IRQ 0 means don't support INTx */
+		printk(KERN_WARNING "kvm: wait device to enable MSI!\n");
+		r = 0;
+	} else {
+		/* Non-sharing INTx mode */
+		r = assigned_device_update_intx(kvm, match, assigned_irq);
+		if (r) {
+			printk(KERN_WARNING "kvm: failed to enable "
+					"INTx device!\n");
+			goto out_release;
+		}
+	}
 
 	mutex_unlock(&kvm->lock);
 	return r;
-- 
1.5.4.5

[PATCH 11/11] KVM: MSI to INTx translate

[Sheng Yang]

Now we use MSI as default one, and translate MSI to INTx when guest need
INTx rather than MSI. For legacy device, we provide support for non-sharing
host IRQ.

Provide a parameter msi2intx for this method. The value is true by default.

Signed-off-by: Sheng Yang <sheng@linux.intel.com>
---
 virt/kvm/kvm_main.c |   67 ++++++++++++++++++++++++++++++++++++++------------
 1 files changed, 51 insertions(+), 16 deletions(-)

diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
index 8e0b599..98f145d 100644
--- a/virt/kvm/kvm_main.c
+++ b/virt/kvm/kvm_main.c
@@ -64,6 +64,9 @@
 MODULE_AUTHOR("Qumranet");
 MODULE_LICENSE("GPL");
 
+static int msi2intx = 1;
+module_param(msi2intx, bool, 0);
+
 DEFINE_SPINLOCK(kvm_lock);
 LIST_HEAD(vm_list);
 
@@ -249,7 +252,8 @@ static int assigned_device_update_intx(struct kvm *kvm,
 		return 0;
 
 	if (irqchip_in_kernel(kvm)) {
-		if (adev->irq_requested_type & KVM_ASSIGNED_DEV_HOST_MSI) {
+		if (!msi2intx &&
+		    adev->irq_requested_type & KVM_ASSIGNED_DEV_HOST_MSI) {
 			free_irq(adev->host_irq, (void *)kvm);
 			pci_disable_msi(adev->dev);
 		}
@@ -284,21 +288,33 @@ static int assigned_device_update_msi(struct kvm *kvm,
 {
 	int r;
 
-	/* x86 don't care upper address of guest msi message addr */
-	adev->guest_msi.address_lo = airq->guest_msi.addr_lo;
-	adev->guest_msi.data = airq->guest_msi.data;
-	adev->ack_notifier.gsi = -1;
+	if (airq->flags & KVM_DEV_IRQ_ASSIGN_ENABLE_MSI) {
+		/* x86 don't care upper address of guest msi message addr */
+		adev->irq_requested_type |= KVM_ASSIGNED_DEV_GUEST_MSI;
+		adev->irq_requested_type &= ~KVM_ASSIGNED_DEV_GUEST_INTX;
+		adev->guest_msi.address_lo = airq->guest_msi.addr_lo;
+		adev->guest_msi.data = airq->guest_msi.data;
+		adev->ack_notifier.gsi = -1;
+	} else if (msi2intx) {
+		adev->irq_requested_type |= KVM_ASSIGNED_DEV_GUEST_INTX;
+		adev->irq_requested_type &= ~KVM_ASSIGNED_DEV_GUEST_MSI;
+		adev->guest_irq = airq->guest_irq;
+		adev->ack_notifier.gsi = airq->guest_irq;
+	}
 
 	if (adev->irq_requested_type & KVM_ASSIGNED_DEV_HOST_MSI)
 		return 0;
 
 	if (irqchip_in_kernel(kvm)) {
-		if (adev->irq_requested_type & KVM_ASSIGNED_DEV_HOST_INTX)
-			free_irq(adev->host_irq, (void *)adev);
-
-		r = pci_enable_msi(adev->dev);
-		if (r)
-			return r;
+		if (!msi2intx) {
+			if (adev->irq_requested_type &
+					KVM_ASSIGNED_DEV_HOST_INTX)
+				free_irq(adev->host_irq, (void *)adev);
+
+			r = pci_enable_msi(adev->dev);
+			if (r)
+				return r;
+		}
 
 		adev->host_irq = adev->dev->irq;
 		if (request_irq(adev->host_irq, kvm_assigned_dev_intr, 0,
@@ -306,8 +322,10 @@ static int assigned_device_update_msi(struct kvm *kvm,
 			return -EIO;
 	}
 
-	adev->irq_requested_type = KVM_ASSIGNED_DEV_GUEST_MSI |
-				   KVM_ASSIGNED_DEV_HOST_MSI;
+	if (!msi2intx)
+		adev->irq_requested_type = KVM_ASSIGNED_DEV_GUEST_MSI;
+
+	adev->irq_requested_type |= KVM_ASSIGNED_DEV_HOST_MSI;
 	return 0;
 }
 #endif
@@ -345,10 +363,19 @@ static int kvm_vm_ioctl_assign_irq(struct kvm *kvm,
 				goto out_release;
 			else
 				match->irq_source_id = r;
+
+#ifdef CONFIG_X86
+			/* Determine host device irq type, we can know the
+			 * result from dev->msi_enabled */
+			if (msi2intx)
+				pci_enable_msi(match->dev);
+#endif
 		}
 	}
 
-	if (assigned_irq->flags & KVM_DEV_IRQ_ASSIGN_ENABLE_MSI) {
+	if ((!msi2intx &&
+	     (assigned_irq->flags & KVM_DEV_IRQ_ASSIGN_ENABLE_MSI)) ||
+	    (msi2intx && match->dev->msi_enabled)) {
 #ifdef CONFIG_X86
 		r = assigned_device_update_msi(kvm, match, assigned_irq);
 		if (r) {
@@ -361,8 +388,16 @@ static int kvm_vm_ioctl_assign_irq(struct kvm *kvm,
 #endif
 	} else if (assigned_irq->host_irq == 0 && match->dev->irq == 0) {
 		/* Host device IRQ 0 means don't support INTx */
-		printk(KERN_WARNING "kvm: wait device to enable MSI!\n");
-		r = 0;
+		if (!msi2intx) {
+			printk(KERN_WARNING
+			       "kvm: wait device to enable MSI!\n");
+			r = 0;
+		} else {
+			printk(KERN_WARNING
+			       "kvm: failed to enable MSI device!\n");
+			r = -ENOTTY;
+			goto out_release;
+		}
 	} else {
 		/* Non-sharing INTx mode */
 		r = assigned_device_update_intx(kvm, match, assigned_irq);
-- 
1.5.4.5

Re: [PATCH 01/11] KVM: Move ack notifier register and IRQ sourcd ID request

[Avi Kivity]

Sheng Yang wrote:
> Signed-off-by: Sheng Yang <sheng@linux.intel.com>
> ---
>  virt/kvm/kvm_main.c |   30 +++++++++++++++++++-----------
>  1 files changed, 19 insertions(+), 11 deletions(-)
>   

Why is this needed?  comment for the changelog.


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

Re: [PATCH 02/11] KVM: Separate update irq to a single function

[Avi Kivity]

Sheng Yang wrote:
> Signed-off-by: Sheng Yang <sheng@linux.intel.com>
>   

Again, missing explanation in changelog comment.

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

Re: [PATCH 03/11] KVM: Replace irq_requested with more generic irq_requested_type

[Avi Kivity]

Sheng Yang wrote:
> Separate guest irq type and host irq type, for we can support guest using INTx
> with host using MSI (but not opposite combination).
>
> Signed-off-by: Sheng Yang <sheng@linux.intel.com>
> ---
>  include/linux/kvm_host.h |    4 +++-
>  virt/kvm/kvm_main.c      |    9 +++++----
>  2 files changed, 8 insertions(+), 5 deletions(-)
>
> diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h
> index 3a0fb77..c3d4b96 100644
> --- a/include/linux/kvm_host.h
> +++ b/include/linux/kvm_host.h
> @@ -307,7 +307,9 @@ struct kvm_assigned_dev_kernel {
>  	int host_devfn;
>  	int host_irq;
>  	int guest_irq;
> -	int irq_requested;
> +#define KVM_ASSIGNED_DEV_GUEST_INTX	(1 << 0)
> +#define KVM_ASSIGNED_DEV_HOST_INTX	(1 << 8)
> +	unsigned long irq_requested_type;
>  	int irq_source_id;
>  	struct pci_dev *dev

Any particular reason the bits were not assigned sequentially?

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

Re: [PATCH 05/11] KVM: Add fields for MSI device assignment

[Avi Kivity]

Sheng Yang wrote:
> Prepared for kvm_arch_assigned_device_msi_dispatch().
>
> @@ -507,10 +507,17 @@ struct kvm_assigned_irq {
>  	__u32 guest_irq;
>  	__u32 flags;
>  	union {
> +		struct {
> +			__u32 addr_lo;
> +			__u32 addr_hi;
>   

__u64 addr;

?

> +			__u32 data;
>   


> @@ -307,8 +308,11 @@ struct kvm_assigned_dev_kernel {
>  	int host_devfn;
>  	int host_irq;
>  	int guest_irq;
> +	struct msi_msg guest_msi;
>  #define KVM_ASSIGNED_DEV_GUEST_INTX	(1 << 0)
> +#define KVM_ASSIGNED_DEV_GUEST_MSI	(1 << 1)
>  #define KVM_ASSIGNED_DEV_HOST_INTX	(1 << 8)
> +#define KVM_ASSIGNED_DEV_HOST_MSI	(1 << 9)
>   

Okay, I see the reason for non sequential assignment.

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

Re: [PATCH 07/11] x86: Rename MSI macro name

[Avi Kivity]

Sheng Yang wrote:
> Signed-off-by: Sheng Yang <sheng@linux.intel.com>
> ---
>  arch/x86/include/asm/msidef.h |    4 ++--
>  arch/x86/kernel/io_apic.c     |    4 ++--
>  2 files changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/arch/x86/include/asm/msidef.h b/arch/x86/include/asm/msidef.h
> index 6706b30..988cb27 100644
> --- a/arch/x86/include/asm/msidef.h
> +++ b/arch/x86/include/asm/msidef.h
> @@ -15,8 +15,8 @@
>  					 MSI_DATA_VECTOR_MASK)
>  
>  #define MSI_DATA_DELIVERY_MODE_SHIFT	8
> -#define  MSI_DATA_DELIVERY_FIXED	(0 << MSI_DATA_DELIVERY_MODE_SHIFT)
> -#define  MSI_DATA_DELIVERY_LOWPRI	(1 << MSI_DATA_DELIVERY_MODE_SHIFT)
> +#define  MSI_DATA_DELIVERY_FIXED_BIT	(0 << MSI_DATA_DELIVERY_MODE_SHIFT)
> +#define  MSI_DATA_DELIVERY_LOWPRI_BIT	(1 << MSI_DATA_DELIVERY_MODE_SHIFT)
>   

These are usually named _MASK, not _BIT.  But I recommend dropping this 
patch, it can only cause conflicts with other development.

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

Re: [PATCH 09/11] KVM: Add assigned_device_msi_dispatch()

[Avi Kivity]

Sheng Yang wrote:
> The function is used to dispatch MSI to lapic according to MSI message
> address and message data.
>
> +
> +	deliver_bitmask = kvm_ioapic_get_delivery_bitmask(ioapic,
> +				dest_id, dest_mode);
> +	switch (delivery_mode) {
> +	case MSI_DATA_DELIVERY_LOWPRI:
> +		vcpu = kvm_get_lowest_prio_vcpu(ioapic->kvm, vector,
> +				deliver_bitmask);
> +		if (vcpu != NULL)
> +			kvm_apic_set_irq(vcpu, vector, trig_mode);
> +		else
> +			printk(KERN_INFO "kvm: null lowest priority vcpu!\n");
> +		break;
> +	case MSI_DATA_DELIVERY_FIXED:
> +		for (vcpu_id = 0; deliver_bitmask != 0; vcpu_id++) {
> +			if (!(deliver_bitmask & (1 << vcpu_id)))
> +				continue;
> +			deliver_bitmask &= ~(1 << vcpu_id);
> +			vcpu = ioapic->kvm->vcpus[vcpu_id];
> +			if (vcpu)
> +				kvm_apic_set_irq(vcpu, vector, trig_mode);
> +		}
> +		break;
> +	default:
>   

This duplicates the ioapic code.  That's fine for now, but eventually 
we'll want to refactor this.


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

Re: [PATCH 10/11] KVM: Enable MSI for device assignment

[Avi Kivity]

Sheng Yang wrote:
> We enable guest MSI and host MSI support in this patch. The userspace want to
> enable MSI should set KVM_DEV_IRQ_ASSIGN_ENABLE_MSI in the assigned_irq's flag.
> Function would return -ENOTTY if can't enable MSI, userspace shouldn't set MSI
> Enable bit when KVM_ASSIGN_IRQ return -ENOTTY with
> KVM_DEV_IRQ_ASSIGN_ENABLE_MSI.
>
> Userspace can tell the support of MSI device from #ifdef KVM_CAP_DEVICE_MSI.
>
> Signed-off-by: Sheng Yang <sheng@linux.intel.com>
> ---
>  include/linux/kvm.h |    3 ++
>  virt/kvm/kvm_main.c |   81 +++++++++++++++++++++++++++++++++++++++++++++++----
>  2 files changed, 78 insertions(+), 6 deletions(-)
>
> diff --git a/include/linux/kvm.h b/include/linux/kvm.h
> index bb283c3..e7dae05 100644
> --- a/include/linux/kvm.h
> +++ b/include/linux/kvm.h
> @@ -392,6 +392,9 @@ struct kvm_trace_rec {
>  #endif
>  #define KVM_CAP_IOMMU 18
>  #define KVM_CAP_NMI 19
> +#if defined(CONFIG_X86)||defined(CONFIG_IA64)
> +#define KVM_CAP_DEVICE_MSI 20
> +#endif
>   

Since the code only enables x86 for now, please drop ia64.

> diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
> index c41488f..8e0b599 100644
> --- a/virt/kvm/kvm_main.c
> +++ b/virt/kvm/kvm_main.c
> @@ -158,9 +158,15 @@ static void kvm_assigned_dev_interrupt_work_handler(struct work_struct *work)
>  	 * 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);
> +	if (assigned_dev->irq_requested_type & KVM_ASSIGNED_DEV_GUEST_INTX)
> +		kvm_set_irq(assigned_dev->kvm,
> +			    assigned_dev->irq_source_id,
> +			    assigned_dev->guest_irq, 1);
> +	else if (assigned_dev->irq_requested_type &
> +				KVM_ASSIGNED_DEV_GUEST_MSI) {
> +		assigned_device_msi_dispatch(assigned_dev);
> +		enable_irq(assigned_dev->host_irq);
> +	}
>   

Please move this logic to kvm_set_irq().  Hmm, that's not possible right 
now, so we can leave it as is for now.


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

Re: [PATCH 05/11] KVM: Add fields for MSI device assignment

[Sheng Yang]

On Sunday 23 November 2008 18:10:47 Avi Kivity wrote:
> Sheng Yang wrote:
> > Prepared for kvm_arch_assigned_device_msi_dispatch().
> >
> > @@ -507,10 +507,17 @@ struct kvm_assigned_irq {
> >  	__u32 guest_irq;
> >  	__u32 flags;
> >  	union {
> > +		struct {
> > +			__u32 addr_lo;
> > +			__u32 addr_hi;
>
> __u64 addr;
>
> ?

Here I followed the spec that distinguish the "Message Address" and "Message 
Upper address". And the native Linux structure:

struct msi_msg {
	u32	address_lo;	/* low 32 bits of msi message address */
	u32	address_hi;	/* high 32 bits of msi message address */
	u32	data;		/* 16 bits of msi message data */
};

For now, we needn't care about address_hi. I can only see address_hi used in 
hypertransport part... So I think keep it independence here is OK.

(In fact, PCI spec defined message data length is u64, but as you see, now 
msi_msg for whole Linux only have u32...)

-- 
regards
Yang, Sheng

>
> > +			__u32 data;
> >
> >
> >
> > @@ -307,8 +308,11 @@ struct kvm_assigned_dev_kernel {
> >  	int host_devfn;
> >  	int host_irq;
> >  	int guest_irq;
> > +	struct msi_msg guest_msi;
> >  #define KVM_ASSIGNED_DEV_GUEST_INTX	(1 << 0)
> > +#define KVM_ASSIGNED_DEV_GUEST_MSI	(1 << 1)
> >  #define KVM_ASSIGNED_DEV_HOST_INTX	(1 << 8)
> > +#define KVM_ASSIGNED_DEV_HOST_MSI	(1 << 9)
>
> Okay, I see the reason for non sequential assignment.

Re: [PATCH 07/11] x86: Rename MSI macro name

[Sheng Yang]

On Sunday 23 November 2008 18:12:54 Avi Kivity wrote:
> Sheng Yang wrote:
> > Signed-off-by: Sheng Yang <sheng@linux.intel.com>
> > ---
> >  arch/x86/include/asm/msidef.h |    4 ++--
> >  arch/x86/kernel/io_apic.c     |    4 ++--
> >  2 files changed, 4 insertions(+), 4 deletions(-)
> >
> > diff --git a/arch/x86/include/asm/msidef.h
> > b/arch/x86/include/asm/msidef.h index 6706b30..988cb27 100644
> > --- a/arch/x86/include/asm/msidef.h
> > +++ b/arch/x86/include/asm/msidef.h
> > @@ -15,8 +15,8 @@
> >  					 MSI_DATA_VECTOR_MASK)
> >
> >  #define MSI_DATA_DELIVERY_MODE_SHIFT	8
> > -#define  MSI_DATA_DELIVERY_FIXED	(0 << MSI_DATA_DELIVERY_MODE_SHIFT)
> > -#define  MSI_DATA_DELIVERY_LOWPRI	(1 << MSI_DATA_DELIVERY_MODE_SHIFT)
> > +#define  MSI_DATA_DELIVERY_FIXED_BIT	(0 << MSI_DATA_DELIVERY_MODE_SHIFT)
> > +#define  MSI_DATA_DELIVERY_LOWPRI_BIT	(1 <<
> > MSI_DATA_DELIVERY_MODE_SHIFT)
>
> These are usually named _MASK, not _BIT.  But I recommend dropping this
> patch, it can only cause conflicts with other development.

Yeah, I also don't like this. It's easily cause chaos... I will compromise on 
it, use IOAPIC_LOWEST_PRIORITY and IOAPIC_FIXED for now, and drop the related 
two patches.

-- 
regards
Yang, Sheng

Re: [PATCH 09/11] KVM: Add assigned_device_msi_dispatch()

[Sheng Yang]

On Sunday 23 November 2008 18:22:47 Avi Kivity wrote:
> Sheng Yang wrote:
> > The function is used to dispatch MSI to lapic according to MSI message
> > address and message data.
> >
> > +
> > +	deliver_bitmask = kvm_ioapic_get_delivery_bitmask(ioapic,
> > +				dest_id, dest_mode);
> > +	switch (delivery_mode) {
> > +	case MSI_DATA_DELIVERY_LOWPRI:
> > +		vcpu = kvm_get_lowest_prio_vcpu(ioapic->kvm, vector,
> > +				deliver_bitmask);
> > +		if (vcpu != NULL)
> > +			kvm_apic_set_irq(vcpu, vector, trig_mode);
> > +		else
> > +			printk(KERN_INFO "kvm: null lowest priority vcpu!\n");
> > +		break;
> > +	case MSI_DATA_DELIVERY_FIXED:
> > +		for (vcpu_id = 0; deliver_bitmask != 0; vcpu_id++) {
> > +			if (!(deliver_bitmask & (1 << vcpu_id)))
> > +				continue;
> > +			deliver_bitmask &= ~(1 << vcpu_id);
> > +			vcpu = ioapic->kvm->vcpus[vcpu_id];
> > +			if (vcpu)
> > +				kvm_apic_set_irq(vcpu, vector, trig_mode);
> > +		}
> > +		break;
> > +	default:
>
> This duplicates the ioapic code.  That's fine for now, but eventually
> we'll want to refactor this.

Sure. Put on my todo list... 

-- 
regards
Yang, Sheng

Re: [PATCH 05/11] KVM: Add fields for MSI device assignment

[Avi Kivity]

Sheng Yang wrote:
>>>  	union {
>>> +		struct {
>>> +			__u32 addr_lo;
>>> +			__u32 addr_hi;
>>>       
>> __u64 addr;
>>
>> ?
>>     
>
> Here I followed the spec that distinguish the "Message Address" and "Message 
> Upper address". And the native Linux structure:
>
> struct msi_msg {
> 	u32	address_lo;	/* low 32 bits of msi message address */
> 	u32	address_hi;	/* high 32 bits of msi message address */
> 	u32	data;		/* 16 bits of msi message data */
> };
>
> For now, we needn't care about address_hi. I can only see address_hi used in 
> hypertransport part... So I think keep it independence here is OK.
>
>   

Fair enough, documentation wins.

> (In fact, PCI spec defined message data length is u64, but as you see, now 
> msi_msg for whole Linux only have u32...)
>   

Well in that case please define data as __u64, so we don't have 
surprises later on.

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

Re: [PATCH 05/11] KVM: Add fields for MSI device assignment

[Avi Kivity]

Avi Kivity wrote:
>> (In fact, PCI spec defined message data length is u64, but as you 
>> see, now msi_msg for whole Linux only have u32...)
>>   
>
> Well in that case please define data as __u64, so we don't have 
> surprises later on.
>

btw, if you agree with this, don't resend; I'll edit the patch.

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

Re: [PATCH 05/11] KVM: Add fields for MSI device assignment

[Sheng Yang]

On Tue, Nov 25, 2008 at 04:49:49PM +0200, Avi Kivity wrote:
> Avi Kivity wrote:
>>> (In fact, PCI spec defined message data length is u64, but as you  
>>> see, now msi_msg for whole Linux only have u32...)
>>>   
>>
>> Well in that case please define data as __u64, so we don't have  
>> surprises later on.
>>
>
> btw, if you agree with this, don't resend; I'll edit the patch.

Sorry, I found I made a mistake. Intel SDM 3A misleaded me... The PCI spec
defined message data as u16(!!), but Intel SDM 3A got a figure(Figure 8-25) that
reserved all bits up to bit 63, so I thought message data got u64 long...(At
last moment, I remembered the figure on PCI spec and felt something
strange...)

Now I think the latest patchset(v6) is OK to apply.
--
regards
Yang, Sheng

>
> -- 
> error compiling committee.c: too many arguments to function
>
> --
> To unsubscribe from this list: send the line "unsubscribe kvm" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html