Re: [PATCH v5 06/15] vfio/common: Use a single tracepoint for skipped sections

[Cédric Le Goater]

On 3/7/23 13:54, Joao Martins wrote:
> In preparation to turn more of the memory listener checks into
> common functions, one of the affected places is how we trace when
> sections are skipped. Right now there is one for each. Change it
> into one single tracepoint `vfio_listener_region_skip` which receives
> a name which refers to the callback i.e. region_add and region_del.
>
> Suggested-by: Avihai Horon <avihaih@nvidia.com>
> Signed-off-by: Joao Martins <joao.m.martins@oracle.com>

Reviewed-by: Cédric Le Goater <clg@redhat.com>

Thanks,

C.

> ---
>   hw/vfio/common.c     | 4 ++--
>   hw/vfio/trace-events | 3 +--
>   2 files changed, 3 insertions(+), 4 deletions(-)
>
> diff --git a/hw/vfio/common.c b/hw/vfio/common.c
> index 99acb998eb14..1cb62efa9743 100644
> --- a/hw/vfio/common.c
> +++ b/hw/vfio/common.c
> @@ -945,7 +945,7 @@ static void vfio_listener_region_add(MemoryListener 
> *listener,
>       Error *err = NULL;
>   
>       if (vfio_listener_skipped_section(section)) {
> -        trace_vfio_listener_region_add_skip(
> +        trace_vfio_listener_region_skip("region_add",
>                   section->offset_within_address_space,
>                   section->offset_within_address_space +
>                   int128_get64(int128_sub(section->size, int128_one())));
> @@ -1183,7 +1183,7 @@ static void vfio_listener_region_del(MemoryListener 
> *listener,
>       bool try_unmap = true;
>   
>       if (vfio_listener_skipped_section(section)) {
> -        trace_vfio_listener_region_del_skip(
> +        trace_vfio_listener_region_skip("region_del",
>                   section->offset_within_address_space,
>                   section->offset_within_address_space +
>                   int128_get64(int128_sub(section->size, int128_one())));
> diff --git a/hw/vfio/trace-events b/hw/vfio/trace-events
> index 669d9fe07cd9..7173e6a5c721 100644
> --- a/hw/vfio/trace-events
> +++ b/hw/vfio/trace-events
> @@ -96,13 +96,12 @@ vfio_pci_igd_lpc_bridge_enabled(const char *name) "%s"
>   vfio_region_write(const char *name, int index, uint64_t addr, uint64_t data, unsigned size) " 
> (%s:region%d+0x%"PRIx64", 0x%"PRIx64 ", %d)"
>   vfio_region_read(char *name, int index, uint64_t addr, unsigned size, uint64_t data) " 
> (%s:region%d+0x%"PRIx64", %d) = 0x%"PRIx64
>   vfio_iommu_map_notify(const char *op, uint64_t iova_start, uint64_t iova_end) "iommu %s @ 
> 0x%"PRIx64" - 0x%"PRIx64
> -vfio_listener_region_add_skip(uint64_t start, uint64_t end) "SKIPPING region_add 
> 0x%"PRIx64" - 0x%"PRIx64
> +vfio_listener_region_skip(const char *name, uint64_t start, uint64_t end) "SKIPPING %s 
> 0x%"PRIx64" - 0x%"PRIx64
>   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_known_safe_misalignment(const char *name, uint64_t iova, uint64_t offset_within_region, uintptr_t page_size) 
> "Region \"%s\" iova=0x%"PRIx64" offset_within_region=0x%"PRIx64" 
> qemu_real_host_page_size=0x%"PRIxPTR
>   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
>   vfio_disconnect_container(int fd) "close container->fd=%d"
>   vfio_put_group(int fd) "close group->fd=%d"