* [PATCH v1 0/2] arm64: Implement SMCCC v1.3 SVE register saving hint @ 2021-05-18 10:09 Mark Brown 2021-05-18 10:09 ` [PATCH v1 1/2] arm64: asmlinkage: Enable use of BTI_C macro in SYM_CODE Mark Brown 2021-05-18 10:09 ` [PATCH v1 2/2] arm64: smccc: Support SMCCC v1.3 SVE register saving hint Mark Brown 0 siblings, 2 replies; 10+ messages in thread From: Mark Brown @ 2021-05-18 10:09 UTC (permalink / raw) To: Catalin Marinas, Will Deacon Cc: Mark Rutland, Lorenzo Pieralisi, Sudeep Holla, linux-arm-kernel, Mark Brown SMCCC v1.3 provides support for a flag which allows the caller to say that there is no state that needs to be preserved in the SVE registers, meaning that the called code can skip doing this, especially with larger vector lengths this can save a noticable amount of work in the event that the state needs to be saved. Implement support for this, using the TIF_ flags to report if there is live SVE state present. Mark Brown (2): arm64: asmlinkage: Enable use of BTI_C macro in SYM_CODE arm64: smccc: Support SMCCC v1.3 SVE register saving hint arch/arm64/include/asm/linkage.h | 4 ++++ arch/arm64/kernel/smccc-call.S | 39 ++++++++++++++++++++++++++++++++ drivers/firmware/smccc/smccc.c | 4 ++++ include/linux/arm-smccc.h | 22 ++++++++++++++++-- 4 files changed, 67 insertions(+), 2 deletions(-) base-commit: d07f6ca923ea0927a1024dfccafc5b53b61cfecc -- 2.20.1 _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH v1 1/2] arm64: asmlinkage: Enable use of BTI_C macro in SYM_CODE 2021-05-18 10:09 [PATCH v1 0/2] arm64: Implement SMCCC v1.3 SVE register saving hint Mark Brown @ 2021-05-18 10:09 ` Mark Brown 2021-05-18 10:09 ` [PATCH v1 2/2] arm64: smccc: Support SMCCC v1.3 SVE register saving hint Mark Brown 1 sibling, 0 replies; 10+ messages in thread From: Mark Brown @ 2021-05-18 10:09 UTC (permalink / raw) To: Catalin Marinas, Will Deacon Cc: Mark Rutland, Lorenzo Pieralisi, Sudeep Holla, linux-arm-kernel, Mark Brown SYM_CODE sections may require BTI landing pads but won't have them automatically generated by the symbol definiton macros. Make an empty version of the BTI_C macro available so that they can more easily create the landing pads if they need them. Signed-off-by: Mark Brown <broonie@kernel.org> --- arch/arm64/include/asm/linkage.h | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/arch/arm64/include/asm/linkage.h b/arch/arm64/include/asm/linkage.h index ba89a9af820a..d98f9af8b8ec 100644 --- a/arch/arm64/include/asm/linkage.h +++ b/arch/arm64/include/asm/linkage.h @@ -42,6 +42,10 @@ SYM_START(name, SYM_L_WEAK, SYM_A_NONE) \ BTI_C +#else + +#define BTI_C + #endif /* -- 2.20.1 _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH v1 2/2] arm64: smccc: Support SMCCC v1.3 SVE register saving hint 2021-05-18 10:09 [PATCH v1 0/2] arm64: Implement SMCCC v1.3 SVE register saving hint Mark Brown 2021-05-18 10:09 ` [PATCH v1 1/2] arm64: asmlinkage: Enable use of BTI_C macro in SYM_CODE Mark Brown @ 2021-05-18 10:09 ` Mark Brown 2021-05-18 10:53 ` Marc Zyngier 2021-05-18 16:43 ` Sudeep Holla 1 sibling, 2 replies; 10+ messages in thread From: Mark Brown @ 2021-05-18 10:09 UTC (permalink / raw) To: Catalin Marinas, Will Deacon Cc: Mark Rutland, Lorenzo Pieralisi, Sudeep Holla, linux-arm-kernel, Mark Brown 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 slightly non-standard function which we call immediately before issuing the SMCCC call to make our checks. This causes an extra function call on any system built with SVE support, and extra checks when SVE is detected at runtime, but these costs are expected to be reasonable in the context of doing a SMCCC call in the first place. Signed-off-by: Mark Brown <broonie@kernel.org> --- arch/arm64/kernel/smccc-call.S | 39 ++++++++++++++++++++++++++++++++++ drivers/firmware/smccc/smccc.c | 4 ++++ include/linux/arm-smccc.h | 22 +++++++++++++++++-- 3 files changed, 63 insertions(+), 2 deletions(-) diff --git a/arch/arm64/kernel/smccc-call.S b/arch/arm64/kernel/smccc-call.S index d62447964ed9..217bfb03a6c9 100644 --- a/arch/arm64/kernel/smccc-call.S +++ b/arch/arm64/kernel/smccc-call.S @@ -7,8 +7,47 @@ #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 x15 and 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_CODE_START(__smccc_sve_check) + BTI_C + +alternative_if ARM64_SVE + + adrp x15, smccc_has_sve_hint + ldr x15, [x15] + cbz x15, 1f + + get_current_task x15 + ldr x15, [x15, #TSK_TI_FLAGS] + and x16, x15, #TIF_FOREIGN_FPSTATE // Any live FP state? + cbnz x16, 1f + mov x16, #TIF_SVE // Does that state include SVE? + and x16, x15, x16 + cbnz x16, 2f + +1: orr x0, x0, ARM_SMCCC_1_3_SVE_HINT +alternative_else_nop_endif + +2: ret +SYM_CODE_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..611f3bc9c131 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,18 @@ asmlinkage void __arm_smccc_hvc(unsigned long a0, unsigned long a1, #endif +#ifdef CONFIG_ARM64_SVE + +#define SMCCC_SVE_CHECK "bl __smccc_sve_check \n" +#define smccc_sve_clobbers "x15", "x16", "lr", + +#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 +381,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 +396,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 _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH v1 2/2] arm64: smccc: Support SMCCC v1.3 SVE register saving hint 2021-05-18 10:09 ` [PATCH v1 2/2] arm64: smccc: Support SMCCC v1.3 SVE register saving hint Mark Brown @ 2021-05-18 10:53 ` Marc Zyngier 2021-05-18 11:53 ` Mark Brown 2021-05-18 16:53 ` Ard Biesheuvel 2021-05-18 16:43 ` Sudeep Holla 1 sibling, 2 replies; 10+ messages in thread From: Marc Zyngier @ 2021-05-18 10:53 UTC (permalink / raw) To: Mark Brown Cc: Catalin Marinas, Will Deacon, Mark Rutland, Lorenzo Pieralisi, Sudeep Holla, linux-arm-kernel Hi Mark, On Tue, 18 May 2021 11:09:19 +0100, Mark Brown <broonie@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 > slightly non-standard function which we call immediately before issuing the > SMCCC call to make our checks. This causes an extra function call on any > system built with SVE support, and extra checks when SVE is detected at > runtime, but these costs are expected to be reasonable in the context of > doing a SMCCC call in the first place. > > Signed-off-by: Mark Brown <broonie@kernel.org> > --- > arch/arm64/kernel/smccc-call.S | 39 ++++++++++++++++++++++++++++++++++ > drivers/firmware/smccc/smccc.c | 4 ++++ > include/linux/arm-smccc.h | 22 +++++++++++++++++-- > 3 files changed, 63 insertions(+), 2 deletions(-) > > diff --git a/arch/arm64/kernel/smccc-call.S b/arch/arm64/kernel/smccc-call.S > index d62447964ed9..217bfb03a6c9 100644 > --- a/arch/arm64/kernel/smccc-call.S > +++ b/arch/arm64/kernel/smccc-call.S > @@ -7,8 +7,47 @@ > > #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 x15 and 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_CODE_START(__smccc_sve_check) > + BTI_C > + > +alternative_if ARM64_SVE > + > + adrp x15, smccc_has_sve_hint The adrp instruction will give you a 4kB aligned address, which results in 1 chance out of 512 to point to the right location. The adr_l macro is probably what you want. > + ldr x15, [x15] > + cbz x15, 1f > + > + get_current_task x15 > + ldr x15, [x15, #TSK_TI_FLAGS] > + and x16, x15, #TIF_FOREIGN_FPSTATE // Any live FP state? > + cbnz x16, 1f > + mov x16, #TIF_SVE // Does that state include SVE? > + and x16, x15, x16 > + cbnz x16, 2f > + > +1: orr x0, x0, ARM_SMCCC_1_3_SVE_HINT > +alternative_else_nop_endif > + > +2: ret > +SYM_CODE_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..611f3bc9c131 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,18 @@ asmlinkage void __arm_smccc_hvc(unsigned long a0, unsigned long a1, > > #endif > > +#ifdef CONFIG_ARM64_SVE > + > +#define SMCCC_SVE_CHECK "bl __smccc_sve_check \n" > +#define smccc_sve_clobbers "x15", "x16", "lr", > + > +#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 +381,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 +396,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 \ What happens when this is called from a context where __smccc_sve_check isn't mapped, nor does "current" mean anything? See arch/arm64/kvm/hyp/nvhe/psci-relay.c for an example. > + inst "\n" : \ > "=r" (r0), "=r" (r1), "=r" (r2), "=r" (r3) \ > __constraints(__count_args(__VA_ARGS__))); \ > if (___res) \ Thanks, M. -- Without deviation from the norm, progress is not possible. _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v1 2/2] arm64: smccc: Support SMCCC v1.3 SVE register saving hint 2021-05-18 10:53 ` Marc Zyngier @ 2021-05-18 11:53 ` Mark Brown 2021-05-18 12:14 ` Marc Zyngier 2021-05-18 16:53 ` Ard Biesheuvel 1 sibling, 1 reply; 10+ messages in thread From: Mark Brown @ 2021-05-18 11:53 UTC (permalink / raw) To: Marc Zyngier Cc: Catalin Marinas, Will Deacon, Mark Rutland, Lorenzo Pieralisi, Sudeep Holla, linux-arm-kernel [-- Attachment #1.1: Type: text/plain, Size: 1342 bytes --] On Tue, May 18, 2021 at 11:53:50AM +0100, Marc Zyngier wrote: > Mark Brown <broonie@kernel.org> wrote: > > +alternative_if ARM64_SVE > > + > > + adrp x15, smccc_has_sve_hint > The adrp instruction will give you a 4kB aligned address, which > results in 1 chance out of 512 to point to the right location. The > adr_l macro is probably what you want. Ouch, yes. This was working surprisingly well in my testing. > > 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 \ > What happens when this is called from a context where > __smccc_sve_check isn't mapped, nor does "current" mean anything? See > arch/arm64/kvm/hyp/nvhe/psci-relay.c for an example. Ah, oh dear. I have to admit I haven't entirely been able to follow the contexts the various bits of KVM run in yet, nor how much of the normal kernel environment is being maintained :/ . I do see some ifdefery with __KVM_NVHE_HYPERVISOR__ elsewhere which could be used to take care of that particular case either by providing a __hyp mapping or just not trying to set the flag there (the latter seems safer) but I'm guessing there's others. Do we have a reliable way of identifying such contexts? [-- Attachment #1.2: signature.asc --] [-- Type: application/pgp-signature, Size: 488 bytes --] [-- Attachment #2: Type: text/plain, Size: 176 bytes --] _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v1 2/2] arm64: smccc: Support SMCCC v1.3 SVE register saving hint 2021-05-18 11:53 ` Mark Brown @ 2021-05-18 12:14 ` Marc Zyngier 2021-05-18 13:01 ` Mark Brown 0 siblings, 1 reply; 10+ messages in thread From: Marc Zyngier @ 2021-05-18 12:14 UTC (permalink / raw) To: Mark Brown Cc: Catalin Marinas, Will Deacon, Mark Rutland, Lorenzo Pieralisi, Sudeep Holla, linux-arm-kernel On Tue, 18 May 2021 12:53:48 +0100, Mark Brown <broonie@kernel.org> wrote: > > [1 <text/plain; us-ascii (7bit)>] > On Tue, May 18, 2021 at 11:53:50AM +0100, Marc Zyngier wrote: > > Mark Brown <broonie@kernel.org> wrote: > > > > +alternative_if ARM64_SVE > > > + > > > + adrp x15, smccc_has_sve_hint > > > The adrp instruction will give you a 4kB aligned address, which > > results in 1 chance out of 512 to point to the right location. The > > adr_l macro is probably what you want. > > Ouch, yes. This was working surprisingly well in my testing. It always does. In my experience, it starts failing the minute you push the code out. Sod's law. > > > > 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 \ > > > What happens when this is called from a context where > > __smccc_sve_check isn't mapped, nor does "current" mean anything? See > > arch/arm64/kvm/hyp/nvhe/psci-relay.c for an example. > > Ah, oh dear. I have to admit I haven't entirely been able to follow the > contexts the various bits of KVM run in yet, nor how much of the normal > kernel environment is being maintained :/ . A good approximation is *none*. We have the hypervisor text, and some data that we map as required. But unless the functions have been compiled as part of the hypervisor object, this won't go anywhere. I'm surprised it would even link, TBH, due to the symbol repainting that we have to prevent linking against unsuspecting kernel symbols. > I do see some ifdefery with __KVM_NVHE_HYPERVISOR__ elsewhere which > could be used to take care of that particular case either by > providing a __hyp mapping or just not trying to set the flag there > (the latter seems safer) but I'm guessing there's others. Do we > have a reliable way of identifying such contexts? __KVM_NVHE_HYPERVISOR__ usually is a good indication that we're compiling for the nVHE EL2 object. I guess that skipping the optimisation would be good enough for KVM, until we decide to provide a nVHE-specific helper that uses the private per-cpu information. M. -- Without deviation from the norm, progress is not possible. _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v1 2/2] arm64: smccc: Support SMCCC v1.3 SVE register saving hint 2021-05-18 12:14 ` Marc Zyngier @ 2021-05-18 13:01 ` Mark Brown 0 siblings, 0 replies; 10+ messages in thread From: Mark Brown @ 2021-05-18 13:01 UTC (permalink / raw) To: Marc Zyngier Cc: Catalin Marinas, Will Deacon, Mark Rutland, Lorenzo Pieralisi, Sudeep Holla, linux-arm-kernel [-- Attachment #1.1: Type: text/plain, Size: 885 bytes --] On Tue, May 18, 2021 at 01:14:30PM +0100, Marc Zyngier wrote: > Mark Brown <broonie@kernel.org> wrote: > > I do see some ifdefery with __KVM_NVHE_HYPERVISOR__ elsewhere which > > could be used to take care of that particular case either by > > providing a __hyp mapping or just not trying to set the flag there > > (the latter seems safer) but I'm guessing there's others. Do we > > have a reliable way of identifying such contexts? > __KVM_NVHE_HYPERVISOR__ usually is a good indication that we're > compiling for the nVHE EL2 object. I guess that skipping the > optimisation would be good enough for KVM, until we decide to provide > a nVHE-specific helper that uses the private per-cpu information. Yes, I think skipping it for now is going to be safest and it's certainly easiest. I didn't *spot* any other affected cases, but eqally I didn't spot this one first time around. [-- Attachment #1.2: signature.asc --] [-- Type: application/pgp-signature, Size: 488 bytes --] [-- Attachment #2: Type: text/plain, Size: 176 bytes --] _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v1 2/2] arm64: smccc: Support SMCCC v1.3 SVE register saving hint 2021-05-18 10:53 ` Marc Zyngier 2021-05-18 11:53 ` Mark Brown @ 2021-05-18 16:53 ` Ard Biesheuvel 1 sibling, 0 replies; 10+ messages in thread From: Ard Biesheuvel @ 2021-05-18 16:53 UTC (permalink / raw) To: Marc Zyngier Cc: Mark Brown, Catalin Marinas, Will Deacon, Mark Rutland, Lorenzo Pieralisi, Sudeep Holla, Linux ARM On Tue, 18 May 2021 at 12:55, Marc Zyngier <maz@kernel.org> wrote: > > Hi Mark, > > On Tue, 18 May 2021 11:09:19 +0100, > Mark Brown <broonie@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 > > slightly non-standard function which we call immediately before issuing the > > SMCCC call to make our checks. This causes an extra function call on any > > system built with SVE support, and extra checks when SVE is detected at > > runtime, but these costs are expected to be reasonable in the context of > > doing a SMCCC call in the first place. > > > > Signed-off-by: Mark Brown <broonie@kernel.org> > > --- > > arch/arm64/kernel/smccc-call.S | 39 ++++++++++++++++++++++++++++++++++ > > drivers/firmware/smccc/smccc.c | 4 ++++ > > include/linux/arm-smccc.h | 22 +++++++++++++++++-- > > 3 files changed, 63 insertions(+), 2 deletions(-) > > > > diff --git a/arch/arm64/kernel/smccc-call.S b/arch/arm64/kernel/smccc-call.S > > index d62447964ed9..217bfb03a6c9 100644 > > --- a/arch/arm64/kernel/smccc-call.S > > +++ b/arch/arm64/kernel/smccc-call.S > > @@ -7,8 +7,47 @@ > > > > #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 x15 and 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_CODE_START(__smccc_sve_check) > > + BTI_C > > + > > +alternative_if ARM64_SVE > > + > > + adrp x15, smccc_has_sve_hint > > The adrp instruction will give you a 4kB aligned address, which > results in 1 chance out of 512 to point to the right location. The > adr_l macro is probably what you want. > ... or use ldr_l, in which case the ldr below can be omitted as well. > > + ldr x15, [x15] > > + cbz x15, 1f > > + > > + get_current_task x15 > > + ldr x15, [x15, #TSK_TI_FLAGS] > > + and x16, x15, #TIF_FOREIGN_FPSTATE // Any live FP state? > > + cbnz x16, 1f > > + mov x16, #TIF_SVE // Does that state include SVE? > > + and x16, x15, x16 > > + cbnz x16, 2f > > + > > +1: orr x0, x0, ARM_SMCCC_1_3_SVE_HINT > > +alternative_else_nop_endif > > + > > +2: ret > > +SYM_CODE_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..611f3bc9c131 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,18 @@ asmlinkage void __arm_smccc_hvc(unsigned long a0, unsigned long a1, > > > > #endif > > > > +#ifdef CONFIG_ARM64_SVE > > + > > +#define SMCCC_SVE_CHECK "bl __smccc_sve_check \n" > > +#define smccc_sve_clobbers "x15", "x16", "lr", > > + > > +#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 +381,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 +396,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 \ > > What happens when this is called from a context where > __smccc_sve_check isn't mapped, nor does "current" mean anything? See > arch/arm64/kvm/hyp/nvhe/psci-relay.c for an example. > > > + inst "\n" : \ > > "=r" (r0), "=r" (r1), "=r" (r2), "=r" (r3) \ > > __constraints(__count_args(__VA_ARGS__))); \ > > if (___res) \ > > Thanks, > > M. > > -- > Without deviation from the norm, progress is not possible. > > _______________________________________________ > linux-arm-kernel mailing list > linux-arm-kernel@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v1 2/2] arm64: smccc: Support SMCCC v1.3 SVE register saving hint 2021-05-18 10:09 ` [PATCH v1 2/2] arm64: smccc: Support SMCCC v1.3 SVE register saving hint Mark Brown 2021-05-18 10:53 ` Marc Zyngier @ 2021-05-18 16:43 ` Sudeep Holla 2021-05-18 16:55 ` Mark Brown 1 sibling, 1 reply; 10+ messages in thread From: Sudeep Holla @ 2021-05-18 16:43 UTC (permalink / raw) To: Mark Brown Cc: Catalin Marinas, Will Deacon, Mark Rutland, Lorenzo Pieralisi, Marc Zyngier, linux-arm-kernel Hi Mark, On Tue, May 18, 2021 at 11:09:19AM +0100, Mark Brown 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 > slightly non-standard function which we call immediately before issuing the > SMCCC call to make our checks. This causes an extra function call on any > system built with SVE support, and extra checks when SVE is detected at > runtime, but these costs are expected to be reasonable in the context of > doing a SMCCC call in the first place. > I am adding another interface that extends the input/output register set for v1.2 and above. I assume we may need these changes there too. I have cc-ed you in the updated version I just posted. The lore link is not yet available, so sharing the one for earlier version[1] -- Regards, Sudeep [1] https://lore.kernel.org/r/20210505093843.3308691-2-sudeep.holla@arm.com/ _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v1 2/2] arm64: smccc: Support SMCCC v1.3 SVE register saving hint 2021-05-18 16:43 ` Sudeep Holla @ 2021-05-18 16:55 ` Mark Brown 0 siblings, 0 replies; 10+ messages in thread From: Mark Brown @ 2021-05-18 16:55 UTC (permalink / raw) To: Sudeep Holla Cc: Catalin Marinas, Will Deacon, Mark Rutland, Lorenzo Pieralisi, Marc Zyngier, linux-arm-kernel [-- Attachment #1.1: Type: text/plain, Size: 633 bytes --] On Tue, May 18, 2021 at 05:43:02PM +0100, Sudeep Holla wrote: > I am adding another interface that extends the input/output register set > for v1.2 and above. I assume we may need these changes there too. I have > cc-ed you in the updated version I just posted. The lore link is not yet > available, so sharing the one for earlier version[1] Thanks, should just be the same update as we do in the existing SMCCC macro to call __smccc_sve_check from the new SMCCC_1_2 macro before we load the registers from the looks of it. It won't do any harm to miss it, it's just an optimisation, and the patches probably don't even conflict. [-- Attachment #1.2: signature.asc --] [-- Type: application/pgp-signature, Size: 488 bytes --] [-- Attachment #2: Type: text/plain, Size: 176 bytes --] _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2021-05-18 16:58 UTC | newest] Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2021-05-18 10:09 [PATCH v1 0/2] arm64: Implement SMCCC v1.3 SVE register saving hint Mark Brown 2021-05-18 10:09 ` [PATCH v1 1/2] arm64: asmlinkage: Enable use of BTI_C macro in SYM_CODE Mark Brown 2021-05-18 10:09 ` [PATCH v1 2/2] arm64: smccc: Support SMCCC v1.3 SVE register saving hint Mark Brown 2021-05-18 10:53 ` Marc Zyngier 2021-05-18 11:53 ` Mark Brown 2021-05-18 12:14 ` Marc Zyngier 2021-05-18 13:01 ` Mark Brown 2021-05-18 16:53 ` Ard Biesheuvel 2021-05-18 16:43 ` Sudeep Holla 2021-05-18 16:55 ` Mark Brown
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.