[PATCH V6 0/6] Fast mmio eventfd fixes

[PATCH V6 0/6] Fast mmio eventfd fixes

[Jason Wang]

Hi:

This series fixes two issues of fast mmio eventfd:

1) A single iodev instance were registerd on two buses: KVM_MMIO_BUS
   and KVM_FAST_MMIO_BUS. This will cause double in
   ioeventfd_destructor()
2) A zero length iodev on KVM_MMIO_BUS will never be found but
   kvm_io_bus_cmp(). This will lead e.g the eventfd will be trapped by
   qemu instead of host.

1 is fixed by allocating two instances of iodev and introduce a new
capability for userspace. 2 is fixed by ignore the actual length if
the length of iodev is zero in kvm_io_bus_cmp().

Please review.

Changes from V5:
- move patch of explicitly checking for KVM_MMIO_BUS to patch 1 and
  remove the unnecessary checks
- even more grammar and typo fixes
- rabase to kvm.git
- document KVM_CAP_FAST_MMIO

Changes from V4:
- move the location of kvm_assign_ioeventfd() in patch 1 which reduce
  the change set.
- commit log typo fixes
- switch to use kvm_deassign_ioeventfd_id) when fail to register to
  fast mmio bus
- change kvm_io_bus_cmp() as Paolo's suggestions
- introduce a new capability to avoid new userspace crash old kernel
- add a new patch that only try to register mmio eventfd on fast mmio
  bus

Changes from V3:

- Don't do search on two buses when trying to do write on
  KVM_MMIO_BUS. This fixes a small regression found by vmexit.flat.
- Since we don't do search on two buses, change kvm_io_bus_cmp() to
  let it can find zero length iodevs.
- Fix the unnecessary lines in tracepoint patch.

Changes from V2:
- Tweak styles and comment suggested by Cornelia.

Changes from v1:
- change ioeventfd_bus_from_flags() to return KVM_FAST_MMIO_BUS when
  needed to save lots of unnecessary changes.

Jason Wang (6):
  kvm: don't try to register to KVM_FAST_MMIO_BUS for non mmio eventfd
  kvm: factor out core eventfd assign/deassign logic
  kvm: fix double free for fast mmio eventfd
  kvm: fix zero length mmio searching
  kvm: add tracepoint for fast mmio
  kvm: add fast mmio capabilitiy

 Documentation/virtual/kvm/api.txt |   7 ++-
 arch/x86/kvm/trace.h              |  18 ++++++
 arch/x86/kvm/vmx.c                |   1 +
 arch/x86/kvm/x86.c                |   1 +
 include/uapi/linux/kvm.h          |   1 +
 virt/kvm/eventfd.c                | 124 ++++++++++++++++++++++----------------
 virt/kvm/kvm_main.c               |  20 +++++-
 7 files changed, 118 insertions(+), 54 deletions(-)

-- 
2.1.4

[PATCH V6 1/6] kvm: don't try to register to KVM_FAST_MMIO_BUS for non mmio eventfd

[Jason Wang]

We only want zero length mmio eventfd to be registered on
KVM_FAST_MMIO_BUS. So check this explicitly when arg->len is zero to
make sure this.

Cc: stable@vger.kernel.org
Cc: Gleb Natapov <gleb@kernel.org>
Cc: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Jason Wang <jasowang@redhat.com>
---
 virt/kvm/eventfd.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/virt/kvm/eventfd.c b/virt/kvm/eventfd.c
index 9ff4193..e404806 100644
--- a/virt/kvm/eventfd.c
+++ b/virt/kvm/eventfd.c
@@ -846,7 +846,7 @@ kvm_assign_ioeventfd(struct kvm *kvm, struct kvm_ioeventfd *args)
 	/* When length is ignored, MMIO is also put on a separate bus, for
 	 * faster lookups.
 	 */
