MSI-X mask emulation support for assigned device

It would be used by KVM later.

Cc Jesse Barnes <[email protected]>

Cc [email protected]

Signed-off-by Sheng Yang <[email protected]>

We need to query the entry later.

virt/kvm/irq_comm.c +10 -0

+struct kvm_kernel_irq_routing_entry *kvm_get_irq_routing_entry(struct kvm *kvm,
+							       int gsi)
+{
+	int count = 0;
+	struct kvm_kernel_irq_routing_entry *ei = NULL;
+	struct kvm_irq_routing_table *irq_rt;
+	struct hlist_node *n;
+
+	rcu_read_lock();
+	irq_rt = rcu_dereference(kvm->irq_routing);

Quoted 19 lines · Sheng Yang

We need to query the entry later.

+struct kvm_kernel_irq_routing_entry *kvm_get_irq_routing_entry(struct
kvm *kvm, + int gsi)

virt/kvm/irq_comm.c +14 -0

+{
+	int count = 0;
+	struct kvm_kernel_irq_routing_entry *ei = NULL;
+	struct kvm_irq_routing_table *irq_rt;
+	struct hlist_node *n;
+
+	rcu_read_lock();
+	irq_rt = rcu_dereference(kvm->irq_routing);
+	if (gsi<  irq_rt->nr_rt_entries)
+		hlist_for_each_entry(ei, n,&irq_rt->map[gsi], link)
+			count++;
+	rcu_read_unlock();
+	if (count == 1)
+		return ei;

virt/kvm/irq_comm.c out: · line 443

Quoted 4 lines

virt/kvm/irq_comm.c +4 -0

+
+	return NULL;
+}
+

I believe this is incorrect rcu usage. rcu_read_lock() prevents ei from being destroyed under us, but rcu_read_unlock() removes that protection, and a future dereference of ei may access freed memory.

  }

virt/kvm/irq_comm.c +6 -0

+int kvm_get_irq_routing_entry(struct kvm *kvm, int gsi,
+		struct kvm_kernel_irq_routing_entry *entry)
+{
+	int count = 0;
+	struct kvm_kernel_irq_routing_entry *ei = NULL;
+	struct kvm_irq_routing_table *irq_rt;

The feature wouldn't be enabled until later patch set msix_flags_enabled. It would be enabled along with mask support in kernel.

arch/x86/include/asm/kvm_host.h +0 -0

index e209078..2bb69ba 100644

arch/x86/include/asm/kvm_host.h +31 -0

@@ -666,11 +666,35 @@ msix_nr_out:
  	return r;
  }
 
