Re: [XEN PATCH v1 5/5] xen/arm: ffa: support notification

[Bertrand Marquis <Bertrand.Marquis@xxxxxxx>]

Hi Jens,

> On 10 Apr 2024, at 16:27, Jens Wiklander <jens.wiklander@xxxxxxxxxx> wrote:
> 
> On Wed, Apr 10, 2024 at 9:49 AM Bertrand Marquis
> <Bertrand.Marquis@xxxxxxx> wrote:
>> 
>> Hi Jens,
>> 
>>> On 9 Apr 2024, at 17:36, Jens Wiklander <jens.wiklander@xxxxxxxxxx> wrote:
>>> 
>>> Add support for FF-A notifications, currently limited to an SP (Secure
>>> Partition) sending an asynchronous notification to a guest.
>>> 
>>> Guests and Xen itself are made aware of pending notifications with an
>>> interrupt. The interrupt handler retrieves the notifications using the
>>> FF-A ABI and deliver them to their destinations.
>>> 
>>> Signed-off-by: Jens Wiklander <jens.wiklander@xxxxxxxxxx>
>>> ---
>>> xen/arch/arm/tee/Makefile      |   1 +
>>> xen/arch/arm/tee/ffa.c         |  58 ++++++
>>> xen/arch/arm/tee/ffa_notif.c   | 319 +++++++++++++++++++++++++++++++++
>>> xen/arch/arm/tee/ffa_private.h |  71 ++++++++
>>> 4 files changed, 449 insertions(+)
>>> create mode 100644 xen/arch/arm/tee/ffa_notif.c
>>> 
>>> diff --git a/xen/arch/arm/tee/Makefile b/xen/arch/arm/tee/Makefile
>>> index f0112a2f922d..7c0f46f7f446 100644
>>> --- a/xen/arch/arm/tee/Makefile
>>> +++ b/xen/arch/arm/tee/Makefile
>>> @@ -2,5 +2,6 @@ obj-$(CONFIG_FFA) += ffa.o
>>> obj-$(CONFIG_FFA) += ffa_shm.o
>>> obj-$(CONFIG_FFA) += ffa_partinfo.o
>>> obj-$(CONFIG_FFA) += ffa_rxtx.o
>>> +obj-$(CONFIG_FFA) += ffa_notif.o
>>> obj-y += tee.o
>>> obj-$(CONFIG_OPTEE) += optee.o
>>> diff --git a/xen/arch/arm/tee/ffa.c b/xen/arch/arm/tee/ffa.c
>>> index 5209612963e1..ce9757bfeed1 100644
>>> --- a/xen/arch/arm/tee/ffa.c
>>> +++ b/xen/arch/arm/tee/ffa.c
>>> @@ -39,6 +39,9 @@
>>> *   - at most 32 shared memory regions per guest
>>> * o FFA_MSG_SEND_DIRECT_REQ:
>>> *   - only supported from a VM to an SP
>>> + * o FFA_NOTIFICATION_*:
>>> + *   - only supports global notifications, that is, per vCPU notifications
>>> + *     are not supported
>>> *
>>> * There are some large locked sections with ffa_tx_buffer_lock and
>>> * ffa_rx_buffer_lock. Especially the ffa_tx_buffer_lock spinlock used
>>> @@ -194,6 +197,8 @@ out:
>>> 
>>> static void handle_features(struct cpu_user_regs *regs)
>>> {
>>> +    struct domain *d = current->domain;
>>> +    struct ffa_ctx *ctx = d->arch.tee;
>>>    uint32_t a1 = get_user_reg(regs, 1);
>>>    unsigned int n;
>>> 
>>> @@ -240,6 +245,30 @@ static void handle_features(struct cpu_user_regs *regs)
>>>        BUILD_BUG_ON(PAGE_SIZE != FFA_PAGE_SIZE);
>>>        ffa_set_regs_success(regs, 0, 0);
>>>        break;
>>> +    case FFA_FEATURE_NOTIF_PEND_INTR:
>>> +        if ( ctx->notif.enabled )
>>> +            ffa_set_regs_success(regs, FFA_NOTIF_PEND_INTR_ID, 0);
>>> +        else
>>> +            ffa_set_regs_error(regs, FFA_RET_NOT_SUPPORTED);
>>> +        break;
>>> +    case FFA_FEATURE_SCHEDULE_RECV_INTR:
>>> +        if ( ctx->notif.enabled )
>>> +            ffa_set_regs_success(regs, FFA_NOTIF_PEND_INTR_ID, 0);
>> 
>> This should return the RECV_INTR, not the PEND one.
> 
> Thanks, I'll fix it.
> 
>> 
>>> +        else
>>> +            ffa_set_regs_error(regs, FFA_RET_NOT_SUPPORTED);
>>> +        break;
>>> +
>>> +    case FFA_NOTIFICATION_BIND:
>>> +    case FFA_NOTIFICATION_UNBIND:
>>> +    case FFA_NOTIFICATION_GET:
>>> +    case FFA_NOTIFICATION_SET:
>>> +    case FFA_NOTIFICATION_INFO_GET_32:
>>> +    case FFA_NOTIFICATION_INFO_GET_64:
>>> +        if ( ctx->notif.enabled )
>>> +            ffa_set_regs_success(regs, 0, 0);
>>> +        else
>>> +            ffa_set_regs_error(regs, FFA_RET_NOT_SUPPORTED);
>>> +        break;
>>>    default:
>>>        ffa_set_regs_error(regs, FFA_RET_NOT_SUPPORTED);
>>>        break;
>>> @@ -305,6 +334,30 @@ static bool ffa_handle_call(struct cpu_user_regs *regs)
>>>                                                     get_user_reg(regs, 1)),
>>>                                   get_user_reg(regs, 3));
>>>        break;
>>> +    case FFA_NOTIFICATION_BIND:
>>> +        e = ffa_handle_notification_bind(get_user_reg(regs, 1),
>>> +                                         get_user_reg(regs, 2),
>>> +                                         get_user_reg(regs, 3),
>>> +                                         get_user_reg(regs, 4));
>> 
>> I would suggest to pass regs and handle the get_user_regs in the function.
> 
> OK
> 
>> 
>>> +        break;
>>> +    case FFA_NOTIFICATION_UNBIND:
>>> +        e = ffa_handle_notification_unbind(get_user_reg(regs, 1),
>>> +                                           get_user_reg(regs, 3),
>>> +                                           get_user_reg(regs, 4));
>> 
>> same here
> 
> OK
> 
>> 
>>> +        break;
>>> +    case FFA_NOTIFICATION_INFO_GET_32:
>>> +    case FFA_NOTIFICATION_INFO_GET_64:
>>> +        ffa_handle_notification_info_get(regs);
>>> +        return true;
>>> +    case FFA_NOTIFICATION_GET:
>>> +        ffa_handle_notification_get(regs);
>>> +        return true;
>>> +    case FFA_NOTIFICATION_SET:
>>> +        e = ffa_handle_notification_set(get_user_reg(regs, 1),
>>> +                                        get_user_reg(regs, 2),
>>> +                                        get_user_reg(regs, 3),
>>> +                                        get_user_reg(regs, 4));
>> 
>> same here
> 
> OK
> 
>> 
>>> +        break;
>>> 
>>>    default:
>>>        gprintk(XENLOG_ERR, "ffa: unhandled fid 0x%x\n", fid);
>>> @@ -348,6 +401,9 @@ static int ffa_domain_init(struct domain *d)
>>>    if ( !ffa_partinfo_domain_init(d) )
>>>        return -EIO;
>>> 
>>> +    if ( !ffa_notif_domain_init(d) )
>>> +        return -ENOMEM;
>> 
>> Having this function deciding on the return code is a bit weird.
>> I would suggest to have ffa_notif_domain_init returning an int
>> and deciding on the error code and this one just returning the
>> error if !=0.
>> 
>> If possible the same principle should be applied for the partinfo.
> 
> OK, I'll fix it.
> 
>> 
>>> +
>>>    return 0;
>>> }
>>> 
>>> @@ -423,6 +479,7 @@ static int ffa_domain_teardown(struct domain *d)
>>>        return 0;
>>> 
>>>    ffa_rxtx_domain_destroy(d);
>>> +    ffa_notif_domain_destroy(d);
>>> 
>>>    ffa_domain_teardown_continue(ctx, true /* first_time */);
>>> 
>>> @@ -502,6 +559,7 @@ static bool ffa_probe(void)
>>>    if ( !ffa_partinfo_init() )
>>>        goto err_rxtx_destroy;
>>> 
>>> +    ffa_notif_init();
>>>    INIT_LIST_HEAD(&ffa_teardown_head);
>>>    init_timer(&ffa_teardown_timer, ffa_teardown_timer_callback, NULL, 0);
>>> 
>>> diff --git a/xen/arch/arm/tee/ffa_notif.c b/xen/arch/arm/tee/ffa_notif.c
>>> new file mode 100644
>>> index 000000000000..0173ee515362
>>> --- /dev/null
>>> +++ b/xen/arch/arm/tee/ffa_notif.c
>>> @@ -0,0 +1,319 @@
>>> +/* SPDX-License-Identifier: GPL-2.0-only */
>>> +/*
>>> + * Copyright (C) 2024  Linaro Limited
>>> + */
>>> +
>>> +#include <xen/const.h>
>>> +#include <xen/list.h>
>>> +#include <xen/spinlock.h>
>>> +#include <xen/types.h>
>>> +
>>> +#include <asm/smccc.h>
>>> +#include <asm/regs.h>
>>> +
>>> +#include "ffa_private.h"
>>> +
>>> +static bool __ro_after_init notif_enabled;
>>> +
>>> +int ffa_handle_notification_bind(uint32_t src_dst, uint32_t flags,
>>> +                                 uint32_t bitmap_lo, uint32_t bitmap_hi)
>>> +{
>>> +    struct domain *d = current->domain;
>>> +
>>> +    if ( !notif_enabled )
>>> +        return FFA_RET_NOT_SUPPORTED;
>>> +
>>> +    if ( (src_dst & 0xffff) != ffa_get_vm_id(d) )
>>> +        return FFA_RET_INVALID_PARAMETERS;
>> 
>> s/0xffff/0xFFFFU/
> 
> OK
> 
>> 
>>> +
>>> +    if ( flags )    /* Only global notifications are supported */
>>> +        return FFA_RET_DENIED;
>>> +
>>> +    /*
>>> +     * 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);
>>> +}
>>> +
>>> +int ffa_handle_notification_unbind(uint32_t src_dst, uint32_t bitmap_lo,
>>> +                                   uint32_t bitmap_hi)
>>> +{
>>> +    struct domain *d = current->domain;
>>> +
>>> +    if ( !notif_enabled )
>>> +        return FFA_RET_NOT_SUPPORTED;
>>> +
>>> +    if ( (src_dst & 0xffff) != ffa_get_vm_id(d) )
>>> +        return FFA_RET_INVALID_PARAMETERS;
>> 
>> s/0xffff/0xFFFFU/
> 
> OK
> 
>> 
>>> +
>>> +    /*
>>> +     * 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);
>>> +}
>>> +
>>> +void ffa_handle_notification_info_get(struct cpu_user_regs *regs)
>>> +{
>>> +    struct domain *d = current->domain;
>>> +    struct ffa_ctx *ctx = d->arch.tee;
>>> +    bool pending_global;
>>> +
>>> +    if ( !notif_enabled )
>>> +    {
>>> +        ffa_set_regs_error(regs, FFA_RET_NOT_SUPPORTED);
>>> +        return;
>>> +    }
>>> +
>>> +    spin_lock(&ctx->notif.lock);
>>> +    pending_global = ctx->notif.secure_pending;
>>> +    ctx->notif.secure_pending = false;
>>> +    spin_unlock(&ctx->notif.lock);
>>> +
>>> +    if ( pending_global )
>>> +    {
>>> +        /* A pending global notification for the guest */
>>> +        ffa_set_regs(regs, FFA_SUCCESS_64, 0,
>>> +                     1U << FFA_NOTIF_INFO_GET_ID_COUNT_SHIFT, 
>>> ffa_get_vm_id(d),
>>> +                     0, 0, 0, 0);
>>> +    }
>>> +    else
>>> +    {
>>> +        /* Report an error if there where no pending global notification */
>>> +        ffa_set_regs_error(regs, FFA_RET_NO_DATA);
>>> +    }
>>> +}
>>> +
>>> +void ffa_handle_notification_get(struct cpu_user_regs *regs)
>>> +{
>>> +    struct domain *d = current->domain;
>>> +    uint32_t recv = get_user_reg(regs, 1);
>>> +    uint32_t flags = get_user_reg(regs, 2);
>>> +    uint32_t w2 = 0;
>>> +    uint32_t w3 = 0;
>>> +    uint32_t w4 = 0;
>>> +    uint32_t w5 = 0;
>>> +    uint32_t w6 = 0;
>>> +    uint32_t w7 = 0;
>>> +
>>> +    if ( !notif_enabled )
>>> +    {
>>> +        ffa_set_regs_error(regs, FFA_RET_NOT_SUPPORTED);
>>> +        return;
>>> +    }
>>> +
>>> +    if ( (recv & 0xffff) != ffa_get_vm_id(d) )
>> s/0xffff/0xFFFFU/
> 
> OK
> 
>> 
>>> +    {
>>> +        ffa_set_regs_error(regs, FFA_RET_INVALID_PARAMETERS);
>>> +        return;
>>> +    }
>>> +
>>> +    if ( flags & ( FFA_NOTIF_FLAG_BITMAP_SP | FFA_NOTIF_FLAG_BITMAP_SPM ) )
>>> +    {
>>> +        struct arm_smccc_1_2_regs arg = {
>>> +            .a0 = FFA_NOTIFICATION_GET,
>>> +            .a1 = recv,
>>> +            .a2 = flags & ( FFA_NOTIF_FLAG_BITMAP_SP |
>>> +                            FFA_NOTIF_FLAG_BITMAP_SPM ),
>>> +        };
>>> +        struct arm_smccc_1_2_regs resp;
>>> +        int32_t e;
>>> +
>>> +        arm_smccc_1_2_smc(&arg, &resp);
>>> +        e = ffa_get_ret_code(&resp);
>>> +        if ( e )
>>> +        {
>>> +            ffa_set_regs_error(regs, e);
>>> +            return;
>>> +        }
>>> +
>>> +        if ( flags & FFA_NOTIF_FLAG_BITMAP_SP )
>>> +        {
>>> +            w2 = resp.a2;
>>> +            w3 = resp.a3;
>>> +        }
>>> +
>>> +        if ( flags & FFA_NOTIF_FLAG_BITMAP_SPM )
>>> +            w6 = resp.a6;
>>> +    }
>>> +
>>> +    ffa_set_regs(regs, FFA_SUCCESS_32, 0, w2, w3, w4, w5, w6, w7);
>>> +}
>>> +
>>> +int ffa_handle_notification_set(uint32_t src_dst, uint32_t flags,
>>> +                                uint32_t bitmap_lo, uint32_t bitmap_hi)
>>> +{
>>> +    struct domain *d = current->domain;
>>> +
>>> +    if ( !notif_enabled )
>>> +        return FFA_RET_NOT_SUPPORTED;
>>> +
>>> +    if ( (src_dst >> 16) != ffa_get_vm_id(d) )
>>> +        return FFA_RET_INVALID_PARAMETERS;
>> 
>> This needs some checking as i would have used the lowest bits here
>> for the source and not the highest. The spec is using the same description
>> for all ABIs so I am wondering if you are not using the destination instead 
>> of
>> the source here.
> 
> This is a bit tricky because not all ABI functions define Sender and
> Receiver in the same way. For FFA_NOTIFICATION_BIND it's the Sender
> and Receiver of the notification, while for instance,
> FFA_MSG_SEND_DIRECT_REQ defines it as the Sender and Receiver of the
> message.
> 
> When the Hypervisor invokes FFA_NOTIFICATION_SET it's the Sender and
> Receiver of the notification, that is, the guest is the same as the
> sender of the notification. So the guest ID should go into BIT[31:16],
> and the receiver of the notification in BIT[15:0].
> 
> When the guest invokes FFA_NOTIFICATION_SET the Hypervisor is
> requested to signal notifications to the Sender endpoint BIT[31:16].
> What's expected in BIT[15:0] isn't mentioned so I assume the
> Hypervisor should ignore it since it already knows the guest ID.
> 
> Following that analysis, we should replace the if statement above with:
> src_dst = ((uint32_t)ffa_get_vm_id(d) << 16) | (src_dst >> 16)
> 
> But I'm not certain I've understood the specification correctly, in
> particular the part where the guest invokes FFA_NOTIFICATION_SET.
> 
> What's your take on this?
> 
> I don't use this function in my tests so it's perhaps better to wait
> with the implementation of this function until it's used.

You are right, this is a bit imprecise.

In notification_set Sender is defined as "signal to the sender" and then saying 
that "sender" is the VM when the hypervisor push it to the SPMC.

I will dig on that and come back to you on this.

Cheers
Bertrand

> 
>> 
>>> +
>>> +    /*
>>> +     * 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_SET, src_dst, flags, bitmap_lo,
>>> +                           bitmap_hi);
>>> +}
>>> +
>> 
>> The following function would deserve some explanation in a comment
>> to clear up a bit what is done here and why.
> 
> I'll add a description.
> 
>> 
>>> +static uint16_t get_id_from_resp(struct arm_smccc_1_2_regs *resp,
>>> +                                 unsigned int n)
>>> +{
>>> +    unsigned int ids_per_reg;
>>> +    unsigned int reg_idx;
>>> +    unsigned int reg_shift;
>>> +
>>> +    if ( smccc_is_conv_64(resp->a0) )
>>> +        ids_per_reg = 4;
>>> +    else
>>> +        ids_per_reg = 2;
>>> +
>>> +    reg_idx = n / ids_per_reg + 3;
>>> +    reg_shift = ( n % ids_per_reg ) * 16;
>>> +
>>> +    switch ( reg_idx )
>>> +    {
>>> +    case 3:
>>> +        return resp->a3 >> reg_shift;
>>> +    case 4:
>>> +        return resp->a4 >> reg_shift;
>>> +    case 5:
>>> +        return resp->a5 >> reg_shift;
>>> +    case 6:
>>> +        return resp->a6 >> reg_shift;
>>> +    case 7:
>>> +        return resp->a7 >> reg_shift;
>>> +    default:
>>> +        ASSERT(0); /* "Can't happen" */
>>> +        return 0;
>>> +    }
>>> +}
>>> +
>>> +static void notif_irq_handler(int irq, void *data)
>>> +{
>>> +    const struct arm_smccc_1_2_regs arg = {
>>> +        .a0 = FFA_NOTIFICATION_INFO_GET_64,
>>> +    };
>>> +    struct arm_smccc_1_2_regs resp;
>>> +    unsigned int id_pos;
>>> +    unsigned int list_count;
>>> +    uint64_t ids_count;
>>> +    unsigned int n;
>>> +    int32_t res;
>>> +
>>> +    do {
>>> +        arm_smccc_1_2_smc(&arg, &resp);
>>> +        res = ffa_get_ret_code(&resp);
>>> +        if ( res )
>>> +        {
>>> +            if ( res != FFA_RET_NO_DATA )
>>> +                printk(XENLOG_ERR "ffa: notification info get failed: 
>>> error %d\n",
>>> +                       res);
>>> +            return;
>>> +        }
>>> +
>>> +        ids_count = resp.a2 >> FFA_NOTIF_INFO_GET_ID_LIST_SHIFT;
>>> +        list_count = ( resp.a2 >> FFA_NOTIF_INFO_GET_ID_COUNT_SHIFT ) &
>>> +                     FFA_NOTIF_INFO_GET_ID_COUNT_MASK;
>>> +
>>> +        id_pos = 0;
>>> +        for ( n = 0; n < list_count; n++ )
>>> +        {
>>> +            unsigned int count = ((ids_count >> 2 * n) & 0x3) + 1;
>>> +            struct domain *d;
>>> +
>>> +            d = ffa_get_domain_by_vm_id(get_id_from_resp(&resp, id_pos));
>>> +
>>> +            if ( d )
>>> +            {
>>> +                struct ffa_ctx *ctx = d->arch.tee;
>>> +
>>> +                spin_lock(&ctx->notif.lock);
>>> +                ctx->notif.secure_pending = true;
>>> +                spin_unlock(&ctx->notif.lock);
>>> +
>>> +                /*
>>> +                 * Since we're only delivering global notification, always
>>> +                 * deliver to the first vCPU. It doesn't matter which we
>>> +                 * chose, as long as it's available.
>>> +                 */
>>> +                vgic_inject_irq(d, d->vcpu[0], FFA_NOTIF_PEND_INTR_ID, 
>>> true);
>>> +
>>> +                put_domain(d);
>>> +            }
>>> +
>>> +            id_pos += count;
>>> +        }
>>> +
>>> +    } while (resp.a2 & FFA_NOTIF_INFO_GET_MORE_FLAG);
>>> +}
>>> +
>>> +static int32_t ffa_notification_bitmap_create(uint16_t vm_id,
>>> +                                              uint32_t vcpu_count)
>>> +{
>>> +    return ffa_simple_call(FFA_NOTIFICATION_BITMAP_CREATE, vm_id, 
>>> vcpu_count,
>>> +                           0, 0);
>>> +}
>>> +
>>> +static int32_t ffa_notification_bitmap_destroy(uint16_t vm_id)
>>> +{
>>> +    return ffa_simple_call(FFA_NOTIFICATION_BITMAP_DESTROY, vm_id, 0, 0, 
>>> 0);
>>> +}
>>> +
>>> +void ffa_notif_init(void)
>>> +{
>>> +    const struct arm_smccc_1_2_regs arg = {
>>> +        .a0 = FFA_FEATURES,
>>> +        .a1 = FFA_FEATURE_SCHEDULE_RECV_INTR,
>>> +    };
>>> +    struct arm_smccc_1_2_regs resp;
>>> +    unsigned int irq;
>>> +    int ret;
>>> +
>>> +    arm_smccc_1_2_smc(&arg, &resp);
>>> +    if ( resp.a0 != FFA_SUCCESS_32 )
>>> +        return;
>>> +
>>> +    irq = resp.a2;
>>> +    if ( irq >= NR_GIC_SGI )
>>> +        irq_set_type(irq, IRQ_TYPE_EDGE_RISING);
>>> +    ret = request_irq(irq, 0, notif_irq_handler, "FF-A notif", NULL);
>>> +    if ( ret )
>>> +        printk(XENLOG_ERR "ffa: request_irq irq %u failed: error %d\n",
>>> +               irq, ret);
>>> +    notif_enabled = !ret;
>>> +}
>>> +
>>> +bool ffa_notif_domain_init(struct domain *d)
>>> +{
>>> +    struct ffa_ctx *ctx = d->arch.tee;
>>> +    int32_t res;
>>> +
>>> +    if ( !notif_enabled )
>>> +        return true;
>>> +
>>> +    res = ffa_notification_bitmap_create(ffa_get_vm_id(d), d->max_vcpus);
>>> +    if ( res )
>>> +        return false;
>>> +
>>> +    ctx->notif.enabled = true;
>>> +
>>> +    return true;
>>> +}
>>> +
>>> +void ffa_notif_domain_destroy(struct domain *d)
>>> +{
>>> +    struct ffa_ctx *ctx = d->arch.tee;
>>> +
>>> +    if ( ctx->notif.enabled )
>>> +    {
>>> +        ffa_notification_bitmap_destroy(ffa_get_vm_id(d));
>>> +        ctx->notif.enabled = false;
>>> +    }
>>> +}
>>> diff --git a/xen/arch/arm/tee/ffa_private.h b/xen/arch/arm/tee/ffa_private.h
>>> index 98236cbf14a3..26c2af164d38 100644
>>> --- a/xen/arch/arm/tee/ffa_private.h
>>> +++ b/xen/arch/arm/tee/ffa_private.h
>>> @@ -25,6 +25,7 @@
>>> #define FFA_RET_DENIED                  -6
>>> #define FFA_RET_RETRY                   -7
>>> #define FFA_RET_ABORTED                 -8
>>> +#define FFA_RET_NO_DATA                 -9
>>> 
>>> /* FFA_VERSION helpers */
>>> #define FFA_VERSION_MAJOR_SHIFT         16U
>>> @@ -60,6 +61,8 @@
>>> */
>>> #define FFA_PAGE_SIZE                   SZ_4K
>>> 
>>> +#define FFA_NOTIF_BITMAP_SIZE           64
>>> +
>> 
>> This does not seem to be used.
> 
> You're right, I'll remove it.
> 
>> 
>>> /*
>>> * The number of pages used for each of the RX and TX buffers shared with
>>> * the SPMC.
>>> @@ -97,6 +100,18 @@
>>> */
>>> #define FFA_MAX_SHM_COUNT               32
>>> 
>>> +/*
>>> + * TODO How to manage the available SGIs? SGI 8-15 seem to be entirely
>>> + * unused, but that may change.
>> 
>> I am a bit wondering what your TODO means here.
>> Do you mean that we should have a way to "allocate a free SGI" ?
> 
> As long as only the FF-A mediator adds a special meaning to certain
> virtual SGIs in the 8-15 range it might be overkill with an allocator.
> But if other parts start to do the same we may have conflicts if it's
> not managed centrally.
> 
> Thanks,
> Jens
> 
>> 
>>> + *
>>> + * SGI is the preferred delivery mechanism. SGIs 8-15 are normally not used
>>> + * by a guest as they in a non-virtualized system typically are assigned to
>>> + * the secure world. Here we're free to use SGI 8-15 since they are virtual
>>> + * and have nothing to do with the secure world.
>>> + */
>>> +#define FFA_NOTIF_PEND_INTR_ID      8
>>> +#define FFA_SCHEDULE_RECV_INTR_ID   9
>>> +
>>> /*
>>> * The time we wait until trying to tear down a domain again if it was
>>> * blocked initially.
>>> @@ -175,6 +190,21 @@
>>> */
>>> #define FFA_PARTITION_INFO_GET_COUNT_FLAG BIT(0, U)
>>> 
>>> +/* Flags used in calls to FFA_NOTIFICATION_GET interface  */
>>> +#define FFA_NOTIF_FLAG_BITMAP_SP        BIT(0, U)
>>> +#define FFA_NOTIF_FLAG_BITMAP_VM        BIT(1, U)
>>> +#define FFA_NOTIF_FLAG_BITMAP_SPM       BIT(2, U)
>>> +#define FFA_NOTIF_FLAG_BITMAP_HYP       BIT(3, U)
>>> +
>>> +#define FFA_NOTIF_INFO_GET_MORE_FLAG        BIT(0, U)
>>> +#define FFA_NOTIF_INFO_GET_ID_LIST_SHIFT    12
>>> +#define FFA_NOTIF_INFO_GET_ID_COUNT_SHIFT   7
>>> +#define FFA_NOTIF_INFO_GET_ID_COUNT_MASK    0x1F
>>> +
>>> +/* Feature IDs used with FFA_FEATURES */
>>> +#define FFA_FEATURE_NOTIF_PEND_INTR     0x1U
>>> +#define FFA_FEATURE_SCHEDULE_RECV_INTR  0x2U
>>> +
>>> /* Function IDs */
>>> #define FFA_ERROR                       0x84000060U
>>> #define FFA_SUCCESS_32                  0x84000061U
>>> @@ -213,6 +243,27 @@
>>> #define FFA_MEM_FRAG_TX                 0x8400007BU
>>> #define FFA_MSG_SEND                    0x8400006EU
>>> #define FFA_MSG_POLL                    0x8400006AU
>>> +#define FFA_NOTIFICATION_BITMAP_CREATE  0x8400007DU
>>> +#define FFA_NOTIFICATION_BITMAP_DESTROY 0x8400007EU
>>> +#define FFA_NOTIFICATION_BIND           0x8400007FU
>>> +#define FFA_NOTIFICATION_UNBIND         0x84000080U
>>> +#define FFA_NOTIFICATION_SET            0x84000081U
>>> +#define FFA_NOTIFICATION_GET            0x84000082U
>>> +#define FFA_NOTIFICATION_INFO_GET_32    0x84000083U
>>> +#define FFA_NOTIFICATION_INFO_GET_64    0xC4000083U
>>> +
>>> +struct ffa_ctx_notif {
>>> +    bool enabled;
>>> +
>>> +    /* Used to serialize access to the rest of this struct */
>>> +    spinlock_t lock;
>>> +
>>> +    /*
>>> +     * True if domain is reported by FFA_NOTIFICATION_INFO_GET to have
>>> +     * pending global notifications.
>>> +     */
>>> +    bool secure_pending;
>>> +};
>>> 
>>> struct ffa_ctx {
>>>    void *rx;
>>> @@ -228,6 +279,7 @@ struct ffa_ctx {
>>>    struct list_head shm_list;
>>>    /* Number of allocated shared memory object */
>>>    unsigned int shm_count;
>>> +    struct ffa_ctx_notif notif;
>>>    /*
>>>     * tx_lock is used to serialize access to tx
>>>     * rx_lock is used to serialize access to rx
>>> @@ -271,12 +323,31 @@ uint32_t ffa_handle_rxtx_map(uint32_t fid, register_t 
>>> tx_addr,
>>> uint32_t ffa_handle_rxtx_unmap(void);
>>> int32_t ffa_handle_rx_release(void);
>>> 
>>> +void ffa_notif_init(void);
>>> +bool ffa_notif_domain_init(struct domain *d);
>>> +void ffa_notif_domain_destroy(struct domain *d);
>>> +
>>> +int ffa_handle_notification_bind(uint32_t src_dst, uint32_t flags,
>>> +                                 uint32_t bitmap_lo, uint32_t bitmap_hi);
>>> +int ffa_handle_notification_unbind(uint32_t src_dst, uint32_t bitmap_lo,
>>> +                                   uint32_t bitmap_hi);
>>> +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(uint32_t src_dst, uint32_t flags,
>>> +                                uint32_t bitmap_lo, uint32_t bitmap_hi);
>>> +
>>> static inline uint16_t ffa_get_vm_id(const struct domain *d)
>>> {
>>>    /* +1 since 0 is reserved for the hypervisor in FF-A */
>>>    return d->domain_id + 1;
>>> }
>>> 
>>> +static inline struct domain *ffa_get_domain_by_vm_id(uint16_t vm_id)
>>> +{
>>> +    /* -1 to match ffa_get_vm_id() */
>>> +    return get_domain_by_id(vm_id - 1);
>>> +}
>>> +
>>> static inline void ffa_set_regs(struct cpu_user_regs *regs, register_t v0,
>>>                                register_t v1, register_t v2, register_t v3,
>>>                                register_t v4, register_t v5, register_t v6,
>>> --
>>> 2.34.1