[PATCH v2 0/4] KVM: arm64: pKVM host proxy FF-A fixes

[PATCH v2 0/4] KVM: arm64: pKVM host proxy FF-A fixes

[Sebastian Ene]

Hello,


This series contains some small fixes for the host pKVM proxy code. I included
some of the patches that I already sent on the list as part of this series
to make it easier to keep track of them.

I verified the functionality with OPTEE as a TEE-OS.

Changelog:
 
v1 -> v2:

* applied the feedback received from Will on the FFA_VERSION patch:
  The spec requires that no other calls to be issued prior to the FFA
  version negotiation and the current change reflects this. After the
  version negotiation phase is complete with Trustzone we will just
  return the hypervisor version.

* corrected some mistakes on the FFA_PARTITION_INFO_GET patch:
  - don't trim the number of bytes copied from the hypervisor buffer
  - introduce FFA_1_0_PARTITON_INFO_SZ definition
  - simplify the logic when the input flag is specified 

* collected the Ack from Will and embbeded it in the commit - Thanks Will ! 

v1:

* previously posted FFA_PARTITION_INFO_GET patch here:
https://lore.kernel.org/kvmarm/20240411133249.2134696-1-sebastianene@google.com/
 -> minor changes from the previous version, look for the current
    ffa_version in the host_buffer structure

* previously posted "Fix the identification range for the FF-A smcs" here:
https://lore.kernel.org/kvmarm/20240322124303.309423-1-sebastianene@google.com/

Thank you,
Sebastian

Sebastian Ene (4):
  KVM: arm64: Trap FFA_VERSION host call in pKVM
  KVM: arm64: Add support for FFA_PARTITION_INFO_GET
  KVM: arm64: Fix the identification range for the FF-A smcs
  KVM: arm64: Use FF-A 1.1 with pKVM

 arch/arm64/kvm/hyp/include/nvhe/ffa.h |   2 +-
 arch/arm64/kvm/hyp/nvhe/ffa.c         | 183 +++++++++++++++++++++-----
 include/linux/arm_ffa.h               |   3 +
 3 files changed, 154 insertions(+), 34 deletions(-)

-- 
2.45.0.rc1.225.g2a3ae87e7f-goog


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v2 1/4] KVM: arm64: Trap FFA_VERSION host call in pKVM

[Sebastian Ene]

The pKVM hypervisor initializes with FF-A version 1.0. The spec requires
that no other FF-A calls to be issued before the version negotiation
phase is complete. Split the hypervisor proxy initialization code in two
parts so that we can move the later one after the host negotiates its
version.
Without trapping the call, the host drivers can negotiate a higher
version number with TEE which can result in a different memory layout
described during the memory sharing calls.

Signed-off-by: Sebastian Ene <sebastianene@google.com>
---
 arch/arm64/kvm/hyp/nvhe/ffa.c | 123 +++++++++++++++++++++++++---------
 1 file changed, 92 insertions(+), 31 deletions(-)

diff --git a/arch/arm64/kvm/hyp/nvhe/ffa.c b/arch/arm64/kvm/hyp/nvhe/ffa.c
index 320f2eaa14a9..72f1206c85fb 100644
--- a/arch/arm64/kvm/hyp/nvhe/ffa.c
+++ b/arch/arm64/kvm/hyp/nvhe/ffa.c
@@ -1,4 +1,5 @@
 // SPDX-License-Identifier: GPL-2.0-only
+//
 /*
  * FF-A v1.0 proxy to filter out invalid memory-sharing SMC calls issued by
  * the host. FF-A is a slightly more palatable abbreviation of "Arm Firmware
@@ -67,6 +68,9 @@ struct kvm_ffa_buffers {
  */
 static struct kvm_ffa_buffers hyp_buffers;
 static struct kvm_ffa_buffers host_buffers;
