Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1222510 > unrolled thread

[PATCH V4 0/4] Fast MMIO eventfd fixes

Started byJason Wang <jasowang@redhat.com>
First post2015-09-11 05:20 +0200
Last post2015-09-13 11:00 +0200
Articles 10 — 4 participants

Back to article view | Back to linux.kernel

Contents

  [PATCH V4 0/4] Fast MMIO eventfd fixes Jason Wang <jasowang@redhat.com> - 2015-09-11 05:20 +0200
    [PATCH V4 3/4] kvm: fix zero length mmio searching Jason Wang <jasowang@redhat.com> - 2015-09-11 05:20 +0200
      Re: [PATCH V4 3/4] kvm: fix zero length mmio searching Paolo Bonzini <pbonzini@redhat.com> - 2015-09-11 10:30 +0200
        Re: [PATCH V4 3/4] kvm: fix zero length mmio searching Cornelia Huck <cornelia.huck@de.ibm.com> - 2015-09-11 10:40 +0200
          Re: [PATCH V4 3/4] kvm: fix zero length mmio searching Jason Wang <jasowang@redhat.com> - 2015-09-11 11:30 +0200
    Re: [PATCH V4 0/4] Fast MMIO eventfd fixes "Michael S. Tsirkin" <mst@redhat.com> - 2015-09-11 10:20 +0200
      Re: [PATCH V4 0/4] Fast MMIO eventfd fixes Paolo Bonzini <pbonzini@redhat.com> - 2015-09-11 10:40 +0200
        Re: [PATCH V4 0/4] Fast MMIO eventfd fixes Jason Wang <jasowang@redhat.com> - 2015-09-11 11:30 +0200
          Re: [PATCH V4 0/4] Fast MMIO eventfd fixes "Michael S. Tsirkin" <mst@redhat.com> - 2015-09-13 11:00 +0200
        Re: [PATCH V4 0/4] Fast MMIO eventfd fixes "Michael S. Tsirkin" <mst@redhat.com> - 2015-09-13 11:00 +0200

#1222510 — [PATCH V4 0/4] Fast MMIO eventfd fixes

FromJason Wang <jasowang@redhat.com>
Date2015-09-11 05:20 +0200
Subject[PATCH V4 0/4] Fast MMIO eventfd fixes
Message-ID<q7n2x-645-3@gated-at.bofh.it>
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. 2 is fixed by ignore
the actual length if the length of iodev is zero in kvm_io_bus_cmp().

Please review.

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 (4):
  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

 arch/x86/kvm/trace.h |  18 ++++++++
 arch/x86/kvm/vmx.c   |   1 +
 arch/x86/kvm/x86.c   |   1 +
 virt/kvm/eventfd.c   | 124 ++++++++++++++++++++++++++++++---------------------
 virt/kvm/kvm_main.c  |   4 +-
 5 files changed, 96 insertions(+), 52 deletions(-)

-- 
2.1.4

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [next] | [standalone]

#1222511 — [PATCH V4 3/4] kvm: fix zero length mmio searching

FromJason Wang <jasowang@redhat.com>
Date2015-09-11 05:20 +0200
Subject[PATCH V4 3/4] kvm: fix zero length mmio searching
Message-ID<q7n2y-645-11@gated-at.bofh.it>
In reply to#1222510
Currently, if we had a zero length mmio eventfd assigned on
KVM_MMIO_BUS. It will never found by kvm_io_bus_cmp() since it always
compare the kvm_io_range() with the length that guest wrote. This will
lead 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: Gleb Natapov <gleb@kernel.org>
Cc: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Jason Wang <jasowang@redhat.com>
---
 virt/kvm/kvm_main.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
index d8db2f8f..d4c3b66 100644
--- a/virt/kvm/kvm_main.c
+++ b/virt/kvm/kvm_main.c
@@ -3071,9 +3071,11 @@ 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)
 {
+	int len = r2->len ? r1->len : 0;
+
 	if (r1->addr < r2->addr)
 		return -1;
-	if (r1->addr + r1->len > r2->addr + r2->len)
+	if (r1->addr + len > r2->addr + r2->len)
 		return 1;
 	return 0;
 }
-- 
2.1.4

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1222619 — Re: [PATCH V4 3/4] kvm: fix zero length mmio searching

FromPaolo Bonzini <pbonzini@redhat.com>
Date2015-09-11 10:30 +0200
SubjectRe: [PATCH V4 3/4] kvm: fix zero length mmio searching
Message-ID<q7rSy-4w9-19@gated-at.bofh.it>
In reply to#1222511

On 11/09/2015 05:17, Jason Wang wrote:
> +	int len = r2->len ? r1->len : 0;
> +
>  	if (r1->addr < r2->addr)
>  		return -1;
> -	if (r1->addr + r1->len > r2->addr + r2->len)
> +	if (r1->addr + len > r2->addr + r2->len)
>  		return 1;

