[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH] pci: Display PCI IRQ pin in "info pci"
From: |
Peter Xu |
Subject: |
Re: [PATCH] pci: Display PCI IRQ pin in "info pci" |
Date: |
Mon, 25 May 2020 10:14:43 -0400 |
Hi, Markus,
Thanks for commenting. Please see below.
On Mon, May 25, 2020 at 10:19:09AM +0200, Markus Armbruster wrote:
> Peter Xu <address@hidden> writes:
>
> > 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.
> > ---
> [...]
> > diff --git a/qapi/misc.json b/qapi/misc.json
> > index c18fe681fb..f8d33ddb4e 100644
> > --- a/qapi/misc.json
> > +++ b/qapi/misc.json
> > @@ -403,6 +403,8 @@
> > #
> > # @irq: if an IRQ is assigned to the device, the IRQ number
> > #
> > +# @irq_pin: the IRQ pin, zero means no IRQ (since 5.1)
> > +#
>
> For the IRQ number, we make the member optional, and use "member absent"
> for "no IRQ assigned".
>
> For the IRQ pin, we make the member mandatory, and use zero value for
> "no IRQ assigned".
>
> Any particular reason for the difference?
I have two reasons.
Spec-wise, "irq" (PCI_INTERRUPT_LINE) is optional which should depend on
"irq_pin" (PCI_INTERRUPT_PIN), while "irq_pin" itself is not optional according
to PCI local bus spec 3.0:
Interrupt Pin
The Interrupt Pin register tells which interrupt pin the device (or
device function) uses. A value of 1 corresponds to INTA# . A value of 2
corresponds to INTB# . A value of 3 corresponds to INTC# . A value of 4
corresponds to INTD# . Devices (or device functions) that do not use an
interrupt pin must put a 0 in this register. The values 05h through FFh
are reserved. This register is read-only. Refer to Section 2.2.6 for
further description of the usage of the INTx# pins.
So it should be a value between 0-4, inclusive. It applies even if the device
does not support INTx.
Code-wise, we can also make "irq_pin" optional just like "irq" (which will
automatically create the has_irq_pin variable). However, then it means we'll
have two booleans for the same purpose for intx in PciDeviceInfo, which seems
to be an unnecessary duplication.
So I chose the simple to make it mandatory.
Thanks,
--
Peter Xu