Re: [PATCH RFC v2 1/4] x86/mm: Shadow and p2m changes for PV mem_access

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

On 26/08/2014 23:27, Aravindh Puthiyaparambil (aravindp) wrote:
>>> Just to be certain as to where we stand:
>>>
>>> 1. The "page table RW bit flipping" solution is not viable because pausing
>>> the domain synchronously takes too long for many vcpus domains. Plus
>> there is
>>> the added issue of vcpu vs domain heuristics. This is the case even after
>>> solving the page boundary and multiple page copy issues.
>>>
>>> 2. The "CR0.WP with interrupts disabled" solution is not viable because of
>>> NMIs. Or did I misunderstand?
>> For this second option, NMIs are a concern. Whether that makes it
>> not viable I'm not certain. We really need to weigh benefits and risks
>> here, and from a project wide perspective I'm currently viewing the
> From what I can tell, Andrew does think that this route is a viable option and I will defer to you and him about this. If there is agreement that this approach is acceptable, I will send out another version of the patches implementing it.

I currently see the CR0.WP=0 with interrupts disabled as the most viable
solution going.  That is not to say that there isn't a better solution,
but I can't currently spot a plausible alternative.

I do not see the NMI path as problematic with respect to WP=0 or
interrupts disabled.  The logic is very deliberately self contained and
minimal, as it specifically can interrupt other critical regions with
interrupts otherwise disabled in Xen.

>
>> PV mem-access feature as a niche thing, the more that I'm unaware
>> of really wide spread use if HVM mem-access capabilities. I.e. the
>> most I can currently see happening is for it to go in clearly marked
>> experimental, provided that no code path used outside of that
>> feature suffers in any way (functionality and performance). But of
>> course I'm open to be convinced otherwise, or overruled by a
>> majority of other maintainers.
> I agree that PV/HVM mem_access feature is indeed niche, however it is a value add feature for Xen when compared to other hypervisors. It is attracting users who are interested in developing security and guest inspection/introspection products. And yes, I agree that the code added for mem_access should not adversely affect other areas of the project. I would hope this feature area is given encouragement to grow by the community. Just my two bits...

I would also agree that PV mem_access is niche, but that is not to say
that it isn't valuable as part of the Xen ecosystem.

If it can be implemented with zero (or more realistically, minimal)
overhead to the rest of Xen (especially in the case where it is not
used), and explicitly documented as "by using PV mem_access, you as the
admin/user are accepting that there is an overhead", then I think that
it is ok.

After all, a working feature (if it doesn't adversely interfere with Xen
when disabled) is better than half a feature which doesn't function,
even if it is somewhat slow.  I have to admit that this is rare where
there is only one apparently feasible solution, which isn't necessarily
fantastic overall. 

Having said all of this, I am merely a community member who is voicing
an opinion.  It is ultimately Jan and Tim you have to convince to get
any patches accepted.

~Andrew