| Message ID | a22e5375df48d16cb4c0b3d80dde8593522b3ddd.1742824138.git.bertrand.marquis@arm.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | FF-A VM to VM support | expand |
Hi Bertrand, On Mon, Mar 24, 2025 at 2:53 PM Bertrand Marquis <bertrand.marquis@arm.com> 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@arm.com> > --- > 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 | 31 ++++++++++++++++++++++++++++++ > 3 files changed, 49 insertions(+) Reviewed-by: Jens Wiklander <jens.wiklander@linaro.org> Cheers, Jens > > diff --git a/docs/misc/xen-command-line.pandoc b/docs/misc/xen-command-line.pandoc > index 89db6e83be66..0c2ff542a138 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 supported 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` of `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..066b5ba40f9d 100644 > --- a/xen/arch/arm/tee/tee.c > +++ b/xen/arch/arm/tee/tee.c > @@ -19,12 +19,17 @@ > #include <xen/errno.h> > #include <xen/init.h> > #include <xen/types.h> > +#include <xen/param.h> > > #include <asm/tee/tee.h> > > 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. > @@ -81,14 +86,40 @@ static int __init tee_init(void) > { > const struct tee_mediator_desc *desc; > > + if ( strcmp(opt_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 ( strcmp(opt_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 ( strcmp(opt_mediator, "") ) > + { > + panic("TEE mediator %s from command line probe failed\n", > + opt_mediator); > + return -EFAULT; > + } > + } > + > + if ( strcmp(opt_mediator, "") ) > + { > + panic("TEE Mediator %s from command line not supported\n", > + opt_mediator); > + return -EINVAL; > } > > return 0; > -- > 2.47.1 >
Hi Bertrand, On 24/03/2025 13:52, Bertrand Marquis 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@arm.com> > --- > 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 | 31 ++++++++++++++++++++++++++++++ > 3 files changed, 49 insertions(+) > > diff --git a/docs/misc/xen-command-line.pandoc b/docs/misc/xen-command-line.pandoc > index 89db6e83be66..0c2ff542a138 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 supported TEEs supported by Xen and use typo: I think there is one too many "supported". Maybe keep the one after TEEs? > +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` of `ffa` if the corresponding mediators typo: s/of/or/ > +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..066b5ba40f9d 100644 > --- a/xen/arch/arm/tee/tee.c > +++ b/xen/arch/arm/tee/tee.c > @@ -19,12 +19,17 @@ > #include <xen/errno.h> > #include <xen/init.h> > #include <xen/types.h> > +#include <xen/param.h> Coding style: The includes are order. So this wants to be moved before xen/types.h > > #include <asm/tee/tee.h> > > 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. > @@ -81,14 +86,40 @@ static int __init tee_init(void) > { > const struct tee_mediator_desc *desc; > > + if ( strcmp(opt_mediator, "") ) You are using 'strcmp(opt_mediator, "")' several time in the code. This should never changed, so can we introduce a boolean within the function to indicate whether opt_mediator is set? > + 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 ( strcmp(opt_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 ( strcmp(opt_mediator, "") ) > + { > + panic("TEE mediator %s from command line probe failed\n", > + opt_mediator); > + return -EFAULT; > + } > + } > + > + if ( strcmp(opt_mediator, "") ) > + { > + panic("TEE Mediator %s from command line not supported\n", > + opt_mediator); > + return -EINVAL; > } > > return 0; Cheers,
Hi Julien, > On 26 Mar 2025, at 23:49, Julien Grall <julien@xen.org> wrote: > > Hi Bertrand, > > On 24/03/2025 13:52, Bertrand Marquis 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@arm.com> >> --- >> 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 | 31 ++++++++++++++++++++++++++++++ >> 3 files changed, 49 insertions(+) >> diff --git a/docs/misc/xen-command-line.pandoc b/docs/misc/xen-command-line.pandoc >> index 89db6e83be66..0c2ff542a138 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 supported TEEs supported by Xen and use > > > typo: I think there is one too many "supported". Maybe keep the one after TEEs? ack > >> +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` of `ffa` if the corresponding mediators > > typo: s/of/or/ ack > >> +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..066b5ba40f9d 100644 >> --- a/xen/arch/arm/tee/tee.c >> +++ b/xen/arch/arm/tee/tee.c >> @@ -19,12 +19,17 @@ >> #include <xen/errno.h> >> #include <xen/init.h> >> #include <xen/types.h> >> +#include <xen/param.h> > > Coding style: The includes are order. So this wants to be moved before xen/types.h ack > >> #include <asm/tee/tee.h> >> 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. >> @@ -81,14 +86,40 @@ static int __init tee_init(void) >> { >> const struct tee_mediator_desc *desc; >> + if ( strcmp(opt_mediator, "") ) > > You are using 'strcmp(opt_mediator, "")' several time in the code. This should never changed, so can we introduce a boolean within the function to indicate whether opt_mediator is set? Very good point, I will do that. > >> + 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 ( strcmp(opt_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 ( strcmp(opt_mediator, "") ) >> + { >> + panic("TEE mediator %s from command line probe failed\n", >> + opt_mediator); >> + return -EFAULT; >> + } >> + } >> + >> + if ( strcmp(opt_mediator, "") ) >> + { >> + panic("TEE Mediator %s from command line not supported\n", >> + opt_mediator); >> + return -EINVAL; >> } >> return 0; Thanks a lot for the review. Cheers Bertrand > > Cheers, > > -- > Julien Grall
diff --git a/docs/misc/xen-command-line.pandoc b/docs/misc/xen-command-line.pandoc index 89db6e83be66..0c2ff542a138 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 supported 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` of `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..066b5ba40f9d 100644 --- a/xen/arch/arm/tee/tee.c +++ b/xen/arch/arm/tee/tee.c @@ -19,12 +19,17 @@ #include <xen/errno.h> #include <xen/init.h> #include <xen/types.h> +#include <xen/param.h> #include <asm/tee/tee.h> 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. @@ -81,14 +86,40 @@ static int __init tee_init(void) { const struct tee_mediator_desc *desc; + if ( strcmp(opt_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 ( strcmp(opt_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 ( strcmp(opt_mediator, "") ) + { + panic("TEE mediator %s from command line probe failed\n", + opt_mediator); + return -EFAULT; + } + } + + if ( strcmp(opt_mediator, "") ) + { + panic("TEE Mediator %s from command line not supported\n", + opt_mediator); + return -EINVAL; } return 0;
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@arm.com> --- 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 | 31 ++++++++++++++++++++++++++++++ 3 files changed, 49 insertions(+)