diff mbox series

[v4,14/17] KVM: arm64: FFA: Remove access of endpoint memory access descriptor array

Message ID 20231005-ffa_v1-1_notif-v4-14-cddd3237809c@arm.com (mailing list archive)
State New, archived
Headers show
Series firmware: arm_ffa: Add FF-A v1.1 support(notification + new memory descriptor format) | expand

Commit Message

Sudeep Holla Oct. 5, 2023, 2:45 p.m. UTC
FF-A v1.1 removes the fixed location of endpoint memory access descriptor
array within the memory transaction descriptor structure. In preparation
to remove the ep_mem_access member from the ffa_mem_region structure,
provide the accessor to fetch the offset and use the same in FF-A proxy
implementation.

The accessor take the FF-A version as the argument from which the memory
access descriptor format can be determined. v1.0 uses the old format while
v1.1 onwards use the new format specified in the v1.1 specification.

Cc: Marc Zyngier <maz@kernel.org>
Cc: Oliver Upton <oliver.upton@linux.dev>
Cc: Will Deacon <will@kernel.org>
Cc: Quentin Perret <qperret@google.com>
Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
---
 arch/arm64/kvm/hyp/nvhe/ffa.c | 10 ++++++++--
 include/linux/arm_ffa.h       |  6 ++++++
 2 files changed, 14 insertions(+), 2 deletions(-)

Comments

Sudeep Holla Oct. 6, 2023, 3:25 p.m. UTC | #1
On Thu, Oct 05, 2023 at 03:45:07PM +0100, Sudeep Holla wrote:
> FF-A v1.1 removes the fixed location of endpoint memory access descriptor
> array within the memory transaction descriptor structure. In preparation
> to remove the ep_mem_access member from the ffa_mem_region structure,
> provide the accessor to fetch the offset and use the same in FF-A proxy
> implementation.
> 
> The accessor take the FF-A version as the argument from which the memory
> access descriptor format can be determined. v1.0 uses the old format while
> v1.1 onwards use the new format specified in the v1.1 specification.
> 
> Cc: Marc Zyngier <maz@kernel.org>
> Cc: Oliver Upton <oliver.upton@linux.dev>
> Cc: Will Deacon <will@kernel.org>
> Cc: Quentin Perret <qperret@google.com>
> Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
> ---
>  arch/arm64/kvm/hyp/nvhe/ffa.c | 10 ++++++++--
>  include/linux/arm_ffa.h       |  6 ++++++
>  2 files changed, 14 insertions(+), 2 deletions(-)
>

Hi Marc,

Does this look better with respect to FF-A version passed as parameter.
Since this case needs both major and minor(to handle difference between
v1.0 and v1.1), I am passing the full version(both major and minor) in
the compact form.

If you happy, please ack. I would like to take it with other FF-A changes.

Regards,
Sudeep

