pci: disable nv_msi_ht_cap_quirk_leaf quirk on non-x86 hw
diff mbox series

Message ID 20180730094213.11973-1-kwizart@gmail.com
State New
Headers show
Series
  • pci: disable nv_msi_ht_cap_quirk_leaf quirk on non-x86 hw
Related show

Commit Message

Nicolas Chauvet July 30, 2018, 9:42 a.m. UTC
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(+)

Comments

Nicolas Chauvet March 29, 2019, 9:45 a.m. UTC | #1
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.
Thierry Reding March 29, 2019, 11:22 a.m. UTC | #2
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
Nicolas Chauvet March 29, 2019, 12:06 p.m. UTC | #3
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)
Manikanta Maddireddy April 15, 2019, 4:40 a.m. UTC | #4
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.
>
Thierry Reding April 15, 2019, 8:15 a.m. UTC | #5
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>

Patch
diff mbox series

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)
 {