Re: [PATCH] pci: fix incorrect value returned from pcie_get_speed_cap

From: Bjorn Helgaas <helgaas-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
To: Mikulas Patocka <mpatocka-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
Cc: Tal Gilboa <talgi-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>,
	linux-pci-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	Alex Deucher
	<alexdeucher-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>,
	amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org,
	Tariq Toukan <tariqt-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
Subject: Re: [PATCH] pci: fix incorrect value returned from pcie_get_speed_cap
Date: Mon, 26 Nov 2018 16:40:52 -0600	[thread overview]
Message-ID: <20181126224051.GB212532@google.com> (raw)
In-Reply-To: <20181120004704.GA191199-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org>

On Mon, Nov 19, 2018 at 06:47:04PM -0600, Bjorn Helgaas wrote:
> On Tue, Oct 30, 2018 at 12:36:08PM -0400, Mikulas Patocka wrote:
> > The macros PCI_EXP_LNKCAP_SLS_*GB are values, not bit masks. We must mask
> > the register and compare it against them.
> > 
> > This patch fixes errors "amdgpu: [powerplay] failed to send message 261
> > ret is 0" errors when PCIe-v3 card is plugged into PCIe-v1 slot, because
> > the slot is being incorrectly reported as PCIe-v3 capable.
> > 
> > Signed-off-by: Mikulas Patocka <mpatocka@redhat.com>
> > Fixes: 6cf57be0f78e ("PCI: Add pcie_get_speed_cap() to find max supported link speed")
> > Cc: stable@vger.kernel.org	# v4.17+
> > 
> > ---
> >  drivers/pci/pci.c |    8 ++++----
> >  1 file changed, 4 insertions(+), 4 deletions(-)
> > 
> > Index: linux-4.19/drivers/pci/pci.c
> > ===================================================================
> > --- linux-4.19.orig/drivers/pci/pci.c	2018-10-30 16:58:58.000000000 +0100
> > +++ linux-4.19/drivers/pci/pci.c	2018-10-30 16:58:58.000000000 +0100
> > @@ -5492,13 +5492,13 @@ enum pci_bus_speed pcie_get_speed_cap(st
> >  
> >  	pcie_capability_read_dword(dev, PCI_EXP_LNKCAP, &lnkcap);
> >  	if (lnkcap) {
> > -		if (lnkcap & PCI_EXP_LNKCAP_SLS_16_0GB)
> > +		if ((lnkcap & PCI_EXP_LNKCAP_SLS) == PCI_EXP_LNKCAP_SLS_16_0GB)
> >  			return PCIE_SPEED_16_0GT;
> > -		else if (lnkcap & PCI_EXP_LNKCAP_SLS_8_0GB)
> > +		else if ((lnkcap & PCI_EXP_LNKCAP_SLS) == PCI_EXP_LNKCAP_SLS_8_0GB)
> >  			return PCIE_SPEED_8_0GT;
> > -		else if (lnkcap & PCI_EXP_LNKCAP_SLS_5_0GB)
> > +		else if ((lnkcap & PCI_EXP_LNKCAP_SLS) ==PCI_EXP_LNKCAP_SLS_5_0GB)
> >  			return PCIE_SPEED_5_0GT;
> > -		else if (lnkcap & PCI_EXP_LNKCAP_SLS_2_5GB)
> > +		else if ((lnkcap & PCI_EXP_LNKCAP_SLS) == PCI_EXP_LNKCAP_SLS_2_5GB)
> >  			return PCIE_SPEED_2_5GT;
> >  	}

> We also need similar fixes in pci_set_bus_speed(), pcie_speeds()
> (hfi1), cobalt_pcie_status_show(), hba_ioctl_callback(),
> qla24xx_pci_info_str(), and maybe a couple other places.

Does anybody want to volunteer to fix the places above as well?  I
found them by grepping for PCI_EXP_LNKCAP, and they're all broken in
ways similar to pcie_get_speed_cap().  Possibly some of these places
could use pcie_get_speed_cap() directly.

Bjorn
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx