| Message ID | d7b10cdc-b33a-0297-458d-1bad42727fa0@suse.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [RFC] x86emul: unconditionally deliver #UD for LWP insns | expand |
On 17/07/2019 07:42, Jan Beulich wrote: > This is to accompany commit ("x86/svm: Drop support for AMD's 91f86f8634 > Lightweight Profiling"). > > Signed-off-by: Jan Beulich <jbeulich@suse.com> Acked-by: Andrew Cooper <andrew.cooper3@citrix.com> > --- > With AMD apparently having abandoned XOP encoded insns, another option > would seem to be to simply wire all unrecognized ones into #UD (rather > then returning UNIMPLEMENTED/UNRECOGNIZED). There are still some XOP instructions which actually work on Fam17h processors, if you ignore CPUID and go blindly executing. Given no official statement that XOP is dead, I'd keep the support we currently have. ~Andrew
On 17.07.2019 13:42, Andrew Cooper wrote: > On 17/07/2019 07:42, Jan Beulich wrote: >> This is to accompany commit ("x86/svm: Drop support for AMD's > > 91f86f8634 Hmm, odd - I'm sure I had checked, resulting in the impression that this didn't get committed yet. >> Lightweight Profiling"). >> >> Signed-off-by: Jan Beulich <jbeulich@suse.com> > Acked-by: Andrew Cooper <andrew.cooper3@citrix.com> > >> --- >> With AMD apparently having abandoned XOP encoded insns, another option >> would seem to be to simply wire all unrecognized ones into #UD (rather >> then returning UNIMPLEMENTED/UNRECOGNIZED). > > There are still some XOP instructions which actually work on Fam17h > processors, if you ignore CPUID and go blindly executing. > > Given no official statement that XOP is dead, I'd keep the support we > currently have. Then my remark wasn't clear enough: I'm not suggesting to rip out XOP insn support we have. I'm instead considering whether to wire all unsupported XOP encodings into #UD (rather than return UNIMPLEMENTED/UNRECOGNIZED for them), not just the LWP ones. Jan
On 17/07/2019 14:07, Jan Beulich wrote: > On 17.07.2019 13:42, Andrew Cooper wrote: >> On 17/07/2019 07:42, Jan Beulich wrote: >>> This is to accompany commit ("x86/svm: Drop support for AMD's >> 91f86f8634 > Hmm, odd - I'm sure I had checked, resulting in the impression that > this didn't get committed yet. > >>> Lightweight Profiling"). >>> >>> Signed-off-by: Jan Beulich <jbeulich@suse.com> >> Acked-by: Andrew Cooper <andrew.cooper3@citrix.com> >> >>> --- >>> With AMD apparently having abandoned XOP encoded insns, another option >>> would seem to be to simply wire all unrecognized ones into #UD (rather >>> then returning UNIMPLEMENTED/UNRECOGNIZED). >> There are still some XOP instructions which actually work on Fam17h >> processors, if you ignore CPUID and go blindly executing. >> >> Given no official statement that XOP is dead, I'd keep the support we >> currently have. > Then my remark wasn't clear enough: I'm not suggesting to rip out > XOP insn support we have. I'm instead considering whether to wire > all unsupported XOP encodings into #UD (rather than return > UNIMPLEMENTED/UNRECOGNIZED for them), not just the LWP ones. Ah, in which case, no. Turning all unknown instructions into EXCEPTION/#UD will break introspection, which uses UNRECOGNISED to cover the gaps in the emulator by single-stepping the vcpu. ~Andrew
On 17.07.2019 19:09, Andrew Cooper wrote: > On 17/07/2019 14:07, Jan Beulich wrote: >> On 17.07.2019 13:42, Andrew Cooper wrote: >>> On 17/07/2019 07:42, Jan Beulich wrote: >>>> With AMD apparently having abandoned XOP encoded insns, another option >>>> would seem to be to simply wire all unrecognized ones into #UD (rather >>>> then returning UNIMPLEMENTED/UNRECOGNIZED). >>> There are still some XOP instructions which actually work on Fam17h >>> processors, if you ignore CPUID and go blindly executing. >>> >>> Given no official statement that XOP is dead, I'd keep the support we >>> currently have. >> Then my remark wasn't clear enough: I'm not suggesting to rip out >> XOP insn support we have. I'm instead considering whether to wire >> all unsupported XOP encodings into #UD (rather than return >> UNIMPLEMENTED/UNRECOGNIZED for them), not just the LWP ones. > > Ah, in which case, no. Turning all unknown instructions into > EXCEPTION/#UD will break introspection, which uses UNRECOGNISED to cover > the gaps in the emulator by single-stepping the vcpu. But there are no gaps: The only ones we didn't cover (afaik) were the LWP ones, and those would get made raise #UD by the patch here anyway. If there are really opcodes where "unrecognized" is relevant, then we should special case _those_ imo, rather than leaving an opcode space which apparently won't see further extensions all go the "unrecognized" route. Jan
On 18/07/2019 10:18, Jan Beulich wrote: > On 17.07.2019 19:09, Andrew Cooper wrote: >> On 17/07/2019 14:07, Jan Beulich wrote: >>> On 17.07.2019 13:42, Andrew Cooper wrote: >>>> On 17/07/2019 07:42, Jan Beulich wrote: >>>>> With AMD apparently having abandoned XOP encoded insns, another option >>>>> would seem to be to simply wire all unrecognized ones into #UD (rather >>>>> then returning UNIMPLEMENTED/UNRECOGNIZED). >>>> There are still some XOP instructions which actually work on Fam17h >>>> processors, if you ignore CPUID and go blindly executing. >>>> >>>> Given no official statement that XOP is dead, I'd keep the support we >>>> currently have. >>> Then my remark wasn't clear enough: I'm not suggesting to rip out >>> XOP insn support we have. I'm instead considering whether to wire >>> all unsupported XOP encodings into #UD (rather than return >>> UNIMPLEMENTED/UNRECOGNIZED for them), not just the LWP ones. >> Ah, in which case, no. Turning all unknown instructions into >> EXCEPTION/#UD will break introspection, which uses UNRECOGNISED to cover >> the gaps in the emulator by single-stepping the vcpu. > But there are no gaps: [F]XSAVE et al, VT-x, SVM to name several gaps. Not to mention running current versions of Xen on newer hardware. > The only ones we didn't cover (afaik) were the > LWP ones, and those would get made raise #UD by the patch here anyway. > If there are really opcodes where "unrecognized" is relevant, then we > should special case _those_ imo, rather than leaving an opcode space > which apparently won't see further extensions all go the "unrecognized" > route. In terms of "not crashing the guest", it is very important to know if there are instructions which are accepted by the pipeline, trap for an access violation, and that x86_emulate() doesn't know about. If we do emulation support perfectly, along with CPUID feature handling etc, then this set should be 0, but it is still nonzero even with this patch. For instructions we know about, and know that we told the guest it couldn't use, then #UD is fine and correct to do. Given that approximately all of access violations are well-formed non-malicious code, then an instruction which x86_emulate() doesn't know about is most likely a hole or a CPUID handling issue. Either way, blindly converting this to #UD is not the appropriate course of action. ~Andrew
On 18.07.2019 11:30, Andrew Cooper wrote: > On 18/07/2019 10:18, Jan Beulich wrote: >> On 17.07.2019 19:09, Andrew Cooper wrote: >>> On 17/07/2019 14:07, Jan Beulich wrote: >>>> On 17.07.2019 13:42, Andrew Cooper wrote: >>>>> On 17/07/2019 07:42, Jan Beulich wrote: >>>>>> With AMD apparently having abandoned XOP encoded insns, another option >>>>>> would seem to be to simply wire all unrecognized ones into #UD (rather >>>>>> then returning UNIMPLEMENTED/UNRECOGNIZED). >>>>> There are still some XOP instructions which actually work on Fam17h >>>>> processors, if you ignore CPUID and go blindly executing. >>>>> >>>>> Given no official statement that XOP is dead, I'd keep the support we >>>>> currently have. >>>> Then my remark wasn't clear enough: I'm not suggesting to rip out >>>> XOP insn support we have. I'm instead considering whether to wire >>>> all unsupported XOP encodings into #UD (rather than return >>>> UNIMPLEMENTED/UNRECOGNIZED for them), not just the LWP ones. >>> Ah, in which case, no. Turning all unknown instructions into >>> EXCEPTION/#UD will break introspection, which uses UNRECOGNISED to cover >>> the gaps in the emulator by single-stepping the vcpu. >> But there are no gaps: > > [F]XSAVE et al, VT-x, SVM to name several gaps. > > Not to mention running current versions of Xen on newer hardware. None of these are XOP encoded. I've never been considering to wire everything into #UD. Jan
--- a/xen/arch/x86/x86_emulate/x86_emulate.c +++ b/xen/arch/x86/x86_emulate/x86_emulate.c @@ -10367,6 +10367,16 @@ x86_emulate( } goto unrecognized_insn; + case X86EMUL_OPC_XOP(09, 0x12): /* XOP Grp3 */ + switch ( modrm_reg & 7 ) + { + case 0: /* llwpcb r */ + case 1: /* slwpcb r */ + /* LWP is unsupported, so produce #UD unconditionally. */ + generate_exception(EXC_UD); + } + goto unrecognized_insn; + case X86EMUL_OPC_XOP(09, 0x82): /* vfrczss xmm/m128,xmm */ case X86EMUL_OPC_XOP(09, 0x83): /* vfrczsd xmm/m128,xmm */ generate_exception_if(vex.l, EXC_UD); @@ -10451,6 +10461,16 @@ x86_emulate( break; } + case X86EMUL_OPC_XOP(0a, 0x12): /* XOP Grp4 */ + switch ( modrm_reg & 7 ) + { + case 0: /* lwpins $imm32,r/m,r */ + case 1: /* lwpval $imm32,r/m,r */ + /* LWP is unsupported, so produce #UD unconditionally. */ + generate_exception(EXC_UD); + } + goto unrecognized_insn; + default: unimplemented_insn: rc = X86EMUL_UNIMPLEMENTED;
This is to accompany commit ("x86/svm: Drop support for AMD's Lightweight Profiling"). Signed-off-by: Jan Beulich <jbeulich@suse.com> --- With AMD apparently having abandoned XOP encoded insns, another option would seem to be to simply wire all unrecognized ones into #UD (rather then returning UNIMPLEMENTED/UNRECOGNIZED). --- TODO/RFC: Insert commit ID of referenced commit.