[RFC][PATCH 0/10][v3] Enable MSI for KVM assigned device

[RFC][PATCH 0/10][v3] Enable MSI for KVM assigned device

[Sheng Yang]

Hi, Avi

Here is v3 for MSI on assigned devices.

Still not conclude userspace patch, but the patchset changes a lot since v2.

After discuss with Xiantao, we decide to reuse MSI dispatch function for x86
and IA64. I am working on that now. After get it done, I will send out v4.

Another significant change is, this patchset not only enable MSI for MSI
capable guests(patch 1-9), also enable MSI by default for all guests(patch
10). I implement a MSI to INTx convert in patch 10, which is a little tricky,
but don't have much trouble in theory (the only thing we are worried is about
device would use different "register" to indicate interrupt reason for INTx
mode and MSI mode, but didn't find this kind of device yet). We may need a
black/white list for device which can work in this way. This also avoid host
sharing interrupt trouble, and provide better scalability for the guest which
lacks of MSI capablility. The patch also provide non-sharing host INTx support
for legacy devices. So we suggest this can be a default solution for current
KVM device assignment support.

Comments are welcome!

--
regards
Yang, Sheng

[PATCH 01/10] 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 43fea09..3ecc5a8 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/10] 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 3ecc5a8..ac1d652 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/10] 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 bb92be2..f002f9b 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 ac1d652..6ca04ce 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(kvm, &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/10] 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 6ca04ce..ddfd7ac 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/10] 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      |    6 +++++-
 include/linux/kvm_host.h |    4 ++++
 2 files changed, 9 insertions(+), 1 deletions(-)

diff --git a/include/linux/kvm.h b/include/linux/kvm.h
index 44fd7fa..8ec74b8 100644
--- a/include/linux/kvm.h
+++ b/include/linux/kvm.h
@@ -506,11 +506,15 @@ struct kvm_assigned_irq {
 	__u32 host_irq;
 	__u32 guest_irq;
 	__u32 flags;
+	__u32 guest_msi_data;
+	__u32 guest_msi_addr;
 	union {
-		__u32 reserved[12];
+		__u32 reserved[10];
 	};
 };
 
 #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 f002f9b..1cae239 100644
