[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v2] vfio: Fix vfio_get_dev_region() trace event
From: |
Alex Williamson |
Subject: |
Re: [PATCH v2] vfio: Fix vfio_get_dev_region() trace event |
Date: |
Fri, 3 Mar 2023 08:42:55 -0700 |
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