Re: [PATCH v1 2/3] xen/dom0less: refactor architecture-specific DomU construction

[Oleksii Kurochko <oleksii.kurochko@xxxxxxxxx>]

On 5/14/25 9:25 AM, Julien Grall wrote:

> Hi
>       Oleksii,
>       
>       
>       On 13/05/2025 15:29, Oleksii Kurochko wrote:
>       
> > Refactor construct_domU() to improve
> >         architecture separation and reduce
> >         
> >         reliance on ARM-specific logic in common code:
> >         
> >         - Drop set_domain_type() from generic code. This function is
> >         specific
> >         
> >            to ARM and serves no purpose on other architectures like
> >         RISC-V,
> >         
> >            which lack the arch.type field in kernel_info.
> >         
> >       
>       
>       So you will only ever boot one type of domain on RISC-V? IOW, no
>       32-bit
>       
>       or else?
>       
>     
The bitness of the guest and host should match. So, an RV32 host should run
RV32 guests, and an RV64 host should run RV64 guests and so on.
(I'm not really sure that on RISC-V it is possible to run with RV64 host
an RV32 guest, but need to double-check)
Therefore, my plan for RISC-V is to have the following:
  #ifdef CONFIG_RISCV_64
  #define is_32bit_domain(d) (0)
  #define is_64bit_domain(d) (1)
  #else
  #define is_32bit_domain(d) (1)
  #define is_64bit_domain(d) (0)
  #endif

With this definition, there's no need to use (d)->arch.type for RISC-V.
>       
> > - Introduce arch_construct_domU() to
> >         encapsulate architecture-specific
> >         
> >            DomU construction steps.
> >         
> >         - Implement arch_construct_domU() for ARM. This includes:
> >         
> >            - Setting the domain type for CONFIG_ARM64.
> >         
> >            - Handling static memory allocation if xen,static-mem is
> >         present in
> >         
> >              the device tree.
> >         
> >            - Processing static shared memory.
> >         
> >         - Move call of make_resv_memory_node() to Arm's
> >         make_arch_nodes() as
> >         
> >            this call is specific to CONFIG_STATIC_SHM which is ARM
> >         specific,
> >         
> >            at least, now.
> >         
> >       
>       
>       This looks shortsighted. If there is a plan to use
>       CONFIG_STATIC_SHM on RISC-V (I don't see why not today), then
>       
>       I think the code should stick in common/ even if it is not fully
>       usable
>       
>       yet (that's the whole point of have CONFIG_* options).
>       
>     
We don't use this feature in the downstream repo, but I can imagine some cases where it could be useful. So, for now, its
use is purely theoretical and it is a reason why I agreed with Michal and returned back these changes to Arm.

~ Oleksii