| Message ID | 20180730094213.11973-1-kwizart@gmail.com |
|---|---|
| State | Deferred |
| Headers | show |
| Series | pci: disable nv_msi_ht_cap_quirk_leaf quirk on non-x86 hw | expand |
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.
On Mon, Jul 30, 2018 at 11:42:13AM +0200, Nicolas Chauvet wrote: > 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 > > Signed-off-by: Nicolas Chauvet <kwizart@gmail.com> > --- > drivers/pci/quirks.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c > index f439de848658..9a2bba5d84c7 100644 > --- a/drivers/pci/quirks.c > +++ b/drivers/pci/quirks.c > @@ -2777,8 +2777,11 @@ static void nv_msi_ht_cap_quirk_leaf(struct pci_dev *dev) > { > return __nv_msi_ht_cap_quirk(dev, 0); > } > +/* This quirk is only relevant on x86 NVIDIA pci host bridges */ > +#if IS_ENABLED(CONFIG_X86) > 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 I think we probably want to turn this around. HyperTransport is not specific to x86, so I think we probably want to make this: #if !IS_ENABLED(CONFIG_ARM) && !IS_ENABLED(CONFIG_ARM64) to exclude it only on 32-bit and 64-bit ARM builds. Thierry
Le ven. 29 mars 2019 à 12:22, Thierry Reding <thierry.reding@gmail.com> a écrit : > > On Mon, Jul 30, 2018 at 11:42:13AM +0200, Nicolas Chauvet wrote: > > 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 > > > > Signed-off-by: Nicolas Chauvet <kwizart@gmail.com> > > --- > > drivers/pci/quirks.c | 3 +++ > > 1 file changed, 3 insertions(+) > > > > diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c > > index f439de848658..9a2bba5d84c7 100644 > > --- a/drivers/pci/quirks.c > > +++ b/drivers/pci/quirks.c > > @@ -2777,8 +2777,11 @@ static void nv_msi_ht_cap_quirk_leaf(struct pci_dev *dev) > > { > > return __nv_msi_ht_cap_quirk(dev, 0); > > } > > +/* This quirk is only relevant on x86 NVIDIA pci host bridges */ > > +#if IS_ENABLED(CONFIG_X86) > > 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 > > I think we probably want to turn this around. HyperTransport is not > specific to x86, so I think we probably want to make this: > > #if !IS_ENABLED(CONFIG_ARM) && !IS_ENABLED(CONFIG_ARM64) > > to exclude it only on 32-bit and 64-bit ARM builds. I've assumed there is no NVIDIA (PCI/MSI/HT) related chipset elsewhere than x86 And if there are any in the future (either for ppc64, riscv or else) that it may not require the quirk... Is this false assumption ? If the case, I'm sending a v3 with the requested change. Thx for the review. -- - Nicolas (kwizart)
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. >
On Fri, Mar 29, 2019 at 01:06:31PM +0100, Nicolas Chauvet wrote: > Le ven. 29 mars 2019 à 12:22, Thierry Reding > <thierry.reding@gmail.com> a écrit : > > > > On Mon, Jul 30, 2018 at 11:42:13AM +0200, Nicolas Chauvet wrote: > > > 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 > > > > > > Signed-off-by: Nicolas Chauvet <kwizart@gmail.com> > > > --- > > > drivers/pci/quirks.c | 3 +++ > > > 1 file changed, 3 insertions(+) > > > > > > diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c > > > index f439de848658..9a2bba5d84c7 100644 > > > --- a/drivers/pci/quirks.c > > > +++ b/drivers/pci/quirks.c > > > @@ -2777,8 +2777,11 @@ static void nv_msi_ht_cap_quirk_leaf(struct pci_dev *dev) > > > { > > > return __nv_msi_ht_cap_quirk(dev, 0); > > > } > > > +/* This quirk is only relevant on x86 NVIDIA pci host bridges */ > > > +#if IS_ENABLED(CONFIG_X86) > > > 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 > > > > I think we probably want to turn this around. HyperTransport is not > > specific to x86, so I think we probably want to make this: > > > > #if !IS_ENABLED(CONFIG_ARM) && !IS_ENABLED(CONFIG_ARM64) > > > > to exclude it only on 32-bit and 64-bit ARM builds. > > I've assumed there is no NVIDIA (PCI/MSI/HT) related chipset elsewhere than x86 > And if there are any in the future (either for ppc64, riscv or else) > that it may not require the quirk... > > Is this false assumption ? If the case, I'm sending a v3 with the > requested change. According to Wikipedia, HyperTransport has been used in PowerPC systems and can be present in recent MIPS systems. I'm not sure if any of those would have NVIDIA host bridges, though. I think it'd be safer to restrict this to 32-bit and 64-bit ARM. The reason is that this quirk is currently applied regardless of the system architecture, so if we all of a sudden now restrict it to x86 we might break things on systems that we don't know about. On the other hand if we prevent this quirk only on ARM systems, we can be pretty sur that no systems will break. The only systems that I'm aware of that are ARM and have an NVIDIA PCI host bridge are Tegra and we know that the quirk is not needed there. Also, if you're going to respin: s/pci/PCI/ in the comment that you added. With the above changes: Acked-by: Thierry Reding <treding@nvidia.com>
diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c index f439de848658..9a2bba5d84c7 100644 --- a/drivers/pci/quirks.c +++ b/drivers/pci/quirks.c @@ -2777,8 +2777,11 @@ static void nv_msi_ht_cap_quirk_leaf(struct pci_dev *dev) { return __nv_msi_ht_cap_quirk(dev, 0); } +/* This quirk is only relevant on x86 NVIDIA pci host bridges */ +#if IS_ENABLED(CONFIG_X86) 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) {
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 Signed-off-by: Nicolas Chauvet <kwizart@gmail.com> --- drivers/pci/quirks.c | 3 +++ 1 file changed, 3 insertions(+)