Re: [PATCH 02/12] xen/arm: ffa: Fix MEM_SHARE NS attribute handling

[Jens Wiklander <jens.wiklander@xxxxxxxxxx>]

Hi Bertrand,

On Tue, Feb 3, 2026 at 6:38 PM Bertrand Marquis
<bertrand.marquis@xxxxxxx> 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.

>
> Signed-off-by: Bertrand Marquis <bertrand.marquis@xxxxxxx>
> ---
>  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.

Cheers,
Jens