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

[Paul Durrant <xadimgnik@xxxxxxxxx>]

> -----Original Message-----
> From: Jan Beulich <jbeulich@xxxxxxxx>
> Sent: 02 February 2021 15:15
> To: xen-devel@xxxxxxxxxxxxxxxxxxxx
> Cc: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>; Wei Liu <wl@xxxxxxx>; Roger 
> Pau Monné
> <roger.pau@xxxxxxxxxx>; Paul Durrant <paul@xxxxxxx>; Julien Grall 
> <julien@xxxxxxx>; Stefano Stabellini
> <sstabellini@xxxxxxxxxx>; George Dunlap <george.dunlap@xxxxxxxxxx>
> 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@xxxxxxxx>
> ---
> 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