linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/2] Compiler Attributes, s390: Provide and use __uninitialized macro
@ 2024-02-05 15:48 Heiko Carstens
  2024-02-05 15:48 ` [PATCH 1/2] Compiler Attributes: Add " Heiko Carstens
  2024-02-05 15:48 ` [PATCH 2/2] s390/fpu: make use of " Heiko Carstens
  0 siblings, 2 replies; 9+ messages in thread
From: Heiko Carstens @ 2024-02-05 15:48 UTC (permalink / raw)
  To: Kees Cook, Nathan Chancellor, Nick Desaulniers
  Cc: linux-kernel, linux-s390, Vasily Gorbik, Alexander Gordeev

With INIT_STACK_ALL_PATTERN or INIT_STACK_ALL_ZERO enabled the kernel will
be compiled with -ftrivial-auto-var-init=<...> which causes initialization
of stack variables at function entry time.

In order to avoid the performance impact that comes with this users can use
the "uninitialized" attribute to prevent such initialization.

In particular code sections in s390 specific kernel code which use floating
point or vector registers all come with a 520 byte stack variable to save
already in use registers, if required.

If the above named config options are enabled this stack variable will
always be initialized on function entry in addition to saving register
contents, which contradicts the intend (performance improvement) of such
code sections.

Therefore provide a generic __uninitialized macro and an s390 specific
DECLARE_KERNEL_FPU_ONSTACK() macro which provides a kernel fpu variable
with an __uninitialized attribute, and convert all existing code to use
this.

If people are ok which this approach, I'd like to carry this via the s390
tree to avoid potential merge conflicts, since there is a larger fpu code
rework pending

Thanks,
Heiko

Heiko Carstens (2):
  Compiler Attributes: Add __uninitialized macro
  s390/fpu: make use of __uninitialized macro

 arch/s390/crypto/chacha-glue.c      |  2 +-
 arch/s390/crypto/crc32-vx.c         |  2 +-
 arch/s390/include/asm/fpu/types.h   |  3 +++
 arch/s390/kernel/sysinfo.c          |  2 +-
 include/linux/compiler_attributes.h | 12 ++++++++++++
 lib/raid6/s390vx.uc                 |  4 ++--
 6 files changed, 20 insertions(+), 5 deletions(-)

-- 
2.40.1


^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH 1/2] Compiler Attributes: Add __uninitialized macro
  2024-02-05 15:48 [PATCH 0/2] Compiler Attributes, s390: Provide and use __uninitialized macro Heiko Carstens
@ 2024-02-05 15:48 ` Heiko Carstens
  2024-02-05 16:21   ` Kees Cook
  2024-02-06  1:28   ` Nathan Chancellor
  2024-02-05 15:48 ` [PATCH 2/2] s390/fpu: make use of " Heiko Carstens
  1 sibling, 2 replies; 9+ messages in thread
From: Heiko Carstens @ 2024-02-05 15:48 UTC (permalink / raw)
  To: Kees Cook, Nathan Chancellor, Nick Desaulniers
  Cc: linux-kernel, linux-s390, Vasily Gorbik, Alexander Gordeev

With INIT_STACK_ALL_PATTERN or INIT_STACK_ALL_ZERO enabled the kernel will
be compiled with -ftrivial-auto-var-init=<...> which causes initialization
of stack variables at function entry time.

In order to avoid the performance impact that comes with this users can use
the "uninitialized" attribute to prevent such initialization.

Therefore provide the __uninitialized macro which can be used for cases
where INIT_STACK_ALL_PATTERN or INIT_STACK_ALL_ZERO is enabled, but only
selected variables should not be initialized.

Signed-off-by: Heiko Carstens <hca@linux.ibm.com>
---
 include/linux/compiler_attributes.h | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/include/linux/compiler_attributes.h b/include/linux/compiler_attributes.h
