2023-11-30 16:30:30

by Uros Bizjak

[permalink] [raw]
Subject: [PATCH] x86/percpu: Declare const_pcpu_hot as extern const variable

const_pcpu_hot is aliased by linker to pcpu_hot, so there is no need
to use DECLARE_PER_CPU_ALIGNED macro. Also, declare const_pcpu_hot
as extern to avoid allocating storage space for the aliased structure.

Reported-by: kernel test robot <[email protected]>
Closes: https://lore.kernel.org/oe-kbuild-all/[email protected]/

Fixes: ed2f752e0e0a ("x86/percpu: Introduce const-qualified const_pcpu_hot to micro-optimize code generation")
Cc: Andy Lutomirski <[email protected]>
Cc: Brian Gerst <[email protected]>
Cc: Denys Vlasenko <[email protected]>
Cc: Ingo Molnar <[email protected]>
Cc: H. Peter Anvin <[email protected]>
Cc: Linus Torvalds <[email protected]>
Cc: Peter Zijlstra <[email protected]>
Cc: Thomas Gleixner <[email protected]>
Cc: Josh Poimboeuf <[email protected]>
Signed-off-by: Uros Bizjak <[email protected]>
---
arch/x86/include/asm/current.h | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/arch/x86/include/asm/current.h b/arch/x86/include/asm/current.h
index 0538d2436673..9fbd7cb2dc86 100644
--- a/arch/x86/include/asm/current.h
+++ b/arch/x86/include/asm/current.h
@@ -37,8 +37,7 @@ static_assert(sizeof(struct pcpu_hot) == 64);
DECLARE_PER_CPU_ALIGNED(struct pcpu_hot, pcpu_hot);

/* const-qualified alias to pcpu_hot, aliased by linker. */
-DECLARE_PER_CPU_ALIGNED(const struct pcpu_hot __percpu_seg_override,
- const_pcpu_hot);
+extern const struct pcpu_hot __percpu_seg_override const_pcpu_hot;

