Re: [PATCH 4/4] x86/PoD: Command line option to prohibit any PoD operations

[Andrew Cooper <andrew.cooper3@citrix.com>]

On 02/11/15 14:32, Ian Campbell wrote:
> On Mon, 2015-11-02 at 06:53 -0700, Jan Beulich wrote:
>>>>> On 30.10.15 at 19:33, <andrew.cooper3@citrix.com> wrote:
>>> --- a/xen/arch/x86/hvm/hvm.c
>>> +++ b/xen/arch/x86/hvm/hvm.c
>>> @@ -92,6 +92,9 @@ unsigned long __section(".bss.page_aligned")
>>>  static bool_t __initdata opt_hap_enabled = 1;
>>>  boolean_param("hap", opt_hap_enabled);
>>>  
>>> +bool_t opt_pod_enabled = 1;
>> __read_mostly?
>>
>>> --- a/xen/common/memory.c
>>> +++ b/xen/common/memory.c
>>> @@ -818,6 +818,10 @@ long do_memory_op(unsigned long cmd,
>>> XEN_GUEST_HANDLE_PARAM(void) arg)
>>>          if ( unlikely(start_extent >= reservation.nr_extents) )
>>>              return start_extent;
>>>  
>>> +        if ( unlikely(!opt_pod_enabled) &&
>>> +             (reservation.mem_flags & XENMEMF_populate_on_demand) )
>>> +            return start_extent;
>> A few lines down we can see that XENMEMF_populate_on_demand
>> gets honored only for XENMEM_populate_physmap. Perhaps you
>> shouldn't fail the other two which ignore the flag anyway?
> Setting an unexpected flag surely ought to be an error? Admittedly that
> particular ship may have sailed WRT this public ABI.
>
>>  And
>> perhaps you should also fold this into the existing check?
>>
>> Also I don't think this is going to build on ARM.

Oops - I forgot to check for that, but you are correct.

> Because opt_pod_enabled is on x86 only, I suppose.
>
> I wouldn't be averse to a "const bool_t opt_pod_enabled = 0" in some
> convenient place, seems better than the alternative of ifdefs around here.

I will see what I can do.

~Andrew