A whitelist for it does not really makes sense. Let's remove it and
then we can introduce a blacklist when really needed, with msi_broken.
That's patch 1.

Then, I let the msi_init() always success in patch 2, along with it I
removed caller checks around it.

The goal of this series is to fix the edu device leak. Yeah it's
slightly weird, but it's the truth...

Please review. Thanks.

Peter Xu (2):
  msi: remove msi_nonbroken
  msi: remove return code for msi_init()

 hw/audio/intel-hda.c               | 18 +----------------
 hw/i386/amd_iommu.c                |  2 +-
 hw/i386/kvm/apic.c                 |  4 ----
 hw/i386/xen/xen_apic.c             |  1 -
 hw/ide/ich.c                       |  6 +-----
 hw/intc/apic.c                     |  2 --
 hw/intc/arm_gicv2m.c               |  1 -
 hw/intc/arm_gicv3_its_common.c     |  2 --
 hw/intc/openpic.c                  |  1 -
 hw/intc/openpic_kvm.c              |  1 -
 hw/misc/edu.c                      |  4 +---
 hw/net/e1000e.c                    |  6 +-----
 hw/net/trace-events                |  1 -
 hw/net/vmxnet3.c                   |  8 ++------
 hw/pci-bridge/ioh3420.c            | 17 ++++------------
 hw/pci-bridge/pci_bridge_dev.c     | 19 +-----------------
 hw/pci-bridge/xio3130_downstream.c | 11 +++-------
 hw/pci-bridge/xio3130_upstream.c   | 11 +++-------
 hw/pci/msi.c                       | 41 +++++---------------------------------
 hw/pci/msix.c                      |  6 ------
 hw/ppc/spapr.c                     |  6 +-----
 hw/ppc/spapr_pci.c                 | 12 +++++------
 hw/s390x/s390-pci-bus.c            |  1 -
 hw/scsi/megasas.c                  | 18 +----------------
 hw/scsi/mptsas.c                   | 20 ++-----------------
 hw/scsi/trace-events               |  1 -
 hw/scsi/vmw_pvscsi.c               | 12 +++--------
 hw/usb/hcd-xhci.c                  | 16 +--------------
 hw/vfio/pci.c                      | 13 ++----------
 include/hw/pci/msi.h               |  8 +++-----
 30 files changed, 41 insertions(+), 228 deletions(-)

-- 
2.7.4

On Wed, May 31, 2017 at 05:42:34PM +0800, Peter Xu wrote:
> A whitelist for it does not really makes sense. Let's remove it and
> then we can introduce a blacklist when really needed, with msi_broken.
> That's patch 1.

Ok this paragraph does not make sense if not mentioning what's "it"...

Please just read the commit messages of patch 1. It should be much
better.

Sorry for such a bad cover letter.

> 
> Then, I let the msi_init() always success in patch 2, along with it I
> removed caller checks around it.
> 
> The goal of this series is to fix the edu device leak. Yeah it's
> slightly weird, but it's the truth...
> 
> Please review. Thanks.
> 
> Peter Xu (2):
>   msi: remove msi_nonbroken
>   msi: remove return code for msi_init()
> 
>  hw/audio/intel-hda.c               | 18 +----------------
>  hw/i386/amd_iommu.c                |  2 +-
>  hw/i386/kvm/apic.c                 |  4 ----
>  hw/i386/xen/xen_apic.c             |  1 -
>  hw/ide/ich.c                       |  6 +-----
>  hw/intc/apic.c                     |  2 --
>  hw/intc/arm_gicv2m.c               |  1 -
>  hw/intc/arm_gicv3_its_common.c     |  2 --
>  hw/intc/openpic.c                  |  1 -
>  hw/intc/openpic_kvm.c              |  1 -
>  hw/misc/edu.c                      |  4 +---
>  hw/net/e1000e.c                    |  6 +-----
>  hw/net/trace-events                |  1 -
>  hw/net/vmxnet3.c                   |  8 ++------
>  hw/pci-bridge/ioh3420.c            | 17 ++++------------
>  hw/pci-bridge/pci_bridge_dev.c     | 19 +-----------------
>  hw/pci-bridge/xio3130_downstream.c | 11 +++-------
>  hw/pci-bridge/xio3130_upstream.c   | 11 +++-------
>  hw/pci/msi.c                       | 41 +++++---------------------------------
>  hw/pci/msix.c                      |  6 ------
>  hw/ppc/spapr.c                     |  6 +-----
>  hw/ppc/spapr_pci.c                 | 12 +++++------
>  hw/s390x/s390-pci-bus.c            |  1 -
>  hw/scsi/megasas.c                  | 18 +----------------
>  hw/scsi/mptsas.c                   | 20 ++-----------------
>  hw/scsi/trace-events               |  1 -
>  hw/scsi/vmw_pvscsi.c               | 12 +++--------
>  hw/usb/hcd-xhci.c                  | 16 +--------------
>  hw/vfio/pci.c                      | 13 ++----------
>  include/hw/pci/msi.h               |  8 +++-----
>  30 files changed, 41 insertions(+), 228 deletions(-)
> 
> -- 
> 2.7.4
> 

