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

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

On Fri, 2017-07-07 at 18:52 +0200, Paolo Bonzini wrote:
> > +3. KVMI_PAUSE_GUEST
> > +-------------------
> > +
> > +:Architectures: all
> > +:Versions: >= 1
> > +:Parameters: {}
> > +:Returns: ↴
> > +
> > +::
> > +
> > +	struct kvmi_error_code {
> > +		__s32 err;
> > +		__u32 padding;
> > +	};
> > +
> > +This command will pause all vcpus threads, by getting them out of guest mode
> > +and put them in the "waiting introspection commands" state.
> 
> Pausing all threads synchronously is a tricky concept.  The main issue
> is that the guest could be paused at the userspace level.  Then KVM
> would not be running (userspace is stuck in pthread_cond_wait,
> typically) and could not acknowledge the pause.
> 
> Worse, KVM is not able to distinguish userspace that has paused the VM
> from userspace that is doing MMIO or userspace that has a bug and hung
> somewhere.  And even worse, there are cases where userspace wants to
> modify registers while doing port I/O (the awful VMware RPCI port).  So
> I'd rather avoid this.

I should give more details here: we don't need to pause the vCPU-s in
the sense widely understood but just prevent them from entering the
guest for a short period of time. In our particular case, we need this
when we start introspecting a VM that's already running. For this we
kick the vCPU-s out of the guest so that our scan of the memory does
not race with the guest kernel/applications.

Another use case is when we inject applications into a running guest.
We also kick the vCPU-s out while we atomically make changes to kernel
specific structures.

> > +8. KVMI_GET_MTRR_TYPE
> > +---------------------
> > +
> > +:Architectures: x86
> > +:Versions: >= 1
> > +:Parameters: ↴
> > +
> > +::
> > +
> > +	struct kvmi_mtrr_type {
> > +		__u64 gpa;
> > +	};
> > +
> > +:Returns: ↴
> > +
> > +::
> > +
> > +	struct kvmi_mtrr_type_reply {
> > +		__s32 err;
> > +		__u32 type;
> > +	};
> > +
> > +Returns the guest memory type for a specific physical address.
> 
> What is this used for?  KVM ignores the guest MTRRs, so if possible I'd
> rather avoid it.

We use it do identify cacheable memory which usually indicates device
memory, something we don't want to touch. We are also looking into
making use of the page attributes (PAT) or other PTE-bits instead of
MTRR, but for the time being MTRR-s are still being relied upon.

> > +9. KVMI_GET_MTRRS
> > +-----------------
> > +
> > +:Architectures: x86
> > +:Versions: >= 1
> > +:Parameters: ↴
> > +
> > +::
> > +
> > +	struct kvmi_mtrrs {
> > +		__u16 vcpu;
> > +		__u16 padding[3];
> > +	};
> > +
> > +:Returns: ↴
> > +
> > +::
> > +
> > +	struct kvmi_mtrrs_reply {
> > +		__s32 err;
> > +		__u32 padding;
> > +		__u64 pat;
> > +		__u64 cap;
> > +		__u64 type;
> > +	};
> > +
> > +Returns MSR_IA32_CR_PAT, MSR_MTRRcap and MSR_MTRRdefType for the specified
> > +vCPU.
> 
> This could use KVM_GET_REGISTERS, couldn't it?

Yes, I belive it can be folded into that command.

> > +14. KVMI_INJECT_BREAKPOINT
> > +--------------------------
> > +
> > +:Architectures: all
> > +:Versions: >= 1
> > +:Parameters: ↴
> > +
> > +::
> > +
> > +	struct kvmi_inject_breakpoint {
> > +		__u16 vcpu;
> > +		__u16 padding[3];
> > +	};
> > +
> > +:Returns: ↴
> > +
> > +::
> > +
> > +	struct kvmi_error_code {
> > +		__s32 err;
> > +		__u32 padding;
> > +	};
> > +
> > +Injects a breakpoint for the specified vCPU. This command is usually sent in
> > +response to an event and as such the proper GPR-s will be set with the reply.
> 
> What is a "breakpoint" in this context?

A simple INT3. It's what most of our patches consist of. We keep track
of them and handle the ones we own while pass (reinject) the ones used
by the guest (debuggers or whatnot).

-- 
Mihai Donțu