diff mbox series

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

Message ID 904055064f4b4e4514acf1e64751672eca045c05.1677079672.git.jens.wiklander@linaro.org (mailing list archive)
State Superseded
Headers show
Series Xen FF-A mediator | expand

Commit Message

Jens Wiklander Feb. 22, 2023, 3:33 p.m. UTC
Adds defines for framework direct request/response messages.

Signed-off-by: Jens Wiklander <jens.wiklander@linaro.org>
---
 xen/arch/arm/tee/ffa.c | 9 +++++++++
 1 file changed, 9 insertions(+)

Comments

Bertrand Marquis Feb. 24, 2023, 9:38 a.m. UTC | #1
Hi Jens,

> On 22 Feb 2023, at 16:33, Jens Wiklander <jens.wiklander@linaro.org> wrote:
> 
> Adds defines for framework direct request/response messages.
> 
> Signed-off-by: Jens Wiklander <jens.wiklander@linaro.org>
> ---
> 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

> +#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 ?

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 ?

> +#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
Jens Wiklander March 3, 2023, 7:01 a.m. UTC | #2
Hi Bertrand,

On Fri, Feb 24, 2023 at 10:39 AM Bertrand Marquis
<Bertrand.Marquis@arm.com> wrote:
>
> Hi Jens,
>
> > On 22 Feb 2023, at 16:33, Jens Wiklander <jens.wiklander@linaro.org> wrote:
> >
> > Adds defines for framework direct request/response messages.
> >
> > Signed-off-by: Jens Wiklander <jens.wiklander@linaro.org>
> > ---
> > 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
>
Bertrand Marquis March 3, 2023, 8:14 a.m. UTC | #3
HI Jens,

> On 3 Mar 2023, at 08:01, Jens Wiklander <jens.wiklander@linaro.org> wrote:
> 
> Hi Bertrand,
> 
> On Fri, Feb 24, 2023 at 10:39 AM Bertrand Marquis
> <Bertrand.Marquis@arm.com> wrote:
>> 
>> Hi Jens,
>> 
>>> On 22 Feb 2023, at 16:33, Jens Wiklander <jens.wiklander@linaro.org> wrote:
>>> 
>>> Adds defines for framework direct request/response messages.
>>> 
>>> Signed-off-by: Jens Wiklander <jens.wiklander@linaro.org>
>>> ---
>>> 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
diff mbox series

Patch

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 */
+#define FFA_MSG_FLAG_FRAMEWORK          BIT(31, U)
+#define FFA_MSG_TYPE_MASK               0xFFU;
+#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