linux-kernel - Re: [PATCH v11 19/20] x86/kvmclock: Skip kvmclock when Secure TSC is available

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <156dc1ab-1239-0508-1161-ab0cd13d35b1@amd.com>
Date: Tue, 1 Oct 2024 09:56:00 +0530
From: "Nikunj A. Dadhania" <nikunj@....com>
To: Thomas Gleixner <tglx@...utronix.de>,
 Sean Christopherson <seanjc@...gle.com>
Cc: linux-kernel@...r.kernel.org, thomas.lendacky@....com, bp@...en8.de,
 x86@...nel.org, kvm@...r.kernel.org, mingo@...hat.com,
 dave.hansen@...ux.intel.com, pgonda@...gle.com, pbonzini@...hat.com,
 peterz@...radead.org, gautham.shenoy@....com
Subject: Re: [PATCH v11 19/20] x86/kvmclock: Skip kvmclock when Secure TSC is
 available

On 10/1/2024 2:50 AM, Thomas Gleixner wrote:
> On Mon, Sep 30 2024 at 11:57, Nikunj A. Dadhania wrote:
>> TSC Clock Rating Adjustment:
>> * During TSC initialization, downgrade the TSC clock rating to 200 if TSC is not
>>   constant/reliable, placing it below HPET.
> 
> Downgrading a constant TSC is a bad idea. Reliable just means that it
> does not need a watchdog clocksource. If it's non-constant it's
> downgraded anyway.
> 
>> * Ensure the kvm-clock rating is set to 299 by default in the 
>>   struct clocksource kvm_clock.
>> * Avoid changing the kvm clock rating based on the availability of reliable
>>   clock sources. Let the TSC clock source determine and downgrade itself.
> 
> Why downgrade? If it's the best one you want to upgrade it so it's
> preferred over the others.

Thanks for confirming that upgrading the TSC rating is fine.

> The above will make sure that the PV clocksource rating remain
>> unaffected.
>>
>> Clock soure selection order when the ratings match:
>> * Currently, clocks are registered and enqueued based on their rating.
>> * When clock ratings are tied, use the advertised clock frequency(freq_khz) as a
>>   secondary key to favor clocks with better frequency.
>>
>> This approach improves the selection process by considering both rating and
>> frequency. Something like below:
> 
> What does the frequency tell us? Not really anything. It's not
> necessarily the better clocksource.
> 
> Higher frequency gives you a slightly better resolution, but as all of
> this is usually sub-nanosecond resolution already that's not making a
> difference in practice.
> 
> So if you know you want TSC to be selected, then upgrade the rating of
> both the early and the regular TSC clocksource and be done with it.

Sure Thomas, I will modify the patch accordingly and send an RFC.

Also I realized that, for the guests, instead of rdtsc(), we should be 
calling rdtsc_ordered() to make sure that time moves forward even when
vCPUs are migrated.

Thanks,
Nikunj