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

[Jan Beulich <jbeulich@xxxxxxxx>]

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.

Jan