[PATCH v2 2/5] x86/traps: Consolidate user fixups in the #GP handler

Sohil Mehta posted 5 patches 1 month ago
There is a newer version of this series
[PATCH v2 2/5] x86/traps: Consolidate user fixups in the #GP handler
Posted by Sohil Mehta 1 month ago
Move the UMIP exception fixup under the common "if (user_mode(regs))"
condition where the rest of user mode fixups reside. Also, move the UMIP
feature check into its fixup function to keep the calling code
consistent and clean.

No functional change intended.

Suggested-by: Dave Hansen <dave.hansen@intel.com>
Signed-off-by: Sohil Mehta <sohil.mehta@intel.com>
Acked-by: Dave Hansen <dave.hansen@linux.intel.com>
---
v2:
 - No change
---
 arch/x86/kernel/traps.c | 8 +++-----
 arch/x86/kernel/umip.c  | 3 +++
 2 files changed, 6 insertions(+), 5 deletions(-)

diff --git a/arch/x86/kernel/traps.c b/arch/x86/kernel/traps.c
index 4dbff8ef9b1c..614a281bd419 100644
--- a/arch/x86/kernel/traps.c
+++ b/arch/x86/kernel/traps.c
@@ -921,11 +921,6 @@ DEFINE_IDTENTRY_ERRORCODE(exc_general_protection)
 
 	cond_local_irq_enable(regs);
 
-	if (static_cpu_has(X86_FEATURE_UMIP)) {
-		if (user_mode(regs) && fixup_umip_exception(regs))
-			goto exit;
-	}
-
 	if (v8086_mode(regs)) {
 		local_irq_enable();
 		handle_vm86_fault((struct kernel_vm86_regs *) regs, error_code);
@@ -940,6 +935,9 @@ DEFINE_IDTENTRY_ERRORCODE(exc_general_protection)
 		if (fixup_vdso_exception(regs, X86_TRAP_GP, error_code, 0))
 			goto exit;
 
+		if (fixup_umip_exception(regs))
+			goto exit;
+
 		gp_user_force_sig_segv(regs, X86_TRAP_GP, error_code, desc);
 		goto exit;
 	}
diff --git a/arch/x86/kernel/umip.c b/arch/x86/kernel/umip.c
index d432f3824f0c..3ce99cbcf187 100644
--- a/arch/x86/kernel/umip.c
+++ b/arch/x86/kernel/umip.c
@@ -354,6 +354,9 @@ bool fixup_umip_exception(struct pt_regs *regs)
 	void __user *uaddr;
 	struct insn insn;
 
+	if (!cpu_feature_enabled(X86_FEATURE_UMIP))
+		return false;
+
 	if (!regs)
 		return false;
 