index 28566624f008..f5859b8c68b4 100644
--- a/include/linux/compiler_attributes.h
+++ b/include/linux/compiler_attributes.h
@@ -333,6 +333,18 @@
  */
 #define __section(section)              __attribute__((__section__(section)))
 
+/*
+ * Optional: only supported since gcc >= 12
+ *
+ *   gcc: https://gcc.gnu.org/onlinedocs/gcc/Common-Variable-Attributes.html#index-uninitialized-variable-attribute
+ * clang: https://clang.llvm.org/docs/AttributeReference.html#uninitialized
+ */
+#if __has_attribute(__uninitialized__)
+# define __uninitialized		__attribute__((__uninitialized__))
+#else
+# define __uninitialized
+#endif
+
 /*
  *   gcc: https://gcc.gnu.org/onlinedocs/gcc/Common-Function-Attributes.html#index-unused-function-attribute
  *   gcc: https://gcc.gnu.org/onlinedocs/gcc/Common-Type-Attributes.html#index-unused-type-attribute
-- 
2.40.1


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH 2/2] s390/fpu: make use of __uninitialized macro
  2024-02-05 15:48 [PATCH 0/2] Compiler Attributes, s390: Provide and use __uninitialized macro Heiko Carstens
  2024-02-05 15:48 ` [PATCH 1/2] Compiler Attributes: Add " Heiko Carstens
@ 2024-02-05 15:48 ` Heiko Carstens
  2024-02-05 16:26   ` Kees Cook
  2024-02-06  1:31   ` Nathan Chancellor
  1 sibling, 2 replies; 9+ messages in thread
From: Heiko Carstens @ 2024-02-05 15:48 UTC (permalink / raw)
  To: Kees Cook, Nathan Chancellor, Nick Desaulniers
  Cc: linux-kernel, linux-s390, Vasily Gorbik, Alexander Gordeev

Code sections in s390 specific kernel code which use floating point or
vector registers all come with a 520 byte stack variable to save already in
use registers, if required.

With INIT_STACK_ALL_PATTERN or INIT_STACK_ALL_ZERO enabled this variable
will always be initialized on function entry in addition to saving register
contents, which contradicts the intend (performance improvement) of such
code sections.

Therefore provide a DECLARE_KERNEL_FPU_ONSTACK() macro which provides
struct kernel_fpu variables with an __uninitialized attribute, and convert
all existing code to use this.

This way only this specific type of stack variable will not be initialized,
regardless of config options.

Signed-off-by: Heiko Carstens <hca@linux.ibm.com>
---
 arch/s390/crypto/chacha-glue.c    | 2 +-
 arch/s390/crypto/crc32-vx.c       | 2 +-
 arch/s390/include/asm/fpu/types.h | 3 +++
 arch/s390/kernel/sysinfo.c        | 2 +-
 lib/raid6/s390vx.uc               | 4 ++--
 5 files changed, 8 insertions(+), 5 deletions(-)

diff --git a/arch/s390/crypto/chacha-glue.c b/arch/s390/crypto/chacha-glue.c
index ed9959e6f714..a823132fc563 100644
--- a/arch/s390/crypto/chacha-glue.c
+++ b/arch/s390/crypto/chacha-glue.c
@@ -22,7 +22,7 @@ static void chacha20_crypt_s390(u32 *state, u8 *dst, const u8 *src,
 				unsigned int nbytes, const u32 *key,
 				u32 *counter)
 {
-	struct kernel_fpu vxstate;
+	DECLARE_KERNEL_FPU_ONSTACK(vxstate);
 
 	kernel_fpu_begin(&vxstate, KERNEL_VXR);
 	chacha20_vx(dst, src, nbytes, key, counter);
diff --git a/arch/s390/crypto/crc32-vx.c b/arch/s390/crypto/crc32-vx.c
index 017143e9cef7..6ae3e3ff5b0a 100644
--- a/arch/s390/crypto/crc32-vx.c
+++ b/arch/s390/crypto/crc32-vx.c
@@ -49,8 +49,8 @@ u32 crc32c_le_vgfm_16(u32 crc, unsigned char const *buf, size_t size);
 	static u32 __pure ___fname(u32 crc,				    \
 				unsigned char const *data, size_t datalen)  \
 	{								    \
-		struct kernel_fpu vxstate;				    \
 		unsigned long prealign, aligned, remaining;		    \
+		DECLARE_KERNEL_FPU_ONSTACK(vxstate);			    \
 									    \
 		if (datalen < VX_MIN_LEN + VX_ALIGN_MASK)		    \
 			return ___crc32_sw(crc, data, datalen);		    \
diff --git a/arch/s390/include/asm/fpu/types.h b/arch/s390/include/asm/fpu/types.h
index d889e9436865..b1afa13c07b7 100644
--- a/arch/s390/include/asm/fpu/types.h
+++ b/arch/s390/include/asm/fpu/types.h
@@ -35,4 +35,7 @@ struct kernel_fpu {
 	};
 };
 
+#define DECLARE_KERNEL_FPU_ONSTACK(name)	\
+	struct kernel_fpu name __uninitialized
+
 #endif /* _ASM_S390_FPU_TYPES_H */
diff --git a/arch/s390/kernel/sysinfo.c b/arch/s390/kernel/sysinfo.c
index f6f8f498c9be..b439f17516eb 100644
--- a/arch/s390/kernel/sysinfo.c
+++ b/arch/s390/kernel/sysinfo.c
@@ -426,9 +426,9 @@ subsys_initcall(create_proc_service_level);
  */
 void s390_adjust_jiffies(void)
 {
+	DECLARE_KERNEL_FPU_ONSTACK(fpu);
 	struct sysinfo_1_2_2 *info;
 	unsigned long capability;
-	struct kernel_fpu fpu;
 
 	info = (void *) get_zeroed_page(GFP_KERNEL);
 	if (!info)
diff --git a/lib/raid6/s390vx.uc b/lib/raid6/s390vx.uc
index cd0b9e95f499..7b0b355e1a26 100644
--- a/lib/raid6/s390vx.uc
+++ b/lib/raid6/s390vx.uc
@@ -81,7 +81,7 @@ static inline void COPY_VEC(int x, int y)
 
 static void raid6_s390vx$#_gen_syndrome(int disks, size_t bytes, void **ptrs)
 {
-	struct kernel_fpu vxstate;
+	DECLARE_KERNEL_FPU_ONSTACK(vxstate);
 	u8 **dptr, *p, *q;
 	int d, z, z0;
 
@@ -114,7 +114,7 @@ static void raid6_s390vx$#_gen_syndrome(int disks, size_t bytes, void **ptrs)
 static void raid6_s390vx$#_xor_syndrome(int disks, int start, int stop,
 					size_t bytes, void **ptrs)
 {
-	struct kernel_fpu vxstate;
+	DECLARE_KERNEL_FPU_ONSTACK(vxstate);
 	u8 **dptr, *p, *q;
 	int d, z, z0;
 
-- 
2.40.1


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [PATCH 1/2] Compiler Attributes: Add __uninitialized macro
  2024-02-05 15:48 ` [PATCH 1/2] Compiler Attributes: Add " Heiko Carstens
@ 2024-02-05 16:21   ` Kees Cook
  2024-02-06  1:28   ` Nathan Chancellor
  1 sibling, 0 replies; 9+ messages in thread
From: Kees Cook @ 2024-02-05 16:21 UTC (permalink / raw)
  To: Heiko Carstens
  Cc: Nathan Chancellor, Nick Desaulniers, linux-kernel, linux-s390,
	Vasily Gorbik, Alexander Gordeev

On Mon, Feb 05, 2024 at 04:48:43PM +0100, Heiko Carstens wrote:
> With INIT_STACK_ALL_PATTERN or INIT_STACK_ALL_ZERO enabled the kernel will
> be compiled with -ftrivial-auto-var-init=<...> which causes initialization
> of stack variables at function entry time.
> 
> In order to avoid the performance impact that comes with this users can use
> the "uninitialized" attribute to prevent such initialization.
> 
> Therefore provide the __uninitialized macro which can be used for cases
> where INIT_STACK_ALL_PATTERN or INIT_STACK_ALL_ZERO is enabled, but only
> selected variables should not be initialized.
> 
> Signed-off-by: Heiko Carstens <hca@linux.ibm.com>

Thanks! We had something similar a while back with syscall entry:
efa90c11f62e ("stack: Constrain and fix stack offset randomization with Clang builds")

Acked-by: Kees Cook <keescook@chromium.org>

-- 
Kees Cook

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 2/2] s390/fpu: make use of __uninitialized macro
  2024-02-05 15:48 ` [PATCH 2/2] s390/fpu: make use of " Heiko Carstens
@ 2024-02-05 16:26   ` Kees Cook
  2024-02-05 16:35     ` Heiko Carstens
  2024-02-06  1:31   ` Nathan Chancellor
  1 sibling, 1 reply; 9+ messages in thread
From: Kees Cook @ 2024-02-05 16:26 UTC (permalink / raw)
  To: Heiko Carstens
  Cc: Nathan Chancellor, Nick Desaulniers, linux-kernel, linux-s390,
	Vasily Gorbik, Alexander Gordeev

On Mon, Feb 05, 2024 at 04:48:44PM +0100, Heiko Carstens wrote:
> Code sections in s390 specific kernel code which use floating point or
> vector registers all come with a 520 byte stack variable to save already in
> use registers, if required.
> 
> With INIT_STACK_ALL_PATTERN or INIT_STACK_ALL_ZERO enabled this variable
> will always be initialized on function entry in addition to saving register
> contents, which contradicts the intend (performance improvement) of such
> code sections.
> 
> Therefore provide a DECLARE_KERNEL_FPU_ONSTACK() macro which provides
> struct kernel_fpu variables with an __uninitialized attribute, and convert
> all existing code to use this.
> 
> This way only this specific type of stack variable will not be initialized,
> regardless of config options.
> 
> Signed-off-by: Heiko Carstens <hca@linux.ibm.com>
> ---
>  arch/s390/crypto/chacha-glue.c    | 2 +-
>  arch/s390/crypto/crc32-vx.c       | 2 +-
>  arch/s390/include/asm/fpu/types.h | 3 +++
>  arch/s390/kernel/sysinfo.c        | 2 +-
>  lib/raid6/s390vx.uc               | 4 ++--
>  5 files changed, 8 insertions(+), 5 deletions(-)
> 
> diff --git a/arch/s390/crypto/chacha-glue.c b/arch/s390/crypto/chacha-glue.c
> index ed9959e6f714..a823132fc563 100644
> --- a/arch/s390/crypto/chacha-glue.c
> +++ b/arch/s390/crypto/chacha-glue.c
> @@ -22,7 +22,7 @@ static void chacha20_crypt_s390(u32 *state, u8 *dst, const u8 *src,
>  				unsigned int nbytes, const u32 *key,
>  				u32 *counter)
>  {
> -	struct kernel_fpu vxstate;
> +	DECLARE_KERNEL_FPU_ONSTACK(vxstate);
>  
>  	kernel_fpu_begin(&vxstate, KERNEL_VXR);
>  	chacha20_vx(dst, src, nbytes, key, counter);
> diff --git a/arch/s390/crypto/crc32-vx.c b/arch/s390/crypto/crc32-vx.c
> index 017143e9cef7..6ae3e3ff5b0a 100644
> --- a/arch/s390/crypto/crc32-vx.c
> +++ b/arch/s390/crypto/crc32-vx.c
> @@ -49,8 +49,8 @@ u32 crc32c_le_vgfm_16(u32 crc, unsigned char const *buf, size_t size);
>  	static u32 __pure ___fname(u32 crc,				    \
>  				unsigned char const *data, size_t datalen)  \
>  	{								    \
> -		struct kernel_fpu vxstate;				    \
>  		unsigned long prealign, aligned, remaining;		    \
> +		DECLARE_KERNEL_FPU_ONSTACK(vxstate);			    \
>  									    \
>  		if (datalen < VX_MIN_LEN + VX_ALIGN_MASK)		    \
>  			return ___crc32_sw(crc, data, datalen);		    \
> diff --git a/arch/s390/include/asm/fpu/types.h b/arch/s390/include/asm/fpu/types.h
> index d889e9436865..b1afa13c07b7 100644
> --- a/arch/s390/include/asm/fpu/types.h
> +++ b/arch/s390/include/asm/fpu/types.h
> @@ -35,4 +35,7 @@ struct kernel_fpu {
>  	};
>  };
>  
> +#define DECLARE_KERNEL_FPU_ONSTACK(name)	\
> +	struct kernel_fpu name __uninitialized

Are there cases when struct kernel_fpu should be initialized? e.g.
should the attribute just be added to the struct definition instead of
marking each use?

-Kees

> +
>  #endif /* _ASM_S390_FPU_TYPES_H */
> diff --git a/arch/s390/kernel/sysinfo.c b/arch/s390/kernel/sysinfo.c
> index f6f8f498c9be..b439f17516eb 100644
> --- a/arch/s390/kernel/sysinfo.c
> +++ b/arch/s390/kernel/sysinfo.c
> @@ -426,9 +426,9 @@ subsys_initcall(create_proc_service_level);
>   */
>  void s390_adjust_jiffies(void)
>  {
> +	DECLARE_KERNEL_FPU_ONSTACK(fpu);
>  	struct sysinfo_1_2_2 *info;
>  	unsigned long capability;
> -	struct kernel_fpu fpu;
>  
>  	info = (void *) get_zeroed_page(GFP_KERNEL);
>  	if (!info)
> diff --git a/lib/raid6/s390vx.uc b/lib/raid6/s390vx.uc
> index cd0b9e95f499..7b0b355e1a26 100644
> --- a/lib/raid6/s390vx.uc
> +++ b/lib/raid6/s390vx.uc
> @@ -81,7 +81,7 @@ static inline void COPY_VEC(int x, int y)
>  
>  static void raid6_s390vx$#_gen_syndrome(int disks, size_t bytes, void **ptrs)
>  {
> -	struct kernel_fpu vxstate;
> +	DECLARE_KERNEL_FPU_ONSTACK(vxstate);
>  	u8 **dptr, *p, *q;
>  	int d, z, z0;
>  
> @@ -114,7 +114,7 @@ static void raid6_s390vx$#_gen_syndrome(int disks, size_t bytes, void **ptrs)
>  static void raid6_s390vx$#_xor_syndrome(int disks, int start, int stop,
>  					size_t bytes, void **ptrs)
>  {
> -	struct kernel_fpu vxstate;
> +	DECLARE_KERNEL_FPU_ONSTACK(vxstate);
>  	u8 **dptr, *p, *q;
>  	int d, z, z0;
>  
> -- 
> 2.40.1
> 

-- 
Kees Cook

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 2/2] s390/fpu: make use of __uninitialized macro
  2024-02-05 16:26   ` Kees Cook
@ 2024-02-05 16:35     ` Heiko Carstens
  2024-02-05 17:00       ` Kees Cook
  0 siblings, 1 reply; 9+ messages in thread
From: Heiko Carstens @ 2024-02-05 16:35 UTC (permalink / raw)
  To: Kees Cook
  Cc: Nathan Chancellor, Nick Desaulniers, linux-kernel, linux-s390,
	Vasily Gorbik, Alexander Gordeev

On Mon, Feb 05, 2024 at 08:26:32AM -0800, Kees Cook wrote:
> > +#define DECLARE_KERNEL_FPU_ONSTACK(name)	\
> > +	struct kernel_fpu name __uninitialized
> 
> Are there cases when struct kernel_fpu should be initialized? e.g.
> should the attribute just be added to the struct definition instead of
> marking each use?

I tried that, but failed:

./arch/s390/include/asm/fpu/types.h:36:3: warning: '__uninitialized__' attribute only applies to local variables [-Wignored-attributes]
   36 | } __uninitialized;
      |   ^
./include/linux/compiler_attributes.h:343:42: note: expanded from macro '__uninitialized'
  343 | # define __uninitialized                __attribute__((__uninitialized__))
      |                                                        ^

That's why I came up with this macro. I'd prefer to have this added only to
the struct definition, but looks like this is not possible (or I just can't
figure out who to do that).

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 2/2] s390/fpu: make use of __uninitialized macro
  2024-02-05 16:35     ` Heiko Carstens
@ 2024-02-05 17:00       ` Kees Cook
  0 siblings, 0 replies; 9+ messages in thread
