[PATCH v2] x86/speculation: Use DECLARE_PER_CPU for x86_spec_ctrl_current

[PATCH v2] x86/speculation: Use DECLARE_PER_CPU for x86_spec_ctrl_current

[Nathan Chancellor]

Clang warns:

  arch/x86/kernel/cpu/bugs.c:58:21: error: section attribute is specified on redeclared variable [-Werror,-Wsection]
  DEFINE_PER_CPU(u64, x86_spec_ctrl_current);
                      ^
  arch/x86/include/asm/nospec-branch.h:283:12: note: previous declaration is here
  extern u64 x86_spec_ctrl_current;
             ^
  1 error generated.

The declaration should be using DECLARE_PER_CPU instead so all
attributes stay in sync.

Cc: stable@vger.kernel.org
Fixes: fc02735b14ff ("KVM: VMX: Prevent guest RSB poisoning attacks with eIBRS")
Reported-by: kernel test robot <lkp@intel.com>
Signed-off-by: Nathan Chancellor <nathan@kernel.org>
---

v1 -> v2: https://lore.kernel.org/20220713152222.1697913-1-nathan@kernel.org/

* Use asm/percpu.h instead of linux/percpu.h to avoid static call
  include errors.

 arch/x86/include/asm/nospec-branch.h | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/arch/x86/include/asm/nospec-branch.h b/arch/x86/include/asm/nospec-branch.h
index bb05ed4f46bd..10a3bfc1eb23 100644
--- a/arch/x86/include/asm/nospec-branch.h
+++ b/arch/x86/include/asm/nospec-branch.h
@@ -11,6 +11,7 @@
 #include <asm/cpufeatures.h>
 #include <asm/msr-index.h>
 #include <asm/unwind_hints.h>
+#include <asm/percpu.h>
 
 #define RETPOLINE_THUNK_SIZE	32
 
@@ -280,7 +281,7 @@ static inline void indirect_branch_prediction_barrier(void)
 
 /* The Intel SPEC CTRL MSR base value cache */
 extern u64 x86_spec_ctrl_base;
-extern u64 x86_spec_ctrl_current;
+DECLARE_PER_CPU(u64, x86_spec_ctrl_current);
 extern void write_spec_ctrl_current(u64 val, bool force);
 extern u64 spec_ctrl_current(void);
 

base-commit: 72a8e05d4f66b5af7854df4490e3135168694b6b
-- 
2.37.1

Re: [PATCH v2] x86/speculation: Use DECLARE_PER_CPU for x86_spec_ctrl_current

[Nick Desaulniers]

On Wed, Jul 13, 2022 at 8:25 AM Nathan Chancellor <nathan@kernel.org> wrote:
>
> Clang warns:
>
>   arch/x86/kernel/cpu/bugs.c:58:21: error: section attribute is specified on redeclared variable [-Werror,-Wsection]
>   DEFINE_PER_CPU(u64, x86_spec_ctrl_current);
>                       ^
>   arch/x86/include/asm/nospec-branch.h:283:12: note: previous declaration is here
>   extern u64 x86_spec_ctrl_current;
>              ^
>   1 error generated.
>
> The declaration should be using DECLARE_PER_CPU instead so all
> attributes stay in sync.
>
> Cc: stable@vger.kernel.org
> Fixes: fc02735b14ff ("KVM: VMX: Prevent guest RSB poisoning attacks with eIBRS")
> Reported-by: kernel test robot <lkp@intel.com>
> Signed-off-by: Nathan Chancellor <nathan@kernel.org>

Thanks for the patch!
Reviewed-by: Nick Desaulniers <ndesaulniers@google.com>
Tested-by: Nick Desaulniers <ndesaulniers@google.com>

> ---
>
> v1 -> v2: https://lore.kernel.org/20220713152222.1697913-1-nathan@kernel.org/
>
> * Use asm/percpu.h instead of linux/percpu.h to avoid static call
>   include errors.
>
>  arch/x86/include/asm/nospec-branch.h | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/arch/x86/include/asm/nospec-branch.h b/arch/x86/include/asm/nospec-branch.h
> index bb05ed4f46bd..10a3bfc1eb23 100644
> --- a/arch/x86/include/asm/nospec-branch.h
> +++ b/arch/x86/include/asm/nospec-branch.h
> @@ -11,6 +11,7 @@
>  #include <asm/cpufeatures.h>
>  #include <asm/msr-index.h>
>  #include <asm/unwind_hints.h>
> +#include <asm/percpu.h>
>
>  #define RETPOLINE_THUNK_SIZE   32
>
> @@ -280,7 +281,7 @@ static inline void indirect_branch_prediction_barrier(void)
>
>  /* The Intel SPEC CTRL MSR base value cache */
>  extern u64 x86_spec_ctrl_base;
> -extern u64 x86_spec_ctrl_current;
> +DECLARE_PER_CPU(u64, x86_spec_ctrl_current);
>  extern void write_spec_ctrl_current(u64 val, bool force);
>  extern u64 spec_ctrl_current(void);
>
>
> base-commit: 72a8e05d4f66b5af7854df4490e3135168694b6b
> --
> 2.37.1
>
>


