Re: [PATCH v3 2/8] xen/common: dom0less: make some parts of Arm's CONFIG_DOM0LESS common

[Stefano Stabellini <sstabellini@xxxxxxxxxx>]

On Mon, 5 May 2025, Oleksii Kurochko wrote:
> On 5/2/25 7:55 PM, Stefano Stabellini wrote:
> 
> On Fri, 2 May 2025, Oleksii Kurochko wrote:
> Move some parts of Arm's Dom0Less code to be reused by other architectures.
> At the moment, RISC-V is going to reuse these parts.
> 
> Move dom0less-build.h from the Arm-specific directory to asm-generic
> as these header is expected to be the same across acrhictectures with
> some updates: add the following declaration of construct_domU(),
> and arch_create_domUs() as there are some parts which are still
> architecture-specific.
> 
> Introduce HAS_DOM0LESS to provide ability to enable generic Dom0less
> code for an architecture.
> 
> Relocate the CONFIG_DOM0LESS configuration to the common with adding
> "depends on HAS_DOM0LESS" to not break builds for architectures which
> don't support CONFIG_DOM0LESS config, especically it would be useful
> to not provide stubs for  construct_domU(), arch_create_domUs()
> in case of *-randconfig which may set CONFIG_DOM0LESS=y.
> 
> Move is_dom0less_mode() function to the common code, as it depends on
> boot modules that are already part of the common code.
> 
> Move create_domUs() function to the common code with some updates:
> - Add arch_create_domUs() to cover parsing of arch-specific features,
>   for example, SVE (Scalar Vector Extension ) exists only in Arm.
> 
> Signed-off-by: Oleksii Kurochko <oleksii.kurochko@xxxxxxxxx>
> ---
> It was suggested to change dom0less to predefined domus or similar
> (https://lore.kernel.org/xen-devel/cd2a3644-c9c6-4e77-9491-2988703906c0@xxxxxxxxx/T/#m1d5e81e5f1faca98a3c51efe4f35af25010edbf0):
> 
> I decided to go with dom0less name for now as it will require a lot of places 
> to change,
> including CI's test, and IMO we could do in a separate patch.
> If it is necessry to do now, I'll be happy to do that in next version of the 
> current
> patch series.
> I think it is fine to use dom0less for now, it will make the code easier
> to review and it is not necessary to change the name at this point.
> 
> The patch looks good to me, except for a couple of minor suggestions I
> have below. I could make them on commit. With those:
> 
> Reviewed-by: Stefano Stabellini <sstabellini@xxxxxxxxxx>
> 
> During the randconfig testing the following issue occurs:
>   
> common/device-tree/dom0less-build.c: In function 'create_domUs':
> common/device-tree/dom0less-build.c:156:42: error: implicit declaration of 
> function 'gnttab_dom0_frames'; did you mean 'gnttab_map_frame'? 
> [-Werror=implicit-function-declaration]
>   156 |                 d_cfg.max_grant_frames = gnttab_dom0_frames();
>       |                                          ^~~~~~~~~~~~~~~~~~
>       |                                          gnttab_map_frame
> common/device-tree/dom0less-build.c:156:42: error: nested extern declaration 
> of 'gnttab_dom0_frames' [-Werror=nested-externs]
> 
> I fixed that by the following #ifdef-ing:
> ...
>         d_cfg.max_grant_frames = -1;
> ...
> 
>         if ( dt_property_read_u32(node, "capabilities", &val) )
>         {
> ...
> 
> #ifdef CONFIG_GRANT_TABLE
>                 d_cfg.max_grant_frames = gnttab_dom0_frames();
> #endif
>                 d_cfg.max_evtchn_port = -1;
>                 flags |= CDF_hardware;
>                 iommu = true;
>             }
> 
> Do you agree with such fix?
> 
> If the CONFIG_GRANT_TABLE=n then the init value (d_cfg.max_grant_frames = 
> -1;) will be used.

Yes, OK