Re: [PATCH] PCI: disable nv_msi_ht_cap_quirk_leaf quirk on arm/arm64

[Bjorn Helgaas <helgaas@kernel.org>]

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
>