From: Kees Cook @ 2024-02-05 17:00 UTC (permalink / raw)
  To: Heiko Carstens
  Cc: Nathan Chancellor, Nick Desaulniers, linux-kernel, linux-s390,
	Vasily Gorbik, Alexander Gordeev

On Mon, Feb 05, 2024 at 05:35:29PM +0100, Heiko Carstens wrote:
> On Mon, Feb 05, 2024 at 08:26:32AM -0800, Kees Cook wrote:
> > > +#define DECLARE_KERNEL_FPU_ONSTACK(name)	\
> > > +	struct kernel_fpu name __uninitialized
> > 
> > Are there cases when struct kernel_fpu should be initialized? e.g.
> > should the attribute just be added to the struct definition instead of
> > marking each use?
> 
> I tried that, but failed:
> 
> ./arch/s390/include/asm/fpu/types.h:36:3: warning: '__uninitialized__' attribute only applies to local variables [-Wignored-attributes]
>    36 | } __uninitialized;
>       |   ^

Oh. That's extremely disappointing. I think we may want to consider
opening bug reports with GCC and Clang for this. Not that it'll help us
now, since it needs to actually work today. Bummer!

In that case:

Reviewed-by: Kees Cook <keescook@chromium.org>

-- 
Kees Cook

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 1/2] Compiler Attributes: Add __uninitialized macro
  2024-02-05 15:48 ` [PATCH 1/2] Compiler Attributes: Add " Heiko Carstens
  2024-02-05 16:21   ` Kees Cook
