1. Patchew
  2. Xen
  3. View series

[PATCH v2] xen/arm: Move TEE mediators in a kconfig submenu

Bertrand Marquis posted 1 patch 9 months, 1 week ago
Patches applied successfully (tree, apply log)
git fetch https://gitlab.com/xen-project/patchew/xen tags/patchew/c55ce2891172a696e8a29f8c9bcc9dd33ebe9e50.1689931326.git.bertrand.marquis@arm.com
There is a newer version of this series
xen/arch/arm/Kconfig      |  7 -------
xen/arch/arm/tee/Kconfig  | 17 ++++++++++++++---
xen/arch/arm/tee/Makefile |  2 +-
3 files changed, 15 insertions(+), 11 deletions(-)
[PATCH v2] xen/arm: Move TEE mediators in a kconfig submenu
Posted by Bertrand Marquis 9 months, 1 week ago 
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.

Signed-off-by: Bertrand Marquis <bertrand.marquis@arm.com>
---
Changes in v2:
- only included tee subdirectory in makefile if CONFIG_TEE is selected
  (reverts to state before patch)
- remove help in hidden TEE config
- rebase on top of staging
---
 xen/arch/arm/Kconfig      |  7 -------
 xen/arch/arm/tee/Kconfig  | 17 ++++++++++++++---
 xen/arch/arm/tee/Makefile |  2 +-
 3 files changed, 15 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/tee/Kconfig b/xen/arch/arm/tee/Kconfig
index db3ea78faaaa..c5b0f88d7522 100644
--- a/xen/arch/arm/tee/Kconfig
+++ b/xen/arch/arm/tee/Kconfig
@@ -1,7 +1,14 @@
+menu "TEE mediators"
+	visible if UNSUPPORTED
+
+config TEE
+	bool
+	default n
+
 config OPTEE
-	bool "Enable OP-TEE mediator"
+	bool "Enable OP-TEE mediator (UNSUPPORTED)" if UNSUPPORTED
 	default n
-	depends on TEE
+	select TEE
 	help
 	  Enable the OP-TEE mediator. It allows guests to access
 	  OP-TEE running on your platform. This requires
@@ -12,10 +19,14 @@ config OPTEE
 config FFA
 	bool "Enable FF-A mediator support (UNSUPPORTED)" if UNSUPPORTED
 	default n
-	depends on ARM_64 && TEE
+	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
-- 
2.39.2 (Apple Git-143)
Re: [PATCH v2] xen/arm: Move TEE mediators in a kconfig submenu
Posted by Julien Grall 9 months, 1 week ago 
Hi Bertrand,

On 21/07/2023 10:23, 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.

Is this a left over?

> 
> Signed-off-by: Bertrand Marquis <bertrand.marquis@arm.com>
> ---
> Changes in v2:
> - only included tee subdirectory in makefile if CONFIG_TEE is selected
>    (reverts to state before patch)
> - remove help in hidden TEE config
> - rebase on top of staging
> ---
>   xen/arch/arm/Kconfig      |  7 -------
>   xen/arch/arm/tee/Kconfig  | 17 ++++++++++++++---
>   xen/arch/arm/tee/Makefile |  2 +-
>   3 files changed, 15 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/tee/Kconfig b/xen/arch/arm/tee/Kconfig
> index db3ea78faaaa..c5b0f88d7522 100644
> --- a/xen/arch/arm/tee/Kconfig
> +++ b/xen/arch/arm/tee/Kconfig
> @@ -1,7 +1,14 @@
> +menu "TEE mediators"
> +	visible if UNSUPPORTED
> +
> +config TEE
> +	bool
> +	default n
> +
>   config OPTEE
> -	bool "Enable OP-TEE mediator"
> +	bool "Enable OP-TEE mediator (UNSUPPORTED)" if UNSUPPORTED
>   	default n
> -	depends on TEE
> +	select TEE
>   	help
>   	  Enable the OP-TEE mediator. It allows guests to access
>   	  OP-TEE running on your platform. This requires
> @@ -12,10 +19,14 @@ config OPTEE
>   config FFA
>   	bool "Enable FF-A mediator support (UNSUPPORTED)" if UNSUPPORTED
>   	default n
> -	depends on ARM_64 && TEE
> +	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

Same here? The rest LGTM and I would be happy to do the changes on commit.

