Re: [Qemu-devel] [PATCH] virtio: Implement userspace forwarding for host notifiers

[Michael S. Tsirkin]

On Thu, Nov 19, 2015 at 01:06:17PM +0100, Paolo Bonzini wrote:
> 
> 
> On 19/11/2015 13:01, Michael S. Tsirkin wrote:
> >> >  You know... I took a look at this, and yes, i could simply hook up 
> >> > emulation into memory region handlers. And everything that
> >> > expects KVM eventfd binding will magically start working, probably 
> >> > rendering some bypass code obsolete.
> >> >  I have only one concern against this. qemu is a large piece of 
> >> > software, consisting of lots of components. I cannot test absolutely
> >> > everything in every configuration. I suggest, old code was written with 
> >> > the assumption that if memory_region_add_eventfd() works, we
> >> > are really using KVM acceleration. If we break this assumption, how much 
> >> > code will mysteriously misbehave instead of throwing
> > First of all, memory_region_add_eventfd simply exits on failure.
> > It seems unlikely you will break something which isn't already
> > broken.
> > 
> > Further:
> > 
> > $ git grep memory_region_add_eventfd
> > hw/misc/ivshmem.c:    memory_region_add_eventfd(&s->ivshmem_mmio,
> > hw/misc/pci-testdev.c:    memory_region_add_eventfd(test->mr,
> > hw/virtio/virtio-mmio.c:        memory_region_add_eventfd(&proxy->iomem, 
> > VIRTIO_MMIO_QUEUENOTIFY, 4,
> > hw/virtio/virtio-pci.c:                memory_region_add_eventfd(modern_mr, 
> > modern_addr, 0,
> > hw/virtio/virtio-pci.c:                memory_region_add_eventfd(modern_mr, 
> > modern_addr, 2,
> > hw/virtio/virtio-pci.c:                
> > memory_region_add_eventfd(modern_notify_mr, 0, 2,
> > hw/virtio/virtio-pci.c:            memory_region_add_eventfd(legacy_mr, 
> > legacy_addr, 2,
> > include/exec/memory.h: * memory_region_add_eventfd: Request an eventfd to 
> > be triggered when a word
> > include/exec/memory.h:void memory_region_add_eventfd(MemoryRegion *mr,
> > include/exec/memory.h: * memory_region_add_eventfd() call.
> > memory.c:void memory_region_add_eventfd(MemoryRegion *mr,
> > 
> > Not such a bit deal to audit all call sites, is it?
> > 
> > Cc memory API maintainer for an opinion.
> > 
> > Paolo, do you see anything wrong with making
> > memory_region_add_eventfd work (slowly) without kvm ioeventfd support?
> > 
> 
> Sure, it's even been on the todo list for a while.  Stefan Hajnoczi had
> a patch, the only ugly thing was that it slowed down a little all
> non-ioeventfd accesses, but I guess that's okay because it's probably
> not measurable.
> 
> http://lists.nongnu.org/archive/html/qemu-devel/2015-07/msg04710.html
> 
> Paolo
> 
> Paolo

Maybe it can be imporved to probe the kernel and only take action if kvm
does not support ioeventfd.

-- 
MST