All of lore.kernel.org
 help / color / mirror / Atom feed
From: Alexandre Chartre <alexandre.chartre@oracle.com>
To: Thomas Gleixner <tglx@linutronix.de>,
	LKML <linux-kernel@vger.kernel.org>
Cc: x86@kernel.org, Steven Rostedt <rostedt@goodmis.org>,
	Brian Gerst <brgerst@gmail.com>, Juergen Gross <jgross@suse.com>,
	Paolo Bonzini <pbonzini@redhat.com>,
	Arnd Bergmann <arnd@arndb.de>
Subject: Re: [patch 08/24] x86/entry: Convert Divide Error to IDTENTRY
Date: Fri, 28 Feb 2020 15:58:03 +0100	[thread overview]
Message-ID: <4afd832a-2a78-798a-97e0-d1e3636cc290@oracle.com> (raw)
In-Reply-To: <20200225222648.970676604@linutronix.de>


On 2/25/20 11:16 PM, Thomas Gleixner wrote:
> Convert #DE to IDTENTRY:
>    - Implement the C entry point with DEFINE_IDTENTRY
>    - Emit the ASM stub with DECLARE_IDTENTRY
>    - Remove the ASM idtentry in 64bit
>    - Remove the open coded ASM entry code in 32bit
>    - Fixup the XEN/PV code
> 
> No functional change.
> 
> Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
> ---
>   arch/x86/entry/entry_32.S       |    7 -------
>   arch/x86/entry/entry_64.S       |    1 -
>   arch/x86/include/asm/idtentry.h |    3 +++
>   arch/x86/include/asm/traps.h    |    3 ---
>   arch/x86/kernel/idt.c           |    2 +-
>   arch/x86/kernel/traps.c         |    7 ++++++-
>   arch/x86/xen/enlighten_pv.c     |    7 ++++++-
>   arch/x86/xen/xen-asm_64.S       |    2 +-
>   8 files changed, 17 insertions(+), 15 deletions(-)
> 
> --- a/arch/x86/entry/entry_32.S
> +++ b/arch/x86/entry/entry_32.S
> @@ -1386,13 +1386,6 @@ SYM_CODE_START(alignment_check)
>   	jmp	common_exception
>   SYM_CODE_END(alignment_check)
>   
> -SYM_CODE_START(divide_error)
> -	ASM_CLAC
> -	pushl	$0				# no error code
> -	pushl	$do_divide_error
> -	jmp	common_exception
> -SYM_CODE_END(divide_error)
> -
>   #ifdef CONFIG_X86_MCE
>   SYM_CODE_START(machine_check)
>   	ASM_CLAC
> --- a/arch/x86/entry/entry_64.S
> +++ b/arch/x86/entry/entry_64.S
> @@ -1061,7 +1061,6 @@ apicinterrupt IRQ_WORK_VECTOR			irq_work
>    * Exception entry points.
>    */
>   
> -idtentry	X86_TRAP_DE		divide_error		do_divide_error			has_error_code=0
>   idtentry	X86_TRAP_OF		overflow		do_overflow			has_error_code=0
>   idtentry	X86_TRAP_BP		int3			do_int3				has_error_code=0
>   idtentry	X86_TRAP_BR		bounds			do_bounds			has_error_code=0
> --- a/arch/x86/include/asm/idtentry.h
> +++ b/arch/x86/include/asm/idtentry.h
> @@ -76,4 +76,7 @@ static __always_inline void __##func(str
>   
>   #endif /* __ASSEMBLY__ */
>   
> +/* Simple exception entries: */
> +DECLARE_IDTENTRY(X86_TRAP_DE,		exc_divide_error);
> +

I think this macro has a tricky behavior because it will declare C functions
when included in a C file, and define assembly idtentry when included in an
assembly file.

I see your point which is to have a single statement which provides both C
and assembly functions, but this dual behavior is not obvious when reading
the code. Maybe add a comment to explain this behavior?

Reviewed-by: Alexandre Chartre <alexandre.chartre@oracle.com>

alex.


