| Message ID | 20200416115658.20406-3-geert+renesas@glider.be (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | arm64: Vendor-specific errata improvements | expand |
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 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. True. But at least the dependency restricts the confusion to a smaller audience. > * 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. Oh, you also want to build a "generic" guest kernel, with all ARCH_* symbols disabled. Let's hope a maleficent user cannot disable errata mitigations in the guest kernel and break the host ;-) 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... > 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. OK, makes sense. Gr{oetje,eeting}s, Geert
On Thu, Apr 16, 2020 at 05:38:07PM +0200, Geert Uytterhoeven wrote: > 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. > > 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: > > * 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. > > Oh, you also want to build a "generic" guest kernel, with all ARCH_* > symbols disabled. Yup! As above I do this today for building test kernels I run on a number of different hosts, and I'm aware of other use-cases (e.g. WSL2 or docker for mac) where you may want to do this to minimize the core kernel either for size or security reasons. > Let's hope a maleficent user cannot disable errata mitigations in the > guest kernel and break the host ;-) Indeed ;) For cases where a malicious guest could cause harm we've added workarounds in KVM, so unless we've missed something that shouldn't be the case. Otherwise, a guest missing these is just shooting itself in the foot. > 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 to not allow the user to select a driver if they really want it. I guess there might be platform-specific PCIe drivers too, which might work with VFIO regardless. Thanks, Mark.
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
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
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(+)