lkml.org 
[lkml]   [2018]   [Jun]   [6]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
From
SubjectRe: [PATCH v2 8/8] x86/vdso: Move out the CPU number store
Date
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

\
 
 \ /
  Last update: 2018-06-06 20:27    [W:0.068 / U:0.440 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site