diff mbox series

[6/6] arm64: Hoist CONFIG option out of ALTERNATIVE in uaccess.h

Message ID 20200311180416.6509-7-richard.henderson@linaro.org (mailing list archive)
State New, archived
Headers show
Series arm64: gcc asm flag outputs | expand

Commit Message

Richard Henderson March 11, 2020, 6:04 p.m. UTC 
From: Richard Henderson <rth@twiddle.net>

The placement of the CONFIG check, within the asm, is less than
ideal within uaccess.h.  When we have

	if (cond)
		asm("something")

and "something" turns out to be empty, the if cannot be removed
by the compiler.

Signed-off-by: Richard Henderson <rth@twiddle.net>
---
 arch/arm64/include/asm/uaccess.h | 31 ++++++++++++++++---------------
 1 file changed, 16 insertions(+), 15 deletions(-)

Comments

Mark Rutland March 13, 2020, 10:46 a.m. UTC | #1 
Hi Richard,

On Wed, Mar 11, 2020 at 11:04:16AM -0700, Richard Henderson wrote:
> From: Richard Henderson <rth@twiddle.net>
> 
> The placement of the CONFIG check, within the asm, is less than
> ideal within uaccess.h.  When we have
> 
> 	if (cond)
> 		asm("something")
> 
> and "something" turns out to be empty, the if cannot be removed
> by the compiler.

Given the config argument to ALTERNATIVE() is unfortunate for codegen,
and IMO hinder clarity, I think it'd be worth removing that completely.

Regardless of that, and regardless of the rest of the series, this patch
looks good to me, so FWIW:

Acked-by: Mark Rutland <mark.rutland@arm.com>

Mark.

> 
> Signed-off-by: Richard Henderson <rth@twiddle.net>
> ---
>  arch/arm64/include/asm/uaccess.h | 31 ++++++++++++++++---------------
>  1 file changed, 16 insertions(+), 15 deletions(-)
> 
> diff --git a/arch/arm64/include/asm/uaccess.h b/arch/arm64/include/asm/uaccess.h
> index ca1acd7b95c3..90be003101f4 100644
> --- a/arch/arm64/include/asm/uaccess.h
> +++ b/arch/arm64/include/asm/uaccess.h
> @@ -43,11 +43,14 @@ static inline void set_fs(mm_segment_t fs)
>  	 * Enable/disable UAO so that copy_to_user() etc can access
>  	 * kernel memory with the unprivileged instructions.
>  	 */
> -	if (IS_ENABLED(CONFIG_ARM64_UAO) && fs == KERNEL_DS)
> -		asm(ALTERNATIVE("nop", SET_PSTATE_UAO(1), ARM64_HAS_UAO));
> -	else
> -		asm(ALTERNATIVE("nop", SET_PSTATE_UAO(0), ARM64_HAS_UAO,
> -				CONFIG_ARM64_UAO));
> +	if (IS_ENABLED(CONFIG_ARM64_UAO)) {
> +		if (fs == KERNEL_DS)
> +			asm(ALTERNATIVE("nop", SET_PSTATE_UAO(1),
> +					ARM64_HAS_UAO));
> +		else
> +			asm(ALTERNATIVE("nop", SET_PSTATE_UAO(0),
> +					ARM64_HAS_UAO));
> +	}
>  }
>  
>  #define segment_eq(a, b)	((a) == (b))
> @@ -178,28 +181,26 @@ static inline bool uaccess_ttbr0_enable(void)
>  
>  static inline void __uaccess_disable_hw_pan(void)
>  {
> -	asm(ALTERNATIVE("nop", SET_PSTATE_PAN(0), ARM64_HAS_PAN,
> -			CONFIG_ARM64_PAN));
> +	if (IS_ENABLED(CONFIG_ARM64_PAN))
> +		asm(ALTERNATIVE("nop", SET_PSTATE_PAN(0), ARM64_HAS_PAN));
>  }
>  
>  static inline void __uaccess_enable_hw_pan(void)
>  {
> -	asm(ALTERNATIVE("nop", SET_PSTATE_PAN(1), ARM64_HAS_PAN,
> -			CONFIG_ARM64_PAN));
> +	if (IS_ENABLED(CONFIG_ARM64_PAN))
> +		asm(ALTERNATIVE("nop", SET_PSTATE_PAN(1), ARM64_HAS_PAN));
>  }
>  
>  #define __uaccess_disable(alt)						\
>  do {									\
> -	if (!uaccess_ttbr0_disable())					\
> -		asm(ALTERNATIVE("nop", SET_PSTATE_PAN(1), alt,		\
> -				CONFIG_ARM64_PAN));			\
> +	if (IS_ENABLED(CONFIG_ARM64_PAN) && !uaccess_ttbr0_disable())	\
> +		asm(ALTERNATIVE("nop", SET_PSTATE_PAN(1), alt));	\
>  } while (0)
>  
>  #define __uaccess_enable(alt)						\
>  do {									\
> -	if (!uaccess_ttbr0_enable())					\
> -		asm(ALTERNATIVE("nop", SET_PSTATE_PAN(0), alt,		\
> -				CONFIG_ARM64_PAN));			\
> +	if (IS_ENABLED(CONFIG_ARM64_PAN) && !uaccess_ttbr0_enable())	\
> +		asm(ALTERNATIVE("nop", SET_PSTATE_PAN(0), alt));	\
>  } while (0)
>  
>  static inline void uaccess_disable(void)
> -- 
> 2.20.1
> 
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
diff mbox series