--- a/include/linux/kvm_host.h
+++ b/include/linux/kvm_host.h
@@ -307,8 +307,12 @@ struct kvm_assigned_dev_kernel {
 	int host_devfn;
 	int host_irq;
 	int guest_irq;
+	u32 guest_msi_addr;
+	u32 guest_msi_data;
 #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/10] 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 |    4 ++--
 virt/kvm/ioapic.h |    2 ++
 2 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/virt/kvm/ioapic.c b/virt/kvm/ioapic.c
index c8f939c..1e0ff8c 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 ioapic_get_delivery_bitmask(struct kvm_ioapic *ioapic, u8 dest,
+				u8 dest_mode)
 {
 	u32 mask = 0;
 	int i;
diff --git a/virt/kvm/ioapic.h b/virt/kvm/ioapic.h
index cd7ae76..42972f5 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 ioapic_get_delivery_bitmask(struct kvm_ioapic *ioapic, u8 dest,
+				u8 dest_mode);
 
 #endif
-- 
1.5.4.5

[PATCH 07/10] x86: Add MSI delivery mode mask

[Sheng Yang]

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

diff --git a/include/asm-x86/msidef.h b/include/asm-x86/msidef.h
index 296f29c..579b09d 100644
--- a/include/asm-x86/msidef.h
+++ b/include/asm-x86/msidef.h
@@ -17,6 +17,8 @@
 #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_VAL	0
+#define  MSI_DATA_DELIVERY_LOWPRI_VAL	1
 
 #define MSI_DATA_LEVEL_SHIFT		14
 #define	 MSI_DATA_LEVEL_DEASSERT	(0 << MSI_DATA_LEVEL_SHIFT)
-- 
1.5.4.5

[PATCH 08/10] 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.

(Any way to replace macro GET_VAL_FROM_SHIFT()?)

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

diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
index ddfd7ac..be0f943 100644
--- a/virt/kvm/kvm_main.c
+++ b/virt/kvm/kvm_main.c
@@ -46,6 +46,7 @@
 #include <asm/io.h>
 #include <asm/uaccess.h>
 #include <asm/pgtable.h>
+#include <asm/msidef.h>
 
 #ifdef KVM_COALESCED_MMIO_PAGE_OFFSET
 #include "coalesced_mmio.h"
@@ -78,6 +79,50 @@ static long kvm_vcpu_ioctl(struct file *file, unsigned int ioctl,
 bool kvm_rebooting;
 
 #ifdef KVM_CAP_DEVICE_ASSIGNMENT
+static void assigned_device_msi_dispatch(struct kvm_assigned_dev_kernel *dev)
+{
+#define GET_VAL_FROM_SHIFT(v, s) (((v) & (1 << s)) >> s)
+	int vcpu_id;
+	struct kvm_vcpu *vcpu;
+	struct kvm_ioapic *ioapic = ioapic_irqchip(dev->kvm);
+	u8 dest_id = MSI_ADDR_DEST_ID(dev->guest_msi_addr);
+	u8 vector = MSI_DATA_VECTOR(dev->guest_msi_data);
+	u8 dest_mode = GET_VAL_FROM_SHIFT(dev->guest_msi_addr,
+					  MSI_ADDR_DEST_MODE_SHIFT);
+	u8 trig_mode = GET_VAL_FROM_SHIFT(dev->guest_msi_data,
+					  MSI_DATA_TRIGGER_SHIFT);
+	u8 delivery_mode = GET_VAL_FROM_SHIFT(dev->guest_msi_data,
+					  MSI_DATA_DELIVERY_MODE_SHIFT);
+	u32 deliver_bitmask;
+
+	BUG_ON(!ioapic);
+
+	deliver_bitmask = ioapic_get_delivery_bitmask(ioapic,
+						      dest_id, dest_mode);
+	switch (delivery_mode) {
+	case MSI_DATA_DELIVERY_LOWPRI_VAL:
+		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_VAL:
+		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");
+	}
+}
+
 static struct kvm_assigned_dev_kernel *kvm_find_assigned_dev(struct list_head *head,
 						      int assigned_dev_id)
 {
-- 
1.5.4.5

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

[Sheng Yang]

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

diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
index be0f943..0f851ef 100644
--- a/virt/kvm/kvm_main.c
+++ b/virt/kvm/kvm_main.c
@@ -149,9 +149,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);
 }
@@ -187,6 +193,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(kvm, &assigned_dev->ack_notifier);
 	kvm_free_irq_source_id(kvm, assigned_dev->irq_source_id);
@@ -232,6 +240,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;
 
@@ -255,6 +268,38 @@ static int assigned_device_update_intx(struct kvm *kvm,
 	return 0;
 }
 
+static int assigned_device_update_msi(struct kvm *kvm,
+			struct kvm_assigned_dev_kernel *adev,
+			struct kvm_assigned_irq *airq)
+{
+	int r;
+
+	adev->guest_msi_addr = airq->guest_msi_addr;
+	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;
+}
+
 static int kvm_vm_ioctl_assign_irq(struct kvm *kvm,
 				   struct kvm_assigned_irq
 				   *assigned_irq)
@@ -291,9 +336,26 @@ 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) {
+		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 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 10/10] KVM: MSI to INTx translate

[Sheng Yang]

Now we use MSI as default one

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

diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
index 0f851ef..34b5d2f 100644
--- a/virt/kvm/kvm_main.c
+++ b/virt/kvm/kvm_main.c
@@ -233,6 +233,8 @@ static int assigned_device_update_intx(struct kvm *kvm,
 			struct kvm_assigned_dev_kernel *adev,
 			struct kvm_assigned_irq *airq)
 {
+	BUG_ON(!!adev->irq_requested_type & KVM_ASSIGNED_DEV_HOST_MSI);
+
 	adev->guest_irq = airq->guest_irq;
 	adev->ack_notifier.gsi = airq->guest_irq;
 
@@ -240,11 +242,6 @@ 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;
 
@@ -272,31 +269,32 @@ static int assigned_device_update_msi(struct kvm *kvm,
 			struct kvm_assigned_dev_kernel *adev,
 			struct kvm_assigned_irq *airq)
 {
-	int r;
+	BUG_ON(!!adev->irq_requested_type & KVM_ASSIGNED_DEV_HOST_INTX);
 
-	adev->guest_msi_addr = airq->guest_msi_addr;
-	adev->guest_msi_data = airq->guest_msi_data;
-	adev->ack_notifier.gsi = -1;
+	if (airq->flags & KVM_DEV_IRQ_ASSIGN_ENABLE_MSI) {
+		adev->irq_requested_type |= KVM_ASSIGNED_DEV_GUEST_MSI;
+		adev->irq_requested_type &= ~KVM_ASSIGNED_DEV_GUEST_INTX;
+		adev->guest_msi_addr = airq->guest_msi_addr;
+		adev->guest_msi_data = airq->guest_msi_data;
+		adev->ack_notifier.gsi = -1;
+	} else {
+		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;
-
 		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;
+	adev->irq_requested_type |= KVM_ASSIGNED_DEV_HOST_MSI;
 	return 0;
 }
 
@@ -333,10 +331,15 @@ static int kvm_vm_ioctl_assign_irq(struct kvm *kvm,
 				goto out_release;
 			else
 				match->irq_source_id = r;
+
+			/* Determine host device irq type, we can know the
+			 * result from dev->msi_enabled */
+			pci_enable_msi(match->dev);
 		}
 	}
 
-	if (assigned_irq->flags & KVM_DEV_IRQ_ASSIGN_ENABLE_MSI) {
+	if (match->dev->msi_enabled) {
+		/* Device support MSI */
 		r = assigned_device_update_msi(kvm, match, assigned_irq);
 		if (r) {
 			printk(KERN_WARNING "kvm: failed to enable "
@@ -344,9 +347,10 @@ static int kvm_vm_ioctl_assign_irq(struct kvm *kvm,
 			goto out_release;
 		}
 	} 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;
+		/* Device only support MSI but can't enable */
+		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 08/10] KVM: Add assigned_device_msi_dispatch()

[Sheng Yang]

On Thursday 30 October 2008 16:20:16 Sheng Yang wrote:
> The function is used to dispatch MSI to lapic according to MSI message
> address and message data.
>
> (Any way to replace macro GET_VAL_FROM_SHIFT()?)
>
> Signed-off-by: Sheng Yang <sheng@linux.intel.com>
> ---
>  virt/kvm/kvm_main.c |   45 +++++++++++++++++++++++++++++++++++++++++++++
>  1 files changed, 45 insertions(+), 0 deletions(-)
>
> diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
> index ddfd7ac..be0f943 100644
> --- a/virt/kvm/kvm_main.c
> +++ b/virt/kvm/kvm_main.c
> @@ -46,6 +46,7 @@
>  #include <asm/io.h>
>  #include <asm/uaccess.h>
>  #include <asm/pgtable.h>
> +#include <asm/msidef.h>
>
>  #ifdef KVM_COALESCED_MMIO_PAGE_OFFSET
>  #include "coalesced_mmio.h"
> @@ -78,6 +79,50 @@ static long kvm_vcpu_ioctl(struct file *file, unsigned
> int ioctl, bool kvm_rebooting;
>
>  #ifdef KVM_CAP_DEVICE_ASSIGNMENT
> +static void assigned_device_msi_dispatch(struct kvm_assigned_dev_kernel
> *dev) +{
> +#define GET_VAL_FROM_SHIFT(v, s) (((v) & (1 << s)) >> s)
> +	int vcpu_id;
> +	struct kvm_vcpu *vcpu;
> +	struct kvm_ioapic *ioapic = ioapic_irqchip(dev->kvm);
> +	u8 dest_id = MSI_ADDR_DEST_ID(dev->guest_msi_addr);
> +	u8 vector = MSI_DATA_VECTOR(dev->guest_msi_data);

Oops... These two macros are wrong used, would be fixed in next version.
-- 
regards
Yang, Sheng

> +	u8 dest_mode = GET_VAL_FROM_SHIFT(dev->guest_msi_addr,
> +					  MSI_ADDR_DEST_MODE_SHIFT);
> +	u8 trig_mode = GET_VAL_FROM_SHIFT(dev->guest_msi_data,
> +					  MSI_DATA_TRIGGER_SHIFT);
> +	u8 delivery_mode = GET_VAL_FROM_SHIFT(dev->guest_msi_data,
> +					  MSI_DATA_DELIVERY_MODE_SHIFT);
> +	u32 deliver_bitmask;
> +
> +	BUG_ON(!ioapic);
> +
> +	deliver_bitmask = ioapic_get_delivery_bitmask(ioapic,
> +						      dest_id, dest_mode);
> +	switch (delivery_mode) {
> +	case MSI_DATA_DELIVERY_LOWPRI_VAL:
> +		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_VAL:
> +		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");
> +	}
> +}
> +
>  static struct kvm_assigned_dev_kernel *kvm_find_assigned_dev(struct
> list_head *head, int assigned_dev_id)
>  {

[PATCH 08/10] 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.

(Any way to replace macro GET_VAL_FROM_SHIFT()?)

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

diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
index c69c108..d23f555 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,57 @@ 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)
+{
+#define GET_VAL_FROM_SHIFT(v, s) (((v) & (1 << s)) >> s)
+	int vcpu_id;
+	struct kvm_vcpu *vcpu;
+	struct kvm_ioapic *ioapic = ioapic_irqchip(dev->kvm);
+	u8 dest_id = (dev->guest_msi_addr & MSI_ADDR_DEST_ID_MASK)
+			>> MSI_ADDR_DEST_ID_SHIFT;
+	u8 vector = (dev->guest_msi_data & MSI_DATA_VECTOR_MASK)
+			>> MSI_DATA_VECTOR_SHIFT;
+	u8 dest_mode = GET_VAL_FROM_SHIFT(dev->guest_msi_addr,
+					  MSI_ADDR_DEST_MODE_SHIFT);
+	u8 trig_mode = GET_VAL_FROM_SHIFT(dev->guest_msi_data,
+					  MSI_DATA_TRIGGER_SHIFT);
+	u8 delivery_mode = GET_VAL_FROM_SHIFT(dev->guest_msi_data,
+					  MSI_DATA_DELIVERY_MODE_SHIFT);
+	u32 deliver_bitmask;
+
+	BUG_ON(!ioapic);
+
+	deliver_bitmask = ioapic_get_delivery_bitmask(ioapic,
+						      dest_id, dest_mode);
+	switch (delivery_mode) {
+	case MSI_DATA_DELIVERY_LOWPRI_VAL:
+		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_VAL:
+		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

Re: [PATCH 08/10] 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.
>
> (Any way to replace macro GET_VAL_FROM_SHIFT()?)
>   

> +#ifdef CONFIG_X86
> +#include <asm/msidef.h>
> +#endif
>   

The include can be unconditional.

> +
> +#ifdef CONFIG_X86
> +static void assigned_device_msi_dispatch(struct kvm_assigned_dev_kernel *dev)
> +{
> +#define GET_VAL_FROM_SHIFT(v, s) (((v) & (1 << s)) >> s)
>   

test_bit() ?

> +	int vcpu_id;
> +	struct kvm_vcpu *vcpu;
> +	struct kvm_ioapic *ioapic = ioapic_irqchip(dev->kvm);
> +	u8 dest_id = (dev->guest_msi_addr & MSI_ADDR_DEST_ID_MASK)
> +			>> MSI_ADDR_DEST_ID_SHIFT;
> +	u8 vector = (dev->guest_msi_data & MSI_DATA_VECTOR_MASK)
> +			>> MSI_DATA_VECTOR_SHIFT;
> +	u8 dest_mode = GET_VAL_FROM_SHIFT(dev->guest_msi_addr,
> +					  MSI_ADDR_DEST_MODE_SHIFT);
> +	u8 trig_mode = GET_VAL_FROM_SHIFT(dev->guest_msi_data,
> +					  MSI_DATA_TRIGGER_SHIFT);
> +	u8 delivery_mode = GET_VAL_FROM_SHIFT(dev->guest_msi_data,
> +					  MSI_DATA_DELIVERY_MODE_SHIFT);
>   

int is usually better than u8.

> +	u32 deliver_bitmask;
> +
> +	case MSI_DATA_DELIVERY_FIXED_VAL:
> +		for (vcpu_id = 0; deliver_bitmask != 0; vcpu_id++) {
>   

__ffs() and __clear_bit() can make this a bit faster.  Can be done later 
though.

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

Re: [PATCH 08/10] KVM: Add assigned_device_msi_dispatch()

[Sheng Yang]

On Tuesday 04 November 2008 19:17:03 Avi Kivity wrote:
> Sheng Yang wrote:
> > The function is used to dispatch MSI to lapic according to MSI message
> > address and message data.
> >
> > (Any way to replace macro GET_VAL_FROM_SHIFT()?)
> >
> >
> > +#ifdef CONFIG_X86
> > +#include <asm/msidef.h>
> > +#endif
>
> The include can be unconditional.

it's pity that IA64 don't have this head file... I will separate the msi.c and 
add this later.

>
> > +
> > +#ifdef CONFIG_X86
> > +static void assigned_device_msi_dispatch(struct kvm_assigned_dev_kernel
> > *dev) +{
> > +#define GET_VAL_FROM_SHIFT(v, s) (((v) & (1 << s)) >> s)
>
> test_bit() ?

Oh, indeed here. :)
>
> > +	int vcpu_id;
> > +	struct kvm_vcpu *vcpu;
> > +	struct kvm_ioapic *ioapic = ioapic_irqchip(dev->kvm);
> > +	u8 dest_id = (dev->guest_msi_addr & MSI_ADDR_DEST_ID_MASK)
> > +			>> MSI_ADDR_DEST_ID_SHIFT;
> > +	u8 vector = (dev->guest_msi_data & MSI_DATA_VECTOR_MASK)
> > +			>> MSI_DATA_VECTOR_SHIFT;
> > +	u8 dest_mode = GET_VAL_FROM_SHIFT(dev->guest_msi_addr,
> > +					  MSI_ADDR_DEST_MODE_SHIFT);
> > +	u8 trig_mode = GET_VAL_FROM_SHIFT(dev->guest_msi_data,
> > +					  MSI_DATA_TRIGGER_SHIFT);
> > +	u8 delivery_mode = GET_VAL_FROM_SHIFT(dev->guest_msi_data,
> > +					  MSI_DATA_DELIVERY_MODE_SHIFT);
>
> int is usually better than u8.

OK.
>
> > +	u32 deliver_bitmask;
> > +
> > +	case MSI_DATA_DELIVERY_FIXED_VAL:
> > +		for (vcpu_id = 0; deliver_bitmask != 0; vcpu_id++) {
>
> __ffs() and __clear_bit() can make this a bit faster.  Can be done later
> though.

OK, also would optimize IOAPIC side later.

-- 
regards
Yang, Sheng