Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1289852 > unrolled thread

[PATCHV2 1/3] x86, ras: Add new infrastructure for machine check fixup tables

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  [PATCHV2 1/3] x86, ras: Add new infrastructure for machine check fixup tables Tony Luck <tony.luck@intel.com> - 2015-12-11 20:40 +0100
    Re: [PATCHV2 1/3] x86, ras: Add new infrastructure for machine check  fixup tables Andy Lutomirski <luto@amacapital.net> - 2015-12-11 21:10 +0100
      RE: [PATCHV2 1/3] x86, ras: Add new infrastructure for machine  check fixup tables "Luck, Tony" <tony.luck@intel.com> - 2015-12-11 22:10 +0100
    Re: [PATCHV2 1/3] x86, ras: Add new infrastructure for machine check  fixup tables Borislav Petkov <bp@alien8.de> - 2015-12-12 11:20 +0100
      Re: [PATCHV2 1/3] x86, ras: Add new infrastructure for machine check  fixup tables Ross Zwisler <zwisler@gmail.com> - 2015-12-14 19:00 +0100
        Re: [PATCHV2 1/3] x86, ras: Add new infrastructure for machine check  fixup tables Borislav Petkov <bp@alien8.de> - 2015-12-14 23:30 +0100
      Re: [PATCHV2 1/3] x86, ras: Add new infrastructure for machine check  fixup tables "Luck, Tony" <tony.luck@intel.com> - 2015-12-15 02:10 +0100
        Re: [PATCHV2 1/3] x86, ras: Add new infrastructure for machine check  fixup tables Borislav Petkov <bp@alien8.de> - 2015-12-15 10:50 +0100
          Re: [PATCHV2 1/3] x86, ras: Add new infrastructure for machine check  fixup tables Borislav Petkov <bp@alien8.de> - 2015-12-15 11:50 +0100

#1289852 — [PATCHV2 1/3] x86, ras: Add new infrastructure for machine check fixup tables

Copy the existing page fault fixup mechanisms to create a new table
to be used when fixing machine checks. Note:
1) At this time we only provide a macro to annotate assembly code
2) We assume all fixups will in code builtin to the kernel.
3) Only for x86_64
4) New code under CONFIG_MCE_KERNEL_RECOVERY

Signed-off-by: Tony Luck <tony.luck@intel.com>
---
 arch/x86/Kconfig                  |  4 ++++
 arch/x86/include/asm/asm.h        | 10 ++++++++--
 arch/x86/include/asm/uaccess.h    |  8 ++++++++
 arch/x86/mm/extable.c             | 19 +++++++++++++++++++
 include/asm-generic/vmlinux.lds.h |  6 ++++++
 include/linux/module.h            |  1 +
 kernel/extable.c                  | 20 ++++++++++++++++++++
 7 files changed, 66 insertions(+), 2 deletions(-)

diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
index 96d058a87100..db5c6e1d6e37 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -1001,6 +1001,10 @@ config X86_MCE_INJECT
 	  If you don't know what a machine check is and you don't do kernel
 	  QA it is safe to say n.
 
+config MCE_KERNEL_RECOVERY
+	depends on X86_MCE && X86_64
+	def_bool y
+
 config X86_THERMAL_VECTOR
 	def_bool y
 	depends on X86_MCE_INTEL
diff --git a/arch/x86/include/asm/asm.h b/arch/x86/include/asm/asm.h
index 189679aba703..a5d483ac11fa 100644
--- a/arch/x86/include/asm/asm.h
+++ b/arch/x86/include/asm/asm.h
@@ -44,13 +44,19 @@
 
 /* Exception table entry */
 #ifdef __ASSEMBLY__
-# define _ASM_EXTABLE(from,to)					\
-	.pushsection "__ex_table","a" ;				\
+# define __ASM_EXTABLE(from, to, table)				\
+	.pushsection table, "a" ;				\
 	.balign 8 ;						\
 	.long (from) - . ;					\
 	.long (to) - . ;					\
 	.popsection
 
