Re: [PATCH v5 08/10] xen: Split HAS_DEVICE_TREE in two

[Alejandro Vallejo <alejandro.garciavallejo@amd.com>]

On Thu Jul 3, 2025 at 7:55 AM CEST, Jan Beulich wrote:
> On 02.07.2025 17:28, Alejandro Vallejo wrote:
>> On Wed Jul 2, 2025 at 3:30 PM CEST, Jan Beulich wrote:
>>> On 01.07.2025 12:57, Alejandro Vallejo wrote:
>>>> @@ -85,7 +86,11 @@ config HAS_ALTERNATIVE
>>>>  config HAS_COMPAT
>>>>  	bool
>>>>  
>>>> -config HAS_DEVICE_TREE
>>>> +config HAS_DEVICE_TREE_DISCOVERY
>>>> +	bool
>>>> +	select DEVICE_TREE_PARSE
>>>> +
>>>> +config DEVICE_TREE_PARSE
>>>>  	bool
>>>>  	select LIBFDT
>>>>  
>>>
>>> We're in the middle of various ([almost] alphabetically sorted) HAS_* here.
>>> Thus DEVICE_TREE_PARSE wants to move elsewhere. Or we want to move back to
>>> naming it HAS_DEVICE_TREE_PARSE, but I think there was a reason why we didn't
>>> want it like that? Just that I don't recall what that reason was ...
>> 
>> AIUI, HAS_X are options selected by your architecture. Things that tell you
>> whether X is supported in your arch or not. In this case, HAS_DEVICE_TREE_PARSE
>> would be supported everywhere, while DEVICE_TREE_PARSE is not an arch property,
>> but an option selected by DOM0LESS_BOOT (which appears in menuconfig) and
>> HAS_DEVICE_TREE_DISCOVERY.
>
> It's on the edge here, I agree. Yet we have other cases where one HAS_* selects
> another HAS_*, and I think we're in the process of gaining more.

HAS_* selecting another HAS_* makes sense to me, but that's not what would be
going on here. On x86:

    HAS_DOM0LESS => DOM0LESS_BOOT[y] => HAS_DEVICE_TREE_PARSE

thus leading to HAS_DEVICE_TREE_PARSE being indirectly selectable via
DOM0LESS_BOOT, which is hardly a HAS_* relationship.

>
>> I can move it elsewhere, but it's unfortunate to separate two so closely
>> related options.
>
> Imo there's only one of two options - move it or rename it.

I'll just move it elsewhere then.

>
>>>> --- a/xen/common/sched/Kconfig
>>>> +++ b/xen/common/sched/Kconfig
>>>> @@ -67,7 +67,7 @@ endmenu
>>>>  
>>>>  config BOOT_TIME_CPUPOOLS
>>>>  	bool "Create cpupools at boot time"
>>>> -	depends on HAS_DEVICE_TREE
>>>> +	depends on HAS_DEVICE_TREE_DISCOVERY
>>>
>>> Is this correct? CPU pool creation isn't driven by DT discovery, I thought,
>>> but is a software-only thing much like dom0less?
>> 
>> In principle it would be possible. But I haven't tested the configuration, so
>> I'd rather err on the side of caution and not enable features prematurely.
>> 
>> In time, this should become DOM0LESS_BOOT || HAS_DEVICE_TREE_DISCOVERY.
>
> DEVICE_TREE_PARSE, that is?

Yes :)

Cheers,
Alejandro