correct section for cpu_tss_rw?

correct section for cpu_tss_rw?

[Nick Desaulniers]

(emailing the folks listed from running `./scripts/get_maintainer.pl
-f` on arch/x86/kernel/process.c, arch/x86/include/asm/processor.h,
and include/linux/percpu-defs.h)

Clang emits the following warning:

arch/x86/kernel/process.c:50:11: warning: section does not match
previous declaration [-Wsection]
__visible DEFINE_PER_CPU_SHARED_ALIGNED(struct tss_struct, cpu_tss_rw) = {
          ^
./include/linux/percpu-defs.h:144:2: note: expanded from macro
'DEFINE_PER_CPU_SHARED_ALIGNED'
        DEFINE_PER_CPU_SECTION(type, name, PER_CPU_SHARED_ALIGNED_SECTION) \
        ^
./include/linux/percpu-defs.h:104:2: note: expanded from macro
'DEFINE_PER_CPU_SECTION'
        __PCPU_ATTRS(sec) PER_CPU_DEF_ATTRIBUTES                        \
        ^
./include/linux/percpu-defs.h:49:26: note: expanded from macro '__PCPU_ATTRS'
        __percpu __attribute__((section(PER_CPU_BASE_SECTION sec)))     \
                                ^
./arch/x86/include/asm/processor.h:365:1: note: previous attribute is here
DECLARE_PER_CPU_PAGE_ALIGNED(struct tss_struct, cpu_tss_rw);
^
./include/linux/percpu-defs.h:159:2: note: expanded from macro
'DECLARE_PER_CPU_PAGE_ALIGNED'
        DECLARE_PER_CPU_SECTION(type, name, "..page_aligned")           \
        ^
./include/linux/percpu-defs.h:101:9: note: expanded from macro
'DECLARE_PER_CPU_SECTION'
        extern __PCPU_ATTRS(sec) __typeof__(type) name
               ^
./include/linux/percpu-defs.h:49:26: note: expanded from macro '__PCPU_ATTRS'
        __percpu __attribute__((section(PER_CPU_BASE_SECTION sec)))     \
                                ^

it seems that from commit c482feefe1a ("x86/entry/64: Make
cpu_entry_area.tss read-only") that cpu_tss_rw is declared but then
defined in two different sections. (Though, it looks like this issue
predates that commit).

In include/linux/percpu-defs.h, there's two pairs of macros:

DECLARE_PER_CPU_SHARED_ALIGNED
DEFINE_PER_CPU_SHARED_ALIGNED
DECLARE_PER_CPU_PAGE_ALIGNED
DEFINE_PER_CPU_PAGE_ALIGNED

It seems that cpu_tss_rw is defined as SHARED_ALIGNED, but then
declared as PAGE_ALIGNED.  Should be an easy fix (that I'm happy to
author), but what section *should* cpu_tss_rw be in (SHARED_ALIGNED or
PAGE_ALIGNED)?  That affects whether I fix the declaration or
definition (and thus the .h or the .c file).

>From the comment in arch/x86/kernel/process.c#50:
 43 /*
 44  * per-CPU TSS segments. Threads are completely 'soft' on Linux,
 45  * no more per-task TSS's. The TSS size is kept cacheline-aligned
 46  * so they are allowed to end up in the .data..cacheline_aligned
 47  * section. Since TSS's are completely CPU-local, we want them
 48  * on exact cacheline boundaries, to eliminate cacheline
ping-pong.
 49  */
 50 __visible DEFINE_PER_CPU_SHARED_ALIGNED(struct tss_struct, cpu_tss_rw) = {

I suspect that cache-line alignment is stricter than page alignment,
so the declaration should be fixed, but I was not sure and wanted to
check?

-- 
Thanks,
~Nick Desaulniers

Re: correct section for cpu_tss_rw?

[Thomas Gleixner]

On Tue, 2 Jan 2018, Nick Desaulniers wrote:
> (emailing the folks listed from running `./scripts/get_maintainer.pl
> -f` on arch/x86/kernel/process.c, arch/x86/include/asm/processor.h,
> and include/linux/percpu-defs.h)
> 
> Clang emits the following warning:
> 
> arch/x86/kernel/process.c:50:11: warning: section does not match
> previous declaration [-Wsection]
> __visible DEFINE_PER_CPU_SHARED_ALIGNED(struct tss_struct, cpu_tss_rw) = {
>           ^
> ./include/linux/percpu-defs.h:144:2: note: expanded from macro
> 'DEFINE_PER_CPU_SHARED_ALIGNED'
>         DEFINE_PER_CPU_SECTION(type, name, PER_CPU_SHARED_ALIGNED_SECTION) \
>         ^
> ./include/linux/percpu-defs.h:104:2: note: expanded from macro
> 'DEFINE_PER_CPU_SECTION'
>         __PCPU_ATTRS(sec) PER_CPU_DEF_ATTRIBUTES                        \
>         ^
> ./include/linux/percpu-defs.h:49:26: note: expanded from macro '__PCPU_ATTRS'
>         __percpu __attribute__((section(PER_CPU_BASE_SECTION sec)))     \
>                                 ^
> ./arch/x86/include/asm/processor.h:365:1: note: previous attribute is here
> DECLARE_PER_CPU_PAGE_ALIGNED(struct tss_struct, cpu_tss_rw);
> ^
> ./include/linux/percpu-defs.h:159:2: note: expanded from macro
> 'DECLARE_PER_CPU_PAGE_ALIGNED'
>         DECLARE_PER_CPU_SECTION(type, name, "..page_aligned")           \
>         ^
> ./include/linux/percpu-defs.h:101:9: note: expanded from macro
> 'DECLARE_PER_CPU_SECTION'
>         extern __PCPU_ATTRS(sec) __typeof__(type) name
>                ^
> ./include/linux/percpu-defs.h:49:26: note: expanded from macro '__PCPU_ATTRS'
>         __percpu __attribute__((section(PER_CPU_BASE_SECTION sec)))     \
>                                 ^
> 
> it seems that from commit c482feefe1a ("x86/entry/64: Make
> cpu_entry_area.tss read-only") that cpu_tss_rw is declared but then
> defined in two different sections. (Though, it looks like this issue
> predates that commit).
>
> It seems that cpu_tss_rw is defined as SHARED_ALIGNED, but then
> declared as PAGE_ALIGNED.  Should be an easy fix (that I'm happy to
> author), but what section *should* cpu_tss_rw be in (SHARED_ALIGNED or
> PAGE_ALIGNED)?  That affects whether I fix the declaration or
> definition (and thus the .h or the .c file).
> 
> >From the comment in arch/x86/kernel/process.c#50:
>  43 /*
>  44  * per-CPU TSS segments. Threads are completely 'soft' on Linux,
>  45  * no more per-task TSS's. The TSS size is kept cacheline-aligned
>  46  * so they are allowed to end up in the .data..cacheline_aligned
>  47  * section. Since TSS's are completely CPU-local, we want them
>  48  * on exact cacheline boundaries, to eliminate cacheline
> ping-pong.
>  49  */
>  50 __visible DEFINE_PER_CPU_SHARED_ALIGNED(struct tss_struct, cpu_tss_rw) = {
> 
> I suspect that cache-line alignment is stricter than page alignment,
> so the declaration should be fixed, but I was not sure and wanted to
> check?

It must be page aligned.

Thanks,

	tglx

[PATCH] x86/process: define cpu_tss_rw in same section as declaration

[Nick Desaulniers]

cpu_tss_rw was declared with the macro

DECLARE_PER_CPU_PAGE_ALIGNED

but then defined with the macro

DEFINE_PER_CPU_SHARED_ALIGNED

leading to section mismatch warnings. Prefer the macro

DEFINE_PER_CPU_PAGE_ALIGNED

Suggested-by: Thomas Gleixner <tglx@linutronix.de>
Signed-off-by: Nick Desaulniers <ndesaulniers@google.com>
---
 arch/x86/kernel/process.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/x86/kernel/process.c b/arch/x86/kernel/process.c
index aed9d94bd46f..832a6acd730f 100644
--- a/arch/x86/kernel/process.c
+++ b/arch/x86/kernel/process.c
@@ -47,7 +47,7 @@
  * section. Since TSS's are completely CPU-local, we want them
  * on exact cacheline boundaries, to eliminate cacheline ping-pong.
  */
-__visible DEFINE_PER_CPU_SHARED_ALIGNED(struct tss_struct, cpu_tss_rw) = {
+__visible DEFINE_PER_CPU_PAGE_ALIGNED(struct tss_struct, cpu_tss_rw) = {
 	.x86_tss = {
 		/*
 		 * .sp0 is only used when entering ring 0 from a lower
-- 
2.16.0.rc0.223.g4a4ac83678-goog

[tip:x86/pti] x86/process: Define cpu_tss_rw in same section as declaration

[tip-bot for Nick Desaulniers]

Commit-ID:  2fd9c41aea47f4ad071accf94b94f94f2c4d31eb
Gitweb:     https://git.kernel.org/tip/2fd9c41aea47f4ad071accf94b94f94f2c4d31eb
Author:     Nick Desaulniers <ndesaulniers@google.com>
AuthorDate: Wed, 3 Jan 2018 12:39:52 -0800
Committer:  Thomas Gleixner <tglx@linutronix.de>
CommitDate: Wed, 3 Jan 2018 23:19:33 +0100

x86/process: Define cpu_tss_rw in same section as declaration

cpu_tss_rw is declared with DECLARE_PER_CPU_PAGE_ALIGNED
but then defined with DEFINE_PER_CPU_SHARED_ALIGNED
leading to section mismatch warnings.

Use DEFINE_PER_CPU_PAGE_ALIGNED consistently. This is necessary because
it's mapped to the cpu entry area and must be page aligned.

[ tglx: Massaged changelog a bit ]

Fixes: 1a935bc3d4ea ("x86/entry: Move SYSENTER_stack to the beginning of struct tss_struct")
Suggested-by: Thomas Gleixner <tglx@linutronix.de>
Signed-off-by: Nick Desaulniers <ndesaulniers@google.com>
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Cc: thomas.lendacky@amd.com
Cc: Borislav Petkov <bpetkov@suse.de>
Cc: tklauser@distanz.ch
Cc: minipli@googlemail.com
Cc: me@kylehuey.com
Cc: namit@vmware.com
Cc: luto@kernel.org
Cc: jpoimboe@redhat.com
Cc: tj@kernel.org
Cc: cl@linux.com
Cc: bp@suse.de
Cc: thgarnie@google.com
Cc: kirill.shutemov@linux.intel.com
Cc: stable@vger.kernel.org
Link: https://lkml.kernel.org/r/20180103203954.183360-1-ndesaulniers@google.com

---
 arch/x86/kernel/process.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/x86/kernel/process.c b/arch/x86/kernel/process.c
index 5174159..3cb2486 100644
--- a/arch/x86/kernel/process.c
+++ b/arch/x86/kernel/process.c
@@ -47,7 +47,7 @@
  * section. Since TSS's are completely CPU-local, we want them
  * on exact cacheline boundaries, to eliminate cacheline ping-pong.
  */
-__visible DEFINE_PER_CPU_SHARED_ALIGNED(struct tss_struct, cpu_tss_rw) = {
+__visible DEFINE_PER_CPU_PAGE_ALIGNED(struct tss_struct, cpu_tss_rw) = {
 	.x86_tss = {
 		/*
 		 * .sp0 is only used when entering ring 0 from a lower