-	if (!args->len && !(args->flags & KVM_IOEVENTFD_FLAG_PIO)) {
+	if (!args->len && bus_idx == KVM_MMIO_BUS) {
 		ret = kvm_io_bus_register_dev(kvm, KVM_FAST_MMIO_BUS,
 					      p->addr, 0, &p->dev);
 		if (ret < 0)
@@ -901,7 +901,7 @@ kvm_deassign_ioeventfd(struct kvm *kvm, struct kvm_ioeventfd *args)
 			continue;
 
 		kvm_io_bus_unregister_dev(kvm, bus_idx, &p->dev);
-		if (!p->length) {
+		if (!p->length && p->bus_idx == KVM_MMIO_BUS) {
 			kvm_io_bus_unregister_dev(kvm, KVM_FAST_MMIO_BUS,
 						  &p->dev);
 		}
-- 
2.1.4

[PATCH V6 2/6] kvm: factor out core eventfd assign/deassign logic

[Jason Wang]

This patch factors out core eventfd assign/deassign logic and leaves
the argument checking and bus index selection to callers.

Cc: stable@vger.kernel.org
Cc: Gleb Natapov <gleb@kernel.org>
Cc: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Jason Wang <jasowang@redhat.com>
---
 virt/kvm/eventfd.c | 85 ++++++++++++++++++++++++++++++++----------------------
 1 file changed, 50 insertions(+), 35 deletions(-)

diff --git a/virt/kvm/eventfd.c b/virt/kvm/eventfd.c
index e404806..0829c7f 100644
--- a/virt/kvm/eventfd.c
+++ b/virt/kvm/eventfd.c
@@ -771,40 +771,14 @@ static enum kvm_bus ioeventfd_bus_from_flags(__u32 flags)
 	return KVM_MMIO_BUS;
 }
 
-static int
-kvm_assign_ioeventfd(struct kvm *kvm, struct kvm_ioeventfd *args)
+static int kvm_assign_ioeventfd_idx(struct kvm *kvm,
+				enum kvm_bus bus_idx,
+				struct kvm_ioeventfd *args)
 {
-	enum kvm_bus              bus_idx;
-	struct _ioeventfd        *p;
-	struct eventfd_ctx       *eventfd;
-	int                       ret;
-
-	bus_idx = ioeventfd_bus_from_flags(args->flags);
-	/* must be natural-word sized, or 0 to ignore length */
-	switch (args->len) {
-	case 0:
-	case 1:
-	case 2:
-	case 4:
-	case 8:
-		break;
-	default:
-		return -EINVAL;
-	}
-
-	/* check for range overflow */
-	if (args->addr + args->len < args->addr)
-		return -EINVAL;
 
-	/* check for extra flags that we don't understand */
-	if (args->flags & ~KVM_IOEVENTFD_VALID_FLAG_MASK)
-		return -EINVAL;
-
-	/* ioeventfd with no length can't be combined with DATAMATCH */
-	if (!args->len &&
-	    args->flags & (KVM_IOEVENTFD_FLAG_PIO |
-			   KVM_IOEVENTFD_FLAG_DATAMATCH))
-		return -EINVAL;
+	struct eventfd_ctx *eventfd;
+	struct _ioeventfd *p;
+	int ret;
 
 	eventfd = eventfd_ctx_fdget(args->fd);
 	if (IS_ERR(eventfd))
@@ -873,14 +847,13 @@ fail:
 }
 
 static int
-kvm_deassign_ioeventfd(struct kvm *kvm, struct kvm_ioeventfd *args)
+kvm_deassign_ioeventfd_idx(struct kvm *kvm, enum kvm_bus bus_idx,
+			   struct kvm_ioeventfd *args)
 {
-	enum kvm_bus              bus_idx;
 	struct _ioeventfd        *p, *tmp;
 	struct eventfd_ctx       *eventfd;
 	int                       ret = -ENOENT;
 
-	bus_idx = ioeventfd_bus_from_flags(args->flags);
 	eventfd = eventfd_ctx_fdget(args->fd);
 	if (IS_ERR(eventfd))
 		return PTR_ERR(eventfd);
@@ -918,6 +891,48 @@ kvm_deassign_ioeventfd(struct kvm *kvm, struct kvm_ioeventfd *args)
 	return ret;
 }
 
+static int kvm_deassign_ioeventfd(struct kvm *kvm, struct kvm_ioeventfd *args)
+{
+	enum kvm_bus bus_idx = ioeventfd_bus_from_flags(args->flags);
+
+	return kvm_deassign_ioeventfd_idx(kvm, bus_idx, args);
+}
+
+static int
+kvm_assign_ioeventfd(struct kvm *kvm, struct kvm_ioeventfd *args)
+{
+	enum kvm_bus              bus_idx;
+
+	bus_idx = ioeventfd_bus_from_flags(args->flags);
+	/* must be natural-word sized, or 0 to ignore length */
+	switch (args->len) {
+	case 0:
+	case 1:
+	case 2:
+	case 4:
+	case 8:
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	/* check for range overflow */
+	if (args->addr + args->len < args->addr)
+		return -EINVAL;
+
+	/* check for extra flags that we don't understand */
+	if (args->flags & ~KVM_IOEVENTFD_VALID_FLAG_MASK)
+		return -EINVAL;
+
+	/* ioeventfd with no length can't be combined with DATAMATCH */
+	if (!args->len &&
+	    args->flags & (KVM_IOEVENTFD_FLAG_PIO |
+			   KVM_IOEVENTFD_FLAG_DATAMATCH))
+		return -EINVAL;
+
+	return kvm_assign_ioeventfd_idx(kvm, bus_idx, args);
+}
+
 int
 kvm_ioeventfd(struct kvm *kvm, struct kvm_ioeventfd *args)
 {
-- 
2.1.4

[PATCH V6 3/6] kvm: fix double free for fast mmio eventfd

[Jason Wang]

We register wildcard mmio eventfd on two buses, once for KVM_MMIO_BUS
and once on KVM_FAST_MMIO_BUS but with a single iodev
instance. This will lead to an issue: kvm_io_bus_destroy() knows
nothing about the devices on two buses pointing to a single dev. Which
will lead to double free[1] during exit. Fix this by allocating two
instances of iodevs then registering one on KVM_MMIO_BUS and another
on KVM_FAST_MMIO_BUS.

CPU: 1 PID: 2894 Comm: qemu-system-x86 Not tainted 3.19.0-26-generic #28-Ubuntu
Hardware name: LENOVO 2356BG6/2356BG6, BIOS G7ET96WW (2.56 ) 09/12/2013
task: ffff88009ae0c4b0 ti: ffff88020e7f0000 task.ti: ffff88020e7f0000
RIP: 0010:[<ffffffffc07e25d8>]  [<ffffffffc07e25d8>] ioeventfd_release+0x28/0x60 [kvm]
RSP: 0018:ffff88020e7f3bc8  EFLAGS: 00010292
RAX: dead000000200200 RBX: ffff8801ec19c900 RCX: 000000018200016d
RDX: ffff8801ec19cf80 RSI: ffffea0008bf1d40 RDI: ffff8801ec19c900
RBP: ffff88020e7f3bd8 R08: 000000002fc75a01 R09: 000000018200016d
R10: ffffffffc07df6ae R11: ffff88022fc75a98 R12: ffff88021e7cc000
R13: ffff88021e7cca48 R14: ffff88021e7cca50 R15: ffff8801ec19c880
FS:  00007fc1ee3e6700(0000) GS:ffff88023e240000(0000) knlGS:0000000000000000
CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: 00007f8f389d8000 CR3: 000000023dc13000 CR4: 00000000001427e0
Stack:
ffff88021e7cc000 0000000000000000 ffff88020e7f3be8 ffffffffc07e2622
ffff88020e7f3c38 ffffffffc07df69a ffff880232524160 ffff88020e792d80
 0000000000000000 ffff880219b78c00 0000000000000008 ffff8802321686a8
Call Trace:
[<ffffffffc07e2622>] ioeventfd_destructor+0x12/0x20 [kvm]
[<ffffffffc07df69a>] kvm_put_kvm+0xca/0x210 [kvm]
[<ffffffffc07df818>] kvm_vcpu_release+0x18/0x20 [kvm]
[<ffffffff811f69f7>] __fput+0xe7/0x250
[<ffffffff811f6bae>] ____fput+0xe/0x10
[<ffffffff81093f04>] task_work_run+0xd4/0xf0
[<ffffffff81079358>] do_exit+0x368/0xa50
[<ffffffff81082c8f>] ? recalc_sigpending+0x1f/0x60
[<ffffffff81079ad5>] do_group_exit+0x45/0xb0
[<ffffffff81085c71>] get_signal+0x291/0x750
[<ffffffff810144d8>] do_signal+0x28/0xab0
[<ffffffff810f3a3b>] ? do_futex+0xdb/0x5d0
[<ffffffff810b7028>] ? __wake_up_locked_key+0x18/0x20
[<ffffffff810f3fa6>] ? SyS_futex+0x76/0x170
[<ffffffff81014fc9>] do_notify_resume+0x69/0xb0
[<ffffffff817cb9af>] int_signal+0x12/0x17
Code: 5d c3 90 0f 1f 44 00 00 55 48 89 e5 53 48 89 fb 48 83 ec 08 48 8b 7f 20 e8 06 d6 a5 c0 48 8b 43 08 48 8b 13 48 89 df 48 89 42 08 <48> 89 10 48 b8 00 01 10 00 00
 RIP  [<ffffffffc07e25d8>] ioeventfd_release+0x28/0x60 [kvm]
 RSP <ffff88020e7f3bc8>

Cc: stable@vger.kernel.org
Cc: Gleb Natapov <gleb@kernel.org>
Cc: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Jason Wang <jasowang@redhat.com>
---
 virt/kvm/eventfd.c | 43 +++++++++++++++++++++++++------------------
 1 file changed, 25 insertions(+), 18 deletions(-)

diff --git a/virt/kvm/eventfd.c b/virt/kvm/eventfd.c
index 0829c7f..79db453 100644
--- a/virt/kvm/eventfd.c
+++ b/virt/kvm/eventfd.c
@@ -817,16 +817,6 @@ static int kvm_assign_ioeventfd_idx(struct kvm *kvm,
 	if (ret < 0)
 		goto unlock_fail;
 
-	/* When length is ignored, MMIO is also put on a separate bus, for
-	 * faster lookups.
-	 */
-	if (!args->len && bus_idx == KVM_MMIO_BUS) {
-		ret = kvm_io_bus_register_dev(kvm, KVM_FAST_MMIO_BUS,
-					      p->addr, 0, &p->dev);
-		if (ret < 0)
-			goto register_fail;
-	}
-
 	kvm->buses[bus_idx]->ioeventfd_count++;
 	list_add_tail(&p->list, &kvm->ioeventfds);
 
@@ -834,8 +824,6 @@ static int kvm_assign_ioeventfd_idx(struct kvm *kvm,
 
 	return 0;
 
-register_fail:
-	kvm_io_bus_unregister_dev(kvm, bus_idx, &p->dev);
 unlock_fail:
 	mutex_unlock(&kvm->slots_lock);
 
@@ -874,10 +862,6 @@ kvm_deassign_ioeventfd_idx(struct kvm *kvm, enum kvm_bus bus_idx,
 			continue;
 
 		kvm_io_bus_unregister_dev(kvm, bus_idx, &p->dev);
-		if (!p->length && p->bus_idx == KVM_MMIO_BUS) {
-			kvm_io_bus_unregister_dev(kvm, KVM_FAST_MMIO_BUS,
-						  &p->dev);
-		}
 		kvm->buses[bus_idx]->ioeventfd_count--;
 		ioeventfd_release(p);
 		ret = 0;
@@ -894,14 +878,19 @@ kvm_deassign_ioeventfd_idx(struct kvm *kvm, enum kvm_bus bus_idx,
 static int kvm_deassign_ioeventfd(struct kvm *kvm, struct kvm_ioeventfd *args)
 {
 	enum kvm_bus bus_idx = ioeventfd_bus_from_flags(args->flags);
+	int ret = kvm_deassign_ioeventfd_idx(kvm, bus_idx, args);
+
+	if (!args->len && bus_idx == KVM_MMIO_BUS)
+		kvm_deassign_ioeventfd_idx(kvm, KVM_FAST_MMIO_BUS, args);
 
-	return kvm_deassign_ioeventfd_idx(kvm, bus_idx, args);
+	return ret;
 }
 
 static int
 kvm_assign_ioeventfd(struct kvm *kvm, struct kvm_ioeventfd *args)
 {
 	enum kvm_bus              bus_idx;
+	int ret;
 
 	bus_idx = ioeventfd_bus_from_flags(args->flags);
 	/* must be natural-word sized, or 0 to ignore length */
@@ -930,7 +919,25 @@ kvm_assign_ioeventfd(struct kvm *kvm, struct kvm_ioeventfd *args)
 			   KVM_IOEVENTFD_FLAG_DATAMATCH))
 		return -EINVAL;
 
-	return kvm_assign_ioeventfd_idx(kvm, bus_idx, args);
+	ret = kvm_assign_ioeventfd_idx(kvm, bus_idx, args);
+	if (ret)
+		goto fail;
+
+	/* When length is ignored, MMIO is also put on a separate bus, for
+	 * faster lookups.
+	 */
+	if (!args->len && bus_idx == KVM_MMIO_BUS) {
+		ret = kvm_assign_ioeventfd_idx(kvm, KVM_FAST_MMIO_BUS, args);
+		if (ret < 0)
+			goto fast_fail;
+	}
+
+	return 0;
+
+fast_fail:
+	kvm_deassign_ioeventfd_idx(kvm, bus_idx, args);
+fail:
+	return ret;
 }
 
 int
-- 
2.1.4

[PATCH V6 4/6] kvm: fix zero length mmio searching

[Jason Wang]

Currently, if we had a zero length mmio eventfd assigned on
KVM_MMIO_BUS. It will never be found by kvm_io_bus_cmp() since it
always compares the kvm_io_range() with the length that guest
wrote. This will cause e.g for vhost, kick will be trapped by qemu
userspace instead of vhost. Fixing this by using zero length if an
iodevice is zero length.

Cc: stable@vger.kernel.org
Cc: Gleb Natapov <gleb@kernel.org>
Cc: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Jason Wang <jasowang@redhat.com>
---
 virt/kvm/kvm_main.c | 19 +++++++++++++++++--
 1 file changed, 17 insertions(+), 2 deletions(-)

diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
index eb4c9d2..9af68db 100644
--- a/virt/kvm/kvm_main.c
+++ b/virt/kvm/kvm_main.c
@@ -3157,10 +3157,25 @@ static void kvm_io_bus_destroy(struct kvm_io_bus *bus)
 static inline int kvm_io_bus_cmp(const struct kvm_io_range *r1,
 				 const struct kvm_io_range *r2)
 {
-	if (r1->addr < r2->addr)
+	gpa_t addr1 = r1->addr;
+	gpa_t addr2 = r2->addr;
+
+	if (addr1 < addr2)
 		return -1;
-	if (r1->addr + r1->len > r2->addr + r2->len)
+
+	/* If r2->len == 0, match the exact address.  If r2->len != 0,
+	 * accept any overlapping write.  Any order is acceptable for
+	 * overlapping ranges, because kvm_io_bus_get_first_dev ensures
+	 * we process all of them.
+	 */
+	if (r2->len) {
+		addr1 += r1->len;
+		addr2 += r2->len;
+	}
+
+	if (addr1 > addr2)
 		return 1;
+
 	return 0;
 }
 
-- 
2.1.4

[PATCH V6 5/6] kvm: add tracepoint for fast mmio

[Jason Wang]

Cc: Gleb Natapov <gleb@kernel.org>
Cc: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Jason Wang <jasowang@redhat.com>
---
 arch/x86/kvm/trace.h | 18 ++++++++++++++++++
 arch/x86/kvm/vmx.c   |  1 +
 arch/x86/kvm/x86.c   |  1 +
 3 files changed, 20 insertions(+)

diff --git a/arch/x86/kvm/trace.h b/arch/x86/kvm/trace.h
index 4eae7c3..ce4abe3 100644
--- a/arch/x86/kvm/trace.h
+++ b/arch/x86/kvm/trace.h
@@ -129,6 +129,24 @@ TRACE_EVENT(kvm_pio,
 );
 
 /*
+ * Tracepoint for fast mmio.
+ */
+TRACE_EVENT(kvm_fast_mmio,
+	TP_PROTO(u64 gpa),
+	TP_ARGS(gpa),
+
+	TP_STRUCT__entry(
+		__field(u64,	gpa)
+	),
+
+	TP_fast_assign(
+		__entry->gpa		= gpa;
+	),
+
+	TP_printk("fast mmio at gpa 0x%llx", __entry->gpa)
+);
+
+/*
  * Tracepoint for cpuid.
  */
 TRACE_EVENT(kvm_cpuid,
diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
index d019868..ff1234a 100644
--- a/arch/x86/kvm/vmx.c
+++ b/arch/x86/kvm/vmx.c
@@ -5767,6 +5767,7 @@ static int handle_ept_misconfig(struct kvm_vcpu *vcpu)
 	gpa = vmcs_read64(GUEST_PHYSICAL_ADDRESS);
 	if (!kvm_io_bus_write(vcpu, KVM_FAST_MMIO_BUS, gpa, 0, NULL)) {
 		skip_emulated_instruction(vcpu);
+		trace_kvm_fast_mmio(gpa);
 		return 1;
 	}
 
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index a60bdbc..1ec3965 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -8015,6 +8015,7 @@ bool kvm_arch_has_noncoherent_dma(struct kvm *kvm)
 EXPORT_SYMBOL_GPL(kvm_arch_has_noncoherent_dma);
 
 EXPORT_TRACEPOINT_SYMBOL_GPL(kvm_exit);
+EXPORT_TRACEPOINT_SYMBOL_GPL(kvm_fast_mmio);
 EXPORT_TRACEPOINT_SYMBOL_GPL(kvm_inj_virq);
 EXPORT_TRACEPOINT_SYMBOL_GPL(kvm_page_fault);
 EXPORT_TRACEPOINT_SYMBOL_GPL(kvm_msr);
-- 
2.1.4

[PATCH V6 6/6] kvm: add fast mmio capabilitiy

[Jason Wang]

Cc: Gleb Natapov <gleb@kernel.org>
Cc: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Jason Wang <jasowang@redhat.com>
---
 Documentation/virtual/kvm/api.txt | 7 ++++++-
 include/uapi/linux/kvm.h          | 1 +
 virt/kvm/kvm_main.c               | 1 +
 3 files changed, 8 insertions(+), 1 deletion(-)

diff --git a/Documentation/virtual/kvm/api.txt b/Documentation/virtual/kvm/api.txt
index d9eccee..26661ef 100644
--- a/Documentation/virtual/kvm/api.txt
+++ b/Documentation/virtual/kvm/api.txt
@@ -1598,7 +1598,7 @@ provided event instead of triggering an exit.
 struct kvm_ioeventfd {
 	__u64 datamatch;
 	__u64 addr;        /* legal pio/mmio address */
-	__u32 len;         /* 1, 2, 4, or 8 bytes    */
+	__u32 len;         /* 0, 1, 2, 4, or 8 bytes    */
 	__s32 fd;
 	__u32 flags;
 	__u8  pad[36];
@@ -1621,6 +1621,11 @@ to the registered address is equal to datamatch in struct kvm_ioeventfd.
 For virtio-ccw devices, addr contains the subchannel id and datamatch the
 virtqueue index.
 
+With KVM_CAP_FAST_MMIO, a zero length mmio eventfd is allowed for
+kernel to ignore the length of guest write and get a possible faster
+response. Note the speedup may only work on some specific
+architectures and setups. Otherwise, it's as fast as wildcard mmio
+eventfd.
 
 4.60 KVM_DIRTY_TLB
 
diff --git a/include/uapi/linux/kvm.h b/include/uapi/linux/kvm.h
index a9256f0..ad72a61 100644
--- a/include/uapi/linux/kvm.h
+++ b/include/uapi/linux/kvm.h
@@ -824,6 +824,7 @@ struct kvm_ppc_smmu_info {
 #define KVM_CAP_MULTI_ADDRESS_SPACE 118
 #define KVM_CAP_GUEST_DEBUG_HW_BPS 119
 #define KVM_CAP_GUEST_DEBUG_HW_WPS 120
+#define KVM_CAP_FAST_MMIO 121
 
 #ifdef KVM_CAP_IRQ_ROUTING
 
diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
index 9af68db..645f55d 100644
--- a/virt/kvm/kvm_main.c
+++ b/virt/kvm/kvm_main.c
@@ -2717,6 +2717,7 @@ static long kvm_vm_ioctl_check_extension_generic(struct kvm *kvm, long arg)
 	case KVM_CAP_IRQFD:
 	case KVM_CAP_IRQFD_RESAMPLE:
 #endif
+	case KVM_CAP_FAST_MMIO:
 	case KVM_CAP_CHECK_EXTENSION_VM:
 		return 1;
 #ifdef CONFIG_HAVE_KVM_IRQ_ROUTING
-- 
2.1.4

Re: [PATCH V6 1/6] kvm: don't try to register to KVM_FAST_MMIO_BUS for non mmio eventfd

[Cornelia Huck]

On Tue, 15 Sep 2015 14:41:54 +0800
Jason Wang <jasowang@redhat.com> wrote:

> We only want zero length mmio eventfd to be registered on
> KVM_FAST_MMIO_BUS. So check this explicitly when arg->len is zero to
> make sure this.
> 
> Cc: stable@vger.kernel.org
> Cc: Gleb Natapov <gleb@kernel.org>
> Cc: Paolo Bonzini <pbonzini@redhat.com>
> Signed-off-by: Jason Wang <jasowang@redhat.com>
> ---
>  virt/kvm/eventfd.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)

Reviewed-by: Cornelia Huck <cornelia.huck@de.ibm.com>

Re: [PATCH V6 2/6] kvm: factor out core eventfd assign/deassign logic

[Cornelia Huck]

On Tue, 15 Sep 2015 14:41:55 +0800
Jason Wang <jasowang@redhat.com> wrote:

> This patch factors out core eventfd assign/deassign logic and leaves
> the argument checking and bus index selection to callers.
> 
> Cc: stable@vger.kernel.org
> Cc: Gleb Natapov <gleb@kernel.org>
> Cc: Paolo Bonzini <pbonzini@redhat.com>
> Signed-off-by: Jason Wang <jasowang@redhat.com>
> ---
>  virt/kvm/eventfd.c | 85 ++++++++++++++++++++++++++++++++----------------------
>  1 file changed, 50 insertions(+), 35 deletions(-)

Reviewed-by: Cornelia Huck <cornelia.huck@de.ibm.com>

Re: [PATCH V6 3/6] kvm: fix double free for fast mmio eventfd

[Cornelia Huck]

On Tue, 15 Sep 2015 14:41:56 +0800
Jason Wang <jasowang@redhat.com> wrote:

> We register wildcard mmio eventfd on two buses, once for KVM_MMIO_BUS
> and once on KVM_FAST_MMIO_BUS but with a single iodev
> instance. This will lead to an issue: kvm_io_bus_destroy() knows
> nothing about the devices on two buses pointing to a single dev. Which
> will lead to double free[1] during exit. Fix this by allocating two
> instances of iodevs then registering one on KVM_MMIO_BUS and another
> on KVM_FAST_MMIO_BUS.
> 
> CPU: 1 PID: 2894 Comm: qemu-system-x86 Not tainted 3.19.0-26-generic #28-Ubuntu
> Hardware name: LENOVO 2356BG6/2356BG6, BIOS G7ET96WW (2.56 ) 09/12/2013
> task: ffff88009ae0c4b0 ti: ffff88020e7f0000 task.ti: ffff88020e7f0000
> RIP: 0010:[<ffffffffc07e25d8>]  [<ffffffffc07e25d8>] ioeventfd_release+0x28/0x60 [kvm]
> RSP: 0018:ffff88020e7f3bc8  EFLAGS: 00010292
> RAX: dead000000200200 RBX: ffff8801ec19c900 RCX: 000000018200016d
> RDX: ffff8801ec19cf80 RSI: ffffea0008bf1d40 RDI: ffff8801ec19c900
> RBP: ffff88020e7f3bd8 R08: 000000002fc75a01 R09: 000000018200016d
> R10: ffffffffc07df6ae R11: ffff88022fc75a98 R12: ffff88021e7cc000
> R13: ffff88021e7cca48 R14: ffff88021e7cca50 R15: ffff8801ec19c880
> FS:  00007fc1ee3e6700(0000) GS:ffff88023e240000(0000) knlGS:0000000000000000
> CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> CR2: 00007f8f389d8000 CR3: 000000023dc13000 CR4: 00000000001427e0
> Stack:
> ffff88021e7cc000 0000000000000000 ffff88020e7f3be8 ffffffffc07e2622
> ffff88020e7f3c38 ffffffffc07df69a ffff880232524160 ffff88020e792d80
>  0000000000000000 ffff880219b78c00 0000000000000008 ffff8802321686a8
> Call Trace:
> [<ffffffffc07e2622>] ioeventfd_destructor+0x12/0x20 [kvm]
> [<ffffffffc07df69a>] kvm_put_kvm+0xca/0x210 [kvm]
> [<ffffffffc07df818>] kvm_vcpu_release+0x18/0x20 [kvm]
> [<ffffffff811f69f7>] __fput+0xe7/0x250
> [<ffffffff811f6bae>] ____fput+0xe/0x10
> [<ffffffff81093f04>] task_work_run+0xd4/0xf0
> [<ffffffff81079358>] do_exit+0x368/0xa50
> [<ffffffff81082c8f>] ? recalc_sigpending+0x1f/0x60
> [<ffffffff81079ad5>] do_group_exit+0x45/0xb0
> [<ffffffff81085c71>] get_signal+0x291/0x750
> [<ffffffff810144d8>] do_signal+0x28/0xab0
> [<ffffffff810f3a3b>] ? do_futex+0xdb/0x5d0
> [<ffffffff810b7028>] ? __wake_up_locked_key+0x18/0x20
> [<ffffffff810f3fa6>] ? SyS_futex+0x76/0x170
> [<ffffffff81014fc9>] do_notify_resume+0x69/0xb0
> [<ffffffff817cb9af>] int_signal+0x12/0x17
> Code: 5d c3 90 0f 1f 44 00 00 55 48 89 e5 53 48 89 fb 48 83 ec 08 48 8b 7f 20 e8 06 d6 a5 c0 48 8b 43 08 48 8b 13 48 89 df 48 89 42 08 <48> 89 10 48 b8 00 01 10 00 00
>  RIP  [<ffffffffc07e25d8>] ioeventfd_release+0x28/0x60 [kvm]
>  RSP <ffff88020e7f3bc8>
> 
> Cc: stable@vger.kernel.org
> Cc: Gleb Natapov <gleb@kernel.org>
> Cc: Paolo Bonzini <pbonzini@redhat.com>
> Signed-off-by: Jason Wang <jasowang@redhat.com>
> ---
>  virt/kvm/eventfd.c | 43 +++++++++++++++++++++++++------------------
>  1 file changed, 25 insertions(+), 18 deletions(-)

Reviewed-by: Cornelia Huck <cornelia.huck@de.ibm.com>

Re: [PATCH V6 4/6] kvm: fix zero length mmio searching

[Cornelia Huck]

On Tue, 15 Sep 2015 14:41:57 +0800
Jason Wang <jasowang@redhat.com> wrote:

> Currently, if we had a zero length mmio eventfd assigned on
> KVM_MMIO_BUS. It will never be found by kvm_io_bus_cmp() since it
> always compares the kvm_io_range() with the length that guest
> wrote. This will cause e.g for vhost, kick will be trapped by qemu
> userspace instead of vhost. Fixing this by using zero length if an
> iodevice is zero length.
> 
> Cc: stable@vger.kernel.org
> Cc: Gleb Natapov <gleb@kernel.org>
> Cc: Paolo Bonzini <pbonzini@redhat.com>
> Signed-off-by: Jason Wang <jasowang@redhat.com>
> ---
>  virt/kvm/kvm_main.c | 19 +++++++++++++++++--
>  1 file changed, 17 insertions(+), 2 deletions(-)

Reviewed-by: Cornelia Huck <cornelia.huck@de.ibm.com>

Re: [PATCH V6 6/6] kvm: add fast mmio capabilitiy

[Paolo Bonzini]

On 15/09/2015 08:41, Jason Wang wrote:
> +With KVM_CAP_FAST_MMIO, a zero length mmio eventfd is allowed for
> +kernel to ignore the length of guest write and get a possible faster
> +response. Note the speedup may only work on some specific
> +architectures and setups. Otherwise, it's as fast as wildcard mmio
> +eventfd.

I don't really like tying the capability to MMIO, especially since
zero length ioeventfd is already accepted for virtio-ccw.

What about the following?

diff --git a/Documentation/virtual/kvm/api.txt b/Documentation/virtual/kvm/api.txt
index 7a3cb48a644d..247944071cc8 100644
--- a/Documentation/virtual/kvm/api.txt
+++ b/Documentation/virtual/kvm/api.txt
@@ -1627,11 +1627,10 @@ to the registered address is equal to datamatch in struct kvm_ioeventfd.
 For virtio-ccw devices, addr contains the subchannel id and datamatch the
 virtqueue index.
 
-With KVM_CAP_FAST_MMIO, a zero length mmio eventfd is allowed for
-kernel to ignore the length of guest write and get a possible faster
-response. Note the speedup may only work on some specific
-architectures and setups. Otherwise, it's as fast as wildcard mmio
-eventfd.
+With KVM_CAP_IOEVENTFD_ANY_LENGTH, a zero length ioeventfd is allowed, and
+the kernel will ignore the length of guest write and get a faster vmexit.
+The speedup may only apply to specific architectures, but the ioeventfd will
+work anyway.
 
 4.60 KVM_DIRTY_TLB
 
diff --git a/include/uapi/linux/kvm.h b/include/uapi/linux/kvm.h
index b4f6aeaf94a6..03f3618612aa 100644
--- a/include/uapi/linux/kvm.h
+++ b/include/uapi/linux/kvm.h
@@ -830,7 +830,7 @@ struct kvm_ppc_smmu_info {
 #define KVM_CAP_GUEST_DEBUG_HW_BPS 119
 #define KVM_CAP_GUEST_DEBUG_HW_WPS 120
 #define KVM_CAP_SPLIT_IRQCHIP 121
-#define KVM_CAP_FAST_MMIO 122
+#define KVM_CAP_IOEVENTFD_ANY_LENGTH 122
 
 #ifdef KVM_CAP_IRQ_ROUTING
 
diff --git a/virt/kvm/eventfd.c b/virt/kvm/eventfd.c
index 79db45336e3a..1dc8c45d2270 100644
--- a/virt/kvm/eventfd.c
+++ b/virt/kvm/eventfd.c
@@ -914,9 +914,7 @@ kvm_assign_ioeventfd(struct kvm *kvm, struct kvm_ioeventfd *args)
 		return -EINVAL;
 
 	/* ioeventfd with no length can't be combined with DATAMATCH */
-	if (!args->len &&
-	    args->flags & (KVM_IOEVENTFD_FLAG_PIO |
-			   KVM_IOEVENTFD_FLAG_DATAMATCH))
+	if (!args->len && (args->flags & KVM_IOEVENTFD_FLAG_DATAMATCH))
 		return -EINVAL;
 
 	ret = kvm_assign_ioeventfd_idx(kvm, bus_idx, args);
diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
index 0780d970d087..0b48aadedcee 100644
--- a/virt/kvm/kvm_main.c
+++ b/virt/kvm/kvm_main.c
@@ -2717,7 +2717,7 @@ static long kvm_vm_ioctl_check_extension_generic(struct kvm *kvm, long arg)
 	case KVM_CAP_IRQFD:
 	case KVM_CAP_IRQFD_RESAMPLE:
 #endif
-	case KVM_CAP_FAST_MMIO:
+	case KVM_CAP_IOEVENTFD_ANY_LENGTH:
 	case KVM_CAP_CHECK_EXTENSION_VM:
 		return 1;
 #ifdef CONFIG_HAVE_KVM_IRQ_ROUTING


Paolo

Re: [PATCH V6 0/6] Fast mmio eventfd fixes

[Paolo Bonzini]

On 15/09/2015 08:41, Jason Wang wrote:
> Hi:
> 
> This series fixes two issues of fast mmio eventfd:
> 
> 1) A single iodev instance were registerd on two buses: KVM_MMIO_BUS
>    and KVM_FAST_MMIO_BUS. This will cause double in
>    ioeventfd_destructor()
> 2) A zero length iodev on KVM_MMIO_BUS will never be found but
>    kvm_io_bus_cmp(). This will lead e.g the eventfd will be trapped by
>    qemu instead of host.
> 
> 1 is fixed by allocating two instances of iodev and introduce a new
> capability for userspace. 2 is fixed by ignore the actual length if
> the length of iodev is zero in kvm_io_bus_cmp().
> 
> Please review.

Applied to kvm/queue and will send patches 1-4 for 4.3-rc.  Thanks!

Paolo

> Changes from V5:
> - move patch of explicitly checking for KVM_MMIO_BUS to patch 1 and
>   remove the unnecessary checks
> - even more grammar and typo fixes
> - rabase to kvm.git
> - document KVM_CAP_FAST_MMIO
> 
> Changes from V4:
> - move the location of kvm_assign_ioeventfd() in patch 1 which reduce
>   the change set.
> - commit log typo fixes
> - switch to use kvm_deassign_ioeventfd_id) when fail to register to
>   fast mmio bus
> - change kvm_io_bus_cmp() as Paolo's suggestions
> - introduce a new capability to avoid new userspace crash old kernel
> - add a new patch that only try to register mmio eventfd on fast mmio
>   bus
> 
> Changes from V3:
> 
> - Don't do search on two buses when trying to do write on
>   KVM_MMIO_BUS. This fixes a small regression found by vmexit.flat.
> - Since we don't do search on two buses, change kvm_io_bus_cmp() to
>   let it can find zero length iodevs.
> - Fix the unnecessary lines in tracepoint patch.
> 
> Changes from V2:
> - Tweak styles and comment suggested by Cornelia.
> 
> Changes from v1:
> - change ioeventfd_bus_from_flags() to return KVM_FAST_MMIO_BUS when
>   needed to save lots of unnecessary changes.
> 
> Jason Wang (6):
>   kvm: don't try to register to KVM_FAST_MMIO_BUS for non mmio eventfd
>   kvm: factor out core eventfd assign/deassign logic
>   kvm: fix double free for fast mmio eventfd
>   kvm: fix zero length mmio searching
>   kvm: add tracepoint for fast mmio
>   kvm: add fast mmio capabilitiy
> 
>  Documentation/virtual/kvm/api.txt |   7 ++-
>  arch/x86/kvm/trace.h              |  18 ++++++
>  arch/x86/kvm/vmx.c                |   1 +
>  arch/x86/kvm/x86.c                |   1 +
>  include/uapi/linux/kvm.h          |   1 +
>  virt/kvm/eventfd.c                | 124 ++++++++++++++++++++++----------------
>  virt/kvm/kvm_main.c               |  20 +++++-
>  7 files changed, 118 insertions(+), 54 deletions(-)
> 

Re: [PATCH V6 6/6] kvm: add fast mmio capabilitiy

[Cornelia Huck]

On Tue, 15 Sep 2015 17:07:55 +0200
Paolo Bonzini <pbonzini@redhat.com> wrote:

> On 15/09/2015 08:41, Jason Wang wrote:
> > +With KVM_CAP_FAST_MMIO, a zero length mmio eventfd is allowed for
> > +kernel to ignore the length of guest write and get a possible faster
> > +response. Note the speedup may only work on some specific
> > +architectures and setups. Otherwise, it's as fast as wildcard mmio
> > +eventfd.
> 
> I don't really like tying the capability to MMIO, especially since
> zero length ioeventfd is already accepted for virtio-ccw.

Actually, zero length ioeventfd does not make sense for virtio-ccw; we
just don't check it (although we probably should).

> 
> What about the following?
> 
> diff --git a/Documentation/virtual/kvm/api.txt b/Documentation/virtual/kvm/api.txt
> index 7a3cb48a644d..247944071cc8 100644
> --- a/Documentation/virtual/kvm/api.txt
> +++ b/Documentation/virtual/kvm/api.txt
> @@ -1627,11 +1627,10 @@ to the registered address is equal to datamatch in struct kvm_ioeventfd.
>  For virtio-ccw devices, addr contains the subchannel id and datamatch the
>  virtqueue index.
> 
> -With KVM_CAP_FAST_MMIO, a zero length mmio eventfd is allowed for
> -kernel to ignore the length of guest write and get a possible faster
> -response. Note the speedup may only work on some specific
> -architectures and setups. Otherwise, it's as fast as wildcard mmio
> -eventfd.
> +With KVM_CAP_IOEVENTFD_ANY_LENGTH, a zero length ioeventfd is allowed, and
> +the kernel will ignore the length of guest write and get a faster vmexit.

s/get/may get/ ?

> +The speedup may only apply to specific architectures, but the ioeventfd will
> +work anyway.

Re: [PATCH V6 6/6] kvm: add fast mmio capabilitiy

[Paolo Bonzini]

On 15/09/2015 18:13, Cornelia Huck wrote:
> On Tue, 15 Sep 2015 17:07:55 +0200
> Paolo Bonzini <pbonzini@redhat.com> wrote:
> 
>> On 15/09/2015 08:41, Jason Wang wrote:
>>> +With KVM_CAP_FAST_MMIO, a zero length mmio eventfd is allowed for
>>> +kernel to ignore the length of guest write and get a possible faster
>>> +response. Note the speedup may only work on some specific
>>> +architectures and setups. Otherwise, it's as fast as wildcard mmio
>>> +eventfd.
>>
>> I don't really like tying the capability to MMIO, especially since
>> zero length ioeventfd is already accepted for virtio-ccw.
> 
> Actually, zero length ioeventfd does not make sense for virtio-ccw;

Can you explain why?  If there is any non-zero valid length, "wildcard
length" (represented by zero) would also make sense.

Paolo

> we just don't check it (although we probably should).
> 
>>
>> What about the following?
>>
>> diff --git a/Documentation/virtual/kvm/api.txt b/Documentation/virtual/kvm/api.txt
>> index 7a3cb48a644d..247944071cc8 100644
>> --- a/Documentation/virtual/kvm/api.txt
>> +++ b/Documentation/virtual/kvm/api.txt
>> @@ -1627,11 +1627,10 @@ to the registered address is equal to datamatch in struct kvm_ioeventfd.
>>  For virtio-ccw devices, addr contains the subchannel id and datamatch the
>>  virtqueue index.
>>
>> -With KVM_CAP_FAST_MMIO, a zero length mmio eventfd is allowed for
>> -kernel to ignore the length of guest write and get a possible faster
>> -response. Note the speedup may only work on some specific
>> -architectures and setups. Otherwise, it's as fast as wildcard mmio
>> -eventfd.
>> +With KVM_CAP_IOEVENTFD_ANY_LENGTH, a zero length ioeventfd is allowed, and
>> +the kernel will ignore the length of guest write and get a faster vmexit.
> 
> s/get/may get/ ?
> 
>> +The speedup may only apply to specific architectures, but the ioeventfd will
>> +work anyway.
> 

Re: [PATCH V6 6/6] kvm: add fast mmio capabilitiy

[Cornelia Huck]

On Tue, 15 Sep 2015 18:29:49 +0200
Paolo Bonzini <pbonzini@redhat.com> wrote:

> On 15/09/2015 18:13, Cornelia Huck wrote:
> > On Tue, 15 Sep 2015 17:07:55 +0200
> > Paolo Bonzini <pbonzini@redhat.com> wrote:
> > 
> >> On 15/09/2015 08:41, Jason Wang wrote:
> >>> +With KVM_CAP_FAST_MMIO, a zero length mmio eventfd is allowed for
> >>> +kernel to ignore the length of guest write and get a possible faster
> >>> +response. Note the speedup may only work on some specific
> >>> +architectures and setups. Otherwise, it's as fast as wildcard mmio
> >>> +eventfd.
> >>
> >> I don't really like tying the capability to MMIO, especially since
> >> zero length ioeventfd is already accepted for virtio-ccw.
> > 
> > Actually, zero length ioeventfd does not make sense for virtio-ccw;
> 
> Can you explain why?  If there is any non-zero valid length, "wildcard
> length" (represented by zero) would also make sense.

What is a wildcard match supposed to mean in this case? The datamatch
field contains the queue index for the device specified in the address
field. The hypercall interface associated with the eventfd always has
device + queue index in its parameters; there is no interface for
"notify device with all its queues".

But maybe I'm just lacking imagination :)

Re: [PATCH V6 6/6] kvm: add fast mmio capabilitiy

[Paolo Bonzini]

On 15/09/2015 18:44, Cornelia Huck wrote:
>> > Can you explain why?  If there is any non-zero valid length, "wildcard
>> > length" (represented by zero) would also make sense.
> What is a wildcard match supposed to mean in this case? The datamatch
> field contains the queue index for the device specified in the address
> field. The hypercall interface associated with the eventfd always has
> device + queue index in its parameters; there is no interface for
> "notify device with all its queues".

Ah, I see.  Because all valid virtio-ccw ioeventfds are datamatch, no
valid virtio-ccw ioeventfd is wildcard-length.

Paolo

Re: [PATCH V6 0/6] Fast mmio eventfd fixes

[Michael S. Tsirkin]

On Tue, Sep 15, 2015 at 05:08:49PM +0200, Paolo Bonzini wrote:
> 
> 
> On 15/09/2015 08:41, Jason Wang wrote:
> > Hi:
> > 
> > This series fixes two issues of fast mmio eventfd:
> > 
> > 1) A single iodev instance were registerd on two buses: KVM_MMIO_BUS
> >    and KVM_FAST_MMIO_BUS. This will cause double in
> >    ioeventfd_destructor()
> > 2) A zero length iodev on KVM_MMIO_BUS will never be found but
> >    kvm_io_bus_cmp(). This will lead e.g the eventfd will be trapped by
> >    qemu instead of host.
> > 
> > 1 is fixed by allocating two instances of iodev and introduce a new
> > capability for userspace. 2 is fixed by ignore the actual length if
> > the length of iodev is zero in kvm_io_bus_cmp().
> > 
> > Please review.
> 
> Applied to kvm/queue and will send patches 1-4 for 4.3-rc.  Thanks!
> 
> Paolo

I'd prefer at least 6 to be there as well:
without 6 userspace can't safely use the code, and without 5,
it can't trace it.

> > Changes from V5:
> > - move patch of explicitly checking for KVM_MMIO_BUS to patch 1 and
> >   remove the unnecessary checks
> > - even more grammar and typo fixes
> > - rabase to kvm.git
> > - document KVM_CAP_FAST_MMIO
> > 
> > Changes from V4:
> > - move the location of kvm_assign_ioeventfd() in patch 1 which reduce
> >   the change set.
> > - commit log typo fixes
> > - switch to use kvm_deassign_ioeventfd_id) when fail to register to
> >   fast mmio bus
> > - change kvm_io_bus_cmp() as Paolo's suggestions
> > - introduce a new capability to avoid new userspace crash old kernel
> > - add a new patch that only try to register mmio eventfd on fast mmio
> >   bus
> > 
> > Changes from V3:
> > 
> > - Don't do search on two buses when trying to do write on
> >   KVM_MMIO_BUS. This fixes a small regression found by vmexit.flat.
> > - Since we don't do search on two buses, change kvm_io_bus_cmp() to
> >   let it can find zero length iodevs.
> > - Fix the unnecessary lines in tracepoint patch.
> > 
> > Changes from V2:
> > - Tweak styles and comment suggested by Cornelia.
> > 
> > Changes from v1:
> > - change ioeventfd_bus_from_flags() to return KVM_FAST_MMIO_BUS when
> >   needed to save lots of unnecessary changes.
> > 
> > Jason Wang (6):
> >   kvm: don't try to register to KVM_FAST_MMIO_BUS for non mmio eventfd
> >   kvm: factor out core eventfd assign/deassign logic
> >   kvm: fix double free for fast mmio eventfd
> >   kvm: fix zero length mmio searching
> >   kvm: add tracepoint for fast mmio
> >   kvm: add fast mmio capabilitiy
> > 
> >  Documentation/virtual/kvm/api.txt |   7 ++-
> >  arch/x86/kvm/trace.h              |  18 ++++++
> >  arch/x86/kvm/vmx.c                |   1 +
> >  arch/x86/kvm/x86.c                |   1 +
> >  include/uapi/linux/kvm.h          |   1 +
> >  virt/kvm/eventfd.c                | 124 ++++++++++++++++++++++----------------
> >  virt/kvm/kvm_main.c               |  20 +++++-
> >  7 files changed, 118 insertions(+), 54 deletions(-)
> > 

Re: [PATCH V6 0/6] Fast mmio eventfd fixes

[Paolo Bonzini]

On 15/09/2015 21:26, Michael S. Tsirkin wrote:
> > Applied to kvm/queue and will send patches 1-4 for 4.3-rc.  Thanks!
> 
> I'd prefer at least 6 to be there as well:
> without 6 userspace can't safely use the code, and without 5,
> it can't trace it.

The idea is to just make old userspace work without crashing.  New
features do not belong in stable releases.

Paolo

Re: [PATCH V6 0/6] Fast mmio eventfd fixes

[Michael S. Tsirkin]

On Tue, Sep 15, 2015 at 02:41:53PM +0800, Jason Wang wrote:
> Hi:
> 
> This series fixes two issues of fast mmio eventfd:
> 
> 1) A single iodev instance were registerd on two buses: KVM_MMIO_BUS
>    and KVM_FAST_MMIO_BUS. This will cause double in
>    ioeventfd_destructor()
> 2) A zero length iodev on KVM_MMIO_BUS will never be found but
>    kvm_io_bus_cmp(). This will lead e.g the eventfd will be trapped by
>    qemu instead of host.
> 
> 1 is fixed by allocating two instances of iodev and introduce a new
> capability for userspace. 2 is fixed by ignore the actual length if
> the length of iodev is zero in kvm_io_bus_cmp().
> 
> Please review.
> Changes from V5:
> - move patch of explicitly checking for KVM_MMIO_BUS to patch 1 and
>   remove the unnecessary checks
> - even more grammar and typo fixes
> - rabase to kvm.git
> - document KVM_CAP_FAST_MMIO

What's up with userspace using this capability?
Did patches ever get posted?

> 
> Changes from V4:
> - move the location of kvm_assign_ioeventfd() in patch 1 which reduce
>   the change set.
> - commit log typo fixes
> - switch to use kvm_deassign_ioeventfd_id) when fail to register to
>   fast mmio bus
> - change kvm_io_bus_cmp() as Paolo's suggestions
> - introduce a new capability to avoid new userspace crash old kernel
> - add a new patch that only try to register mmio eventfd on fast mmio
>   bus
> 
> Changes from V3:
> 
> - Don't do search on two buses when trying to do write on
>   KVM_MMIO_BUS. This fixes a small regression found by vmexit.flat.
> - Since we don't do search on two buses, change kvm_io_bus_cmp() to
>   let it can find zero length iodevs.
> - Fix the unnecessary lines in tracepoint patch.
> 
> Changes from V2:
> - Tweak styles and comment suggested by Cornelia.
> 
> Changes from v1:
> - change ioeventfd_bus_from_flags() to return KVM_FAST_MMIO_BUS when
>   needed to save lots of unnecessary changes.
> 
> Jason Wang (6):
>   kvm: don't try to register to KVM_FAST_MMIO_BUS for non mmio eventfd
>   kvm: factor out core eventfd assign/deassign logic
>   kvm: fix double free for fast mmio eventfd
>   kvm: fix zero length mmio searching
>   kvm: add tracepoint for fast mmio
>   kvm: add fast mmio capabilitiy
> 
>  Documentation/virtual/kvm/api.txt |   7 ++-
>  arch/x86/kvm/trace.h              |  18 ++++++
>  arch/x86/kvm/vmx.c                |   1 +
>  arch/x86/kvm/x86.c                |   1 +
>  include/uapi/linux/kvm.h          |   1 +
>  virt/kvm/eventfd.c                | 124 ++++++++++++++++++++++----------------
>  virt/kvm/kvm_main.c               |  20 +++++-
>  7 files changed, 118 insertions(+), 54 deletions(-)
> 
> -- 
> 2.1.4

