[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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


  • To: Luca Fancellu <luca.fancellu@xxxxxxx>, <xen-devel@xxxxxxxxxxxxxxxxxxxx>
  • From: Michal Orzel <michal.orzel@xxxxxxx>
  • Date: Mon, 13 Nov 2023 12:58:54 +0100
  • Arc-authentication-results: i=1; mx.microsoft.com 1; spf=pass (sender ip is 165.204.84.17) smtp.rcpttodomain=arm.com smtp.mailfrom=amd.com; dmarc=pass (p=quarantine sp=quarantine pct=100) action=none header.from=amd.com; dkim=none (message not signed); arc=none (0)
  • Arc-message-signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-AntiSpam-MessageData-ChunkCount:X-MS-Exchange-AntiSpam-MessageData-0:X-MS-Exchange-AntiSpam-MessageData-1; bh=DFcdHmvglp8nFlt2nI7NLPot0eHOX51TIHzSDCXJ/90=; b=ZQ9aP4WX8Ef/YcFewkmTJdfKlzHoADG8q0/qwEfMvgUnahyHowdovCixfqsZ7H99+zaA0plKXXL2J4myjMnXdKvymXgFjVLck/UBDqNxtlArAmg4GG795fQH1jmJkABz9yabkwtbX+0S5/s6T58iJ8m4TQT20h4kXLShgzsTtvLofXV9nuRWrH5oZhlBoeDiIalcv5XOxtxFwVNKXCv8pvuvuXcHky/Zd4CdG76Q97PYJq1ikN2iBpAB7qetWb+hs78z3PnD18BgGb7JV03/mdoXZiyjAZhPDGOCqPMW0pg0fYZcdsxFWSRyLXK96DaRtENS3WtsRqD4qoXJjOVHeA==
  • Arc-seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=NYk8yfOjdlssEUXmDAoUERcmGFWdYl33UE4EPwfymPvdW1UUyuaG0yuPMWjtVJ80LJSOOPSvazI/FcMHivQ+dmz+NvAqg+jDJz+6LNy3mDHiFXCxf91hR8dw7ey2GnqTaGWjHQv0BX5qLT4UKzl+5t98pp5+5bl968cLovWuRzrlHzRdNEOReBHWgdv3ZtoizDs9g8eO5iEFpMNoiKVWJNBSc70aczBE9t3o/sShsFNMPgYolYHMILa7oVtlAtirFMMUIw6JhmelSmjE/p8HqzigUXJnCOFdhgLKSCoLufNmvQedtXGU/Al0Rj6pEuQsL4HdMWLATa30922oZcUHBg==
  • Cc: <rahul.singh@xxxxxxx>, Stefano Stabellini <sstabellini@xxxxxxxxxx>, "Julien Grall" <julien@xxxxxxx>, Bertrand Marquis <bertrand.marquis@xxxxxxx>, Volodymyr Babchuk <Volodymyr_Babchuk@xxxxxxxx>
  • Delivery-date: Mon, 13 Nov 2023 11:59:11 +0000
  • List-id: Xen developer discussion <xen-devel.lists.xenproject.org>

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 <luca.fancellu@xxxxxxx>
> ---
> 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




 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.