+# define _ASM_EXTABLE(from, to)					\
+	__ASM_EXTABLE(from, to, "__ex_table")
+
+# define _ASM_MCEXTABLE(from, to)				\
+	__ASM_EXTABLE(from, to, "__mcex_table")
+
 # define _ASM_EXTABLE_EX(from,to)				\
 	.pushsection "__ex_table","a" ;				\
 	.balign 8 ;						\
diff --git a/arch/x86/include/asm/uaccess.h b/arch/x86/include/asm/uaccess.h
index a8df874f3e88..7b02ca1991b4 100644
--- a/arch/x86/include/asm/uaccess.h
+++ b/arch/x86/include/asm/uaccess.h
@@ -111,6 +111,14 @@ struct exception_table_entry {
 #define ARCH_HAS_SEARCH_EXTABLE
 
 extern int fixup_exception(struct pt_regs *regs);
+#ifdef CONFIG_MCE_KERNEL_RECOVERY
+extern int fixup_mcexception(struct pt_regs *regs, u64 addr);
+#else
+static inline int fixup_mcexception(struct pt_regs *regs, u64 addr)
+{
+	return 0;
+}
+#endif
 extern int early_fixup_exception(unsigned long *ip);
 
 /*
diff --git a/arch/x86/mm/extable.c b/arch/x86/mm/extable.c
index 903ec1e9c326..a461c4212758 100644
--- a/arch/x86/mm/extable.c
+++ b/arch/x86/mm/extable.c
@@ -49,6 +49,25 @@ int fixup_exception(struct pt_regs *regs)
 	return 0;
 }
 
+#ifdef CONFIG_MCE_KERNEL_RECOVERY
+int fixup_mcexception(struct pt_regs *regs, u64 addr)
+{
+	const struct exception_table_entry *fixup;
+	unsigned long new_ip;
+
+	fixup = search_mcexception_tables(regs->ip);
+	if (fixup) {
+		new_ip = ex_fixup_addr(fixup);
+
+		regs->ip = new_ip;
+		regs->ax = BIT(63) | addr;
+		return 1;
+	}
+
+	return 0;
+}
+#endif
+
 /* Restricted version used during very early boot */
 int __init early_fixup_exception(unsigned long *ip)
 {
diff --git a/include/asm-generic/vmlinux.lds.h b/include/asm-generic/vmlinux.lds.h
index 1781e54ea6d3..21bb20d1172a 100644
--- a/include/asm-generic/vmlinux.lds.h
+++ b/include/asm-generic/vmlinux.lds.h
@@ -473,6 +473,12 @@
 		VMLINUX_SYMBOL(__start___ex_table) = .;			\
 		*(__ex_table)						\
 		VMLINUX_SYMBOL(__stop___ex_table) = .;			\
+	}								\
+	. = ALIGN(align);						\
+	__mcex_table : AT(ADDR(__mcex_table) - LOAD_OFFSET) {		\
+		VMLINUX_SYMBOL(__start___mcex_table) = .;		\
+		*(__mcex_table)						\
+		VMLINUX_SYMBOL(__stop___mcex_table) = .;		\
 	}
 
 /*
diff --git a/include/linux/module.h b/include/linux/module.h
index 3a19c79918e0..ffecbfcc462c 100644
--- a/include/linux/module.h
+++ b/include/linux/module.h
@@ -270,6 +270,7 @@ extern const typeof(name) __mod_##type##__##name##_device_table		\
 
 /* Given an address, look for it in the exception tables */
 const struct exception_table_entry *search_exception_tables(unsigned long add);
+const struct exception_table_entry *search_mcexception_tables(unsigned long a);
 
 struct notifier_block;
 
diff --git a/kernel/extable.c b/kernel/extable.c
index e820ccee9846..7b224fbcb708 100644
--- a/kernel/extable.c
+++ b/kernel/extable.c
@@ -34,6 +34,10 @@ DEFINE_MUTEX(text_mutex);
 
 extern struct exception_table_entry __start___ex_table[];
 extern struct exception_table_entry __stop___ex_table[];
+#ifdef CONFIG_MCE_KERNEL_RECOVERY
+extern struct exception_table_entry __start___mcex_table[];
+extern struct exception_table_entry __stop___mcex_table[];
+#endif
 
 /* Cleared by build time tools if the table is already sorted. */
 u32 __initdata __visible main_extable_sort_needed = 1;
@@ -45,6 +49,10 @@ void __init sort_main_extable(void)
 		pr_notice("Sorting __ex_table...\n");
 		sort_extable(__start___ex_table, __stop___ex_table);
 	}
