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

[Jan Beulich <jbeulich@xxxxxxxx>]

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.

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

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

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

Wait, no. You discover devices via DT, but you don't "discover" what domain
to pass them through. For the latter, DT is again only a vehicle.

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

Yet the platform as such hardly even knows of XSM (or in fact any of the 
software
that's going to run there).

I think we need DT maintainer input here.

Jan