[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] 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 Attachment:
OpenPGP_0xB0DE9DD628BF132F.asc Attachment:
OpenPGP_signature
|
![]() |
Lists.xenproject.org is hosted with RackSpace, monitoring our |