On Fri, Feb 6, 2026 at 5:18 PM Bertrand Marquis <[email protected]> wrote: > > > > > On 6 Feb 2026, at 10:28, Jens Wiklander <[email protected]> wrote: > > > > Hi Bertrand, > > > > On Tue, Feb 3, 2026 at 6:38 PM Bertrand Marquis > > <[email protected]> wrote: > >> > >> The FF-A memory attribute encoding is currently a literal value (0x2f), > >> which makes reviews and validation harder. In addition, MEM_SHARE > >> accepts the NS (non-secure) attribute bit even though the normal world > >> must not set it according to FF-A specification. > >> > >> Introduce named attribute bit masks and express FFA_NORMAL_MEM_REG_ATTR > >> in terms of them for clarity. > >> > >> Reject MEM_SHARE descriptors with the NS bit set, returning > >> INVALID_PARAMETERS to match FF-A v1.1 rules that prohibit normal world > >> from setting this bit. > >> > >> Functional impact: MEM_SHARE now rejects descriptors with NS bit set, > >> which were previously accepted but violate FF-A specification. > > > > To be fair, it was also rejected earlier, but with a different error code. > > True, will adapt the impact comment to say: > > Functional impact: MEM_SHARE now rejects descriptors with NS bit set > with the right error code, INVALID_PARAMETER. > > Tell me if that would be ok for you and if it could be fixed on commit with > your R-b if it is the case.
Sounds good. With that, please add: Reviewed-by: Jens Wiklander <[email protected]> Cheers, Jens > > > > >> > >> Signed-off-by: Bertrand Marquis <[email protected]> > >> --- > >> xen/arch/arm/tee/ffa_private.h | 17 ++++++++++++++++- > >> xen/arch/arm/tee/ffa_shm.c | 6 ++++++ > >> 2 files changed, 22 insertions(+), 1 deletion(-) > >> > >> diff --git a/xen/arch/arm/tee/ffa_private.h > >> b/xen/arch/arm/tee/ffa_private.h > >> index cd7ecabc7eff..b625f1c72914 100644 > >> --- a/xen/arch/arm/tee/ffa_private.h > >> +++ b/xen/arch/arm/tee/ffa_private.h > >> @@ -129,11 +129,26 @@ > >> #define FFA_HANDLE_HYP_FLAG BIT(63, ULL) > >> #define FFA_HANDLE_INVALID 0xffffffffffffffffULL > >> > >> +/* NS attribute was introduced in v1.1 */ > >> +#define FFA_MEM_ATTR_NS BIT(6, U) > >> + > >> +#define FFA_MEM_ATTR_TYPE_DEV (1U << 3) > >> +#define FFA_MEM_ATTR_TYPE_MEM (2U << 4) > >> + > >> +#define FFA_MEM_ATTR_NC (1U << 2) > >> +#define FFA_MEM_ATTR_WB (3U << 2) > >> + > >> +#define FFA_MEM_ATTR_NON_SHARE (0U) > >> +#define FFA_MEM_ATTR_OUT_SHARE (2U) > >> +#define FFA_MEM_ATTR_INN_SHARE (3U) > >> + > >> /* > >> * Memory attributes: Normal memory, Write-Back cacheable, Inner shareable > >> * Defined in FF-A-1.1-REL0 Table 10.18 at page 175. > >> */ > >> -#define FFA_NORMAL_MEM_REG_ATTR 0x2fU > >> +#define FFA_NORMAL_MEM_REG_ATTR (FFA_MEM_ATTR_TYPE_MEM | \ > >> + FFA_MEM_ATTR_WB | \ > >> + FFA_MEM_ATTR_INN_SHARE) > >> /* > >> * Memory access permissions: Read-write > >> * Defined in FF-A-1.1-REL0 Table 10.15 at page 168. > >> diff --git a/xen/arch/arm/tee/ffa_shm.c b/xen/arch/arm/tee/ffa_shm.c > >> index 8282bacf85d3..90800e44a86a 100644 > >> --- a/xen/arch/arm/tee/ffa_shm.c > >> +++ b/xen/arch/arm/tee/ffa_shm.c > >> @@ -512,6 +512,12 @@ void ffa_handle_mem_share(struct cpu_user_regs *regs) > >> if ( ret ) > >> goto out_unlock; > >> > >> + if ( trans.mem_reg_attr & FFA_MEM_ATTR_NS ) > >> + { > >> + ret = FFA_RET_INVALID_PARAMETERS; > >> + goto out_unlock; > >> + } > >> + > >> if ( trans.mem_reg_attr != FFA_NORMAL_MEM_REG_ATTR ) > >> { > >> ret = FFA_RET_NOT_SUPPORTED; > >> -- > >> 2.50.1 (Apple Git-155) > >> > > > > Looks good, but I think the commit message needs a small update or > > clarification. > > Thanks. > > Cheers > Bertrand > > > > > Cheers, > > Jens > >
