[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <aBn9rMiVI_7CcdED@google.com>
Date: Tue, 6 May 2025 12:16:44 +0000
From: Sebastian Ene <sebastianene@...gle.com>
To: Per Larsen <perl@...unant.com>
Cc: linux-kernel@...r.kernel.org, catalin.marinas@....com,
james.morse@....com, jean-philippe@...aro.org,
kernel-team@...roid.com, kvmarm@...ts.linux.dev,
linux-arm-kernel@...ts.infradead.org, lpieralisi@...nel.org,
maz@...nel.org, oliver.upton@...ux.dev, qperret@...gle.com,
qwandor@...gle.com, sudeep.holla@....com, suzuki.poulose@....com,
tabba@...gle.com, will@...nel.org, yuzenghui@...wei.com,
armellel@...gle.com, arve@...roid.com, ahomescu@...gle.com,
Per Larsen <perlarsen@...gle.com>
Subject: Re: [PATCH 3/3] KVM: arm64: Support FFA_MSG_SEND_DIRECT_REQ2 in host
handler
On Fri, May 02, 2025 at 02:21:08AM -0700, Per Larsen wrote:
> From: Per Larsen <perlarsen@...gle.com>
>
> FF-A 1.2 adds the DIRECT_REQ2 messaging interface which is similar to
> the existing FFA_MSG_SEND_DIRECT_{REQ,RESP} functions except that it
> uses the SMC calling convention v1.2 which allows calls to use x4-x17 as
> argument and return registers. Add support for FFA_MSG_SEND_DIRECT_REQ2
> in the host ffa handler.
>
> Signed-off-by: Per Larsen <perlarsen@...gle.com>
> Signed-off-by: Per Larsen <perl@...unant.com>
> ---
> arch/arm64/kvm/hyp/nvhe/ffa.c | 111 +++++++++++++++++++++++++++++++++-
> include/linux/arm_ffa.h | 2 +
> 2 files changed, 111 insertions(+), 2 deletions(-)
>
> diff --git a/arch/arm64/kvm/hyp/nvhe/ffa.c b/arch/arm64/kvm/hyp/nvhe/ffa.c
> index 8102dd6a19f7..d8c066f3c5c9 100644
> --- a/arch/arm64/kvm/hyp/nvhe/ffa.c
> +++ b/arch/arm64/kvm/hyp/nvhe/ffa.c
> @@ -79,6 +79,14 @@ static void ffa_to_smccc_error(struct arm_smccc_res *res, u64 ffa_errno)
> };
> }
>
> +static void ffa_to_smccc_1_2_error(struct arm_smccc_1_2_regs *regs, u64 ffa_errno)
> +{
> + *regs = (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)
> {
> if (ret == FFA_RET_SUCCESS) {
> @@ -89,11 +97,26 @@ static void ffa_to_smccc_res_prop(struct arm_smccc_res *res, int ret, u64 prop)
> }
> }
>
> +static void ffa_to_smccc_1_2_regs_prop(struct arm_smccc_1_2_regs *regs, int ret, u64 prop)
> +{
> + if (ret == FFA_RET_SUCCESS) {
> + *regs = (struct arm_smccc_1_2_regs) { .a0 = FFA_SUCCESS,
> + .a2 = prop };
> + } else {
NIT: no paranthesis needed here
> + ffa_to_smccc_1_2_error(regs, ret);
> + }
> +}
> +
> static void ffa_to_smccc_res(struct arm_smccc_res *res, int ret)
> {
> ffa_to_smccc_res_prop(res, ret, 0);
> }
>
> +static void ffa_to_smccc_1_2_regs(struct arm_smccc_1_2_regs *regs, int ret)
> +{
> + ffa_to_smccc_1_2_regs_prop(regs, ret, 0);
> +}
> +
> static void ffa_set_retval(u64 func_id,
> struct kvm_cpu_context *ctxt,
> struct arm_smccc_res *res)
> @@ -127,6 +150,29 @@ static void ffa_set_retval(u64 func_id,
> }
> }
>
> +static void ffa_set_retval_smccc_1_2(struct kvm_cpu_context *ctxt,
> + struct arm_smccc_1_2_regs *regs)
> +{
> + cpu_reg(ctxt, 0) = regs->a0;
> + cpu_reg(ctxt, 1) = regs->a1;
> + cpu_reg(ctxt, 2) = regs->a2;
> + cpu_reg(ctxt, 3) = regs->a3;
> + cpu_reg(ctxt, 4) = regs->a4;
> + cpu_reg(ctxt, 5) = regs->a5;
> + cpu_reg(ctxt, 6) = regs->a6;
> + cpu_reg(ctxt, 7) = regs->a7;
> + cpu_reg(ctxt, 8) = regs->a8;
> + cpu_reg(ctxt, 9) = regs->a9;
> + cpu_reg(ctxt, 10) = regs->a10;
> + cpu_reg(ctxt, 11) = regs->a11;
> + cpu_reg(ctxt, 12) = regs->a12;
> + cpu_reg(ctxt, 13) = regs->a13;
> + cpu_reg(ctxt, 14) = regs->a14;
> + cpu_reg(ctxt, 15) = regs->a15;
> + cpu_reg(ctxt, 16) = regs->a16;
> + cpu_reg(ctxt, 17) = regs->a17;
> +}
> +
> /* Call SMC64 using SMCCC 1.2 if hyp negotiated FF-A 1.2 falling back to 1.1 */
> static void arm_smccc_1_2_smc_fallback(u64 func_id, u64 a1, u64 a2, u64 a3,
> u64 a4, u64 a5, u64 a6, u64 a7,
> @@ -681,7 +727,6 @@ static bool ffa_call_supported(u64 func_id)
> case FFA_NOTIFICATION_GET:
> case FFA_NOTIFICATION_INFO_GET:
> /* Unimplemented interfaces added in FF-A 1.2 */
> - case FFA_MSG_SEND_DIRECT_REQ2:
>
> /*
> * FFA_MSG_SEND_DIRECT_RESP2 is not meant to be invoked by the host or
> @@ -709,6 +754,20 @@ static bool ffa_call_supported(u64 func_id)
> return true;
> }
>
> +/*
> + * Must a given FFA function use the SMC calling convention v1.2?
> + */
> +static bool ffa_call_needs_smccc_1_2(u64 func_id)
> +{
> + switch (func_id) {
> + case FFA_MSG_SEND_DIRECT_REQ2:
> + case FFA_MSG_SEND_DIRECT_RESP2:
> + return true;
> + }
> +
> + return false;
> +}
> +
> static bool do_ffa_features(struct arm_smccc_res *res,
> struct kvm_cpu_context *ctxt)
> {
> @@ -882,9 +941,47 @@ static void do_ffa_part_get(struct arm_smccc_res *res,
> hyp_spin_unlock(&host_buffers.lock);
> }
>
> +static void do_ffa_direct_msg2(struct arm_smccc_1_2_regs *regs,
> + struct kvm_cpu_context *ctxt,
> + u64 vm_handle)
> +{
> + DECLARE_REG(u32, func_id, ctxt, 0);
> + DECLARE_REG(u32, endp, ctxt, 1);
> + DECLARE_REG(u64, uuid_lo, ctxt, 2);
> + DECLARE_REG(u64, uuid_hi, ctxt, 3);
> + DECLARE_REG(u64, x4, ctxt, 4);
> + DECLARE_REG(u64, x5, ctxt, 5);
> + DECLARE_REG(u64, x6, ctxt, 6);
> + DECLARE_REG(u64, x7, ctxt, 7);
> + DECLARE_REG(u64, x8, ctxt, 8);
> + DECLARE_REG(u64, x9, ctxt, 9);
> + DECLARE_REG(u64, x10, ctxt, 10);
> + DECLARE_REG(u64, x11, ctxt, 11);
> + DECLARE_REG(u64, x12, ctxt, 12);
> + DECLARE_REG(u64, x13, ctxt, 13);
> + DECLARE_REG(u64, x14, ctxt, 14);
> + DECLARE_REG(u64, x15, ctxt, 15);
> + DECLARE_REG(u64, x16, ctxt, 16);
> + DECLARE_REG(u64, x17, ctxt, 17);
> +
> + if (FIELD_GET(FFA_SRC_ENDPOINT_MASK, endp) != vm_handle) {
> + ffa_to_smccc_1_2_regs(regs, FFA_RET_INVALID_PARAMETERS);
> + return;
> + }
> +
> + struct arm_smccc_1_2_regs args = {
> + func_id, endp, uuid_lo, uuid_hi,
> + x4, x5, x6, x7, x8, x9, x10,
> + x11, x12, x13, x14, x15, x16, x17
> + };
Maybe use cpu_reg(ctxt, ) to avoid copying two times the values and drop
the DECLARE_REG ?
> +
> + arm_smccc_1_2_smc(&args, regs);
> +}
> +
> bool kvm_host_ffa_handler(struct kvm_cpu_context *host_ctxt, u32 func_id)
> {
> struct arm_smccc_res res;
> + struct arm_smccc_1_2_regs regs;
>
> /*
> * There's no way we can tell what a non-standard SMC call might
> @@ -940,14 +1037,24 @@ bool kvm_host_ffa_handler(struct kvm_cpu_context *host_ctxt, u32 func_id)
> case FFA_PARTITION_INFO_GET:
> do_ffa_part_get(&res, host_ctxt);
> goto out_handled;
> + case FFA_MSG_SEND_DIRECT_REQ2:
> + if (hyp_ffa_version >= FFA_VERSION_1_2) {
The version is accessed without a lock.
> + do_ffa_direct_msg2(®s, host_ctxt, HOST_FFA_ID);
> + goto out_handled;
> + }
> + goto out_not_supported;
> }
>
> if (ffa_call_supported(func_id))
> return false; /* Pass through */
>
> +out_not_supported:
> ffa_to_smccc_error(&res, FFA_RET_NOT_SUPPORTED);
> out_handled:
> - ffa_set_retval(func_id, host_ctxt, &res);
> + if (ffa_call_needs_smccc_1_2(func_id))
> + ffa_set_retval_smccc_1_2(host_ctxt, ®s);
> + else
> + ffa_set_retval(func_id, host_ctxt, &res);
> return true;
> }
>
> diff --git a/include/linux/arm_ffa.h b/include/linux/arm_ffa.h
> index 5bded24dc24f..4d541cf12ceb 100644
> --- a/include/linux/arm_ffa.h
> +++ b/include/linux/arm_ffa.h
> @@ -268,6 +268,8 @@ bool ffa_partition_check_property(struct ffa_device *dev, u32 property)
> (ffa_partition_check_property(dev, FFA_PARTITION_DIRECT_REQ2_RECV) && \
> !dev->mode_32bit)
>
> +#define FFA_SRC_ENDPOINT_MASK GENMASK(31, 16)
> +
> /* For use with FFA_MSG_SEND_DIRECT_{REQ,RESP} which pass data via registers */
> struct ffa_send_direct_data {
> unsigned long data0; /* w3/x3 */
> --
> 2.49.0
>
Powered by blists - more mailing lists