Re: [Qemu-devel] [PATCH RFC v2 0/9] hw/pci: set irq without selecting INTx pin

[Alex Williamson]

On Wed, 2013-10-02 at 16:05 +0300, Marcel Apfelbaum wrote:
> On Wed, 2013-10-02 at 15:58 +0300, Michael S. Tsirkin wrote:
> > On Wed, Oct 02, 2013 at 03:41:25PM +0300, Marcel Apfelbaum wrote:
> > > Note: Added RFC because not all affected devices were
> > >       checked yet. 
> > 
> > What do you have in mind exactly?
> Sanity test for *all* modified devices.
> Any other thoughts?
> 
> From self code review, I can say that besides 2 devices
> (vmxnet3 and vfio), the code is re-factored with
> no side effects.
> 
> vmxnet3: I reviewed linux driver code and when using legacy interrupts
>          only INTx 0 is used.
> vfio: Also here only INTx 0 is used, so the interface selecting
>       the interrupt line seems to be unnecessary at least for
>       legacy interrupts.

vfio uses the same pin as the physical device, so you can't guarantee
that's always INTA.  Thanks,

Alex

> > 
> > > Interrupt pin is selected and saved into PCI_INTERRUPT_PIN
> > > register during device initialization. Devices should not call
> > > directly qemu_set_irq and specify the INTx pin.
> > > 
> > > Added pci_* wrappers to replace qemu_set_irq, qemu_irq_raise,
> > > qemu_irq_lower and qemu_irq_pulse, setting the irq
> > > based on PCI_INTERRUPT_PIN.
> > > 
> > > Added interface to allocate and free single irq.
> > > Added pci_allocate_irq wrapper to be used by devices that
> > > still need PCIDevice infrastructure to assert irqs.
> > > 
> > > Removed irq field from PCIDevice, not needed anymore.
> > > 
> > > Special cases of replacements were done in separate patches,
> > > all others in one patch "hw: set interrupts using pci irq wrappers"
> > 
> > Looks good to me overall.
> > Acked-by: Michael S. Tsirkin <address@hidden>
> > 
> > 
> > > Changes from v1:
> > >  - Addressed Michael S. Tsirkin's comments:
> > >    - pci_set_irq directly calls pci_irq handler
> > >    - removed irq field from PCIDevice
> > >  - Added qemu interface to allocate single irq
> > >  - Added pci wrappers to allocate and free pci irq
> > >  - Added pci irq wrappers for all qemu methods
> > >    setting irq and not only qemu_set_irq 
> > >  - Replace all qemu irq setters with pci
> > >    wrappers
> > > 
> > > Marcel Apfelbaum (9):
> > >   hw/core: Add interface to allocate and free a single IRQ
> > >   hw/pci: add pci wrappers for allocating and asserting irqs
> > >   hw/pci-bridge: set PCI_INTERRUPT_PIN register before shpc init
> > >   hw/vmxnet3: set interrupts using pci irq wrappers
> > >   hw/vfio: set interrupts using pci irq wrappers
> > >   hw/xhci: set interrupts using pci irq wrappers
> > >   hw: set interrupts using pci irq wrappers
> > >   hw/pcie: AER and hot-plug events must use device's interrupt
> > >   hw/pci: removed irq field from PCIDevice
> > > 
> > >  hw/audio/ac97.c                |  4 ++--
> > >  hw/audio/es1370.c              |  4 ++--
> > >  hw/audio/intel-hda.c           |  2 +-
> > >  hw/block/nvme.c                |  2 +-
> > >  hw/char/serial-pci.c           |  5 +++--
> > >  hw/char/tpci200.c              |  8 ++++----
> > >  hw/core/irq.c                  | 16 ++++++++++++++++
> > >  hw/display/qxl.c               |  2 +-
> > >  hw/ide/cmd646.c                |  2 +-
> > >  hw/ide/ich.c                   |  3 ++-
> > >  hw/isa/vt82c686.c              |  2 +-
> > >  hw/misc/ivshmem.c              |  2 +-
> > >  hw/misc/vfio.c                 | 11 ++++++-----
> > >  hw/net/e1000.c                 |  2 +-
> > >  hw/net/eepro100.c              |  4 ++--
> > >  hw/net/ne2000.c                |  3 ++-
> > >  hw/net/pcnet-pci.c             |  3 ++-
> > >  hw/net/rtl8139.c               |  2 +-
> > >  hw/net/vmxnet3.c               | 13 +++++++++++--
> > >  hw/pci-bridge/pci_bridge_dev.c |  2 +-
> > >  hw/pci/pci.c                   | 26 +++++++++++++++++++++-----
> > >  hw/pci/pcie.c                  |  4 ++--
> > >  hw/pci/pcie_aer.c              |  4 ++--
> > >  hw/pci/shpc.c                  |  2 +-
> > >  hw/scsi/esp-pci.c              |  3 ++-
> > >  hw/scsi/lsi53c895a.c           |  2 +-
> > >  hw/scsi/megasas.c              |  6 +++---
> > >  hw/scsi/vmw_pvscsi.c           |  2 +-
> > >  hw/usb/hcd-ehci-pci.c          |  2 +-
> > >  hw/usb/hcd-ohci.c              |  2 +-
> > >  hw/usb/hcd-uhci.c              |  2 +-
> > >  hw/usb/hcd-xhci.c              |  7 ++-----
> > >  hw/virtio/virtio-pci.c         |  4 ++--
> > >  include/hw/irq.h               |  7 +++++++
> > >  include/hw/pci/pci.h           | 22 +++++++++++++++++++---
> > >  include/hw/pci/pcie.h          | 18 ------------------
> > >  36 files changed, 127 insertions(+), 78 deletions(-)
> > > 
> > > -- 
> > > 1.8.3.1
> 
> 
>