Re: [PATCH 1/2] firmware: arm_ffa: Honor maximum RX/TX buffer size

[Sudeep Holla]

On Tue, Jun 02, 2026 at 11:12:03AM -0500, Seth Forshee wrote:
> On Tue, Jun 02, 2026 at 04:56:48PM +0100, Sudeep Holla wrote:
> > On Mon, Jun 01, 2026 at 03:45:11PM -0500, Seth Forshee wrote:
> > > FFA_FEATURES in v1.2+ supports a maximum RXTX_MAP size. This maximum
> > > size is not checked when page-aligning the RX/TX buffer size, and
> > > FFA_RXTX_MAP may fail when passed a size greater than the maximum.
> > > 
> > > Decode the maximum buffer size returned from FFA_FEATURES and limit the
> > > buffer size based on this value if it is non-zero (zero indicates no
> > > maximum). Include verification that the max returned by the SPMC is
> > > larger than the minimum, otherwise use the minimum.
> > > 
> > > While there, also update RXTX_MAP_MIN_BUFSZ() to use FIELD_GET() for
> > > consistency.
> > > 
> > > Fixes: 83210251fd70 ("firmware: arm_ffa: Use the correct buffer size during RXTX_MAP")
> > > Assisted-by: Claude:claude-opus-4-8
> > > Signed-off-by: Seth Forshee <sforshee@xxxxxxxxxx>
> > > ---
> > >  drivers/firmware/arm_ffa/driver.c | 29 +++++++++++++++++++++++++++--
> > >  1 file changed, 27 insertions(+), 2 deletions(-)
> > > 
> > > diff --git a/drivers/firmware/arm_ffa/driver.c b/drivers/firmware/arm_ffa/driver.c
> > > index b9f17fda7243..dc45724a29ba 100644
> > > --- a/drivers/firmware/arm_ffa/driver.c
> > > +++ b/drivers/firmware/arm_ffa/driver.c
> > > @@ -32,6 +32,7 @@
> > >  #include <linux/interrupt.h>
> > >  #include <linux/io.h>
> > >  #include <linux/kernel.h>
> > > +#include <linux/minmax.h>
> > >  #include <linux/module.h>
> > >  #include <linux/mm.h>
> > >  #include <linux/mutex.h>
> > > @@ -55,7 +56,9 @@
> > >  	(FIELD_PREP(SENDER_ID_MASK, (s)) | FIELD_PREP(RECEIVER_ID_MASK, (r)))
> > >  
> > >  #define RXTX_MAP_MIN_BUFSZ_MASK	GENMASK(1, 0)
> > > -#define RXTX_MAP_MIN_BUFSZ(x)	((x) & RXTX_MAP_MIN_BUFSZ_MASK)
> > > +#define RXTX_MAP_MAX_BUFSZ_MASK	GENMASK(31, 16)
> > > +#define RXTX_MAP_MIN_BUFSZ(x)	(FIELD_GET(RXTX_MAP_MIN_BUFSZ_MASK, (x)))
> > > +#define RXTX_MAP_MAX_BUFSZ(x)	(FIELD_GET(RXTX_MAP_MAX_BUFSZ_MASK, (x)))
> > >  
> > >  #define FFA_MAX_NOTIFICATIONS		64
> > >  
> > > @@ -2086,11 +2089,13 @@ static void ffa_notifications_setup(void)
> > >  	ffa_notifications_cleanup();
> > >  }
> > >  
> > > +#define FFA_SUPPORTS_RXTX_MAX_BUFSZ(version)	((version) > FFA_VERSION_1_1)
> > > +
> > >  static int __init ffa_init(void)
> > >  {
> > >  	int ret;
> > >  	u32 buf_sz;
> > > -	size_t rxtx_bufsz = SZ_4K;
> > > +	size_t rxtx_bufsz = SZ_4K, rxtx_max_bufsz = 0;
> > >  
> > >  	ret = ffa_transport_init(&invoke_ffa_fn);
> > >  	if (ret)
> > > @@ -2118,9 +2123,29 @@ static int __init ffa_init(void)
> > >  			rxtx_bufsz = SZ_16K;
> > >  		else
> > >  			rxtx_bufsz = SZ_4K;
> > > +
> > > +		if (FFA_SUPPORTS_RXTX_MAX_BUFSZ(drv_info->version)) {
> > 
> > This is not even compile tested ? I don't think this is defined anyway.
> > Anyways I don't think we need this condition as v1.1 or below had it
> > MBZ, so it is fine to drop it.
> 
> It is added just above ffa_init(). The code as been compile and run
> tested on multiple platforms, as noted in the cover letter.
> 

Ah sorry, my bad. It failed to apply and I must have messed it up.
I have some changes in linux-next which conflicts and I dropped it when
resolving. Sorry for that.

> I hadn't checked older spec versions, so if they have it as MBZ then I
> agree we can drop the check.
> 
> > 
> > > +			rxtx_max_bufsz = (size_t)RXTX_MAP_MAX_BUFSZ(buf_sz) * SZ_4K;
> > 
> > The cast is unnecessary, its is 16 bit, so max 0xFFFF << 12.
> 
> Correct, though linters complain about it which is why I added it. I can
> drop it though if you prefer.
> 

Can you be more specific ? Any way to trigger it ?

> > 
> > 
> > > +			if (rxtx_max_bufsz != 0 && rxtx_max_bufsz < rxtx_bufsz) {
> > > +				/*
> > > +				 * Per spec the maximum must be >= the minimum, or
> > > +				 * else zero if there is no size limit. If the SPMC
> > > +				 * violates this constraint, use the minimum as the
> > > +				 * effective maximum.
> > > +				 */
> > 
> > I don't see any text implying the above from the spec, drop it. Please point
> > it to me if you find. Even otherwise it seems obvious here and doesn't need
> > the note.
> 
> I found it in v1.3 of the spec in Table 13.15: "Size must be greater or
> equal to the minimum buffer size, else MBZ if there is no size limit." I
> will remove the comment.
> 

Ah v1.3, sorry I was just looking at v1.2.

> Thanks,
> Seth

-- 
Regards,
Sudeep