* Re: [PATCH] pci: disable nv_msi_ht_cap_quirk_leaf quirk on non-x86 hw [not found] ` <CABr+WT=ANAs4DgvbfcgQ2wnX5yhT9HTgLJh1LE8Q09fyCa0mBg@mail.gmail.com> @ 2019-04-15 4:40 ` Manikanta Maddireddy 0 siblings, 0 replies; 9+ messages in thread From: Manikanta Maddireddy @ 2019-04-15 4:40 UTC (permalink / raw) To: Nicolas Chauvet, Thierry Reding, Jonathan Hunter, Bjorn Helgaas, Lorenzo Pieralisi Cc: linux-tegra, linux-pci To: Bjorn and Lorenzo for review, CC: linux-pci@vger.kernel.org Reviewed-by: Manikanta Maddireddy <mmaddireddy@nvidia.com> Thierry, We have similar change in down stream kernel, please review this patch and provide Ack. Thanks, Manikanta On 29-Mar-19 3:15 PM, Nicolas Chauvet wrote: > Le ven. 14 sept. 2018 à 14:59, Nicolas Chauvet <kwizart@gmail.com> a écrit : >> This patch disable the use of nv_msi_ht_cap_quirk_leaf quirk on non-x86 >> NVIDIA devices such as Tegra >> >> This fixes the following output: >> "pci 0000:00:01.0: nv_msi_ht_cap_quirk didn't locate host bridge" >> as experienced on a Trimslice device with pci host bridge enabled >> >> v2: use __maybe_unused to avoid a warning on nv_msi_ht_cap_quirk_leaf > Any review for this change ? > Thx in advances. > ^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH] PCI: disable nv_msi_ht_cap_quirk_leaf quirk on arm/arm64 [not found] <20180730094213.11973-1-kwizart@gmail.com> [not found] ` <CABr+WT=ANAs4DgvbfcgQ2wnX5yhT9HTgLJh1LE8Q09fyCa0mBg@mail.gmail.com> @ 2019-04-15 9:25 ` Nicolas Chauvet 2019-04-15 13:48 ` Bjorn Helgaas 1 sibling, 1 reply; 9+ messages in thread From: Nicolas Chauvet @ 2019-04-15 9:25 UTC (permalink / raw) To: Thierry Reding, Jonathan Hunter, Manikanta Maddireddy, Bjorn Helgaas, Lorenzo Pieralisi Cc: linux-tegra, linux-pci, Nicolas Chauvet This patch disable the use of nv_msi_ht_cap_quirk_leaf quirk on arm and arm64 NVIDIA devices such as Tegra This fixes the following output: "pci 0000:00:01.0: nv_msi_ht_cap_quirk didn't locate host bridge" as experienced on a Trimslice device with PCI host bridge enabled v3: exclude the quirk for arm and arm64 instead of only for x86 v2: use __maybe_unused to avoid a warning on nv_msi_ht_cap_quirk_leaf Signed-off-by: Nicolas Chauvet <kwizart@gmail.com> Reviewed-by: Manikanta Maddireddy <mmaddireddy@nvidia.com> Acked-by: Thierry Reding <treding@nvidia.com> --- drivers/pci/quirks.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c index a59ad09ce911..1ac65f09d7ee 100644 --- a/drivers/pci/quirks.c +++ b/drivers/pci/quirks.c @@ -2811,12 +2811,15 @@ static void nv_msi_ht_cap_quirk_all(struct pci_dev *dev) DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_AL, PCI_ANY_ID, nv_msi_ht_cap_quirk_all); DECLARE_PCI_FIXUP_RESUME_EARLY(PCI_VENDOR_ID_AL, PCI_ANY_ID, nv_msi_ht_cap_quirk_all); -static void nv_msi_ht_cap_quirk_leaf(struct pci_dev *dev) +static void __maybe_unused nv_msi_ht_cap_quirk_leaf(struct pci_dev *dev) { return __nv_msi_ht_cap_quirk(dev, 0); } +/* HyperTransport is not relevant on theses arches */ +#if !IS_ENABLED(CONFIG_ARM) && !IS_ENABLED(CONFIG_ARM64) DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_NVIDIA, PCI_ANY_ID, nv_msi_ht_cap_quirk_leaf); DECLARE_PCI_FIXUP_RESUME_EARLY(PCI_VENDOR_ID_NVIDIA, PCI_ANY_ID, nv_msi_ht_cap_quirk_leaf); +#endif static void quirk_msi_intx_disable_bug(struct pci_dev *dev) { -- 2.17.2 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH] PCI: disable nv_msi_ht_cap_quirk_leaf quirk on arm/arm64 2019-04-15 9:25 ` [PATCH] PCI: disable nv_msi_ht_cap_quirk_leaf quirk on arm/arm64 Nicolas Chauvet @ 2019-04-15 13:48 ` Bjorn Helgaas 2019-04-16 9:52 ` Manikanta Maddireddy 2019-04-16 11:35 ` Thierry Reding 0 siblings, 2 replies; 9+ messages in thread From: Bjorn Helgaas @ 2019-04-15 13:48 UTC (permalink / raw) To: Nicolas Chauvet Cc: Thierry Reding, Jonathan Hunter, Manikanta Maddireddy, Lorenzo Pieralisi, linux-tegra, linux-pci On Mon, Apr 15, 2019 at 11:25:37AM +0200, Nicolas Chauvet wrote: > This patch disable the use of nv_msi_ht_cap_quirk_leaf quirk on arm and > arm64 NVIDIA devices such as Tegra > > This fixes the following output: > "pci 0000:00:01.0: nv_msi_ht_cap_quirk didn't locate host bridge" > as experienced on a Trimslice device with PCI host bridge enabled > > v3: exclude the quirk for arm and arm64 instead of only for x86 > > v2: use __maybe_unused to avoid a warning on nv_msi_ht_cap_quirk_leaf > > Signed-off-by: Nicolas Chauvet <kwizart@gmail.com> > Reviewed-by: Manikanta Maddireddy <mmaddireddy@nvidia.com> > Acked-by: Thierry Reding <treding@nvidia.com> > --- > drivers/pci/quirks.c | 5 ++++- > 1 file changed, 4 insertions(+), 1 deletion(-) > > diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c > index a59ad09ce911..1ac65f09d7ee 100644 > --- a/drivers/pci/quirks.c > +++ b/drivers/pci/quirks.c > @@ -2811,12 +2811,15 @@ static void nv_msi_ht_cap_quirk_all(struct pci_dev *dev) > DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_AL, PCI_ANY_ID, nv_msi_ht_cap_quirk_all); > DECLARE_PCI_FIXUP_RESUME_EARLY(PCI_VENDOR_ID_AL, PCI_ANY_ID, nv_msi_ht_cap_quirk_all); > > -static void nv_msi_ht_cap_quirk_leaf(struct pci_dev *dev) > +static void __maybe_unused nv_msi_ht_cap_quirk_leaf(struct pci_dev *dev) > { > return __nv_msi_ht_cap_quirk(dev, 0); > } > +/* HyperTransport is not relevant on theses arches */ > +#if !IS_ENABLED(CONFIG_ARM) && !IS_ENABLED(CONFIG_ARM64) I don't see any of the previous versions or discussion on the list, so I can't tell what led to this #ifdef, but I'd prefer to avoid #ifdefs like this because they're checking for arch at the wrong level. If you're seeing that warning message, apparently your system contains an Nvidia device that has an HT_CAPTYPE_MSI_MAPPING capability, so HyperTransport is at least in the picture, even on arm/arm64. The quirk assumes a host bridge at 00:00.0, which is really a device-specific assumption, or maybe something specific to HyperTransport. It's certainly not required by the PCI specs, which don't require a host bridge to be visible as a PCI device at all. I'd be tempted to just change that pci_warn() to a pci_info() or maybe even remove it altogether. It would be nice to have the complete "lspci -vv" and dmesg log archived for future reference, since this quirk has such a broad reach (matching PCI_ANY_ID) and it has a long and ugly history already. > DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_NVIDIA, PCI_ANY_ID, nv_msi_ht_cap_quirk_leaf); > DECLARE_PCI_FIXUP_RESUME_EARLY(PCI_VENDOR_ID_NVIDIA, PCI_ANY_ID, nv_msi_ht_cap_quirk_leaf); > +#endif > > static void quirk_msi_intx_disable_bug(struct pci_dev *dev) > { > -- > 2.17.2 > ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] PCI: disable nv_msi_ht_cap_quirk_leaf quirk on arm/arm64 2019-04-15 13:48 ` Bjorn Helgaas @ 2019-04-16 9:52 ` Manikanta Maddireddy 2019-04-16 11:35 ` Thierry Reding 1 sibling, 0 replies; 9+ messages in thread From: Manikanta Maddireddy @ 2019-04-16 9:52 UTC (permalink / raw) To: Bjorn Helgaas, Nicolas Chauvet Cc: Thierry Reding, Jonathan Hunter, Lorenzo Pieralisi, linux-tegra, linux-pci On 15-Apr-19 7:18 PM, Bjorn Helgaas wrote: > On Mon, Apr 15, 2019 at 11:25:37AM +0200, Nicolas Chauvet wrote: >> This patch disable the use of nv_msi_ht_cap_quirk_leaf quirk on arm and >> arm64 NVIDIA devices such as Tegra >> >> This fixes the following output: >> "pci 0000:00:01.0: nv_msi_ht_cap_quirk didn't locate host bridge" >> as experienced on a Trimslice device with PCI host bridge enabled >> >> v3: exclude the quirk for arm and arm64 instead of only for x86 >> >> v2: use __maybe_unused to avoid a warning on nv_msi_ht_cap_quirk_leaf >> >> Signed-off-by: Nicolas Chauvet <kwizart@gmail.com> >> Reviewed-by: Manikanta Maddireddy <mmaddireddy@nvidia.com> >> Acked-by: Thierry Reding <treding@nvidia.com> >> --- >> drivers/pci/quirks.c | 5 ++++- >> 1 file changed, 4 insertions(+), 1 deletion(-) >> >> diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c >> index a59ad09ce911..1ac65f09d7ee 100644 >> --- a/drivers/pci/quirks.c >> +++ b/drivers/pci/quirks.c >> @@ -2811,12 +2811,15 @@ static void nv_msi_ht_cap_quirk_all(struct pci_dev *dev) >> DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_AL, PCI_ANY_ID, nv_msi_ht_cap_quirk_all); >> DECLARE_PCI_FIXUP_RESUME_EARLY(PCI_VENDOR_ID_AL, PCI_ANY_ID, nv_msi_ht_cap_quirk_all); >> >> -static void nv_msi_ht_cap_quirk_leaf(struct pci_dev *dev) >> +static void __maybe_unused nv_msi_ht_cap_quirk_leaf(struct pci_dev *dev) >> { >> return __nv_msi_ht_cap_quirk(dev, 0); >> } >> +/* HyperTransport is not relevant on theses arches */ >> +#if !IS_ENABLED(CONFIG_ARM) && !IS_ENABLED(CONFIG_ARM64) > I don't see any of the previous versions or discussion on the list, so > I can't tell what led to this #ifdef, but I'd prefer to avoid #ifdefs > like this because they're checking for arch at the wrong level. > > If you're seeing that warning message, apparently your system contains > an Nvidia device that has an HT_CAPTYPE_MSI_MAPPING capability, so > HyperTransport is at least in the picture, even on arm/arm64. > > The quirk assumes a host bridge at 00:00.0, which is really a > device-specific assumption, or maybe something specific to > HyperTransport. It's certainly not required by the PCI specs, which > don't require a host bridge to be visible as a PCI device at all. > > I'd be tempted to just change that pci_warn() to a pci_info() or maybe > even remove it altogether. It would be nice to have the complete > "lspci -vv" and dmesg log archived for future reference, since this > quirk has such a broad reach (matching PCI_ANY_ID) and it has a long > and ugly history already. Nvidia HW team confirmed that HyperTransport is supported in AMD CPU + Nvidia NForce motherboard and not supported in Tegra. We are seeing the warn log because Tegra host bridge enumerates from device number 1, as shown in below lspci output taken from Tegra210. HT_CAPTYPE_MSI_MAPPING capability is present in Tegra even though it doesn't support and ARCH check is not best approach here. So changing pci_warn() to pci_info() should be fine. # lspci -D 0000:00:01.0 Class 0604: Device 10de:0fae (rev a1) 0000:00:02.0 Class 0604: Device 10de:0faf (rev a1) 0000:01:00.0 Class 0200: Device 10ec:8168 (rev 0c) 0000:02:00.0 Class 0280: Device 14e4:43ec (rev 02) # lspci -t -[0000:00]-+-01.0-[01]----00.0 \-02.0-[02]----00.0 dmesg log: tegra-pcie 1003000.pcie: 4x1, 1x1 configuration tegra-pcie 1003000.pcie: probing port 0, using 4 lanes tegra-pcie 1003000.pcie: probing port 1, using 1 lanes tegra-pcie 1003000.pcie: PCI host bridge to bus 0000:00 pci_bus 0000:00: root bus resource [io 0x0000-0xffff] pci_bus 0000:00: root bus resource [mem 0x13000000-0x1fffffff] pci_bus 0000:00: root bus resource [mem 0x20000000-0x3fffffff pref] pci_bus 0000:00: root bus resource [bus 00-ff] pci_bus 0000:00: 2-byte config write to 0000:00:01.0 offset 0x4 may corrupt adjacent RW1C bits pci_bus 0000:00: 2-byte config write to 0000:00:01.0 offset 0x4 may corrupt adjacent RW1C bits pci 0000:00:01.0: enabling Extended Tags pci_bus 0000:00: 2-byte config write to 0000:00:01.0 offset 0x88 may corrupt adjacent RW1C bits pci_bus 0000:00: 2-byte config write to 0000:00:01.0 offset 0x3e may corrupt adjacent RW1C bits pci_bus 0000:00: 2-byte config write to 0000:00:01.0 offset 0x52 may corrupt adjacent RW1C bits pci_bus 0000:00: 2-byte config write to 0000:00:01.0 offset 0x4c may corrupt adjacent RW1C bits pci_bus 0000:00: 2-byte config write to 0000:00:02.0 offset 0x4 may corrupt adjacent RW1C bits pci_bus 0000:00: 2-byte config write to 0000:00:02.0 offset 0x4 may corrupt adjacent RW1C bits pci 0000:00:02.0: enabling Extended Tags pci_bus 0000:00: 2-byte config write to 0000:00:02.0 offset 0x88 may corrupt adjacent RW1C bits pci_bus 0000:00: 2-byte config write to 0000:00:02.0 offset 0x3e may corrupt adjacent RW1C bits pci 0000:00:01.0: bridge configuration invalid ([bus 00-00]), reconfiguring pci 0000:00:02.0: bridge configuration invalid ([bus 00-00]), reconfiguring pci 0000:00:02.0: BAR 14: assigned [mem 0x13000000-0x135fffff] pci 0000:00:01.0: BAR 14: assigned [mem 0x13600000-0x136fffff] pci 0000:00:01.0: BAR 15: assigned [mem 0x20000000-0x200fffff 64bit pref] pci 0000:00:01.0: BAR 13: assigned [io 0x1000-0x1fff] pci 0000:01:00.0: BAR 4: assigned [mem 0x20000000-0x20003fff 64bit pref] pci 0000:01:00.0: BAR 2: assigned [mem 0x13600000-0x13600fff 64bit] pci 0000:01:00.0: BAR 0: assigned [io 0x1000-0x10ff] pci 0000:00:01.0: PCI bridge to [bus 01] pci 0000:00:01.0: bridge window [io 0x1000-0x1fff] pci 0000:00:01.0: bridge window [mem 0x13600000-0x136fffff] pci 0000:00:01.0: bridge window [mem 0x20000000-0x200fffff 64bit pref] pci 0000:02:00.0: BAR 2: assigned [mem 0x13000000-0x133fffff 64bit] pci 0000:02:00.0: BAR 0: assigned [mem 0x13400000-0x13407fff 64bit] pci 0000:00:02.0: PCI bridge to [bus 02] pci 0000:00:02.0: bridge window [mem 0x13000000-0x135fffff] pci 0000:00:01.0: nv_msi_ht_cap_quirk didn't locate host bridge pcieport 0000:00:01.0: enabling device (0000 -> 0003) pci 0000:00:02.0: nv_msi_ht_cap_quirk didn't locate host bridge pcieport 0000:00:02.0: enabling device (0000 -> 0002) # lspci -vv 00:01.0 Class 0604: Device 10de:0fae (rev a1) Control: I/O+ Mem+ BusMaster+ SpecCycle- MemWINV- VGASnoop- ParErr- Stepping- SERR- FastB2B- DisINTx- Status: Cap+ 66MHz- UDF- FastB2B- ParErr- DEVSEL=fast >TAbort- <TAbort- <MAbort- >SERR- <PERR- INTx- Latency: 0 Interrupt: pin A routed to IRQ 10 Bus: primary=00, secondary=01, subordinate=01, sec-latency=0 I/O behind bridge: 00001000-00001fff [size=4K] Memory behind bridge: 13600000-136fffff [size=1M] Prefetchable memory behind bridge: 0000000020000000-00000000200fffff [size=1M] Secondary status: 66MHz- FastB2B- ParErr- DEVSEL=fast >TAbort- <TAbort- <MAbort- <SERR- <PERR- BridgeCtl: Parity- SERR+ NoISA- VGA- MAbort- >Reset- FastB2B- PriDiscTmr- SecDiscTmr- DiscTmrStat- DiscTmrSERREn- Capabilities: [40] Subsystem: Device 10de:0000 Capabilities: [48] Power Management version 3 Flags: PMEClk- DSI- D1- D2- AuxCurrent=0mA PME(D0+,D1+,D2+,D3hot+,D3cold+) Status: D0 NoSoftRst- PME-Enable- DSel=0 DScale=0 PME- Capabilities: [50] MSI: Enable- Count=1/2 Maskable- 64bit+ Address: 0000000179eea000 Data: 0000 Capabilities: [60] HyperTransport: MSI Mapping Enable- Fixed- Mapping Address Base: 00000000fee00000 Capabilities: [80] Express (v2) Root Port (Slot+), MSI 00 DevCap: MaxPayload 128 bytes, PhantFunc 0 ExtTag+ RBE+ DevCtl: Report errors: Correctable- Non-Fatal- Fatal- Unsupported- RlxdOrd+ ExtTag+ PhantFunc- AuxPwr- NoSnoop+ MaxPayload 128 bytes, MaxReadReq 512 bytes DevSta: CorrErr- UncorrErr- FatalErr- UnsuppReq- AuxPwr- TransPend- LnkCap: Port #0, Speed 5GT/s, Width x4, ASPM L0s, Exit Latency L0s <512ns ClockPM- Surprise- LLActRep+ BwNot+ ASPMOptComp- LnkCtl: ASPM Disabled; RCB 64 bytes Disabled- CommClk+ ExtSynch- ClockPM- AutWidDis- BWInt+ AutBWInt- LnkSta: Speed 2.5GT/s, Width x1, TrErr- Train- SlotClk+ DLActive+ BWMgmt- ABWMgmt- SltCap: AttnBtn- PwrCtrl- MRL- AttnInd- PwrInd- HotPlug- Surprise- Slot #0, PowerLimit 0.000W; Interlock- NoCompl- SltCtl: Enable: AttnBtn- PwrFlt- MRL- PresDet- CmdCplt- HPIrq- LinkChg- Control: AttnInd Off, PwrInd On, Power- Interlock- SltSta: Status: AttnBtn- PowerFlt- MRL- CmdCplt- PresDet+ Interlock- Changed: MRL- PresDet+ LinkState+ RootCtl: ErrCorrectable- ErrNon-Fatal- ErrFatal- PMEIntEna- CRSVisible- RootCap: CRSVisible- RootSta: PME ReqID 0000, PMEStatus- PMEPending- DevCap2: Completion Timeout: Range AB, TimeoutDis+, LTR+, OBFF Not Supported ARIFwd- AtomicOpsCap: Routing- 32bit- 64bit- 128bitCAS- DevCtl2: Completion Timeout: 50us to 50ms, TimeoutDis-, LTR+, OBFF Disabled ARIFwd- AtomicOpsCtl: ReqEn- EgressBlck- LnkCtl2: Target Link Speed: 5GT/s, EnterCompliance- SpeedDis- Transmit Margin: Normal Operating Range, EnterModifiedCompliance- ComplianceSOS- Compliance De-emphasis: -6dB LnkSta2: Current De-emphasis Level: -3.5dB, EqualizationComplete-, EqualizationPhase1- EqualizationPhase2-, EqualizationPhase3-, LinkEqualizationRequest- Capabilities: [100 v1] Advanced Error Reporting UESta: DLP- SDES- TLP- FCP- CmpltTO- CmpltAbrt- UnxCmplt- RxOF- MalfTLP- ECRC- UnsupReq- ACSViol- UEMsk: DLP- SDES- TLP- FCP- CmpltTO- CmpltAbrt- UnxCmplt- RxOF- MalfTLP- ECRC- UnsupReq- ACSViol- UESvrt: DLP+ SDES- TLP- FCP+ CmpltTO- CmpltAbrt- UnxCmplt- RxOF+ MalfTLP+ ECRC- UnsupReq- ACSViol- CESta: RxErr- BadTLP- BadDLLP- Rollover- Timeout- NonFatalErr- CEMsk: RxErr- BadTLP- BadDLLP- Rollover- Timeout- NonFatalErr+ AERCap: First Error Pointer: 00, ECRCGenCap+ ECRCGenEn- ECRCChkCap+ ECRCChkEn- MultHdrRecCap- MultHdrRecEn- TLPPfxPres- HdrLogCap- HeaderLog: 00000000 00000000 00000000 00000000 RootCmd: CERptEn- NFERptEn- FERptEn- RootSta: CERcvd- MultCERcvd- UERcvd- MultUERcvd- FirstFatal- NonFatalMsg- FatalMsg- IntMsg 0 ErrorSrc: ERR_COR: 0000 ERR_FATAL/NONFATAL: 0000 Capabilities: [140 v1] L1 PM Substates L1SubCap: PCI-PM_L1.2+ PCI-PM_L1.1+ ASPM_L1.2+ ASPM_L1.1+ L1_PM_Substates+ PortCommonModeRestoreTime=255us PortTPowerOnTime=10us L1SubCtl1: PCI-PM_L1.2- PCI-PM_L1.1- ASPM_L1.2- ASPM_L1.1- T_CommonMode=255us LTR1.2_Threshold=0ns L1SubCtl2: T_PwrOn=10us Kernel driver in use: pcieport 00:02.0 Class 0604: Device 10de:0faf (rev a1) Control: I/O- Mem+ BusMaster+ SpecCycle- MemWINV- VGASnoop- ParErr- Stepping- SERR- FastB2B- DisINTx- Status: Cap+ 66MHz- UDF- FastB2B- ParErr- DEVSEL=fast >TAbort- <TAbort- <MAbort- >SERR- <PERR- INTx- Latency: 0 Interrupt: pin A routed to IRQ 10 Bus: primary=00, secondary=02, subordinate=02, sec-latency=0 I/O behind bridge: None Memory behind bridge: 13000000-135fffff [size=6M] Prefetchable memory behind bridge: None Secondary status: 66MHz- FastB2B- ParErr- DEVSEL=fast >TAbort- <TAbort- <MAbort- <SERR- <PERR- BridgeCtl: Parity- SERR+ NoISA- VGA- MAbort- >Reset- FastB2B- PriDiscTmr- SecDiscTmr- DiscTmrStat- DiscTmrSERREn- Capabilities: [40] Subsystem: Device 10de:0000 Capabilities: [48] Power Management version 3 Flags: PMEClk- DSI- D1- D2- AuxCurrent=0mA PME(D0+,D1+,D2+,D3hot+,D3cold+) Status: D0 NoSoftRst- PME-Enable- DSel=0 DScale=0 PME- Capabilities: [50] MSI: Enable- Count=1/2 Maskable- 64bit+ Address: 0000000179eea000 Data: 0000 Capabilities: [60] HyperTransport: MSI Mapping Enable- Fixed- Mapping Address Base: 00000000fee00000 Capabilities: [80] Express (v2) Root Port (Slot+), MSI 00 DevCap: MaxPayload 128 bytes, PhantFunc 0 ExtTag+ RBE+ DevCtl: Report errors: Correctable- Non-Fatal- Fatal- Unsupported- RlxdOrd+ ExtTag+ PhantFunc- AuxPwr- NoSnoop+ MaxPayload 128 bytes, MaxReadReq 512 bytes DevSta: CorrErr- UncorrErr- FatalErr- UnsuppReq- AuxPwr- TransPend- LnkCap: Port #1, Speed 5GT/s, Width x1, ASPM L0s, Exit Latency L0s <512ns ClockPM- Surprise- LLActRep+ BwNot+ ASPMOptComp- LnkCtl: ASPM Disabled; RCB 64 bytes Disabled- CommClk+ ExtSynch- ClockPM- AutWidDis- BWInt+ AutBWInt- LnkSta: Speed 2.5GT/s, Width x1, TrErr- Train- SlotClk+ DLActive+ BWMgmt- ABWMgmt- SltCap: AttnBtn- PwrCtrl- MRL- AttnInd- PwrInd- HotPlug- Surprise- Slot #0, PowerLimit 0.000W; Interlock- NoCompl- SltCtl: Enable: AttnBtn- PwrFlt- MRL- PresDet- CmdCplt- HPIrq- LinkChg- Control: AttnInd Off, PwrInd On, Power- Interlock- SltSta: Status: AttnBtn- PowerFlt- MRL- CmdCplt- PresDet+ Interlock- Changed: MRL- PresDet+ LinkState+ RootCtl: ErrCorrectable- ErrNon-Fatal- ErrFatal- PMEIntEna- CRSVisible- RootCap: CRSVisible- RootSta: PME ReqID 0000, PMEStatus- PMEPending- DevCap2: Completion Timeout: Range AB, TimeoutDis+, LTR+, OBFF Not Supported ARIFwd- AtomicOpsCap: Routing- 32bit- 64bit- 128bitCAS- DevCtl2: Completion Timeout: 50us to 50ms, TimeoutDis-, LTR+, OBFF Disabled ARIFwd- AtomicOpsCtl: ReqEn- EgressBlck- LnkCtl2: Target Link Speed: 5GT/s, EnterCompliance- SpeedDis- Transmit Margin: Normal Operating Range, EnterModifiedCompliance- ComplianceSOS- Compliance De-emphasis: -6dB LnkSta2: Current De-emphasis Level: -3.5dB, EqualizationComplete-, EqualizationPhase1- EqualizationPhase2-, EqualizationPhase3-, LinkEqualizationRequest- Capabilities: [100 v1] Advanced Error Reporting UESta: DLP- SDES- TLP- FCP- CmpltTO- CmpltAbrt- UnxCmplt- RxOF- MalfTLP- ECRC- UnsupReq- ACSViol- UEMsk: DLP- SDES- TLP- FCP- CmpltTO- CmpltAbrt- UnxCmplt- RxOF- MalfTLP- ECRC- UnsupReq- ACSViol- UESvrt: DLP+ SDES- TLP- FCP+ CmpltTO- CmpltAbrt- UnxCmplt- RxOF+ MalfTLP+ ECRC- UnsupReq- ACSViol- CESta: RxErr- BadTLP- BadDLLP- Rollover- Timeout- NonFatalErr- CEMsk: RxErr- BadTLP- BadDLLP- Rollover- Timeout- NonFatalErr+ AERCap: First Error Pointer: 00, ECRCGenCap+ ECRCGenEn- E[ 916.955543] pci 0000:01:00.0: invalid short VPD tag 00 at offset 1 CRCChkCap+ ECRCChkEn- MultHdrRecCap- MultHdrRecEn- TLPPfxPres- HdrLogCap- HeaderLog: 00000000 00000000 00000000 00000000 RootCmd: CERptEn- NFERptEn- FERptEn- RootSta: CERcvd- MultCERcvd- UERcvd- MultUERcvd- FirstFatal- NonFatalMsg- FatalMsg- IntMsg 0 ErrorSrc: ERR_COR: 0000 ERR_FATAL/NONFATAL: 0000 Capabilities: [140 v1] L1 PM Substates L1SubCap: PCI-PM_L1.2+ PCI-PM_L1.1+ ASPM_L1.2+ ASPM_L1.1+ L1_PM_Substates+ PortCommonModeRestoreTime=255us PortTPowerOnTime=10us L1SubCtl1: PCI-PM_L1.2- PCI-PM_L1.1- ASPM_L1.2- ASPM_L1.1- T_CommonMode=255us LTR1.2_Threshold=0ns L1SubCtl2: T_PwrOn=10us Kernel driver in use: pcieport 01:00.0 Class 0200: Device 10ec:8168 (rev 0c) Subsystem: Device 10ec:0123 Control: I/O- Mem- BusMaster- SpecCycle- MemWINV- VGASnoop- ParErr- Stepping- SERR- FastB2B- DisINTx- Status: Cap+ 66MHz- UDF- FastB2B- ParErr- DEVSEL=fast >TAbort- <TAbort- <MAbort- >SERR- <PERR- INTx- Interrupt: pin A routed to IRQ 255 Region 0: I/O ports at 1000 [disabled] [size=256] Region 2: Memory at 13600000 (64-bit, non-prefetchable) [disabled] [size=4K] Region 4: Memory at 20000000 (64-bit, prefetchable) [disabled] [size=16K] Capabilities: [40] Power Management version 3 Flags: PMEClk- DSI- D1+ D2+ AuxCurrent=375mA PME(D0+,D1+,D2+,D3hot+,D3cold+) Status: D0 NoSoftRst+ PME-Enable- DSel=0 DScale=0 PME- Capabilities: [50] MSI: Enable- Count=1/1 Maskable- 64bit+ Address: 0000000000000000 Data: 0000 Capabilities: [70] Express (v2) Endpoint, MSI 01 DevCap: MaxPayload 128 bytes, PhantFunc 0, Latency L0s <512ns, L1 <64us ExtTag- AttnBtn- AttnInd- PwrInd- RBE+ FLReset- SlotPowerLimit 0.000W DevCtl: Report errors: Correctable- Non-Fatal- Fatal- Unsupported- RlxdOrd+ ExtTag- PhantFunc- AuxPwr- NoSnoop- MaxPayload 128 bytes, MaxReadReq 512 bytes DevSta: CorrErr- UncorrErr- FatalErr- UnsuppReq- AuxPwr+ TransPend- LnkCap: Port #0, Speed 2.5GT/s, Width x1, ASPM L0s L1, Exit Latency L0s unlimited, L1 <64us ClockPM+ Surprise- LLActRep- BwNot- ASPMOptComp+ LnkCtl: ASPM Disabled; RCB 64 bytes Disabled- CommClk+ ExtSynch- ClockPM- AutWidDis- BWInt- AutBWInt- LnkSta: Speed 2.5GT/s, Width x1, TrErr- Train- SlotClk+ DLActive- BWMgmt- ABWMgmt- DevCap2: Completion Timeout: Range ABCD, TimeoutDis+, LTR+, OBFF Via message/WAKE# AtomicOpsCap: 32bit- 64bit- 128bitCAS- DevCtl2: Completion Timeout: 50us to 50ms, TimeoutDis-, LTR-, OBFF Disabled AtomicOpsCtl: ReqEn- LnkCtl2: Target Link Speed: 2.5GT/s, EnterCompliance- SpeedDis- Transmit Margin: Normal Operating Range, EnterModifiedCompliance- ComplianceSOS- Compliance De-emphasis: -6dB LnkSta2: Current De-emphasis Level: -6dB, EqualizationComplete-, EqualizationPhase1- EqualizationPhase2-, EqualizationPhase3-, LinkEqualizationRequest- Capabilities: [b0] MSI-X: Enable- Count=4 Masked- Vector table: BAR=4 offset=00000000 PBA: BAR=4 offset=00000800 Capabilities: [d0] Vital Product Data pcilib: sysfs_read_vpd: read failed: Input/output error Not readable Capabilities: [100 v1] Advanced Error Reporting UESta: DLP- SDES- TLP- FCP- CmpltTO- CmpltAbrt- UnxCmplt- RxOF- MalfTLP- ECRC- UnsupReq- ACSViol- UEMsk: DLP- SDES- TLP- FCP- CmpltTO- CmpltAbrt- UnxCmplt- RxOF- MalfTLP- ECRC- UnsupReq- ACSViol- UESvrt: DLP+ SDES+ TLP- FCP+ CmpltTO- CmpltAbrt- UnxCmplt- RxOF+ MalfTLP+ ECRC- UnsupReq- ACSViol- CESta: RxErr- BadTLP- BadDLLP- Rollover- Timeout- NonFatalErr- CEMsk: RxErr- BadTLP- BadDLLP- Rollover- Timeout- NonFatalErr+ AERCap: First Error Pointer: 00, ECRCGenCap+ ECRCGenEn- ECRCChkCap+ ECRCChkEn- MultHdrRecCap- MultHdrRecEn- TLPPfxPres- HdrLogCap- HeaderLog: 00000000 00000000 00000000 00000000 Capabilities: [140 v1] Virtual Channel Caps: LPEVC=0 RefClk=100ns PATEntryBits=1 Arb: Fixed- WRR32- WRR64- WRR128- Ctrl: ArbSelect=Fixed Status: InProgress- VC0: Caps: PATOffset=00 MaxTimeSlots=1 RejSnoopTrans- Arb: Fixed- WRR32- WRR64- WRR128- TWRR128- WRR256- Ctrl: Enable+ ID=0 ArbSelect=Fixed TC/VC=ff Status: NegoPending- InProgress- Capabilities: [160 v1] Device Serial Number 01-00-00-00-68-4c-e0-00 Capabilities: [170 v1] Latency Tolerance Reporting Max snoop latency: 0ns Max no snoop latency: 0ns 02:00.0 Class 0280: Device 14e4:43ec (rev 02) Subsystem: Device 17aa:0777 Control: I/O- Mem- BusMaster- SpecCycle- MemWINV- VGASnoop- ParErr- Stepping- SERR- FastB2B- DisINTx- Status: Cap+ 66MHz- UDF- FastB2B- ParErr- DEVSEL=fast >TAbort- <TAbort- <MAbort- >SERR- <PERR- INTx- Interrupt: pin A routed to IRQ 0 Region 0: Memory at 13400000 (64-bit, non-prefetchable) [disabled] [size=32K] Region 2: Memory at 13000000 (64-bit, non-prefetchable) [disabled] [size=4M] Capabilities: [48] Power Management version 3 Flags: PMEClk- DSI- D1+ D2+ AuxCurrent=0mA PME(D0+,D1+,D2+,D3hot+,D3cold+) Status: D0 NoSoftRst+ PME-Enable- DSel=0 DScale=2 PME- Capabilities: [58] MSI: Enable- Count=1/16 Maskable- 64bit+ Address: 0000000000000000 Data: 0000 Capabilities: [68] Vendor Specific Information: Len=44 <?> Capabilities: [ac] Express (v2) Endpoint, MSI 00 DevCap: MaxPayload 256 bytes, PhantFunc 0, Latency L0s <4us, L1 unlimited ExtTag- AttnBtn- AttnInd- PwrInd- RBE+ FLReset- SlotPowerLimit 0.000W DevCtl: Report errors: Correctable- Non-Fatal- Fatal- Unsupported- RlxdOrd+ ExtTag- PhantFunc- AuxPwr+ NoSnoop+ MaxPayload 128 bytes, MaxReadReq 512 bytes DevSta: CorrErr- UncorrErr- FatalErr- UnsuppReq- AuxPwr+ TransPend- LnkCap: Port #0, Speed 2.5GT/s, Width x1, ASPM L0s L1, Exit Latency L0s <2us, L1 <32us ClockPM+ Surprise- LLActRep- BwNot- ASPMOptComp+ LnkCtl: ASPM Disabled; RCB 64 bytes Disabled- CommClk+ ExtSynch- ClockPM- AutWidDis- BWInt- AutBWInt- LnkSta: Speed 2.5GT/s, Width x1, TrErr- Train- SlotClk+ DLActive- BWMgmt- ABWMgmt- DevCap2: Completion Timeout: Range ABCD, TimeoutDis+, LTR+, OBFF Via WAKE# AtomicOpsCap: 32bit- 64bit- 128bitCAS- DevCtl2: Completion Timeout: 50us to 50ms, TimeoutDis-, LTR-, OBFF Disabled AtomicOpsCtl: ReqEn- LnkCtl2: Target Link Speed: 2.5GT/s, EnterCompliance- SpeedDis- Transmit Margin: Normal Operating Range, EnterModifiedCompliance- ComplianceSOS- Compliance De-emphasis: -6dB LnkSta2: Current De-emphasis Level: -3.5dB, EqualizationComplete-, EqualizationPhase1- EqualizationPhase2-, EqualizationPhase3-, LinkEqualizationRequest- Capabilities: [100 v1] Advanced Error Reporting UESta: DLP- SDES- TLP- FCP- CmpltTO- CmpltAbrt- UnxCmplt- RxOF- MalfTLP- ECRC- UnsupReq- ACSViol- UEMsk: DLP- SDES- TLP- FCP- CmpltTO- CmpltAbrt- UnxCmplt- RxOF- MalfTLP- ECRC- UnsupReq- ACSViol- UESvrt: DLP+ SDES+ TLP- FCP+ CmpltTO- CmpltAbrt- UnxCmplt- RxOF+ MalfTLP+ ECRC- UnsupReq- ACSViol- CESta: RxErr- BadTLP- BadDLLP- Rollover- Timeout- NonFatalErr- CEMsk: RxErr- BadTLP- BadDLLP- Rollover- Timeout- NonFatalErr+ AERCap: First Error Pointer: 00, ECRCGenCap+ ECRCGenEn- ECRCChkCap+ ECRCChkEn- MultHdrRecCap- MultHdrRecEn- TLPPfxPres- HdrLogCap- HeaderLog: 00000000 00000000 00000000 00000000 Capabilities: [13c v1] Device Serial Number 00-00-a2-ff-ff-75-5c-93 Capabilities: [150 v1] Power Budgeting <?> Capabilities: [160 v1] Virtual Channel Caps: LPEVC=0 RefClk=100ns PATEntryBits=1 Arb: Fixed- WRR32- WRR64- WRR128- Ctrl: ArbSelect=Fixed Status: InProgress- VC0: Caps: PATOffset=00 MaxTimeSlots=1 RejSnoopTrans- Arb: Fixed- WRR32- WRR64- WRR128- TWRR128- WRR256- Ctrl: Enable+ ID=0 ArbSelect=Fixed TC/VC=ff Status: NegoPending- InProgress- Capabilities: [1b0 v1] Latency Tolerance Reporting Max snoop latency: 0ns Max no snoop latency: 0ns Capabilities: [220 v1] #15 Capabilities: [240 v1] L1 PM Substates L1SubCap: PCI-PM_L1.2+ PCI-PM_L1.1+ ASPM_L1.2+ ASPM_L1.1+ L1_PM_Substates+ PortCommonModeRestoreTime=0us PortTPowerOnTime=50us L1SubCtl1: PCI-PM_L1.2- PCI-PM_L1.1- ASPM_L1.2- ASPM_L1.1- T_CommonMode=0us LTR1.2_Threshold=0ns L1SubCtl2: T_PwrOn=10us Manikanta >> DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_NVIDIA, PCI_ANY_ID, nv_msi_ht_cap_quirk_leaf); >> DECLARE_PCI_FIXUP_RESUME_EARLY(PCI_VENDOR_ID_NVIDIA, PCI_ANY_ID, nv_msi_ht_cap_quirk_leaf); >> +#endif >> >> static void quirk_msi_intx_disable_bug(struct pci_dev *dev) >> { >> -- >> 2.17.2 >> ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] PCI: disable nv_msi_ht_cap_quirk_leaf quirk on arm/arm64 2019-04-15 13:48 ` Bjorn Helgaas 2019-04-16 9:52 ` Manikanta Maddireddy @ 2019-04-16 11:35 ` Thierry Reding 2019-04-16 19:27 ` Manikanta Maddireddy ` (2 more replies) 1 sibling, 3 replies; 9+ messages in thread From: Thierry Reding @ 2019-04-16 11:35 UTC (permalink / raw) To: Bjorn Helgaas Cc: Nicolas Chauvet, Jonathan Hunter, Manikanta Maddireddy, Lorenzo Pieralisi, linux-tegra, linux-pci [-- Attachment #1: Type: text/plain, Size: 5297 bytes --] On Mon, Apr 15, 2019 at 08:48:13AM -0500, Bjorn Helgaas wrote: > On Mon, Apr 15, 2019 at 11:25:37AM +0200, Nicolas Chauvet wrote: > > This patch disable the use of nv_msi_ht_cap_quirk_leaf quirk on arm and > > arm64 NVIDIA devices such as Tegra > > > > This fixes the following output: > > "pci 0000:00:01.0: nv_msi_ht_cap_quirk didn't locate host bridge" > > as experienced on a Trimslice device with PCI host bridge enabled > > > > v3: exclude the quirk for arm and arm64 instead of only for x86 > > > > v2: use __maybe_unused to avoid a warning on nv_msi_ht_cap_quirk_leaf > > > > Signed-off-by: Nicolas Chauvet <kwizart@gmail.com> > > Reviewed-by: Manikanta Maddireddy <mmaddireddy@nvidia.com> > > Acked-by: Thierry Reding <treding@nvidia.com> > > --- > > drivers/pci/quirks.c | 5 ++++- > > 1 file changed, 4 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c > > index a59ad09ce911..1ac65f09d7ee 100644 > > --- a/drivers/pci/quirks.c > > +++ b/drivers/pci/quirks.c > > @@ -2811,12 +2811,15 @@ static void nv_msi_ht_cap_quirk_all(struct pci_dev *dev) > > DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_AL, PCI_ANY_ID, nv_msi_ht_cap_quirk_all); > > DECLARE_PCI_FIXUP_RESUME_EARLY(PCI_VENDOR_ID_AL, PCI_ANY_ID, nv_msi_ht_cap_quirk_all); > > > > -static void nv_msi_ht_cap_quirk_leaf(struct pci_dev *dev) > > +static void __maybe_unused nv_msi_ht_cap_quirk_leaf(struct pci_dev *dev) > > { > > return __nv_msi_ht_cap_quirk(dev, 0); > > } > > +/* HyperTransport is not relevant on theses arches */ > > +#if !IS_ENABLED(CONFIG_ARM) && !IS_ENABLED(CONFIG_ARM64) > > I don't see any of the previous versions or discussion on the list, so > I can't tell what led to this #ifdef, but I'd prefer to avoid #ifdefs > like this because they're checking for arch at the wrong level. > > If you're seeing that warning message, apparently your system contains > an Nvidia device that has an HT_CAPTYPE_MSI_MAPPING capability, so > HyperTransport is at least in the picture, even on arm/arm64. > > The quirk assumes a host bridge at 00:00.0, which is really a > device-specific assumption, or maybe something specific to > HyperTransport. It's certainly not required by the PCI specs, which > don't require a host bridge to be visible as a PCI device at all. > > I'd be tempted to just change that pci_warn() to a pci_info() or maybe > even remove it altogether. It would be nice to have the complete > "lspci -vv" and dmesg log archived for future reference, since this > quirk has such a broad reach (matching PCI_ANY_ID) and it has a long > and ugly history already. I've been thinking about this a bit and it seems to me like the intention of the quirk is to make sure that all devices have their HT MSI mapping capabilities disabled if the system doesn't support HT. As you said, the fact that a host bridge may not be present isn't an error, however, it also means that we have to assume that HT is not supported in such cases and make sure the HT MSI mapping capability gets disabled for all devices. The following patch tries to address this and gets rid of the warning on Tegra systems. Thoughts? Thierry --- >8 --- From 69fb3b269cc852964385436694d79765d5c1cab3 Mon Sep 17 00:00:00 2001 From: Thierry Reding <treding@nvidia.com> Date: Tue, 16 Apr 2019 13:27:53 +0200 Subject: [PATCH] PCI: Be less noisy if no HT host bridge exists Embedded systems will typically not expose a host bridge's configuration space, so such setups must be assumed not to support HyperTransport, and consequently the HT MSI mapping capability should be disabled for all devices. Signed-off-by: Thierry Reding <treding@nvidia.com> --- drivers/pci/quirks.c | 34 +++++++++++++++++++--------------- 1 file changed, 19 insertions(+), 15 deletions(-) diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c index f9cd4d432e05..705891c88073 100644 --- a/drivers/pci/quirks.c +++ b/drivers/pci/quirks.c @@ -2792,24 +2792,28 @@ static void __nv_msi_ht_cap_quirk(struct pci_dev *dev, int all) */ host_bridge = pci_get_domain_bus_and_slot(pci_domain_nr(dev->bus), 0, PCI_DEVFN(0, 0)); - if (host_bridge == NULL) { - pci_warn(dev, "nv_msi_ht_cap_quirk didn't locate host bridge\n"); - return; - } - - pos = pci_find_ht_capability(host_bridge, HT_CAPTYPE_SLAVE); - if (pos != 0) { - /* Host bridge is to HT */ - if (found == 1) { - /* it is not enabled, try to enable it */ - if (all) - ht_enable_msi_mapping(dev); - else - nv_ht_enable_msi_mapping(dev); + if (host_bridge) { + pos = pci_find_ht_capability(host_bridge, HT_CAPTYPE_SLAVE); + if (pos != 0) { + /* Host bridge is to HT */ + if (found == 1) { + /* it is not enabled, try to enable it */ + if (all) + ht_enable_msi_mapping(dev); + else + nv_ht_enable_msi_mapping(dev); + } + goto out; } - goto out; } + /* + * Some systems, such as embedded devices, may not expose the host + * bridge's configuration space. Assume such systems to not support + * HyperTransport and disable the HT MSI mapping capability for all + * devices. + */ + /* HT MSI is not enabled */ if (found == 1) goto out; -- 2.21.0 [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 833 bytes --] ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH] PCI: disable nv_msi_ht_cap_quirk_leaf quirk on arm/arm64 2019-04-16 11:35 ` Thierry Reding @ 2019-04-16 19:27 ` Manikanta Maddireddy 2019-04-18 22:56 ` Bjorn Helgaas 2019-04-18 23:17 ` Bjorn Helgaas 2 siblings, 0 replies; 9+ messages in thread From: Manikanta Maddireddy @ 2019-04-16 19:27 UTC (permalink / raw) To: Thierry Reding, Bjorn Helgaas Cc: Nicolas Chauvet, Jonathan Hunter, Lorenzo Pieralisi, linux-tegra, linux-pci On 16-Apr-19 5:05 PM, Thierry Reding wrote: > On Mon, Apr 15, 2019 at 08:48:13AM -0500, Bjorn Helgaas wrote: >> On Mon, Apr 15, 2019 at 11:25:37AM +0200, Nicolas Chauvet wrote: >>> This patch disable the use of nv_msi_ht_cap_quirk_leaf quirk on arm and >>> arm64 NVIDIA devices such as Tegra >>> >>> This fixes the following output: >>> "pci 0000:00:01.0: nv_msi_ht_cap_quirk didn't locate host bridge" >>> as experienced on a Trimslice device with PCI host bridge enabled >>> >>> v3: exclude the quirk for arm and arm64 instead of only for x86 >>> >>> v2: use __maybe_unused to avoid a warning on nv_msi_ht_cap_quirk_leaf >>> >>> Signed-off-by: Nicolas Chauvet <kwizart@gmail.com> >>> Reviewed-by: Manikanta Maddireddy <mmaddireddy@nvidia.com> >>> Acked-by: Thierry Reding <treding@nvidia.com> >>> --- >>> drivers/pci/quirks.c | 5 ++++- >>> 1 file changed, 4 insertions(+), 1 deletion(-) >>> >>> diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c >>> index a59ad09ce911..1ac65f09d7ee 100644 >>> --- a/drivers/pci/quirks.c >>> +++ b/drivers/pci/quirks.c >>> @@ -2811,12 +2811,15 @@ static void nv_msi_ht_cap_quirk_all(struct pci_dev *dev) >>> DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_AL, PCI_ANY_ID, nv_msi_ht_cap_quirk_all); >>> DECLARE_PCI_FIXUP_RESUME_EARLY(PCI_VENDOR_ID_AL, PCI_ANY_ID, nv_msi_ht_cap_quirk_all); >>> >>> -static void nv_msi_ht_cap_quirk_leaf(struct pci_dev *dev) >>> +static void __maybe_unused nv_msi_ht_cap_quirk_leaf(struct pci_dev *dev) >>> { >>> return __nv_msi_ht_cap_quirk(dev, 0); >>> } >>> +/* HyperTransport is not relevant on theses arches */ >>> +#if !IS_ENABLED(CONFIG_ARM) && !IS_ENABLED(CONFIG_ARM64) >> I don't see any of the previous versions or discussion on the list, so >> I can't tell what led to this #ifdef, but I'd prefer to avoid #ifdefs >> like this because they're checking for arch at the wrong level. >> >> If you're seeing that warning message, apparently your system contains >> an Nvidia device that has an HT_CAPTYPE_MSI_MAPPING capability, so >> HyperTransport is at least in the picture, even on arm/arm64. >> >> The quirk assumes a host bridge at 00:00.0, which is really a >> device-specific assumption, or maybe something specific to >> HyperTransport. It's certainly not required by the PCI specs, which >> don't require a host bridge to be visible as a PCI device at all. >> >> I'd be tempted to just change that pci_warn() to a pci_info() or maybe >> even remove it altogether. It would be nice to have the complete >> "lspci -vv" and dmesg log archived for future reference, since this >> quirk has such a broad reach (matching PCI_ANY_ID) and it has a long >> and ugly history already. > I've been thinking about this a bit and it seems to me like the > intention of the quirk is to make sure that all devices have their HT > MSI mapping capabilities disabled if the system doesn't support HT. > As you said, the fact that a host bridge may not be present isn't an > error, however, it also means that we have to assume that HT is not > supported in such cases and make sure the HT MSI mapping capability gets > disabled for all devices. > > The following patch tries to address this and gets rid of the warning on > Tegra systems. > > Thoughts? > > Thierry > --- >8 --- > From 69fb3b269cc852964385436694d79765d5c1cab3 Mon Sep 17 00:00:00 2001 > From: Thierry Reding <treding@nvidia.com> > Date: Tue, 16 Apr 2019 13:27:53 +0200 > Subject: [PATCH] PCI: Be less noisy if no HT host bridge exists > > Embedded systems will typically not expose a host bridge's configuration > space, so such setups must be assumed not to support HyperTransport, and > consequently the HT MSI mapping capability should be disabled for all > devices. > > Signed-off-by: Thierry Reding <treding@nvidia.com> > --- > drivers/pci/quirks.c | 34 +++++++++++++++++++--------------- > 1 file changed, 19 insertions(+), 15 deletions(-) > > diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c > index f9cd4d432e05..705891c88073 100644 > --- a/drivers/pci/quirks.c > +++ b/drivers/pci/quirks.c > @@ -2792,24 +2792,28 @@ static void __nv_msi_ht_cap_quirk(struct pci_dev *dev, int all) > */ > host_bridge = pci_get_domain_bus_and_slot(pci_domain_nr(dev->bus), 0, > PCI_DEVFN(0, 0)); > - if (host_bridge == NULL) { > - pci_warn(dev, "nv_msi_ht_cap_quirk didn't locate host bridge\n"); > - return; > - } > - > - pos = pci_find_ht_capability(host_bridge, HT_CAPTYPE_SLAVE); > - if (pos != 0) { > - /* Host bridge is to HT */ > - if (found == 1) { > - /* it is not enabled, try to enable it */ > - if (all) > - ht_enable_msi_mapping(dev); > - else > - nv_ht_enable_msi_mapping(dev); > + if (host_bridge) { > + pos = pci_find_ht_capability(host_bridge, HT_CAPTYPE_SLAVE); > + if (pos != 0) { > + /* Host bridge is to HT */ > + if (found == 1) { > + /* it is not enabled, try to enable it */ > + if (all) > + ht_enable_msi_mapping(dev); > + else > + nv_ht_enable_msi_mapping(dev); > + } > + goto out; > } > - goto out; > } > > + /* > + * Some systems, such as embedded devices, may not expose the host > + * bridge's configuration space. Assume such systems to not support > + * HyperTransport and disable the HT MSI mapping capability for all > + * devices. > + */ > + > /* HT MSI is not enabled */ > if (found == 1) > goto out; I verified this patch on Tegra210. Reviewed-by: Manikanta Maddireddy <mmaddireddy@nvidia.com <mailto:mmaddireddy@nvidia.com>> Tested-by: Manikanta Maddireddy <mmaddireddy@nvidia.com <mailto:mmaddireddy@nvidia.com>> ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] PCI: disable nv_msi_ht_cap_quirk_leaf quirk on arm/arm64 2019-04-16 11:35 ` Thierry Reding 2019-04-16 19:27 ` Manikanta Maddireddy @ 2019-04-18 22:56 ` Bjorn Helgaas 2019-04-18 23:17 ` Bjorn Helgaas 2 siblings, 0 replies; 9+ messages in thread From: Bjorn Helgaas @ 2019-04-18 22:56 UTC (permalink / raw) To: Thierry Reding Cc: Nicolas Chauvet, Jonathan Hunter, Manikanta Maddireddy, Lorenzo Pieralisi, linux-tegra, linux-pci On Tue, Apr 16, 2019 at 01:35:04PM +0200, Thierry Reding wrote: > On Mon, Apr 15, 2019 at 08:48:13AM -0500, Bjorn Helgaas wrote: > > On Mon, Apr 15, 2019 at 11:25:37AM +0200, Nicolas Chauvet wrote: > > > This patch disable the use of nv_msi_ht_cap_quirk_leaf quirk on arm and > > > arm64 NVIDIA devices such as Tegra > > > > > > This fixes the following output: > > > "pci 0000:00:01.0: nv_msi_ht_cap_quirk didn't locate host bridge" > > > as experienced on a Trimslice device with PCI host bridge enabled > > > > > > v3: exclude the quirk for arm and arm64 instead of only for x86 > > > > > > v2: use __maybe_unused to avoid a warning on nv_msi_ht_cap_quirk_leaf > > > > > > Signed-off-by: Nicolas Chauvet <kwizart@gmail.com> > > > Reviewed-by: Manikanta Maddireddy <mmaddireddy@nvidia.com> > > > Acked-by: Thierry Reding <treding@nvidia.com> > > > --- > > > drivers/pci/quirks.c | 5 ++++- > > > 1 file changed, 4 insertions(+), 1 deletion(-) > > > > > > diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c > > > index a59ad09ce911..1ac65f09d7ee 100644 > > > --- a/drivers/pci/quirks.c > > > +++ b/drivers/pci/quirks.c > > > @@ -2811,12 +2811,15 @@ static void nv_msi_ht_cap_quirk_all(struct pci_dev *dev) > > > DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_AL, PCI_ANY_ID, nv_msi_ht_cap_quirk_all); > > > DECLARE_PCI_FIXUP_RESUME_EARLY(PCI_VENDOR_ID_AL, PCI_ANY_ID, nv_msi_ht_cap_quirk_all); > > > > > > -static void nv_msi_ht_cap_quirk_leaf(struct pci_dev *dev) > > > +static void __maybe_unused nv_msi_ht_cap_quirk_leaf(struct pci_dev *dev) > > > { > > > return __nv_msi_ht_cap_quirk(dev, 0); > > > } > > > +/* HyperTransport is not relevant on theses arches */ > > > +#if !IS_ENABLED(CONFIG_ARM) && !IS_ENABLED(CONFIG_ARM64) > > > > I don't see any of the previous versions or discussion on the list, so > > I can't tell what led to this #ifdef, but I'd prefer to avoid #ifdefs > > like this because they're checking for arch at the wrong level. > > > > If you're seeing that warning message, apparently your system contains > > an Nvidia device that has an HT_CAPTYPE_MSI_MAPPING capability, so > > HyperTransport is at least in the picture, even on arm/arm64. > > > > The quirk assumes a host bridge at 00:00.0, which is really a > > device-specific assumption, or maybe something specific to > > HyperTransport. It's certainly not required by the PCI specs, which > > don't require a host bridge to be visible as a PCI device at all. > > > > I'd be tempted to just change that pci_warn() to a pci_info() or maybe > > even remove it altogether. It would be nice to have the complete > > "lspci -vv" and dmesg log archived for future reference, since this > > quirk has such a broad reach (matching PCI_ANY_ID) and it has a long > > and ugly history already. > > I've been thinking about this a bit and it seems to me like the > intention of the quirk is to make sure that all devices have their HT > MSI mapping capabilities disabled if the system doesn't support HT. > As you said, the fact that a host bridge may not be present isn't an > error, however, it also means that we have to assume that HT is not > supported in such cases and make sure the HT MSI mapping capability gets > disabled for all devices. > > The following patch tries to address this and gets rid of the warning on > Tegra systems. One thing I still don't understand here is whether this quirk is actually for a defect in Nvidia and ALi endpoints, or if it's related to an issue in certain host bridges, or if it's just a generic HT-related thing that's correctly implemented in hardware but Linux doesn't know how to deal with. Having this quirk run forever and ever on every Nvidia and ALI endpoint just seems like massive overkill. We assume a host bridge must be xxxx:00:00.0, which I don't think is true in HT. __nv_msi_ht_cap_quirk() doesn't look at the topology at all, so it apparently assumes any device in domain xxxx with an HT MSI capability must be part of the xxxx:00:00.0 HT hierarchy. I don't think that's true either. It seems like what we really want to do is look for HT_CAPTYPE_SLAVE in generic code for every device we enumerate and set an "ht_tree" bit in its pci_dev if we find it. Then inherit that for all children, and use that to enable/disable HT MSI mapping. I bet there's no actual hardware defect here that we're working around. I suspect this could all be done in generic code as above. Maybe we think we only need this for Nvidia/ALi because of differences in how BIOS leaves things set up. And that probably depends mostly on the host bridge, not the endpoints that the quirks apply to. Sigh. I suppose it's probably not worth trying to unravel all this at this stage of the game. The commits below have lots of interesting information if anybody *were* to dig into this: de7453065d5d ("PCI: don't enable too much HT MSI mapping") 1dec6b054dd1 ("PCI: don't enable too many HT MSI mappings") 439a7733e8fc ("PCI: enable nv_msi_ht_cap_quirk for ALi bridges") 9dc625e72309 ("PCI: quirks: set 'En' bit of MSI Mapping for devices onHT-based nvidia platform") 6bae1d96c6d7 ("PCI: quirk: enable MSI Mapping on HT1000") 1d84b5424efb ("PCI: Add MSI quirk for ServerWorks HT1000 PCIX bridge.") e3008dedff4b ("PCI: disable MSI by default on systems with Serverworks HT1000 chips") 6397c75cbc4d ("MSI: Blacklist PCI-E chipsets depending on Hypertransport MSI capability") Bjorn > --- >8 --- > From 69fb3b269cc852964385436694d79765d5c1cab3 Mon Sep 17 00:00:00 2001 > From: Thierry Reding <treding@nvidia.com> > Date: Tue, 16 Apr 2019 13:27:53 +0200 > Subject: [PATCH] PCI: Be less noisy if no HT host bridge exists > > Embedded systems will typically not expose a host bridge's configuration > space, so such setups must be assumed not to support HyperTransport, and > consequently the HT MSI mapping capability should be disabled for all > devices. > > Signed-off-by: Thierry Reding <treding@nvidia.com> > --- > drivers/pci/quirks.c | 34 +++++++++++++++++++--------------- > 1 file changed, 19 insertions(+), 15 deletions(-) > > diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c > index f9cd4d432e05..705891c88073 100644 > --- a/drivers/pci/quirks.c > +++ b/drivers/pci/quirks.c > @@ -2792,24 +2792,28 @@ static void __nv_msi_ht_cap_quirk(struct pci_dev *dev, int all) > */ > host_bridge = pci_get_domain_bus_and_slot(pci_domain_nr(dev->bus), 0, > PCI_DEVFN(0, 0)); > - if (host_bridge == NULL) { > - pci_warn(dev, "nv_msi_ht_cap_quirk didn't locate host bridge\n"); > - return; > - } > - > - pos = pci_find_ht_capability(host_bridge, HT_CAPTYPE_SLAVE); > - if (pos != 0) { > - /* Host bridge is to HT */ > - if (found == 1) { > - /* it is not enabled, try to enable it */ > - if (all) > - ht_enable_msi_mapping(dev); > - else > - nv_ht_enable_msi_mapping(dev); > + if (host_bridge) { > + pos = pci_find_ht_capability(host_bridge, HT_CAPTYPE_SLAVE); > + if (pos != 0) { > + /* Host bridge is to HT */ > + if (found == 1) { > + /* it is not enabled, try to enable it */ > + if (all) > + ht_enable_msi_mapping(dev); > + else > + nv_ht_enable_msi_mapping(dev); > + } > + goto out; > } > - goto out; > } > > + /* > + * Some systems, such as embedded devices, may not expose the host > + * bridge's configuration space. Assume such systems to not support > + * HyperTransport and disable the HT MSI mapping capability for all > + * devices. > + */ > + > /* HT MSI is not enabled */ > if (found == 1) > goto out; > -- > 2.21.0 > ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] PCI: disable nv_msi_ht_cap_quirk_leaf quirk on arm/arm64 2019-04-16 11:35 ` Thierry Reding 2019-04-16 19:27 ` Manikanta Maddireddy 2019-04-18 22:56 ` Bjorn Helgaas @ 2019-04-18 23:17 ` Bjorn Helgaas 2019-04-23 11:02 ` Thierry Reding 2 siblings, 1 reply; 9+ messages in thread From: Bjorn Helgaas @ 2019-04-18 23:17 UTC (permalink / raw) To: Thierry Reding Cc: Nicolas Chauvet, Jonathan Hunter, Manikanta Maddireddy, Lorenzo Pieralisi, linux-tegra, linux-pci On Tue, Apr 16, 2019 at 01:35:04PM +0200, Thierry Reding wrote: > On Mon, Apr 15, 2019 at 08:48:13AM -0500, Bjorn Helgaas wrote: > > On Mon, Apr 15, 2019 at 11:25:37AM +0200, Nicolas Chauvet wrote: > > > This patch disable the use of nv_msi_ht_cap_quirk_leaf quirk on arm and > > > arm64 NVIDIA devices such as Tegra > > > > > > This fixes the following output: > > > "pci 0000:00:01.0: nv_msi_ht_cap_quirk didn't locate host bridge" > > > as experienced on a Trimslice device with PCI host bridge enabled > > > > > > v3: exclude the quirk for arm and arm64 instead of only for x86 > > > > > > v2: use __maybe_unused to avoid a warning on nv_msi_ht_cap_quirk_leaf > > > > > > Signed-off-by: Nicolas Chauvet <kwizart@gmail.com> > > > Reviewed-by: Manikanta Maddireddy <mmaddireddy@nvidia.com> > > > Acked-by: Thierry Reding <treding@nvidia.com> > > > --- > > > drivers/pci/quirks.c | 5 ++++- > > > 1 file changed, 4 insertions(+), 1 deletion(-) > > > > > > diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c > > > index a59ad09ce911..1ac65f09d7ee 100644 > > > --- a/drivers/pci/quirks.c > > > +++ b/drivers/pci/quirks.c > > > @@ -2811,12 +2811,15 @@ static void nv_msi_ht_cap_quirk_all(struct pci_dev *dev) > > > DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_AL, PCI_ANY_ID, nv_msi_ht_cap_quirk_all); > > > DECLARE_PCI_FIXUP_RESUME_EARLY(PCI_VENDOR_ID_AL, PCI_ANY_ID, nv_msi_ht_cap_quirk_all); > > > > > > -static void nv_msi_ht_cap_quirk_leaf(struct pci_dev *dev) > > > +static void __maybe_unused nv_msi_ht_cap_quirk_leaf(struct pci_dev *dev) > > > { > > > return __nv_msi_ht_cap_quirk(dev, 0); > > > } > > > +/* HyperTransport is not relevant on theses arches */ > > > +#if !IS_ENABLED(CONFIG_ARM) && !IS_ENABLED(CONFIG_ARM64) > > > > I don't see any of the previous versions or discussion on the list, so > > I can't tell what led to this #ifdef, but I'd prefer to avoid #ifdefs > > like this because they're checking for arch at the wrong level. > > > > If you're seeing that warning message, apparently your system contains > > an Nvidia device that has an HT_CAPTYPE_MSI_MAPPING capability, so > > HyperTransport is at least in the picture, even on arm/arm64. > > > > The quirk assumes a host bridge at 00:00.0, which is really a > > device-specific assumption, or maybe something specific to > > HyperTransport. It's certainly not required by the PCI specs, which > > don't require a host bridge to be visible as a PCI device at all. > > > > I'd be tempted to just change that pci_warn() to a pci_info() or maybe > > even remove it altogether. It would be nice to have the complete > > "lspci -vv" and dmesg log archived for future reference, since this > > quirk has such a broad reach (matching PCI_ANY_ID) and it has a long > > and ugly history already. > > I've been thinking about this a bit and it seems to me like the > intention of the quirk is to make sure that all devices have their HT > MSI mapping capabilities disabled if the system doesn't support HT. > As you said, the fact that a host bridge may not be present isn't an > error, however, it also means that we have to assume that HT is not > supported in such cases and make sure the HT MSI mapping capability gets > disabled for all devices. > > The following patch tries to address this and gets rid of the warning on > Tegra systems. Also interesting that Nicolas' original patch took this quirk completely out of the picture for ARM/ARM64, while your patch actively disables HT MSI mapping if there's no HT host bridge. If HT MSI Mapping Capability is enabled, the device is supposed to map MSIs into HyperTransport interrupt messages (HyperTransport I/O Link spec r3.10e, sec 7.12). I don't know what that even means if the device is on a PCIe link instead of an HT chain. > --- >8 --- > From 69fb3b269cc852964385436694d79765d5c1cab3 Mon Sep 17 00:00:00 2001 > From: Thierry Reding <treding@nvidia.com> > Date: Tue, 16 Apr 2019 13:27:53 +0200 > Subject: [PATCH] PCI: Be less noisy if no HT host bridge exists > > Embedded systems will typically not expose a host bridge's configuration > space, so such setups must be assumed not to support HyperTransport, and > consequently the HT MSI mapping capability should be disabled for all > devices. > > Signed-off-by: Thierry Reding <treding@nvidia.com> > --- > drivers/pci/quirks.c | 34 +++++++++++++++++++--------------- > 1 file changed, 19 insertions(+), 15 deletions(-) > > diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c > index f9cd4d432e05..705891c88073 100644 > --- a/drivers/pci/quirks.c > +++ b/drivers/pci/quirks.c > @@ -2792,24 +2792,28 @@ static void __nv_msi_ht_cap_quirk(struct pci_dev *dev, int all) > */ > host_bridge = pci_get_domain_bus_and_slot(pci_domain_nr(dev->bus), 0, > PCI_DEVFN(0, 0)); > - if (host_bridge == NULL) { > - pci_warn(dev, "nv_msi_ht_cap_quirk didn't locate host bridge\n"); > - return; > - } > - > - pos = pci_find_ht_capability(host_bridge, HT_CAPTYPE_SLAVE); > - if (pos != 0) { > - /* Host bridge is to HT */ > - if (found == 1) { > - /* it is not enabled, try to enable it */ > - if (all) > - ht_enable_msi_mapping(dev); > - else > - nv_ht_enable_msi_mapping(dev); > + if (host_bridge) { > + pos = pci_find_ht_capability(host_bridge, HT_CAPTYPE_SLAVE); > + if (pos != 0) { > + /* Host bridge is to HT */ > + if (found == 1) { > + /* it is not enabled, try to enable it */ > + if (all) > + ht_enable_msi_mapping(dev); > + else > + nv_ht_enable_msi_mapping(dev); > + } > + goto out; > } > - goto out; > } > > + /* > + * Some systems, such as embedded devices, may not expose the host > + * bridge's configuration space. Assume such systems to not support > + * HyperTransport and disable the HT MSI mapping capability for all > + * devices. > + */ > + > /* HT MSI is not enabled */ > if (found == 1) > goto out; > -- > 2.21.0 > ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] PCI: disable nv_msi_ht_cap_quirk_leaf quirk on arm/arm64 2019-04-18 23:17 ` Bjorn Helgaas @ 2019-04-23 11:02 ` Thierry Reding 0 siblings, 0 replies; 9+ messages in thread From: Thierry Reding @ 2019-04-23 11:02 UTC (permalink / raw) To: Bjorn Helgaas Cc: Nicolas Chauvet, Jonathan Hunter, Manikanta Maddireddy, Lorenzo Pieralisi, linux-tegra, linux-pci [-- Attachment #1: Type: text/plain, Size: 6936 bytes --] On Thu, Apr 18, 2019 at 06:17:04PM -0500, Bjorn Helgaas wrote: > On Tue, Apr 16, 2019 at 01:35:04PM +0200, Thierry Reding wrote: > > On Mon, Apr 15, 2019 at 08:48:13AM -0500, Bjorn Helgaas wrote: > > > On Mon, Apr 15, 2019 at 11:25:37AM +0200, Nicolas Chauvet wrote: > > > > This patch disable the use of nv_msi_ht_cap_quirk_leaf quirk on arm and > > > > arm64 NVIDIA devices such as Tegra > > > > > > > > This fixes the following output: > > > > "pci 0000:00:01.0: nv_msi_ht_cap_quirk didn't locate host bridge" > > > > as experienced on a Trimslice device with PCI host bridge enabled > > > > > > > > v3: exclude the quirk for arm and arm64 instead of only for x86 > > > > > > > > v2: use __maybe_unused to avoid a warning on nv_msi_ht_cap_quirk_leaf > > > > > > > > Signed-off-by: Nicolas Chauvet <kwizart@gmail.com> > > > > Reviewed-by: Manikanta Maddireddy <mmaddireddy@nvidia.com> > > > > Acked-by: Thierry Reding <treding@nvidia.com> > > > > --- > > > > drivers/pci/quirks.c | 5 ++++- > > > > 1 file changed, 4 insertions(+), 1 deletion(-) > > > > > > > > diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c > > > > index a59ad09ce911..1ac65f09d7ee 100644 > > > > --- a/drivers/pci/quirks.c > > > > +++ b/drivers/pci/quirks.c > > > > @@ -2811,12 +2811,15 @@ static void nv_msi_ht_cap_quirk_all(struct pci_dev *dev) > > > > DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_AL, PCI_ANY_ID, nv_msi_ht_cap_quirk_all); > > > > DECLARE_PCI_FIXUP_RESUME_EARLY(PCI_VENDOR_ID_AL, PCI_ANY_ID, nv_msi_ht_cap_quirk_all); > > > > > > > > -static void nv_msi_ht_cap_quirk_leaf(struct pci_dev *dev) > > > > +static void __maybe_unused nv_msi_ht_cap_quirk_leaf(struct pci_dev *dev) > > > > { > > > > return __nv_msi_ht_cap_quirk(dev, 0); > > > > } > > > > +/* HyperTransport is not relevant on theses arches */ > > > > +#if !IS_ENABLED(CONFIG_ARM) && !IS_ENABLED(CONFIG_ARM64) > > > > > > I don't see any of the previous versions or discussion on the list, so > > > I can't tell what led to this #ifdef, but I'd prefer to avoid #ifdefs > > > like this because they're checking for arch at the wrong level. > > > > > > If you're seeing that warning message, apparently your system contains > > > an Nvidia device that has an HT_CAPTYPE_MSI_MAPPING capability, so > > > HyperTransport is at least in the picture, even on arm/arm64. > > > > > > The quirk assumes a host bridge at 00:00.0, which is really a > > > device-specific assumption, or maybe something specific to > > > HyperTransport. It's certainly not required by the PCI specs, which > > > don't require a host bridge to be visible as a PCI device at all. > > > > > > I'd be tempted to just change that pci_warn() to a pci_info() or maybe > > > even remove it altogether. It would be nice to have the complete > > > "lspci -vv" and dmesg log archived for future reference, since this > > > quirk has such a broad reach (matching PCI_ANY_ID) and it has a long > > > and ugly history already. > > > > I've been thinking about this a bit and it seems to me like the > > intention of the quirk is to make sure that all devices have their HT > > MSI mapping capabilities disabled if the system doesn't support HT. > > As you said, the fact that a host bridge may not be present isn't an > > error, however, it also means that we have to assume that HT is not > > supported in such cases and make sure the HT MSI mapping capability gets > > disabled for all devices. > > > > The following patch tries to address this and gets rid of the warning on > > Tegra systems. > > Also interesting that Nicolas' original patch took this quirk > completely out of the picture for ARM/ARM64, while your patch actively > disables HT MSI mapping if there's no HT host bridge. I should note here that this is just my interpretation of what I think the quirk was originally meant to be. I don't have any hardware to test this on, nor do I know much (well, nothing, to be frank) about Hyper- Transport. So my thinking was that if there's no HT host bridge, then it doesn't make any sense to enable HT MSI mapping either, which I think may have been the intent of this quirk: make sure that if the system is not HT capable, that none of the PCI devices try to use HT MSI mapping. Thierry > If HT MSI Mapping Capability is enabled, the device is supposed to map > MSIs into HyperTransport interrupt messages (HyperTransport I/O Link > spec r3.10e, sec 7.12). I don't know what that even means if the > device is on a PCIe link instead of an HT chain. > > > --- >8 --- > > From 69fb3b269cc852964385436694d79765d5c1cab3 Mon Sep 17 00:00:00 2001 > > From: Thierry Reding <treding@nvidia.com> > > Date: Tue, 16 Apr 2019 13:27:53 +0200 > > Subject: [PATCH] PCI: Be less noisy if no HT host bridge exists > > > > Embedded systems will typically not expose a host bridge's configuration > > space, so such setups must be assumed not to support HyperTransport, and > > consequently the HT MSI mapping capability should be disabled for all > > devices. > > > > Signed-off-by: Thierry Reding <treding@nvidia.com> > > --- > > drivers/pci/quirks.c | 34 +++++++++++++++++++--------------- > > 1 file changed, 19 insertions(+), 15 deletions(-) > > > > diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c > > index f9cd4d432e05..705891c88073 100644 > > --- a/drivers/pci/quirks.c > > +++ b/drivers/pci/quirks.c > > @@ -2792,24 +2792,28 @@ static void __nv_msi_ht_cap_quirk(struct pci_dev *dev, int all) > > */ > > host_bridge = pci_get_domain_bus_and_slot(pci_domain_nr(dev->bus), 0, > > PCI_DEVFN(0, 0)); > > - if (host_bridge == NULL) { > > - pci_warn(dev, "nv_msi_ht_cap_quirk didn't locate host bridge\n"); > > - return; > > - } > > - > > - pos = pci_find_ht_capability(host_bridge, HT_CAPTYPE_SLAVE); > > - if (pos != 0) { > > - /* Host bridge is to HT */ > > - if (found == 1) { > > - /* it is not enabled, try to enable it */ > > - if (all) > > - ht_enable_msi_mapping(dev); > > - else > > - nv_ht_enable_msi_mapping(dev); > > + if (host_bridge) { > > + pos = pci_find_ht_capability(host_bridge, HT_CAPTYPE_SLAVE); > > + if (pos != 0) { > > + /* Host bridge is to HT */ > > + if (found == 1) { > > + /* it is not enabled, try to enable it */ > > + if (all) > > + ht_enable_msi_mapping(dev); > > + else > > + nv_ht_enable_msi_mapping(dev); > > + } > > + goto out; > > } > > - goto out; > > } > > > > + /* > > + * Some systems, such as embedded devices, may not expose the host > > + * bridge's configuration space. Assume such systems to not support > > + * HyperTransport and disable the HT MSI mapping capability for all > > + * devices. > > + */ > > + > > /* HT MSI is not enabled */ > > if (found == 1) > > goto out; > > -- > > 2.21.0 > > > > [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 833 bytes --] ^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2019-04-23 11:02 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
[not found] <20180730094213.11973-1-kwizart@gmail.com>
[not found] ` <CABr+WT=ANAs4DgvbfcgQ2wnX5yhT9HTgLJh1LE8Q09fyCa0mBg@mail.gmail.com>
2019-04-15 4:40 ` [PATCH] pci: disable nv_msi_ht_cap_quirk_leaf quirk on non-x86 hw Manikanta Maddireddy
2019-04-15 9:25 ` [PATCH] PCI: disable nv_msi_ht_cap_quirk_leaf quirk on arm/arm64 Nicolas Chauvet
2019-04-15 13:48 ` Bjorn Helgaas
2019-04-16 9:52 ` Manikanta Maddireddy
2019-04-16 11:35 ` Thierry Reding
2019-04-16 19:27 ` Manikanta Maddireddy
2019-04-18 22:56 ` Bjorn Helgaas
2019-04-18 23:17 ` Bjorn Helgaas
2019-04-23 11:02 ` Thierry Reding
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).