On 4/11/22 11:01, Jon Kohler wrote: > static enum tsx_ctrl_states x86_get_tsx_auto_mode(void) > { > + /* > + * Hardware will always abort a TSX transaction if both CPUID bits > + * RTM_ALWAYS_ABORT and TSX_FORCE_ABORT are set. In this case, it is > + * better not to enumerate CPUID.RTM and CPUID.HLE bits. Clear them > + * here. > + */ > + if (boot_cpu_has(X86_FEATURE_RTM_ALWAYS_ABORT) && > + boot_cpu_has(X86_FEATURE_TSX_FORCE_ABORT)) { > + tsx_clear_cpuid(); > + setup_clear_cpu_cap(X86_FEATURE_RTM); > + setup_clear_cpu_cap(X86_FEATURE_HLE); > + return TSX_CTRL_RTM_ALWAYS_ABORT; > + } I don't really like hiding the setup_clear_cpu_cap() like this. Right now, all of the setup_clear_cpu_cap()'s are in a single function and they are pretty easy to figure out. This seems like logic that deserves to be appended down to the last if() block of code in tsx_init() instead of squirreled away in a "get mode" function. Does this work? if (tsx_ctrl_state == TSX_CTRL_DISABLE) { ... } else if (tsx_ctrl_state == TSX_CTRL_ENABLE) { ... } else if (tsx_ctrl_state == TSX_CTRL_RTM_ALWAYS_ABORT) { tsx_clear_cpuid(); setup_clear_cpu_cap(X86_FEATURE_RTM); setup_clear_cpu_cap(X86_FEATURE_HLE); }