hw/pci: Display correct size for unmapped BARs in HMP 'info pci'

[PATCH] hw/pci: Display correct size for unmapped BARs in HMP 'info pci'

Posted by Philippe Mathieu-Daudé 3 years, 1 month ago

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;
+        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));
         }
     }
 
-- 
2.38.1

Re: [PATCH] hw/pci: Display correct size for unmapped BARs in HMP 'info pci'

Posted by BALATON Zoltan 3 years, 1 month ago

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));
>         }
>     }
>
>

Re: [PATCH] hw/pci: Display correct size for unmapped BARs in HMP 'info pci'

Posted by Philippe Mathieu-Daudé 3 years, 1 month ago

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!

Re: [PATCH] hw/pci: Display correct size for unmapped BARs in HMP 'info pci'

Posted by Philippe Mathieu-Daudé 3 years, 1 month ago

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!

Re: [PATCH] hw/pci: Display correct size for unmapped BARs in HMP 'info pci'

Posted by Philippe Mathieu-Daudé 3 years, 1 month ago

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]