+#ifdef CONFIG_MCE_KERNEL_RECOVERY
+	if (__stop___mcex_table > __start___mcex_table)
+		sort_extable(__start___mcex_table, __stop___mcex_table);
+#endif
 }
 
 /* Given an address, look for it in the exception tables. */
@@ -58,6 +66,18 @@ const struct exception_table_entry *search_exception_tables(unsigned long addr)
 	return e;
 }
 
+#ifdef CONFIG_MCE_KERNEL_RECOVERY
+/* Given an address, look for it in the machine check exception tables. */
+const struct exception_table_entry *search_mcexception_tables(
+				    unsigned long addr)
+{
+	const struct exception_table_entry *e;
+
+	e = search_extable(__start___mcex_table, __stop___mcex_table-1, addr);
+	return e;
+}
+#endif
+
 static inline int init_kernel_text(unsigned long addr)
 {
 	if (addr >= (unsigned long)_sinittext &&
-- 
2.1.4

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [next] | [standalone]

#1289879 — Re: [PATCHV2 1/3] x86, ras: Add new infrastructure for machine check fixup tables

On Thu, Dec 10, 2015 at 1:58 PM, Tony Luck <tony.luck@intel.com> wrote:
> Copy the existing page fault fixup mechanisms to create a new table
> to be used when fixing machine checks. Note:
> 1) At this time we only provide a macro to annotate assembly code
> 2) We assume all fixups will in code builtin to the kernel.
> 3) Only for x86_64
> 4) New code under CONFIG_MCE_KERNEL_RECOVERY
>
> Signed-off-by: Tony Luck <tony.luck@intel.com>
> ---

> +#ifdef CONFIG_MCE_KERNEL_RECOVERY
> +int fixup_mcexception(struct pt_regs *regs, u64 addr)
> +{
> +       const struct exception_table_entry *fixup;
> +       unsigned long new_ip;
> +
> +       fixup = search_mcexception_tables(regs->ip);
> +       if (fixup) {
> +               new_ip = ex_fixup_addr(fixup);
> +
> +               regs->ip = new_ip;
> +               regs->ax = BIT(63) | addr;

Can this be an actual #define?

--Andy
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1289900 — RE: [PATCHV2 1/3] x86, ras: Add new infrastructure for machine check fixup tables

Pj4gKyAgICAgICAgICAgICAgIHJlZ3MtPmlwID0gbmV3X2lwOw0KPj4gKyAgICAgICAgICAgICAg
IHJlZ3MtPmF4ID0gQklUKDYzKSB8IGFkZHI7DQo+DQo+IENhbiB0aGlzIGJlIGFuIGFjdHVhbCAj
ZGVmaW5lPw0KDQpEb2ghICBZZXMsIG9mIGNvdXJzZS4gVGhhdCB3b3VsZCBiZSBtdWNoIGJldHRl
ci4NCg0KTm93IEkgbmVlZCB0byB0aGluayBvZiBhIGdvb2QgbmFtZSBmb3IgaXQuDQoNCi1Ub255
DQo=
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1290158 — Re: [PATCHV2 1/3] x86, ras: Add new infrastructure for machine check fixup tables

On Thu, Dec 10, 2015 at 01:58:04PM -0800, Tony Luck wrote:
> Copy the existing page fault fixup mechanisms to create a new table
> to be used when fixing machine checks. Note:
> 1) At this time we only provide a macro to annotate assembly code
> 2) We assume all fixups will in code builtin to the kernel.
> 3) Only for x86_64
> 4) New code under CONFIG_MCE_KERNEL_RECOVERY
> 
> Signed-off-by: Tony Luck <tony.luck@intel.com>
> ---
>  arch/x86/Kconfig                  |  4 ++++
>  arch/x86/include/asm/asm.h        | 10 ++++++++--
>  arch/x86/include/asm/uaccess.h    |  8 ++++++++
>  arch/x86/mm/extable.c             | 19 +++++++++++++++++++
>  include/asm-generic/vmlinux.lds.h |  6 ++++++
>  include/linux/module.h            |  1 +
>  kernel/extable.c                  | 20 ++++++++++++++++++++
>  7 files changed, 66 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
> index 96d058a87100..db5c6e1d6e37 100644
> --- a/arch/x86/Kconfig
> +++ b/arch/x86/Kconfig
> @@ -1001,6 +1001,10 @@ config X86_MCE_INJECT
>  	  If you don't know what a machine check is and you don't do kernel
>  	  QA it is safe to say n.
>  
> +config MCE_KERNEL_RECOVERY
> +	depends on X86_MCE && X86_64
> +	def_bool y

