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

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 20.11.18 at 15:37, <andrew.cooper3@xxxxxxxxxx> wrote:
> With PVRDTSCP mode removed, handling of MSR_TSC_AUX can move into the common
> code.  Move its storage into struct vcpu_msrs (dropping the HVM-specific
> msr_tsc_aux), and add an RDPID feature check as this bit also enumerates the
> presence of the MSR.
> 
> Introduce cpu_has_rdpid along with the synthesized cpu_has_msr_tsc_aux to
> correct the context switch paths, as MSR_TSC_AUX is enumerated by either
> RDTSCP or RDPID.
> 
> Drop hvm_msr_tsc_aux() entirely, and use v->arch.msrs->tsc_aux directly.
> Update hvm_load_cpu_ctxt() to check that the incoming ctxt.msr_tsc_aux isn't
> out of range.  In practice, no previous version of Xen ever wrote an
> out-of-range value.  Add MSR_TSC_AUX to the list of MSRs migrated for PV
> guests, but leave the HVM path using the existing space in hvm_hw_cpu.
> 
> Signed-off-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
> ---
> CC: Jan Beulich <JBeulich@xxxxxxxx>
> CC: Wei Liu <wei.liu2@xxxxxxxxxx>
> CC: Roger Pau Monné <roger.pau@xxxxxxxxxx>
> CC: Konrad Rzeszutek Wilk <konrad.wilk@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>
> 
> v2:
>  * Rebase over "x86/msr: Handle MSR_AMD64_DR{0-3}_ADDRESS_MASK in the new 
> MSR infrastructure"
>  * Move the HVM msr_tsc_aux check earlier in hvm_load_cpu_ctxt()
>  * Introduce cpu_has_msr_tsc_aux
> 
> RFC: I'm not overly happy with cpu_has_msr_tsc_aux because in practice all
> hardware with rdpid has rdtscp, making this an effectively dead conditional in
> the context switch path.

Except for virtualized environments, where features can artificially
be made absent. Otherwise I certainly would be fine with ...

>  I'm tempted to go with
> 
>   #define cpu_has_msr_tsc_aux     (cpu_has_rdtscp /* || cpu_has_rdpid */)
> 
> to get the point across, but without the extra jump.

... this. Hence, however,
Reviewed-by: Jan Beulich <jbeulich@xxxxxxxx>
for the patch as it is now. A reasonable compiler should be able to
convert the || into | and a single branch anyway.

Jan


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