[2/2,RFC] arm64: Add dependencies to vendor-specific errata

Currently the user is asked about enabling support for each and every
vendor-specific erratum, even when support for the specific platform is
not enabled.

Fix this by adding platform dependencies to the config options
controlling support for vendor-specific errata.

Note that FUJITSU_ERRATUM_010001 is left untouched, as no config symbol
exists for the Fujitsu A64FX platform.

Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
 arch/arm64/Kconfig | 10 ++++++++++
 1 file changed, 10 insertions(+)

Hi Geert,

On Thu, Apr 16, 2020 at 01:56:58PM +0200, Geert Uytterhoeven wrote:
> Currently the user is asked about enabling support for each and every
> vendor-specific erratum, even when support for the specific platform is
> not enabled.
> 
> Fix this by adding platform dependencies to the config options
> controlling support for vendor-specific errata.
> 
> Note that FUJITSU_ERRATUM_010001 is left untouched, as no config symbol
> exists for the Fujitsu A64FX platform.
> 
> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>

I'm not su1re that it makes sense to do this in general, becaose the
ARCH_* platform symbols are about plactform/SoC support (e.g. pinctrl
drivers), and these are (mostly) CPU-local and/or VM-visible.

I think that it makes sense for those to be independent because:

* future SoCs in the same family might not need the same CPU errata
  workarounds, and it's arguably just as confusing to have the option
  there.

* It prevents building a minimal VM image with all (non-virtualized)
  platform support disabled, but all possible (VM-visible) errata
  options enabled. I do that occassionally for testing/analysis, and I
  can imagine this is useful for those building images that are only
  intended to be used in VMs.

I think the change to SOCIONEXT_SYNQUACER_PREITS makes sense given
that's a platform-level detail. Arguably that should be moved into
drivers/irqchip/Kconfig.

Thanks,
Mark.

> ---
>  arch/arm64/Kconfig | 10 ++++++++++
>  1 file changed, 10 insertions(+)
> 
> diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
> index 8d33d7fed6d8549b..81f52f0b988e6350 100644
> --- a/arch/arm64/Kconfig
> +++ b/arch/arm64/Kconfig
> @@ -622,6 +622,8 @@ config ARM64_ERRATUM_1542419
>  
>  	  If unsure, say Y.
>  
> +if ARCH_THUNDER2
> +
>  config CAVIUM_ERRATUM_22375
>  	bool "Cavium erratum 22375, 24313"
>  	default y
> @@ -697,6 +699,8 @@ config CAVIUM_TX2_ERRATUM_219
>  
>  	  If unsure, say Y.
>  
> +endif # ARCH_THUNDER2
> +
>  config FUJITSU_ERRATUM_010001
>  	bool "Fujitsu-A64FX erratum E#010001: Undefined fault may occur wrongly"
>  	default y
> @@ -718,6 +722,7 @@ config FUJITSU_ERRATUM_010001
>  
>  config HISILICON_ERRATUM_161600802
>  	bool "Hip07 161600802: Erroneous redistributor VLPI base"
> +	depends on ARCH_HISI
>  	default y
>  	help
>  	  The HiSilicon Hip07 SoC uses the wrong redistributor base
> @@ -726,6 +731,8 @@ config HISILICON_ERRATUM_161600802
>  
>  	  If unsure, say Y.
>  
> +if ARCH_QCOM
> +
>  config QCOM_FALKOR_ERRATUM_1003
>  	bool "Falkor E1003: Incorrect translation due to ASID change"
>  	default y
> @@ -768,8 +775,11 @@ config QCOM_FALKOR_ERRATUM_E1041
>  
>  	  If unsure, say Y.
>  
> +endif # ARCH_QCOM
> +
>  config SOCIONEXT_SYNQUACER_PREITS
>  	bool "Socionext Synquacer: Workaround for GICv3 pre-ITS"
> +	depends on ARCH_SYNQUACER
>  	default y
>  	help
>  	  Socionext Synquacer SoCs implement a separate h/w block to generate
> -- 
> 2.17.1
> 
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

On Thu, Apr 16, 2020 at 2:56 PM Mark Rutland <mark.rutland@arm.com> wrote:
> On Thu, Apr 16, 2020 at 01:56:58PM +0200, Geert Uytterhoeven wrote:
> > Currently the user is asked about enabling support for each and every
> > vendor-specific erratum, even when support for the specific platform is
> > not enabled.
> >
> > Fix this by adding platform dependencies to the config options
> > controlling support for vendor-specific errata.
> >
> > Note that FUJITSU_ERRATUM_010001 is left untouched, as no config symbol
> > exists for the Fujitsu A64FX platform.
> >
> > Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
>
> I'm not su1re that it makes sense to do this in general, becaose the
> ARCH_* platform symbols are about plactform/SoC support (e.g. pinctrl
> drivers), and these are (mostly) CPU-local and/or VM-visible.
>
> I think that it makes sense for those to be independent because:
>
> * future SoCs in the same family might not need the same CPU errata
>   workarounds, and it's arguably just as confusing to have the option
>   there.
>
> * It prevents building a minimal VM image with all (non-virtualized)
>   platform support disabled, but all possible (VM-visible) errata
>   options enabled. I do that occassionally for testing/analysis, and I
>   can imagine this is useful for those building images that are only
>   intended to be used in VMs.

Most architectures over time grow a CPU selection option that is
at least somewhat orthogonal to the platform selection. I think so far
arm64 has intentionally resisted this based on the idea that the CPUs
are mostly equal and differences are better handled at runtime.

If we decide to revisit this in the future that might help both the
errata selection and give a way to e.g. build for an ARMv8.2
baseline.
This does seem pretty far out at the moment of course, given
that most SoCs we work on are still based on Cortex-A53 or A72 ;-)

> I think the change to SOCIONEXT_SYNQUACER_PREITS makes sense given
> that's a platform-level detail. Arguably that should be moved into
> drivers/irqchip/Kconfig.

Agreed

       Arnd

Hi Mark,

On Thu, Apr 16, 2020 at 5:57 PM Mark Rutland <mark.rutland@arm.com> wrote:
> On Thu, Apr 16, 2020 at 05:38:07PM +0200, Geert Uytterhoeven wrote:
> > And perhaps you do want to enable some platform-specific drivers for
> > VFIO pass-through?  Hence having ARCH_* dependencies on those drivers
> > means they cannot be enabled :-( Hmm...
>
> IIRC platform device passthrough requires an corresponding VFIO platform
> driver in the host to handle reset and so on, but it does seem a shame

If your SoC has a reset controller, that problem has been solved in a generic
way, cfr. "[PATCH v5] vfio: platform: Add generic reset controller support"
(https://lore.kernel.org/lkml/20181113131508.18246-1-geert+renesas@glider.be/).
Unfortunately not yet upstream.

Combine with "hw/arm/sysbus-fdt: Add support for instantiating generic devices"
(https://github.com/geertu/qemu/commit/180318003c08594e8e852b2285a98184f905bfa9)
and you're set ;-)

> to not allow the user to select a driver if they really want it.

I forgot you can add "|| VIRTIO_MMIO" to the dependencies of drivers for
devices that can be used with VFIO pass-through.

> I guess there might be platform-specific PCIe drivers too, which might
> work with VFIO regardless.

Indeed. PCI is business as usual.

Gr{oetje,eeting}s,

                        Geert

On 16.04.20 13:56:58, Geert Uytterhoeven wrote:
> Currently the user is asked about enabling support for each and every
> vendor-specific erratum, even when support for the specific platform is
> not enabled.
> 
> Fix this by adding platform dependencies to the config options
> controlling support for vendor-specific errata.
> 
> Note that FUJITSU_ERRATUM_010001 is left untouched, as no config symbol
> exists for the Fujitsu A64FX platform.
> 
> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
> ---
>  arch/arm64/Kconfig | 10 ++++++++++
>  1 file changed, 10 insertions(+)

> diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
> index 8d33d7fed6d8549b..81f52f0b988e6350 100644
> --- a/arch/arm64/Kconfig
> +++ b/arch/arm64/Kconfig
> @@ -622,6 +622,8 @@ config ARM64_ERRATUM_1542419
>  
>  	  If unsure, say Y.
>  
> +if ARCH_THUNDER2
> +
>  config CAVIUM_ERRATUM_22375
>  	bool "Cavium erratum 22375, 24313"
>  	default y
> @@ -697,6 +699,8 @@ config CAVIUM_TX2_ERRATUM_219
>  
>  	  If unsure, say Y.
>  
> +endif # ARCH_THUNDER2

For Cavium servers these ARCH_* options are only used to enable some
minor (mostly platform) drivers (i2c, spi, gpio, etc.), so the options
are not of much use and I better would like to get rid of them
completely, which makes us independent and more flexible when enabling
or disabling options. In ThunderX* systems there are not many devices
that are soc or board specific, most of them are detected using
generic methods like pci or acpi. So the situation is rather
comparable to x86 systems (there are options based on vendor or core)
than to ARM embedded chips.

Using the ARCH_* options to control also enablement of cpu errata
handling would add a strong dependency here and will make that options
a requirement.

How about having core/vendor specific submenus that make switching off
options easier?

Thanks,

-Robert