Re: [PATCH v3 07/17] firmware: arm_ffa: Implement the NOTIFICATION_INFO_GET interface

[Jens Wiklander]

On Wed, Oct 4, 2023 at 5:11 PM Sudeep Holla <sudeep.holla@xxxxxxx> wrote:
>
> On Wed, Oct 04, 2023 at 11:10:21AM +0200, Jens Wiklander wrote:
> > On Fri, Sep 29, 2023 at 04:02:56PM +0100, Sudeep Holla wrote:
> > > The receiver’s scheduler uses the FFA_NOTIFICATION_INFO_GET interface
> > > to retrieve the list of endpoints that have pending notifications and
> > > must be run. A notification could be signaled by a sender in the secure
> > > world to a VM. The Hypervisor needs to determine which VM and vCPU
> > > (in case a per-vCPU notification is signaled) has a pending notification
> > > in this scenario. It must obtain this information through an invocation
> > > of the FFA_NOTIFICATION_INFO_GET.
> > >
> > > Add the implementation of the NOTIFICATION_INFO_GET interface
> > > and prepare to use this to handle the schedule receiver interrupt.
> > > Implementation of handling notifications will be added later.
> > >
> > > Signed-off-by: Sudeep Holla <sudeep.holla@xxxxxxx>
> > > ---
> > >  drivers/firmware/arm_ffa/driver.c | 70 +++++++++++++++++++++++++++++++++++++++
> > >  1 file changed, 70 insertions(+)
> > >
> > > diff --git a/drivers/firmware/arm_ffa/driver.c b/drivers/firmware/arm_ffa/driver.c
> > > index 02eedb7bc171..dfeeb751bebe 100644
> > > --- a/drivers/firmware/arm_ffa/driver.c
> > > +++ b/drivers/firmware/arm_ffa/driver.c
> > > @@ -602,6 +602,13 @@ static int ffa_notification_bitmap_destroy(void)
> > >     (FIELD_PREP(RECEIVER_VCPU_MASK, (vcpu_r)) | \
> > >      FIELD_PREP(RECEIVER_ID_MASK, (r)))
> > >
> > > +#define NOTIFICATION_INFO_GET_MORE_PEND_MASK       BIT(0)
> > > +#define NOTIFICATION_INFO_GET_ID_COUNT             GENMASK(11, 7)
> > > +#define ID_LIST_MASK_64                            GENMASK(51, 12)
> > > +#define ID_LIST_MASK_32                            GENMASK(31, 12)
> > > +#define MAX_IDS_64                         20
> > > +#define MAX_IDS_32                         10
> > > +
> > >  static int ffa_notification_bind_common(u16 dst_id, u64 bitmap,
> > >                                     u32 flags, bool is_bind)
> > >  {
> > > @@ -673,6 +680,69 @@ static int ffa_notification_get(u32 flags, struct ffa_notify_bitmaps *notify)
> > >     return 0;
> > >  }
> > >
> > > +static void __do_sched_recv_cb(u16 partition_id, u16 vcpu, bool is_per_vcpu)
> > > +{
> > > +   pr_err("Callback for partition 0x%x failed.\n", partition_id);
> > > +}
> > > +
> > > +static void ffa_notification_info_get(bool is_64b)
> > > +{
> > > +   int idx, list, max_ids, lists_cnt, ids_processed, ids_count[MAX_IDS_64];
> > > +   ffa_value_t ret;
> > > +   u64 id_list;
> > > +
> > > +   do {
> > > +           invoke_ffa_fn((ffa_value_t){
> > > +                     .a0 = FFA_FN_NATIVE(NOTIFICATION_INFO_GET),
> > > +                     }, &ret);
> > > +
> > > +           if (ret.a0 != FFA_FN_NATIVE(SUCCESS)) {
> >
> > The specification doesn't require a response using 64-bit SMCCC, it is
> > valid to respond using 32-bit SMCCC too.
> >
>
> Do you mean to say the we need to support 64b NOTIFICATION_INFO_GET
> returning 32b FFA_SUCCESS ? If so do we need to assume only 32bits are
> used in the result. As per the specification,
>
> "0xC4000061(FFA_FN64_SUCCESS): This function ID is used only if any result
>  register encodes a 64-bit parameter."
>
> My question is what that means allowing 32bit FFA_SUCCESS here?

My bad, this is the one place where a 64-bit FFA_SUCCESS is more or
less required. I'll fix my side of things.

Thanks,
Jens