[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH] softmmu/memory: fix memory_region_ioeventfd_equal()
From: |
Stefan Hajnoczi |
Subject: |
Re: [PATCH] softmmu/memory: fix memory_region_ioeventfd_equal() |
Date: |
Mon, 19 Oct 2020 11:25:27 +0100 |
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 <eafanasova@gmail.com>
> ---
> 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, &kvm->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