Re: [PATCH v4 04/11] xen: introduce domain-layout.h with common domain_use_host_layout()

[Luca Fancellu <Luca.Fancellu@xxxxxxx>]

Hi Oleksii,

> On 28 Apr 2026, at 15:33, Oleksii Kurochko <oleksii.kurochko@xxxxxxxxx> wrote:
> 
> domain_use_host_layout() is not architecture-specific and may be needed
> on x86 [1]. Replace the ARM-specific macro in asm/domain.h with a common
> static inline in a new dedicated header, xen/domain-layout.h.
> 
> xen/domain.h would be the natural home, but placing it there would
> require including xen/paging.h (for paging_mode_translate()) and
> xen/sched.h (for is_hardware_domain()), which would introduce circular
> dependencies. A separate header that callers opt into avoids this.
> 
> Adjust the implementation to take paging_mode_translate() into account
> so it works correctly for all architectures, including x86. Some extra
> details about implementation [2] and [3].
> 
> [1] 
> https://lore.kernel.org/xen-devel/alpine.DEB.2.22.394.2602161038120.359097@ubuntu-linux-20-04-desktop/
> [2] 
> https://lore.kernel.org/xen-devel/alpine.DEB.2.22.394.2602271742400.3148344@ubuntu-linux-20-04-desktop/
> [3] 
> https://lore.kernel.org/xen-devel/alpine.DEB.2.22.394.2602271750190.3148344@ubuntu-linux-20-04-desktop/
> 
> Suggested-by: Stefano Stabellini <sstabellini@xxxxxxxxxx>
> Signed-off-by: Oleksii Kurochko <oleksii.kurochko@xxxxxxxxx>
> ---
> Changes in v4:
> - Update the comment above domain_use_host_layout().
> ---
> Changes in v3:
> - Make argument of domain_use_host_layout() const.
> - Create a separate header to avoid circular heder dependecy and making
>   domain_use_host_layour() as static inline.
> - Rework domain_use_host_layout() to be protected by paging_mode_translate().
> - Update the commit message.
> ---
> Changes in v2:
> - Drop ifdef around defintion of domain_use_host_layout() as it
>   was suggested generic version. It could be returned back when
>   the real use case for it will appear.
> - Add Suggested-by: and update the commit message.
> - Make domain_use_host_layout() function instead of macros to
>   avoid ciclular header dependecies. Look at more details in
>   the commit message.
> ---
> ---
> xen/arch/arm/domain_build.c           |  1 +
> xen/arch/arm/include/asm/domain.h     | 14 --------------
> xen/arch/arm/vgic-v3.c                |  1 +
> xen/common/device-tree/domain-build.c |  1 +
> xen/include/xen/domain-layout.h       | 27 +++++++++++++++++++++++++++
> 5 files changed, 30 insertions(+), 14 deletions(-)
> create mode 100644 xen/include/xen/domain-layout.h
> 
> diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
> index ad665cd3c045..1efddc60ef0a 100644
> --- a/xen/arch/arm/domain_build.c
> +++ b/xen/arch/arm/domain_build.c
> @@ -2,6 +2,7 @@
> #include <xen/init.h>
> #include <xen/bootinfo.h>
> #include <xen/compile.h>
> +#include <xen/domain-layout.h>
> #include <xen/dom0less-build.h>
> #include <xen/fdt-domain-build.h>
> #include <xen/fdt-kernel.h>
> diff --git a/xen/arch/arm/include/asm/domain.h 
> b/xen/arch/arm/include/asm/domain.h
> index b24f02d269be..46a5cdc0c800 100644
> --- a/xen/arch/arm/include/asm/domain.h
> +++ b/xen/arch/arm/include/asm/domain.h
> @@ -18,20 +18,6 @@ struct hvm_domain
>     uint64_t              params[HVM_NR_PARAMS];
> };
> 
> -/*
> - * Is the domain using the host memory layout?
> - *
> - * Direct-mapped domain will always have the RAM mapped with GFN == MFN.
> - * To avoid any trouble finding space, it is easier to force using the
> - * host memory layout.
> - *
> - * The hardware domain will use the host layout regardless of
> - * direct-mapped because some OS may rely on a specific address ranges
> - * for the devices.
> - */
> -#define domain_use_host_layout(d) (is_domain_direct_mapped(d) || \
> -                                   is_hardware_domain(d))
> -
> struct vtimer {
>     struct vcpu *v;
>     int irq;
> diff --git a/xen/arch/arm/vgic-v3.c b/xen/arch/arm/vgic-v3.c
> index 77aab5c3c293..77517c303061 100644
> --- a/xen/arch/arm/vgic-v3.c
> +++ b/xen/arch/arm/vgic-v3.c
> @@ -10,6 +10,7 @@
>  */
> 
> #include <xen/bitops.h>
> +#include <xen/domain-layout.h>
> #include <xen/init.h>
> #include <xen/irq.h>
> #include <xen/lib.h>
> diff --git a/xen/common/device-tree/domain-build.c 
> b/xen/common/device-tree/domain-build.c
> index c51520ebadf9..6949203dacdc 100644
> --- a/xen/common/device-tree/domain-build.c
> +++ b/xen/common/device-tree/domain-build.c
> @@ -1,6 +1,7 @@
> /* SPDX-License-Identifier: GPL-2.0-only */
> 
> #include <xen/bootinfo.h>
> +#include <xen/domain-layout.h>
> #include <xen/fdt-domain-build.h>
> #include <xen/init.h>
> #include <xen/lib.h>
> diff --git a/xen/include/xen/domain-layout.h b/xen/include/xen/domain-layout.h
> new file mode 100644
> index 000000000000..0532a27b44ce
> --- /dev/null
> +++ b/xen/include/xen/domain-layout.h

New files should have SPDX tag

> @@ -0,0 +1,27 @@
> +#ifndef __XEN_DOMAIN_LAYOUT_H__
> +#define __XEN_DOMAIN_LAYOUT_H__

I think this include guard doesn’t satisfy the coding style:
https://xenbits.xen.org/gitweb/?p=xen.git;a=blob;f=CODING_STYLE;h=aae5a47ac20345978b3c465b9d85f1d5f6774731;hb=refs/heads/staging#l167

Apart from this, the rest looks ok to me, after fixing the above I will leave 
my R-by

Cheers,
Luca