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

[Seth Forshee <sforshee@nvidia.com>]

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@nvidia.com>
> > ---
> >  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.

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.

> 
> 
> > +			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.

Thanks,
Seth