Re: [PATCH] x86/HVM: refine when to send mapcache invalidation request to qemu

[Hongyan Xia <hx242@xxxxxxx>]

On Thu, 2020-12-10 at 14:37 +0100, Jan Beulich wrote:
> On 10.12.2020 14:09, Hongyan Xia wrote:
> > On Mon, 2020-09-28 at 12:44 +0200, Jan Beulich wrote:
> > > Plus finally there's no point sending the request for the local
> > > domain
> > > when the domain acted upon is a different one. If anything that
> > > domain's
> > > qemu's mapcache may need invalidating, but it's unclear how
> > > useful
> > > this
> > > would be: That remote domain may not execute hypercalls at all,
> > > and
> > > hence may never make it to the point where the request actually
> > > gets
> > > issued. I guess the assumption is that such manipulation is not
> > > supposed
> > > to happen anymore once the guest has been started?
> > 
> > I may still want to set the invalidation signal to true even if the
> > domain acted on is not the local domain. I know the remote domain
> > may
> > never reach the point to issue the invalidate, but it sounds to me
> > that
> > the problem is not whether we should set the signal but whether we
> > can
> > change where the signal is checked to make sure the point of issue
> > can
> > be reliably triggered, and the latter can be done in a future
> > patch.
> 
> One of Paul's replies was quite helpful here: The main thing to

Hmm, I seem to not be able to see the whole thread...

> worry about is for the vCPU to not continue running before the
> invalidation request was signaled (or else, aiui, qemu may serve
> a subsequent emulation request by the guest incorrectly, because
> of using the stale mapping). Hence I believe for a non-paused
> guest remote operations simply cannot be allowed when the may
> lead to the need for invalidation. Therefore yes, if we assume
> the guest is paused in such cases, we could drop the "is current"
> check, but we'd then still need to arrange for actual signaling
> before the guest gets to run again. I wonder whether
> handle_hvm_io_completion() (or its caller, hvm_do_resume(),
> right after that other call) wouldn't be a good place to do so.

Actually, the existing code must assume that when QEMU is up, the only
one that manipulates the p2m is the guest itself like you said. If the
caller is XENMEM_decrease_reservation, the code does not even check
which p2m this is for and unconditionally sets the QEMU invalidate flag
for the current domain. Athough this assumption may simply be wrong
now, so I agree care should be taken for remote p2m ops (I may need to
read the code more to know how this should be done).

Hongyan