>   #endif
> --- a/arch/x86/include/asm/traps.h
> +++ b/arch/x86/include/asm/traps.h
> @@ -11,7 +11,6 @@
>   
>   #define dotraplinkage __visible
>   
> -asmlinkage void divide_error(void);
>   asmlinkage void debug(void);
>   asmlinkage void nmi(void);
>   asmlinkage void int3(void);
> @@ -38,7 +37,6 @@ asmlinkage void machine_check(void);
>   asmlinkage void simd_coprocessor_error(void);
>   
>   #if defined(CONFIG_X86_64) && defined(CONFIG_XEN_PV)
> -asmlinkage void xen_divide_error(void);
>   asmlinkage void xen_xennmi(void);
>   asmlinkage void xen_xendebug(void);
>   asmlinkage void xen_int3(void);
> @@ -62,7 +60,6 @@ asmlinkage void xen_machine_check(void);
>   asmlinkage void xen_simd_coprocessor_error(void);
>   #endif
>   
> -dotraplinkage void do_divide_error(struct pt_regs *regs, long error_code);
>   dotraplinkage void do_debug(struct pt_regs *regs, long error_code);
>   dotraplinkage void do_nmi(struct pt_regs *regs, long error_code);
>   dotraplinkage void do_int3(struct pt_regs *regs, long error_code);
> --- a/arch/x86/kernel/idt.c
> +++ b/arch/x86/kernel/idt.c
> @@ -70,7 +70,7 @@ static const __initconst struct idt_data
>    * set up TSS.
>    */
>   static const __initconst struct idt_data def_idts[] = {
> -	INTG(X86_TRAP_DE,		divide_error),
> +	INTG(X86_TRAP_DE,		asm_exc_divide_error),
>   	INTG(X86_TRAP_NMI,		nmi),
>   	INTG(X86_TRAP_BR,		bounds),
>   	INTG(X86_TRAP_UD,		invalid_op),
> --- a/arch/x86/kernel/traps.c
> +++ b/arch/x86/kernel/traps.c
> @@ -279,6 +279,12 @@ static inline void __user *error_get_tra
>   	return (void __user *)uprobe_get_trap_addr(regs);
>   }
>   
> +DEFINE_IDTENTRY(exc_divide_error)
> +{
> +	do_error_trap(regs, 0, "divide_error", X86_TRAP_DE, SIGFPE,
> +		      FPE_INTDIV, error_get_trap_addr(regs));
> +}
> +
>   #define IP ((void __user *)uprobe_get_trap_addr(regs))
>   #define DO_ERROR(trapnr, signr, sicode, addr, str, name)		   \
>   dotraplinkage void do_##name(struct pt_regs *regs, long error_code)	   \
> @@ -286,7 +292,6 @@ dotraplinkage void do_##name(struct pt_r
>   	do_error_trap(regs, error_code, str, trapnr, signr, sicode, addr); \
>   }
>   
> -DO_ERROR(X86_TRAP_DE,     SIGFPE,  FPE_INTDIV,   IP, "divide error",        divide_error)
>   DO_ERROR(X86_TRAP_OF,     SIGSEGV,          0, NULL, "overflow",            overflow)
>   DO_ERROR(X86_TRAP_UD,     SIGILL,  ILL_ILLOPN,   IP, "invalid opcode",      invalid_op)
>   DO_ERROR(X86_TRAP_OLD_MF, SIGFPE,           0, NULL, "coprocessor segment overrun", coprocessor_segment_overrun)
> --- a/arch/x86/xen/enlighten_pv.c
> +++ b/arch/x86/xen/enlighten_pv.c
> @@ -602,6 +602,11 @@ struct trap_array_entry {
>   	bool ist_okay;
>   };
>   
> +#define TRAP_ENTRY(func, ist_ok) {			\
> +	.orig		= asm_##func,			\
> +	.xen		= xen_asm_##func,		\
> +	.ist_okay	= ist_ok }
> +
>   static struct trap_array_entry trap_array[] = {
>   	{ debug,                       xen_xendebug,                    true },
>   	{ double_fault,                xen_double_fault,                true },
> @@ -615,7 +620,7 @@ static struct trap_array_entry trap_arra
>   	{ entry_INT80_compat,          xen_entry_INT80_compat,          false },
>   #endif
>   	{ page_fault,                  xen_page_fault,                  false },
> -	{ divide_error,                xen_divide_error,                false },
> +	TRAP_ENTRY(exc_divide_error,			false ),
>   	{ bounds,                      xen_bounds,                      false },
>   	{ invalid_op,                  xen_invalid_op,                  false },
>   	{ device_not_available,        xen_device_not_available,        false },
> --- a/arch/x86/xen/xen-asm_64.S
> +++ b/arch/x86/xen/xen-asm_64.S
> @@ -28,7 +28,7 @@ SYM_CODE_END(xen_\name)
>   _ASM_NOKPROBE(xen_\name)
>   .endm
>   
> -xen_pv_trap divide_error
> +xen_pv_trap asm_exc_divide_error
>   xen_pv_trap debug
>   xen_pv_trap xendebug
>   xen_pv_trap int3
> 
> 

  reply	other threads:[~2020-02-28 14:58 UTC|newest]

Thread overview: 40+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-02-25 22:16 [patch 00/24] x86/entry: Consolidation - Part III Thomas Gleixner
2020-02-25 22:16 ` [patch 01/24] x86/traps: Split trap numbers out in a seperate header Thomas Gleixner
2020-02-26  5:52   ` Andy Lutomirski
2020-02-28  9:39   ` Alexandre Chartre
2020-02-25 22:16 ` [patch 02/24] x86/entry/64: Avoid pointless code when CONTEXT_TRACKING=n Thomas Gleixner
2020-02-26  5:52   ` Andy Lutomirski
2020-02-28  9:40   ` Alexandre Chartre
2020-03-02 22:25   ` Frederic Weisbecker
2020-02-25 22:16 ` [patch 03/24] x86/entry/64: Reorder idtentries Thomas Gleixner
2020-02-26  5:53   ` Andy Lutomirski
2020-02-28  9:40   ` Alexandre Chartre
2020-02-25 22:16 ` [patch 04/24] x86/entry: Distangle idtentry Thomas Gleixner
2020-02-28 10:27   ` Alexandre Chartre
2020-02-25 22:16 ` [patch 05/24] x86/entry/32: Provide macro to emit IDT entry stubs Thomas Gleixner
2020-02-28 10:33   ` Alexandre Chartre
2020-02-25 22:16 ` [patch 06/24] x86/idtentry: Provide macros to define/declare IDT entry points Thomas Gleixner
2020-02-28 10:42   ` Alexandre Chartre
2020-03-04 12:46     ` Thomas Gleixner
2020-02-25 22:16 ` [patch 07/24] x86/traps: Prepare for using DEFINE_IDTENTRY Thomas Gleixner
2020-02-28 14:13   ` Alexandre Chartre
2020-02-28 14:18     ` Thomas Gleixner
2020-02-25 22:16 ` [patch 08/24] x86/entry: Convert Divide Error to IDTENTRY Thomas Gleixner
2020-02-28 14:58   ` Alexandre Chartre [this message]
2020-03-04 13:09     ` Thomas Gleixner
2020-02-25 22:16 ` [patch 09/24] x86/entry: Convert Overflow exception " Thomas Gleixner
2020-02-25 22:16 ` [patch 10/24] x86/entry: Convert INT3 " Thomas Gleixner
2020-02-25 22:16 ` [patch 11/24] x86/entry: Convert Bounds " Thomas Gleixner
2020-02-25 22:16 ` [patch 12/24] x86/entry: Convert Invalid Opcode " Thomas Gleixner
2020-02-25 22:16 ` [patch 13/24] x86/entry: Convert Device not available " Thomas Gleixner
2020-02-25 22:16 ` [patch 14/24] x86/entry: Convert Coprocessor segment overrun " Thomas Gleixner
2020-02-25 22:16 ` [patch 15/24] x86/entry: Provide IDTENTRY_ERRORCODE Thomas Gleixner
2020-02-25 22:16 ` [patch 16/24] x86/entry: Convert Invalid TSS exception to IDTENTRY Thomas Gleixner
2020-02-25 22:16 ` [patch 17/24] x86/entry: Convert Segment not present " Thomas Gleixner
2020-02-25 22:16 ` [patch 18/24] x86/entry: Convert Stack segment " Thomas Gleixner
2020-02-25 22:16 ` [patch 19/24] x86/entry: Convert General protection " Thomas Gleixner
2020-02-25 22:16 ` [patch 20/24] x86/entry: Convert Spurious interrupt bug " Thomas Gleixner
2020-02-25 22:16 ` [patch 21/24] x86/entry: Convert Coprocessor error " Thomas Gleixner
2020-02-25 22:16 ` [patch 22/24] x86/entry: Convert Alignment check " Thomas Gleixner
2020-02-25 22:16 ` [patch 23/24] x86/entry: Convert SIMD coprocessor error " Thomas Gleixner
2020-02-25 22:16 ` [patch 24/24] x86/entry/32: Convert IRET exception to IDTENTRY_SW Thomas Gleixner

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=4afd832a-2a78-798a-97e0-d1e3636cc290@oracle.com \
    --to=alexandre.chartre@oracle.com \
    --cc=arnd@arndb.de \
    --cc=brgerst@gmail.com \
    --cc=jgross@suse.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=pbonzini@redhat.com \
    --cc=rostedt@goodmis.org \
    --cc=tglx@linutronix.de \
    --cc=x86@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.