[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [PATCH 2/2] x86/hvm: Corrections to RDTSCP intercept handling
> -----Original Message----- > From: Andrew Cooper [mailto:andrew.cooper3@xxxxxxxxxx] > Sent: 30 November 2018 17:07 > To: Xen-devel <xen-devel@xxxxxxxxxxxxx> > Cc: Andrew Cooper <Andrew.Cooper3@xxxxxxxxxx>; Jan Beulich > <JBeulich@xxxxxxxx>; Wei Liu <wei.liu2@xxxxxxxxxx>; Roger Pau Monne > <roger.pau@xxxxxxxxxx>; Paul Durrant <Paul.Durrant@xxxxxxxxxx>; Jun > Nakajima <jun.nakajima@xxxxxxxxx>; Kevin Tian <kevin.tian@xxxxxxxxx>; > Boris Ostrovsky <boris.ostrovsky@xxxxxxxxxx>; Suravee Suthikulpanit > <suravee.suthikulpanit@xxxxxxx>; Brian Woods <brian.woods@xxxxxxx>; > Juergen Gross <jgross@xxxxxxxx> > 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@xxxxxxxxxx> > Signed-off-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx> Reviewed-by: Paul Durrant <paul.durrant@xxxxxxxxxx> With one observation... > --- > CC: Jan Beulich <JBeulich@xxxxxxxx> > CC: Wei Liu <wei.liu2@xxxxxxxxxx> > CC: Roger Pau Monné <roger.pau@xxxxxxxxxx> > CC: Paul Durrant <paul.durrant@xxxxxxxxxx> > CC: Jun Nakajima <jun.nakajima@xxxxxxxxx> > CC: Kevin Tian <kevin.tian@xxxxxxxxx> > CC: Boris Ostrovsky <boris.ostrovsky@xxxxxxxxxx> > CC: Suravee Suthikulpanit <suravee.suthikulpanit@xxxxxxx> > CC: Brian Woods <brian.woods@xxxxxxx> > CC: Juergen Gross <jgross@xxxxxxxx> > > 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@xxxxxxxxxxxxxxxxxxxx https://lists.xenproject.org/mailman/listinfo/xen-devel
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |