Re: [XEN PATCH v8 09/22] xen/arm: ffa: add direct request support

[Jens Wiklander <jens.wiklander@xxxxxxxxxx>]

Hi Henry,

On Thu, Apr 13, 2023 at 1:16 PM Henry Wang <Henry.Wang@xxxxxxx> wrote:
>
> Hi Jens,
>
> > -----Original Message-----
> > Subject: [XEN PATCH v8 09/22] xen/arm: ffa: add direct request support
> >
> > Adds support for sending a FF-A direct request. Checks that the SP also
> > supports handling a 32-bit direct request. 64-bit direct requests are
> > not used by the mediator itself so there is not need to check for that.
> >
> > Signed-off-by: Jens Wiklander <jens.wiklander@xxxxxxxxxx>
> > ---
> >  xen/arch/arm/tee/ffa.c | 112
> > +++++++++++++++++++++++++++++++++++++++++
> >  1 file changed, 112 insertions(+)
> >
> > diff --git a/xen/arch/arm/tee/ffa.c b/xen/arch/arm/tee/ffa.c
> > index f129879c5b81..f2cce955d981 100644
> > --- a/xen/arch/arm/tee/ffa.c
> > +++ b/xen/arch/arm/tee/ffa.c
> > @@ -181,6 +181,56 @@ static bool ffa_get_version(uint32_t *vers)
> >      return true;
> >  }
> >
> > +static int32_t get_ffa_ret_code(const struct arm_smccc_1_2_regs *resp)
> > +{
> > +    switch ( resp->a0 )
> > +    {
> > +    case FFA_ERROR:
> > +        if ( resp->a2 )
> > +            return resp->a2;
> > +        else
> > +            return FFA_RET_NOT_SUPPORTED;
> > +    case FFA_SUCCESS_32:
> > +    case FFA_SUCCESS_64:
> > +        return FFA_RET_OK;
> > +    default:
> > +        return FFA_RET_NOT_SUPPORTED;
> > +    }
> > +}
> > +
> > +static int32_t ffa_simple_call(uint32_t fid, register_t a1, register_t a2,
> > +                               register_t a3, register_t a4)
> > +{
> > +    const struct arm_smccc_1_2_regs arg = {
> > +        .a0 = fid,
> > +        .a1 = a1,
> > +        .a2 = a2,
> > +        .a3 = a3,
> > +        .a4 = a4,
> > +    };
> > +    struct arm_smccc_1_2_regs resp;
> > +
> > +    arm_smccc_1_2_smc(&arg, &resp);
> > +
> > +    return get_ffa_ret_code(&resp);
> > +}
> > +
> > +static int32_t ffa_features(uint32_t id)
> > +{
> > +    return ffa_simple_call(FFA_FEATURES, id, 0, 0, 0);
> > +}
> > +
> > +static bool check_mandatory_feature(uint32_t id)
> > +{
> > +    int32_t ret = ffa_features(id);
> > +
> > +    if (ret)
>
> Coding style nit: You need spaces before and after "ret", i.e.
> if ( ret )
>
> With this fixed:
>
> Reviewed-by: Henry Wang <Henry.Wang@xxxxxxx>

I'll fix it.

Thanks,
Jens

>
> Kind regards,
> Henry