Patch

diff --git a/arch/arm64/include/asm/uaccess.h b/arch/arm64/include/asm/uaccess.h
index ca1acd7b95c3..90be003101f4 100644
--- a/arch/arm64/include/asm/uaccess.h
+++ b/arch/arm64/include/asm/uaccess.h
@@ -43,11 +43,14 @@  static inline void set_fs(mm_segment_t fs)
 	 * Enable/disable UAO so that copy_to_user() etc can access
 	 * kernel memory with the unprivileged instructions.
 	 */
-	if (IS_ENABLED(CONFIG_ARM64_UAO) && fs == KERNEL_DS)
-		asm(ALTERNATIVE("nop", SET_PSTATE_UAO(1), ARM64_HAS_UAO));
-	else
-		asm(ALTERNATIVE("nop", SET_PSTATE_UAO(0), ARM64_HAS_UAO,
-				CONFIG_ARM64_UAO));
+	if (IS_ENABLED(CONFIG_ARM64_UAO)) {
+		if (fs == KERNEL_DS)
+			asm(ALTERNATIVE("nop", SET_PSTATE_UAO(1),
+					ARM64_HAS_UAO));
+		else
+			asm(ALTERNATIVE("nop", SET_PSTATE_UAO(0),
+					ARM64_HAS_UAO));
+	}
 }
 
 #define segment_eq(a, b)	((a) == (b))
@@ -178,28 +181,26 @@  static inline bool uaccess_ttbr0_enable(void)
 
 static inline void __uaccess_disable_hw_pan(void)
 {
-	asm(ALTERNATIVE("nop", SET_PSTATE_PAN(0), ARM64_HAS_PAN,
-			CONFIG_ARM64_PAN));
+	if (IS_ENABLED(CONFIG_ARM64_PAN))
+		asm(ALTERNATIVE("nop", SET_PSTATE_PAN(0), ARM64_HAS_PAN));
 }
 
 static inline void __uaccess_enable_hw_pan(void)
 {
-	asm(ALTERNATIVE("nop", SET_PSTATE_PAN(1), ARM64_HAS_PAN,
-			CONFIG_ARM64_PAN));
+	if (IS_ENABLED(CONFIG_ARM64_PAN))
+		asm(ALTERNATIVE("nop", SET_PSTATE_PAN(1), ARM64_HAS_PAN));
 }
 
 #define __uaccess_disable(alt)						\
 do {									\
-	if (!uaccess_ttbr0_disable())					\
-		asm(ALTERNATIVE("nop", SET_PSTATE_PAN(1), alt,		\
-				CONFIG_ARM64_PAN));			\
+	if (IS_ENABLED(CONFIG_ARM64_PAN) && !uaccess_ttbr0_disable())	\
+		asm(ALTERNATIVE("nop", SET_PSTATE_PAN(1), alt));	\
 } while (0)
 
 #define __uaccess_enable(alt)						\
 do {									\
-	if (!uaccess_ttbr0_enable())					\
-		asm(ALTERNATIVE("nop", SET_PSTATE_PAN(0), alt,		\
-				CONFIG_ARM64_PAN));			\
+	if (IS_ENABLED(CONFIG_ARM64_PAN) && !uaccess_ttbr0_enable())	\
+		asm(ALTERNATIVE("nop", SET_PSTATE_PAN(0), alt));	\
 } while (0)
 
 static inline void uaccess_disable(void)