Message ID | 20220118153306.282681-3-eric.auger@redhat.com (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
Series | TPM-CRB: Remove spurious error report when used with VFIO | expand |
On Tue, 18 Jan 2022 16:33:06 +0100 Eric Auger <eric.auger@redhat.com> wrote: > Failing to DMA MAP a ram_device should not cause an error message. > This is currently happening with the TPM CRB command region and > this is causing confusion. > > We may want to keep the trace for debug purpose though. > > Signed-off-by: Eric Auger <eric.auger@redhat.com> > Tested-by: Stefan Berger <stefanb@linux.ibm.com> > --- > hw/vfio/common.c | 15 ++++++++++++++- > hw/vfio/trace-events | 1 + > 2 files changed, 15 insertions(+), 1 deletion(-) Thanks! Looks good to me. Stefan, I can provide an ack here if you want to send a pull request for both or likewise I can send a pull request with your ack on the previous patch. I suppose the patches themselves are technically independent if you want to split them. Whichever you prefer. Acked-by: Alex Williamson <alex.williamson@redhat.com> > > diff --git a/hw/vfio/common.c b/hw/vfio/common.c > index 080046e3f5..9caa560b07 100644 > --- a/hw/vfio/common.c > +++ b/hw/vfio/common.c > @@ -884,7 +884,20 @@ static void vfio_listener_region_add(MemoryListener *listener, > if (unlikely((section->offset_within_address_space & > ~qemu_real_host_page_mask) != > (section->offset_within_region & ~qemu_real_host_page_mask))) { > - error_report("%s received unaligned region", __func__); > + if (memory_region_is_ram_device(section->mr)) { /* just debug purpose */ > + trace_vfio_listener_region_add_bad_offset_alignment( > + memory_region_name(section->mr), > + section->offset_within_address_space, > + section->offset_within_region, qemu_real_host_page_size); > + } else { /* error case we don't want to be fatal */ > + error_report("%s received unaligned region %s iova=0x%"PRIx64 > + " offset_within_region=0x%"PRIx64 > + " qemu_real_host_page_mask=0x%"PRIx64, > + __func__, memory_region_name(section->mr), > + section->offset_within_address_space, > + section->offset_within_region, > + qemu_real_host_page_mask); > + } > return; > } > > diff --git a/hw/vfio/trace-events b/hw/vfio/trace-events > index 0ef1b5f4a6..ccd9d7610d 100644 > --- a/hw/vfio/trace-events > +++ b/hw/vfio/trace-events > @@ -100,6 +100,7 @@ vfio_listener_region_add_skip(uint64_t start, uint64_t end) "SKIPPING region_add > vfio_spapr_group_attach(int groupfd, int tablefd) "Attached groupfd %d to liobn fd %d" > vfio_listener_region_add_iommu(uint64_t start, uint64_t end) "region_add [iommu] 0x%"PRIx64" - 0x%"PRIx64 > vfio_listener_region_add_ram(uint64_t iova_start, uint64_t iova_end, void *vaddr) "region_add [ram] 0x%"PRIx64" - 0x%"PRIx64" [%p]" > +vfio_listener_region_add_bad_offset_alignment(const char *name, uint64_t iova, uint64_t offset_within_region, uint64_t page_size) "Region \"%s\" @0x%"PRIx64", offset_within_region=0x%"PRIx64", qemu_real_host_page_mask=0x%"PRIx64 " cannot be mapped for DMA" > vfio_listener_region_add_no_dma_map(const char *name, uint64_t iova, uint64_t size, uint64_t page_size) "Region \"%s\" 0x%"PRIx64" size=0x%"PRIx64" is not aligned to 0x%"PRIx64" and cannot be mapped for DMA" > vfio_listener_region_del_skip(uint64_t start, uint64_t end) "SKIPPING region_del 0x%"PRIx64" - 0x%"PRIx64 > vfio_listener_region_del(uint64_t start, uint64_t end) "region_del 0x%"PRIx64" - 0x%"PRIx64
On 1/19/22 15:13, Alex Williamson wrote: > On Tue, 18 Jan 2022 16:33:06 +0100 > Eric Auger <eric.auger@redhat.com> wrote: > >> Failing to DMA MAP a ram_device should not cause an error message. >> This is currently happening with the TPM CRB command region and >> this is causing confusion. >> >> We may want to keep the trace for debug purpose though. >> >> Signed-off-by: Eric Auger <eric.auger@redhat.com> >> Tested-by: Stefan Berger <stefanb@linux.ibm.com> >> --- >> hw/vfio/common.c | 15 ++++++++++++++- >> hw/vfio/trace-events | 1 + >> 2 files changed, 15 insertions(+), 1 deletion(-) > Thanks! Looks good to me. > > Stefan, I can provide an ack here if you want to send a pull request > for both or likewise I can send a pull request with your ack on the > previous patch. I suppose the patches themselves are technically > independent if you want to split them. Whichever you prefer. > > Acked-by: Alex Williamson <alex.williamson@redhat.com> If you want to send the PR, please go ahead. Stefan > >> diff --git a/hw/vfio/common.c b/hw/vfio/common.c >> index 080046e3f5..9caa560b07 100644 >> --- a/hw/vfio/common.c >> +++ b/hw/vfio/common.c >> @@ -884,7 +884,20 @@ static void vfio_listener_region_add(MemoryListener *listener, >> if (unlikely((section->offset_within_address_space & >> ~qemu_real_host_page_mask) != >> (section->offset_within_region & ~qemu_real_host_page_mask))) { >> - error_report("%s received unaligned region", __func__); >> + if (memory_region_is_ram_device(section->mr)) { /* just debug purpose */ >> + trace_vfio_listener_region_add_bad_offset_alignment( >> + memory_region_name(section->mr), >> + section->offset_within_address_space, >> + section->offset_within_region, qemu_real_host_page_size); >> + } else { /* error case we don't want to be fatal */ >> + error_report("%s received unaligned region %s iova=0x%"PRIx64 >> + " offset_within_region=0x%"PRIx64 >> + " qemu_real_host_page_mask=0x%"PRIx64, >> + __func__, memory_region_name(section->mr), >> + section->offset_within_address_space, >> + section->offset_within_region, >> + qemu_real_host_page_mask); >> + } >> return; >> } >> >> diff --git a/hw/vfio/trace-events b/hw/vfio/trace-events >> index 0ef1b5f4a6..ccd9d7610d 100644 >> --- a/hw/vfio/trace-events >> +++ b/hw/vfio/trace-events >> @@ -100,6 +100,7 @@ vfio_listener_region_add_skip(uint64_t start, uint64_t end) "SKIPPING region_add >> vfio_spapr_group_attach(int groupfd, int tablefd) "Attached groupfd %d to liobn fd %d" >> vfio_listener_region_add_iommu(uint64_t start, uint64_t end) "region_add [iommu] 0x%"PRIx64" - 0x%"PRIx64 >> vfio_listener_region_add_ram(uint64_t iova_start, uint64_t iova_end, void *vaddr) "region_add [ram] 0x%"PRIx64" - 0x%"PRIx64" [%p]" >> +vfio_listener_region_add_bad_offset_alignment(const char *name, uint64_t iova, uint64_t offset_within_region, uint64_t page_size) "Region \"%s\" @0x%"PRIx64", offset_within_region=0x%"PRIx64", qemu_real_host_page_mask=0x%"PRIx64 " cannot be mapped for DMA" >> vfio_listener_region_add_no_dma_map(const char *name, uint64_t iova, uint64_t size, uint64_t page_size) "Region \"%s\" 0x%"PRIx64" size=0x%"PRIx64" is not aligned to 0x%"PRIx64" and cannot be mapped for DMA" >> vfio_listener_region_del_skip(uint64_t start, uint64_t end) "SKIPPING region_del 0x%"PRIx64" - 0x%"PRIx64 >> vfio_listener_region_del(uint64_t start, uint64_t end) "region_del 0x%"PRIx64" - 0x%"PRIx64
diff --git a/hw/vfio/common.c b/hw/vfio/common.c index 080046e3f5..9caa560b07 100644 --- a/hw/vfio/common.c +++ b/hw/vfio/common.c @@ -884,7 +884,20 @@ static void vfio_listener_region_add(MemoryListener *listener, if (unlikely((section->offset_within_address_space & ~qemu_real_host_page_mask) != (section->offset_within_region & ~qemu_real_host_page_mask))) { - error_report("%s received unaligned region", __func__); + if (memory_region_is_ram_device(section->mr)) { /* just debug purpose */ + trace_vfio_listener_region_add_bad_offset_alignment( + memory_region_name(section->mr), + section->offset_within_address_space, + section->offset_within_region, qemu_real_host_page_size); + } else { /* error case we don't want to be fatal */ + error_report("%s received unaligned region %s iova=0x%"PRIx64 + " offset_within_region=0x%"PRIx64 + " qemu_real_host_page_mask=0x%"PRIx64, + __func__, memory_region_name(section->mr), + section->offset_within_address_space, + section->offset_within_region, + qemu_real_host_page_mask); + } return; } diff --git a/hw/vfio/trace-events b/hw/vfio/trace-events index 0ef1b5f4a6..ccd9d7610d 100644 --- a/hw/vfio/trace-events +++ b/hw/vfio/trace-events @@ -100,6 +100,7 @@ vfio_listener_region_add_skip(uint64_t start, uint64_t end) "SKIPPING region_add vfio_spapr_group_attach(int groupfd, int tablefd) "Attached groupfd %d to liobn fd %d" vfio_listener_region_add_iommu(uint64_t start, uint64_t end) "region_add [iommu] 0x%"PRIx64" - 0x%"PRIx64 vfio_listener_region_add_ram(uint64_t iova_start, uint64_t iova_end, void *vaddr) "region_add [ram] 0x%"PRIx64" - 0x%"PRIx64" [%p]" +vfio_listener_region_add_bad_offset_alignment(const char *name, uint64_t iova, uint64_t offset_within_region, uint64_t page_size) "Region \"%s\" @0x%"PRIx64", offset_within_region=0x%"PRIx64", qemu_real_host_page_mask=0x%"PRIx64 " cannot be mapped for DMA" vfio_listener_region_add_no_dma_map(const char *name, uint64_t iova, uint64_t size, uint64_t page_size) "Region \"%s\" 0x%"PRIx64" size=0x%"PRIx64" is not aligned to 0x%"PRIx64" and cannot be mapped for DMA" vfio_listener_region_del_skip(uint64_t start, uint64_t end) "SKIPPING region_del 0x%"PRIx64" - 0x%"PRIx64 vfio_listener_region_del(uint64_t start, uint64_t end) "region_del 0x%"PRIx64" - 0x%"PRIx64