[Qemu-devel] [trivial PATCH] hw/pci: fix pci_update_mappings() trace events

[Qemu-devel] [trivial PATCH] hw/pci: fix pci_update_mappings() trace events

[Laszlo Ersek]

The current trace prototypes and (matching) trace calls lead to
"unorthodox" PCI BDF notation in at least the stderr trace backend. For
example, the four BARs of a QXL video card at 00:01.0 (bus 0, slot 1,
function 0) are traced like this (PID and timestamps removed):

  pci_update_mappings_add d=0x7f14a73bf890 00:00.1 0,0x84000000+0x4000000
  pci_update_mappings_add d=0x7f14a73bf890 00:00.1 1,0x80000000+0x4000000
  pci_update_mappings_add d=0x7f14a73bf890 00:00.1 2,0x88200000+0x2000
  pci_update_mappings_add d=0x7f14a73bf890 00:00.1 3,0xd060+0x20

The slot and function values are in reverse order.

Stick with the conventional BDF notation.

Cc: "Michael S. Tsirkin" <mst@redhat.com>
Cc: Don Koch <dkoch@verizon.com>
Cc: qemu-trivial@nongnu.org
Fixes: 7828d75045
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
---
 hw/pci/pci.c | 4 ++--
 trace-events | 4 ++--
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/hw/pci/pci.c b/hw/pci/pci.c
index 9f57aea..47f42ea 100644
--- a/hw/pci/pci.c
+++ b/hw/pci/pci.c
@@ -1154,16 +1154,16 @@ static void pci_update_mappings(PCIDevice *d)
         /* now do the real mapping */
         if (r->addr != PCI_BAR_UNMAPPED) {
             trace_pci_update_mappings_del(d, pci_bus_num(d->bus),
-                                          PCI_FUNC(d->devfn),
                                           PCI_SLOT(d->devfn),
+                                          PCI_FUNC(d->devfn),
                                           i, r->addr, r->size);
             memory_region_del_subregion(r->address_space, r->memory);
         }
         r->addr = new_addr;
         if (r->addr != PCI_BAR_UNMAPPED) {
             trace_pci_update_mappings_add(d, pci_bus_num(d->bus),
-                                          PCI_FUNC(d->devfn),
                                           PCI_SLOT(d->devfn),
+                                          PCI_FUNC(d->devfn),
                                           i, r->addr, r->size);
             memory_region_add_subregion_overlap(r->address_space,
                                                 r->addr, r->memory, 1);
diff --git a/trace-events b/trace-events
index 8f9614a..5e78f18 100644
--- a/trace-events
+++ b/trace-events
@@ -1303,8 +1303,8 @@ spapr_pci_lsi_set(const char *busname, int pin, uint32_t irq) "%s PIN%d IRQ %u"
 spapr_pci_msi_retry(unsigned config_addr, unsigned req_num, unsigned max_irqs) "Guest device at %x asked %u, have only %u"
 
 # hw/pci/pci.c
-pci_update_mappings_del(void *d, uint32_t bus, uint32_t func, uint32_t slot, int bar, uint64_t addr, uint64_t size) "d=%p %02x:%02x.%x %d,%#"PRIx64"+%#"PRIx64
-pci_update_mappings_add(void *d, uint32_t bus, uint32_t func, uint32_t slot, int bar, uint64_t addr, uint64_t size) "d=%p %02x:%02x.%x %d,%#"PRIx64"+%#"PRIx64
+pci_update_mappings_del(void *d, uint32_t bus, uint32_t slot, uint32_t func, int bar, uint64_t addr, uint64_t size) "d=%p %02x:%02x.%x %d,%#"PRIx64"+%#"PRIx64
+pci_update_mappings_add(void *d, uint32_t bus, uint32_t slot, uint32_t func, int bar, uint64_t addr, uint64_t size) "d=%p %02x:%02x.%x %d,%#"PRIx64"+%#"PRIx64
 
 # hw/net/pcnet.c
 pcnet_s_reset(void *s) "s=%p"
-- 
1.8.3.1

Re: [Qemu-devel] [Qemu-trivial] [trivial PATCH] hw/pci: fix pci_update_mappings() trace events

[Marcel Apfelbaum]

On 09/02/2015 12:33 AM, Laszlo Ersek wrote:
> The current trace prototypes and (matching) trace calls lead to
> "unorthodox" PCI BDF notation in at least the stderr trace backend. For
> example, the four BARs of a QXL video card at 00:01.0 (bus 0, slot 1,
> function 0) are traced like this (PID and timestamps removed):
>
>    pci_update_mappings_add d=0x7f14a73bf890 00:00.1 0,0x84000000+0x4000000
>    pci_update_mappings_add d=0x7f14a73bf890 00:00.1 1,0x80000000+0x4000000
>    pci_update_mappings_add d=0x7f14a73bf890 00:00.1 2,0x88200000+0x2000
>    pci_update_mappings_add d=0x7f14a73bf890 00:00.1 3,0xd060+0x20
>
> The slot and function values are in reverse order.
>
> Stick with the conventional BDF notation.
>
> Cc: "Michael S. Tsirkin" <mst@redhat.com>
> Cc: Don Koch <dkoch@verizon.com>
> Cc: qemu-trivial@nongnu.org
> Fixes: 7828d75045
> Signed-off-by: Laszlo Ersek <lersek@redhat.com>
> ---
>   hw/pci/pci.c | 4 ++--
>   trace-events | 4 ++--
>   2 files changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/hw/pci/pci.c b/hw/pci/pci.c
> index 9f57aea..47f42ea 100644
> --- a/hw/pci/pci.c
> +++ b/hw/pci/pci.c
> @@ -1154,16 +1154,16 @@ static void pci_update_mappings(PCIDevice *d)
>           /* now do the real mapping */
>           if (r->addr != PCI_BAR_UNMAPPED) {
>               trace_pci_update_mappings_del(d, pci_bus_num(d->bus),
> -                                          PCI_FUNC(d->devfn),
>                                             PCI_SLOT(d->devfn),
> +                                          PCI_FUNC(d->devfn),
>                                             i, r->addr, r->size);
>               memory_region_del_subregion(r->address_space, r->memory);
>           }
>           r->addr = new_addr;
>           if (r->addr != PCI_BAR_UNMAPPED) {
>               trace_pci_update_mappings_add(d, pci_bus_num(d->bus),
> -                                          PCI_FUNC(d->devfn),
>                                             PCI_SLOT(d->devfn),
> +                                          PCI_FUNC(d->devfn),
>                                             i, r->addr, r->size);
>               memory_region_add_subregion_overlap(r->address_space,
>                                                   r->addr, r->memory, 1);
> diff --git a/trace-events b/trace-events
> index 8f9614a..5e78f18 100644
> --- a/trace-events
> +++ b/trace-events
> @@ -1303,8 +1303,8 @@ spapr_pci_lsi_set(const char *busname, int pin, uint32_t irq) "%s PIN%d IRQ %u"
>   spapr_pci_msi_retry(unsigned config_addr, unsigned req_num, unsigned max_irqs) "Guest device at %x asked %u, have only %u"
>
>   # hw/pci/pci.c
> -pci_update_mappings_del(void *d, uint32_t bus, uint32_t func, uint32_t slot, int bar, uint64_t addr, uint64_t size) "d=%p %02x:%02x.%x %d,%#"PRIx64"+%#"PRIx64
> -pci_update_mappings_add(void *d, uint32_t bus, uint32_t func, uint32_t slot, int bar, uint64_t addr, uint64_t size) "d=%p %02x:%02x.%x %d,%#"PRIx64"+%#"PRIx64
> +pci_update_mappings_del(void *d, uint32_t bus, uint32_t slot, uint32_t func, int bar, uint64_t addr, uint64_t size) "d=%p %02x:%02x.%x %d,%#"PRIx64"+%#"PRIx64
> +pci_update_mappings_add(void *d, uint32_t bus, uint32_t slot, uint32_t func, int bar, uint64_t addr, uint64_t size) "d=%p %02x:%02x.%x %d,%#"PRIx64"+%#"PRIx64
>
>   # hw/net/pcnet.c
>   pcnet_s_reset(void *s) "s=%p"
>


Reviewed-by: Marcel Apfelbaum <marcel@redhat.com>

Thanks,
Marcel

Re: [Qemu-devel] [Qemu-trivial] [trivial PATCH] hw/pci: fix pci_update_mappings() trace events

[Michael Tokarev]

02.09.2015 00:33, Laszlo Ersek wrote:
> Stick with the conventional BDF notation.

Applied to -trivial, thanks!

/mjt