MSI-X MMIO support for KVM

Then we can support mask bit operation of assigned devices now.

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

---
 arch/x86/include/asm/kvm_host.h |    1 +
 arch/x86/kvm/Makefile           |    2 +-
 arch/x86/kvm/mmu.c              |    2 +
 arch/x86/kvm/x86.c              |   40 ++++-
 include/linux/kvm.h             |   28 ++++
 include/linux/kvm_host.h        |   36 +++++
 virt/kvm/assigned-dev.c         |   44 ++++++
 virt/kvm/kvm_main.c             |   38 +++++-
 virt/kvm/msix_mmio.c            |  301 +++++++++++++++++++++++++++++++++++++++
 virt/kvm/msix_mmio.h            |   25 ++++
 10 files changed, 504 insertions(+), 13 deletions(-)
 create mode 100644 virt/kvm/msix_mmio.c
 create mode 100644 virt/kvm/msix_mmio.h

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

index aa75f21..4a390a4 100644
@@ -635,6 +635,7 @@ enum emulation_result {
 	EMULATE_DONE,       /* no further processing */
 	EMULATE_DO_MMIO,      /* kvm_run filled with mmio request */
 	EMULATE_FAIL,         /* can't emulate this instruction */
+	EMULATE_USERSPACE_EXIT, /* we need exit to userspace */
 };
 
 #define EMULTYPE_NO_DECODE	    (1 << 0)

arch/x86/kvm/Makefile +1 -1

index f15501f..3a0d851 100644
@@ -7,7 +7,7 @@ CFLAGS_vmx.o := -I.
 
 kvm-y			+= $(addprefix ../../../virt/kvm/, kvm_main.o ioapic.o \
 				coalesced_mmio.o irq_comm.o eventfd.o \
-				assigned-dev.o)
+				assigned-dev.o msix_mmio.o)
 kvm-$(CONFIG_IOMMU_API)	+= $(addprefix ../../../virt/kvm/, iommu.o)
 kvm-$(CONFIG_KVM_ASYNC_PF)	+= $(addprefix ../../../virt/kvm/, async_pf.o)
 

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

index 9cafbb4..912dca4 100644
@@ -3358,6 +3358,8 @@ int kvm_mmu_page_fault(struct kvm_vcpu *vcpu, gva_t cr2, u32 error_code,
 	case EMULATE_DO_MMIO:
 		++vcpu->stat.mmio_exits;
 		/* fall through */
+	case EMULATE_USERSPACE_EXIT:
+		/* fall through */
 	case EMULATE_FAIL:
 		return 0;
 	default:

arch/x86/kvm/x86.c +3 -1

index 21b84e2..87308eb 100644
@@ -1966,6 +1966,7 @@ int kvm_dev_ioctl_check_extension(long ext)
 	case KVM_CAP_X86_ROBUST_SINGLESTEP:
 	case KVM_CAP_XSAVE:
 	case KVM_CAP_ASYNC_PF:
+	case KVM_CAP_MSIX_MMIO:
 		r = 1;
 		break;
 	case KVM_CAP_COALESCED_MMIO:

@@ -3809,6 +3810,7 @@ static int emulator_write_emulated_onepage(unsigned long addr,
 {
 	gpa_t                 gpa;
 	struct kvm_io_ext_data ext_data;
+	int r;
 
 	gpa = kvm_mmu_gva_to_gpa_write(vcpu, addr, exception);
 

@@ -3824,18 +3826,32 @@ static int emulator_write_emulated_onepage(unsigned long addr,
 
 mmio:
 	trace_kvm_mmio(KVM_TRACE_MMIO_WRITE, bytes, gpa, *(u64 *)val);
+	r = vcpu_mmio_write(vcpu, gpa, bytes, val, &ext_data);
 	/*
 	 * Is this MMIO handled locally?
 	 */
-	if (!vcpu_mmio_write(vcpu, gpa, bytes, val, &ext_data))

arch/x86/kvm/x86.c +27 -1

+		vcpu->userspace_exit_needed = 1;
+		vcpu->run->exit_reason = KVM_EXIT_MSIX_ROUTING_UPDATE;
+		vcpu->run->msix_routing.dev_id =
+			ext_data.msix_routing.dev_id;
+		vcpu->run->msix_routing.type =
+			ext_data.msix_routing.type;
+		vcpu->run->msix_routing.entry_idx =
+			ext_data.msix_routing.entry_idx;
+		vcpu->run->msix_routing.flags =
+			ext_data.msix_routing.flags;
+	} else  {
+		vcpu->mmio_needed = 1;
+		vcpu->run->exit_reason = KVM_EXIT_MMIO;
+		vcpu->run->mmio.phys_addr = vcpu->mmio_phys_addr = gpa;
+		vcpu->run->mmio.len = vcpu->mmio_size = bytes;
+		vcpu->run->mmio.is_write = vcpu->mmio_is_write = 1;
+		memcpy(vcpu->run->mmio.data, val, bytes);
+	}
 
 	return X86EMUL_CONTINUE;
 }

@@ -4469,6 +4485,8 @@ done:
 		r = EMULATE_DO_MMIO;
 	} else if (r == EMULATION_RESTART)
 		goto restart;
+	else if (vcpu->userspace_exit_needed)
+		r = EMULATE_USERSPACE_EXIT;
 	else
 		r = EMULATE_DONE;
 

@@ -5397,12 +5415,18 @@ int kvm_arch_vcpu_ioctl_run(struct kvm_vcpu *vcpu, struct kvm_run *kvm_run)
 		}
 	}
 
-	if (vcpu->arch.pio.count || vcpu->mmio_needed) {
+	if (vcpu->arch.pio.count || vcpu->mmio_needed ||
+			vcpu->userspace_exit_needed) {
 		if (vcpu->mmio_needed) {
 			memcpy(vcpu->mmio_data, kvm_run->mmio.data, 8);
 			vcpu->mmio_read_completed = 1;
 			vcpu->mmio_needed = 0;
 		}
+		if (vcpu->userspace_exit_needed) {
+			vcpu->userspace_exit_needed = 0;
+			r = 0;
+			goto out;
+		}
 		vcpu->srcu_idx = srcu_read_lock(&vcpu->kvm->srcu);
 		r = emulate_instruction(vcpu, EMULTYPE_NO_DECODE);
 		srcu_read_unlock(&vcpu->kvm->srcu, vcpu->srcu_idx);

include/linux/kvm.h +28 -0

index ea2dc1a..4393e4e 100644
@@ -161,6 +161,7 @@ struct kvm_pit_config {
 #define KVM_EXIT_NMI              16
 #define KVM_EXIT_INTERNAL_ERROR   17
 #define KVM_EXIT_OSI              18
+#define KVM_EXIT_MSIX_ROUTING_UPDATE 19
 
 /* For KVM_EXIT_INTERNAL_ERROR */
 #define KVM_INTERNAL_ERROR_EMULATION 1

@@ -264,6 +265,13 @@ struct kvm_run {
 		struct {
 			__u64 gprs[32];
 		} osi;
+		/* KVM_EXIT_MSIX_ROUTING_UPDATE*/
+		struct {
+			__u32 dev_id;
+			__u16 type;
+			__u16 entry_idx;
+			__u64 flags;
+		} msix_routing;
 		/* Fix the size of the union. */
 		char padding[256];
 	};

@@ -541,6 +549,7 @@ struct kvm_ppc_pvinfo {
 #define KVM_CAP_PPC_GET_PVINFO 57
 #define KVM_CAP_PPC_IRQ_LEVEL 58
 #define KVM_CAP_ASYNC_PF 59
+#define KVM_CAP_MSIX_MMIO 60
 
 #ifdef KVM_CAP_IRQ_ROUTING
 

@@ -672,6 +681,9 @@ 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)
+/* Available with KVM_CAP_MSIX_MMIO */
+#define KVM_REGISTER_MSIX_MMIO    _IOW(KVMIO,  0x7d, struct kvm_msix_mmio_user)
+#define KVM_UNREGISTER_MSIX_MMIO  _IOW(KVMIO,  0x7e, struct kvm_msix_mmio_user)
 /* 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)

@@ -795,4 +807,20 @@ struct kvm_assigned_msix_entry {
 	__u16 padding[3];
 };
 
+#define KVM_MSIX_MMIO_TYPE_ASSIGNED_DEV	    (1 << 0)
+
+#define KVM_MSIX_MMIO_TYPE_BASE_TABLE	    (1 << 8)
+
+#define KVM_MSIX_MMIO_TYPE_DEV_MASK	    0x00ff
+#define KVM_MSIX_MMIO_TYPE_BASE_MASK	    0xff00
+struct kvm_msix_mmio_user {
+	__u32 dev_id;
+	__u16 type;
+	__u16 max_entries_nr;
+	__u64 base_addr;
+	__u64 base_va;
+	__u64 flags;
+	__u64 reserved[4];
+};
+
 #endif /* __LINUX_KVM_H */

include/linux/kvm_host.h +0 -0

index a32c53e..d6c05e3 100644

include/linux/kvm_host.h +3 -0

+	};
 };
 
 int kvm_io_bus_write(struct kvm *kvm, enum kvm_bus bus_idx, gpa_t addr,

@@ -165,6 +174,8 @@ struct kvm_vcpu {
 	} async_pf;
 #endif
 
+	int userspace_exit_needed;
+
 	struct kvm_vcpu_arch arch;
 };
 

@@ -238,6 +249,27 @@ struct kvm_memslots {
 					KVM_PRIVATE_MEM_SLOTS];
 };

include/linux/kvm_host.h +16 -0

