Re: [Xen-devel] [PATCH 3/4] x86/msr: Handle MSR_TSC_AUX consistently for PV and HVM guests

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 15.11.18 at 22:47, <andrew.cooper3@xxxxxxxxxx> wrote:
> @@ -1040,7 +1040,10 @@ static int hvm_load_cpu_ctxt(struct domain *d, 
> hvm_domain_context_t *h)
>      if ( hvm_funcs.tsc_scaling.setup )
>          hvm_funcs.tsc_scaling.setup(v);
>  
> -    v->arch.hvm.msr_tsc_aux = ctxt.msr_tsc_aux;
> +    if ( ctxt.msr_tsc_aux != (uint32_t)ctxt.msr_tsc_aux )
> +        return -EINVAL;
> +
> +    v->arch.msrs->tsc_aux = ctxt.msr_tsc_aux;

The check (but not the setting of the value) wants to move up,
next to the other error returns. We should at least try to avoid
(where possible) failures getting reported after part of the state
was already modified.

> --- a/xen/arch/x86/hvm/vmx/vmx.c
> +++ b/xen/arch/x86/hvm/vmx/vmx.c
> @@ -512,7 +512,7 @@ static void vmx_restore_guest_msrs(struct vcpu *v)
>      wrmsrl(MSR_SYSCALL_MASK,   v->arch.hvm.vmx.sfmask);
>  
>      if ( cpu_has_rdtscp )
> -        wrmsr_tsc_aux(hvm_msr_tsc_aux(v));
> +        wrmsr_tsc_aux(v->arch.msrs->tsc_aux);

Any reason you don't also add an RDPID feature check here?

With the first issue taken care of and the second at least
explained,
Reviewed-by: Jan Beulich <jbeulich@xxxxxxxx>

Jan



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