Re: [Xen-devel] [PATCH v2 3/3] x86/hvm: Corrections to RDTSCP intercept handling

[Roger Pau Monné <roger.pau@xxxxxxxxxx>]

On Thu, Dec 13, 2018 at 08:22:52PM +0000, Andrew 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
                                              ^ extra have
> 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:147:d1v0 svm_get_insn_len: Mismatch between expected and 
> actual instruction:
>   (XEN) emulate.c:152:d1v0   insn_index 8, opcode 0xf0031 modrm 0
>   (XEN) emulate.c:154:d1v0   rip 0x10475f, nextrip 0x104762, len 3
>   (XEN) SVM insn len emulation failed (1): d1v0 64bit @ 0008:0010475f -> 0f 
> 01 f9 0f 31 5b 31 ff 31 c0 e9 c2 db ff ff 00
>   (d1) ******************************
>   (d1) PANIC: Unhandled exception at 0008:000000000010475f
>   (d1) Vec 13 #GP[0000]
>   (d1) ******************************
> 
> First, teach svm_get_insn_len() 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: Brian Woods <brian.woods@xxxxxxx>
> Reviewed-by: Jan Beulich <jbeulich@xxxxxxxx>
> Reviewed-by: Kevin Tian <kevin.tian@xxxxxxxxx>
> ---
> diff --git a/xen/arch/x86/hvm/vmx/vmx.c b/xen/arch/x86/hvm/vmx/vmx.c
> index 7f77d1f..2166b0d 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;

Given the usage of currd below I think this could be constified.

Reviewed-by: Roger Pau Monné <roger.pau@xxxxxxxxxx>

Thanks, Roger.

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel