| Message ID | 20220430084846.3127041-4-chenhuacai@loongson.cn (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | PCI: Loongson pci improvements and quirks | expand |
On Sat, Apr 30, 2022 at 04:48:43PM +0800, Huacai Chen wrote: > On LS2K/LS7A, some unexisting devices don't return 0xffffffff when > scanning. This is a hardware flaw but we can only avoid it by software > now. s/unexisting/non-existant/ (many occurrences: subject line, commit log, comments below) What happens in other situations that normally cause Unsupported Request or similar errors? For example, memory reads/writes to a device in D3hot should cause an Unsupported Request error. I'm wondering whether other error handling assumptions might be broken on LS2K/LS7A. > Signed-off-by: Huacai Chen <chenhuacai@loongson.cn> > --- > drivers/pci/controller/pci-loongson.c | 11 +++++++++-- > 1 file changed, 9 insertions(+), 2 deletions(-) > > diff --git a/drivers/pci/controller/pci-loongson.c b/drivers/pci/controller/pci-loongson.c > index adbfa4a2330f..48316daa1f23 100644 > --- a/drivers/pci/controller/pci-loongson.c > +++ b/drivers/pci/controller/pci-loongson.c > @@ -138,6 +138,8 @@ static void __iomem *pci_loongson_map_bus(struct pci_bus *bus, unsigned int devf > int where) > { > unsigned char busnum = bus->number; > + unsigned int device = PCI_SLOT(devfn); > + unsigned int function = PCI_FUNC(devfn); > struct loongson_pci *priv = pci_bus_to_loongson_pci(bus); > > if (pci_is_root_bus(bus)) > @@ -147,8 +149,13 @@ static void __iomem *pci_loongson_map_bus(struct pci_bus *bus, unsigned int devf > * Do not read more than one device on the bus other than > * the host bus. For our hardware the root bus is always bus 0. > */ > - if (priv->data->flags & FLAG_DEV_FIX && > - !pci_is_root_bus(bus) && PCI_SLOT(devfn) > 0) > + if ((priv->data->flags & FLAG_DEV_FIX) && bus->self) { > + if (!pci_is_root_bus(bus) && (device > 0)) > + return NULL; > + } > + > + /* Don't access unexisting devices */ > + if (pci_is_root_bus(bus) && (device >= 9 && device <= 20 && function > 0)) Yuck. This is pretty nasty magic. If this is something that might be fixed in future versions of the hardware, maybe you should factor this out into a function pointer in loongson_pci_data or something. > return NULL; > > /* CFG0 can only access standard space */ > -- > 2.27.0 >
Hi, Bjorn, On Wed, Jun 1, 2022 at 7:14 AM Bjorn Helgaas <helgaas@kernel.org> wrote: > > On Sat, Apr 30, 2022 at 04:48:43PM +0800, Huacai Chen wrote: > > On LS2K/LS7A, some unexisting devices don't return 0xffffffff when > > scanning. This is a hardware flaw but we can only avoid it by software > > now. > > s/unexisting/non-existant/ (many occurrences: subject line, commit > log, comments below) OK, thanks. > > What happens in other situations that normally cause Unsupported > Request or similar errors? For example, memory reads/writes to a > device in D3hot should cause an Unsupported Request error. I'm > wondering whether other error handling assumptions might be broken > on LS2K/LS7A. Hardware engineers told me that the problem is due to pin multiplexing, under some configurations, a PCI device is unusable but the read request doesn't return 0xffffffff. > > > Signed-off-by: Huacai Chen <chenhuacai@loongson.cn> > > --- > > drivers/pci/controller/pci-loongson.c | 11 +++++++++-- > > 1 file changed, 9 insertions(+), 2 deletions(-) > > > > diff --git a/drivers/pci/controller/pci-loongson.c b/drivers/pci/controller/pci-loongson.c > > index adbfa4a2330f..48316daa1f23 100644 > > --- a/drivers/pci/controller/pci-loongson.c > > +++ b/drivers/pci/controller/pci-loongson.c > > @@ -138,6 +138,8 @@ static void __iomem *pci_loongson_map_bus(struct pci_bus *bus, unsigned int devf > > int where) > > { > > unsigned char busnum = bus->number; > > + unsigned int device = PCI_SLOT(devfn); > > + unsigned int function = PCI_FUNC(devfn); > > struct loongson_pci *priv = pci_bus_to_loongson_pci(bus); > > > > if (pci_is_root_bus(bus)) > > @@ -147,8 +149,13 @@ static void __iomem *pci_loongson_map_bus(struct pci_bus *bus, unsigned int devf > > * Do not read more than one device on the bus other than > > * the host bus. For our hardware the root bus is always bus 0. > > */ > > - if (priv->data->flags & FLAG_DEV_FIX && > > - !pci_is_root_bus(bus) && PCI_SLOT(devfn) > 0) > > + if ((priv->data->flags & FLAG_DEV_FIX) && bus->self) { > > + if (!pci_is_root_bus(bus) && (device > 0)) > > + return NULL; > > + } > > + > > + /* Don't access unexisting devices */ > > + if (pci_is_root_bus(bus) && (device >= 9 && device <= 20 && function > 0)) > > Yuck. This is pretty nasty magic. If this is something that might be > fixed in future versions of the hardware, maybe you should factor this > out into a function pointer in loongson_pci_data or something. OK, seems providing a pdev_is_existant() is better. Huacai > > > return NULL; > > > > /* CFG0 can only access standard space */ > > -- > > 2.27.0 > >
On Thu, Jun 02, 2022 at 12:28:40PM +0800, Huacai Chen wrote: > Hi, Bjorn, > > On Wed, Jun 1, 2022 at 7:14 AM Bjorn Helgaas <helgaas@kernel.org> wrote: > > On Sat, Apr 30, 2022 at 04:48:43PM +0800, Huacai Chen wrote: > > > On LS2K/LS7A, some unexisting devices don't return 0xffffffff when > > > scanning. This is a hardware flaw but we can only avoid it by software > > > now. > > > > What happens in other situations that normally cause Unsupported > > Request or similar errors? For example, memory reads/writes to a > > device in D3hot should cause an Unsupported Request error. I'm > > wondering whether other error handling assumptions might be broken > > on LS2K/LS7A. > > Hardware engineers told me that the problem is due to pin > multiplexing, under some configurations, a PCI device is unusable but > the read request doesn't return 0xffffffff. What happens if a driver does a mem read to a device that's in D3hot? > > > Signed-off-by: Huacai Chen <chenhuacai@loongson.cn> > > > --- > > > drivers/pci/controller/pci-loongson.c | 11 +++++++++-- > > > 1 file changed, 9 insertions(+), 2 deletions(-) > > > > > > diff --git a/drivers/pci/controller/pci-loongson.c b/drivers/pci/controller/pci-loongson.c > > > index adbfa4a2330f..48316daa1f23 100644 > > > --- a/drivers/pci/controller/pci-loongson.c > > > +++ b/drivers/pci/controller/pci-loongson.c > > > @@ -138,6 +138,8 @@ static void __iomem *pci_loongson_map_bus(struct pci_bus *bus, unsigned int devf > > > int where) > > > { > > > unsigned char busnum = bus->number; > > > + unsigned int device = PCI_SLOT(devfn); > > > + unsigned int function = PCI_FUNC(devfn); > > > struct loongson_pci *priv = pci_bus_to_loongson_pci(bus); > > > > > > if (pci_is_root_bus(bus)) > > > @@ -147,8 +149,13 @@ static void __iomem *pci_loongson_map_bus(struct pci_bus *bus, unsigned int devf > > > * Do not read more than one device on the bus other than > > > * the host bus. For our hardware the root bus is always bus 0. > > > */ > > > - if (priv->data->flags & FLAG_DEV_FIX && > > > - !pci_is_root_bus(bus) && PCI_SLOT(devfn) > 0) > > > + if ((priv->data->flags & FLAG_DEV_FIX) && bus->self) { > > > + if (!pci_is_root_bus(bus) && (device > 0)) > > > + return NULL; > > > + } > > > + > > > + /* Don't access unexisting devices */ > > > + if (pci_is_root_bus(bus) && (device >= 9 && device <= 20 && function > 0)) > > > > Yuck. This is pretty nasty magic. If this is something that might be > > fixed in future versions of the hardware, maybe you should factor this > > out into a function pointer in loongson_pci_data or something. > OK, seems providing a pdev_is_existant() is better. > > Huacai > > > > > return NULL; > > > > > > /* CFG0 can only access standard space */ > > > -- > > > 2.27.0 > > >
在 2022/6/2 17:23, Bjorn Helgaas 写道: > On Thu, Jun 02, 2022 at 12:28:40PM +0800, Huacai Chen wrote: >> Hi, Bjorn, >> >> On Wed, Jun 1, 2022 at 7:14 AM Bjorn Helgaas <helgaas@kernel.org> wrote: >>> On Sat, Apr 30, 2022 at 04:48:43PM +0800, Huacai Chen wrote: >>>> On LS2K/LS7A, some unexisting devices don't return 0xffffffff when >>>> scanning. This is a hardware flaw but we can only avoid it by software >>>> now. >>> What happens in other situations that normally cause Unsupported >>> Request or similar errors? For example, memory reads/writes to a >>> device in D3hot should cause an Unsupported Request error. I'm >>> wondering whether other error handling assumptions might be broken >>> on LS2K/LS7A. >> Hardware engineers told me that the problem is due to pin >> multiplexing, under some configurations, a PCI device is unusable but >> the read request doesn't return 0xffffffff. > What happens if a driver does a mem read to a device that's in D3hot? Just did experiment on my hardware (which is a MIPS-era LS3A4000 with LS7A). It turns out that the hardware simply returns 0xffffffff for all reads and ignore writes. The PCIe controller of LS7A is a customized dwc and it should response with a SLVERR transaction on LS7A's internalAXI bus. However the bus we used to connect LS7A with CPU is incapable to pass this SLVERR information to CPU and thus it just provides a false result. All LS7A's on-chip devices connected on PCI bus don't expose any PCI power management capability. Though they have power management registers elsewhere and generally we won't touch them when kernel is running. Thanks - Jiaxun
diff --git a/drivers/pci/controller/pci-loongson.c b/drivers/pci/controller/pci-loongson.c index adbfa4a2330f..48316daa1f23 100644 --- a/drivers/pci/controller/pci-loongson.c +++ b/drivers/pci/controller/pci-loongson.c @@ -138,6 +138,8 @@ static void __iomem *pci_loongson_map_bus(struct pci_bus *bus, unsigned int devf int where) { unsigned char busnum = bus->number; + unsigned int device = PCI_SLOT(devfn); + unsigned int function = PCI_FUNC(devfn); struct loongson_pci *priv = pci_bus_to_loongson_pci(bus); if (pci_is_root_bus(bus)) @@ -147,8 +149,13 @@ static void __iomem *pci_loongson_map_bus(struct pci_bus *bus, unsigned int devf * Do not read more than one device on the bus other than * the host bus. For our hardware the root bus is always bus 0. */ - if (priv->data->flags & FLAG_DEV_FIX && - !pci_is_root_bus(bus) && PCI_SLOT(devfn) > 0) + if ((priv->data->flags & FLAG_DEV_FIX) && bus->self) { + if (!pci_is_root_bus(bus) && (device > 0)) + return NULL; + } + + /* Don't access unexisting devices */ + if (pci_is_root_bus(bus) && (device >= 9 && device <= 20 && function > 0)) return NULL; /* CFG0 can only access standard space */
On LS2K/LS7A, some unexisting devices don't return 0xffffffff when scanning. This is a hardware flaw but we can only avoid it by software now. Signed-off-by: Huacai Chen <chenhuacai@loongson.cn> --- drivers/pci/controller/pci-loongson.c | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-)