+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)
 {
@@ -640,6 +644,83 @@ static bool do_ffa_features(struct arm_smccc_res *res,
 	return true;
 }
 
+static int hyp_ffa_post_init(void)
+{
+	size_t min_rxtx_sz;
+	struct arm_smccc_res res;
+
+	arm_smccc_1_1_smc(FFA_ID_GET, 0, 0, 0, 0, 0, 0, 0, &res);
+	if (res.a0 != FFA_SUCCESS)
+		return -EOPNOTSUPP;
+
+	if (res.a2 != HOST_FFA_ID)
+		return -EINVAL;
+
+	arm_smccc_1_1_smc(FFA_FEATURES, FFA_FN64_RXTX_MAP,
+			  0, 0, 0, 0, 0, 0, &res);
+	if (res.a0 != FFA_SUCCESS)
+		return -EOPNOTSUPP;
+
+	switch (res.a2) {
+	case FFA_FEAT_RXTX_MIN_SZ_4K:
+		min_rxtx_sz = SZ_4K;
+		break;
+	case FFA_FEAT_RXTX_MIN_SZ_16K:
+		min_rxtx_sz = SZ_16K;
+		break;
+	case FFA_FEAT_RXTX_MIN_SZ_64K:
+		min_rxtx_sz = SZ_64K;
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	if (min_rxtx_sz > PAGE_SIZE)
+		return -EOPNOTSUPP;
+
+	return 0;
+}
+
+static void do_ffa_version(struct arm_smccc_res *res,
+			   struct kvm_cpu_context *ctxt)
+{
+	DECLARE_REG(u32, ffa_req_version, ctxt, 1);
+
+	if (FFA_MAJOR_VERSION(ffa_req_version) != 1) {
+		res->a0 = FFA_RET_NOT_SUPPORTED;
+		return;
+	}
+
+	hyp_spin_lock(&version_lock);
+	if (has_version_negotiated) {
+		res->a0 = hyp_ffa_version;
+		goto unlock;
+	}
+
+	/*
+	 * If the client driver tries to downgrade the version, we need to ask
+	 * first if TEE supports it.
+	 */
+	if (FFA_MINOR_VERSION(ffa_req_version) < FFA_MINOR_VERSION(hyp_ffa_version)) {
+		arm_smccc_1_1_smc(FFA_VERSION, ffa_req_version, 0,
+				  0, 0, 0, 0, 0,
+				  res);
+		if (res->a0 == FFA_RET_NOT_SUPPORTED)
+			goto unlock;
+
+		hyp_ffa_version = ffa_req_version;
+	}
+
+	if (hyp_ffa_post_init())
+		res->a0 = FFA_RET_NOT_SUPPORTED;
+	else {
+		has_version_negotiated = true;
+		res->a0 = hyp_ffa_version;
+	}
+unlock:
+	hyp_spin_unlock(&version_lock);
+}
+
 bool kvm_host_ffa_handler(struct kvm_cpu_context *host_ctxt, u32 func_id)
 {
 	struct arm_smccc_res res;
@@ -660,6 +741,11 @@ bool kvm_host_ffa_handler(struct kvm_cpu_context *host_ctxt, u32 func_id)
 	if (!is_ffa_call(func_id))
 		return false;
 
+	if (!has_version_negotiated && func_id != FFA_VERSION) {
+		ffa_to_smccc_error(&res, FFA_RET_INVALID_PARAMETERS);
+		goto out_handled;
+	}
+
 	switch (func_id) {
 	case FFA_FEATURES:
 		if (!do_ffa_features(&res, host_ctxt))
@@ -686,6 +772,9 @@ bool kvm_host_ffa_handler(struct kvm_cpu_context *host_ctxt, u32 func_id)
 	case FFA_MEM_FRAG_TX:
 		do_ffa_mem_frag_tx(&res, host_ctxt);
 		goto out_handled;
+	case FFA_VERSION:
+		do_ffa_version(&res, host_ctxt);
+		goto out_handled;
 	}
 
 	if (ffa_call_supported(func_id))
@@ -700,7 +789,6 @@ bool kvm_host_ffa_handler(struct kvm_cpu_context *host_ctxt, u32 func_id)
 int hyp_ffa_init(void *pages)
 {
 	struct arm_smccc_res res;
-	size_t min_rxtx_sz;
 	void *tx, *rx;
 
 	if (kvm_host_psci_config.smccc_version < ARM_SMCCC_VERSION_1_2)
@@ -726,35 +814,7 @@ int hyp_ffa_init(void *pages)
 	if (FFA_MAJOR_VERSION(res.a0) != 1)
 		return -EOPNOTSUPP;
 
-	arm_smccc_1_1_smc(FFA_ID_GET, 0, 0, 0, 0, 0, 0, 0, &res);
-	if (res.a0 != FFA_SUCCESS)
-		return -EOPNOTSUPP;
-
-	if (res.a2 != HOST_FFA_ID)
-		return -EINVAL;
-
-	arm_smccc_1_1_smc(FFA_FEATURES, FFA_FN64_RXTX_MAP,
-			  0, 0, 0, 0, 0, 0, &res);
-	if (res.a0 != FFA_SUCCESS)
-		return -EOPNOTSUPP;
-
-	switch (res.a2) {
-	case FFA_FEAT_RXTX_MIN_SZ_4K:
-		min_rxtx_sz = SZ_4K;
-		break;
-	case FFA_FEAT_RXTX_MIN_SZ_16K:
-		min_rxtx_sz = SZ_16K;
-		break;
-	case FFA_FEAT_RXTX_MIN_SZ_64K:
-		min_rxtx_sz = SZ_64K;
-		break;
-	default:
-		return -EINVAL;
-	}
-
-	if (min_rxtx_sz > PAGE_SIZE)
-		return -EOPNOTSUPP;
-
+	hyp_ffa_version = FFA_VERSION_1_0;
 	tx = pages;
 	pages += KVM_FFA_MBOX_NR_PAGES * PAGE_SIZE;
 	rx = pages;
@@ -773,8 +833,9 @@ int hyp_ffa_init(void *pages)
 	};
 
 	host_buffers = (struct kvm_ffa_buffers) {
-		.lock	= __HYP_SPIN_LOCK_UNLOCKED,
+		.lock   = __HYP_SPIN_LOCK_UNLOCKED,
 	};
 
+	version_lock = __HYP_SPIN_LOCK_UNLOCKED;
 	return 0;
 }
-- 
2.45.0.rc1.225.g2a3ae87e7f-goog


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v2 1/4] KVM: arm64: Trap FFA_VERSION host call in pKVM

[Sudeep Holla]

On Wed, May 15, 2024 at 05:22:55PM +0000, Sebastian Ene wrote:
> The pKVM hypervisor initializes with FF-A version 1.0. The spec requires
> that no other FF-A calls to be issued before the version negotiation
> phase is complete. Split the hypervisor proxy initialization code in two
> parts so that we can move the later one after the host negotiates its
> version.

Blank line here would be nice.

> Without trapping the call, the host drivers can negotiate a higher
> version number with TEE which can result in a different memory layout
> described during the memory sharing calls.
>

LGTM(apart from minor nits),

Reviewed-by: Sudeep Holla <sudeep.holla@arm.com>

> Signed-off-by: Sebastian Ene <sebastianene@google.com>
> ---
>  arch/arm64/kvm/hyp/nvhe/ffa.c | 123 +++++++++++++++++++++++++---------
>  1 file changed, 92 insertions(+), 31 deletions(-)
>
> diff --git a/arch/arm64/kvm/hyp/nvhe/ffa.c b/arch/arm64/kvm/hyp/nvhe/ffa.c
> index 320f2eaa14a9..72f1206c85fb 100644
> --- a/arch/arm64/kvm/hyp/nvhe/ffa.c
> +++ b/arch/arm64/kvm/hyp/nvhe/ffa.c
> @@ -1,4 +1,5 @@
>  // SPDX-License-Identifier: GPL-2.0-only
> +//

It should be OK but still spurious for $subject 😉

[...]

> @@ -700,7 +789,6 @@ bool kvm_host_ffa_handler(struct kvm_cpu_context *host_ctxt, u32 func_id)
>  int hyp_ffa_init(void *pages)
>  {
>  	struct arm_smccc_res res;
> -	size_t min_rxtx_sz;
>  	void *tx, *rx;
>
>  	if (kvm_host_psci_config.smccc_version < ARM_SMCCC_VERSION_1_2)
> @@ -726,35 +814,7 @@ int hyp_ffa_init(void *pages)
>  	if (FFA_MAJOR_VERSION(res.a0) != 1)
>  		return -EOPNOTSUPP;
>
> -	arm_smccc_1_1_smc(FFA_ID_GET, 0, 0, 0, 0, 0, 0, 0, &res);
> -	if (res.a0 != FFA_SUCCESS)
> -		return -EOPNOTSUPP;
> -
> -	if (res.a2 != HOST_FFA_ID)
> -		return -EINVAL;
> -
> -	arm_smccc_1_1_smc(FFA_FEATURES, FFA_FN64_RXTX_MAP,
> -			  0, 0, 0, 0, 0, 0, &res);
> -	if (res.a0 != FFA_SUCCESS)
> -		return -EOPNOTSUPP;
> -
> -	switch (res.a2) {
> -	case FFA_FEAT_RXTX_MIN_SZ_4K:
> -		min_rxtx_sz = SZ_4K;
> -		break;
> -	case FFA_FEAT_RXTX_MIN_SZ_16K:
> -		min_rxtx_sz = SZ_16K;
> -		break;
> -	case FFA_FEAT_RXTX_MIN_SZ_64K:
> -		min_rxtx_sz = SZ_64K;
> -		break;
> -	default:
> -		return -EINVAL;
> -	}
> -
> -	if (min_rxtx_sz > PAGE_SIZE)
> -		return -EOPNOTSUPP;
> -
> +	hyp_ffa_version = FFA_VERSION_1_0;
>  	tx = pages;
>  	pages += KVM_FFA_MBOX_NR_PAGES * PAGE_SIZE;
>  	rx = pages;
> @@ -773,8 +833,9 @@ int hyp_ffa_init(void *pages)
>  	};
>
>  	host_buffers = (struct kvm_ffa_buffers) {
> -		.lock	= __HYP_SPIN_LOCK_UNLOCKED,
> +		.lock   = __HYP_SPIN_LOCK_UNLOCKED,

Spurious or intentional whitespace change ? I can't make out from the mail.

--
Regards,
Sudeep

Re: [PATCH v2 1/4] KVM: arm64: Trap FFA_VERSION host call in pKVM

[Sebastian Ene]

On Wed, Jun 12, 2024 at 02:58:31PM +0100, Sudeep Holla wrote:
> On Wed, May 15, 2024 at 05:22:55PM +0000, Sebastian Ene wrote:
> > The pKVM hypervisor initializes with FF-A version 1.0. The spec requires
> > that no other FF-A calls to be issued before the version negotiation
> > phase is complete. Split the hypervisor proxy initialization code in two
> > parts so that we can move the later one after the host negotiates its
> > version.
> 
> Blank line here would be nice.
> 

Added blank line.

> > Without trapping the call, the host drivers can negotiate a higher
> > version number with TEE which can result in a different memory layout
> > described during the memory sharing calls.
> >
> 
> LGTM(apart from minor nits),
> 
> Reviewed-by: Sudeep Holla <sudeep.holla@arm.com>
> 

Thanks Sudeep, I will incorporate the tag it in v3.

> > Signed-off-by: Sebastian Ene <sebastianene@google.com>
> > ---
> >  arch/arm64/kvm/hyp/nvhe/ffa.c | 123 +++++++++++++++++++++++++---------
> >  1 file changed, 92 insertions(+), 31 deletions(-)
> >
> > diff --git a/arch/arm64/kvm/hyp/nvhe/ffa.c b/arch/arm64/kvm/hyp/nvhe/ffa.c
> > index 320f2eaa14a9..72f1206c85fb 100644
> > --- a/arch/arm64/kvm/hyp/nvhe/ffa.c
> > +++ b/arch/arm64/kvm/hyp/nvhe/ffa.c
> > @@ -1,4 +1,5 @@
> >  // SPDX-License-Identifier: GPL-2.0-only
> > +//
> 
> It should be OK but still spurious for $subject 😉
> 
> [...]
> 

Dropped this.

> > @@ -700,7 +789,6 @@ bool kvm_host_ffa_handler(struct kvm_cpu_context *host_ctxt, u32 func_id)
> >  int hyp_ffa_init(void *pages)
> >  {
> >  	struct arm_smccc_res res;
> > -	size_t min_rxtx_sz;
> >  	void *tx, *rx;
> >
> >  	if (kvm_host_psci_config.smccc_version < ARM_SMCCC_VERSION_1_2)
> > @@ -726,35 +814,7 @@ int hyp_ffa_init(void *pages)
> >  	if (FFA_MAJOR_VERSION(res.a0) != 1)
> >  		return -EOPNOTSUPP;
> >
> > -	arm_smccc_1_1_smc(FFA_ID_GET, 0, 0, 0, 0, 0, 0, 0, &res);
> > -	if (res.a0 != FFA_SUCCESS)
> > -		return -EOPNOTSUPP;
> > -
> > -	if (res.a2 != HOST_FFA_ID)
> > -		return -EINVAL;
> > -
> > -	arm_smccc_1_1_smc(FFA_FEATURES, FFA_FN64_RXTX_MAP,
> > -			  0, 0, 0, 0, 0, 0, &res);
> > -	if (res.a0 != FFA_SUCCESS)
> > -		return -EOPNOTSUPP;
> > -
> > -	switch (res.a2) {
> > -	case FFA_FEAT_RXTX_MIN_SZ_4K:
> > -		min_rxtx_sz = SZ_4K;
> > -		break;
> > -	case FFA_FEAT_RXTX_MIN_SZ_16K:
> > -		min_rxtx_sz = SZ_16K;
> > -		break;
> > -	case FFA_FEAT_RXTX_MIN_SZ_64K:
> > -		min_rxtx_sz = SZ_64K;
> > -		break;
> > -	default:
> > -		return -EINVAL;
> > -	}
> > -
> > -	if (min_rxtx_sz > PAGE_SIZE)
> > -		return -EOPNOTSUPP;
> > -
> > +	hyp_ffa_version = FFA_VERSION_1_0;
> >  	tx = pages;
> >  	pages += KVM_FFA_MBOX_NR_PAGES * PAGE_SIZE;
> >  	rx = pages;
> > @@ -773,8 +833,9 @@ int hyp_ffa_init(void *pages)
> >  	};
> >
> >  	host_buffers = (struct kvm_ffa_buffers) {
> > -		.lock	= __HYP_SPIN_LOCK_UNLOCKED,
> > +		.lock   = __HYP_SPIN_LOCK_UNLOCKED,
> 
> Spurious or intentional whitespace change ? I can't make out from the mail.
> 
> --

I dropped this, it was a spurious whitespace change.

> Regards,
> Sudeep

Cheers,
Seb

[PATCH v2 2/4] KVM: arm64: Add support for FFA_PARTITION_INFO_GET

[Sebastian Ene]

Handle the FFA_PARTITION_INFO_GET host call inside the pKVM hypervisor
and copy the response message back to the host buffers. Save the
returned FF-A version as we will need it later to interpret the response
from the TEE.

Signed-off-by: Sebastian Ene <sebastianene@google.com>
---
 arch/arm64/kvm/hyp/nvhe/ffa.c | 52 +++++++++++++++++++++++++++++++++++
 include/linux/arm_ffa.h       |  3 ++
 2 files changed, 55 insertions(+)

diff --git a/arch/arm64/kvm/hyp/nvhe/ffa.c b/arch/arm64/kvm/hyp/nvhe/ffa.c
index 72f1206c85fb..f9664c4a348e 100644
--- a/arch/arm64/kvm/hyp/nvhe/ffa.c
+++ b/arch/arm64/kvm/hyp/nvhe/ffa.c
@@ -721,6 +721,55 @@ static void do_ffa_version(struct arm_smccc_res *res,
 	hyp_spin_unlock(&version_lock);
 }
 
+static void do_ffa_part_get(struct arm_smccc_res *res,
+			    struct kvm_cpu_context *ctxt)
+{
+	DECLARE_REG(u32, uuid0, ctxt, 1);
+	DECLARE_REG(u32, uuid1, ctxt, 2);
+	DECLARE_REG(u32, uuid2, ctxt, 3);
+	DECLARE_REG(u32, uuid3, ctxt, 4);
+	DECLARE_REG(u32, flags, ctxt, 5);
+	u32 count, partition_sz, copy_sz;
+
+	hyp_spin_lock(&host_buffers.lock);
+	if (!host_buffers.rx) {
+		ffa_to_smccc_res(res, FFA_RET_BUSY);
+		goto out_unlock;
+	}
+
+	arm_smccc_1_1_smc(FFA_PARTITION_INFO_GET, uuid0, uuid1,
+			  uuid2, uuid3, flags, 0, 0,
+			  res);
+
+	if (res->a0 != FFA_SUCCESS)
+		goto out_unlock;
+
+	count = res->a2;
+	if (!count)
+		goto out_unlock;
+
+	if (hyp_ffa_version > FFA_VERSION_1_0) {
+		/* Get the number of partitions deployed in the system */
+		if (flags & 0x1)
+			goto out_unlock;
+
+		partition_sz  = res->a3;
+	} else {
+		/* FFA_VERSION_1_0 lacks the size in the response */
+		partition_sz = FFA_1_0_PARTITON_INFO_SZ;
+	}
+
+	copy_sz = partition_sz * count;
+	if (copy_sz > KVM_FFA_MBOX_NR_PAGES * PAGE_SIZE) {
+		ffa_to_smccc_res(res, FFA_RET_ABORTED);
+		goto out_unlock;
+	}
+
+	memcpy(host_buffers.rx, hyp_buffers.rx, copy_sz);
+out_unlock:
+	hyp_spin_unlock(&host_buffers.lock);
+}
+
 bool kvm_host_ffa_handler(struct kvm_cpu_context *host_ctxt, u32 func_id)
 {
 	struct arm_smccc_res res;
@@ -775,6 +824,9 @@ bool kvm_host_ffa_handler(struct kvm_cpu_context *host_ctxt, u32 func_id)
 	case FFA_VERSION:
 		do_ffa_version(&res, host_ctxt);
 		goto out_handled;
+	case FFA_PARTITION_INFO_GET:
+		do_ffa_part_get(&res, host_ctxt);
+		goto out_handled;
 	}
 
 	if (ffa_call_supported(func_id))
diff --git a/include/linux/arm_ffa.h b/include/linux/arm_ffa.h
index c906f666ff5d..12a7fa2d6550 100644
--- a/include/linux/arm_ffa.h
+++ b/include/linux/arm_ffa.h
@@ -211,6 +211,9 @@ bool ffa_device_is_valid(struct ffa_device *ffa_dev) { return false; }
 
 extern const struct bus_type ffa_bus_type;
 
+/* The FF-A 1.0 partition structure lacks the uuid[4] */
+#define FFA_1_0_PARTITON_INFO_SZ	(8)
+
 /* FFA transport related */
 struct ffa_partition_info {
 	u16 id;
-- 
2.45.0.rc1.225.g2a3ae87e7f-goog


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v2 2/4] KVM: arm64: Add support for FFA_PARTITION_INFO_GET

[Sudeep Holla]

On Wed, May 15, 2024 at 05:22:56PM +0000, Sebastian Ene wrote:
> Handle the FFA_PARTITION_INFO_GET host call inside the pKVM hypervisor
> and copy the response message back to the host buffers.
>
> Save the returned FF-A version as we will need it later to interpret the
> response from the TEE.
>
I don't understand the context of the above statement and this patch.
Did you mean use the saved version to take apt action/decision within
FFA_PARTITION_INFO_GET handler. Or is it just missed cleanup ?

The change itself looks good.

Reviewed-by: Sudeep Holla <sudeep.holla@arm.com>

--
Regards,
Sudeep

Re: [PATCH v2 2/4] KVM: arm64: Add support for FFA_PARTITION_INFO_GET

[Sebastian Ene]

On Wed, Jun 12, 2024 at 03:30:29PM +0100, Sudeep Holla wrote:
> On Wed, May 15, 2024 at 05:22:56PM +0000, Sebastian Ene wrote:
> > Handle the FFA_PARTITION_INFO_GET host call inside the pKVM hypervisor
> > and copy the response message back to the host buffers.
> >
> > Save the returned FF-A version as we will need it later to interpret the
> > response from the TEE.
> >
> I don't understand the context of the above statement and this patch.
> Did you mean use the saved version to take apt action/decision within
> FFA_PARTITION_INFO_GET handler. Or is it just missed cleanup ?

This is a missed cleanup I will drop this part from the commit message.

Thanks,
Seb

> 
> The change itself looks good.
> 
> Reviewed-by: Sudeep Holla <sudeep.holla@arm.com>
> 
> --
> Regards,
> Sudeep

[PATCH v2 3/4] KVM: arm64: Fix the identification range for the FF-A smcs

[Sebastian Ene]

The FF-A spec 1.2 reserves the following ranges for identifying FF-A
calls:
0x84000060-0x840000FF: FF-A 32-bit calls
0xC4000060-0xC40000FF: FF-A 64-bit calls.

Use the range identification according to the spec and allow calls that
are currently out of the range(eg. FFA_MSG_SEND_DIRECT_REQ2) to be
identified correctly.

Acked-by: Will Deacon <will@kernel.org>
Signed-off-by: Sebastian Ene <sebastianene@google.com>
---
 arch/arm64/kvm/hyp/include/nvhe/ffa.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm64/kvm/hyp/include/nvhe/ffa.h b/arch/arm64/kvm/hyp/include/nvhe/ffa.h
index d9fd5e6c7d3c..146e0aebfa1c 100644
--- a/arch/arm64/kvm/hyp/include/nvhe/ffa.h
+++ b/arch/arm64/kvm/hyp/include/nvhe/ffa.h
@@ -9,7 +9,7 @@
 #include <asm/kvm_host.h>
 
 #define FFA_MIN_FUNC_NUM 0x60
-#define FFA_MAX_FUNC_NUM 0x7F
+#define FFA_MAX_FUNC_NUM 0xFF
 
 int hyp_ffa_init(void *pages);
 bool kvm_host_ffa_handler(struct kvm_cpu_context *host_ctxt, u32 func_id);
-- 
2.45.0.rc1.225.g2a3ae87e7f-goog


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v2 3/4] KVM: arm64: Fix the identification range for the FF-A smcs

[Sudeep Holla]

On Wed, May 15, 2024 at 05:22:57PM +0000, Sebastian Ene wrote:
> The FF-A spec 1.2 reserves the following ranges for identifying FF-A
> calls:
> 0x84000060-0x840000FF: FF-A 32-bit calls
> 0xC4000060-0xC40000FF: FF-A 64-bit calls.
>
> Use the range identification according to the spec and allow calls that
> are currently out of the range(eg. FFA_MSG_SEND_DIRECT_REQ2) to be
> identified correctly.
>

Reviewed-by: Sudeep Holla <sudeep.holla@arm.com>

Not sure if this needs to be fixes though. With addition of notifications
in the FF-A driver(since v6.7), host can send FF-A messages beyond 0x7F.
But since the pKVM FF-A proxy support is not yet updated to v1.1, so I
don't think it needs to be tagged as fix. Just thought I will mention it
here anyways.

--
Regards,
Sudeep

Re: [PATCH v2 3/4] KVM: arm64: Fix the identification range for the FF-A smcs

[Sebastian Ene]

On Wed, Jun 12, 2024 at 03:38:26PM +0100, Sudeep Holla wrote:
> On Wed, May 15, 2024 at 05:22:57PM +0000, Sebastian Ene wrote:
> > The FF-A spec 1.2 reserves the following ranges for identifying FF-A
> > calls:
> > 0x84000060-0x840000FF: FF-A 32-bit calls
> > 0xC4000060-0xC40000FF: FF-A 64-bit calls.
> >
> > Use the range identification according to the spec and allow calls that
> > are currently out of the range(eg. FFA_MSG_SEND_DIRECT_REQ2) to be
> > identified correctly.
> >
> 
> Reviewed-by: Sudeep Holla <sudeep.holla@arm.com>
> 
> Not sure if this needs to be fixes though. With addition of notifications
> in the FF-A driver(since v6.7), host can send FF-A messages beyond 0x7F.
> But since the pKVM FF-A proxy support is not yet updated to v1.1, so I
> don't think it needs to be tagged as fix. Just thought I will mention it
> here anyways.

Yes, good point. I will rewrite the title of the commit to remove the
fixes tag.

> 
> --
> Regards,
> Sudeep

Thanks,
Seb

[PATCH v2 4/4] KVM: arm64: Use FF-A 1.1 with pKVM

[Sebastian Ene]

Now that the layout of the structures is compatible with 1.1 it is time
to probe the 1.1 version of the FF-A protocol inside the hypervisor. If
the TEE doesn't support it, it should return the minimum supported
version.

Signed-off-by: Sebastian Ene <sebastianene@google.com>
---
 arch/arm64/kvm/hyp/nvhe/ffa.c | 12 ++++++++----
 1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/arch/arm64/kvm/hyp/nvhe/ffa.c b/arch/arm64/kvm/hyp/nvhe/ffa.c
index f9664c4a348e..bdd70eb4114e 100644
--- a/arch/arm64/kvm/hyp/nvhe/ffa.c
+++ b/arch/arm64/kvm/hyp/nvhe/ffa.c
@@ -458,7 +458,7 @@ static __always_inline void do_ffa_mem_xfer(const u64 func_id,
 	memcpy(buf, host_buffers.tx, fraglen);
 
 	ep_mem_access = (void *)buf +
-			ffa_mem_desc_offset(buf, 0, FFA_VERSION_1_0);
+			ffa_mem_desc_offset(buf, 0, hyp_ffa_version);
 	offset = ep_mem_access->composite_off;
 	if (!offset || buf->ep_count != 1 || buf->sender_id != HOST_FFA_ID) {
 		ret = FFA_RET_INVALID_PARAMETERS;
@@ -537,7 +537,7 @@ static void do_ffa_mem_reclaim(struct arm_smccc_res *res,
 	fraglen = res->a2;
 
 	ep_mem_access = (void *)buf +
-			ffa_mem_desc_offset(buf, 0, FFA_VERSION_1_0);
+			ffa_mem_desc_offset(buf, 0, hyp_ffa_version);
 	offset = ep_mem_access->composite_off;
 	/*
 	 * We can trust the SPMD to get this right, but let's at least
@@ -846,7 +846,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_0, 0, 0, 0, 0, 0, 0, &res);
+	arm_smccc_1_1_smc(FFA_VERSION, FFA_VERSION_1_1, 0, 0, 0, 0, 0, 0, &res);
 	if (res.a0 == FFA_RET_NOT_SUPPORTED)
 		return 0;
 
@@ -866,7 +866,11 @@ int hyp_ffa_init(void *pages)
 	if (FFA_MAJOR_VERSION(res.a0) != 1)
 		return -EOPNOTSUPP;
 
-	hyp_ffa_version = FFA_VERSION_1_0;
+	if (FFA_MINOR_VERSION(res.a0) < FFA_MINOR_VERSION(FFA_VERSION_1_1))
+		hyp_ffa_version = res.a0;
+	else
+		hyp_ffa_version = FFA_VERSION_1_1;
+
 	tx = pages;
 	pages += KVM_FFA_MBOX_NR_PAGES * PAGE_SIZE;
 	rx = pages;
-- 
2.45.0.rc1.225.g2a3ae87e7f-goog


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v2 4/4] KVM: arm64: Use FF-A 1.1 with pKVM

[Sudeep Holla]

On Wed, May 15, 2024 at 05:22:58PM +0000, Sebastian Ene wrote:
> Now that the layout of the structures is compatible with 1.1 it is time
> to probe the 1.1 version of the FF-A protocol inside the hypervisor. If
> the TEE doesn't support it, it should return the minimum supported
> version.
>

LGTM,

Reviewed-by: Sudeep Holla <sudeep.holla@arm.com>

> Signed-off-by: Sebastian Ene <sebastianene@google.com>
> ---
>  arch/arm64/kvm/hyp/nvhe/ffa.c | 12 ++++++++----
>  1 file changed, 8 insertions(+), 4 deletions(-)
>
> diff --git a/arch/arm64/kvm/hyp/nvhe/ffa.c b/arch/arm64/kvm/hyp/nvhe/ffa.c
> index f9664c4a348e..bdd70eb4114e 100644
> --- a/arch/arm64/kvm/hyp/nvhe/ffa.c
> +++ b/arch/arm64/kvm/hyp/nvhe/ffa.c
> @@ -458,7 +458,7 @@ static __always_inline void do_ffa_mem_xfer(const u64 func_id,
>  	memcpy(buf, host_buffers.tx, fraglen);
>
>  	ep_mem_access = (void *)buf +
> -			ffa_mem_desc_offset(buf, 0, FFA_VERSION_1_0);
> +			ffa_mem_desc_offset(buf, 0, hyp_ffa_version);
>  	offset = ep_mem_access->composite_off;
>  	if (!offset || buf->ep_count != 1 || buf->sender_id != HOST_FFA_ID) {
>  		ret = FFA_RET_INVALID_PARAMETERS;
> @@ -537,7 +537,7 @@ static void do_ffa_mem_reclaim(struct arm_smccc_res *res,
>  	fraglen = res->a2;
>
>  	ep_mem_access = (void *)buf +
> -			ffa_mem_desc_offset(buf, 0, FFA_VERSION_1_0);
> +			ffa_mem_desc_offset(buf, 0, hyp_ffa_version);
>  	offset = ep_mem_access->composite_off;
>  	/*
>  	 * We can trust the SPMD to get this right, but let's at least
> @@ -846,7 +846,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_0, 0, 0, 0, 0, 0, 0, &res);
> +	arm_smccc_1_1_smc(FFA_VERSION, FFA_VERSION_1_1, 0, 0, 0, 0, 0, 0, &res);
>  	if (res.a0 == FFA_RET_NOT_SUPPORTED)
>  		return 0;
>
> @@ -866,7 +866,11 @@ int hyp_ffa_init(void *pages)
>  	if (FFA_MAJOR_VERSION(res.a0) != 1)
>  		return -EOPNOTSUPP;
>
> -	hyp_ffa_version = FFA_VERSION_1_0;
> +	if (FFA_MINOR_VERSION(res.a0) < FFA_MINOR_VERSION(FFA_VERSION_1_1))

It can be even <= instead of <, in that way else part is just handling
downgrading part and will be explicit. But that's just my taste and not
a must change as nothing changes with or without it.

--
Regards,
Sudeep

Re: [PATCH v2 0/4] KVM: arm64: pKVM host proxy FF-A fixes

[Sudeep Holla]

On Wed, May 15, 2024 at 05:22:54PM +0000, Sebastian Ene wrote:
> Hello,
> 
> 
> This series contains some small fixes for the host pKVM proxy code. I included
> some of the patches that I already sent on the list as part of this series
> to make it easier to keep track of them.
> 
> I verified the functionality with OPTEE as a TEE-OS.
>

For the series,

Tested-by: Sudeep Holla <sudeep.holla@arm.com>

I have been using this series along with couple of patches I posted to
workaround v1.0 and FF-A bus handling[1].

-- 
Regards,
Sudeep

[1] https://lore.kernel.org/all/20240515094028.1947976-1-sudeep.holla@arm.com/