* [PATCH v2] vfio: Fix vfio_get_dev_region() trace event
@ 2023-03-03 7:43 Cédric Le Goater
2023-03-03 9:34 ` Philippe Mathieu-Daudé
0 siblings, 1 reply; 6+ messages in thread
From: Cédric Le Goater @ 2023-03-03 7:43 UTC (permalink / raw)
To: qemu-devel; +Cc: Alex Williamson, Cédric Le Goater
From: Cédric Le Goater <clg@redhat.com>
Simply revert 'x8' to fix the typo and remove the ending '8'
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"
vfio_dma_unmap_overflow_workaround(void) ""
vfio_get_dirty_bitmap(int fd, uint64_t iova, uint64_t size, uint64_t bitmap_size, uint64_t start) "container fd=%d, iova=0x%"PRIx64" size= 0x%"PRIx64" bitmap_size=0x%"PRIx64" start=0x%"PRIx64
vfio_iommu_map_dirty_notify(uint64_t iova_start, uint64_t iova_end) "iommu dirty @ 0x%"PRIx64" - 0x%"PRIx64
--
2.39.2
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH v2] vfio: Fix vfio_get_dev_region() trace event
2023-03-03 7:43 [PATCH v2] vfio: Fix vfio_get_dev_region() trace event Cédric Le Goater
@ 2023-03-03 9:34 ` Philippe Mathieu-Daudé
2023-03-03 9:45 ` Cédric Le Goater
2023-03-03 15:42 ` Alex Williamson
0 siblings, 2 replies; 6+ messages in thread
From: Philippe Mathieu-Daudé @ 2023-03-03 9:34 UTC (permalink / raw)
To: Cédric Le Goater, qemu-devel; +Cc: Alex Williamson, Cédric Le Goater
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'
>
> 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)
linux-headers/linux/vfio.h:323:#define VFIO_REGION_TYPE_PCI_VENDOR_MASK
(0xffff)
linux-headers/linux/vfio.h:324:#define VFIO_REGION_TYPE_GFX
(1)
linux-headers/linux/vfio.h:325:#define VFIO_REGION_TYPE_CCW
(2)
linux-headers/linux/vfio.h:326:#define
VFIO_REGION_TYPE_MIGRATION_DEPRECATED (3)
linux-headers/linux/vfio.h:328:/* sub-types for VFIO_REGION_TYPE_PCI_* */
linux-headers/linux/vfio.h:331:#define
VFIO_REGION_SUBTYPE_INTEL_IGD_OPREGION (1)
linux-headers/linux/vfio.h:332:#define
VFIO_REGION_SUBTYPE_INTEL_IGD_HOST_CFG (2)
linux-headers/linux/vfio.h:333:#define
VFIO_REGION_SUBTYPE_INTEL_IGD_LPC_CFG (3)
linux-headers/linux/vfio.h:341:#define
VFIO_REGION_SUBTYPE_NVIDIA_NVLINK2_RAM (1)
linux-headers/linux/vfio.h:350:#define
VFIO_REGION_SUBTYPE_IBM_NVLINK2_ATSD (1)
linux-headers/linux/vfio.h:352:/* sub-types for VFIO_REGION_TYPE_GFX */
linux-headers/linux/vfio.h:353:#define VFIO_REGION_SUBTYPE_GFX_EDID
(1)
linux-headers/linux/vfio.h:402:/* sub-types for VFIO_REGION_TYPE_CCW */
linux-headers/linux/vfio.h:403:#define VFIO_REGION_SUBTYPE_CCW_ASYNC_CMD
(1)
linux-headers/linux/vfio.h:404:#define VFIO_REGION_SUBTYPE_CCW_SCHIB
(2)
linux-headers/linux/vfio.h:405:#define VFIO_REGION_SUBTYPE_CCW_CRW
(3)
linux-headers/linux/vfio.h:407:/* sub-types for
VFIO_REGION_TYPE_MIGRATION */
linux-headers/linux/vfio.h:408:#define
VFIO_REGION_SUBTYPE_MIGRATION_DEPRECATED (1)
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2] vfio: Fix vfio_get_dev_region() trace event
2023-03-03 9:34 ` Philippe Mathieu-Daudé
@ 2023-03-03 9:45 ` Cédric Le Goater
2023-03-03 10:31 ` Philippe Mathieu-Daudé
2023-03-03 15:42 ` Alex Williamson
1 sibling, 1 reply; 6+ messages in thread
From: Cédric Le Goater @ 2023-03-03 9:45 UTC (permalink / raw)
To: Philippe Mathieu-Daudé, Cédric Le Goater, qemu-devel
Cc: Alex Williamson
On 3/3/23 10:34, Philippe Mathieu-Daudé 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'
>>
>> 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 :)
well, yes, but it also depends on other possible outputs from the OS or,
more important, from gdb when dumping memory. It it better if they match.
Since it is an hex number, may be I should add a '0x' prefix also ? That's
a lot of versions for a 2 bytes patch ! :)
Thanks,
C.
>
> $ git grep -E 'VFIO_REGION_(SUB)?TYPE_' linux-headers/
> linux-headers/linux/vfio.h:322:#define VFIO_REGION_TYPE_PCI_VENDOR_TYPE (1 << 31)
> linux-headers/linux/vfio.h:323:#define VFIO_REGION_TYPE_PCI_VENDOR_MASK (0xffff)
> linux-headers/linux/vfio.h:324:#define VFIO_REGION_TYPE_GFX (1)
> linux-headers/linux/vfio.h:325:#define VFIO_REGION_TYPE_CCW (2)
> linux-headers/linux/vfio.h:326:#define VFIO_REGION_TYPE_MIGRATION_DEPRECATED (3)
> linux-headers/linux/vfio.h:328:/* sub-types for VFIO_REGION_TYPE_PCI_* */
> linux-headers/linux/vfio.h:331:#define VFIO_REGION_SUBTYPE_INTEL_IGD_OPREGION (1)
> linux-headers/linux/vfio.h:332:#define VFIO_REGION_SUBTYPE_INTEL_IGD_HOST_CFG (2)
> linux-headers/linux/vfio.h:333:#define VFIO_REGION_SUBTYPE_INTEL_IGD_LPC_CFG (3)
> linux-headers/linux/vfio.h:341:#define VFIO_REGION_SUBTYPE_NVIDIA_NVLINK2_RAM (1)
> linux-headers/linux/vfio.h:350:#define VFIO_REGION_SUBTYPE_IBM_NVLINK2_ATSD (1)
> linux-headers/linux/vfio.h:352:/* sub-types for VFIO_REGION_TYPE_GFX */
> linux-headers/linux/vfio.h:353:#define VFIO_REGION_SUBTYPE_GFX_EDID (1)
> linux-headers/linux/vfio.h:402:/* sub-types for VFIO_REGION_TYPE_CCW */
> linux-headers/linux/vfio.h:403:#define VFIO_REGION_SUBTYPE_CCW_ASYNC_CMD (1)
> linux-headers/linux/vfio.h:404:#define VFIO_REGION_SUBTYPE_CCW_SCHIB (2)
> linux-headers/linux/vfio.h:405:#define VFIO_REGION_SUBTYPE_CCW_CRW (3)
> linux-headers/linux/vfio.h:407:/* sub-types for VFIO_REGION_TYPE_MIGRATION */
> linux-headers/linux/vfio.h:408:#define VFIO_REGION_SUBTYPE_MIGRATION_DEPRECATED (1)
>
> Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2] vfio: Fix vfio_get_dev_region() trace event
2023-03-03 9:45 ` Cédric Le Goater
@ 2023-03-03 10:31 ` Philippe Mathieu-Daudé
0 siblings, 0 replies; 6+ messages in thread
From: Philippe Mathieu-Daudé @ 2023-03-03 10:31 UTC (permalink / raw)
To: Cédric Le Goater, Cédric Le Goater, qemu-devel; +Cc: Alex Williamson
On 3/3/23 10:45, Cédric Le Goater wrote:
> On 3/3/23 10:34, Philippe Mathieu-Daudé 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'
>>>
>>> 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 :)
>
> well, yes, but it also depends on other possible outputs from the OS or,
> more important, from gdb when dumping memory. It it better if they match.
>
> Since it is an hex number, may be I should add a '0x' prefix also ? That's
> a lot of versions for a 2 bytes patch ! :)
Per the coding style if you don't want to use the 0x prefix for
readability, add a "(in hex)" hint:
https://qemu-project.gitlab.io/qemu/devel/style.html#trace-events-style
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2] vfio: Fix vfio_get_dev_region() trace event
2023-03-03 9:34 ` Philippe Mathieu-Daudé
2023-03-03 9:45 ` Cédric Le Goater
@ 2023-03-03 15:42 ` Alex Williamson
2023-03-03 22:02 ` Cédric Le Goater
1 sibling, 1 reply; 6+ messages in thread
From: Alex Williamson @ 2023-03-03 15:42 UTC (permalink / raw)
To: Philippe Mathieu-Daudé
Cc: Cédric Le Goater, qemu-devel, Cédric Le Goater
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.
> >
> > 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
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2] vfio: Fix vfio_get_dev_region() trace event
2023-03-03 15:42 ` Alex Williamson
@ 2023-03-03 22:02 ` Cédric Le Goater
0 siblings, 0 replies; 6+ messages in thread
From: Cédric Le Goater @ 2023-03-03 22:02 UTC (permalink / raw)
To: Alex Williamson, Philippe Mathieu-Daudé
Cc: qemu-devel, Cédric Le Goater
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
>
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2023-03-03 22:02 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-03-03 7:43 [PATCH v2] vfio: Fix vfio_get_dev_region() trace event Cédric Le Goater
2023-03-03 9:34 ` Philippe Mathieu-Daudé
2023-03-03 9:45 ` Cédric Le Goater
2023-03-03 10:31 ` Philippe Mathieu-Daudé
2023-03-03 15:42 ` Alex Williamson
2023-03-03 22:02 ` Cédric Le Goater
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).