-- 
Thanks,
~Nick Desaulniers

Re: [PATCH v2] x86/speculation: Use DECLARE_PER_CPU for x86_spec_ctrl_current

[Peter Zijlstra]

On Wed, Jul 13, 2022 at 08:24:37AM -0700, Nathan Chancellor wrote:
> Clang warns:
> 
>   arch/x86/kernel/cpu/bugs.c:58:21: error: section attribute is specified on redeclared variable [-Werror,-Wsection]
>   DEFINE_PER_CPU(u64, x86_spec_ctrl_current);
>                       ^
>   arch/x86/include/asm/nospec-branch.h:283:12: note: previous declaration is here
>   extern u64 x86_spec_ctrl_current;
>              ^
>   1 error generated.
> 
> The declaration should be using DECLARE_PER_CPU instead so all
> attributes stay in sync.
> 
> Cc: stable@vger.kernel.org
> Fixes: fc02735b14ff ("KVM: VMX: Prevent guest RSB poisoning attacks with eIBRS")
> Reported-by: kernel test robot <lkp@intel.com>
> Signed-off-by: Nathan Chancellor <nathan@kernel.org>
> ---
> 
> v1 -> v2: https://lore.kernel.org/20220713152222.1697913-1-nathan@kernel.org/
> 
> * Use asm/percpu.h instead of linux/percpu.h to avoid static call
>   include errors.
> 
>  arch/x86/include/asm/nospec-branch.h | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/x86/include/asm/nospec-branch.h b/arch/x86/include/asm/nospec-branch.h
> index bb05ed4f46bd..10a3bfc1eb23 100644
> --- a/arch/x86/include/asm/nospec-branch.h
> +++ b/arch/x86/include/asm/nospec-branch.h
> @@ -11,6 +11,7 @@
>  #include <asm/cpufeatures.h>
>  #include <asm/msr-index.h>
>  #include <asm/unwind_hints.h>
> +#include <asm/percpu.h>
>  
>  #define RETPOLINE_THUNK_SIZE	32
>  

When I tried this earlier today I ran into cyclic headers, you sure this
works?

Re: [PATCH v2] x86/speculation: Use DECLARE_PER_CPU for x86_spec_ctrl_current

[Nathan Chancellor]

On Wed, Jul 13, 2022 at 06:22:41PM +0200, Peter Zijlstra wrote:
> On Wed, Jul 13, 2022 at 08:24:37AM -0700, Nathan Chancellor wrote:
> > Clang warns:
> > 
> >   arch/x86/kernel/cpu/bugs.c:58:21: error: section attribute is specified on redeclared variable [-Werror,-Wsection]
> >   DEFINE_PER_CPU(u64, x86_spec_ctrl_current);
> >                       ^
> >   arch/x86/include/asm/nospec-branch.h:283:12: note: previous declaration is here
> >   extern u64 x86_spec_ctrl_current;
> >              ^
> >   1 error generated.
> > 
> > The declaration should be using DECLARE_PER_CPU instead so all
> > attributes stay in sync.
> > 
> > Cc: stable@vger.kernel.org
> > Fixes: fc02735b14ff ("KVM: VMX: Prevent guest RSB poisoning attacks with eIBRS")
> > Reported-by: kernel test robot <lkp@intel.com>
> > Signed-off-by: Nathan Chancellor <nathan@kernel.org>
> > ---
> > 
> > v1 -> v2: https://lore.kernel.org/20220713152222.1697913-1-nathan@kernel.org/
> > 
> > * Use asm/percpu.h instead of linux/percpu.h to avoid static call
> >   include errors.
> > 
> >  arch/x86/include/asm/nospec-branch.h | 3 ++-
> >  1 file changed, 2 insertions(+), 1 deletion(-)
> > 
> > diff --git a/arch/x86/include/asm/nospec-branch.h b/arch/x86/include/asm/nospec-branch.h
> > index bb05ed4f46bd..10a3bfc1eb23 100644
> > --- a/arch/x86/include/asm/nospec-branch.h
> > +++ b/arch/x86/include/asm/nospec-branch.h
> > @@ -11,6 +11,7 @@
> >  #include <asm/cpufeatures.h>
> >  #include <asm/msr-index.h>
> >  #include <asm/unwind_hints.h>
> > +#include <asm/percpu.h>
> >  
> >  #define RETPOLINE_THUNK_SIZE	32
> >  
> 
> When I tried this earlier today I ran into cyclic headers, you sure this
> works?

