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

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

 



On Tue, Apr 27, 2021 at 10:03 AM Aaron Lewis <aaronlewis@xxxxxxxxxx> wrote:
>
> Add a fallback mechanism to the in-kernel instruction emulator that
> allows userspace the opportunity to process an instruction the emulator
> was unable to.  When the in-kernel instruction emulator fails to process
> an instruction it will either inject a #UD into the guest or exit to
> userspace with exit reason KVM_INTERNAL_ERROR.  This is because it does
> not know how to proceed in an appropriate manner.  This feature lets
> userspace get involved to see if it can figure out a better path
> forward.
>
> Signed-off-by: Aaron Lewis <aaronlewis@xxxxxxxxxx>
...
> +static void prepare_emulation_failure_exit(struct kvm_vcpu *vcpu)
> +{
> +       struct x86_emulate_ctxt *ctxt = vcpu->arch.emulate_ctxt;
> +       u64 insn_size = ctxt->fetch.end - ctxt->fetch.data;
> +       struct kvm_run *run = vcpu->run;
> +       const u64 max_insn_size = 15;

Nit: insn_size and max_insn_size could both be just unsigned int.

> +       run->exit_reason = KVM_EXIT_INTERNAL_ERROR;
> +       run->emulation_failure.suberror = KVM_INTERNAL_ERROR_EMULATION;
> +       run->emulation_failure.ndata = 0;
> +       run->emulation_failure.flags = 0;
> +
> +       if (insn_size) {
> +               run->emulation_failure.ndata = 3;
> +               run->emulation_failure.flags |=
> +                       KVM_INTERNAL_ERROR_EMULATION_FLAG_INSTRUCTION_BYTES;
> +               run->emulation_failure.insn_size = insn_size;
> +               memset(run->emulation_failure.insn_bytes, 0x90, max_insn_size);
> +               memcpy(run->emulation_failure.insn_bytes,
> +                      ctxt->fetch.data, insn_size);

It's tempting to check that insn_size doesn't exceed 15, but I'm sure
we all trust the emulator.

> +       }
> +}
> +
>  static int handle_emulation_failure(struct kvm_vcpu *vcpu, int emulation_type)
>  {
> +       struct kvm *kvm = vcpu->kvm;
> +
>         ++vcpu->stat.insn_emulation_fail;
>         trace_kvm_emulate_insn_failed(vcpu);
>
> @@ -7129,10 +7159,9 @@ static int handle_emulation_failure(struct kvm_vcpu *vcpu, int emulation_type)
>                 return 1;
>         }
>
> -       if (emulation_type & EMULTYPE_SKIP) {
> -               vcpu->run->exit_reason = KVM_EXIT_INTERNAL_ERROR;
> -               vcpu->run->internal.suberror = KVM_INTERNAL_ERROR_EMULATION;
> -               vcpu->run->internal.ndata = 0;
> +       if (kvm->arch.exit_on_emulation_error ||
> +           (emulation_type & EMULTYPE_SKIP)) {
> +               prepare_emulation_failure_exit(vcpu);
>                 return 0;
>         }
>
> diff --git a/include/uapi/linux/kvm.h b/include/uapi/linux/kvm.h
> index f6afee209620..87009222c20c 100644
> --- a/include/uapi/linux/kvm.h
> +++ b/include/uapi/linux/kvm.h
> @@ -279,6 +279,9 @@ struct kvm_xen_exit {
>  /* Encounter unexpected vm-exit reason */
>  #define KVM_INTERNAL_ERROR_UNEXPECTED_EXIT_REASON      4
>
> +/* Flags that describe what fields in emulation_failure hold valid data. */
> +#define KVM_INTERNAL_ERROR_EMULATION_FLAG_INSTRUCTION_BYTES (1ULL << 0)
> +
>  /* for KVM_RUN, returned by mmap(vcpu_fd, offset=0) */
>  struct kvm_run {
>         /* in */
> @@ -382,6 +385,25 @@ struct kvm_run {
>                         __u32 ndata;
>                         __u64 data[16];
>                 } internal;
> +               /*
> +                * KVM_INTERNAL_ERROR_EMULATION
> +                *
> +                * "struct emulation_failure" is an overlay of "struct internal"
> +                * that is used for the KVM_INTERNAL_ERROR_EMULATION sub-type of
> +                * KVM_EXIT_INTERNAL_ERROR.  Note, unlike other internal error
> +                * sub-types, this struct is ABI!  It also needs to be backwards
> +                * compabile with "struct internal".  Take special care that
> +                * "ndata" is correct, that new fields are enumerated in "flags",
> +                * and that each flag enumerates fields that are 64-bit aligned
> +                * and sized (so that ndata+internal.data[] is valid/accurate).
> +                */
> +               struct {
> +                       __u32 suberror;
> +                       __u32 ndata;
> +                       __u64 flags;
> +                       __u8  insn_size;
> +                       __u8  insn_bytes[15];
> +               } emulation_failure;
>                 /* KVM_EXIT_OSI */
>                 struct {
>                         __u64 gprs[32];
> @@ -1078,6 +1100,7 @@ struct kvm_ppc_resize_hpt {
>  #define KVM_CAP_DIRTY_LOG_RING 192
>  #define KVM_CAP_X86_BUS_LOCK_EXIT 193
>  #define KVM_CAP_PPC_DAWR1 194
> +#define KVM_CAP_EXIT_ON_EMULATION_FAILURE 195
>
>  #ifdef KVM_CAP_IRQ_ROUTING
>
> diff --git a/tools/include/uapi/linux/kvm.h b/tools/include/uapi/linux/kvm.h
> index f6afee209620..87009222c20c 100644
> --- a/tools/include/uapi/linux/kvm.h
> +++ b/tools/include/uapi/linux/kvm.h
> @@ -279,6 +279,9 @@ struct kvm_xen_exit {
>  /* Encounter unexpected vm-exit reason */
>  #define KVM_INTERNAL_ERROR_UNEXPECTED_EXIT_REASON      4
>
> +/* Flags that describe what fields in emulation_failure hold valid data. */
> +#define KVM_INTERNAL_ERROR_EMULATION_FLAG_INSTRUCTION_BYTES (1ULL << 0)
> +
>  /* for KVM_RUN, returned by mmap(vcpu_fd, offset=0) */
>  struct kvm_run {
>         /* in */
> @@ -382,6 +385,25 @@ struct kvm_run {
>                         __u32 ndata;
>                         __u64 data[16];
>                 } internal;
> +               /*
> +                * KVM_INTERNAL_ERROR_EMULATION
> +                *
> +                * "struct emulation_failure" is an overlay of "struct internal"
> +                * that is used for the KVM_INTERNAL_ERROR_EMULATION sub-type of
> +                * KVM_EXIT_INTERNAL_ERROR.  Note, unlike other internal error
> +                * sub-types, this struct is ABI!  It also needs to be backwards
> +                * compabile with "struct internal".  Take special care that
> +                * "ndata" is correct, that new fields are enumerated in "flags",
> +                * and that each flag enumerates fields that are 64-bit aligned
> +                * and sized (so that ndata+internal.data[] is valid/accurate).
> +                */
> +               struct {
> +                       __u32 suberror;
> +                       __u32 ndata;
> +                       __u64 flags;
> +                       __u8  insn_size;
> +                       __u8  insn_bytes[15];
> +               } emulation_failure;
>                 /* KVM_EXIT_OSI */
>                 struct {
>                         __u64 gprs[32];
> @@ -1078,6 +1100,7 @@ struct kvm_ppc_resize_hpt {
>  #define KVM_CAP_DIRTY_LOG_RING 192
>  #define KVM_CAP_X86_BUS_LOCK_EXIT 193
>  #define KVM_CAP_PPC_DAWR1 194
> +#define KVM_CAP_EXIT_ON_EMULATION_FAILURE 195
>
>  #ifdef KVM_CAP_IRQ_ROUTING
>
> --
> 2.31.1.498.g6c1eba8ee3d-goog
>



[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