Cheers,

-- 
Julien Grall
Re: [PATCH v2] xen/arm: Move TEE mediators in a kconfig submenu
Posted by Bertrand Marquis 9 months, 1 week ago 
Hi,

> On 21 Jul 2023, at 12:00, Julien Grall <julien@xen.org> wrote:
> 
> Hi Bertrand,
> 
> On 21/07/2023 10:23, 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.
> 
> Is this a left over?

Oups yes, please remove.

> 
>> Signed-off-by: Bertrand Marquis <bertrand.marquis@arm.com>
>> ---
>> Changes in v2:
>> - only included tee subdirectory in makefile if CONFIG_TEE is selected
>>   (reverts to state before patch)
>> - remove help in hidden TEE config
>> - rebase on top of staging
>> ---
>>  xen/arch/arm/Kconfig      |  7 -------
>>  xen/arch/arm/tee/Kconfig  | 17 ++++++++++++++---
>>  xen/arch/arm/tee/Makefile |  2 +-
>>  3 files changed, 15 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/tee/Kconfig b/xen/arch/arm/tee/Kconfig
>> index db3ea78faaaa..c5b0f88d7522 100644
>> --- a/xen/arch/arm/tee/Kconfig
>> +++ b/xen/arch/arm/tee/Kconfig
>> @@ -1,7 +1,14 @@
>> +menu "TEE mediators"
>> + visible if UNSUPPORTED
>> +
>> +config TEE
>> + bool
>> + default n
>> +
>>  config OPTEE
>> - bool "Enable OP-TEE mediator"
>> + bool "Enable OP-TEE mediator (UNSUPPORTED)" if UNSUPPORTED
>>   default n
>> - depends on TEE
>> + select TEE
>>   help
>>     Enable the OP-TEE mediator. It allows guests to access
>>     OP-TEE running on your platform. This requires
>> @@ -12,10 +19,14 @@ config OPTEE
>>  config FFA
>>   bool "Enable FF-A mediator support (UNSUPPORTED)" if UNSUPPORTED
>>   default n
>> - depends on ARM_64 && TEE
>> + 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
> 
> Same here? The rest LGTM and I would be happy to do the changes on commit.

This one is not, I think it is more logic to keep that one like this so that if someone
creates a TEE not depending on this it could still put it here without compiling tee.o
for no reason.

Now there is still a discussion around visibility and if UNSUPPORTED that we need
to settle (even though current behaviour is right and as i expected, the effect of the
"visibility" is not what I expected).

Cheers
Bertrand

> 
> Cheers,
> 
> -- 
> Julien Grall
Re: [PATCH v2] xen/arm: Move TEE mediators in a kconfig submenu
Posted by Julien Grall 9 months, 1 week ago 
Hi,

On 21/07/2023 13:28, Bertrand Marquis wrote:
>> On 21 Jul 2023, at 12:00, Julien Grall <julien@xen.org> wrote:
>>
>> Hi Bertrand,
>>
>> On 21/07/2023 10:23, 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.
>>
>> Is this a left over?
> 
> Oups yes, please remove.
> 
>>
>>> Signed-off-by: Bertrand Marquis <bertrand.marquis@arm.com>
>>> ---
>>> Changes in v2:
>>> - only included tee subdirectory in makefile if CONFIG_TEE is selected
>>>    (reverts to state before patch)
>>> - remove help in hidden TEE config
>>> - rebase on top of staging
>>> ---
>>>   xen/arch/arm/Kconfig      |  7 -------
>>>   xen/arch/arm/tee/Kconfig  | 17 ++++++++++++++---
>>>   xen/arch/arm/tee/Makefile |  2 +-
>>>   3 files changed, 15 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/tee/Kconfig b/xen/arch/arm/tee/Kconfig
>>> index db3ea78faaaa..c5b0f88d7522 100644
>>> --- a/xen/arch/arm/tee/Kconfig
>>> +++ b/xen/arch/arm/tee/Kconfig
>>> @@ -1,7 +1,14 @@
>>> +menu "TEE mediators"
>>> + visible if UNSUPPORTED
>>> +
>>> +config TEE
>>> + bool
>>> + default n
>>> +
>>>   config OPTEE
>>> - bool "Enable OP-TEE mediator"
>>> + bool "Enable OP-TEE mediator (UNSUPPORTED)" if UNSUPPORTED
>>>    default n
>>> - depends on TEE
>>> + select TEE
>>>    help
>>>      Enable the OP-TEE mediator. It allows guests to access
>>>      OP-TEE running on your platform. This requires
>>> @@ -12,10 +19,14 @@ config OPTEE
>>>   config FFA
>>>    bool "Enable FF-A mediator support (UNSUPPORTED)" if UNSUPPORTED
>>>    default n
>>> - depends on ARM_64 && TEE
>>> + 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
>>
>> Same here? The rest LGTM and I would be happy to do the changes on commit.
> 
> This one is not, I think it is more logic to keep that one like this so that if someone
> creates a TEE not depending on this it could still put it here without compiling tee.o
> for no reason.
This seems pretty unlikely to me because the purpose of TEE is to 
provide a generic interface to the common code. At least, the developer 
would need to have a very good reason to diverge.

