Skip to content

Commit d521271

Browse files
mrutland-armgregkh
authored andcommitted
arm64: smccc: Remove broken support for SMCCCv1.3 SVE discard hint
commit 8c462d5 upstream. SMCCCv1.3 added a hint bit which callers can set in an SMCCC function ID (AKA "FID") to indicate that it is acceptable for the SMCCC implementation to discard SVE and/or SME state over a specific SMCCC call. The kernel support for using this hint is broken and SMCCC calls may clobber the SVE and/or SME state of arbitrary tasks, though FPSIMD state is unaffected. The kernel support is intended to use the hint when there is no SVE or SME state to save, and to do this it checks whether TIF_FOREIGN_FPSTATE is set or TIF_SVE is clear in assembly code: | ldr <flags>, [<current_task>, #TSK_TI_FLAGS] | tbnz <flags>, #TIF_FOREIGN_FPSTATE, 1f // Any live FP state? | tbnz <flags>, #TIF_SVE, 2f // Does that state include SVE? | | 1: orr <fid>, <fid>, ARM_SMCCC_1_3_SVE_HINT | 2: | << SMCCC call using FID >> This is not safe as-is: (1) SMCCC calls can be made in a preemptible context and preemption can result in TIF_FOREIGN_FPSTATE being set or cleared at arbitrary points in time. Thus checking for TIF_FOREIGN_FPSTATE provides no guarantee. (2) TIF_FOREIGN_FPSTATE only indicates that the live FP/SVE/SME state in the CPU does not belong to the current task, and does not indicate that clobbering this state is acceptable. When the live CPU state is clobbered it is necessary to update fpsimd_last_state.st to ensure that a subsequent context switch will reload FP/SVE/SME state from memory rather than consuming the clobbered state. This and the SMCCC call itself must happen in a critical section with preemption disabled to avoid races. (3) Live SVE/SME state can exist with TIF_SVE clear (e.g. with only TIF_SME set), and checking TIF_SVE alone is insufficient. Remove the broken support for the SMCCCv1.3 SVE saving hint. This is effectively a revert of commits: * cfa7ff9 ("arm64: smccc: Support SMCCC v1.3 SVE register saving hint") * a7c3acc ("arm64: smccc: Save lr before calling __arm_smccc_sve_check()") ... leaving behind the ARM_SMCCC_VERSION_1_3 and ARM_SMCCC_1_3_SVE_HINT definitions, since these are simply definitions from the SMCCC specification, and the latter is used in KVM via ARM_SMCCC_CALL_HINTS. If we want to bring this back in future, we'll probably want to handle this logic in C where we can use all the usual FPSIMD/SVE/SME helper functions, and that'll likely require some rework of the SMCCC code and/or its callers. Fixes: cfa7ff9 ("arm64: smccc: Support SMCCC v1.3 SVE register saving hint") Signed-off-by: Mark Rutland <[email protected]> Cc: Ard Biesheuvel <[email protected]> Cc: Catalin Marinas <[email protected]> Cc: Marc Zyngier <[email protected]> Cc: Mark Brown <[email protected]> Cc: Will Deacon <[email protected]> Cc: [email protected] Reviewed-by: Mark Brown <[email protected]> Link: https://lore.kernel.org/r/[email protected] Signed-off-by: Will Deacon <[email protected]> Signed-off-by: Greg Kroah-Hartman <[email protected]>
1 parent fea1062 commit d521271

File tree

3 files changed

+6
-65
lines changed

3 files changed

+6
-65
lines changed

arch/arm64/kernel/smccc-call.S

Lines changed: 3 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -7,48 +7,19 @@
77

88
#include <asm/asm-offsets.h>
99
#include <asm/assembler.h>
10-
#include <asm/thread_info.h>
11-
12-
/*
13-
* If we have SMCCC v1.3 and (as is likely) no SVE state in
14-
* the registers then set the SMCCC hint bit to say there's no
15-
* need to preserve it. Do this by directly adjusting the SMCCC
16-
* function value which is already stored in x0 ready to be called.
17-
*/
18-
SYM_FUNC_START(__arm_smccc_sve_check)
19-
20-
ldr_l x16, smccc_has_sve_hint
21-
cbz x16, 2f
22-
23-
get_current_task x16
24-
ldr x16, [x16, #TSK_TI_FLAGS]
25-
tbnz x16, #TIF_FOREIGN_FPSTATE, 1f // Any live FP state?
26-
tbnz x16, #TIF_SVE, 2f // Does that state include SVE?
27-
28-
1: orr x0, x0, ARM_SMCCC_1_3_SVE_HINT
29-
30-
2: ret
31-
SYM_FUNC_END(__arm_smccc_sve_check)
32-
EXPORT_SYMBOL(__arm_smccc_sve_check)
3310

3411
.macro SMCCC instr
35-
stp x29, x30, [sp, #-16]!
36-
mov x29, sp
37-
alternative_if ARM64_SVE
38-
bl __arm_smccc_sve_check
39-
alternative_else_nop_endif
4012
\instr #0
41-
ldr x4, [sp, #16]
13+
ldr x4, [sp]
4214
stp x0, x1, [x4, #ARM_SMCCC_RES_X0_OFFS]
4315
stp x2, x3, [x4, #ARM_SMCCC_RES_X2_OFFS]
44-
ldr x4, [sp, #24]
16+
ldr x4, [sp, #8]
4517
cbz x4, 1f /* no quirk structure */
4618
ldr x9, [x4, #ARM_SMCCC_QUIRK_ID_OFFS]
4719
cmp x9, #ARM_SMCCC_QUIRK_QCOM_A6
4820
b.ne 1f
4921
str x6, [x4, ARM_SMCCC_QUIRK_STATE_OFFS]
50-
1: ldp x29, x30, [sp], #16
51-
ret
22+
1: ret
5223
.endm
5324

5425
/*

drivers/firmware/smccc/smccc.c

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,6 @@ static u32 smccc_version = ARM_SMCCC_VERSION_1_0;
1616
static enum arm_smccc_conduit smccc_conduit = SMCCC_CONDUIT_NONE;
1717

1818
bool __ro_after_init smccc_trng_available = false;
19-
u64 __ro_after_init smccc_has_sve_hint = false;
2019
s32 __ro_after_init smccc_soc_id_version = SMCCC_RET_NOT_SUPPORTED;
2120
s32 __ro_after_init smccc_soc_id_revision = SMCCC_RET_NOT_SUPPORTED;
2221

@@ -28,9 +27,6 @@ void __init arm_smccc_version_init(u32 version, enum arm_smccc_conduit conduit)
2827
smccc_conduit = conduit;
2928

3029
smccc_trng_available = smccc_probe_trng();
31-
if (IS_ENABLED(CONFIG_ARM64_SVE) &&
32-
smccc_version >= ARM_SMCCC_VERSION_1_3)
33-
smccc_has_sve_hint = true;
3430

3531
if ((smccc_version >= ARM_SMCCC_VERSION_1_2) &&
3632
(smccc_conduit != SMCCC_CONDUIT_NONE)) {

include/linux/arm-smccc.h

Lines changed: 3 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -227,8 +227,6 @@ u32 arm_smccc_get_version(void);
227227

228228
void __init arm_smccc_version_init(u32 version, enum arm_smccc_conduit conduit);
229229

230-
extern u64 smccc_has_sve_hint;
231-
232230
/**
233231
* arm_smccc_get_soc_id_version()
234232
*
@@ -326,15 +324,6 @@ struct arm_smccc_quirk {
326324
} state;
327325
};
328326

329-
/**
330-
* __arm_smccc_sve_check() - Set the SVE hint bit when doing SMC calls
331-
*
332-
* Sets the SMCCC hint bit to indicate if there is live state in the SVE
333-
* registers, this modifies x0 in place and should never be called from C
334-
* code.
335-
*/
336-
asmlinkage unsigned long __arm_smccc_sve_check(unsigned long x0);
337-
338327
/**
339328
* __arm_smccc_smc() - make SMC calls
340329
* @a0-a7: arguments passed in registers 0 to 7
@@ -402,20 +391,6 @@ asmlinkage void __arm_smccc_hvc(unsigned long a0, unsigned long a1,
402391

403392
#endif
404393

405-
/* nVHE hypervisor doesn't have a current thread so needs separate checks */
406-
#if defined(CONFIG_ARM64_SVE) && !defined(__KVM_NVHE_HYPERVISOR__)
407-
408-
#define SMCCC_SVE_CHECK ALTERNATIVE("nop \n", "bl __arm_smccc_sve_check \n", \
409-
ARM64_SVE)
410-
#define smccc_sve_clobbers "x16", "x30", "cc",
411-
412-
#else
413-
414-
#define SMCCC_SVE_CHECK
415-
#define smccc_sve_clobbers
416-
417-
#endif
418-
419394
#define __constraint_read_2 "r" (arg0)
420395
#define __constraint_read_3 __constraint_read_2, "r" (arg1)
421396
#define __constraint_read_4 __constraint_read_3, "r" (arg2)
@@ -486,12 +461,11 @@ asmlinkage void __arm_smccc_hvc(unsigned long a0, unsigned long a1,
486461
register unsigned long r3 asm("r3"); \
487462
CONCATENATE(__declare_arg_, \
488463
COUNT_ARGS(__VA_ARGS__))(__VA_ARGS__); \
489-
asm volatile(SMCCC_SVE_CHECK \
490-
inst "\n" : \
464+
asm volatile(inst "\n" : \
491465
"=r" (r0), "=r" (r1), "=r" (r2), "=r" (r3) \
492466
: CONCATENATE(__constraint_read_, \
493467
COUNT_ARGS(__VA_ARGS__)) \
494-
: smccc_sve_clobbers "memory"); \
468+
: "memory"); \
495469
if (___res) \
496470
*___res = (typeof(*___res)){r0, r1, r2, r3}; \
497471
} while (0)
@@ -540,7 +514,7 @@ asmlinkage void __arm_smccc_hvc(unsigned long a0, unsigned long a1,
540514
asm ("" : \
541515
: CONCATENATE(__constraint_read_, \
542516
COUNT_ARGS(__VA_ARGS__)) \
543-
: smccc_sve_clobbers "memory"); \
517+
: "memory"); \
544518
if (___res) \
545519
___res->a0 = SMCCC_RET_NOT_SUPPORTED; \
546520
} while (0)

0 commit comments

Comments
 (0)