Yes, I did my regular set of x86 builds with clang and they all passed.

Cheers,
Nathan

Re: [PATCH v2] x86/speculation: Use DECLARE_PER_CPU for x86_spec_ctrl_current

[Jakub Kicinski]

On Wed, 13 Jul 2022 08:24:37 -0700 Nathan Chancellor wrote:
> Clang warns:
> 
>   arch/x86/kernel/cpu/bugs.c:58:21: error: section attribute is specified on redeclared variable [-Werror,-Wsection]
>   DEFINE_PER_CPU(u64, x86_spec_ctrl_current);
>                       ^
>   arch/x86/include/asm/nospec-branch.h:283:12: note: previous declaration is here
>   extern u64 x86_spec_ctrl_current;
>              ^
>   1 error generated.
> 
> The declaration should be using DECLARE_PER_CPU instead so all
> attributes stay in sync.
> 
> Cc: stable@vger.kernel.org
> Fixes: fc02735b14ff ("KVM: VMX: Prevent guest RSB poisoning attacks with eIBRS")
> Reported-by: kernel test robot <lkp@intel.com>
> Signed-off-by: Nathan Chancellor <nathan@kernel.org>

Hi, sorry to bother, any idea on the ETA for this fix getting into
Linus's tree? I'm trying to figure out if we should wait with 
forwarding the networking trees or this will take a while.

Re: [PATCH v2] x86/speculation: Use DECLARE_PER_CPU for x86_spec_ctrl_current

[Linus Torvalds]

On Thu, Jul 14, 2022 at 2:30 PM Jakub Kicinski <kuba@kernel.org> wrote:
>
> Hi, sorry to bother, any idea on the ETA for this fix getting into
> Linus's tree? I'm trying to figure out if we should wait with
> forwarding the networking trees or this will take a while.

Well, I have that patch by now in my own clang tree, but was holding
it off just because I was expecting a few other fixes for the fallout.

But if this particular one causes problems for maintainers, I can
easily just take it right away just cherry-pick it from my own
test-tree to my "main" tree.

            Linus

Re: [PATCH v2] x86/speculation: Use DECLARE_PER_CPU for x86_spec_ctrl_current

[Linus Torvalds]

On Thu, Jul 14, 2022 at 2:51 PM Linus Torvalds
<torvalds@linux-foundation.org> wrote:
>
> But if this particular one causes problems for maintainers, I can
> easily just take it right away just cherry-pick it from my own
> test-tree to my "main" tree.

Oh, and as I did that cherry-pick, I suddenly remembered that I think
PeterZ had a slightly different version of it - the one I picked up is
Nathan's "v2".

PeterZ, do you have preferences? I've waited this long, I might as
well wait a bit more before I push out whatever version people prefer.

              Linus

Re: [PATCH v2] x86/speculation: Use DECLARE_PER_CPU for x86_spec_ctrl_current

[Jakub Kicinski]

On Thu, 14 Jul 2022 14:51:52 -0700 Linus Torvalds wrote:
> On Thu, Jul 14, 2022 at 2:30 PM Jakub Kicinski <kuba@kernel.org> wrote:
> > Hi, sorry to bother, any idea on the ETA for this fix getting into
> > Linus's tree? I'm trying to figure out if we should wait with
> > forwarding the networking trees or this will take a while.  
> 
> Well, I have that patch by now in my own clang tree, but was holding
> it off just because I was expecting a few other fixes for the fallout.
> 
> But if this particular one causes problems for maintainers, I can
> easily just take it right away just cherry-pick it from my own
> test-tree to my "main" tree.

I have clang 13, let me double check this fix is enough for the build
to complete without disabling WERROR. If it's not a hassle it'd
certainly make my life easier if the fix gotten applied now...

