From: Marc Zyngier <maz@kernel.org>
To: Mark Brown <broonie@kernel.org>
Cc: Catalin Marinas <catalin.marinas@arm.com>,
Will Deacon <will@kernel.org>,
Mark Rutland <mark.rutland@arm.com>,
Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>,
Sudeep Holla <sudeep.holla@arm.com>,
linux-arm-kernel@lists.infradead.org
Subject: Re: [PATCH v1 2/2] arm64: smccc: Support SMCCC v1.3 SVE register saving hint
Date: Tue, 18 May 2021 11:53:50 +0100 [thread overview]
Message-ID: <87pmxotirl.wl-maz@kernel.org> (raw)
In-Reply-To: <20210518100919.6674-3-broonie@kernel.org>
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
next prev parent reply other threads:[~2021-05-18 10:55 UTC|newest]
Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top
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 [this message]
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
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=87pmxotirl.wl-maz@kernel.org \
--to=maz@kernel.org \
--cc=broonie@kernel.org \
--cc=catalin.marinas@arm.com \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=lorenzo.pieralisi@arm.com \
--cc=mark.rutland@arm.com \
--cc=sudeep.holla@arm.com \
--cc=will@kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).