[V4,3/4] kvm: fix zero length mmio searching
diff mbox series

Message ID 1441941457-23630-4-git-send-email-jasowang@redhat.com
State New, archived
Headers show
Series
  • Fast MMIO eventfd fixes
Related show

Commit Message

Jason Wang Sept. 11, 2015, 3:17 a.m. UTC
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(-)

Comments

Paolo Bonzini Sept. 11, 2015, 8:26 a.m. UTC | #1
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/
Cornelia Huck Sept. 11, 2015, 8:31 a.m. UTC | #2
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/
Jason Wang Sept. 11, 2015, 9:26 a.m. UTC | #3
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/

Patch
diff mbox series

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;
 }