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 <c...@kaod.org> Thanks, C. > Signed-off-by: Greg Kurz <gr...@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: >