RE: [PATCH v2 2/2] IOREQ: refine when to send mapcache invalidation request

From: Paul Durrant <xadimgnik@gmail.com>
To: "'Jan Beulich'" <jbeulich@suse.com>, <xen-devel@lists.xenproject.org>
Cc: "'Andrew Cooper'" <andrew.cooper3@citrix.com>,
	"'Wei Liu'" <wl@xen.org>,
	"'Roger Pau Monné'" <roger.pau@citrix.com>,
	"'Julien Grall'" <julien@xen.org>,
	"'Stefano Stabellini'" <sstabellini@kernel.org>,
	"'George Dunlap'" <george.dunlap@citrix.com>
Subject: RE: [PATCH v2 2/2] IOREQ: refine when to send mapcache invalidation request
Date: Thu, 4 Feb 2021 09:26:04 -0000	[thread overview]
Message-ID: <03fb01d6fad7$c39087b0$4ab19710$@xen.org> (raw)
In-Reply-To: <e2682f84-b3bb-b9fb-edd8-863b9036de95@suse.com>

> -----Original Message-----
> From: Jan Beulich <jbeulich@suse.com>
> Sent: 02 February 2021 15:15
> To: xen-devel@lists.xenproject.org
> Cc: Andrew Cooper <andrew.cooper3@citrix.com>; Wei Liu <wl@xen.org>; Roger Pau Monné
> <roger.pau@citrix.com>; Paul Durrant <paul@xen.org>; Julien Grall <julien@xen.org>; Stefano Stabellini
> <sstabellini@kernel.org>; George Dunlap <george.dunlap@citrix.com>
> Subject: [PATCH v2 2/2] IOREQ: refine when to send mapcache invalidation request
> 
> XENMEM_decrease_reservation isn't the only means by which pages can get
> removed from a guest, yet all removals ought to be signaled to qemu. Put
> setting of the flag into the central p2m_remove_page() underlying all
> respective hypercalls as well as a few similar places, mainly in PoD
> code.
> 
> Additionally there's no point sending the request for the local domain
> when the domain acted upon is a different one. The latter domain's ioreq
> server mapcaches need invalidating. We assume that domain to be paused
> at the point the operation takes place, so sending the request in this
> case happens from the hvm_do_resume() path, which as one of its first
> steps calls handle_hvm_io_completion().
> 
> Even without the remote operation aspect a single domain-wide flag
> doesn't do: Guests may e.g. decrease-reservation on multiple vCPU-s in
> parallel. Each of them needs to issue an invalidation request in due
> course, in particular because exiting to guest context should not happen
> before the request was actually seen by (all) the emulator(s).
> 
> Signed-off-by: Jan Beulich <jbeulich@suse.com>
> ---
> v2: Preemption related adjustment split off. Make flag per-vCPU. More
>     places to set the flag. Also handle acting on a remote domain.
>     Re-base.

I'm wondering if a per-vcpu flag is overkill actually. We just need to make sure that we don't miss sending an invalidation where multiple vcpus are in play. The mapcache in the emulator is global so issuing an invalidate for every vcpu is going to cause an unnecessary storm of ioreqs, isn't it? Could we get away with the per-domain atomic counter?

  Paul