On 11/09/2017 17:36, Adalbert Lazar wrote: > +13. KVMI_MSR_CONTROL > +-------------------- > +14. KVMI_CONTROL_VE > +------------------- Decide on one. :) I prefer KVMI_CONTROL_* 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. - 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. 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. > +5. KVMI_EVENT_HYPERCALL > +----------------------- > +* *KVMI_EVENT_ACTION_ALLOW* - acknowledged Should there be a return value in the reply, that is written into eg. RAX? 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? Otherwise looks good. I guess you can send code patches on v4. ;) Thanks, Paolo