+	gpa_t pba_base_addr;
+	hva_t pba_base_va;
+};
+
+struct kvm_msix_mmio_dev {
+	struct kvm *kvm;
+	struct kvm_io_device table_dev;
+	int mmio_nr;
+	struct kvm_msix_mmio mmio[KVM_MSIX_MMIO_MAX];
+	struct mutex lock;
+};
+
 struct kvm {
 	spinlock_t mmu_lock;
 	raw_spinlock_t requests_lock;

@@ -286,6 +318,7 @@ struct kvm {
 	long mmu_notifier_count;
 #endif
 	long tlbs_dirty;
+	struct kvm_msix_mmio_dev msix_mmio_dev;
 };
 
 /* The guest did something we don't support. */

@@ -558,6 +591,9 @@ void kvm_unregister_irq_ack_notifier(struct kvm *kvm,
 int kvm_request_irq_source_id(struct kvm *kvm);
 void kvm_free_irq_source_id(struct kvm *kvm, int irq_source_id);
 
+int kvm_assigned_device_update_msix_mask_bit(struct kvm *kvm,
+			int assigned_dev_id, int entry, bool mask);
+
 /* For vcpu->arch.iommu_flags */
 #define KVM_IOMMU_CACHE_COHERENCY	0x1
 

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

index ae72ae6..d1598a6 100644
@@ -18,6 +18,7 @@
 #include <linux/interrupt.h>
 #include <linux/slab.h>
 #include "irq.h"
+#include "msix_mmio.h"
 
 static struct kvm_assigned_dev_kernel *kvm_find_assigned_dev(struct list_head *head,
 						      int assigned_dev_id)

@@ -191,12 +192,25 @@ static void kvm_free_assigned_irq(struct kvm *kvm,
 	kvm_deassign_irq(kvm, assigned_dev, assigned_dev->irq_requested_type);
 }
 
+static void assigned_device_free_msix_mmio(struct kvm *kvm,
+				struct kvm_assigned_dev_kernel *adev)
+{
+	struct kvm_msix_mmio mmio;
+
+	mmio.dev_id = adev->assigned_dev_id;
+	mmio.type = KVM_MSIX_MMIO_TYPE_ASSIGNED_DEV |
+		    KVM_MSIX_MMIO_TYPE_BASE_TABLE;
+	kvm_free_msix_mmio(kvm, &mmio);
+}
+
 static void kvm_free_assigned_device(struct kvm *kvm,
 				     struct kvm_assigned_dev_kernel
 				     *assigned_dev)
 {
 	kvm_free_assigned_irq(kvm, assigned_dev);
 
+	assigned_device_free_msix_mmio(kvm, assigned_dev);
+
 	__pci_reset_function(assigned_dev->dev);
 	pci_restore_state(assigned_dev->dev);
 

@@ -785,3 +799,33 @@ out:
 	return r;
 }
 

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

+
+	adev = kvm_find_assigned_dev(&kvm->arch.assigned_dev_head,
+				      assigned_dev_id);
+	if (!adev)
+		goto out;
+
+	/* For non-MSIX enabled devices, entries_nr == 0 */
+	for (i = 0; i < adev->entries_nr; i++)
+		if (adev->host_msix_entries[i].entry == entry) {
+			if (mask)
+				disable_irq_nosync(
+					adev->host_msix_entries[i].vector);
+			else
+				enable_irq(adev->host_msix_entries[i].vector);
+			r = 0;
+			break;
+		}
+out:
+	return r;
+}

virt/kvm/kvm_main.c +20 -0

index a61f90e..f211e49 100644
@@ -56,6 +56,7 @@
 
 #include "coalesced_mmio.h"
 #include "async_pf.h"
+#include "msix_mmio.h"
 
 #define CREATE_TRACE_POINTS
 #include <trace/events/kvm.h>

@@ -509,6 +510,7 @@ static void kvm_destroy_vm(struct kvm *kvm)
 	struct mm_struct *mm = kvm->mm;
 
 	kvm_arch_sync_events(kvm);
+	kvm_unregister_msix_mmio_dev(kvm);
 	spin_lock(&kvm_lock);
 	list_del(&kvm->vm_list);
 	spin_unlock(&kvm_lock);

@@ -1877,6 +1879,24 @@ static long kvm_vm_ioctl(struct file *filp,
 		mutex_unlock(&kvm->lock);
 		break;
 #endif
+	case KVM_REGISTER_MSIX_MMIO: {
+		struct kvm_msix_mmio_user mmio_user;
+
+		r = -EFAULT;
+		if (copy_from_user(&mmio_user, argp, sizeof mmio_user))
+			goto out;
+		r = kvm_vm_ioctl_register_msix_mmio(kvm, &mmio_user);
+		break;
+	}
+	case KVM_UNREGISTER_MSIX_MMIO: {
+		struct kvm_msix_mmio_user mmio_user;
+
+		r = -EFAULT;
+		if (copy_from_user(&mmio_user, argp, sizeof mmio_user))
+			goto out;
+		r = kvm_vm_ioctl_unregister_msix_mmio(kvm, &mmio_user);
+		break;
+	}
 	default:
 		r = kvm_arch_vm_ioctl(filp, ioctl, arg);
 		if (r == -ENOTTY)

virt/kvm/kvm_main.c +8 -4

 	r = anon_inode_getfd("kvm-vm", &kvm_vm_fops, kvm, O_RDWR);
 	if (r < 0)
 		kvm_put_kvm(kvm);

@@ -2223,14 +2249,18 @@ static void kvm_io_bus_destroy(struct kvm_io_bus *bus)
 int kvm_io_bus_write(struct kvm *kvm, enum kvm_bus bus_idx, gpa_t addr,
 		     int len, const void *val, struct kvm_io_ext_data *ext_data)
 {
-	int i;
+	int i, r = -EOPNOTSUPP;
 	struct kvm_io_bus *bus;
 
 	bus = srcu_dereference(kvm->buses[bus_idx], &kvm->srcu);
-	for (i = 0; i < bus->dev_count; i++)
-		if (!kvm_iodevice_write(bus->devs[i], addr, len, val, ext_data))
+	for (i = 0; i < bus->dev_count; i++) {
+		r = kvm_iodevice_write(bus->devs[i], addr, len, val, ext_data);
+		if (r == -ENOTSYNC)
+			break;
+		else if (!r)
 			return 0;
-	return -EOPNOTSUPP;
+	}
+	return r;
 }
 
 /* kvm_io_bus_read - called under kvm->slots_lock */

virt/kvm/msix_mmio.c new file +12 -0

new file mode 100644
index 0000000..b2a5f86
@@ -0,0 +1,301 @@
+/*
+ * MSI-X MMIO emulation
+ *
+ * Copyright (c) 2010 Intel Corporation
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2.  See
+ * the COPYING file in the top-level directory.
+ *
+ * Author:
+ *   Sheng Yang <[email protected]>
+ */
+

virt/kvm/msix_mmio.c new file +16 -0

+}
+
+/* Caller must hold dev->lock */
+static int get_mmio_table_index(struct kvm_msix_mmio_dev *dev,
+				gpa_t addr, int len)
+{
+	gpa_t start, end;
+	int i, r = -EINVAL;
+
+	for (i = 0; i < dev->mmio_nr; i++) {
+		start = dev->mmio[i].table_base_addr;
+		end = dev->mmio[i].table_base_addr + PCI_MSIX_ENTRY_SIZE *
+			dev->mmio[i].max_entries_nr;
+		if (addr >= start && addr + len <= end) {
+			r = i;
+			break;

virt/kvm/msix_mmio.c new file +8 -0

+	struct kvm_msix_mmio_dev *mmio_dev =
+		container_of(this, struct kvm_msix_mmio_dev, table_dev);
+	struct kvm_msix_mmio *mmio;
+	int idx, ret = 0, entry, offset, r;
+
+	mutex_lock(&mmio_dev->lock);
+	idx = get_mmio_table_index(mmio_dev, addr, len);
+	if (idx < 0) {

virt/kvm/msix_mmio.c new file +21 -0

+	struct kvm_msix_mmio_dev *mmio_dev =
+		container_of(this, struct kvm_msix_mmio_dev, table_dev);
+	struct kvm_msix_mmio *mmio;
+	int idx, entry, offset, ret = 0, r = 0;
+	void __user *entry_base;
+	__le32 __user *ctrl_pos;
+	__le32 old_ctrl, new_ctrl;
+
+	mutex_lock(&mmio_dev->kvm->lock);
+	mutex_lock(&mmio_dev->lock);
+	idx = get_mmio_table_index(mmio_dev, addr, len);
+	if (idx < 0) {
+		ret = -EOPNOTSUPP;
+		goto out;
+	}
+	if ((len != 4 && len != 8) || addr & (len - 1))
+		goto out;
+
+	offset = addr % PCI_MSIX_ENTRY_SIZE;
+
+	mmio = &mmio_dev->mmio[idx];

virt/kvm/msix_mmio.c new file +8 -0

+
+	if (offset + len < PCI_MSIX_ENTRY_VECTOR_CTRL) {
+		ret = -ENOTSYNC;
+		goto out;
+	}
+
+	if (get_user(new_ctrl, ctrl_pos))
+		goto out;

virt/kvm/msix_mmio.c new file +48 -0

+	if (r)
+		ret = -ENOTSYNC;
+out:
+	mutex_unlock(&mmio_dev->lock);
+	mutex_unlock(&mmio_dev->kvm->lock);
+	return ret;
+}
+
+static const struct kvm_io_device_ops msix_mmio_table_ops = {
+	.read     = msix_table_mmio_read,
+	.write    = msix_table_mmio_write,
+};
+
+int kvm_register_msix_mmio_dev(struct kvm *kvm)
+{
+	int ret;
+
+	kvm_iodevice_init(&kvm->msix_mmio_dev.table_dev, &msix_mmio_table_ops);
+	mutex_init(&kvm->msix_mmio_dev.lock);
+	kvm->msix_mmio_dev.kvm = kvm;
+	mutex_lock(&kvm->slots_lock);
+	ret = kvm_io_bus_register_dev(kvm, KVM_MMIO_BUS,
+				      &kvm->msix_mmio_dev.table_dev);
+	mutex_unlock(&kvm->slots_lock);
+	return ret;
+}
+
+int kvm_unregister_msix_mmio_dev(struct kvm *kvm)
+{
+	int ret;
+
+	mutex_lock(&kvm->slots_lock);
+	ret = kvm_io_bus_unregister_dev(kvm, KVM_MMIO_BUS,
+				      &kvm->msix_mmio_dev.table_dev);
+	mutex_unlock(&kvm->slots_lock);
+	return ret;
+}
+
+int kvm_vm_ioctl_register_msix_mmio(struct kvm *kvm,
+				    struct kvm_msix_mmio_user *mmio_user)
+{
+	struct kvm_msix_mmio_dev *mmio_dev = &kvm->msix_mmio_dev;
+	struct kvm_msix_mmio *mmio = NULL;
+	int r = 0, i;
+
+	mutex_lock(&mmio_dev->lock);
+	for (i = 0; i < mmio_dev->mmio_nr; i++) {
+		if (mmio_dev->mmio[i].dev_id == mmio_user->dev_id &&

virt/kvm/msix_mmio.c new file +16 -0

+		goto out;
+	}
+	/* All reserved currently */
+	if (mmio_user->flags) {
+		r = -EINVAL;
+		goto out;
+	}
+
+	if ((mmio_user->type & KVM_MSIX_MMIO_TYPE_DEV_MASK) !=
+			KVM_MSIX_MMIO_TYPE_ASSIGNED_DEV) {
+		r = -EINVAL;
+		goto out;
+	}
+	if ((mmio_user->type & KVM_MSIX_MMIO_TYPE_BASE_MASK) !=
+			KVM_MSIX_MMIO_TYPE_BASE_TABLE) {
+		r = -EINVAL;

virt/kvm/msix_mmio.c new file +20 -0

+		goto out;
+	}
+	if (!mmio) {
+		if (mmio_dev->mmio_nr == KVM_MSIX_MMIO_MAX) {
+			r = -ENOSPC;
+			goto out;
+		}
+		mmio = &mmio_dev->mmio[mmio_dev->mmio_nr];
+		mmio_dev->mmio_nr++;
+	}
+
+	mmio->max_entries_nr = mmio_user->max_entries_nr;
+	mmio->dev_id = mmio_user->dev_id;
+	mmio->flags = mmio_user->flags;
+
+	if ((mmio_user->type & KVM_MSIX_MMIO_TYPE_DEV_MASK) ==
+			KVM_MSIX_MMIO_TYPE_ASSIGNED_DEV)
+		mmio->type = KVM_MSIX_MMIO_TYPE_ASSIGNED_DEV;
+	if ((mmio_user->type & KVM_MSIX_MMIO_TYPE_BASE_MASK) ==
+			KVM_MSIX_MMIO_TYPE_BASE_TABLE) {

virt/kvm/msix_mmio.c new file +24 -0

+{
+	struct kvm_msix_mmio_dev *mmio_dev = &kvm->msix_mmio_dev;
+	int r = -EINVAL, i, j;
+
+	if (!mmio)
+		return 0;
+
+	mutex_lock(&mmio_dev->lock);
+	BUG_ON(mmio_dev->mmio_nr > KVM_MSIX_MMIO_MAX);
+	for (i = 0; i < mmio_dev->mmio_nr; i++) {
+		if (mmio_dev->mmio[i].dev_id == mmio->dev_id &&
+		    mmio_dev->mmio[i].type == mmio->type) {
+			r = 0;
+			for (j = i; j < mmio_dev->mmio_nr - 1; j++)
+				mmio_dev->mmio[j] = mmio_dev->mmio[j + 1];
+			mmio_dev->mmio[mmio_dev->mmio_nr].max_entries_nr = 0;
+			mmio_dev->mmio[mmio_dev->mmio_nr].dev_id = 0;
+			mmio_dev->mmio[mmio_dev->mmio_nr].type = 0;
+			mmio_dev->mmio_nr--;
+			break;
+		}
+	}
+	mutex_unlock(&mmio_dev->lock);
+	return r;

virt/kvm/msix_mmio.c new file +3 -0

+	return kvm_free_msix_mmio(kvm, &mmio);
+}
+

virt/kvm/msix_mmio.h new file +5 -0

new file mode 100644
index 0000000..01b6587
@@ -0,0 +1,25 @@
+#ifndef __KVM_MSIX_MMIO_H__
+#define __KVM_MSIX_MMIO_H__
+/*
+ * MSI-X MMIO emulation
+ *

virt/kvm/msix_mmio.h new file +10 -0

+
+int kvm_register_msix_mmio_dev(struct kvm *kvm);
+int kvm_unregister_msix_mmio_dev(struct kvm *kvm);
+int kvm_vm_ioctl_register_msix_mmio(struct kvm *kvm,
+				    struct kvm_msix_mmio_user *mmio_user);
+int kvm_vm_ioctl_unregister_msix_mmio(struct kvm *kvm,
+				      struct kvm_msix_mmio_user *mmio_user);
+int kvm_free_msix_mmio(struct kvm *kvm, struct kvm_msix_mmio *mmio_user);
+
+#endif

Signature · 1 line

1.7.0.1

Quoted 3 lines · Sheng Yang

Then we can support mask bit operation of assigned devices now.

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

A general question: we implement mmio read and write operation here, but seem to do nothing
about ordering. In particular, pci mmio reads are normally assumed to flush all writes out to the device
and all device writes in to the CPU.

How exactly does it work here?

Quoted 114 lines

---

 arch/x86/include/asm/kvm_host.h |    1 +
 arch/x86/kvm/Makefile           |    2 +-
 arch/x86/kvm/mmu.c              |    2 +
 arch/x86/kvm/x86.c              |   40 ++++-
 include/linux/kvm.h             |   28 ++++
 include/linux/kvm_host.h        |   36 +++++
 virt/kvm/assigned-dev.c         |   44 ++++++
 virt/kvm/kvm_main.c             |   38 +++++-
 virt/kvm/msix_mmio.c            |  301
 +++++++++++++++++++++++++++++++++++++++ virt/kvm/msix_mmio.h           
 |   25 ++++
 10 files changed, 504 insertions(+), 13 deletions(-)
 create mode 100644 virt/kvm/msix_mmio.c
 create mode 100644 virt/kvm/msix_mmio.h

(quoted diff) +0 -0

b/arch/x86/include/asm/kvm_host.h index aa75f21..4a390a4 100644

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

@@ -635,6 +635,7 @@ enum emulation_result {
 
 	EMULATE_DONE,       /* no further processing */
 	EMULATE_DO_MMIO,      /* kvm_run filled with mmio request */
 	EMULATE_FAIL,         /* can't emulate this instruction */
 
+	EMULATE_USERSPACE_EXIT, /* we need exit to userspace */
 

};

#define EMULTYPE_NO_DECODE (1 << 0)

arch/x86/kvm/Makefile +1 -1

index f15501f..3a0d851 100644
@@ -7,7 +7,7 @@ CFLAGS_vmx.o := -I.
 
 kvm-y			+= $(addprefix ../../../virt/kvm/, kvm_main.o ioapic.o \
 
 				coalesced_mmio.o irq_comm.o eventfd.o \
 
-				assigned-dev.o)
+				assigned-dev.o msix_mmio.o)
 

kvm-$(CONFIG_IOMMU_API) += $(addprefix ../../../virt/kvm/, iommu.o)
kvm-$(CONFIG_KVM_ASYNC_PF) += $(addprefix ../../../virt/kvm/,
async_pf.o)

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

index 9cafbb4..912dca4 100644
@@ -3358,6 +3358,8 @@ int kvm_mmu_page_fault(struct kvm_vcpu *vcpu, gva_t
 cr2, u32 error_code,
 
 	case EMULATE_DO_MMIO:
 		++vcpu->stat.mmio_exits;
 		/* fall through */
 
+	case EMULATE_USERSPACE_EXIT:
+		/* fall through */

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

 	case EMULATE_FAIL:
 		return 0;
 
 	default:

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

index 21b84e2..87308eb 100644
@@ -1966,6 +1966,7 @@ int kvm_dev_ioctl_check_extension(long ext)
 
 	case KVM_CAP_X86_ROBUST_SINGLESTEP:
 	case KVM_CAP_XSAVE:
 
 	case KVM_CAP_ASYNC_PF:
+	case KVM_CAP_MSIX_MMIO:
 		r = 1;

arch/x86/kvm/x86.c +3 -1

 		break;
 
 	case KVM_CAP_COALESCED_MMIO:

@@ -3809,6 +3810,7 @@ static int emulator_write_emulated_onepage(unsigned
 long addr,
 
 {
 
 	gpa_t                 gpa;
 	struct kvm_io_ext_data ext_data;
 
+	int r;
 
 	gpa = kvm_mmu_gva_to_gpa_write(vcpu, addr, exception);

@@ -3824,18 +3826,32 @@ static int
 emulator_write_emulated_onepage(unsigned long addr,
 
 mmio:
 	trace_kvm_mmio(KVM_TRACE_MMIO_WRITE, bytes, gpa, *(u64 *)val);
 
+	r = vcpu_mmio_write(vcpu, gpa, bytes, val, &ext_data);
 
 	/*
 
 	 * Is this MMIO handled locally?
 	 */
 
-	if (!vcpu_mmio_write(vcpu, gpa, bytes, val, &ext_data))
+	if (!r)
 
 		return X86EMUL_CONTINUE;
 

arch/x86/kvm/x86.c static int emulator_write_emulated_onepage(unsigned long addr, · line 3836

Quoted 7 lines

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

-	vcpu->mmio_needed = 1;
-	vcpu->run->exit_reason = KVM_EXIT_MMIO;
-	vcpu->run->mmio.phys_addr = vcpu->mmio_phys_addr = gpa;
-	vcpu->run->mmio.len = vcpu->mmio_size = bytes;
-	vcpu->run->mmio.is_write = vcpu->mmio_is_write = 1;
-	memcpy(vcpu->run->mmio.data, val, bytes);
+	if (r == -ENOTSYNC) {

Can you replace -ENOTSYNC with KVM_EXIT_MSIX_ROUTING_UPDATE all over please?

Quoted 165 lines

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

+		vcpu->userspace_exit_needed = 1;
+		vcpu->run->exit_reason = KVM_EXIT_MSIX_ROUTING_UPDATE;
+		vcpu->run->msix_routing.dev_id =
+			ext_data.msix_routing.dev_id;
+		vcpu->run->msix_routing.type =
+			ext_data.msix_routing.type;
+		vcpu->run->msix_routing.entry_idx =
+			ext_data.msix_routing.entry_idx;
+		vcpu->run->msix_routing.flags =
+			ext_data.msix_routing.flags;
+	} else  {
+		vcpu->mmio_needed = 1;
+		vcpu->run->exit_reason = KVM_EXIT_MMIO;
+		vcpu->run->mmio.phys_addr = vcpu->mmio_phys_addr = gpa;
+		vcpu->run->mmio.len = vcpu->mmio_size = bytes;
+		vcpu->run->mmio.is_write = vcpu->mmio_is_write = 1;
+		memcpy(vcpu->run->mmio.data, val, bytes);
+	}
 
 	return X86EMUL_CONTINUE;
 
 }

@@ -4469,6 +4485,8 @@ done:
 		r = EMULATE_DO_MMIO;
 
 	} else if (r == EMULATION_RESTART)
 
 		goto restart;
 
+	else if (vcpu->userspace_exit_needed)
+		r = EMULATE_USERSPACE_EXIT;

arch/x86/kvm/x86.c +7 -1

 	else
 
 		r = EMULATE_DONE;

@@ -5397,12 +5415,18 @@ int kvm_arch_vcpu_ioctl_run(struct kvm_vcpu
 *vcpu, struct kvm_run *kvm_run)
 
 		}
 
 	}
 
-	if (vcpu->arch.pio.count || vcpu->mmio_needed) {
+	if (vcpu->arch.pio.count || vcpu->mmio_needed ||
+			vcpu->userspace_exit_needed) {
 
 		if (vcpu->mmio_needed) {
 
 			memcpy(vcpu->mmio_data, kvm_run->mmio.data, 8);
 			vcpu->mmio_read_completed = 1;
 			vcpu->mmio_needed = 0;
 
 		}
 
+		if (vcpu->userspace_exit_needed) {
+			vcpu->userspace_exit_needed = 0;
+			r = 0;
+			goto out;
+		}
 
 		vcpu->srcu_idx = srcu_read_lock(&vcpu->kvm->srcu);
 		r = emulate_instruction(vcpu, EMULTYPE_NO_DECODE);
 		srcu_read_unlock(&vcpu->kvm->srcu, vcpu->srcu_idx);

include/linux/kvm.h +1 -0

index ea2dc1a..4393e4e 100644
@@ -161,6 +161,7 @@ struct kvm_pit_config {
 
 #define KVM_EXIT_NMI              16
 #define KVM_EXIT_INTERNAL_ERROR   17
 #define KVM_EXIT_OSI              18
 
+#define KVM_EXIT_MSIX_ROUTING_UPDATE 19
 

/* For KVM_EXIT_INTERNAL_ERROR */
#define KVM_INTERNAL_ERROR_EMULATION 1

include/linux/kvm.h +8 -0

@@ -264,6 +265,13 @@ struct kvm_run {
 
 		struct {
 
 			__u64 gprs[32];
 
 		} osi;
 
+		/* KVM_EXIT_MSIX_ROUTING_UPDATE*/
+		struct {
+			__u32 dev_id;
+			__u16 type;
+			__u16 entry_idx;
+			__u64 flags;
+		} msix_routing;
 
 		/* Fix the size of the union. */
 		char padding[256];
 
 	};

@@ -541,6 +549,7 @@ struct kvm_ppc_pvinfo {
 
 #define KVM_CAP_PPC_GET_PVINFO 57
 #define KVM_CAP_PPC_IRQ_LEVEL 58
 #define KVM_CAP_ASYNC_PF 59
 
+#define KVM_CAP_MSIX_MMIO 60
 

#ifdef KVM_CAP_IRQ_ROUTING

include/linux/kvm.h +2 -0

@@ -672,6 +681,9 @@ 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)
 
+/* Available with KVM_CAP_MSIX_MMIO */
+#define KVM_REGISTER_MSIX_MMIO    _IOW(KVMIO,  0x7d, struct

kvm_msix_mmio_user) +#define KVM_UNREGISTER_MSIX_MMIO _IOW(KVMIO, 0x7e, struct kvm_msix_mmio_user)

/* 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)

include/linux/kvm.h +16 -0

@@ -795,4 +807,20 @@ struct kvm_assigned_msix_entry {
 
 	__u16 padding[3];
 
 };
 
+#define KVM_MSIX_MMIO_TYPE_ASSIGNED_DEV	    (1 << 0)
+
+#define KVM_MSIX_MMIO_TYPE_BASE_TABLE	    (1 << 8)
+
+#define KVM_MSIX_MMIO_TYPE_DEV_MASK	    0x00ff
+#define KVM_MSIX_MMIO_TYPE_BASE_MASK	    0xff00
+struct kvm_msix_mmio_user {
+	__u32 dev_id;
+	__u16 type;
+	__u16 max_entries_nr;
+	__u64 base_addr;
+	__u64 base_va;
+	__u64 flags;
+	__u64 reserved[4];
+};
+
 
 #endif /* __LINUX_KVM_H */

include/linux/kvm_host.h +0 -0

index a32c53e..d6c05e3 100644
@@ -68,8 +68,17 @@ enum kvm_bus {
 
 	KVM_NR_BUSES
 
 };
 

include/linux/kvm_host.h enum kvm_bus {

Quoted 5 lines

include/linux/kvm_host.h +1 -0

+#define KVM_IO_EXT_DATA_TYPE_MSIX_ROUTING   1
 
 struct kvm_io_ext_data {
 
 	int type;   /* Extended data type */

type seems unused for now. Do we need it?
I'd guess exit type should usually be sufficient.

include/linux/kvm_host.h enum kvm_bus { · line 80

Quoted 8 lines

include/linux/kvm_host.h +7 -0

+	union {
+		struct {
+			u32 dev_id;	/* Device ID */
+			u16 type;	/* Device type */
+			u16 entry_idx;  /* Accessed MSI-X entry index */
+			u64 flags;
+		} msix_routing;

So this is only for when we exit on msix routing?

Quoted 26 lines

include/linux/kvm_host.h +3 -0

+	};
 
 };
 
 int kvm_io_bus_write(struct kvm *kvm, enum kvm_bus bus_idx, gpa_t addr,

@@ -165,6 +174,8 @@ struct kvm_vcpu {
 
 	} async_pf;
 
 #endif
 
+	int userspace_exit_needed;
+
 

include/linux/kvm_host.h +0 -0

 	struct kvm_vcpu_arch arch;
 
 };

@@ -238,6 +249,27 @@ struct kvm_memslots {
 
 					KVM_PRIVATE_MEM_SLOTS];
 
 };
 

include/linux/kvm_host.h struct kvm_memslots { · line 260

Quoted 9 lines

include/linux/kvm_host.h +9 -0

+#define KVM_MSIX_MMIO_MAX    32
+
+struct kvm_msix_mmio {
+	u32 dev_id;
+	u16 type;
+	u16 max_entries_nr;
+	u64 flags;
+	gpa_t table_base_addr;
+	hva_t table_base_va;

void __user* would make for less casting in code.

Or even

	struct kvm_msix_entry {
		__le32 addr_lo;
		__le32 addr_hi;
		__le32 data;
		__le32 ctrl;
	};

	struct kvm_msix_entry __user *table;

and then we can do table[entry].ctrl
and so on.

Quoted 100 lines

include/linux/kvm_host.h +12 -0

+	gpa_t pba_base_addr;
+	hva_t pba_base_va;
+};
+
+struct kvm_msix_mmio_dev {
+	struct kvm *kvm;
+	struct kvm_io_device table_dev;
+	int mmio_nr;
+	struct kvm_msix_mmio mmio[KVM_MSIX_MMIO_MAX];
+	struct mutex lock;
+};
+
 

struct kvm {

include/linux/kvm_host.h +4 -0

 	spinlock_t mmu_lock;
 	raw_spinlock_t requests_lock;

@@ -286,6 +318,7 @@ struct kvm {
 
 	long mmu_notifier_count;
 
 #endif
 
 	long tlbs_dirty;
 
+	struct kvm_msix_mmio_dev msix_mmio_dev;
 
 };
 
 /* The guest did something we don't support. */

@@ -558,6 +591,9 @@ void kvm_unregister_irq_ack_notifier(struct kvm *kvm,
 
 int kvm_request_irq_source_id(struct kvm *kvm);
 void kvm_free_irq_source_id(struct kvm *kvm, int irq_source_id);
 
+int kvm_assigned_device_update_msix_mask_bit(struct kvm *kvm,
+			int assigned_dev_id, int entry, bool mask);
+
 
 /* For vcpu->arch.iommu_flags */

#define KVM_IOMMU_CACHE_COHERENCY 0x1

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

index ae72ae6..d1598a6 100644
@@ -18,6 +18,7 @@
 
 #include <linux/interrupt.h>
 #include <linux/slab.h>
 #include "irq.h"
 
+#include "msix_mmio.h"
 

static struct kvm_assigned_dev_kernel *kvm_find_assigned_dev(struct list_head *head,

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

 						      int assigned_dev_id)

@@ -191,12 +192,25 @@ static void kvm_free_assigned_irq(struct kvm *kvm,
 
 	kvm_deassign_irq(kvm, assigned_dev, assigned_dev->irq_requested_type);
 
 }
 
+static void assigned_device_free_msix_mmio(struct kvm *kvm,
+				struct kvm_assigned_dev_kernel *adev)
+{
+	struct kvm_msix_mmio mmio;
+
+	mmio.dev_id = adev->assigned_dev_id;
+	mmio.type = KVM_MSIX_MMIO_TYPE_ASSIGNED_DEV |
+		    KVM_MSIX_MMIO_TYPE_BASE_TABLE;
+	kvm_free_msix_mmio(kvm, &mmio);
+}
+
 
 static void kvm_free_assigned_device(struct kvm *kvm,
 
 				     struct kvm_assigned_dev_kernel
 				     *assigned_dev)
 
 {
 
 	kvm_free_assigned_irq(kvm, assigned_dev);
 
+	assigned_device_free_msix_mmio(kvm, assigned_dev);
+
 
 	__pci_reset_function(assigned_dev->dev);
 	pci_restore_state(assigned_dev->dev);

@@ -785,3 +799,33 @@ out:
 	return r;
 
 }
 

virt/kvm/assigned-dev.c out: · line 811

Quoted 10 lines

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

+/* The caller should hold kvm->lock */
+int kvm_assigned_device_update_msix_mask_bit(struct kvm *kvm,
+				int assigned_dev_id, int entry, bool mask)
+{
+	int r = -EFAULT;
+	struct kvm_assigned_dev_kernel *adev;
+	int i;
+
+	if (!irqchip_in_kernel(kvm))
+		return r;

This is wrong, EFAULT should only be returned on
bad/misaligned address.

Can we check this during setup instead?
And then this check can go away or become
BUG_ON, and the function be void.

Quoted 85 lines

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

+
+	adev = kvm_find_assigned_dev(&kvm->arch.assigned_dev_head,
+				      assigned_dev_id);
+	if (!adev)
+		goto out;
+
+	/* For non-MSIX enabled devices, entries_nr == 0 */
+	for (i = 0; i < adev->entries_nr; i++)
+		if (adev->host_msix_entries[i].entry == entry) {
+			if (mask)
+				disable_irq_nosync(
+					adev->host_msix_entries[i].vector);
+			else
+				enable_irq(adev->host_msix_entries[i].vector);
+			r = 0;
+			break;
+		}
+out:
+	return r;
+}

virt/kvm/kvm_main.c +1 -0

index a61f90e..f211e49 100644
@@ -56,6 +56,7 @@
 
 #include "coalesced_mmio.h"
 #include "async_pf.h"
 
+#include "msix_mmio.h"
 
 #define CREATE_TRACE_POINTS

#include <trace/events/kvm.h>

virt/kvm/kvm_main.c +1 -0

@@ -509,6 +510,7 @@ static void kvm_destroy_vm(struct kvm *kvm)
 
 	struct mm_struct *mm = kvm->mm;
 
 	kvm_arch_sync_events(kvm);
 
+	kvm_unregister_msix_mmio_dev(kvm);
 

virt/kvm/kvm_main.c +18 -0

 	spin_lock(&kvm_lock);
 	list_del(&kvm->vm_list);
 	spin_unlock(&kvm_lock);

@@ -1877,6 +1879,24 @@ static long kvm_vm_ioctl(struct file *filp,
 
 		mutex_unlock(&kvm->lock);
 		break;
 
 #endif
 
+	case KVM_REGISTER_MSIX_MMIO: {
+		struct kvm_msix_mmio_user mmio_user;
+
+		r = -EFAULT;
+		if (copy_from_user(&mmio_user, argp, sizeof mmio_user))
+			goto out;
+		r = kvm_vm_ioctl_register_msix_mmio(kvm, &mmio_user);
+		break;
+	}
+	case KVM_UNREGISTER_MSIX_MMIO: {
+		struct kvm_msix_mmio_user mmio_user;
+
+		r = -EFAULT;
+		if (copy_from_user(&mmio_user, argp, sizeof mmio_user))
+			goto out;
+		r = kvm_vm_ioctl_unregister_msix_mmio(kvm, &mmio_user);
+		break;
+	}

virt/kvm/kvm_main.c +0 -0

 	default:
 		r = kvm_arch_vm_ioctl(filp, ioctl, arg);
 		if (r == -ENOTTY)

@@ -1988,6 +2008,12 @@ static int kvm_dev_ioctl_create_vm(void)
 
 		return r;
 
 	}
 
 #endif

virt/kvm/kvm_main.c static int kvm_dev_ioctl_create_vm(void) · line 2016

Quoted 6 lines

virt/kvm/kvm_main.c +6 -0

+	r = kvm_register_msix_mmio_dev(kvm);
+	if (r < 0) {
+		kvm_put_kvm(kvm);
+		return r;
+	}
+

Need to fix error handling below as well?
Better do it with chained gotos if yes.

Quoted 63 lines

virt/kvm/kvm_main.c +8 -4

 	r = anon_inode_getfd("kvm-vm", &kvm_vm_fops, kvm, O_RDWR);
 	if (r < 0)
 
 		kvm_put_kvm(kvm);

@@ -2223,14 +2249,18 @@ static void kvm_io_bus_destroy(struct kvm_io_bus
 *bus)
 
 int kvm_io_bus_write(struct kvm *kvm, enum kvm_bus bus_idx, gpa_t addr,
 
 		     int len, const void *val, struct kvm_io_ext_data *ext_data)
 
 {
 
-	int i;
+	int i, r = -EOPNOTSUPP;
 
 	struct kvm_io_bus *bus;
 
 	bus = srcu_dereference(kvm->buses[bus_idx], &kvm->srcu);
 
-	for (i = 0; i < bus->dev_count; i++)
-		if (!kvm_iodevice_write(bus->devs[i], addr, len, val, ext_data))
+	for (i = 0; i < bus->dev_count; i++) {
+		r = kvm_iodevice_write(bus->devs[i], addr, len, val, ext_data);
+		if (r == -ENOTSYNC)
+			break;
+		else if (!r)
 
 			return 0;
 
-	return -EOPNOTSUPP;
+	}
+	return r;
 
 }
 
 /* kvm_io_bus_read - called under kvm->slots_lock */

virt/kvm/msix_mmio.c new file +17 -0

new file mode 100644
index 0000000..b2a5f86
@@ -0,0 +1,301 @@
+/*
+ * MSI-X MMIO emulation
+ *
+ * Copyright (c) 2010 Intel Corporation
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2.  See
+ * the COPYING file in the top-level directory.
+ *
+ * Author:
+ *   Sheng Yang <[email protected]>
+ */
+
+#include <linux/kvm_host.h>
+#include <linux/kvm.h>
+
+#include "msix_mmio.h"
+#include "iodev.h"

virt/kvm/msix_mmio.c line 22

Quoted 5 lines

virt/kvm/msix_mmio.c new file +2 -0

+
+static int update_msix_mask_bit(struct kvm *kvm, struct kvm_msix_mmio

*mmio, + int entry, u32 flag)

virt/kvm/msix_mmio.c new file +2 -0

+{
+	if (mmio->type & KVM_MSIX_MMIO_TYPE_ASSIGNED_DEV)

Does something else even happen?
If not - BUG_ON.

virt/kvm/msix_mmio.c line 25

Quoted 5 lines

virt/kvm/msix_mmio.c new file +3 -0

+		return kvm_assigned_device_update_msix_mask_bit(kvm,
+				mmio->dev_id, entry, flag);
 
+	return -EFAULT;

EINVAL or something

Quoted 25 lines

virt/kvm/msix_mmio.c new file +23 -0

+}
+
+/* Caller must hold dev->lock */
+static int get_mmio_table_index(struct kvm_msix_mmio_dev *dev,
+				gpa_t addr, int len)
+{
+	gpa_t start, end;
+	int i, r = -EINVAL;
+
+	for (i = 0; i < dev->mmio_nr; i++) {
+		start = dev->mmio[i].table_base_addr;
+		end = dev->mmio[i].table_base_addr + PCI_MSIX_ENTRY_SIZE *
+			dev->mmio[i].max_entries_nr;
+		if (addr >= start && addr + len <= end) {
+			r = i;
+			break;
+		}
+	}
+
+	return r;
+}
+
+static int msix_table_mmio_read(struct kvm_io_device *this, gpa_t addr,

int len, + void *val)

virt/kvm/msix_mmio.c

Quoted 2 lines

virt/kvm/msix_mmio.c new file +2 -0

+{
+	/*TODO: Add big endian support */

likely can be removed.

Quoted 8 lines

virt/kvm/msix_mmio.c new file +8 -0

+	struct kvm_msix_mmio_dev *mmio_dev =
+		container_of(this, struct kvm_msix_mmio_dev, table_dev);
+	struct kvm_msix_mmio *mmio;
+	int idx, ret = 0, entry, offset, r;
+
+	mutex_lock(&mmio_dev->lock);
+	idx = get_mmio_table_index(mmio_dev, addr, len);
+	if (idx < 0) {

virt/kvm/msix_mmio.c line 69

Quoted 10 lines

virt/kvm/msix_mmio.c new file +10 -0

+		ret = -EOPNOTSUPP;
+		goto out;
+	}
+	if ((len != 4 && len != 8) || addr & (len - 1))
+		goto out;
+
+	offset = addr % PCI_MSIX_ENTRY_SIZE;
+	mmio = &mmio_dev->mmio[idx];
+	entry = (addr - mmio->table_base_addr) / PCI_MSIX_ENTRY_SIZE;
+	r = copy_from_user(val, (void __user *)(mmio->table_base_va +

cast above is not needed.

virt/kvm/msix_mmio.c line 82

Quoted 13 lines

virt/kvm/msix_mmio.c new file +13 -0

+			entry * PCI_MSIX_ENTRY_SIZE + offset), len);
+	if (r)
+		goto out;
+out:
+	mutex_unlock(&mmio_dev->lock);
+	return ret;
+}
+
+static int msix_table_mmio_write(struct kvm_io_device *this, gpa_t addr,
+				int len, const void *val,
+				struct kvm_io_ext_data *ext_data)
+{
+	/*TODO: Add big endian support */

comment above can go I think.

Quoted 26 lines

virt/kvm/msix_mmio.c new file +26 -0

+	struct kvm_msix_mmio_dev *mmio_dev =
+		container_of(this, struct kvm_msix_mmio_dev, table_dev);
+	struct kvm_msix_mmio *mmio;
+	int idx, entry, offset, ret = 0, r = 0;
+	void __user *entry_base;
+	__le32 __user *ctrl_pos;
+	__le32 old_ctrl, new_ctrl;
+
+	mutex_lock(&mmio_dev->kvm->lock);
+	mutex_lock(&mmio_dev->lock);
+	idx = get_mmio_table_index(mmio_dev, addr, len);
+	if (idx < 0) {
+		ret = -EOPNOTSUPP;
+		goto out;
+	}
+	if ((len != 4 && len != 8) || addr & (len - 1))
+		goto out;
+
+	offset = addr % PCI_MSIX_ENTRY_SIZE;
+
+	mmio = &mmio_dev->mmio[idx];
+	entry = (addr - mmio->table_base_addr) / PCI_MSIX_ENTRY_SIZE;
+	entry_base = (void __user *)(mmio->table_base_va +
+			entry * PCI_MSIX_ENTRY_SIZE);
+	ctrl_pos = entry_base + PCI_MSIX_ENTRY_VECTOR_CTRL;
+

virt/kvm/msix_mmio.c line 113

Quoted 5 lines

virt/kvm/msix_mmio.c new file +5 -0

+	if (get_user(old_ctrl, ctrl_pos))
+		goto out;
+
+	/* Don't allow writing to other fields when entry is unmasked */
+	if (!(old_ctrl & PCI_MSIX_ENTRY_CTRL_MASKBIT) &&

This should be le32_to_cpu(old_ctrl)
I think sparse shall warn about this.

virt/kvm/msix_mmio.c line 117

Quoted 5 lines

virt/kvm/msix_mmio.c new file +4 -0

+	    offset != PCI_MSIX_ENTRY_VECTOR_CTRL)
+		goto out;
+
+	if (copy_to_user((void __user *)(entry_base + offset), val, len))

A cast above is no longer needed

virt/kvm/msix_mmio.c line 124

Quoted 7 lines

virt/kvm/msix_mmio.c new file +7 -0

+		goto out;
+
+	ext_data->type = KVM_IO_EXT_DATA_TYPE_MSIX_ROUTING;
+	ext_data->msix_routing.dev_id = mmio->dev_id;
+	ext_data->msix_routing.type = mmio->type;
+	ext_data->msix_routing.entry_idx = entry;
+	ext_data->msix_routing.flags = 0;

Is this strictly necessary?
Can we fill the data in vcpu->run->msix_routing directly instead?
Also, on good path we don't need to fill this structure as there's no exit. Let's only do it then?

Quoted 14 lines

virt/kvm/msix_mmio.c new file +13 -0

+
+	if (offset + len < PCI_MSIX_ENTRY_VECTOR_CTRL) {
+		ret = -ENOTSYNC;
+		goto out;
+	}
+
+	if (get_user(new_ctrl, ctrl_pos))
+		goto out;
+
+	if (old_ctrl == new_ctrl) {
+		if (offset == PCI_MSIX_ENTRY_DATA && len == 8)
+			ret = -ENOTSYNC;
+		goto out;

virt/kvm/msix_mmio.c line 142

Quoted 5 lines

virt/kvm/msix_mmio.c new file +5 -0

+	}
+	if ((old_ctrl & PCI_MSIX_ENTRY_CTRL_MASKBIT) ^
+			(new_ctrl & PCI_MSIX_ENTRY_CTRL_MASKBIT))
+		r = update_msix_mask_bit(mmio_dev->kvm, mmio, entry,
+				!!(new_ctrl & PCI_MSIX_ENTRY_CTRL_MASKBIT));

So if update_msix_mask_bit returns EFAULT above this will trigger msix exit? Why?

Quoted 53 lines

virt/kvm/msix_mmio.c new file +52 -0

+	if (r)
+		ret = -ENOTSYNC;
+out:
+	mutex_unlock(&mmio_dev->lock);
+	mutex_unlock(&mmio_dev->kvm->lock);
+	return ret;
+}
+
+static const struct kvm_io_device_ops msix_mmio_table_ops = {
+	.read     = msix_table_mmio_read,
+	.write    = msix_table_mmio_write,
+};
+
+int kvm_register_msix_mmio_dev(struct kvm *kvm)
+{
+	int ret;
+
+	kvm_iodevice_init(&kvm->msix_mmio_dev.table_dev, &msix_mmio_table_ops);
+	mutex_init(&kvm->msix_mmio_dev.lock);
+	kvm->msix_mmio_dev.kvm = kvm;
+	mutex_lock(&kvm->slots_lock);
+	ret = kvm_io_bus_register_dev(kvm, KVM_MMIO_BUS,
+				      &kvm->msix_mmio_dev.table_dev);
+	mutex_unlock(&kvm->slots_lock);
+	return ret;
+}
+
+int kvm_unregister_msix_mmio_dev(struct kvm *kvm)
+{
+	int ret;
+
+	mutex_lock(&kvm->slots_lock);
+	ret = kvm_io_bus_unregister_dev(kvm, KVM_MMIO_BUS,
+				      &kvm->msix_mmio_dev.table_dev);
+	mutex_unlock(&kvm->slots_lock);
+	return ret;
+}
+
+int kvm_vm_ioctl_register_msix_mmio(struct kvm *kvm,
+				    struct kvm_msix_mmio_user *mmio_user)
+{
+	struct kvm_msix_mmio_dev *mmio_dev = &kvm->msix_mmio_dev;
+	struct kvm_msix_mmio *mmio = NULL;
+	int r = 0, i;
+
+	mutex_lock(&mmio_dev->lock);
+	for (i = 0; i < mmio_dev->mmio_nr; i++) {
+		if (mmio_dev->mmio[i].dev_id == mmio_user->dev_id &&
+		    (mmio_dev->mmio[i].type & KVM_MSIX_MMIO_TYPE_DEV_MASK) ==
+		    (mmio_user->type & KVM_MSIX_MMIO_TYPE_DEV_MASK)) {
+			mmio = &mmio_dev->mmio[i];
+			if (mmio->max_entries_nr != mmio_user->max_entries_nr) {

virt/kvm/msix_mmio.c line 200

Quoted 6 lines

virt/kvm/msix_mmio.c new file +6 -0

+				r = -EINVAL;
+				goto out;
+			}
+			break;
+		}
+	}

what does the loop above do? Pls add a comment.

virt/kvm/msix_mmio.c

Quoted 3 lines

virt/kvm/msix_mmio.c new file +2 -0

+	if (mmio_user->max_entries_nr > KVM_MAX_MSIX_PER_DEV) {
+		r = -EINVAL;

-ENOSPC here as well?

Quoted 17 lines

virt/kvm/msix_mmio.c new file +16 -0

+		goto out;
+	}
+	/* All reserved currently */
+	if (mmio_user->flags) {
+		r = -EINVAL;
+		goto out;
+	}
+
+	if ((mmio_user->type & KVM_MSIX_MMIO_TYPE_DEV_MASK) !=
+			KVM_MSIX_MMIO_TYPE_ASSIGNED_DEV) {
+		r = -EINVAL;
+		goto out;
+	}
+	if ((mmio_user->type & KVM_MSIX_MMIO_TYPE_BASE_MASK) !=
+			KVM_MSIX_MMIO_TYPE_BASE_TABLE) {
+		r = -EINVAL;

virt/kvm/msix_mmio.c line 228

Quoted 10 lines

virt/kvm/msix_mmio.c new file +10 -0

+		goto out;
+	}
+
+#ifndef CONFIG_64BIT
+	if (mmio_user->base_va >= 0xffffffff ||
+	    mmio_user->base_addr >= 0xffffffff) {
+		r = -EINVAL;
+		goto out;
+	}
+#endif

A way to check this without ifdef (long is as pointer in length):

	if ((unsigned long)mmio_user->base_va !=
		mmio_user->base_va)

and for base_va it should be EFAULT I think as with any illegal virtual address.

virt/kvm/msix_mmio.c line 233

Quoted 6 lines

virt/kvm/msix_mmio.c new file +5 -0

+
+	/* Check alignment and accessibility */
+	if ((mmio_user->base_va % PCI_MSIX_ENTRY_SIZE) ||
+	    !access_ok(VERIFY_WRITE, (void __user *)mmio_user->base_va,
+			mmio_user->max_entries_nr * PCI_MSIX_ENTRY_SIZE)) {

One thing to watch for here is wrap around. AFAIK access_ok does not check for it, you must do it yourself.

virt/kvm/msix_mmio.c

Quoted 2 lines

virt/kvm/msix_mmio.c new file +1 -0

+		r = -EINVAL;

EFAULT is usually better for an access error.

Quoted 25 lines

virt/kvm/msix_mmio.c new file +24 -0

+		goto out;
+	}
+	if (!mmio) {
+		if (mmio_dev->mmio_nr == KVM_MSIX_MMIO_MAX) {
+			r = -ENOSPC;
+			goto out;
+		}
+		mmio = &mmio_dev->mmio[mmio_dev->mmio_nr];
+		mmio_dev->mmio_nr++;
+	}
+
+	mmio->max_entries_nr = mmio_user->max_entries_nr;
+	mmio->dev_id = mmio_user->dev_id;
+	mmio->flags = mmio_user->flags;
+
+	if ((mmio_user->type & KVM_MSIX_MMIO_TYPE_DEV_MASK) ==
+			KVM_MSIX_MMIO_TYPE_ASSIGNED_DEV)
+		mmio->type = KVM_MSIX_MMIO_TYPE_ASSIGNED_DEV;
+	if ((mmio_user->type & KVM_MSIX_MMIO_TYPE_BASE_MASK) ==
+			KVM_MSIX_MMIO_TYPE_BASE_TABLE) {
+		mmio->type |= KVM_MSIX_MMIO_TYPE_BASE_TABLE;
+		mmio->table_base_addr = mmio_user->base_addr;
+		mmio->table_base_va = mmio_user->base_va;
+	}

virt/kvm/msix_mmio.c line 264

Quoted 6 lines

virt/kvm/msix_mmio.c new file +6 -0

+out:
+	mutex_unlock(&mmio_dev->lock);
+	return r;
+}
+
+int kvm_free_msix_mmio(struct kvm *kvm, struct kvm_msix_mmio *mmio)

Pls add a comment documenting what this does.

Quoted 31 lines

virt/kvm/msix_mmio.c new file +30 -0

+{
+	struct kvm_msix_mmio_dev *mmio_dev = &kvm->msix_mmio_dev;
+	int r = -EINVAL, i, j;
+
+	if (!mmio)
+		return 0;
+
+	mutex_lock(&mmio_dev->lock);
+	BUG_ON(mmio_dev->mmio_nr > KVM_MSIX_MMIO_MAX);
+	for (i = 0; i < mmio_dev->mmio_nr; i++) {
+		if (mmio_dev->mmio[i].dev_id == mmio->dev_id &&
+		    mmio_dev->mmio[i].type == mmio->type) {
+			r = 0;
+			for (j = i; j < mmio_dev->mmio_nr - 1; j++)
+				mmio_dev->mmio[j] = mmio_dev->mmio[j + 1];
+			mmio_dev->mmio[mmio_dev->mmio_nr].max_entries_nr = 0;
+			mmio_dev->mmio[mmio_dev->mmio_nr].dev_id = 0;
+			mmio_dev->mmio[mmio_dev->mmio_nr].type = 0;
+			mmio_dev->mmio_nr--;
+			break;
+		}
+	}
+	mutex_unlock(&mmio_dev->lock);
+	return r;
+}
+
+int kvm_vm_ioctl_unregister_msix_mmio(struct kvm *kvm,
+				      struct kvm_msix_mmio_user *mmio_user)
+{
+	struct kvm_msix_mmio mmio;

virt/kvm/msix_mmio.c line 298

Quoted 4 lines

virt/kvm/msix_mmio.c new file +4 -0

+
+	mmio.dev_id = mmio_user->dev_id;
+	mmio.type = mmio_user->type;
+

So you pass in kvm_msix_mmio but only 2 fields are valid? This is very confusing. Please just pass the two values as function params to kvm_free_msix_mmio.

Quoted 21 lines

virt/kvm/msix_mmio.c new file +3 -0

+	return kvm_free_msix_mmio(kvm, &mmio);
+}
+

virt/kvm/msix_mmio.h new file +11 -0

new file mode 100644
index 0000000..01b6587
@@ -0,0 +1,25 @@
+#ifndef __KVM_MSIX_MMIO_H__
+#define __KVM_MSIX_MMIO_H__
+/*
+ * MSI-X MMIO emulation
+ *
+ * Copyright (c) 2010 Intel Corporation
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2.  See
+ * the COPYING file in the top-level directory.
+ *
+ * Author:

virt/kvm/msix_mmio.h line 15

Quoted 4 lines

virt/kvm/msix_mmio.h new file +4 -0

+ *   Sheng Yang <[email protected]>
+ */
+
+#include <linux/pci.h>

I don't see where you use this include.
OTOH you do need to declare struct kvm_msix_mmio_user and struct kvm here.

Quoted 11 lines

virt/kvm/msix_mmio.h new file +8 -0

+
+int kvm_register_msix_mmio_dev(struct kvm *kvm);
+int kvm_unregister_msix_mmio_dev(struct kvm *kvm);
+int kvm_vm_ioctl_register_msix_mmio(struct kvm *kvm,
+				    struct kvm_msix_mmio_user *mmio_user);
+int kvm_vm_ioctl_unregister_msix_mmio(struct kvm *kvm,
+				      struct kvm_msix_mmio_user *mmio_user);
+int kvm_free_msix_mmio(struct kvm *kvm, struct kvm_msix_mmio

*mmio_user); +
+#endif

Quoted 11 lines · On Monday 28 February 2011 19:27:29 Michael S. Tsirkin wrote:

Quoted 3 lines · Sheng Yang

Then we can support mask bit operation of assigned devices now.

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

A general question: we implement mmio read and write operation here, but seem to do nothing
about ordering. In particular, pci mmio reads are normally assumed to flush all writes out to the device
and all device writes in to the CPU.

Quoted 1 line

How exactly does it work here?

I don't understand your problem... We emulate all operation here, where is "ordering" issue?

And Michael, thanks for you detail comments, but could you give comments all at once? Now I've tried my best to address comments, but still feeling endless comments are coming.

And I would leave Intel this Friday, I want to get it done before I leave.

Quoted 114 lines

Quoted 114 lines

---

 arch/x86/include/asm/kvm_host.h |    1 +
 arch/x86/kvm/Makefile           |    2 +-
 arch/x86/kvm/mmu.c              |    2 +
 arch/x86/kvm/x86.c              |   40 ++++-
 include/linux/kvm.h             |   28 ++++
 include/linux/kvm_host.h        |   36 +++++
 virt/kvm/assigned-dev.c         |   44 ++++++
 virt/kvm/kvm_main.c             |   38 +++++-
 virt/kvm/msix_mmio.c            |  301
 +++++++++++++++++++++++++++++++++++++++ virt/kvm/msix_mmio.h           
 |   25 ++++
 10 files changed, 504 insertions(+), 13 deletions(-)
 create mode 100644 virt/kvm/msix_mmio.c
 create mode 100644 virt/kvm/msix_mmio.h

(quoted diff) +0 -0

b/arch/x86/include/asm/kvm_host.h index aa75f21..4a390a4 100644

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

@@ -635,6 +635,7 @@ enum emulation_result {
 
 	EMULATE_DONE,       /* no further processing */
 	EMULATE_DO_MMIO,      /* kvm_run filled with mmio request */
 	EMULATE_FAIL,         /* can't emulate this instruction */
 
+	EMULATE_USERSPACE_EXIT, /* we need exit to userspace */
 

};

#define EMULTYPE_NO_DECODE (1 << 0)

arch/x86/kvm/Makefile +1 -1

index f15501f..3a0d851 100644
@@ -7,7 +7,7 @@ CFLAGS_vmx.o := -I.
 
 kvm-y			+= $(addprefix ../../../virt/kvm/, kvm_main.o ioapic.o \
 
 				coalesced_mmio.o irq_comm.o eventfd.o \
 
-				assigned-dev.o)
+				assigned-dev.o msix_mmio.o)
 

kvm-$(CONFIG_IOMMU_API) += $(addprefix ../../../virt/kvm/, iommu.o)
kvm-$(CONFIG_KVM_ASYNC_PF) += $(addprefix ../../../virt/kvm/,
async_pf.o)

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

index 9cafbb4..912dca4 100644
@@ -3358,6 +3358,8 @@ int kvm_mmu_page_fault(struct kvm_vcpu *vcpu, gva_t
 cr2, u32 error_code,
 
 	case EMULATE_DO_MMIO:
 		++vcpu->stat.mmio_exits;
 		/* fall through */
 
+	case EMULATE_USERSPACE_EXIT:
+		/* fall through */

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

 	case EMULATE_FAIL:
 		return 0;
 
 	default:

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

index 21b84e2..87308eb 100644
@@ -1966,6 +1966,7 @@ int kvm_dev_ioctl_check_extension(long ext)
 
 	case KVM_CAP_X86_ROBUST_SINGLESTEP:
 	case KVM_CAP_XSAVE:
 
 	case KVM_CAP_ASYNC_PF:
+	case KVM_CAP_MSIX_MMIO:
 		r = 1;

arch/x86/kvm/x86.c +3 -1

 		break;
 
 	case KVM_CAP_COALESCED_MMIO:

@@ -3809,6 +3810,7 @@ static int emulator_write_emulated_onepage(unsigned
 long addr,
 
 {
 
 	gpa_t                 gpa;
 	struct kvm_io_ext_data ext_data;
 
+	int r;
 
 	gpa = kvm_mmu_gva_to_gpa_write(vcpu, addr, exception);

@@ -3824,18 +3826,32 @@ static int
 emulator_write_emulated_onepage(unsigned long addr,
 
 mmio:
 	trace_kvm_mmio(KVM_TRACE_MMIO_WRITE, bytes, gpa, *(u64 *)val);
 
+	r = vcpu_mmio_write(vcpu, gpa, bytes, val, &ext_data);
 
 	/*
 
 	 * Is this MMIO handled locally?
 	 */
 
-	if (!vcpu_mmio_write(vcpu, gpa, bytes, val, &ext_data))
+	if (!r)
 
 		return X86EMUL_CONTINUE;
 

Quoted 10 lines

arch/x86/kvm/x86.c static int emulator_write_emulated_onepage(unsigned long addr, · line 3836

Quoted 7 lines

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

-	vcpu->mmio_needed = 1;
-	vcpu->run->exit_reason = KVM_EXIT_MMIO;
-	vcpu->run->mmio.phys_addr = vcpu->mmio_phys_addr = gpa;
-	vcpu->run->mmio.len = vcpu->mmio_size = bytes;
-	vcpu->run->mmio.is_write = vcpu->mmio_is_write = 1;
-	memcpy(vcpu->run->mmio.data, val, bytes);
+	if (r == -ENOTSYNC) {

Can you replace -ENOTSYNC with KVM_EXIT_MSIX_ROUTING_UPDATE all over please?

How about let Avi/Marcelo decide?

Quoted 165 lines

Quoted 165 lines

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

+		vcpu->userspace_exit_needed = 1;
+		vcpu->run->exit_reason = KVM_EXIT_MSIX_ROUTING_UPDATE;
+		vcpu->run->msix_routing.dev_id =
+			ext_data.msix_routing.dev_id;
+		vcpu->run->msix_routing.type =
+			ext_data.msix_routing.type;
+		vcpu->run->msix_routing.entry_idx =
+			ext_data.msix_routing.entry_idx;
+		vcpu->run->msix_routing.flags =
+			ext_data.msix_routing.flags;
+	} else  {
+		vcpu->mmio_needed = 1;
+		vcpu->run->exit_reason = KVM_EXIT_MMIO;
+		vcpu->run->mmio.phys_addr = vcpu->mmio_phys_addr = gpa;
+		vcpu->run->mmio.len = vcpu->mmio_size = bytes;
+		vcpu->run->mmio.is_write = vcpu->mmio_is_write = 1;
+		memcpy(vcpu->run->mmio.data, val, bytes);
+	}
 
 	return X86EMUL_CONTINUE;
 
 }

@@ -4469,6 +4485,8 @@ done:
 		r = EMULATE_DO_MMIO;
 
 	} else if (r == EMULATION_RESTART)
 
 		goto restart;
 
+	else if (vcpu->userspace_exit_needed)
+		r = EMULATE_USERSPACE_EXIT;

arch/x86/kvm/x86.c +7 -1

 	else
 
 		r = EMULATE_DONE;

@@ -5397,12 +5415,18 @@ int kvm_arch_vcpu_ioctl_run(struct kvm_vcpu
 *vcpu, struct kvm_run *kvm_run)
 
 		}
 
 	}
 
-	if (vcpu->arch.pio.count || vcpu->mmio_needed) {
+	if (vcpu->arch.pio.count || vcpu->mmio_needed ||
+			vcpu->userspace_exit_needed) {
 
 		if (vcpu->mmio_needed) {
 
 			memcpy(vcpu->mmio_data, kvm_run->mmio.data, 8);
 			vcpu->mmio_read_completed = 1;
 			vcpu->mmio_needed = 0;
 
 		}
 
+		if (vcpu->userspace_exit_needed) {
+			vcpu->userspace_exit_needed = 0;
+			r = 0;
+			goto out;
+		}
 
 		vcpu->srcu_idx = srcu_read_lock(&vcpu->kvm->srcu);
 		r = emulate_instruction(vcpu, EMULTYPE_NO_DECODE);
 		srcu_read_unlock(&vcpu->kvm->srcu, vcpu->srcu_idx);

include/linux/kvm.h +1 -0

index ea2dc1a..4393e4e 100644
@@ -161,6 +161,7 @@ struct kvm_pit_config {
 
 #define KVM_EXIT_NMI              16
 #define KVM_EXIT_INTERNAL_ERROR   17
 #define KVM_EXIT_OSI              18
 
+#define KVM_EXIT_MSIX_ROUTING_UPDATE 19
 

/* For KVM_EXIT_INTERNAL_ERROR */
#define KVM_INTERNAL_ERROR_EMULATION 1

include/linux/kvm.h +8 -0

@@ -264,6 +265,13 @@ struct kvm_run {
 
 		struct {
 
 			__u64 gprs[32];
 
 		} osi;
 
+		/* KVM_EXIT_MSIX_ROUTING_UPDATE*/
+		struct {
+			__u32 dev_id;
+			__u16 type;
+			__u16 entry_idx;
+			__u64 flags;
+		} msix_routing;
 
 		/* Fix the size of the union. */
 		char padding[256];
 
 	};

@@ -541,6 +549,7 @@ struct kvm_ppc_pvinfo {
 
 #define KVM_CAP_PPC_GET_PVINFO 57
 #define KVM_CAP_PPC_IRQ_LEVEL 58
 #define KVM_CAP_ASYNC_PF 59
 
+#define KVM_CAP_MSIX_MMIO 60
 

#ifdef KVM_CAP_IRQ_ROUTING

include/linux/kvm.h +2 -0

@@ -672,6 +681,9 @@ 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)
 
+/* Available with KVM_CAP_MSIX_MMIO */
+#define KVM_REGISTER_MSIX_MMIO    _IOW(KVMIO,  0x7d, struct

kvm_msix_mmio_user) +#define KVM_UNREGISTER_MSIX_MMIO _IOW(KVMIO, 0x7e, struct kvm_msix_mmio_user)

/* 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)

include/linux/kvm.h +16 -0

@@ -795,4 +807,20 @@ struct kvm_assigned_msix_entry {
 
 	__u16 padding[3];
 
 };
 
+#define KVM_MSIX_MMIO_TYPE_ASSIGNED_DEV	    (1 << 0)
+
+#define KVM_MSIX_MMIO_TYPE_BASE_TABLE	    (1 << 8)
+
+#define KVM_MSIX_MMIO_TYPE_DEV_MASK	    0x00ff
+#define KVM_MSIX_MMIO_TYPE_BASE_MASK	    0xff00
+struct kvm_msix_mmio_user {
+	__u32 dev_id;
+	__u16 type;
+	__u16 max_entries_nr;
+	__u64 base_addr;
+	__u64 base_va;
+	__u64 flags;
+	__u64 reserved[4];
+};
+
 
 #endif /* __LINUX_KVM_H */

include/linux/kvm_host.h +0 -0

index a32c53e..d6c05e3 100644
@@ -68,8 +68,17 @@ enum kvm_bus {
 
 	KVM_NR_BUSES
 
 };
 

Quoted 8 lines

include/linux/kvm_host.h enum kvm_bus {

Quoted 5 lines

include/linux/kvm_host.h +1 -0

+#define KVM_IO_EXT_DATA_TYPE_MSIX_ROUTING   1
 
 struct kvm_io_ext_data {
 
 	int type;   /* Extended data type */

type seems unused for now. Do we need it?
I'd guess exit type should usually be sufficient.

I want to get this structure self-explained.

Quoted 10 lines

include/linux/kvm_host.h enum kvm_bus { · line 80

Quoted 8 lines

include/linux/kvm_host.h +7 -0

+	union {
+		struct {
+			u32 dev_id;	/* Device ID */
+			u16 type;	/* Device type */
+			u16 entry_idx;  /* Accessed MSI-X entry index */
+			u64 flags;
+		} msix_routing;

So this is only for when we exit on msix routing?

Yes for now.

Quoted 26 lines

Quoted 26 lines

include/linux/kvm_host.h +3 -0

+	};
 
 };
 
 int kvm_io_bus_write(struct kvm *kvm, enum kvm_bus bus_idx, gpa_t addr,

@@ -165,6 +174,8 @@ struct kvm_vcpu {
 
 	} async_pf;
 
 #endif
 
+	int userspace_exit_needed;
+
 

include/linux/kvm_host.h +0 -0

 	struct kvm_vcpu_arch arch;
 
 };

@@ -238,6 +249,27 @@ struct kvm_memslots {
 
 					KVM_PRIVATE_MEM_SLOTS];
 
 };
 

Quoted 11 lines

include/linux/kvm_host.h struct kvm_memslots { · line 260

Quoted 9 lines

include/linux/kvm_host.h +9 -0

+#define KVM_MSIX_MMIO_MAX    32
+
+struct kvm_msix_mmio {
+	u32 dev_id;
+	u16 type;
+	u16 max_entries_nr;
+	u64 flags;
+	gpa_t table_base_addr;
+	hva_t table_base_va;

void __user* would make for less casting in code.

OK.

Quoted 12 lines

Or even

	struct kvm_msix_entry {
		__le32 addr_lo;
		__le32 addr_hi;
		__le32 data;
		__le32 ctrl;
	};

	struct kvm_msix_entry __user *table;

Quoted 2 lines

and then we can do table[entry].ctrl
and so on.

It's a userspace address, I think use it in this way maybe misleading. Personally I want to keep the current way.

Quoted 114 lines

Quoted 100 lines

include/linux/kvm_host.h +12 -0

+	gpa_t pba_base_addr;
+	hva_t pba_base_va;
+};
+
+struct kvm_msix_mmio_dev {
+	struct kvm *kvm;
+	struct kvm_io_device table_dev;
+	int mmio_nr;
+	struct kvm_msix_mmio mmio[KVM_MSIX_MMIO_MAX];
+	struct mutex lock;
+};
+
 

struct kvm {

include/linux/kvm_host.h +4 -0

 	spinlock_t mmu_lock;
 	raw_spinlock_t requests_lock;

@@ -286,6 +318,7 @@ struct kvm {
 
 	long mmu_notifier_count;
 
 #endif
 
 	long tlbs_dirty;
 
+	struct kvm_msix_mmio_dev msix_mmio_dev;
 
 };
 
 /* The guest did something we don't support. */

@@ -558,6 +591,9 @@ void kvm_unregister_irq_ack_notifier(struct kvm *kvm,
 
 int kvm_request_irq_source_id(struct kvm *kvm);
 void kvm_free_irq_source_id(struct kvm *kvm, int irq_source_id);
 
+int kvm_assigned_device_update_msix_mask_bit(struct kvm *kvm,
+			int assigned_dev_id, int entry, bool mask);
+
 
 /* For vcpu->arch.iommu_flags */

#define KVM_IOMMU_CACHE_COHERENCY 0x1

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

index ae72ae6..d1598a6 100644
@@ -18,6 +18,7 @@
 
 #include <linux/interrupt.h>
 #include <linux/slab.h>
 #include "irq.h"
 
+#include "msix_mmio.h"
 

static struct kvm_assigned_dev_kernel *kvm_find_assigned_dev(struct list_head *head,

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

 						      int assigned_dev_id)

@@ -191,12 +192,25 @@ static void kvm_free_assigned_irq(struct kvm *kvm,
 
 	kvm_deassign_irq(kvm, assigned_dev, assigned_dev->irq_requested_type);
 
 }
 
+static void assigned_device_free_msix_mmio(struct kvm *kvm,
+				struct kvm_assigned_dev_kernel *adev)
+{
+	struct kvm_msix_mmio mmio;
+
+	mmio.dev_id = adev->assigned_dev_id;
+	mmio.type = KVM_MSIX_MMIO_TYPE_ASSIGNED_DEV |
+		    KVM_MSIX_MMIO_TYPE_BASE_TABLE;
+	kvm_free_msix_mmio(kvm, &mmio);
+}
+
 
 static void kvm_free_assigned_device(struct kvm *kvm,
 
 				     struct kvm_assigned_dev_kernel
 				     *assigned_dev)
 
 {
 
 	kvm_free_assigned_irq(kvm, assigned_dev);
 
+	assigned_device_free_msix_mmio(kvm, assigned_dev);
+
 
 	__pci_reset_function(assigned_dev->dev);
 	pci_restore_state(assigned_dev->dev);

@@ -785,3 +799,33 @@ out:
 	return r;
 
 }
 

virt/kvm/assigned-dev.c out: · line 811

Quoted 10 lines

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

+/* The caller should hold kvm->lock */
+int kvm_assigned_device_update_msix_mask_bit(struct kvm *kvm,
+				int assigned_dev_id, int entry, bool mask)
+{
+	int r = -EFAULT;
+	struct kvm_assigned_dev_kernel *adev;
+	int i;
+
+	if (!irqchip_in_kernel(kvm))
+		return r;

This is wrong, EFAULT should only be returned on
bad/misaligned address.

Quoted 3 lines

Can we check this during setup instead?
And then this check can go away or become
BUG_ON, and the function be void.

I think BUG_ON is OK.

Quoted 85 lines

Quoted 85 lines

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

+
+	adev = kvm_find_assigned_dev(&kvm->arch.assigned_dev_head,
+				      assigned_dev_id);
+	if (!adev)
+		goto out;
+
+	/* For non-MSIX enabled devices, entries_nr == 0 */
+	for (i = 0; i < adev->entries_nr; i++)
+		if (adev->host_msix_entries[i].entry == entry) {
+			if (mask)
+				disable_irq_nosync(
+					adev->host_msix_entries[i].vector);
+			else
+				enable_irq(adev->host_msix_entries[i].vector);
+			r = 0;
+			break;
+		}
+out:
+	return r;
+}

virt/kvm/kvm_main.c +1 -0

index a61f90e..f211e49 100644
@@ -56,6 +56,7 @@
 
 #include "coalesced_mmio.h"
 #include "async_pf.h"
 
+#include "msix_mmio.h"
 
 #define CREATE_TRACE_POINTS

#include <trace/events/kvm.h>

virt/kvm/kvm_main.c +1 -0

@@ -509,6 +510,7 @@ static void kvm_destroy_vm(struct kvm *kvm)
 
 	struct mm_struct *mm = kvm->mm;
 
 	kvm_arch_sync_events(kvm);
 
+	kvm_unregister_msix_mmio_dev(kvm);
 

virt/kvm/kvm_main.c +18 -0

 	spin_lock(&kvm_lock);
 	list_del(&kvm->vm_list);
 	spin_unlock(&kvm_lock);

@@ -1877,6 +1879,24 @@ static long kvm_vm_ioctl(struct file *filp,
 
 		mutex_unlock(&kvm->lock);
 		break;
 
 #endif
 
+	case KVM_REGISTER_MSIX_MMIO: {
+		struct kvm_msix_mmio_user mmio_user;
+
+		r = -EFAULT;
+		if (copy_from_user(&mmio_user, argp, sizeof mmio_user))
+			goto out;
+		r = kvm_vm_ioctl_register_msix_mmio(kvm, &mmio_user);
+		break;
+	}
+	case KVM_UNREGISTER_MSIX_MMIO: {
+		struct kvm_msix_mmio_user mmio_user;
+
+		r = -EFAULT;
+		if (copy_from_user(&mmio_user, argp, sizeof mmio_user))
+			goto out;
+		r = kvm_vm_ioctl_unregister_msix_mmio(kvm, &mmio_user);
+		break;
+	}

virt/kvm/kvm_main.c +0 -0

 	default:
 		r = kvm_arch_vm_ioctl(filp, ioctl, arg);
 		if (r == -ENOTTY)

@@ -1988,6 +2008,12 @@ static int kvm_dev_ioctl_create_vm(void)
 
 		return r;
 
 	}
 
 #endif

Quoted 9 lines

virt/kvm/kvm_main.c static int kvm_dev_ioctl_create_vm(void) · line 2016

Quoted 6 lines

virt/kvm/kvm_main.c +6 -0

+	r = kvm_register_msix_mmio_dev(kvm);
+	if (r < 0) {
+		kvm_put_kvm(kvm);
+		return r;
+	}
+

Need to fix error handling below as well?
Better do it with chained gotos if yes.

Let's make it another separate patch.

Quoted 63 lines

Quoted 63 lines

virt/kvm/kvm_main.c +8 -4

 	r = anon_inode_getfd("kvm-vm", &kvm_vm_fops, kvm, O_RDWR);
 	if (r < 0)
 
 		kvm_put_kvm(kvm);

@@ -2223,14 +2249,18 @@ static void kvm_io_bus_destroy(struct kvm_io_bus
 *bus)
 
 int kvm_io_bus_write(struct kvm *kvm, enum kvm_bus bus_idx, gpa_t addr,
 
 		     int len, const void *val, struct kvm_io_ext_data *ext_data)
 
 {
 
-	int i;
+	int i, r = -EOPNOTSUPP;
 
 	struct kvm_io_bus *bus;
 
 	bus = srcu_dereference(kvm->buses[bus_idx], &kvm->srcu);
 
-	for (i = 0; i < bus->dev_count; i++)
-		if (!kvm_iodevice_write(bus->devs[i], addr, len, val, ext_data))
+	for (i = 0; i < bus->dev_count; i++) {
+		r = kvm_iodevice_write(bus->devs[i], addr, len, val, ext_data);
+		if (r == -ENOTSYNC)
+			break;
+		else if (!r)
 
 			return 0;
 
-	return -EOPNOTSUPP;
+	}
+	return r;
 
 }
 
 /* kvm_io_bus_read - called under kvm->slots_lock */

virt/kvm/msix_mmio.c new file +17 -0

new file mode 100644
index 0000000..b2a5f86
@@ -0,0 +1,301 @@
+/*
+ * MSI-X MMIO emulation
+ *
+ * Copyright (c) 2010 Intel Corporation
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2.  See
+ * the COPYING file in the top-level directory.
+ *
+ * Author:
+ *   Sheng Yang <[email protected]>
+ */
+
+#include <linux/kvm_host.h>
+#include <linux/kvm.h>
+
+#include "msix_mmio.h"
+#include "iodev.h"

Quoted 8 lines

virt/kvm/msix_mmio.c line 22

Quoted 5 lines

virt/kvm/msix_mmio.c new file +2 -0

+
+static int update_msix_mask_bit(struct kvm *kvm, struct kvm_msix_mmio

*mmio, + int entry, u32 flag)

virt/kvm/msix_mmio.c new file +2 -0

+{
+	if (mmio->type & KVM_MSIX_MMIO_TYPE_ASSIGNED_DEV)

Does something else even happen?
If not - BUG_ON.

Not for now, the next would be your vfio? Would use BUG_ON for now.

Quoted 7 lines

virt/kvm/msix_mmio.c line 25

Quoted 5 lines

virt/kvm/msix_mmio.c new file +3 -0

+		return kvm_assigned_device_update_msix_mask_bit(kvm,
+				mmio->dev_id, entry, flag);
 
+	return -EFAULT;

EINVAL or something

OK

Quoted 93 lines

Quoted 25 lines

virt/kvm/msix_mmio.c new file +23 -0

+}
+
+/* Caller must hold dev->lock */
+static int get_mmio_table_index(struct kvm_msix_mmio_dev *dev,
+				gpa_t addr, int len)
+{
+	gpa_t start, end;
+	int i, r = -EINVAL;
+
+	for (i = 0; i < dev->mmio_nr; i++) {
+		start = dev->mmio[i].table_base_addr;
+		end = dev->mmio[i].table_base_addr + PCI_MSIX_ENTRY_SIZE *
+			dev->mmio[i].max_entries_nr;
+		if (addr >= start && addr + len <= end) {
+			r = i;
+			break;
+		}
+	}
+
+	return r;
+}
+
+static int msix_table_mmio_read(struct kvm_io_device *this, gpa_t addr,

int len, + void *val)

virt/kvm/msix_mmio.c

Quoted 2 lines

virt/kvm/msix_mmio.c new file +2 -0

+{
+	/*TODO: Add big endian support */

likely can be removed.

Quoted 8 lines

virt/kvm/msix_mmio.c new file +8 -0

+	struct kvm_msix_mmio_dev *mmio_dev =
+		container_of(this, struct kvm_msix_mmio_dev, table_dev);
+	struct kvm_msix_mmio *mmio;
+	int idx, ret = 0, entry, offset, r;
+
+	mutex_lock(&mmio_dev->lock);
+	idx = get_mmio_table_index(mmio_dev, addr, len);
+	if (idx < 0) {

virt/kvm/msix_mmio.c line 69

Quoted 10 lines

virt/kvm/msix_mmio.c new file +10 -0

+		ret = -EOPNOTSUPP;
+		goto out;
+	}
+	if ((len != 4 && len != 8) || addr & (len - 1))
+		goto out;
+
+	offset = addr % PCI_MSIX_ENTRY_SIZE;
+	mmio = &mmio_dev->mmio[idx];
+	entry = (addr - mmio->table_base_addr) / PCI_MSIX_ENTRY_SIZE;
+	r = copy_from_user(val, (void __user *)(mmio->table_base_va +

cast above is not needed.

virt/kvm/msix_mmio.c line 82

Quoted 13 lines

virt/kvm/msix_mmio.c new file +13 -0

+			entry * PCI_MSIX_ENTRY_SIZE + offset), len);
+	if (r)
+		goto out;
+out:
+	mutex_unlock(&mmio_dev->lock);
+	return ret;
+}
+
+static int msix_table_mmio_write(struct kvm_io_device *this, gpa_t addr,
+				int len, const void *val,
+				struct kvm_io_ext_data *ext_data)
+{
+	/*TODO: Add big endian support */

comment above can go I think.

Quoted 26 lines

virt/kvm/msix_mmio.c new file +26 -0

+	struct kvm_msix_mmio_dev *mmio_dev =
+		container_of(this, struct kvm_msix_mmio_dev, table_dev);
+	struct kvm_msix_mmio *mmio;
+	int idx, entry, offset, ret = 0, r = 0;
+	void __user *entry_base;
+	__le32 __user *ctrl_pos;
+	__le32 old_ctrl, new_ctrl;
+
+	mutex_lock(&mmio_dev->kvm->lock);
+	mutex_lock(&mmio_dev->lock);
+	idx = get_mmio_table_index(mmio_dev, addr, len);
+	if (idx < 0) {
+		ret = -EOPNOTSUPP;
+		goto out;
+	}
+	if ((len != 4 && len != 8) || addr & (len - 1))
+		goto out;
+
+	offset = addr % PCI_MSIX_ENTRY_SIZE;
+
+	mmio = &mmio_dev->mmio[idx];
+	entry = (addr - mmio->table_base_addr) / PCI_MSIX_ENTRY_SIZE;
+	entry_base = (void __user *)(mmio->table_base_va +
+			entry * PCI_MSIX_ENTRY_SIZE);
+	ctrl_pos = entry_base + PCI_MSIX_ENTRY_VECTOR_CTRL;
+

Quoted 8 lines

virt/kvm/msix_mmio.c line 113

Quoted 5 lines

virt/kvm/msix_mmio.c new file +5 -0

+	if (get_user(old_ctrl, ctrl_pos))
+		goto out;
+
+	/* Don't allow writing to other fields when entry is unmasked */
+	if (!(old_ctrl & PCI_MSIX_ENTRY_CTRL_MASKBIT) &&

This should be le32_to_cpu(old_ctrl)
I think sparse shall warn about this.

Sadly it didn't.

Quoted 8 lines

virt/kvm/msix_mmio.c line 117

Quoted 5 lines

virt/kvm/msix_mmio.c new file +4 -0

+	    offset != PCI_MSIX_ENTRY_VECTOR_CTRL)
+		goto out;
+
+	if (copy_to_user((void __user *)(entry_base + offset), val, len))

A cast above is no longer needed

Quoted 13 lines

virt/kvm/msix_mmio.c line 124

Quoted 7 lines

virt/kvm/msix_mmio.c new file +7 -0

+		goto out;
+
+	ext_data->type = KVM_IO_EXT_DATA_TYPE_MSIX_ROUTING;
+	ext_data->msix_routing.dev_id = mmio->dev_id;
+	ext_data->msix_routing.type = mmio->type;
+	ext_data->msix_routing.entry_idx = entry;
+	ext_data->msix_routing.flags = 0;

Is this strictly necessary?
Can we fill the data in vcpu->run->msix_routing directly instead?
Also, on good path we don't need to fill this structure as there's no exit. Let's only do it then?