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

[Jan Beulich <jbeulich@xxxxxxxx>]

On 21.07.2023 14:27, Bertrand Marquis wrote:
> Hi Jan,
> 
>> On 21 Jul 2023, at 12:45, Jan Beulich <jbeulich@xxxxxxxx> wrote:
>>
>> On 21.07.2023 11:02, Bertrand Marquis wrote:
>>>> On 21 Jul 2023, at 10:52, Jan Beulich <jbeulich@xxxxxxxx> wrote:
>>>> On 21.07.2023 10:02, Bertrand Marquis wrote:
>>>>> --- a/xen/arch/arm/tee/Kconfig
>>>>> +++ b/xen/arch/arm/tee/Kconfig
>>>>> @@ -1,7 +1,17 @@
>>>>> +menu "TEE mediators"
>>>>> + visible if UNSUPPORTED
>>>>
>>>> With this ...
>>>>
>>>>> +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.
>>>>> +
>>>>> config OPTEE
>>>>> - bool "Enable OP-TEE mediator"
>>>>> + bool "Enable OP-TEE mediator (UNSUPPORTED)" if UNSUPPORTED
>>>>
>>>> ... you shouldn't need the "if" here, and ...
>>>>
>>>>> 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
>>>>> @@ -13,9 +23,13 @@ config FFA
>>>>> bool "Enable FF-A mediator support (UNSUPPORTED)" if UNSUPPORTED
>>>>
>>>> ... you could drop the one here. I think.
>>>
>>> visible if is only for the GUI/ncurse display but the if is required
>>> to make sure that a .config file cannot set CONFIG_TEE or
>>> CONFIG_FFA if. UNSUPPORTED is not selected.
>>
>> Is what you describe "depends on"? "if" controls merely prompt
>> visibility aiui.
> 
> So you think that having  CONFIG_FFA without CONFIG_UNSUPPORTED
> would be a valid configuration and the if is only here for the gui ?
> 
> I tested that with the following procedure:
> - use menuconfig, select UNSUPPORTED and FFA
> - edit .config and disable UNSUPPORTED but keep FFA
> - build
> - CONFIG_FFA is removed from .config
> 
> Now what puzzles me is that i did the same but removing the if UNSUPPORTED
> for TEE and FFA and i have exactly the same behaviour.
> 
> So it seems that "if UNSUPPORTED" and visibility all behave in the same way
> as depends on which i was not expecting.

Hmm, maybe that's a bug in our variant of kconfig (we didn't sync
for quite some time)?

> So what should i keep or remove here ?

My understanding so far was that "visibility" merely hides all prompts
underneath (but then I use the command line version of the tool, not
menuconfig), so it largely is shorthand for adding "if" to all enclosed
prompts. Therefore I think all the "if UNSUPPORTED" are redundant and
could be dropped. But then I'm also working from the understanding that
"depends on" would behave somewhat differently ...

Jan