* [PATCH 0/3] vdpa: Check iova range on memory regions ops
@ 2021-10-05 8:01 Eugenio Pérez
2021-10-05 8:01 ` [PATCH 1/3] vdpa: Skip protected ram IOMMU mappings Eugenio Pérez
` (2 more replies)
0 siblings, 3 replies; 15+ messages in thread
From: Eugenio Pérez @ 2021-10-05 8:01 UTC (permalink / raw)
To: qemu-devel
Cc: Parav Pandit, Michael S. Tsirkin, Jason Wang, virtualization,
Stefan Hajnoczi, Eli Cohen, Stefano Garzarella
At this moment vdpa will not send memory regions bigger than 1<<63.
However, actual iova range could be way more restrictive than that.
Since we can obtain the range through vdpa ioctl call, just save it
from the beginning of the operation and check against it.
Eugenio Pérez (3):
vdpa: Skip protected ram IOMMU mappings
vdpa: Add vhost_vdpa_section_end
vdpa: Check for iova range at mappings changes
include/hw/virtio/vhost-vdpa.h | 2 +
hw/virtio/vhost-vdpa.c | 83 +++++++++++++++++++++++++---------
hw/virtio/trace-events | 1 +
3 files changed, 65 insertions(+), 21 deletions(-)
--
2.27.0
^ permalink raw reply [flat|nested] 15+ messages in thread* [PATCH 1/3] vdpa: Skip protected ram IOMMU mappings 2021-10-05 8:01 [PATCH 0/3] vdpa: Check iova range on memory regions ops Eugenio Pérez @ 2021-10-05 8:01 ` Eugenio Pérez 2021-10-05 8:01 ` [PATCH 2/3] vdpa: Add vhost_vdpa_section_end Eugenio Pérez 2021-10-05 8:01 ` [PATCH 3/3] vdpa: Check for iova range at mappings changes Eugenio Pérez 2 siblings, 0 replies; 15+ messages in thread From: Eugenio Pérez @ 2021-10-05 8:01 UTC (permalink / raw) To: qemu-devel Cc: Parav Pandit, Michael S. Tsirkin, Jason Wang, virtualization, Stefan Hajnoczi, Eli Cohen, Stefano Garzarella Following the logic of commit 56918a126ae ("memory: Add RAM_PROTECTED flag to skip IOMMU mappings") with VFIO, skip memory sections inaccessible via normal mechanisms, including DMA. Signed-off-by: Eugenio Pérez <eperezma@redhat.com> --- hw/virtio/vhost-vdpa.c | 1 + 1 file changed, 1 insertion(+) diff --git a/hw/virtio/vhost-vdpa.c b/hw/virtio/vhost-vdpa.c index 47d7a5a23d..ea1aa71ad8 100644 --- a/hw/virtio/vhost-vdpa.c +++ b/hw/virtio/vhost-vdpa.c @@ -28,6 +28,7 @@ static bool vhost_vdpa_listener_skipped_section(MemoryRegionSection *section) { return (!memory_region_is_ram(section->mr) && !memory_region_is_iommu(section->mr)) || + memory_region_is_protected(section->mr) || /* vhost-vDPA doesn't allow MMIO to be mapped */ memory_region_is_ram_device(section->mr) || /* -- 2.27.0 ^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH 2/3] vdpa: Add vhost_vdpa_section_end 2021-10-05 8:01 [PATCH 0/3] vdpa: Check iova range on memory regions ops Eugenio Pérez 2021-10-05 8:01 ` [PATCH 1/3] vdpa: Skip protected ram IOMMU mappings Eugenio Pérez @ 2021-10-05 8:01 ` Eugenio Pérez 2021-10-05 8:15 ` Michael S. Tsirkin 2021-10-05 8:01 ` [PATCH 3/3] vdpa: Check for iova range at mappings changes Eugenio Pérez 2 siblings, 1 reply; 15+ messages in thread From: Eugenio Pérez @ 2021-10-05 8:01 UTC (permalink / raw) To: qemu-devel Cc: Parav Pandit, Michael S. Tsirkin, Jason Wang, virtualization, Stefan Hajnoczi, Eli Cohen, Stefano Garzarella Abstract this operation, that will be reused when validating the region against the iova range that the device supports. Signed-off-by: Eugenio Pérez <eperezma@redhat.com> --- hw/virtio/vhost-vdpa.c | 18 +++++++++++------- 1 file changed, 11 insertions(+), 7 deletions(-) diff --git a/hw/virtio/vhost-vdpa.c b/hw/virtio/vhost-vdpa.c index ea1aa71ad8..a1de6c7c9c 100644 --- a/hw/virtio/vhost-vdpa.c +++ b/hw/virtio/vhost-vdpa.c @@ -24,6 +24,15 @@ #include "trace.h" #include "qemu-common.h" +static Int128 vhost_vdpa_section_end(const MemoryRegionSection *section) +{ + Int128 llend = int128_make64(section->offset_within_address_space); + llend = int128_add(llend, section->size); + llend = int128_and(llend, int128_exts64(TARGET_PAGE_MASK)); + + return llend; +} + static bool vhost_vdpa_listener_skipped_section(MemoryRegionSection *section) { return (!memory_region_is_ram(section->mr) && @@ -160,10 +169,7 @@ static void vhost_vdpa_listener_region_add(MemoryListener *listener, } iova = TARGET_PAGE_ALIGN(section->offset_within_address_space); - llend = int128_make64(section->offset_within_address_space); - llend = int128_add(llend, section->size); - llend = int128_and(llend, int128_exts64(TARGET_PAGE_MASK)); - + llend = vhost_vdpa_section_end(section); if (int128_ge(int128_make64(iova), llend)) { return; } @@ -221,9 +227,7 @@ static void vhost_vdpa_listener_region_del(MemoryListener *listener, } iova = TARGET_PAGE_ALIGN(section->offset_within_address_space); - llend = int128_make64(section->offset_within_address_space); - llend = int128_add(llend, section->size); - llend = int128_and(llend, int128_exts64(TARGET_PAGE_MASK)); + llend = vhost_vdpa_section_end(section); trace_vhost_vdpa_listener_region_del(v, iova, int128_get64(llend)); -- 2.27.0 ^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [PATCH 2/3] vdpa: Add vhost_vdpa_section_end 2021-10-05 8:01 ` [PATCH 2/3] vdpa: Add vhost_vdpa_section_end Eugenio Pérez @ 2021-10-05 8:15 ` Michael S. Tsirkin 0 siblings, 0 replies; 15+ messages in thread From: Michael S. Tsirkin @ 2021-10-05 8:15 UTC (permalink / raw) To: Eugenio Pérez Cc: Parav Pandit, qemu-devel, virtualization, Stefan Hajnoczi, Eli Cohen On Tue, Oct 05, 2021 at 10:01:30AM +0200, Eugenio Pérez wrote: > Abstract this operation, that will be reused when validating the region > against the iova range that the device supports. > > Signed-off-by: Eugenio Pérez <eperezma@redhat.com> Note that as defined end is actually 1 byte beyond end of section. As such it can e.g. overflow if cast to u64. So be careful to use int128 ops with it. Also - document? > --- > hw/virtio/vhost-vdpa.c | 18 +++++++++++------- > 1 file changed, 11 insertions(+), 7 deletions(-) > > diff --git a/hw/virtio/vhost-vdpa.c b/hw/virtio/vhost-vdpa.c > index ea1aa71ad8..a1de6c7c9c 100644 > --- a/hw/virtio/vhost-vdpa.c > +++ b/hw/virtio/vhost-vdpa.c > @@ -24,6 +24,15 @@ > #include "trace.h" > #include "qemu-common.h" > > +static Int128 vhost_vdpa_section_end(const MemoryRegionSection *section) > +{ > + Int128 llend = int128_make64(section->offset_within_address_space); > + llend = int128_add(llend, section->size); > + llend = int128_and(llend, int128_exts64(TARGET_PAGE_MASK)); > + > + return llend; > +} > + > static bool vhost_vdpa_listener_skipped_section(MemoryRegionSection *section) > { > return (!memory_region_is_ram(section->mr) && > @@ -160,10 +169,7 @@ static void vhost_vdpa_listener_region_add(MemoryListener *listener, > } > > iova = TARGET_PAGE_ALIGN(section->offset_within_address_space); > - llend = int128_make64(section->offset_within_address_space); > - llend = int128_add(llend, section->size); > - llend = int128_and(llend, int128_exts64(TARGET_PAGE_MASK)); > - > + llend = vhost_vdpa_section_end(section); > if (int128_ge(int128_make64(iova), llend)) { > return; > } > @@ -221,9 +227,7 @@ static void vhost_vdpa_listener_region_del(MemoryListener *listener, > } > > iova = TARGET_PAGE_ALIGN(section->offset_within_address_space); > - llend = int128_make64(section->offset_within_address_space); > - llend = int128_add(llend, section->size); > - llend = int128_and(llend, int128_exts64(TARGET_PAGE_MASK)); > + llend = vhost_vdpa_section_end(section); > > trace_vhost_vdpa_listener_region_del(v, iova, int128_get64(llend)); > > -- > 2.27.0 _______________________________________________ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 2/3] vdpa: Add vhost_vdpa_section_end @ 2021-10-05 8:15 ` Michael S. Tsirkin 0 siblings, 0 replies; 15+ messages in thread From: Michael S. Tsirkin @ 2021-10-05 8:15 UTC (permalink / raw) To: Eugenio Pérez Cc: Parav Pandit, Jason Wang, qemu-devel, virtualization, Stefan Hajnoczi, Eli Cohen, Stefano Garzarella On Tue, Oct 05, 2021 at 10:01:30AM +0200, Eugenio Pérez wrote: > Abstract this operation, that will be reused when validating the region > against the iova range that the device supports. > > Signed-off-by: Eugenio Pérez <eperezma@redhat.com> Note that as defined end is actually 1 byte beyond end of section. As such it can e.g. overflow if cast to u64. So be careful to use int128 ops with it. Also - document? > --- > hw/virtio/vhost-vdpa.c | 18 +++++++++++------- > 1 file changed, 11 insertions(+), 7 deletions(-) > > diff --git a/hw/virtio/vhost-vdpa.c b/hw/virtio/vhost-vdpa.c > index ea1aa71ad8..a1de6c7c9c 100644 > --- a/hw/virtio/vhost-vdpa.c > +++ b/hw/virtio/vhost-vdpa.c > @@ -24,6 +24,15 @@ > #include "trace.h" > #include "qemu-common.h" > > +static Int128 vhost_vdpa_section_end(const MemoryRegionSection *section) > +{ > + Int128 llend = int128_make64(section->offset_within_address_space); > + llend = int128_add(llend, section->size); > + llend = int128_and(llend, int128_exts64(TARGET_PAGE_MASK)); > + > + return llend; > +} > + > static bool vhost_vdpa_listener_skipped_section(MemoryRegionSection *section) > { > return (!memory_region_is_ram(section->mr) && > @@ -160,10 +169,7 @@ static void vhost_vdpa_listener_region_add(MemoryListener *listener, > } > > iova = TARGET_PAGE_ALIGN(section->offset_within_address_space); > - llend = int128_make64(section->offset_within_address_space); > - llend = int128_add(llend, section->size); > - llend = int128_and(llend, int128_exts64(TARGET_PAGE_MASK)); > - > + llend = vhost_vdpa_section_end(section); > if (int128_ge(int128_make64(iova), llend)) { > return; > } > @@ -221,9 +227,7 @@ static void vhost_vdpa_listener_region_del(MemoryListener *listener, > } > > iova = TARGET_PAGE_ALIGN(section->offset_within_address_space); > - llend = int128_make64(section->offset_within_address_space); > - llend = int128_add(llend, section->size); > - llend = int128_and(llend, int128_exts64(TARGET_PAGE_MASK)); > + llend = vhost_vdpa_section_end(section); > > trace_vhost_vdpa_listener_region_del(v, iova, int128_get64(llend)); > > -- > 2.27.0 ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 2/3] vdpa: Add vhost_vdpa_section_end 2021-10-05 8:15 ` Michael S. Tsirkin (?) @ 2021-10-05 9:52 ` Eugenio Perez Martin 2021-10-05 10:47 ` Michael S. Tsirkin -1 siblings, 1 reply; 15+ messages in thread From: Eugenio Perez Martin @ 2021-10-05 9:52 UTC (permalink / raw) To: Michael S. Tsirkin Cc: Parav Pandit, Jason Wang, qemu-level, virtualization, Stefan Hajnoczi, Eli Cohen, Stefano Garzarella On Tue, Oct 5, 2021 at 10:15 AM Michael S. Tsirkin <mst@redhat.com> wrote: > > On Tue, Oct 05, 2021 at 10:01:30AM +0200, Eugenio Pérez wrote: > > Abstract this operation, that will be reused when validating the region > > against the iova range that the device supports. > > > > Signed-off-by: Eugenio Pérez <eperezma@redhat.com> > > Note that as defined end is actually 1 byte beyond end of section. > As such it can e.g. overflow if cast to u64. > So be careful to use int128 ops with it. You are right, but this is only the result of extracting "llend" calculation in its own function, since it is going to be used a third time in the next commit. This next commit contains a mistake because of this, as you pointed out. Since "last" would be a very misleading name, do you think we could give a better name / type to it? > Also - document? It will be documented with that ("It returns one byte beyond end of section" or similar) too. Thanks! > > > --- > > hw/virtio/vhost-vdpa.c | 18 +++++++++++------- > > 1 file changed, 11 insertions(+), 7 deletions(-) > > > > diff --git a/hw/virtio/vhost-vdpa.c b/hw/virtio/vhost-vdpa.c > > index ea1aa71ad8..a1de6c7c9c 100644 > > --- a/hw/virtio/vhost-vdpa.c > > +++ b/hw/virtio/vhost-vdpa.c > > @@ -24,6 +24,15 @@ > > #include "trace.h" > > #include "qemu-common.h" > > > > +static Int128 vhost_vdpa_section_end(const MemoryRegionSection *section) > > +{ > > + Int128 llend = int128_make64(section->offset_within_address_space); > > + llend = int128_add(llend, section->size); > > + llend = int128_and(llend, int128_exts64(TARGET_PAGE_MASK)); > > + > > + return llend; > > +} > > + > > static bool vhost_vdpa_listener_skipped_section(MemoryRegionSection *section) > > { > > return (!memory_region_is_ram(section->mr) && > > @@ -160,10 +169,7 @@ static void vhost_vdpa_listener_region_add(MemoryListener *listener, > > } > > > > iova = TARGET_PAGE_ALIGN(section->offset_within_address_space); > > - llend = int128_make64(section->offset_within_address_space); > > - llend = int128_add(llend, section->size); > > - llend = int128_and(llend, int128_exts64(TARGET_PAGE_MASK)); > > - > > + llend = vhost_vdpa_section_end(section); > > if (int128_ge(int128_make64(iova), llend)) { > > return; > > } > > @@ -221,9 +227,7 @@ static void vhost_vdpa_listener_region_del(MemoryListener *listener, > > } > > > > iova = TARGET_PAGE_ALIGN(section->offset_within_address_space); > > - llend = int128_make64(section->offset_within_address_space); > > - llend = int128_add(llend, section->size); > > - llend = int128_and(llend, int128_exts64(TARGET_PAGE_MASK)); > > + llend = vhost_vdpa_section_end(section); > > > > trace_vhost_vdpa_listener_region_del(v, iova, int128_get64(llend)); > > > > -- > > 2.27.0 > ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 2/3] vdpa: Add vhost_vdpa_section_end 2021-10-05 9:52 ` Eugenio Perez Martin @ 2021-10-05 10:47 ` Michael S. Tsirkin 0 siblings, 0 replies; 15+ messages in thread From: Michael S. Tsirkin @ 2021-10-05 10:47 UTC (permalink / raw) To: Eugenio Perez Martin Cc: Parav Pandit, qemu-level, virtualization, Stefan Hajnoczi, Eli Cohen On Tue, Oct 05, 2021 at 11:52:37AM +0200, Eugenio Perez Martin wrote: > On Tue, Oct 5, 2021 at 10:15 AM Michael S. Tsirkin <mst@redhat.com> wrote: > > > > On Tue, Oct 05, 2021 at 10:01:30AM +0200, Eugenio Pérez wrote: > > > Abstract this operation, that will be reused when validating the region > > > against the iova range that the device supports. > > > > > > Signed-off-by: Eugenio Pérez <eperezma@redhat.com> > > > > Note that as defined end is actually 1 byte beyond end of section. > > As such it can e.g. overflow if cast to u64. > > So be careful to use int128 ops with it. > > You are right, but this is only the result of extracting "llend" > calculation in its own function, since it is going to be used a third > time in the next commit. This next commit contains a mistake because > of this, as you pointed out. > > Since "last" would be a very misleading name, do you think we could > give a better name / type to it? > > > Also - document? > > It will be documented with that ("It returns one byte beyond end of > section" or similar) too. > > Thanks! that's how c++ containers work so maybe it's not too bad as long as we document this carefully. > > > > > --- > > > hw/virtio/vhost-vdpa.c | 18 +++++++++++------- > > > 1 file changed, 11 insertions(+), 7 deletions(-) > > > > > > diff --git a/hw/virtio/vhost-vdpa.c b/hw/virtio/vhost-vdpa.c > > > index ea1aa71ad8..a1de6c7c9c 100644 > > > --- a/hw/virtio/vhost-vdpa.c > > > +++ b/hw/virtio/vhost-vdpa.c > > > @@ -24,6 +24,15 @@ > > > #include "trace.h" > > > #include "qemu-common.h" > > > > > > +static Int128 vhost_vdpa_section_end(const MemoryRegionSection *section) > > > +{ > > > + Int128 llend = int128_make64(section->offset_within_address_space); > > > + llend = int128_add(llend, section->size); > > > + llend = int128_and(llend, int128_exts64(TARGET_PAGE_MASK)); > > > + > > > + return llend; > > > +} > > > + > > > static bool vhost_vdpa_listener_skipped_section(MemoryRegionSection *section) > > > { > > > return (!memory_region_is_ram(section->mr) && > > > @@ -160,10 +169,7 @@ static void vhost_vdpa_listener_region_add(MemoryListener *listener, > > > } > > > > > > iova = TARGET_PAGE_ALIGN(section->offset_within_address_space); > > > - llend = int128_make64(section->offset_within_address_space); > > > - llend = int128_add(llend, section->size); > > > - llend = int128_and(llend, int128_exts64(TARGET_PAGE_MASK)); > > > - > > > + llend = vhost_vdpa_section_end(section); > > > if (int128_ge(int128_make64(iova), llend)) { > > > return; > > > } > > > @@ -221,9 +227,7 @@ static void vhost_vdpa_listener_region_del(MemoryListener *listener, > > > } > > > > > > iova = TARGET_PAGE_ALIGN(section->offset_within_address_space); > > > - llend = int128_make64(section->offset_within_address_space); > > > - llend = int128_add(llend, section->size); > > > - llend = int128_and(llend, int128_exts64(TARGET_PAGE_MASK)); > > > + llend = vhost_vdpa_section_end(section); > > > > > > trace_vhost_vdpa_listener_region_del(v, iova, int128_get64(llend)); > > > > > > -- > > > 2.27.0 > > _______________________________________________ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 2/3] vdpa: Add vhost_vdpa_section_end @ 2021-10-05 10:47 ` Michael S. Tsirkin 0 siblings, 0 replies; 15+ messages in thread From: Michael S. Tsirkin @ 2021-10-05 10:47 UTC (permalink / raw) To: Eugenio Perez Martin Cc: Parav Pandit, Jason Wang, qemu-level, virtualization, Stefan Hajnoczi, Eli Cohen, Stefano Garzarella On Tue, Oct 05, 2021 at 11:52:37AM +0200, Eugenio Perez Martin wrote: > On Tue, Oct 5, 2021 at 10:15 AM Michael S. Tsirkin <mst@redhat.com> wrote: > > > > On Tue, Oct 05, 2021 at 10:01:30AM +0200, Eugenio Pérez wrote: > > > Abstract this operation, that will be reused when validating the region > > > against the iova range that the device supports. > > > > > > Signed-off-by: Eugenio Pérez <eperezma@redhat.com> > > > > Note that as defined end is actually 1 byte beyond end of section. > > As such it can e.g. overflow if cast to u64. > > So be careful to use int128 ops with it. > > You are right, but this is only the result of extracting "llend" > calculation in its own function, since it is going to be used a third > time in the next commit. This next commit contains a mistake because > of this, as you pointed out. > > Since "last" would be a very misleading name, do you think we could > give a better name / type to it? > > > Also - document? > > It will be documented with that ("It returns one byte beyond end of > section" or similar) too. > > Thanks! that's how c++ containers work so maybe it's not too bad as long as we document this carefully. > > > > > --- > > > hw/virtio/vhost-vdpa.c | 18 +++++++++++------- > > > 1 file changed, 11 insertions(+), 7 deletions(-) > > > > > > diff --git a/hw/virtio/vhost-vdpa.c b/hw/virtio/vhost-vdpa.c > > > index ea1aa71ad8..a1de6c7c9c 100644 > > > --- a/hw/virtio/vhost-vdpa.c > > > +++ b/hw/virtio/vhost-vdpa.c > > > @@ -24,6 +24,15 @@ > > > #include "trace.h" > > > #include "qemu-common.h" > > > > > > +static Int128 vhost_vdpa_section_end(const MemoryRegionSection *section) > > > +{ > > > + Int128 llend = int128_make64(section->offset_within_address_space); > > > + llend = int128_add(llend, section->size); > > > + llend = int128_and(llend, int128_exts64(TARGET_PAGE_MASK)); > > > + > > > + return llend; > > > +} > > > + > > > static bool vhost_vdpa_listener_skipped_section(MemoryRegionSection *section) > > > { > > > return (!memory_region_is_ram(section->mr) && > > > @@ -160,10 +169,7 @@ static void vhost_vdpa_listener_region_add(MemoryListener *listener, > > > } > > > > > > iova = TARGET_PAGE_ALIGN(section->offset_within_address_space); > > > - llend = int128_make64(section->offset_within_address_space); > > > - llend = int128_add(llend, section->size); > > > - llend = int128_and(llend, int128_exts64(TARGET_PAGE_MASK)); > > > - > > > + llend = vhost_vdpa_section_end(section); > > > if (int128_ge(int128_make64(iova), llend)) { > > > return; > > > } > > > @@ -221,9 +227,7 @@ static void vhost_vdpa_listener_region_del(MemoryListener *listener, > > > } > > > > > > iova = TARGET_PAGE_ALIGN(section->offset_within_address_space); > > > - llend = int128_make64(section->offset_within_address_space); > > > - llend = int128_add(llend, section->size); > > > - llend = int128_and(llend, int128_exts64(TARGET_PAGE_MASK)); > > > + llend = vhost_vdpa_section_end(section); > > > > > > trace_vhost_vdpa_listener_region_del(v, iova, int128_get64(llend)); > > > > > > -- > > > 2.27.0 > > ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 2/3] vdpa: Add vhost_vdpa_section_end 2021-10-05 10:47 ` Michael S. Tsirkin (?) @ 2021-10-05 13:14 ` Eugenio Perez Martin -1 siblings, 0 replies; 15+ messages in thread From: Eugenio Perez Martin @ 2021-10-05 13:14 UTC (permalink / raw) To: Michael S. Tsirkin Cc: Parav Pandit, Jason Wang, qemu-level, virtualization, Stefan Hajnoczi, Eli Cohen, Stefano Garzarella On Tue, Oct 5, 2021 at 12:47 PM Michael S. Tsirkin <mst@redhat.com> wrote: > > On Tue, Oct 05, 2021 at 11:52:37AM +0200, Eugenio Perez Martin wrote: > > On Tue, Oct 5, 2021 at 10:15 AM Michael S. Tsirkin <mst@redhat.com> wrote: > > > > > > On Tue, Oct 05, 2021 at 10:01:30AM +0200, Eugenio Pérez wrote: > > > > Abstract this operation, that will be reused when validating the region > > > > against the iova range that the device supports. > > > > > > > > Signed-off-by: Eugenio Pérez <eperezma@redhat.com> > > > > > > Note that as defined end is actually 1 byte beyond end of section. > > > As such it can e.g. overflow if cast to u64. > > > So be careful to use int128 ops with it. > > > > You are right, but this is only the result of extracting "llend" > > calculation in its own function, since it is going to be used a third > > time in the next commit. This next commit contains a mistake because > > of this, as you pointed out. > > > > Since "last" would be a very misleading name, do you think we could > > give a better name / type to it? > > > > > Also - document? > > > > It will be documented with that ("It returns one byte beyond end of > > section" or similar) too. > > > > Thanks! > > that's how c++ containers work so maybe it's not too bad as long > as we document this carefully. > I tend to see it that way except when the name is "last", that I read as "last one addressable/valid", as discussed in the VHOST_VDPA_GET_IOVA_RANGE call mail thread. So end = past range, last = last one valid. It would be great to have something like void * / hwaddr, or c++ chrono time_point<second> vs time_point<millisecond>, that moves to type system the verification of not mixing different range types. But this may be overthinking at this moment. Thanks! > > > > > > > --- > > > > hw/virtio/vhost-vdpa.c | 18 +++++++++++------- > > > > 1 file changed, 11 insertions(+), 7 deletions(-) > > > > > > > > diff --git a/hw/virtio/vhost-vdpa.c b/hw/virtio/vhost-vdpa.c > > > > index ea1aa71ad8..a1de6c7c9c 100644 > > > > --- a/hw/virtio/vhost-vdpa.c > > > > +++ b/hw/virtio/vhost-vdpa.c > > > > @@ -24,6 +24,15 @@ > > > > #include "trace.h" > > > > #include "qemu-common.h" > > > > > > > > +static Int128 vhost_vdpa_section_end(const MemoryRegionSection *section) > > > > +{ > > > > + Int128 llend = int128_make64(section->offset_within_address_space); > > > > + llend = int128_add(llend, section->size); > > > > + llend = int128_and(llend, int128_exts64(TARGET_PAGE_MASK)); > > > > + > > > > + return llend; > > > > +} > > > > + > > > > static bool vhost_vdpa_listener_skipped_section(MemoryRegionSection *section) > > > > { > > > > return (!memory_region_is_ram(section->mr) && > > > > @@ -160,10 +169,7 @@ static void vhost_vdpa_listener_region_add(MemoryListener *listener, > > > > } > > > > > > > > iova = TARGET_PAGE_ALIGN(section->offset_within_address_space); > > > > - llend = int128_make64(section->offset_within_address_space); > > > > - llend = int128_add(llend, section->size); > > > > - llend = int128_and(llend, int128_exts64(TARGET_PAGE_MASK)); > > > > - > > > > + llend = vhost_vdpa_section_end(section); > > > > if (int128_ge(int128_make64(iova), llend)) { > > > > return; > > > > } > > > > @@ -221,9 +227,7 @@ static void vhost_vdpa_listener_region_del(MemoryListener *listener, > > > > } > > > > > > > > iova = TARGET_PAGE_ALIGN(section->offset_within_address_space); > > > > - llend = int128_make64(section->offset_within_address_space); > > > > - llend = int128_add(llend, section->size); > > > > - llend = int128_and(llend, int128_exts64(TARGET_PAGE_MASK)); > > > > + llend = vhost_vdpa_section_end(section); > > > > > > > > trace_vhost_vdpa_listener_region_del(v, iova, int128_get64(llend)); > > > > > > > > -- > > > > 2.27.0 > > > > ^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH 3/3] vdpa: Check for iova range at mappings changes 2021-10-05 8:01 [PATCH 0/3] vdpa: Check iova range on memory regions ops Eugenio Pérez 2021-10-05 8:01 ` [PATCH 1/3] vdpa: Skip protected ram IOMMU mappings Eugenio Pérez 2021-10-05 8:01 ` [PATCH 2/3] vdpa: Add vhost_vdpa_section_end Eugenio Pérez @ 2021-10-05 8:01 ` Eugenio Pérez 2021-10-05 8:14 ` Michael S. Tsirkin 2 siblings, 1 reply; 15+ messages in thread From: Eugenio Pérez @ 2021-10-05 8:01 UTC (permalink / raw) To: qemu-devel Cc: Parav Pandit, Michael S. Tsirkin, Jason Wang, virtualization, Stefan Hajnoczi, Eli Cohen, Stefano Garzarella Check vdpa device range before updating memory regions so we don't add any outside of it, and report the invalid change if any. Signed-off-by: Eugenio Pérez <eperezma@redhat.com> --- include/hw/virtio/vhost-vdpa.h | 2 + hw/virtio/vhost-vdpa.c | 68 ++++++++++++++++++++++++++-------- hw/virtio/trace-events | 1 + 3 files changed, 55 insertions(+), 16 deletions(-) diff --git a/include/hw/virtio/vhost-vdpa.h b/include/hw/virtio/vhost-vdpa.h index a8963da2d9..c288cf7ecb 100644 --- a/include/hw/virtio/vhost-vdpa.h +++ b/include/hw/virtio/vhost-vdpa.h @@ -13,6 +13,7 @@ #define HW_VIRTIO_VHOST_VDPA_H #include "hw/virtio/virtio.h" +#include "standard-headers/linux/vhost_types.h" typedef struct VhostVDPAHostNotifier { MemoryRegion mr; @@ -24,6 +25,7 @@ typedef struct vhost_vdpa { uint32_t msg_type; bool iotlb_batch_begin_sent; MemoryListener listener; + struct vhost_vdpa_iova_range iova_range; struct vhost_dev *dev; VhostVDPAHostNotifier notifier[VIRTIO_QUEUE_MAX]; } VhostVDPA; diff --git a/hw/virtio/vhost-vdpa.c b/hw/virtio/vhost-vdpa.c index a1de6c7c9c..26d0258723 100644 --- a/hw/virtio/vhost-vdpa.c +++ b/hw/virtio/vhost-vdpa.c @@ -33,20 +33,34 @@ static Int128 vhost_vdpa_section_end(const MemoryRegionSection *section) return llend; } -static bool vhost_vdpa_listener_skipped_section(MemoryRegionSection *section) -{ - return (!memory_region_is_ram(section->mr) && - !memory_region_is_iommu(section->mr)) || - memory_region_is_protected(section->mr) || - /* vhost-vDPA doesn't allow MMIO to be mapped */ - memory_region_is_ram_device(section->mr) || - /* - * Sizing an enabled 64-bit BAR can cause spurious mappings to - * addresses in the upper part of the 64-bit address space. These - * are never accessed by the CPU and beyond the address width of - * some IOMMU hardware. TODO: VDPA should tell us the IOMMU width. - */ - section->offset_within_address_space & (1ULL << 63); +static bool vhost_vdpa_listener_skipped_section(MemoryRegionSection *section, + uint64_t iova_min, + uint64_t iova_max) +{ + Int128 llend; + bool r = (!memory_region_is_ram(section->mr) && + !memory_region_is_iommu(section->mr)) || + memory_region_is_protected(section->mr) || + /* vhost-vDPA doesn't allow MMIO to be mapped */ + memory_region_is_ram_device(section->mr); + if (r) { + return true; + } + + if (section->offset_within_address_space < iova_min) { + error_report("RAM section out of device range (min=%lu, addr=%lu)", + iova_min, section->offset_within_address_space); + return true; + } + + llend = vhost_vdpa_section_end(section); + if (int128_make64(llend) > iova_max) { + error_report("RAM section out of device range (max=%lu, end addr=%lu)", + iova_max, (uint64_t)int128_make64(llend)); + return true; + } + + return false; } static int vhost_vdpa_dma_map(struct vhost_vdpa *v, hwaddr iova, hwaddr size, @@ -158,7 +172,8 @@ static void vhost_vdpa_listener_region_add(MemoryListener *listener, void *vaddr; int ret; - if (vhost_vdpa_listener_skipped_section(section)) { + if (vhost_vdpa_listener_skipped_section(section, v->iova_range.first, + v->iova_range.last)) { return; } @@ -216,7 +231,8 @@ static void vhost_vdpa_listener_region_del(MemoryListener *listener, Int128 llend, llsize; int ret; - if (vhost_vdpa_listener_skipped_section(section)) { + if (vhost_vdpa_listener_skipped_section(section, v->iova_range.first, + v->iova_range.last)) { return; } @@ -284,9 +300,24 @@ static void vhost_vdpa_add_status(struct vhost_dev *dev, uint8_t status) vhost_vdpa_call(dev, VHOST_VDPA_SET_STATUS, &s); } +static int vhost_vdpa_get_iova_range(struct vhost_vdpa *v) +{ + int ret; + + ret = vhost_vdpa_call(v->dev, VHOST_VDPA_GET_IOVA_RANGE, &v->iova_range); + if (ret != 0) { + return ret; + } + + trace_vhost_vdpa_get_iova_range(v->dev, v->iova_range.first, + v->iova_range.last); + return ret; +} + static int vhost_vdpa_init(struct vhost_dev *dev, void *opaque, Error **errp) { struct vhost_vdpa *v; + int r; assert(dev->vhost_ops->backend_type == VHOST_BACKEND_TYPE_VDPA); trace_vhost_vdpa_init(dev, opaque); @@ -296,6 +327,11 @@ static int vhost_vdpa_init(struct vhost_dev *dev, void *opaque, Error **errp) v->listener = vhost_vdpa_memory_listener; v->msg_type = VHOST_IOTLB_MSG_V2; + r = vhost_vdpa_get_iova_range(v); + if (unlikely(!r)) { + return r; + } + vhost_vdpa_add_status(dev, VIRTIO_CONFIG_S_ACKNOWLEDGE | VIRTIO_CONFIG_S_DRIVER); diff --git a/hw/virtio/trace-events b/hw/virtio/trace-events index 8ed19e9d0c..650e521e35 100644 --- a/hw/virtio/trace-events +++ b/hw/virtio/trace-events @@ -52,6 +52,7 @@ vhost_vdpa_set_vring_call(void *dev, unsigned int index, int fd) "dev: %p index: vhost_vdpa_get_features(void *dev, uint64_t features) "dev: %p features: 0x%"PRIx64 vhost_vdpa_set_owner(void *dev) "dev: %p" vhost_vdpa_vq_get_addr(void *dev, void *vq, uint64_t desc_user_addr, uint64_t avail_user_addr, uint64_t used_user_addr) "dev: %p vq: %p desc_user_addr: 0x%"PRIx64" avail_user_addr: 0x%"PRIx64" used_user_addr: 0x%"PRIx64 +vhost_vdpa_get_iova_range(void *dev, uint64_t first, uint64_t last) "dev: %p first: 0x%"PRIx64" last: 0x%"PRIx64 # virtio.c virtqueue_alloc_element(void *elem, size_t sz, unsigned in_num, unsigned out_num) "elem %p size %zd in_num %u out_num %u" -- 2.27.0 ^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [PATCH 3/3] vdpa: Check for iova range at mappings changes 2021-10-05 8:01 ` [PATCH 3/3] vdpa: Check for iova range at mappings changes Eugenio Pérez @ 2021-10-05 8:14 ` Michael S. Tsirkin 0 siblings, 0 replies; 15+ messages in thread From: Michael S. Tsirkin @ 2021-10-05 8:14 UTC (permalink / raw) To: Eugenio Pérez Cc: Parav Pandit, qemu-devel, virtualization, Stefan Hajnoczi, Eli Cohen On Tue, Oct 05, 2021 at 10:01:31AM +0200, Eugenio Pérez wrote: > Check vdpa device range before updating memory regions so we don't add > any outside of it, and report the invalid change if any. > > Signed-off-by: Eugenio Pérez <eperezma@redhat.com> > --- > include/hw/virtio/vhost-vdpa.h | 2 + > hw/virtio/vhost-vdpa.c | 68 ++++++++++++++++++++++++++-------- > hw/virtio/trace-events | 1 + > 3 files changed, 55 insertions(+), 16 deletions(-) > > diff --git a/include/hw/virtio/vhost-vdpa.h b/include/hw/virtio/vhost-vdpa.h > index a8963da2d9..c288cf7ecb 100644 > --- a/include/hw/virtio/vhost-vdpa.h > +++ b/include/hw/virtio/vhost-vdpa.h > @@ -13,6 +13,7 @@ > #define HW_VIRTIO_VHOST_VDPA_H > > #include "hw/virtio/virtio.h" > +#include "standard-headers/linux/vhost_types.h" > > typedef struct VhostVDPAHostNotifier { > MemoryRegion mr; > @@ -24,6 +25,7 @@ typedef struct vhost_vdpa { > uint32_t msg_type; > bool iotlb_batch_begin_sent; > MemoryListener listener; > + struct vhost_vdpa_iova_range iova_range; > struct vhost_dev *dev; > VhostVDPAHostNotifier notifier[VIRTIO_QUEUE_MAX]; > } VhostVDPA; > diff --git a/hw/virtio/vhost-vdpa.c b/hw/virtio/vhost-vdpa.c > index a1de6c7c9c..26d0258723 100644 > --- a/hw/virtio/vhost-vdpa.c > +++ b/hw/virtio/vhost-vdpa.c > @@ -33,20 +33,34 @@ static Int128 vhost_vdpa_section_end(const MemoryRegionSection *section) > return llend; > } > > -static bool vhost_vdpa_listener_skipped_section(MemoryRegionSection *section) > -{ > - return (!memory_region_is_ram(section->mr) && > - !memory_region_is_iommu(section->mr)) || > - memory_region_is_protected(section->mr) || > - /* vhost-vDPA doesn't allow MMIO to be mapped */ > - memory_region_is_ram_device(section->mr) || > - /* > - * Sizing an enabled 64-bit BAR can cause spurious mappings to > - * addresses in the upper part of the 64-bit address space. These > - * are never accessed by the CPU and beyond the address width of > - * some IOMMU hardware. TODO: VDPA should tell us the IOMMU width. > - */ > - section->offset_within_address_space & (1ULL << 63); > +static bool vhost_vdpa_listener_skipped_section(MemoryRegionSection *section, > + uint64_t iova_min, > + uint64_t iova_max) > +{ > + Int128 llend; > + bool r = (!memory_region_is_ram(section->mr) && > + !memory_region_is_iommu(section->mr)) || > + memory_region_is_protected(section->mr) || > + /* vhost-vDPA doesn't allow MMIO to be mapped */ > + memory_region_is_ram_device(section->mr); > + if (r) { > + return true; > + } > + > + if (section->offset_within_address_space < iova_min) { > + error_report("RAM section out of device range (min=%lu, addr=%lu)", > + iova_min, section->offset_within_address_space); > + return true; > + } > + > + llend = vhost_vdpa_section_end(section); > + if (int128_make64(llend) > iova_max) { I am puzzled by this. You are taking a Int128, converting to u64, converting back to Int128, and comparing to u64. Head spins. What is all this back and forth trying to achieve? > + error_report("RAM section out of device range (max=%lu, end addr=%lu)", > + iova_max, (uint64_t)int128_make64(llend)); > + return true; > + } > + > + return false; > } > > static int vhost_vdpa_dma_map(struct vhost_vdpa *v, hwaddr iova, hwaddr size, > @@ -158,7 +172,8 @@ static void vhost_vdpa_listener_region_add(MemoryListener *listener, > void *vaddr; > int ret; > > - if (vhost_vdpa_listener_skipped_section(section)) { > + if (vhost_vdpa_listener_skipped_section(section, v->iova_range.first, > + v->iova_range.last)) { > return; > } > > @@ -216,7 +231,8 @@ static void vhost_vdpa_listener_region_del(MemoryListener *listener, > Int128 llend, llsize; > int ret; > > - if (vhost_vdpa_listener_skipped_section(section)) { > + if (vhost_vdpa_listener_skipped_section(section, v->iova_range.first, > + v->iova_range.last)) { > return; > } > > @@ -284,9 +300,24 @@ static void vhost_vdpa_add_status(struct vhost_dev *dev, uint8_t status) > vhost_vdpa_call(dev, VHOST_VDPA_SET_STATUS, &s); > } > > +static int vhost_vdpa_get_iova_range(struct vhost_vdpa *v) > +{ > + int ret; > + > + ret = vhost_vdpa_call(v->dev, VHOST_VDPA_GET_IOVA_RANGE, &v->iova_range); > + if (ret != 0) { > + return ret; > + } > + > + trace_vhost_vdpa_get_iova_range(v->dev, v->iova_range.first, > + v->iova_range.last); > + return ret; > +} > + > static int vhost_vdpa_init(struct vhost_dev *dev, void *opaque, Error **errp) > { > struct vhost_vdpa *v; > + int r; > assert(dev->vhost_ops->backend_type == VHOST_BACKEND_TYPE_VDPA); > trace_vhost_vdpa_init(dev, opaque); > > @@ -296,6 +327,11 @@ static int vhost_vdpa_init(struct vhost_dev *dev, void *opaque, Error **errp) > v->listener = vhost_vdpa_memory_listener; > v->msg_type = VHOST_IOTLB_MSG_V2; > > + r = vhost_vdpa_get_iova_range(v); > + if (unlikely(!r)) { > + return r; > + } > + > vhost_vdpa_add_status(dev, VIRTIO_CONFIG_S_ACKNOWLEDGE | > VIRTIO_CONFIG_S_DRIVER); > > diff --git a/hw/virtio/trace-events b/hw/virtio/trace-events > index 8ed19e9d0c..650e521e35 100644 > --- a/hw/virtio/trace-events > +++ b/hw/virtio/trace-events > @@ -52,6 +52,7 @@ vhost_vdpa_set_vring_call(void *dev, unsigned int index, int fd) "dev: %p index: > vhost_vdpa_get_features(void *dev, uint64_t features) "dev: %p features: 0x%"PRIx64 > vhost_vdpa_set_owner(void *dev) "dev: %p" > vhost_vdpa_vq_get_addr(void *dev, void *vq, uint64_t desc_user_addr, uint64_t avail_user_addr, uint64_t used_user_addr) "dev: %p vq: %p desc_user_addr: 0x%"PRIx64" avail_user_addr: 0x%"PRIx64" used_user_addr: 0x%"PRIx64 > +vhost_vdpa_get_iova_range(void *dev, uint64_t first, uint64_t last) "dev: %p first: 0x%"PRIx64" last: 0x%"PRIx64 > > # virtio.c > virtqueue_alloc_element(void *elem, size_t sz, unsigned in_num, unsigned out_num) "elem %p size %zd in_num %u out_num %u" > -- > 2.27.0 _______________________________________________ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 3/3] vdpa: Check for iova range at mappings changes @ 2021-10-05 8:14 ` Michael S. Tsirkin 0 siblings, 0 replies; 15+ messages in thread From: Michael S. Tsirkin @ 2021-10-05 8:14 UTC (permalink / raw) To: Eugenio Pérez Cc: Parav Pandit, Jason Wang, qemu-devel, virtualization, Stefan Hajnoczi, Eli Cohen, Stefano Garzarella On Tue, Oct 05, 2021 at 10:01:31AM +0200, Eugenio Pérez wrote: > Check vdpa device range before updating memory regions so we don't add > any outside of it, and report the invalid change if any. > > Signed-off-by: Eugenio Pérez <eperezma@redhat.com> > --- > include/hw/virtio/vhost-vdpa.h | 2 + > hw/virtio/vhost-vdpa.c | 68 ++++++++++++++++++++++++++-------- > hw/virtio/trace-events | 1 + > 3 files changed, 55 insertions(+), 16 deletions(-) > > diff --git a/include/hw/virtio/vhost-vdpa.h b/include/hw/virtio/vhost-vdpa.h > index a8963da2d9..c288cf7ecb 100644 > --- a/include/hw/virtio/vhost-vdpa.h > +++ b/include/hw/virtio/vhost-vdpa.h > @@ -13,6 +13,7 @@ > #define HW_VIRTIO_VHOST_VDPA_H > > #include "hw/virtio/virtio.h" > +#include "standard-headers/linux/vhost_types.h" > > typedef struct VhostVDPAHostNotifier { > MemoryRegion mr; > @@ -24,6 +25,7 @@ typedef struct vhost_vdpa { > uint32_t msg_type; > bool iotlb_batch_begin_sent; > MemoryListener listener; > + struct vhost_vdpa_iova_range iova_range; > struct vhost_dev *dev; > VhostVDPAHostNotifier notifier[VIRTIO_QUEUE_MAX]; > } VhostVDPA; > diff --git a/hw/virtio/vhost-vdpa.c b/hw/virtio/vhost-vdpa.c > index a1de6c7c9c..26d0258723 100644 > --- a/hw/virtio/vhost-vdpa.c > +++ b/hw/virtio/vhost-vdpa.c > @@ -33,20 +33,34 @@ static Int128 vhost_vdpa_section_end(const MemoryRegionSection *section) > return llend; > } > > -static bool vhost_vdpa_listener_skipped_section(MemoryRegionSection *section) > -{ > - return (!memory_region_is_ram(section->mr) && > - !memory_region_is_iommu(section->mr)) || > - memory_region_is_protected(section->mr) || > - /* vhost-vDPA doesn't allow MMIO to be mapped */ > - memory_region_is_ram_device(section->mr) || > - /* > - * Sizing an enabled 64-bit BAR can cause spurious mappings to > - * addresses in the upper part of the 64-bit address space. These > - * are never accessed by the CPU and beyond the address width of > - * some IOMMU hardware. TODO: VDPA should tell us the IOMMU width. > - */ > - section->offset_within_address_space & (1ULL << 63); > +static bool vhost_vdpa_listener_skipped_section(MemoryRegionSection *section, > + uint64_t iova_min, > + uint64_t iova_max) > +{ > + Int128 llend; > + bool r = (!memory_region_is_ram(section->mr) && > + !memory_region_is_iommu(section->mr)) || > + memory_region_is_protected(section->mr) || > + /* vhost-vDPA doesn't allow MMIO to be mapped */ > + memory_region_is_ram_device(section->mr); > + if (r) { > + return true; > + } > + > + if (section->offset_within_address_space < iova_min) { > + error_report("RAM section out of device range (min=%lu, addr=%lu)", > + iova_min, section->offset_within_address_space); > + return true; > + } > + > + llend = vhost_vdpa_section_end(section); > + if (int128_make64(llend) > iova_max) { I am puzzled by this. You are taking a Int128, converting to u64, converting back to Int128, and comparing to u64. Head spins. What is all this back and forth trying to achieve? > + error_report("RAM section out of device range (max=%lu, end addr=%lu)", > + iova_max, (uint64_t)int128_make64(llend)); > + return true; > + } > + > + return false; > } > > static int vhost_vdpa_dma_map(struct vhost_vdpa *v, hwaddr iova, hwaddr size, > @@ -158,7 +172,8 @@ static void vhost_vdpa_listener_region_add(MemoryListener *listener, > void *vaddr; > int ret; > > - if (vhost_vdpa_listener_skipped_section(section)) { > + if (vhost_vdpa_listener_skipped_section(section, v->iova_range.first, > + v->iova_range.last)) { > return; > } > > @@ -216,7 +231,8 @@ static void vhost_vdpa_listener_region_del(MemoryListener *listener, > Int128 llend, llsize; > int ret; > > - if (vhost_vdpa_listener_skipped_section(section)) { > + if (vhost_vdpa_listener_skipped_section(section, v->iova_range.first, > + v->iova_range.last)) { > return; > } > > @@ -284,9 +300,24 @@ static void vhost_vdpa_add_status(struct vhost_dev *dev, uint8_t status) > vhost_vdpa_call(dev, VHOST_VDPA_SET_STATUS, &s); > } > > +static int vhost_vdpa_get_iova_range(struct vhost_vdpa *v) > +{ > + int ret; > + > + ret = vhost_vdpa_call(v->dev, VHOST_VDPA_GET_IOVA_RANGE, &v->iova_range); > + if (ret != 0) { > + return ret; > + } > + > + trace_vhost_vdpa_get_iova_range(v->dev, v->iova_range.first, > + v->iova_range.last); > + return ret; > +} > + > static int vhost_vdpa_init(struct vhost_dev *dev, void *opaque, Error **errp) > { > struct vhost_vdpa *v; > + int r; > assert(dev->vhost_ops->backend_type == VHOST_BACKEND_TYPE_VDPA); > trace_vhost_vdpa_init(dev, opaque); > > @@ -296,6 +327,11 @@ static int vhost_vdpa_init(struct vhost_dev *dev, void *opaque, Error **errp) > v->listener = vhost_vdpa_memory_listener; > v->msg_type = VHOST_IOTLB_MSG_V2; > > + r = vhost_vdpa_get_iova_range(v); > + if (unlikely(!r)) { > + return r; > + } > + > vhost_vdpa_add_status(dev, VIRTIO_CONFIG_S_ACKNOWLEDGE | > VIRTIO_CONFIG_S_DRIVER); > > diff --git a/hw/virtio/trace-events b/hw/virtio/trace-events > index 8ed19e9d0c..650e521e35 100644 > --- a/hw/virtio/trace-events > +++ b/hw/virtio/trace-events > @@ -52,6 +52,7 @@ vhost_vdpa_set_vring_call(void *dev, unsigned int index, int fd) "dev: %p index: > vhost_vdpa_get_features(void *dev, uint64_t features) "dev: %p features: 0x%"PRIx64 > vhost_vdpa_set_owner(void *dev) "dev: %p" > vhost_vdpa_vq_get_addr(void *dev, void *vq, uint64_t desc_user_addr, uint64_t avail_user_addr, uint64_t used_user_addr) "dev: %p vq: %p desc_user_addr: 0x%"PRIx64" avail_user_addr: 0x%"PRIx64" used_user_addr: 0x%"PRIx64 > +vhost_vdpa_get_iova_range(void *dev, uint64_t first, uint64_t last) "dev: %p first: 0x%"PRIx64" last: 0x%"PRIx64 > > # virtio.c > virtqueue_alloc_element(void *elem, size_t sz, unsigned in_num, unsigned out_num) "elem %p size %zd in_num %u out_num %u" > -- > 2.27.0 ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 3/3] vdpa: Check for iova range at mappings changes 2021-10-05 8:14 ` Michael S. Tsirkin (?) @ 2021-10-05 9:58 ` Eugenio Perez Martin 2021-10-05 10:46 ` Michael S. Tsirkin -1 siblings, 1 reply; 15+ messages in thread From: Eugenio Perez Martin @ 2021-10-05 9:58 UTC (permalink / raw) To: Michael S. Tsirkin Cc: Parav Pandit, Jason Wang, qemu-level, virtualization, Stefan Hajnoczi, Eli Cohen, Stefano Garzarella On Tue, Oct 5, 2021 at 10:14 AM Michael S. Tsirkin <mst@redhat.com> wrote: > > On Tue, Oct 05, 2021 at 10:01:31AM +0200, Eugenio Pérez wrote: > > Check vdpa device range before updating memory regions so we don't add > > any outside of it, and report the invalid change if any. > > > > Signed-off-by: Eugenio Pérez <eperezma@redhat.com> > > --- > > include/hw/virtio/vhost-vdpa.h | 2 + > > hw/virtio/vhost-vdpa.c | 68 ++++++++++++++++++++++++++-------- > > hw/virtio/trace-events | 1 + > > 3 files changed, 55 insertions(+), 16 deletions(-) > > > > diff --git a/include/hw/virtio/vhost-vdpa.h b/include/hw/virtio/vhost-vdpa.h > > index a8963da2d9..c288cf7ecb 100644 > > --- a/include/hw/virtio/vhost-vdpa.h > > +++ b/include/hw/virtio/vhost-vdpa.h > > @@ -13,6 +13,7 @@ > > #define HW_VIRTIO_VHOST_VDPA_H > > > > #include "hw/virtio/virtio.h" > > +#include "standard-headers/linux/vhost_types.h" > > > > typedef struct VhostVDPAHostNotifier { > > MemoryRegion mr; > > @@ -24,6 +25,7 @@ typedef struct vhost_vdpa { > > uint32_t msg_type; > > bool iotlb_batch_begin_sent; > > MemoryListener listener; > > + struct vhost_vdpa_iova_range iova_range; > > struct vhost_dev *dev; > > VhostVDPAHostNotifier notifier[VIRTIO_QUEUE_MAX]; > > } VhostVDPA; > > diff --git a/hw/virtio/vhost-vdpa.c b/hw/virtio/vhost-vdpa.c > > index a1de6c7c9c..26d0258723 100644 > > --- a/hw/virtio/vhost-vdpa.c > > +++ b/hw/virtio/vhost-vdpa.c > > @@ -33,20 +33,34 @@ static Int128 vhost_vdpa_section_end(const MemoryRegionSection *section) > > return llend; > > } > > > > -static bool vhost_vdpa_listener_skipped_section(MemoryRegionSection *section) > > -{ > > - return (!memory_region_is_ram(section->mr) && > > - !memory_region_is_iommu(section->mr)) || > > - memory_region_is_protected(section->mr) || > > - /* vhost-vDPA doesn't allow MMIO to be mapped */ > > - memory_region_is_ram_device(section->mr) || > > - /* > > - * Sizing an enabled 64-bit BAR can cause spurious mappings to > > - * addresses in the upper part of the 64-bit address space. These > > - * are never accessed by the CPU and beyond the address width of > > - * some IOMMU hardware. TODO: VDPA should tell us the IOMMU width. > > - */ > > - section->offset_within_address_space & (1ULL << 63); > > +static bool vhost_vdpa_listener_skipped_section(MemoryRegionSection *section, > > + uint64_t iova_min, > > + uint64_t iova_max) > > +{ > > + Int128 llend; > > + bool r = (!memory_region_is_ram(section->mr) && > > + !memory_region_is_iommu(section->mr)) || > > + memory_region_is_protected(section->mr) || > > + /* vhost-vDPA doesn't allow MMIO to be mapped */ > > + memory_region_is_ram_device(section->mr); > > + if (r) { > > + return true; > > + } > > + > > + if (section->offset_within_address_space < iova_min) { > > + error_report("RAM section out of device range (min=%lu, addr=%lu)", > > + iova_min, section->offset_within_address_space); > > + return true; > > + } > > + > > + llend = vhost_vdpa_section_end(section); > > + if (int128_make64(llend) > iova_max) { > > I am puzzled by this. > You are taking a Int128, converting to u64, converting > back to Int128, and comparing to u64. > Head spins. What is all this back and forth trying to achieve? > You are totally right, this series was extracted from a longer one where I didn't use vhost_vdpa_section_end, but raw addresses. Then I applied int128_make64 to the wrong variable, too fast. To be sure we are on the same page, to do: if (int128_ge(int128_make64(iova), llend)) { // error message return; } The same way as vhost_vdpa_listener_region_{add,del} would be ok? Thanks! > > + error_report("RAM section out of device range (max=%lu, end addr=%lu)", > > + iova_max, (uint64_t)int128_make64(llend)); > > + return true; > > + } > > + > > + return false; > > } > > > > static int vhost_vdpa_dma_map(struct vhost_vdpa *v, hwaddr iova, hwaddr size, > > @@ -158,7 +172,8 @@ static void vhost_vdpa_listener_region_add(MemoryListener *listener, > > void *vaddr; > > int ret; > > > > - if (vhost_vdpa_listener_skipped_section(section)) { > > + if (vhost_vdpa_listener_skipped_section(section, v->iova_range.first, > > + v->iova_range.last)) { > > return; > > } > > > > @@ -216,7 +231,8 @@ static void vhost_vdpa_listener_region_del(MemoryListener *listener, > > Int128 llend, llsize; > > int ret; > > > > - if (vhost_vdpa_listener_skipped_section(section)) { > > + if (vhost_vdpa_listener_skipped_section(section, v->iova_range.first, > > + v->iova_range.last)) { > > return; > > } > > > > @@ -284,9 +300,24 @@ static void vhost_vdpa_add_status(struct vhost_dev *dev, uint8_t status) > > vhost_vdpa_call(dev, VHOST_VDPA_SET_STATUS, &s); > > } > > > > +static int vhost_vdpa_get_iova_range(struct vhost_vdpa *v) > > +{ > > + int ret; > > + > > + ret = vhost_vdpa_call(v->dev, VHOST_VDPA_GET_IOVA_RANGE, &v->iova_range); > > + if (ret != 0) { > > + return ret; > > + } > > + > > + trace_vhost_vdpa_get_iova_range(v->dev, v->iova_range.first, > > + v->iova_range.last); > > + return ret; > > +} > > + > > static int vhost_vdpa_init(struct vhost_dev *dev, void *opaque, Error **errp) > > { > > struct vhost_vdpa *v; > > + int r; > > assert(dev->vhost_ops->backend_type == VHOST_BACKEND_TYPE_VDPA); > > trace_vhost_vdpa_init(dev, opaque); > > > > @@ -296,6 +327,11 @@ static int vhost_vdpa_init(struct vhost_dev *dev, void *opaque, Error **errp) > > v->listener = vhost_vdpa_memory_listener; > > v->msg_type = VHOST_IOTLB_MSG_V2; > > > > + r = vhost_vdpa_get_iova_range(v); > > + if (unlikely(!r)) { > > + return r; > > + } > > + > > vhost_vdpa_add_status(dev, VIRTIO_CONFIG_S_ACKNOWLEDGE | > > VIRTIO_CONFIG_S_DRIVER); > > > > diff --git a/hw/virtio/trace-events b/hw/virtio/trace-events > > index 8ed19e9d0c..650e521e35 100644 > > --- a/hw/virtio/trace-events > > +++ b/hw/virtio/trace-events > > @@ -52,6 +52,7 @@ vhost_vdpa_set_vring_call(void *dev, unsigned int index, int fd) "dev: %p index: > > vhost_vdpa_get_features(void *dev, uint64_t features) "dev: %p features: 0x%"PRIx64 > > vhost_vdpa_set_owner(void *dev) "dev: %p" > > vhost_vdpa_vq_get_addr(void *dev, void *vq, uint64_t desc_user_addr, uint64_t avail_user_addr, uint64_t used_user_addr) "dev: %p vq: %p desc_user_addr: 0x%"PRIx64" avail_user_addr: 0x%"PRIx64" used_user_addr: 0x%"PRIx64 > > +vhost_vdpa_get_iova_range(void *dev, uint64_t first, uint64_t last) "dev: %p first: 0x%"PRIx64" last: 0x%"PRIx64 > > > > # virtio.c > > virtqueue_alloc_element(void *elem, size_t sz, unsigned in_num, unsigned out_num) "elem %p size %zd in_num %u out_num %u" > > -- > > 2.27.0 > ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 3/3] vdpa: Check for iova range at mappings changes 2021-10-05 9:58 ` Eugenio Perez Martin @ 2021-10-05 10:46 ` Michael S. Tsirkin 0 siblings, 0 replies; 15+ messages in thread From: Michael S. Tsirkin @ 2021-10-05 10:46 UTC (permalink / raw) To: Eugenio Perez Martin Cc: Parav Pandit, qemu-level, virtualization, Stefan Hajnoczi, Eli Cohen On Tue, Oct 05, 2021 at 11:58:12AM +0200, Eugenio Perez Martin wrote: > On Tue, Oct 5, 2021 at 10:14 AM Michael S. Tsirkin <mst@redhat.com> wrote: > > > > On Tue, Oct 05, 2021 at 10:01:31AM +0200, Eugenio Pérez wrote: > > > Check vdpa device range before updating memory regions so we don't add > > > any outside of it, and report the invalid change if any. > > > > > > Signed-off-by: Eugenio Pérez <eperezma@redhat.com> > > > --- > > > include/hw/virtio/vhost-vdpa.h | 2 + > > > hw/virtio/vhost-vdpa.c | 68 ++++++++++++++++++++++++++-------- > > > hw/virtio/trace-events | 1 + > > > 3 files changed, 55 insertions(+), 16 deletions(-) > > > > > > diff --git a/include/hw/virtio/vhost-vdpa.h b/include/hw/virtio/vhost-vdpa.h > > > index a8963da2d9..c288cf7ecb 100644 > > > --- a/include/hw/virtio/vhost-vdpa.h > > > +++ b/include/hw/virtio/vhost-vdpa.h > > > @@ -13,6 +13,7 @@ > > > #define HW_VIRTIO_VHOST_VDPA_H > > > > > > #include "hw/virtio/virtio.h" > > > +#include "standard-headers/linux/vhost_types.h" > > > > > > typedef struct VhostVDPAHostNotifier { > > > MemoryRegion mr; > > > @@ -24,6 +25,7 @@ typedef struct vhost_vdpa { > > > uint32_t msg_type; > > > bool iotlb_batch_begin_sent; > > > MemoryListener listener; > > > + struct vhost_vdpa_iova_range iova_range; > > > struct vhost_dev *dev; > > > VhostVDPAHostNotifier notifier[VIRTIO_QUEUE_MAX]; > > > } VhostVDPA; > > > diff --git a/hw/virtio/vhost-vdpa.c b/hw/virtio/vhost-vdpa.c > > > index a1de6c7c9c..26d0258723 100644 > > > --- a/hw/virtio/vhost-vdpa.c > > > +++ b/hw/virtio/vhost-vdpa.c > > > @@ -33,20 +33,34 @@ static Int128 vhost_vdpa_section_end(const MemoryRegionSection *section) > > > return llend; > > > } > > > > > > -static bool vhost_vdpa_listener_skipped_section(MemoryRegionSection *section) > > > -{ > > > - return (!memory_region_is_ram(section->mr) && > > > - !memory_region_is_iommu(section->mr)) || > > > - memory_region_is_protected(section->mr) || > > > - /* vhost-vDPA doesn't allow MMIO to be mapped */ > > > - memory_region_is_ram_device(section->mr) || > > > - /* > > > - * Sizing an enabled 64-bit BAR can cause spurious mappings to > > > - * addresses in the upper part of the 64-bit address space. These > > > - * are never accessed by the CPU and beyond the address width of > > > - * some IOMMU hardware. TODO: VDPA should tell us the IOMMU width. > > > - */ > > > - section->offset_within_address_space & (1ULL << 63); > > > +static bool vhost_vdpa_listener_skipped_section(MemoryRegionSection *section, > > > + uint64_t iova_min, > > > + uint64_t iova_max) > > > +{ > > > + Int128 llend; > > > + bool r = (!memory_region_is_ram(section->mr) && > > > + !memory_region_is_iommu(section->mr)) || > > > + memory_region_is_protected(section->mr) || > > > + /* vhost-vDPA doesn't allow MMIO to be mapped */ > > > + memory_region_is_ram_device(section->mr); > > > + if (r) { > > > + return true; > > > + } > > > + > > > + if (section->offset_within_address_space < iova_min) { > > > + error_report("RAM section out of device range (min=%lu, addr=%lu)", > > > + iova_min, section->offset_within_address_space); > > > + return true; > > > + } > > > + > > > + llend = vhost_vdpa_section_end(section); > > > + if (int128_make64(llend) > iova_max) { > > > > I am puzzled by this. > > You are taking a Int128, converting to u64, converting > > back to Int128, and comparing to u64. > > Head spins. What is all this back and forth trying to achieve? > > > > You are totally right, this series was extracted from a longer one > where I didn't use vhost_vdpa_section_end, but raw addresses. Then I > applied int128_make64 to the wrong variable, too fast. > > To be sure we are on the same page, to do: > > if (int128_ge(int128_make64(iova), llend)) { > // error message > return; > } > > The same way as vhost_vdpa_listener_region_{add,del} would be ok? > > Thanks! should be ok, yea > > > + error_report("RAM section out of device range (max=%lu, end addr=%lu)", > > > + iova_max, (uint64_t)int128_make64(llend)); > > > + return true; > > > + } > > > + > > > + return false; > > > } > > > > > > static int vhost_vdpa_dma_map(struct vhost_vdpa *v, hwaddr iova, hwaddr size, > > > @@ -158,7 +172,8 @@ static void vhost_vdpa_listener_region_add(MemoryListener *listener, > > > void *vaddr; > > > int ret; > > > > > > - if (vhost_vdpa_listener_skipped_section(section)) { > > > + if (vhost_vdpa_listener_skipped_section(section, v->iova_range.first, > > > + v->iova_range.last)) { > > > return; > > > } > > > > > > @@ -216,7 +231,8 @@ static void vhost_vdpa_listener_region_del(MemoryListener *listener, > > > Int128 llend, llsize; > > > int ret; > > > > > > - if (vhost_vdpa_listener_skipped_section(section)) { > > > + if (vhost_vdpa_listener_skipped_section(section, v->iova_range.first, > > > + v->iova_range.last)) { > > > return; > > > } > > > > > > @@ -284,9 +300,24 @@ static void vhost_vdpa_add_status(struct vhost_dev *dev, uint8_t status) > > > vhost_vdpa_call(dev, VHOST_VDPA_SET_STATUS, &s); > > > } > > > > > > +static int vhost_vdpa_get_iova_range(struct vhost_vdpa *v) > > > +{ > > > + int ret; > > > + > > > + ret = vhost_vdpa_call(v->dev, VHOST_VDPA_GET_IOVA_RANGE, &v->iova_range); > > > + if (ret != 0) { > > > + return ret; > > > + } > > > + > > > + trace_vhost_vdpa_get_iova_range(v->dev, v->iova_range.first, > > > + v->iova_range.last); > > > + return ret; > > > +} > > > + > > > static int vhost_vdpa_init(struct vhost_dev *dev, void *opaque, Error **errp) > > > { > > > struct vhost_vdpa *v; > > > + int r; > > > assert(dev->vhost_ops->backend_type == VHOST_BACKEND_TYPE_VDPA); > > > trace_vhost_vdpa_init(dev, opaque); > > > > > > @@ -296,6 +327,11 @@ static int vhost_vdpa_init(struct vhost_dev *dev, void *opaque, Error **errp) > > > v->listener = vhost_vdpa_memory_listener; > > > v->msg_type = VHOST_IOTLB_MSG_V2; > > > > > > + r = vhost_vdpa_get_iova_range(v); > > > + if (unlikely(!r)) { > > > + return r; > > > + } > > > + > > > vhost_vdpa_add_status(dev, VIRTIO_CONFIG_S_ACKNOWLEDGE | > > > VIRTIO_CONFIG_S_DRIVER); > > > > > > diff --git a/hw/virtio/trace-events b/hw/virtio/trace-events > > > index 8ed19e9d0c..650e521e35 100644 > > > --- a/hw/virtio/trace-events > > > +++ b/hw/virtio/trace-events > > > @@ -52,6 +52,7 @@ vhost_vdpa_set_vring_call(void *dev, unsigned int index, int fd) "dev: %p index: > > > vhost_vdpa_get_features(void *dev, uint64_t features) "dev: %p features: 0x%"PRIx64 > > > vhost_vdpa_set_owner(void *dev) "dev: %p" > > > vhost_vdpa_vq_get_addr(void *dev, void *vq, uint64_t desc_user_addr, uint64_t avail_user_addr, uint64_t used_user_addr) "dev: %p vq: %p desc_user_addr: 0x%"PRIx64" avail_user_addr: 0x%"PRIx64" used_user_addr: 0x%"PRIx64 > > > +vhost_vdpa_get_iova_range(void *dev, uint64_t first, uint64_t last) "dev: %p first: 0x%"PRIx64" last: 0x%"PRIx64 > > > > > > # virtio.c > > > virtqueue_alloc_element(void *elem, size_t sz, unsigned in_num, unsigned out_num) "elem %p size %zd in_num %u out_num %u" > > > -- > > > 2.27.0 > > _______________________________________________ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 3/3] vdpa: Check for iova range at mappings changes @ 2021-10-05 10:46 ` Michael S. Tsirkin 0 siblings, 0 replies; 15+ messages in thread From: Michael S. Tsirkin @ 2021-10-05 10:46 UTC (permalink / raw) To: Eugenio Perez Martin Cc: Parav Pandit, Jason Wang, qemu-level, virtualization, Stefan Hajnoczi, Eli Cohen, Stefano Garzarella On Tue, Oct 05, 2021 at 11:58:12AM +0200, Eugenio Perez Martin wrote: > On Tue, Oct 5, 2021 at 10:14 AM Michael S. Tsirkin <mst@redhat.com> wrote: > > > > On Tue, Oct 05, 2021 at 10:01:31AM +0200, Eugenio Pérez wrote: > > > Check vdpa device range before updating memory regions so we don't add > > > any outside of it, and report the invalid change if any. > > > > > > Signed-off-by: Eugenio Pérez <eperezma@redhat.com> > > > --- > > > include/hw/virtio/vhost-vdpa.h | 2 + > > > hw/virtio/vhost-vdpa.c | 68 ++++++++++++++++++++++++++-------- > > > hw/virtio/trace-events | 1 + > > > 3 files changed, 55 insertions(+), 16 deletions(-) > > > > > > diff --git a/include/hw/virtio/vhost-vdpa.h b/include/hw/virtio/vhost-vdpa.h > > > index a8963da2d9..c288cf7ecb 100644 > > > --- a/include/hw/virtio/vhost-vdpa.h > > > +++ b/include/hw/virtio/vhost-vdpa.h > > > @@ -13,6 +13,7 @@ > > > #define HW_VIRTIO_VHOST_VDPA_H > > > > > > #include "hw/virtio/virtio.h" > > > +#include "standard-headers/linux/vhost_types.h" > > > > > > typedef struct VhostVDPAHostNotifier { > > > MemoryRegion mr; > > > @@ -24,6 +25,7 @@ typedef struct vhost_vdpa { > > > uint32_t msg_type; > > > bool iotlb_batch_begin_sent; > > > MemoryListener listener; > > > + struct vhost_vdpa_iova_range iova_range; > > > struct vhost_dev *dev; > > > VhostVDPAHostNotifier notifier[VIRTIO_QUEUE_MAX]; > > > } VhostVDPA; > > > diff --git a/hw/virtio/vhost-vdpa.c b/hw/virtio/vhost-vdpa.c > > > index a1de6c7c9c..26d0258723 100644 > > > --- a/hw/virtio/vhost-vdpa.c > > > +++ b/hw/virtio/vhost-vdpa.c > > > @@ -33,20 +33,34 @@ static Int128 vhost_vdpa_section_end(const MemoryRegionSection *section) > > > return llend; > > > } > > > > > > -static bool vhost_vdpa_listener_skipped_section(MemoryRegionSection *section) > > > -{ > > > - return (!memory_region_is_ram(section->mr) && > > > - !memory_region_is_iommu(section->mr)) || > > > - memory_region_is_protected(section->mr) || > > > - /* vhost-vDPA doesn't allow MMIO to be mapped */ > > > - memory_region_is_ram_device(section->mr) || > > > - /* > > > - * Sizing an enabled 64-bit BAR can cause spurious mappings to > > > - * addresses in the upper part of the 64-bit address space. These > > > - * are never accessed by the CPU and beyond the address width of > > > - * some IOMMU hardware. TODO: VDPA should tell us the IOMMU width. > > > - */ > > > - section->offset_within_address_space & (1ULL << 63); > > > +static bool vhost_vdpa_listener_skipped_section(MemoryRegionSection *section, > > > + uint64_t iova_min, > > > + uint64_t iova_max) > > > +{ > > > + Int128 llend; > > > + bool r = (!memory_region_is_ram(section->mr) && > > > + !memory_region_is_iommu(section->mr)) || > > > + memory_region_is_protected(section->mr) || > > > + /* vhost-vDPA doesn't allow MMIO to be mapped */ > > > + memory_region_is_ram_device(section->mr); > > > + if (r) { > > > + return true; > > > + } > > > + > > > + if (section->offset_within_address_space < iova_min) { > > > + error_report("RAM section out of device range (min=%lu, addr=%lu)", > > > + iova_min, section->offset_within_address_space); > > > + return true; > > > + } > > > + > > > + llend = vhost_vdpa_section_end(section); > > > + if (int128_make64(llend) > iova_max) { > > > > I am puzzled by this. > > You are taking a Int128, converting to u64, converting > > back to Int128, and comparing to u64. > > Head spins. What is all this back and forth trying to achieve? > > > > You are totally right, this series was extracted from a longer one > where I didn't use vhost_vdpa_section_end, but raw addresses. Then I > applied int128_make64 to the wrong variable, too fast. > > To be sure we are on the same page, to do: > > if (int128_ge(int128_make64(iova), llend)) { > // error message > return; > } > > The same way as vhost_vdpa_listener_region_{add,del} would be ok? > > Thanks! should be ok, yea > > > + error_report("RAM section out of device range (max=%lu, end addr=%lu)", > > > + iova_max, (uint64_t)int128_make64(llend)); > > > + return true; > > > + } > > > + > > > + return false; > > > } > > > > > > static int vhost_vdpa_dma_map(struct vhost_vdpa *v, hwaddr iova, hwaddr size, > > > @@ -158,7 +172,8 @@ static void vhost_vdpa_listener_region_add(MemoryListener *listener, > > > void *vaddr; > > > int ret; > > > > > > - if (vhost_vdpa_listener_skipped_section(section)) { > > > + if (vhost_vdpa_listener_skipped_section(section, v->iova_range.first, > > > + v->iova_range.last)) { > > > return; > > > } > > > > > > @@ -216,7 +231,8 @@ static void vhost_vdpa_listener_region_del(MemoryListener *listener, > > > Int128 llend, llsize; > > > int ret; > > > > > > - if (vhost_vdpa_listener_skipped_section(section)) { > > > + if (vhost_vdpa_listener_skipped_section(section, v->iova_range.first, > > > + v->iova_range.last)) { > > > return; > > > } > > > > > > @@ -284,9 +300,24 @@ static void vhost_vdpa_add_status(struct vhost_dev *dev, uint8_t status) > > > vhost_vdpa_call(dev, VHOST_VDPA_SET_STATUS, &s); > > > } > > > > > > +static int vhost_vdpa_get_iova_range(struct vhost_vdpa *v) > > > +{ > > > + int ret; > > > + > > > + ret = vhost_vdpa_call(v->dev, VHOST_VDPA_GET_IOVA_RANGE, &v->iova_range); > > > + if (ret != 0) { > > > + return ret; > > > + } > > > + > > > + trace_vhost_vdpa_get_iova_range(v->dev, v->iova_range.first, > > > + v->iova_range.last); > > > + return ret; > > > +} > > > + > > > static int vhost_vdpa_init(struct vhost_dev *dev, void *opaque, Error **errp) > > > { > > > struct vhost_vdpa *v; > > > + int r; > > > assert(dev->vhost_ops->backend_type == VHOST_BACKEND_TYPE_VDPA); > > > trace_vhost_vdpa_init(dev, opaque); > > > > > > @@ -296,6 +327,11 @@ static int vhost_vdpa_init(struct vhost_dev *dev, void *opaque, Error **errp) > > > v->listener = vhost_vdpa_memory_listener; > > > v->msg_type = VHOST_IOTLB_MSG_V2; > > > > > > + r = vhost_vdpa_get_iova_range(v); > > > + if (unlikely(!r)) { > > > + return r; > > > + } > > > + > > > vhost_vdpa_add_status(dev, VIRTIO_CONFIG_S_ACKNOWLEDGE | > > > VIRTIO_CONFIG_S_DRIVER); > > > > > > diff --git a/hw/virtio/trace-events b/hw/virtio/trace-events > > > index 8ed19e9d0c..650e521e35 100644 > > > --- a/hw/virtio/trace-events > > > +++ b/hw/virtio/trace-events > > > @@ -52,6 +52,7 @@ vhost_vdpa_set_vring_call(void *dev, unsigned int index, int fd) "dev: %p index: > > > vhost_vdpa_get_features(void *dev, uint64_t features) "dev: %p features: 0x%"PRIx64 > > > vhost_vdpa_set_owner(void *dev) "dev: %p" > > > vhost_vdpa_vq_get_addr(void *dev, void *vq, uint64_t desc_user_addr, uint64_t avail_user_addr, uint64_t used_user_addr) "dev: %p vq: %p desc_user_addr: 0x%"PRIx64" avail_user_addr: 0x%"PRIx64" used_user_addr: 0x%"PRIx64 > > > +vhost_vdpa_get_iova_range(void *dev, uint64_t first, uint64_t last) "dev: %p first: 0x%"PRIx64" last: 0x%"PRIx64 > > > > > > # virtio.c > > > virtqueue_alloc_element(void *elem, size_t sz, unsigned in_num, unsigned out_num) "elem %p size %zd in_num %u out_num %u" > > > -- > > > 2.27.0 > > ^ permalink raw reply [flat|nested] 15+ messages in thread
end of thread, other threads:[~2021-10-05 13:18 UTC | newest] Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2021-10-05 8:01 [PATCH 0/3] vdpa: Check iova range on memory regions ops Eugenio Pérez 2021-10-05 8:01 ` [PATCH 1/3] vdpa: Skip protected ram IOMMU mappings Eugenio Pérez 2021-10-05 8:01 ` [PATCH 2/3] vdpa: Add vhost_vdpa_section_end Eugenio Pérez 2021-10-05 8:15 ` Michael S. Tsirkin 2021-10-05 8:15 ` Michael S. Tsirkin 2021-10-05 9:52 ` Eugenio Perez Martin 2021-10-05 10:47 ` Michael S. Tsirkin 2021-10-05 10:47 ` Michael S. Tsirkin 2021-10-05 13:14 ` Eugenio Perez Martin 2021-10-05 8:01 ` [PATCH 3/3] vdpa: Check for iova range at mappings changes Eugenio Pérez 2021-10-05 8:14 ` Michael S. Tsirkin 2021-10-05 8:14 ` Michael S. Tsirkin 2021-10-05 9:58 ` Eugenio Perez Martin 2021-10-05 10:46 ` Michael S. Tsirkin 2021-10-05 10:46 ` Michael S. Tsirkin
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.