| Message ID | 20190823010709.24879-8-sean.j.christopherson@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | KVM: x86: Remove emulation_result enums | expand |
> On 23 Aug 2019, at 4:07, Sean Christopherson <sean.j.christopherson@intel.com> wrote: > > Add an explicit emulation type for forced #UD emulation and use it to > detect that KVM should unconditionally inject a #UD instead of falling > into its standard emulation failure handling. > > Signed-off-by: Sean Christopherson <sean.j.christopherson@intel.com> The name "forced emulation on #UD" is not clear to me. If I understand correctly, EMULTYPE_TRAP_UD is currently used to indicate that in case the x86 emulator fails to decode instruction, the caller would like the x86 emulator to fail early such that it can handle this condition properly. Thus, I would rename it EMULTYPE_TRAP_DECODE_FAILURE. But this new flag seems to do the same. So I’m left confused. I’m probably missing something trivial here. -Liran > --- > arch/x86/include/asm/kvm_host.h | 1 + > arch/x86/kvm/x86.c | 5 +++-- > 2 files changed, 4 insertions(+), 2 deletions(-) > > diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h > index d1d5b5ca1195..a38c93362945 100644 > --- a/arch/x86/include/asm/kvm_host.h > +++ b/arch/x86/include/asm/kvm_host.h > @@ -1318,6 +1318,7 @@ enum emulation_result { > #define EMULTYPE_TRAP_UD (1 << 1) > #define EMULTYPE_SKIP (1 << 2) > #define EMULTYPE_ALLOW_RETRY (1 << 3) > +#define EMULTYPE_TRAP_UD_FORCED (1 << 4) > #define EMULTYPE_VMWARE_GP (1 << 5) > int kvm_emulate_instruction(struct kvm_vcpu *vcpu, int emulation_type); > int kvm_emulate_instruction_from_buffer(struct kvm_vcpu *vcpu, > diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c > index 228ca71d5b01..a1f9e36b2d58 100644 > --- a/arch/x86/kvm/x86.c > +++ b/arch/x86/kvm/x86.c > @@ -5337,7 +5337,7 @@ int handle_ud(struct kvm_vcpu *vcpu) > sig, sizeof(sig), &e) == 0 && > memcmp(sig, "\xf\xbkvm", sizeof(sig)) == 0) { > kvm_rip_write(vcpu, kvm_rip_read(vcpu) + sizeof(sig)); > - emul_type = 0; > + emul_type = EMULTYPE_TRAP_UD_FORCED; > } > > er = kvm_emulate_instruction(vcpu, emul_type); > @@ -6532,7 +6532,8 @@ int x86_emulate_instruction(struct kvm_vcpu *vcpu, > trace_kvm_emulate_insn_start(vcpu); > ++vcpu->stat.insn_emulation; > if (r != EMULATION_OK) { > - if (emulation_type & EMULTYPE_TRAP_UD) > + if ((emulation_type & EMULTYPE_TRAP_UD) || > + (emulation_type & EMULTYPE_TRAP_UD_FORCED)) > return EMULATE_FAIL; > if (reexecute_instruction(vcpu, cr2, write_fault_to_spt, > emulation_type)) > -- > 2.22.0 >
On Fri, Aug 23, 2019 at 04:47:14PM +0300, Liran Alon wrote: > > > > On 23 Aug 2019, at 4:07, Sean Christopherson <sean.j.christopherson@intel.com> wrote: > > > > Add an explicit emulation type for forced #UD emulation and use it to > > detect that KVM should unconditionally inject a #UD instead of falling > > into its standard emulation failure handling. > > > > Signed-off-by: Sean Christopherson <sean.j.christopherson@intel.com> > > The name "forced emulation on #UD" is not clear to me. > > If I understand correctly, EMULTYPE_TRAP_UD is currently used to indicate > that in case the x86 emulator fails to decode instruction, the caller would > like the x86 emulator to fail early such that it can handle this condition > properly. Thus, I would rename it EMULTYPE_TRAP_DECODE_FAILURE. EMULTYPE_TRAP_UD is used when KVM intercepts a #UD from hardware. KVM only emulates select instructions in this case in order to minmize the emulator attack surface, e.g.: if (unlikely(ctxt->ud) && likely(!(ctxt->d & EmulateOnUD))) return EMULATION_FAILED; To enable testing of the emulator, KVM recognizes a special "opcode" that triggers full emulation on #UD, e.g. ctxt->ud is false when the #UD was triggered with the magic prefix. The prefix is only recognized when the module param force_emulation_prefix is toggled on, hence the name EMULTYPE_TRAP_UD_FORCED. > But this new flag seems to do the same. So I’m left confused. I’m probably > missing something trivial here.
> On 23 Aug 2019, at 17:44, Sean Christopherson <sean.j.christopherson@intel.com> wrote: > > On Fri, Aug 23, 2019 at 04:47:14PM +0300, Liran Alon wrote: >> >> >>> On 23 Aug 2019, at 4:07, Sean Christopherson <sean.j.christopherson@intel.com> wrote: >>> >>> Add an explicit emulation type for forced #UD emulation and use it to >>> detect that KVM should unconditionally inject a #UD instead of falling >>> into its standard emulation failure handling. >>> >>> Signed-off-by: Sean Christopherson <sean.j.christopherson@intel.com> >> >> The name "forced emulation on #UD" is not clear to me. >> >> If I understand correctly, EMULTYPE_TRAP_UD is currently used to indicate >> that in case the x86 emulator fails to decode instruction, the caller would >> like the x86 emulator to fail early such that it can handle this condition >> properly. Thus, I would rename it EMULTYPE_TRAP_DECODE_FAILURE. > > EMULTYPE_TRAP_UD is used when KVM intercepts a #UD from hardware. KVM > only emulates select instructions in this case in order to minmize the > emulator attack surface, e.g.: > > if (unlikely(ctxt->ud) && likely(!(ctxt->d & EmulateOnUD))) > return EMULATION_FAILED; > > To enable testing of the emulator, KVM recognizes a special "opcode" that > triggers full emulation on #UD, e.g. ctxt->ud is false when the #UD was > triggered with the magic prefix. The prefix is only recognized when the > module param force_emulation_prefix is toggled on, hence the name > EMULTYPE_TRAP_UD_FORCED. Ah-ha. This makes sense. Thanks for the explanation. I would say it’s worth putting a comment in it in code… > >> But this new flag seems to do the same. So I’m left confused. I’m probably >> missing something trivial here.
diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h index d1d5b5ca1195..a38c93362945 100644 --- a/arch/x86/include/asm/kvm_host.h +++ b/arch/x86/include/asm/kvm_host.h @@ -1318,6 +1318,7 @@ enum emulation_result { #define EMULTYPE_TRAP_UD (1 << 1) #define EMULTYPE_SKIP (1 << 2) #define EMULTYPE_ALLOW_RETRY (1 << 3) +#define EMULTYPE_TRAP_UD_FORCED (1 << 4) #define EMULTYPE_VMWARE_GP (1 << 5) int kvm_emulate_instruction(struct kvm_vcpu *vcpu, int emulation_type); int kvm_emulate_instruction_from_buffer(struct kvm_vcpu *vcpu, diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c index 228ca71d5b01..a1f9e36b2d58 100644 --- a/arch/x86/kvm/x86.c +++ b/arch/x86/kvm/x86.c @@ -5337,7 +5337,7 @@ int handle_ud(struct kvm_vcpu *vcpu) sig, sizeof(sig), &e) == 0 && memcmp(sig, "\xf\xbkvm", sizeof(sig)) == 0) { kvm_rip_write(vcpu, kvm_rip_read(vcpu) + sizeof(sig)); - emul_type = 0; + emul_type = EMULTYPE_TRAP_UD_FORCED; } er = kvm_emulate_instruction(vcpu, emul_type); @@ -6532,7 +6532,8 @@ int x86_emulate_instruction(struct kvm_vcpu *vcpu, trace_kvm_emulate_insn_start(vcpu); ++vcpu->stat.insn_emulation; if (r != EMULATION_OK) { - if (emulation_type & EMULTYPE_TRAP_UD) + if ((emulation_type & EMULTYPE_TRAP_UD) || + (emulation_type & EMULTYPE_TRAP_UD_FORCED)) return EMULATE_FAIL; if (reexecute_instruction(vcpu, cr2, write_fault_to_spt, emulation_type))
Add an explicit emulation type for forced #UD emulation and use it to detect that KVM should unconditionally inject a #UD instead of falling into its standard emulation failure handling. Signed-off-by: Sean Christopherson <sean.j.christopherson@intel.com> --- arch/x86/include/asm/kvm_host.h | 1 + arch/x86/kvm/x86.c | 5 +++-- 2 files changed, 4 insertions(+), 2 deletions(-)