Re: [PATCH] softmmu/memory: fix memory_region_ioeventfd_equal()

2020-10-19 Thread Stefan Hajnoczi 
On Sat, Oct 17, 2020 at 02:01:02PM -0700, Elena Afanasova wrote:
> Eventfd can be registered with a zero length when fast_mmio is true.
> Handle this case properly when dispatching through QEMU.
> 
> Signed-off-by: Elena Afanasova 
> ---
>  softmmu/memory.c | 15 +++
>  1 file changed, 11 insertions(+), 4 deletions(-)
> 
> diff --git a/softmmu/memory.c b/softmmu/memory.c
> index 403ff3abc9..3ca2154a64 100644
> --- a/softmmu/memory.c
> +++ b/softmmu/memory.c
> @@ -203,10 +203,17 @@ static bool 
> memory_region_ioeventfd_before(MemoryRegionIoeventfd *a,
>  }
>  
>  static bool memory_region_ioeventfd_equal(MemoryRegionIoeventfd *a,
> -  MemoryRegionIoeventfd *b)
> -{
> -return !memory_region_ioeventfd_before(a, b)
> -&& !memory_region_ioeventfd_before(b, a);
> +  MemoryRegionIoeventfd *mrb)

Why rename b -> mrb?

> +{
> +if (int128_eq(a->addr.start, mrb->addr.start) &&
> +(!int128_nz(mrb->addr.size) ||
> + int128_eq(a->addr.size, mrb->addr.size)) &&
> +(a->match_data == mrb->match_data) &&
> +((mrb->match_data && (a->data == mrb->data)) || 
> !mrb->match_data) &&
> +(a->e == mrb->e))
> +return true;

The kernel behaves differently in that a and b are symmetric:

  static bool
  ioeventfd_check_collision(struct kvm *kvm, struct _ioeventfd *p)
  {
struct _ioeventfd *_p;

list_for_each_entry(_p, >ioeventfds, list)
if (_p->bus_idx == p->bus_idx &&
_p->addr == p->addr &&
(!_p->length || !p->length ||
 (_p->length == p->length &&
  (_p->wildcard || p->wildcard ||
   _p->datamatch == p->datamatch
return true;

return false;
  }

For example, if a->addr.size == 0 then the ioeventfd matches whereas
this patch only does it for b.

Please match the kernel behavior.

A different approach is setting ioeventfd.addr.size to zero when
mr->ioeventfds[i].addr.size is zero in the
memory_region_dispatch_write_eventfds() loop. That way
memory_region_ioeventfd_equal() doesn't need to be modified.


signature.asc
Description: PGP signature

[PATCH] softmmu/memory: fix memory_region_ioeventfd_equal()

2020-10-17 Thread Elena Afanasova 
Eventfd can be registered with a zero length when fast_mmio is true.
Handle this case properly when dispatching through QEMU.

Signed-off-by: Elena Afanasova 
---
 softmmu/memory.c | 15 +++
 1 file changed, 11 insertions(+), 4 deletions(-)

diff --git a/softmmu/memory.c b/softmmu/memory.c
index 403ff3abc9..3ca2154a64 100644
--- a/softmmu/memory.c
+++ b/softmmu/memory.c
@@ -203,10 +203,17 @@ static bool 
memory_region_ioeventfd_before(MemoryRegionIoeventfd *a,
 }
 
 static bool memory_region_ioeventfd_equal(MemoryRegionIoeventfd *a,
-  MemoryRegionIoeventfd *b)
-{
-return !memory_region_ioeventfd_before(a, b)
-&& !memory_region_ioeventfd_before(b, a);
+  MemoryRegionIoeventfd *mrb)
+{
+if (int128_eq(a->addr.start, mrb->addr.start) &&
+(!int128_nz(mrb->addr.size) ||
+ int128_eq(a->addr.size, mrb->addr.size)) &&
+(a->match_data == mrb->match_data) &&
+((mrb->match_data && (a->data == mrb->data)) || !mrb->match_data) 
&&
+(a->e == mrb->e))
+return true;
+
+return false;
 }
 
 /* Range of memory in the global map.  Addresses are absolute. */
-- 
2.25.1