Re: [PATCH 04/20] x86/shadow: Only apply shadow heuristics when in guest context

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

On 19/02/15 13:10, Tim Deegan wrote:
> At 17:16 +0000 on 12 Feb (1423757763), Andrew Cooper wrote:
>> It is incorrect to be applying these heuristics because of toolstack actions.
> It might be incorrect if we did, but we don't. :P
>
> The guess_wrmap() heuristics should only be called on v == current()
> because they use the shadow linear map, but this patch actually
> _relaxes_ that constraint so that they could run on other vcpus of the
> same guest (harmlessly).
>
> The second heuristic uses a stored smfn of a PTE, and uses the correct
> callback for the shadow page's type, so is safe to call from any vcpu
> so long as the domain's shadow lock is held.  The check you add here
> is needed _only_ because you are changing from using 'v' to using
> 'curr' for the test.
>
> The changes you've made in multi.c are again only needed because you
> are s/v/curr/ in the same patch, and again if anything are relaxing
> the checks.  Luckily they don't cover any mode-specific operations, so
> AFAICT the code is still safe after all these changes, and at least no
> worse than existing paths that use vcpu[0].
>
> So this patch is safe, but it's not a bugfix, and I think the
> changeset description needs to be rewritten.  What you're actually
> doing is removing uses of the vcpu parameter in these functions,
> replacing them with uses of current.  The changes to gating are a
> side-effect.
>
> With that change, Reviewed-by: Tim Deegan <tim@xen.org>

Thanks for taking the time to review this.  I will rewrite the commit
message and submit a v2.

~Andrew