[for-rc,1/4] IB/hfi1: Allow for all speeds higher than gen3

Commit Message

Dennis Dalessandro Oct. 25, 2019, 7:58 p.m. UTC

From: James Erwin <[e-mail]>

The driver avoids the gen3 speed bump when the parent
bus speed isn't identical to gen3, 8.0GT/s.  This is not
compatible with gen4 and newer speeds.

Fix by relaxing the test to explicitly look for the lower
capability speeds which inherently allows for all future speeds.

Fixes: 7724105686e7 ("IB/hfi1: add driver files")
Cc: <[e-mail]>
Reviewed-by: Dennis Dalessandro <[e-mail]>
Reviewed-by: Kaike Wan <[e-mail]>
Signed-off-by: James Erwin <[e-mail]>
Signed-off-by: Mike Marciniszyn <[e-mail]>
Signed-off-by: Dennis Dalessandro <[e-mail]>
---
 drivers/infiniband/hw/hfi1/pcie.c |    4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

Comments

Jason Gunthorpe Oct. 29, 2019, 7:52 p.m. UTC | #1

On Fri, Oct 25, 2019 at 03:58:24PM -0400, Dennis Dalessandro wrote:
> From: James Erwin <[e-mail]>
> 
> The driver avoids the gen3 speed bump when the parent
> bus speed isn't identical to gen3, 8.0GT/s.  This is not
> compatible with gen4 and newer speeds.
> 
> Fix by relaxing the test to explicitly look for the lower
> capability speeds which inherently allows for all future speeds.

This description does not seem like stable material to me.

Jason

Marciniszyn, Mike Oct. 29, 2019, 9:19 p.m. UTC | #2

> Subject: Re: [PATCH for-rc 1/4] IB/hfi1: Allow for all speeds higher than gen3
> 
> On Fri, Oct 25, 2019 at 03:58:24PM -0400, Dennis Dalessandro wrote:
> > From: James Erwin <[e-mail]>
> >
> > The driver avoids the gen3 speed bump when the parent
> > bus speed isn't identical to gen3, 8.0GT/s.  This is not
> > compatible with gen4 and newer speeds.
> >
> > Fix by relaxing the test to explicitly look for the lower
> > capability speeds which inherently allows for all future speeds.
> 
> This description does not seem like stable material to me.
> 

Having a card unknowingly operate at half speed would seem pretty serious to me.

Perhaps the description should say:

IB/hfi1: Insure full Gen3 speed in a Gen4 system

Mike

Jason Gunthorpe Oct. 30, 2019, 6:07 p.m. UTC | #3

On Tue, Oct 29, 2019 at 09:19:34PM +0000, Marciniszyn, Mike wrote:
> > Subject: Re: [PATCH for-rc 1/4] IB/hfi1: Allow for all speeds higher than gen3
> > 
> > On Fri, Oct 25, 2019 at 03:58:24PM -0400, Dennis Dalessandro wrote:
> > > From: James Erwin <[e-mail]>
> > >
> > > The driver avoids the gen3 speed bump when the parent
> > > bus speed isn't identical to gen3, 8.0GT/s.  This is not
> > > compatible with gen4 and newer speeds.
> > >
> > > Fix by relaxing the test to explicitly look for the lower
> > > capability speeds which inherently allows for all future speeds.
> > 
> > This description does not seem like stable material to me.
> > 
> 
> Having a card unknowingly operate at half speed would seem pretty serious to me.

Since gen4 systems are really new this also sounds like a new feature
to me.. You need to be concerned that changing the pci setup doesn't
cause regressions on existing systems too.

> Perhaps the description should say:
> 
> IB/hfi1: Insure full Gen3 speed in a Gen4 system

And maybe explain what the actual user visible impact is here. Sounds
like plugging a card into a gen4 system will not run at gen3 speeds?

Jason

Marciniszyn, Mike Oct. 30, 2019, 8:14 p.m. UTC | #4

> 
> Since gen4 systems are really new this also sounds like a new feature
> to me.. You need to be concerned that changing the pci setup doesn't
> cause regressions on existing systems too.
> 

We have covered this with all of our types of cards and servers (gen3 and gen4).

> > Perhaps the description should say:
> >
> > IB/hfi1: Insure full Gen3 speed in a Gen4 system
> 
> And maybe explain what the actual user visible impact is here. Sounds
> like plugging a card into a gen4 system will not run at gen3 speeds?
> 

Ok.   I will reissue the patch some changes in the commit message.

Mike

Marciniszyn, Mike Nov. 1, 2019, 7:39 p.m. UTC | #5

> 
> Ok.   I will reissue the patch some changes in the commit message.
> 
> Mike

The new patch is https://lore.kernel.org/linux-rdma/[e-mail]/T/#u.

Mike

Patch

diff --git a/drivers/infiniband/hw/hfi1/pcie.c b/drivers/infiniband/hw/hfi1/pcie.c
index 61aa550..61362bd 100644
--- a/drivers/infiniband/hw/hfi1/pcie.c
+++ b/drivers/infiniband/hw/hfi1/pcie.c
@@ -319,7 +319,9 @@  int pcie_speeds(struct hfi1_devdata *dd)
 	/*
 	 * bus->max_bus_speed is set from the bridge's linkcap Max Link Speed
 	 */
-	if (parent && dd->pcidev->bus->max_bus_speed != PCIE_SPEED_8_0GT) {
+	if (parent &&
+	    (dd->pcidev->bus->max_bus_speed == PCIE_SPEED_2_5GT ||
+	     dd->pcidev->bus->max_bus_speed == PCIE_SPEED_5_0GT)) {
 		dd_dev_info(dd, "Parent PCIe bridge does not support Gen3\n");
 		dd->link_gen3_capable = 0;
 	}