On 13.03.2024 13:24, George Dunlap wrote:
> The primary purpose of TSC scaling, from our perspective, is to
> maintain the fiction of an "invariant TSC" across migrates between
> platforms with different clock speeds.
>
> On AMD, the TscRateMSR CPUID bit is unconditionally enabled in the
> "host cpuid", even if the hardware doesn't actually support it.
> According to c/s fd14a1943c4 ("nestedsvm: Support TSC Rate MSR"),
> testing showed that emulating TSC scaling in an L1 was more expensive
> than emulating TSC scaling on an L0 (due to extra sets of vmexit /
> vmenter).
>
> However, the current implementation seems to be broken.
>
> First of all, the final L2 scaling ratio should be a composition of
> the L0 scaling ratio and the L1 scaling ratio; there's no indication
> this is being done anywhere.
>
> Secondly, it's not clear that the L1 tsc scaling ratio actually
> affects the L0 tsc scaling ratio. The stored value (ns_tscratio) is
> used to affect the tsc *offset*, but doesn't seem to actually be
> factored into d->hvm.tsc_scaling_ratio. (Which shouldn't be
> per-domain anyway, but per-vcpu.) Having the *offset* scaled
> according to the nested scaling without the actual RDTSC itself also
> being scaled has got to produce inconsistent results.
>
> For now, just disable the functionality entirely until we can
> implement it properly:
>
> - Don't set TSCRATEMSR in the host CPUID policy
"host" is stale here; it's "HVM max" now.
> - Remove MSR_AMD64_TSC_RATIO emulation handling, so that the guest
> guests a #GP if it tries to access them (as it should when
> TSCRATEMSR is clear)
>
> - Remove ns_tscratio from struct nestedhvm, and all code that touches
> it
>
> Unfortunately this means ripping out the scaling calculation stuff as
> well, since it's only used in the nested case; it's there in the git
> tree if we need it for reference when we re-introduce it.
>
> Signed-off-by: George Dunlap <george.dun...@cloud.com>
Acked-by: Jan Beulich <jbeul...@suse.com>
Jan