Re: [PATCH V6 0/6] Fast mmio eventfd fixes

[Jason Wang]

On 11/09/2015 01:11 AM, Michael S. Tsirkin wrote:
> On Tue, Sep 15, 2015 at 02:41:53PM +0800, Jason Wang wrote:
>> Hi:
>>
>> This series fixes two issues of fast mmio eventfd:
>>
>> 1) A single iodev instance were registerd on two buses: KVM_MMIO_BUS
>>    and KVM_FAST_MMIO_BUS. This will cause double in
>>    ioeventfd_destructor()
>> 2) A zero length iodev on KVM_MMIO_BUS will never be found but
>>    kvm_io_bus_cmp(). This will lead e.g the eventfd will be trapped by
>>    qemu instead of host.
>>
>> 1 is fixed by allocating two instances of iodev and introduce a new
>> capability for userspace. 2 is fixed by ignore the actual length if
>> the length of iodev is zero in kvm_io_bus_cmp().
>>
>> Please review.
>> Changes from V5:
>> - move patch of explicitly checking for KVM_MMIO_BUS to patch 1 and
>>   remove the unnecessary checks
>> - even more grammar and typo fixes
>> - rabase to kvm.git
>> - document KVM_CAP_FAST_MMIO
> What's up with userspace using this capability?

