[PATCH v3] arm64: smccc: Support SMCCC v1.3 SVE register saving hint
Ard Biesheuvel
ardb at kernel.org
Mon May 24 03:13:17 PDT 2021
On Wed, 19 May 2021 at 19:15, Mark Brown <broonie at kernel.org> wrote:
>
> SMCCC v1.2 requires that all SVE state be preserved over SMC calls which
> introduces substantial overhead in the common case where there is no SVE
> state in the registers. To avoid this SMCCC v1.3 introduces a flag which
> allows the caller to say that there is no state that needs to be preserved
> in the registers. Make use of this flag, setting it if the SMCCC version
> indicates support for it and the TIF_ flags indicate that there is no live
> SVE state in the registers, this avoids placing any constraints on when
> SMCCC calls can be done or triggering extra saving and reloading of SVE
> register state in the kernel.
>
> This would be straightforward enough except for the rather entertaining
> inline assembly we use to do SMCCC v1.1 calls to allow us to take advantage
> of the limited number of registers it clobbers. Deal with this by having a
> function which we call immediately before issuing the SMCCC call to make
> our checks and set the flag. Using alternatives the overhead if SVE is
> supported but not detected at runtime can be reduced to a single NOP.
>
> Signed-off-by: Mark Brown <broonie at kernel.org>
I think this looks fine now, with one nit below.
Reviewed-by: Ard Biesheuvel <ardb at kernel.org>
> ---
> arch/arm64/kernel/smccc-call.S | 31 +++++++++++++++++++++++++++++++
> drivers/firmware/smccc/smccc.c | 4 ++++
> include/linux/arm-smccc.h | 24 ++++++++++++++++++++++--
> 3 files changed, 57 insertions(+), 2 deletions(-)
>
> diff --git a/arch/arm64/kernel/smccc-call.S b/arch/arm64/kernel/smccc-call.S
> index d62447964ed9..d6f2e72b7cba 100644
> --- a/arch/arm64/kernel/smccc-call.S
> +++ b/arch/arm64/kernel/smccc-call.S
> @@ -7,8 +7,39 @@
>
> #include <asm/asm-offsets.h>
> #include <asm/assembler.h>
> +#include <asm/thread_info.h>
> +
> +/*
> + * If we have SMCCC v1.3 and (as is likely) no SVE state in
> + * the registers then set the SMCCC hint bit to say there's no
> + * need to preserve it. Do this by directly adjusting the SMCCC
> + * function value which is already stored in x0 ready to be called.
> + *
> + * Since we need scratch registers but wish to avoid having to handle
> + * the stack we expect the caller to preserve x16 if needed, the only
> + * callers are expected to be the call below and the inline asm in
> + * linux/arm-smccc.h for SMCCC 1.1 and later calls.
> + */
> +SYM_FUNC_START(__smccc_sve_check)
> +
> + ldr_l x16, smccc_has_sve_hint
> + cbz x16, 2f
> +
> + get_current_task x16
> + ldr x16, [x16, #TSK_TI_FLAGS]
> + tbnz x16, #TIF_FOREIGN_FPSTATE, 1f // Any live FP state?
> + tbnz x16, #TIF_SVE, 2f // Does that state include SVE?
> +
> +1: orr x0, x0, ARM_SMCCC_1_3_SVE_HINT
> +
> +2: ret
> +SYM_FUNC_END(__smccc_sve_check)
> +EXPORT_SYMBOL(__smccc_sve_check)
>
> .macro SMCCC instr
> +alternative_if ARM64_SVE
> + bl __smccc_sve_check
> +alternative_else_nop_endif
> \instr #0
> ldr x4, [sp]
> stp x0, x1, [x4, #ARM_SMCCC_RES_X0_OFFS]
> diff --git a/drivers/firmware/smccc/smccc.c b/drivers/firmware/smccc/smccc.c
> index 028f81d702cc..9f937b125ab0 100644
> --- a/drivers/firmware/smccc/smccc.c
> +++ b/drivers/firmware/smccc/smccc.c
> @@ -15,6 +15,7 @@ static u32 smccc_version = ARM_SMCCC_VERSION_1_0;
> static enum arm_smccc_conduit smccc_conduit = SMCCC_CONDUIT_NONE;
>
> bool __ro_after_init smccc_trng_available = false;
> +u64 __ro_after_init smccc_has_sve_hint = false;
>
> void __init arm_smccc_version_init(u32 version, enum arm_smccc_conduit conduit)
> {
> @@ -22,6 +23,9 @@ void __init arm_smccc_version_init(u32 version, enum arm_smccc_conduit conduit)
> smccc_conduit = conduit;
>
> smccc_trng_available = smccc_probe_trng();
> + if (IS_ENABLED(CONFIG_ARM64_SVE) &&
> + smccc_version >= ARM_SMCCC_VERSION_1_3)
> + smccc_has_sve_hint = true;
> }
>
> enum arm_smccc_conduit arm_smccc_1_1_get_conduit(void)
> diff --git a/include/linux/arm-smccc.h b/include/linux/arm-smccc.h
> index 6861489a1890..b22966bb174a 100644
> --- a/include/linux/arm-smccc.h
> +++ b/include/linux/arm-smccc.h
> @@ -63,6 +63,9 @@
> #define ARM_SMCCC_VERSION_1_0 0x10000
> #define ARM_SMCCC_VERSION_1_1 0x10001
> #define ARM_SMCCC_VERSION_1_2 0x10002
> +#define ARM_SMCCC_VERSION_1_3 0x10003
> +
> +#define ARM_SMCCC_1_3_SVE_HINT 0x10000
>
> #define ARM_SMCCC_VERSION_FUNC_ID \
> ARM_SMCCC_CALL_VAL(ARM_SMCCC_FAST_CALL, \
> @@ -216,6 +219,8 @@ u32 arm_smccc_get_version(void);
>
> void __init arm_smccc_version_init(u32 version, enum arm_smccc_conduit conduit);
>
> +extern u64 smccc_has_sve_hint;
> +
> /**
> * struct arm_smccc_res - Result from SMC/HVC call
> * @a0-a3 result values from registers 0 to 3
> @@ -297,6 +302,20 @@ asmlinkage void __arm_smccc_hvc(unsigned long a0, unsigned long a1,
>
> #endif
>
> +/* nVHE hypervisor doesn't have a current thread so needs separate checks */
> +#if defined(CONFIG_ARM64_SVE) && !defined(__KVM_NVHE_HYPERVISOR__)
> +
> +#define SMCCC_SVE_CHECK ALTERNATIVE("nop \n", "bl __smccc_sve_check \n", \
> + ARM64_SVE)
> +#define smccc_sve_clobbers "x16", "lr",
> +
This should probably include "cc" as well, even though the only
interposing veneer we might reasonably expect here is a PLT generated
by the module loader, which doesn't touch the flags, nor does your
implementation of __smccc_sve_check().
However, the linker itself may insert veneers too, if the kernel image
grows beyond 128 MB, and the wording in the AAPCS64 about what such
veneers are permitted to do is ambiguous:
"Any veneer inserted must preserve the contents of all registers
except IP0, IP1 (r16, r17) and the condition code flags"
which could mean it should preserve
[the contents of all registers except IP0, IP1 (r16, r17)]
AND
[the condition code flags]
or alternatively, that it should preserve
the contents of all registers except
[IP0, IP1 (r16, r17) AND the condition code flags]
The subsequent sentence reads
"a conforming program must assume that a veneer that alters IP0 and/or
IP1 may be inserted at any branch instruction that is exposed to a
relocation that supports long branches"
which suggests that the flags should be preserved, but I really don't
know which interpretation was intended here by the author. Note that
shared libraries in user space that use lazy binding will carry
veneers that make non-trivial calls into libdl.so, so I assume that
the flags should be considered clobbered.
> +#else
> +
> +#define SMCCC_SVE_CHECK
> +#define smccc_sve_clobbers
> +
> +#endif
> +
> #define ___count_args(_0, _1, _2, _3, _4, _5, _6, _7, _8, x, ...) x
>
> #define __count_args(...) \
> @@ -364,7 +383,7 @@ asmlinkage void __arm_smccc_hvc(unsigned long a0, unsigned long a1,
>
> #define ___constraints(count) \
> : __constraint_read_ ## count \
> - : "memory"
> + : smccc_sve_clobbers "memory"
> #define __constraints(count) ___constraints(count)
>
> /*
> @@ -379,7 +398,8 @@ asmlinkage void __arm_smccc_hvc(unsigned long a0, unsigned long a1,
> register unsigned long r2 asm("r2"); \
> register unsigned long r3 asm("r3"); \
> __declare_args(__count_args(__VA_ARGS__), __VA_ARGS__); \
> - asm volatile(inst "\n" : \
> + asm volatile(SMCCC_SVE_CHECK \
> + inst "\n" : \
> "=r" (r0), "=r" (r1), "=r" (r2), "=r" (r3) \
> __constraints(__count_args(__VA_ARGS__))); \
> if (___res) \
> --
> 2.20.1
>
More information about the linux-arm-kernel
mailing list