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

[Oleksii Kurochko <oleksii.kurochko@xxxxxxxxx>]

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.

~ Oleksii