Re: [RFC PATCH v2 1/1] kvm: Add documentation and ABI/API header for VM introspection

["Mihai Donțu" <mdontu@bitdefender.com>]

On Mon, 2017-08-07 at 15:49 +0200, Paolo Bonzini wrote:
> On 07/08/2017 15:25, Mihai Donțu wrote:
> > > "Pause all VCPUs and stop all DMA" would definitely be a layering
> > > violation, so it cannot be added.
> > > 
> > > "Pause all VCPUs" is basically a shortcut for many "pause the VCPU with
> > > a given id" commands.  I lean towards omitting it.
> > 
> > The case where the introspector wants to scan the guest memory needs a
> > KVMI_PAUSE_VM, which as discussed in a previous email, can be the
> > actual qemu 'pause' command.
> 
> Do you mean it needs to stop DMA as well?

No, DMA can proceed normally. I remain of the opinion that KVMI users
must know what guest memory ranges are OK to access by looking at MTRR-
s, PAT or guest kernel structures, or a combination of all three.

> > However, we would like to limit the
> > communication channels we have with the host and not use qmp (or
> > libvirt/etc. if qmp is not exposed). Instead, have a command that
> > triggers a KVM_RUN exit to qemu which in turn will call the underlying
> > pause function used by qmp. Would that be OK with you?
> 
> You would have to send back something on completion, and then I am
> worried of races and deadlocks.  Plus, pausing a VM at the QEMU level is
> a really expensive operation, so I don't think it's a good idea to let
> the introspector do this.  You can pause all VCPUs, or use memory page
> permissions.

Pausing all vCPU-s was my first thought, I was just trying to follow
your statement: "I lean towards omitting it". :-)

It will take a bit of user-space-fu, in that after issuing N vCPU pause
commands in a row we will have to wait for N events, which might race
with other events (MSR, CRx etc.) which need handling otherwise the
pause ones will not arrive ... I wonder if there's a way to do this
cleanly in kernel (piggyback on the code for pausing a single vCPU and
then somehow 'coalesce' all pause events into a single
KVMI_EVENT_VCPUS_PAUSED).

> > > However, now that I'm thinking of it, we need a new event for "new VCPU
> > > created".  When the event is enabled, newly-created VCPUs should be in
> > > paused mode.
> > 
> > I assume you are thinking about vCPU hotplug here. If so, yes, an event
> > that gives the introspector the chance to update its internal
> > bookkeeping would be useful.
> 
> Yes, exactly.

OK. We will update the design document.

-- 
Mihai Donțu