xen/arm: ffa: v1.2 improvements

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

Posted by Bertrand Marquis 6 days, 10 hours ago

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.

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

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

Posted by Jens Wiklander 3 days, 18 hours ago

Hi Bertrand,

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

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

Posted by Bertrand Marquis 3 days, 11 hours ago


> On 6 Feb 2026, at 10:28, Jens Wiklander <jens.wiklander@linaro.org> wrote:
> 
> Hi Bertrand,
> 
> On Tue, Feb 3, 2026 at 6:38 PM Bertrand Marquis
> <bertrand.marquis@arm.com> 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 <bertrand.marquis@arm.com>
>> ---
>> 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

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

Posted by Jens Wiklander 18 hours ago

On Fri, Feb 6, 2026 at 5:18 PM Bertrand Marquis
<Bertrand.Marquis@arm.com> wrote:
>
>
>
> > On 6 Feb 2026, at 10:28, Jens Wiklander <jens.wiklander@linaro.org> wrote:
> >
> > Hi Bertrand,
> >
> > On Tue, Feb 3, 2026 at 6:38 PM Bertrand Marquis
> > <bertrand.marquis@arm.com> 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 <jens.wiklander@linaro.org>

Cheers,
Jens

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