xen/arm: ffa: fix bind/unbind notification

[XEN PATCH] xen/arm: ffa: fix bind/unbind notification

Posted by Jens Wiklander 1 month, 1 week ago

The notification bitmask is in passed in the FF-A ABI in two 32-bit
registers w3 and w4. The lower 32-bits should go in w3 and the higher in
w4. These two registers has unfortunately been swapped for
FFA_NOTIFICATION_BIND and FFA_NOTIFICATION_UNBIND in the FF-A mediator.
So fix that by using the correct registers.

Fixes: b490f470f58d ("xen/arm: ffa: support notification")
Signed-off-by: Jens Wiklander <jens.wiklander@linaro.org>
---
 xen/arch/arm/tee/ffa_notif.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/xen/arch/arm/tee/ffa_notif.c b/xen/arch/arm/tee/ffa_notif.c
index 21b9e78f6399..00efaf8f7353 100644
--- a/xen/arch/arm/tee/ffa_notif.c
+++ b/xen/arch/arm/tee/ffa_notif.c
@@ -40,8 +40,8 @@ int ffa_handle_notification_bind(struct cpu_user_regs *regs)
      * We only support notifications from SP so no need to check the sender
      * endpoint ID, the SPMC will take care of that for us.
      */
-    return ffa_simple_call(FFA_NOTIFICATION_BIND, src_dst, flags, bitmap_hi,
-                           bitmap_lo);
+    return ffa_simple_call(FFA_NOTIFICATION_BIND, src_dst, flags, bitmap_lo,
+                           bitmap_hi);
 }
 
 int ffa_handle_notification_unbind(struct cpu_user_regs *regs)
@@ -61,8 +61,8 @@ int ffa_handle_notification_unbind(struct cpu_user_regs *regs)
      * We only support notifications from SP so no need to check the
      * destination endpoint ID, the SPMC will take care of that for us.
      */
-    return  ffa_simple_call(FFA_NOTIFICATION_UNBIND, src_dst, 0, bitmap_hi,
-                            bitmap_lo);
+    return  ffa_simple_call(FFA_NOTIFICATION_UNBIND, src_dst, 0, bitmap_lo,
+                            bitmap_hi);
 }
 
 void ffa_handle_notification_info_get(struct cpu_user_regs *regs)
-- 
2.43.0

Re: [XEN PATCH] xen/arm: ffa: fix bind/unbind notification

Posted by Bertrand Marquis 1 month, 1 week ago

Hi Jens,

Thanks a lot for the finding.

> On 3 Feb 2025, at 11:21, Jens Wiklander <jens.wiklander@linaro.org> wrote:
> 
> The notification bitmask is in passed in the FF-A ABI in two 32-bit
> registers w3 and w4. The lower 32-bits should go in w3 and the higher in
> w4. These two registers has unfortunately been swapped for
> FFA_NOTIFICATION_BIND and FFA_NOTIFICATION_UNBIND in the FF-A mediator.
> So fix that by using the correct registers.
> 
> Fixes: b490f470f58d ("xen/arm: ffa: support notification")
> Signed-off-by: Jens Wiklander <jens.wiklander@linaro.org>

Reviewed-by: Bertrand Marquis <bertrand.marquis@arm.com>

@Oleksii: This is a fix of a bug, can this be considered for 4.20 ?

Thanks
Bertrand

