Hi Luca, Apart from pending question on static event channels code movement, a few NITs.
On 13/11/2023 10:08, 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 > - allocate_bank_memory > > 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. > > The function allocate_bank_memory is used only by dom0less code > at the moment, but it's been decided to leave it in domain_build.c > in case that in the future the dom0 code can use it. > > Where spotted, fix code style issues. > > No functional change is intended. > > Signed-off-by: Luca Fancellu <[email protected]> > --- > Changes from v3: > - remove header in dom0less-build.h (Michal) > Changes from v2: > - move allocate_bank_memory back in domain_build.c, remove header > from dom0less-build.c. > --- > xen/arch/arm/Makefile | 1 + > xen/arch/arm/dom0less-build.c | 1018 +++++++++++++++++ > xen/arch/arm/domain_build.c | 1265 +++------------------ > xen/arch/arm/include/asm/dom0less-build.h | 20 + > xen/arch/arm/include/asm/domain_build.h | 60 + > xen/arch/arm/include/asm/setup.h | 1 - > xen/arch/arm/setup.c | 33 +- > 7 files changed, 1240 insertions(+), 1158 deletions(-) > create mode 100644 xen/arch/arm/dom0less-build.c > create mode 100644 xen/arch/arm/include/asm/dom0less-build.h > [...] > diff --git a/xen/arch/arm/include/asm/dom0less-build.h > b/xen/arch/arm/include/asm/dom0less-build.h > new file mode 100644 > index 000000000000..022ae61ead7c > --- /dev/null > +++ b/xen/arch/arm/include/asm/dom0less-build.h > @@ -0,0 +1,20 @@ > +/* SPDX-License-Identifier: GPL-2.0-only */ > + > +#ifndef __ARM_DOM0LESS_BUILD_H_ The majority of header files have a guard starting with __ASM so I would stick to the scheme. > +#define __ARM_DOM0LESS_BUILD_H_ > + > +#include <asm/kernel.h> > + > +void create_domUs(void); > +bool is_dom0less_mode(void); > + > +#endif /* __ARM_DOM0LESS_BUILD_H_ */ 1 space between #endif and comment > + > +/* > + * Local variables: > + * mode: C > + * c-file-style: "BSD" > + * c-basic-offset: 4 > + * indent-tabs-mode: nil > + * End:b What does 'b' means here? Only 2 files have that. The comments apply to other files as well. Apart from that, the movement LGTM. ~Michal
