[PATCH 0/10][v3] GSI->MSG route layer for MSI/MSI-X

[PATCH 0/10][v3] GSI->MSG route layer for MSI/MSI-X

[Sheng Yang]

Update from v2:
Add gsi_msg_pending_bitmap, in order to support MSI-X mutiply interrupts.

And this one depends on the former "Device assignment code clean up and MSI
disable support" patchset.

--
regards
Yang, Sheng

[PATCH 01/10] KVM: Add a route layer to convert MSI message to GSI

[Sheng Yang]

Avi's purpose, to use single kvm_set_irq() to deal with all interrupt, including
MSI. So here is it.

struct gsi_msg is a mapping from a special gsi(with KVM_GSI_MSG_MASK) to
MSI/MSI-X message address/data.

Now we support up to 256 gsi_msg mapping, and gsi_msg is allocated by kernel and
provide two ioctls to userspace, which is more flexiable.

Signed-off-by: Sheng Yang <sheng@linux.intel.com>
---
 include/linux/kvm.h      |   14 +++++++++
 include/linux/kvm_host.h |   16 ++++++++++
 virt/kvm/irq_comm.c      |   70 ++++++++++++++++++++++++++++++++++++++++++++++
 virt/kvm/kvm_main.c      |   66 +++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 166 insertions(+), 0 deletions(-)

