Re: [PATCH v3 03/17] firmware: arm_ffa: Implement the notification bind and unbind interface

[Olivier Deprez]

Hi Jens,

> dst_id and drv_info->vm_id should be swapped.

I'm curious about this because swapping like this actually makes hafnium fail. Need to check from the spec.

Regards,
Olivier.



________________________________________
From: Jens Wiklander <jens.wiklander@xxxxxxxxxx>
Sent: 04 October 2023 11:11
To: Sudeep Holla
Cc: linux-arm-kernel@xxxxxxxxxxxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx; Marc Bonnici; Coboy Chen; Lorenzo Pieralisi; Olivier Deprez
Subject: Re: [PATCH v3 03/17] firmware: arm_ffa: Implement the notification bind and unbind interface

On Fri, Sep 29, 2023 at 04:02:52PM +0100, Sudeep Holla wrote:
> A receiver endpoint must bind a notification to any sender endpoint
> before the latter can signal the notification to the former. The receiver
> assigns one or more doorbells to a specific sender. Only the sender can
> ring these doorbells.
>
> A receiver uses the FFA_NOTIFICATION_BIND interface to bind one or more
> notifications to the sender. A receiver un-binds a notification from a
> sender endpoint to stop the notification from being signaled. It uses
> the FFA_NOTIFICATION_UNBIND interface to do this.
>
> Allow the FF-A driver to be able to bind and unbind a given notification
> ID to a specific partition ID. This will be used to register and
> unregister notification callbacks from the FF-A client drivers.
>
> Signed-off-by: Sudeep Holla <sudeep.holla@xxxxxxx>
> ---
>  drivers/firmware/arm_ffa/driver.c | 29 +++++++++++++++++++++++++++++
>  1 file changed, 29 insertions(+)
>
> diff --git a/drivers/firmware/arm_ffa/driver.c b/drivers/firmware/arm_ffa/driver.c
> index efa4e7fb15e3..26bf9c4e3b5f 100644
> --- a/drivers/firmware/arm_ffa/driver.c
> +++ b/drivers/firmware/arm_ffa/driver.c
> @@ -587,6 +587,35 @@ static int ffa_notification_bitmap_destroy(void)
>       return 0;
>  }
>
> +#define NOTIFICATION_LOW_MASK                GENMASK(31, 0)
> +#define NOTIFICATION_HIGH_MASK               GENMASK(63, 32)
> +#define NOTIFICATION_BITMAP_HIGH(x)  \
> +             ((u32)(FIELD_GET(NOTIFICATION_HIGH_MASK, (x))))
> +#define NOTIFICATION_BITMAP_LOW(x)   \
> +             ((u32)(FIELD_GET(NOTIFICATION_LOW_MASK, (x))))
> +
> +static int ffa_notification_bind_common(u16 dst_id, u64 bitmap,
> +                                     u32 flags, bool is_bind)
> +{
> +     ffa_value_t ret;
> +     u32 func, src_dst_ids = PACK_TARGET_INFO(dst_id, drv_info->vm_id);

dst_id and drv_info->vm_id should be swapped.

Thanks,
Jens

> +
> +     func = is_bind ? FFA_NOTIFICATION_BIND : FFA_NOTIFICATION_UNBIND;
> +
> +     invoke_ffa_fn((ffa_value_t){
> +               .a0 = func, .a1 = src_dst_ids, .a2 = flags,
> +               .a3 = NOTIFICATION_BITMAP_LOW(bitmap),
> +               .a4 = NOTIFICATION_BITMAP_HIGH(bitmap),
> +               }, &ret);
> +
> +     if (ret.a0 == FFA_ERROR)
> +             return ffa_to_linux_errno((int)ret.a2);
> +     else if (ret.a0 != FFA_SUCCESS)
> +             return -EINVAL;
> +
> +     return 0;
> +}
> +
>  static void ffa_set_up_mem_ops_native_flag(void)
>  {
>       if (!ffa_features(FFA_FN_NATIVE(MEM_LEND), 0, NULL, NULL) ||
>
> --
> 2.42.0
>