hw/pci/pci-hmp-cmds.c | 26 ++++++++++++++++++-------- 1 file changed, 18 insertions(+), 8 deletions(-)
When BAR aren't configured, we get:
(qemu) info pci
Bus 0, device 0, function 0:
Host bridge: PCI device dead:beef
...
BAR4: 32 bit memory at 0xffffffffffffffff [0x00000ffe].
BAR5: I/O at 0xffffffffffffffff [0x0ffe].
Improve logging to not display bogus sizes:
BAR4: 32 bit memory (not configured)
BAR5: I/O (not configured)
Remove trailing dot which is not used in other commands format.
Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
hw/pci/pci-hmp-cmds.c | 26 ++++++++++++++++++--------
1 file changed, 18 insertions(+), 8 deletions(-)
diff --git a/hw/pci/pci-hmp-cmds.c b/hw/pci/pci-hmp-cmds.c
index b09fce9377..8421c3f74a 100644
--- a/hw/pci/pci-hmp-cmds.c
+++ b/hw/pci/pci-hmp-cmds.c
@@ -83,15 +83,25 @@ static void hmp_info_pci_device(Monitor *mon, const PciDeviceInfo *dev)
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);
+ if (addr != UINT64_MAX) {
+ monitor_printf(mon, "I/O at 0x%04" PRIx64
+ " [0x%04" PRIx64 "]\n",
+ addr, addr + size - 1);
+ } else {
+ monitor_printf(mon, "I/O (not configured)\n");
+ }
} 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);
+ if (addr != UINT64_MAX) {
+ 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);
+ } else {
+ monitor_printf(mon, "%d bit%s memory (not configured)\n",
+ region->value->mem_type_64 ? 64 : 32,
+ region->value->prefetch ? " prefetchable" : "");
+ }
}
}
--
2.45.2
On Thu, Aug 01, 2024 at 12:07:42PM +0200, Philippe Mathieu-Daudé wrote:
> When BAR aren't configured, we get:
>
> (qemu) info pci
> Bus 0, device 0, function 0:
> Host bridge: PCI device dead:beef
> ...
> BAR4: 32 bit memory at 0xffffffffffffffff [0x00000ffe].
> BAR5: I/O at 0xffffffffffffffff [0x0ffe].
>
> Improve logging to not display bogus sizes:
>
> BAR4: 32 bit memory (not configured)
> BAR5: I/O (not configured)
>
> Remove trailing dot which is not used in other commands format.
>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
> hw/pci/pci-hmp-cmds.c | 26 ++++++++++++++++++--------
> 1 file changed, 18 insertions(+), 8 deletions(-)
>
> diff --git a/hw/pci/pci-hmp-cmds.c b/hw/pci/pci-hmp-cmds.c
> index b09fce9377..8421c3f74a 100644
> --- a/hw/pci/pci-hmp-cmds.c
> +++ b/hw/pci/pci-hmp-cmds.c
> @@ -83,15 +83,25 @@ static void hmp_info_pci_device(Monitor *mon, const PciDeviceInfo *dev)
> 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);
> + if (addr != UINT64_MAX) {
> + monitor_printf(mon, "I/O at 0x%04" PRIx64
> + " [0x%04" PRIx64 "]\n",
> + addr, addr + size - 1);
> + } else {
> + monitor_printf(mon, "I/O (not configured)\n");
> + }
> } 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);
> + if (addr != UINT64_MAX) {
> + 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);
> + } else {
> + monitor_printf(mon, "%d bit%s memory (not configured)\n",
> + region->value->mem_type_64 ? 64 : 32,
> + region->value->prefetch ? " prefetchable" : "");
> + }
> }
> }
what makes bar unconfigured is that memory space is disabled,
not that it has a special value.
>
> --
> 2.45.2
On 1/8/24 12:27, Michael S. Tsirkin wrote:
> On Thu, Aug 01, 2024 at 12:07:42PM +0200, Philippe Mathieu-Daudé wrote:
>> When BAR aren't configured, we get:
>>
>> (qemu) info pci
>> Bus 0, device 0, function 0:
>> Host bridge: PCI device dead:beef
>> ...
>> BAR4: 32 bit memory at 0xffffffffffffffff [0x00000ffe].
>> BAR5: I/O at 0xffffffffffffffff [0x0ffe].
>>
>> Improve logging to not display bogus sizes:
>>
>> BAR4: 32 bit memory (not configured)
>> BAR5: I/O (not configured)
>>
>> Remove trailing dot which is not used in other commands format.
>>
>> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>> ---
>> hw/pci/pci-hmp-cmds.c | 26 ++++++++++++++++++--------
>> 1 file changed, 18 insertions(+), 8 deletions(-)
>>
>> diff --git a/hw/pci/pci-hmp-cmds.c b/hw/pci/pci-hmp-cmds.c
>> index b09fce9377..8421c3f74a 100644
>> --- a/hw/pci/pci-hmp-cmds.c
>> +++ b/hw/pci/pci-hmp-cmds.c
>> @@ -83,15 +83,25 @@ static void hmp_info_pci_device(Monitor *mon, const PciDeviceInfo *dev)
>> 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);
>> + if (addr != UINT64_MAX) {
>> + monitor_printf(mon, "I/O at 0x%04" PRIx64
>> + " [0x%04" PRIx64 "]\n",
>> + addr, addr + size - 1);
>> + } else {
>> + monitor_printf(mon, "I/O (not configured)\n");
>> + }
>> } 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);
>> + if (addr != UINT64_MAX) {
>> + 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);
>> + } else {
>> + monitor_printf(mon, "%d bit%s memory (not configured)\n",
>> + region->value->mem_type_64 ? 64 : 32,
>> + region->value->prefetch ? " prefetchable" : "");
>> + }
>> }
>> }
>
> what makes bar unconfigured is that memory space is disabled,
> not that it has a special value.
I tried to add a PciMemoryRegion::enabled field then realized
unmapped regions are advertised using addr = PCI_BAR_UNMAPPED
(which is UINT64_MAX):
typedef struct PCIIORegion {
pcibus_t addr; /* current PCI mapping address. -1 means not mapped */
#define PCI_BAR_UNMAPPED (~(pcibus_t)0)
OK if I respin this patch with s/UINT64_MAX/PCI_BAR_UNMAPPED/?
On Thu, Aug 01, 2024 at 02:36:11PM +0200, Philippe Mathieu-Daudé wrote:
> On 1/8/24 12:27, Michael S. Tsirkin wrote:
> > On Thu, Aug 01, 2024 at 12:07:42PM +0200, Philippe Mathieu-Daudé wrote:
> > > When BAR aren't configured, we get:
> > >
> > > (qemu) info pci
> > > Bus 0, device 0, function 0:
> > > Host bridge: PCI device dead:beef
> > > ...
> > > BAR4: 32 bit memory at 0xffffffffffffffff [0x00000ffe].
> > > BAR5: I/O at 0xffffffffffffffff [0x0ffe].
> > >
> > > Improve logging to not display bogus sizes:
> > >
> > > BAR4: 32 bit memory (not configured)
> > > BAR5: I/O (not configured)
> > >
> > > Remove trailing dot which is not used in other commands format.
> > >
> > > Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> > > ---
> > > hw/pci/pci-hmp-cmds.c | 26 ++++++++++++++++++--------
> > > 1 file changed, 18 insertions(+), 8 deletions(-)
> > >
> > > diff --git a/hw/pci/pci-hmp-cmds.c b/hw/pci/pci-hmp-cmds.c
> > > index b09fce9377..8421c3f74a 100644
> > > --- a/hw/pci/pci-hmp-cmds.c
> > > +++ b/hw/pci/pci-hmp-cmds.c
> > > @@ -83,15 +83,25 @@ static void hmp_info_pci_device(Monitor *mon, const PciDeviceInfo *dev)
> > > 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);
> > > + if (addr != UINT64_MAX) {
> > > + monitor_printf(mon, "I/O at 0x%04" PRIx64
> > > + " [0x%04" PRIx64 "]\n",
> > > + addr, addr + size - 1);
> > > + } else {
> > > + monitor_printf(mon, "I/O (not configured)\n");
> > > + }
> > > } 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);
> > > + if (addr != UINT64_MAX) {
> > > + 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);
> > > + } else {
> > > + monitor_printf(mon, "%d bit%s memory (not configured)\n",
> > > + region->value->mem_type_64 ? 64 : 32,
> > > + region->value->prefetch ? " prefetchable" : "");
> > > + }
> > > }
> > > }
> >
> > what makes bar unconfigured is that memory space is disabled,
> > not that it has a special value.
>
> I tried to add a PciMemoryRegion::enabled field then realized
> unmapped regions are advertised using addr = PCI_BAR_UNMAPPED
> (which is UINT64_MAX):
>
> typedef struct PCIIORegion {
> pcibus_t addr; /* current PCI mapping address. -1 means not mapped */
> #define PCI_BAR_UNMAPPED (~(pcibus_t)0)
>
> OK if I respin this patch with s/UINT64_MAX/PCI_BAR_UNMAPPED/?
ok
On 1/8/24 14:36, Philippe Mathieu-Daudé wrote:
> On 1/8/24 12:27, Michael S. Tsirkin wrote:
>> On Thu, Aug 01, 2024 at 12:07:42PM +0200, Philippe Mathieu-Daudé wrote:
>>> When BAR aren't configured, we get:
>>>
>>> (qemu) info pci
>>> Bus 0, device 0, function 0:
>>> Host bridge: PCI device dead:beef
>>> ...
>>> BAR4: 32 bit memory at 0xffffffffffffffff [0x00000ffe].
>>> BAR5: I/O at 0xffffffffffffffff [0x0ffe].
>>>
>>> Improve logging to not display bogus sizes:
>>>
>>> BAR4: 32 bit memory (not configured)
>>> BAR5: I/O (not configured)
>>>
>>> Remove trailing dot which is not used in other commands format.
>>>
>>> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>>> ---
>>> hw/pci/pci-hmp-cmds.c | 26 ++++++++++++++++++--------
>>> 1 file changed, 18 insertions(+), 8 deletions(-)
>>>
>>> diff --git a/hw/pci/pci-hmp-cmds.c b/hw/pci/pci-hmp-cmds.c
>>> index b09fce9377..8421c3f74a 100644
>>> --- a/hw/pci/pci-hmp-cmds.c
>>> +++ b/hw/pci/pci-hmp-cmds.c
>>> @@ -83,15 +83,25 @@ static void hmp_info_pci_device(Monitor *mon,
>>> const PciDeviceInfo *dev)
>>> 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);
>>> + if (addr != UINT64_MAX) {
>>> + monitor_printf(mon, "I/O at 0x%04" PRIx64
>>> + " [0x%04" PRIx64 "]\n",
>>> + addr, addr + size - 1);
>>> + } else {
>>> + monitor_printf(mon, "I/O (not configured)\n");
>>> + }
>>> } 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);
>>> + if (addr != UINT64_MAX) {
>>> + 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);
>>> + } else {
>>> + monitor_printf(mon, "%d bit%s memory (not
>>> configured)\n",
>>> + region->value->mem_type_64 ? 64 : 32,
>>> + region->value->prefetch ? "
>>> prefetchable" : "");
>>> + }
>>> }
>>> }
>>
>> what makes bar unconfigured is that memory space is disabled,
>> not that it has a special value.
>
> I tried to add a PciMemoryRegion::enabled field then realized
> unmapped regions are advertised using addr = PCI_BAR_UNMAPPED
> (which is UINT64_MAX):
>
> typedef struct PCIIORegion {
> pcibus_t addr; /* current PCI mapping address. -1 means not mapped */
> #define PCI_BAR_UNMAPPED (~(pcibus_t)0)
>
> OK if I respin this patch with s/UINT64_MAX/PCI_BAR_UNMAPPED/?
and s/configured/mapped/ in printf.
© 2016 - 2024 Red Hat, Inc.