Messages in this thread | | | From | "Bae, Chang Seok" <> | Subject | Re: [PATCH v2 8/8] x86/vdso: Move out the CPU number store | Date | Wed, 6 Jun 2018 18:27:21 +0000 |
| |
On Wed, Jun 6, 2018 at 10:25 AM Andy Lutomirski <luto@kernel.org> wrote: > On Wed, Jun 6, 2018 at 9:23 AM Chang S. Bae <chang.seok.bae@intel.com> wrote: >> >> The CPU (and node) number will be written, as early enough, >> to the segment limit of per CPU data and TSC_AUX MSR entry. >> The information has been retrieved by vgetcpu in user space >> and will be also loaded from the paranoid entry, when >> FSGSBASE enabled. So, it is moved out from vDSO to the CPU >> initialization path where IST setup is serialized. >> > Please split this into two patches. One patch should do the cleanups > and one patch should move the code. >> diff --git a/arch/x86/entry/vdso/vgetcpu.c b/arch/x86/entry/vdso/vgetcpu.c >> index 8ec3d1f..3284069 100644 >> --- a/arch/x86/entry/vdso/vgetcpu.c >> +++ b/arch/x86/entry/vdso/vgetcpu.c >> @@ -18,9 +18,9 @@ __vdso_getcpu(unsigned *cpu, unsigned *node, struct getcpu_cache *unused) >> p = __getcpu(); >> >> if (cpu) >> - *cpu = p & VGETCPU_CPU_MASK; >> + *cpu = lsl_tscp_to_cpu(p); >> if (node) >> - *node = p >> 12; >> + *node = lsl_tscp_to_node(p); >> return 0; > This goes in the cleanup patch. >> +/* Bit size and mask of CPU number stored in the per CPU data (and TSC_AUX) */ >> +#define LSL_TSCP_CPU_SIZE 12 >> +#define LSL_TSCP_CPU_MASK 0xfff >> + >> +#ifndef __ASSEMBLY__ >> + >> +/* Helper functions to store/load CPU and node numbers */ >> + >> +static inline unsigned long make_lsl_tscp(int cpu, unsigned long node) >> +{ >> + return ((node << LSL_TSCP_CPU_SIZE) | cpu); >> +} >> + >> +static inline unsigned int lsl_tscp_to_cpu(unsigned long x) >> +{ >> + return (x & LSL_TSCP_CPU_MASK); >> +} >> + >> +static inline unsigned int lsl_tscp_to_node(unsigned long x) >> +{ >> + return (x >> LSL_TSCP_CPU_SIZE); >> +} > As do these. But maybe they should be #ifdef CONFIG_X86_64 to make it > clear that they are not presently supported on 32-bit systems. > (Unless I'm wrong and they are supported.) >> diff --git a/arch/x86/kernel/cpu/common.c b/arch/x86/kernel/cpu/common.c >> index 38276f5..c7b54f0 100644 >> --- a/arch/x86/kernel/cpu/common.c >> +++ b/arch/x86/kernel/cpu/common.c >> @@ -1665,6 +1665,11 @@ void cpu_init(void) >> >> wrmsrl(MSR_FS_BASE, 0); >> wrmsrl(MSR_KERNEL_GS_BASE, 0); >> +#ifdef CONFIG_NUMA >> + write_rdtscp_aux(make_lsl_tscp(cpu, early_cpu_to_node(cpu))); >> +#else >> + write_rdtscp_aux(make_lsl_tscp(cpu, 0)); >> +#endif > Why the ifdef? early_cpu_to_node() should return 0 if !CONFIG_NUMA. >> +static inline void setup_cpu_number_segment(int cpu) >> +{ >> +#ifdef CONFIG_NUMA >> + unsigned long node = early_cpu_to_node(cpu); >> +#else >> + unsigned long node = 0; >> +#endif > This duplicates half the rdtscp_aux code. How about making this one > function setup_cpu_number() that does all of it? >> + struct desc_struct d = GDT_ENTRY_INIT(0x40f5, 0x0, >> + make_lsl_tscp(cpu, node)); > Please get rid of the GDT_ENTRY_INIT stuff and just fill out all the > fields, just like it is in the code that you're moving. No one enjoys > decoding hex segment descriptors. (Well, Linus and hpa probably > *love* doing it just to show off.)
Will apply as what you commented.
Thanks Chang
| |