1. Patchew
  2. QEMU
  3. [Qemu-devel] [PATCH 0/3] spapr/pci: Improve MSI tracking
  4. View patch

[Qemu-devel] [PATCH 1/3] spapr/pci: Consolidate de-allocation of MSIs

Greg Kurz posted 3 patches 6 years, 6 months ago
Maintainers: David Gibson <david@gibson.dropbear.id.au>
[Qemu-devel] [PATCH 1/3] spapr/pci: Consolidate de-allocation of MSIs
Posted by Greg Kurz 6 years, 6 months ago 
When freeing MSIs, we need to:
- remove them from the machine's MSI bitmap
- remove them from the IC backend
- remove them from the PHB's MSI cache

This is currently open coded in two places in rtas_ibm_change_msi(),
and we're about to need this in spapr_phb_reset() as well. Instead of
duplicating this code again, make it a destroy function for the PHB's
MSI cache. Removing an MSI device from the cache will call the destroy
function internally.

Signed-off-by: Greg Kurz <groug@kaod.org>
---
 hw/ppc/spapr_pci.c |   24 +++++++++++++++---------
 1 file changed, 15 insertions(+), 9 deletions(-)

diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c
index 2d5697d119f4..bc22568bfa71 100644
--- a/hw/ppc/spapr_pci.c
+++ b/hw/ppc/spapr_pci.c
@@ -336,10 +336,6 @@ static void rtas_ibm_change_msi(PowerPCCPU *cpu, SpaprMachineState *spapr,
             return;
         }
 
-        if (!smc->legacy_irq_allocation) {
-            spapr_irq_msi_free(spapr, msi->first_irq, msi->num);
-        }
-        spapr_irq_free(spapr, msi->first_irq, msi->num);
         if (msi_present(pdev)) {
             spapr_msi_setmsg(pdev, 0, false, 0, 0);
         }
@@ -409,10 +405,6 @@ static void rtas_ibm_change_msi(PowerPCCPU *cpu, SpaprMachineState *spapr,
 
     /* Release previous MSIs */
     if (msi) {
-        if (!smc->legacy_irq_allocation) {
-            spapr_irq_msi_free(spapr, msi->first_irq, msi->num);
-        }
-        spapr_irq_free(spapr, msi->first_irq, msi->num);
         g_hash_table_remove(phb->msi, &config_addr);
     }
 
@@ -1806,6 +1798,19 @@ static void spapr_phb_unrealize(DeviceState *dev, Error **errp)
     memory_region_del_subregion(get_system_memory(), &sphb->mem32window);
 }
 
