[PATCH for-rc] IB/hfi: Only read capability registers if the capability exists

[PATCH for-rc] IB/hfi: Only read capability registers if the capability exists

[Dennis Dalessandro]

From: Michael J. Ruhl <michael.j.ruhl@intel.com>

During driver init, various registers are saved to allow restoration
after an FLR or gen3 bump.  Some of these registers are not available
in some circumstances (i.e. Virtual machines).

Delete unnecessary register read/write, and only access register if
the capability exists.

Cc: <stable@vger.kernel.org> # 4.14.x
Fixes: a618b7e40af2 ("IB/hfi1: Move saving PCI values to a separate function")
Reviewed-by: Mike Marciniszyn <mike.marciniszyn@intel.com>
Signed-off-by: Michael J. Ruhl <michael.j.ruhl@intel.com>
Signed-off-by: Dennis Dalessandro <dennis.dalessandro@intel.com>
---
 drivers/infiniband/hw/hfi1/hfi.h  |    1 -
 drivers/infiniband/hw/hfi1/pcie.c |   30 ++++++++++++------------------
 2 files changed, 12 insertions(+), 19 deletions(-)

diff --git a/drivers/infiniband/hw/hfi1/hfi.h b/drivers/infiniband/hw/hfi1/hfi.h
index dc5b133..2fa651c 100644
--- a/drivers/infiniband/hw/hfi1/hfi.h
+++ b/drivers/infiniband/hw/hfi1/hfi.h
@@ -1131,7 +1131,6 @@ struct hfi1_devdata {
 	u16 pcie_lnkctl;
 	u16 pcie_devctl2;
 	u32 pci_msix0;
-	u32 pci_lnkctl3;
 	u32 pci_tph2;
 
 	/*
diff --git a/drivers/infiniband/hw/hfi1/pcie.c b/drivers/infiniband/hw/hfi1/pcie.c
index 09e50fd..8c7e7a6 100644
--- a/drivers/infiniband/hw/hfi1/pcie.c
+++ b/drivers/infiniband/hw/hfi1/pcie.c
@@ -411,15 +411,12 @@ int restore_pci_variables(struct hfi1_devdata *dd)
 	if (ret)
 		goto error;
 
-	ret = pci_write_config_dword(dd->pcidev, PCIE_CFG_SPCIE1,
-				     dd->pci_lnkctl3);
-	if (ret)
-		goto error;
-
-	ret = pci_write_config_dword(dd->pcidev, PCIE_CFG_TPH2, dd->pci_tph2);
-	if (ret)
-		goto error;
-
+	if (pci_find_ext_capability(dd->pcidev, PCI_EXT_CAP_ID_TPH)) {
+		ret = pci_write_config_dword(dd->pcidev, PCIE_CFG_TPH2,
+					     dd->pci_tph2);
+		if (ret)
+			goto error;
+	}
 	return 0;
 
 error:
@@ -469,15 +466,12 @@ int save_pci_variables(struct hfi1_devdata *dd)
 	if (ret)
 		goto error;
 
-	ret = pci_read_config_dword(dd->pcidev, PCIE_CFG_SPCIE1,
-				    &dd->pci_lnkctl3);
-	if (ret)
-		goto error;
-
-	ret = pci_read_config_dword(dd->pcidev, PCIE_CFG_TPH2, &dd->pci_tph2);
-	if (ret)
-		goto error;
-
+	if (pci_find_ext_capability(dd->pcidev, PCI_EXT_CAP_ID_TPH)) {
+		ret = pci_read_config_dword(dd->pcidev, PCIE_CFG_TPH2,
+					    &dd->pci_tph2);
+		if (ret)
+			goto error;
+	}
 	return 0;
 
 error:

Re: [PATCH for-rc] IB/hfi: Only read capability registers if the capability exists

[Jason Gunthorpe]

On Fri, Dec 22, 2017 at 08:47:20AM -0800, Dennis Dalessandro wrote:
> From: Michael J. Ruhl <michael.j.ruhl-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
> 
> During driver init, various registers are saved to allow restoration
> after an FLR or gen3 bump.  Some of these registers are not available
> in some circumstances (i.e. Virtual machines).
> 
> Delete unnecessary register read/write, and only access register if
> the capability exists.
> 
> Cc: <stable-u79uwXL29TY76Z2rM5mHXA@public.gmane.org> # 4.14.x
> Fixes: a618b7e40af2 ("IB/hfi1: Move saving PCI values to a separate function")
> Reviewed-by: Mike Marciniszyn <mike.marciniszyn-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
> Signed-off-by: Michael J. Ruhl <michael.j.ruhl-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
> Signed-off-by: Dennis Dalessandro <dennis.dalessandro-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
>  drivers/infiniband/hw/hfi1/hfi.h  |    1 -
>  drivers/infiniband/hw/hfi1/pcie.c |   30 ++++++++++++------------------
>  2 files changed, 12 insertions(+), 19 deletions(-)

Since we are getting late in the RC cycle can you help me clarify the
user visible bug here? No need to resend

I'm guessing when hfi1 is used in a virtual machine with passthrough
(SRIOV?) it fails in ?? way?

Jason
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH for-rc] IB/hfi: Only read capability registers if the capability exists

[Dennis Dalessandro]

On 12/22/2017 11:53 AM, Jason Gunthorpe wrote:
> On Fri, Dec 22, 2017 at 08:47:20AM -0800, Dennis Dalessandro wrote:
>> From: Michael J. Ruhl <michael.j.ruhl@intel.com>
>>
>> During driver init, various registers are saved to allow restoration
>> after an FLR or gen3 bump.  Some of these registers are not available
>> in some circumstances (i.e. Virtual machines).
>>
>> Delete unnecessary register read/write, and only access register if
>> the capability exists.
>>
>> Cc: <stable@vger.kernel.org> # 4.14.x
>> Fixes: a618b7e40af2 ("IB/hfi1: Move saving PCI values to a separate function")
>> Reviewed-by: Mike Marciniszyn <mike.marciniszyn@intel.com>
>> Signed-off-by: Michael J. Ruhl <michael.j.ruhl@intel.com>
>> Signed-off-by: Dennis Dalessandro <dennis.dalessandro@intel.com>
>>   drivers/infiniband/hw/hfi1/hfi.h  |    1 -
>>   drivers/infiniband/hw/hfi1/pcie.c |   30 ++++++++++++------------------
>>   2 files changed, 12 insertions(+), 19 deletions(-)
> 
> Since we are getting late in the RC cycle can you help me clarify the
> user visible bug here? No need to resend
> 
> I'm guessing when hfi1 is used in a virtual machine with passthrough
> (SRIOV?) it fails in ?? way?

Ugh. I actually amended the commit message to include a blurb on the 
impact. Sorry about that, not sure what happened. It should have had 
this line of text:

"
Without this patch the driver will not load on a virtual machine.
"

So basically we just don't work on VMs. There is no panic or anything. I 
was on the fence whether it could go for this late of an -rc or not.

-Denny