Re: [PATCH 2/5] arm64: Add support for FEAT_{LS64, LS64_V, LS64_ACCDATA}

[Marc Zyngier <maz@kernel.org>]

On Mon, 02 Dec 2024 13:55:01 +0000,
Yicong Yang <yangyicong@huawei.com> wrote:
> 
> From: Yicong Yang <yangyicong@hisilicon.com>
> 
> Armv8.7 introduces single-copy atomic 64-byte loads and stores
> instructions and its variants named under FEAT_{LS64, LS64_V,
> LS64_ACCDATA}. These features are identified by ID_AA64ISAR1_EL1.LS64
> and the use of such instructions in userspace (EL0) can be trapped.
> In order to support the use of corresponding instructions in userspace:
> - Make ID_AA64ISAR1_EL1.LS64 visbile to userspace
> - Add identifying and enabling in the cpufeature list
> - Expose these support of these features to userspace through HWCAP
>   and cpuinfo
> 
> Signed-off-by: Yicong Yang <yangyicong@hisilicon.com>
> ---
>  Documentation/arch/arm64/booting.rst    | 28 ++++++++++
>  Documentation/arch/arm64/elf_hwcaps.rst |  9 ++++
>  arch/arm64/include/asm/hwcap.h          |  3 ++
>  arch/arm64/include/uapi/asm/hwcap.h     |  3 ++
>  arch/arm64/kernel/cpufeature.c          | 70 ++++++++++++++++++++++++-
>  arch/arm64/kernel/cpuinfo.c             |  3 ++
>  arch/arm64/tools/cpucaps                |  3 ++
>  7 files changed, 118 insertions(+), 1 deletion(-)
> 
> diff --git a/Documentation/arch/arm64/booting.rst b/Documentation/arch/arm64/booting.rst
> index 3278fb4bf219..c35cfe9da501 100644
> --- a/Documentation/arch/arm64/booting.rst
> +++ b/Documentation/arch/arm64/booting.rst
> @@ -449,6 +449,34 @@ Before jumping into the kernel, the following conditions must be met:
>  
>      - HFGWTR_EL2.nGCS_EL0 (bit 52) must be initialised to 0b1.
>  
> +  For CPUs support for 64-byte loads and stores without status (FEAT_LS64):
> +
> +  - If the kernel is entered at EL1 and EL2 is present:
> +
> +    - HCRX_EL2.EnALS (bit 1) must be initialised to 0b1.
> +
> +  For CPUs support for 64-byte loads and stores with status (FEAT_LS64_V):
> +
> +  - If the kernel is entered at EL1 and EL2 is present:
> +
> +    - HCRX_EL2.EnASR (bit 2) must be initialised to 0b1.
> +
> +  For CPUs support for 64-byte EL0 stores with status (FEAT_LS64_ACCDATA):
> +
> +  - If EL3 is present:
> +
> +    - SCR_EL3.EnAS0 (bit 36) must be initialised to 0b1.
> +
> +    - SCR_EL3.ADEn (bit 37) must be initialised to 0b1.
> +
> +  - If the kernel is entered at EL1 and EL2 is present:
> +
> +    - HCRX_EL2.EnAS0 (bit 0) must be initialised to 0b1.
> +
> +    - HFGRTR_EL2.nACCDATA_EL1 (bit 50) must be initialised to 0b1.
> +
> +    - HFGWTR_EL2.nACCDATA_EL1 (bit 50) must be initialised to 0b1.
> +
>  The requirements described above for CPU mode, caches, MMUs, architected
>  timers, coherency and system registers apply to all CPUs.  All CPUs must
>  enter the kernel in the same exception level.  Where the values documented
> diff --git a/Documentation/arch/arm64/elf_hwcaps.rst b/Documentation/arch/arm64/elf_hwcaps.rst
> index 2ff922a406ad..6cb2594f0803 100644
> --- a/Documentation/arch/arm64/elf_hwcaps.rst
> +++ b/Documentation/arch/arm64/elf_hwcaps.rst
> @@ -372,6 +372,15 @@ HWCAP2_SME_SF8DP4
>  HWCAP2_POE
>      Functionality implied by ID_AA64MMFR3_EL1.S1POE == 0b0001.
>  
> +HWCAP3_LS64
> +    Functionality implied by ID_AA64ISAR1_EL1.LS64 == 0b0001.
> +
> +HWCAP3_LS64_V
> +    Functionality implied by ID_AA64ISAR1_EL1.LS64 == 0b0010.
> +
> +HWCAP3_LS64_ACCDATA
> +    Functionality implied by ID_AA64ISAR1_EL1.LS64 == 0b0011.
> +

I don't mind the two others, but I seriously question exposing ST64BV0
to userspace. How is ACCDATA_EL1 populated? How is it context-switched?

As it stands, this either does the wrong thing by always having the
low 32bit set to an UNKNOWN value, or actively leaks kernel data.
TBH, I don't see it being usable in practice (the more I read this
part of the architecture, the more broken it looks).

Thanks,

	M.

-- 
Without deviation from the norm, progress is not possible.