Re: [PATCH v2 09/12] x86: make mem-paging configuarable and default it to off for being unsupported

[Roger Pau Monné <roger.pau@xxxxxxxxxx>]

On Fri, Apr 30, 2021 at 04:16:24PM +0200, Jan Beulich wrote:
> On 30.04.2021 11:55, Roger Pau Monné wrote:
> > On Mon, Apr 12, 2021 at 04:12:41PM +0200, Jan Beulich wrote:
> >> --- a/xen/arch/x86/x86_64/compat/mm.c
> >> +++ b/xen/arch/x86/x86_64/compat/mm.c
> >> @@ -155,8 +155,10 @@ int compat_arch_memory_op(unsigned long
> >>      case XENMEM_get_sharing_shared_pages:
> >>          return mem_sharing_get_nr_shared_mfns();
> >>  
> >> +#ifdef CONFIG_MEM_PAGING
> >>      case XENMEM_paging_op:
> >>          return mem_paging_memop(guest_handle_cast(arg, 
> >> xen_mem_paging_op_t));
> >> +#endif
> >>  
> >>  #ifdef CONFIG_MEM_SHARING
> >>      case XENMEM_sharing_op:
> >> --- a/xen/arch/x86/x86_64/mm.c
> >> +++ b/xen/arch/x86/x86_64/mm.c
> >> @@ -994,8 +994,10 @@ long subarch_memory_op(unsigned long cmd
> >>      case XENMEM_get_sharing_shared_pages:
> >>          return mem_sharing_get_nr_shared_mfns();
> >>  
> >> +#ifdef CONFIG_MEM_PAGING
> >>      case XENMEM_paging_op:
> >>          return mem_paging_memop(guest_handle_cast(arg, 
> >> xen_mem_paging_op_t));
> >> +#endif
> > 
> > I would create a dummy handler when !CONFIG_MEM_PAGING in
> > asm-x86/mem_paging.h.
> 
> I was simply following the neighboring mem-sharing approach,
> which you've stripped here, but which is still visible in the
> xen/arch/x86/x86_64/compat/mm.c change above. I think the two
> are helpful to be similar in such aspects.
> 
> >> --- a/xen/include/asm-x86/p2m.h
> >> +++ b/xen/include/asm-x86/p2m.h
> >> @@ -136,11 +136,16 @@ typedef unsigned int p2m_query_t;
> >>  #define P2M_PAGEABLE_TYPES (p2m_to_mask(p2m_ram_rw) \
> >>                              | p2m_to_mask(p2m_ram_logdirty) )
> >>  
> >> +#ifdef CONFIG_MEM_PAGING
> >>  #define P2M_PAGING_TYPES (p2m_to_mask(p2m_ram_paging_out)        \
> >>                            | p2m_to_mask(p2m_ram_paged)           \
> >>                            | p2m_to_mask(p2m_ram_paging_in))
> >>  
> >>  #define P2M_PAGED_TYPES (p2m_to_mask(p2m_ram_paged))
> >> +#else
> >> +#define P2M_PAGING_TYPES 0
> >> +#define P2M_PAGED_TYPES 0
> >> +#endif
> > 
> > Since you don't guard the p2m related paged types in p2m_type_t is it
> > worth having diverging P2M_{PAGING/PAGED}_TYPES?
> > 
> > I guess it might be required in order to force the compiler to DCE
> > without having to add yet more CONFIG_MEM_PAGING guards?
> 
> Indeed, this is the reason.
> 
> > I don't really have a strong opinion on whether the code should be
> > removed, IMO it's best to start by making it off by default at build
> > time and remove it in a later release?
> 
> Matches my way of thinking. I also wouldn't want to be the one to
> delete code completely out of the blue.

Acked-by: Roger Pau Monné <roger.pau@xxxxxxxxxx>

Thanks.