[PATCH] powerpc/pseries/msi: Add an empty irq_write_msi_msg() handler

[PATCH] powerpc/pseries/msi: Add an empty irq_write_msi_msg() handler

[Cédric Le Goater]

The IPR drivers tests for MSI support at probe time with MSI vector 0
and when done, frees the IRQ with free_irq(). This test was introduced
by 95fecd90397e ("ipr: add test for MSI interrupt support") as an
improvement of commit 5a9ef25b14d3 ("[SCSI] ipr: add MSI support")
because a boot failure was reported on a Bimini PowerPC system :

  https://x-lore.kernel.org/all/1242926159.3007.5.camel@localhost.localdomain/

It was finally decided to remove MSI support on Bimini systems in
6eb0ac03899a ("powerpc/maple: Add a quirk to disable MSI for IPR on
Bimini").

Linux 5.15-rc1 added MSI domain support to the pseries machine and
when free_irq is called() in the driver, msi_domain_deactivate() also
is. This resets the MSI table entry of the associate vector by calling
__pci_write_msi_msg() with an empty message and breaks any further
activation of the same vector. In the case of the IPR driver, it
breaks the initialization sequence of the IOA.

Introduce an empty irq_write_msi_msg() handler in the MSI domain of
the pseries machine to avoid clearing the MSI vector entry. Updating
the entry is not strictly necessary since it is initialized by the
underlying hypervisor, PowerVM or QEMU/KVM.

Cc: Wen Xiong <wenxiong@linux.vnet.ibm.com>
Cc: Douglas Miller <dougmill@linux.vnet.ibm.com>
Cc: Brian King <brking@linux.vnet.ibm.com>
Fixes: a5f3d2c17b07 ("powerpc/pseries/pci: Add MSI domains")
Signed-off-by: Cédric Le Goater <clg@kaod.org>
---

 We could also revert commit 95fecd90397e ("ipr: add test for MSI
 interrupt support") which doesn't seem very useful nowdays. Or
 rewrite the test to improve how MSI vectors are used.

 Please advise !

 Thanks,


 arch/powerpc/platforms/pseries/msi.c | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

diff --git a/arch/powerpc/platforms/pseries/msi.c b/arch/powerpc/platforms/pseries/msi.c
index 1b305e411862..37eb35f5194d 100644
--- a/arch/powerpc/platforms/pseries/msi.c
+++ b/arch/powerpc/platforms/pseries/msi.c
@@ -507,12 +507,27 @@ static void pseries_msi_unmask(struct irq_data *d)
 	irq_chip_unmask_parent(d);
 }
 
+static void pseries_msi_write_msg(struct irq_data *data, struct msi_msg *msg)
+{
+	struct msi_desc *entry = irq_data_get_msi_desc(data);
+
+	/* Do not update the MSIx vector table. This is not strictly
+	 * necessary since the table is initialized by the underlying
+	 * hypervisor, PowerVM or QEMU/KVM. However, if the MSIx
+	 * vector entry is cleared, any further activation will fail.
+	 * This can happen in some drivers (IPR) which deactivate the
+	 * IRQ used for testing MSI support.
+	 */
+	entry->msg = *msg;
+}
+
 static struct irq_chip pseries_pci_msi_irq_chip = {
 	.name		= "pSeries-PCI-MSI",
 	.irq_shutdown	= pseries_msi_shutdown,
 	.irq_mask	= pseries_msi_mask,
 	.irq_unmask	= pseries_msi_unmask,
 	.irq_eoi	= irq_chip_eoi_parent,
+	.irq_write_msi_msg	= pseries_msi_write_msg,
 };
 
 static struct msi_domain_info pseries_msi_domain_info = {
-- 
2.31.1

Re: [PATCH] powerpc/pseries/msi: Add an empty irq_write_msi_msg() handler

[Cédric Le Goater]

On 9/30/21 12:25, Cédric Le Goater wrote:
> The IPR drivers tests for MSI support at probe time with MSI vector 0
> and when done, frees the IRQ with free_irq(). This test was introduced
> by 95fecd90397e ("ipr: add test for MSI interrupt support") as an
> improvement of commit 5a9ef25b14d3 ("[SCSI] ipr: add MSI support")
> because a boot failure was reported on a Bimini PowerPC system :
> 
>    https://x-lore.kernel.org/all/1242926159.3007.5.camel@localhost.localdomain/
> 
> It was finally decided to remove MSI support on Bimini systems in
> 6eb0ac03899a ("powerpc/maple: Add a quirk to disable MSI for IPR on
> Bimini").
> 
> Linux 5.15-rc1 added MSI domain support to the pseries machine and
> when free_irq is called() in the driver, msi_domain_deactivate() also
> is. This resets the MSI table entry of the associate vector by calling
> __pci_write_msi_msg() with an empty message and breaks any further
> activation of the same vector. In the case of the IPR driver, it
> breaks the initialization sequence of the IOA.
> 
> Introduce an empty irq_write_msi_msg() handler in the MSI domain of
> the pseries machine to avoid clearing the MSI vector entry. Updating
> the entry is not strictly necessary since it is initialized by the
> underlying hypervisor, PowerVM or QEMU/KVM.
> 
> Cc: Wen Xiong <wenxiong@linux.vnet.ibm.com>
> Cc: Douglas Miller <dougmill@linux.vnet.ibm.com>
> Cc: Brian King <brking@linux.vnet.ibm.com>
> Fixes: a5f3d2c17b07 ("powerpc/pseries/pci: Add MSI domains")
> Signed-off-by: Cédric Le Goater <clg@kaod.org>

This is missing :

Reported-by: Abdul Haleem <abdhalee@linux.vnet.ibm.com>

Thanks,

C.

> ---
> 
>   We could also revert commit 95fecd90397e ("ipr: add test for MSI
>   interrupt support") which doesn't seem very useful nowdays. Or
>   rewrite the test to improve how MSI vectors are used.
> 
>   Please advise !
> 
>   Thanks,
> 
> 
>   arch/powerpc/platforms/pseries/msi.c | 15 +++++++++++++++
>   1 file changed, 15 insertions(+)
> 
> diff --git a/arch/powerpc/platforms/pseries/msi.c b/arch/powerpc/platforms/pseries/msi.c
> index 1b305e411862..37eb35f5194d 100644
> --- a/arch/powerpc/platforms/pseries/msi.c
> +++ b/arch/powerpc/platforms/pseries/msi.c
> @@ -507,12 +507,27 @@ static void pseries_msi_unmask(struct irq_data *d)
>   	irq_chip_unmask_parent(d);
>   }
>   
> +static void pseries_msi_write_msg(struct irq_data *data, struct msi_msg *msg)
> +{
> +	struct msi_desc *entry = irq_data_get_msi_desc(data);
> +
> +	/* Do not update the MSIx vector table. This is not strictly
> +	 * necessary since the table is initialized by the underlying
> +	 * hypervisor, PowerVM or QEMU/KVM. However, if the MSIx
> +	 * vector entry is cleared, any further activation will fail.
> +	 * This can happen in some drivers (IPR) which deactivate the
> +	 * IRQ used for testing MSI support.
> +	 */
> +	entry->msg = *msg;
> +}
> +
>   static struct irq_chip pseries_pci_msi_irq_chip = {
>   	.name		= "pSeries-PCI-MSI",
>   	.irq_shutdown	= pseries_msi_shutdown,
>   	.irq_mask	= pseries_msi_mask,
>   	.irq_unmask	= pseries_msi_unmask,
>   	.irq_eoi	= irq_chip_eoi_parent,
> +	.irq_write_msi_msg	= pseries_msi_write_msg,
>   };
>   
>   static struct msi_domain_info pseries_msi_domain_info = {
> 

Re: [PATCH] powerpc/pseries/msi: Add an empty irq_write_msi_msg() handler

[Mahesh J Salgaonkar]

On 2021-09-30 12:25:35 Thu, Cédric Le Goater wrote:
> The IPR drivers tests for MSI support at probe time with MSI vector 0
> and when done, frees the IRQ with free_irq(). This test was introduced
> by 95fecd90397e ("ipr: add test for MSI interrupt support") as an
> improvement of commit 5a9ef25b14d3 ("[SCSI] ipr: add MSI support")
> because a boot failure was reported on a Bimini PowerPC system :
> 
>   https://x-lore.kernel.org/all/1242926159.3007.5.camel@localhost.localdomain/
> 
> It was finally decided to remove MSI support on Bimini systems in
> 6eb0ac03899a ("powerpc/maple: Add a quirk to disable MSI for IPR on
> Bimini").
> 
> Linux 5.15-rc1 added MSI domain support to the pseries machine and
> when free_irq is called() in the driver, msi_domain_deactivate() also
> is. This resets the MSI table entry of the associate vector by calling
> __pci_write_msi_msg() with an empty message and breaks any further
> activation of the same vector. In the case of the IPR driver, it
> breaks the initialization sequence of the IOA.
> 
> Introduce an empty irq_write_msi_msg() handler in the MSI domain of
> the pseries machine to avoid clearing the MSI vector entry. Updating
> the entry is not strictly necessary since it is initialized by the
> underlying hypervisor, PowerVM or QEMU/KVM.
> 
> Cc: Wen Xiong <wenxiong@linux.vnet.ibm.com>
> Cc: Douglas Miller <dougmill@linux.vnet.ibm.com>
> Cc: Brian King <brking@linux.vnet.ibm.com>
> Fixes: a5f3d2c17b07 ("powerpc/pseries/pci: Add MSI domains")
> Signed-off-by: Cédric Le Goater <clg@kaod.org>

Tested-by: Mahesh Salgaonkar <mahesh@linux.ibm.com>

This fixes the issue reported at
https://lore.kernel.org/linuxppc-dev/65f0085f-c6a9-e3ea-4d60-fcf09b7c7360@linux.vnet.ibm.com/T/#u
by Abdul.

Thanks,
-- 
Mahesh J Salgaonkar

Re: [PATCH] powerpc/pseries/msi: Add an empty irq_write_msi_msg() handler

[Michael Ellerman]

On Thu, 30 Sep 2021 12:25:35 +0200, Cédric Le Goater wrote:
> The IPR drivers tests for MSI support at probe time with MSI vector 0
> and when done, frees the IRQ with free_irq(). This test was introduced
> by 95fecd90397e ("ipr: add test for MSI interrupt support") as an
> improvement of commit 5a9ef25b14d3 ("[SCSI] ipr: add MSI support")
> because a boot failure was reported on a Bimini PowerPC system :
> 
>   https://x-lore.kernel.org/all/1242926159.3007.5.camel@localhost.localdomain/
> 
> [...]

Applied to powerpc/fixes.

[1/1] powerpc/pseries/msi: Add an empty irq_write_msi_msg() handler
      https://git.kernel.org/powerpc/c/5a4b0320783a19f877dd595813569b3c25f4ff81

cheers