Re: [PATCH] pci: Display PCI IRQ pin in "info pci"

[Dr. David Alan Gilbert]

* Peter Xu (address@hidden) wrote:
> On Tue, Mar 17, 2020 at 08:21:17PM +0000, Dr. David Alan Gilbert wrote:
> > * Peter Xu (address@hidden) wrote:
> > > Sometimes it would be good to be able to read the pin number along
> > > with the IRQ number allocated.  Since we'll dump the IRQ number, no
> > > reason to not dump the pin information.  For example, the vfio-pci
> > > device will overwrite the pin with the hardware pin number.  It would
> > > be nice to know the pin number of one assigned device from QMP/HMP.
> > > 
> > > CC: Dr. David Alan Gilbert <address@hidden>
> > > CC: Alex Williamson <address@hidden>
> > > CC: Michael S. Tsirkin <address@hidden>
> > > CC: Marcel Apfelbaum <address@hidden>
> > > CC: Julia Suvorova <address@hidden>
> > > CC: Markus Armbruster <address@hidden>
> > > Signed-off-by: Peter Xu <address@hidden>
> > > ---
> > > 
> > > This helped me to debug an IRQ sharing issue, so may good to have it
> > > in master too.
> > > ---
> > >  hw/pci/pci.c       | 1 +
> > >  monitor/hmp-cmds.c | 3 ++-
> > >  qapi/misc.json     | 6 ++++--
> > >  3 files changed, 7 insertions(+), 3 deletions(-)
> > > 
> > > diff --git a/hw/pci/pci.c b/hw/pci/pci.c
> > > index e1ed6677e1..7ebf532ac9 100644
> > > --- a/hw/pci/pci.c
> > > +++ b/hw/pci/pci.c
> > > @@ -1769,6 +1769,7 @@ static PciDeviceInfo 
> > > *qmp_query_pci_device(PCIDevice *dev, PCIBus *bus,
> > >      info->regions = qmp_query_pci_regions(dev);
> > >      info->qdev_id = g_strdup(dev->qdev.id ? dev->qdev.id : "");
> > >  
> > > +    info->irq_pin = dev->config[PCI_INTERRUPT_PIN];
> > >      if (dev->config[PCI_INTERRUPT_PIN] != 0) {
> > >          info->has_irq = true;
> > >          info->irq = dev->config[PCI_INTERRUPT_LINE];
> > > diff --git a/monitor/hmp-cmds.c b/monitor/hmp-cmds.c
> > > index 58724031ea..04c86bbb03 100644
> > > --- a/monitor/hmp-cmds.c
> > > +++ b/monitor/hmp-cmds.c
> > > @@ -680,7 +680,8 @@ static void hmp_info_pci_device(Monitor *mon, const 
> > > PciDeviceInfo *dev)
> > >      }
> > >  
> > >      if (dev->has_irq) {
> > > -        monitor_printf(mon, "      IRQ %" PRId64 ".\n", dev->irq);
> > > +        monitor_printf(mon, "      IRQ %" PRId64 ", pin %c\n",
> > > +                       dev->irq, (char)('A' + dev->irq_pin - 1));
> > 
> > Can we trust dev->irq_pin not to be something silly and generate a
> > non-printable?
> 
> It should be strictly a value between 1-4 corresponds to irq pin A-D
> (note the has_irq check masked out the zero value, or it could be
> 0-4), so I think it should always make sense. Otherwise I see it a
> qemu bug somewhere...

OK, then 

Acked-by: Dr. David Alan Gilbert <address@hidden>

for HMP.

Dave


> Thanks,
> 
> -- 
> Peter Xu
> 
--
Dr. David Alan Gilbert / address@hidden / Manchester, UK