Hi Julien, > On 21 Jul 2023, at 10:52, Julien Grall <[email protected]> wrote: > > Hi Bertrand, > > On 21/07/2023 09:02, Bertrand Marquis wrote: >> Rework TEE mediators to put them under a submenu in Kconfig. >> The submenu is only visible if UNSUPPORTED is activated as all currently >> existing mediators are UNSUPPORTED. >> While there rework a bit the configuration so that OP-TEE and FF-A >> mediators are selecting the generic TEE interface instead of depending >> on it. >> Make the TEE option hidden as it is of no interest for anyone to select >> it without one of the mediators so having them select it instead should >> be enough. >> Rework makefile inclusion and selection so that generic TEE is included >> only when selected and include the tee Makefile all the time making the >> directory tee self contained. > The problem is now we will always recurse to the directory even if there is > nothing to build. I would rather prefer reducing the build time (even if here > it would be minimal) over "self-contained" directory.
Makes sense, I will restore the obj-$(CONFIG_TEE) += tee/ in the Makfile. > >> Signed-off-by: Bertrand Marquis <[email protected]> > > I wasn't able to apply this patch cleanly on the latest staging. I will rebase properly and send a v2. > >> --- >> xen/arch/arm/Kconfig | 7 ------- >> xen/arch/arm/Makefile | 2 +- >> xen/arch/arm/tee/Kconfig | 18 ++++++++++++++++-- >> xen/arch/arm/tee/Makefile | 2 +- >> 4 files changed, 18 insertions(+), 11 deletions(-) >> diff --git a/xen/arch/arm/Kconfig b/xen/arch/arm/Kconfig >> index 439cc94f3344..fd57a82dd284 100644 >> --- a/xen/arch/arm/Kconfig >> +++ b/xen/arch/arm/Kconfig >> @@ -175,13 +175,6 @@ config ARM64_BTI >> Branch Target Identification support. >> This feature is not supported in Xen. >> -config TEE >> - bool "Enable TEE mediators support (UNSUPPORTED)" if UNSUPPORTED >> - default n >> - help >> - This option enables generic TEE mediators support. It allows guests >> - to access real TEE via one of TEE mediators implemented in XEN. >> - >> source "arch/arm/tee/Kconfig" >> config STATIC_SHM >> diff --git a/xen/arch/arm/Makefile b/xen/arch/arm/Makefile >> index 7bf07e992046..d47d5c20aa73 100644 >> --- a/xen/arch/arm/Makefile >> +++ b/xen/arch/arm/Makefile >> @@ -5,7 +5,7 @@ obj-$(CONFIG_HAS_PCI) += pci/ >> ifneq ($(CONFIG_NO_PLAT),y) >> obj-y += platforms/ >> endif >> -obj-$(CONFIG_TEE) += tee/ >> +obj-y += tee/ >> obj-$(CONFIG_HAS_VPCI) += vpci.o >> obj-$(CONFIG_HAS_ALTERNATIVE) += alternative.o >> diff --git a/xen/arch/arm/tee/Kconfig b/xen/arch/arm/tee/Kconfig >> index 923f08ba8cb7..cecbf7e12b43 100644 >> --- a/xen/arch/arm/tee/Kconfig >> +++ b/xen/arch/arm/tee/Kconfig >> @@ -1,7 +1,17 @@ >> +menu "TEE mediators" >> + visible if UNSUPPORTED >> + >> +config TEE >> + bool >> + default n >> + help >> + This option enables generic TEE mediators support. It allows guests >> + to access real TEE via one of TEE mediators implemented in XEN. > > We don't typically add an 'help' section for non-select option. In fact, it > looks like menuconfig will not show the 'help'. Yes i kept that one but it can be removed. Will clean on v2. > >> + >> config OPTEE >> - bool "Enable OP-TEE mediator" >> + bool "Enable OP-TEE mediator (UNSUPPORTED)" if UNSUPPORTED > > Given this is under 'TEE mediators', do we still need the 'if UNSUPPORTED'? As explained to Jan, i used visible if which does not enforce the dependency, just a hint for the display. I thought that it was clearer to keep things like that to make it easier to have one of them supported in the future and i added the visible if just so that there was not an empty menu. I could also just use a if UNSUPPORTED ... endif to surround everything if that is clearer. Please tell me and i will push a v2. > >> default n >> - depends on TEE >> + select TEE > > I was sort of hoping we could remove 'select TEE'. But I understand why you > did it that way, you have one less selection to do. So I am Ok with that. The fact that OPTEE or FFA was hidden before TEE was selected was kind of weird i thought. An other solution is to keep the select and leave TEE visible but I do not really see the reason to select TEE without one of optee or ffa selected so i did it like that. Cheers Bertrand > >> help >> Enable the OP-TEE mediator. It allows guests to access >> OP-TEE running on your platform. This requires >> @@ -13,9 +23,13 @@ config FFA >> bool "Enable FF-A mediator support (UNSUPPORTED)" if UNSUPPORTED >> default n >> depends on ARM_64 >> + select TEE >> help >> This option enables a minimal FF-A mediator. The mediator is >> generic as it follows the FF-A specification [1], but it only >> implements a small subset of the specification. >> [1] https://developer.arm.com/documentation/den0077/latest >> + >> +endmenu >> + >> diff --git a/xen/arch/arm/tee/Makefile b/xen/arch/arm/tee/Makefile >> index 58a1015e40e0..1ef49a271fdb 100644 >> --- a/xen/arch/arm/tee/Makefile >> +++ b/xen/arch/arm/tee/Makefile >> @@ -1,3 +1,3 @@ >> obj-$(CONFIG_FFA) += ffa.o >> -obj-y += tee.o >> +obj-$(CONFIG_TEE) += tee.o >> obj-$(CONFIG_OPTEE) += optee.o > > Cheers, > > -- > Julien Grall