> ---
> xen/arch/arm/tee/ffa_notif.c | 8 ++++----
> 1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/xen/arch/arm/tee/ffa_notif.c b/xen/arch/arm/tee/ffa_notif.c
> index 21b9e78f6399..00efaf8f7353 100644
> --- a/xen/arch/arm/tee/ffa_notif.c
> +++ b/xen/arch/arm/tee/ffa_notif.c
> @@ -40,8 +40,8 @@ int ffa_handle_notification_bind(struct cpu_user_regs *regs)
>      * We only support notifications from SP so no need to check the sender
>      * endpoint ID, the SPMC will take care of that for us.
>      */
> -    return ffa_simple_call(FFA_NOTIFICATION_BIND, src_dst, flags, bitmap_hi,
> -                           bitmap_lo);
> +    return ffa_simple_call(FFA_NOTIFICATION_BIND, src_dst, flags, bitmap_lo,
> +                           bitmap_hi);
> }
> 
> int ffa_handle_notification_unbind(struct cpu_user_regs *regs)
> @@ -61,8 +61,8 @@ int ffa_handle_notification_unbind(struct cpu_user_regs *regs)
>      * We only support notifications from SP so no need to check the
>      * destination endpoint ID, the SPMC will take care of that for us.
>      */
> -    return  ffa_simple_call(FFA_NOTIFICATION_UNBIND, src_dst, 0, bitmap_hi,
> -                            bitmap_lo);
> +    return  ffa_simple_call(FFA_NOTIFICATION_UNBIND, src_dst, 0, bitmap_lo,
> +                            bitmap_hi);
> }
> 
> void ffa_handle_notification_info_get(struct cpu_user_regs *regs)
> -- 
> 2.43.0
>

Re: [XEN PATCH] xen/arm: ffa: fix bind/unbind notification

Posted by Oleksii Kurochko 1 month, 1 week ago

Hi Bertrand,

On 2/3/25 11:42 AM, Bertrand Marquis wrote:
> Hi Jens,
>
> Thanks a lot for the finding.
>
>> On 3 Feb 2025, at 11:21, Jens Wiklander<jens.wiklander@linaro.org> wrote:
>>
>> The notification bitmask is in passed in the FF-A ABI in two 32-bit
>> registers w3 and w4. The lower 32-bits should go in w3 and the higher in
>> w4. These two registers has unfortunately been swapped for
>> FFA_NOTIFICATION_BIND and FFA_NOTIFICATION_UNBIND in the FF-A mediator.
>> So fix that by using the correct registers.
>>
>> Fixes: b490f470f58d ("xen/arm: ffa: support notification")
>> Signed-off-by: Jens Wiklander<jens.wiklander@linaro.org>
> Reviewed-by: Bertrand Marquis<bertrand.marquis@arm.com>
>
> @Oleksii: This is a fix of a bug, can this be considered for 4.20 ?

The fix is straightforward, so let's include this fix in 4.20:

R-Acked-by: Oleksii Kurochko <oleksii.kurochko@gmail.com>

Thanks.

~ Oleksii

>
> Thanks
> Bertrand
>
>> ---
>> xen/arch/arm/tee/ffa_notif.c | 8 ++++----
>> 1 file changed, 4 insertions(+), 4 deletions(-)
>>
>> diff --git a/xen/arch/arm/tee/ffa_notif.c b/xen/arch/arm/tee/ffa_notif.c
>> index 21b9e78f6399..00efaf8f7353 100644
>> --- a/xen/arch/arm/tee/ffa_notif.c
>> +++ b/xen/arch/arm/tee/ffa_notif.c
>> @@ -40,8 +40,8 @@ int ffa_handle_notification_bind(struct cpu_user_regs *regs)
>>       * We only support notifications from SP so no need to check the sender
>>       * endpoint ID, the SPMC will take care of that for us.
>>       */
>> -    return ffa_simple_call(FFA_NOTIFICATION_BIND, src_dst, flags, bitmap_hi,
>> -                           bitmap_lo);
>> +    return ffa_simple_call(FFA_NOTIFICATION_BIND, src_dst, flags, bitmap_lo,
>> +                           bitmap_hi);
>> }
>>
>> int ffa_handle_notification_unbind(struct cpu_user_regs *regs)
>> @@ -61,8 +61,8 @@ int ffa_handle_notification_unbind(struct cpu_user_regs *regs)
>>       * We only support notifications from SP so no need to check the
>>       * destination endpoint ID, the SPMC will take care of that for us.
>>       */
>> -    return  ffa_simple_call(FFA_NOTIFICATION_UNBIND, src_dst, 0, bitmap_hi,
>> -                            bitmap_lo);
>> +    return  ffa_simple_call(FFA_NOTIFICATION_UNBIND, src_dst, 0, bitmap_lo,
>> +                            bitmap_hi);
>> }
>>
>> void ffa_handle_notification_info_get(struct cpu_user_regs *regs)
>> -- 
>> 2.43.0
>>