Re: [PATCH v7 6/6] x86/arch_prctl: Add ARCH_[GET|SET]_CPUID
flat view
From: Andy Lutomirski <luto@amacapital.net>
Date: 2016-10-27 22:35:17
Also in:
linux-api, lkml
On Thu, Oct 27, 2016 at 4:15 AM, Thomas Gleixner [off-list ref] wrote:
This is insane. The compiler makes that a conditional jump and then in switch_cpuid_faulting we get another one. Further switch_cpuid_faulting() calls into lib/msr which is adding even more overhead. msr_set/clear_bit() are nice for random driver code, but complete overkill for the context switch hotpath. That's just not acceptable for switch_to(). We keep adding cruft and then wonder why context switches slow down despite machines getting faster. This can and needs to be done smarter. See untested patch below. The resulting code has a single conditional jump, which is obviously the check for a change between prev and next. Everything else is done with straight linear shift,add,and,rdmsr,wrmsr instructions.
...
quoted hunk ↗ jump to hunk
#define MSR_IA32_SYSENTER_ESP 0x00000175--- a/arch/x86/kernel/process.c +++ b/arch/x86/kernel/process.c@@ -193,12 +193,17 @@ int set_tsc_mode(unsigned int val) return 0; } -static void switch_cpuid_faulting(bool on) +#define CPUID_FAULT_ON_MASK (~0ULL) +#define CPUID_FAULT_OFF_MASK (~CPUID_FAULT_ENABLE) + +static void cpuid_fault_ctrl(u64 msk) { - if (on) - msr_set_bit(MSR_MISC_FEATURES_ENABLES, 0); - else - msr_clear_bit(MSR_MISC_FEATURES_ENABLES, 0); + u64 msrval; + + rdmsrl(MSR_MISC_FEATURES_ENABLES, msrval); + msrval |= CPUID_FAULT_ENABLE; + msrval &= msk; + wrmsrl(MSR_MISC_FEATURES_ENABLES, msrval); }
Let's just do this right from day one:
static void set_cpuid_faulting(bool on)
{
u64 msrval;
DEBUG_LOCKS_WARN_ON(!irqs_disabled());
msrval = this_cpu_read(msr_misc_features_enables_shadow);
msrval &= CPUID_FAULT_ENABLE;
msrval |= (on << CPUID_FAULT_ENABLE_BIT);
this_cpu_write(msr_misc_features_enables_shadow, msrval);
wrmsrl(MSR_MISC_FEATURES_ENABLES, msrval);
}
RDMSR may be considerably faster than WRMSR, but that doesn't mean it's *fast*.
Obviously this needs some initialization code, but that's fine IMO.
--Andy