@ 2024-02-06  1:28   ` Nathan Chancellor
  1 sibling, 0 replies; 9+ messages in thread
From: Nathan Chancellor @ 2024-02-06  1:28 UTC (permalink / raw)
  To: Heiko Carstens
  Cc: Kees Cook, Nick Desaulniers, linux-kernel, linux-s390,
	Vasily Gorbik, Alexander Gordeev

On Mon, Feb 05, 2024 at 04:48:43PM +0100, Heiko Carstens wrote:
> With INIT_STACK_ALL_PATTERN or INIT_STACK_ALL_ZERO enabled the kernel will
> be compiled with -ftrivial-auto-var-init=<...> which causes initialization
> of stack variables at function entry time.
> 
> In order to avoid the performance impact that comes with this users can use
> the "uninitialized" attribute to prevent such initialization.
> 
> Therefore provide the __uninitialized macro which can be used for cases
> where INIT_STACK_ALL_PATTERN or INIT_STACK_ALL_ZERO is enabled, but only
> selected variables should not be initialized.
> 
> Signed-off-by: Heiko Carstens <hca@linux.ibm.com>

Reviewed-by: Nathan Chancellor <nathan@kernel.org>

> ---
>  include/linux/compiler_attributes.h | 12 ++++++++++++
>  1 file changed, 12 insertions(+)
> 
> diff --git a/include/linux/compiler_attributes.h b/include/linux/compiler_attributes.h
> index 28566624f008..f5859b8c68b4 100644
> --- a/include/linux/compiler_attributes.h
> +++ b/include/linux/compiler_attributes.h
> @@ -333,6 +333,18 @@
>   */
>  #define __section(section)              __attribute__((__section__(section)))
>  
> +/*
> + * Optional: only supported since gcc >= 12
> + *
> + *   gcc: https://gcc.gnu.org/onlinedocs/gcc/Common-Variable-Attributes.html#index-uninitialized-variable-attribute
> + * clang: https://clang.llvm.org/docs/AttributeReference.html#uninitialized
> + */
> +#if __has_attribute(__uninitialized__)
> +# define __uninitialized		__attribute__((__uninitialized__))
> +#else
> +# define __uninitialized
> +#endif
> +
>  /*
>   *   gcc: https://gcc.gnu.org/onlinedocs/gcc/Common-Function-Attributes.html#index-unused-function-attribute
>   *   gcc: https://gcc.gnu.org/onlinedocs/gcc/Common-Type-Attributes.html#index-unused-type-attribute
> -- 
> 2.40.1
> 

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 2/2] s390/fpu: make use of __uninitialized macro
  2024-02-05 15:48 ` [PATCH 2/2] s390/fpu: make use of " Heiko Carstens
  2024-02-05 16:26   ` Kees Cook
@ 2024-02-06  1:31   ` Nathan Chancellor
  1 sibling, 0 replies; 9+ messages in thread