Shouldn't that depend on NVDIMM or whatnot? Looks too generic now.

> +
>  config X86_THERMAL_VECTOR
>  	def_bool y
>  	depends on X86_MCE_INTEL
> diff --git a/arch/x86/include/asm/asm.h b/arch/x86/include/asm/asm.h
> index 189679aba703..a5d483ac11fa 100644
> --- a/arch/x86/include/asm/asm.h
> +++ b/arch/x86/include/asm/asm.h
> @@ -44,13 +44,19 @@
>  
>  /* Exception table entry */
>  #ifdef __ASSEMBLY__
> -# define _ASM_EXTABLE(from,to)					\
> -	.pushsection "__ex_table","a" ;				\
> +# define __ASM_EXTABLE(from, to, table)				\
> +	.pushsection table, "a" ;				\
>  	.balign 8 ;						\
>  	.long (from) - . ;					\
>  	.long (to) - . ;					\
>  	.popsection
>  
> +# define _ASM_EXTABLE(from, to)					\
> +	__ASM_EXTABLE(from, to, "__ex_table")
> +
> +# define _ASM_MCEXTABLE(from, to)				\
> +	__ASM_EXTABLE(from, to, "__mcex_table")
> +
>  # define _ASM_EXTABLE_EX(from,to)				\
>  	.pushsection "__ex_table","a" ;				\
>  	.balign 8 ;						\
> diff --git a/arch/x86/include/asm/uaccess.h b/arch/x86/include/asm/uaccess.h
> index a8df874f3e88..7b02ca1991b4 100644
> --- a/arch/x86/include/asm/uaccess.h
> +++ b/arch/x86/include/asm/uaccess.h
> @@ -111,6 +111,14 @@ struct exception_table_entry {
>  #define ARCH_HAS_SEARCH_EXTABLE
>  
>  extern int fixup_exception(struct pt_regs *regs);
> +#ifdef CONFIG_MCE_KERNEL_RECOVERY
> +extern int fixup_mcexception(struct pt_regs *regs, u64 addr);
> +#else
> +static inline int fixup_mcexception(struct pt_regs *regs, u64 addr)
> +{
> +	return 0;
> +}
> +#endif
>  extern int early_fixup_exception(unsigned long *ip);

No need for "extern"

>  
>  /*
> diff --git a/arch/x86/mm/extable.c b/arch/x86/mm/extable.c
> index 903ec1e9c326..a461c4212758 100644
> --- a/arch/x86/mm/extable.c
> +++ b/arch/x86/mm/extable.c
> @@ -49,6 +49,25 @@ int fixup_exception(struct pt_regs *regs)
>  	return 0;
>  }
>  
> +#ifdef CONFIG_MCE_KERNEL_RECOVERY
> +int fixup_mcexception(struct pt_regs *regs, u64 addr)
> +{

If you move the #ifdef here, you can save yourself the ifdeffery in the
header above.

> +	const struct exception_table_entry *fixup;
> +	unsigned long new_ip;
> +
> +	fixup = search_mcexception_tables(regs->ip);
> +	if (fixup) {
> +		new_ip = ex_fixup_addr(fixup);
> +
> +		regs->ip = new_ip;
> +		regs->ax = BIT(63) | addr;
> +		return 1;
> +	}
> +
> +	return 0;
> +}
> +#endif
> +
>  /* Restricted version used during very early boot */
>  int __init early_fixup_exception(unsigned long *ip)
>  {
> diff --git a/include/asm-generic/vmlinux.lds.h b/include/asm-generic/vmlinux.lds.h
> index 1781e54ea6d3..21bb20d1172a 100644
> --- a/include/asm-generic/vmlinux.lds.h
> +++ b/include/asm-generic/vmlinux.lds.h
> @@ -473,6 +473,12 @@
>  		VMLINUX_SYMBOL(__start___ex_table) = .;			\
>  		*(__ex_table)						\
>  		VMLINUX_SYMBOL(__stop___ex_table) = .;			\
> +	}								\
> +	. = ALIGN(align);						\
> +	__mcex_table : AT(ADDR(__mcex_table) - LOAD_OFFSET) {		\
> +		VMLINUX_SYMBOL(__start___mcex_table) = .;		\
> +		*(__mcex_table)						\
> +		VMLINUX_SYMBOL(__stop___mcex_table) = .;		\

Of all the places, this one is missing #ifdef CONFIG_MCE_KERNEL_RECOVERY.

>  	}
>  
>  /*
> diff --git a/include/linux/module.h b/include/linux/module.h
> index 3a19c79918e0..ffecbfcc462c 100644
> --- a/include/linux/module.h
> +++ b/include/linux/module.h
> @@ -270,6 +270,7 @@ extern const typeof(name) __mod_##type##__##name##_device_table		\
>  
>  /* Given an address, look for it in the exception tables */
>  const struct exception_table_entry *search_exception_tables(unsigned long add);
> +const struct exception_table_entry *search_mcexception_tables(unsigned long a);
>  
>  struct notifier_block;
>  
> diff --git a/kernel/extable.c b/kernel/extable.c
> index e820ccee9846..7b224fbcb708 100644
> --- a/kernel/extable.c
> +++ b/kernel/extable.c
> @@ -34,6 +34,10 @@ DEFINE_MUTEX(text_mutex);
>  
>  extern struct exception_table_entry __start___ex_table[];
>  extern struct exception_table_entry __stop___ex_table[];
> +#ifdef CONFIG_MCE_KERNEL_RECOVERY
> +extern struct exception_table_entry __start___mcex_table[];
> +extern struct exception_table_entry __stop___mcex_table[];
> +#endif
>  
>  /* Cleared by build time tools if the table is already sorted. */
>  u32 __initdata __visible main_extable_sort_needed = 1;
> @@ -45,6 +49,10 @@ void __init sort_main_extable(void)
>  		pr_notice("Sorting __ex_table...\n");
>  		sort_extable(__start___ex_table, __stop___ex_table);
>  	}
> +#ifdef CONFIG_MCE_KERNEL_RECOVERY
> +	if (__stop___mcex_table > __start___mcex_table)
> +		sort_extable(__start___mcex_table, __stop___mcex_table);
> +#endif
>  }
>  
>  /* Given an address, look for it in the exception tables. */
> @@ -58,6 +66,18 @@ const struct exception_table_entry *search_exception_tables(unsigned long addr)
>  	return e;
>  }
>  
> +#ifdef CONFIG_MCE_KERNEL_RECOVERY
> +/* Given an address, look for it in the machine check exception tables. */
> +const struct exception_table_entry *search_mcexception_tables(
> +				    unsigned long addr)
> +{
> +	const struct exception_table_entry *e;
> +
> +	e = search_extable(__start___mcex_table, __stop___mcex_table-1, addr);
> +	return e;
> +}
> +#endif

