|
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [PATCH v1 2/3] xen/dom0less: refactor architecture-specific DomU construction
On 14/05/2025 12:06, Julien Grall wrote: > Hi, > > On 14/05/2025 10:57, Oleksii Kurochko wrote: >> >> 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. > > I strongly disagree with this statement. If a feature is not > architecture specific (like static shared memory), then the code ought > to be in common code so it can be re-used by others. But the code is not common. There are still 900 lines in arch spec dir. > > Also, AFAIK, the bulk of the static shared memory code is in common. So > it would be rather easy to add support for RISC-V if we wanted to. > > Given the code is already in common, it is rather silly to move the code IMO it should not be made common in the first place. I don't like exposing callers with the big chunk of code sitting in the arch specific directory. My opinion is that they should be exposed at once when the support for other arches appear. Today we ended up with caller protected with #ifdef and the majority of code in arch dir. > back to Arm for then moving back to common potentially in a few weeks time. What about static memory code? With the recent Oleksii code movement, the inline helpers like allocate_static_memory() became unreachable when the feature is disabled and the main purpose for helpers was to avoid ifdefery. > > So: > > Nacked-by: Julien Grall <jgrall@xxxxxxxxxx> Ok. No more discussion from my side then. ~Michal
|
 |
Lists.xenproject.org is hosted with RackSpace, monitoring our |