Re: [PATCH v3 1/1] virtio-pci: Add lookup subregion of VirtIOPCIRegion MR

[David Hildenbrand <david@redhat.com>]

On 03.09.24 14:03, Gao Shiyuan via wrote:
> Now virtio_address_space_lookup only lookup common/isr/device/notify
> MR and exclude their subregions.
> 
> When VHOST_USER_PROTOCOL_F_HOST_NOTIFIER enable, the notify MR has
> host-notifier subregions and we need use host-notifier MR to
> notify the hardware accelerator directly instead of eventfd notify.
> 
> Further more, maybe common/isr/device MR also has subregions in
> the future, so need memory_region_find for each MR incluing
> their subregions.
> 
> Add lookup subregion of VirtIOPCIRegion MR instead of only lookup container MR.
> 
> Fixes: a93c8d8 ("virtio-pci: Replace modern_as with direct access to modern_bar")
> 
> Co-developed-by: Zuo Boqun <zuoboqun@baidu.com>
> Signed-off-by: Gao Shiyuan <gaoshiyuan@baidu.com>
> Signed-off-by: Zuo Boqun <zuoboqun@baidu.com>
> ---
>   hw/virtio/virtio-pci.c | 8 ++++++--
>   1 file changed, 6 insertions(+), 2 deletions(-)
> 
> ---
> v2 -> v3:
> * modify commit message
> * remove unused variable and move mrs to the inner block
> * replace error_report with assert
> 
> v1 -> v2:
> * modify commit message
> * replace direct iteration over subregions with memory_region_find.
> 
> diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c
> index 524b63e5c7..4d832fe845 100644
> --- a/hw/virtio/virtio-pci.c
> +++ b/hw/virtio/virtio-pci.c
> @@ -615,8 +615,12 @@ static MemoryRegion *virtio_address_space_lookup(VirtIOPCIProxy *proxy,
>           reg = &proxy->regs[i];
>           if (*off >= reg->offset &&
>               *off + len <= reg->offset + reg->size) {
> -            *off -= reg->offset;
> -            return &reg->mr;
> +            MemoryRegionSection mrs = memory_region_find(&reg->mr,
> +                                        *off - reg->offset, len);
> +            assert(mrs.mr);

We are able to trigger that assert:

https://gitlab.com/qemu-project/qemu/-/issues/2576

Can you take a look and send a fix?

-- 
Cheers,

David / dhildenb