-- 
Peter Xu

On 31/05/2017 11:50, Peter Xu wrote:
> On Wed, May 31, 2017 at 05:42:34PM +0800, Peter Xu wrote:
>> A whitelist for it does not really makes sense. Let's remove it and
>> then we can introduce a blacklist when really needed, with msi_broken.
>> That's patch 1.
> Ok this paragraph does not make sense if not mentioning what's "it"...
> 
> Please just read the commit messages of patch 1. It should be much
> better.

I think fixing the leak in case we have to reintroduce msi_(non)broken should
be as simple as

diff --git a/hw/misc/edu.c b/hw/misc/edu.c
index 401039c100..01acacf142 100644
--- a/hw/misc/edu.c
+++ b/hw/misc/edu.c
@@ -343,6 +343,12 @@ static void pci_edu_realize(PCIDevice *pdev, Error **errp)
     EduState *edu = DO_UPCAST(EduState, pdev, pdev);
     uint8_t *pci_conf = pdev->config;
 
+    pci_config_set_interrupt_pin(pci_conf, 1);
+
+    if (msi_init(pdev, 0, 1, true, false, errp)) {
+        return;
+    }
+
     timer_init_ms(&edu->dma_timer, QEMU_CLOCK_VIRTUAL, edu_dma_timer, edu);
 
     qemu_mutex_init(&edu->thr_mutex);
@@ -350,12 +356,6 @@ static void pci_edu_realize(PCIDevice *pdev, Error **errp)
     qemu_thread_create(&edu->thread, "edu", edu_fact_thread,
                        edu, QEMU_THREAD_JOINABLE);
 
-    pci_config_set_interrupt_pin(pci_conf, 1);
-
-    if (msi_init(pdev, 0, 1, true, false, errp)) {
-        return;
-    }
-
     memory_region_init_io(&edu->mmio, OBJECT(edu), &edu_mmio_ops, edu,
                     "edu-mmio", 1 << 20);
     pci_register_bar(pdev, 0, PCI_BASE_ADDRESS_SPACE_MEMORY, &edu->mmio);


Then the two patches can be even squashed together.

Paolo

On Wed, May 31, 2017 at 02:55:59PM +0200, Paolo Bonzini wrote:
> 
> 
> On 31/05/2017 11:50, Peter Xu wrote:
> > On Wed, May 31, 2017 at 05:42:34PM +0800, Peter Xu wrote:
> >> A whitelist for it does not really makes sense. Let's remove it and
> >> then we can introduce a blacklist when really needed, with msi_broken.
> >> That's patch 1.
> > Ok this paragraph does not make sense if not mentioning what's "it"...
> > 
> > Please just read the commit messages of patch 1. It should be much
> > better.
> 
> I think fixing the leak in case we have to reintroduce msi_(non)broken should
> be as simple as
> 
> diff --git a/hw/misc/edu.c b/hw/misc/edu.c
> index 401039c100..01acacf142 100644
> --- a/hw/misc/edu.c
> +++ b/hw/misc/edu.c
> @@ -343,6 +343,12 @@ static void pci_edu_realize(PCIDevice *pdev, Error **errp)
>      EduState *edu = DO_UPCAST(EduState, pdev, pdev);
>      uint8_t *pci_conf = pdev->config;
>  
> +    pci_config_set_interrupt_pin(pci_conf, 1);
> +
> +    if (msi_init(pdev, 0, 1, true, false, errp)) {
> +        return;
> +    }
> +
>      timer_init_ms(&edu->dma_timer, QEMU_CLOCK_VIRTUAL, edu_dma_timer, edu);
>  
>      qemu_mutex_init(&edu->thr_mutex);
> @@ -350,12 +356,6 @@ static void pci_edu_realize(PCIDevice *pdev, Error **errp)
>      qemu_thread_create(&edu->thread, "edu", edu_fact_thread,
>                         edu, QEMU_THREAD_JOINABLE);
>  
> -    pci_config_set_interrupt_pin(pci_conf, 1);
> -
> -    if (msi_init(pdev, 0, 1, true, false, errp)) {
> -        return;
> -    }
> -
>      memory_region_init_io(&edu->mmio, OBJECT(edu), &edu_mmio_ops, edu,
>                      "edu-mmio", 1 << 20);
>      pci_register_bar(pdev, 0, PCI_BASE_ADDRESS_SPACE_MEMORY, &edu->mmio);
> 
> 
> Then the two patches can be even squashed together.

Okay. So finally we still need this change... :)

Please anyone let me know whether this series needs a repost... And
also please feel free to do squashing of the two if that's not needed.

Thanks,

-- 
Peter Xu