Re: [PATCH 0/3] powerpc: Cleanup and convert to MSI parent domain

[Nam Cao <namcao@linutronix.de>]

> On Tue, Jul 22, 2025 at 02:05:55PM +0530, Gautam Menghani wrote:
> > I am seeing a boot failure after applying this series on top of the pci
> > tree [1]. Note that this error was seen on a system where I have a
> > dedicated NVME. Systems without dedicated disk boot fine
> 
> Thanks for the report.
> 
> Using QEMU, I cannot reproduce the exact same problem, but I do observe a
> different one. They are likely from the same root cause.
> 
> Let me investigate..

So the problem is due to the pair msi_prepare() and msi_post_free(). Before
this series, msi_prepare() is called whenever interrupt is allocated.
However, after this series, msi_prepare() is called only at domain
creation.

For most device drivers, this difference does not have any impact. However,
the NVME driver is slightly "special", it does this:

	1. Allocate interrupts
	2. Free interrupts
	3. Allocate interrupts again

Before this series:

	(1) calls msi_prepare()
	(2) calls msi_post_free()
	(3) calls msi_prepare() again

and it happens to work. However, after this series:

	(1) calls msi_prepare()
	(2) calls msi_post_free()
	(3) does not call either

and we are in trouble.

A simple solution is using msi_teardown() instead, which is called at
domain destruction. It makes more sense this way as well, because
msi_teardown() is supposed to reverse what msi_prepare() does.

This would also remove the only user of msi_post_free(), allowing us to
delete that callback.

The below patch fixes the problem that I saw with QEMU. Does it fix the
problem on your side as well?

Best regards,
Nam


diff --git a/arch/powerpc/platforms/pseries/msi.c b/arch/powerpc/platforms/pseries/msi.c
index 70be6e24427d..7da142dd5baa 100644
--- a/arch/powerpc/platforms/pseries/msi.c
+++ b/arch/powerpc/platforms/pseries/msi.c
@@ -441,12 +441,12 @@ static int pseries_msi_ops_prepare(struct irq_domain *domain, struct device *dev
  * 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.
  */
-static void pseries_msi_post_free(struct irq_domain *domain, struct device *dev)
+static void pseries_msi_ops_teardown(struct irq_domain *domain, msi_alloc_info_t *arg)
 {
-	if (WARN_ON_ONCE(!dev_is_pci(dev)))
-		return;
+	struct msi_desc *desc = arg->desc;
+	struct pci_dev *pdev = msi_desc_to_pci_dev(desc);
 
-	rtas_disable_msi(to_pci_dev(dev));
+	rtas_disable_msi(pdev);
 }
 
 static void pseries_msi_shutdown(struct irq_data *d)
@@ -482,7 +482,7 @@ static bool pseries_init_dev_msi_info(struct device *dev, struct irq_domain *dom
 	chip->irq_write_msi_msg	= pseries_msi_write_msg;
 
 	info->ops->msi_prepare = pseries_msi_ops_prepare;
-	info->ops->msi_post_free = pseries_msi_post_free;
+	info->ops->msi_teardown = pseries_msi_ops_teardown;
 
 	return true;
 }