Adding Marc. On 4/30/21 10:03 AM, Cédric Le Goater wrote: > The MSI domain clears the IRQ with msi_domain_free(), which calls > irq_domain_free_irqs_top(), which clears the handler data. This is a > problem for the XIVE controller since we need to unmap MMIO pages and > free a specific XIVE structure. > > The 'msi_free()' handler is called before irq_domain_free_irqs_top() > when the handler data is still available. Use that to clear the XIVE > controller data. This feels like a clumsy way of doing so.
irq_domain_free_irqs_parent() would be my preferred way to clear the lowlevel handler data but we can't today. Could there be a smarter way ? Thanks, C. > Cc: Thomas Gleixner <t...@linutronix.de> > Signed-off-by: Cédric Le Goater <c...@kaod.org> > > --- > arch/powerpc/include/asm/xive.h | 1 + > arch/powerpc/platforms/pseries/msi.c | 16 +++++++++++++++- > arch/powerpc/sysdev/xive/common.c | 5 ++++- > 3 files changed, 20 insertions(+), 2 deletions(-) > > diff --git a/arch/powerpc/include/asm/xive.h b/arch/powerpc/include/asm/xive.h > index aa094a8655b0..20ae50ab083c 100644 > --- a/arch/powerpc/include/asm/xive.h > +++ b/arch/powerpc/include/asm/xive.h > @@ -111,6 +111,7 @@ void xive_native_free_vp_block(u32 vp_base); > int xive_native_populate_irq_data(u32 hw_irq, > struct xive_irq_data *data); > void xive_cleanup_irq_data(struct xive_irq_data *xd); > +void xive_irq_free_data(unsigned int virq); > void xive_native_free_irq(u32 irq); > int xive_native_configure_irq(u32 hw_irq, u32 target, u8 prio, u32 sw_irq); > > diff --git a/arch/powerpc/platforms/pseries/msi.c > b/arch/powerpc/platforms/pseries/msi.c > index a41c448520d4..da9d63a088bb 100644 > --- a/arch/powerpc/platforms/pseries/msi.c > +++ b/arch/powerpc/platforms/pseries/msi.c > @@ -529,6 +529,19 @@ static int pseries_msi_ops_prepare(struct irq_domain > *domain, struct device *dev > return rtas_prepare_msi_irqs(pdev, nvec, type, arg); > } > > +/* > + * ->msi_free() is called before irq_domain_free_irqs_top() when the > + * handler data is still available. Use that to clear the XIVE > + * controller data. > + */ > +static void pseries_msi_ops_msi_free(struct irq_domain *domain, > + struct msi_domain_info *info, > + unsigned int irq) > +{ > + if (xive_enabled()) > + xive_irq_free_data(irq); > +} > + > /* > * RTAS can not disable one MSI at a time. It's all or nothing. Do it > * at the end after all IRQs have been freed. > @@ -546,6 +559,7 @@ static void pseries_msi_domain_free_irqs(struct > irq_domain *domain, > > static struct msi_domain_ops pseries_pci_msi_domain_ops = { > .msi_prepare = pseries_msi_ops_prepare, > + .msi_free = pseries_msi_ops_msi_free, > .domain_free_irqs = pseries_msi_domain_free_irqs, > }; > > @@ -660,7 +674,7 @@ static void pseries_irq_domain_free(struct irq_domain > *domain, unsigned int virq > > pr_debug("%s bridge %pOF %d #%d\n", __func__, phb->dn, virq, nr_irqs); > > - irq_domain_free_irqs_parent(domain, virq, nr_irqs); > + /* XIVE domain data is cleared through ->msi_free() */ > } > > static const struct irq_domain_ops pseries_irq_domain_ops = { > diff --git a/arch/powerpc/sysdev/xive/common.c > b/arch/powerpc/sysdev/xive/common.c > index 3485baf9ec8c..191cd80ec534 100644 > --- a/arch/powerpc/sysdev/xive/common.c > +++ b/arch/powerpc/sysdev/xive/common.c > @@ -980,6 +980,8 @@ EXPORT_SYMBOL_GPL(is_xive_irq); > > void xive_cleanup_irq_data(struct xive_irq_data *xd) > { > + pr_debug("%s for HW %x\n", __func__, xd->hw_irq); > + > if (xd->eoi_mmio) { > unmap_kernel_range((unsigned long)xd->eoi_mmio, > 1u << xd->esb_shift); > @@ -1025,7 +1027,7 @@ static int xive_irq_alloc_data(unsigned int virq, > irq_hw_number_t hw) > return 0; > } > > -static void xive_irq_free_data(unsigned int virq) > +void xive_irq_free_data(unsigned int virq) > { > struct xive_irq_data *xd = irq_get_handler_data(virq); > > @@ -1035,6 +1037,7 @@ static void xive_irq_free_data(unsigned int virq) > xive_cleanup_irq_data(xd); > kfree(xd); > } > +EXPORT_SYMBOL_GPL(xive_irq_free_data); > > #ifdef CONFIG_SMP > >