You can make this one a bit more readable by doing:

/* Given an address, look for it in the machine check exception tables. */
const struct exception_table_entry *
search_mcexception_tables(unsigned long addr)
{
#ifdef CONFIG_MCE_KERNEL_RECOVERY
        return search_extable(__start___mcex_table,
                               __stop___mcex_table - 1, addr);
#endif
}

-- 
Regards/Gruss,
    Boris.

ECO tip #101: Trim your mails when you reply.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1291393 — Re: [PATCHV2 1/3] x86, ras: Add new infrastructure for machine check fixup tables

On Sat, Dec 12, 2015 at 3:11 AM, Borislav Petkov <bp@alien8.de> wrote:
> On Thu, Dec 10, 2015 at 01:58:04PM -0800, Tony Luck wrote:
<>
>> +#ifdef CONFIG_MCE_KERNEL_RECOVERY
>> +/* Given an address, look for it in the machine check exception tables. */
>> +const struct exception_table_entry *search_mcexception_tables(
>> +                                 unsigned long addr)
>> +{
>> +     const struct exception_table_entry *e;
>> +
>> +     e = search_extable(__start___mcex_table, __stop___mcex_table-1, addr);
>> +     return e;
>> +}
>> +#endif
>
> You can make this one a bit more readable by doing:
>
> /* Given an address, look for it in the machine check exception tables. */
> const struct exception_table_entry *
> search_mcexception_tables(unsigned long addr)
> {
> #ifdef CONFIG_MCE_KERNEL_RECOVERY
>         return search_extable(__start___mcex_table,
>                                __stop___mcex_table - 1, addr);
> #endif
> }

