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

["Michael S. Tsirkin" <mst@redhat.com>]

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