It was renamed to KVM_CAP_IOEVENTFD_ANY_LENGTH.

> Did patches ever get posted?

See https://lkml.org/lkml/2015/9/28/208

>
>> Changes from V4:
>> - move the location of kvm_assign_ioeventfd() in patch 1 which reduce
>>   the change set.
>> - commit log typo fixes
>> - switch to use kvm_deassign_ioeventfd_id) when fail to register to
>>   fast mmio bus
>> - change kvm_io_bus_cmp() as Paolo's suggestions
>> - introduce a new capability to avoid new userspace crash old kernel
>> - add a new patch that only try to register mmio eventfd on fast mmio
>>   bus
>>
>> Changes from V3:
>>
>> - Don't do search on two buses when trying to do write on
>>   KVM_MMIO_BUS. This fixes a small regression found by vmexit.flat.
>> - Since we don't do search on two buses, change kvm_io_bus_cmp() to
>>   let it can find zero length iodevs.
>> - Fix the unnecessary lines in tracepoint patch.
>>
>> Changes from V2:
>> - Tweak styles and comment suggested by Cornelia.
>>
>> Changes from v1:
>> - change ioeventfd_bus_from_flags() to return KVM_FAST_MMIO_BUS when
>>   needed to save lots of unnecessary changes.
>>
>> Jason Wang (6):
>>   kvm: don't try to register to KVM_FAST_MMIO_BUS for non mmio eventfd
>>   kvm: factor out core eventfd assign/deassign logic
>>   kvm: fix double free for fast mmio eventfd
>>   kvm: fix zero length mmio searching
>>   kvm: add tracepoint for fast mmio
>>   kvm: add fast mmio capabilitiy
>>
>>  Documentation/virtual/kvm/api.txt |   7 ++-
>>  arch/x86/kvm/trace.h              |  18 ++++++
>>  arch/x86/kvm/vmx.c                |   1 +
>>  arch/x86/kvm/x86.c                |   1 +
>>  include/uapi/linux/kvm.h          |   1 +
>>  virt/kvm/eventfd.c                | 124 ++++++++++++++++++++++----------------
>>  virt/kvm/kvm_main.c               |  20 +++++-
>>  7 files changed, 118 insertions(+), 54 deletions(-)
>>
>> -- 
>> 2.1.4
> --
> To unsubscribe from this list: send the line "unsubscribe kvm" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH V6 0/6] Fast mmio eventfd fixes

