> On 13 Nov 2023, at 11:58, Michal Orzel <[email protected]> wrote: > > Hi Luca, > > Apart from pending question on static event channels code movement, a few > NITs.
Related to that, it seems to me that this part can be handled by a separate patch/serie from this one, moving code from domain_build.c to a new module, so I’m wondering, is this point a blocker for this serie? > > 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. Sure, makes sense > >> +#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 I’ll fix > >> + >> +/* >> + * 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. Mmm... I think I’ve copied this part from smccc.h and I didn’t realised there is a typo (or at least I think it is) because looking into CODING_STYLE, there is no ‘b’ in ‘End:’, I’ll fix it. > > The comments apply to other files as well. > > Apart from that, the movement LGTM. > > ~Michal >
