Re: PML (Page Modification Logging) design for Xen

[Kai Huang <kai.huang@linux.intel.com>]

On 03/11/2015 11:53 PM, George Dunlap wrote:
> On Wed, Mar 11, 2015 at 11:11 AM, Andrew Cooper
> <andrew.cooper3@citrix.com> wrote:
>> On 11/03/15 10:59, George Dunlap wrote:
>>> On Tue, Feb 17, 2015 at 10:19 AM, Jan Beulich <JBeulich@suse.com> wrote:
>>>>>>> On 12.02.15 at 03:39, <kai.huang@linux.intel.com> wrote:
>>>>> On 02/11/2015 07:52 PM, Andrew Cooper wrote:
>>>>>> On 11/02/15 08:28, Kai Huang wrote:
>>>>>>> Design
>>>>>>> ======
>>>>>>>
>>>>>>> - PML feature is used globally
>>>>>>>
>>>>>>> A new Xen boot parameter, say 'opt_enable_pml', will be introduced to
>>>>>>> control PML feature detection, and PML feature will only be detected
>>>>>>> if opt_enable_pml = 1. Once PML feature is detected, it will be used
>>>>>>> for dirty logging for all domains globally. Currently we don't support
>>>>>>> to use PML on basis of per-domain as it will require additional
>>>>>>> control from XL tool.
>>>>>> Rather than adding in a new top level command line option for an ept
>>>>>> subfeature, it would be preferable to add an "ept=" option which has
>>>>>> "pml" as a sub boolean.
>>>>> Which is good to me, if Jan agrees.
>>>>>
>>>>> Jan, which do you prefer here?
>>>> A single "ept=" option as Andrew suggested.
>>> Sorry to be coming late to this party -- what's the logic behind
>>> having this enabled with "ept="? You're not changing anything about
>>> how EPT itself works; you're adding a secondary feature which happens
>>> to depend on ept.  Is there another hypervisor command-line option you
>>> had in mind that works this way?
>> iommu=
> Every option in iommu actually changes something about the way the
> IOMMU actually works.  Analogous options for ept might be enabling /
> disabling, setting a maximum entry size (1G, 2M, 4k), enabling the
> no-execute bit, &c.
>
> AFAICT PMT is a completely separate functionality.

Indeed, it doesn't impact functionality of existing EPT mechanism, 
though it depends on EPT mechanism to work (by checking D-bit updating 
from 0 to 1).

> Enabling it as an
> option behind ept would be like saying you should enable BTS
> (branch-trace-store) behind an option called "mmu=", since the
> addresses in the BTS go through the MMU.
>
> That's my $0.02 anyway...
To me a single opt_pml_enabled bool type parameter is OK, but I will 
keep using "ept=pml" as Jan/Andrew/Tim agreed, unless any of them oppose it.

>
>>> It might also be nice to be able to enable or disable this feature
>>> with a sysctl call; but that's just a nice-to-have.
>> This feature should either be used or not.  It is impractical to
>> enable/disable at runtime.
>>
>> However, it absolutely wants a knob for tweaking.  The likely
>> consequence of a bug in the implementation is VM memory corruption on
>> migrate, and you can get away with missing a large amount of a domains
>> memory before it blows up noticeably.
> Those paragraphs sound to me like they say the opposite things.
>
> And in any case, it's being enabled and disabled for particular
> domains when they enable or disable logdirty mode, right?  It
> shouldn't be hard at all to just fallback to the non-PML case if it's
> been disabled.
>
> Handling the case of enabling or disabling PML on domains that are
> already in logdirty mode is, I agree, probably more trouble than it's
> worth.  We can just document it to say that it will only have an
> effect on domains that start logdirty in the future.
Currently I only plan to support PML on boot parameter, but I can 
certainly add sysctl call to enable/disable PML dynamically if you guys 
think it's necessary in the future, which should be a separate 
nice-to-have feature and won't impact existing PML functionality.

Does this sound good to all of you?

Thanks,
-Kai
>
>   -George