[Michael S. Tsirkin]

On Mon, Nov 09, 2015 at 12:35:45PM +0800, Jason Wang wrote:
> 
> 
> On 11/09/2015 01:11 AM, Michael S. Tsirkin wrote:
> > On Tue, Sep 15, 2015 at 02:41:53PM +0800, Jason Wang wrote:
> >> Hi:
> >>
> >> This series fixes two issues of fast mmio eventfd:
> >>
> >> 1) A single iodev instance were registerd on two buses: KVM_MMIO_BUS
> >>    and KVM_FAST_MMIO_BUS. This will cause double in
> >>    ioeventfd_destructor()
> >> 2) A zero length iodev on KVM_MMIO_BUS will never be found but
> >>    kvm_io_bus_cmp(). This will lead e.g the eventfd will be trapped by
> >>    qemu instead of host.
> >>
> >> 1 is fixed by allocating two instances of iodev and introduce a new
> >> capability for userspace. 2 is fixed by ignore the actual length if
> >> the length of iodev is zero in kvm_io_bus_cmp().
> >>
> >> Please review.
> >> Changes from V5:
> >> - move patch of explicitly checking for KVM_MMIO_BUS to patch 1 and
> >>   remove the unnecessary checks
> >> - even more grammar and typo fixes
> >> - rabase to kvm.git
> >> - document KVM_CAP_FAST_MMIO
> > What's up with userspace using this capability?
> 
> It was renamed to KVM_CAP_IOEVENTFD_ANY_LENGTH.
> 
> > Did patches ever get posted?
> 
> See https://lkml.org/lkml/2015/9/28/208

