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

[Peter Xu]

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...

Thanks,

-- 
Peter Xu