On Mon, Feb 21, 2022 at 03:01:03PM +0000, Mark Brown wrote:
On Mon, Feb 21, 2022 at 02:32:38PM +0000, Catalin Marinas wrote:
On Mon, Feb 07, 2022 at 03:20:39PM +0000, Mark Brown wrote:
+/*
- This must be called after sme_kernel_enable(), we rely on the
- feature table being sorted to ensure this.
- */
+void fa64_kernel_enable(const struct arm64_cpu_capabilities *__always_unused p) +{
- /* Allow use of FA64 */
- write_sysreg_s(read_sysreg_s(SYS_SMCR_EL1) | SMCR_ELx_FA64_MASK,
SYS_SMCR_EL1);+}
+#endif /* CONFIG_ARM64_SVE */
I think instead of worrying about the order, we could check the sanitised register value in sme_kernel_enable() and set the FA64 bit.
There's going to be a ordering/clarity issue whatever way round we do it
- the FA64 feature bit is in a different feature register to the main
SME feature bitfield and it's not as abundantly clear as might be ideal that it will have been sanitised when we're getting callbacks for the main SME feature, there's an awful lot of sharp edges with this code. Having things this way round felt more idiomatic to me.
You may want to add a comment in the cpu_feature[] array that it should be placed after SME.
Also to me 'fa64_kernel_enable' somehow implies that the kernel cares about FA64 for itself but AFAICT we never run the kernel in streaming mode.
We do run the kernel in streaming mode - entering the kernel through a syscall or preemption will not change the streaming mode state, and we need to be in streaming mode in order to save or restore the register state for streaming mode. In particular we need FA64 enabled for EL1 in order to context switch FFR when in streaming mode, without it we'll generate an exception when we execute the rdffr or wrffr. We don't do any real floating point work in streaming mode but we absolutely need to run in streaming mode and only exit streaming mode when restoring a context where it is disabled, when using floating point in the kernel or when idling the CPU.
So, IIUC, for Linux it is mandatory that FEAT_SME_FA64 is supported, otherwise we won't be able to enable SME. Does the architecture say this feature as optional? Which A64 instructions are not available if FA64 is disabled? I hope it's only the SVE ones but I thought we can still do load/store of the state even with FA64 disabled.
Anyway, if we can't even context switch without FA64 while in streaming mode, I think we should move the check in the main SME .matches function and enable it in sme_kernel_enable(), no need for an additional feature.
I think we should also update booting.rst to require that the FA64 is enabled at EL2 and EL3.