static __always_inline struct task_struct *get_current(void)
{
--
2.42.0


2023-11-30 21:16:57

by tip-bot2 for Jacob Pan

[permalink] [raw]
Subject: [tip: x86/percpu] x86/percpu: Declare const_pcpu_hot as extern const variable

The following commit has been merged into the x86/percpu branch of tip:

Commit-ID: 4604c052b84d66407f5e68045a1939685eac401e
Gitweb: https://git.kernel.org/tip/4604c052b84d66407f5e68045a1939685eac401e
Author: Uros Bizjak <[email protected]>
AuthorDate: Thu, 30 Nov 2023 17:27:35 +01:00
Committer: Ingo Molnar <[email protected]>
CommitterDate: Thu, 30 Nov 2023 20:19:33 +01:00

x86/percpu: Declare const_pcpu_hot as extern const variable

const_pcpu_hot is aliased by linker to pcpu_hot, so there is no need
to use the DECLARE_PER_CPU_ALIGNED() macro. Also, declare const_pcpu_hot
as extern to avoid allocating storage space for the aliased structure.

Fixes: ed2f752e0e0a ("x86/percpu: Introduce const-qualified const_pcpu_hot to micro-optimize code generation")
Reported-by: kernel test robot <[email protected]>
Signed-off-by: Uros Bizjak <[email protected]>
Signed-off-by: Ingo Molnar <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
Closes: https://lore.kernel.org/oe-kbuild-all/[email protected]/
---
arch/x86/include/asm/current.h | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/arch/x86/include/asm/current.h b/arch/x86/include/asm/current.h
index 0538d24..9fbd7cb 100644
--- a/arch/x86/include/asm/current.h
+++ b/arch/x86/include/asm/current.h
@@ -37,8 +37,7 @@ static_assert(sizeof(struct pcpu_hot) == 64);
DECLARE_PER_CPU_ALIGNED(struct pcpu_hot, pcpu_hot);

/* const-qualified alias to pcpu_hot, aliased by linker. */
-DECLARE_PER_CPU_ALIGNED(const struct pcpu_hot __percpu_seg_override,
- const_pcpu_hot);
+extern const struct pcpu_hot __percpu_seg_override const_pcpu_hot;

static __always_inline struct task_struct *get_current(void)
{

2023-12-01 15:51:10

by Borislav Petkov

[permalink] [raw]
Subject: Re: [tip: x86/percpu] x86/percpu: Declare const_pcpu_hot as extern const variable

On Thu, Nov 30, 2023 at 09:16:30PM -0000, tip-bot2 for Uros Bizjak wrote:
> The following commit has been merged into the x86/percpu branch of tip:
>
> Commit-ID: 4604c052b84d66407f5e68045a1939685eac401e
> Gitweb: https://git.kernel.org/tip/4604c052b84d66407f5e68045a1939685eac401e
> Author: Uros Bizjak <[email protected]>
> AuthorDate: Thu, 30 Nov 2023 17:27:35 +01:00
> Committer: Ingo Molnar <[email protected]>
> CommitterDate: Thu, 30 Nov 2023 20:19:33 +01:00
>
> x86/percpu: Declare const_pcpu_hot as extern const variable
>
> const_pcpu_hot is aliased by linker to pcpu_hot, so there is no need
> to use the DECLARE_PER_CPU_ALIGNED() macro. Also, declare const_pcpu_hot
> as extern to avoid allocating storage space for the aliased structure.
>
> Fixes: ed2f752e0e0a ("x86/percpu: Introduce const-qualified const_pcpu_hot to micro-optimize code generation")
> Reported-by: kernel test robot <[email protected]>
> Signed-off-by: Uros Bizjak <[email protected]>
> Signed-off-by: Ingo Molnar <[email protected]>
> Link: https://lore.kernel.org/r/[email protected]
> Closes: https://lore.kernel.org/oe-kbuild-all/[email protected]/
> ---
> arch/x86/include/asm/current.h | 3 +--
> 1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/arch/x86/include/asm/current.h b/arch/x86/include/asm/current.h
> index 0538d24..9fbd7cb 100644
> --- a/arch/x86/include/asm/current.h
> +++ b/arch/x86/include/asm/current.h
> @@ -37,8 +37,7 @@ static_assert(sizeof(struct pcpu_hot) == 64);
> DECLARE_PER_CPU_ALIGNED(struct pcpu_hot, pcpu_hot);
>
> /* const-qualified alias to pcpu_hot, aliased by linker. */
> -DECLARE_PER_CPU_ALIGNED(const struct pcpu_hot __percpu_seg_override,
> - const_pcpu_hot);
> +extern const struct pcpu_hot __percpu_seg_override const_pcpu_hot;
>
> static __always_inline struct task_struct *get_current(void)
> {

WARNING: modpost: EXPORT symbol "const_pcpu_hot" [vmlinux] version generation failed, symbol will not be versioned.
Is "const_pcpu_hot" prototyped in <asm/asm-prototypes.h>?
WARNING: modpost: "const_pcpu_hot" [arch/x86/kvm/kvm.ko] has no CRC!
WARNING: modpost: "const_pcpu_hot" [arch/x86/kvm/kvm-intel.ko] has no CRC!
WARNING: modpost: "const_pcpu_hot" [kernel/trace/preemptirq_delay_test.ko] has no CRC!
WARNING: modpost: "const_pcpu_hot" [fs/exfat/exfat.ko] has no CRC!
WARNING: modpost: "const_pcpu_hot" [fs/ntfs/ntfs.ko] has no CRC!
WARNING: modpost: "const_pcpu_hot" [fs/xfs/xfs.ko] has no CRC!
WARNING: modpost: "const_pcpu_hot" [fs/btrfs/btrfs.ko] has no CRC!
WARNING: modpost: "const_pcpu_hot" [drivers/acpi/acpi_pad.ko] has no CRC!
WARNING: modpost: "const_pcpu_hot" [drivers/gpu/drm/display/drm_display_helper.ko] has no CRC!
WARNING: modpost: "const_pcpu_hot" [drivers/gpu/drm/drm.ko] has no CRC!
WARNING: modpost: "const_pcpu_hot" [drivers/gpu/drm/ttm/ttm.ko] has no CRC!
WARNING: modpost: "const_pcpu_hot" [drivers/gpu/drm/i915/i915.ko] has no CRC!
WARNING: modpost: "const_pcpu_hot" [drivers/misc/mei/mei.ko] has no CRC!
WARNING: modpost: "const_pcpu_hot" [drivers/usb/core/usbcore.ko] has no CRC!
WARNING: modpost: "const_pcpu_hot" [drivers/usb/class/cdc-acm.ko] has no CRC!
WARNING: modpost: "const_pcpu_hot" [drivers/usb/class/usblp.ko] has no CRC!
WARNING: modpost: "const_pcpu_hot" [drivers/usb/storage/usb-storage.ko] has no CRC!

And the usual fix for those things:

diff --git a/arch/x86/include/asm/asm-prototypes.h b/arch/x86/include/asm/asm-prototypes.h
index b1a98fa38828..7e6440bae63d 100644
--- a/arch/x86/include/asm/asm-prototypes.h
+++ b/arch/x86/include/asm/asm-prototypes.h
@@ -13,6 +13,7 @@
#include <asm/preempt.h>
#include <asm/asm.h>
#include <asm/gsseg.h>
+#include <asm/current.h>

#ifndef CONFIG_X86_CMPXCHG64
extern void cmpxchg8b_emu(void);

does NOT fix it.

--
Regards/Gruss,
Boris.

https://people.kernel.org/tglx/notes-about-netiquette

2023-12-01 16:41:52

by Uros Bizjak

[permalink] [raw]
Subject: Re: [tip: x86/percpu] x86/percpu: Declare const_pcpu_hot as extern const variable

On Fri, Dec 1, 2023 at 5:25 PM Borislav Petkov <[email protected]> wrote:
>
> On Thu, Nov 30, 2023 at 09:16:30PM -0000, tip-bot2 for Uros Bizjak wrote:
> > The following commit has been merged into the x86/percpu branch of tip:
> >
> > Commit-ID: 4604c052b84d66407f5e68045a1939685eac401e
> > Gitweb: https://git.kernel.org/tip/4604c052b84d66407f5e68045a1939685eac401e
> > Author: Uros Bizjak <[email protected]>
> > AuthorDate: Thu, 30 Nov 2023 17:27:35 +01:00
> > Committer: Ingo Molnar <[email protected]>
> > CommitterDate: Thu, 30 Nov 2023 20:19:33 +01:00
> >
> > x86/percpu: Declare const_pcpu_hot as extern const variable
> >
> > const_pcpu_hot is aliased by linker to pcpu_hot, so there is no need
> > to use the DECLARE_PER_CPU_ALIGNED() macro. Also, declare const_pcpu_hot
> > as extern to avoid allocating storage space for the aliased structure.
> >
> > Fixes: ed2f752e0e0a ("x86/percpu: Introduce const-qualified const_pcpu_hot to micro-optimize code generation")
> > Reported-by: kernel test robot <[email protected]>
> > Signed-off-by: Uros Bizjak <[email protected]>
> > Signed-off-by: Ingo Molnar <[email protected]>
> > Link: https://lore.kernel.org/r/[email protected]
> > Closes: https://lore.kernel.org/oe-kbuild-all/[email protected]/
> > ---
> > arch/x86/include/asm/current.h | 3 +--
> > 1 file changed, 1 insertion(+), 2 deletions(-)
> >
> > diff --git a/arch/x86/include/asm/current.h b/arch/x86/include/asm/current.h
> > index 0538d24..9fbd7cb 100644
> > --- a/arch/x86/include/asm/current.h
> > +++ b/arch/x86/include/asm/current.h
> > @@ -37,8 +37,7 @@ static_assert(sizeof(struct pcpu_hot) == 64);
> > DECLARE_PER_CPU_ALIGNED(struct pcpu_hot, pcpu_hot);
> >
> > /* const-qualified alias to pcpu_hot, aliased by linker. */
> > -DECLARE_PER_CPU_ALIGNED(const struct pcpu_hot __percpu_seg_override,
> > - const_pcpu_hot);
> > +extern const struct pcpu_hot __percpu_seg_override const_pcpu_hot;
> >
> > static __always_inline struct task_struct *get_current(void)
> > {
>
> WARNING: modpost: EXPORT symbol "const_pcpu_hot" [vmlinux] version generation failed, symbol will not be versioned.
> Is "const_pcpu_hot" prototyped in <asm/asm-prototypes.h>?
> WARNING: modpost: "const_pcpu_hot" [arch/x86/kvm/kvm.ko] has no CRC!
> WARNING: modpost: "const_pcpu_hot" [arch/x86/kvm/kvm-intel.ko] has no CRC!
> WARNING: modpost: "const_pcpu_hot" [kernel/trace/preemptirq_delay_test.ko] has no CRC!
> WARNING: modpost: "const_pcpu_hot" [fs/exfat/exfat.ko] has no CRC!
> WARNING: modpost: "const_pcpu_hot" [fs/ntfs/ntfs.ko] has no CRC!
> WARNING: modpost: "const_pcpu_hot" [fs/xfs/xfs.ko] has no CRC!
> WARNING: modpost: "const_pcpu_hot" [fs/btrfs/btrfs.ko] has no CRC!
> WARNING: modpost: "const_pcpu_hot" [drivers/acpi/acpi_pad.ko] has no CRC!
> WARNING: modpost: "const_pcpu_hot" [drivers/gpu/drm/display/drm_display_helper.ko] has no CRC!
> WARNING: modpost: "const_pcpu_hot" [drivers/gpu/drm/drm.ko] has no CRC!
> WARNING: modpost: "const_pcpu_hot" [drivers/gpu/drm/ttm/ttm.ko] has no CRC!
> WARNING: modpost: "const_pcpu_hot" [drivers/gpu/drm/i915/i915.ko] has no CRC!
> WARNING: modpost: "const_pcpu_hot" [drivers/misc/mei/mei.ko] has no CRC!
> WARNING: modpost: "const_pcpu_hot" [drivers/usb/core/usbcore.ko] has no CRC!
> WARNING: modpost: "const_pcpu_hot" [drivers/usb/class/cdc-acm.ko] has no CRC!
> WARNING: modpost: "const_pcpu_hot" [drivers/usb/class/usblp.ko] has no CRC!
> WARNING: modpost: "const_pcpu_hot" [drivers/usb/storage/usb-storage.ko] has no CRC!
>
> And the usual fix for those things:
>
> diff --git a/arch/x86/include/asm/asm-prototypes.h b/arch/x86/include/asm/asm-prototypes.h
> index b1a98fa38828..7e6440bae63d 100644
> --- a/arch/x86/include/asm/asm-prototypes.h
> +++ b/arch/x86/include/asm/asm-prototypes.h
> @@ -13,6 +13,7 @@
> #include <asm/preempt.h>
> #include <asm/asm.h>
> #include <asm/gsseg.h>
> +#include <asm/current.h>
>
> #ifndef CONFIG_X86_CMPXCHG64
> extern void cmpxchg8b_emu(void);
>
> does NOT fix it.

I looked into this problem a bit, it is triggered by CONFIG_MODVERSIONS=y

The problem is, that genksyms does not know the __seg_gs keyword and
simply ignores the symbol.

Running:

make V=1 arch/x86/kernel/cpu/common.symtypes

There is NOTHING regarding const_pcpu_hot in the resulting common.symtypes file.

The unpatched source should also fail here due to __seg_gs keyword,
but it magically didn't.

I don't know what to do here.

Uros.

2023-12-02 09:41:31

by Uros Bizjak

[permalink] [raw]
Subject: Re: [tip: x86/percpu] x86/percpu: Declare const_pcpu_hot as extern const variable

On Fri, Dec 1, 2023 at 5:41 PM Uros Bizjak <[email protected]> wrote:
>
> On Fri, Dec 1, 2023 at 5:25 PM Borislav Petkov <[email protected]> wrote:
> >
> > On Thu, Nov 30, 2023 at 09:16:30PM -0000, tip-bot2 for Uros Bizjak wrote:
> > > The following commit has been merged into the x86/percpu branch of tip:
> > >
> > > Commit-ID: 4604c052b84d66407f5e68045a1939685eac401e
> > > Gitweb: https://git.kernel.org/tip/4604c052b84d66407f5e68045a1939685eac401e
> > > Author: Uros Bizjak <[email protected]>
> > > AuthorDate: Thu, 30 Nov 2023 17:27:35 +01:00
> > > Committer: Ingo Molnar <[email protected]>
> > > CommitterDate: Thu, 30 Nov 2023 20:19:33 +01:00
> > >
> > > x86/percpu: Declare const_pcpu_hot as extern const variable
> > >
> > > const_pcpu_hot is aliased by linker to pcpu_hot, so there is no need
> > > to use the DECLARE_PER_CPU_ALIGNED() macro. Also, declare const_pcpu_hot
> > > as extern to avoid allocating storage space for the aliased structure.
> > >
> > > Fixes: ed2f752e0e0a ("x86/percpu: Introduce const-qualified const_pcpu_hot to micro-optimize code generation")
> > > Reported-by: kernel test robot <[email protected]>
> > > Signed-off-by: Uros Bizjak <[email protected]>
> > > Signed-off-by: Ingo Molnar <[email protected]>
> > > Link: https://lore.kernel.org/r/[email protected]
> > > Closes: https://lore.kernel.org/oe-kbuild-all/[email protected]/
> > > ---
> > > arch/x86/include/asm/current.h | 3 +--
> > > 1 file changed, 1 insertion(+), 2 deletions(-)
> > >
> > > diff --git a/arch/x86/include/asm/current.h b/arch/x86/include/asm/current.h
> > > index 0538d24..9fbd7cb 100644
> > > --- a/arch/x86/include/asm/current.h
> > > +++ b/arch/x86/include/asm/current.h
> > > @@ -37,8 +37,7 @@ static_assert(sizeof(struct pcpu_hot) == 64);
> > > DECLARE_PER_CPU_ALIGNED(struct pcpu_hot, pcpu_hot);
> > >
> > > /* const-qualified alias to pcpu_hot, aliased by linker. */
> > > -DECLARE_PER_CPU_ALIGNED(const struct pcpu_hot __percpu_seg_override,
> > > - const_pcpu_hot);
> > > +extern const struct pcpu_hot __percpu_seg_override const_pcpu_hot;
> > >
> > > static __always_inline struct task_struct *get_current(void)
> > > {
> >
> > WARNING: modpost: EXPORT symbol "const_pcpu_hot" [vmlinux] version generation failed, symbol will not be versioned.
> > Is "const_pcpu_hot" prototyped in <asm/asm-prototypes.h>?
> > WARNING: modpost: "const_pcpu_hot" [arch/x86/kvm/kvm.ko] has no CRC!
> > WARNING: modpost: "const_pcpu_hot" [arch/x86/kvm/kvm-intel.ko] has no CRC!
> > WARNING: modpost: "const_pcpu_hot" [kernel/trace/preemptirq_delay_test.ko] has no CRC!
> > WARNING: modpost: "const_pcpu_hot" [fs/exfat/exfat.ko] has no CRC!
> > WARNING: modpost: "const_pcpu_hot" [fs/ntfs/ntfs.ko] has no CRC!
> > WARNING: modpost: "const_pcpu_hot" [fs/xfs/xfs.ko] has no CRC!
> > WARNING: modpost: "const_pcpu_hot" [fs/btrfs/btrfs.ko] has no CRC!
> > WARNING: modpost: "const_pcpu_hot" [drivers/acpi/acpi_pad.ko] has no CRC!
> > WARNING: modpost: "const_pcpu_hot" [drivers/gpu/drm/display/drm_display_helper.ko] has no CRC!
> > WARNING: modpost: "const_pcpu_hot" [drivers/gpu/drm/drm.ko] has no CRC!
> > WARNING: modpost: "const_pcpu_hot" [drivers/gpu/drm/ttm/ttm.ko] has no CRC!
> > WARNING: modpost: "const_pcpu_hot" [drivers/gpu/drm/i915/i915.ko] has no CRC!
> > WARNING: modpost: "const_pcpu_hot" [drivers/misc/mei/mei.ko] has no CRC!
> > WARNING: modpost: "const_pcpu_hot" [drivers/usb/core/usbcore.ko] has no CRC!
> > WARNING: modpost: "const_pcpu_hot" [drivers/usb/class/cdc-acm.ko] has no CRC!
> > WARNING: modpost: "const_pcpu_hot" [drivers/usb/class/usblp.ko] has no CRC!
> > WARNING: modpost: "const_pcpu_hot" [drivers/usb/storage/usb-storage.ko] has no CRC!
> >
> > And the usual fix for those things:
> >
> > diff --git a/arch/x86/include/asm/asm-prototypes.h b/arch/x86/include/asm/asm-prototypes.h
> > index b1a98fa38828..7e6440bae63d 100644
> > --- a/arch/x86/include/asm/asm-prototypes.h
> > +++ b/arch/x86/include/asm/asm-prototypes.h
> > @@ -13,6 +13,7 @@
> > #include <asm/preempt.h>
> > #include <asm/asm.h>
> > #include <asm/gsseg.h>
> > +#include <asm/current.h>
> >
> > #ifndef CONFIG_X86_CMPXCHG64
> > extern void cmpxchg8b_emu(void);
> >
> > does NOT fix it.
>
> I looked into this problem a bit, it is triggered by CONFIG_MODVERSIONS=y
>
> The problem is, that genksyms does not know the __seg_gs keyword and
> simply ignores the symbol.
>
> Running:
>
> make V=1 arch/x86/kernel/cpu/common.symtypes
>
> There is NOTHING regarding const_pcpu_hot in the resulting common.symtypes file.
>
> The unpatched source should also fail here due to __seg_gs keyword,
> but it magically didn't.
>
> I don't know what to do here.

I have a solution for both problems, genksyms and sparse issue. The
exported declaration should be in generic named address space and the
symbol casted to __seg_gs address space at usage site.

Let me polish and test the patch a bit, I'll post the finished patch tomorrow.

Uros.