Re: [PATCH v2 2/2] hw/vfio/common: Silence ram device offset alignment error traces

[Alex Williamson]

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