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

[Michal Orzel <michal.orzel@xxxxxxx>]

Hi Julien,

On 20/11/2023 14:40, Julien Grall wrote:
> 
> 
> On 20/11/2023 07:34, Michal Orzel wrote:
>> Hi Luca,
>>
>> On 14/11/2023 10:03, 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 <luca.fancellu@xxxxxxx>
>> Reviewed-by: Michal Orzel <michal.orzel@xxxxxxx>
>>
>> with one remark...
>>
>>> ---
>>> Changes from v4:
>>>   - fixed name in inclusion macro __ASM_* instead of __ARM_*, fixed
>>>     emacs local variable 'end:', style fix (Michal)
>>> Changes from v3:
>>>   - remove header in dom0less-build.c (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..81446cbd8bf3
>>> --- /dev/null
>>> +++ b/xen/arch/arm/include/asm/dom0less-build.h
>>> @@ -0,0 +1,20 @@
>>> +/* SPDX-License-Identifier: GPL-2.0-only */
>>> +
>>> +#ifndef __ASM_DOM0LESS_BUILD_H_
>>> +#define __ASM_DOM0LESS_BUILD_H_
>>> +
>>> +#include <asm/kernel.h>
>> No need for this include, especially if you are removing it in the 
>> subsequent patch.
> 
> Are you suggesting that this patch builds without including asm/kernel.h?
Yes.

> 
>> This could be done on commit (+ there is a conflcit between your series and 
>> Henry's mmu/mpu split)
> 
> While I am in general happy to handle some changes on commit, I don't
> want to solve conflict between series.
> 
> One will have to rebase on top of the others. I suggest that this is
> just rebased on top of Henry, purely because I will likely commit it
> today or tomorrow.
Sounds ok.

~Michal