Re: [RFC PATCH v3 1/1] kvm: add documentation for the VM introspection subsystem

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Wed, 2017-09-13 at 00:11 +0200, Paolo Bonzini wrote:
> On 11/09/2017 17:36, Adalbert Lazar wrote:
> > +13. KVMI_MSR_CONTROL
> > +--------------------
> > +14. KVMI_CONTROL_VE
> > +-------------------
> 
> 
> Decide on one. :)  I prefer KVMI_CONTROL_*

I'm sorry about that. An obvious consistency mistake that got past me.

KVMI_CONTROL_* it is.

> About events:
> 
> - page faults:
> 
> > +
> > +* *KVMI_EVENT_ACTION_ALLOW* -  allow the page fault via emulation (with
> > +  custom input if ``ctx_size`` > 0). The use of custom input is to trick
> > +  the guest software into believing it has read certain data, in order
> > +  to hide the content of certain memory areas (eg. hide injected code
> > +  from integrity checkers). If ``trap_access`` is not zero, the REP
> > +  prefixed instruction should be emulated just once.
> > +* *KVMI_EVENT_ACTION_SKIP* - the introspection tool did the emulation
> > +  (incremented the program counter and changed the registers/memory
> > +  affected by the emulation). It is used to prevent unwanted changes to
> > +  memory.
> > +* *KVMI_EVENT_ACTION_RETRY* - re-enter the guest and let it re-trigger
> > +   the page fault
> 
> SKIP and RETRY are the same, aren't they?  I would just use RETRY
> everywhere.  SKIP implies that KVM takes care of modifying the
> instruction pointer for example, RETRY does not.

I've re-checked with my colleagues and it looks like I've misunderstood
things. The conclusion is that SKIP and RETRY should be the same from
KVM's point of view. The user space component, however, needs some
extra logic to adjust the RIP accordingly (skip over an instruction or
jump to an 'unrelated' address).

We will use RETRY and drop SKIP.

> - singlestepping:
> 
> > +* *KVMI_EVENT_ACTION_ALLOW* - disable the single-stepping and continue normal
> > +  guest execution
> > +* *KVMI_EVENT_ACTION_RETRY* - continue guest execution, which will lead to
> > +  a new *KVMI_EVENT_SINGLESTEP* event
> 
> Because singlestep is a trap event (happens after execution of the
> instruction), RETRY has the same meaning as SKIP elsewhere: KVM has
> nothing to do.  Is this correct?  So we can use RETRY.

Correct.

> ALLOW here is not a very good name, but I have no better idea so I guess
> it's okay.  Maybe rename ALLOW to CONTINUE everywhere, so it makes more
> sense here too. :)
> 
> - paused:
> 
> > 
> > +	struct kvmi_event_reply
> > +
> > +This event is sent in response to a *KVMI_PAUSE_VCPU* command, unless it
> > +is canceled by another *KVMI_PAUSE_VCPU* command (with ``cancel`` set to 1).
> > +
> > +The introspector can respond with one of the following actions:
> > +
> > +* *KVMI_EVENT_ACTION_ALLOW* - resume
> > +* *KVMI_EVENT_ACTION_CRASH* - stop the guest
> 
> ALLOW here is just a "do nothing" action and the API might as well use
> RETRY.

OK.

> > +5. KVMI_EVENT_HYPERCALL
> > +-----------------------
> > +* *KVMI_EVENT_ACTION_ALLOW* - acknowledged
> 
> Should there be a return value in the reply, that is written into eg. RAX?

We can provide a value for RAX with the event response or assume the
introspection tool called SET_REGISTERS with the appropriate RAX.

For now we took your suggestion. We'll revisit if needed.

> In general, I think actions other than INJECT should be documented
> generically:
> 
> - CONTINUE (formerly ALLOW) - complete the instruction that caused the
> event (i.e. execute it without triggering the event again, possibly
> using information coming from the reply)
> 
> - RETRY (formerly SKIP or RETRY) - re-enter the guest (the event may
> happen again)
> 
> - CRASH - self-documenting
> 
> RETRY and CRASH should always be allowed.  The specific behavior of
> CONTINUE can be documented for each event that allows it.
> 
> Do you need events for reading CRn or MSRs?

There is another VMI project, Nitro, that uses MSR read events to do a
sort of strace on syscalls and we were thinking on porting that
feature. However, users of that API with which we are in contact,
expressed the intention of moving to a breakpoint based system rather
than trapping for every MSR read. Since we don't have a use case for it
either, we decided to hold off on that unless somebody jumps in and
asks for it.

> Otherwise looks good. I guess you can send code patches on v4. ;)

Thank you,

-- 
Mihai Donțu




[Index of Archives]     [KVM ARM]     [KVM ia64]     [KVM ppc]     [Virtualization Tools]     [Spice Development]     [Libvirt]     [Libvirt Users]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite Questions]     [Linux Kernel]     [Linux SCSI]     [XFree86]

  Powered by Linux