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

[Julien Grall <julien@xxxxxxx>]

Hi Jan,

On 24/06/2022 10:43, Jan Beulich wrote:
> 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.
Fair point. I decided to move this change in a prerequisite name the new 
Kconfig ARCH_MAP_DOMAIN_PAGE.
Cheers,

--
Julien Grall