Re: [PATCH v2 3/5] arm/dom0less: put dom0less feature code in a separate module

[Stefano Stabellini <sstabellini@xxxxxxxxxx>]

On Fri, 29 Sep 2023, Luca Fancellu wrote:
> > On 29 Sep 2023, at 01:33, Stefano Stabellini <sstabellini@xxxxxxxxxx> wrote:
> > 
> > On Wed, 27 Sep 2023, Luca Fancellu wrote:
> >> Currently the dom0less feature code is mostly inside domain_build.c
> >> and setup.c, it is a feature that may not be useful to everyone so
> >> put the code in a different compilation module in order to make it
> >> easier to disable the feature in the future.
> >> 
> >> Move gic_interrupt_t in domain_build.h to use it with the function
> >> declaration, move its comment above the declaration.
> >> 
> >> The following functions are now visible externally from domain_build
> >> because they are used also from the dom0less-build module:
> >> - get_allocation_size
> >> - set_interrupt
> >> - domain_fdt_begin_node
> >> - make_memory_node
> >> - make_resv_memory_node
> >> - make_hypervisor_node
> >> - make_psci_node
> >> - make_cpus_node
> >> - make_timer_node
> >> - handle_device_interrupts
> >> - construct_domain
> >> - process_shm
> >> 
> >> The functions allocate_static_memory and assign_static_memory_11
> >> are now externally visible, so put their declarations into
> >> domain_build.h and move the #else and stub definition in the header
> >> as well.
> >> 
> >> Move is_dom0less_mode from setup.c to dom0less-build.c and make it
> >> externally visible.
> >> 
> >> Where spotted, fix code style issues.
> >> 
> >> No functional change is intended.
> >> 
> >> Signed-off-by: Luca Fancellu <luca.fancellu@xxxxxxx>
> > 
> > This is great! A couple of questions.
> > 
> > Why was allocate_static_memory not moved to dom0less-build.c ?
> 
> My aim is to decouple the features, so in patch 4 we move (just once as 
> Julien suggested)
> the static memory code on a module on its own, because we can have a guest 
> booted with
> dom0less feature but having it with static memory is optional.

OK


> > Would it make sense to also move construct_dom0 to dom0less-build.c
> > given the similarities with construct_domU? I am not sure about this.
> > 
> 
> We can’t do that because the final goal of this serie is to have a Kconfig 
> disabling dom0less,
> so in that case we will end up removing from the compilation also 
> construct_dom0.

OK. Probably we can't do much better than this.


One more question on the code movement, and I would also like Julien and
Bertrand to express their opinions on this.

Given that code movement is painful from a git history perspective, and
given that we have to move dom0less code to xen/common anyway to make
it available to RISC-V and also x86, could we do it in one shot here?

I am not asking to refactor the code to make it buildable on RISC-V or
X86. I am only asking to move the code to xen/common/dom0less (or
xen/common/hyperlaunch or xen/common/domain-builder as Danien and
Christopher called it) the same way you are doing it as part of this
series. It shouldn't require additional work on your side I hope.