Re: [PATCH 6/7] xen/arm: mm: Move domain_{,un}map_* helpers in a separate file

[Jan Beulich <jbeulich@xxxxxxxx>]

On 24.06.2022 11:11, Julien Grall wrote:
> From: Julien Grall <jgrall@xxxxxxxxxx>
> 
> The file xen/arch/mm.c has been growing quite a lot. It now contains
> various independent part of the MM subsytem.
> 
> One of them is the helpers to map/unmap a page when CONFIG_DOMAIN_PAGE
> (only used by arm32). Move them in a new file xen/arch/arm/domain_page.c.
> 
> In order to be able to use CONFIG_DOMAIN_PAGE in the Makefile, a new
> Kconfig option is introduced that is selected by x86 and arm32.
> 
> Signed-off-by: Julien Grall <jgrall@xxxxxxxxxx>

In principle
Acked-by: Jan Beulich <jbeulich@xxxxxxxx>

But ...

> --- a/xen/arch/x86/Kconfig
> +++ b/xen/arch/x86/Kconfig
> @@ -10,6 +10,7 @@ config X86
>       select ALTERNATIVE_CALL
>       select ARCH_SUPPORTS_INT128
>       select CORE_PARKING
> +     select DOMAIN_PAGE
>       select HAS_ALTERNATIVE
>       select HAS_COMPAT
>       select HAS_CPUFREQ
> diff --git a/xen/arch/x86/include/asm/config.h 
> b/xen/arch/x86/include/asm/config.h
> index 07bcd158314b..fbc4bb3416bd 100644
> --- a/xen/arch/x86/include/asm/config.h
> +++ b/xen/arch/x86/include/asm/config.h
> @@ -22,7 +22,6 @@
>  #define CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS 1
>  #define CONFIG_DISCONTIGMEM 1
>  #define CONFIG_NUMA_EMU 1
> -#define CONFIG_DOMAIN_PAGE 1

... while I realize it has been named this way, I wonder whether ...

> --- a/xen/common/Kconfig
> +++ b/xen/common/Kconfig
> @@ -11,6 +11,9 @@ config COMPAT
>  config CORE_PARKING
>       bool
>  
> +config DOMAIN_PAGE
> +     bool

... this isn't a good opportunity to make the name match what it is
about - MAP_DOMAIN_PAGE. See e.g. {clear,copy}_domain_page() which
aren't under this guard, and domain pages in general is a concept we
can't get away without in the first place.

Jan