Also, right now, make will not recurse to the directory tee if 
CONFIG_TEE=n. So I would rather not have this change.

Cheers,

-- 
Julien Grall
Re: [PATCH v2] xen/arm: Move TEE mediators in a kconfig submenu
Posted by Bertrand Marquis 9 months, 1 week ago 
Hi Julien,

> On 21 Jul 2023, at 14:44, Julien Grall <julien@xen.org> wrote:
> 
> Hi,
> 
> On 21/07/2023 13:28, Bertrand Marquis wrote:
>>> On 21 Jul 2023, at 12:00, Julien Grall <julien@xen.org> wrote:
>>> 
>>> Hi Bertrand,
>>> 
>>> On 21/07/2023 10:23, 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.
>>> 
>>> Is this a left over?
>> Oups yes, please remove.
>>> 
>>>> Signed-off-by: Bertrand Marquis <bertrand.marquis@arm.com>
>>>> ---
>>>> Changes in v2:
>>>> - only included tee subdirectory in makefile if CONFIG_TEE is selected
>>>>   (reverts to state before patch)
>>>> - remove help in hidden TEE config
>>>> - rebase on top of staging
>>>> ---
>>>>  xen/arch/arm/Kconfig      |  7 -------
>>>>  xen/arch/arm/tee/Kconfig  | 17 ++++++++++++++---
>>>>  xen/arch/arm/tee/Makefile |  2 +-
>>>>  3 files changed, 15 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/tee/Kconfig b/xen/arch/arm/tee/Kconfig
>>>> index db3ea78faaaa..c5b0f88d7522 100644
>>>> --- a/xen/arch/arm/tee/Kconfig
>>>> +++ b/xen/arch/arm/tee/Kconfig
>>>> @@ -1,7 +1,14 @@
>>>> +menu "TEE mediators"
>>>> + visible if UNSUPPORTED
>>>> +
>>>> +config TEE
>>>> + bool
>>>> + default n
>>>> +
>>>>  config OPTEE
>>>> - bool "Enable OP-TEE mediator"
>>>> + bool "Enable OP-TEE mediator (UNSUPPORTED)" if UNSUPPORTED
>>>>   default n
>>>> - depends on TEE
>>>> + select TEE
>>>>   help
>>>>     Enable the OP-TEE mediator. It allows guests to access
>>>>     OP-TEE running on your platform. This requires
>>>> @@ -12,10 +19,14 @@ config OPTEE
>>>>  config FFA
>>>>   bool "Enable FF-A mediator support (UNSUPPORTED)" if UNSUPPORTED
>>>>   default n
>>>> - depends on ARM_64 && TEE
>>>> + 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
>>> 
>>> Same here? The rest LGTM and I would be happy to do the changes on commit.
>> This one is not, I think it is more logic to keep that one like this so that if someone
>> creates a TEE not depending on this it could still put it here without compiling tee.o
>> for no reason.
> This seems pretty unlikely to me because the purpose of TEE is to provide a generic interface to the common code. At least, the developer would need to have a very good reason to diverge.
> 
> Also, right now, make will not recurse to the directory tee if CONFIG_TEE=n. So I would rather not have this change.

Ok will fix in v3.

Cheers
Bertrand

> 
> Cheers,
> 
> -- 
> Julien Grall

Patchew 2.1-devel-0870f84

© 2016 - 2024 Red Hat, Inc.