* [PATCH] vdpa: do not save failed dma maps in SVQ iova tree
@ 2022-08-02 14:39 Eugenio Pérez
2022-08-02 14:48 ` Eugenio Perez Martin
2022-08-03 8:09 ` Jason Wang
0 siblings, 2 replies; 7+ messages in thread
From: Eugenio Pérez @ 2022-08-02 14:39 UTC (permalink / raw)
To: qemu-devel; +Cc: Jason Wang, Michael S. Tsirkin
If a map fails for whatever reason, it must not be saved in the tree.
Otherwise, qemu will try to unmap it in cleanup, leaving to more errors.
Fixes: 34e3c94eda ("vdpa: Add custom IOTLB translations to SVQ")
Signed-off-by: Eugenio Pérez <eperezma@redhat.com>
---
hw/virtio/vhost-vdpa.c | 20 +++++++++++++-------
1 file changed, 13 insertions(+), 7 deletions(-)
diff --git a/hw/virtio/vhost-vdpa.c b/hw/virtio/vhost-vdpa.c
index 3ff9ce3501..e44c23dce5 100644
--- a/hw/virtio/vhost-vdpa.c
+++ b/hw/virtio/vhost-vdpa.c
@@ -176,6 +176,7 @@ static void vhost_vdpa_listener_commit(MemoryListener *listener)
static void vhost_vdpa_listener_region_add(MemoryListener *listener,
MemoryRegionSection *section)
{
+ DMAMap mem_region = {};
struct vhost_vdpa *v = container_of(listener, struct vhost_vdpa, listener);
hwaddr iova;
Int128 llend, llsize;
@@ -212,13 +213,13 @@ static void vhost_vdpa_listener_region_add(MemoryListener *listener,
llsize = int128_sub(llend, int128_make64(iova));
if (v->shadow_vqs_enabled) {
- DMAMap mem_region = {
- .translated_addr = (hwaddr)(uintptr_t)vaddr,
- .size = int128_get64(llsize) - 1,
- .perm = IOMMU_ACCESS_FLAG(true, section->readonly),
- };
+ int r;
- int r = vhost_iova_tree_map_alloc(v->iova_tree, &mem_region);
+ mem_region.translated_addr = (hwaddr)(uintptr_t)vaddr,
+ mem_region.size = int128_get64(llsize) - 1,
+ mem_region.perm = IOMMU_ACCESS_FLAG(true, section->readonly),
+
+ r = vhost_iova_tree_map_alloc(v->iova_tree, &mem_region);
if (unlikely(r != IOVA_OK)) {
error_report("Can't allocate a mapping (%d)", r);
goto fail;
@@ -232,11 +233,16 @@ static void vhost_vdpa_listener_region_add(MemoryListener *listener,
vaddr, section->readonly);
if (ret) {
error_report("vhost vdpa map fail!");
- goto fail;
+ goto fail_map;
}
return;
+fail_map:
+ if (v->shadow_vqs_enabled) {
+ vhost_iova_tree_remove(v->iova_tree, &mem_region);
+ }
+
fail:
/*
* On the initfn path, store the first error in the container so we
--
2.31.1
^ permalink raw reply related [flat|nested] 7+ messages in thread* Re: [PATCH] vdpa: do not save failed dma maps in SVQ iova tree 2022-08-02 14:39 [PATCH] vdpa: do not save failed dma maps in SVQ iova tree Eugenio Pérez @ 2022-08-02 14:48 ` Eugenio Perez Martin 2022-08-03 8:09 ` Jason Wang 1 sibling, 0 replies; 7+ messages in thread From: Eugenio Perez Martin @ 2022-08-02 14:48 UTC (permalink / raw) To: qemu-level; +Cc: Jason Wang, Michael S. Tsirkin On Tue, Aug 2, 2022 at 4:41 PM Eugenio Pérez <eperezma@redhat.com> wrote: > > If a map fails for whatever reason, it must not be saved in the tree. > Otherwise, qemu will try to unmap it in cleanup, leaving to more errors. > I forgot to add: Reported-by: Lei Yang <leiyang@redhat.com> > Fixes: 34e3c94eda ("vdpa: Add custom IOTLB translations to SVQ") > Signed-off-by: Eugenio Pérez <eperezma@redhat.com> > --- > hw/virtio/vhost-vdpa.c | 20 +++++++++++++------- > 1 file changed, 13 insertions(+), 7 deletions(-) > > diff --git a/hw/virtio/vhost-vdpa.c b/hw/virtio/vhost-vdpa.c > index 3ff9ce3501..e44c23dce5 100644 > --- a/hw/virtio/vhost-vdpa.c > +++ b/hw/virtio/vhost-vdpa.c > @@ -176,6 +176,7 @@ static void vhost_vdpa_listener_commit(MemoryListener *listener) > static void vhost_vdpa_listener_region_add(MemoryListener *listener, > MemoryRegionSection *section) > { > + DMAMap mem_region = {}; > struct vhost_vdpa *v = container_of(listener, struct vhost_vdpa, listener); > hwaddr iova; > Int128 llend, llsize; > @@ -212,13 +213,13 @@ static void vhost_vdpa_listener_region_add(MemoryListener *listener, > > llsize = int128_sub(llend, int128_make64(iova)); > if (v->shadow_vqs_enabled) { > - DMAMap mem_region = { > - .translated_addr = (hwaddr)(uintptr_t)vaddr, > - .size = int128_get64(llsize) - 1, > - .perm = IOMMU_ACCESS_FLAG(true, section->readonly), > - }; > + int r; > > - int r = vhost_iova_tree_map_alloc(v->iova_tree, &mem_region); > + mem_region.translated_addr = (hwaddr)(uintptr_t)vaddr, > + mem_region.size = int128_get64(llsize) - 1, > + mem_region.perm = IOMMU_ACCESS_FLAG(true, section->readonly), > + > + r = vhost_iova_tree_map_alloc(v->iova_tree, &mem_region); > if (unlikely(r != IOVA_OK)) { > error_report("Can't allocate a mapping (%d)", r); > goto fail; > @@ -232,11 +233,16 @@ static void vhost_vdpa_listener_region_add(MemoryListener *listener, > vaddr, section->readonly); > if (ret) { > error_report("vhost vdpa map fail!"); > - goto fail; > + goto fail_map; > } > > return; > > +fail_map: > + if (v->shadow_vqs_enabled) { > + vhost_iova_tree_remove(v->iova_tree, &mem_region); > + } > + > fail: > /* > * On the initfn path, store the first error in the container so we > -- > 2.31.1 > > ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] vdpa: do not save failed dma maps in SVQ iova tree 2022-08-02 14:39 [PATCH] vdpa: do not save failed dma maps in SVQ iova tree Eugenio Pérez 2022-08-02 14:48 ` Eugenio Perez Martin @ 2022-08-03 8:09 ` Jason Wang 2022-08-03 8:12 ` Eugenio Perez Martin 1 sibling, 1 reply; 7+ messages in thread From: Jason Wang @ 2022-08-03 8:09 UTC (permalink / raw) To: Eugenio Pérez; +Cc: qemu-devel, Michael S. Tsirkin On Tue, Aug 2, 2022 at 10:39 PM Eugenio Pérez <eperezma@redhat.com> wrote: > > If a map fails for whatever reason, it must not be saved in the tree. > Otherwise, qemu will try to unmap it in cleanup, leaving to more errors. > > Fixes: 34e3c94eda ("vdpa: Add custom IOTLB translations to SVQ") > Signed-off-by: Eugenio Pérez <eperezma@redhat.com> > --- > hw/virtio/vhost-vdpa.c | 20 +++++++++++++------- > 1 file changed, 13 insertions(+), 7 deletions(-) > > diff --git a/hw/virtio/vhost-vdpa.c b/hw/virtio/vhost-vdpa.c > index 3ff9ce3501..e44c23dce5 100644 > --- a/hw/virtio/vhost-vdpa.c > +++ b/hw/virtio/vhost-vdpa.c > @@ -176,6 +176,7 @@ static void vhost_vdpa_listener_commit(MemoryListener *listener) > static void vhost_vdpa_listener_region_add(MemoryListener *listener, > MemoryRegionSection *section) > { > + DMAMap mem_region = {}; > struct vhost_vdpa *v = container_of(listener, struct vhost_vdpa, listener); > hwaddr iova; > Int128 llend, llsize; > @@ -212,13 +213,13 @@ static void vhost_vdpa_listener_region_add(MemoryListener *listener, > > llsize = int128_sub(llend, int128_make64(iova)); > if (v->shadow_vqs_enabled) { > - DMAMap mem_region = { > - .translated_addr = (hwaddr)(uintptr_t)vaddr, > - .size = int128_get64(llsize) - 1, > - .perm = IOMMU_ACCESS_FLAG(true, section->readonly), > - }; Nit: can we keep this part unchanged? Thanks > + int r; > > - int r = vhost_iova_tree_map_alloc(v->iova_tree, &mem_region); > + mem_region.translated_addr = (hwaddr)(uintptr_t)vaddr, > + mem_region.size = int128_get64(llsize) - 1, > + mem_region.perm = IOMMU_ACCESS_FLAG(true, section->readonly), > + > + r = vhost_iova_tree_map_alloc(v->iova_tree, &mem_region); > if (unlikely(r != IOVA_OK)) { > error_report("Can't allocate a mapping (%d)", r); > goto fail; > @@ -232,11 +233,16 @@ static void vhost_vdpa_listener_region_add(MemoryListener *listener, > vaddr, section->readonly); > if (ret) { > error_report("vhost vdpa map fail!"); > - goto fail; > + goto fail_map; > } > > return; > > +fail_map: > + if (v->shadow_vqs_enabled) { > + vhost_iova_tree_remove(v->iova_tree, &mem_region); > + } > + > fail: > /* > * On the initfn path, store the first error in the container so we > -- > 2.31.1 > ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] vdpa: do not save failed dma maps in SVQ iova tree 2022-08-03 8:09 ` Jason Wang @ 2022-08-03 8:12 ` Eugenio Perez Martin 2022-08-04 4:51 ` Jason Wang 0 siblings, 1 reply; 7+ messages in thread From: Eugenio Perez Martin @ 2022-08-03 8:12 UTC (permalink / raw) To: Jason Wang; +Cc: qemu-devel, Michael S. Tsirkin On Wed, Aug 3, 2022 at 10:09 AM Jason Wang <jasowang@redhat.com> wrote: > > On Tue, Aug 2, 2022 at 10:39 PM Eugenio Pérez <eperezma@redhat.com> wrote: > > > > If a map fails for whatever reason, it must not be saved in the tree. > > Otherwise, qemu will try to unmap it in cleanup, leaving to more errors. > > > > Fixes: 34e3c94eda ("vdpa: Add custom IOTLB translations to SVQ") > > Signed-off-by: Eugenio Pérez <eperezma@redhat.com> > > --- > > hw/virtio/vhost-vdpa.c | 20 +++++++++++++------- > > 1 file changed, 13 insertions(+), 7 deletions(-) > > > > diff --git a/hw/virtio/vhost-vdpa.c b/hw/virtio/vhost-vdpa.c > > index 3ff9ce3501..e44c23dce5 100644 > > --- a/hw/virtio/vhost-vdpa.c > > +++ b/hw/virtio/vhost-vdpa.c > > @@ -176,6 +176,7 @@ static void vhost_vdpa_listener_commit(MemoryListener *listener) > > static void vhost_vdpa_listener_region_add(MemoryListener *listener, > > MemoryRegionSection *section) > > { > > + DMAMap mem_region = {}; > > struct vhost_vdpa *v = container_of(listener, struct vhost_vdpa, listener); > > hwaddr iova; > > Int128 llend, llsize; > > @@ -212,13 +213,13 @@ static void vhost_vdpa_listener_region_add(MemoryListener *listener, > > > > llsize = int128_sub(llend, int128_make64(iova)); > > if (v->shadow_vqs_enabled) { > > - DMAMap mem_region = { > > - .translated_addr = (hwaddr)(uintptr_t)vaddr, > > - .size = int128_get64(llsize) - 1, > > - .perm = IOMMU_ACCESS_FLAG(true, section->readonly), > > - }; > > Nit: can we keep this part unchanged? > We can, but that implies we should look for iova again at fail_map tag. If you are ok with that I'm fine to perform the search again. > Thanks > > > + int r; > > > > - int r = vhost_iova_tree_map_alloc(v->iova_tree, &mem_region); > > + mem_region.translated_addr = (hwaddr)(uintptr_t)vaddr, > > + mem_region.size = int128_get64(llsize) - 1, > > + mem_region.perm = IOMMU_ACCESS_FLAG(true, section->readonly), > > + > > + r = vhost_iova_tree_map_alloc(v->iova_tree, &mem_region); > > if (unlikely(r != IOVA_OK)) { > > error_report("Can't allocate a mapping (%d)", r); > > goto fail; > > @@ -232,11 +233,16 @@ static void vhost_vdpa_listener_region_add(MemoryListener *listener, > > vaddr, section->readonly); > > if (ret) { > > error_report("vhost vdpa map fail!"); > > - goto fail; > > + goto fail_map; > > } > > > > return; > > > > +fail_map: > > + if (v->shadow_vqs_enabled) { > > + vhost_iova_tree_remove(v->iova_tree, &mem_region); > > + } > > + > > fail: > > /* > > * On the initfn path, store the first error in the container so we > > -- > > 2.31.1 > > > ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] vdpa: do not save failed dma maps in SVQ iova tree 2022-08-03 8:12 ` Eugenio Perez Martin @ 2022-08-04 4:51 ` Jason Wang 2022-08-04 15:02 ` Eugenio Perez Martin 0 siblings, 1 reply; 7+ messages in thread From: Jason Wang @ 2022-08-04 4:51 UTC (permalink / raw) To: Eugenio Perez Martin; +Cc: qemu-devel, Michael S. Tsirkin 在 2022/8/3 16:12, Eugenio Perez Martin 写道: > On Wed, Aug 3, 2022 at 10:09 AM Jason Wang <jasowang@redhat.com> wrote: >> On Tue, Aug 2, 2022 at 10:39 PM Eugenio Pérez <eperezma@redhat.com> wrote: >>> If a map fails for whatever reason, it must not be saved in the tree. >>> Otherwise, qemu will try to unmap it in cleanup, leaving to more errors. >>> >>> Fixes: 34e3c94eda ("vdpa: Add custom IOTLB translations to SVQ") >>> Signed-off-by: Eugenio Pérez <eperezma@redhat.com> >>> --- >>> hw/virtio/vhost-vdpa.c | 20 +++++++++++++------- >>> 1 file changed, 13 insertions(+), 7 deletions(-) >>> >>> diff --git a/hw/virtio/vhost-vdpa.c b/hw/virtio/vhost-vdpa.c >>> index 3ff9ce3501..e44c23dce5 100644 >>> --- a/hw/virtio/vhost-vdpa.c >>> +++ b/hw/virtio/vhost-vdpa.c >>> @@ -176,6 +176,7 @@ static void vhost_vdpa_listener_commit(MemoryListener *listener) >>> static void vhost_vdpa_listener_region_add(MemoryListener *listener, >>> MemoryRegionSection *section) >>> { >>> + DMAMap mem_region = {}; >>> struct vhost_vdpa *v = container_of(listener, struct vhost_vdpa, listener); >>> hwaddr iova; >>> Int128 llend, llsize; >>> @@ -212,13 +213,13 @@ static void vhost_vdpa_listener_region_add(MemoryListener *listener, >>> >>> llsize = int128_sub(llend, int128_make64(iova)); >>> if (v->shadow_vqs_enabled) { >>> - DMAMap mem_region = { >>> - .translated_addr = (hwaddr)(uintptr_t)vaddr, >>> - .size = int128_get64(llsize) - 1, >>> - .perm = IOMMU_ACCESS_FLAG(true, section->readonly), >>> - }; >> Nit: can we keep this part unchanged? >> > We can, but that implies we should look for iova again at fail_map > tag. If you are ok with that I'm fine to perform the search again. I meant something like: diff --git a/hw/virtio/vhost-vdpa.c b/hw/virtio/vhost-vdpa.c index 9a2daef7e3..edf40868e3 100644 --- a/hw/virtio/vhost-vdpa.c +++ b/hw/virtio/vhost-vdpa.c @@ -232,11 +232,15 @@ static void vhost_vdpa_listener_region_add(MemoryListener *listener, vaddr, section->readonly); if (ret) { error_report("vhost vdpa map fail!"); - goto fail; + goto fail_unmap; } return; +fail_unmap: + if (v->shadow_vqs_enabled) { + vhost_iova_tree_remove(v->iova_tree, &mem_region); + } fail: /* * On the initfn path, store the first error in the container so we Thanks > >> Thanks >> >>> + int r; >>> >>> - int r = vhost_iova_tree_map_alloc(v->iova_tree, &mem_region); >>> + mem_region.translated_addr = (hwaddr)(uintptr_t)vaddr, >>> + mem_region.size = int128_get64(llsize) - 1, >>> + mem_region.perm = IOMMU_ACCESS_FLAG(true, section->readonly), >>> + >>> + r = vhost_iova_tree_map_alloc(v->iova_tree, &mem_region); >>> if (unlikely(r != IOVA_OK)) { >>> error_report("Can't allocate a mapping (%d)", r); >>> goto fail; >>> @@ -232,11 +233,16 @@ static void vhost_vdpa_listener_region_add(MemoryListener *listener, >>> vaddr, section->readonly); >>> if (ret) { >>> error_report("vhost vdpa map fail!"); >>> - goto fail; >>> + goto fail_map; >>> } >>> >>> return; >>> >>> +fail_map: >>> + if (v->shadow_vqs_enabled) { >>> + vhost_iova_tree_remove(v->iova_tree, &mem_region); >>> + } >>> + >>> fail: >>> /* >>> * On the initfn path, store the first error in the container so we >>> -- >>> 2.31.1 >>> ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH] vdpa: do not save failed dma maps in SVQ iova tree 2022-08-04 4:51 ` Jason Wang @ 2022-08-04 15:02 ` Eugenio Perez Martin 2022-08-04 15:50 ` Eugenio Perez Martin 0 siblings, 1 reply; 7+ messages in thread From: Eugenio Perez Martin @ 2022-08-04 15:02 UTC (permalink / raw) To: Jason Wang; +Cc: qemu-devel, Michael S. Tsirkin On Thu, Aug 4, 2022 at 6:51 AM Jason Wang <jasowang@redhat.com> wrote: > > > 在 2022/8/3 16:12, Eugenio Perez Martin 写道: > > On Wed, Aug 3, 2022 at 10:09 AM Jason Wang <jasowang@redhat.com> wrote: > >> On Tue, Aug 2, 2022 at 10:39 PM Eugenio Pérez <eperezma@redhat.com> wrote: > >>> If a map fails for whatever reason, it must not be saved in the tree. > >>> Otherwise, qemu will try to unmap it in cleanup, leaving to more errors. > >>> > >>> Fixes: 34e3c94eda ("vdpa: Add custom IOTLB translations to SVQ") > >>> Signed-off-by: Eugenio Pérez <eperezma@redhat.com> > >>> --- > >>> hw/virtio/vhost-vdpa.c | 20 +++++++++++++------- > >>> 1 file changed, 13 insertions(+), 7 deletions(-) > >>> > >>> diff --git a/hw/virtio/vhost-vdpa.c b/hw/virtio/vhost-vdpa.c > >>> index 3ff9ce3501..e44c23dce5 100644 > >>> --- a/hw/virtio/vhost-vdpa.c > >>> +++ b/hw/virtio/vhost-vdpa.c > >>> @@ -176,6 +176,7 @@ static void vhost_vdpa_listener_commit(MemoryListener *listener) > >>> static void vhost_vdpa_listener_region_add(MemoryListener *listener, > >>> MemoryRegionSection *section) > >>> { > >>> + DMAMap mem_region = {}; > >>> struct vhost_vdpa *v = container_of(listener, struct vhost_vdpa, listener); > >>> hwaddr iova; > >>> Int128 llend, llsize; > >>> @@ -212,13 +213,13 @@ static void vhost_vdpa_listener_region_add(MemoryListener *listener, > >>> > >>> llsize = int128_sub(llend, int128_make64(iova)); > >>> if (v->shadow_vqs_enabled) { > >>> - DMAMap mem_region = { > >>> - .translated_addr = (hwaddr)(uintptr_t)vaddr, > >>> - .size = int128_get64(llsize) - 1, > >>> - .perm = IOMMU_ACCESS_FLAG(true, section->readonly), > >>> - }; > >> Nit: can we keep this part unchanged? > >> > > We can, but that implies we should look for iova again at fail_map > > tag. If you are ok with that I'm fine to perform the search again. > > > I meant something like: > > diff --git a/hw/virtio/vhost-vdpa.c b/hw/virtio/vhost-vdpa.c > index 9a2daef7e3..edf40868e3 100644 > --- a/hw/virtio/vhost-vdpa.c > +++ b/hw/virtio/vhost-vdpa.c > @@ -232,11 +232,15 @@ static void > vhost_vdpa_listener_region_add(MemoryListener *listener, > vaddr, section->readonly); > if (ret) { > error_report("vhost vdpa map fail!"); > - goto fail; > + goto fail_unmap; > } > > return; > > +fail_unmap: > + if (v->shadow_vqs_enabled) { > + vhost_iova_tree_remove(v->iova_tree, &mem_region); > + } > fail: > /* > * On the initfn path, store the first error in the container so we > Sorry, still not following. mem_region does not exist in the error path, since it's declared in the if (v->shadow_vqs_enabled){} block. We can left first part unchanged if we do a lookup for the mem region, based on the translated addr. Thanks! > Thanks > > > > > >> Thanks > >> > >>> + int r; > >>> > >>> - int r = vhost_iova_tree_map_alloc(v->iova_tree, &mem_region); > >>> + mem_region.translated_addr = (hwaddr)(uintptr_t)vaddr, > >>> + mem_region.size = int128_get64(llsize) - 1, > >>> + mem_region.perm = IOMMU_ACCESS_FLAG(true, section->readonly), > >>> + > >>> + r = vhost_iova_tree_map_alloc(v->iova_tree, &mem_region); > >>> if (unlikely(r != IOVA_OK)) { > >>> error_report("Can't allocate a mapping (%d)", r); > >>> goto fail; > >>> @@ -232,11 +233,16 @@ static void vhost_vdpa_listener_region_add(MemoryListener *listener, > >>> vaddr, section->readonly); > >>> if (ret) { > >>> error_report("vhost vdpa map fail!"); > >>> - goto fail; > >>> + goto fail_map; > >>> } > >>> > >>> return; > >>> > >>> +fail_map: > >>> + if (v->shadow_vqs_enabled) { > >>> + vhost_iova_tree_remove(v->iova_tree, &mem_region); > >>> + } > >>> + > >>> fail: > >>> /* > >>> * On the initfn path, store the first error in the container so we > >>> -- > >>> 2.31.1 > >>> > ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] vdpa: do not save failed dma maps in SVQ iova tree 2022-08-04 15:02 ` Eugenio Perez Martin @ 2022-08-04 15:50 ` Eugenio Perez Martin 0 siblings, 0 replies; 7+ messages in thread From: Eugenio Perez Martin @ 2022-08-04 15:50 UTC (permalink / raw) To: Jason Wang; +Cc: qemu-devel, Michael S. Tsirkin On Thu, Aug 4, 2022 at 5:02 PM Eugenio Perez Martin <eperezma@redhat.com> wrote: > > On Thu, Aug 4, 2022 at 6:51 AM Jason Wang <jasowang@redhat.com> wrote: > > > > > > 在 2022/8/3 16:12, Eugenio Perez Martin 写道: > > > On Wed, Aug 3, 2022 at 10:09 AM Jason Wang <jasowang@redhat.com> wrote: > > >> On Tue, Aug 2, 2022 at 10:39 PM Eugenio Pérez <eperezma@redhat.com> wrote: > > >>> If a map fails for whatever reason, it must not be saved in the tree. > > >>> Otherwise, qemu will try to unmap it in cleanup, leaving to more errors. > > >>> > > >>> Fixes: 34e3c94eda ("vdpa: Add custom IOTLB translations to SVQ") > > >>> Signed-off-by: Eugenio Pérez <eperezma@redhat.com> > > >>> --- > > >>> hw/virtio/vhost-vdpa.c | 20 +++++++++++++------- > > >>> 1 file changed, 13 insertions(+), 7 deletions(-) > > >>> > > >>> diff --git a/hw/virtio/vhost-vdpa.c b/hw/virtio/vhost-vdpa.c > > >>> index 3ff9ce3501..e44c23dce5 100644 > > >>> --- a/hw/virtio/vhost-vdpa.c > > >>> +++ b/hw/virtio/vhost-vdpa.c > > >>> @@ -176,6 +176,7 @@ static void vhost_vdpa_listener_commit(MemoryListener *listener) > > >>> static void vhost_vdpa_listener_region_add(MemoryListener *listener, > > >>> MemoryRegionSection *section) > > >>> { > > >>> + DMAMap mem_region = {}; > > >>> struct vhost_vdpa *v = container_of(listener, struct vhost_vdpa, listener); > > >>> hwaddr iova; > > >>> Int128 llend, llsize; > > >>> @@ -212,13 +213,13 @@ static void vhost_vdpa_listener_region_add(MemoryListener *listener, > > >>> > > >>> llsize = int128_sub(llend, int128_make64(iova)); > > >>> if (v->shadow_vqs_enabled) { > > >>> - DMAMap mem_region = { > > >>> - .translated_addr = (hwaddr)(uintptr_t)vaddr, > > >>> - .size = int128_get64(llsize) - 1, > > >>> - .perm = IOMMU_ACCESS_FLAG(true, section->readonly), > > >>> - }; > > >> Nit: can we keep this part unchanged? > > >> > > > We can, but that implies we should look for iova again at fail_map > > > tag. If you are ok with that I'm fine to perform the search again. > > > > > > I meant something like: > > > > diff --git a/hw/virtio/vhost-vdpa.c b/hw/virtio/vhost-vdpa.c > > index 9a2daef7e3..edf40868e3 100644 > > --- a/hw/virtio/vhost-vdpa.c > > +++ b/hw/virtio/vhost-vdpa.c > > @@ -232,11 +232,15 @@ static void > > vhost_vdpa_listener_region_add(MemoryListener *listener, > > vaddr, section->readonly); > > if (ret) { > > error_report("vhost vdpa map fail!"); > > - goto fail; > > + goto fail_unmap; > > } > > > > return; > > > > +fail_unmap: > > + if (v->shadow_vqs_enabled) { > > + vhost_iova_tree_remove(v->iova_tree, &mem_region); > > + } > > fail: > > /* > > * On the initfn path, store the first error in the container so we > > > > Sorry, still not following. > > mem_region does not exist in the error path, since it's declared in > the if (v->shadow_vqs_enabled){} block. We can left first part > unchanged if we do a lookup for the mem region, based on the > translated addr. > Sending v2 expanding the fix without this comment, please let me know ifI misunderstood something. Thanks! ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2022-08-04 15:52 UTC | newest] Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2022-08-02 14:39 [PATCH] vdpa: do not save failed dma maps in SVQ iova tree Eugenio Pérez 2022-08-02 14:48 ` Eugenio Perez Martin 2022-08-03 8:09 ` Jason Wang 2022-08-03 8:12 ` Eugenio Perez Martin 2022-08-04 4:51 ` Jason Wang 2022-08-04 15:02 ` Eugenio Perez Martin 2022-08-04 15:50 ` Eugenio Perez Martin
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.