Re: [PATCH-for-9.1?] hw/pci/pci-hmp-cmds: Avoid displaying bogus size in 'info pci'

[Philippe Mathieu-Daudé]

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/?