| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <aKRV8FEuO5ZOcEfn@willie-the-truck>
Date: Tue, 19 Aug 2025 11:46:08 +0100
From: Will Deacon <will@...nel.org>
To: perlarsen@...gle.com
Cc: Marc Zyngier <maz@...nel.org>, Oliver Upton <oliver.upton@...ux.dev>,
Joey Gouly <joey.gouly@....com>,
Suzuki K Poulose <suzuki.poulose@....com>,
Zenghui Yu <yuzenghui@...wei.com>,
Catalin Marinas <catalin.marinas@....com>,
Sudeep Holla <sudeep.holla@....com>,
linux-arm-kernel@...ts.infradead.org, kvmarm@...ts.linux.dev,
linux-kernel@...r.kernel.org, ahomescu@...gle.com,
armellel@...gle.com, arve@...roid.com, ayrton@...gle.com,
qperret@...gle.com, sebastianene@...gle.com, qwandor@...gle.com
Subject: Re: [PATCH v10 2/6] KVM: arm64: Use SMCCC 1.2 for FF-A
initialization and in host handler
On Sat, Aug 09, 2025 at 02:33:20AM +0000, Per Larsen via B4 Relay wrote:
> From: Per Larsen <perlarsen@...gle.com>
>
> SMCCC 1.1 and prior allows four registers to be sent back as a result
> of an FF-A interface. SMCCC 1.2 increases the number of results that can
> be sent back to 8 and 16 for 32-bit and 64-bit SMC/HVCs respectively.
>
> FF-A 1.0 references SMCCC 1.2 (reference [4] on page xi) and FF-A 1.2
> explicitly requires SMCCC 1.2 so it should be safe to use this version
> unconditionally. Moreover, it is simpler to implement FF-A features
> without having to worry about compatibility with SMCCC 1.1 and older.
>
> SMCCC 1.2 requires that SMC32/HVC32 from aarch64 mode preserves x8-x30
> but given that there is no reliable way to distinguish 32-bit/64-bit
> calls, we assume SMC64 unconditionally. This has the benefit of being
> consistent with the handling of calls that are passed through, i.e., not
> proxied. (A cleaner solution will become available in FF-A 1.3.)
>
> Update the FF-A initialization and host handler code to use SMCCC 1.2.
>
> Signed-off-by: Per Larsen <perlarsen@...gle.com>
> ---
> arch/arm64/kvm/hyp/nvhe/Makefile | 1 +
> arch/arm64/kvm/hyp/nvhe/ffa.c | 188 +++++++++++++++++++++++++--------------
> 2 files changed, 120 insertions(+), 69 deletions(-)
>
> diff --git a/arch/arm64/kvm/hyp/nvhe/Makefile b/arch/arm64/kvm/hyp/nvhe/Makefile
> index 0b0a68b663d4bd202a7036384bf8a1748cc97ca5..a244ec25f8c5bd0a744f7791102265323ecc681c 100644
> --- a/arch/arm64/kvm/hyp/nvhe/Makefile
> +++ b/arch/arm64/kvm/hyp/nvhe/Makefile
> @@ -27,6 +27,7 @@ hyp-obj-y := timer-sr.o sysreg-sr.o debug-sr.o switch.o tlb.o hyp-init.o host.o
> cache.o setup.o mm.o mem_protect.o sys_regs.o pkvm.o stacktrace.o ffa.o
> hyp-obj-y += ../vgic-v3-sr.o ../aarch32.o ../vgic-v2-cpuif-proxy.o ../entry.o \
> ../fpsimd.o ../hyp-entry.o ../exception.o ../pgtable.o
> +hyp-obj-y += ../../../kernel/smccc-call.o
> hyp-obj-$(CONFIG_LIST_HARDENED) += list_debug.o
> hyp-obj-y += $(lib-objs)
>
> diff --git a/arch/arm64/kvm/hyp/nvhe/ffa.c b/arch/arm64/kvm/hyp/nvhe/ffa.c
> index 2c199d40811efb5bfae199c4a67d8ae3d9307357..19239f133a1cfb86db1b85251035709481cdef5b 100644
> --- a/arch/arm64/kvm/hyp/nvhe/ffa.c
> +++ b/arch/arm64/kvm/hyp/nvhe/ffa.c
> @@ -71,36 +71,63 @@ static u32 hyp_ffa_version;
> static bool has_version_negotiated;
> static hyp_spinlock_t version_lock;
>
> -static void ffa_to_smccc_error(struct arm_smccc_res *res, u64 ffa_errno)
> +static void ffa_to_smccc_error(struct arm_smccc_1_2_regs *res, u64 ffa_errno)
> {
> - *res = (struct arm_smccc_res) {
> + *res = (struct arm_smccc_1_2_regs) {
> .a0 = FFA_ERROR,
> .a2 = ffa_errno,
> };
> }
>
> -static void ffa_to_smccc_res_prop(struct arm_smccc_res *res, int ret, u64 prop)
> +static void ffa_to_smccc_res_prop(struct arm_smccc_1_2_regs *res, int ret, u64 prop)
> {
> if (ret == FFA_RET_SUCCESS) {
> - *res = (struct arm_smccc_res) { .a0 = FFA_SUCCESS,
> - .a2 = prop };
> + *res = (struct arm_smccc_1_2_regs) { .a0 = FFA_SUCCESS,
> + .a2 = prop };
> } else {
> ffa_to_smccc_error(res, ret);
> }
> }
>
> -static void ffa_to_smccc_res(struct arm_smccc_res *res, int ret)
> +static void ffa_to_smccc_res(struct arm_smccc_1_2_regs *res, int ret)
> {
> ffa_to_smccc_res_prop(res, ret, 0);
> }
>
> static void ffa_set_retval(struct kvm_cpu_context *ctxt,
> - struct arm_smccc_res *res)
> + struct arm_smccc_1_2_regs *res)
> {
> cpu_reg(ctxt, 0) = res->a0;
> cpu_reg(ctxt, 1) = res->a1;
> cpu_reg(ctxt, 2) = res->a2;
> cpu_reg(ctxt, 3) = res->a3;
> + cpu_reg(ctxt, 4) = res->a4;
> + cpu_reg(ctxt, 5) = res->a5;
> + cpu_reg(ctxt, 6) = res->a6;
> + cpu_reg(ctxt, 7) = res->a7;
> +
> + /*
> + * DEN0028C 2.6: SMC32/HVC32 call from aarch64 must preserve x8-x30.
> + *
> + * In FF-A 1.2, we cannot rely on the function ID sent by the caller to
> + * deteect 32-bit calls as there are cases where a 32-bit interface can
typo: deteect
> + * have a 64-bit response 1.2 (e.g. FFA_MSG_WAIT or FFA_RUN). This will
> + * be addressed in FF-A 1.3. We also cannot rely on function IDs in the
> + * response.
Do you know what the FF-A 1.3 update is likely to say? It would be nice
to get an idea of what the code might need to do in future in case this
approach is going in a completely different direction.
> @@ -666,17 +703,21 @@ static bool do_ffa_features(struct arm_smccc_res *res,
> static int hyp_ffa_post_init(void)
> {
> size_t min_rxtx_sz;
> - struct arm_smccc_res res;
> + struct arm_smccc_1_2_regs res;
>
> - arm_smccc_1_1_smc(FFA_ID_GET, 0, 0, 0, 0, 0, 0, 0, &res);
> + arm_smccc_1_2_smc(&(struct arm_smccc_1_2_regs){
> + .a0 = FFA_ID_GET,
> + }, &res);
Regardless of the problems with the spec, it's really nice to get rid
of all those zero arguments that come with the arm_smccc_1_1_smc()
macro.
Will
Powered by blists - more mailing lists