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

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

[-- Attachment #1.1: Type: text/plain, Size: 1892 bytes --]

On 25/08/14 13:45, Gianluca Guida wrote:
> On Fri, Aug 22, 2014 at 10:34 AM, Andrew Cooper
> <andrew.cooper3@citrix.com <mailto:andrew.cooper3@citrix.com>> wrote:
>
>     I am concerned with the addition of a the vcpu specifics to
>     shadow_write_entries().  Most of the shadow code is already vcpu
>     centric
>     where it should be domain centric, and steps are being made to
>     alleviate
>     these problems.
>
>
> The historical reason the code is set up this way, if you are
> referring to the fact that every shadow operation is vcpu-specific
> while the interface to it is domain specific, is due to the design
> choice of leaving the door open to experiment with per-vcpu shadows.
> That always looked like a nice feature, I am not sure anybody ever
> implemented it. I would advocate -- for the sake of code consistency
> -- to keep the current shadow internal interfaces per-vcpu in upcoming
> patches, and change it when you propose your domain-centric patch,
> effectively killing this probably never-exploited opportunity.
>
> Honestly, haven't been following shadow code in a while, so probably
> consistency has already been lost, in which case you should feel free
> to ignore this comment.
>
> Gianluca

I appreciate that it was done with vcpus to experiment with per-vcpu
shadows, but it is fundamentally wrong (even in a per-vcpu shadows case)
for certain paths to arbitrarily use d->vcpu[0] when calling into the
shadow code.  At the very least, it adversely messes with the heuristics.

There are fundamentally two separate entry paths into the shadow code. 
First from the pagefault handler, which certainly is vcpu-centric, but
also from toolstack operations, which is very much domain centric.  A
per-vcpu shadow setup would need to use for_each_vcpu() under the hood,
as it certainly couldn't trust that the caller has handed an appropriate
vcpu.

~Andrew

[-- Attachment #1.2: Type: text/html, Size: 3203 bytes --]

[-- Attachment #2: Type: text/plain, Size: 126 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel