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

[H. Peter Anvin]

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@xxxxxxxxx>
> Signed-off-by: Sohil Mehta <sohil.mehta@xxxxxxxxx>
> Acked-by: Dave Hansen <dave.hansen@xxxxxxxxxxxxxxx>
> ---
> 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);
}