linux-kernel - Re: [PATCH linux-next 2/2] x86/xen/time: cleanup xen_tsc_safe

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: <cd9acc63-c352-93c8-56bf-52ef69e5e124@suse.com>
Date:   Tue, 21 Feb 2023 09:47:24 +0100
From:   Juergen Gross <jgross@...e.com>
To:     Krister Johansen <kjlx@...pleofstupid.com>,
        Thomas Gleixner <tglx@...utronix.de>
Cc:     xen-devel@...ts.xenproject.org, linux-kernel@...r.kernel.org,
        Jan Beulich <jbeulich@...e.com>,
        Boris Ostrovsky <boris.ostrovsky@...cle.com>,
        Ingo Molnar <mingo@...hat.com>, Borislav Petkov <bp@...en8.de>,
        Dave Hansen <dave.hansen@...ux.intel.com>, x86@...nel.org,
        "H. Peter Anvin" <hpa@...or.com>,
        Marcelo Tosatti <mtosatti@...hat.com>,
        Anthony Liguori <aliguori@...zon.com>,
        David Reaver <me@...idreaver.com>,
        Brendan Gregg <brendan@...el.com>
Subject: Re: [PATCH linux-next 2/2] x86/xen/time: cleanup
 xen_tsc_safe_clocksource

On 21.02.23 06:51, Krister Johansen wrote:
> On Mon, Feb 20, 2023 at 08:14:40PM -0800, Krister Johansen wrote:
>> On Mon, Feb 20, 2023 at 11:01:18PM +0100, Thomas Gleixner wrote:
>>> On Mon, Feb 20 2023 at 09:17, Krister Johansen wrote:
>>>> @@ -495,8 +496,7 @@ static int __init xen_tsc_safe_clocksource(void)
>>>>   	/* Leaf 4, sub-leaf 0 (0x40000x03) */
>>>>   	cpuid_count(xen_cpuid_base() + 3, 0, &eax, &ebx, &ecx, &edx);
>>>>   
>>>> -	/* tsc_mode = no_emulate (2) */
>>>> -	if (ebx != 2)
>>>> +	if (ebx != XEN_CPUID_TSC_MODE_NEVER_EMULATE)
>>>>   		return 0;
>>>>   
>>>>   	return 1;
>>>
>>> What about removing more stupidity from that function?
>>>
>>> static bool __init xen_tsc_safe_clocksource(void)
>>> {
>>> 	u32 eax, ebx. ecx, edx;
>>>   
>>> 	/* Leaf 4, sub-leaf 0 (0x40000x03) */
>>> 	cpuid_count(xen_cpuid_base() + 3, 0, &eax, &ebx, &ecx, &edx);
>>>
>>> 	return ebx == XEN_CPUID_TSC_MODE_NEVER_EMULATE;
>>> }
>>
>> I'm all for simplifying.  I'm happy to clean up that return to be more
>> idiomatic.  I was under the impression, perhaps mistaken, though, that
>> the X86_FEATURE_CONSTANT_TSC, X86_FEATURE_NONSTOP_TSC, and
>> check_tsc_unstable() checks were actually serving a purpose: to ensure
>> that we don't rely on the tsc in environments where it's being emulated
>> and the OS would be better served by using a PV clock.  Specifically,
>> kvmclock_init() makes a very similar set of checks that I also thought
>> were load-bearing.
> 
> Bah, what I meant to say was emulated, unstable, or otherwise unsuitable
> for use as a clocksource.  IOW, even if TSC_MODE_NEVER_EMULATE is
> set, it's possible that a user is attempting a migration from a cpu
> that's not invariant, and we'd still want to check for that case and
> fall back to a PV clocksource, correct?

But Thomas' suggestion wasn't changing any behavior compared to your
patch. It just makes it easier to read.

If you are unsure your patch is correct, please verify the correctness
before sending it.


Juergen

Download attachment "OpenPGP_0xB0DE9DD628BF132F.asc" of type "application/pgp-keys" (3099 bytes)

Download attachment "OpenPGP_signature" of type "application/pgp-signature" (496 bytes)