With this code if CONFIG_MCE_KERNEL_RECOVERY isn't defined you'll get
a compiler error that the function doesn't have a return statement,
right?  I think we need an #else to return NULL, or to have the #ifdef
encompass the whole function definition as it was in Tony's version.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1291613 — Re: [PATCHV2 1/3] x86, ras: Add new infrastructure for machine check fixup tables

On Mon, Dec 14, 2015 at 10:58:45AM -0700, Ross Zwisler wrote:
> With this code if CONFIG_MCE_KERNEL_RECOVERY isn't defined you'll get
> a compiler error that the function doesn't have a return statement,
> right?  I think we need an #else to return NULL, or to have the #ifdef
> encompass the whole function definition as it was in Tony's version.

Right, correct.

Thanks.

-- 
Regards/Gruss,
    Boris.

ECO tip #101: Trim your mails when you reply.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1291737 — Re: [PATCHV2 1/3] x86, ras: Add new infrastructure for machine check fixup tables

On Sat, Dec 12, 2015 at 11:11:42AM +0100, Borislav Petkov wrote:
> > +config MCE_KERNEL_RECOVERY
> > +	depends on X86_MCE && X86_64
> > +	def_bool y
> 
> Shouldn't that depend on NVDIMM or whatnot? Looks too generic now.

Not sure what the "whatnot" would be though.  Making it depend on
X86_MCE should keep it out of the tiny configurations. By the time
you have MCE support, this seems like a pretty small incremental
change.

> > +#ifdef CONFIG_MCE_KERNEL_RECOVERY
> > +int fixup_mcexception(struct pt_regs *regs, u64 addr)
> > +{
> 
> If you move the #ifdef here, you can save yourself the ifdeffery in the
> header above.

I realized I didn't need the inline stub function in the header.

> > diff --git a/include/asm-generic/vmlinux.lds.h b/include/asm-generic/vmlinux.lds.h
> > index 1781e54ea6d3..21bb20d1172a 100644
> > --- a/include/asm-generic/vmlinux.lds.h
> > +++ b/include/asm-generic/vmlinux.lds.h
> > @@ -473,6 +473,12 @@
> >  		VMLINUX_SYMBOL(__start___ex_table) = .;			\
> >  		*(__ex_table)						\
> >  		VMLINUX_SYMBOL(__stop___ex_table) = .;			\
> > +	}								\
> > +	. = ALIGN(align);						\
> > +	__mcex_table : AT(ADDR(__mcex_table) - LOAD_OFFSET) {		\
> > +		VMLINUX_SYMBOL(__start___mcex_table) = .;		\
> > +		*(__mcex_table)						\
> > +		VMLINUX_SYMBOL(__stop___mcex_table) = .;		\
> 
> Of all the places, this one is missing #ifdef CONFIG_MCE_KERNEL_RECOVERY.

Is there some cpp magic to use an #ifdef inside a multi-line macro like this?
Impact of not having the #ifdef is two extra symbols (the start/stop ones)
in the symbol table of the final binary. If that's unacceptable I can fall
back to an earlier unpublished version that had separate EXCEPTION_TABLE and
MCEXCEPTION_TABLE macros with both invoked in the x86 vmlinux.lds.S file.

> You can make this one a bit more readable by doing:
> 
> /* Given an address, look for it in the machine check exception tables. */
> const struct exception_table_entry *
> search_mcexception_tables(unsigned long addr)
> {
> #ifdef CONFIG_MCE_KERNEL_RECOVERY
>         return search_extable(__start___mcex_table,
>                                __stop___mcex_table - 1, addr);
> #endif
> }

I got rid of the local variable and the return ... but left the
#ifdef/#endif around the whole function.

-Tony
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1291981 — Re: [PATCHV2 1/3] x86, ras: Add new infrastructure for machine check fixup tables

On Mon, Dec 14, 2015 at 05:00:59PM -0800, Luck, Tony wrote:
> Not sure what the "whatnot" would be though.  Making it depend on
> X86_MCE should keep it out of the tiny configurations. By the time
> you have MCE support, this seems like a pretty small incremental
> change.

Ok, so it is called CONFIG_LIBNVDIMM. Do you see a use case for this
stuff except on machines with NVDIMM hw? CONFIG_LIBNVDIMM can select it
but on !NVDIMM systems you don't really need it enabled.

> Is there some cpp magic to use an #ifdef inside a multi-line macro like this?
> Impact of not having the #ifdef is two extra symbols (the start/stop ones)
> in the symbol table of the final binary. If that's unacceptable I can fall
> back to an earlier unpublished version that had separate EXCEPTION_TABLE and
> MCEXCEPTION_TABLE macros with both invoked in the x86 vmlinux.lds.S file.

I think what is more important is that this should be in the
x86-specific linker script, not in the generic one. And yes, we should
strive to be clean and not pullute the kernel image with symbols which
are unused, i.e. when CONFIG_MCE_KERNEL_RECOVERY is not enabled.

This below seems to build ok here, ontop of yours. It could be a
MCEXCEPTION_TABLE macro, as you say:

Index: b/include/asm-generic/vmlinux.lds.h
===================================================================
--- a/include/asm-generic/vmlinux.lds.h	2015-12-15 10:17:25.568046033 +0100
+++ b/include/asm-generic/vmlinux.lds.h	2015-12-15 10:07:06.064034490 +0100
@@ -484,12 +484,6 @@
 		*(__ex_table)						\
 		VMLINUX_SYMBOL(__stop___ex_table) = .;			\
 	}								\
-	. = ALIGN(align);						\
-	__mcex_table : AT(ADDR(__mcex_table) - LOAD_OFFSET) {		\
-		VMLINUX_SYMBOL(__start___mcex_table) = .;		\
-		*(__mcex_table)						\
-		VMLINUX_SYMBOL(__stop___mcex_table) = .;		\
-	}
 
 /*
  * Init task
Index: b/arch/x86/kernel/vmlinux.lds.S
===================================================================
--- a/arch/x86/kernel/vmlinux.lds.S	2015-12-14 11:38:58.188150070 +0100
+++ b/arch/x86/kernel/vmlinux.lds.S	2015-12-15 10:09:04.624036699 +0100
@@ -110,7 +110,17 @@ SECTIONS
 
 	NOTES :text :note
 
-	EXCEPTION_TABLE(16) :text = 0x9090
+	EXCEPTION_TABLE(16)
+
+#ifdef CONFIG_MCE_KERNEL_RECOVERY
+	. = ALIGN(16);
+	__mcex_table : AT(ADDR(__mcex_table) - LOAD_OFFSET) {
+		VMLINUX_SYMBOL(__start___mcex_table) = .;
+		*(__mcex_table)
+		VMLINUX_SYMBOL(__stop___mcex_table) = .;
+	}
+#endif
+	:text = 0x9090
 
 #if defined(CONFIG_DEBUG_RODATA)
 	/* .text should occupy whole number of pages */

-- 
Regards/Gruss,
    Boris.

ECO tip #101: Trim your mails when you reply.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1292038 — Re: [PATCHV2 1/3] x86, ras: Add new infrastructure for machine check fixup tables

On Tue, Dec 15, 2015 at 10:46:53AM +0100, Borislav Petkov wrote:
> I think what is more important is that this should be in the
> x86-specific linker script, not in the generic one.

And related to that, I think all those additions to kernel/extable.c
should be somewhere in arch/x86/ and not in generic code.

-- 
Regards/Gruss,
    Boris.

ECO tip #101: Trim your mails when you reply.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web