[v2] FF-A VM to VM support

[PATCH v2 3/5] xen/arm: ffa: Add buffer full notification support

Posted by Bertrand Marquis 9 months, 1 week ago

Add support to raise a Rx buffer full notification to a VM.
This function will be used for indirect message support between VM and
is only activated if CONFIG_FFA_VM_TO_VM is selected.

Even if there are 32 framework notifications possible, right now only
one is defined so the implementation is simplified to only handle the
buffer full notification using a boolean. If other framework
notifications have to be supported one day, the design will have to be
modified to handle it properly.

Signed-off-by: Bertrand Marquis <bertrand.marquis@arm.com>
---
Changes in v2:
- Code style fix
---
 xen/arch/arm/tee/ffa_notif.c   | 26 +++++++++++++++++++++-----
 xen/arch/arm/tee/ffa_private.h | 13 +++++++++++++
 2 files changed, 34 insertions(+), 5 deletions(-)

diff --git a/xen/arch/arm/tee/ffa_notif.c b/xen/arch/arm/tee/ffa_notif.c
index 00efaf8f7353..d19aa5c5bef6 100644
--- a/xen/arch/arm/tee/ffa_notif.c
+++ b/xen/arch/arm/tee/ffa_notif.c
@@ -93,6 +93,7 @@ void ffa_handle_notification_info_get(struct cpu_user_regs *regs)
 void ffa_handle_notification_get(struct cpu_user_regs *regs)
 {
     struct domain *d = current->domain;
+    struct ffa_ctx *ctx = d->arch.tee;
     uint32_t recv = get_user_reg(regs, 1);
     uint32_t flags = get_user_reg(regs, 2);
     uint32_t w2 = 0;
@@ -132,11 +133,7 @@ void ffa_handle_notification_get(struct cpu_user_regs *regs)
          */
         if ( ( flags  & FFA_NOTIF_FLAG_BITMAP_SP ) &&
              ( flags & FFA_NOTIF_FLAG_BITMAP_SPM ) )
-        {
-                struct ffa_ctx *ctx = d->arch.tee;
-
-                ACCESS_ONCE(ctx->notif.secure_pending) = false;
-        }
+            ACCESS_ONCE(ctx->notif.secure_pending) = false;
 
         arm_smccc_1_2_smc(&arg, &resp);
         e = ffa_get_ret_code(&resp);
@@ -156,6 +153,12 @@ void ffa_handle_notification_get(struct cpu_user_regs *regs)
             w6 = resp.a6;
     }
 
+#ifdef CONFIG_FFA_VM_TO_VM
+    if ( flags & FFA_NOTIF_FLAG_BITMAP_HYP &&
+          test_and_clear_bool(ctx->notif.buff_full_pending) )
+        w7 = FFA_NOTIF_RX_BUFFER_FULL;
+#endif
+
     ffa_set_regs(regs, FFA_SUCCESS_32, 0, w2, w3, w4, w5, w6, w7);
 }
 
@@ -178,6 +181,19 @@ int ffa_handle_notification_set(struct cpu_user_regs *regs)
                            bitmap_hi);
 }
 
+#ifdef CONFIG_FFA_VM_TO_VM
+void ffa_raise_rx_buffer_full(struct domain *d)
+{
+    struct ffa_ctx *ctx = d->arch.tee;
+
+    if ( !ctx )
+        return;
+
+    if ( !test_and_set_bool(ctx->notif.buff_full_pending) )
+        vgic_inject_irq(d, d->vcpu[0], notif_sri_irq, true);
+}
+#endif
+
 /*
  * Extract a 16-bit ID (index n) from the successful return value from
  * FFA_NOTIFICATION_INFO_GET_64 or FFA_NOTIFICATION_INFO_GET_32. IDs are
diff --git a/xen/arch/arm/tee/ffa_private.h b/xen/arch/arm/tee/ffa_private.h
index bd6877d8c632..1f5067d5d0c9 100644
--- a/xen/arch/arm/tee/ffa_private.h
+++ b/xen/arch/arm/tee/ffa_private.h
@@ -210,6 +210,8 @@
 #define FFA_NOTIF_INFO_GET_ID_COUNT_SHIFT   7
 #define FFA_NOTIF_INFO_GET_ID_COUNT_MASK    0x1F
 
+#define FFA_NOTIF_RX_BUFFER_FULL        BIT(0, U)
+
 /* Feature IDs used with FFA_FEATURES */
 #define FFA_FEATURE_NOTIF_PEND_INTR     0x1U
 #define FFA_FEATURE_SCHEDULE_RECV_INTR  0x2U
@@ -298,6 +300,13 @@ struct ffa_ctx_notif {
      * pending global notifications.
      */
     bool secure_pending;
+
+#ifdef CONFIG_FFA_VM_TO_VM
+    /*
+     * Pending Hypervisor framework notifications
+     */
+    bool buff_full_pending;
+#endif
 };
 
 struct ffa_ctx {
@@ -369,6 +378,10 @@ void ffa_handle_notification_info_get(struct cpu_user_regs *regs);
 void ffa_handle_notification_get(struct cpu_user_regs *regs);
 int ffa_handle_notification_set(struct cpu_user_regs *regs);
 
+#ifdef CONFIG_FFA_VM_TO_VM
+void ffa_raise_rx_buffer_full(struct domain *d);
+#endif
+
 void ffa_handle_msg_send_direct_req(struct cpu_user_regs *regs, uint32_t fid);
 int32_t ffa_handle_msg_send2(struct cpu_user_regs *regs);
 
-- 
2.47.1

Re: [PATCH v2 3/5] xen/arm: ffa: Add buffer full notification support

Posted by Jens Wiklander 8 months, 4 weeks ago

Hi Bertrand,

On Mon, Mar 10, 2025 at 3:51 PM Bertrand Marquis
<bertrand.marquis@arm.com> wrote:
>
> Add support to raise a Rx buffer full notification to a VM.
> This function will be used for indirect message support between VM and
> is only activated if CONFIG_FFA_VM_TO_VM is selected.
>
> Even if there are 32 framework notifications possible, right now only
> one is defined so the implementation is simplified to only handle the
> buffer full notification using a boolean. If other framework
> notifications have to be supported one day, the design will have to be
> modified to handle it properly.
>
> Signed-off-by: Bertrand Marquis <bertrand.marquis@arm.com>
> ---
> Changes in v2:
> - Code style fix
> ---
>  xen/arch/arm/tee/ffa_notif.c   | 26 +++++++++++++++++++++-----
>  xen/arch/arm/tee/ffa_private.h | 13 +++++++++++++
>  2 files changed, 34 insertions(+), 5 deletions(-)
>
> diff --git a/xen/arch/arm/tee/ffa_notif.c b/xen/arch/arm/tee/ffa_notif.c
> index 00efaf8f7353..d19aa5c5bef6 100644
> --- a/xen/arch/arm/tee/ffa_notif.c
> +++ b/xen/arch/arm/tee/ffa_notif.c
> @@ -93,6 +93,7 @@ void ffa_handle_notification_info_get(struct cpu_user_regs *regs)
>  void ffa_handle_notification_get(struct cpu_user_regs *regs)
>  {
>      struct domain *d = current->domain;
> +    struct ffa_ctx *ctx = d->arch.tee;
>      uint32_t recv = get_user_reg(regs, 1);
>      uint32_t flags = get_user_reg(regs, 2);
>      uint32_t w2 = 0;
> @@ -132,11 +133,7 @@ void ffa_handle_notification_get(struct cpu_user_regs *regs)
>           */
>          if ( ( flags  & FFA_NOTIF_FLAG_BITMAP_SP ) &&
>               ( flags & FFA_NOTIF_FLAG_BITMAP_SPM ) )
> -        {
> -                struct ffa_ctx *ctx = d->arch.tee;
> -
> -                ACCESS_ONCE(ctx->notif.secure_pending) = false;
> -        }
> +            ACCESS_ONCE(ctx->notif.secure_pending) = false;
>
>          arm_smccc_1_2_smc(&arg, &resp);
>          e = ffa_get_ret_code(&resp);
> @@ -156,6 +153,12 @@ void ffa_handle_notification_get(struct cpu_user_regs *regs)
>              w6 = resp.a6;
>      }
>
> +#ifdef CONFIG_FFA_VM_TO_VM
> +    if ( flags & FFA_NOTIF_FLAG_BITMAP_HYP &&
> +          test_and_clear_bool(ctx->notif.buff_full_pending) )
> +        w7 = FFA_NOTIF_RX_BUFFER_FULL;
> +#endif
> +
>      ffa_set_regs(regs, FFA_SUCCESS_32, 0, w2, w3, w4, w5, w6, w7);
>  }
>
> @@ -178,6 +181,19 @@ int ffa_handle_notification_set(struct cpu_user_regs *regs)
>                             bitmap_hi);
>  }
>
> +#ifdef CONFIG_FFA_VM_TO_VM
> +void ffa_raise_rx_buffer_full(struct domain *d)
> +{
> +    struct ffa_ctx *ctx = d->arch.tee;
> +
> +    if ( !ctx )
> +        return;
> +
> +    if ( !test_and_set_bool(ctx->notif.buff_full_pending) )
> +        vgic_inject_irq(d, d->vcpu[0], notif_sri_irq, true);
> +}
> +#endif
> +
>  /*
>   * Extract a 16-bit ID (index n) from the successful return value from
>   * FFA_NOTIFICATION_INFO_GET_64 or FFA_NOTIFICATION_INFO_GET_32. IDs are
> diff --git a/xen/arch/arm/tee/ffa_private.h b/xen/arch/arm/tee/ffa_private.h
> index bd6877d8c632..1f5067d5d0c9 100644
> --- a/xen/arch/arm/tee/ffa_private.h
> +++ b/xen/arch/arm/tee/ffa_private.h
> @@ -210,6 +210,8 @@
>  #define FFA_NOTIF_INFO_GET_ID_COUNT_SHIFT   7
>  #define FFA_NOTIF_INFO_GET_ID_COUNT_MASK    0x1F
>
> +#define FFA_NOTIF_RX_BUFFER_FULL        BIT(0, U)
> +
>  /* Feature IDs used with FFA_FEATURES */
>  #define FFA_FEATURE_NOTIF_PEND_INTR     0x1U
>  #define FFA_FEATURE_SCHEDULE_RECV_INTR  0x2U
> @@ -298,6 +300,13 @@ struct ffa_ctx_notif {
>       * pending global notifications.
>       */
>      bool secure_pending;
> +
> +#ifdef CONFIG_FFA_VM_TO_VM
> +    /*
> +     * Pending Hypervisor framework notifications
> +     */
> +    bool buff_full_pending;
> +#endif

It doesn't matter if there are one or two bools in this struct.
However, the ifdef prevents using IS_ENABLED(CONFIG_FFA_VM_TO_VM)
instead of an ifdef in the logic controlling access to this field.

Cheers,
Jens

>  };
>
>  struct ffa_ctx {
> @@ -369,6 +378,10 @@ void ffa_handle_notification_info_get(struct cpu_user_regs *regs);
>  void ffa_handle_notification_get(struct cpu_user_regs *regs);
>  int ffa_handle_notification_set(struct cpu_user_regs *regs);
>
> +#ifdef CONFIG_FFA_VM_TO_VM
> +void ffa_raise_rx_buffer_full(struct domain *d);
> +#endif
> +
>  void ffa_handle_msg_send_direct_req(struct cpu_user_regs *regs, uint32_t fid);
>  int32_t ffa_handle_msg_send2(struct cpu_user_regs *regs);
>
> --
> 2.47.1
>

Re: [PATCH v2 3/5] xen/arm: ffa: Add buffer full notification support

Posted by Bertrand Marquis 8 months, 4 weeks ago

Hi Jens,

> On 20 Mar 2025, at 15:51, Jens Wiklander <jens.wiklander@linaro.org> wrote:
> 
> Hi Bertrand,
> 
> On Mon, Mar 10, 2025 at 3:51 PM Bertrand Marquis
> <bertrand.marquis@arm.com> wrote:
>> 
>> Add support to raise a Rx buffer full notification to a VM.
>> This function will be used for indirect message support between VM and
>> is only activated if CONFIG_FFA_VM_TO_VM is selected.
>> 
>> Even if there are 32 framework notifications possible, right now only
>> one is defined so the implementation is simplified to only handle the
>> buffer full notification using a boolean. If other framework
>> notifications have to be supported one day, the design will have to be
>> modified to handle it properly.
>> 
>> Signed-off-by: Bertrand Marquis <bertrand.marquis@arm.com>
>> ---
>> Changes in v2:
>> - Code style fix
>> ---
>> xen/arch/arm/tee/ffa_notif.c   | 26 +++++++++++++++++++++-----
>> xen/arch/arm/tee/ffa_private.h | 13 +++++++++++++
>> 2 files changed, 34 insertions(+), 5 deletions(-)
>> 
>> diff --git a/xen/arch/arm/tee/ffa_notif.c b/xen/arch/arm/tee/ffa_notif.c
>> index 00efaf8f7353..d19aa5c5bef6 100644
>> --- a/xen/arch/arm/tee/ffa_notif.c
>> +++ b/xen/arch/arm/tee/ffa_notif.c
>> @@ -93,6 +93,7 @@ void ffa_handle_notification_info_get(struct cpu_user_regs *regs)
>> void ffa_handle_notification_get(struct cpu_user_regs *regs)
>> {
>>     struct domain *d = current->domain;
>> +    struct ffa_ctx *ctx = d->arch.tee;
>>     uint32_t recv = get_user_reg(regs, 1);
>>     uint32_t flags = get_user_reg(regs, 2);
>>     uint32_t w2 = 0;
>> @@ -132,11 +133,7 @@ void ffa_handle_notification_get(struct cpu_user_regs *regs)
>>          */
>>         if ( ( flags  & FFA_NOTIF_FLAG_BITMAP_SP ) &&
>>              ( flags & FFA_NOTIF_FLAG_BITMAP_SPM ) )
>> -        {
>> -                struct ffa_ctx *ctx = d->arch.tee;
>> -
>> -                ACCESS_ONCE(ctx->notif.secure_pending) = false;
>> -        }
>> +            ACCESS_ONCE(ctx->notif.secure_pending) = false;
>> 
>>         arm_smccc_1_2_smc(&arg, &resp);
>>         e = ffa_get_ret_code(&resp);
>> @@ -156,6 +153,12 @@ void ffa_handle_notification_get(struct cpu_user_regs *regs)
>>             w6 = resp.a6;
>>     }
>> 
>> +#ifdef CONFIG_FFA_VM_TO_VM
>> +    if ( flags & FFA_NOTIF_FLAG_BITMAP_HYP &&
>> +          test_and_clear_bool(ctx->notif.buff_full_pending) )
>> +        w7 = FFA_NOTIF_RX_BUFFER_FULL;
>> +#endif
>> +
>>     ffa_set_regs(regs, FFA_SUCCESS_32, 0, w2, w3, w4, w5, w6, w7);
>> }
>> 
>> @@ -178,6 +181,19 @@ int ffa_handle_notification_set(struct cpu_user_regs *regs)
>>                            bitmap_hi);
>> }
>> 
>> +#ifdef CONFIG_FFA_VM_TO_VM
>> +void ffa_raise_rx_buffer_full(struct domain *d)
>> +{
>> +    struct ffa_ctx *ctx = d->arch.tee;
>> +
>> +    if ( !ctx )
>> +        return;
>> +
>> +    if ( !test_and_set_bool(ctx->notif.buff_full_pending) )
>> +        vgic_inject_irq(d, d->vcpu[0], notif_sri_irq, true);
>> +}
>> +#endif
>> +
>> /*
>>  * Extract a 16-bit ID (index n) from the successful return value from
>>  * FFA_NOTIFICATION_INFO_GET_64 or FFA_NOTIFICATION_INFO_GET_32. IDs are
>> diff --git a/xen/arch/arm/tee/ffa_private.h b/xen/arch/arm/tee/ffa_private.h
>> index bd6877d8c632..1f5067d5d0c9 100644
>> --- a/xen/arch/arm/tee/ffa_private.h
>> +++ b/xen/arch/arm/tee/ffa_private.h
>> @@ -210,6 +210,8 @@
>> #define FFA_NOTIF_INFO_GET_ID_COUNT_SHIFT   7
>> #define FFA_NOTIF_INFO_GET_ID_COUNT_MASK    0x1F
>> 
>> +#define FFA_NOTIF_RX_BUFFER_FULL        BIT(0, U)
>> +
>> /* Feature IDs used with FFA_FEATURES */
>> #define FFA_FEATURE_NOTIF_PEND_INTR     0x1U
>> #define FFA_FEATURE_SCHEDULE_RECV_INTR  0x2U
>> @@ -298,6 +300,13 @@ struct ffa_ctx_notif {
>>      * pending global notifications.
>>      */
>>     bool secure_pending;
>> +
>> +#ifdef CONFIG_FFA_VM_TO_VM
>> +    /*
>> +     * Pending Hypervisor framework notifications
>> +     */
>> +    bool buff_full_pending;
>> +#endif
> 
> It doesn't matter if there are one or two bools in this struct.
> However, the ifdef prevents using IS_ENABLED(CONFIG_FFA_VM_TO_VM)
> instead of an ifdef in the logic controlling access to this field.

Agree, I will make this field present all the time and switch to IS_ENABLED when
now possible.

Cheers
Bertrand

> 
> Cheers,
> Jens
> 
>> };
>> 
>> struct ffa_ctx {
>> @@ -369,6 +378,10 @@ void ffa_handle_notification_info_get(struct cpu_user_regs *regs);
>> void ffa_handle_notification_get(struct cpu_user_regs *regs);
>> int ffa_handle_notification_set(struct cpu_user_regs *regs);
>> 
>> +#ifdef CONFIG_FFA_VM_TO_VM
>> +void ffa_raise_rx_buffer_full(struct domain *d);
>> +#endif
>> +
>> void ffa_handle_msg_send_direct_req(struct cpu_user_regs *regs, uint32_t fid);
>> int32_t ffa_handle_msg_send2(struct cpu_user_regs *regs);
>> 
>> --
>> 2.47.1

[PATCH v2 1/5] xen/arm: Create tee command line parameter
[PATCH v2 2/5] xen/arm: ffa: Introduce VM to VM support
[PATCH v2 3/5] xen/arm: ffa: Add buffer full notification support
[PATCH v2 4/5] xen/arm: ffa: Add indirect message between VM
[PATCH v2 5/5] xen/arm: ffa: Enable VM to VM without firmware