Re: [PATCH v2] x86/speculation: Use DECLARE_PER_CPU for x86_spec_ctrl_current

[Peter Zijlstra]

On Thu, Jul 14, 2022 at 02:56:12PM -0700, Linus Torvalds wrote:
> On Thu, Jul 14, 2022 at 2:51 PM Linus Torvalds
> <torvalds@linux-foundation.org> wrote:
> >
> > But if this particular one causes problems for maintainers, I can
> > easily just take it right away just cherry-pick it from my own
> > test-tree to my "main" tree.
> 
> Oh, and as I did that cherry-pick, I suddenly remembered that I think
> PeterZ had a slightly different version of it - the one I picked up is
> Nathan's "v2".
> 
> PeterZ, do you have preferences? I've waited this long, I might as
> well wait a bit more before I push out whatever version people prefer.

Nathan's is much nicer; I got bit by header hell and punted.

Re: [PATCH v2] x86/speculation: Use DECLARE_PER_CPU for x86_spec_ctrl_current

[Linus Torvalds]

On Thu, Jul 14, 2022 at 2:56 PM Jakub Kicinski <kuba@kernel.org> wrote:
>
> I have clang 13, let me double check this fix is enough for the build
> to complete without disabling WERROR.

I have clang 14 locally, and it builds fine with that (and doesn't
build without it).

I actually normally build the kernel with both gcc and clang. My
"upstream" kernel I build with gcc, and then I have my "private random
collection of patches" kernel that I build with clang and that are
just rebased on top of the kernel-of-the-day.

This is all entirely for historical reasons - part of my "private
random collection of patches" used to be the "asm goto with outputs",
which had clang support first.

But then the reason I never even noticed the build breakage with the
retbleed patches until much too late was that those I just had as a
third fork off my upstream kernel, so despite me usually building with
clang too, that only got attention from gcc.

So it's really just a microcosm version of the exact same bigger issue
we always have with those embargoed hw security patches: they end up
missing out on all the usual test environments.

Anyway, I cherry-picked Nathan's patch from my clang tree and pushed
it out as commit db886979683a ("x86/speculation: Use DECLARE_PER_CPU
for x86_spec_ctrl_current").

                 Linus

Re: [PATCH v2] x86/speculation: Use DECLARE_PER_CPU for x86_spec_ctrl_current

[Jakub Kicinski]

On Thu, 14 Jul 2022 15:14:53 -0700 Linus Torvalds wrote:
> On Thu, Jul 14, 2022 at 2:56 PM Jakub Kicinski <kuba@kernel.org> wrote:
> > I have clang 13, let me double check this fix is enough for the build
> > to complete without disabling WERROR.  
> 
> I have clang 14 locally, and it builds fine with that (and doesn't
> build without it).

FWIW I can confirm - builds with clang 13 as well.

> I actually normally build the kernel with both gcc and clang. My
> "upstream" kernel I build with gcc, and then I have my "private random
> collection of patches" kernel that I build with clang and that are
> just rebased on top of the kernel-of-the-day.
> 
> This is all entirely for historical reasons - part of my "private
> random collection of patches" used to be the "asm goto with outputs",
> which had clang support first.
> 
> But then the reason I never even noticed the build breakage with the
> retbleed patches until much too late was that those I just had as a
> third fork off my upstream kernel, so despite me usually building with
> clang too, that only got attention from gcc.
> 
> So it's really just a microcosm version of the exact same bigger issue
> we always have with those embargoed hw security patches: they end up
> missing out on all the usual test environments.
> 
> Anyway, I cherry-picked Nathan's patch from my clang tree and pushed
> it out as commit db886979683a ("x86/speculation: Use DECLARE_PER_CPU
> for x86_spec_ctrl_current").

Awesome, thanks!

Re: [PATCH v2] x86/speculation: Use DECLARE_PER_CPU for x86_spec_ctrl_current

[Borislav Petkov]

On Thu, Jul 14, 2022 at 03:14:53PM -0700, Linus Torvalds wrote:
> Anyway, I cherry-picked Nathan's patch from my clang tree and pushed
> it out as commit db886979683a ("x86/speculation: Use DECLARE_PER_CPU
> for x86_spec_ctrl_current").

... and I've zapped it from my lineup.

Thx.

-- 
Regards/Gruss,
    Boris.

SUSE Software Solutions Germany GmbH
GF: Ivo Totev, Andrew Myers, Andrew McDonald, Martje Boudien Moerman
(HRB 36809, AG Nürnberg)