From: Nathan Chancellor @ 2024-02-06  1:31 UTC (permalink / raw)
  To: Heiko Carstens
  Cc: Kees Cook, Nick Desaulniers, linux-kernel, linux-s390,
	Vasily Gorbik, Alexander Gordeev

On Mon, Feb 05, 2024 at 04:48:44PM +0100, Heiko Carstens wrote:
> Code sections in s390 specific kernel code which use floating point or
> vector registers all come with a 520 byte stack variable to save already in
> use registers, if required.
> 
> With INIT_STACK_ALL_PATTERN or INIT_STACK_ALL_ZERO enabled this variable
> will always be initialized on function entry in addition to saving register
> contents, which contradicts the intend (performance improvement) of such
> code sections.
> 
> Therefore provide a DECLARE_KERNEL_FPU_ONSTACK() macro which provides
> struct kernel_fpu variables with an __uninitialized attribute, and convert
> all existing code to use this.
> 
> This way only this specific type of stack variable will not be initialized,
> regardless of config options.
> 
> Signed-off-by: Heiko Carstens <hca@linux.ibm.com>

Reviewed-by: Nathan Chancellor <nathan@kernel.org>

> ---
>  arch/s390/crypto/chacha-glue.c    | 2 +-
>  arch/s390/crypto/crc32-vx.c       | 2 +-
>  arch/s390/include/asm/fpu/types.h | 3 +++
>  arch/s390/kernel/sysinfo.c        | 2 +-
>  lib/raid6/s390vx.uc               | 4 ++--
>  5 files changed, 8 insertions(+), 5 deletions(-)
> 
> diff --git a/arch/s390/crypto/chacha-glue.c b/arch/s390/crypto/chacha-glue.c
> index ed9959e6f714..a823132fc563 100644
> --- a/arch/s390/crypto/chacha-glue.c
> +++ b/arch/s390/crypto/chacha-glue.c
> @@ -22,7 +22,7 @@ static void chacha20_crypt_s390(u32 *state, u8 *dst, const u8 *src,
>  				unsigned int nbytes, const u32 *key,
>  				u32 *counter)
>  {
> -	struct kernel_fpu vxstate;
> +	DECLARE_KERNEL_FPU_ONSTACK(vxstate);
>  
>  	kernel_fpu_begin(&vxstate, KERNEL_VXR);
>  	chacha20_vx(dst, src, nbytes, key, counter);
> diff --git a/arch/s390/crypto/crc32-vx.c b/arch/s390/crypto/crc32-vx.c
> index 017143e9cef7..6ae3e3ff5b0a 100644
> --- a/arch/s390/crypto/crc32-vx.c
> +++ b/arch/s390/crypto/crc32-vx.c
> @@ -49,8 +49,8 @@ u32 crc32c_le_vgfm_16(u32 crc, unsigned char const *buf, size_t size);
>  	static u32 __pure ___fname(u32 crc,				    \
>  				unsigned char const *data, size_t datalen)  \
>  	{								    \
> -		struct kernel_fpu vxstate;				    \
>  		unsigned long prealign, aligned, remaining;		    \
> +		DECLARE_KERNEL_FPU_ONSTACK(vxstate);			    \
>  									    \
>  		if (datalen < VX_MIN_LEN + VX_ALIGN_MASK)		    \
>  			return ___crc32_sw(crc, data, datalen);		    \
> diff --git a/arch/s390/include/asm/fpu/types.h b/arch/s390/include/asm/fpu/types.h
> index d889e9436865..b1afa13c07b7 100644
> --- a/arch/s390/include/asm/fpu/types.h
> +++ b/arch/s390/include/asm/fpu/types.h
> @@ -35,4 +35,7 @@ struct kernel_fpu {
>  	};
>  };
>  
> +#define DECLARE_KERNEL_FPU_ONSTACK(name)	\
> +	struct kernel_fpu name __uninitialized
> +
>  #endif /* _ASM_S390_FPU_TYPES_H */
> diff --git a/arch/s390/kernel/sysinfo.c b/arch/s390/kernel/sysinfo.c
> index f6f8f498c9be..b439f17516eb 100644
> --- a/arch/s390/kernel/sysinfo.c
> +++ b/arch/s390/kernel/sysinfo.c
> @@ -426,9 +426,9 @@ subsys_initcall(create_proc_service_level);
>   */
>  void s390_adjust_jiffies(void)
>  {
> +	DECLARE_KERNEL_FPU_ONSTACK(fpu);
>  	struct sysinfo_1_2_2 *info;
>  	unsigned long capability;
> -	struct kernel_fpu fpu;
>  
>  	info = (void *) get_zeroed_page(GFP_KERNEL);
>  	if (!info)
> diff --git a/lib/raid6/s390vx.uc b/lib/raid6/s390vx.uc
> index cd0b9e95f499..7b0b355e1a26 100644
> --- a/lib/raid6/s390vx.uc
> +++ b/lib/raid6/s390vx.uc
> @@ -81,7 +81,7 @@ static inline void COPY_VEC(int x, int y)
>  
>  static void raid6_s390vx$#_gen_syndrome(int disks, size_t bytes, void **ptrs)
>  {
> -	struct kernel_fpu vxstate;
> +	DECLARE_KERNEL_FPU_ONSTACK(vxstate);
>  	u8 **dptr, *p, *q;
>  	int d, z, z0;
>  
> @@ -114,7 +114,7 @@ static void raid6_s390vx$#_gen_syndrome(int disks, size_t bytes, void **ptrs)
>  static void raid6_s390vx$#_xor_syndrome(int disks, int start, int stop,
>  					size_t bytes, void **ptrs)
>  {
> -	struct kernel_fpu vxstate;
> +	DECLARE_KERNEL_FPU_ONSTACK(vxstate);
>  	u8 **dptr, *p, *q;
>  	int d, z, z0;
>  
> -- 
> 2.40.1
> 

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2024-02-06  1:31 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-02-05 15:48 [PATCH 0/2] Compiler Attributes, s390: Provide and use __uninitialized macro Heiko Carstens
2024-02-05 15:48 ` [PATCH 1/2] Compiler Attributes: Add " Heiko Carstens
2024-02-05 16:21   ` Kees Cook
2024-02-06  1:28   ` Nathan Chancellor
2024-02-05 15:48 ` [PATCH 2/2] s390/fpu: make use of " Heiko Carstens
2024-02-05 16:26   ` Kees Cook
2024-02-05 16:35     ` Heiko Carstens
2024-02-05 17:00       ` Kees Cook
2024-02-06  1:31   ` Nathan Chancellor

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).