Patchwork [1/1] powerpc: Force 32 bit MSIs on systems lacking firmware support

login
register
mail settings
Submitter Brian King
Date May 21, 2013, 9:54 p.m.
Message ID <201305212154.r4LLs4Zu026123@d01av03.pok.ibm.com>
Download mbox | patch
Permalink /patch/245414/
State Superseded
Headers show

Comments

Brian King - May 21, 2013, 9:54 p.m.
Recent commit e61133dda480062d221f09e4fc18f66763f8ecd0 added support
for a new firmware feature to force an adapter to use 32 bit MSIs.
However, this firmware is not available for all systems. The hack below
allows devices needing 32 bit MSIs to work on these systems as well.
It is careful to only enable this on Gen2 slots, which should limit
this to configurations where this hack is needed and tested to work.

Signed-off-by: Brian King <brking@linux.vnet.ibm.com>
---

 arch/powerpc/platforms/pseries/msi.c |   31 +++++++++++++++++++++++++++----
 1 file changed, 27 insertions(+), 4 deletions(-)
Michael Ellerman - May 22, 2013, 4:36 a.m.
On Tue, May 21, 2013 at 04:54:04PM -0500, Brian King wrote:
> 
> Recent commit e61133dda480062d221f09e4fc18f66763f8ecd0 added support
> for a new firmware feature to force an adapter to use 32 bit MSIs.
> However, this firmware is not available for all systems. The hack below
> allows devices needing 32 bit MSIs to work on these systems as well.
> It is careful to only enable this on Gen2 slots, which should limit
> this to configurations where this hack is needed and tested to work.

Sorry I know you've already sent this to me once, but I didn't get time
to reply.

