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

[Michael S. Tsirkin]

On Tue, Aug 20, 2024 at 07:56:31PM +0800, Gao Shiyuan wrote:
> When VHOST_USER_PROTOCOL_F_HOST_NOTIFIER feature negotiated and
> virtio_queue_set_host_notifier_mr success on system blk
> device's queue, the VM can't load MBR if the notify region's
> address above 4GB.
> 
> Assign the address of notify region in the modern bar above 4G, the vp_notify
> in SeaBIOS will use PCI Cfg Capability to write notify region. This will trap
> into QEMU and be handled by the host bridge when we don't enable mmconfig.
> QEMU will call virtio_write_config and since it writes to the BAR region
> through the PCI Cfg Capability, it will call virtio_address_space_write.
> 
> virtio_queue_set_host_notifier_mr add host notifier subregion of notify region
> MR, QEMU need write the mmap address instead of eventfd notify the hardware
> accelerator at the vhost-user backend. So virtio_address_space_lookup in
> virtio_address_space_write need return a host-notifier subregion of notify MR
> instead of notify MR.
> 
> 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 <zuobo...@baidu.com>
> Signed-off-by: Gao Shiyuan <gaoshiy...@baidu.com>
> Signed-off-by: Zuo Boqun <zuobo...@baidu.com>
> ---
>  hw/virtio/virtio-pci.c | 14 ++++++++++++--
>  1 file changed, 12 insertions(+), 2 deletions(-)
> 
> ---
> 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 9534730bba..5d2d27a6a3 100644
> --- a/hw/virtio/virtio-pci.c
> +++ b/hw/virtio/virtio-pci.c
> @@ -610,19 +610,29 @@ static MemoryRegion 
> *virtio_address_space_lookup(VirtIOPCIProxy *proxy,
>  {
>      int i;
>      VirtIOPCIRegion *reg;
> +    MemoryRegion *mr = NULL;
> +    MemoryRegionSection mrs;
>  
>      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;
> +            mrs = memory_region_find(&reg->mr, *off - reg->offset, len);
> +            if (!mrs.mr) {
> +                error_report("Failed to find memory region for address"
> +                             "0x%" PRIx64 "", *off);
> +                return NULL;
> +            }


I'm not sure when can this happen. If it can't assert will do.


> +            *off = mrs.offset_within_region;
> +            memory_region_unref(mrs.mr);
> +            return mrs.mr;
>          }
>      }
>  
>      return NULL;
>  }
>  
> +
>  /* Below are generic functions to do memcpy from/to an address space,
>   * without byteswaps, with input validation.
>   *
> -- 
> 2.39.3 (Apple Git-146)