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