Re: [PATCH 6/6] x86/vdso: Move out the CPU number store
On Mon, Jun 4, 2018 at 10:36 PM H. Peter Anvin wrote: > > On 06/04/18 20:57, Mika Penttilä wrote: > > > > This won't work on X86-32 because it actually uses the segment limit with > > fs: access. So there > > is a reason why the lsl based method is X86-64 only. > > > > > > Why does that matter in any shape, way, or form? The LSL instruction > doesn't touch any of the segment registers, it just uses a segment > selector number. > > > > I see... we have a VERY unfortunate name collision: the x86-64 > GDT_ENTRY_PERC_PU and the i386 GDT_ENTRY_PERCPU are in fact two > completely different things, with the latter being the actual percpu > offset used by the kernel. > > So yes, this patch is wrong, because the naming of the x86-64 segment is > insane especially in the proximity of the -- it should be something > like GDT_ENTRY_CPU_NUMBER. > > Unfortunately we probably can't use the same GDT entry on x86-32 and > x86-64, because entry 15 (selector 0x7b) is USER_DS, which is something > we really don't want to screw with. This means i386 programs that > execute LSL directly for whatever reason will have to understand the > difference, but most of the other segment numbers differ as well, > including user space %cs (USER_CS/USER32_CS) and %ds/%es/%ss (USER_DS). > Perhaps we could bump down segments 23-28 by one and put it as 23, that > way the CPU_NUMBER segment would always be at %ss+80 for the default > (flat, initial) user space %ss. (We want to specify using %ss rather > than %ds, because it is less likely to be changed and because 64 bits, > too, have %ss defined, but not %ds.) > > > > Rename the x86-64 segment to CPU_NUMBER, fixing the naming conflict. Yes, agreed. Probably as its own patch *before* the rest of this cleanup. > Add 1 to GDT entry numbers 23-28 for i386 (all of these are > kernel-internal segments and so have no impact on user space). > Add i386 CPU_NUMBER equivalent to x86-64 at GDT entry 23. > Document the above relationship between segments. Sure, but also as a standalone patch, please. > > OK, everyone? > > -hpa
Re: [PATCH 6/6] x86/vdso: Move out the CPU number store
On Mon, Jun 4, 2018 at 10:36 PM H. Peter Anvin wrote: > > On 06/04/18 20:57, Mika Penttilä wrote: > > > > This won't work on X86-32 because it actually uses the segment limit with > > fs: access. So there > > is a reason why the lsl based method is X86-64 only. > > > > > > Why does that matter in any shape, way, or form? The LSL instruction > doesn't touch any of the segment registers, it just uses a segment > selector number. > > > > I see... we have a VERY unfortunate name collision: the x86-64 > GDT_ENTRY_PERC_PU and the i386 GDT_ENTRY_PERCPU are in fact two > completely different things, with the latter being the actual percpu > offset used by the kernel. > > So yes, this patch is wrong, because the naming of the x86-64 segment is > insane especially in the proximity of the -- it should be something > like GDT_ENTRY_CPU_NUMBER. > > Unfortunately we probably can't use the same GDT entry on x86-32 and > x86-64, because entry 15 (selector 0x7b) is USER_DS, which is something > we really don't want to screw with. This means i386 programs that > execute LSL directly for whatever reason will have to understand the > difference, but most of the other segment numbers differ as well, > including user space %cs (USER_CS/USER32_CS) and %ds/%es/%ss (USER_DS). > Perhaps we could bump down segments 23-28 by one and put it as 23, that > way the CPU_NUMBER segment would always be at %ss+80 for the default > (flat, initial) user space %ss. (We want to specify using %ss rather > than %ds, because it is less likely to be changed and because 64 bits, > too, have %ss defined, but not %ds.) > > > > Rename the x86-64 segment to CPU_NUMBER, fixing the naming conflict. Yes, agreed. Probably as its own patch *before* the rest of this cleanup. > Add 1 to GDT entry numbers 23-28 for i386 (all of these are > kernel-internal segments and so have no impact on user space). > Add i386 CPU_NUMBER equivalent to x86-64 at GDT entry 23. > Document the above relationship between segments. Sure, but also as a standalone patch, please. > > OK, everyone? > > -hpa
Re: [PATCH 6/6] x86/vdso: Move out the CPU number store
On 06/05/2018 08:36 AM, H. Peter Anvin wrote: > On 06/04/18 20:57, Mika Penttilä wrote: >> >> This won't work on X86-32 because it actually uses the segment limit with >> fs: access. So there >> is a reason why the lsl based method is X86-64 only. >> > > > > Why does that matter in any shape, way, or form? The LSL instruction > doesn't touch any of the segment registers, it just uses a segment > selector number. > > > > I see... we have a VERY unfortunate name collision: the x86-64 > GDT_ENTRY_PERC_PU and the i386 GDT_ENTRY_PERCPU are in fact two > completely different things, with the latter being the actual percpu > offset used by the kernel. > > So yes, this patch is wrong, because the naming of the x86-64 segment is > insane especially in the proximity of the -- it should be something > like GDT_ENTRY_CPU_NUMBER. > > Unfortunately we probably can't use the same GDT entry on x86-32 and > x86-64, because entry 15 (selector 0x7b) is USER_DS, which is something > we really don't want to screw with. This means i386 programs that > execute LSL directly for whatever reason will have to understand the > difference, but most of the other segment numbers differ as well, > including user space %cs (USER_CS/USER32_CS) and %ds/%es/%ss (USER_DS). > Perhaps we could bump down segments 23-28 by one and put it as 23, that > way the CPU_NUMBER segment would always be at %ss+80 for the default > (flat, initial) user space %ss. (We want to specify using %ss rather > than %ds, because it is less likely to be changed and because 64 bits, > too, have %ss defined, but not %ds.) > > > > Rename the x86-64 segment to CPU_NUMBER, fixing the naming conflict. > Add 1 to GDT entry numbers 23-28 for i386 (all of these are > kernel-internal segments and so have no impact on user space). > Add i386 CPU_NUMBER equivalent to x86-64 at GDT entry 23. > Document the above relationship between segments. > > OK, everyone? > > -hpa > Yes GDT_ENTRY_PER_CPU and GDT_ENTRY_PERCPU meaning two totally different things is really confusing, the proposal seems ok to me! --Mika
Re: [PATCH 6/6] x86/vdso: Move out the CPU number store
On 06/05/2018 08:36 AM, H. Peter Anvin wrote: > On 06/04/18 20:57, Mika Penttilä wrote: >> >> This won't work on X86-32 because it actually uses the segment limit with >> fs: access. So there >> is a reason why the lsl based method is X86-64 only. >> > > > > Why does that matter in any shape, way, or form? The LSL instruction > doesn't touch any of the segment registers, it just uses a segment > selector number. > > > > I see... we have a VERY unfortunate name collision: the x86-64 > GDT_ENTRY_PERC_PU and the i386 GDT_ENTRY_PERCPU are in fact two > completely different things, with the latter being the actual percpu > offset used by the kernel. > > So yes, this patch is wrong, because the naming of the x86-64 segment is > insane especially in the proximity of the -- it should be something > like GDT_ENTRY_CPU_NUMBER. > > Unfortunately we probably can't use the same GDT entry on x86-32 and > x86-64, because entry 15 (selector 0x7b) is USER_DS, which is something > we really don't want to screw with. This means i386 programs that > execute LSL directly for whatever reason will have to understand the > difference, but most of the other segment numbers differ as well, > including user space %cs (USER_CS/USER32_CS) and %ds/%es/%ss (USER_DS). > Perhaps we could bump down segments 23-28 by one and put it as 23, that > way the CPU_NUMBER segment would always be at %ss+80 for the default > (flat, initial) user space %ss. (We want to specify using %ss rather > than %ds, because it is less likely to be changed and because 64 bits, > too, have %ss defined, but not %ds.) > > > > Rename the x86-64 segment to CPU_NUMBER, fixing the naming conflict. > Add 1 to GDT entry numbers 23-28 for i386 (all of these are > kernel-internal segments and so have no impact on user space). > Add i386 CPU_NUMBER equivalent to x86-64 at GDT entry 23. > Document the above relationship between segments. > > OK, everyone? > > -hpa > Yes GDT_ENTRY_PER_CPU and GDT_ENTRY_PERCPU meaning two totally different things is really confusing, the proposal seems ok to me! --Mika
Re: [PATCH 6/6] x86/vdso: Move out the CPU number store
On 06/04/18 20:57, Mika Penttilä wrote: > > This won't work on X86-32 because it actually uses the segment limit with fs: > access. So there > is a reason why the lsl based method is X86-64 only. > Why does that matter in any shape, way, or form? The LSL instruction doesn't touch any of the segment registers, it just uses a segment selector number. I see... we have a VERY unfortunate name collision: the x86-64 GDT_ENTRY_PERC_PU and the i386 GDT_ENTRY_PERCPU are in fact two completely different things, with the latter being the actual percpu offset used by the kernel. So yes, this patch is wrong, because the naming of the x86-64 segment is insane especially in the proximity of the -- it should be something like GDT_ENTRY_CPU_NUMBER. Unfortunately we probably can't use the same GDT entry on x86-32 and x86-64, because entry 15 (selector 0x7b) is USER_DS, which is something we really don't want to screw with. This means i386 programs that execute LSL directly for whatever reason will have to understand the difference, but most of the other segment numbers differ as well, including user space %cs (USER_CS/USER32_CS) and %ds/%es/%ss (USER_DS). Perhaps we could bump down segments 23-28 by one and put it as 23, that way the CPU_NUMBER segment would always be at %ss+80 for the default (flat, initial) user space %ss. (We want to specify using %ss rather than %ds, because it is less likely to be changed and because 64 bits, too, have %ss defined, but not %ds.) Rename the x86-64 segment to CPU_NUMBER, fixing the naming conflict. Add 1 to GDT entry numbers 23-28 for i386 (all of these are kernel-internal segments and so have no impact on user space). Add i386 CPU_NUMBER equivalent to x86-64 at GDT entry 23. Document the above relationship between segments. OK, everyone? -hpa
Re: [PATCH 6/6] x86/vdso: Move out the CPU number store
On 06/04/18 20:57, Mika Penttilä wrote: > > This won't work on X86-32 because it actually uses the segment limit with fs: > access. So there > is a reason why the lsl based method is X86-64 only. > Why does that matter in any shape, way, or form? The LSL instruction doesn't touch any of the segment registers, it just uses a segment selector number. I see... we have a VERY unfortunate name collision: the x86-64 GDT_ENTRY_PERC_PU and the i386 GDT_ENTRY_PERCPU are in fact two completely different things, with the latter being the actual percpu offset used by the kernel. So yes, this patch is wrong, because the naming of the x86-64 segment is insane especially in the proximity of the -- it should be something like GDT_ENTRY_CPU_NUMBER. Unfortunately we probably can't use the same GDT entry on x86-32 and x86-64, because entry 15 (selector 0x7b) is USER_DS, which is something we really don't want to screw with. This means i386 programs that execute LSL directly for whatever reason will have to understand the difference, but most of the other segment numbers differ as well, including user space %cs (USER_CS/USER32_CS) and %ds/%es/%ss (USER_DS). Perhaps we could bump down segments 23-28 by one and put it as 23, that way the CPU_NUMBER segment would always be at %ss+80 for the default (flat, initial) user space %ss. (We want to specify using %ss rather than %ds, because it is less likely to be changed and because 64 bits, too, have %ss defined, but not %ds.) Rename the x86-64 segment to CPU_NUMBER, fixing the naming conflict. Add 1 to GDT entry numbers 23-28 for i386 (all of these are kernel-internal segments and so have no impact on user space). Add i386 CPU_NUMBER equivalent to x86-64 at GDT entry 23. Document the above relationship between segments. OK, everyone? -hpa
Re: [PATCH 6/6] x86/vdso: Move out the CPU number store
On 06/05/2018 07:44 AM, Bae, Chang Seok wrote:
>>> diff --git a/arch/x86/kernel/setup_percpu.c b/arch/x86/kernel/setup_percpu.c
>>> index ea554f8..e716e94 100644
>>> --- a/arch/x86/kernel/setup_percpu.c
>>> +++ b/arch/x86/kernel/setup_percpu.c
>>> @@ -155,12 +155,21 @@ static void __init pcpup_populate_pte(unsigned long
>>> addr)
>>>
>>> static inline void setup_percpu_segment(int cpu)
>>> {
>>> -#ifdef CONFIG_X86_32
>>> - struct desc_struct d = GDT_ENTRY_INIT(0x8092, per_cpu_offset(cpu),
>>> - 0xF);
>>> +#ifdef CONFIG_NUMA
>>> + unsigned long node = early_cpu_to_node(cpu);
>>> +#else
>>> + unsigned long node = 0;
>>> +#endif
>>> + struct desc_struct d = GDT_ENTRY_INIT(0x0, per_cpu_offset(cpu),
>>> + make_lsl_tscp(cpu, node));
>>> +
>>> + d.type = 5; /* R0 data, expand down, accessed */
>>> + d.dpl = 3; /* Visible to user code */
>>> + d.s = 1;/* Not a system segment */
>>> + d.p = 1;/* Present */
>>> + d.d = 1;/* 32-bit */
>>>
>>> write_gdt_entry(get_cpu_gdt_rw(cpu), GDT_ENTRY_PERCPU, , DESCTYPE_S);
>>> -#endif
>>> }
>
>> This won't work on X86-32 because it actually uses the segment limit with
>> fs: access. So there
>> is a reason why the lsl based method is X86-64 only.
>
> The limit will be consumed in X86-64 only, while the unification with i386
> was suggested for a
> different reason.
>
> Thanks,
> Chang
>
The unification affects i386, and the limit is consumed by the processor with
fs: access.
The limit was 0xF before, now it depends on the cpu/node. So accesses on
small number cpus
are likely to fault.
--Mika
Re: [PATCH 6/6] x86/vdso: Move out the CPU number store
On 06/05/2018 07:44 AM, Bae, Chang Seok wrote:
>>> diff --git a/arch/x86/kernel/setup_percpu.c b/arch/x86/kernel/setup_percpu.c
>>> index ea554f8..e716e94 100644
>>> --- a/arch/x86/kernel/setup_percpu.c
>>> +++ b/arch/x86/kernel/setup_percpu.c
>>> @@ -155,12 +155,21 @@ static void __init pcpup_populate_pte(unsigned long
>>> addr)
>>>
>>> static inline void setup_percpu_segment(int cpu)
>>> {
>>> -#ifdef CONFIG_X86_32
>>> - struct desc_struct d = GDT_ENTRY_INIT(0x8092, per_cpu_offset(cpu),
>>> - 0xF);
>>> +#ifdef CONFIG_NUMA
>>> + unsigned long node = early_cpu_to_node(cpu);
>>> +#else
>>> + unsigned long node = 0;
>>> +#endif
>>> + struct desc_struct d = GDT_ENTRY_INIT(0x0, per_cpu_offset(cpu),
>>> + make_lsl_tscp(cpu, node));
>>> +
>>> + d.type = 5; /* R0 data, expand down, accessed */
>>> + d.dpl = 3; /* Visible to user code */
>>> + d.s = 1;/* Not a system segment */
>>> + d.p = 1;/* Present */
>>> + d.d = 1;/* 32-bit */
>>>
>>> write_gdt_entry(get_cpu_gdt_rw(cpu), GDT_ENTRY_PERCPU, , DESCTYPE_S);
>>> -#endif
>>> }
>
>> This won't work on X86-32 because it actually uses the segment limit with
>> fs: access. So there
>> is a reason why the lsl based method is X86-64 only.
>
> The limit will be consumed in X86-64 only, while the unification with i386
> was suggested for a
> different reason.
>
> Thanks,
> Chang
>
The unification affects i386, and the limit is consumed by the processor with
fs: access.
The limit was 0xF before, now it depends on the cpu/node. So accesses on
small number cpus
are likely to fault.
--Mika
Re: [PATCH 6/6] x86/vdso: Move out the CPU number store
>> diff --git a/arch/x86/kernel/setup_percpu.c b/arch/x86/kernel/setup_percpu.c
>> index ea554f8..e716e94 100644
>> --- a/arch/x86/kernel/setup_percpu.c
>> +++ b/arch/x86/kernel/setup_percpu.c
>> @@ -155,12 +155,21 @@ static void __init pcpup_populate_pte(unsigned long
>> addr)
>>
>> static inline void setup_percpu_segment(int cpu)
>> {
>> -#ifdef CONFIG_X86_32
>> -struct desc_struct d = GDT_ENTRY_INIT(0x8092, per_cpu_offset(cpu),
>> - 0xF);
>> +#ifdef CONFIG_NUMA
>> +unsigned long node = early_cpu_to_node(cpu);
>> +#else
>> +unsigned long node = 0;
>> +#endif
>> +struct desc_struct d = GDT_ENTRY_INIT(0x0, per_cpu_offset(cpu),
>> + make_lsl_tscp(cpu, node));
>> +
>> +d.type = 5; /* R0 data, expand down, accessed */
>> +d.dpl = 3; /* Visible to user code */
>> +d.s = 1;/* Not a system segment */
>> +d.p = 1;/* Present */
>> +d.d = 1;/* 32-bit */
>>
>> write_gdt_entry(get_cpu_gdt_rw(cpu), GDT_ENTRY_PERCPU, , DESCTYPE_S);
>> -#endif
>> }
> This won't work on X86-32 because it actually uses the segment limit with fs:
> access. So there
> is a reason why the lsl based method is X86-64 only.
The limit will be consumed in X86-64 only, while the unification with i386 was
suggested for a
different reason.
Thanks,
Chang
Re: [PATCH 6/6] x86/vdso: Move out the CPU number store
>> diff --git a/arch/x86/kernel/setup_percpu.c b/arch/x86/kernel/setup_percpu.c
>> index ea554f8..e716e94 100644
>> --- a/arch/x86/kernel/setup_percpu.c
>> +++ b/arch/x86/kernel/setup_percpu.c
>> @@ -155,12 +155,21 @@ static void __init pcpup_populate_pte(unsigned long
>> addr)
>>
>> static inline void setup_percpu_segment(int cpu)
>> {
>> -#ifdef CONFIG_X86_32
>> -struct desc_struct d = GDT_ENTRY_INIT(0x8092, per_cpu_offset(cpu),
>> - 0xF);
>> +#ifdef CONFIG_NUMA
>> +unsigned long node = early_cpu_to_node(cpu);
>> +#else
>> +unsigned long node = 0;
>> +#endif
>> +struct desc_struct d = GDT_ENTRY_INIT(0x0, per_cpu_offset(cpu),
>> + make_lsl_tscp(cpu, node));
>> +
>> +d.type = 5; /* R0 data, expand down, accessed */
>> +d.dpl = 3; /* Visible to user code */
>> +d.s = 1;/* Not a system segment */
>> +d.p = 1;/* Present */
>> +d.d = 1;/* 32-bit */
>>
>> write_gdt_entry(get_cpu_gdt_rw(cpu), GDT_ENTRY_PERCPU, , DESCTYPE_S);
>> -#endif
>> }
> This won't work on X86-32 because it actually uses the segment limit with fs:
> access. So there
> is a reason why the lsl based method is X86-64 only.
The limit will be consumed in X86-64 only, while the unification with i386 was
suggested for a
different reason.
Thanks,
Chang
Re: [PATCH 6/6] x86/vdso: Move out the CPU number store
On 06/04/2018 10:24 PM, Chang S. Bae 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.
>
> Now, redundant setting of the segment in entry/vdso/vma.c
> was removed; a substantial code removal. It removes a
> hotplug notifier, makes a facility useful to both the kernel
> and userspace unconditionally available much sooner, and
> unification with i386. (Thanks to HPA for suggesting the
> cleanup)
>
> Signed-off-by: Chang S. Bae
> Cc: H. Peter Anvin
> Cc: Dave Hansen
> Cc: Andy Lutomirski
> Cc: Andi Kleen
> Cc: Thomas Gleixner
> Cc: Ingo Molnar
> ---
> diff --git a/arch/x86/kernel/setup_percpu.c b/arch/x86/kernel/setup_percpu.c
> index ea554f8..e716e94 100644
> --- a/arch/x86/kernel/setup_percpu.c
> +++ b/arch/x86/kernel/setup_percpu.c
> @@ -155,12 +155,21 @@ static void __init pcpup_populate_pte(unsigned long
> addr)
>
> static inline void setup_percpu_segment(int cpu)
> {
> -#ifdef CONFIG_X86_32
> - struct desc_struct d = GDT_ENTRY_INIT(0x8092, per_cpu_offset(cpu),
> - 0xF);
> +#ifdef CONFIG_NUMA
> + unsigned long node = early_cpu_to_node(cpu);
> +#else
> + unsigned long node = 0;
> +#endif
> + struct desc_struct d = GDT_ENTRY_INIT(0x0, per_cpu_offset(cpu),
> +make_lsl_tscp(cpu, node));
> +
> + d.type = 5; /* R0 data, expand down, accessed */
> + d.dpl = 3; /* Visible to user code */
> + d.s = 1;/* Not a system segment */
> + d.p = 1;/* Present */
> + d.d = 1;/* 32-bit */
>
> write_gdt_entry(get_cpu_gdt_rw(cpu), GDT_ENTRY_PERCPU, , DESCTYPE_S);
> -#endif
> }
This won't work on X86-32 because it actually uses the segment limit with fs:
access. So there
is a reason why the lsl based method is X86-64 only.
-Mika
>
> void __init setup_per_cpu_areas(void)
>
Re: [PATCH 6/6] x86/vdso: Move out the CPU number store
On 06/04/2018 10:24 PM, Chang S. Bae 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.
>
> Now, redundant setting of the segment in entry/vdso/vma.c
> was removed; a substantial code removal. It removes a
> hotplug notifier, makes a facility useful to both the kernel
> and userspace unconditionally available much sooner, and
> unification with i386. (Thanks to HPA for suggesting the
> cleanup)
>
> Signed-off-by: Chang S. Bae
> Cc: H. Peter Anvin
> Cc: Dave Hansen
> Cc: Andy Lutomirski
> Cc: Andi Kleen
> Cc: Thomas Gleixner
> Cc: Ingo Molnar
> ---
> diff --git a/arch/x86/kernel/setup_percpu.c b/arch/x86/kernel/setup_percpu.c
> index ea554f8..e716e94 100644
> --- a/arch/x86/kernel/setup_percpu.c
> +++ b/arch/x86/kernel/setup_percpu.c
> @@ -155,12 +155,21 @@ static void __init pcpup_populate_pte(unsigned long
> addr)
>
> static inline void setup_percpu_segment(int cpu)
> {
> -#ifdef CONFIG_X86_32
> - struct desc_struct d = GDT_ENTRY_INIT(0x8092, per_cpu_offset(cpu),
> - 0xF);
> +#ifdef CONFIG_NUMA
> + unsigned long node = early_cpu_to_node(cpu);
> +#else
> + unsigned long node = 0;
> +#endif
> + struct desc_struct d = GDT_ENTRY_INIT(0x0, per_cpu_offset(cpu),
> +make_lsl_tscp(cpu, node));
> +
> + d.type = 5; /* R0 data, expand down, accessed */
> + d.dpl = 3; /* Visible to user code */
> + d.s = 1;/* Not a system segment */
> + d.p = 1;/* Present */
> + d.d = 1;/* 32-bit */
>
> write_gdt_entry(get_cpu_gdt_rw(cpu), GDT_ENTRY_PERCPU, , DESCTYPE_S);
> -#endif
> }
This won't work on X86-32 because it actually uses the segment limit with fs:
access. So there
is a reason why the lsl based method is X86-64 only.
-Mika
>
> void __init setup_per_cpu_areas(void)
>