+static void update_msix_mask(struct kvm_assigned_dev_kernel *assigned_dev,
+			     int index)
+{
+	int irq;
+	struct irq_desc *desc;
+
+	if (!assigned_dev->dev->msix_enabled ||
+	    !(assigned_dev->irq_requested_type&  KVM_DEV_IRQ_HOST_MSIX))
+		return;
+
+	irq = assigned_dev->host_msix_entries[index].vector;
+	BUG_ON(irq == 0);
+	desc = irq_to_desc(irq);
+	BUG_ON(!desc->msi_desc);
+
+	if (assigned_dev->guest_msix_entries[index].flags&
+			KVM_ASSIGNED_MSIX_MASK) {
+		desc->chip->mask(irq);
+		flush_work(&assigned_dev->interrupt_work);
+	} else
+		desc->chip->unmask(irq);
+}
+
  static int kvm_vm_ioctl_set_msix_entry(struct kvm *kvm,
  				       struct kvm_assigned_msix_entry *entry)
  {
  	int r = 0, i;
  	struct kvm_assigned_dev_kernel *adev;
+	bool entry_masked;
 
  	mutex_lock(&kvm->lock);
 

@@ -688,6 +712,21 @@ static int kvm_vm_ioctl_set_msix_entry(struct kvm *kvm,
  			adev->guest_msix_entries[i].entry = entry->entry;
  			adev->guest_msix_entries[i].vector = entry->gsi;
  			adev->host_msix_entries[i].entry = entry->entry;
+			if (!kvm->arch.msix_flags_enabled)
+				break;
+			entry_masked = adev->guest_msix_entries[i].flags&
+				KVM_ASSIGNED_MSIX_MASK;
+			if ((entry->flags&  KVM_MSIX_FLAG_MASK)&&
+					!entry_masked) {
+				adev->guest_msix_entries[i].flags |=

The feature wouldn't be enabled until later patch set msix_flags_enabled. It would be enabled along with mask support in kernel.

Signed-off-by Sheng Yang <[email protected]>

---
 arch/x86/include/asm/kvm_host.h |    2 ++
 include/linux/kvm.h             |    6 +++++-
 include/linux/kvm_host.h        |    1 +
 virt/kvm/assigned-dev.c         |   39 +++++++++++++++++++++++++++++++++++++++
 4 files changed, 47 insertions(+), 1 deletions(-)

arch/x86/include/asm/kvm_host.h +2 -0

index e209078..2bb69ba 100644
@@ -456,6 +456,8 @@ struct kvm_arch {
 	/* fields used by HYPER-V emulation */
 	u64 hv_guest_os_id;
 	u64 hv_hypercall;
+
+	bool msix_flags_enabled;
 };
 
 struct kvm_vm_stat {

include/linux/kvm.h +5 -1

index 919ae53..a699ec9 100644
@@ -787,11 +787,15 @@ struct kvm_assigned_msix_nr {
 };
 
 #define KVM_MAX_MSIX_PER_DEV		256
+
+#define KVM_MSIX_FLAG_MASK	1
+
 struct kvm_assigned_msix_entry {
 	__u32 assigned_dev_id;
 	__u32 gsi;
 	__u16 entry; /* The index of entry in the MSI-X table */
-	__u16 padding[3];
+	__u16 flags;
+	__u16 padding[2];
 };
 
 #endif /* __LINUX_KVM_H */

include/linux/kvm_host.h +1 -0

index 30f83cd..81a6284 100644
@@ -438,6 +438,7 @@ struct kvm_irq_ack_notifier {
 };
 
 #define KVM_ASSIGNED_MSIX_PENDING		0x1
+#define KVM_ASSIGNED_MSIX_MASK			0x2
 struct kvm_guest_msix_entry {
 	u32 vector;
 	u16 entry;

virt/kvm/assigned-dev.c +12 -0

index 7c98928..bf96ea7 100644
@@ -666,11 +666,35 @@ msix_nr_out:
 	return r;
 }
 
+static void update_msix_mask(struct kvm_assigned_dev_kernel *assigned_dev,
+			     int index)
+{
+	int irq;
+	struct irq_desc *desc;
+
+	if (!assigned_dev->dev->msix_enabled ||
+	    !(assigned_dev->irq_requested_type & KVM_DEV_IRQ_HOST_MSIX))
+		return;
+
+	irq = assigned_dev->host_msix_entries[index].vector;
+	BUG_ON(irq == 0);

Quoted 102 lines · Sheng Yang

The feature wouldn't be enabled until later patch set msix_flags_enabled. It would be enabled along with mask support in kernel.

Signed-off-by Sheng Yang <[email protected]>

---

 arch/x86/include/asm/kvm_host.h |    2 ++
 include/linux/kvm.h             |    6 +++++-
 include/linux/kvm_host.h        |    1 +
 virt/kvm/assigned-dev.c         |   39

+++++++++++++++++++++++++++++++++++++++ 4 files changed, 47 insertions(+), 1 deletions(-)

(quoted diff) +0 -0

b/arch/x86/include/asm/kvm_host.h index e209078..2bb69ba 100644

arch/x86/include/asm/kvm_host.h +2 -0

@@ -456,6 +456,8 @@ struct kvm_arch {
 
 	/* fields used by HYPER-V emulation */
 	u64 hv_guest_os_id;
 	u64 hv_hypercall;
 
+
+	bool msix_flags_enabled;
 

};

struct kvm_vm_stat {

include/linux/kvm.h +5 -1

index 919ae53..a699ec9 100644
@@ -787,11 +787,15 @@ struct kvm_assigned_msix_nr {
 
 };
 
 #define KVM_MAX_MSIX_PER_DEV		256
 
+
+#define KVM_MSIX_FLAG_MASK	1
+
 
 struct kvm_assigned_msix_entry {
 
 	__u32 assigned_dev_id;
 	__u32 gsi;
 	__u16 entry; /* The index of entry in the MSI-X table */
 
-	__u16 padding[3];
+	__u16 flags;
+	__u16 padding[2];
 
 };
 
 #endif /* __LINUX_KVM_H */

include/linux/kvm_host.h +1 -0

index 30f83cd..81a6284 100644
@@ -438,6 +438,7 @@ struct kvm_irq_ack_notifier {
 
 };
 
 #define KVM_ASSIGNED_MSIX_PENDING		0x1
 
+#define KVM_ASSIGNED_MSIX_MASK			0x2
 

struct kvm_guest_msix_entry {

include/linux/kvm_host.h +0 -0

 	u32 vector;
 	u16 entry;

virt/kvm/assigned-dev.c +16 -0

index 7c98928..bf96ea7 100644
@@ -666,11 +666,35 @@ msix_nr_out:
 	return r;
 
 }
 
+static void update_msix_mask(struct kvm_assigned_dev_kernel
 *assigned_dev, +			     int index)
+{
+	int irq;
+	struct irq_desc *desc;
+
+	if (!assigned_dev->dev->msix_enabled ||
+	    !(assigned_dev->irq_requested_type & KVM_DEV_IRQ_HOST_MSIX))
+		return;
+
+	irq = assigned_dev->host_msix_entries[index].vector;
+	BUG_ON(irq == 0);
+	desc = irq_to_desc(irq);
+	BUG_ON(!desc->msi_desc);
+
+	if (assigned_dev->guest_msix_entries[index].flags &
+			KVM_ASSIGNED_MSIX_MASK) {

virt/kvm/assigned-dev.c msix_nr_out: · line 690

Quoted 5 lines

virt/kvm/assigned-dev.c +5 -0

+		desc->chip->mask(irq);
+		flush_work(&assigned_dev->interrupt_work);
+	} else
+		desc->chip->unmask(irq);
+}

Bypassing irq handling code like this is wrong (see all the state keeping in kernel/irq/handle.c).

You need a guarantee that MSIX per-vector mask is used for disable_irq/enable_irq, right? I can't see how this provides it.

Quoted 3 lines

You need a guarantee that MSIX per-vector mask is used for disable_irq/enable_irq, right? I can't see how this provides it.

This one meant to directly operate the mask/unmask bit of the MSI-X table, to emulate the mask/unmask behavior that guest want. In the previous patch I used enable_irq()/disable_irq(), but they won't directly operate the MSI-X table unless it's necessary, and Michael want to read the table in userspace then he prefer using mask/unmask directly.

Quoted 9 lines · Sheng Yang

Quoted 2 lines

You need a guarantee that MSIX per-vector mask is used for disable_irq/enable_irq, right? I can't see how this provides it.

This one meant to directly operate the mask/unmask bit of the MSI-X table, to emulate the mask/unmask behavior that guest want. In the previous patch I used enable_irq()/disable_irq(), but they won't directly operate the MSI-X table unless it's necessary, and Michael want to read the table in userspace then he prefer using mask/unmask directly.

As I said, the main problem was really that the implementation proposed only works for interrupts used by assigned devices. I would like for it to work for irqfd as well.

include/linux/kvm_host.h +1 -0

+	struct kvm_io_device msix_mmio_dev;
  };
 
  struct kvm_irq_mask_notifier {

virt/kvm/assigned-dev.c +0 -0

index bf96ea7..5d2adc4 100644
@@ -739,6 +739,137 @@ msix_entry_out:
 

virt/kvm/assigned-dev.c +13 -0

+	}
+	return false;
+}
+
+static int msix_mmio_read(struct kvm_io_device *this, gpa_t addr, int len,
+			  void *val)
+{
+	struct kvm_assigned_dev_kernel *adev =
+			container_of(this, struct kvm_assigned_dev_kernel,
+				     msix_mmio_dev);
+	int idx, r = 0;
+	u32 entry[4];
+	struct kvm_kernel_irq_routing_entry *e;

virt/kvm/assigned-dev.c +46 -0

+		goto out;
+	}
+
+	e = kvm_get_irq_routing_entry(adev->kvm,
+			adev->guest_msix_entries[idx].vector);
+	if (!e || e->type != KVM_IRQ_ROUTING_MSI) {
+		printk(KERN_WARNING "KVM: Wrong MSI-X routing entry! "
+			"addr 0x%llx, len %d\n", addr, len);
+		r = -EOPNOTSUPP;
+		goto out;
+	}
+	entry[0] = e->msi.address_lo;
+	entry[1] = e->msi.address_hi;
+	entry[2] = e->msi.data;
+	entry[3] = !!(adev->guest_msix_entries[idx].flags&
+			KVM_ASSIGNED_MSIX_MASK);
+	memcpy(val,&entry[addr % PCI_MSIX_ENTRY_SIZE / 4], len);
+
+out:
+	mutex_unlock(&adev->kvm->lock);
+	return r;
+}
+
+static int msix_mmio_write(struct kvm_io_device *this, gpa_t addr, int len,
+			   const void *val)
+{
+	struct kvm_assigned_dev_kernel *adev =
+			container_of(this, struct kvm_assigned_dev_kernel,
+				     msix_mmio_dev);
+	int idx, r = 0;
+	unsigned long new_val = *(unsigned long *)val;
+	bool entry_masked;
+
+	mutex_lock(&adev->kvm->lock);
+	if (!msix_mmio_in_range(adev, addr, len,&idx)) {
+		r = -EOPNOTSUPP;
+		goto out;
+	}
+	if ((addr&  0x3) || len != 4) {
+		printk(KERN_WARNING
+			"KVM: Unaligned writing for device MSI-X MMIO! "
+			"addr 0x%llx, len %d, val 0x%lx\n",
+			addr, len, new_val);
+		r = -EOPNOTSUPP;
+		goto out;
+	}

virt/kvm/assigned-dev.c +6 -0

+		printk(KERN_WARNING
+			"KVM: Bad writing for device MSI-X MMIO! "
+			"addr 0x%llx, len %d, val 0x%lx\n",
+			addr, len, new_val);
+		r = -EOPNOTSUPP;
+		goto out;

virt/kvm/assigned-dev.c +11 -0

+out:
+	mutex_unlock(&adev->kvm->lock);
+
+	return r;
+}
+
+static const struct kvm_io_device_ops msix_mmio_ops = {
+	.read     = msix_mmio_read,
+	.write    = msix_mmio_write,
+};
+
  #endif
 
  long kvm_vm_ioctl_assigned_device(struct kvm *kvm, unsigned ioctl,

virt/kvm/assigned-dev.c msix_entry_out: · line 839

Quoted 7 lines

virt/kvm/assigned-dev.c +7 -0

+		/* Only allow entry modification when entry was masked */
+		if (!entry_masked) {
+			printk(KERN_WARNING
+				"KVM: guest try to write unmasked MSI-X entry. "
+				"addr 0x%llx, len %d, val 0x%lx\n",
+				addr, len, new_val);
+			r = 0;

What does the spec says about this situation?

Quoted 4 lines · Sheng Yang

It would be work with KVM_CAP_DEVICE_MSIX_MASK, which we would enable in the last patch.

include/linux/kvm.h struct kvm_assigned_msix_entry { · line 803

Quoted 3 lines

include/linux/kvm.h +3 -0

+struct kvm_assigned_msix_mmio {
+	__u32 assigned_dev_id;
+	__u64 base_addr;

Different alignment and size on 32 and 64 bits.

Is base_addr a guest physical address? Do we need a size or it it fixed?

include/linux/kvm.h struct kvm_assigned_dev_kernel { · line 470

Quoted 13 lines

include/linux/kvm.h +5 -0

+	__u32 flags;
+	__u32 reserved[2];
+};
+

@@ -465,6 +465,8 @@ struct kvm_assigned_dev_kernel {
 
  	struct pci_dev *dev;
  	struct kvm *kvm;
  	spinlock_t assigned_dev_lock;
 
+	u64 msix_mmio_base;

gpa_t.

Quoted 16 lines

include/linux/kvm_host.h +1 -0

+	struct kvm_io_device msix_mmio_dev;
 

  };
  
  struct kvm_irq_mask_notifier {

virt/kvm/assigned-dev.c +0 -0

index bf96ea7..5d2adc4 100644
@@ -739,6 +739,137 @@ msix_entry_out:
  	return r;
 
  }
 

virt/kvm/assigned-dev.c msix_entry_out: · line 749

Quoted 8 lines

virt/kvm/assigned-dev.c +8 -0

+
+static bool msix_mmio_in_range(struct kvm_assigned_dev_kernel *adev,
+			      gpa_t addr, int len, int *idx)
+{
+	int i;
+
+	if (!(adev->irq_requested_type&  KVM_DEV_IRQ_HOST_MSIX))
+		return false;

Just don't install the io_device in that case.

virt/kvm/assigned-dev.c msix_entry_out: · line 759

Quoted 11 lines

virt/kvm/assigned-dev.c +10 -0

+	BUG_ON(adev->msix_mmio_base == 0);
+	for (i = 0; i<  adev->entries_nr; i++) {
+		u64 start, end;
+		start = adev->msix_mmio_base +
+			adev->guest_msix_entries[i].entry * PCI_MSIX_ENTRY_SIZE;
+		end = start + PCI_MSIX_ENTRY_SIZE;
+		if (addr>= start&&  addr + len<= end) {
+			*idx = i;
+			return true;
+		}

What if it's a partial hit? write part of an entry and part of another entry?

Quoted 14 lines

virt/kvm/assigned-dev.c +12 -0

+	}
+	return false;
+}
+
+static int msix_mmio_read(struct kvm_io_device *this, gpa_t addr, int
 len, +			  void *val)
+{
+	struct kvm_assigned_dev_kernel *adev =
+			container_of(this, struct kvm_assigned_dev_kernel,
+				     msix_mmio_dev);
+	int idx, r = 0;
+	u32 entry[4];
+	struct kvm_kernel_irq_routing_entry *e;

virt/kvm/assigned-dev.c msix_entry_out: · line 782

Quoted 10 lines

virt/kvm/assigned-dev.c +10 -0

+
+	mutex_lock(&adev->kvm->lock);
+	if (!msix_mmio_in_range(adev, addr, len,&idx)) {
+		r = -EOPNOTSUPP;
+		goto out;
+	}
+	if ((addr&  0x3) || len != 4) {
+		printk(KERN_WARNING
+			"KVM: Unaligned reading for device MSI-X MMIO! "
+			"addr 0x%llx, len %d\n", addr, len);

Guest exploitable printk()

virt/kvm/assigned-dev.c msix_entry_out: · line 784

Quoted 1 line

virt/kvm/assigned-dev.c +1 -0

+		r = -EOPNOTSUPP;

If the guest assigned the device to another guest, it allows the nested guest to kill the non-nested guest. Need to exit in a graceful fashion.

Quoted 51 lines

virt/kvm/assigned-dev.c +49 -0

+		goto out;
+	}
+
+	e = kvm_get_irq_routing_entry(adev->kvm,
+			adev->guest_msix_entries[idx].vector);
+	if (!e || e->type != KVM_IRQ_ROUTING_MSI) {
+		printk(KERN_WARNING "KVM: Wrong MSI-X routing entry! "
+			"addr 0x%llx, len %d\n", addr, len);
+		r = -EOPNOTSUPP;
+		goto out;
+	}
+	entry[0] = e->msi.address_lo;
+	entry[1] = e->msi.address_hi;
+	entry[2] = e->msi.data;
+	entry[3] = !!(adev->guest_msix_entries[idx].flags&
+			KVM_ASSIGNED_MSIX_MASK);
+	memcpy(val,&entry[addr % PCI_MSIX_ENTRY_SIZE / 4], len);
+
+out:
+	mutex_unlock(&adev->kvm->lock);
+	return r;
+}
+
+static int msix_mmio_write(struct kvm_io_device *this, gpa_t addr, int
 len, +			   const void *val)
+{
+	struct kvm_assigned_dev_kernel *adev =
+			container_of(this, struct kvm_assigned_dev_kernel,
+				     msix_mmio_dev);
+	int idx, r = 0;
+	unsigned long new_val = *(unsigned long *)val;
+	bool entry_masked;
+
+	mutex_lock(&adev->kvm->lock);
+	if (!msix_mmio_in_range(adev, addr, len,&idx)) {
+		r = -EOPNOTSUPP;
+		goto out;
+	}
+	if ((addr&  0x3) || len != 4) {
+		printk(KERN_WARNING
+			"KVM: Unaligned writing for device MSI-X MMIO! "
+			"addr 0x%llx, len %d, val 0x%lx\n",
+			addr, len, new_val);
+		r = -EOPNOTSUPP;
+		goto out;
+	}
+	entry_masked = adev->guest_msix_entries[idx].flags&
+			KVM_ASSIGNED_MSIX_MASK;
+	if (addr % PCI_MSIX_ENTRY_SIZE != PCI_MSIX_ENTRY_VECTOR_CTRL) {
+		/* Only allow entry modification when entry was masked */

virt/kvm/assigned-dev.c msix_entry_out: · line 839

Quoted 6 lines

virt/kvm/assigned-dev.c +6 -0

+		if (!entry_masked) {
+			printk(KERN_WARNING
+				"KVM: guest try to write unmasked MSI-X entry. "
+				"addr 0x%llx, len %d, val 0x%lx\n",
+				addr, len, new_val);
+			r = 0;

What does the spec says about this situation?

virt/kvm/assigned-dev.c msix_entry_out: · line 841

Quoted 3 lines

virt/kvm/assigned-dev.c +2 -0

+		} else
+			/* Leave it to QEmu */

s/qemu/userspace/

virt/kvm/assigned-dev.c msix_entry_out: · line 843

Quoted 1 line

virt/kvm/assigned-dev.c +1 -0

+			r = -EOPNOTSUPP;

What would userspace do in this situation? I hope you documented precisely what the kernel handles and what it doesn't?

I prefer more kernel code in the kernel to having an interface which is hard to use correctly.

virt/kvm/assigned-dev.c msix_entry_out:

Quoted 3 lines

virt/kvm/assigned-dev.c +3 -0

+		goto out;
+	}
+	if (new_val&  ~1ul) {

Is there a #define for this bit?

Quoted 7 lines

virt/kvm/assigned-dev.c +6 -0

+		printk(KERN_WARNING
+			"KVM: Bad writing for device MSI-X MMIO! "
+			"addr 0x%llx, len %d, val 0x%lx\n",
+			addr, len, new_val);
+		r = -EOPNOTSUPP;
+		goto out;

virt/kvm/assigned-dev.c msix_entry_out: · line 861

Quoted 10 lines

virt/kvm/assigned-dev.c +10 -0

+	}
+	if (new_val == 1&&  !entry_masked) {
+		adev->guest_msix_entries[idx].flags |=
+			KVM_ASSIGNED_MSIX_MASK;
+		update_msix_mask(adev, idx);
+	} else if (new_val == 0&&  entry_masked) {
+		adev->guest_msix_entries[idx].flags&=
+			~KVM_ASSIGNED_MSIX_MASK;
+		update_msix_mask(adev, idx);
+	}

Ah, I see you do reuse update_msix_mask().

Quoted 15 lines

virt/kvm/assigned-dev.c +11 -0

+out:
+	mutex_unlock(&adev->kvm->lock);
+
+	return r;
+}
+
+static const struct kvm_io_device_ops msix_mmio_ops = {
+	.read     = msix_mmio_read,
+	.write    = msix_mmio_write,
+};
+
 
  #endif
 
  long kvm_vm_ioctl_assigned_device(struct kvm *kvm, unsigned ioctl,

Quoted 4 lines

Quoted 1 line

+ r = -EOPNOTSUPP;

If the guest assigned the device to another guest, it allows the nested guest to kill the non-nested guest. Need to exit in a graceful fashion.

Don't understand... It wouldn't result in kill but return to QEmu/userspace.

Quoted 9 lines

Quoted 8 lines

+ if (addr % PCI_MSIX_ENTRY_SIZE != PCI_MSIX_ENTRY_VECTOR_CTRL) {
+ /* Only allow entry modification when entry was masked */
+ if (!entry_masked) {
+ printk(KERN_WARNING
+ "KVM: guest try to write unmasked MSI-X entry. "
+ "addr 0x%llx, len %d, val 0x%lx\n",
+ addr, len, new_val);
+ r = 0;

Quoted 1 line

What does the spec says about this situation?

As Michael pointed out. The spec said the result is "undefined" indeed.

Quoted 6 lines

Quoted 4 lines

+ goto out;
+ }
+ if (new_val& ~1ul) {

Is there a #define for this bit?

Sorry I didn't find it. mask_msi_irq() also use the number 1... Maybe we can add one.

Quoted 6 lines · Sheng Yang

Quoted 4 lines

Quoted 1 line

+ r = -EOPNOTSUPP;

If the guest assigned the device to another guest, it allows the nested guest to kill the non-nested guest. Need to exit in a graceful fashion.

Don't understand... It wouldn't result in kill but return to QEmu/userspace.

What would qemu do on EOPNOTSUPP? It has no way of knowing that this was triggered by an unsupported msix access. What can it do?

Best to just ignore the write.

If you're worried about debugging, we can have a trace_kvm_discard() tracepoint that logs the address and a type enum field that explains why an access was discarded.

Quoted 9 lines

Quoted 9 lines

Quoted 8 lines

+ if (addr % PCI_MSIX_ENTRY_SIZE != PCI_MSIX_ENTRY_VECTOR_CTRL) {
+ /* Only allow entry modification when entry was masked */
+ if (!entry_masked) {
+ printk(KERN_WARNING
+ "KVM: guest try to write unmasked MSI-X entry. "
+ "addr 0x%llx, len %d, val 0x%lx\n",
+ addr, len, new_val);
+ r = 0;

Quoted 3 lines

Quoted 1 line

What does the spec says about this situation?

As Michael pointed out. The spec said the result is "undefined" indeed.

Ok. Then we should silently discard the write instead of allowing the guest to flood host dmesg.

Quoted 9 lines

Quoted 6 lines

Quoted 4 lines

+ goto out;
+ }
+ if (new_val& ~1ul) {

Is there a #define for this bit?

Sorry I didn't find it. mask_msi_irq() also use the number 1... Maybe we can add one.

Yes please.

Signature · 1 line

error compiling committee.c: too many arguments to function

Quoted 11 lines · Avi Kivity

Quoted 6 lines · Sheng Yang

Quoted 4 lines

Quoted 1 line

  +		r = -EOPNOTSUPP;

If the guest assigned the device to another guest, it allows the nested guest to kill the non-nested guest. Need to exit in a graceful fashion.

Don't understand... It wouldn't result in kill but return to QEmu/userspace.

What would qemu do on EOPNOTSUPP? It has no way of knowing that this was triggered by an unsupported msix access. What can it do?

Quoted 5 lines

Best to just ignore the write.

If you're worried about debugging, we can have a trace_kvm_discard() tracepoint that logs the address and a type enum field that explains why an access was discarded.

The issue is that the same page is used for mask and entry programming.

Quoted 14 lines · Michael S. Tsirkin

Quoted 13 lines · Avi Kivity

Quoted 6 lines · Sheng Yang

Quoted 4 lines

Quoted 1 line

  +		r = -EOPNOTSUPP;

If the guest assigned the device to another guest, it allows the nested guest to kill the non-nested guest. Need to exit in a graceful fashion.

Don't understand... It wouldn't result in kill but return to QEmu/userspace.

What would qemu do on EOPNOTSUPP? It has no way of knowing that this was triggered by an unsupported msix access. What can it do?

Best to just ignore the write.

Quoted 5 lines

Quoted 3 lines

If you're worried about debugging, we can have a trace_kvm_discard() tracepoint that logs the address and a type enum field that explains why an access was discarded.

The issue is that the same page is used for mask and entry programming.

Yeah. For that use the normal mmio exit_reason. I was referring to misaligned writes.

I'm not happy with partial emulation, but I'm not happy either with yet another interface to communicate the decoded MSI BAR writes to userspace. Shall we just reprogram the irq routing table?

Signature · 2 lines

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

It would be work with KVM_CAP_DEVICE_MSIX_MASK, which we would enable in the last patch.

Signed-off-by Sheng Yang <[email protected]>

---
 include/linux/kvm.h      |    7 +++
 include/linux/kvm_host.h |    2 +
 virt/kvm/assigned-dev.c  |  131 ++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 140 insertions(+), 0 deletions(-)

include/linux/kvm.h +0 -0

index a699ec9..0a7bd34 100644

include/linux/kvm.h +5 -0

+	__u64 base_addr;
+	__u32 flags;
+	__u32 reserved[2];
+};
+
 #endif /* __LINUX_KVM_H */

include/linux/kvm_host.h +2 -0

index 81a6284..b67082f 100644
@@ -465,6 +465,8 @@ struct kvm_assigned_dev_kernel {
 	struct pci_dev *dev;
 	struct kvm *kvm;
 	spinlock_t assigned_dev_lock;
+	u64 msix_mmio_base;
+	struct kvm_io_device msix_mmio_dev;
 };
 
 struct kvm_irq_mask_notifier {

virt/kvm/assigned-dev.c +9 -0

index bf96ea7..5d2adc4 100644
@@ -739,6 +739,137 @@ msix_entry_out:
 
 	return r;
 }
+
+static bool msix_mmio_in_range(struct kvm_assigned_dev_kernel *adev,
+			      gpa_t addr, int len, int *idx)
+{
+	int i;
+
+	if (!(adev->irq_requested_type & KVM_DEV_IRQ_HOST_MSIX))
+		return false;
+	BUG_ON(adev->msix_mmio_base == 0);

virt/kvm/assigned-dev.c +72 -0

+	return false;
+}
+
+static int msix_mmio_read(struct kvm_io_device *this, gpa_t addr, int len,
+			  void *val)
+{
+	struct kvm_assigned_dev_kernel *adev =
+			container_of(this, struct kvm_assigned_dev_kernel,
+				     msix_mmio_dev);
+	int idx, r = 0;
+	u32 entry[4];
+	struct kvm_kernel_irq_routing_entry *e;
+
+	mutex_lock(&adev->kvm->lock);
+	if (!msix_mmio_in_range(adev, addr, len, &idx)) {
+		r = -EOPNOTSUPP;
+		goto out;
+	}
+	if ((addr & 0x3) || len != 4) {
+		printk(KERN_WARNING
+			"KVM: Unaligned reading for device MSI-X MMIO! "
+			"addr 0x%llx, len %d\n", addr, len);
+		r = -EOPNOTSUPP;
+		goto out;
+	}
+
+	e = kvm_get_irq_routing_entry(adev->kvm,
+			adev->guest_msix_entries[idx].vector);
+	if (!e || e->type != KVM_IRQ_ROUTING_MSI) {
+		printk(KERN_WARNING "KVM: Wrong MSI-X routing entry! "
+			"addr 0x%llx, len %d\n", addr, len);
+		r = -EOPNOTSUPP;
+		goto out;
+	}
+	entry[0] = e->msi.address_lo;
+	entry[1] = e->msi.address_hi;
+	entry[2] = e->msi.data;
+	entry[3] = !!(adev->guest_msix_entries[idx].flags &
+			KVM_ASSIGNED_MSIX_MASK);
+	memcpy(val, &entry[addr % PCI_MSIX_ENTRY_SIZE / 4], len);
+
+out:
+	mutex_unlock(&adev->kvm->lock);
+	return r;
+}
+
+static int msix_mmio_write(struct kvm_io_device *this, gpa_t addr, int len,
+			   const void *val)
+{
+	struct kvm_assigned_dev_kernel *adev =
+			container_of(this, struct kvm_assigned_dev_kernel,
+				     msix_mmio_dev);
+	int idx, r = 0;
+	unsigned long new_val = *(unsigned long *)val;
+	bool entry_masked;
+
+	mutex_lock(&adev->kvm->lock);
+	if (!msix_mmio_in_range(adev, addr, len, &idx)) {
+		r = -EOPNOTSUPP;
+		goto out;
+	}
+	if ((addr & 0x3) || len != 4) {
+		printk(KERN_WARNING
+			"KVM: Unaligned writing for device MSI-X MMIO! "
+			"addr 0x%llx, len %d, val 0x%lx\n",
+			addr, len, new_val);
+		r = -EOPNOTSUPP;
+		goto out;
+	}
+	entry_masked = adev->guest_msix_entries[idx].flags &
+			KVM_ASSIGNED_MSIX_MASK;
+	if (addr % PCI_MSIX_ENTRY_SIZE != PCI_MSIX_ENTRY_VECTOR_CTRL) {

virt/kvm/assigned-dev.c +30 -0

+		goto out;
+	}
+	if (new_val & ~1ul) {
+		printk(KERN_WARNING
+			"KVM: Bad writing for device MSI-X MMIO! "
+			"addr 0x%llx, len %d, val 0x%lx\n",
+			addr, len, new_val);
+		r = -EOPNOTSUPP;
+		goto out;
+	}
+	if (new_val == 1 && !entry_masked) {
+		adev->guest_msix_entries[idx].flags |=
+			KVM_ASSIGNED_MSIX_MASK;
+		update_msix_mask(adev, idx);
+	} else if (new_val == 0 && entry_masked) {
+		adev->guest_msix_entries[idx].flags &=
+			~KVM_ASSIGNED_MSIX_MASK;
+		update_msix_mask(adev, idx);
+	}
+out:
+	mutex_unlock(&adev->kvm->lock);
+
+	return r;
+}
+
+static const struct kvm_io_device_ops msix_mmio_ops = {
+	.read     = msix_mmio_read,
+	.write    = msix_mmio_write,
+};
+
 #endif
 
 long kvm_vm_ioctl_assigned_device(struct kvm *kvm, unsigned ioctl,

Signature · 1 line

1.7.0.1

Quoted 4 lines · Sheng Yang

It would be work with KVM_CAP_DEVICE_MSIX_MASK, which we would enable in the last patch.

Signed-off-by Sheng Yang <[email protected]>

Merge this with patch 8 - it does not make sense to add a bunch of users of the field msix_mmio_base but init it in the next patch.

Quoted 7 lines

---

 include/linux/kvm.h      |    7 +++
 include/linux/kvm_host.h |    2 +
 virt/kvm/assigned-dev.c  |  131

++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed,

Quoted 7 lines

insertions(+), 0 deletions(-)

include/linux/kvm.h +0 -0

index a699ec9..0a7bd34 100644

include/linux/kvm.h struct kvm_assigned_msix_entry { · line 802

Quoted 7 lines

include/linux/kvm.h +2 -0

@@ -798,4 +798,11 @@ struct kvm_assigned_msix_entry {
 
 	__u16 padding[2];
 
 };
 
+struct kvm_assigned_msix_mmio {
+	__u32 assigned_dev_id;

I think avi commented - there's padding here.

Quoted 44 lines

include/linux/kvm.h +5 -0

+	__u64 base_addr;
+	__u32 flags;
+	__u32 reserved[2];
+};
+
 
 #endif /* __LINUX_KVM_H */

include/linux/kvm_host.h +2 -0

index 81a6284..b67082f 100644
@@ -465,6 +465,8 @@ struct kvm_assigned_dev_kernel {
 
 	struct pci_dev *dev;
 	struct kvm *kvm;
 	spinlock_t assigned_dev_lock;
 
+	u64 msix_mmio_base;
+	struct kvm_io_device msix_mmio_dev;
 

};

struct kvm_irq_mask_notifier {

virt/kvm/assigned-dev.c +9 -0

index bf96ea7..5d2adc4 100644
@@ -739,6 +739,137 @@ msix_entry_out:
 	return r;
 
 }
 
+
+static bool msix_mmio_in_range(struct kvm_assigned_dev_kernel *adev,
+			      gpa_t addr, int len, int *idx)
+{
+	int i;
+
+	if (!(adev->irq_requested_type & KVM_DEV_IRQ_HOST_MSIX))
+		return false;
+	BUG_ON(adev->msix_mmio_base == 0);

virt/kvm/assigned-dev.c msix_entry_out: · line 761

Quoted 10 lines

virt/kvm/assigned-dev.c +10 -0

+	for (i = 0; i < adev->entries_nr; i++) {
+		u64 start, end;
+		start = adev->msix_mmio_base +
+			adev->guest_msix_entries[i].entry * PCI_MSIX_ENTRY_SIZE;
+		end = start + PCI_MSIX_ENTRY_SIZE;
+		if (addr >= start && addr + len <= end) {
+			*idx = i;
+			return true;
+		}
+	}

We really should not need guest_msix_entries at all:
if we are emulating MSIX in kernel anyway, let us just emulate it there. Doing half setup from qemu and half from kvm will just create problems.

If you do it all in kernel, you will simply need a single range check to see whether this is mask write.

Quoted 73 lines

virt/kvm/assigned-dev.c +70 -0

+	return false;
+}
+
+static int msix_mmio_read(struct kvm_io_device *this, gpa_t addr, int
 len, +			  void *val)
+{
+	struct kvm_assigned_dev_kernel *adev =
+			container_of(this, struct kvm_assigned_dev_kernel,
+				     msix_mmio_dev);
+	int idx, r = 0;
+	u32 entry[4];
+	struct kvm_kernel_irq_routing_entry *e;
+
+	mutex_lock(&adev->kvm->lock);
+	if (!msix_mmio_in_range(adev, addr, len, &idx)) {
+		r = -EOPNOTSUPP;
+		goto out;
+	}
+	if ((addr & 0x3) || len != 4) {
+		printk(KERN_WARNING
+			"KVM: Unaligned reading for device MSI-X MMIO! "
+			"addr 0x%llx, len %d\n", addr, len);
+		r = -EOPNOTSUPP;
+		goto out;
+	}
+
+	e = kvm_get_irq_routing_entry(adev->kvm,
+			adev->guest_msix_entries[idx].vector);
+	if (!e || e->type != KVM_IRQ_ROUTING_MSI) {
+		printk(KERN_WARNING "KVM: Wrong MSI-X routing entry! "
+			"addr 0x%llx, len %d\n", addr, len);
+		r = -EOPNOTSUPP;
+		goto out;
+	}
+	entry[0] = e->msi.address_lo;
+	entry[1] = e->msi.address_hi;
+	entry[2] = e->msi.data;
+	entry[3] = !!(adev->guest_msix_entries[idx].flags &
+			KVM_ASSIGNED_MSIX_MASK);
+	memcpy(val, &entry[addr % PCI_MSIX_ENTRY_SIZE / 4], len);
+
+out:
+	mutex_unlock(&adev->kvm->lock);
+	return r;
+}
+
+static int msix_mmio_write(struct kvm_io_device *this, gpa_t addr, int
 len, +			   const void *val)
+{
+	struct kvm_assigned_dev_kernel *adev =
+			container_of(this, struct kvm_assigned_dev_kernel,
+				     msix_mmio_dev);
+	int idx, r = 0;
+	unsigned long new_val = *(unsigned long *)val;
+	bool entry_masked;
+
+	mutex_lock(&adev->kvm->lock);
+	if (!msix_mmio_in_range(adev, addr, len, &idx)) {
+		r = -EOPNOTSUPP;
+		goto out;
+	}
+	if ((addr & 0x3) || len != 4) {
+		printk(KERN_WARNING
+			"KVM: Unaligned writing for device MSI-X MMIO! "
+			"addr 0x%llx, len %d, val 0x%lx\n",
+			addr, len, new_val);
+		r = -EOPNOTSUPP;
+		goto out;
+	}
+	entry_masked = adev->guest_msix_entries[idx].flags &
+			KVM_ASSIGNED_MSIX_MASK;
+	if (addr % PCI_MSIX_ENTRY_SIZE != PCI_MSIX_ENTRY_VECTOR_CTRL) {

virt/kvm/assigned-dev.c msix_entry_out: · line 842

Quoted 10 lines

virt/kvm/assigned-dev.c +10 -0

+		/* Only allow entry modification when entry was masked */
+		if (!entry_masked) {
+			printk(KERN_WARNING
+				"KVM: guest try to write unmasked MSI-X entry. "
+				"addr 0x%llx, len %d, val 0x%lx\n",
+				addr, len, new_val);
+			r = 0;
+		} else
+			/* Leave it to QEmu */
+			r = -EOPNOTSUPP;

So half the emulation is here half is there... Let's just put it all in kernel and be done with it?

Quoted 34 lines

virt/kvm/assigned-dev.c +30 -0

+		goto out;
+	}
+	if (new_val & ~1ul) {
+		printk(KERN_WARNING
+			"KVM: Bad writing for device MSI-X MMIO! "
+			"addr 0x%llx, len %d, val 0x%lx\n",
+			addr, len, new_val);
+		r = -EOPNOTSUPP;
+		goto out;
+	}
+	if (new_val == 1 && !entry_masked) {
+		adev->guest_msix_entries[idx].flags |=
+			KVM_ASSIGNED_MSIX_MASK;
+		update_msix_mask(adev, idx);
+	} else if (new_val == 0 && entry_masked) {
+		adev->guest_msix_entries[idx].flags &=
+			~KVM_ASSIGNED_MSIX_MASK;
+		update_msix_mask(adev, idx);
+	}
+out:
+	mutex_unlock(&adev->kvm->lock);
+
+	return r;
+}
+
+static const struct kvm_io_device_ops msix_mmio_ops = {
+	.read     = msix_mmio_read,
+	.write    = msix_mmio_write,
+};
+
 
 #endif
 
 long kvm_vm_ioctl_assigned_device(struct kvm *kvm, unsigned ioctl,

This patch enable per-vector mask for assigned devices using MSI-X.

Documentation/kvm/api.txt +11 -0

@@ -1087,6 +1087,28 @@ of 4 instructions that make up a hypercall.
  If any additional field gets added to this structure later on, a bit for that
  additional piece of information will be set in the flags bitmap.
 
+4.47 KVM_ASSIGN_REG_MSIX_MMIO
+
+Capability: KVM_CAP_DEVICE_MSIX_MASK
+Architectures: x86
+Type: vm ioctl
+Parameters: struct kvm_assigned_msix_mmio (in)
+Returns: 0 on success, !0 on error
+
+struct kvm_assigned_msix_mmio {
+	/* Assigned device's ID */
+	__u32 assigned_dev_id;

This patch enable per-vector mask for assigned devices using MSI-X.

Signed-off-by Sheng Yang <[email protected]>

---
 Documentation/kvm/api.txt |   22 ++++++++++++++++
 arch/x86/kvm/x86.c        |    6 ++++
 include/linux/kvm.h       |    8 +++++-
 virt/kvm/assigned-dev.c   |   60 +++++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 95 insertions(+), 1 deletions(-)

Documentation/kvm/api.txt +22 -0

index d82d637..f324a50 100644
@@ -1087,6 +1087,28 @@ of 4 instructions that make up a hypercall.
 If any additional field gets added to this structure later on, a bit for that
 additional piece of information will be set in the flags bitmap.
 
+4.47 KVM_ASSIGN_REG_MSIX_MMIO
+
+Capability: KVM_CAP_DEVICE_MSIX_MASK
+Architectures: x86
+Type: vm ioctl
+Parameters: struct kvm_assigned_msix_mmio (in)
+Returns: 0 on success, !0 on error
+
+struct kvm_assigned_msix_mmio {
+	/* Assigned device's ID */
+	__u32 assigned_dev_id;
+	/* MSI-X table MMIO address */
+	__u64 base_addr;
+	/* Must be 0 */
+	__u32 flags;
+	/* Must be 0, reserved for future use */
+	__u64 reserved;
+};
+
+This ioctl would enable in-kernel MSI-X emulation, which would handle MSI-X
+mask bit in the kernel.
+
 5. The kvm_run structure
 
 Application code obtains a pointer to the kvm_run structure by

arch/x86/kvm/x86.c +6 -0

index fc62546..ba07a2f 100644
@@ -1927,6 +1927,8 @@ int kvm_dev_ioctl_check_extension(long ext)
 	case KVM_CAP_X86_ROBUST_SINGLESTEP:
 	case KVM_CAP_XSAVE:
 	case KVM_CAP_ENABLE_CAP:
+	case KVM_CAP_DEVICE_MSIX_EXT:
+	case KVM_CAP_DEVICE_MSIX_MASK:
 		r = 1;
 		break;
 	case KVM_CAP_COALESCED_MMIO:

@@ -2717,6 +2719,10 @@ static int kvm_vcpu_ioctl_enable_cap(struct kvm_vcpu *vcpu,
 		return -EINVAL;
 
 	switch (cap->cap) {
+	case KVM_CAP_DEVICE_MSIX_EXT:
+		vcpu->kvm->arch.msix_flags_enabled = true;
+		r = 0;
+		break;
 	default:
 		r = -EINVAL;
 		break;

include/linux/kvm.h +7 -1

index 0a7bd34..1494ed0 100644
@@ -540,6 +540,10 @@ struct kvm_ppc_pvinfo {
 #endif
 #define KVM_CAP_PPC_GET_PVINFO 57
 #define KVM_CAP_PPC_IRQ_LEVEL 58
+#ifdef __KVM_HAVE_MSIX
+#define KVM_CAP_DEVICE_MSIX_EXT 59
+#define KVM_CAP_DEVICE_MSIX_MASK 60
+#endif
 
 #ifdef KVM_CAP_IRQ_ROUTING
 

@@ -671,6 +675,8 @@ struct kvm_clock_data {
 #define KVM_XEN_HVM_CONFIG        _IOW(KVMIO,  0x7a, struct kvm_xen_hvm_config)
 #define KVM_SET_CLOCK             _IOW(KVMIO,  0x7b, struct kvm_clock_data)
 #define KVM_GET_CLOCK             _IOR(KVMIO,  0x7c, struct kvm_clock_data)
+#define KVM_ASSIGN_REG_MSIX_MMIO  _IOW(KVMIO,  0x7d, \
+					struct kvm_assigned_msix_mmio)
 /* Available with KVM_CAP_PIT_STATE2 */
 #define KVM_GET_PIT2              _IOR(KVMIO,  0x9f, struct kvm_pit_state2)
 #define KVM_SET_PIT2              _IOW(KVMIO,  0xa0, struct kvm_pit_state2)

@@ -802,7 +808,7 @@ struct kvm_assigned_msix_mmio {
 	__u32 assigned_dev_id;
 	__u64 base_addr;
 	__u32 flags;
-	__u32 reserved[2];
+	__u64 reserved;
 };
 
 #endif /* __LINUX_KVM_H */

virt/kvm/assigned-dev.c +2 -0

index 5d2adc4..9573194 100644
@@ -17,6 +17,8 @@
 #include <linux/pci.h>
 #include <linux/interrupt.h>
 #include <linux/slab.h>
+#include <linux/irqnr.h>
+
 #include "irq.h"
 
 static struct kvm_assigned_dev_kernel *kvm_find_assigned_dev(struct list_head *head,

virt/kvm/assigned-dev.c +56 -0

+			mutex_lock(&kvm->slots_lock);
+			kvm_io_bus_unregister_dev(kvm, KVM_MMIO_BUS,
+					&assigned_dev->msix_mmio_dev);
+			mutex_unlock(&kvm->slots_lock);
+		}
+#endif
 		for (i = 0; i < assigned_dev->entries_nr; i++)
 			disable_irq_nosync(assigned_dev->
 					   host_msix_entries[i].vector);

@@ -318,6 +328,15 @@ static int assigned_device_enable_host_msix(struct kvm *kvm,
 			goto err;
 	}
 
+	if (dev->msix_mmio_base) {
+		mutex_lock(&kvm->slots_lock);
+		r = kvm_io_bus_register_dev(kvm, KVM_MMIO_BUS,
+				&dev->msix_mmio_dev);
+		mutex_unlock(&kvm->slots_lock);
+		if (r)
+			goto err;
+	}
+
 	return 0;
 err:
 	for (i -= 1; i >= 0; i--)

@@ -870,6 +889,31 @@ static const struct kvm_io_device_ops msix_mmio_ops = {
 	.write    = msix_mmio_write,
 };
 
+static int kvm_vm_ioctl_register_msix_mmio(struct kvm *kvm,
+				struct kvm_assigned_msix_mmio *msix_mmio)
+{
+	int r = 0;
+	struct kvm_assigned_dev_kernel *adev;
+
+	mutex_lock(&kvm->lock);
+	adev = kvm_find_assigned_dev(&kvm->arch.assigned_dev_head,
+				      msix_mmio->assigned_dev_id);
+	if (!adev) {
+		r = -EINVAL;
+		goto out;
+	}
+	if (msix_mmio->base_addr == 0) {
+		r = -EINVAL;
+		goto out;
+	}
+	adev->msix_mmio_base = msix_mmio->base_addr;
+
+	kvm_iodevice_init(&adev->msix_mmio_dev, &msix_mmio_ops);
+out:
+	mutex_unlock(&kvm->lock);
+
+	return r;
+}
 #endif
 
 long kvm_vm_ioctl_assigned_device(struct kvm *kvm, unsigned ioctl,

@@ -982,6 +1026,22 @@ long kvm_vm_ioctl_assigned_device(struct kvm *kvm, unsigned ioctl,
 			goto out;
 		break;
 	}
+	case KVM_ASSIGN_REG_MSIX_MMIO: {
+		struct kvm_assigned_msix_mmio msix_mmio;
+
+		r = -EFAULT;
+		if (copy_from_user(&msix_mmio, argp, sizeof(msix_mmio)))
+			goto out;
+
+		r = -EINVAL;
+		if (msix_mmio.flags != 0 || msix_mmio.reserved != 0)
+			goto out;
+
+		r = kvm_vm_ioctl_register_msix_mmio(kvm, &msix_mmio);
+		if (r)
+			goto out;
+		break;
+	}
 #endif
 	}
 out:

Signature · 1 line

1.7.0.1

This patch enable per-vector mask for assigned devices using MSI-X.

Signed-off-by Sheng Yang <[email protected]>

---
 Documentation/kvm/api.txt |   22 ++++++++++++++++
 arch/x86/kvm/x86.c        |    6 ++++
 include/linux/kvm.h       |    8 +++++-
 virt/kvm/assigned-dev.c   |   60

+++++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 95 insertions(+), 1 deletions(-)

Documentation/kvm/api.txt +0 -0

index d82d637..f324a50 100644
@@ -1087,6 +1087,28 @@ of 4 instructions that make up a hypercall.
 If any additional field gets added to this structure later on, a bit for

that additional piece of information will be set in the flags bitmap.

+4.47 KVM_ASSIGN_REG_MSIX_MMIO

Documentation/kvm/api.txt +19 -0

+
+Capability: KVM_CAP_DEVICE_MSIX_MASK
+Architectures: x86
+Type: vm ioctl
+Parameters: struct kvm_assigned_msix_mmio (in)
+Returns: 0 on success, !0 on error
+
+struct kvm_assigned_msix_mmio {
+	/* Assigned device's ID */
+	__u32 assigned_dev_id;
+	/* MSI-X table MMIO address */
+	__u64 base_addr;
+	/* Must be 0 */
+	__u32 flags;
+	/* Must be 0, reserved for future use */
+	__u64 reserved;
+};
+
+This ioctl would enable in-kernel MSI-X emulation, which would handle

MSI-X +mask bit in the kernel.

Documentation/kvm/api.txt +1 -0

+
 5. The kvm_run structure
 
 Application code obtains a pointer to the kvm_run structure by

arch/x86/kvm/x86.c +6 -0

index fc62546..ba07a2f 100644
@@ -1927,6 +1927,8 @@ int kvm_dev_ioctl_check_extension(long ext)
 	case KVM_CAP_X86_ROBUST_SINGLESTEP:
 	case KVM_CAP_XSAVE:
 	case KVM_CAP_ENABLE_CAP:
+	case KVM_CAP_DEVICE_MSIX_EXT:
+	case KVM_CAP_DEVICE_MSIX_MASK:
 		r = 1;
 		break;
 	case KVM_CAP_COALESCED_MMIO:

@@ -2717,6 +2719,10 @@ static int kvm_vcpu_ioctl_enable_cap(struct kvm_vcpu
 *vcpu, return -EINVAL;
 
 	switch (cap->cap) {
+	case KVM_CAP_DEVICE_MSIX_EXT:
+		vcpu->kvm->arch.msix_flags_enabled = true;
+		r = 0;
+		break;
 	default:
 		r = -EINVAL;
 		break;

include/linux/kvm.h +4 -0

index 0a7bd34..1494ed0 100644
@@ -540,6 +540,10 @@ struct kvm_ppc_pvinfo {
 #endif
 #define KVM_CAP_PPC_GET_PVINFO 57
 #define KVM_CAP_PPC_IRQ_LEVEL 58
+#ifdef __KVM_HAVE_MSIX
+#define KVM_CAP_DEVICE_MSIX_EXT 59
+#define KVM_CAP_DEVICE_MSIX_MASK 60
+#endif
 
 #ifdef KVM_CAP_IRQ_ROUTING
 

@@ -671,6 +675,8 @@ struct kvm_clock_data {
 #define KVM_XEN_HVM_CONFIG        _IOW(KVMIO,  0x7a, struct
 kvm_xen_hvm_config) #define KVM_SET_CLOCK             _IOW(KVMIO,  0x7b,
 struct kvm_clock_data) #define KVM_GET_CLOCK             _IOR(KVMIO, 

0x7c, struct kvm_clock_data) +#define KVM_ASSIGN_REG_MSIX_MMIO _IOW(KVMIO, 0x7d, \

include/linux/kvm.h +1 -0

+					struct kvm_assigned_msix_mmio)
 /* Available with KVM_CAP_PIT_STATE2 */
 #define KVM_GET_PIT2              _IOR(KVMIO,  0x9f, struct

kvm_pit_state2) #define KVM_SET_PIT2 _IOW(KVMIO, 0xa0,
struct kvm_pit_state2) @@ -802,7 +808,7 @@ struct kvm_assigned_msix_mmio {

include/linux/kvm.h +1 -1

 	__u32 assigned_dev_id;
 	__u64 base_addr;
 	__u32 flags;
-	__u32 reserved[2];
+	__u64 reserved;
 };
 
 #endif /* __LINUX_KVM_H */

virt/kvm/assigned-dev.c +2 -0

index 5d2adc4..9573194 100644
@@ -17,6 +17,8 @@
 #include <linux/pci.h>
 #include <linux/interrupt.h>
 #include <linux/slab.h>
+#include <linux/irqnr.h>
+
 #include "irq.h"
 
 static struct kvm_assigned_dev_kernel *kvm_find_assigned_dev(struct

list_head *head, @@ -169,6 +171,14 @@ static void deassign_host_irq(struct kvm *kvm, */

virt/kvm/assigned-dev.c +58 -0

 	if (assigned_dev->irq_requested_type & KVM_DEV_IRQ_HOST_MSIX) {
 		int i;
+#ifdef __KVM_HAVE_MSIX
+		if (assigned_dev->msix_mmio_base) {
+			mutex_lock(&kvm->slots_lock);
+			kvm_io_bus_unregister_dev(kvm, KVM_MMIO_BUS,
+					&assigned_dev->msix_mmio_dev);
+			mutex_unlock(&kvm->slots_lock);
+		}
+#endif
 		for (i = 0; i < assigned_dev->entries_nr; i++)
 			disable_irq_nosync(assigned_dev->
 					   host_msix_entries[i].vector);

@@ -318,6 +328,15 @@ static int assigned_device_enable_host_msix(struct kvm
 *kvm, goto err;
 	}
 
+	if (dev->msix_mmio_base) {
+		mutex_lock(&kvm->slots_lock);
+		r = kvm_io_bus_register_dev(kvm, KVM_MMIO_BUS,
+				&dev->msix_mmio_dev);
+		mutex_unlock(&kvm->slots_lock);
+		if (r)
+			goto err;
+	}
+
 	return 0;
 err:
 	for (i -= 1; i >= 0; i--)

@@ -870,6 +889,31 @@ static const struct kvm_io_device_ops msix_mmio_ops =
 { .write    = msix_mmio_write,
 };
 
+static int kvm_vm_ioctl_register_msix_mmio(struct kvm *kvm,
+				struct kvm_assigned_msix_mmio *msix_mmio)
+{
+	int r = 0;
+	struct kvm_assigned_dev_kernel *adev;
+
+	mutex_lock(&kvm->lock);
+	adev = kvm_find_assigned_dev(&kvm->arch.assigned_dev_head,
+				      msix_mmio->assigned_dev_id);
+	if (!adev) {
+		r = -EINVAL;
+		goto out;
+	}
+	if (msix_mmio->base_addr == 0) {
+		r = -EINVAL;
+		goto out;
+	}
+	adev->msix_mmio_base = msix_mmio->base_addr;
+
+	kvm_iodevice_init(&adev->msix_mmio_dev, &msix_mmio_ops);
+out:
+	mutex_unlock(&kvm->lock);
+
+	return r;
+}
 #endif
 
 long kvm_vm_ioctl_assigned_device(struct kvm *kvm, unsigned ioctl,

@@ -982,6 +1026,22 @@ long kvm_vm_ioctl_assigned_device(struct kvm *kvm,
 unsigned ioctl, goto out;
 		break;
 	}
+	case KVM_ASSIGN_REG_MSIX_MMIO: {
+		struct kvm_assigned_msix_mmio msix_mmio;
+
+		r = -EFAULT;
+		if (copy_from_user(&msix_mmio, argp, sizeof(msix_mmio)))
+			goto out;
+
+		r = -EINVAL;
+		if (msix_mmio.flags != 0 || msix_mmio.reserved != 0)
+			goto out;
+
+		r = kvm_vm_ioctl_register_msix_mmio(kvm, &msix_mmio);
+		if (r)
+			goto out;
+		break;
+	}
 #endif
 	}
 out:

Quoted 1 line · On Wednesday 20 October 2010 16:26:32 Sheng Yang wrote:

This patch enable per-vector mask for assigned devices using MSI-X.

The basic idea of kernel and QEmu's responsibilities are:

1. Because QEmu owned the irq routing table, so the change of table should still go to the QEmu, like we did in msix_mmio_write().

2. And the others things can be done in kernel, for performance. Here we covered the reading(converted entry from routing table and mask bit state of enabled MSI-X entries), and writing the mask bit for enabled MSI-X entries. Originally we only has mask bit handled in kernel, but later we found that Linux kernel would read MSI-X mmio just after every writing to mask bit, in order to flush the writing. So we add reading MSI data/addr as well.

3. Disabled entries's mask bit accessing would go to QEmu, because it may result in disable/enable MSI-X. Explained later.

4. Only QEmu has knowledge of PCI configuration space, so it's QEmu to decide enable/disable MSI-X for device.
.

5. There is an distinction between enabled entry and disabled entry of MSI-X table.

The entries we had used for pci_enable_msix()(not necessary in sequence number) are already enabled, the others are disabled. When device's MSI-X is enabled and guest want to enable an disabled entry, we would go back to QEmu because this vector didn't exist in the routing table. Also due to pci_enable_msix() in kernel didn't allow us to enable vectors one by one, but all at once. So we have to disable MSI-X first, then enable it with new entries, which contained the new vector guest want to use. This situation is only happen when device is being initialized. After that, kernel can know and handle the mask bit of the enabled entry.

I've also considered handle all MMIO operation in kernel, and changing irq routing in kernel directly. But as long as irq routing is owned by QEmu, I think it's better to leave to it...

Notice the mask/unmask bits must be handled together, either in kernel or in userspace. Because if kernel has handled enabled vector's mask bit directly, it would be unsync with QEmu's records. It doesn't matter when QEmu don't access the related record. And the only place QEmu want to consult it's enabled entries' mask bit state is writing to MSI addr/data. The writing should be discarded if the entry is unmasked. This checking has already been done by kernel in this patchset, so we are fine here.

If we want to access the enabled entries' mask bit in the future, we can directly access device's MMIO.

That's the reason why I have followed Michael's advice to use mask/unmask directly.
Hope this would make the patches more clear. I meant to add comments for this changeset, but miss it later.

--
regards
Yang, Sheng

Quoted 200 lines

Signed-off-by Sheng Yang <[email protected]>

---
 Documentation/kvm/api.txt |   22 ++++++++++++++++
 arch/x86/kvm/x86.c        |    6 ++++
 include/linux/kvm.h       |    8 +++++-
 virt/kvm/assigned-dev.c   |   60

+++++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 95 insertions(+), 1 deletions(-)

Documentation/kvm/api.txt +0 -0

index d82d637..f324a50 100644
@@ -1087,6 +1087,28 @@ of 4 instructions that make up a hypercall.
 If any additional field gets added to this structure later on, a bit for

that additional piece of information will be set in the flags bitmap.

+4.47 KVM_ASSIGN_REG_MSIX_MMIO

Documentation/kvm/api.txt +19 -0

+
+Capability: KVM_CAP_DEVICE_MSIX_MASK
+Architectures: x86
+Type: vm ioctl
+Parameters: struct kvm_assigned_msix_mmio (in)
+Returns: 0 on success, !0 on error
+
+struct kvm_assigned_msix_mmio {
+	/* Assigned device's ID */
+	__u32 assigned_dev_id;
+	/* MSI-X table MMIO address */
+	__u64 base_addr;
+	/* Must be 0 */
+	__u32 flags;
+	/* Must be 0, reserved for future use */
+	__u64 reserved;
+};
+
+This ioctl would enable in-kernel MSI-X emulation, which would handle

MSI-X +mask bit in the kernel.

Documentation/kvm/api.txt +1 -0

+
 5. The kvm_run structure
 
 Application code obtains a pointer to the kvm_run structure by

arch/x86/kvm/x86.c +6 -0

index fc62546..ba07a2f 100644
@@ -1927,6 +1927,8 @@ int kvm_dev_ioctl_check_extension(long ext)
 	case KVM_CAP_X86_ROBUST_SINGLESTEP:
 	case KVM_CAP_XSAVE:
 	case KVM_CAP_ENABLE_CAP:
+	case KVM_CAP_DEVICE_MSIX_EXT:
+	case KVM_CAP_DEVICE_MSIX_MASK:
 		r = 1;
 		break;
 	case KVM_CAP_COALESCED_MMIO:

@@ -2717,6 +2719,10 @@ static int kvm_vcpu_ioctl_enable_cap(struct kvm_vcpu
 *vcpu, return -EINVAL;
 
 	switch (cap->cap) {
+	case KVM_CAP_DEVICE_MSIX_EXT:
+		vcpu->kvm->arch.msix_flags_enabled = true;
+		r = 0;
+		break;
 	default:
 		r = -EINVAL;
 		break;

include/linux/kvm.h +4 -0

index 0a7bd34..1494ed0 100644
@@ -540,6 +540,10 @@ struct kvm_ppc_pvinfo {
 #endif
 #define KVM_CAP_PPC_GET_PVINFO 57
 #define KVM_CAP_PPC_IRQ_LEVEL 58
+#ifdef __KVM_HAVE_MSIX
+#define KVM_CAP_DEVICE_MSIX_EXT 59
+#define KVM_CAP_DEVICE_MSIX_MASK 60
+#endif
 
 #ifdef KVM_CAP_IRQ_ROUTING
 

@@ -671,6 +675,8 @@ struct kvm_clock_data {
 #define KVM_XEN_HVM_CONFIG        _IOW(KVMIO,  0x7a, struct
 kvm_xen_hvm_config) #define KVM_SET_CLOCK             _IOW(KVMIO,  0x7b,
 struct kvm_clock_data) #define KVM_GET_CLOCK             _IOR(KVMIO, 

0x7c, struct kvm_clock_data) +#define KVM_ASSIGN_REG_MSIX_MMIO _IOW(KVMIO, 0x7d, \

include/linux/kvm.h +1 -0

+					struct kvm_assigned_msix_mmio)
 /* Available with KVM_CAP_PIT_STATE2 */
 #define KVM_GET_PIT2              _IOR(KVMIO,  0x9f, struct

kvm_pit_state2) #define KVM_SET_PIT2 _IOW(KVMIO, 0xa0,
struct kvm_pit_state2) @@ -802,7 +808,7 @@ struct kvm_assigned_msix_mmio {

include/linux/kvm.h +1 -1

 	__u32 assigned_dev_id;
 	__u64 base_addr;
 	__u32 flags;
-	__u32 reserved[2];
+	__u64 reserved;
 };
 
 #endif /* __LINUX_KVM_H */

virt/kvm/assigned-dev.c +2 -0

index 5d2adc4..9573194 100644
@@ -17,6 +17,8 @@
 #include <linux/pci.h>
 #include <linux/interrupt.h>
 #include <linux/slab.h>
+#include <linux/irqnr.h>
+
 #include "irq.h"
 
 static struct kvm_assigned_dev_kernel *kvm_find_assigned_dev(struct

list_head *head, @@ -169,6 +171,14 @@ static void deassign_host_irq(struct kvm *kvm, */

virt/kvm/assigned-dev.c +58 -0

 	if (assigned_dev->irq_requested_type & KVM_DEV_IRQ_HOST_MSIX) {
 		int i;
+#ifdef __KVM_HAVE_MSIX
+		if (assigned_dev->msix_mmio_base) {
+			mutex_lock(&kvm->slots_lock);
+			kvm_io_bus_unregister_dev(kvm, KVM_MMIO_BUS,
+					&assigned_dev->msix_mmio_dev);
+			mutex_unlock(&kvm->slots_lock);
+		}
+#endif
 		for (i = 0; i < assigned_dev->entries_nr; i++)
 			disable_irq_nosync(assigned_dev->
 					   host_msix_entries[i].vector);

@@ -318,6 +328,15 @@ static int assigned_device_enable_host_msix(struct kvm
 *kvm, goto err;
 	}
 
+	if (dev->msix_mmio_base) {
+		mutex_lock(&kvm->slots_lock);
+		r = kvm_io_bus_register_dev(kvm, KVM_MMIO_BUS,
+				&dev->msix_mmio_dev);
+		mutex_unlock(&kvm->slots_lock);
+		if (r)
+			goto err;
+	}
+
 	return 0;
 err:
 	for (i -= 1; i >= 0; i--)

@@ -870,6 +889,31 @@ static const struct kvm_io_device_ops msix_mmio_ops =
 { .write    = msix_mmio_write,
 };
 
+static int kvm_vm_ioctl_register_msix_mmio(struct kvm *kvm,
+				struct kvm_assigned_msix_mmio *msix_mmio)
+{
+	int r = 0;
+	struct kvm_assigned_dev_kernel *adev;
+
+	mutex_lock(&kvm->lock);
+	adev = kvm_find_assigned_dev(&kvm->arch.assigned_dev_head,
+				      msix_mmio->assigned_dev_id);
+	if (!adev) {
+		r = -EINVAL;
+		goto out;
+	}
+	if (msix_mmio->base_addr == 0) {
+		r = -EINVAL;
+		goto out;
+	}
+	adev->msix_mmio_base = msix_mmio->base_addr;
+
+	kvm_iodevice_init(&adev->msix_mmio_dev, &msix_mmio_ops);
+out:
+	mutex_unlock(&kvm->lock);
+
+	return r;
+}
 #endif
 
 long kvm_vm_ioctl_assigned_device(struct kvm *kvm, unsigned ioctl,

@@ -982,6 +1026,22 @@ long kvm_vm_ioctl_assigned_device(struct kvm *kvm,
 unsigned ioctl, goto out;
 		break;
 	}
+	case KVM_ASSIGN_REG_MSIX_MMIO: {
+		struct kvm_assigned_msix_mmio msix_mmio;
+
+		r = -EFAULT;
+		if (copy_from_user(&msix_mmio, argp, sizeof(msix_mmio)))
+			goto out;
+
+		r = -EINVAL;
+		if (msix_mmio.flags != 0 || msix_mmio.reserved != 0)
+			goto out;
+
+		r = kvm_vm_ioctl_register_msix_mmio(kvm, &msix_mmio);
+		if (r)
+			goto out;
+		break;
+	}
 #endif
 	}
 out:

Quoted 16 lines · Sheng Yang

Quoted 1 line · On Wednesday 20 October 2010 16:26:32 Sheng Yang wrote:

This patch enable per-vector mask for assigned devices using MSI-X.

The basic idea of kernel and QEmu's responsibilities are:

1. Because QEmu owned the irq routing table, so the change of table should still go to the QEmu, like we did in msix_mmio_write().

2. And the others things can be done in kernel, for performance. Here we covered the reading(converted entry from routing table and mask bit state of enabled MSI-X entries), and writing the mask bit for enabled MSI-X entries. Originally we only has mask bit handled in kernel, but later we found that Linux kernel would read MSI-X mmio just after every writing to mask bit, in order to flush the writing. So we add reading MSI data/addr as well.

Quoted 6 lines

3. Disabled entries's mask bit accessing would go to QEmu, because it may result in disable/enable MSI-X. Explained later.

4. Only QEmu has knowledge of PCI configuration space, so it's QEmu to decide enable/disable MSI-X for device.
.

Config space yes, but it's a simple global yes/no after all.

Quoted 2 lines

5. There is an distinction between enabled entry and disabled entry of MSI-X table.

That's my point. There's no such thing as 'enabled entries' in the spec. There are only masked and unmasked entries.

Current interface deals with gsi numbers so qemu had to work around this. The hack used there is removing gsi for masked vector which has 0 address and data. It works because this is what linux and windows guests happen to do, but it is out of spec: vector/data value for a masked entry have no meaning.

Since you are building a new interface, can design it without constraints...

Quoted 10 lines

The entries we had used for pci_enable_msix()(not necessary in sequence number) are already enabled, the others are disabled. When device's MSI-X is enabled and guest want to enable an disabled entry, we would go back to QEmu because this vector didn't exist in the routing table. Also due to pci_enable_msix() in kernel didn't allow us to enable vectors one by one, but all at once. So we have to disable MSI-X first, then enable it with new entries, which contained the new vector guest want to use. This situation is only happen when device is being initialized. After that, kernel can know and handle the mask bit of the enabled entry.

Quoted 3 lines

I've also considered handle all MMIO operation in kernel, and changing irq routing in kernel directly. But as long as irq routing is owned by QEmu, I think it's better to leave to it...

Yes, this is my suggestion, except we don't need no routing :) To inject MSI you just need address/data pairs.
Look at kvm_set_msi: given address/data you can just inject the interrupt. No need for table lookups.

Quoted 9 lines

Notice the mask/unmask bits must be handled together, either in kernel or in userspace. Because if kernel has handled enabled vector's mask bit directly, it would be unsync with QEmu's records. It doesn't matter when QEmu don't access the related record. And the only place QEmu want to consult it's enabled entries' mask bit state is writing to MSI addr/data. The writing should be discarded if the entry is unmasked. This checking has already been done by kernel in this patchset, so we are fine here.

Quoted 2 lines

If we want to access the enabled entries' mask bit in the future, we can directly access device's MMIO.

We really must implement this for correctness, btw. If you do not pass reads to the device, messages intended for the masked entry might still be in flight.

Quoted 13 lines

That's the reason why I have followed Michael's advice to use mask/unmask directly.
Hope this would make the patches more clear. I meant to add comments for this changeset, but miss it later.

--
regards
Yang, Sheng

Quoted 3 lines

Signed-off-by Sheng Yang <[email protected]>

---

Quoted 6 lines

Quoted 6 lines

 Documentation/kvm/api.txt |   22 ++++++++++++++++
 arch/x86/kvm/x86.c        |    6 ++++
 include/linux/kvm.h       |    8 +++++-
 virt/kvm/assigned-dev.c   |   60

+++++++++++++++++++++++++++++++++++++++++++++ 4 files changed,

Quoted 239 lines

Quoted 239 lines

insertions(+), 1 deletions(-)

Documentation/kvm/api.txt +0 -0

index d82d637..f324a50 100644
@@ -1087,6 +1087,28 @@ of 4 instructions that make up a hypercall.
 
 If any additional field gets added to this structure later on, a bit
 for
 

that additional piece of information will be set in the flags bitmap.

+4.47 KVM_ASSIGN_REG_MSIX_MMIO

Documentation/kvm/api.txt +19 -0

+
+Capability: KVM_CAP_DEVICE_MSIX_MASK
+Architectures: x86
+Type: vm ioctl
+Parameters: struct kvm_assigned_msix_mmio (in)
+Returns: 0 on success, !0 on error
+
+struct kvm_assigned_msix_mmio {
+	/* Assigned device's ID */
+	__u32 assigned_dev_id;
+	/* MSI-X table MMIO address */
+	__u64 base_addr;
+	/* Must be 0 */
+	__u32 flags;
+	/* Must be 0, reserved for future use */
+	__u64 reserved;
+};
+
+This ioctl would enable in-kernel MSI-X emulation, which would handle

MSI-X +mask bit in the kernel.

Documentation/kvm/api.txt +1 -0

+
 
 5. The kvm_run structure
 
 Application code obtains a pointer to the kvm_run structure by

arch/x86/kvm/x86.c +2 -0

index fc62546..ba07a2f 100644
@@ -1927,6 +1927,8 @@ int kvm_dev_ioctl_check_extension(long ext)
 
 	case KVM_CAP_X86_ROBUST_SINGLESTEP:
 	case KVM_CAP_XSAVE:
 
 	case KVM_CAP_ENABLE_CAP:
+	case KVM_CAP_DEVICE_MSIX_EXT:
 
+	case KVM_CAP_DEVICE_MSIX_MASK:

arch/x86/kvm/x86.c +4 -0

 		r = 1;
 		break;
 
 	case KVM_CAP_COALESCED_MMIO:

@@ -2717,6 +2719,10 @@ static int kvm_vcpu_ioctl_enable_cap(struct
 kvm_vcpu *vcpu, return -EINVAL;
 
 	switch (cap->cap) {
 
+	case KVM_CAP_DEVICE_MSIX_EXT:
+		vcpu->kvm->arch.msix_flags_enabled = true;
+		r = 0;
+		break;
 
 	default:

arch/x86/kvm/x86.c +0 -0

 		r = -EINVAL;
 		break;

include/linux/kvm.h +4 -0

index 0a7bd34..1494ed0 100644
@@ -540,6 +540,10 @@ struct kvm_ppc_pvinfo {
 
 #endif
 #define KVM_CAP_PPC_GET_PVINFO 57
 #define KVM_CAP_PPC_IRQ_LEVEL 58
 
+#ifdef __KVM_HAVE_MSIX
+#define KVM_CAP_DEVICE_MSIX_EXT 59
+#define KVM_CAP_DEVICE_MSIX_MASK 60
+#endif
 

#ifdef KVM_CAP_IRQ_ROUTING

include/linux/kvm.h +0 -0

@@ -671,6 +675,8 @@ struct kvm_clock_data {
 
 #define KVM_XEN_HVM_CONFIG        _IOW(KVMIO,  0x7a, struct
 
 kvm_xen_hvm_config) #define KVM_SET_CLOCK             _IOW(KVMIO, 

0x7b, struct kvm_clock_data) #define KVM_GET_CLOCK _IOR(KVMIO, 0x7c, struct kvm_clock_data) +#define KVM_ASSIGN_REG_MSIX_MMIO _IOW(KVMIO, 0x7d, \

include/linux/kvm.h +1 -0

+					struct kvm_assigned_msix_mmio)
 
 /* Available with KVM_CAP_PIT_STATE2 */
 #define KVM_GET_PIT2              _IOR(KVMIO,  0x9f, struct

kvm_pit_state2) #define KVM_SET_PIT2 _IOW(KVMIO, 0xa0,
struct kvm_pit_state2) @@ -802,7 +808,7 @@ struct
kvm_assigned_msix_mmio {

include/linux/kvm.h +1 -1

 	__u32 assigned_dev_id;
 	__u64 base_addr;
 	__u32 flags;
 
-	__u32 reserved[2];
+	__u64 reserved;
 
 };
 
 #endif /* __LINUX_KVM_H */

virt/kvm/assigned-dev.c +2 -0

index 5d2adc4..9573194 100644
@@ -17,6 +17,8 @@
 
 #include <linux/pci.h>
 #include <linux/interrupt.h>
 #include <linux/slab.h>
 
+#include <linux/irqnr.h>
+
 

#include "irq.h"

static struct kvm_assigned_dev_kernel *kvm_find_assigned_dev(struct

list_head *head, @@ -169,6 +171,14 @@ static void
deassign_host_irq(struct kvm *kvm, */

virt/kvm/assigned-dev.c +17 -0

 	if (assigned_dev->irq_requested_type & KVM_DEV_IRQ_HOST_MSIX) {
 
 		int i;
 
+#ifdef __KVM_HAVE_MSIX
+		if (assigned_dev->msix_mmio_base) {
+			mutex_lock(&kvm->slots_lock);
+			kvm_io_bus_unregister_dev(kvm, KVM_MMIO_BUS,
+					&assigned_dev->msix_mmio_dev);
+			mutex_unlock(&kvm->slots_lock);
+		}
+#endif
 
 		for (i = 0; i < assigned_dev->entries_nr; i++)
 
 			disable_irq_nosync(assigned_dev->
 
 					   host_msix_entries[i].vector);

@@ -318,6 +328,15 @@ static int assigned_device_enable_host_msix(struct
 kvm *kvm, goto err;
 
 	}
 
+	if (dev->msix_mmio_base) {
+		mutex_lock(&kvm->slots_lock);
+		r = kvm_io_bus_register_dev(kvm, KVM_MMIO_BUS,
+				&dev->msix_mmio_dev);
+		mutex_unlock(&kvm->slots_lock);
+		if (r)
+			goto err;
+	}
+
 
 	return 0;

err:

virt/kvm/assigned-dev.c +25 -0

 	for (i -= 1; i >= 0; i--)

@@ -870,6 +889,31 @@ static const struct kvm_io_device_ops
 msix_mmio_ops = { .write    = msix_mmio_write,
 
 };
 
+static int kvm_vm_ioctl_register_msix_mmio(struct kvm *kvm,
+				struct kvm_assigned_msix_mmio *msix_mmio)
+{
+	int r = 0;
+	struct kvm_assigned_dev_kernel *adev;
+
+	mutex_lock(&kvm->lock);
+	adev = kvm_find_assigned_dev(&kvm->arch.assigned_dev_head,
+				      msix_mmio->assigned_dev_id);
+	if (!adev) {
+		r = -EINVAL;
+		goto out;
+	}
+	if (msix_mmio->base_addr == 0) {
+		r = -EINVAL;
+		goto out;
+	}
+	adev->msix_mmio_base = msix_mmio->base_addr;
+
+	kvm_iodevice_init(&adev->msix_mmio_dev, &msix_mmio_ops);
+out:
+	mutex_unlock(&kvm->lock);
+
+	return r;
+}
 
 #endif

long kvm_vm_ioctl_assigned_device(struct kvm *kvm, unsigned ioctl,

virt/kvm/assigned-dev.c +16 -0

@@ -982,6 +1026,22 @@ long kvm_vm_ioctl_assigned_device(struct kvm
 *kvm, unsigned ioctl, goto out;
 
 		break;
 
 	}
 
+	case KVM_ASSIGN_REG_MSIX_MMIO: {
+		struct kvm_assigned_msix_mmio msix_mmio;
+
+		r = -EFAULT;
+		if (copy_from_user(&msix_mmio, argp, sizeof(msix_mmio)))
+			goto out;
+
+		r = -EINVAL;
+		if (msix_mmio.flags != 0 || msix_mmio.reserved != 0)
+			goto out;
+
+		r = kvm_vm_ioctl_register_msix_mmio(kvm, &msix_mmio);
+		if (r)
+			goto out;
+		break;
+	}

#endif

}

out:

Quoted 29 lines · On Thursday 21 October 2010 16:39:07 Michael S. Tsirkin wrote:

Quoted 16 lines · Sheng Yang

Quoted 1 line · On Wednesday 20 October 2010 16:26:32 Sheng Yang wrote:

This patch enable per-vector mask for assigned devices using MSI-X.

The basic idea of kernel and QEmu's responsibilities are:

1. Because QEmu owned the irq routing table, so the change of table should still go to the QEmu, like we did in msix_mmio_write().

2. And the others things can be done in kernel, for performance. Here we covered the reading(converted entry from routing table and mask bit state of enabled MSI-X entries), and writing the mask bit for enabled MSI-X entries. Originally we only has mask bit handled in kernel, but later we found that Linux kernel would read MSI-X mmio just after every writing to mask bit, in order to flush the writing. So we add reading MSI data/addr as well.

Quoted 6 lines

3. Disabled entries's mask bit accessing would go to QEmu, because it may result in disable/enable MSI-X. Explained later.

4. Only QEmu has knowledge of PCI configuration space, so it's QEmu to decide enable/disable MSI-X for device.
.

Config space yes, but it's a simple global yes/no after all.

Quoted 2 lines

5. There is an distinction between enabled entry and disabled entry of MSI-X table.

Quoted 8 lines

That's my point. There's no such thing as 'enabled entries' in the spec. There are only masked and unmasked entries.

Current interface deals with gsi numbers so qemu had to work around this. The hack used there is removing gsi for masked vector which has 0 address and data. It works because this is what linux and windows guests happen to do, but it is out of spec: vector/data value for a masked entry have no meaning.

Well, I just realized something unnatural about the 0 contained data/address entry. So I checked spec again, and found the mask bit should be set after reset. So after fix this, I think unmasked 0 address/data entry shouldn't be there anymore.

Quoted 2 lines

Since you are building a new interface, can design it without constraints...

A constraint is pci_enable_msix().

We have to use it to allocate irq for each
entry, as well as program the entry in the real hardware. pci_enable_msix() is only a yes/no choice. We can't add new enabled entries after pci_enable_msix(),

and we can only enable/disable/mask/unmask one IRQ through kernel API, not the entry in the MSI-X table. And we still have to allocate new IRQ for new entry. When guest unmask "disabled entry", we have to disable and enable MSI-X again in order to use the new entry. That's why "enabled/disabled entry" concept existed.

So even guest only unmasked one entry, it's a totally different work for KVM underlaying. This logic won't change no matter where the MMIO handling is. And in fact I don't like this kind of tricky things in kernel...

Quoted 10 lines

Quoted 10 lines

The entries we had used for pci_enable_msix()(not necessary in sequence number) are already enabled, the others are disabled. When device's MSI-X is enabled and guest want to enable an disabled entry, we would go back to QEmu because this vector didn't exist in the routing table. Also due to pci_enable_msix() in kernel didn't allow us to enable vectors one by one, but all at once. So we have to disable MSI-X first, then enable it with new entries, which contained the new vector guest want to use. This situation is only happen when device is being initialized. After that, kernel can know and handle the mask bit of the enabled entry.

Quoted 8 lines

Quoted 3 lines

I've also considered handle all MMIO operation in kernel, and changing irq routing in kernel directly. But as long as irq routing is owned by QEmu, I think it's better to leave to it...

Yes, this is my suggestion, except we don't need no routing :) To inject MSI you just need address/data pairs.
Look at kvm_set_msi: given address/data you can just inject the interrupt. No need for table lookups.

You still need to look up data/address pair in the guest MSI-X table. The routing table used here is just an replacement for the table, because we can construct the entry according to the routing table. Two choices, using the routing table, or creating an new MSI-X table.

Still, the key is about who to own the routing/MSI-X table. If kernel own it, it would be straightforward to intercept all the MMIO in the kernel; but if it's QEmu, we still need go back to QEmu for it.

Quoted 9 lines

Quoted 9 lines

Notice the mask/unmask bits must be handled together, either in kernel or in userspace. Because if kernel has handled enabled vector's mask bit directly, it would be unsync with QEmu's records. It doesn't matter when QEmu don't access the related record. And the only place QEmu want to consult it's enabled entries' mask bit state is writing to MSI addr/data. The writing should be discarded if the entry is unmasked. This checking has already been done by kernel in this patchset, so we are fine here.

Quoted 6 lines

Quoted 2 lines

If we want to access the enabled entries' mask bit in the future, we can directly access device's MMIO.

We really must implement this for correctness, btw. If you do not pass reads to the device, messages intended for the masked entry might still be in flight.

Oh, yes, kernel would also mask the device as well. I would take this into consideration.

--
regards
Yang, Sheng

Quoted 13 lines

Quoted 13 lines

That's the reason why I have followed Michael's advice to use mask/unmask directly.
Hope this would make the patches more clear. I meant to add comments for this changeset, but miss it later.

--
regards
Yang, Sheng

Quoted 3 lines

Signed-off-by Sheng Yang <[email protected]>

---

Quoted 6 lines

Quoted 6 lines

Quoted 6 lines

 Documentation/kvm/api.txt |   22 ++++++++++++++++
 arch/x86/kvm/x86.c        |    6 ++++
 include/linux/kvm.h       |    8 +++++-
 virt/kvm/assigned-dev.c   |   60

+++++++++++++++++++++++++++++++++++++++++++++ 4 files changed,

95

Quoted 239 lines

Quoted 239 lines

Quoted 239 lines

insertions(+), 1 deletions(-)

Documentation/kvm/api.txt +0 -0

index d82d637..f324a50 100644
@@ -1087,6 +1087,28 @@ of 4 instructions that make up a hypercall.
 
 If any additional field gets added to this structure later on, a bit
 for
 

that additional piece of information will be set in the flags bitmap.

+4.47 KVM_ASSIGN_REG_MSIX_MMIO

Documentation/kvm/api.txt +19 -0

+
+Capability: KVM_CAP_DEVICE_MSIX_MASK
+Architectures: x86
+Type: vm ioctl
+Parameters: struct kvm_assigned_msix_mmio (in)
+Returns: 0 on success, !0 on error
+
+struct kvm_assigned_msix_mmio {
+	/* Assigned device's ID */
+	__u32 assigned_dev_id;
+	/* MSI-X table MMIO address */
+	__u64 base_addr;
+	/* Must be 0 */
+	__u32 flags;
+	/* Must be 0, reserved for future use */
+	__u64 reserved;
+};
+
+This ioctl would enable in-kernel MSI-X emulation, which would handle

MSI-X +mask bit in the kernel.

Documentation/kvm/api.txt +1 -0

+
 
 5. The kvm_run structure
 
 Application code obtains a pointer to the kvm_run structure by

arch/x86/kvm/x86.c +2 -0

index fc62546..ba07a2f 100644
@@ -1927,6 +1927,8 @@ int kvm_dev_ioctl_check_extension(long ext)
 
 	case KVM_CAP_X86_ROBUST_SINGLESTEP:
 	case KVM_CAP_XSAVE:
 
 	case KVM_CAP_ENABLE_CAP:
+	case KVM_CAP_DEVICE_MSIX_EXT:
 
+	case KVM_CAP_DEVICE_MSIX_MASK:

arch/x86/kvm/x86.c +4 -0

 		r = 1;
 		break;
 
 	case KVM_CAP_COALESCED_MMIO:

@@ -2717,6 +2719,10 @@ static int kvm_vcpu_ioctl_enable_cap(struct
 kvm_vcpu *vcpu, return -EINVAL;
 
 	switch (cap->cap) {
 
+	case KVM_CAP_DEVICE_MSIX_EXT:
+		vcpu->kvm->arch.msix_flags_enabled = true;
+		r = 0;
+		break;
 
 	default:

arch/x86/kvm/x86.c +0 -0

 		r = -EINVAL;
 		break;

include/linux/kvm.h +4 -0

index 0a7bd34..1494ed0 100644
@@ -540,6 +540,10 @@ struct kvm_ppc_pvinfo {
 
 #endif
 #define KVM_CAP_PPC_GET_PVINFO 57
 #define KVM_CAP_PPC_IRQ_LEVEL 58
 
+#ifdef __KVM_HAVE_MSIX
+#define KVM_CAP_DEVICE_MSIX_EXT 59
+#define KVM_CAP_DEVICE_MSIX_MASK 60
+#endif
 

#ifdef KVM_CAP_IRQ_ROUTING

include/linux/kvm.h +0 -0

@@ -671,6 +675,8 @@ struct kvm_clock_data {
 
 #define KVM_XEN_HVM_CONFIG        _IOW(KVMIO,  0x7a, struct
 
 kvm_xen_hvm_config) #define KVM_SET_CLOCK             _IOW(KVMIO, 

0x7b, struct kvm_clock_data) #define KVM_GET_CLOCK _IOR(KVMIO, 0x7c, struct kvm_clock_data) +#define KVM_ASSIGN_REG_MSIX_MMIO _IOW(KVMIO, 0x7d, \

include/linux/kvm.h +1 -0

+					struct kvm_assigned_msix_mmio)
 
 /* Available with KVM_CAP_PIT_STATE2 */
 #define KVM_GET_PIT2              _IOR(KVMIO,  0x9f, struct

kvm_pit_state2) #define KVM_SET_PIT2 _IOW(KVMIO, 0xa0,
struct kvm_pit_state2) @@ -802,7 +808,7 @@ struct
kvm_assigned_msix_mmio {

include/linux/kvm.h +1 -1

 	__u32 assigned_dev_id;
 	__u64 base_addr;
 	__u32 flags;
 
-	__u32 reserved[2];
+	__u64 reserved;
 
 };
 
 #endif /* __LINUX_KVM_H */

virt/kvm/assigned-dev.c +2 -0

index 5d2adc4..9573194 100644
@@ -17,6 +17,8 @@
 
 #include <linux/pci.h>
 #include <linux/interrupt.h>
 #include <linux/slab.h>
 
+#include <linux/irqnr.h>
+
 

#include "irq.h"

static struct kvm_assigned_dev_kernel *kvm_find_assigned_dev(struct

list_head *head, @@ -169,6 +171,14 @@ static void
deassign_host_irq(struct kvm *kvm, */

virt/kvm/assigned-dev.c +17 -0

 	if (assigned_dev->irq_requested_type & KVM_DEV_IRQ_HOST_MSIX) {
 
 		int i;
 
+#ifdef __KVM_HAVE_MSIX
+		if (assigned_dev->msix_mmio_base) {
+			mutex_lock(&kvm->slots_lock);
+			kvm_io_bus_unregister_dev(kvm, KVM_MMIO_BUS,
+					&assigned_dev->msix_mmio_dev);
+			mutex_unlock(&kvm->slots_lock);
+		}
+#endif
 
 		for (i = 0; i < assigned_dev->entries_nr; i++)
 
 			disable_irq_nosync(assigned_dev->
 
 					   host_msix_entries[i].vector);

@@ -318,6 +328,15 @@ static int assigned_device_enable_host_msix(struct
 kvm *kvm, goto err;
 
 	}
 
+	if (dev->msix_mmio_base) {
+		mutex_lock(&kvm->slots_lock);
+		r = kvm_io_bus_register_dev(kvm, KVM_MMIO_BUS,
+				&dev->msix_mmio_dev);
+		mutex_unlock(&kvm->slots_lock);
+		if (r)
+			goto err;
+	}
+
 
 	return 0;

err:

virt/kvm/assigned-dev.c +25 -0

 	for (i -= 1; i >= 0; i--)

@@ -870,6 +889,31 @@ static const struct kvm_io_device_ops
 msix_mmio_ops = { .write    = msix_mmio_write,
 
 };
 
+static int kvm_vm_ioctl_register_msix_mmio(struct kvm *kvm,
+				struct kvm_assigned_msix_mmio *msix_mmio)
+{
+	int r = 0;
+	struct kvm_assigned_dev_kernel *adev;
+
+	mutex_lock(&kvm->lock);
+	adev = kvm_find_assigned_dev(&kvm->arch.assigned_dev_head,
+				      msix_mmio->assigned_dev_id);
+	if (!adev) {
+		r = -EINVAL;
+		goto out;
+	}
+	if (msix_mmio->base_addr == 0) {
+		r = -EINVAL;
+		goto out;
+	}
+	adev->msix_mmio_base = msix_mmio->base_addr;
+
+	kvm_iodevice_init(&adev->msix_mmio_dev, &msix_mmio_ops);
+out:
+	mutex_unlock(&kvm->lock);
+
+	return r;
+}
 
 #endif

long kvm_vm_ioctl_assigned_device(struct kvm *kvm, unsigned ioctl,

virt/kvm/assigned-dev.c +16 -0

@@ -982,6 +1026,22 @@ long kvm_vm_ioctl_assigned_device(struct kvm
 *kvm, unsigned ioctl, goto out;
 
 		break;
 
 	}
 
+	case KVM_ASSIGN_REG_MSIX_MMIO: {
+		struct kvm_assigned_msix_mmio msix_mmio;
+
+		r = -EFAULT;
+		if (copy_from_user(&msix_mmio, argp, sizeof(msix_mmio)))
+			goto out;
+
+		r = -EINVAL;
+		if (msix_mmio.flags != 0 || msix_mmio.reserved != 0)
+			goto out;
+
+		r = kvm_vm_ioctl_register_msix_mmio(kvm, &msix_mmio);
+		if (r)
+			goto out;
+		break;
+	}

#endif

}

out:

Quoted 39 lines · Sheng Yang

Quoted 29 lines · On Thursday 21 October 2010 16:39:07 Michael S. Tsirkin wrote:

Quoted 16 lines · Sheng Yang

Quoted 1 line · On Wednesday 20 October 2010 16:26:32 Sheng Yang wrote:

This patch enable per-vector mask for assigned devices using MSI-X.

The basic idea of kernel and QEmu's responsibilities are:

1. Because QEmu owned the irq routing table, so the change of table should still go to the QEmu, like we did in msix_mmio_write().

2. And the others things can be done in kernel, for performance. Here we covered the reading(converted entry from routing table and mask bit state of enabled MSI-X entries), and writing the mask bit for enabled MSI-X entries. Originally we only has mask bit handled in kernel, but later we found that Linux kernel would read MSI-X mmio just after every writing to mask bit, in order to flush the writing. So we add reading MSI data/addr as well.

Quoted 6 lines

3. Disabled entries's mask bit accessing would go to QEmu, because it may result in disable/enable MSI-X. Explained later.

4. Only QEmu has knowledge of PCI configuration space, so it's QEmu to decide enable/disable MSI-X for device.
.

Config space yes, but it's a simple global yes/no after all.

Quoted 2 lines

5. There is an distinction between enabled entry and disabled entry of MSI-X table.

Quoted 8 lines

That's my point. There's no such thing as 'enabled entries' in the spec. There are only masked and unmasked entries.

Current interface deals with gsi numbers so qemu had to work around this. The hack used there is removing gsi for masked vector which has 0 address and data. It works because this is what linux and windows guests happen to do, but it is out of spec: vector/data value for a masked entry have no meaning.

Quoted 4 lines

Well, I just realized something unnatural about the 0 contained data/address entry. So I checked spec again, and found the mask bit should be set after reset. So after fix this, I think unmasked 0 address/data entry shouldn't be there anymore.

You are right that this 0 check is completely out of spec. But see below. The issue is the spec does not require you to mask an entry if the device does not use it. And we do not want to waste host vectors. One can imagine a logic where we would detect that an interrupt has not been used in a long while, mask it and give up
a host vector. Then check the pending bit once in a while to see whether device started using it.

Quoted 4 lines

Quoted 2 lines

Since you are building a new interface, can design it without constraints...

A constraint is pci_enable_msix().

It's an internal kernel API. No one prevents us from changing it.

Quoted 5 lines

We have to use it to allocate irq for each
entry, as well as program the entry in the real hardware. pci_enable_msix() is only a yes/no choice. We can't add new enabled entries after pci_enable_msix(),

With the current APIs.

Quoted 10 lines

and we can only enable/disable/mask/unmask one IRQ through kernel API, not the entry in the MSI-X table. And we still have to allocate new IRQ for new entry. When guest unmask "disabled entry", we have to disable and enable MSI-X again in order to use the new entry. That's why "enabled/disabled entry" concept existed.

So even guest only unmasked one entry, it's a totally different work for KVM underlaying. This logic won't change no matter where the MMIO handling is. And in fact I don't like this kind of tricky things in kernel...

A more fundamental problem is that host vectors are a limited resource, we don't want to waste them on entries that will end up unused. One can imagine some kind of logic where we check the pending bit on a masked entry and after a while give up the host vector.

This is what I said: making it spec compliant is harder as it will need core kernel changes. Still, it seems silly to design a kerne/userspace API around an internal API limitation ...

Quoted 26 lines

Quoted 12 lines

Quoted 12 lines

The entries we had used for pci_enable_msix()(not necessary in sequence number) are already enabled, the others are disabled. When device's MSI-X is enabled and guest want to enable an disabled entry, we would go back to QEmu because this vector didn't exist in the routing table. Also due to pci_enable_msix() in kernel didn't allow us to enable vectors one by one, but all at once. So we have to disable MSI-X first, then enable it with new entries, which contained the new vector guest want to use. This situation is only happen when device is being initialized. After that, kernel can know and handle the mask bit of the enabled entry.

Quoted 8 lines

Quoted 3 lines

I've also considered handle all MMIO operation in kernel, and changing irq routing in kernel directly. But as long as irq routing is owned by QEmu, I think it's better to leave to it...

Yes, this is my suggestion, except we don't need no routing :) To inject MSI you just need address/data pairs.
Look at kvm_set_msi: given address/data you can just inject the interrupt. No need for table lookups.

You still need to look up data/address pair in the guest MSI-X table. The routing table used here is just an replacement for the table, because we can construct the entry according to the routing table. Two choices, using the routing table, or creating an new MSI-X table.

Quoted 3 lines

Still, the key is about who to own the routing/MSI-X table. If kernel own it, it would be straightforward to intercept all the MMIO in the kernel; but if it's QEmu, we still need go back to QEmu for it.

Looks cleaner to do it in kernel...

Quoted 41 lines

Quoted 9 lines

Quoted 9 lines

Notice the mask/unmask bits must be handled together, either in kernel or in userspace. Because if kernel has handled enabled vector's mask bit directly, it would be unsync with QEmu's records. It doesn't matter when QEmu don't access the related record. And the only place QEmu want to consult it's enabled entries' mask bit state is writing to MSI addr/data. The writing should be discarded if the entry is unmasked. This checking has already been done by kernel in this patchset, so we are fine here.

Quoted 6 lines

Quoted 2 lines

If we want to access the enabled entries' mask bit in the future, we can directly access device's MMIO.

We really must implement this for correctness, btw. If you do not pass reads to the device, messages intended for the masked entry might still be in flight.

Oh, yes, kernel would also mask the device as well. I would take this into consideration.

--
regards
Yang, Sheng

Quoted 18 lines

Quoted 18 lines

That's the reason why I have followed Michael's advice to use mask/unmask directly.
Hope this would make the patches more clear. I meant to add comments for this changeset, but miss it later.

--
regards
Yang, Sheng

Quoted 9 lines

Signed-off-by Sheng Yang <[email protected]>

---

 Documentation/kvm/api.txt |   22 ++++++++++++++++
 arch/x86/kvm/x86.c        |    6 ++++
 include/linux/kvm.h       |    8 +++++-
 virt/kvm/assigned-dev.c   |   60

+++++++++++++++++++++++++++++++++++++++++++++ 4 files

Quoted 244 lines

95

Quoted 241 lines

Quoted 241 lines

Quoted 241 lines

insertions(+), 1 deletions(-)

Documentation/kvm/api.txt +0 -0

index d82d637..f324a50 100644
@@ -1087,6 +1087,28 @@ of 4 instructions that make up a hypercall.
 
 If any additional field gets added to this structure later on, a
 bit for
 

that additional piece of information will be set in the flags bitmap.

+4.47 KVM_ASSIGN_REG_MSIX_MMIO

Documentation/kvm/api.txt +19 -0

+
+Capability: KVM_CAP_DEVICE_MSIX_MASK
+Architectures: x86
+Type: vm ioctl
+Parameters: struct kvm_assigned_msix_mmio (in)
+Returns: 0 on success, !0 on error
+
+struct kvm_assigned_msix_mmio {
+	/* Assigned device's ID */
+	__u32 assigned_dev_id;
+	/* MSI-X table MMIO address */
+	__u64 base_addr;
+	/* Must be 0 */
+	__u32 flags;
+	/* Must be 0, reserved for future use */
+	__u64 reserved;
+};
+
+This ioctl would enable in-kernel MSI-X emulation, which would

handle MSI-X +mask bit in the kernel.

Documentation/kvm/api.txt +1 -0

+
 
 5. The kvm_run structure
 
 Application code obtains a pointer to the kvm_run structure by

arch/x86/kvm/x86.c +2 -0

index fc62546..ba07a2f 100644
@@ -1927,6 +1927,8 @@ int kvm_dev_ioctl_check_extension(long ext)
 
 	case KVM_CAP_X86_ROBUST_SINGLESTEP:
 	case KVM_CAP_XSAVE:
 
 	case KVM_CAP_ENABLE_CAP:
+	case KVM_CAP_DEVICE_MSIX_EXT:
 
+	case KVM_CAP_DEVICE_MSIX_MASK:

arch/x86/kvm/x86.c +4 -0

 		r = 1;
 		break;
 
 	case KVM_CAP_COALESCED_MMIO:

@@ -2717,6 +2719,10 @@ static int kvm_vcpu_ioctl_enable_cap(struct
 kvm_vcpu *vcpu, return -EINVAL;
 
 	switch (cap->cap) {
 
+	case KVM_CAP_DEVICE_MSIX_EXT:
+		vcpu->kvm->arch.msix_flags_enabled = true;
+		r = 0;
+		break;
 
 	default:

arch/x86/kvm/x86.c +0 -0

 		r = -EINVAL;
 		break;

include/linux/kvm.h +4 -0

index 0a7bd34..1494ed0 100644
@@ -540,6 +540,10 @@ struct kvm_ppc_pvinfo {
 
 #endif
 #define KVM_CAP_PPC_GET_PVINFO 57
 #define KVM_CAP_PPC_IRQ_LEVEL 58
 
+#ifdef __KVM_HAVE_MSIX
+#define KVM_CAP_DEVICE_MSIX_EXT 59
+#define KVM_CAP_DEVICE_MSIX_MASK 60
+#endif
 

#ifdef KVM_CAP_IRQ_ROUTING

include/linux/kvm.h +0 -0

@@ -671,6 +675,8 @@ struct kvm_clock_data {
 
 #define KVM_XEN_HVM_CONFIG        _IOW(KVMIO,  0x7a, struct
 
 kvm_xen_hvm_config) #define KVM_SET_CLOCK             _IOW(KVMIO,

0x7b, struct kvm_clock_data) #define KVM_GET_CLOCK _IOR(KVMIO, 0x7c, struct kvm_clock_data) +#define KVM_ASSIGN_REG_MSIX_MMIO _IOW(KVMIO, 0x7d, \

include/linux/kvm.h +1 -0

+					struct kvm_assigned_msix_mmio)
 
 /* Available with KVM_CAP_PIT_STATE2 */
 #define KVM_GET_PIT2              _IOR(KVMIO,  0x9f, struct

kvm_pit_state2) #define KVM_SET_PIT2 _IOW(KVMIO,
0xa0, struct kvm_pit_state2) @@ -802,7 +808,7 @@ struct kvm_assigned_msix_mmio {

include/linux/kvm.h +1 -1

 	__u32 assigned_dev_id;
 	__u64 base_addr;
 	__u32 flags;
 
-	__u32 reserved[2];
+	__u64 reserved;
 
 };
 
 #endif /* __LINUX_KVM_H */

virt/kvm/assigned-dev.c +2 -0

index 5d2adc4..9573194 100644
@@ -17,6 +17,8 @@
 
 #include <linux/pci.h>
 #include <linux/interrupt.h>
 #include <linux/slab.h>
 
+#include <linux/irqnr.h>
+
 

#include "irq.h"

static struct kvm_assigned_dev_kernel
*kvm_find_assigned_dev(struct

list_head *head, @@ -169,6 +171,14 @@ static void
deassign_host_irq(struct kvm *kvm, */

virt/kvm/assigned-dev.c +17 -0

 	if (assigned_dev->irq_requested_type & KVM_DEV_IRQ_HOST_MSIX) {
 
 		int i;
 
+#ifdef __KVM_HAVE_MSIX
+		if (assigned_dev->msix_mmio_base) {
+			mutex_lock(&kvm->slots_lock);
+			kvm_io_bus_unregister_dev(kvm, KVM_MMIO_BUS,
+					&assigned_dev->msix_mmio_dev);
+			mutex_unlock(&kvm->slots_lock);
+		}
+#endif
 
 		for (i = 0; i < assigned_dev->entries_nr; i++)
 
 			disable_irq_nosync(assigned_dev->
 
 					   host_msix_entries[i].vector);

@@ -318,6 +328,15 @@ static int
 assigned_device_enable_host_msix(struct kvm *kvm, goto err;
 
 	}
 
+	if (dev->msix_mmio_base) {
+		mutex_lock(&kvm->slots_lock);
+		r = kvm_io_bus_register_dev(kvm, KVM_MMIO_BUS,
+				&dev->msix_mmio_dev);
+		mutex_unlock(&kvm->slots_lock);
+		if (r)
+			goto err;
+	}
+
 
 	return 0;

err:

virt/kvm/assigned-dev.c +25 -0

 	for (i -= 1; i >= 0; i--)

@@ -870,6 +889,31 @@ static const struct kvm_io_device_ops
 msix_mmio_ops = { .write    = msix_mmio_write,
 
 };
 
+static int kvm_vm_ioctl_register_msix_mmio(struct kvm *kvm,
+				struct kvm_assigned_msix_mmio *msix_mmio)
+{
+	int r = 0;
+	struct kvm_assigned_dev_kernel *adev;
+
+	mutex_lock(&kvm->lock);
+	adev = kvm_find_assigned_dev(&kvm->arch.assigned_dev_head,
+				      msix_mmio->assigned_dev_id);
+	if (!adev) {
+		r = -EINVAL;
+		goto out;
+	}
+	if (msix_mmio->base_addr == 0) {
+		r = -EINVAL;
+		goto out;
+	}
+	adev->msix_mmio_base = msix_mmio->base_addr;
+
+	kvm_iodevice_init(&adev->msix_mmio_dev, &msix_mmio_ops);
+out:
+	mutex_unlock(&kvm->lock);
+
+	return r;
+}
 
 #endif

long kvm_vm_ioctl_assigned_device(struct kvm *kvm, unsigned ioctl,

virt/kvm/assigned-dev.c +16 -0

@@ -982,6 +1026,22 @@ long kvm_vm_ioctl_assigned_device(struct kvm
 *kvm, unsigned ioctl, goto out;
 
 		break;
 
 	}
 
+	case KVM_ASSIGN_REG_MSIX_MMIO: {
+		struct kvm_assigned_msix_mmio msix_mmio;
+
+		r = -EFAULT;
+		if (copy_from_user(&msix_mmio, argp, sizeof(msix_mmio)))
+			goto out;
+
+		r = -EINVAL;
+		if (msix_mmio.flags != 0 || msix_mmio.reserved != 0)
+			goto out;
+
+		r = kvm_vm_ioctl_register_msix_mmio(kvm, &msix_mmio);
+		if (r)
+			goto out;
+		break;
+	}

#endif

}

out: