Re: [PATCH V6 12/13] pci, pci-thunder-ecam: Add ACPI support for ThunderX ECAM.

[Tomasz Nowicki <tn@semihalf.com>]

On 15.04.2016 19:06, Tomasz Nowicki wrote:
> Passes 1.x miss PCI enhanced allocation (EA) header for fixed-BARs,
> thus these passes should use Cavium-specific config access functions
> that synthesize the missing EA capabilities.
>
> We already have DT driver which addresses errata requirements and
> allows to use special PCI config accessors. Currently this driver uses
> compatible = "cavium,pci-host-thunder-ecam" to mach against the errata.
> For ACPI case we need explicit errata number and corresponding
> DECLARE_ACPI_MCFG_FIXUP fixup code.
>
> Signed-off-by: Tomasz Nowicki <tn@semihalf.com>
> ---
>   arch/arm64/Kconfig                  | 14 ++++++++++++++
>   arch/arm64/include/asm/cpufeature.h |  3 ++-
>   arch/arm64/kernel/cpu_errata.c      |  8 ++++++++
>   drivers/pci/host/pci-thunder-ecam.c | 35 +++++++++++++++++++++++++++++++++++
>   4 files changed, 59 insertions(+), 1 deletion(-)
>
> diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
> index 1bded87..b7614b8 100644
> --- a/arch/arm64/Kconfig
> +++ b/arch/arm64/Kconfig
> @@ -445,6 +445,20 @@ config CAVIUM_ERRATUM_27456
>
>   	  If unsure, say Y.
>
> +config CAVIUM_ERRATUM_24575
> +	bool "Cavium erratum 24575: Enhanced Allocation (EA) emualaion"
> +	default y
> +	help
> +	  Enable workaround for erratum 24575.
> +
> +	  Early versions of the Cavium Thunder CN88XX processor are missing
> +	  Enhanced Allocation (EA) capabilities for the fixed BAR addresses used
> +	  by the on-SoC hardware blocks. The erratum adds config access
> +	  functions that synthesize the missing EA capabilities for versions
> +	  that are missing that information.
> +
> +	  If unsure, say Y.
> +
>   endmenu
>
>
> diff --git a/arch/arm64/include/asm/cpufeature.h b/arch/arm64/include/asm/cpufeature.h
> index b9b6494..a78364e 100644
> --- a/arch/arm64/include/asm/cpufeature.h
> +++ b/arch/arm64/include/asm/cpufeature.h
> @@ -35,8 +35,9 @@
>   #define ARM64_ALT_PAN_NOT_UAO			10
>   #define ARM64_HAS_VIRT_HOST_EXTN		11
>   #define ARM64_WORKAROUND_CAVIUM_27456		12
> +#define ARM64_WORKAROUND_CAVIUM_24575		13
>
> -#define ARM64_NCAPS				13
> +#define ARM64_NCAPS				14
>
>   #ifndef __ASSEMBLY__
>
> diff --git a/arch/arm64/kernel/cpu_errata.c b/arch/arm64/kernel/cpu_errata.c
> index 06afd04..89c13d7 100644
> --- a/arch/arm64/kernel/cpu_errata.c
> +++ b/arch/arm64/kernel/cpu_errata.c
> @@ -97,6 +97,14 @@ const struct arm64_cpu_capabilities arm64_errata[] = {
>   			   (1 << MIDR_VARIANT_SHIFT) | 1),
>   	},
>   #endif
> +#ifdef CONFIG_CAVIUM_ERRATUM_24575
> +	{
> +	/* Cavium ThunderX, pass 1.x */
> +		.desc = "Cavium erratum 24575",
> +		.capability = ARM64_WORKAROUND_CAVIUM_24575,
> +		MIDR_RANGE(MIDR_THUNDERX, 0x00, 0x01),
> +	},
> +#endif
>   	{
>   	}
>   };
> diff --git a/drivers/pci/host/pci-thunder-ecam.c b/drivers/pci/host/pci-thunder-ecam.c
> index f67c6d7..01de697 100644
> --- a/drivers/pci/host/pci-thunder-ecam.c
> +++ b/drivers/pci/host/pci-thunder-ecam.c
> @@ -11,10 +11,14 @@
>   #include <linux/ioport.h>
>   #include <linux/of_pci.h>
>   #include <linux/of.h>
> +#include <linux/pci-acpi.h>
>   #include <linux/platform_device.h>
>
> +#include <asm/virt.h>
> +
>   #include "../ecam.h"
>
> +
>   static void set_val(u32 v, int where, int size, u32 *val)
>   {
>   	int shift = (where & 3) * 8;
> @@ -376,5 +380,36 @@ static struct platform_driver thunder_ecam_driver = {
>   };
>   module_platform_driver(thunder_ecam_driver);
>
> +#ifdef CONFIG_ACPI
> +
> +static bool needs_cavium_erratum_24575(struct pci_cfg_fixup *fixup,
> +				      struct acpi_pci_root *root)
> +{
> +	/*
> +	 * We must match errata code and be hypervisor, quirk does not apply
> +	 * for virtual machines.
> +	 */
> +	return cpus_have_cap(ARM64_WORKAROUND_CAVIUM_24575) &&
> +	       is_hyp_mode_available();
> +}
> +
> +DECLARE_ACPI_MCFG_FIXUP(NULL, needs_cavium_erratum_24575, &pci_thunder_ecam_ops,
> +			0, PCI_MCFG_BUS_ANY);
> +DECLARE_ACPI_MCFG_FIXUP(NULL, needs_cavium_erratum_24575, &pci_thunder_ecam_ops,
> +			1, PCI_MCFG_BUS_ANY);
> +DECLARE_ACPI_MCFG_FIXUP(NULL, needs_cavium_erratum_24575, &pci_thunder_ecam_ops,
> +			2, PCI_MCFG_BUS_ANY);
> +DECLARE_ACPI_MCFG_FIXUP(NULL, needs_cavium_erratum_24575, &pci_thunder_ecam_ops,
> +			3, PCI_MCFG_BUS_ANY);
> +DECLARE_ACPI_MCFG_FIXUP(NULL, needs_cavium_erratum_24575, &pci_thunder_ecam_ops,
> +			10, PCI_MCFG_BUS_ANY);
> +DECLARE_ACPI_MCFG_FIXUP(NULL, needs_cavium_erratum_24575, &pci_thunder_ecam_ops,
> +			11, PCI_MCFG_BUS_ANY);
> +DECLARE_ACPI_MCFG_FIXUP(NULL, needs_cavium_erratum_24575, &pci_thunder_ecam_ops,
> +			12, PCI_MCFG_BUS_ANY);
> +DECLARE_ACPI_MCFG_FIXUP(NULL, needs_cavium_erratum_24575, &pci_thunder_ecam_ops,
> +			13, PCI_MCFG_BUS_ANY);
> +#endif
> +

I wonder if we can identify quirk based on _DSD properties, like that:

Name (_DSD, Package () {
	ToUUID("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"),
	Package () {
		Package (2) {"cavium,pci-host-thunder-ecam", 1},
	}
})

similar for PEM driver:

Name (_DSD, Package () {
	ToUUID("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"),
	Package () {
		Package (2) {"cavium,pci-host-thunder-pem", 1},
	}
})

Do you think it is right thing to do?

Tomasz