| Message ID | 20230103164825.95329-1-philmd@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | hw/pci: Display correct size for unmapped BARs in HMP 'info pci' | expand |
On Tue, 3 Jan 2023, Philippe Mathieu-Daudé wrote: > When a BAR is not mapped, the displayed size is shifted by 1 byte: > > (qemu) info pci > ... > Bus 0, device 11, function 0: > Ethernet controller: PCI device 1022:2000 > PCI subsystem 0000:0000 > IRQ 10, pin A > BAR0: I/O at 0xffffffffffffffff [0x001e]. > BAR1: 32 bit memory at 0xffffffffffffffff [0x0000001e]. <=== > BAR6: 32 bit memory at 0xffffffffffffffff [0x0003fffe]. <=== > id "" > Bus 0, device 18, function 0: > VGA controller: PCI device 1013:00b8 > PCI subsystem 1af4:1100 > BAR0: 32 bit prefetchable memory at 0x10000000 [0x11ffffff]. > BAR1: 32 bit memory at 0x12050000 [0x12050fff]. > BAR6: 32 bit memory at 0xffffffffffffffff [0x0000fffe]. <=== > id "" > > Only substract this byte when the BAR is mapped to display > the correct size: > > (qemu) info pci > ... > Bus 0, device 11, function 0: > Ethernet controller: PCI device 1022:2000 > PCI subsystem 0000:0000 > IRQ 10, pin A > BAR0: I/O at 0xffffffffffffffff [0x001f]. > BAR1: 32 bit memory at 0xffffffffffffffff [0x0000001f]. <=== > BAR6: 32 bit memory at 0xffffffffffffffff [0x0003ffff]. <=== > id "" > Bus 0, device 18, function 0: > VGA controller: PCI device 1013:00b8 > PCI subsystem 1af4:1100 > BAR0: 32 bit prefetchable memory at 0x10000000 [0x11ffffff]. > BAR1: 32 bit memory at 0x12050000 [0x12050fff]. > BAR6: 32 bit memory at 0xffffffffffffffff [0x0000ffff]. <=== > id "" > > Fixes: 0ac32c8375 ("PCI interrupt support - 'info pci' monitor command") > Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org> > --- > hw/pci/pci-hmp-cmds.c | 6 ++++-- > 1 file changed, 4 insertions(+), 2 deletions(-) > > diff --git a/hw/pci/pci-hmp-cmds.c b/hw/pci/pci-hmp-cmds.c > index fb7591d6ab..8cfa5f9cd1 100644 > --- a/hw/pci/pci-hmp-cmds.c > +++ b/hw/pci/pci-hmp-cmds.c > @@ -75,22 +75,24 @@ static void hmp_info_pci_device(Monitor *mon, const PciDeviceInfo *dev) > > for (region = dev->regions; region; region = region->next) { > uint64_t addr, size; > + bool mapped; > > addr = region->value->address; > size = region->value->size; As size is not used for anything else, you could adjust the value here once either as size = region->value->size - (addr != -1 ? 1 : 0); or in an if then you don't need the bool and tweak the value when printing. This looks simpler to me. Regards, BALATON Zoltan > + mapped = addr != -1; > > monitor_printf(mon, " BAR%" PRId64 ": ", region->value->bar); > > if (!strcmp(region->value->type, "io")) { > monitor_printf(mon, "I/O at 0x%04" PRIx64 > " [0x%04" PRIx64 "].\n", > - addr, addr + size - 1); > + addr, addr + size + (mapped ? -1 : 0)); > } else { > monitor_printf(mon, "%d bit%s memory at 0x%08" PRIx64 > " [0x%08" PRIx64 "].\n", > region->value->mem_type_64 ? 64 : 32, > region->value->prefetch ? " prefetchable" : "", > - addr, addr + size - 1); > + addr, addr + size + (mapped ? -1 : 0)); > } > } > >
On 3/1/23 18:39, BALATON Zoltan wrote: > On Tue, 3 Jan 2023, Philippe Mathieu-Daudé wrote: >> When a BAR is not mapped, the displayed size is shifted by 1 byte: >> >> (qemu) info pci >> ... >> Bus 0, device 11, function 0: >> Ethernet controller: PCI device 1022:2000 >> PCI subsystem 0000:0000 >> IRQ 10, pin A >> BAR0: I/O at 0xffffffffffffffff [0x001e]. >> BAR1: 32 bit memory at 0xffffffffffffffff [0x0000001e]. <=== >> BAR6: 32 bit memory at 0xffffffffffffffff [0x0003fffe]. <=== >> id "" >> Bus 0, device 18, function 0: >> VGA controller: PCI device 1013:00b8 >> PCI subsystem 1af4:1100 >> BAR0: 32 bit prefetchable memory at 0x10000000 [0x11ffffff]. >> BAR1: 32 bit memory at 0x12050000 [0x12050fff]. >> BAR6: 32 bit memory at 0xffffffffffffffff [0x0000fffe]. <=== >> id "" >> >> Only substract this byte when the BAR is mapped to display >> the correct size: >> >> (qemu) info pci >> ... >> Bus 0, device 11, function 0: >> Ethernet controller: PCI device 1022:2000 >> PCI subsystem 0000:0000 >> IRQ 10, pin A >> BAR0: I/O at 0xffffffffffffffff [0x001f]. >> BAR1: 32 bit memory at 0xffffffffffffffff [0x0000001f]. <=== >> BAR6: 32 bit memory at 0xffffffffffffffff [0x0003ffff]. <=== >> id "" >> Bus 0, device 18, function 0: >> VGA controller: PCI device 1013:00b8 >> PCI subsystem 1af4:1100 >> BAR0: 32 bit prefetchable memory at 0x10000000 [0x11ffffff]. >> BAR1: 32 bit memory at 0x12050000 [0x12050fff]. >> BAR6: 32 bit memory at 0xffffffffffffffff [0x0000ffff]. <=== >> id "" >> >> Fixes: 0ac32c8375 ("PCI interrupt support - 'info pci' monitor command") >> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org> >> --- >> hw/pci/pci-hmp-cmds.c | 6 ++++-- >> 1 file changed, 4 insertions(+), 2 deletions(-) >> >> diff --git a/hw/pci/pci-hmp-cmds.c b/hw/pci/pci-hmp-cmds.c >> index fb7591d6ab..8cfa5f9cd1 100644 >> --- a/hw/pci/pci-hmp-cmds.c >> +++ b/hw/pci/pci-hmp-cmds.c >> @@ -75,22 +75,24 @@ static void hmp_info_pci_device(Monitor *mon, >> const PciDeviceInfo *dev) >> >> for (region = dev->regions; region; region = region->next) { >> uint64_t addr, size; >> + bool mapped; >> >> addr = region->value->address; >> size = region->value->size; > > As size is not used for anything else, you could adjust the value here > once either as > > size = region->value->size - (addr != -1 ? 1 : 0); > > or in an if then you don't need the bool and tweak the value when > printing. This looks simpler to me. Good idea, thanks!
On 3/1/23 20:39, Philippe Mathieu-Daudé wrote: > On 3/1/23 18:39, BALATON Zoltan wrote: >> On Tue, 3 Jan 2023, Philippe Mathieu-Daudé wrote: >>> When a BAR is not mapped, the displayed size is shifted by 1 byte: >>> >>> (qemu) info pci >>> ... >>> Bus 0, device 11, function 0: >>> Ethernet controller: PCI device 1022:2000 >>> PCI subsystem 0000:0000 >>> IRQ 10, pin A >>> BAR0: I/O at 0xffffffffffffffff [0x001e]. >>> BAR1: 32 bit memory at 0xffffffffffffffff [0x0000001e]. <=== >>> BAR6: 32 bit memory at 0xffffffffffffffff [0x0003fffe]. <=== >>> id "" >>> Bus 0, device 18, function 0: >>> VGA controller: PCI device 1013:00b8 >>> PCI subsystem 1af4:1100 >>> BAR0: 32 bit prefetchable memory at 0x10000000 [0x11ffffff]. >>> BAR1: 32 bit memory at 0x12050000 [0x12050fff]. >>> BAR6: 32 bit memory at 0xffffffffffffffff [0x0000fffe]. <=== >>> id "" >>> >>> Only substract this byte when the BAR is mapped to display >>> the correct size: >>> >>> (qemu) info pci >>> ... >>> Bus 0, device 11, function 0: >>> Ethernet controller: PCI device 1022:2000 >>> PCI subsystem 0000:0000 >>> IRQ 10, pin A >>> BAR0: I/O at 0xffffffffffffffff [0x001f]. >>> BAR1: 32 bit memory at 0xffffffffffffffff [0x0000001f]. <=== >>> BAR6: 32 bit memory at 0xffffffffffffffff [0x0003ffff]. <=== >>> id "" >>> Bus 0, device 18, function 0: >>> VGA controller: PCI device 1013:00b8 >>> PCI subsystem 1af4:1100 >>> BAR0: 32 bit prefetchable memory at 0x10000000 [0x11ffffff]. Hmm actually here 0x11ffffff isn't the size but the higher address, >>> BAR1: 32 bit memory at 0x12050000 [0x12050fff]. >>> BAR6: 32 bit memory at 0xffffffffffffffff [0x0000ffff]. <=== while here this is the size. Confusing. Wouldn't it be simpler to only display the size? >>> id "" >>> >>> Fixes: 0ac32c8375 ("PCI interrupt support - 'info pci' monitor command") >>> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org> >>> --- >>> hw/pci/pci-hmp-cmds.c | 6 ++++-- >>> 1 file changed, 4 insertions(+), 2 deletions(-) >>> >>> diff --git a/hw/pci/pci-hmp-cmds.c b/hw/pci/pci-hmp-cmds.c >>> index fb7591d6ab..8cfa5f9cd1 100644 >>> --- a/hw/pci/pci-hmp-cmds.c >>> +++ b/hw/pci/pci-hmp-cmds.c >>> @@ -75,22 +75,24 @@ static void hmp_info_pci_device(Monitor *mon, >>> const PciDeviceInfo *dev) >>> >>> for (region = dev->regions; region; region = region->next) { >>> uint64_t addr, size; >>> + bool mapped; >>> >>> addr = region->value->address; >>> size = region->value->size; >> >> As size is not used for anything else, you could adjust the value here >> once either as >> >> size = region->value->size - (addr != -1 ? 1 : 0); >> >> or in an if then you don't need the bool and tweak the value when >> printing. This looks simpler to me. > > Good idea, thanks!
On 3/1/23 20:45, Philippe Mathieu-Daudé wrote: > On 3/1/23 20:39, Philippe Mathieu-Daudé wrote: >> On 3/1/23 18:39, BALATON Zoltan wrote: >>> On Tue, 3 Jan 2023, Philippe Mathieu-Daudé wrote: >>>> When a BAR is not mapped, the displayed size is shifted by 1 byte: >>>> >>>> (qemu) info pci >>>> ... >>>> Bus 0, device 11, function 0: >>>> Ethernet controller: PCI device 1022:2000 >>>> PCI subsystem 0000:0000 >>>> IRQ 10, pin A >>>> BAR0: I/O at 0xffffffffffffffff [0x001e]. >>>> BAR1: 32 bit memory at 0xffffffffffffffff [0x0000001e]. <=== >>>> BAR6: 32 bit memory at 0xffffffffffffffff [0x0003fffe]. <=== >>>> id "" >>>> Bus 0, device 18, function 0: >>>> VGA controller: PCI device 1013:00b8 >>>> PCI subsystem 1af4:1100 >>>> BAR0: 32 bit prefetchable memory at 0x10000000 [0x11ffffff]. >>>> BAR1: 32 bit memory at 0x12050000 [0x12050fff]. >>>> BAR6: 32 bit memory at 0xffffffffffffffff [0x0000fffe]. <=== >>>> id "" >>>> >>>> Only substract this byte when the BAR is mapped to display >>>> the correct size: >>>> >>>> (qemu) info pci >>>> ... >>>> Bus 0, device 11, function 0: >>>> Ethernet controller: PCI device 1022:2000 >>>> PCI subsystem 0000:0000 >>>> IRQ 10, pin A >>>> BAR0: I/O at 0xffffffffffffffff [0x001f]. >>>> BAR1: 32 bit memory at 0xffffffffffffffff [0x0000001f]. <=== >>>> BAR6: 32 bit memory at 0xffffffffffffffff [0x0003ffff]. <=== >>>> id "" >>>> Bus 0, device 18, function 0: >>>> VGA controller: PCI device 1013:00b8 >>>> PCI subsystem 1af4:1100 >>>> BAR0: 32 bit prefetchable memory at 0x10000000 [0x11ffffff]. > > Hmm actually here 0x11ffffff isn't the size but the higher address, > >>>> BAR1: 32 bit memory at 0x12050000 [0x12050fff]. >>>> BAR6: 32 bit memory at 0xffffffffffffffff [0x0000ffff]. <=== > > while here this is the size. Confusing. Wouldn't it be simpler to only > display the size? Or range and size: Bus 0, device 18, function 0: VGA controller: PCI device 1013:00b8 PCI subsystem 1af4:1100 BAR0: 32 bit prefetchable memory at 0x10000000-0x12000000 [0x02000000] BAR1: 32 bit memory at 0x12050000-0x12051000 [0x00001000] BAR6: 32 bit memory at 0xffffffffffffffff-0xffffffffffffffff [0x00010000]
diff --git a/hw/pci/pci-hmp-cmds.c b/hw/pci/pci-hmp-cmds.c index fb7591d6ab..8cfa5f9cd1 100644 --- a/hw/pci/pci-hmp-cmds.c +++ b/hw/pci/pci-hmp-cmds.c @@ -75,22 +75,24 @@ static void hmp_info_pci_device(Monitor *mon, const PciDeviceInfo *dev) for (region = dev->regions; region; region = region->next) { uint64_t addr, size; + bool mapped; addr = region->value->address; size = region->value->size; + mapped = addr != -1; monitor_printf(mon, " BAR%" PRId64 ": ", region->value->bar); if (!strcmp(region->value->type, "io")) { monitor_printf(mon, "I/O at 0x%04" PRIx64 " [0x%04" PRIx64 "].\n", - addr, addr + size - 1); + addr, addr + size + (mapped ? -1 : 0)); } else { monitor_printf(mon, "%d bit%s memory at 0x%08" PRIx64 " [0x%08" PRIx64 "].\n", region->value->mem_type_64 ? 64 : 32, region->value->prefetch ? " prefetchable" : "", - addr, addr + size - 1); + addr, addr + size + (mapped ? -1 : 0)); } }
When a BAR is not mapped, the displayed size is shifted by 1 byte: (qemu) info pci ... Bus 0, device 11, function 0: Ethernet controller: PCI device 1022:2000 PCI subsystem 0000:0000 IRQ 10, pin A BAR0: I/O at 0xffffffffffffffff [0x001e]. BAR1: 32 bit memory at 0xffffffffffffffff [0x0000001e]. <=== BAR6: 32 bit memory at 0xffffffffffffffff [0x0003fffe]. <=== id "" Bus 0, device 18, function 0: VGA controller: PCI device 1013:00b8 PCI subsystem 1af4:1100 BAR0: 32 bit prefetchable memory at 0x10000000 [0x11ffffff]. BAR1: 32 bit memory at 0x12050000 [0x12050fff]. BAR6: 32 bit memory at 0xffffffffffffffff [0x0000fffe]. <=== id "" Only substract this byte when the BAR is mapped to display the correct size: (qemu) info pci ... Bus 0, device 11, function 0: Ethernet controller: PCI device 1022:2000 PCI subsystem 0000:0000 IRQ 10, pin A BAR0: I/O at 0xffffffffffffffff [0x001f]. BAR1: 32 bit memory at 0xffffffffffffffff [0x0000001f]. <=== BAR6: 32 bit memory at 0xffffffffffffffff [0x0003ffff]. <=== id "" Bus 0, device 18, function 0: VGA controller: PCI device 1013:00b8 PCI subsystem 1af4:1100 BAR0: 32 bit prefetchable memory at 0x10000000 [0x11ffffff]. BAR1: 32 bit memory at 0x12050000 [0x12050fff]. BAR6: 32 bit memory at 0xffffffffffffffff [0x0000ffff]. <=== id "" Fixes: 0ac32c8375 ("PCI interrupt support - 'info pci' monitor command") Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org> --- hw/pci/pci-hmp-cmds.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-)