Re: [XEN PATCH v7 07/20] xen/arm: ffa: add defines for framework direct request/response messages

[Jens Wiklander <jens.wiklander@xxxxxxxxxx>]

Hi Bertrand,

On Fri, Feb 24, 2023 at 10:39 AM Bertrand Marquis
<Bertrand.Marquis@xxxxxxx> wrote:
>
> Hi Jens,
>
> > On 22 Feb 2023, at 16:33, Jens Wiklander <jens.wiklander@xxxxxxxxxx> wrote:
> >
> > Adds defines for framework direct request/response messages.
> >
> > Signed-off-by: Jens Wiklander <jens.wiklander@xxxxxxxxxx>
> > ---
> > xen/arch/arm/tee/ffa.c | 9 +++++++++
> > 1 file changed, 9 insertions(+)
> >
> > diff --git a/xen/arch/arm/tee/ffa.c b/xen/arch/arm/tee/ffa.c
> > index f4562ed2defc..d04bac9cc47f 100644
> > --- a/xen/arch/arm/tee/ffa.c
> > +++ b/xen/arch/arm/tee/ffa.c
> > @@ -56,6 +56,15 @@
> > #define FFA_MY_VERSION          MAKE_FFA_VERSION(FFA_MY_VERSION_MAJOR, \
> >                                                  FFA_MY_VERSION_MINOR)
> >
> > +/* Framework direct request/response */
>
> In the previous patch you were more verbose in the comment which was nice.
> I would suggest here to use the same "format":
>
> Flags used for the MSG_SEND_DIRECT_REQ/RESP:
> BIT(31): Framework or partition message
> BIT(7-0): Message type for frameworks messages

OK, I'll update.

>
> > +#define FFA_MSG_FLAG_FRAMEWORK          BIT(31, U)
> > +#define FFA_MSG_TYPE_MASK               0xFFU;
>
> Maybe more coherent to name this FFA_MSG_FLAG_TYPE_MASK ?

This is a balancing act, in this case, I don't think that adding FLAG_
helps much.

>
> I am a bit unsure here because we could also keep it like that and just
> add _TYPE to other definitions after.
>
> What do you think ?

I think the defines are long enough as they are.

Cheers,
Jens

>
> > +#define FFA_MSG_PSCI                    0x0U
> > +#define FFA_MSG_SEND_VM_CREATED         0x4U
> > +#define FFA_MSG_RESP_VM_CREATED         0x5U
> > +#define FFA_MSG_SEND_VM_DESTROYED       0x6U
> > +#define FFA_MSG_RESP_VM_DESTROYED       0x7U
> > +
> > /*
> >  * Flags used for the FFA_PARTITION_INFO_GET return message:
> >  * BIT(0): Supports receipt of direct requests
> > --
> > 2.34.1
> >
>
> Cheers
> Bertrand
>