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

[Gao,Shiyuan]

> >--- 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;
>
> `mr` looks unused.
>
> >+    MemoryRegionSection mrs;
>
> Please, can you move this declaration in the inner block where it's
> used?

ok, I will move to inner block and remove unused `mr`.

>
> >
> >     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;
> >+            }
> >+            *off = mrs.offset_within_region;
> >+            memory_region_unref(mrs.mr);
> >+            return mrs.mr;
> >         }
> >     }
> >
> >     return NULL;
> > }
> >
> >+
>
> Unrelated change.

Perhaps this would be clearer but not universal in Version 1.

Without this patch, 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.

Further more, maybe common/isr/device MR also has subregions in
the future, so need memory_region_find for each MR incluing
their subregions and this will be more universal.

@@ -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;
         }
     }