-- 
2.43.0
Re: [PATCH v2 2/5] x86/traps: Consolidate user fixups in the #GP handler
Posted by H. Peter Anvin 1 month ago
On 2026-03-05 13:40, Sohil Mehta wrote:
> Move the UMIP exception fixup under the common "if (user_mode(regs))"
> condition where the rest of user mode fixups reside. Also, move the UMIP
> feature check into its fixup function to keep the calling code
> consistent and clean.
> 
> No functional change intended.
> 
> Suggested-by: Dave Hansen <dave.hansen@intel.com>
> Signed-off-by: Sohil Mehta <sohil.mehta@intel.com>
> Acked-by: Dave Hansen <dave.hansen@linux.intel.com>
> ---
> v2:
>  - No change
> ---
>  arch/x86/kernel/traps.c | 8 +++-----
>  arch/x86/kernel/umip.c  | 3 +++
>  2 files changed, 6 insertions(+), 5 deletions(-)
> 
> diff --git a/arch/x86/kernel/traps.c b/arch/x86/kernel/traps.c
> index 4dbff8ef9b1c..614a281bd419 100644
> --- a/arch/x86/kernel/traps.c
> +++ b/arch/x86/kernel/traps.c
> @@ -921,11 +921,6 @@ DEFINE_IDTENTRY_ERRORCODE(exc_general_protection)
>  
>  	cond_local_irq_enable(regs);
>  
> -	if (static_cpu_has(X86_FEATURE_UMIP)) {
> -		if (user_mode(regs) && fixup_umip_exception(regs))
> -			goto exit;
> -	}
> -
>  	if (v8086_mode(regs)) {
>  		local_irq_enable();
>  		handle_vm86_fault((struct kernel_vm86_regs *) regs, error_code);
> @@ -940,6 +935,9 @@ DEFINE_IDTENTRY_ERRORCODE(exc_general_protection)
>  		if (fixup_vdso_exception(regs, X86_TRAP_GP, error_code, 0))
>  			goto exit;
>  
> +		if (fixup_umip_exception(regs))
> +			goto exit;
> +
>  		gp_user_force_sig_segv(regs, X86_TRAP_GP, error_code, desc);
>  		goto exit;
>  	}
> diff --git a/arch/x86/kernel/umip.c b/arch/x86/kernel/umip.c
> index d432f3824f0c..3ce99cbcf187 100644
> --- a/arch/x86/kernel/umip.c
> +++ b/arch/x86/kernel/umip.c
> @@ -354,6 +354,9 @@ bool fixup_umip_exception(struct pt_regs *regs)
>  	void __user *uaddr;
>  	struct insn insn;
>  
> +	if (!cpu_feature_enabled(X86_FEATURE_UMIP))
> +		return false;
> +
>  	if (!regs)
>  		return false;
>  

[General comment, not really applicable to this patch]