> diff -puN arch/powerpc/platforms/pseries/msi.c~powerpc_32bit_msi_hack_on_papr arch/powerpc/platforms/pseries/msi.c
> --- linux/arch/powerpc/platforms/pseries/msi.c~powerpc_32bit_msi_hack_on_papr	2013-05-15 10:44:46.000000000 -0500
> +++ linux-bjking1/arch/powerpc/platforms/pseries/msi.c	2013-05-20 15:24:52.000000000 -0500
> @@ -397,10 +397,11 @@ static int check_msix_entries(struct pci
>  static int rtas_setup_msi_irqs(struct pci_dev *pdev, int nvec_in, int type)
>  {
>  	struct pci_dn *pdn;
> -	int hwirq, virq, i, rc;
> +	int hwirq, virq, i, rc = -1;

I'd rather you didn't do a catch-all initialisation like this, it's too
easy to miss a return path.

>  	struct msi_desc *entry;
>  	struct msi_msg msg;
>  	int nvec = nvec_in;
> +	int use_32bit_msi_hack = 0;
>  
>  	pdn = get_pdn(pdev);
>  	if (!pdn)
> @@ -428,15 +429,37 @@ static int rtas_setup_msi_irqs(struct pc
>  	 */
>  again:
>  	if (type == PCI_CAP_ID_MSI) {
> -		if (pdn->force_32bit_msi)
> +		if (pdn->force_32bit_msi) {
>  			rc = rtas_change_msi(pdn, RTAS_CHANGE_32MSI_FN, nvec);
> -		else
> +			if (rc < 0) {
> +				/* We only want to run the 32 bit MSI hack below if
> +				 the max bus speed is Gen2 speed. */
> +				if (pdev->bus->max_bus_speed != PCIE_SPEED_5_0GT)
> +					return rc;
> +
> +				use_32bit_msi_hack = 1;
> +			}
> +		}
> +
> +		if (rc < 0)
>  			rc = rtas_change_msi(pdn, RTAS_CHANGE_MSI_FN, nvec);
>  
> -		if (rc < 0 && !pdn->force_32bit_msi) {
> +		if (rc < 0) {
>  			pr_debug("rtas_msi: trying the old firmware call.\n");
>  			rc = rtas_change_msi(pdn, RTAS_CHANGE_FN, nvec);
>  		}
> +
> +		if (use_32bit_msi_hack && rc > 0) {
> +			int pos;
> +			u32 addr_hi, addr_lo;
> +
> +			dev_info(&pdev->dev, "rtas_msi: No 32 bit MSI firmware support, forcing 32 bit MSI\n");
> +			pos = pci_find_capability(pdev, PCI_CAP_ID_MSI);
> +			pci_read_config_dword(pdev, pos + PCI_MSI_ADDRESS_HI, &addr_hi);
> +			addr_lo = 0xffff0000 | ((addr_hi >> (48 - 32)) << 4);
> +			pci_write_config_dword(pdev, pos + PCI_MSI_ADDRESS_LO, addr_lo);
> +			pci_write_config_dword(pdev, pos + PCI_MSI_ADDRESS_HI, 0);

This is basically baking knowledge of phyp's address layout into the
kernel right? Which is OK, but it needs a big fat comment describing
exactly what it's doing and why it's safe.

cheers
Benjamin Herrenschmidt - May 22, 2013, 4:44 a.m.
On Wed, 2013-05-22 at 14:36 +1000, Michael Ellerman wrote:
> This is basically baking knowledge of phyp's address layout into the
> kernel right? Which is OK, but it needs a big fat comment describing
> exactly what it's doing and why it's safe.

Not pHyp really but the HW, basically this should work with any IODA1
host bridge (P7IOC, Torrent, ...). The "assumption" here is that RTAS
MSI + PCIe Gen2 == IODA1 :-)

Cheers,
Ben.
Mike Qiu - May 22, 2013, 6:28 a.m.
于 2013/5/22 5:54, Brian King 写道:
> Recent commit e61133dda480062d221f09e4fc18f66763f8ecd0 added support
> for a new firmware feature to force an adapter to use 32 bit MSIs.
> However, this firmware is not available for all systems. The hack below
> allows devices needing 32 bit MSIs to work on these systems as well.
> It is careful to only enable this on Gen2 slots, which should limit
> this to configurations where this hack is needed and tested to work.
>
> Signed-off-by: Brian King <brking@linux.vnet.ibm.com>
> ---
>
>   arch/powerpc/platforms/pseries/msi.c |   31 +++++++++++++++++++++++++++----
>   1 file changed, 27 insertions(+), 4 deletions(-)
>
> diff -puN arch/powerpc/platforms/pseries/msi.c~powerpc_32bit_msi_hack_on_papr arch/powerpc/platforms/pseries/msi.c
> --- linux/arch/powerpc/platforms/pseries/msi.c~powerpc_32bit_msi_hack_on_papr	2013-05-15 10:44:46.000000000 -0500
> +++ linux-bjking1/arch/powerpc/platforms/pseries/msi.c	2013-05-20 15:24:52.000000000 -0500
> @@ -397,10 +397,11 @@ static int check_msix_entries(struct pci
>   static int rtas_setup_msi_irqs(struct pci_dev *pdev, int nvec_in, int type)
>   {
>   	struct pci_dn *pdn;
> -	int hwirq, virq, i, rc;
> +	int hwirq, virq, i, rc = -1;
>   	struct msi_desc *entry;
>   	struct msi_msg msg;
>   	int nvec = nvec_in;
> +	int use_32bit_msi_hack = 0;
>
>   	pdn = get_pdn(pdev);
>   	if (!pdn)
> @@ -428,15 +429,37 @@ static int rtas_setup_msi_irqs(struct pc
>   	 */
>   again:
>   	if (type == PCI_CAP_ID_MSI) {
> -		if (pdn->force_32bit_msi)
> +		if (pdn->force_32bit_msi) {
>   			rc = rtas_change_msi(pdn, RTAS_CHANGE_32MSI_FN, nvec);
> -		else
> +			if (rc < 0) {
> +				/* We only want to run the 32 bit MSI hack below if
> +				 the max bus speed is Gen2 speed. */
> +				if (pdev->bus->max_bus_speed != PCIE_SPEED_5_0GT)
> +					return rc;
> +
> +				use_32bit_msi_hack = 1;
> +			}
> +		}
> +
> +		if (rc < 0)
>   			rc = rtas_change_msi(pdn, RTAS_CHANGE_MSI_FN, nvec);
>
> -		if (rc < 0 && !pdn->force_32bit_msi) {
> +		if (rc < 0) {
>   			pr_debug("rtas_msi: trying the old firmware call.\n");
>   			rc = rtas_change_msi(pdn, RTAS_CHANGE_FN, nvec);
>   		}
> +
> +		if (use_32bit_msi_hack && rc > 0) {
> +			int pos;
> +			u32 addr_hi, addr_lo;
> +
> +			dev_info(&pdev->dev, "rtas_msi: No 32 bit MSI firmware support, forcing 32 bit MSI\n");
> +			pos = pci_find_capability(pdev, PCI_CAP_ID_MSI);
> +			pci_read_config_dword(pdev, pos + PCI_MSI_ADDRESS_HI, &addr_hi);
> +			addr_lo = 0xffff0000 | ((addr_hi >> (48 - 32)) << 4);
> +			pci_write_config_dword(pdev, pos + PCI_MSI_ADDRESS_LO, addr_lo);
> +			pci_write_config_dword(pdev, pos + PCI_MSI_ADDRESS_HI, 0);
I think here you can use catched dev->msi_cap for better.

Thanks
Mike
> +		}
>   	} else
>   		rc = rtas_change_msi(pdn, RTAS_CHANGE_MSIX_FN, nvec);
>
> _
>
> _______________________________________________
> Linuxppc-dev mailing list
> Linuxppc-dev@lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/linuxppc-dev
>
>

Patch

diff -puN arch/powerpc/platforms/pseries/msi.c~powerpc_32bit_msi_hack_on_papr arch/powerpc/platforms/pseries/msi.c
--- linux/arch/powerpc/platforms/pseries/msi.c~powerpc_32bit_msi_hack_on_papr	2013-05-15 10:44:46.000000000 -0500
+++ linux-bjking1/arch/powerpc/platforms/pseries/msi.c	2013-05-20 15:24:52.000000000 -0500
@@ -397,10 +397,11 @@  static int check_msix_entries(struct pci
 static int rtas_setup_msi_irqs(struct pci_dev *pdev, int nvec_in, int type)
 {
 	struct pci_dn *pdn;
-	int hwirq, virq, i, rc;
+	int hwirq, virq, i, rc = -1;
 	struct msi_desc *entry;
 	struct msi_msg msg;
 	int nvec = nvec_in;
+	int use_32bit_msi_hack = 0;
 
 	pdn = get_pdn(pdev);
 	if (!pdn)
@@ -428,15 +429,37 @@  static int rtas_setup_msi_irqs(struct pc
 	 */
 again:
 	if (type == PCI_CAP_ID_MSI) {
-		if (pdn->force_32bit_msi)
+		if (pdn->force_32bit_msi) {
 			rc = rtas_change_msi(pdn, RTAS_CHANGE_32MSI_FN, nvec);
-		else
+			if (rc < 0) {
+				/* We only want to run the 32 bit MSI hack below if
+				 the max bus speed is Gen2 speed. */
+				if (pdev->bus->max_bus_speed != PCIE_SPEED_5_0GT)
+					return rc;
+
+				use_32bit_msi_hack = 1;
+			}
+		}
+
+		if (rc < 0)
 			rc = rtas_change_msi(pdn, RTAS_CHANGE_MSI_FN, nvec);
 
-		if (rc < 0 && !pdn->force_32bit_msi) {
+		if (rc < 0) {
 			pr_debug("rtas_msi: trying the old firmware call.\n");
 			rc = rtas_change_msi(pdn, RTAS_CHANGE_FN, nvec);
 		}
+
+		if (use_32bit_msi_hack && rc > 0) {
+			int pos;
+			u32 addr_hi, addr_lo;
+
+			dev_info(&pdev->dev, "rtas_msi: No 32 bit MSI firmware support, forcing 32 bit MSI\n");
+			pos = pci_find_capability(pdev, PCI_CAP_ID_MSI);
+			pci_read_config_dword(pdev, pos + PCI_MSI_ADDRESS_HI, &addr_hi);
+			addr_lo = 0xffff0000 | ((addr_hi >> (48 - 32)) << 4);
+			pci_write_config_dword(pdev, pos + PCI_MSI_ADDRESS_LO, addr_lo);
+			pci_write_config_dword(pdev, pos + PCI_MSI_ADDRESS_HI, 0);
+		}
 	} else
 		rc = rtas_change_msi(pdn, RTAS_CHANGE_MSIX_FN, nvec);