Re: [PATCH v15 09/13] tsc: Use the GUEST_TSC_FREQ MSR for discovering TSC frequency

["Nikunj A. Dadhania" <nikunj@amd.com>]

On 1/3/2025 5:34 PM, Borislav Petkov wrote:
> On Thu, Jan 02, 2025 at 06:40:38PM +0530, Nikunj A. Dadhania wrote:

>> ○ Modern CPU/VMs: VMs running on platforms supporting constant, non-stop and reliable TSC
> 
> Modern?

Meaning platforms/CPU that support SNP/TDX, they have constant, non-stop and invariant TSC.

> What guarantees do you have on "modern" setups that the HV has no control over
> the TSC MSRs? None.

None.

But, non-secure guests uses the regular TSC when the guest is booted with 
TscInvariant bit set, although it doesn't switch the sched clock and
tsc calibration. The guest initially picks up kvm-clock instead 
of tsc-early as it was registered earlier and both the clocks have the same 
clock rating(299). But at a later point in time clocksource switches to 
regular TSC(clock rating 300) from kvm-clock

[    0.000000] kvm-clock: Using msrs 4b564d01 and 4b564d00
[    0.000001] kvm-clock: using sched offset of 1799357702246960 cycles
[    0.001493] clocksource: kvm-clock: mask: 0xffffffffffffffff max_cycles: 0x1cd42e4dffb, max_idle_ns: 881590591483 ns
[    0.006289] tsc: Detected 1996.249 MHz processor
[    0.305123] clocksource: tsc-early: mask: 0xffffffffffffffff max_cycles: 0x398cadd9d93, max_idle_ns: 881590552906 ns
[    1.045759] clocksource: Switched to clocksource kvm-clock
[    1.141326] clocksource: tsc: mask: 0xffffffffffffffff max_cycles: 0x398cadd9d93, max_idle_ns: 881590552906 ns
[    1.144634] clocksource: Switched to clocksource tsc

 
> The only guarantee you have is when the TSC MSRs are not intercepted - IOW,
> you're a STSC guest.
> 
> So none of that modern stuff means anything - your only case is a STSC guest
> where you can somewhat reliably know in the guest that the host is not lying
> to you.

That was my understanding and implementation in v11, where Sean suggested that
VMs running on CPUs supporting stable and always running TSC, should switch over
to TSC properly[1] and [2], in a generic way.

> 
> So the only configuration is a STSC guest - everything else should use
> kvm-clock.

That is not right, if non-secure guest is booted with TscInvariant bit set, guest
will start using TSC as the clocksource, unfortunately sched clock keeps on using
kvm-clock :(

> 
> AFAIU.
> 
>>> After asking so many questions, it sounds to me like this patch and patch 12
>>> should be merged into one and there it should be explained what the strategy
>>> is when a STSC guest loads and how kvmclock is supposed to be handled, what is
>>> allowed, what is warned about, when the guest terminates what is tainted,
>>> yadda yadda. 
>>>> This all belongs in a single patch logically.
> 
> Now, why aren't you merging patch 9 and 12 into one and calling it:
>
> "Switch Secure TSC guests away from kvm-clock"
> 
> or so, where you switch a STSC guest to use the TSC MSRs and
> warn/taint/terminate the guest if the user chooses otherwise?

I am going to merge both the patches, but wanted your input on the commit
wordings.

Thanks,
Nikunj

1. https://lore.kernel.org/kvm/ZuR2t1QrBpPc1Sz2@google.com/
2. https://lore.kernel.org/kvm/ZurCbP7MesWXQbqZ@google.com/