I like this kind cleanups. However, if this had been a hot path (which it
isn't) and if UMIP wasn't very common (which it is), then it probably would be
desirable to push the cpu_feature_enabled() into the call site. This is
trivially done with an inline function in the header file where this is exported:

static inline bool fixup_umip_exception(struct pt_regs *regs)
{
	return cpu_feature_enabled(X86_FEATURE_UMIP) &&
		__fixup_umip_exception(regs);
}
Re: [PATCH v2 2/5] x86/traps: Consolidate user fixups in the #GP handler
Posted by H. Peter Anvin 1 month ago
On 2026-03-05 13:40, Sohil Mehta wrote:
> Move the UMIP exception fixup under the common "if (user_mode(regs))"
> condition where the rest of user mode fixups reside. Also, move the UMIP
> feature check into its fixup function to keep the calling code
> consistent and clean.
> 
> No functional change intended.
> 
> Suggested-by: Dave Hansen <dave.hansen@intel.com>
> Signed-off-by: Sohil Mehta <sohil.mehta@intel.com>
> Acked-by: Dave Hansen <dave.hansen@linux.intel.com>
> ---
> v2:
>  - No change
> ---
>  arch/x86/kernel/traps.c | 8 +++-----
>  arch/x86/kernel/umip.c  | 3 +++
>  2 files changed, 6 insertions(+), 5 deletions(-)
> 
> diff --git a/arch/x86/kernel/traps.c b/arch/x86/kernel/traps.c
> index 4dbff8ef9b1c..614a281bd419 100644
> --- a/arch/x86/kernel/traps.c
> +++ b/arch/x86/kernel/traps.c
> @@ -921,11 +921,6 @@ DEFINE_IDTENTRY_ERRORCODE(exc_general_protection)
>  
>  	cond_local_irq_enable(regs);
>  
> -	if (static_cpu_has(X86_FEATURE_UMIP)) {
> -		if (user_mode(regs) && fixup_umip_exception(regs))
> -			goto exit;
> -	}
> -
>  	if (v8086_mode(regs)) {
>  		local_irq_enable();
>  		handle_vm86_fault((struct kernel_vm86_regs *) regs, error_code);
> @@ -940,6 +935,9 @@ DEFINE_IDTENTRY_ERRORCODE(exc_general_protection)
>  		if (fixup_vdso_exception(regs, X86_TRAP_GP, error_code, 0))
>  			goto exit;
>  
> +		if (fixup_umip_exception(regs))
> +			goto exit;
> +
>  		gp_user_force_sig_segv(regs, X86_TRAP_GP, error_code, desc);
>  		goto exit;
>  	}
> diff --git a/arch/x86/kernel/umip.c b/arch/x86/kernel/umip.c
> index d432f3824f0c..3ce99cbcf187 100644
> --- a/arch/x86/kernel/umip.c
> +++ b/arch/x86/kernel/umip.c
> @@ -354,6 +354,9 @@ bool fixup_umip_exception(struct pt_regs *regs)
>  	void __user *uaddr;
>  	struct insn insn;
>  
> +	if (!cpu_feature_enabled(X86_FEATURE_UMIP))
> +		return false;
> +
>  	if (!regs)
>  		return false;
>  

Reviewed-by: H. Peter Anvin (Intel) <hpa@zytor.com>
[tip: x86/cpu] x86/traps: Consolidate user fixups in the #GP handler
Posted by tip-bot2 for Sohil Mehta 2 weeks, 6 days ago
The following commit has been merged into the x86/cpu branch of tip:

Commit-ID:     97b8c8927ee107c5a1bfe990106209beb054d3bf
Gitweb:        https://git.kernel.org/tip/97b8c8927ee107c5a1bfe990106209beb054d3bf
Author:        Sohil Mehta <sohil.mehta@intel.com>
AuthorDate:    Thu, 05 Mar 2026 13:40:23 -08:00
Committer:     Dave Hansen <dave.hansen@linux.intel.com>
CommitterDate: Thu, 05 Mar 2026 13:49:26 -08:00

x86/traps: Consolidate user fixups in the #GP handler

Move the UMIP exception fixup under the common "if (user_mode(regs))"
condition where the rest of user mode fixups reside. Also, move the UMIP
feature check into its fixup function to keep the calling code
consistent and clean.

No functional change intended.

Suggested-by: Dave Hansen <dave.hansen@intel.com>
Signed-off-by: Sohil Mehta <sohil.mehta@intel.com>
Signed-off-by: Dave Hansen <dave.hansen@linux.intel.com>
Acked-by: Dave Hansen <dave.hansen@linux.intel.com>
Link: https://patch.msgid.link/20260305214026.3887452-3-sohil.mehta@intel.com
---
 arch/x86/kernel/traps.c | 8 +++-----
 arch/x86/kernel/umip.c  | 3 +++
 2 files changed, 6 insertions(+), 5 deletions(-)

diff --git a/arch/x86/kernel/traps.c b/arch/x86/kernel/traps.c
index 4dbff8e..614a281 100644
--- a/arch/x86/kernel/traps.c
+++ b/arch/x86/kernel/traps.c
@@ -921,11 +921,6 @@ DEFINE_IDTENTRY_ERRORCODE(exc_general_protection)
 
 	cond_local_irq_enable(regs);
 
-	if (static_cpu_has(X86_FEATURE_UMIP)) {
-		if (user_mode(regs) && fixup_umip_exception(regs))
-			goto exit;
-	}
-
 	if (v8086_mode(regs)) {
 		local_irq_enable();
 		handle_vm86_fault((struct kernel_vm86_regs *) regs, error_code);
@@ -940,6 +935,9 @@ DEFINE_IDTENTRY_ERRORCODE(exc_general_protection)
 		if (fixup_vdso_exception(regs, X86_TRAP_GP, error_code, 0))
 			goto exit;
 
+		if (fixup_umip_exception(regs))
+			goto exit;
+
 		gp_user_force_sig_segv(regs, X86_TRAP_GP, error_code, desc);
 		goto exit;
 	}
diff --git a/arch/x86/kernel/umip.c b/arch/x86/kernel/umip.c
index d432f38..3ce99cb 100644
--- a/arch/x86/kernel/umip.c
+++ b/arch/x86/kernel/umip.c
@@ -354,6 +354,9 @@ bool fixup_umip_exception(struct pt_regs *regs)
 	void __user *uaddr;
 	struct insn insn;
 
+	if (!cpu_feature_enabled(X86_FEATURE_UMIP))
+		return false;
+
 	if (!regs)
 		return false;