+static void spapr_phb_destroy_msi(gpointer opaque)
+{
+    SpaprMachineState *spapr = SPAPR_MACHINE(qdev_get_machine());
+    SpaprMachineClass *smc = SPAPR_MACHINE_GET_CLASS(spapr);
+    spapr_pci_msi *msi = opaque;
+
+    if (!smc->legacy_irq_allocation) {
+        spapr_irq_msi_free(spapr, msi->first_irq, msi->num);
+    }
+    spapr_irq_free(spapr, msi->first_irq, msi->num);
+    g_free(msi);
+}
+
 static void spapr_phb_realize(DeviceState *dev, Error **errp)
 {
     /* We don't use SPAPR_MACHINE() in order to exit gracefully if the user
@@ -2017,7 +2022,8 @@ static void spapr_phb_realize(DeviceState *dev, Error **errp)
                                     spapr_tce_get_iommu(tcet));
     }
 
-    sphb->msi = g_hash_table_new_full(g_int_hash, g_int_equal, g_free, g_free);
+    sphb->msi = g_hash_table_new_full(g_int_hash, g_int_equal, g_free,
+                                      spapr_phb_destroy_msi);
     return;
 
 unrealize:
Re: [Qemu-devel] [PATCH 1/3] spapr/pci: Consolidate de-allocation of MSIs
Posted by David Gibson 6 years, 6 months ago 
On Fri, Jul 26, 2019 at 04:44:38PM +0200, Greg Kurz wrote:
> When freeing MSIs, we need to:
> - remove them from the machine's MSI bitmap
> - remove them from the IC backend
> - remove them from the PHB's MSI cache
> 
> This is currently open coded in two places in rtas_ibm_change_msi(),
> and we're about to need this in spapr_phb_reset() as well. Instead of
> duplicating this code again, make it a destroy function for the PHB's
> MSI cache. Removing an MSI device from the cache will call the destroy
> function internally.
> 
> Signed-off-by: Greg Kurz <groug@kaod.org>

Applied to ppc-for-4.2.

> ---
>  hw/ppc/spapr_pci.c |   24 +++++++++++++++---------
>  1 file changed, 15 insertions(+), 9 deletions(-)
> 
> diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c
> index 2d5697d119f4..bc22568bfa71 100644
> --- a/hw/ppc/spapr_pci.c
> +++ b/hw/ppc/spapr_pci.c
> @@ -336,10 +336,6 @@ static void rtas_ibm_change_msi(PowerPCCPU *cpu, SpaprMachineState *spapr,
>              return;
>          }
>  
> -        if (!smc->legacy_irq_allocation) {
> -            spapr_irq_msi_free(spapr, msi->first_irq, msi->num);
> -        }
> -        spapr_irq_free(spapr, msi->first_irq, msi->num);
>          if (msi_present(pdev)) {
>              spapr_msi_setmsg(pdev, 0, false, 0, 0);
>          }
> @@ -409,10 +405,6 @@ static void rtas_ibm_change_msi(PowerPCCPU *cpu, SpaprMachineState *spapr,
>  
>      /* Release previous MSIs */
>      if (msi) {
> -        if (!smc->legacy_irq_allocation) {
> -            spapr_irq_msi_free(spapr, msi->first_irq, msi->num);
> -        }
> -        spapr_irq_free(spapr, msi->first_irq, msi->num);
>          g_hash_table_remove(phb->msi, &config_addr);
>      }
>  
> @@ -1806,6 +1798,19 @@ static void spapr_phb_unrealize(DeviceState *dev, Error **errp)
>      memory_region_del_subregion(get_system_memory(), &sphb->mem32window);
>  }
>  
> +static void spapr_phb_destroy_msi(gpointer opaque)
> +{
> +    SpaprMachineState *spapr = SPAPR_MACHINE(qdev_get_machine());
> +    SpaprMachineClass *smc = SPAPR_MACHINE_GET_CLASS(spapr);
> +    spapr_pci_msi *msi = opaque;
> +
> +    if (!smc->legacy_irq_allocation) {
> +        spapr_irq_msi_free(spapr, msi->first_irq, msi->num);
> +    }
> +    spapr_irq_free(spapr, msi->first_irq, msi->num);
> +    g_free(msi);
> +}
> +
>  static void spapr_phb_realize(DeviceState *dev, Error **errp)
>  {
>      /* We don't use SPAPR_MACHINE() in order to exit gracefully if the user
> @@ -2017,7 +2022,8 @@ static void spapr_phb_realize(DeviceState *dev, Error **errp)
>                                      spapr_tce_get_iommu(tcet));
>      }
>  
> -    sphb->msi = g_hash_table_new_full(g_int_hash, g_int_equal, g_free, g_free);
> +    sphb->msi = g_hash_table_new_full(g_int_hash, g_int_equal, g_free,
> +                                      spapr_phb_destroy_msi);
>      return;
>  
>  unrealize:
> 

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson
Re: [Qemu-devel] [PATCH 1/3] spapr/pci: Consolidate de-allocation of MSIs
Posted by Cédric Le Goater 6 years, 6 months ago 
On 26/07/2019 16:44, Greg Kurz wrote:
> When freeing MSIs, we need to:
> - remove them from the machine's MSI bitmap
> - remove them from the IC backend
> - remove them from the PHB's MSI cache
> 
> This is currently open coded in two places in rtas_ibm_change_msi(),
> and we're about to need this in spapr_phb_reset() as well. Instead of
> duplicating this code again, make it a destroy function for the PHB's
> MSI cache. Removing an MSI device from the cache will call the destroy
> function internally.

This looks good.



Reviewed-by: Cédric Le Goater <clg@kaod.org>

Thanks,

C.


> Signed-off-by: Greg Kurz <groug@kaod.org>
> ---
>  hw/ppc/spapr_pci.c |   24 +++++++++++++++---------
>  1 file changed, 15 insertions(+), 9 deletions(-)
> 
> diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c
> index 2d5697d119f4..bc22568bfa71 100644
> --- a/hw/ppc/spapr_pci.c
> +++ b/hw/ppc/spapr_pci.c
> @@ -336,10 +336,6 @@ static void rtas_ibm_change_msi(PowerPCCPU *cpu, SpaprMachineState *spapr,
>              return;
>          }
>  
> -        if (!smc->legacy_irq_allocation) {
> -            spapr_irq_msi_free(spapr, msi->first_irq, msi->num);
> -        }
> -        spapr_irq_free(spapr, msi->first_irq, msi->num);
>          if (msi_present(pdev)) {
>              spapr_msi_setmsg(pdev, 0, false, 0, 0);
>          }
> @@ -409,10 +405,6 @@ static void rtas_ibm_change_msi(PowerPCCPU *cpu, SpaprMachineState *spapr,
>  
>      /* Release previous MSIs */
>      if (msi) {
> -        if (!smc->legacy_irq_allocation) {
> -            spapr_irq_msi_free(spapr, msi->first_irq, msi->num);
> -        }
> -        spapr_irq_free(spapr, msi->first_irq, msi->num);
>          g_hash_table_remove(phb->msi, &config_addr);
>      }
>
> @@ -1806,6 +1798,19 @@ static void spapr_phb_unrealize(DeviceState *dev, Error **errp)
>      memory_region_del_subregion(get_system_memory(), &sphb->mem32window);
>  }
>  
> +static void spapr_phb_destroy_msi(gpointer opaque)
> +{
> +    SpaprMachineState *spapr = SPAPR_MACHINE(qdev_get_machine());
> +    SpaprMachineClass *smc = SPAPR_MACHINE_GET_CLASS(spapr);
> +    spapr_pci_msi *msi = opaque;
> +
> +    if (!smc->legacy_irq_allocation) {
> +        spapr_irq_msi_free(spapr, msi->first_irq, msi->num);
> +    }
> +    spapr_irq_free(spapr, msi->first_irq, msi->num);
> +    g_free(msi);
> +}
> +
>  static void spapr_phb_realize(DeviceState *dev, Error **errp)
>  {
>      /* We don't use SPAPR_MACHINE() in order to exit gracefully if the user
> @@ -2017,7 +2022,8 @@ static void spapr_phb_realize(DeviceState *dev, Error **errp)
>                                      spapr_tce_get_iommu(tcet));
>      }
>  
> -    sphb->msi = g_hash_table_new_full(g_int_hash, g_int_equal, g_free, g_free);
> +    sphb->msi = g_hash_table_new_full(g_int_hash, g_int_equal, g_free,
> +                                      spapr_phb_destroy_msi);
>      return;
>  
>  unrealize:
>

Patchew 2.1-devel-b67e4c2

© 2016 - 2026 Red Hat, Inc.