accel/kvm/kvm-all.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-)
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
Signed-off-by: Elena Afanasova <eafanasova@gmail.com>
---
accel/kvm/kvm-all.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/accel/kvm/kvm-all.c b/accel/kvm/kvm-all.c
index 9ef5daf4c5..baaa54249d 100644
--- a/accel/kvm/kvm-all.c
+++ b/accel/kvm/kvm-all.c
@@ -2239,8 +2239,10 @@ static int kvm_init(MachineState *ms)
kvm_memory_listener_register(s, &s->memory_listener,
&address_space_memory, 0);
- memory_listener_register(&kvm_io_listener,
- &address_space_io);
+ if (kvm_eventfds_allowed) {
+ memory_listener_register(&kvm_io_listener,
+ &address_space_io);
+ }
memory_listener_register(&kvm_coalesced_pio_listener,
&address_space_io);
--
2.25.1
On Sat, Oct 17, 2020 at 02:01:01PM -0700, Elena Afanasova wrote: > Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> > Signed-off-by: Elena Afanasova <eafanasova@gmail.com> > --- > accel/kvm/kvm-all.c | 6 ++++-- > 1 file changed, 4 insertions(+), 2 deletions(-) Additional information for the commit description: MMIO eventfd_add/del are only registered when kvm_eventfds_allowed is true but for PIO they are registered unconditionally. This is a bug because kvm.ko ioeventfd should not be used if kvm_eventfds_allowed is false. This issue is a latent bug because ioeventfd is usually available when KVM is enabled. > > diff --git a/accel/kvm/kvm-all.c b/accel/kvm/kvm-all.c > index 9ef5daf4c5..baaa54249d 100644 > --- a/accel/kvm/kvm-all.c > +++ b/accel/kvm/kvm-all.c > @@ -2239,8 +2239,10 @@ static int kvm_init(MachineState *ms) > > kvm_memory_listener_register(s, &s->memory_listener, > &address_space_memory, 0); > - memory_listener_register(&kvm_io_listener, > - &address_space_io); > + if (kvm_eventfds_allowed) { > + memory_listener_register(&kvm_io_listener, > + &address_space_io); > + } > memory_listener_register(&kvm_coalesced_pio_listener, > &address_space_io); > > -- > 2.25.1 >
On Sat, Oct 17, 2020 at 02:01:01PM -0700, Elena Afanasova wrote: > Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> > Signed-off-by: Elena Afanasova <eafanasova@gmail.com> > --- > accel/kvm/kvm-all.c | 6 ++++-- > 1 file changed, 4 insertions(+), 2 deletions(-) Thanks, applied just this patch to my block-next tree: https://github.com/stefanha/qemu/commits/block-next Stefan
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)
+{
+ 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
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.
© 2016 - 2024 Red Hat, Inc.