Talking about userspace here.
QEMU freeze is approaching, it really should
use this to avoid regressions.


> >
> >> Changes from V4:
> >> - move the location of kvm_assign_ioeventfd() in patch 1 which reduce
> >>   the change set.
> >> - commit log typo fixes
> >> - switch to use kvm_deassign_ioeventfd_id) when fail to register to
> >>   fast mmio bus
> >> - change kvm_io_bus_cmp() as Paolo's suggestions
> >> - introduce a new capability to avoid new userspace crash old kernel
> >> - add a new patch that only try to register mmio eventfd on fast mmio
> >>   bus
> >>
> >> Changes from V3:
> >>
> >> - Don't do search on two buses when trying to do write on
> >>   KVM_MMIO_BUS. This fixes a small regression found by vmexit.flat.
> >> - Since we don't do search on two buses, change kvm_io_bus_cmp() to
> >>   let it can find zero length iodevs.
> >> - Fix the unnecessary lines in tracepoint patch.
> >>
> >> Changes from V2:
> >> - Tweak styles and comment suggested by Cornelia.
> >>
> >> Changes from v1:
> >> - change ioeventfd_bus_from_flags() to return KVM_FAST_MMIO_BUS when
> >>   needed to save lots of unnecessary changes.
> >>
> >> Jason Wang (6):
> >>   kvm: don't try to register to KVM_FAST_MMIO_BUS for non mmio eventfd
> >>   kvm: factor out core eventfd assign/deassign logic
> >>   kvm: fix double free for fast mmio eventfd
> >>   kvm: fix zero length mmio searching
> >>   kvm: add tracepoint for fast mmio
> >>   kvm: add fast mmio capabilitiy
> >>
> >>  Documentation/virtual/kvm/api.txt |   7 ++-
> >>  arch/x86/kvm/trace.h              |  18 ++++++
> >>  arch/x86/kvm/vmx.c                |   1 +
> >>  arch/x86/kvm/x86.c                |   1 +
> >>  include/uapi/linux/kvm.h          |   1 +
> >>  virt/kvm/eventfd.c                | 124 ++++++++++++++++++++++----------------
> >>  virt/kvm/kvm_main.c               |  20 +++++-
> >>  7 files changed, 118 insertions(+), 54 deletions(-)
> >>
> >> -- 
> >> 2.1.4
> > --
> > To unsubscribe from this list: send the line "unsubscribe kvm" in
> > the body of a message to majordomo@vger.kernel.org
> > More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH V6 0/6] Fast mmio eventfd fixes