diff --git a/include/linux/kvm.h b/include/linux/kvm.h
index c24f207..a75e01f 100644
--- a/include/linux/kvm.h
+++ b/include/linux/kvm.h
@@ -396,6 +396,9 @@ struct kvm_trace_rec {
 #if defined(CONFIG_X86)
 #define KVM_CAP_SET_GUEST_DEBUG 23
 #endif
+#if defined(CONFIG_X86)
+#define KVM_CAP_GSI_MSG 24
+#endif
 
 /*
  * ioctls for VM fds
@@ -429,6 +432,8 @@ struct kvm_trace_rec {
 				   struct kvm_assigned_pci_dev)
 #define KVM_ASSIGN_IRQ _IOR(KVMIO, 0x70, \
 			    struct kvm_assigned_irq)
+#define KVM_REQUEST_GSI_MSG _IOWR(KVMIO, 0x71, struct kvm_assigned_gsi_msg)
+#define KVM_FREE_GSI_MSG _IOR(KVMIO, 0x72, struct kvm_assigned_gsi_msg)
 
 /*
  * ioctls for vcpu fds
@@ -549,4 +554,13 @@ struct kvm_assigned_irq {
 #define KVM_DEV_IRQ_ASSIGN_MSI_ACTION	(1 << 0)
 #define KVM_DEV_IRQ_ASSIGN_ENABLE_MSI	(1 << 1)
 
+struct kvm_assigned_gsi_msg {
+	__u32 gsi;
+	struct {
+		__u32 addr_lo;
+		__u32 addr_hi;
+		__u32 data;
+	} msg;
+};
+
 #endif
diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h
index d63e9a4..0e5741a 100644
--- a/include/linux/kvm_host.h
+++ b/include/linux/kvm_host.h
@@ -132,6 +132,10 @@ struct kvm {
 	unsigned long mmu_notifier_seq;
 	long mmu_notifier_count;
 #endif
+	struct hlist_head gsi_msg_list;
+	struct mutex gsi_msg_lock;
+#define KVM_NR_GSI_MSG	    256
+	DECLARE_BITMAP(gsi_msg_bitmap, KVM_NR_GSI_MSG);
 };
 
 /* The guest did something we don't support. */
@@ -319,6 +323,14 @@ struct kvm_assigned_dev_kernel {
 	struct pci_dev *dev;
 	struct kvm *kvm;
 };
+
+#define KVM_GSI_MSG_MASK    0x1000000ull
+struct kvm_gsi_msg {
+	u32 gsi;
+	struct msi_msg msg;
+	struct hlist_node link;
+};
+
 void kvm_set_irq(struct kvm *kvm, int irq_source_id, int irq, int level);
 void kvm_notify_acked_irq(struct kvm *kvm, unsigned gsi);
 void kvm_register_irq_ack_notifier(struct kvm *kvm,
@@ -326,6 +338,10 @@ void kvm_register_irq_ack_notifier(struct kvm *kvm,
 void kvm_unregister_irq_ack_notifier(struct kvm_irq_ack_notifier *kian);
 int kvm_request_irq_source_id(struct kvm *kvm);
 void kvm_free_irq_source_id(struct kvm *kvm, int irq_source_id);
+int kvm_update_gsi_msg(struct kvm *kvm, struct kvm_gsi_msg *gsi_msg);
+struct kvm_gsi_msg *kvm_find_gsi_msg(struct kvm *kvm, u32 gsi);
+void kvm_free_gsi_msg(struct kvm *kvm, struct kvm_gsi_msg *gsi_msg);
+void kvm_free_gsi_msg_list(struct kvm *kvm);
 
 #ifdef CONFIG_DMAR
 int kvm_iommu_map_pages(struct kvm *kvm, gfn_t base_gfn,
diff --git a/virt/kvm/irq_comm.c b/virt/kvm/irq_comm.c
index aa5d1e5..abfab46 100644
--- a/virt/kvm/irq_comm.c
+++ b/virt/kvm/irq_comm.c
@@ -99,3 +99,73 @@ void kvm_free_irq_source_id(struct kvm *kvm, int irq_source_id)
 		clear_bit(irq_source_id, &kvm->arch.irq_states[i]);
 	clear_bit(irq_source_id, &kvm->arch.irq_sources_bitmap);
 }
+
+int kvm_update_gsi_msg(struct kvm *kvm, struct kvm_gsi_msg *gsi_msg)
+{
+	struct kvm_gsi_msg *found_msg, *new_gsi_msg;
+	int r, gsi;
+
+	mutex_lock(&kvm->gsi_msg_lock);
+	/* Find whether we need a update or a new entry */
+	found_msg = kvm_find_gsi_msg(kvm, gsi_msg->gsi);
+	if (found_msg)
+		*found_msg = *gsi_msg;
+	else {
+		gsi = find_first_zero_bit(kvm->gsi_msg_bitmap, KVM_NR_GSI_MSG);
+		if (gsi >= KVM_NR_GSI_MSG) {
+			r = -ENOSPC;
+			goto out;
+		}
+		__set_bit(gsi, kvm->gsi_msg_bitmap);
+		gsi_msg->gsi = gsi | KVM_GSI_MSG_MASK;
+		new_gsi_msg = kzalloc(sizeof(*new_gsi_msg), GFP_KERNEL);
+		if (!new_gsi_msg) {
+			r = -ENOMEM;
+			goto out;
+		}
+		*new_gsi_msg = *gsi_msg;
+		hlist_add_head(&new_gsi_msg->link, &kvm->gsi_msg_list);
+	}
+	r = 0;
+out:
+	mutex_unlock(&kvm->gsi_msg_lock);
+	return r;
+}
+
+/* Call with kvm->gsi_msg_lock hold */
+struct kvm_gsi_msg *kvm_find_gsi_msg(struct kvm *kvm, u32 gsi)
+{
+	struct kvm_gsi_msg *gsi_msg;
+	struct hlist_node *n;
+
+	if (!(gsi & KVM_GSI_MSG_MASK))
+		return NULL;
+	hlist_for_each_entry(gsi_msg, n, &kvm->gsi_msg_list, link)
+		if (gsi_msg->gsi == gsi)
+			goto out;
+	gsi_msg = NULL;
+out:
+	return gsi_msg;
+}
+
+/* Call with kvm->gsi_msg_lock hold */
+void kvm_free_gsi_msg(struct kvm *kvm, struct kvm_gsi_msg *gsi_msg)
+{
+	if (!gsi_msg)
+		return;
+	__clear_bit(gsi_msg->gsi & ~KVM_GSI_MSG_MASK, kvm->gsi_msg_bitmap);
+	hlist_del(&gsi_msg->link);
+	kfree(gsi_msg);
+}
+
+void kvm_free_gsi_msg_list(struct kvm *kvm)
+{
+	struct kvm_gsi_msg *gsi_msg;
+	struct hlist_node *pos, *n;
+
+	mutex_lock(&kvm->gsi_msg_lock);
+	hlist_for_each_entry_safe(gsi_msg, pos, n, &kvm->gsi_msg_list, link)
+		kvm_free_gsi_msg(kvm, gsi_msg);
+	mutex_unlock(&kvm->gsi_msg_lock);
+}
+
diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
index 111738b..26bccf9 100644
--- a/virt/kvm/kvm_main.c
+++ b/virt/kvm/kvm_main.c
@@ -813,6 +813,8 @@ static struct kvm *kvm_create_vm(void)
 #ifdef KVM_COALESCED_MMIO_PAGE_OFFSET
 	kvm_coalesced_mmio_init(kvm);
 #endif
+	INIT_HLIST_HEAD(&kvm->gsi_msg_list);
+	mutex_init(&kvm->gsi_msg_lock);
 out:
 	return kvm;
 }
@@ -850,6 +852,7 @@ static void kvm_destroy_vm(struct kvm *kvm)
 {
 	struct mm_struct *mm = kvm->mm;
 
+	kvm_free_gsi_msg_list(kvm);
 	spin_lock(&kvm_lock);
 	list_del(&kvm->vm_list);
 	spin_unlock(&kvm_lock);
@@ -1578,6 +1581,45 @@ static int kvm_vcpu_ioctl_set_sigmask(struct kvm_vcpu *vcpu, sigset_t *sigset)
 	return 0;
 }
 
+static int kvm_vm_ioctl_request_gsi_msg(struct kvm *kvm,
+					struct kvm_assigned_gsi_msg *agsi_msg)
+{
+	struct kvm_gsi_msg gsi_msg;
+	int r;
+
+	gsi_msg.gsi = agsi_msg->gsi;
+	gsi_msg.msg.address_lo = agsi_msg->msg.addr_lo;
+	gsi_msg.msg.address_hi = agsi_msg->msg.addr_hi;
+	gsi_msg.msg.data = agsi_msg->msg.data;
+
+	r = kvm_update_gsi_msg(kvm, &gsi_msg);
+	if (r == 0)
+		agsi_msg->gsi = gsi_msg.gsi;
+	return r;
+}
+
+static int kvm_vm_ioctl_free_gsi_msg(struct kvm *kvm,
+				     struct kvm_assigned_gsi_msg *agsi_msg)
+{
+	struct kvm_gsi_msg *gsi_msg;
+	u32 gsi;
+	int r;
+
+	gsi = agsi_msg->gsi;
+	mutex_lock(&kvm->gsi_msg_lock);
+	gsi_msg = kvm_find_gsi_msg(kvm, gsi);
+	if (!gsi_msg || memcmp(gsi_msg, agsi_msg, sizeof(agsi_msg)) != 0) {
+		printk(KERN_WARNING "kvm: illegal gsi->msi_msg mapping!");
+		r = -EINVAL;
+		goto out;
+	}
+	kvm_free_gsi_msg(kvm, gsi_msg);
+	r = 0;
+out:
+	mutex_unlock(&kvm->gsi_msg_lock);
+	return r;
+}
+
 static long kvm_vcpu_ioctl(struct file *filp,
 			   unsigned int ioctl, unsigned long arg)
 {
@@ -1860,6 +1902,30 @@ static long kvm_vm_ioctl(struct file *filp,
 		break;
 	}
 #endif
+	case KVM_REQUEST_GSI_MSG: {
+		struct kvm_assigned_gsi_msg agsi_msg;
+		r = -EFAULT;
+		if (copy_from_user(&agsi_msg, argp, sizeof agsi_msg))
+			goto out;
+		r = kvm_vm_ioctl_request_gsi_msg(kvm, &agsi_msg);
+		if (r)
+			goto out;
+		r = -EFAULT;
+		if (copy_to_user(argp, &agsi_msg, sizeof agsi_msg))
+			goto out;
+		r = 0;
+		break;
+	}
+	case KVM_FREE_GSI_MSG: {
+		struct kvm_assigned_gsi_msg agsi_msg;
+		r = -EFAULT;
+		if (copy_from_user(&agsi_msg, argp, sizeof agsi_msg))
+			goto out;
+		r = kvm_vm_ioctl_free_gsi_msg(kvm, &agsi_msg);
+		if (r)
+			goto out;
+		break;
+	}
 	default:
 		r = kvm_arch_vm_ioctl(filp, ioctl, arg);
 	}
-- 
1.5.4.5

[PATCH 02/10] KVM: Using gsi_msg mapping for MSI device assignment

[Sheng Yang]

Convert MSI userspace interface to support gsi_msg mapping(and nobody should
be the user of the old interface...).

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

diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h
index 0e5741a..aa2606b 100644
--- a/include/linux/kvm_host.h
+++ b/include/linux/kvm_host.h
@@ -313,7 +313,6 @@ struct kvm_assigned_dev_kernel {
 	int host_irq;
 	bool host_irq_disabled;
 	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)
diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
index 26bccf9..3494861 100644
--- a/virt/kvm/kvm_main.c
+++ b/virt/kvm/kvm_main.c
@@ -92,20 +92,30 @@ 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);
+	struct kvm_gsi_msg *gsi_msg;
+	int dest_id, vector, dest_mode, trig_mode, delivery_mode;
 	u32 deliver_bitmask;
 
 	BUG_ON(!ioapic);
 
+	mutex_lock(&dev->kvm->gsi_msg_lock);
+	gsi_msg = kvm_find_gsi_msg(dev->kvm, dev->guest_irq);
+	if (!gsi_msg) {
+		printk(KERN_WARNING "kvm: fail to find correlated gsi_msg\n");
+		return;
+	}
+	mutex_unlock(&dev->kvm->gsi_msg_lock);
+
+	dest_id = (gsi_msg->msg.address_lo & MSI_ADDR_DEST_ID_MASK)
+			>> MSI_ADDR_DEST_ID_SHIFT;
+	vector = (gsi_msg->msg.data & MSI_DATA_VECTOR_MASK)
+			>> MSI_DATA_VECTOR_SHIFT;
+	dest_mode = test_bit(MSI_ADDR_DEST_MODE_SHIFT,
+				(unsigned long *)&gsi_msg->msg.address_lo);
+	trig_mode = test_bit(MSI_DATA_TRIGGER_SHIFT,
+				(unsigned long *)&gsi_msg->msg.data);
+	delivery_mode = test_bit(MSI_DATA_DELIVERY_MODE_SHIFT,
+				(unsigned long *)&gsi_msg->msg.data);
 	deliver_bitmask = kvm_ioapic_get_delivery_bitmask(ioapic,
 				dest_id, dest_mode);
 	/* IOAPIC delivery mode value is the same as MSI here */
@@ -316,17 +326,16 @@ static int assigned_device_update_msi(struct kvm *kvm,
 {
 	int r;
 
+	adev->guest_irq = airq->guest_irq;
+
 	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;
 	} else {
 		/*
-- 
1.5.4.5

[PATCH 03/10] KVM: Improve MSI dispatch function

[Sheng Yang]

Prepare to merge with kvm_set_irq().

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

diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
index 3494861..599257e 100644
--- a/virt/kvm/kvm_main.c
+++ b/virt/kvm/kvm_main.c
@@ -87,7 +87,7 @@ static bool kvm_rebooting;
 #ifdef KVM_CAP_DEVICE_ASSIGNMENT
 
 #ifdef CONFIG_X86
-static void assigned_device_msi_dispatch(struct kvm_assigned_dev_kernel *dev)
+static void assigned_device_msi_dispatch(struct kvm_assigned_dev_kernel *dev, u32 gsi)
 {
 	int vcpu_id;
 	struct kvm_vcpu *vcpu;
@@ -99,7 +99,7 @@ static void assigned_device_msi_dispatch(struct kvm_assigned_dev_kernel *dev)
 	BUG_ON(!ioapic);
 
 	mutex_lock(&dev->kvm->gsi_msg_lock);
-	gsi_msg = kvm_find_gsi_msg(dev->kvm, dev->guest_irq);
+	gsi_msg = kvm_find_gsi_msg(dev->kvm, gsi);
 	if (!gsi_msg) {
 		printk(KERN_WARNING "kvm: fail to find correlated gsi_msg\n");
 		return;
@@ -143,7 +143,7 @@ static void assigned_device_msi_dispatch(struct kvm_assigned_dev_kernel *dev)
 	}
 }
 #else
-static void assigned_device_msi_dispatch(struct kvm_assigned_dev_kernel *dev) {}
+static void assigned_device_msi_dispatch(struct kvm_assigned_dev_kernel *dev, u32 gsi) {}
 #endif
 
 static struct kvm_assigned_dev_kernel *kvm_find_assigned_dev(struct list_head *head,
@@ -178,7 +178,7 @@ static void kvm_assigned_dev_interrupt_work_handler(struct work_struct *work)
 			    assigned_dev->guest_irq, 1);
 	else if (assigned_dev->irq_requested_type &
 				KVM_ASSIGNED_DEV_GUEST_MSI) {
-		assigned_device_msi_dispatch(assigned_dev);
+		assigned_device_msi_dispatch(assigned_dev, assigned_dev->guest_irq);
 		enable_irq(assigned_dev->host_irq);
 		assigned_dev->host_irq_disabled = false;
 	}
-- 
1.5.4.5

[PATCH 04/10] KVM: Using ioapic_irqchip() macro for kvm_set_irq

[Sheng Yang]

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

diff --git a/virt/kvm/irq_comm.c b/virt/kvm/irq_comm.c
index abfab46..47243ef 100644
--- a/virt/kvm/irq_comm.c
+++ b/virt/kvm/irq_comm.c
@@ -39,7 +39,7 @@ void kvm_set_irq(struct kvm *kvm, int irq_source_id, int irq, int level)
 	 * IOAPIC.  So set the bit in both. The guest will ignore
 	 * writes to the unused one.
 	 */
-	kvm_ioapic_set_irq(kvm->arch.vioapic, irq, !!(*irq_state));
+	kvm_ioapic_set_irq(ioapic_irqchip(kvm), irq, !!(*irq_state));
 #ifdef CONFIG_X86
 	kvm_pic_set_irq(pic_irqchip(kvm), irq, !!(*irq_state));
 #endif
-- 
1.5.4.5

[PATCH 05/10] KVM: Merge MSI handling to kvm_set_irq

[Sheng Yang]

Using kvm_set_irq to handle all interrupt injection.

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

diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h
index aa2606b..5b671b6 100644
--- a/include/linux/kvm_host.h
+++ b/include/linux/kvm_host.h
@@ -330,7 +330,7 @@ struct kvm_gsi_msg {
 	struct hlist_node link;
 };
 
-void kvm_set_irq(struct kvm *kvm, int irq_source_id, int irq, int level);
+void kvm_set_irq(struct kvm *kvm, int irq_source_id, u32 gsi, int level);
 void kvm_notify_acked_irq(struct kvm *kvm, unsigned gsi);
 void kvm_register_irq_ack_notifier(struct kvm *kvm,
 				   struct kvm_irq_ack_notifier *kian);
diff --git a/virt/kvm/irq_comm.c b/virt/kvm/irq_comm.c
index 47243ef..63cdf01 100644
--- a/virt/kvm/irq_comm.c
+++ b/virt/kvm/irq_comm.c
@@ -20,28 +20,96 @@
  */
 
 #include <linux/kvm_host.h>
+
+#ifdef CONFIG_X86
+#include <asm/msidef.h>
+#endif
+
 #include "irq.h"
 
 #include "ioapic.h"
 
 /* This should be called with the kvm->lock mutex held */
-void kvm_set_irq(struct kvm *kvm, int irq_source_id, int irq, int level)
+void kvm_set_irq(struct kvm *kvm, int irq_source_id, u32 gsi, int level)
 {
-	unsigned long *irq_state = (unsigned long *)&kvm->arch.irq_states[irq];
-
-	/* Logical OR for level trig interrupt */
-	if (level)
-		set_bit(irq_source_id, irq_state);
-	else
-		clear_bit(irq_source_id, irq_state);
-
-	/* Not possible to detect if the guest uses the PIC or the
-	 * IOAPIC.  So set the bit in both. The guest will ignore
-	 * writes to the unused one.
-	 */
-	kvm_ioapic_set_irq(ioapic_irqchip(kvm), irq, !!(*irq_state));
+	unsigned long *irq_state;
+#ifdef CONFIG_X86
+	int vcpu_id;
+	struct kvm_vcpu *vcpu;
+	struct kvm_ioapic *ioapic = ioapic_irqchip(kvm);
+	struct kvm_gsi_msg *gsi_msg;
+	int dest_id, vector, dest_mode, trig_mode, delivery_mode;
+	u32 deliver_bitmask;
+
+	BUG_ON(!ioapic);
+#endif
+
+	if (!(gsi & KVM_GSI_MSG_MASK)) {
+		int irq = gsi;
+
+		irq_state = (unsigned long *)&kvm->arch.irq_states[irq];
+
+		/* Logical OR for level trig interrupt */
+		if (level)
+			set_bit(irq_source_id, irq_state);
+		else
+			clear_bit(irq_source_id, irq_state);
+
+		/* Not possible to detect if the guest uses the PIC or the
+		 * IOAPIC.  So set the bit in both. The guest will ignore
+		 * writes to the unused one.
+		 */
+		kvm_ioapic_set_irq(ioapic, irq, !!(*irq_state));
 #ifdef CONFIG_X86
-	kvm_pic_set_irq(pic_irqchip(kvm), irq, !!(*irq_state));
+		kvm_pic_set_irq(pic_irqchip(kvm), irq, !!(*irq_state));
+#endif
+		return;
+	}
+
+#ifdef CONFIG_X86
+	mutex_lock(&kvm->gsi_msg_lock);
+	gsi_msg = kvm_find_gsi_msg(kvm, gsi);
+	mutex_unlock(&kvm->gsi_msg_lock);
+	if (!gsi_msg) {
+		printk(KERN_WARNING "kvm: fail to find correlated gsi_msg\n");
+		return;
+	}
+
+	dest_id = (gsi_msg->msg.address_lo & MSI_ADDR_DEST_ID_MASK)
+			>> MSI_ADDR_DEST_ID_SHIFT;
+	vector = (gsi_msg->msg.data & MSI_DATA_VECTOR_MASK)
+			>> MSI_DATA_VECTOR_SHIFT;
+	dest_mode = test_bit(MSI_ADDR_DEST_MODE_SHIFT,
+				(unsigned long *)&gsi_msg->msg.address_lo);
+	trig_mode = test_bit(MSI_DATA_TRIGGER_SHIFT,
+				(unsigned long *)&gsi_msg->msg.data);
+	delivery_mode = test_bit(MSI_DATA_DELIVERY_MODE_SHIFT,
+				(unsigned long *)&gsi_msg->msg.data);
+	deliver_bitmask = kvm_ioapic_get_delivery_bitmask(ioapic,
+				dest_id, dest_mode);
+	/* IOAPIC delivery mode value is the same as MSI here */
+	switch (delivery_mode) {
+	case IOAPIC_LOWEST_PRIORITY:
+		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 IOAPIC_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");
+	}
 #endif
 }
 
diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
index 599257e..a51e630 100644
--- a/virt/kvm/kvm_main.c
+++ b/virt/kvm/kvm_main.c
@@ -47,10 +47,6 @@
 #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
@@ -86,66 +82,6 @@ static bool kvm_rebooting;
 
 #ifdef KVM_CAP_DEVICE_ASSIGNMENT
 
-#ifdef CONFIG_X86
-static void assigned_device_msi_dispatch(struct kvm_assigned_dev_kernel *dev, u32 gsi)
-{
-	int vcpu_id;
-	struct kvm_vcpu *vcpu;
-	struct kvm_ioapic *ioapic = ioapic_irqchip(dev->kvm);
-	struct kvm_gsi_msg *gsi_msg;
-	int dest_id, vector, dest_mode, trig_mode, delivery_mode;
-	u32 deliver_bitmask;
-
-	BUG_ON(!ioapic);
-
-	mutex_lock(&dev->kvm->gsi_msg_lock);
-	gsi_msg = kvm_find_gsi_msg(dev->kvm, gsi);
-	if (!gsi_msg) {
-		printk(KERN_WARNING "kvm: fail to find correlated gsi_msg\n");
-		return;
-	}
-	mutex_unlock(&dev->kvm->gsi_msg_lock);
-
-	dest_id = (gsi_msg->msg.address_lo & MSI_ADDR_DEST_ID_MASK)
-			>> MSI_ADDR_DEST_ID_SHIFT;
-	vector = (gsi_msg->msg.data & MSI_DATA_VECTOR_MASK)
-			>> MSI_DATA_VECTOR_SHIFT;
-	dest_mode = test_bit(MSI_ADDR_DEST_MODE_SHIFT,
-				(unsigned long *)&gsi_msg->msg.address_lo);
-	trig_mode = test_bit(MSI_DATA_TRIGGER_SHIFT,
-				(unsigned long *)&gsi_msg->msg.data);
-	delivery_mode = test_bit(MSI_DATA_DELIVERY_MODE_SHIFT,
-				(unsigned long *)&gsi_msg->msg.data);
-	deliver_bitmask = kvm_ioapic_get_delivery_bitmask(ioapic,
-				dest_id, dest_mode);
-	/* IOAPIC delivery mode value is the same as MSI here */
-	switch (delivery_mode) {
-	case IOAPIC_LOWEST_PRIORITY:
-		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 IOAPIC_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, u32 gsi) {}
-#endif
-
 static struct kvm_assigned_dev_kernel *kvm_find_assigned_dev(struct list_head *head,
 						      int assigned_dev_id)
 {
@@ -172,16 +108,15 @@ static void kvm_assigned_dev_interrupt_work_handler(struct work_struct *work)
 	 * finer-grained lock, update this
 	 */
 	mutex_lock(&assigned_dev->kvm->lock);
-	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, assigned_dev->guest_irq);
+
+	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_MSI) {
 		enable_irq(assigned_dev->host_irq);
 		assigned_dev->host_irq_disabled = false;
 	}
+
 	mutex_unlock(&assigned_dev->kvm->lock);
 	kvm_put_kvm(assigned_dev->kvm);
 }
-- 
1.5.4.5

[PATCH 06/10] KVM: Split IOAPIC structure

[Sheng Yang]

Prepared for reuse ioapic_redir_entry for MSI.

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

diff --git a/include/linux/kvm_types.h b/include/linux/kvm_types.h
index 5f4a18c..46e3d8d 100644
--- a/include/linux/kvm_types.h
+++ b/include/linux/kvm_types.h
@@ -52,4 +52,21 @@ struct kvm_pio_request {
 	int rep;
 };
 
+union kvm_ioapic_redirect_entry {
+	u64 bits;
+	struct {
+		u8 vector;
+		u8 delivery_mode:3;
+		u8 dest_mode:1;
+		u8 delivery_status:1;
+		u8 polarity:1;
+		u8 remote_irr:1;
+		u8 trig_mode:1;
+		u8 mask:1;
+		u8 reserve:7;
+		u8 reserved[4];
+		u8 dest_id;
+	} fields;
+};
+
 #endif /* __KVM_TYPES_H__ */
diff --git a/virt/kvm/ioapic.c b/virt/kvm/ioapic.c
index 23b81cf..ebb2ab5 100644
--- a/virt/kvm/ioapic.c
+++ b/virt/kvm/ioapic.c
@@ -85,7 +85,7 @@ static unsigned long ioapic_read_indirect(struct kvm_ioapic *ioapic,
 
 static void ioapic_service(struct kvm_ioapic *ioapic, unsigned int idx)
 {
-	union ioapic_redir_entry *pent;
+	union kvm_ioapic_redirect_entry *pent;
 
 	pent = &ioapic->redirtbl[idx];
 
@@ -272,7 +272,7 @@ void kvm_ioapic_set_irq(struct kvm_ioapic *ioapic, int irq, int level)
 {
 	u32 old_irr = ioapic->irr;
 	u32 mask = 1 << irq;
-	union ioapic_redir_entry entry;
+	union kvm_ioapic_redirect_entry entry;
 
 	if (irq >= 0 && irq < IOAPIC_NUM_PINS) {
 		entry = ioapic->redirtbl[irq];
@@ -291,7 +291,7 @@ void kvm_ioapic_set_irq(struct kvm_ioapic *ioapic, int irq, int level)
 static void __kvm_ioapic_update_eoi(struct kvm_ioapic *ioapic, int gsi,
 				    int trigger_mode)
 {
-	union ioapic_redir_entry *ent;
+	union kvm_ioapic_redirect_entry *ent;
 
 	ent = &ioapic->redirtbl[gsi];
 
diff --git a/virt/kvm/ioapic.h b/virt/kvm/ioapic.h
index 49c9581..ee5b0bd 100644
--- a/virt/kvm/ioapic.h
+++ b/virt/kvm/ioapic.h
@@ -40,22 +40,7 @@ struct kvm_ioapic {
 	u32 id;
 	u32 irr;
 	u32 pad;
-	union ioapic_redir_entry {
-		u64 bits;
-		struct {
-			u8 vector;
-			u8 delivery_mode:3;
-			u8 dest_mode:1;
-			u8 delivery_status:1;
-			u8 polarity:1;
-			u8 remote_irr:1;
-			u8 trig_mode:1;
-			u8 mask:1;
-			u8 reserve:7;
-			u8 reserved[4];
-			u8 dest_id;
-		} fields;
-	} redirtbl[IOAPIC_NUM_PINS];
+	union kvm_ioapic_redirect_entry redirtbl[IOAPIC_NUM_PINS];
 	struct kvm_io_device dev;
 	struct kvm *kvm;
 	void (*ack_notifier)(void *opaque, int irq);
-- 
1.5.4.5

[PATCH 07/10] KVM: Unified the delivery of IOAPIC and MSI

[Sheng Yang]

Signed-off-by: Sheng Yang <sheng@linux.intel.com>
---
 include/linux/kvm_host.h |    3 ++
 virt/kvm/ioapic.c        |   84 +++++++++++++++++-----------------------------
 virt/kvm/irq_comm.c      |   75 ++++++++++++++++++++++++----------------
 3 files changed, 79 insertions(+), 83 deletions(-)

diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h
index 5b671b6..4f92317 100644
--- a/include/linux/kvm_host.h
+++ b/include/linux/kvm_host.h
@@ -330,6 +330,9 @@ struct kvm_gsi_msg {
 	struct hlist_node link;
 };
 
+void kvm_get_intr_delivery_bitmask(struct kvm_ioapic *ioapic,
+				   union kvm_ioapic_redirect_entry *entry,
+				   u32 *deliver_bitmask);
 void kvm_set_irq(struct kvm *kvm, int irq_source_id, u32 gsi, int level);
 void kvm_notify_acked_irq(struct kvm *kvm, unsigned gsi);
 void kvm_register_irq_ack_notifier(struct kvm *kvm,
diff --git a/virt/kvm/ioapic.c b/virt/kvm/ioapic.c
index ebb2ab5..af9f5de 100644
--- a/virt/kvm/ioapic.c
+++ b/virt/kvm/ioapic.c
@@ -195,75 +195,53 @@ u32 kvm_ioapic_get_delivery_bitmask(struct kvm_ioapic *ioapic, u8 dest,
 
 static int ioapic_deliver(struct kvm_ioapic *ioapic, int irq)
 {
-	u8 dest = ioapic->redirtbl[irq].fields.dest_id;
-	u8 dest_mode = ioapic->redirtbl[irq].fields.dest_mode;
-	u8 delivery_mode = ioapic->redirtbl[irq].fields.delivery_mode;
-	u8 vector = ioapic->redirtbl[irq].fields.vector;
-	u8 trig_mode = ioapic->redirtbl[irq].fields.trig_mode;
+	union kvm_ioapic_redirect_entry entry = ioapic->redirtbl[irq];
 	u32 deliver_bitmask;
 	struct kvm_vcpu *vcpu;
 	int vcpu_id, r = 0;
 
 	ioapic_debug("dest=%x dest_mode=%x delivery_mode=%x "
 		     "vector=%x trig_mode=%x\n",
-		     dest, dest_mode, delivery_mode, vector, trig_mode);
+		     entry.fields.dest, entry.fields.dest_mode,
+		     entry.fields.delivery_mode, entry.fields.vector,
+		     entry.fields.trig_mode);
 
-	deliver_bitmask = kvm_ioapic_get_delivery_bitmask(ioapic, dest,
-							  dest_mode);
+	kvm_get_intr_delivery_bitmask(ioapic, &entry, &deliver_bitmask);
 	if (!deliver_bitmask) {
 		ioapic_debug("no target on destination\n");
 		return 0;
 	}
 
-	switch (delivery_mode) {
-	case IOAPIC_LOWEST_PRIORITY:
-		vcpu = kvm_get_lowest_prio_vcpu(ioapic->kvm, vector,
-				deliver_bitmask);
+	/* Always delivery PIT interrupt to vcpu 0 */
 #ifdef CONFIG_X86
-		if (irq == 0)
-			vcpu = ioapic->kvm->vcpus[0];
+	if (irq == 0)
+		deliver_bitmask = 1 << 0;
 #endif
-		if (vcpu != NULL)
-			r = ioapic_inj_irq(ioapic, vcpu, vector,
-				       trig_mode, delivery_mode);
-		else
-			ioapic_debug("null lowest prio vcpu: "
-				     "mask=%x vector=%x delivery_mode=%x\n",
-				     deliver_bitmask, vector, IOAPIC_LOWEST_PRIORITY);
-		break;
-	case IOAPIC_FIXED:
-#ifdef CONFIG_X86
-		if (irq == 0)
-			deliver_bitmask = 1;
-#endif
-		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) {
-				r = ioapic_inj_irq(ioapic, vcpu, vector,
-					       trig_mode, delivery_mode);
-			}
-		}
-		break;
-	case IOAPIC_NMI:
-		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)
+
+	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) {
+			if (entry.fields.delivery_mode ==
+					IOAPIC_LOWEST_PRIORITY ||
+			    entry.fields.delivery_mode == IOAPIC_FIXED)
+				r = ioapic_inj_irq(ioapic, vcpu,
+						   entry.fields.vector,
+						   entry.fields.trig_mode,
+						   entry.fields.delivery_mode);
+			else if (entry.fields.delivery_mode == IOAPIC_NMI)
 				ioapic_inj_nmi(vcpu);
 			else
-				ioapic_debug("NMI to vcpu %d failed\n",
-						vcpu->vcpu_id);
-		}
-		break;
-	default:
-		printk(KERN_WARNING "Unsupported delivery mode %d\n",
-		       delivery_mode);
-		break;
+				ioapic_debug("unsupported delivery mode %x!\n",
+					     entry.fields.delivery_mode);
+		} else
+			ioapic_debug("null destination vcpu: "
+				     "mask=%x vector=%x delivery_mode=%x\n",
+				     entry.fields.deliver_bitmask,
+				     entry.fields.vector,
+				     entry.fields.delivery_mode);
 	}
 	return r;
 }
diff --git a/virt/kvm/irq_comm.c b/virt/kvm/irq_comm.c
index 63cdf01..d89d8b2 100644
--- a/virt/kvm/irq_comm.c
+++ b/virt/kvm/irq_comm.c
@@ -29,6 +29,29 @@
 
 #include "ioapic.h"
 
+void kvm_get_intr_delivery_bitmask(struct kvm_ioapic *ioapic,
+				   union kvm_ioapic_redirect_entry *entry,
+				   u32 *deliver_bitmask)
+{
+	struct kvm_vcpu *vcpu;
+
+	*deliver_bitmask = kvm_ioapic_get_delivery_bitmask(ioapic,
+				entry->fields.dest_id, entry->fields.dest_mode);
+	switch (entry->fields.delivery_mode) {
+	case IOAPIC_LOWEST_PRIORITY:
+		vcpu = kvm_get_lowest_prio_vcpu(ioapic->kvm,
+				entry->fields.vector, *deliver_bitmask);
+		*deliver_bitmask = 1 << vcpu->vcpu_id;
+		break;
+	case IOAPIC_FIXED:
+	case IOAPIC_NMI:
+		break;
+	default:
+		printk(KERN_INFO "kvm: unsupported delivery mode\n");
+		*deliver_bitmask = 0;
+	}
+}
+
 /* This should be called with the kvm->lock mutex held */
 void kvm_set_irq(struct kvm *kvm, int irq_source_id, u32 gsi, int level)
 {
@@ -38,7 +61,7 @@ void kvm_set_irq(struct kvm *kvm, int irq_source_id, u32 gsi, int level)
 	struct kvm_vcpu *vcpu;
 	struct kvm_ioapic *ioapic = ioapic_irqchip(kvm);
 	struct kvm_gsi_msg *gsi_msg;
-	int dest_id, vector, dest_mode, trig_mode, delivery_mode;
+	union kvm_ioapic_redirect_entry entry;
 	u32 deliver_bitmask;
 
 	BUG_ON(!ioapic);
@@ -75,40 +98,32 @@ void kvm_set_irq(struct kvm *kvm, int irq_source_id, u32 gsi, int level)
 		return;
 	}
 
-	dest_id = (gsi_msg->msg.address_lo & MSI_ADDR_DEST_ID_MASK)
+	entry.bits = 0;
+	entry.fields.dest_id = (gsi_msg->msg.address_lo & MSI_ADDR_DEST_ID_MASK)
 			>> MSI_ADDR_DEST_ID_SHIFT;
-	vector = (gsi_msg->msg.data & MSI_DATA_VECTOR_MASK)
+	entry.fields.vector = (gsi_msg->msg.data & MSI_DATA_VECTOR_MASK)
 			>> MSI_DATA_VECTOR_SHIFT;
-	dest_mode = test_bit(MSI_ADDR_DEST_MODE_SHIFT,
+	entry.fields.dest_mode = test_bit(MSI_ADDR_DEST_MODE_SHIFT,
 				(unsigned long *)&gsi_msg->msg.address_lo);
-	trig_mode = test_bit(MSI_DATA_TRIGGER_SHIFT,
+	entry.fields.trig_mode = test_bit(MSI_DATA_TRIGGER_SHIFT,
 				(unsigned long *)&gsi_msg->msg.data);
-	delivery_mode = test_bit(MSI_DATA_DELIVERY_MODE_SHIFT,
+	entry.fields.delivery_mode = test_bit(MSI_DATA_DELIVERY_MODE_SHIFT,
 				(unsigned long *)&gsi_msg->msg.data);
-	deliver_bitmask = kvm_ioapic_get_delivery_bitmask(ioapic,
-				dest_id, dest_mode);
-	/* IOAPIC delivery mode value is the same as MSI here */
-	switch (delivery_mode) {
-	case IOAPIC_LOWEST_PRIORITY:
-		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 IOAPIC_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");
+
+	kvm_get_intr_delivery_bitmask(ioapic, &entry, &deliver_bitmask);
+
+	if (!deliver_bitmask) {
+		printk(KERN_WARNING "kvm: no destination for MSI delivery!");
+		return;
+	}
+	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, entry.fields.vector,
+					 entry.fields.trig_mode);
 	}
 #endif
 }
-- 
1.5.4.5

[PATCH 08/10] KVM: Change API of kvm_ioapic_get_delivery_bitmask

[Sheng Yang]

In order to use with bit ops.

Signed-off-by: Sheng Yang <sheng@linux.intel.com>
---
 virt/kvm/ioapic.c   |   17 ++++++++---------
 virt/kvm/ioapic.h   |    4 ++--
 virt/kvm/irq_comm.c |    5 +++--
 3 files changed, 13 insertions(+), 13 deletions(-)

diff --git a/virt/kvm/ioapic.c b/virt/kvm/ioapic.c
index af9f5de..ebd5ba6 100644
--- a/virt/kvm/ioapic.c
+++ b/virt/kvm/ioapic.c
@@ -153,22 +153,22 @@ static void ioapic_inj_nmi(struct kvm_vcpu *vcpu)
 	kvm_vcpu_kick(vcpu);
 }
 
-u32 kvm_ioapic_get_delivery_bitmask(struct kvm_ioapic *ioapic, u8 dest,
-				    u8 dest_mode)
+void kvm_ioapic_get_delivery_bitmask(struct kvm_ioapic *ioapic, u8 dest,
+				     u8 dest_mode, u32 *mask)
 {
-	u32 mask = 0;
 	int i;
 	struct kvm *kvm = ioapic->kvm;
 	struct kvm_vcpu *vcpu;
 
 	ioapic_debug("dest %d dest_mode %d\n", dest, dest_mode);
 
+	*mask = 0;
 	if (dest_mode == 0) {	/* Physical mode. */
 		if (dest == 0xFF) {	/* Broadcast. */
 			for (i = 0; i < KVM_MAX_VCPUS; ++i)
 				if (kvm->vcpus[i] && kvm->vcpus[i]->arch.apic)
-					mask |= 1 << i;
-			return mask;
+					*mask |= 1 << i;
+			return;
 		}
 		for (i = 0; i < KVM_MAX_VCPUS; ++i) {
 			vcpu = kvm->vcpus[i];
@@ -176,7 +176,7 @@ u32 kvm_ioapic_get_delivery_bitmask(struct kvm_ioapic *ioapic, u8 dest,
 				continue;
 			if (kvm_apic_match_physical_addr(vcpu->arch.apic, dest)) {
 				if (vcpu->arch.apic)
-					mask = 1 << i;
+					*mask = 1 << i;
 				break;
 			}
 		}
@@ -187,10 +187,9 @@ u32 kvm_ioapic_get_delivery_bitmask(struct kvm_ioapic *ioapic, u8 dest,
 				continue;
 			if (vcpu->arch.apic &&
 			    kvm_apic_match_logical_addr(vcpu->arch.apic, dest))
-				mask |= 1 << vcpu->vcpu_id;
+				*mask |= 1 << vcpu->vcpu_id;
 		}
-	ioapic_debug("mask %x\n", mask);
-	return mask;
+	ioapic_debug("mask %x\n", *mask);
 }
 
 static int ioapic_deliver(struct kvm_ioapic *ioapic, int irq)
diff --git a/virt/kvm/ioapic.h b/virt/kvm/ioapic.h
index ee5b0bd..e107dbb 100644
--- a/virt/kvm/ioapic.h
+++ b/virt/kvm/ioapic.h
@@ -70,7 +70,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);
+void kvm_ioapic_get_delivery_bitmask(struct kvm_ioapic *ioapic, u8 dest,
+				     u8 dest_mode, u32 *mask);
 
 #endif
diff --git a/virt/kvm/irq_comm.c b/virt/kvm/irq_comm.c
index d89d8b2..1949587 100644
--- a/virt/kvm/irq_comm.c
+++ b/virt/kvm/irq_comm.c
@@ -35,8 +35,9 @@ void kvm_get_intr_delivery_bitmask(struct kvm_ioapic *ioapic,
 {
 	struct kvm_vcpu *vcpu;
 
-	*deliver_bitmask = kvm_ioapic_get_delivery_bitmask(ioapic,
-				entry->fields.dest_id, entry->fields.dest_mode);
+	kvm_ioapic_get_delivery_bitmask(ioapic, entry->fields.dest_id,
+					entry->fields.dest_mode,
+					deliver_bitmask);
 	switch (entry->fields.delivery_mode) {
 	case IOAPIC_LOWEST_PRIORITY:
 		vcpu = kvm_get_lowest_prio_vcpu(ioapic->kvm,
-- 
1.5.4.5

[PATCH 09/10] KVM: Update intr delivery func to accept unsigned long* bitmap

[Sheng Yang]

Would be used with bit ops, and would be easily extended if KVM_MAX_VCPUS is
increased.

Signed-off-by: Sheng Yang <sheng@linux.intel.com>
---
 arch/x86/kvm/lapic.c     |    8 ++++----
 include/linux/kvm_host.h |    2 +-
 virt/kvm/ioapic.c        |    4 ++--
 virt/kvm/ioapic.h        |    4 ++--
 virt/kvm/irq_comm.c      |    6 +++---
 5 files changed, 12 insertions(+), 12 deletions(-)

diff --git a/arch/x86/kvm/lapic.c b/arch/x86/kvm/lapic.c
index afac68c..c1e4935 100644
--- a/arch/x86/kvm/lapic.c
+++ b/arch/x86/kvm/lapic.c
@@ -403,7 +403,7 @@ static int __apic_accept_irq(struct kvm_lapic *apic, int delivery_mode,
 }
 
 static struct kvm_lapic *kvm_apic_round_robin(struct kvm *kvm, u8 vector,
-				       unsigned long bitmap)
+				       unsigned long *bitmap)
 {
 	int last;
 	int next;
@@ -415,7 +415,7 @@ static struct kvm_lapic *kvm_apic_round_robin(struct kvm *kvm, u8 vector,
 	do {
 		if (++next == KVM_MAX_VCPUS)
 			next = 0;
-		if (kvm->vcpus[next] == NULL || !test_bit(next, &bitmap))
+		if (kvm->vcpus[next] == NULL || !test_bit(next, bitmap))
 			continue;
 		apic = kvm->vcpus[next]->arch.apic;
 		if (apic && apic_enabled(apic))
@@ -431,7 +431,7 @@ static struct kvm_lapic *kvm_apic_round_robin(struct kvm *kvm, u8 vector,
 }
 
 struct kvm_vcpu *kvm_get_lowest_prio_vcpu(struct kvm *kvm, u8 vector,
-		unsigned long bitmap)
+		unsigned long *bitmap)
 {
 	struct kvm_lapic *apic;
 
@@ -502,7 +502,7 @@ static void apic_send_ipi(struct kvm_lapic *apic)
 	}
 
 	if (delivery_mode == APIC_DM_LOWEST) {
-		target = kvm_get_lowest_prio_vcpu(vcpu->kvm, vector, lpr_map);
+		target = kvm_get_lowest_prio_vcpu(vcpu->kvm, vector, &lpr_map);
 		if (target != NULL)
 			__apic_accept_irq(target->arch.apic, delivery_mode,
 					  vector, level, trig_mode);
diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h
index 4f92317..fbf102c 100644
--- a/include/linux/kvm_host.h
+++ b/include/linux/kvm_host.h
@@ -332,7 +332,7 @@ struct kvm_gsi_msg {
 
 void kvm_get_intr_delivery_bitmask(struct kvm_ioapic *ioapic,
 				   union kvm_ioapic_redirect_entry *entry,
-				   u32 *deliver_bitmask);
+				   unsigned long *deliver_bitmask);
 void kvm_set_irq(struct kvm *kvm, int irq_source_id, u32 gsi, int level);
 void kvm_notify_acked_irq(struct kvm *kvm, unsigned gsi);
 void kvm_register_irq_ack_notifier(struct kvm *kvm,
diff --git a/virt/kvm/ioapic.c b/virt/kvm/ioapic.c
index ebd5ba6..164a746 100644
--- a/virt/kvm/ioapic.c
+++ b/virt/kvm/ioapic.c
@@ -154,7 +154,7 @@ static void ioapic_inj_nmi(struct kvm_vcpu *vcpu)
 }
 
 void kvm_ioapic_get_delivery_bitmask(struct kvm_ioapic *ioapic, u8 dest,
-				     u8 dest_mode, u32 *mask)
+				     u8 dest_mode, unsigned long *mask)
 {
 	int i;
 	struct kvm *kvm = ioapic->kvm;
@@ -195,7 +195,7 @@ void kvm_ioapic_get_delivery_bitmask(struct kvm_ioapic *ioapic, u8 dest,
 static int ioapic_deliver(struct kvm_ioapic *ioapic, int irq)
 {
 	union kvm_ioapic_redirect_entry entry = ioapic->redirtbl[irq];
-	u32 deliver_bitmask;
+	unsigned long deliver_bitmask;
 	struct kvm_vcpu *vcpu;
 	int vcpu_id, r = 0;
 
diff --git a/virt/kvm/ioapic.h b/virt/kvm/ioapic.h
index e107dbb..c418a7f 100644
--- a/virt/kvm/ioapic.h
+++ b/virt/kvm/ioapic.h
@@ -65,12 +65,12 @@ static inline struct kvm_ioapic *ioapic_irqchip(struct kvm *kvm)
 }
 
 struct kvm_vcpu *kvm_get_lowest_prio_vcpu(struct kvm *kvm, u8 vector,
-				       unsigned long bitmap);
+				       unsigned long *bitmap);
 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);
 void kvm_ioapic_get_delivery_bitmask(struct kvm_ioapic *ioapic, u8 dest,
-				     u8 dest_mode, u32 *mask);
+				     u8 dest_mode, unsigned long *mask);
 
 #endif
diff --git a/virt/kvm/irq_comm.c b/virt/kvm/irq_comm.c
index 1949587..e74d679 100644
--- a/virt/kvm/irq_comm.c
+++ b/virt/kvm/irq_comm.c
@@ -31,7 +31,7 @@
 
 void kvm_get_intr_delivery_bitmask(struct kvm_ioapic *ioapic,
 				   union kvm_ioapic_redirect_entry *entry,
-				   u32 *deliver_bitmask)
+				   unsigned long *deliver_bitmask)
 {
 	struct kvm_vcpu *vcpu;
 
@@ -41,7 +41,7 @@ void kvm_get_intr_delivery_bitmask(struct kvm_ioapic *ioapic,
 	switch (entry->fields.delivery_mode) {
 	case IOAPIC_LOWEST_PRIORITY:
 		vcpu = kvm_get_lowest_prio_vcpu(ioapic->kvm,
-				entry->fields.vector, *deliver_bitmask);
+				entry->fields.vector, deliver_bitmask);
 		*deliver_bitmask = 1 << vcpu->vcpu_id;
 		break;
 	case IOAPIC_FIXED:
@@ -63,7 +63,7 @@ void kvm_set_irq(struct kvm *kvm, int irq_source_id, u32 gsi, int level)
 	struct kvm_ioapic *ioapic = ioapic_irqchip(kvm);
 	struct kvm_gsi_msg *gsi_msg;
 	union kvm_ioapic_redirect_entry entry;
-	u32 deliver_bitmask;
+	unsigned long deliver_bitmask;
 
 	BUG_ON(!ioapic);
 #endif
-- 
1.5.4.5

[PATCH 10/10] KVM: bit ops for deliver_bitmap

[Sheng Yang]

It's also convenient when we extend KVM supported vcpu number in the future.

Signed-off-by: Sheng Yang <sheng@linux.intel.com>
---
 arch/x86/kvm/lapic.c |    7 ++++---
 virt/kvm/ioapic.c    |   24 +++++++++++++-----------
 virt/kvm/irq_comm.c  |   16 ++++++++--------
 3 files changed, 25 insertions(+), 22 deletions(-)

diff --git a/arch/x86/kvm/lapic.c b/arch/x86/kvm/lapic.c
index c1e4935..359e02c 100644
--- a/arch/x86/kvm/lapic.c
+++ b/arch/x86/kvm/lapic.c
@@ -477,9 +477,10 @@ static void apic_send_ipi(struct kvm_lapic *apic)
 
 	struct kvm_vcpu *target;
 	struct kvm_vcpu *vcpu;
-	unsigned long lpr_map = 0;
+	DECLARE_BITMAP(lpr_map, KVM_MAX_VCPUS);
 	int i;
 
+	bitmap_zero(lpr_map, KVM_MAX_VCPUS);
 	apic_debug("icr_high 0x%x, icr_low 0x%x, "
 		   "short_hand 0x%x, dest 0x%x, trig_mode 0x%x, level 0x%x, "
 		   "dest_mode 0x%x, delivery_mode 0x%x, vector 0x%x\n",
@@ -494,7 +495,7 @@ static void apic_send_ipi(struct kvm_lapic *apic)
 		if (vcpu->arch.apic &&
 		    apic_match_dest(vcpu, apic, short_hand, dest, dest_mode)) {
 			if (delivery_mode == APIC_DM_LOWEST)
-				set_bit(vcpu->vcpu_id, &lpr_map);
+				set_bit(vcpu->vcpu_id, lpr_map);
 			else
 				__apic_accept_irq(vcpu->arch.apic, delivery_mode,
 						  vector, level, trig_mode);
@@ -502,7 +503,7 @@ static void apic_send_ipi(struct kvm_lapic *apic)
 	}
 
 	if (delivery_mode == APIC_DM_LOWEST) {
-		target = kvm_get_lowest_prio_vcpu(vcpu->kvm, vector, &lpr_map);
+		target = kvm_get_lowest_prio_vcpu(vcpu->kvm, vector, lpr_map);
 		if (target != NULL)
 			__apic_accept_irq(target->arch.apic, delivery_mode,
 					  vector, level, trig_mode);
diff --git a/virt/kvm/ioapic.c b/virt/kvm/ioapic.c
index 164a746..bf83f5e 100644
--- a/virt/kvm/ioapic.c
+++ b/virt/kvm/ioapic.c
@@ -195,7 +195,7 @@ void kvm_ioapic_get_delivery_bitmask(struct kvm_ioapic *ioapic, u8 dest,
 static int ioapic_deliver(struct kvm_ioapic *ioapic, int irq)
 {
 	union kvm_ioapic_redirect_entry entry = ioapic->redirtbl[irq];
-	unsigned long deliver_bitmask;
+	DECLARE_BITMAP(deliver_bitmask, KVM_MAX_VCPUS);
 	struct kvm_vcpu *vcpu;
 	int vcpu_id, r = 0;
 
@@ -205,22 +205,24 @@ static int ioapic_deliver(struct kvm_ioapic *ioapic, int irq)
 		     entry.fields.delivery_mode, entry.fields.vector,
 		     entry.fields.trig_mode);
 
-	kvm_get_intr_delivery_bitmask(ioapic, &entry, &deliver_bitmask);
-	if (!deliver_bitmask) {
-		ioapic_debug("no target on destination\n");
-		return 0;
-	}
+	bitmap_zero(deliver_bitmask, KVM_MAX_VCPUS);
 
 	/* Always delivery PIT interrupt to vcpu 0 */
 #ifdef CONFIG_X86
 	if (irq == 0)
-		deliver_bitmask = 1 << 0;
+		set_bit(0, deliver_bitmask);
+	else
 #endif
+		kvm_get_intr_delivery_bitmask(ioapic, &entry, deliver_bitmask);
+
+	if (find_first_bit(deliver_bitmask, KVM_MAX_VCPUS) >= KVM_MAX_VCPUS) {
+		ioapic_debug("no target on destination\n");
+		return 0;
+	}
 
-	for (vcpu_id = 0; deliver_bitmask != 0; vcpu_id++) {
-		if (!(deliver_bitmask & (1 << vcpu_id)))
-			continue;
-		deliver_bitmask &= ~(1 << vcpu_id);
+	while ((vcpu_id = find_first_bit(deliver_bitmask, KVM_MAX_VCPUS))
+			< KVM_MAX_VCPUS) {
+		clear_bit(vcpu_id, deliver_bitmask);
 		vcpu = ioapic->kvm->vcpus[vcpu_id];
 		if (vcpu) {
 			if (entry.fields.delivery_mode ==
diff --git a/virt/kvm/irq_comm.c b/virt/kvm/irq_comm.c
index e74d679..ecda2c1 100644
--- a/virt/kvm/irq_comm.c
+++ b/virt/kvm/irq_comm.c
@@ -42,7 +42,7 @@ void kvm_get_intr_delivery_bitmask(struct kvm_ioapic *ioapic,
 	case IOAPIC_LOWEST_PRIORITY:
 		vcpu = kvm_get_lowest_prio_vcpu(ioapic->kvm,
 				entry->fields.vector, deliver_bitmask);
-		*deliver_bitmask = 1 << vcpu->vcpu_id;
+		set_bit(vcpu->vcpu_id, deliver_bitmask);
 		break;
 	case IOAPIC_FIXED:
 	case IOAPIC_NMI:
@@ -63,11 +63,12 @@ void kvm_set_irq(struct kvm *kvm, int irq_source_id, u32 gsi, int level)
 	struct kvm_ioapic *ioapic = ioapic_irqchip(kvm);
 	struct kvm_gsi_msg *gsi_msg;
 	union kvm_ioapic_redirect_entry entry;
-	unsigned long deliver_bitmask;
+	DECLARE_BITMAP(deliver_bitmask, KVM_MAX_VCPUS);
 
 	BUG_ON(!ioapic);
 #endif
 
+	bitmap_zero(deliver_bitmask, KVM_MAX_VCPUS);
 	if (!(gsi & KVM_GSI_MSG_MASK)) {
 		int irq = gsi;
 
@@ -111,16 +112,15 @@ void kvm_set_irq(struct kvm *kvm, int irq_source_id, u32 gsi, int level)
 	entry.fields.delivery_mode = test_bit(MSI_DATA_DELIVERY_MODE_SHIFT,
 				(unsigned long *)&gsi_msg->msg.data);
 
-	kvm_get_intr_delivery_bitmask(ioapic, &entry, &deliver_bitmask);
+	kvm_get_intr_delivery_bitmask(ioapic, &entry, deliver_bitmask);
 
-	if (!deliver_bitmask) {
+	if (find_first_bit(deliver_bitmask, KVM_MAX_VCPUS) >= KVM_MAX_VCPUS) {
 		printk(KERN_WARNING "kvm: no destination for MSI delivery!");
 		return;
 	}
-	for (vcpu_id = 0; deliver_bitmask != 0; vcpu_id++) {
-		if (!(deliver_bitmask & (1 << vcpu_id)))
-			continue;
-		deliver_bitmask &= ~(1 << vcpu_id);
+	while ((vcpu_id = find_first_bit(deliver_bitmask, KVM_MAX_VCPUS))
+			< KVM_MAX_VCPUS) {
+		clear_bit(vcpu_id, deliver_bitmask);
 		vcpu = ioapic->kvm->vcpus[vcpu_id];
 		if (vcpu)
 			kvm_apic_set_irq(vcpu, entry.fields.vector,
-- 
1.5.4.5

Re: [PATCH 0/10][v3] GSI->MSG route layer for MSI/MSI-X

[Sheng Yang]

On Tuesday 30 December 2008 13:55:52 Sheng Yang wrote:
> Update from v2:
> Add gsi_msg_pending_bitmap, in order to support MSI-X mutiply interrupts.
>
> And this one depends on the former "Device assignment code clean up and MSI
> disable support" patchset.
>
Sorry, a little chaos here. The gsi_msg_pending_bitmap is in MSI-X patchset...

So this one should be almost the same as v2. Just resend.

-- 
regards
Yang, Sheng

Re: [PATCH 01/10] KVM: Add a route layer to convert MSI message to GSI

[Avi Kivity]

Sheng Yang wrote:
> Avi's purpose, to use single kvm_set_irq() to deal with all interrupt, including
> MSI. So here is it.
>
> struct gsi_msg is a mapping from a special gsi(with KVM_GSI_MSG_MASK) to
> MSI/MSI-X message address/data.
>
> Now we support up to 256 gsi_msg mapping, and gsi_msg is allocated by kernel and
> provide two ioctls to userspace, which is more flexiable.
>
>   

> +#define KVM_REQUEST_GSI_MSG _IOWR(KVMIO, 0x71, struct kvm_assigned_gsi_msg)
> +#define KVM_FREE_GSI_MSG _IOR(KVMIO, 0x72, struct kvm_assigned_gsi_msg)
>   

We will also need a pair for PIC and a pair for IOAPIC routing.

How about a single ioctl to set the entire routing table?  It would take 
an array of structures:

struct {
    __u32 gsi;
    __u32 type;
    __u32 flags;
    __u32 reserved;
    union {
         struct {
             __u32 irq;
         } pic;
         struct {
            __u32 ioapic; // can have >1 ioapic
            __u32 inti;
         } ioapic;
         struct {
            ...
         } msi;
         __u32 reserved[8];
    };
};

This way we can solve the HPET irq0/inti2 mess, and also have a simpler 
way of setting MSI.  All the mess in one ioctl.

> +
> +/* Call with kvm->gsi_msg_lock hold */
> +struct kvm_gsi_msg *kvm_find_gsi_msg(struct kvm *kvm, u32 gsi)
> +{
> +	struct kvm_gsi_msg *gsi_msg;
> +	struct hlist_node *n;
> +
> +	if (!(gsi & KVM_GSI_MSG_MASK))
> +		return NULL;
> +	hlist_for_each_entry(gsi_msg, n, &kvm->gsi_msg_list, link)
> +		if (gsi_msg->gsi == gsi)
> +			goto out;
> +	gsi_msg = NULL;
> +out:
> +	return gsi_msg;
> +}
>   

Linear search is a bit sad but fine for now.  Later we can add an array 
indexed by gsi.


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

Re: [PATCH 05/10] KVM: Merge MSI handling to kvm_set_irq

[Avi Kivity]

Sheng Yang wrote:
> Using kvm_set_irq to handle all interrupt injection.
>
>  
>  /* This should be called with the kvm->lock mutex held */
> -void kvm_set_irq(struct kvm *kvm, int irq_source_id, int irq, int level)
> +void kvm_set_irq(struct kvm *kvm, int irq_source_id, u32 gsi, int level)
>  {
> -	unsigned long *irq_state = (unsigned long *)&kvm->arch.irq_states[irq];
> -
> -	/* Logical OR for level trig interrupt */
> -	if (level)
> -		set_bit(irq_source_id, irq_state);
> -	else
> -		clear_bit(irq_source_id, irq_state);
> -
> -	/* Not possible to detect if the guest uses the PIC or the
> -	 * IOAPIC.  So set the bit in both. The guest will ignore
> -	 * writes to the unused one.
> -	 */
> -	kvm_ioapic_set_irq(ioapic_irqchip(kvm), irq, !!(*irq_state));
> +	unsigned long *irq_state;
> +#ifdef CONFIG_X86
> +	int vcpu_id;
> +	struct kvm_vcpu *vcpu;
> +	struct kvm_ioapic *ioapic = ioapic_irqchip(kvm);
> +	struct kvm_gsi_msg *gsi_msg;
> +	int dest_id, vector, dest_mode, trig_mode, delivery_mode;
> +	u32 deliver_bitmask;
> +
> +	BUG_ON(!ioapic);
> +#endif
> +
> +	if (!(gsi & KVM_GSI_MSG_MASK)) {
> +		int irq = gsi;
> +
> +		irq_state = (unsigned long *)&kvm->arch.irq_states[irq];
> +
> +		/* Logical OR for level trig interrupt */
> +		if (level)
> +			set_bit(irq_source_id, irq_state);
> +		else
> +			clear_bit(irq_source_id, irq_state);
> +
> +		/* Not possible to detect if the guest uses the PIC or the
> +		 * IOAPIC.  So set the bit in both. The guest will ignore
> +		 * writes to the unused one.
> +		 */
> +		kvm_ioapic_set_irq(ioapic, irq, !!(*irq_state));
>  #ifdef CONFIG_X86
> -	kvm_pic_set_irq(pic_irqchip(kvm), irq, !!(*irq_state));
> +		kvm_pic_set_irq(pic_irqchip(kvm), irq, !!(*irq_state));
> +#endif
> +		return;
> +	}
> +
> +#ifdef CONFIG_X86
> +	mutex_lock(&kvm->gsi_msg_lock);
>   

The lock is already taken here?

> +	gsi_msg = kvm_find_gsi_msg(kvm, gsi);
> +	mutex_unlock(&kvm->gsi_msg_lock);
> +	if (!gsi_msg) {
> +		printk(KERN_WARNING "kvm: fail to find correlated gsi_msg\n");
> +		return;
> +	}
> +
> +	dest_id = (gsi_msg->msg.address_lo & MSI_ADDR_DEST_ID_MASK)
> +			>> MSI_ADDR_DEST_ID_SHIFT;
> +	vector = (gsi_msg->msg.data & MSI_DATA_VECTOR_MASK)
> +			>> MSI_DATA_VECTOR_SHIFT;
> +	dest_mode = test_bit(MSI_ADDR_DEST_MODE_SHIFT,
> +				(unsigned long *)&gsi_msg->msg.address_lo);
> +	trig_mode = test_bit(MSI_DATA_TRIGGER_SHIFT,
> +				(unsigned long *)&gsi_msg->msg.data);
> +	delivery_mode = test_bit(MSI_DATA_DELIVERY_MODE_SHIFT,
> +				(unsigned long *)&gsi_msg->msg.data);
> +	deliver_bitmask = kvm_ioapic_get_delivery_bitmask(ioapic,
> +				dest_id, dest_mode);
> +	/* IOAPIC delivery mode value is the same as MSI here */
> +	switch (delivery_mode) {
> +	case IOAPIC_LOWEST_PRIORITY:
> +		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 IOAPIC_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");
> +	}
>  #endif
>  }
>  
>   

This looks very messy.  Would be better to have the in-kernel irq 
structure contain a (*set_level)() callback that can take the 
appropriate action.

Also, the CONFIG_X86 worries me, can we have IA64 enable this as well?

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

Re: [PATCH 05/10] KVM: Merge MSI handling to kvm_set_irq

[Sheng Yang]

On Tuesday 30 December 2008 18:48:42 Avi Kivity wrote:
> Sheng Yang wrote:
> > Using kvm_set_irq to handle all interrupt injection.
> >
> >
> >  /* This should be called with the kvm->lock mutex held */
> > -void kvm_set_irq(struct kvm *kvm, int irq_source_id, int irq, int level)
> > +void kvm_set_irq(struct kvm *kvm, int irq_source_id, u32 gsi, int level)
> >  {
> > -	unsigned long *irq_state = (unsigned long *)&kvm->arch.irq_states[irq];
> > -
> > -	/* Logical OR for level trig interrupt */
> > -	if (level)
> > -		set_bit(irq_source_id, irq_state);
> > -	else
> > -		clear_bit(irq_source_id, irq_state);
> > -
> > -	/* Not possible to detect if the guest uses the PIC or the
> > -	 * IOAPIC.  So set the bit in both. The guest will ignore
> > -	 * writes to the unused one.
> > -	 */
> > -	kvm_ioapic_set_irq(ioapic_irqchip(kvm), irq, !!(*irq_state));
> > +	unsigned long *irq_state;
> > +#ifdef CONFIG_X86
> > +	int vcpu_id;
> > +	struct kvm_vcpu *vcpu;
> > +	struct kvm_ioapic *ioapic = ioapic_irqchip(kvm);
> > +	struct kvm_gsi_msg *gsi_msg;
> > +	int dest_id, vector, dest_mode, trig_mode, delivery_mode;
> > +	u32 deliver_bitmask;
> > +
> > +	BUG_ON(!ioapic);
> > +#endif
> > +
> > +	if (!(gsi & KVM_GSI_MSG_MASK)) {
> > +		int irq = gsi;
> > +
> > +		irq_state = (unsigned long *)&kvm->arch.irq_states[irq];
> > +
> > +		/* Logical OR for level trig interrupt */
> > +		if (level)
> > +			set_bit(irq_source_id, irq_state);
> > +		else
> > +			clear_bit(irq_source_id, irq_state);
> > +
> > +		/* Not possible to detect if the guest uses the PIC or the
> > +		 * IOAPIC.  So set the bit in both. The guest will ignore
> > +		 * writes to the unused one.
> > +		 */
> > +		kvm_ioapic_set_irq(ioapic, irq, !!(*irq_state));
> >  #ifdef CONFIG_X86
> > -	kvm_pic_set_irq(pic_irqchip(kvm), irq, !!(*irq_state));
> > +		kvm_pic_set_irq(pic_irqchip(kvm), irq, !!(*irq_state));
> > +#endif
> > +		return;
> > +	}
> > +
> > +#ifdef CONFIG_X86
> > +	mutex_lock(&kvm->gsi_msg_lock);
>
> The lock is already taken here?

Um? For gsi_msg_lock?
>
> > +	gsi_msg = kvm_find_gsi_msg(kvm, gsi);
> > +	mutex_unlock(&kvm->gsi_msg_lock);
> > +	if (!gsi_msg) {
> > +		printk(KERN_WARNING "kvm: fail to find correlated gsi_msg\n");
> > +		return;
> > +	}
> > +
> > +	dest_id = (gsi_msg->msg.address_lo & MSI_ADDR_DEST_ID_MASK)
> > +			>> MSI_ADDR_DEST_ID_SHIFT;
> > +	vector = (gsi_msg->msg.data & MSI_DATA_VECTOR_MASK)
> > +			>> MSI_DATA_VECTOR_SHIFT;
> > +	dest_mode = test_bit(MSI_ADDR_DEST_MODE_SHIFT,
> > +				(unsigned long *)&gsi_msg->msg.address_lo);
> > +	trig_mode = test_bit(MSI_DATA_TRIGGER_SHIFT,
> > +				(unsigned long *)&gsi_msg->msg.data);
> > +	delivery_mode = test_bit(MSI_DATA_DELIVERY_MODE_SHIFT,
> > +				(unsigned long *)&gsi_msg->msg.data);
> > +	deliver_bitmask = kvm_ioapic_get_delivery_bitmask(ioapic,
> > +				dest_id, dest_mode);
> > +	/* IOAPIC delivery mode value is the same as MSI here */
> > +	switch (delivery_mode) {
> > +	case IOAPIC_LOWEST_PRIORITY:
> > +		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 IOAPIC_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");
> > +	}
> >  #endif
> >  }
>
> This looks very messy.  Would be better to have the in-kernel irq
> structure contain a (*set_level)() callback that can take the
> appropriate action.

You means this part which would merged with ioapic, or something else?
>
> Also, the CONFIG_X86 worries me, can we have IA64 enable this as well?

IA64 MSI enabling is on the task list, but it's pity we are too busy 
recently... 

-- 
regards
Yang, Sheng

Re: [PATCH 05/10] KVM: Merge MSI handling to kvm_set_irq

[Avi Kivity]

Sheng Yang wrote:
>>> +	mutex_lock(&kvm->gsi_msg_lock);
>>>       
>> The lock is already taken here?
>>     
>
> Um? For gsi_msg_lock?
>   

Sorry, my mistake.  Will have to get used to all those locks.

Is there a way to avoid the lock?  We're starting to complicate things...

>> This looks very messy.  Would be better to have the in-kernel irq
>> structure contain a (*set_level)() callback that can take the
>> appropriate action.
>>     
>
> You means this part which would merged with ioapic, or something else?
>   

At the very least, separated into functions.

At best, kvm_set_irq() should become generic and just invoke callbacks 
supplied by the irqchip (pit, ioapic, msi).


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

Re: [PATCH 05/10] KVM: Merge MSI handling to kvm_set_irq

[Sheng Yang]

On Tuesday 30 December 2008 19:07:52 Avi Kivity wrote:
> Sheng Yang wrote:
> >>> +	mutex_lock(&kvm->gsi_msg_lock);
> >>
> >> The lock is already taken here?
> >
> > Um? For gsi_msg_lock?
>
> Sorry, my mistake.  Will have to get used to all those locks.
>
> Is there a way to avoid the lock?  We're starting to complicate things...

Well, one list, one lock... But it's not very performance affect one, so maybe 
I can try kvm->lock...
>
> >> This looks very messy.  Would be better to have the in-kernel irq
> >> structure contain a (*set_level)() callback that can take the
> >> appropriate action.
> >
> > You means this part which would merged with ioapic, or something else?
>
> At the very least, separated into functions.
>
> At best, kvm_set_irq() should become generic and just invoke callbacks
> supplied by the irqchip (pit, ioapic, msi).

OK, I would separate it first. Callback function, a little later...

-- 
regards
Yang, Sheng