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

["Cédric Le Goater" <clg@kaod.org>]

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 = {
>