[PATCH v3] firmware: arm_ffa: Add support for IMPDEF value in the memory access descriptor

[Sudeep Holla]

On Wed, Sep 24, 2025 at 09:50:21AM +0800, Lixiang Mao wrote:
> 
> On 9/23/2025 11:09 PM, Sudeep Holla wrote:
> > FF-A v1.2 introduced 16 byte IMPLEMENTATION DEFINED value in the endpoint
> > memory access descriptor to allow any sender could to specify an its any
> > custom value for each receiver. Also this value must be specified by the
> > receiver when retrieving the memory region. The sender must ensure it
> > informs the receiver of this value via an IMPLEMENTATION DEFINED mechanism
> > such as a partition message.
> > 
> > So the FF-A driver can use the message interfaces to communicate the value
> > and set the same in the ffa_mem_region_attributes structures when using
> > the memory interfaces.
> > 
> > The driver ensure that the size of the endpoint memory access descriptors
> > is set correctly based on the FF-A version.
> > 
> > Fixes: 9fac08d9d985 ("firmware: arm_ffa: Upgrade FF-A version to v1.2 in the driver")
> > Reported-by: Lixiang Mao <liximao at qti.qualcomm.com>
> > Tested-by: Lixiang Mao <liximao at qti.qualcomm.com>
> > Message-Id: <20250919155216.34074-1-sudeep.holla at arm.com>
> > Signed-off-by: Sudeep Holla <sudeep.holla at arm.com>
> > ---
> >   drivers/firmware/arm_ffa/driver.c | 37 ++++++++++++++++++++++---------
> >   include/linux/arm_ffa.h           | 21 ++++++++++++++++--
> >   2 files changed, 46 insertions(+), 12 deletions(-)
> > 
> > v2[2]->v3:
> > 	- Moved FFA version checks into macros and tie them with features
> > 	  to improve readability and maintenance
> > 
> > v1[1]->v2[2]:
> > 	- Fix the size of EMAD in ffa_mem_desc_offset() and the loop
> > 	  increment in ffa_setup_and_transmit()
> > 
> > [1] https://lore.kernel.org/all/20250915150119.3106344-1-sudeep.holla@arm.com/
> > [2] https://lore.kernel.org/all/20250919155216.34074-1-sudeep.holla@arm.com/
> > 
> > diff --git a/drivers/firmware/arm_ffa/driver.c b/drivers/firmware/arm_ffa/driver.c
> > index 65bf1685350a..c72ee4756585 100644
> > --- a/drivers/firmware/arm_ffa/driver.c
> > +++ b/drivers/firmware/arm_ffa/driver.c
> > @@ -649,6 +649,26 @@ static u16 ffa_memory_attributes_get(u32 func_id)
> >   	return FFA_MEM_NORMAL | FFA_MEM_WRITE_BACK | FFA_MEM_INNER_SHAREABLE;
> >   }
> > +static void ffa_emad_impdef_value_init(u32 version, void *dst, void *src)
> > +{
> > +	struct ffa_mem_region_attributes *ep_mem_access;
> > +
> > +	if (FFA_EMAD_HAS_IMPDEF_FIELD(version))
> > +		memcpy(dst, src, sizeof(ep_mem_access->impdef_val));
> > +}
> > +
> > +static void
> > +ffa_mem_region_additional_setup(u32 version, struct ffa_mem_region *mem_region)
> > +{
> > +	if (!FFA_MEM_REGION_HAS_EP_MEM_OFFSET(version)) {
> > +		mem_region->ep_mem_size = 0;
> > +	} else {
> > +		mem_region->ep_mem_size = ffa_emad_size_get(version);
> > +		mem_region->ep_mem_offset = sizeof(*mem_region);
> > +		memset(mem_region->reserved, 0, 12);
> > +	}
> > +}
> > +
> >   static int
> >   ffa_setup_and_transmit(u32 func_id, void *buffer, u32 max_fragsize,
> >   		       struct ffa_mem_ops_args *args)
> > @@ -667,27 +687,24 @@ ffa_setup_and_transmit(u32 func_id, void *buffer, u32 max_fragsize,
> >   	mem_region->flags = args->flags;
> >   	mem_region->sender_id = drv_info->vm_id;
> >   	mem_region->attributes = ffa_memory_attributes_get(func_id);
> > -	ep_mem_access = buffer +
> > -			ffa_mem_desc_offset(buffer, 0, drv_info->version);
> >   	composite_offset = ffa_mem_desc_offset(buffer, args->nattrs,
> >   					       drv_info->version);
> > -	for (idx = 0; idx < args->nattrs; idx++, ep_mem_access++) {
> > +	for (idx = 0; idx < args->nattrs; idx++) {
> > +		ep_mem_access = buffer +
> > +			ffa_mem_desc_offset(buffer, idx, drv_info->version);
> >   		ep_mem_access->receiver = args->attrs[idx].receiver;
> >   		ep_mem_access->attrs = args->attrs[idx].attrs;
> >   		ep_mem_access->composite_off = composite_offset;
> >   		ep_mem_access->flag = 0;
> >   		ep_mem_access->reserved = 0;
> > +		ffa_emad_impdef_value_init(drv_info->version,
> > +					   ep_mem_access->impdef_val,
> > +					   args->attrs[idx].impdef_val);
> >   	}
> >   	mem_region->handle = 0;
> >   	mem_region->ep_count = args->nattrs;
> > -	if (drv_info->version <= FFA_VERSION_1_0) {
> > -		mem_region->ep_mem_size = 0;
> > -	} else {
> > -		mem_region->ep_mem_size = sizeof(*ep_mem_access);
> > -		mem_region->ep_mem_offset = sizeof(*mem_region);
> > -		memset(mem_region->reserved, 0, 12);
> > -	}
> > +	ffa_mem_region_additional_setup(drv_info->version, mem_region);
> >   	composite = buffer + composite_offset;
> >   	composite->total_pg_cnt = ffa_get_num_pages_sg(args->sg);
> > diff --git a/include/linux/arm_ffa.h b/include/linux/arm_ffa.h
> > index e1634897e159..c2d1f743e44a 100644
> > --- a/include/linux/arm_ffa.h
> > +++ b/include/linux/arm_ffa.h
> > @@ -337,6 +337,7 @@ struct ffa_mem_region_attributes {
> >   	 * an `struct ffa_mem_region_addr_range`.
> >   	 */
> >   	u32 composite_off;
> > +	u8 impdef_val[16];
> >   	u64 reserved;
> >   };
> > @@ -416,15 +417,31 @@ struct ffa_mem_region {
> >   #define CONSTITUENTS_OFFSET(x)	\
> >   	(offsetof(struct ffa_composite_mem_region, constituents[x]))
> > +#define FFA_EMAD_HAS_IMPDEF_FIELD(version)	(version >= FFA_VERSION_1_2)
> > +#define FFA_MEM_REGION_HAS_EP_MEM_OFFSET(version) (version > FFA_VERSION_1_0)
> Minor: version -->> (version)

Good point, will fix it up when applying. Thanks!

-- 
Regards,
Sudeep