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

[Alejandro Vallejo <agarciav@amd.com>]

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.

I can move it elsewhere, but it's unfortunate to separate two so closely
related options.

>
>> --- 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.

>
>> --- a/xen/include/xsm/dummy.h
>> +++ b/xen/include/xsm/dummy.h
>> @@ -423,7 +423,7 @@ static XSM_INLINE int cf_check xsm_deassign_device(
>>  
>>  #endif /* HAS_PASSTHROUGH && HAS_PCI */
>>  
>> -#if defined(CONFIG_HAS_PASSTHROUGH) && defined(CONFIG_HAS_DEVICE_TREE)
>> +#if defined(CONFIG_HAS_PASSTHROUGH) && defined(CONFIG_HAS_DEVICE_TREE_DISCOVERY)
>>  static XSM_INLINE int cf_check xsm_assign_dtdevice(
>>      XSM_DEFAULT_ARG struct domain *d, const char *dtpath)
>>  {
>> @@ -438,7 +438,7 @@ static XSM_INLINE int cf_check xsm_deassign_dtdevice(
>>      return xsm_default_action(action, current->domain, d);
>>  }
>>  
>> -#endif /* HAS_PASSTHROUGH && HAS_DEVICE_TREE */
>> +#endif /* HAS_PASSTHROUGH && HAS_DEVICE_TREE_DISCOVERY */
>
> Here I'm similarly unsure: Pass-through again isn't a platform thing, is it?

This has to do strictly with passthrough of devices discovered via DT.

>
>> @@ -789,7 +789,7 @@ int xsm_multiboot_policy_init(
>>      struct boot_info *bi, void **policy_buffer, size_t *policy_size);
>>  #endif
>>  
>> -#ifdef CONFIG_HAS_DEVICE_TREE
>> +#ifdef CONFIG_HAS_DEVICE_TREE_DISCOVERY
>>  /*
>>   * Initialize XSM
>>   *
>> @@ -839,7 +839,7 @@ static inline int xsm_multiboot_init(struct boot_info *bi)
>>  }
>>  #endif
>>  
>> -#ifdef CONFIG_HAS_DEVICE_TREE
>> +#ifdef CONFIG_HAS_DEVICE_TREE_DISCOVERY
>>  static inline int xsm_dt_init(void)
>>  {
>>      return 0;
>> @@ -849,7 +849,7 @@ static inline bool has_xsm_magic(paddr_t start)
>>  {
>>      return false;
>>  }
>> -#endif /* CONFIG_HAS_DEVICE_TREE */
>> +#endif /* CONFIG_HAS_DEVICE_TREE_DISCOVERY */
>
> The situation is yet less clear to me here

XSM segregates multibooting and DT-booting, this refers strictly to the latter.

By DT-booting I mean platforms where the DT is given by the platform rather
than the user as a module.

Cheers,
Alejandro