Perhaps better:

	gpa_t addr1 = r1->addr;
	gpa_t addr2 = r2->addr;

	if (addr1 < addr2)
		return -1;

	/* 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;
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1222624 — Re: [PATCH V4 3/4] kvm: fix zero length mmio searching

FromCornelia Huck <cornelia.huck@de.ibm.com>
Date2015-09-11 10:40 +0200
SubjectRe: [PATCH V4 3/4] kvm: fix zero length mmio searching
Message-ID<q7s2e-4Hu-25@gated-at.bofh.it>
In reply to#1222619
On Fri, 11 Sep 2015 10:26:41 +0200
Paolo Bonzini <pbonzini@redhat.com> wrote:

> On 11/09/2015 05:17, Jason Wang wrote:
> > +	int len = r2->len ? r1->len : 0;
> > +
> >  	if (r1->addr < r2->addr)
> >  		return -1;
> > -	if (r1->addr + r1->len > r2->addr + r2->len)
> > +	if (r1->addr + len > r2->addr + r2->len)
> >  		return 1;
> 
> Perhaps better:
> 
> 	gpa_t addr1 = r1->addr;
> 	gpa_t addr2 = r2->addr;
> 
> 	if (addr1 < addr2)
> 		return -1;
> 
> 	/* 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;
> 

+1 to documenting what the semantics are :)

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1222644 — Re: [PATCH V4 3/4] kvm: fix zero length mmio searching

FromJason Wang <jasowang@redhat.com>
Date2015-09-11 11:30 +0200
SubjectRe: [PATCH V4 3/4] kvm: fix zero length mmio searching
Message-ID<q7sOC-5Sc-17@gated-at.bofh.it>
In reply to#1222624

On 09/11/2015 04:31 PM, Cornelia Huck wrote:
> On Fri, 11 Sep 2015 10:26:41 +0200
> Paolo Bonzini <pbonzini@redhat.com> wrote:
>
>> On 11/09/2015 05:17, Jason Wang wrote:
>>> +	int len = r2->len ? r1->len : 0;
>>> +
>>>  	if (r1->addr < r2->addr)
>>>  		return -1;
>>> -	if (r1->addr + r1->len > r2->addr + r2->len)
>>> +	if (r1->addr + len > r2->addr + r2->len)
>>>  		return 1;
>> Perhaps better:
>>
>> 	gpa_t addr1 = r1->addr;
>> 	gpa_t addr2 = r2->addr;
>>
>> 	if (addr1 < addr2)
>> 		return -1;
>>
>> 	/* 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;
>>
> +1 to documenting what the semantics are :)
>

Right, better. Will fix this in V5.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1222605

From"Michael S. Tsirkin" <mst@redhat.com>
Date2015-09-11 10:20 +0200
Message-ID<q7rIR-4kB-3@gated-at.bofh.it>
In reply to#1222510
On Fri, Sep 11, 2015 at 11:17:33AM +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. 2 is fixed by ignore
> the actual length if the length of iodev is zero in kvm_io_bus_cmp().
> 
> Please review.

I think we should add a capability for fast mmio.
This way, userspace can avoid crashing buggy kernels.

> 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 (4):
>   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
> 
>  arch/x86/kvm/trace.h |  18 ++++++++
>  arch/x86/kvm/vmx.c   |   1 +
>  arch/x86/kvm/x86.c   |   1 +
>  virt/kvm/eventfd.c   | 124 ++++++++++++++++++++++++++++++---------------------
>  virt/kvm/kvm_main.c  |   4 +-
>  5 files changed, 96 insertions(+), 52 deletions(-)
> 
> -- 
> 2.1.4
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1222627

FromPaolo Bonzini <pbonzini@redhat.com>
Date2015-09-11 10:40 +0200
Message-ID<q7s2e-4Hu-27@gated-at.bofh.it>
In reply to#1222605

On 11/09/2015 10:15, Michael S. Tsirkin wrote:
> I think we should add a capability for fast mmio.
> This way, userspace can avoid crashing buggy kernels.

I agree.

Paolo
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1222643

FromJason Wang <jasowang@redhat.com>
Date2015-09-11 11:30 +0200
Message-ID<q7sOC-5Sc-11@gated-at.bofh.it>
In reply to#1222627

On 09/11/2015 04:33 PM, Paolo Bonzini wrote:
>
> On 11/09/2015 10:15, Michael S. Tsirkin wrote:
>> I think we should add a capability for fast mmio.
>> This way, userspace can avoid crashing buggy kernels.
> I agree.
>
> Paolo

Right, then qemu will use datamatch eventfd if kenrel dost not have the
capability.

Thanks

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1223561

From"Michael S. Tsirkin" <mst@redhat.com>
Date2015-09-13 11:00 +0200
Message-ID<q8biF-2Kd-5@gated-at.bofh.it>
In reply to#1222643
On Fri, Sep 11, 2015 at 05:28:29PM +0800, Jason Wang wrote:
> 
> 
> On 09/11/2015 04:33 PM, Paolo Bonzini wrote:
> >
> > On 11/09/2015 10:15, Michael S. Tsirkin wrote:
> >> I think we should add a capability for fast mmio.
> >> This way, userspace can avoid crashing buggy kernels.
> > I agree.
> >
> > Paolo
> 
> Right, then qemu will use datamatch eventfd if kenrel dost not have the
> capability.
> 
> Thanks

Wildcard is sufficient I think.

-- 
MST
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1223562

From"Michael S. Tsirkin" <mst@redhat.com>
Date2015-09-13 11:00 +0200
Message-ID<q8biG-2Kd-11@gated-at.bofh.it>
In reply to#1222627
On Fri, Sep 11, 2015 at 10:33:08AM +0200, Paolo Bonzini wrote:
> 
> 
> On 11/09/2015 10:15, Michael S. Tsirkin wrote:
> > I think we should add a capability for fast mmio.
> > This way, userspace can avoid crashing buggy kernels.
> 
> I agree.
> 
> Paolo

Having said that, we can merge these patches directly and add
capability on top.
Cc stable for all of them including the capability.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web