| Message ID | 20230704071931.575888-2-lvivier@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | vhost-vdpa: skip TPM CRB memory section | expand |
On 04.07.23 09:19, Laurent Vivier wrote: > With TPM CRM device, vhost-vdpa reports an error when it tries > to register a listener for a non aligned memory region: > > qemu-system-x86_64: vhost_vdpa_listener_region_add received unaligned region > qemu-system-x86_64: vhost_vdpa_listener_region_del received unaligned region > > This error can be confusing for the user whereas we only need to skip > the region (as it's already done after the error_report()) > > Rather than introducing a special case for TPM CRB memory section > to not display the message in this case, simply replace the > error_report() by a trace function (with more information, like the > memory region name). > > Signed-off-by: Laurent Vivier <lvivier@redhat.com> > --- > hw/virtio/trace-events | 2 ++ > hw/virtio/vhost-vdpa.c | 8 ++++++-- > 2 files changed, 8 insertions(+), 2 deletions(-) > > diff --git a/hw/virtio/trace-events b/hw/virtio/trace-events > index 8f8d05cf9b01..9b0d643b9475 100644 > --- a/hw/virtio/trace-events > +++ b/hw/virtio/trace-events > @@ -34,7 +34,9 @@ vhost_vdpa_dma_map(void *vdpa, int fd, uint32_t msg_type, uint32_t asid, uint64_ > vhost_vdpa_dma_unmap(void *vdpa, int fd, uint32_t msg_type, uint32_t asid, uint64_t iova, uint64_t size, uint8_t type) "vdpa:%p fd: %d msg_type: %"PRIu32" asid: %"PRIu32" iova: 0x%"PRIx64" size: 0x%"PRIx64" type: %"PRIu8 > vhost_vdpa_listener_begin_batch(void *v, int fd, uint32_t msg_type, uint8_t type) "vdpa:%p fd: %d msg_type: %"PRIu32" type: %"PRIu8 > vhost_vdpa_listener_commit(void *v, int fd, uint32_t msg_type, uint8_t type) "vdpa:%p fd: %d msg_type: %"PRIu32" type: %"PRIu8 > +vhost_vdpa_listener_region_add_unaligned(void *v, const char *name, uint64_t offset_as, uint64_t offset_page) "vdpa: %p region %s offset_within_address_space %"PRIu64" offset_within_region %"PRIu64 > vhost_vdpa_listener_region_add(void *vdpa, uint64_t iova, uint64_t llend, void *vaddr, bool readonly) "vdpa: %p iova 0x%"PRIx64" llend 0x%"PRIx64" vaddr: %p read-only: %d" > +vhost_vdpa_listener_region_del_unaligned(void *v, const char *name, uint64_t offset_as, uint64_t offset_page) "vdpa: %p region %s offset_within_address_space %"PRIu64" offset_within_region %"PRIu64 > vhost_vdpa_listener_region_del(void *vdpa, uint64_t iova, uint64_t llend) "vdpa: %p iova 0x%"PRIx64" llend 0x%"PRIx64 > vhost_vdpa_add_status(void *dev, uint8_t status) "dev: %p status: 0x%"PRIx8 > vhost_vdpa_init(void *dev, void *vdpa) "dev: %p vdpa: %p" > diff --git a/hw/virtio/vhost-vdpa.c b/hw/virtio/vhost-vdpa.c > index 3c575a9a6e9e..24d32f0d3728 100644 > --- a/hw/virtio/vhost-vdpa.c > +++ b/hw/virtio/vhost-vdpa.c > @@ -323,7 +323,9 @@ static void vhost_vdpa_listener_region_add(MemoryListener *listener, > > if (unlikely((section->offset_within_address_space & ~TARGET_PAGE_MASK) != > (section->offset_within_region & ~TARGET_PAGE_MASK))) { > - error_report("%s received unaligned region", __func__); > + trace_vhost_vdpa_listener_region_add_unaligned(v, section->mr->name, > + section->offset_within_address_space & ~TARGET_PAGE_MASK, > + section->offset_within_region & ~TARGET_PAGE_MASK); > return; > } > > @@ -405,7 +407,9 @@ static void vhost_vdpa_listener_region_del(MemoryListener *listener, > > if (unlikely((section->offset_within_address_space & ~TARGET_PAGE_MASK) != > (section->offset_within_region & ~TARGET_PAGE_MASK))) { > - error_report("%s received unaligned region", __func__); > + trace_vhost_vdpa_listener_region_del_unaligned(v, section->mr->name, > + section->offset_within_address_space & ~TARGET_PAGE_MASK, > + section->offset_within_region & ~TARGET_PAGE_MASK); > return; > } > Reviewed-by: David Hildenbrand <david@redhat.com> Do we also want to touch the vfio side in vfio_listener_valid_section(), or why is that one unaffected?
On 7/4/23 09:25, David Hildenbrand wrote: > On 04.07.23 09:19, Laurent Vivier wrote: >> With TPM CRM device, vhost-vdpa reports an error when it tries >> to register a listener for a non aligned memory region: >> >> qemu-system-x86_64: vhost_vdpa_listener_region_add received unaligned region >> qemu-system-x86_64: vhost_vdpa_listener_region_del received unaligned region >> >> This error can be confusing for the user whereas we only need to skip >> the region (as it's already done after the error_report()) >> >> Rather than introducing a special case for TPM CRB memory section >> to not display the message in this case, simply replace the >> error_report() by a trace function (with more information, like the >> memory region name). >> >> Signed-off-by: Laurent Vivier <lvivier@redhat.com> >> --- >> hw/virtio/trace-events | 2 ++ >> hw/virtio/vhost-vdpa.c | 8 ++++++-- >> 2 files changed, 8 insertions(+), 2 deletions(-) >> >> diff --git a/hw/virtio/trace-events b/hw/virtio/trace-events >> index 8f8d05cf9b01..9b0d643b9475 100644 >> --- a/hw/virtio/trace-events >> +++ b/hw/virtio/trace-events >> @@ -34,7 +34,9 @@ vhost_vdpa_dma_map(void *vdpa, int fd, uint32_t msg_type, uint32_t >> asid, uint64_ >> vhost_vdpa_dma_unmap(void *vdpa, int fd, uint32_t msg_type, uint32_t asid, uint64_t >> iova, uint64_t size, uint8_t type) "vdpa:%p fd: %d msg_type: %"PRIu32" asid: %"PRIu32" >> iova: 0x%"PRIx64" size: 0x%"PRIx64" type: %"PRIu8 >> vhost_vdpa_listener_begin_batch(void *v, int fd, uint32_t msg_type, uint8_t type) >> "vdpa:%p fd: %d msg_type: %"PRIu32" type: %"PRIu8 >> vhost_vdpa_listener_commit(void *v, int fd, uint32_t msg_type, uint8_t type) "vdpa:%p >> fd: %d msg_type: %"PRIu32" type: %"PRIu8 >> +vhost_vdpa_listener_region_add_unaligned(void *v, const char *name, uint64_t offset_as, >> uint64_t offset_page) "vdpa: %p region %s offset_within_address_space %"PRIu64" >> offset_within_region %"PRIu64 >> vhost_vdpa_listener_region_add(void *vdpa, uint64_t iova, uint64_t llend, void *vaddr, >> bool readonly) "vdpa: %p iova 0x%"PRIx64" llend 0x%"PRIx64" vaddr: %p read-only: %d" >> +vhost_vdpa_listener_region_del_unaligned(void *v, const char *name, uint64_t offset_as, >> uint64_t offset_page) "vdpa: %p region %s offset_within_address_space %"PRIu64" >> offset_within_region %"PRIu64 >> vhost_vdpa_listener_region_del(void *vdpa, uint64_t iova, uint64_t llend) "vdpa: %p >> iova 0x%"PRIx64" llend 0x%"PRIx64 >> vhost_vdpa_add_status(void *dev, uint8_t status) "dev: %p status: 0x%"PRIx8 >> vhost_vdpa_init(void *dev, void *vdpa) "dev: %p vdpa: %p" >> diff --git a/hw/virtio/vhost-vdpa.c b/hw/virtio/vhost-vdpa.c >> index 3c575a9a6e9e..24d32f0d3728 100644 >> --- a/hw/virtio/vhost-vdpa.c >> +++ b/hw/virtio/vhost-vdpa.c >> @@ -323,7 +323,9 @@ static void vhost_vdpa_listener_region_add(MemoryListener *listener, >> if (unlikely((section->offset_within_address_space & ~TARGET_PAGE_MASK) != >> (section->offset_within_region & ~TARGET_PAGE_MASK))) { >> - error_report("%s received unaligned region", __func__); >> + trace_vhost_vdpa_listener_region_add_unaligned(v, section->mr->name, >> + section->offset_within_address_space & ~TARGET_PAGE_MASK, >> + section->offset_within_region & ~TARGET_PAGE_MASK); >> return; >> } >> @@ -405,7 +407,9 @@ static void vhost_vdpa_listener_region_del(MemoryListener *listener, >> if (unlikely((section->offset_within_address_space & ~TARGET_PAGE_MASK) != >> (section->offset_within_region & ~TARGET_PAGE_MASK))) { >> - error_report("%s received unaligned region", __func__); >> + trace_vhost_vdpa_listener_region_del_unaligned(v, section->mr->name, >> + section->offset_within_address_space & ~TARGET_PAGE_MASK, >> + section->offset_within_region & ~TARGET_PAGE_MASK); >> return; >> } > > Reviewed-by: David Hildenbrand <david@redhat.com> > > > Do we also want to touch the vfio side in vfio_listener_valid_section(), or why is that > one unaffected? > I don't know if we can apply the same solution for VFIO. I don't know if the error message is relevant or if we can keep only the trace and remove the error_report() for all the cases (in this case vfio_known_safe_misalignment() becomes useless). Thanks, Laurent
On Tue, Jul 4, 2023 at 3:19 PM Laurent Vivier <lvivier@redhat.com> wrote: > > With TPM CRM device, vhost-vdpa reports an error when it tries > to register a listener for a non aligned memory region: > > qemu-system-x86_64: vhost_vdpa_listener_region_add received unaligned region > qemu-system-x86_64: vhost_vdpa_listener_region_del received unaligned region > > This error can be confusing for the user whereas we only need to skip > the region (as it's already done after the error_report()) > > Rather than introducing a special case for TPM CRB memory section > to not display the message in this case, simply replace the > error_report() by a trace function (with more information, like the > memory region name). > > Signed-off-by: Laurent Vivier <lvivier@redhat.com> Acked-by: Jason Wang <jasowang@redhat.com> Thanks > --- > hw/virtio/trace-events | 2 ++ > hw/virtio/vhost-vdpa.c | 8 ++++++-- > 2 files changed, 8 insertions(+), 2 deletions(-) > > diff --git a/hw/virtio/trace-events b/hw/virtio/trace-events > index 8f8d05cf9b01..9b0d643b9475 100644 > --- a/hw/virtio/trace-events > +++ b/hw/virtio/trace-events > @@ -34,7 +34,9 @@ vhost_vdpa_dma_map(void *vdpa, int fd, uint32_t msg_type, uint32_t asid, uint64_ > vhost_vdpa_dma_unmap(void *vdpa, int fd, uint32_t msg_type, uint32_t asid, uint64_t iova, uint64_t size, uint8_t type) "vdpa:%p fd: %d msg_type: %"PRIu32" asid: %"PRIu32" iova: 0x%"PRIx64" size: 0x%"PRIx64" type: %"PRIu8 > vhost_vdpa_listener_begin_batch(void *v, int fd, uint32_t msg_type, uint8_t type) "vdpa:%p fd: %d msg_type: %"PRIu32" type: %"PRIu8 > vhost_vdpa_listener_commit(void *v, int fd, uint32_t msg_type, uint8_t type) "vdpa:%p fd: %d msg_type: %"PRIu32" type: %"PRIu8 > +vhost_vdpa_listener_region_add_unaligned(void *v, const char *name, uint64_t offset_as, uint64_t offset_page) "vdpa: %p region %s offset_within_address_space %"PRIu64" offset_within_region %"PRIu64 > vhost_vdpa_listener_region_add(void *vdpa, uint64_t iova, uint64_t llend, void *vaddr, bool readonly) "vdpa: %p iova 0x%"PRIx64" llend 0x%"PRIx64" vaddr: %p read-only: %d" > +vhost_vdpa_listener_region_del_unaligned(void *v, const char *name, uint64_t offset_as, uint64_t offset_page) "vdpa: %p region %s offset_within_address_space %"PRIu64" offset_within_region %"PRIu64 > vhost_vdpa_listener_region_del(void *vdpa, uint64_t iova, uint64_t llend) "vdpa: %p iova 0x%"PRIx64" llend 0x%"PRIx64 > vhost_vdpa_add_status(void *dev, uint8_t status) "dev: %p status: 0x%"PRIx8 > vhost_vdpa_init(void *dev, void *vdpa) "dev: %p vdpa: %p" > diff --git a/hw/virtio/vhost-vdpa.c b/hw/virtio/vhost-vdpa.c > index 3c575a9a6e9e..24d32f0d3728 100644 > --- a/hw/virtio/vhost-vdpa.c > +++ b/hw/virtio/vhost-vdpa.c > @@ -323,7 +323,9 @@ static void vhost_vdpa_listener_region_add(MemoryListener *listener, > > if (unlikely((section->offset_within_address_space & ~TARGET_PAGE_MASK) != > (section->offset_within_region & ~TARGET_PAGE_MASK))) { > - error_report("%s received unaligned region", __func__); > + trace_vhost_vdpa_listener_region_add_unaligned(v, section->mr->name, > + section->offset_within_address_space & ~TARGET_PAGE_MASK, > + section->offset_within_region & ~TARGET_PAGE_MASK); > return; > } > > @@ -405,7 +407,9 @@ static void vhost_vdpa_listener_region_del(MemoryListener *listener, > > if (unlikely((section->offset_within_address_space & ~TARGET_PAGE_MASK) != > (section->offset_within_region & ~TARGET_PAGE_MASK))) { > - error_report("%s received unaligned region", __func__); > + trace_vhost_vdpa_listener_region_del_unaligned(v, section->mr->name, > + section->offset_within_address_space & ~TARGET_PAGE_MASK, > + section->offset_within_region & ~TARGET_PAGE_MASK); > return; > } > > -- > 2.41.0 >
diff --git a/hw/virtio/trace-events b/hw/virtio/trace-events index 8f8d05cf9b01..9b0d643b9475 100644 --- a/hw/virtio/trace-events +++ b/hw/virtio/trace-events @@ -34,7 +34,9 @@ vhost_vdpa_dma_map(void *vdpa, int fd, uint32_t msg_type, uint32_t asid, uint64_ vhost_vdpa_dma_unmap(void *vdpa, int fd, uint32_t msg_type, uint32_t asid, uint64_t iova, uint64_t size, uint8_t type) "vdpa:%p fd: %d msg_type: %"PRIu32" asid: %"PRIu32" iova: 0x%"PRIx64" size: 0x%"PRIx64" type: %"PRIu8 vhost_vdpa_listener_begin_batch(void *v, int fd, uint32_t msg_type, uint8_t type) "vdpa:%p fd: %d msg_type: %"PRIu32" type: %"PRIu8 vhost_vdpa_listener_commit(void *v, int fd, uint32_t msg_type, uint8_t type) "vdpa:%p fd: %d msg_type: %"PRIu32" type: %"PRIu8 +vhost_vdpa_listener_region_add_unaligned(void *v, const char *name, uint64_t offset_as, uint64_t offset_page) "vdpa: %p region %s offset_within_address_space %"PRIu64" offset_within_region %"PRIu64 vhost_vdpa_listener_region_add(void *vdpa, uint64_t iova, uint64_t llend, void *vaddr, bool readonly) "vdpa: %p iova 0x%"PRIx64" llend 0x%"PRIx64" vaddr: %p read-only: %d" +vhost_vdpa_listener_region_del_unaligned(void *v, const char *name, uint64_t offset_as, uint64_t offset_page) "vdpa: %p region %s offset_within_address_space %"PRIu64" offset_within_region %"PRIu64 vhost_vdpa_listener_region_del(void *vdpa, uint64_t iova, uint64_t llend) "vdpa: %p iova 0x%"PRIx64" llend 0x%"PRIx64 vhost_vdpa_add_status(void *dev, uint8_t status) "dev: %p status: 0x%"PRIx8 vhost_vdpa_init(void *dev, void *vdpa) "dev: %p vdpa: %p" diff --git a/hw/virtio/vhost-vdpa.c b/hw/virtio/vhost-vdpa.c index 3c575a9a6e9e..24d32f0d3728 100644 --- a/hw/virtio/vhost-vdpa.c +++ b/hw/virtio/vhost-vdpa.c @@ -323,7 +323,9 @@ static void vhost_vdpa_listener_region_add(MemoryListener *listener, if (unlikely((section->offset_within_address_space & ~TARGET_PAGE_MASK) != (section->offset_within_region & ~TARGET_PAGE_MASK))) { - error_report("%s received unaligned region", __func__); + trace_vhost_vdpa_listener_region_add_unaligned(v, section->mr->name, + section->offset_within_address_space & ~TARGET_PAGE_MASK, + section->offset_within_region & ~TARGET_PAGE_MASK); return; } @@ -405,7 +407,9 @@ static void vhost_vdpa_listener_region_del(MemoryListener *listener, if (unlikely((section->offset_within_address_space & ~TARGET_PAGE_MASK) != (section->offset_within_region & ~TARGET_PAGE_MASK))) { - error_report("%s received unaligned region", __func__); + trace_vhost_vdpa_listener_region_del_unaligned(v, section->mr->name, + section->offset_within_address_space & ~TARGET_PAGE_MASK, + section->offset_within_region & ~TARGET_PAGE_MASK); return; }
With TPM CRM device, vhost-vdpa reports an error when it tries to register a listener for a non aligned memory region: qemu-system-x86_64: vhost_vdpa_listener_region_add received unaligned region qemu-system-x86_64: vhost_vdpa_listener_region_del received unaligned region This error can be confusing for the user whereas we only need to skip the region (as it's already done after the error_report()) Rather than introducing a special case for TPM CRB memory section to not display the message in this case, simply replace the error_report() by a trace function (with more information, like the memory region name). Signed-off-by: Laurent Vivier <lvivier@redhat.com> --- hw/virtio/trace-events | 2 ++ hw/virtio/vhost-vdpa.c | 8 ++++++-- 2 files changed, 8 insertions(+), 2 deletions(-)