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

[Paolo Bonzini]

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 kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

[Cornelia Huck]

On Fri, 11 Sep 2015 10:26:41 +0200
Paolo Bonzini  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 kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

[Jason Wang]

On 09/11/2015 04:31 PM, Cornelia Huck wrote:
> On Fri, 11 Sep 2015 10:26:41 +0200
> Paolo Bonzini  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 kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH V4 3/4] 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 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 
Cc: Paolo Bonzini 
Signed-off-by: Jason Wang 
---
 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 kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html