[PATCH 1/3] KVM: arm64: Restrict FF-A host version renegotiation

[PATCH 1/3] KVM: arm64: Restrict FF-A host version renegotiation

[Per Larsen]

FF-A implementations with the same major version must interoperate with
earlier minor versions per DEN0077A 1.2 REL0 13.2.1 but FF-A version 1.1
broke the ABI on several structures and 1.2 relies on SMCCC 1.2 is not
backwards compatible with SMCCC 1.2 (see DEN0028 1.6 G BET0 Appendix F).

If we return the negotiated hypervisor version when the host requests a
lesser minor version, the host will rely on the FF-A interoperability
rules. Since the hypervisor does not currently have the necessary
compatibility paths (e.g. to handle breaking changes to the SMC calling
convention), return NOT_SUPPORTED.

Signed-off-by: Per Larsen <perlarsen@google.com>
Signed-off-by: Per Larsen <perl@immunant.com>
---
 arch/arm64/kvm/hyp/nvhe/ffa.c | 19 ++++++++++++++++++-
 1 file changed, 18 insertions(+), 1 deletion(-)

diff --git a/arch/arm64/kvm/hyp/nvhe/ffa.c b/arch/arm64/kvm/hyp/nvhe/ffa.c
index 3369dd0c4009..10e88207b78e 100644
--- a/arch/arm64/kvm/hyp/nvhe/ffa.c
+++ b/arch/arm64/kvm/hyp/nvhe/ffa.c
@@ -712,7 +712,24 @@ static void do_ffa_version(struct arm_smccc_res *res,

  hyp_spin_lock(&version_lock);
  if (has_version_negotiated) {
- res->a0 = hyp_ffa_version;
+ /*
+ * FF-A implementations with the same major version must
+ * interoperate with earlier minor versions per DEN0077A 1.2
+ * REL0 13.2.1 but FF-A version 1.1 broke the ABI on several
+ * structures and 1.2 relies on SMCCC 1.2 is not backwards
+ * compatible with SMCCC 1.2 (see DEN0028 1.6 G BET0 Appendix F).
+ *
+ * If we return the negotiated hypervisor version when the host
+ * requests a lesser minor version, the host will rely on the
+ * aforementioned FF-A interoperability rules. Since the
+ * hypervisor does not currently have the necessary compatibility
+ * paths (e.g. to paper over the above-mentioned calling
+ * convention changes), return NOT_SUPPORTED.
+ */
+ if (FFA_MINOR_VERSION(ffa_req_version) < FFA_MINOR_VERSION(hyp_ffa_version))
+ res->a0 = FFA_RET_NOT_SUPPORTED;
+ else
+ res->a0 = hyp_ffa_version;
  goto unlock;
  }

-- 
2.49.0.906.g1f30a19c02-goog

Re: [PATCH 1/3] KVM: arm64: Restrict FF-A host version renegotiation

[Marc Zyngier]

On Fri, 02 May 2025 04:52:39 +0100,
Per Larsen <perl@immunant.com> wrote:
> 
> FF-A implementations with the same major version must interoperate with
> earlier minor versions per DEN0077A 1.2 REL0 13.2.1 but FF-A version 1.1
> broke the ABI on several structures and 1.2 relies on SMCCC 1.2 is not
> backwards compatible with SMCCC 1.2 (see DEN0028 1.6 G BET0 Appendix F).
> 
> If we return the negotiated hypervisor version when the host requests a
> lesser minor version, the host will rely on the FF-A interoperability
> rules. Since the hypervisor does not currently have the necessary
> compatibility paths (e.g. to handle breaking changes to the SMC calling
> convention), return NOT_SUPPORTED.
> 
> Signed-off-by: Per Larsen <perlarsen@google.com>
> Signed-off-by: Per Larsen <perl@immunant.com>
> ---
>  arch/arm64/kvm/hyp/nvhe/ffa.c | 19 ++++++++++++++++++-
>  1 file changed, 18 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/arm64/kvm/hyp/nvhe/ffa.c b/arch/arm64/kvm/hyp/nvhe/ffa.c
> index 3369dd0c4009..10e88207b78e 100644
> --- a/arch/arm64/kvm/hyp/nvhe/ffa.c
> +++ b/arch/arm64/kvm/hyp/nvhe/ffa.c
> @@ -712,7 +712,24 @@ static void do_ffa_version(struct arm_smccc_res *res,
> 
>   hyp_spin_lock(&version_lock);
>   if (has_version_negotiated) {
> - res->a0 = hyp_ffa_version;
> + /*
> + * FF-A implementations with the same major version must
> + * interoperate with earlier minor versions per DEN0077A 1.2
> + * REL0 13.2.1 but FF-A version 1.1 broke the ABI on several
> + * structures and 1.2 relies on SMCCC 1.2 is not backwards
> + * compatible with SMCCC 1.2 (see DEN0028 1.6 G BET0 Appendix F).

I can't parse this sentence. Missing words?

> + *
> + * If we return the negotiated hypervisor version when the host
> + * requests a lesser minor version, the host will rely on the
> + * aforementioned FF-A interoperability rules. Since the
> + * hypervisor does not currently have the necessary compatibility
> + * paths (e.g. to paper over the above-mentioned calling
> + * convention changes), return NOT_SUPPORTED.
> + */
> + if (FFA_MINOR_VERSION(ffa_req_version) < FFA_MINOR_VERSION(hyp_ffa_version))
> + res->a0 = FFA_RET_NOT_SUPPORTED;
> + else
> + res->a0 = hyp_ffa_version;
>   goto unlock;
>   }
> 

Something has gone seriously wrong with your email, and the patches
are badly mangled and unusable. They are also sent as individual
patches and not as a thread, which is a sign that you didn't send them
using git. Please fix this for your next posting.

More to the meat of the patches: why should the hypervisor paper over
anything if the spec is broken? Why can't the host just as well decide
for itself what to do?

Thanks,

	M.

-- 
Without deviation from the norm, progress is not possible.

[PATCH 0/3] KVM: arm64: Support FF-A 1.2 and SEND_DIRECT2 ABI

[Per Larsen]

Hi,

The FF-A 1.2 specification introduces a new SEND_DIRECT2 ABI which
allows registers x4-x17 to be used for the message payload. This patch
set prevents the host from using a lower FF-A version than what has
already been negotiated with the hypervisor. This is necessary because
the hypervisor does not have the necessary compatibility paths to
translate from the hypervisor FF-A version to a previous version.

Support for FF-A 1.2 in the hypervisor is added as a precursor to the
addition of the FFA_MSG_SEND_DIRECT_REQ2 messaging interface. The bulk
of this change has to do with the upgrade to SMCCC 1.2 required by
FF-A 1.2. Additionally, unimplemented FF-A 1.2 interfaces are added to
the list of unsupported functions.

Tested by booting Android under QEMU and loading Trusty as the guest
VM and observing the SEND_DIRECT2 ABI being used successfully during
guest boot.

(This is my second attempt at sending out these patches; sorry about
my broken first try and the resulting duplicate messages.)

Thanks,
Per Larsen

Per Larsen (3):
  KVM: arm64: Restrict FF-A host version renegotiation
  KVM: arm64: Bump the supported version of FF-A to 1.2
  KVM: arm64: Support FFA_MSG_SEND_DIRECT_REQ2 in host handler

 arch/arm64/kvm/hyp/nvhe/Makefile |   1 +
 arch/arm64/kvm/hyp/nvhe/ffa.c    | 235 ++++++++++++++++++++++++++++---
 include/linux/arm_ffa.h          |   2 +
 3 files changed, 221 insertions(+), 17 deletions(-)

--
2.49.0

[PATCH 1/3] KVM: arm64: Restrict FF-A host version renegotiation

[Per Larsen]

From: Per Larsen <perlarsen@google.com>

FF-A implementations with the same major version must interoperate with
earlier minor versions per DEN0077A 1.2 REL0 13.2.1 but FF-A version 1.1
broke the ABI on several structures and 1.2 relies on SMCCC 1.2 is not
backwards compatible with SMCCC 1.2 (see DEN0028 1.6 G BET0 Appendix F).

If we return the negotiated hypervisor version when the host requests a
lesser minor version, the host will rely on the FF-A interoperability
rules. Since the hypervisor does not currently have the necessary
compatibility paths (e.g. to handle breaking changes to the SMC calling
convention), return NOT_SUPPORTED.

Signed-off-by: Per Larsen <perlarsen@google.com>
Signed-off-by: Per Larsen <perl@immunant.com>
---
 arch/arm64/kvm/hyp/nvhe/ffa.c | 19 ++++++++++++++++++-
 1 file changed, 18 insertions(+), 1 deletion(-)

diff --git a/arch/arm64/kvm/hyp/nvhe/ffa.c b/arch/arm64/kvm/hyp/nvhe/ffa.c
index 3369dd0c4009..10e88207b78e 100644
--- a/arch/arm64/kvm/hyp/nvhe/ffa.c
+++ b/arch/arm64/kvm/hyp/nvhe/ffa.c
@@ -712,7 +712,24 @@ static void do_ffa_version(struct arm_smccc_res *res,
 
 	hyp_spin_lock(&version_lock);
 	if (has_version_negotiated) {
-		res->a0 = hyp_ffa_version;
+		/*
+		 * FF-A implementations with the same major version must
+		 * interoperate with earlier minor versions per DEN0077A 1.2
+		 * REL0 13.2.1 but FF-A version 1.1 broke the ABI on several
+		 * structures and 1.2 relies on SMCCC 1.2 is not backwards
+		 * compatible with SMCCC 1.2 (see DEN0028 1.6 G BET0 Appendix F).
+		 *
+		 * If we return the negotiated hypervisor version when the host
+		 * requests a lesser minor version, the host will rely on the
+		 * aforementioned FF-A interoperability rules. Since the
+		 * hypervisor does not currently have the necessary compatibility
+		 * paths (e.g. to paper over the above-mentioned calling
+		 * convention changes), return NOT_SUPPORTED.
+		 */
+		if (FFA_MINOR_VERSION(ffa_req_version) < FFA_MINOR_VERSION(hyp_ffa_version))
+			res->a0 = FFA_RET_NOT_SUPPORTED;
+		else
+			res->a0 = hyp_ffa_version;
 		goto unlock;
 	}
 
-- 
2.49.0

[PATCH 2/3] KVM: arm64: Bump the supported version of FF-A to 1.2

[Per Larsen]

From: Per Larsen <perlarsen@google.com>

FF-A version 1.2 introduces the DIRECT_REQ2 ABI. Bump the FF-A version
preferred by the hypervisor as a precursor to implementing the 1.2-only
FFA_MSG_SEND_DIRECT_REQ2 and FFA_MSG_SEND_RESP2 messaging interfaces.

We must also use SMCCC 1.2 for 64-bit SMCs if hypervisor negotiated FF-A
1.2, so ffa_set_retval is updated and a new function to call 64-bit smcs
using SMCCC 1.2 with fallback to SMCCC 1.1 is introduced.

Update deny-list in ffa_call_supported to mark FFA_NOTIFICATION_* and
interfaces added in FF-A 1.2 as unsupported lest they get forwarded.

Co-Developed-by: Ayrton Munoz <ayrton@google.com>
Signed-off-by: Per Larsen <perlarsen@google.com>
Signed-off-by: Per Larsen <perl@immunant.com>
---
 arch/arm64/kvm/hyp/nvhe/Makefile |   1 +
 arch/arm64/kvm/hyp/nvhe/ffa.c    | 109 ++++++++++++++++++++++++++-----
 2 files changed, 94 insertions(+), 16 deletions(-)

diff --git a/arch/arm64/kvm/hyp/nvhe/Makefile b/arch/arm64/kvm/hyp/nvhe/Makefile
index b43426a493df..95404ff16dac 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 10e88207b78e..8102dd6a19f7 100644
--- a/arch/arm64/kvm/hyp/nvhe/ffa.c
+++ b/arch/arm64/kvm/hyp/nvhe/ffa.c
@@ -94,13 +94,57 @@ static void ffa_to_smccc_res(struct arm_smccc_res *res, int ret)
 	ffa_to_smccc_res_prop(res, ret, 0);
 }
 
-static void ffa_set_retval(struct kvm_cpu_context *ctxt,
+static void ffa_set_retval(u64 func_id,
+			   struct kvm_cpu_context *ctxt,
 			   struct arm_smccc_res *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;
+
+	/*
+	 * Other result registers must be zero per DEN0077A but SMC32/HVC32 must
+	 * preserve x8-x30 per DEN0028.
+	 */
+	cpu_reg(ctxt, 4) = 0;
+	cpu_reg(ctxt, 5) = 0;
+	cpu_reg(ctxt, 6) = 0;
+	cpu_reg(ctxt, 7) = 0;
+
+	/* Per DEN0077A v1.2, x8-x17 must be zero for SMC64/HVC64 results*/
+	if (ARM_SMCCC_IS_64(func_id) && hyp_ffa_version >= FFA_VERSION_1_2) {
+		cpu_reg(ctxt, 8) = 0;
+		cpu_reg(ctxt, 9) = 0;
+		cpu_reg(ctxt, 10) = 0;
+		cpu_reg(ctxt, 11) = 0;
+		cpu_reg(ctxt, 12) = 0;
+		cpu_reg(ctxt, 13) = 0;
+		cpu_reg(ctxt, 14) = 0;
+		cpu_reg(ctxt, 15) = 0;
+		cpu_reg(ctxt, 16) = 0;
+		cpu_reg(ctxt, 17) = 0;
+	}
+}
+
+/* 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,
+				       struct arm_smccc_res *res)
+{
+	struct arm_smccc_1_2_regs args, regs;
+
+	/* SMC64 only as SMC32 must preserve x8-x30 per DEN0028 1.6G Sec 2.6 */
+	if (ARM_SMCCC_IS_64(func_id) && hyp_ffa_version >= FFA_VERSION_1_2) {
+		args = (struct arm_smccc_1_2_regs) { func_id, a1, a2, a3, a4,
+						     a5, a6, a7 };
+		arm_smccc_1_2_smc(&args, &regs);
+		*res = (struct arm_smccc_res) { .a0 = regs.a0, .a1 = regs.a1,
+						.a2 = regs.a2, .a3 = regs.a3 };
+		return;
+	}
+
+	arm_smccc_1_1_smc(func_id, a1, a2, a3, a4, a5, a6, a7, res);
 }
 
 static bool is_ffa_call(u64 func_id)
@@ -115,12 +159,12 @@ static int ffa_map_hyp_buffers(u64 ffa_page_count)
 {
 	struct arm_smccc_res res;
 
-	arm_smccc_1_1_smc(FFA_FN64_RXTX_MAP,
-			  hyp_virt_to_phys(hyp_buffers.tx),
-			  hyp_virt_to_phys(hyp_buffers.rx),
-			  ffa_page_count,
-			  0, 0, 0, 0,
-			  &res);
+	arm_smccc_1_2_smc_fallback(FFA_FN64_RXTX_MAP,
+				   hyp_virt_to_phys(hyp_buffers.tx),
+				   hyp_virt_to_phys(hyp_buffers.rx),
+				   ffa_page_count,
+				   0, 0, 0, 0,
+				   &res);
 
 	return res.a0 == FFA_SUCCESS ? FFA_RET_SUCCESS : res.a2;
 }
@@ -174,10 +218,10 @@ static void ffa_mem_reclaim(struct arm_smccc_res *res, u32 handle_lo,
 
 static void ffa_retrieve_req(struct arm_smccc_res *res, u32 len)
 {
-	arm_smccc_1_1_smc(FFA_FN64_MEM_RETRIEVE_REQ,
-			  len, len,
-			  0, 0, 0, 0, 0,
-			  res);
+	arm_smccc_1_2_smc_fallback(FFA_FN64_MEM_RETRIEVE_REQ,
+				   len, len,
+				   0, 0, 0, 0, 0,
+				   res);
 }
 
 static void ffa_rx_release(struct arm_smccc_res *res)
@@ -628,6 +672,37 @@ static bool ffa_call_supported(u64 func_id)
 	case FFA_RXTX_MAP:
 	case FFA_MEM_DONATE:
 	case FFA_MEM_RETRIEVE_REQ:
+	/* Optional notification interfaces added in FF-A 1.1 */
+	case FFA_NOTIFICATION_BITMAP_CREATE:
+	case FFA_NOTIFICATION_BITMAP_DESTROY:
+	case FFA_NOTIFICATION_BIND:
+	case FFA_NOTIFICATION_UNBIND:
+	case FFA_NOTIFICATION_SET:
+	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
+	 * a guest VM because pkvm does not support communication between VMs
+	 * via FF-A. Direct messages can only be sent from a non-secure sender
+	 * endpoint to a secure receiver endpoint. Only receiver endpoints are
+	 * expected to invoke FFA_MSG_SEND_DIRECT_RESP2.
+	 */
+	case FFA_MSG_SEND_DIRECT_RESP2:
+
+	/* Reserved for secure endpoints per DEN0077A 1.2 REL0 Table 13.53. */
+	case FFA_CONSOLE_LOG:
+
+	/*
+	 * Mandatory for secure endpoints per DEN0077A 1.2 REL0 Table 13.1 and
+	 * optional for non-secure endpoints according to Table 13.38.
+	 */
+	case FFA_PARTITION_INFO_GET_REGS:
+
+	/* Reserved for secure endpoints per DEN0077A 1.2 REL0 Table 17.2. */
+	case FFA_EL3_INTR_HANDLE:
 		return false;
 	}
 
@@ -680,7 +755,8 @@ static int hyp_ffa_post_init(void)
 	if (res.a0 != FFA_SUCCESS)
 		return -EOPNOTSUPP;
 
-	switch (res.a2) {
+	/* Bit[0:1] holds minimum buffer size and alignment boundary */
+	switch (res.a2 & 0x3) {
 	case FFA_FEAT_RXTX_MIN_SZ_4K:
 		min_rxtx_sz = SZ_4K;
 		break;
@@ -871,7 +947,7 @@ bool kvm_host_ffa_handler(struct kvm_cpu_context *host_ctxt, u32 func_id)
 
 	ffa_to_smccc_error(&res, FFA_RET_NOT_SUPPORTED);
 out_handled:
-	ffa_set_retval(host_ctxt, &res);
+	ffa_set_retval(func_id, host_ctxt, &res);
 	return true;
 }
 
@@ -883,7 +959,7 @@ int hyp_ffa_init(void *pages)
 	if (kvm_host_psci_config.smccc_version < ARM_SMCCC_VERSION_1_2)
 		return 0;
 
-	arm_smccc_1_1_smc(FFA_VERSION, FFA_VERSION_1_1, 0, 0, 0, 0, 0, 0, &res);
+	arm_smccc_1_1_smc(FFA_VERSION, FFA_VERSION_1_2, 0, 0, 0, 0, 0, 0, &res);
 	if (res.a0 == FFA_RET_NOT_SUPPORTED)
 		return 0;
 
@@ -903,10 +979,11 @@ int hyp_ffa_init(void *pages)
 	if (FFA_MAJOR_VERSION(res.a0) != 1)
 		return -EOPNOTSUPP;
 
-	if (FFA_MINOR_VERSION(res.a0) < FFA_MINOR_VERSION(FFA_VERSION_1_1))
+	/* See do_ffa_guest_version before bumping maximum supported version. */
+	if (FFA_MINOR_VERSION(res.a0) < FFA_MINOR_VERSION(FFA_VERSION_1_2))
 		hyp_ffa_version = res.a0;
 	else
-		hyp_ffa_version = FFA_VERSION_1_1;
+		hyp_ffa_version = FFA_VERSION_1_2;
 
 	tx = pages;
 	pages += KVM_FFA_MBOX_NR_PAGES * PAGE_SIZE;
-- 
2.49.0

[PATCH 3/3] KVM: arm64: Support FFA_MSG_SEND_DIRECT_REQ2 in host handler

[Per Larsen]

From: Per Larsen <perlarsen@google.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@google.com>
Signed-off-by: Per Larsen <perl@immunant.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 {
+		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
+	};
+
+	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) {
+			do_ffa_direct_msg2(&regs, 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, &regs);
+	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

Re: [PATCH 0/3] KVM: arm64: Support FF-A 1.2 and SEND_DIRECT2 ABI

[Sebastian Ene]

On Fri, May 02, 2025 at 02:21:05AM -0700, Per Larsen wrote:
> Hi,
> 
> The FF-A 1.2 specification introduces a new SEND_DIRECT2 ABI which
> allows registers x4-x17 to be used for the message payload. This patch
> set prevents the host from using a lower FF-A version than what has
> already been negotiated with the hypervisor. This is necessary because
> the hypervisor does not have the necessary compatibility paths to
> translate from the hypervisor FF-A version to a previous version.
> 
> Support for FF-A 1.2 in the hypervisor is added as a precursor to the
> addition of the FFA_MSG_SEND_DIRECT_REQ2 messaging interface. The bulk
> of this change has to do with the upgrade to SMCCC 1.2 required by
> FF-A 1.2. Additionally, unimplemented FF-A 1.2 interfaces are added to
> the list of unsupported functions.
> 
> Tested by booting Android under QEMU and loading Trusty as the guest
> VM and observing the SEND_DIRECT2 ABI being used successfully during
> guest boot.
> 
> (This is my second attempt at sending out these patches; sorry about
> my broken first try and the resulting duplicate messages.)

Please tag this series with v2 if this is the second one and include a
changelog.

> 
> Thanks,
> Per Larsen
> 
> Per Larsen (3):
>   KVM: arm64: Restrict FF-A host version renegotiation
>   KVM: arm64: Bump the supported version of FF-A to 1.2
>   KVM: arm64: Support FFA_MSG_SEND_DIRECT_REQ2 in host handler
> 
>  arch/arm64/kvm/hyp/nvhe/Makefile |   1 +
>  arch/arm64/kvm/hyp/nvhe/ffa.c    | 235 ++++++++++++++++++++++++++++---
>  include/linux/arm_ffa.h          |   2 +
>  3 files changed, 221 insertions(+), 17 deletions(-)
> 
> --
> 2.49.0
>

Thanks,
Seb

Re: [PATCH 1/3] KVM: arm64: Restrict FF-A host version renegotiation

[Per Larsen]

From: Per Larsen <perlarsen@google.com>

On Fri, May 02, 2025 at 09:47:45AM +0100, Marc Zyngier wrote:
> On Fri, 02 May 2025 04:52:39 +0100,
> Per Larsen <perl@immunant.com> wrote:
> > 
> > FF-A implementations with the same major version must interoperate with
> > earlier minor versions per DEN0077A 1.2 REL0 13.2.1 but FF-A version 1.1
> > broke the ABI on several structures and 1.2 relies on SMCCC 1.2 is not
> > backwards compatible with SMCCC 1.2 (see DEN0028 1.6 G BET0 Appendix F).
> > 
> > If we return the negotiated hypervisor version when the host requests a
> > lesser minor version, the host will rely on the FF-A interoperability
> > rules. Since the hypervisor does not currently have the necessary
> > compatibility paths (e.g. to handle breaking changes to the SMC calling
> > convention), return NOT_SUPPORTED.
> > 
> > Signed-off-by: Per Larsen <perlarsen@google.com>
> > Signed-off-by: Per Larsen <perl@immunant.com>
> > ---
> >  arch/arm64/kvm/hyp/nvhe/ffa.c | 19 ++++++++++++++++++-
> >  1 file changed, 18 insertions(+), 1 deletion(-)
> > 
> > diff --git a/arch/arm64/kvm/hyp/nvhe/ffa.c b/arch/arm64/kvm/hyp/nvhe/ffa.c
> > index 3369dd0c4009..10e88207b78e 100644
> > --- a/arch/arm64/kvm/hyp/nvhe/ffa.c
> > +++ b/arch/arm64/kvm/hyp/nvhe/ffa.c
> > @@ -712,7 +712,24 @@ static void do_ffa_version(struct arm_smccc_res *res,
> > 
> >   hyp_spin_lock(&version_lock);
> >   if (has_version_negotiated) {
> > - res->a0 = hyp_ffa_version;
> > + /*
> > + * FF-A implementations with the same major version must
> > + * interoperate with earlier minor versions per DEN0077A 1.2
> > + * REL0 13.2.1 but FF-A version 1.1 broke the ABI on several
> > + * structures and 1.2 relies on SMCCC 1.2 is not backwards
> > + * compatible with SMCCC 1.2 (see DEN0028 1.6 G BET0 Appendix F).
> 
> I can't parse this sentence. Missing words?
> 

Yes, I will fix this in v2.

> > + *
> > + * If we return the negotiated hypervisor version when the host
> > + * requests a lesser minor version, the host will rely on the
> > + * aforementioned FF-A interoperability rules. Since the
> > + * hypervisor does not currently have the necessary compatibility
> > + * paths (e.g. to paper over the above-mentioned calling
> > + * convention changes), return NOT_SUPPORTED.
> > + */
> > + if (FFA_MINOR_VERSION(ffa_req_version) < FFA_MINOR_VERSION(hyp_ffa_version))
> > + res->a0 = FFA_RET_NOT_SUPPORTED;
> > + else
> > + res->a0 = hyp_ffa_version;
> >   goto unlock;
> >   }
> > 
> 
> Something has gone seriously wrong with your email, and the patches
> are badly mangled and unusable. They are also sent as individual
> patches and not as a thread, which is a sign that you didn't send them
> using git. Please fix this for your next posting.
> 

Yes, my apologies. I will use git send-email to post v2.

> More to the meat of the patches: why should the hypervisor paper over
> anything if the spec is broken? Why can't the host just as well decide
> for itself what to do?
> 

Asssuming we drop this patch from the series and apply the rest, the
hypervisor and host can negotiate FF-A 1.2. If the host then calls
FFA_VERSION a second time to request FF-A 1.1, the hypervisor would
return version 1.2 (without this patch). Per the spec, that means the
host is can use the compatibility rules (DEN0077A Sec 13.2.1) to go
ahead and use FF-A 1.1 (every function in 1.A must work in a compatible
way in 1.B if B>A). 

However, the hypervisor negotiated version stays at 1.2 so it will use
SMCCC 1.2 for 64-bit interfaces. The host has no way of knowing this and
might as well assume that the hypervisor was implemented to fall back to
SMCCC 1.1 in this particular case. 

I don't even know that the host will ever try to renegotiate as it is
explicitly not allowed by the FF-A spec. There is no way for the
hypervisor to say, "stay at the negotiated version" so we must return
NOT_SUPPORTED. 

> Thanks,
> 
> 	M.
> 
> -- 
> Without deviation from the norm, progress is not possible.
> 
Thanks,
Per

Re: [PATCH 1/3] KVM: arm64: Restrict FF-A host version renegotiation

[Sebastian Ene]

On Fri, May 02, 2025 at 02:21:06AM -0700, Per Larsen wrote:
> From: Per Larsen <perlarsen@google.com>
> 
> FF-A implementations with the same major version must interoperate with
> earlier minor versions per DEN0077A 1.2 REL0 13.2.1 but FF-A version 1.1
> broke the ABI on several structures and 1.2 relies on SMCCC 1.2 is not

The wording here is a bit hard to follow. Why don't we re-write to
something as simple as:

"Prevent the host from re-negotiating a smaller version with the
hypervisor. Once the hypervisor negotiates a version, that should
remain locked in. Fix the current behaviour by returning NOT_SUPPORTED
to avoid the FF-A ineroperability rules with ealier minor versions which
allows the host version to downgrade."

> backwards compatible with SMCCC 1.2 (see DEN0028 1.6 G BET0 Appendix F).
> 
> If we return the negotiated hypervisor version when the host requests a
> lesser minor version, the host will rely on the FF-A interoperability
> rules. Since the hypervisor does not currently have the necessary
> compatibility paths (e.g. to handle breaking changes to the SMC calling
> convention), return NOT_SUPPORTED.



> 
> Signed-off-by: Per Larsen <perlarsen@google.com>
> Signed-off-by: Per Larsen <perl@immunant.com>
> ---
>  arch/arm64/kvm/hyp/nvhe/ffa.c | 19 ++++++++++++++++++-
>  1 file changed, 18 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/arm64/kvm/hyp/nvhe/ffa.c b/arch/arm64/kvm/hyp/nvhe/ffa.c
> index 3369dd0c4009..10e88207b78e 100644
> --- a/arch/arm64/kvm/hyp/nvhe/ffa.c
> +++ b/arch/arm64/kvm/hyp/nvhe/ffa.c
> @@ -712,7 +712,24 @@ static void do_ffa_version(struct arm_smccc_res *res,
>  
>  	hyp_spin_lock(&version_lock);
>  	if (has_version_negotiated) {
> -		res->a0 = hyp_ffa_version;
> +		/*
> +		 * FF-A implementations with the same major version must
> +		 * interoperate with earlier minor versions per DEN0077A 1.2
> +		 * REL0 13.2.1 but FF-A version 1.1 broke the ABI on several
> +		 * structures and 1.2 relies on SMCCC 1.2 is not backwards
> +		 * compatible with SMCCC 1.2 (see DEN0028 1.6 G BET0 Appendix F).
> +		 *
> +		 * If we return the negotiated hypervisor version when the host
> +		 * requests a lesser minor version, the host will rely on the
> +		 * aforementioned FF-A interoperability rules. Since the
> +		 * hypervisor does not currently have the necessary compatibility
> +		 * paths (e.g. to paper over the above-mentioned calling
> +		 * convention changes), return NOT_SUPPORTED.
> +		 */

I would drop this comment as the commit message should be pretty
descriptive.

> +		if (FFA_MINOR_VERSION(ffa_req_version) < FFA_MINOR_VERSION(hyp_ffa_version))
> +			res->a0 = FFA_RET_NOT_SUPPORTED;
> +		else
> +			res->a0 = hyp_ffa_version;
>  		goto unlock;
>  	}
>  
> -- 
> 2.49.0
> 

Thanks,
Seb

Re: [PATCH 2/3] KVM: arm64: Bump the supported version of FF-A to 1.2

[Sebastian Ene]

On Fri, May 02, 2025 at 02:21:07AM -0700, Per Larsen wrote:
> From: Per Larsen <perlarsen@google.com>
> 
> FF-A version 1.2 introduces the DIRECT_REQ2 ABI. Bump the FF-A version
> preferred by the hypervisor as a precursor to implementing the 1.2-only
> FFA_MSG_SEND_DIRECT_REQ2 and FFA_MSG_SEND_RESP2 messaging interfaces.
> 
> We must also use SMCCC 1.2 for 64-bit SMCs if hypervisor negotiated FF-A
> 1.2, so ffa_set_retval is updated and a new function to call 64-bit smcs
> using SMCCC 1.2 with fallback to SMCCC 1.1 is introduced.
> 
> Update deny-list in ffa_call_supported to mark FFA_NOTIFICATION_* and
> interfaces added in FF-A 1.2 as unsupported lest they get forwarded.
> 
> Co-Developed-by: Ayrton Munoz <ayrton@google.com>
> Signed-off-by: Per Larsen <perlarsen@google.com>
> Signed-off-by: Per Larsen <perl@immunant.com>
> ---
>  arch/arm64/kvm/hyp/nvhe/Makefile |   1 +
>  arch/arm64/kvm/hyp/nvhe/ffa.c    | 109 ++++++++++++++++++++++++++-----
>  2 files changed, 94 insertions(+), 16 deletions(-)
> 
> diff --git a/arch/arm64/kvm/hyp/nvhe/Makefile b/arch/arm64/kvm/hyp/nvhe/Makefile
> index b43426a493df..95404ff16dac 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 10e88207b78e..8102dd6a19f7 100644
> --- a/arch/arm64/kvm/hyp/nvhe/ffa.c
> +++ b/arch/arm64/kvm/hyp/nvhe/ffa.c
> @@ -94,13 +94,57 @@ static void ffa_to_smccc_res(struct arm_smccc_res *res, int ret)
>  	ffa_to_smccc_res_prop(res, ret, 0);
>  }
>  
> -static void ffa_set_retval(struct kvm_cpu_context *ctxt,
> +static void ffa_set_retval(u64 func_id,
> +			   struct kvm_cpu_context *ctxt,
>  			   struct arm_smccc_res *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;
> +
> +	/*
> +	 * Other result registers must be zero per DEN0077A but SMC32/HVC32 must
> +	 * preserve x8-x30 per DEN0028.
> +	 */
> +	cpu_reg(ctxt, 4) = 0;
> +	cpu_reg(ctxt, 5) = 0;
> +	cpu_reg(ctxt, 6) = 0;
> +	cpu_reg(ctxt, 7) = 0;
> +
> +	/* Per DEN0077A v1.2, x8-x17 must be zero for SMC64/HVC64 results*/
> +	if (ARM_SMCCC_IS_64(func_id) && hyp_ffa_version >= FFA_VERSION_1_2) {

We don't currently handle SMC32 so ARM_SMCCC_IS_64(func_id) should not
be needed and we can also drop the func_id argument.
Also hyp_ffa_version should be accessed with a lock.

> +		cpu_reg(ctxt, 8) = 0;
> +		cpu_reg(ctxt, 9) = 0;
> +		cpu_reg(ctxt, 10) = 0;
> +		cpu_reg(ctxt, 11) = 0;
> +		cpu_reg(ctxt, 12) = 0;
> +		cpu_reg(ctxt, 13) = 0;
> +		cpu_reg(ctxt, 14) = 0;
> +		cpu_reg(ctxt, 15) = 0;
> +		cpu_reg(ctxt, 16) = 0;
> +		cpu_reg(ctxt, 17) = 0;

How can we know from the hypervisor(which is the caller) whether these registers are
used or not by the calee ? IMO this should be TZ responsibility to mitigate the
risk of leaking information through register if these are not used.

> +	}
> +}
> +
> +/* 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,

Should we rename this to arm_smccc_1_x_smc to keep the same name format
?

> +				       u64 a4, u64 a5, u64 a6, u64 a7,
> +				       struct arm_smccc_res *res)
> +{
> +	struct arm_smccc_1_2_regs args, regs;

Initialize regs with {0} to prevent leakaging the hypervisor
stack values in case the callee doesn't make use of it.

> +
> +	/* SMC64 only as SMC32 must preserve x8-x30 per DEN0028 1.6G Sec 2.6 */
> +	if (ARM_SMCCC_IS_64(func_id) && hyp_ffa_version >= FFA_VERSION_1_2) {
> +		args = (struct arm_smccc_1_2_regs) { func_id, a1, a2, a3, a4,
> +						     a5, a6, a7 };
> +		arm_smccc_1_2_smc(&args, &regs);
> +		*res = (struct arm_smccc_res) { .a0 = regs.a0, .a1 = regs.a1,
> +						.a2 = regs.a2, .a3 = regs.a3 };
> +		return;
> +	}
> +
> +	arm_smccc_1_1_smc(func_id, a1, a2, a3, a4, a5, a6, a7, res);
>  }
>  
>  static bool is_ffa_call(u64 func_id)
> @@ -115,12 +159,12 @@ static int ffa_map_hyp_buffers(u64 ffa_page_count)
>  {
>  	struct arm_smccc_res res;
>  
> -	arm_smccc_1_1_smc(FFA_FN64_RXTX_MAP,
> -			  hyp_virt_to_phys(hyp_buffers.tx),
> -			  hyp_virt_to_phys(hyp_buffers.rx),
> -			  ffa_page_count,
> -			  0, 0, 0, 0,
> -			  &res);
> +	arm_smccc_1_2_smc_fallback(FFA_FN64_RXTX_MAP,
> +				   hyp_virt_to_phys(hyp_buffers.tx),
> +				   hyp_virt_to_phys(hyp_buffers.rx),
> +				   ffa_page_count,
> +				   0, 0, 0, 0,
> +				   &res);
>  
>  	return res.a0 == FFA_SUCCESS ? FFA_RET_SUCCESS : res.a2;
>  }
> @@ -174,10 +218,10 @@ static void ffa_mem_reclaim(struct arm_smccc_res *res, u32 handle_lo,
>  
>  static void ffa_retrieve_req(struct arm_smccc_res *res, u32 len)
>  {
> -	arm_smccc_1_1_smc(FFA_FN64_MEM_RETRIEVE_REQ,
> -			  len, len,
> -			  0, 0, 0, 0, 0,
> -			  res);
> +	arm_smccc_1_2_smc_fallback(FFA_FN64_MEM_RETRIEVE_REQ,
> +				   len, len,
> +				   0, 0, 0, 0, 0,
> +				   res);
>  }
>  
>  static void ffa_rx_release(struct arm_smccc_res *res)
> @@ -628,6 +672,37 @@ static bool ffa_call_supported(u64 func_id)
>  	case FFA_RXTX_MAP:
>  	case FFA_MEM_DONATE:
>  	case FFA_MEM_RETRIEVE_REQ:
> +	/* Optional notification interfaces added in FF-A 1.1 */
> +	case FFA_NOTIFICATION_BITMAP_CREATE:
> +	case FFA_NOTIFICATION_BITMAP_DESTROY:
> +	case FFA_NOTIFICATION_BIND:
> +	case FFA_NOTIFICATION_UNBIND:
> +	case FFA_NOTIFICATION_SET:
> +	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
> +	 * a guest VM because pkvm does not support communication between VMs
> +	 * via FF-A. Direct messages can only be sent from a non-secure sender
> +	 * endpoint to a secure receiver endpoint. Only receiver endpoints are
> +	 * expected to invoke FFA_MSG_SEND_DIRECT_RESP2.
> +	 */
> +	case FFA_MSG_SEND_DIRECT_RESP2:
> +
> +	/* Reserved for secure endpoints per DEN0077A 1.2 REL0 Table 13.53. */

I don't know if the comment is relevant to the upstream code, should we
drop it ? The same with the previous one and few after this.

> +	case FFA_CONSOLE_LOG:
> +
> +	/*
> +	 * Mandatory for secure endpoints per DEN0077A 1.2 REL0 Table 13.1 and
> +	 * optional for non-secure endpoints according to Table 13.38.
> +	 */
> +	case FFA_PARTITION_INFO_GET_REGS:
> +
> +	/* Reserved for secure endpoints per DEN0077A 1.2 REL0 Table 17.2. */
> +	case FFA_EL3_INTR_HANDLE:
>  		return false;
>  	}
>  
> @@ -680,7 +755,8 @@ static int hyp_ffa_post_init(void)
>  	if (res.a0 != FFA_SUCCESS)
>  		return -EOPNOTSUPP;
>  
> -	switch (res.a2) {
> +	/* Bit[0:1] holds minimum buffer size and alignment boundary */
> +	switch (res.a2 & 0x3) {

What is 0x3 ? Can you please define the mask for it ?

>  	case FFA_FEAT_RXTX_MIN_SZ_4K:
>  		min_rxtx_sz = SZ_4K;
>  		break;
> @@ -871,7 +947,7 @@ bool kvm_host_ffa_handler(struct kvm_cpu_context *host_ctxt, u32 func_id)
>  
>  	ffa_to_smccc_error(&res, FFA_RET_NOT_SUPPORTED);
>  out_handled:
> -	ffa_set_retval(host_ctxt, &res);
> +	ffa_set_retval(func_id, host_ctxt, &res);
>  	return true;
>  }
>  
> @@ -883,7 +959,7 @@ int hyp_ffa_init(void *pages)
>  	if (kvm_host_psci_config.smccc_version < ARM_SMCCC_VERSION_1_2)
>  		return 0;
>  
> -	arm_smccc_1_1_smc(FFA_VERSION, FFA_VERSION_1_1, 0, 0, 0, 0, 0, 0, &res);
> +	arm_smccc_1_1_smc(FFA_VERSION, FFA_VERSION_1_2, 0, 0, 0, 0, 0, 0, &res);
>  	if (res.a0 == FFA_RET_NOT_SUPPORTED)
>  		return 0;
>  
> @@ -903,10 +979,11 @@ int hyp_ffa_init(void *pages)
>  	if (FFA_MAJOR_VERSION(res.a0) != 1)
>  		return -EOPNOTSUPP;
>  
> -	if (FFA_MINOR_VERSION(res.a0) < FFA_MINOR_VERSION(FFA_VERSION_1_1))
> +	/* See do_ffa_guest_version before bumping maximum supported version. */
> +	if (FFA_MINOR_VERSION(res.a0) < FFA_MINOR_VERSION(FFA_VERSION_1_2))
>  		hyp_ffa_version = res.a0;
>  	else
> -		hyp_ffa_version = FFA_VERSION_1_1;
> +		hyp_ffa_version = FFA_VERSION_1_2;
>  
>  	tx = pages;
>  	pages += KVM_FFA_MBOX_NR_PAGES * PAGE_SIZE;
> -- 
> 2.49.0
> 

Re: [PATCH 3/3] KVM: arm64: Support FFA_MSG_SEND_DIRECT_REQ2 in host handler

[Sebastian Ene]

On Fri, May 02, 2025 at 02:21:08AM -0700, Per Larsen wrote:
> From: Per Larsen <perlarsen@google.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@google.com>
> Signed-off-by: Per Larsen <perl@immunant.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(&regs, 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, &regs);
> +	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
> 

Re: [PATCH 1/3] KVM: arm64: Restrict FF-A host version renegotiation

[Marc Zyngier]

On Tue, 06 May 2025 10:29:41 +0100,
Per Larsen <perl@immunant.com> wrote:
> 
> From: Per Larsen <perlarsen@google.com>
> 
> On Fri, May 02, 2025 at 09:47:45AM +0100, Marc Zyngier wrote:
> > On Fri, 02 May 2025 04:52:39 +0100,
> > Per Larsen <perl@immunant.com> wrote:
> > > 
> > > FF-A implementations with the same major version must interoperate with
> > > earlier minor versions per DEN0077A 1.2 REL0 13.2.1 but FF-A version 1.1
> > > broke the ABI on several structures and 1.2 relies on SMCCC 1.2 is not
> > > backwards compatible with SMCCC 1.2 (see DEN0028 1.6 G BET0 Appendix F).
> > > 
> > > If we return the negotiated hypervisor version when the host requests a
> > > lesser minor version, the host will rely on the FF-A interoperability
> > > rules. Since the hypervisor does not currently have the necessary
> > > compatibility paths (e.g. to handle breaking changes to the SMC calling
> > > convention), return NOT_SUPPORTED.
> > > 
> > > Signed-off-by: Per Larsen <perlarsen@google.com>
> > > Signed-off-by: Per Larsen <perl@immunant.com>
> > > ---
> > >  arch/arm64/kvm/hyp/nvhe/ffa.c | 19 ++++++++++++++++++-
> > >  1 file changed, 18 insertions(+), 1 deletion(-)
> > > 
> > > diff --git a/arch/arm64/kvm/hyp/nvhe/ffa.c b/arch/arm64/kvm/hyp/nvhe/ffa.c
> > > index 3369dd0c4009..10e88207b78e 100644
> > > --- a/arch/arm64/kvm/hyp/nvhe/ffa.c
> > > +++ b/arch/arm64/kvm/hyp/nvhe/ffa.c
> > > @@ -712,7 +712,24 @@ static void do_ffa_version(struct arm_smccc_res *res,
> > > 
> > >   hyp_spin_lock(&version_lock);
> > >   if (has_version_negotiated) {
> > > - res->a0 = hyp_ffa_version;
> > > + /*
> > > + * FF-A implementations with the same major version must
> > > + * interoperate with earlier minor versions per DEN0077A 1.2
> > > + * REL0 13.2.1 but FF-A version 1.1 broke the ABI on several
> > > + * structures and 1.2 relies on SMCCC 1.2 is not backwards
> > > + * compatible with SMCCC 1.2 (see DEN0028 1.6 G BET0 Appendix F).
> > 
> > I can't parse this sentence. Missing words?
> > 
> 
> Yes, I will fix this in v2.
> 
> > > + *
> > > + * If we return the negotiated hypervisor version when the host
> > > + * requests a lesser minor version, the host will rely on the
> > > + * aforementioned FF-A interoperability rules. Since the
> > > + * hypervisor does not currently have the necessary compatibility
> > > + * paths (e.g. to paper over the above-mentioned calling
> > > + * convention changes), return NOT_SUPPORTED.
> > > + */
> > > + if (FFA_MINOR_VERSION(ffa_req_version) < FFA_MINOR_VERSION(hyp_ffa_version))
> > > + res->a0 = FFA_RET_NOT_SUPPORTED;
> > > + else
> > > + res->a0 = hyp_ffa_version;
> > >   goto unlock;
> > >   }
> > > 
> > 
> > Something has gone seriously wrong with your email, and the patches
> > are badly mangled and unusable. They are also sent as individual
> > patches and not as a thread, which is a sign that you didn't send them
> > using git. Please fix this for your next posting.
> > 
> 
> Yes, my apologies. I will use git send-email to post v2.
> 
> > More to the meat of the patches: why should the hypervisor paper over
> > anything if the spec is broken? Why can't the host just as well decide
> > for itself what to do?
> > 
> 
> Asssuming we drop this patch from the series and apply the rest, the
> hypervisor and host can negotiate FF-A 1.2. If the host then calls
> FFA_VERSION a second time to request FF-A 1.1, the hypervisor would
> return version 1.2 (without this patch).

Why would it do that? Once a particular version has been negotiated, I
expect to be immutable.

> Per the spec, that means the
> host is can use the compatibility rules (DEN0077A Sec 13.2.1) to go
> ahead and use FF-A 1.1 (every function in 1.A must work in a compatible
> way in 1.B if B>A).

I don't interpret this as "you can switch between versions" after the
initial negotiation.

> However, the hypervisor negotiated version stays at 1.2 so it will use
> SMCCC 1.2 for 64-bit interfaces. The host has no way of knowing this and
> might as well assume that the hypervisor was implemented to fall back to
> SMCCC 1.1 in this particular case. 
> 
> I don't even know that the host will ever try to renegotiate as it is
> explicitly not allowed by the FF-A spec. There is no way for the
> hypervisor to say, "stay at the negotiated version" so we must return
> NOT_SUPPORTED. 

If it is not allowed, why should we do *anything*? And if the host is
broken, let's fix the host rather than adding pointless validation
code to EL2.

Thanks,

	M.

-- 
Without deviation from the norm, progress is not possible.

Re: [PATCH 1/3] KVM: arm64: Restrict FF-A host version renegotiation

[Sudeep Holla]

(just adding some additional info not particularly impacting the $subject
 change implementation)

On Thu, May 08, 2025 at 09:55:05AM +0100, Marc Zyngier wrote:
> On Tue, 06 May 2025 10:29:41 +0100,
> Per Larsen <perl@immunant.com> wrote:
> > 

[...]

> > Asssuming we drop this patch from the series and apply the rest, the
> > hypervisor and host can negotiate FF-A 1.2. If the host then calls
> > FFA_VERSION a second time to request FF-A 1.1, the hypervisor would
> > return version 1.2 (without this patch).
> 
> Why would it do that? Once a particular version has been negotiated, I
> expect to be immutable.
> 

Not suggesting that we need to support this, but it is technically possible
today by loading FF-A as a module—first inserting and removing a module with
v1.2 support, then loading one with v1.1 support. It can ever throw error
as not supported to keep it simple.

> > Per the spec, that means the
> > host is can use the compatibility rules (DEN0077A Sec 13.2.1) to go
> > ahead and use FF-A 1.1 (every function in 1.A must work in a compatible
> > way in 1.B if B>A).
> 
> I don't interpret this as "you can switch between versions" after the
> initial negotiation.
> 

Agreed.

> > However, the hypervisor negotiated version stays at 1.2 so it will use
> > SMCCC 1.2 for 64-bit interfaces. The host has no way of knowing this and
> > might as well assume that the hypervisor was implemented to fall back to
> > SMCCC 1.1 in this particular case. 
> > 
> > I don't even know that the host will ever try to renegotiate as it is
> > explicitly not allowed by the FF-A spec. There is no way for the
> > hypervisor to say, "stay at the negotiated version" so we must return
> > NOT_SUPPORTED. 
> 
> If it is not allowed, why should we do *anything*? And if the host is
> broken, let's fix the host rather than adding pointless validation
> code to EL2.
> 

Agreed, it is *not yet" allowed. There were some thoughts for a different
use-case IIUC, need to check the status. IIRC, it was bootloader vs OS
where bootloader like UEFI might negotiate one version(usually older) and
then OS comes and request newer version. To support such a setup, we do
need some additional support in the spec and the current latest v1.2 is not
sufficient.

-- 
Regards,
Sudeep

Re: [PATCH 1/3] KVM: arm64: Restrict FF-A host version renegotiation

[Arve Hjønnevåg]

On Thu, May 8, 2025 at 2:27 AM Sudeep Holla <sudeep.holla@arm.com> wrote:
>
> (just adding some additional info not particularly impacting the $subject
>  change implementation)
>
> On Thu, May 08, 2025 at 09:55:05AM +0100, Marc Zyngier wrote:
> > On Tue, 06 May 2025 10:29:41 +0100,
> > Per Larsen <perl@immunant.com> wrote:
> > >
>
> [...]
>
> > > Asssuming we drop this patch from the series and apply the rest, the
> > > hypervisor and host can negotiate FF-A 1.2. If the host then calls
> > > FFA_VERSION a second time to request FF-A 1.1, the hypervisor would
> > > return version 1.2 (without this patch).
> >
> > Why would it do that? Once a particular version has been negotiated, I
> > expect to be immutable.
> >
>
> Not suggesting that we need to support this, but it is technically possible
> today by loading FF-A as a module—first inserting and removing a module with
> v1.2 support, then loading one with v1.1 support. It can ever throw error
> as not supported to keep it simple.
>

I'm not sure how what you are suggesting here is different from what
this patch does. This patch does not alter what versions the host can
negotiate. The hypervisor already disallows negotiating a different
version once has_version_negotiated is set, the return code just
doesn't always reflect this. If you try to load the 1.0 driver in the
host after unloading the 1.1 driver similar to what you describe
above, then this CL will let the 1.0 driver know that the hypervisor
does not support 1.0 (I use 1.1 to 1.0 as the example here since this
is an issue even without the next CL in this patch series that bumps
the hypervisor supported version to 1.2). Without this CL, the 1.0
driver will now proceed making other ffa calls using 1.0 data
structures that the hypervisor will incorrectly interpret as 1.1 data
structures.

With this CL, loading a 1.2 driver after the initial 1.1 driver will
work as it did before by returning version 1.1 to the 1.2 driver to
let it know that _it_ needs to downgrade to 1.1. if it wants to
proceed. Loading the 1.0 driver after 1.1 will now fail at the version
negotiation stage however. This will be clearer, and more correct,
than getting FFA_RET_INVALID_PARAMETERS return codes from other ffa
calls when passing valid 1.0 parameters to those calls.

--
Arve Hjønnevåg

Re: [PATCH 1/3] KVM: arm64: Restrict FF-A host version renegotiation

[Sudeep Holla]

On Thu, May 08, 2025 at 08:45:01AM -0700, Arve Hjønnevåg wrote:
> On Thu, May 8, 2025 at 2:27 AM Sudeep Holla <sudeep.holla@arm.com> wrote:
> >
> > (just adding some additional info not particularly impacting the $subject
> >  change implementation)
> >
> > On Thu, May 08, 2025 at 09:55:05AM +0100, Marc Zyngier wrote:
> > > On Tue, 06 May 2025 10:29:41 +0100,
> > > Per Larsen <perl@immunant.com> wrote:
> > > >
> >
> > [...]
> >
> > > > Asssuming we drop this patch from the series and apply the rest, the
> > > > hypervisor and host can negotiate FF-A 1.2. If the host then calls
> > > > FFA_VERSION a second time to request FF-A 1.1, the hypervisor would
> > > > return version 1.2 (without this patch).
> > >
> > > Why would it do that? Once a particular version has been negotiated, I
> > > expect to be immutable.
> > >
> >
> > Not suggesting that we need to support this, but it is technically possible
> > today by loading FF-A as a module—first inserting and removing a module with
> > v1.2 support, then loading one with v1.1 support. It can ever throw error
> > as not supported to keep it simple.
> >
> 
> I'm not sure how what you are suggesting here is different from what
> this patch does. This patch does not alter what versions the host can
> negotiate. The hypervisor already disallows negotiating a different
> version once has_version_negotiated is set, the return code just
> doesn't always reflect this. If you try to load the 1.0 driver in the
> host after unloading the 1.1 driver similar to what you describe
> above, then this CL will let the 1.0 driver know that the hypervisor
> does not support 1.0 (I use 1.1 to 1.0 as the example here since this
> is an issue even without the next CL in this patch series that bumps
> the hypervisor supported version to 1.2). Without this CL, the 1.0
> driver will now proceed making other ffa calls using 1.0 data
> structures that the hypervisor will incorrectly interpret as 1.1 data
> structures.
> 
> With this CL, loading a 1.2 driver after the initial 1.1 driver will
> work as it did before by returning version 1.1 to the 1.2 driver to
> let it know that _it_ needs to downgrade to 1.1. if it wants to
> proceed. Loading the 1.0 driver after 1.1 will now fail at the version
> negotiation stage however. This will be clearer, and more correct,
> than getting FFA_RET_INVALID_PARAMETERS return codes from other ffa
> calls when passing valid 1.0 parameters to those calls.
> 

Thanks for the detailed explanation. Sorry I didn't look at the change itself
and I might have made assumptions as I just read

  |  If the host then calls FFA_VERSION a second time to request FF-A 1.1,
  |  the hypervisor would return version 1.2 (without this patch).

So, my point was just that it is OK to even report it as NOT_SUPPORTED
if FF-A proxy doesn't want to deal with all the compatibility for simplicity.

-- 
Regards,
Sudeep