[Jason Wang]

On 11/10/2015 04:19 AM, Michael S. Tsirkin wrote:
> On Mon, Nov 09, 2015 at 12:35:45PM +0800, Jason Wang wrote:
>> > 
>> > 
>> > On 11/09/2015 01:11 AM, Michael S. Tsirkin wrote:
>>> > > On Tue, Sep 15, 2015 at 02:41:53PM +0800, Jason Wang wrote:
>>>> > >> Hi:
>>>> > >>
>>>> > >> This series fixes two issues of fast mmio eventfd:
>>>> > >>
>>>> > >> 1) A single iodev instance were registerd on two buses: KVM_MMIO_BUS
>>>> > >>    and KVM_FAST_MMIO_BUS. This will cause double in
>>>> > >>    ioeventfd_destructor()
>>>> > >> 2) A zero length iodev on KVM_MMIO_BUS will never be found but
>>>> > >>    kvm_io_bus_cmp(). This will lead e.g the eventfd will be trapped by
>>>> > >>    qemu instead of host.
>>>> > >>
>>>> > >> 1 is fixed by allocating two instances of iodev and introduce a new
>>>> > >> capability for userspace. 2 is fixed by ignore the actual length if
>>>> > >> the length of iodev is zero in kvm_io_bus_cmp().
>>>> > >>
>>>> > >> Please review.
>>>> > >> Changes from V5:
>>>> > >> - move patch of explicitly checking for KVM_MMIO_BUS to patch 1 and
>>>> > >>   remove the unnecessary checks
>>>> > >> - even more grammar and typo fixes
>>>> > >> - rabase to kvm.git
>>>> > >> - document KVM_CAP_FAST_MMIO
>>> > > What's up with userspace using this capability?
>> > 
>> > It was renamed to KVM_CAP_IOEVENTFD_ANY_LENGTH.
>> > 
>>> > > Did patches ever get posted?
>> > 
>> > See https://lkml.org/lkml/2015/9/28/208
> Talking about userspace here.
> QEMU freeze is approaching, it really should
> use this to avoid regressions.
>

The patches were posted at
http://lists.gnu.org/archive/html/qemu-devel/2015-11/msg01276.html

(you were in cc list)