Re: [PATCH v5 1/6] xen/arm: Create tee command line parameter

[Jens Wiklander <jens.wiklander@xxxxxxxxxx>]

Hi Bertrand,

On Wed, Apr 16, 2025 at 9:40 AM Bertrand Marquis
<bertrand.marquis@xxxxxxx> wrote:
>
> Add a new command line parameter "tee=" to be used to explicitly select
> what tee mediator is to be used by Xen and fail if it does not exist
> or the probe function for it failed.
>
> Without specifying which tee is to be used, Xen will use the first one
> for which the probe function succeeds which depends on the order of the
> mediator list which depends on the compiler.
> Using the command line argument, it is now possible to explicit request
> a specific TEE mediator and panic on boot if it is not available.
>
> Signed-off-by: Bertrand Marquis <bertrand.marquis@xxxxxxx>
> ---
> Changes in v5:
> - Typo fix and rewording in command line doc (Julien)
> - fix include order in tee.c (Julien)
> - use a local bool instead of retesting the string each time in tee_init
>   (Julien)
> Changes in v4:
> - None
> Changes in v3:
> - Properly classify tee as arm specific (Jan)
> Changes in v2:
> - Patch introduced to add a command line selection of the TEE
> ---
>  docs/misc/xen-command-line.pandoc  | 14 +++++++++++++
>  xen/arch/arm/include/asm/tee/tee.h |  4 ++++
>  xen/arch/arm/tee/tee.c             | 32 ++++++++++++++++++++++++++++++
>  3 files changed, 50 insertions(+)

Looks good.
Reviewed-by: Jens Wiklander <jens.wiklander@xxxxxxxxxx>

Cheers,
Jens

>
> diff --git a/docs/misc/xen-command-line.pandoc 
> b/docs/misc/xen-command-line.pandoc
> index 89db6e83be66..472de1911363 100644
> --- a/docs/misc/xen-command-line.pandoc
> +++ b/docs/misc/xen-command-line.pandoc
> @@ -2651,6 +2651,20 @@ Specify the per-cpu trace buffer size in pages.
>
>  Flag to enable TSC deadline as the APIC timer mode.
>
> +### tee (arm)
> +> `= <string>`
> +
> +Specify the TEE mediator to be probed and use.
> +
> +The default behaviour is to probe all TEEs supported by Xen and use
> +the first one successfully probed. When this parameter is passed, Xen will
> +probe only the TEE mediator passed as argument and boot will fail if this
> +mediator is not properly probed or if the requested TEE is not supported by
> +Xen.
> +
> +This parameter can be set to `optee` or `ffa` if the corresponding mediators
> +are compiled in.
> +
>  ### tevt_mask
>  > `= <integer>`
>
> diff --git a/xen/arch/arm/include/asm/tee/tee.h 
> b/xen/arch/arm/include/asm/tee/tee.h
> index 0169fd746bcd..15d664e28dce 100644
> --- a/xen/arch/arm/include/asm/tee/tee.h
> +++ b/xen/arch/arm/include/asm/tee/tee.h
> @@ -55,6 +55,9 @@ struct tee_mediator_desc {
>      /* Printable name of the TEE. */
>      const char *name;
>
> +    /* Command line name of the TEE (to be used with tee= cmdline option) */
> +    const char *cmdline_name;
> +
>      /* Mediator callbacks as described above. */
>      const struct tee_mediator_ops *ops;
>
> @@ -77,6 +80,7 @@ void tee_free_domain_ctx(struct domain *d);
>  static const struct tee_mediator_desc __tee_desc_##_name __used     \
>  __section(".teemediator.info") = {                                  \
>      .name = _namestr,                                               \
> +    .cmdline_name = #_name,                                         \
>      .ops = _ops,                                                    \
>      .tee_type = _type                                               \
>  }
> diff --git a/xen/arch/arm/tee/tee.c b/xen/arch/arm/tee/tee.c
> index 3f65e45a7892..8501443c8e57 100644
> --- a/xen/arch/arm/tee/tee.c
> +++ b/xen/arch/arm/tee/tee.c
> @@ -18,6 +18,7 @@
>
>  #include <xen/errno.h>
>  #include <xen/init.h>
> +#include <xen/param.h>
>  #include <xen/types.h>
>
>  #include <asm/tee/tee.h>
> @@ -25,6 +26,10 @@
>  extern const struct tee_mediator_desc _steemediator[], _eteemediator[];
>  static const struct tee_mediator_desc __read_mostly *cur_mediator;
>
> +/* Select the TEE mediator using a name on command line. */
> +static char __initdata opt_mediator[16] = "";
> +string_param("tee", opt_mediator);
> +
>  /*
>   * TODO: Add function to alter Dom0 DTB, so we can properly describe
>   * present TEE.
> @@ -80,15 +85,42 @@ uint16_t tee_get_type(void)
>  static int __init tee_init(void)
>  {
>      const struct tee_mediator_desc *desc;
> +    bool select_mediator = strcmp(opt_mediator, "");
> +
> +    if ( select_mediator )
> +        printk(XENLOG_INFO "TEE Mediator %s selected from command line\n",
> +               opt_mediator);
>
> +    /*
> +     * When a specific TEE is selected using the 'tee=' command line
> +     * argument, we panic if the probe fails or if the requested TEE is not
> +     * supported.
> +     */
>      for ( desc = _steemediator; desc != _eteemediator; desc++ )
>      {
> +        if ( select_mediator &&
> +             strncmp(opt_mediator, desc->cmdline_name, sizeof(opt_mediator)) 
> )
> +            continue;
> +
>          if ( desc->ops->probe() )
>          {
>              printk(XENLOG_INFO "Using TEE mediator for %s\n", desc->name);
>              cur_mediator = desc;
>              return 0;
>          }
> +        else if ( select_mediator )
> +        {
> +            panic("TEE mediator %s from command line probe failed\n",
> +                  opt_mediator);
> +            return -EFAULT;
> +        }
> +    }
> +
> +    if ( select_mediator )
> +    {
> +        panic("TEE Mediator %s from command line not supported\n",
> +              opt_mediator);
> +        return -EINVAL;
>      }
>
>      return 0;
> --
> 2.47.1
>