* [PATCH 0/2] Fixes to RDTSCP interception @ 2018-11-30 17:07 Andrew Cooper 2018-11-30 17:07 ` [PATCH 1/2] x86/svm: Improve diagnostics when __get_instruction_length_from_list() fails Andrew Cooper 2018-11-30 17:07 ` [PATCH 2/2] x86/hvm: Corrections to RDTSCP intercept handling Andrew Cooper 0 siblings, 2 replies; 14+ messages in thread From: Andrew Cooper @ 2018-11-30 17:07 UTC (permalink / raw) To: Xen-devel Cc: Juergen Gross, Kevin Tian, Wei Liu, Suravee Suthikulpanit, Jun Nakajima, Andrew Cooper, Paul Durrant, Jan Beulich, Boris Ostrovsky, Brian Woods, Roger Pau Monné For some reason (on EPYC at least) we've gained a regression into master whereby VMs are defaulting to one of the emulated TSC modes. This may be Xen coming to the conclusion that there isn't a reliable TSC. Combined with a debug Xen, this breaks RDTSCP completely. With this series in place and RDTSCP functioning correctly again, VMs are still unwilling to boot. I haven't managed to figure out why yet. Andrew Cooper (2): x86/svm: Improve diagnostics when __get_instruction_length_from_list() fails x86/hvm: Corrections to RDTSCP intercept handling xen/arch/x86/hvm/svm/emulate.c | 27 ++++++++++++++++++++------- xen/arch/x86/hvm/svm/svm.c | 22 +++++++++++++++++----- xen/arch/x86/hvm/vmx/vmx.c | 8 ++++++++ xen/include/asm-x86/hvm/svm/emulate.h | 1 + 4 files changed, 46 insertions(+), 12 deletions(-) -- 2.1.4 _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH 1/2] x86/svm: Improve diagnostics when __get_instruction_length_from_list() fails 2018-11-30 17:07 [PATCH 0/2] Fixes to RDTSCP interception Andrew Cooper @ 2018-11-30 17:07 ` Andrew Cooper 2018-11-30 17:13 ` Woods, Brian ` (2 more replies) 2018-11-30 17:07 ` [PATCH 2/2] x86/hvm: Corrections to RDTSCP intercept handling Andrew Cooper 1 sibling, 3 replies; 14+ messages in thread From: Andrew Cooper @ 2018-11-30 17:07 UTC (permalink / raw) To: Xen-devel Cc: Wei Liu, Suravee Suthikulpanit, Andrew Cooper, Paul Durrant, Jan Beulich, Boris Ostrovsky, Brian Woods, Roger Pau Monné Sadly, a lone: (XEN) emulate.c:156:d2v0 __get_instruction_length_from_list: Mismatch between expected and actual instruction: eip = fffff804564139c0 on the console is of no use trying to identify what went wrong. Dump as much state as we can to help identify what went wrong. Reported-by: Paul Durrant <paul.durrant@citrix.com> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com> --- CC: Jan Beulich <JBeulich@suse.com> CC: Wei Liu <wei.liu2@citrix.com> CC: Roger Pau Monné <roger.pau@citrix.com> CC: Paul Durrant <paul.durrant@citrix.com> CC: Boris Ostrovsky <boris.ostrovsky@oracle.com> CC: Suravee Suthikulpanit <suravee.suthikulpanit@amd.com> CC: Brian Woods <brian.woods@amd.com> RFC: __get_instruction_length_from_list() tries to cope with VMEXIT_IOIO, but IN/OUT instructions aren't in the decode list and I can't spot an entry point from the IOIO path. Am I missing something? Also, I'm not entirely convinced that making modrm an annonymous union is going to work with older CentOS compilers, and therefore am not sure whether that part of the change is worth it. The instruction in question can be obtained from the printed INSN_ constant alone. --- xen/arch/x86/hvm/svm/emulate.c | 26 +++++++++++++++++++------- 1 file changed, 19 insertions(+), 7 deletions(-) diff --git a/xen/arch/x86/hvm/svm/emulate.c b/xen/arch/x86/hvm/svm/emulate.c index 3d04af0..71a1b6e 100644 --- a/xen/arch/x86/hvm/svm/emulate.c +++ b/xen/arch/x86/hvm/svm/emulate.c @@ -56,11 +56,14 @@ static unsigned long svm_nextrip_insn_length(struct vcpu *v) static const struct { unsigned int opcode; - struct { - unsigned int rm:3; - unsigned int reg:3; - unsigned int mod:2; -#define MODRM(mod, reg, rm) { rm, reg, mod } + union { + struct { + unsigned int rm:3; + unsigned int reg:3; + unsigned int mod:2; + }; + unsigned int raw; +#define MODRM(mod, reg, rm) {{ rm, reg, mod }} } modrm; } opc_tab[INSTR_MAX_COUNT] = { [INSTR_PAUSE] = { X86EMUL_OPC_F3(0, 0x90) }, @@ -152,8 +155,17 @@ int __get_instruction_length_from_list(struct vcpu *v, } gdprintk(XENLOG_WARNING, - "%s: Mismatch between expected and actual instruction: " - "eip = %lx\n", __func__, (unsigned long)vmcb->rip); + "%s: Mismatch between expected and actual instruction:\n", + __func__); + gdprintk(XENLOG_WARNING, + " list[0] val %d, { opc %#x, modrm %#x }, list entries: %u\n", + list[0], opc_tab[list[0]].opcode, opc_tab[list[0]].modrm.raw, + list_count); + gdprintk(XENLOG_WARNING, " rip 0x%lx, nextrip 0x%lx, len %lu\n", + vmcb->rip, vmcb->nextrip, vmcb->nextrip - vmcb->rip); + hvm_dump_emulation_state(XENLOG_G_WARNING, "Insn_len", + &ctxt, X86EMUL_UNHANDLEABLE); + hvm_inject_hw_exception(TRAP_gp_fault, 0); return 0; } -- 2.1.4 _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH 1/2] x86/svm: Improve diagnostics when __get_instruction_length_from_list() fails 2018-11-30 17:07 ` [PATCH 1/2] x86/svm: Improve diagnostics when __get_instruction_length_from_list() fails Andrew Cooper @ 2018-11-30 17:13 ` Woods, Brian 2018-12-03 9:10 ` Paul Durrant 2018-12-03 10:32 ` Jan Beulich 2 siblings, 0 replies; 14+ messages in thread From: Woods, Brian @ 2018-11-30 17:13 UTC (permalink / raw) To: Andrew Cooper Cc: Wei Liu, Suthikulpanit, Suravee, Xen-devel, Paul Durrant, Jan Beulich, Boris Ostrovsky, Woods, Brian, Roger Pau Monné On Fri, Nov 30, 2018 at 05:07:19PM +0000, Andy Cooper wrote: > Sadly, a lone: > > (XEN) emulate.c:156:d2v0 __get_instruction_length_from_list: Mismatch between expected and actual instruction: eip = fffff804564139c0 > > on the console is of no use trying to identify what went wrong. Dump as much > state as we can to help identify what went wrong. > > Reported-by: Paul Durrant <paul.durrant@citrix.com> > Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com> Acked-by: Brian Woods <brian.woods@amd.com> -- Brian Woods _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 1/2] x86/svm: Improve diagnostics when __get_instruction_length_from_list() fails 2018-11-30 17:07 ` [PATCH 1/2] x86/svm: Improve diagnostics when __get_instruction_length_from_list() fails Andrew Cooper 2018-11-30 17:13 ` Woods, Brian @ 2018-12-03 9:10 ` Paul Durrant 2018-12-03 10:41 ` Jan Beulich 2018-12-03 10:32 ` Jan Beulich 2 siblings, 1 reply; 14+ messages in thread From: Paul Durrant @ 2018-12-03 9:10 UTC (permalink / raw) To: Xen-devel Cc: Wei Liu, Jan Beulich, Andrew Cooper, Suravee Suthikulpanit, Boris Ostrovsky, Brian Woods, Roger Pau Monne > -----Original Message----- > From: Andrew Cooper [mailto:andrew.cooper3@citrix.com] > Sent: 30 November 2018 17:07 > To: Xen-devel <xen-devel@lists.xen.org> > Cc: Andrew Cooper <Andrew.Cooper3@citrix.com>; Jan Beulich > <JBeulich@suse.com>; Wei Liu <wei.liu2@citrix.com>; Roger Pau Monne > <roger.pau@citrix.com>; Paul Durrant <Paul.Durrant@citrix.com>; Boris > Ostrovsky <boris.ostrovsky@oracle.com>; Suravee Suthikulpanit > <suravee.suthikulpanit@amd.com>; Brian Woods <brian.woods@amd.com> > Subject: [PATCH 1/2] x86/svm: Improve diagnostics when > __get_instruction_length_from_list() fails > > Sadly, a lone: > > (XEN) emulate.c:156:d2v0 __get_instruction_length_from_list: Mismatch > between expected and actual instruction: eip = fffff804564139c0 > > on the console is of no use trying to identify what went wrong. Dump as > much > state as we can to help identify what went wrong. > > Reported-by: Paul Durrant <paul.durrant@citrix.com> > Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com> > --- > CC: Jan Beulich <JBeulich@suse.com> > CC: Wei Liu <wei.liu2@citrix.com> > CC: Roger Pau Monné <roger.pau@citrix.com> > CC: Paul Durrant <paul.durrant@citrix.com> > CC: Boris Ostrovsky <boris.ostrovsky@oracle.com> > CC: Suravee Suthikulpanit <suravee.suthikulpanit@amd.com> > CC: Brian Woods <brian.woods@amd.com> > > RFC: __get_instruction_length_from_list() tries to cope with VMEXIT_IOIO, > but > IN/OUT instructions aren't in the decode list and I can't spot an entry > point > from the IOIO path. Am I missing something? Yes, odd. IOIO are handled in the ifdef NDEBUG blocks but an IOIO exit does indeed not call into __get_instruction_length() but does the same calculation inline. > > Also, I'm not entirely convinced that making modrm an annonymous union is > going to work with older CentOS compilers, and therefore am not sure > whether > that part of the change is worth it. The instruction in question can be > obtained from the printed INSN_ constant alone. You could just dump the bitfield values individually. Paul > --- > xen/arch/x86/hvm/svm/emulate.c | 26 +++++++++++++++++++------- > 1 file changed, 19 insertions(+), 7 deletions(-) > > diff --git a/xen/arch/x86/hvm/svm/emulate.c > b/xen/arch/x86/hvm/svm/emulate.c > index 3d04af0..71a1b6e 100644 > --- a/xen/arch/x86/hvm/svm/emulate.c > +++ b/xen/arch/x86/hvm/svm/emulate.c > @@ -56,11 +56,14 @@ static unsigned long svm_nextrip_insn_length(struct > vcpu *v) > > static const struct { > unsigned int opcode; > - struct { > - unsigned int rm:3; > - unsigned int reg:3; > - unsigned int mod:2; > -#define MODRM(mod, reg, rm) { rm, reg, mod } > + union { > + struct { > + unsigned int rm:3; > + unsigned int reg:3; > + unsigned int mod:2; > + }; > + unsigned int raw; > +#define MODRM(mod, reg, rm) {{ rm, reg, mod }} > } modrm; > } opc_tab[INSTR_MAX_COUNT] = { > [INSTR_PAUSE] = { X86EMUL_OPC_F3(0, 0x90) }, > @@ -152,8 +155,17 @@ int __get_instruction_length_from_list(struct vcpu > *v, > } > > gdprintk(XENLOG_WARNING, > - "%s: Mismatch between expected and actual instruction: " > - "eip = %lx\n", __func__, (unsigned long)vmcb->rip); > + "%s: Mismatch between expected and actual instruction:\n", > + __func__); > + gdprintk(XENLOG_WARNING, > + " list[0] val %d, { opc %#x, modrm %#x }, list entries: > %u\n", > + list[0], opc_tab[list[0]].opcode, > opc_tab[list[0]].modrm.raw, > + list_count); > + gdprintk(XENLOG_WARNING, " rip 0x%lx, nextrip 0x%lx, len %lu\n", > + vmcb->rip, vmcb->nextrip, vmcb->nextrip - vmcb->rip); > + hvm_dump_emulation_state(XENLOG_G_WARNING, "Insn_len", > + &ctxt, X86EMUL_UNHANDLEABLE); > + > hvm_inject_hw_exception(TRAP_gp_fault, 0); > return 0; > } > -- > 2.1.4 _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 1/2] x86/svm: Improve diagnostics when __get_instruction_length_from_list() fails 2018-12-03 9:10 ` Paul Durrant @ 2018-12-03 10:41 ` Jan Beulich 0 siblings, 0 replies; 14+ messages in thread From: Jan Beulich @ 2018-12-03 10:41 UTC (permalink / raw) To: andre.przywara, Andrew Cooper, Paul Durrant Cc: Wei Liu, Xen-devel, Suravee Suthikulpanit, Boris Ostrovsky, Brian Woods, Roger Pau Monne >>> On 03.12.18 at 10:10, <Paul.Durrant@citrix.com> wrote: >> From: Andrew Cooper [mailto:andrew.cooper3@citrix.com] >> Sent: 30 November 2018 17:07 >> >> RFC: __get_instruction_length_from_list() tries to cope with VMEXIT_IOIO, >> but >> IN/OUT instructions aren't in the decode list and I can't spot an entry >> point >> from the IOIO path. Am I missing something? > > Yes, odd. IOIO are handled in the ifdef NDEBUG blocks but an IOIO exit does > indeed not call into __get_instruction_length() but does the same calculation > inline. Indeed commit 147808b8b8 introduced this without there being a visible or stated reason. Andre, any chance you can reconstruct the reason from almost 9 years ago? The __get_instruction_length() invocation the change added is VMX specific ... Jan _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 1/2] x86/svm: Improve diagnostics when __get_instruction_length_from_list() fails 2018-11-30 17:07 ` [PATCH 1/2] x86/svm: Improve diagnostics when __get_instruction_length_from_list() fails Andrew Cooper 2018-11-30 17:13 ` Woods, Brian 2018-12-03 9:10 ` Paul Durrant @ 2018-12-03 10:32 ` Jan Beulich 2018-12-03 11:45 ` Andrew Cooper 2 siblings, 1 reply; 14+ messages in thread From: Jan Beulich @ 2018-12-03 10:32 UTC (permalink / raw) To: Andrew Cooper Cc: Wei Liu, Xen-devel, Paul Durrant, Suravee Suthikulpanit, Boris Ostrovsky, Brian Woods, Roger Pau Monne >>> On 30.11.18 at 18:07, <andrew.cooper3@citrix.com> wrote: > Also, I'm not entirely convinced that making modrm an annonymous union is > going to work with older CentOS compilers, It certainly won't. > and therefore am not sure whether > that part of the change is worth it. The instruction in question can be > obtained from the printed INSN_ constant alone. > --- > xen/arch/x86/hvm/svm/emulate.c | 26 +++++++++++++++++++------- > 1 file changed, 19 insertions(+), 7 deletions(-) > > diff --git a/xen/arch/x86/hvm/svm/emulate.c b/xen/arch/x86/hvm/svm/emulate.c > index 3d04af0..71a1b6e 100644 > --- a/xen/arch/x86/hvm/svm/emulate.c > +++ b/xen/arch/x86/hvm/svm/emulate.c > @@ -56,11 +56,14 @@ static unsigned long svm_nextrip_insn_length(struct vcpu *v) > > static const struct { > unsigned int opcode; > - struct { > - unsigned int rm:3; > - unsigned int reg:3; > - unsigned int mod:2; > -#define MODRM(mod, reg, rm) { rm, reg, mod } > + union { > + struct { > + unsigned int rm:3; > + unsigned int reg:3; > + unsigned int mod:2; > + }; > + unsigned int raw; Why unsigned int instead of uint8_t? > @@ -152,8 +155,17 @@ int __get_instruction_length_from_list(struct vcpu *v, > } > > gdprintk(XENLOG_WARNING, > - "%s: Mismatch between expected and actual instruction: " > - "eip = %lx\n", __func__, (unsigned long)vmcb->rip); > + "%s: Mismatch between expected and actual instruction:\n", > + __func__); > + gdprintk(XENLOG_WARNING, > + " list[0] val %d, { opc %#x, modrm %#x }, list entries: %u\n", > + list[0], opc_tab[list[0]].opcode, opc_tab[list[0]].modrm.raw, > + list_count); > + gdprintk(XENLOG_WARNING, " rip 0x%lx, nextrip 0x%lx, len %lu\n", > + vmcb->rip, vmcb->nextrip, vmcb->nextrip - vmcb->rip); > + hvm_dump_emulation_state(XENLOG_G_WARNING, "Insn_len", > + &ctxt, X86EMUL_UNHANDLEABLE); > + > hvm_inject_hw_exception(TRAP_gp_fault, 0); > return 0; > } The gdprintk()s all expanding to nothing in release builds I'm not fully convinced the added verbosity is worth it. In debug builds adding some debugging code like this shouldn't be a big hurdle. In any event %#lx instead of 0x%lx please. Jan _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 1/2] x86/svm: Improve diagnostics when __get_instruction_length_from_list() fails 2018-12-03 10:32 ` Jan Beulich @ 2018-12-03 11:45 ` Andrew Cooper 0 siblings, 0 replies; 14+ messages in thread From: Andrew Cooper @ 2018-12-03 11:45 UTC (permalink / raw) To: Jan Beulich Cc: Wei Liu, Xen-devel, Paul Durrant, Suravee Suthikulpanit, Boris Ostrovsky, Brian Woods, Roger Pau Monne On 03/12/2018 10:32, Jan Beulich wrote: >>>> On 30.11.18 at 18:07, <andrew.cooper3@citrix.com> wrote: >> Also, I'm not entirely convinced that making modrm an annonymous union is >> going to work with older CentOS compilers, > It certainly won't. > >> and therefore am not sure whether >> that part of the change is worth it. The instruction in question can be >> obtained from the printed INSN_ constant alone. >> --- >> xen/arch/x86/hvm/svm/emulate.c | 26 +++++++++++++++++++------- >> 1 file changed, 19 insertions(+), 7 deletions(-) >> >> diff --git a/xen/arch/x86/hvm/svm/emulate.c b/xen/arch/x86/hvm/svm/emulate.c >> index 3d04af0..71a1b6e 100644 >> --- a/xen/arch/x86/hvm/svm/emulate.c >> +++ b/xen/arch/x86/hvm/svm/emulate.c >> @@ -56,11 +56,14 @@ static unsigned long svm_nextrip_insn_length(struct vcpu *v) >> >> static const struct { >> unsigned int opcode; >> - struct { >> - unsigned int rm:3; >> - unsigned int reg:3; >> - unsigned int mod:2; >> -#define MODRM(mod, reg, rm) { rm, reg, mod } >> + union { >> + struct { >> + unsigned int rm:3; >> + unsigned int reg:3; >> + unsigned int mod:2; >> + }; >> + unsigned int raw; > Why unsigned int instead of uint8_t? Because to being with, this was a diagnostic patch and copied the type above without thinking too much. > >> @@ -152,8 +155,17 @@ int __get_instruction_length_from_list(struct vcpu *v, >> } >> >> gdprintk(XENLOG_WARNING, >> - "%s: Mismatch between expected and actual instruction: " >> - "eip = %lx\n", __func__, (unsigned long)vmcb->rip); >> + "%s: Mismatch between expected and actual instruction:\n", >> + __func__); >> + gdprintk(XENLOG_WARNING, >> + " list[0] val %d, { opc %#x, modrm %#x }, list entries: %u\n", >> + list[0], opc_tab[list[0]].opcode, opc_tab[list[0]].modrm.raw, >> + list_count); >> + gdprintk(XENLOG_WARNING, " rip 0x%lx, nextrip 0x%lx, len %lu\n", >> + vmcb->rip, vmcb->nextrip, vmcb->nextrip - vmcb->rip); >> + hvm_dump_emulation_state(XENLOG_G_WARNING, "Insn_len", >> + &ctxt, X86EMUL_UNHANDLEABLE); >> + >> hvm_inject_hw_exception(TRAP_gp_fault, 0); >> return 0; >> } > The gdprintk()s all expanding to nothing in release builds I'm > not fully convinced the added verbosity is worth it. In debug > builds adding some debugging code like this shouldn't be a > big hurdle. You and I know what diagnostics to put here, but I think Pauls reaction to finding that message demonstrates that most others don't. Nor should they - the peculiarities of first-gen AMD hardware needn't be mandatory knowledge for most contributors. For these diagnostics, they are only reachable in debug builds, and this codepath is only even reachable in release builds on first-gen hardware. However, the difference between what is currently present and this is enough information to actually diagnose the problem. ~Andrew _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH 2/2] x86/hvm: Corrections to RDTSCP intercept handling 2018-11-30 17:07 [PATCH 0/2] Fixes to RDTSCP interception Andrew Cooper 2018-11-30 17:07 ` [PATCH 1/2] x86/svm: Improve diagnostics when __get_instruction_length_from_list() fails Andrew Cooper @ 2018-11-30 17:07 ` Andrew Cooper 2018-11-30 17:19 ` Woods, Brian ` (3 more replies) 1 sibling, 4 replies; 14+ messages in thread From: Andrew Cooper @ 2018-11-30 17:07 UTC (permalink / raw) To: Xen-devel Cc: Juergen Gross, Kevin Tian, Wei Liu, Jan Beulich, Andrew Cooper, Paul Durrant, Jun Nakajima, Boris Ostrovsky, Brian Woods, Suravee Suthikulpanit, Roger Pau Monné For both VT-x and SVM, the RDTSCP intercept will trigger if the pipeline supports the instruction, but the guest may have not have rdtscp in its featureset. Bring the vmexit handlers in line with the main emulator behaviour by optionally handing back #UD. Next on the AMD side, if RDTSCP actually ends up being intercepted on a debug build, we first update regs->rcx, then call __get_instruction_length() asking for RDTSC. As the two instructions are different (and indeed, different lengths!), __get_instruction_length_from_list() fails and hands back a #GP fault. This can demonstrated by putting a guest into tsc_mode="always emulate" and executing an rdtscp instruction: (d1) --- Xen Test Framework --- (d1) Environment: HVM 64bit (Long mode 4 levels) (d1) Test rdtscp (d1) TSC mode 1 (XEN) emulate.c:159:d1v0 __get_instruction_length_from_list: Mismatch between expected and actual instruction: (XEN) emulate.c:163:d1v0 list[0] val 8, { opc 0xf0031, modrm 0 }, list entries: 1 (XEN) emulate.c:165:d1v0 rip 0x10475f, nextrip 0x104762, len 3 (XEN) Insn_len emulation failed (1): d1v0 64bit @ 0008:0010475f -> 0f 01 f9 5b 31 ff 31 c0 e9 c4 db ff ff 00 00 00 (d1) ****************************** (d1) PANIC: Unhandled exception at 0008:000000000010475f (d1) Vec 13 #GP[0000] (d1) ****************************** First, teach __get_instruction_length() to cope with RDTSCP, and improve svm_vmexit_do_rdtsc() to ask for the correct instruction. Move the regs->rcx adjustment into this function to ensure it gets done after we are done potentially raising faults. Reported-by: Paul Durrant <paul.durrant@citrix.com> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com> --- CC: Jan Beulich <JBeulich@suse.com> CC: Wei Liu <wei.liu2@citrix.com> CC: Roger Pau Monné <roger.pau@citrix.com> CC: Paul Durrant <paul.durrant@citrix.com> CC: Jun Nakajima <jun.nakajima@intel.com> CC: Kevin Tian <kevin.tian@intel.com> CC: Boris Ostrovsky <boris.ostrovsky@oracle.com> CC: Suravee Suthikulpanit <suravee.suthikulpanit@amd.com> CC: Brian Woods <brian.woods@amd.com> CC: Juergen Gross <jgross@suse.com> There is a further 4.12 bug/regression here. For some reason, master and staging are now defaulting VMs into an emulated TSC mode. I have yet to figure out why. --- xen/arch/x86/hvm/svm/emulate.c | 1 + xen/arch/x86/hvm/svm/svm.c | 22 +++++++++++++++++----- xen/arch/x86/hvm/vmx/vmx.c | 8 ++++++++ xen/include/asm-x86/hvm/svm/emulate.h | 1 + 4 files changed, 27 insertions(+), 5 deletions(-) diff --git a/xen/arch/x86/hvm/svm/emulate.c b/xen/arch/x86/hvm/svm/emulate.c index 71a1b6e..0290264 100644 --- a/xen/arch/x86/hvm/svm/emulate.c +++ b/xen/arch/x86/hvm/svm/emulate.c @@ -78,6 +78,7 @@ static const struct { [INSTR_STGI] = { X86EMUL_OPC(0x0f, 0x01), MODRM(3, 3, 4) }, [INSTR_CLGI] = { X86EMUL_OPC(0x0f, 0x01), MODRM(3, 3, 5) }, [INSTR_INVLPGA] = { X86EMUL_OPC(0x0f, 0x01), MODRM(3, 3, 7) }, + [INSTR_RDTSCP] = { X86EMUL_OPC(0x0f, 0x01), MODRM(3, 7, 1) }, [INSTR_INVD] = { X86EMUL_OPC(0x0f, 0x08) }, [INSTR_WBINVD] = { X86EMUL_OPC(0x0f, 0x09) }, [INSTR_WRMSR] = { X86EMUL_OPC(0x0f, 0x30) }, diff --git a/xen/arch/x86/hvm/svm/svm.c b/xen/arch/x86/hvm/svm/svm.c index b9a8900..d8d3813 100644 --- a/xen/arch/x86/hvm/svm/svm.c +++ b/xen/arch/x86/hvm/svm/svm.c @@ -2279,14 +2279,28 @@ static void svm_vmexit_do_hlt(struct vmcb_struct *vmcb, hvm_hlt(regs->eflags); } -static void svm_vmexit_do_rdtsc(struct cpu_user_regs *regs) +static void svm_vmexit_do_rdtsc(struct cpu_user_regs *regs, bool rdtscp) { + struct vcpu *curr = current; + const struct domain *currd = curr->domain; + enum instruction_index insn = rdtscp ? INSTR_RDTSCP : INSTR_RDTSC; unsigned int inst_len; - if ( (inst_len = __get_instruction_length(current, INSTR_RDTSC)) == 0 ) + if ( rdtscp && !currd->arch.cpuid->extd.rdtscp && + currd->arch.tsc_mode != TSC_MODE_PVRDTSCP ) + { + hvm_inject_hw_exception(TRAP_invalid_op, X86_EVENT_NO_EC); return; + } + + if ( (inst_len = __get_instruction_length(curr, insn)) == 0 ) + return; + __update_guest_eip(regs, inst_len); + if ( rdtscp ) + regs->rcx = hvm_msr_tsc_aux(curr); + hvm_rdtsc_intercept(regs); } @@ -2968,10 +2982,8 @@ void svm_vmexit_handler(struct cpu_user_regs *regs) break; case VMEXIT_RDTSCP: - regs->rcx = hvm_msr_tsc_aux(v); - /* fall through */ case VMEXIT_RDTSC: - svm_vmexit_do_rdtsc(regs); + svm_vmexit_do_rdtsc(regs, exit_reason == VMEXIT_RDTSCP); break; case VMEXIT_MONITOR: diff --git a/xen/arch/x86/hvm/vmx/vmx.c b/xen/arch/x86/hvm/vmx/vmx.c index 365eeb2..a9f9b9b 100644 --- a/xen/arch/x86/hvm/vmx/vmx.c +++ b/xen/arch/x86/hvm/vmx/vmx.c @@ -3589,6 +3589,7 @@ void vmx_vmexit_handler(struct cpu_user_regs *regs) unsigned long exit_qualification, exit_reason, idtv_info, intr_info = 0; unsigned int vector = 0, mode; struct vcpu *v = current; + struct domain *currd = v->domain; __vmread(GUEST_RIP, ®s->rip); __vmread(GUEST_RSP, ®s->rsp); @@ -3956,6 +3957,13 @@ void vmx_vmexit_handler(struct cpu_user_regs *regs) vmx_invlpg_intercept(exit_qualification); break; case EXIT_REASON_RDTSCP: + if ( !currd->arch.cpuid->extd.rdtscp && + currd->arch.tsc_mode != TSC_MODE_PVRDTSCP ) + { + hvm_inject_hw_exception(TRAP_invalid_op, X86_EVENT_NO_EC); + break; + } + regs->rcx = hvm_msr_tsc_aux(v); /* fall through */ case EXIT_REASON_RDTSC: diff --git a/xen/include/asm-x86/hvm/svm/emulate.h b/xen/include/asm-x86/hvm/svm/emulate.h index 3de8236..ca92abb 100644 --- a/xen/include/asm-x86/hvm/svm/emulate.h +++ b/xen/include/asm-x86/hvm/svm/emulate.h @@ -30,6 +30,7 @@ enum instruction_index { INSTR_HLT, INSTR_INT3, INSTR_RDTSC, + INSTR_RDTSCP, INSTR_PAUSE, INSTR_XSETBV, INSTR_VMRUN, -- 2.1.4 _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH 2/2] x86/hvm: Corrections to RDTSCP intercept handling 2018-11-30 17:07 ` [PATCH 2/2] x86/hvm: Corrections to RDTSCP intercept handling Andrew Cooper @ 2018-11-30 17:19 ` Woods, Brian 2018-12-03 9:17 ` Paul Durrant ` (2 subsequent siblings) 3 siblings, 0 replies; 14+ messages in thread From: Woods, Brian @ 2018-11-30 17:19 UTC (permalink / raw) To: Andrew Cooper Cc: Juergen Gross, Kevin Tian, Wei Liu, Jan Beulich, Xen-devel, Paul Durrant, Jun Nakajima, Boris Ostrovsky, Woods, Brian, Suthikulpanit, Suravee, Roger Pau Monné On Fri, Nov 30, 2018 at 05:07:20PM +0000, Andy Cooper wrote: > For both VT-x and SVM, the RDTSCP intercept will trigger if the pipeline > supports the instruction, but the guest may have not have rdtscp in its > featureset. Bring the vmexit handlers in line with the main emulator > behaviour by optionally handing back #UD. > > Next on the AMD side, if RDTSCP actually ends up being intercepted on a debug > build, we first update regs->rcx, then call __get_instruction_length() asking > for RDTSC. As the two instructions are different (and indeed, different > lengths!), __get_instruction_length_from_list() fails and hands back a #GP > fault. > > This can demonstrated by putting a guest into tsc_mode="always emulate" and > executing an rdtscp instruction: > > (d1) --- Xen Test Framework --- > (d1) Environment: HVM 64bit (Long mode 4 levels) > (d1) Test rdtscp > (d1) TSC mode 1 > (XEN) emulate.c:159:d1v0 __get_instruction_length_from_list: Mismatch between expected and actual instruction: > (XEN) emulate.c:163:d1v0 list[0] val 8, { opc 0xf0031, modrm 0 }, list entries: 1 > (XEN) emulate.c:165:d1v0 rip 0x10475f, nextrip 0x104762, len 3 > (XEN) Insn_len emulation failed (1): d1v0 64bit @ 0008:0010475f -> 0f 01 f9 5b 31 ff 31 c0 e9 c4 db ff ff 00 00 00 > (d1) ****************************** > (d1) PANIC: Unhandled exception at 0008:000000000010475f > (d1) Vec 13 #GP[0000] > (d1) ****************************** > > First, teach __get_instruction_length() to cope with RDTSCP, and improve > svm_vmexit_do_rdtsc() to ask for the correct instruction. Move the regs->rcx > adjustment into this function to ensure it gets done after we are done > potentially raising faults. > > Reported-by: Paul Durrant <paul.durrant@citrix.com> > Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com> As far as the SVM part: Reviewed-by: Brian Woods <brian.woods@amd.com> -- Brian Woods _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 2/2] x86/hvm: Corrections to RDTSCP intercept handling 2018-11-30 17:07 ` [PATCH 2/2] x86/hvm: Corrections to RDTSCP intercept handling Andrew Cooper 2018-11-30 17:19 ` Woods, Brian @ 2018-12-03 9:17 ` Paul Durrant 2018-12-03 11:13 ` Andrew Cooper 2018-12-03 10:47 ` Jan Beulich 2018-12-13 6:24 ` Tian, Kevin 3 siblings, 1 reply; 14+ messages in thread From: Paul Durrant @ 2018-12-03 9:17 UTC (permalink / raw) To: Xen-devel Cc: Juergen Gross, Kevin Tian, Wei Liu, Jan Beulich, Andrew Cooper, Jun Nakajima, Boris Ostrovsky, Brian Woods, Suravee Suthikulpanit, Roger Pau Monne > -----Original Message----- > From: Andrew Cooper [mailto:andrew.cooper3@citrix.com] > Sent: 30 November 2018 17:07 > To: Xen-devel <xen-devel@lists.xen.org> > Cc: Andrew Cooper <Andrew.Cooper3@citrix.com>; Jan Beulich > <JBeulich@suse.com>; Wei Liu <wei.liu2@citrix.com>; Roger Pau Monne > <roger.pau@citrix.com>; Paul Durrant <Paul.Durrant@citrix.com>; Jun > Nakajima <jun.nakajima@intel.com>; Kevin Tian <kevin.tian@intel.com>; > Boris Ostrovsky <boris.ostrovsky@oracle.com>; Suravee Suthikulpanit > <suravee.suthikulpanit@amd.com>; Brian Woods <brian.woods@amd.com>; > Juergen Gross <jgross@suse.com> > Subject: [PATCH 2/2] x86/hvm: Corrections to RDTSCP intercept handling > > For both VT-x and SVM, the RDTSCP intercept will trigger if the pipeline > supports the instruction, but the guest may have not have rdtscp in its > featureset. Bring the vmexit handlers in line with the main emulator > behaviour by optionally handing back #UD. > > Next on the AMD side, if RDTSCP actually ends up being intercepted on a > debug > build, we first update regs->rcx, then call __get_instruction_length() > asking > for RDTSC. As the two instructions are different (and indeed, different > lengths!), __get_instruction_length_from_list() fails and hands back a #GP > fault. > > This can demonstrated by putting a guest into tsc_mode="always emulate" > and > executing an rdtscp instruction: > > (d1) --- Xen Test Framework --- > (d1) Environment: HVM 64bit (Long mode 4 levels) > (d1) Test rdtscp > (d1) TSC mode 1 > (XEN) emulate.c:159:d1v0 __get_instruction_length_from_list: Mismatch > between expected and actual instruction: > (XEN) emulate.c:163:d1v0 list[0] val 8, { opc 0xf0031, modrm 0 }, list > entries: 1 > (XEN) emulate.c:165:d1v0 rip 0x10475f, nextrip 0x104762, len 3 > (XEN) Insn_len emulation failed (1): d1v0 64bit @ 0008:0010475f -> 0f 01 > f9 5b 31 ff 31 c0 e9 c4 db ff ff 00 00 00 > (d1) ****************************** > (d1) PANIC: Unhandled exception at 0008:000000000010475f > (d1) Vec 13 #GP[0000] > (d1) ****************************** > > First, teach __get_instruction_length() to cope with RDTSCP, and improve > svm_vmexit_do_rdtsc() to ask for the correct instruction. Move the regs- > >rcx > adjustment into this function to ensure it gets done after we are done > potentially raising faults. > > Reported-by: Paul Durrant <paul.durrant@citrix.com> > Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com> Reviewed-by: Paul Durrant <paul.durrant@citrix.com> With one observation... > --- > CC: Jan Beulich <JBeulich@suse.com> > CC: Wei Liu <wei.liu2@citrix.com> > CC: Roger Pau Monné <roger.pau@citrix.com> > CC: Paul Durrant <paul.durrant@citrix.com> > CC: Jun Nakajima <jun.nakajima@intel.com> > CC: Kevin Tian <kevin.tian@intel.com> > CC: Boris Ostrovsky <boris.ostrovsky@oracle.com> > CC: Suravee Suthikulpanit <suravee.suthikulpanit@amd.com> > CC: Brian Woods <brian.woods@amd.com> > CC: Juergen Gross <jgross@suse.com> > > There is a further 4.12 bug/regression here. For some reason, master and > staging are now defaulting VMs into an emulated TSC mode. I have yet to > figure out why. > --- > xen/arch/x86/hvm/svm/emulate.c | 1 + > xen/arch/x86/hvm/svm/svm.c | 22 +++++++++++++++++----- > xen/arch/x86/hvm/vmx/vmx.c | 8 ++++++++ > xen/include/asm-x86/hvm/svm/emulate.h | 1 + > 4 files changed, 27 insertions(+), 5 deletions(-) > > diff --git a/xen/arch/x86/hvm/svm/emulate.c > b/xen/arch/x86/hvm/svm/emulate.c > index 71a1b6e..0290264 100644 > --- a/xen/arch/x86/hvm/svm/emulate.c > +++ b/xen/arch/x86/hvm/svm/emulate.c > @@ -78,6 +78,7 @@ static const struct { > [INSTR_STGI] = { X86EMUL_OPC(0x0f, 0x01), MODRM(3, 3, 4) }, > [INSTR_CLGI] = { X86EMUL_OPC(0x0f, 0x01), MODRM(3, 3, 5) }, > [INSTR_INVLPGA] = { X86EMUL_OPC(0x0f, 0x01), MODRM(3, 3, 7) }, > + [INSTR_RDTSCP] = { X86EMUL_OPC(0x0f, 0x01), MODRM(3, 7, 1) }, > [INSTR_INVD] = { X86EMUL_OPC(0x0f, 0x08) }, > [INSTR_WBINVD] = { X86EMUL_OPC(0x0f, 0x09) }, > [INSTR_WRMSR] = { X86EMUL_OPC(0x0f, 0x30) }, > diff --git a/xen/arch/x86/hvm/svm/svm.c b/xen/arch/x86/hvm/svm/svm.c > index b9a8900..d8d3813 100644 > --- a/xen/arch/x86/hvm/svm/svm.c > +++ b/xen/arch/x86/hvm/svm/svm.c > @@ -2279,14 +2279,28 @@ static void svm_vmexit_do_hlt(struct vmcb_struct > *vmcb, > hvm_hlt(regs->eflags); > } > > -static void svm_vmexit_do_rdtsc(struct cpu_user_regs *regs) > +static void svm_vmexit_do_rdtsc(struct cpu_user_regs *regs, bool rdtscp) > { > + struct vcpu *curr = current; > + const struct domain *currd = curr->domain; > + enum instruction_index insn = rdtscp ? INSTR_RDTSCP : INSTR_RDTSC; > unsigned int inst_len; > > - if ( (inst_len = __get_instruction_length(current, INSTR_RDTSC)) == 0 > ) > + if ( rdtscp && !currd->arch.cpuid->extd.rdtscp && > + currd->arch.tsc_mode != TSC_MODE_PVRDTSCP ) > + { > + hvm_inject_hw_exception(TRAP_invalid_op, X86_EVENT_NO_EC); > return; > + } > + > + if ( (inst_len = __get_instruction_length(curr, insn)) == 0 ) > + return; > + > __update_guest_eip(regs, inst_len); > > + if ( rdtscp ) > + regs->rcx = hvm_msr_tsc_aux(curr); > + > hvm_rdtsc_intercept(regs); > } > > @@ -2968,10 +2982,8 @@ void svm_vmexit_handler(struct cpu_user_regs *regs) > break; > > case VMEXIT_RDTSCP: > - regs->rcx = hvm_msr_tsc_aux(v); > - /* fall through */ > case VMEXIT_RDTSC: > - svm_vmexit_do_rdtsc(regs); > + svm_vmexit_do_rdtsc(regs, exit_reason == VMEXIT_RDTSCP); > break; > > case VMEXIT_MONITOR: > diff --git a/xen/arch/x86/hvm/vmx/vmx.c b/xen/arch/x86/hvm/vmx/vmx.c > index 365eeb2..a9f9b9b 100644 > --- a/xen/arch/x86/hvm/vmx/vmx.c > +++ b/xen/arch/x86/hvm/vmx/vmx.c > @@ -3589,6 +3589,7 @@ void vmx_vmexit_handler(struct cpu_user_regs *regs) > unsigned long exit_qualification, exit_reason, idtv_info, intr_info = > 0; > unsigned int vector = 0, mode; > struct vcpu *v = current; > + struct domain *currd = v->domain; ... following the usual rules, you should now convert all uses of v->domain in this function to use currd. > > __vmread(GUEST_RIP, ®s->rip); > __vmread(GUEST_RSP, ®s->rsp); > @@ -3956,6 +3957,13 @@ void vmx_vmexit_handler(struct cpu_user_regs *regs) > vmx_invlpg_intercept(exit_qualification); > break; > case EXIT_REASON_RDTSCP: > + if ( !currd->arch.cpuid->extd.rdtscp && > + currd->arch.tsc_mode != TSC_MODE_PVRDTSCP ) > + { > + hvm_inject_hw_exception(TRAP_invalid_op, X86_EVENT_NO_EC); > + break; > + } > + > regs->rcx = hvm_msr_tsc_aux(v); > /* fall through */ > case EXIT_REASON_RDTSC: > diff --git a/xen/include/asm-x86/hvm/svm/emulate.h b/xen/include/asm- > x86/hvm/svm/emulate.h > index 3de8236..ca92abb 100644 > --- a/xen/include/asm-x86/hvm/svm/emulate.h > +++ b/xen/include/asm-x86/hvm/svm/emulate.h > @@ -30,6 +30,7 @@ enum instruction_index { > INSTR_HLT, > INSTR_INT3, > INSTR_RDTSC, > + INSTR_RDTSCP, > INSTR_PAUSE, > INSTR_XSETBV, > INSTR_VMRUN, > -- > 2.1.4 _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 2/2] x86/hvm: Corrections to RDTSCP intercept handling 2018-12-03 9:17 ` Paul Durrant @ 2018-12-03 11:13 ` Andrew Cooper 2018-12-03 11:23 ` Paul Durrant 0 siblings, 1 reply; 14+ messages in thread From: Andrew Cooper @ 2018-12-03 11:13 UTC (permalink / raw) To: Paul Durrant, Xen-devel Cc: Juergen Gross, Kevin Tian, Wei Liu, Jan Beulich, Jun Nakajima, Boris Ostrovsky, Brian Woods, Suravee Suthikulpanit, Roger Pau Monne On 03/12/2018 09:17, Paul Durrant wrote: >> diff --git a/xen/arch/x86/hvm/vmx/vmx.c b/xen/arch/x86/hvm/vmx/vmx.c >> index 365eeb2..a9f9b9b 100644 >> --- a/xen/arch/x86/hvm/vmx/vmx.c >> +++ b/xen/arch/x86/hvm/vmx/vmx.c >> @@ -3589,6 +3589,7 @@ void vmx_vmexit_handler(struct cpu_user_regs *regs) >> unsigned long exit_qualification, exit_reason, idtv_info, intr_info = >> 0; >> unsigned int vector = 0, mode; >> struct vcpu *v = current; >> + struct domain *currd = v->domain; > ... following the usual rules, you should now convert all uses of v->domain in this function to use currd. If this were new development work then perhaps (although it would take a series to clean vmx_vmexit_handler() up to style). In this case however, the patch needs backporting to all the stable trees, at which point minimum perturbance is the most important aspect. ~Andrew _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 2/2] x86/hvm: Corrections to RDTSCP intercept handling 2018-12-03 11:13 ` Andrew Cooper @ 2018-12-03 11:23 ` Paul Durrant 0 siblings, 0 replies; 14+ messages in thread From: Paul Durrant @ 2018-12-03 11:23 UTC (permalink / raw) To: Andrew Cooper, Xen-devel Cc: Juergen Gross, Kevin Tian, Wei Liu, Jan Beulich, Jun Nakajima, Boris Ostrovsky, Brian Woods, Suravee Suthikulpanit, Roger Pau Monne > -----Original Message----- > From: Andrew Cooper > Sent: 03 December 2018 11:14 > To: Paul Durrant <Paul.Durrant@citrix.com>; Xen-devel <xen- > devel@lists.xen.org> > Cc: Jan Beulich <JBeulich@suse.com>; Wei Liu <wei.liu2@citrix.com>; Roger > Pau Monne <roger.pau@citrix.com>; Jun Nakajima <jun.nakajima@intel.com>; > Kevin Tian <kevin.tian@intel.com>; Boris Ostrovsky > <boris.ostrovsky@oracle.com>; Suravee Suthikulpanit > <suravee.suthikulpanit@amd.com>; Brian Woods <brian.woods@amd.com>; > Juergen Gross <jgross@suse.com> > Subject: Re: [PATCH 2/2] x86/hvm: Corrections to RDTSCP intercept handling > > On 03/12/2018 09:17, Paul Durrant wrote: > >> diff --git a/xen/arch/x86/hvm/vmx/vmx.c b/xen/arch/x86/hvm/vmx/vmx.c > >> index 365eeb2..a9f9b9b 100644 > >> --- a/xen/arch/x86/hvm/vmx/vmx.c > >> +++ b/xen/arch/x86/hvm/vmx/vmx.c > >> @@ -3589,6 +3589,7 @@ void vmx_vmexit_handler(struct cpu_user_regs > *regs) > >> unsigned long exit_qualification, exit_reason, idtv_info, > intr_info = > >> 0; > >> unsigned int vector = 0, mode; > >> struct vcpu *v = current; > >> + struct domain *currd = v->domain; > > ... following the usual rules, you should now convert all uses of v- > >domain in this function to use currd. > > If this were new development work then perhaps (although it would take a > series to clean vmx_vmexit_handler() up to style). > > In this case however, the patch needs backporting to all the stable > trees, at which point minimum perturbance is the most important aspect. Fair enough. Paul > > ~Andrew _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 2/2] x86/hvm: Corrections to RDTSCP intercept handling 2018-11-30 17:07 ` [PATCH 2/2] x86/hvm: Corrections to RDTSCP intercept handling Andrew Cooper 2018-11-30 17:19 ` Woods, Brian 2018-12-03 9:17 ` Paul Durrant @ 2018-12-03 10:47 ` Jan Beulich 2018-12-13 6:24 ` Tian, Kevin 3 siblings, 0 replies; 14+ messages in thread From: Jan Beulich @ 2018-12-03 10:47 UTC (permalink / raw) To: Andrew Cooper Cc: Juergen Gross, Kevin Tian, Wei Liu, Suravee Suthikulpanit, Xen-devel, Paul Durrant, Jun Nakajima, Boris Ostrovsky, Brian Woods, Roger Pau Monne >>> On 30.11.18 at 18:07, <andrew.cooper3@citrix.com> wrote: > For both VT-x and SVM, the RDTSCP intercept will trigger if the pipeline > supports the instruction, but the guest may have not have rdtscp in its > featureset. Bring the vmexit handlers in line with the main emulator > behaviour by optionally handing back #UD. > > Next on the AMD side, if RDTSCP actually ends up being intercepted on a debug > build, we first update regs->rcx, then call __get_instruction_length() asking > for RDTSC. As the two instructions are different (and indeed, different > lengths!), __get_instruction_length_from_list() fails and hands back a #GP > fault. > > This can demonstrated by putting a guest into tsc_mode="always emulate" and > executing an rdtscp instruction: > > (d1) --- Xen Test Framework --- > (d1) Environment: HVM 64bit (Long mode 4 levels) > (d1) Test rdtscp > (d1) TSC mode 1 > (XEN) emulate.c:159:d1v0 __get_instruction_length_from_list: Mismatch between expected and actual instruction: > (XEN) emulate.c:163:d1v0 list[0] val 8, { opc 0xf0031, modrm 0 }, list entries: 1 > (XEN) emulate.c:165:d1v0 rip 0x10475f, nextrip 0x104762, len 3 > (XEN) Insn_len emulation failed (1): d1v0 64bit @ 0008:0010475f -> 0f 01 f9 5b 31 ff 31 c0 e9 c4 db ff ff 00 00 00 > (d1) ****************************** > (d1) PANIC: Unhandled exception at 0008:000000000010475f > (d1) Vec 13 #GP[0000] > (d1) ****************************** > > First, teach __get_instruction_length() to cope with RDTSCP, and improve > svm_vmexit_do_rdtsc() to ask for the correct instruction. Move the regs->rcx > adjustment into this function to ensure it gets done after we are done > potentially raising faults. > > Reported-by: Paul Durrant <paul.durrant@citrix.com> > Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com> Reviewed-by: Jan Beulich <jbeulich@suse.com> with or without Paul's additional remark addressed. Jan _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 2/2] x86/hvm: Corrections to RDTSCP intercept handling 2018-11-30 17:07 ` [PATCH 2/2] x86/hvm: Corrections to RDTSCP intercept handling Andrew Cooper ` (2 preceding siblings ...) 2018-12-03 10:47 ` Jan Beulich @ 2018-12-13 6:24 ` Tian, Kevin 3 siblings, 0 replies; 14+ messages in thread From: Tian, Kevin @ 2018-12-13 6:24 UTC (permalink / raw) To: Andrew Cooper, Xen-devel Cc: Juergen Gross, Wei Liu, Jan Beulich, Paul Durrant, Nakajima, Jun, Boris Ostrovsky, Brian Woods, Suravee Suthikulpanit, Roger Pau Monné > From: Andrew Cooper [mailto:andrew.cooper3@citrix.com] > Sent: Saturday, December 1, 2018 1:07 AM > > For both VT-x and SVM, the RDTSCP intercept will trigger if the pipeline > supports the instruction, but the guest may have not have rdtscp in its > featureset. Bring the vmexit handlers in line with the main emulator > behaviour by optionally handing back #UD. > > Next on the AMD side, if RDTSCP actually ends up being intercepted on a > debug > build, we first update regs->rcx, then call __get_instruction_length() asking > for RDTSC. As the two instructions are different (and indeed, different > lengths!), __get_instruction_length_from_list() fails and hands back a #GP > fault. > > This can demonstrated by putting a guest into tsc_mode="always emulate" > and > executing an rdtscp instruction: > > (d1) --- Xen Test Framework --- > (d1) Environment: HVM 64bit (Long mode 4 levels) > (d1) Test rdtscp > (d1) TSC mode 1 > (XEN) emulate.c:159:d1v0 __get_instruction_length_from_list: Mismatch > between expected and actual instruction: > (XEN) emulate.c:163:d1v0 list[0] val 8, { opc 0xf0031, modrm 0 }, list > entries: 1 > (XEN) emulate.c:165:d1v0 rip 0x10475f, nextrip 0x104762, len 3 > (XEN) Insn_len emulation failed (1): d1v0 64bit @ 0008:0010475f -> 0f 01 > f9 5b 31 ff 31 c0 e9 c4 db ff ff 00 00 00 > (d1) ****************************** > (d1) PANIC: Unhandled exception at 0008:000000000010475f > (d1) Vec 13 #GP[0000] > (d1) ****************************** > > First, teach __get_instruction_length() to cope with RDTSCP, and improve > svm_vmexit_do_rdtsc() to ask for the correct instruction. Move the regs- > >rcx > adjustment into this function to ensure it gets done after we are done > potentially raising faults. > > Reported-by: Paul Durrant <paul.durrant@citrix.com> > Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com> Reviewed-by: Kevin Tian <kevin.tian@intel.com> _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply [flat|nested] 14+ messages in thread
end of thread, other threads:[~2018-12-13 6:24 UTC | newest] Thread overview: 14+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2018-11-30 17:07 [PATCH 0/2] Fixes to RDTSCP interception Andrew Cooper 2018-11-30 17:07 ` [PATCH 1/2] x86/svm: Improve diagnostics when __get_instruction_length_from_list() fails Andrew Cooper 2018-11-30 17:13 ` Woods, Brian 2018-12-03 9:10 ` Paul Durrant 2018-12-03 10:41 ` Jan Beulich 2018-12-03 10:32 ` Jan Beulich 2018-12-03 11:45 ` Andrew Cooper 2018-11-30 17:07 ` [PATCH 2/2] x86/hvm: Corrections to RDTSCP intercept handling Andrew Cooper 2018-11-30 17:19 ` Woods, Brian 2018-12-03 9:17 ` Paul Durrant 2018-12-03 11:13 ` Andrew Cooper 2018-12-03 11:23 ` Paul Durrant 2018-12-03 10:47 ` Jan Beulich 2018-12-13 6:24 ` Tian, Kevin
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.