> diff --git a/arch/arm64/kvm/hyp/nvhe/ffa.c b/arch/arm64/kvm/hyp/nvhe/ffa.c
> index 6e4dba9eadef..320f2eaa14a9 100644
> --- a/arch/arm64/kvm/hyp/nvhe/ffa.c
> +++ b/arch/arm64/kvm/hyp/nvhe/ffa.c
> @@ -423,6 +423,7 @@ static __always_inline void do_ffa_mem_xfer(const u64 func_id,
>  	DECLARE_REG(u32, fraglen, ctxt, 2);
>  	DECLARE_REG(u64, addr_mbz, ctxt, 3);
>  	DECLARE_REG(u32, npages_mbz, ctxt, 4);
> +	struct ffa_mem_region_attributes *ep_mem_access;
>  	struct ffa_composite_mem_region *reg;
>  	struct ffa_mem_region *buf;
>  	u32 offset, nr_ranges;
> @@ -452,7 +453,9 @@ static __always_inline void do_ffa_mem_xfer(const u64 func_id,
>  	buf = hyp_buffers.tx;
>  	memcpy(buf, host_buffers.tx, fraglen);
>  
> -	offset = buf->ep_mem_access[0].composite_off;
> +	ep_mem_access = (void *)buf +
> +			ffa_mem_desc_offset(buf, 0, FFA_VERSION_1_0);
> +	offset = ep_mem_access->composite_off;
>  	if (!offset || buf->ep_count != 1 || buf->sender_id != HOST_FFA_ID) {
>  		ret = FFA_RET_INVALID_PARAMETERS;
>  		goto out_unlock;
> @@ -504,6 +507,7 @@ static void do_ffa_mem_reclaim(struct arm_smccc_res *res,
>  	DECLARE_REG(u32, handle_lo, ctxt, 1);
>  	DECLARE_REG(u32, handle_hi, ctxt, 2);
>  	DECLARE_REG(u32, flags, ctxt, 3);
> +	struct ffa_mem_region_attributes *ep_mem_access;
>  	struct ffa_composite_mem_region *reg;
>  	u32 offset, len, fraglen, fragoff;
>  	struct ffa_mem_region *buf;
> @@ -528,7 +532,9 @@ static void do_ffa_mem_reclaim(struct arm_smccc_res *res,
>  	len = res->a1;
>  	fraglen = res->a2;
>  
> -	offset = buf->ep_mem_access[0].composite_off;
> +	ep_mem_access = (void *)buf +
> +			ffa_mem_desc_offset(buf, 0, FFA_VERSION_1_0);
> +	offset = ep_mem_access->composite_off;
>  	/*
>  	 * We can trust the SPMD to get this right, but let's at least
>  	 * check that we end up with something that doesn't look _completely_
> diff --git a/include/linux/arm_ffa.h b/include/linux/arm_ffa.h
> index 748d0a83a4bc..2444d596b703 100644
> --- a/include/linux/arm_ffa.h
> +++ b/include/linux/arm_ffa.h
> @@ -357,6 +357,12 @@ struct ffa_mem_region {
>  #define CONSTITUENTS_OFFSET(x)	\
>  	(offsetof(struct ffa_composite_mem_region, constituents[x]))
>  
> +static inline u32
> +ffa_mem_desc_offset(struct ffa_mem_region *buf, int count, u32 ffa_version)
> +{
> +	return COMPOSITE_OFFSET(0);
> +}
> +
>  struct ffa_mem_ops_args {
>  	bool use_txbuf;
>  	u32 nattrs;
> 
> -- 
> 2.42.0
>
Marc Zyngier Oct. 6, 2023, 6:25 p.m. UTC | #2
On Thu, 05 Oct 2023 15:45:07 +0100,
Sudeep Holla <sudeep.holla@arm.com> wrote:
> 
> FF-A v1.1 removes the fixed location of endpoint memory access descriptor
> array within the memory transaction descriptor structure. In preparation
> to remove the ep_mem_access member from the ffa_mem_region structure,
> provide the accessor to fetch the offset and use the same in FF-A proxy
> implementation.
> 
> The accessor take the FF-A version as the argument from which the memory
> access descriptor format can be determined. v1.0 uses the old format while
> v1.1 onwards use the new format specified in the v1.1 specification.
> 
> Cc: Marc Zyngier <maz@kernel.org>
> Cc: Oliver Upton <oliver.upton@linux.dev>
> Cc: Will Deacon <will@kernel.org>
> Cc: Quentin Perret <qperret@google.com>
> Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
> ---
>  arch/arm64/kvm/hyp/nvhe/ffa.c | 10 ++++++++--
>  include/linux/arm_ffa.h       |  6 ++++++
>  2 files changed, 14 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/arm64/kvm/hyp/nvhe/ffa.c b/arch/arm64/kvm/hyp/nvhe/ffa.c
> index 6e4dba9eadef..320f2eaa14a9 100644
> --- a/arch/arm64/kvm/hyp/nvhe/ffa.c
> +++ b/arch/arm64/kvm/hyp/nvhe/ffa.c
> @@ -423,6 +423,7 @@ static __always_inline void do_ffa_mem_xfer(const u64 func_id,
>  	DECLARE_REG(u32, fraglen, ctxt, 2);
>  	DECLARE_REG(u64, addr_mbz, ctxt, 3);
>  	DECLARE_REG(u32, npages_mbz, ctxt, 4);
> +	struct ffa_mem_region_attributes *ep_mem_access;
>  	struct ffa_composite_mem_region *reg;
>  	struct ffa_mem_region *buf;
>  	u32 offset, nr_ranges;
> @@ -452,7 +453,9 @@ static __always_inline void do_ffa_mem_xfer(const u64 func_id,
>  	buf = hyp_buffers.tx;
>  	memcpy(buf, host_buffers.tx, fraglen);
>  
> -	offset = buf->ep_mem_access[0].composite_off;
> +	ep_mem_access = (void *)buf +
> +			ffa_mem_desc_offset(buf, 0, FFA_VERSION_1_0);
> +	offset = ep_mem_access->composite_off;
>  	if (!offset || buf->ep_count != 1 || buf->sender_id != HOST_FFA_ID) {
>  		ret = FFA_RET_INVALID_PARAMETERS;
>  		goto out_unlock;
> @@ -504,6 +507,7 @@ static void do_ffa_mem_reclaim(struct arm_smccc_res *res,
>  	DECLARE_REG(u32, handle_lo, ctxt, 1);
>  	DECLARE_REG(u32, handle_hi, ctxt, 2);
>  	DECLARE_REG(u32, flags, ctxt, 3);
> +	struct ffa_mem_region_attributes *ep_mem_access;
>  	struct ffa_composite_mem_region *reg;
>  	u32 offset, len, fraglen, fragoff;
>  	struct ffa_mem_region *buf;
> @@ -528,7 +532,9 @@ static void do_ffa_mem_reclaim(struct arm_smccc_res *res,
>  	len = res->a1;
>  	fraglen = res->a2;
>  
> -	offset = buf->ep_mem_access[0].composite_off;
> +	ep_mem_access = (void *)buf +
> +			ffa_mem_desc_offset(buf, 0, FFA_VERSION_1_0);
> +	offset = ep_mem_access->composite_off;
>  	/*
>  	 * We can trust the SPMD to get this right, but let's at least
>  	 * check that we end up with something that doesn't look _completely_
> diff --git a/include/linux/arm_ffa.h b/include/linux/arm_ffa.h
> index 748d0a83a4bc..2444d596b703 100644
> --- a/include/linux/arm_ffa.h
> +++ b/include/linux/arm_ffa.h
> @@ -357,6 +357,12 @@ struct ffa_mem_region {
>  #define CONSTITUENTS_OFFSET(x)	\
>  	(offsetof(struct ffa_composite_mem_region, constituents[x]))
>  
> +static inline u32
> +ffa_mem_desc_offset(struct ffa_mem_region *buf, int count, u32 ffa_version)
> +{
> +	return COMPOSITE_OFFSET(0);
> +}
> +
>  struct ffa_mem_ops_args {
>  	bool use_txbuf;
>  	u32 nattrs;
> 

I haven't looked at the rest of the series, but for the purpose of
this particular patch, it looks OK to me.

Acked-by: Marc Zyngier <maz@kernel.org>

	M.
diff mbox series

Patch

diff --git a/arch/arm64/kvm/hyp/nvhe/ffa.c b/arch/arm64/kvm/hyp/nvhe/ffa.c
index 6e4dba9eadef..320f2eaa14a9 100644
--- a/arch/arm64/kvm/hyp/nvhe/ffa.c
+++ b/arch/arm64/kvm/hyp/nvhe/ffa.c
@@ -423,6 +423,7 @@  static __always_inline void do_ffa_mem_xfer(const u64 func_id,
 	DECLARE_REG(u32, fraglen, ctxt, 2);
 	DECLARE_REG(u64, addr_mbz, ctxt, 3);
 	DECLARE_REG(u32, npages_mbz, ctxt, 4);
+	struct ffa_mem_region_attributes *ep_mem_access;
 	struct ffa_composite_mem_region *reg;
 	struct ffa_mem_region *buf;
 	u32 offset, nr_ranges;
@@ -452,7 +453,9 @@  static __always_inline void do_ffa_mem_xfer(const u64 func_id,
 	buf = hyp_buffers.tx;
 	memcpy(buf, host_buffers.tx, fraglen);
 
-	offset = buf->ep_mem_access[0].composite_off;
+	ep_mem_access = (void *)buf +
+			ffa_mem_desc_offset(buf, 0, FFA_VERSION_1_0);
+	offset = ep_mem_access->composite_off;
 	if (!offset || buf->ep_count != 1 || buf->sender_id != HOST_FFA_ID) {
 		ret = FFA_RET_INVALID_PARAMETERS;
 		goto out_unlock;
@@ -504,6 +507,7 @@  static void do_ffa_mem_reclaim(struct arm_smccc_res *res,
 	DECLARE_REG(u32, handle_lo, ctxt, 1);
 	DECLARE_REG(u32, handle_hi, ctxt, 2);
 	DECLARE_REG(u32, flags, ctxt, 3);
+	struct ffa_mem_region_attributes *ep_mem_access;
 	struct ffa_composite_mem_region *reg;
 	u32 offset, len, fraglen, fragoff;
 	struct ffa_mem_region *buf;
@@ -528,7 +532,9 @@  static void do_ffa_mem_reclaim(struct arm_smccc_res *res,
 	len = res->a1;
 	fraglen = res->a2;
 
-	offset = buf->ep_mem_access[0].composite_off;
+	ep_mem_access = (void *)buf +
+			ffa_mem_desc_offset(buf, 0, FFA_VERSION_1_0);
+	offset = ep_mem_access->composite_off;
 	/*
 	 * We can trust the SPMD to get this right, but let's at least
 	 * check that we end up with something that doesn't look _completely_
diff --git a/include/linux/arm_ffa.h b/include/linux/arm_ffa.h
index 748d0a83a4bc..2444d596b703 100644
--- a/include/linux/arm_ffa.h
+++ b/include/linux/arm_ffa.h
@@ -357,6 +357,12 @@  struct ffa_mem_region {
 #define CONSTITUENTS_OFFSET(x)	\
 	(offsetof(struct ffa_composite_mem_region, constituents[x]))
 
+static inline u32
+ffa_mem_desc_offset(struct ffa_mem_region *buf, int count, u32 ffa_version)
+{
+	return COMPOSITE_OFFSET(0);
+}
+
 struct ffa_mem_ops_args {
 	bool use_txbuf;
 	u32 nattrs;