[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 Mon, Feb 20 2023 at 21:51, Krister Johansen wrote: > On Mon, Feb 20, 2023 at 08:14:40PM -0800, Krister Johansen wrote: >> > 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? Sure. But a life migration from a NEVER_EMULATE to a non-invariant host is a patently bad idea and has nothing to do with the __init function, which is gone at that point already. What I wanted to say: static bool __init xen_tsc_safe_clocksource(void) { ...... /* 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 didn't have the full context and was just looking at the condition. Now I checked the full context and I think that except for the if (check_tsc_unstable()) check everything else can go away unless you do not trust the hypervisor that it only sets the NEVER_EMULATE bit when CONSTANT and NONSTOP are set as well. But yeah, you might prefer to be paranoid. It's virt after all. Thanks, tglx
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |