Re: [PATCH v2] vfio: Fix vfio_get_dev_region() trace event

["Cédric Le Goater" <clg@kaod.org>]

On 3/3/23 16:42, Alex Williamson wrote:
> On Fri, 3 Mar 2023 10:34:47 +0100
> Philippe Mathieu-Daudé <philmd@linaro.org> wrote:
> 
>> On 3/3/23 08:43, Cédric Le Goater wrote:
>>> From: Cédric Le Goater <clg@redhat.com>
>>>
>>> Simply revert 'x8' to fix the typo and remove the ending '8'
> 
> s/revert/reverse/?  ie. transpose?  Nothing is being reverted afaict.

Transpose. Like Ctrl+T under Emacs.

>>> Fixes: e61a424f05 ("vfio: Create device specific region info helper")
>>> Resolves: https://gitlab.com/qemu-project/qemu/-/issues/1526
>>> Signed-off-by: Cédric Le Goater <clg@redhat.com>
>>> ---
>>>    hw/vfio/trace-events | 2 +-
>>>    1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/hw/vfio/trace-events b/hw/vfio/trace-events
>>> index 669d9fe07c..28e82541a2 100644
>>> --- a/hw/vfio/trace-events
>>> +++ b/hw/vfio/trace-events
>>> @@ -117,7 +117,7 @@ vfio_region_mmaps_set_enabled(const char *name, bool enabled) "Region %s mmaps e
>>>    vfio_region_unmap(const char *name, unsigned long offset, unsigned long end) "Region %s unmap [0x%lx - 0x%lx]"
>>>    vfio_region_sparse_mmap_header(const char *name, int index, int nr_areas) "Device %s region %d: %d sparse mmap entries"
>>>    vfio_region_sparse_mmap_entry(int i, unsigned long start, unsigned long end) "sparse entry %d [0x%lx - 0x%lx]"
>>> -vfio_get_dev_region(const char *name, int index, uint32_t type, uint32_t subtype) "%s index %d, %08x/%0x8"
>>> +vfio_get_dev_region(const char *name, int index, uint32_t type, uint32_t subtype) "%s index %d, %08x/%08x"
>>
>> 8 digits might be over-engineered, but at least is future-proof :)
>>
>> $ git grep -E 'VFIO_REGION_(SUB)?TYPE_' linux-headers/
>> linux-headers/linux/vfio.h:322:#define VFIO_REGION_TYPE_PCI_VENDOR_TYPE
>> (1 << 31)
> 
> This alone justifies printing the full zero-prefixed hex value.  I tend
> to prefer fixed width fields in trace logs, though it's less important
> here than when we're streaming register accesses.  Thanks,
> 
> Alex
>