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

[Bertrand Marquis <Bertrand.Marquis@xxxxxxx>]

HI Jens,

> On 3 Mar 2023, at 08:01, Jens Wiklander <jens.wiklander@xxxxxxxxxx> wrote:
> 
> 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.

Agree you can remove flag.

> 
>> 
>> 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.

Right.

Cheers
Bertrand

> 
> 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