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
> Signed-off-by: Cédric Le Goater
>
> ---
> 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
>
>
