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....@citrix.com>
Thanks.
