Re: [PATCH 1/1] virtio-pci: return RAM device MR when set host notifier success

[Jason Wang]

On Fri, Aug 16, 2024 at 6:29 PM Michael S. Tsirkin <mst@redhat.com> wrote:
>
> On Mon, Aug 12, 2024 at 08:20:27PM +0800, Gao Shiyuan wrote:
> > When vhost-user backend register memory region based host notifiers,
> > we should return RAM device MR of notify region MR's subregion in
> > virtio_address_space_lookup.
> >
> > In seabios, it will use virtio PCI Configration Access Capability
> > access notify region when assign notify region above 4GB. This will
> > exit to QEMU and invoke virtio_address_space_write. When vhost-user
> > backend register memory region based host notifiers, return RAM device
> > MR instead of notify region MR is suitable.
>
>
> I can't really parse this.
>
> > Co-developed-by: Zuo Boqun <zuoboqun@baidu.com>
> > Signed-off-by: Gao Shiyuan <gaoshiyuan@baidu.com>
> > Signed-off-by: Zuo Boqun <zuoboqun@baidu.com>
>
> CC Jason
>
> > ---
> >  hw/virtio/virtio-pci.c | 11 ++++++++++-
> >  1 file changed, 10 insertions(+), 1 deletion(-)
> >
> > diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c
> > index 9534730bba..167ac9718a 100644
> > --- a/hw/virtio/virtio-pci.c
> > +++ b/hw/virtio/virtio-pci.c
> > @@ -610,13 +610,22 @@ static MemoryRegion 
> > *virtio_address_space_lookup(VirtIOPCIProxy *proxy,
> >  {
> >      int i;
> >      VirtIOPCIRegion *reg;
> > +    MemoryRegion *mr, *submr;
> >
> >      for (i = 0; i < ARRAY_SIZE(proxy->regs); ++i) {
> >          reg = &proxy->regs[i];
> >          if (*off >= reg->offset &&
> >              *off + len <= reg->offset + reg->size) {
> >              *off -= reg->offset;
> > -            return &reg->mr;
> > +            mr = &reg->mr;
> > +            QTAILQ_FOREACH(submr, &mr->subregions, subregions_link) {
> > +                if (*off >= submr->addr &&
> > +                    *off + len < submr->addr + submr->size) {
> > +                    *off -= submr->addr;
> > +                    return submr;
> > +                }
> > +            }
> > +            return mr;
> >          }
> >      }
>
> Poking at internals of MR like this is not nice.
> Doesn't memory_region_find work for this?

Or I wonder if this is a side effect of:

commit a93c8d828af186d9a6a1c915a1be8ba22fb89849
Author: Alexey Kardashevskiy <aik@ozlabs.ru>
Date:   Mon Oct 9 14:19:41 2017 +1100

    virtio-pci: Replace modern_as with direct access to modern_bar

    The modern bar is accessed now via yet another address space created just
    for that purpose and it does not really need FlatView and dispatch tree
    as it has a single memory region so it is just a waste of memory. Things
    get even worse when there are dozens or hundreds of virtio-pci devices -
    since these address spaces are global, changing any of them triggers
    rebuilding all address spaces.

    This replaces indirect accesses to the modern BAR with a simple lookup
    and direct calls to memory_region_dispatch_read/write.

    This is expected to save lots of memory at boot time after applying:
    [Qemu-devel] [PULL 00/32] Misc changes for 2017-09-22

    Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
    Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
    Signed-off-by: Michael S. Tsirkin <mst@redhat.com>

Thanks