RE: [XEN PATCH v8 05/22] xen/arm: ffa: add flags for FFA_PARTITION_INFO_GET

[Henry Wang <Henry.Wang@xxxxxxx>]

Hi Jens,

> -----Original Message-----
> Subject: [XEN PATCH v8 05/22] xen/arm: ffa: add flags for
> FFA_PARTITION_INFO_GET
> 
> Defines flags used for the function FFA_PARTITION_INFO_GET.

Nit: Similarly as my comment for patch #4, I would suggest that in
commit message you can mention the documentation number and
the chapter of FFA_PARTITION_INFO_GET. Something like:
"According to DEN0077A version 1.1 REL0, section 13.8, defines
flags used for the function FFA_PARTITION_INFO_GET"

> 
> Signed-off-by: Jens Wiklander <jens.wiklander@xxxxxxxxxx>
> ---
>  xen/arch/arm/tee/ffa.c | 34 ++++++++++++++++++++++++++++++++++
>  1 file changed, 34 insertions(+)
> 
> diff --git a/xen/arch/arm/tee/ffa.c b/xen/arch/arm/tee/ffa.c
> index ba0942e76993..72e7d0575de5 100644
> --- a/xen/arch/arm/tee/ffa.c
> +++ b/xen/arch/arm/tee/ffa.c
> @@ -57,6 +57,40 @@
>  #define FFA_MY_VERSION
> MAKE_FFA_VERSION(FFA_MY_VERSION_MAJOR, \
>                                                   FFA_MY_VERSION_MINOR)
> 
> +/*
> + * Flags to determine partition properties in FFA_PARTITION_INFO_GET
> return
> + * message:
> + * BIT(0): Supports receipt of direct requests
> + * BIT(1): Can send direct requests
> + * BIT(2): Can send and receive indirect messages
> + * BIT(3): Supports receipt of notifications
> + * BIT(4-5): Partition ID is a PE endpoint ID
> + * BIT(6): Partition must be informed about each VM that is created by
> + *         the Hypervisor
> + * BIT(7): Partition must be informed about each VM that is destroyed by
> + *         the Hypervisor
> + * BIT(8): Partition runs in the AArch64 execution state else AArch32
> + *         execution state
> + */
> +#define FFA_PART_PROP_DIRECT_REQ_RECV   BIT(0, U)
> +#define FFA_PART_PROP_DIRECT_REQ_SEND   BIT(1, U)
> +#define FFA_PART_PROP_INDIRECT_MSGS     BIT(2, U)
> +#define FFA_PART_PROP_RECV_NOTIF        BIT(3, U)
> +#define FFA_PART_PROP_IS_MASK           (3U << 4)

I am a bit confused here, here (3U<<4) is "IS_MASK" but...

> +#define FFA_PART_PROP_IS_PE_ID          (0U << 4)
> +#define FFA_PART_PROP_IS_SEPID_INDEP    (1U << 4)
> +#define FFA_PART_PROP_IS_SEPID_DEP      (2U << 4)
> +#define FFA_PART_PROP_IS_AUX_ID         (3U << 4)

...here the same value is used for "IS_AUX_ID". According to
the spec that I referred to, bit[5:4] has the following encoding:
b'11: Partition ID is an auxiliary ID. Hence I guess the above
"IS_MASK" should be removed?

I confirm the values of other fields are consistent with the spec.

Kind regards,
Henry