Re: [PATCH v5 1/2] kvm: x86: Allow userspace to handle emulation errors

[Aaron Lewis <aaronlewis@google.com>]

> > +7.24 KVM_CAP_EXIT_ON_EMULATION_FAILURE
> > +--------------------------------------
> > +
> > +:Architectures: x86
> > +:Parameters: args[0] whether the feature should be enabled or not
> > +
> > +When this capability is enabled the in-kernel instruction emulator packs
> > +the exit struct of KVM_INTERNAL_ERROR with the instruction length and
> > +instruction bytes when an error occurs while emulating an instruction.  This
> > +will also happen when the emulation type is set to EMULTYPE_SKIP, but with this
> > +capability enabled this becomes the default behavior regarless of how the
>
> s/regarless/regardless/
>
> > +emulation type is set unless it is a VMware #GP; in that case a #GP is injected
> > +and KVM does not exit to userspace.
> > +
> > +When this capability is enabled use the emulation_failure struct instead of the
> > +internal struct for the exit struct.  They have the same layout, but the
> > +emulation_failure struct matches the content better.  It also explicitly defines
> > +the 'flags' field which is used to describe the fields in the struct that are
> > +valid (ie: if KVM_INTERNAL_ERROR_EMULATION_FLAG_INSTRUCTION_BYTES is set in the
> > +'flags' field then 'insn_size' and 'insn_bytes' has valid data in them.)
>
> Starting both paragraphs with "With this capability enabled..." would
> probably cause me to stop reading if I didn't enable the capability, but
> as the first paragraph goes on to say, EMULTYPE_SKIP will also cause the
> instruction to be provided.
>

What about this instead?

When this capability is enabled, an emulation failure will result in an exit
to userspace with KVM_INTERNAL_ERROR (except when the emulator was invoked
to handle a VMware backdoor instruction). Furthermore, KVM will now provide up
to 15 instruction bytes for any exit to userspace resulting from an emulation
failure.  When these exits to userspace occur use the emulation_failure struct
instead of the internal struct.  They both have the same layout, but the
emulation_failure struct matches the content better.  It also explicitly
defines the 'flags' field which is used to describe the fields in the struct
that are valid (ie: if KVM_INTERNAL_ERROR_EMULATION_FLAG_INSTRUCTION_BYTES is
set in the 'flags' field then both 'insn_size' and 'insn_bytes' have valid data
in them.)

I left out the part about EMULTYPE_SKIP because that behavior is not
affected by setting KVM_CAP_EXIT_ON_EMULATION_FAILURE, so I thought it
wasn't needed in the documentation here.