LinuxLists.cc - [PATCH] x86 (Linux Tiny): configure out support for some processors

[permalink] [raw]

Subject: Re: [PATCH] x86 (Linux Tiny): configure out support for some processors

Michael Opdenacker wrote:
> This patch against x86/mm tries to revive an original patch
> from Matt Mackall which didn't get merged at that time. It makes
> it possible to disable support code for some processors. This can
> be useful to support only the exact processor type used
> in a given system.

How much is it actually worth?

-hpa

2008-02-08 23:12:10

by Ingo Molnar

[permalink] [raw]

Subject: Re: [PATCH] x86 (Linux Tiny): configure out support for some processors

looks good to me, but the patch needs a serious #ifdef removal pass:

* Michael Opdenacker <[email protected]> wrote:

> switch (c->x86_vendor) {
> +#ifdef CONFIG_CPU_SUP_AMD
> case X86_VENDOR_AMD:
> early_init_amd(c);
> break;
> +#endif
> +#ifdef CONFIG_CPU_SUP_INTEL
> case X86_VENDOR_INTEL:
> early_init_intel(c);
> break;
> +#endif

would be nice to hide these #ifdefs into include files. (define
early_init_intel()/etc. as an empty inline in an include file)

> +#ifdef CONFIG_CPU_SUP_INTEL
> intel_cpu_init();
> +#endif
> +#ifdef CONFIG_CPU_SUP_CYRIX
> cyrix_init_cpu();
> +#endif
> +#ifdef CONFIG_CPU_SUP_NSC
> nsc_init_cpu();
> +#endif
> +#ifdef CONFIG_CPU_SUP_AMD
> amd_init_cpu();
> +#endif
> +#ifdef CONFIG_CPU_SUP_CENTAUR
> centaur_init_cpu();
> +#endif
> +#ifdef CONFIG_CPU_SUP_TRANSMETA
> transmeta_init_cpu();
> +#endif
> +#ifdef CONFIG_CPU_SUP_NEXGEN
> nexgen_init_cpu();
> +#endif
> +#ifdef CONFIG_CPU_SUP_UMC
> umc_init_cpu();
> +#endif

ditto - hide this into cpu.h.

> static int __cpuinit mwait_usable(const struct cpuinfo_x86 *c)
> {
> +#ifdef CONFIG_CPU_SUP_AMD
> if (force_mwait)
> return 1;
> +#endif

same - use cpu.h to define force_mwait to 0 if !CPU_SUP_AMD.

> +#ifdef CONFIG_CPU_SUP_INTEL
> static inline int page_kills_ppro(unsigned long pagenr)
> {
> if (pagenr >= 0x70000 && pagenr <= 0x7003F)
> return 1;
> return 0;
> }
> +#endif

put the #ifdef _inside_ the inline, thus:

> - if (page_is_ram(pfn) && !(bad_ppro && page_kills_ppro(pfn))) {
> + if (page_is_ram(pfn)
> +#ifdef CONFIG_CPU_SUP_INTEL
> + && !(bad_ppro && page_kills_ppro(pfn))
> +#endif

you can avoid this #ifdef. [handle bad_ppro too]

Ingo

2008-02-08 23:21:15

[permalink] [raw]

Subject: Re: [PATCH] x86 (Linux Tiny): configure out support for some processors

On Fri, 2008-02-08 at 23:47 +0100, Michael Opdenacker wrote:
> This patch against x86/mm tries to revive an original patch
> from Matt Mackall which didn't get merged at that time. It makes
> it possible to disable support code for some processors. This can
> be useful to support only the exact processor type used
> in a given system.
>
> I may have made wrong assumptions with the code handling
> force_mwait. As force_mwait is only declared in
> arch/x86/kernel/cpu/amd.c, which is only compiled
> when CONFIG_X86_32 is set, I thought it was safe
> to make the code depend on CONFIG_CPU_SUP_AMD,
> but I could be wrong.
>
> Your comments are more than welcome! To make the code
> cleaner, I could use empty inline functions instead
> of ifdef's, as suggested in Documentation/SubmittingPatches.

Please include the output of size with all these options on and off.

> diff --git a/arch/x86/kernel/process_32.c b/arch/x86/kernel/process_32.c
> index dabdbef..8f9a123 100644
> --- a/arch/x86/kernel/process_32.c
> +++ b/arch/x86/kernel/process_32.c
> @@ -287,8 +287,10 @@ static void mwait_idle(void)
>
> static int __cpuinit mwait_usable(const struct cpuinfo_x86 *c)
> {
> +#ifdef CONFIG_CPU_SUP_AMD
> if (force_mwait)
> return 1;
> +#endif

Probably makes sense to move force_mwait (one word) here and eliminate
these ifdefs.

> diff --git a/arch/x86/mm/init_32.c b/arch/x86/mm/init_32.c
> index 347a8cd..812bfa0 100644
> --- a/arch/x86/mm/init_32.c
> +++ b/arch/x86/mm/init_32.c
> @@ -211,12 +211,14 @@ static void __init kernel_physical_mapping_init(pgd_t *pgd_base)
> }
> }
>
> +#ifdef CONFIG_CPU_SUP_INTEL
> static inline int page_kills_ppro(unsigned long pagenr)
> {
> if (pagenr >= 0x70000 && pagenr <= 0x7003F)
> return 1;
> return 0;
> }
> +#endif
> /*
> * devmem_is_allowed() checks to see if /dev/mem access to a certain address
> @@ -287,7 +289,11 @@ static void __meminit free_new_highpage(struct page *page)
>
> void __init add_one_highpage_init(struct page *page, int pfn, int bad_ppro)
> {
> - if (page_is_ram(pfn) && !(bad_ppro && page_kills_ppro(pfn))) {
> + if (page_is_ram(pfn)
> +#ifdef CONFIG_CPU_SUP_INTEL
> + && !(bad_ppro && page_kills_ppro(pfn))
> +#endif

Yuck. A better way to do this is move the bad_ppro check into
page_kills_ppro and then ifdef out -the body- of the inline.

> @@ -592,7 +598,11 @@ void __init mem_init(void)
> #ifdef CONFIG_FLATMEM
> BUG_ON(!mem_map);
> #endif
> +#ifdef CONFIG_CPU_SUP_INTEL
> bad_ppro = ppro_with_ram_bug();
> +#else
> + bad_ppro = 0;
> +#endif

Again, move the storage for this, let it get initialized to zero
automatically, and initialize it in the CPU-specific code (if ordering
allows).

--
Mathematics is the supreme nostalgia of our time.

2008-02-09 03:49:17

by Taral

[permalink] [raw]

Subject: Re: [PATCH] x86 (Linux Tiny): configure out support for some processors

On 2/8/08, Michael Opdenacker <[email protected]> wrote:
> +config CPU_SUP_INTEL
> + default y
> + bool "Support Intel processors" if PROCESSOR_SELECT
> + help
> + This enables extended support for Intel processors

> -obj-$(CONFIG_X86_32) += intel.o
> +obj-$(CONFIG_CPU_SUP_INTEL) += intel.o

This config option should probably depend on X86_32.

--
Taral <[email protected]>
"Please let me know if there's any further trouble I can give you."
-- Unknown

2008-02-09 08:24:45

by Simon Holm Thøgersen

[permalink] [raw]

Subject: Re: [PATCH] x86 (Linux Tiny): configure out support for some processors

fre, 08 02 2008 kl. 17:20 -0600, skrev Matt Mackall:
> On Fri, 2008-02-08 at 23:47 +0100, Michael Opdenacker wrote:
> > This patch against x86/mm tries to revive an original patch
> > from Matt Mackall which didn't get merged at that time. It makes
> > it possible to disable support code for some processors. This can
> > be useful to support only the exact processor type used
> > in a given system.
> >
> > I may have made wrong assumptions with the code handling
> > force_mwait. As force_mwait is only declared in
> > arch/x86/kernel/cpu/amd.c, which is only compiled
> > when CONFIG_X86_32 is set, I thought it was safe
> > to make the code depend on CONFIG_CPU_SUP_AMD,
> > but I could be wrong.
> >
> > Your comments are more than welcome! To make the code
> > cleaner, I could use empty inline functions instead
> > of ifdef's, as suggested in Documentation/SubmittingPatches.
>
> Please include the output of size with all these options on and off.

The build of my currently running kernel for my laptop has
$ size -t amd.o cyrix.o centaur.o transmeta.o intel.o nexgen.o umc.o
text data bss dec hex filename
2809 316 0 3125 c35 amd.o
2387 856 0 3243 cab cyrix.o
1514 312 0 1826 722 centaur.o
1279 312 0 1591 637 transmeta.o
1783 316 0 2099 833 intel.o
126 312 0 438 1b6 nexgen.o
41 312 0 353 161 umc.o
9939 2736 0 12675 3183 (TOTALS)

That is without optimize for size compilation, with that set I get
$ size -t amd.o cyrix.o centaur.o transmeta.o intel.o nexgen.o umc.o
text data bss dec hex filename
2300 316 0 2616 a38 amd.o
2132 820 0 2952 b88 cyrix.o
1325 312 0 1637 665 centaur.o
1151 312 0 1463 5b7 transmeta.o
1575 316 0 1891 763 intel.o
107 312 0 419 1a3 nexgen.o
41 312 0 353 161 umc.o
8631 2700 0 11331 2c43 (TOTALS)

I don't think the code changes in the patch do much with respect to
size.

Simon Holm Thøgersen

2008-02-09 09:26:36

[permalink] [raw]

Subject: Re: [PATCH] x86 (Linux Tiny): configure out support for some processors

On 02/09/2008 12:20 AM, Matt Mackall wrote:
> Please include the output of size with all these options on and off.
>
Oops, here they are:

Standard kernel (original config: make allnoconfig + CONFIG_EMBEDDED):
> size vmlinux
text data bss dec hex filename
966473 139000 90112 1195585 123e41 vmlinux

Size of vmlinux: 1386005

With the patch (using only CONFIG_CPU_SUP_INTEL):
> size vmlinux
text data bss dec hex filename
957561 136536 90112 1184209 1211d1 vmlinux
(-9812) (-2464)

Size of vmlinux: 1373697 (-12308 bytes)

-12K in the kernel size looks quite nice to me.

Michael.

--
Michael Opdenacker, Free Electrons
Free Embedded Linux Training Materials
on http://free-electrons.com/training
(More than 1500 pages!)

2008-02-09 09:29:23

[permalink] [raw]

Subject: Re: [PATCH] x86 (Linux Tiny): configure out support for some processors

On 02/09/2008 09:30 AM, Simon Holm Thøgersen wrote:
> The build of my currently running kernel for my laptop has
> $ size -t amd.o cyrix.o centaur.o transmeta.o intel.o nexgen.o umc.o
> text data bss dec hex filename
> 2809 316 0 3125 c35 amd.o
> 2387 856 0 3243 cab cyrix.o
> 1514 312 0 1826 722 centaur.o
> 1279 312 0 1591 637 transmeta.o
> 1783 316 0 2099 833 intel.o
> 126 312 0 438 1b6 nexgen.o
> 41 312 0 353 161 umc.o
> 9939 2736 0 12675 3183 (TOTALS)
>
> That is without optimize for size compilation, with that set I get
> $ size -t amd.o cyrix.o centaur.o transmeta.o intel.o nexgen.o umc.o
> text data bss dec hex filename
> 2300 316 0 2616 a38 amd.o
> 2132 820 0 2952 b88 cyrix.o
> 1325 312 0 1637 665 centaur.o
> 1151 312 0 1463 5b7 transmeta.o
> 1575 316 0 1891 763 intel.o
> 107 312 0 419 1a3 nexgen.o
> 41 312 0 353 161 umc.o
> 8631 2700 0 11331 2c43 (TOTALS)
>
> I don't think the code changes in the patch do much with respect to
> size.
>
Thanks for this report. Don't you think it's still useful to save up to
12 K of code that you don't use if you just have an Intel processor (for
example)?

Cheers,

Michael.

--
Michael Opdenacker, Free Electrons
Free Embedded Linux Training Materials
on http://free-electrons.com/training
(More than 1500 pages!)

2008-02-09 09:31:25

[permalink] [raw]

Subject: Re: [PATCH] x86 (Linux Tiny): configure out support for some processors

On 02/09/2008 12:11 AM, Ingo Molnar wrote:
>
> would be nice to hide these #ifdefs into include files. (define
> early_init_intel()/etc. as an empty inline in an include file)
>
Hi Ingo,

Thank you very much for your quick review and your suggestions. I will
try to post the corresponding update tonight.

Cheers,

Michael.

--
Michael Opdenacker, Free Electrons
Free Embedded Linux Training Materials
on http://free-electrons.com/training
(More than 1500 pages!)

2008-02-09 09:59:51

by Simon Holm Thøgersen

[permalink] [raw]

Subject: Re: [PATCH] x86 (Linux Tiny): configure out support for some processors

lør, 09 02 2008 kl. 10:29 +0100, skrev Michael Opdenacker:
> On 02/09/2008 09:30 AM, Simon Holm Thøgersen wrote:
> > The build of my currently running kernel for my laptop has
> > $ size -t amd.o cyrix.o centaur.o transmeta.o intel.o nexgen.o umc.o
> > text data bss dec hex filename
> > 2809 316 0 3125 c35 amd.o
> > 2387 856 0 3243 cab cyrix.o
> > 1514 312 0 1826 722 centaur.o
> > 1279 312 0 1591 637 transmeta.o
> > 1783 316 0 2099 833 intel.o
> > 126 312 0 438 1b6 nexgen.o
> > 41 312 0 353 161 umc.o
> > 9939 2736 0 12675 3183 (TOTALS)
> >
> > That is without optimize for size compilation, with that set I get
> > $ size -t amd.o cyrix.o centaur.o transmeta.o intel.o nexgen.o umc.o
> > text data bss dec hex filename
> > 2300 316 0 2616 a38 amd.o
> > 2132 820 0 2952 b88 cyrix.o
> > 1325 312 0 1637 665 centaur.o
> > 1151 312 0 1463 5b7 transmeta.o
> > 1575 316 0 1891 763 intel.o
> > 107 312 0 419 1a3 nexgen.o
> > 41 312 0 353 161 umc.o
> > 8631 2700 0 11331 2c43 (TOTALS)
> >
> > I don't think the code changes in the patch do much with respect to
> > size.
> >
> Thanks for this report. Don't you think it's still useful to save up to
> 12 K of code that you don't use if you just have an Intel processor (for
> example)?

The last remark was only about the code changes in
arch/x86/kernel/cpu/common.c, arch/x86/kernel/process_32.c,
arch/x86/kernel/process_64.c and arch/x86/mm/init_32.c, which my report
didn't reflect upon since I never applied the patch. The ~12kB reduction
no doubt has a good gain/pain ratio.

Out of curiosity, how small a kernel are you targeting this work for? I
guess your other post on 'make allnoconfig + CONFIG_EMBEDDED' has
disabled more stuff than you would use in practice?

Simon

2008-02-09 21:37:27

[permalink] [raw]

Subject: Re: [PATCH] x86 (Linux Tiny): configure out support for some processors

Michael Opdenacker wrote:
> Thanks for this report. Don't you think it's still useful to save up to
> 12 K of code that you don't use if you just have an Intel processor (for
> example)?

For 12K it better be a very clean patch... which I don't consider this
patch to be; it has way too many #ifdefs in the code.

-hpa

2008-02-11 22:43:19

[permalink] [raw]

Subject: Re: [PATCH] x86 (Linux Tiny): configure out support for some processors

On Saturday 09 February 2008, Ingo Molnar wrote:
> ditto - hide this into cpu.h.

I did hide all the ifdefs into cpu.h as you suggested. See the new patch below.
>
> > static int __cpuinit mwait_usable(const struct cpuinfo_x86 *c)
> > {
> > +#ifdef CONFIG_CPU_SUP_AMD
> > if (force_mwait)
> > return 1;
> > +#endif
>
> same - use cpu.h to define force_mwait to 0 if !CPU_SUP_AMD.

I didn't manage to, as force_mwait is also assigned in the same code.
I worked around the problem by declaring force_mwait elsewhere
(see my explanations in the patch comment)

---

[PATCH] x86 (Linux Tiny): configure out support for some processors

This patch against x86/mm tries to revive an original patch
from Matt Mackall which didn't get merged at that time. It makes
it possible to disable support code for some processors. This can
be useful to support only the exact processor type used
in a given system.

This patch allows to save up to 12K of text and data.

This time, I tried to use as few ifdefs as possible,
and move them away to include files.

Note that I had to modify include/asm-x86/bugs.h to declare
ppro_with_ram_bug() as an inline function. I hope there's
no harm.

Also note that for the sake of convenience and consistency
between 32 and 64 bit, I move the declaration of force_mwait
from kernel/cpu/amd.c to kernel/setup_32.c
(force_mwait is already defined in kernel/setup_64.c).

Thanks for your reviews! Michael.

Signed-off-by: Michael Opdenacker <[email protected]>
---
arch/x86/Kconfig.cpu | 56 +++++++++++++++++
arch/x86/kernel/cpu/Makefile | 28 +++++-----
arch/x86/kernel/cpu/amd.c | 6 +-
arch/x86/kernel/cpu/centaur.c | 2 +-
arch/x86/kernel/cpu/cpu.h | 124 +++++++++++++++++++++++++++++++++++----
arch/x86/kernel/cpu/cyrix.c | 4 +-
arch/x86/kernel/cpu/intel.c | 6 +-
arch/x86/kernel/cpu/nexgen.c | 2 +-
arch/x86/kernel/cpu/transmeta.c | 2 +-
arch/x86/kernel/cpu/umc.c | 2 +-
arch/x86/kernel/setup_32.c | 2 +
arch/x86/mm/init_32.c | 2 +
include/asm-x86/bugs.h | 2 +-
13 files changed, 199 insertions(+), 39 deletions(-)

diff --git a/arch/x86/Kconfig.cpu b/arch/x86/Kconfig.cpu
index e09a6b7..4fcd0fb 100644
--- a/arch/x86/Kconfig.cpu
+++ b/arch/x86/Kconfig.cpu
@@ -396,3 +396,59 @@ config X86_MINIMUM_CPU_FAMILY
config X86_DEBUGCTLMSR
def_bool y
depends on !(M586MMX || M586TSC || M586 || M486 || M386)
+
+menuconfig PROCESSOR_SELECT
+ depends on EMBEDDED && X86_32
+ bool "Supported processor vendors"
+ default n
+ help
+ This lets you choose what x86 vendor support code your kernel
+ will include.
+
+config CPU_SUP_INTEL
+ bool "Support Intel processors" if PROCESSOR_SELECT
+ default y
+ help
+ This enables extended support for Intel processors
+
+config CPU_SUP_CYRIX
+ bool "Support Cyrix processors" if PROCESSOR_SELECT
+ default y
+ help
+ This enables extended support for Cyrix processors
+
+config CPU_SUP_NSC
+ bool "Support NSC processors" if PROCESSOR_SELECT
+ default y
+ help
+ This enables extended support for NSC processors
+
+config CPU_SUP_AMD
+ bool "Support AMD processors" if PROCESSOR_SELECT
+ default y
+ help
+ This enables extended support for AMD processors
+
+config CPU_SUP_CENTAUR
+ bool "Support Centaur processors" if PROCESSOR_SELECT
+ default y
+ help
+ This enables extended support for Centaur processors
+
+config CPU_SUP_TRANSMETA
+ bool "Support Transmeta processors" if PROCESSOR_SELECT
+ default y
+ help
+ This enables extended support for Transmeta processors
+
+config CPU_SUP_NEXGEN
+ bool "Support NexGen processors" if PROCESSOR_SELECT
+ default y
+ help
+ This enables extended support for NexGen processors
+
+config CPU_SUP_UMC
+ bool "Support UMC processors" if PROCESSOR_SELECT
+ default y
+ help
+ This enables extended support for UMC processors
diff --git a/arch/x86/kernel/cpu/Makefile b/arch/x86/kernel/cpu/Makefile
index a0c4d7c..a01cb67 100644
--- a/arch/x86/kernel/cpu/Makefile
+++ b/arch/x86/kernel/cpu/Makefile
@@ -2,20 +2,20 @@
# Makefile for x86-compatible CPU details and quirks
#

-obj-y := intel_cacheinfo.o addon_cpuid_features.o
-obj-y += feature_names.o
+obj-y := intel_cacheinfo.o addon_cpuid_features.o
+obj-y += feature_names.o

-obj-$(CONFIG_X86_32) += common.o proc.o bugs.o
-obj-$(CONFIG_X86_32) += amd.o
-obj-$(CONFIG_X86_32) += cyrix.o
-obj-$(CONFIG_X86_32) += centaur.o
-obj-$(CONFIG_X86_32) += transmeta.o
-obj-$(CONFIG_X86_32) += intel.o
-obj-$(CONFIG_X86_32) += nexgen.o
-obj-$(CONFIG_X86_32) += umc.o
+obj-$(CONFIG_X86_32) += common.o proc.o bugs.o
+obj-$(CONFIG_CPU_SUP_AMD) += amd.o
+obj-$(CONFIG_CPU_SUP_CYRIX) += cyrix.o
+obj-$(CONFIG_CPU_SUP_CENTAUR) += centaur.o
+obj-$(CONFIG_CPU_SUP_TRANSMETA) += transmeta.o
+obj-$(CONFIG_CPU_SUP_INTEL) += intel.o
+obj-$(CONFIG_CPU_SUP_NEXGEN) += nexgen.o
+obj-$(CONFIG_CPU_SUP_UMC) += umc.o

-obj-$(CONFIG_X86_MCE) += mcheck/
-obj-$(CONFIG_MTRR) += mtrr/
-obj-$(CONFIG_CPU_FREQ) += cpufreq/
+obj-$(CONFIG_X86_MCE) += mcheck/
+obj-$(CONFIG_MTRR) += mtrr/
+obj-$(CONFIG_CPU_FREQ) += cpufreq/

-obj-$(CONFIG_X86_LOCAL_APIC) += perfctr-watchdog.o
+obj-$(CONFIG_X86_LOCAL_APIC) += perfctr-watchdog.o
diff --git a/arch/x86/kernel/cpu/amd.c b/arch/x86/kernel/cpu/amd.c
index 693e353..8baeae9 100644
--- a/arch/x86/kernel/cpu/amd.c
+++ b/arch/x86/kernel/cpu/amd.c
@@ -61,9 +61,7 @@ static __cpuinit int amd_apic_timer_broken(void)
}
#endif

-int force_mwait __cpuinitdata;
-
-void __cpuinit early_init_amd(struct cpuinfo_x86 *c)
+void __cpuinit __early_init_amd(struct cpuinfo_x86 *c)
{
if (cpuid_eax(0x80000000) >= 0x80000007) {
c->x86_power = cpuid_edx(0x80000007);
@@ -340,7 +338,7 @@ static struct cpu_dev amd_cpu_dev __cpuinitdata = {
.c_size_cache = amd_size_cache,
};

-int __init amd_init_cpu(void)
+int __init __amd_init_cpu(void)
{
cpu_devs[X86_VENDOR_AMD] = &amd_cpu_dev;
return 0;
diff --git a/arch/x86/kernel/cpu/centaur.c b/arch/x86/kernel/cpu/centaur.c
index 9681fa1..c20f44b 100644
--- a/arch/x86/kernel/cpu/centaur.c
+++ b/arch/x86/kernel/cpu/centaur.c
@@ -464,7 +464,7 @@ static struct cpu_dev centaur_cpu_dev __cpuinitdata = {
.c_size_cache = centaur_size_cache,
};

-int __init centaur_init_cpu(void)
+int __init __centaur_init_cpu(void)
{
cpu_devs[X86_VENDOR_CENTAUR] = &centaur_cpu_dev;
return 0;
diff --git a/arch/x86/kernel/cpu/cpu.h b/arch/x86/kernel/cpu/cpu.h
index e0b38c3..3e54edc 100644
--- a/arch/x86/kernel/cpu/cpu.h
+++ b/arch/x86/kernel/cpu/cpu.h
@@ -24,15 +24,117 @@ extern struct cpu_dev * cpu_devs [X86_VENDOR_NUM];
extern int get_model_name(struct cpuinfo_x86 *c);
extern void display_cacheinfo(struct cpuinfo_x86 *c);

-extern void early_init_intel(struct cpuinfo_x86 *c);
-extern void early_init_amd(struct cpuinfo_x86 *c);
-
/* Specific CPU type init functions */
-int intel_cpu_init(void);
-int amd_init_cpu(void);
-int cyrix_init_cpu(void);
-int nsc_init_cpu(void);
-int centaur_init_cpu(void);
-int transmeta_init_cpu(void);
-int nexgen_init_cpu(void);
-int umc_init_cpu(void);
+
+#ifdef CONFIG_CPU_SUP_INTEL
+int __cpuinit __ppro_with_ram_bug(void);
+static inline int __cpuinit ppro_with_ram_bug(void)
+{
+ return __ppro_with_ram_bug();
+}
+
+void __cpuinit __early_init_intel(struct cpuinfo_x86 *c);
+static inline void __cpuinit early_init_intel(struct cpuinfo_x86 *c)
+{
+ __early_init_intel(c);
+}
+
+int __intel_cpu_init(void);
+static inline int intel_cpu_init(void)
+{
+ return __intel_cpu_init();
+}
+#else
+static inline int __cpuinit ppro_with_ram_bug(void)
+{
+ return 0;
+}
+
+static inline void __cpuinit early_init_intel(struct cpuinfo_x86 *c) {}
+
+static inline int intel_cpu_init(void)
+{
+ return 0;
+}
+#endif
+
+#ifdef CONFIG_CPU_SUP_AMD
+int __amd_init_cpu(void);
+static inline int amd_init_cpu(void)
+{
+ return __amd_init_cpu();
+}
+
+void __cpuinit __early_init_amd(struct cpuinfo_x86 *c);
+static inline void __cpuinit early_init_amd(struct cpuinfo_x86 *c)
+{
+ __early_init_amd(c);
+}
+#else
+static inline int amd_init_cpu(void)
+{
+ return 0;
+}
+
+static inline void __cpuinit early_init_amd(struct cpuinfo_x86 *c) {}
+#endif
+
+int __cyrix_init_cpu(void);
+static inline int cyrix_init_cpu(void)
+{
+#ifdef CONFIG_CPU_SUP_CYRIX
+ return __cyrix_init_cpu();
+#else
+ return 0;
+#endif
+}
+
+int __nsc_init_cpu(void);
+static inline int nsc_init_cpu(void)
+{
+#ifdef CONFIG_CPU_SUP_NSC
+ return __nsc_init_cpu();
+#else
+ return 0;
+#endif
+}
+
+int __centaur_init_cpu(void);
+static inline int centaur_init_cpu(void)
+{
+#ifdef CONFIG_CPU_SUP_CENTAUR
+ return __centaur_init_cpu();
+#else
+ return 0;
+#endif
+}
+
+int __transmeta_init_cpu(void);
+static inline int transmeta_init_cpu(void)
+{
+#ifdef CONFIG_CPU_SUP_TRANSMETA
+ return __transmeta_init_cpu();
+#else
+ return 0;
+#endif
+}
+
+int __nexgen_init_cpu(void);
+static inline int nexgen_init_cpu(void)
+{
+#ifdef CONFIG_CPU_SUP_NEXGEN
+ return __nexgen_init_cpu();
+#else
+ return 0;
+#endif
+}
+
+int __umc_init_cpu(void);
+static inline int umc_init_cpu(void)
+{
+#ifdef CONFIG_CPU_SUP_UMC
+ return __umc_init_cpu();
+#else
+ return 0;
+#endif
+}
diff --git a/arch/x86/kernel/cpu/cyrix.c b/arch/x86/kernel/cpu/cyrix.c
index 7139b02..038aa60 100644
--- a/arch/x86/kernel/cpu/cyrix.c
+++ b/arch/x86/kernel/cpu/cyrix.c
@@ -439,7 +439,7 @@ static struct cpu_dev cyrix_cpu_dev __cpuinitdata = {
.c_identify = cyrix_identify,
};

-int __init cyrix_init_cpu(void)
+int __init __cyrix_init_cpu(void)
{
cpu_devs[X86_VENDOR_CYRIX] = &cyrix_cpu_dev;
return 0;
@@ -451,7 +451,7 @@ static struct cpu_dev nsc_cpu_dev __cpuinitdata = {
.c_init = init_nsc,
};

-int __init nsc_init_cpu(void)
+int __init __nsc_init_cpu(void)
{
cpu_devs[X86_VENDOR_NSC] = &nsc_cpu_dev;
return 0;
diff --git a/arch/x86/kernel/cpu/intel.c b/arch/x86/kernel/cpu/intel.c
index fae31ce..7eaac8e 100644
--- a/arch/x86/kernel/cpu/intel.c
+++ b/arch/x86/kernel/cpu/intel.c
@@ -30,7 +30,7 @@
struct movsl_mask movsl_mask __read_mostly;
#endif

-void __cpuinit early_init_intel(struct cpuinfo_x86 *c)
+void __cpuinit __early_init_intel(struct cpuinfo_x86 *c)
{
/* Netburst reports 64 bytes clflush size, but does IO in 128 bytes */
if (c->x86 == 15 && c->x86_cache_alignment == 64)
@@ -46,7 +46,7 @@ void __cpuinit early_init_intel(struct cpuinfo_x86 *c)
* This is called before we do cpu ident work
*/

-int __cpuinit ppro_with_ram_bug(void)
+int __cpuinit __ppro_with_ram_bug(void)
{
/* Uses data from early_cpu_detect now */
if (boot_cpu_data.x86_vendor == X86_VENDOR_INTEL &&
@@ -294,7 +294,7 @@ static struct cpu_dev intel_cpu_dev __cpuinitdata = {
.c_size_cache = intel_size_cache,
};

-__init int intel_cpu_init(void)
+__init int __intel_cpu_init(void)
{
cpu_devs[X86_VENDOR_INTEL] = &intel_cpu_dev;
return 0;
diff --git a/arch/x86/kernel/cpu/nexgen.c b/arch/x86/kernel/cpu/nexgen.c
index 961fbe1..c607e80 100644
--- a/arch/x86/kernel/cpu/nexgen.c
+++ b/arch/x86/kernel/cpu/nexgen.c
@@ -53,7 +53,7 @@ static struct cpu_dev nexgen_cpu_dev __cpuinitdata = {
.c_identify = nexgen_identify,
};

-int __init nexgen_init_cpu(void)
+int __init __nexgen_init_cpu(void)
{
cpu_devs[X86_VENDOR_NEXGEN] = &nexgen_cpu_dev;
return 0;
diff --git a/arch/x86/kernel/cpu/transmeta.c b/arch/x86/kernel/cpu/transmeta.c
index 200fb3f..5538853 100644
--- a/arch/x86/kernel/cpu/transmeta.c
+++ b/arch/x86/kernel/cpu/transmeta.c
@@ -109,7 +109,7 @@ static struct cpu_dev transmeta_cpu_dev __cpuinitdata = {
.c_identify = transmeta_identify,
};

-int __init transmeta_init_cpu(void)
+int __init __transmeta_init_cpu(void)
{
cpu_devs[X86_VENDOR_TRANSMETA] = &transmeta_cpu_dev;
return 0;
diff --git a/arch/x86/kernel/cpu/umc.c b/arch/x86/kernel/cpu/umc.c
index a7a4e75..c8ef9f6 100644
--- a/arch/x86/kernel/cpu/umc.c
+++ b/arch/x86/kernel/cpu/umc.c
@@ -19,7 +19,7 @@ static struct cpu_dev umc_cpu_dev __cpuinitdata = {
},
};

-int __init umc_init_cpu(void)
+int __init __umc_init_cpu(void)
{
cpu_devs[X86_VENDOR_UMC] = &umc_cpu_dev;
return 0;
diff --git a/arch/x86/kernel/setup_32.c b/arch/x86/kernel/setup_32.c
index 691ab4c..395bfb4 100644
--- a/arch/x86/kernel/setup_32.c
+++ b/arch/x86/kernel/setup_32.c
@@ -65,6 +65,8 @@
#include <bios_ebda.h>
#include <asm/cacheflush.h>

+int force_mwait __cpuinitdata;
+
/* This value is set up by the early boot code to point to the value
immediately after the boot time page tables. It contains a *physical*
address, and must not be in the .bss segment! */
diff --git a/arch/x86/mm/init_32.c b/arch/x86/mm/init_32.c
index 347a8cd..1179648 100644
--- a/arch/x86/mm/init_32.c
+++ b/arch/x86/mm/init_32.c
@@ -48,6 +48,8 @@
#include <asm/paravirt.h>
#include <asm/setup.h>

+#include "../kernel/cpu/cpu.h"
+
unsigned int __VMALLOC_RESERVE = 128 << 20;

DEFINE_PER_CPU(struct mmu_gather, mmu_gathers);
diff --git a/include/asm-x86/bugs.h b/include/asm-x86/bugs.h
index 021cbdd..a876041 100644
--- a/include/asm-x86/bugs.h
+++ b/include/asm-x86/bugs.h
@@ -2,6 +2,6 @@
#define _ASM_X86_BUGS_H

extern void check_bugs(void);
-int ppro_with_ram_bug(void);
+inline int ppro_with_ram_bug(void);

#endif /* _ASM_X86_BUGS_H */
--
1.5.2.5

--
Michael Opdenacker, Free Electrons
Free Embedded Linux Training Materials
on http://free-electrons.com/training
(More than 1500 pages!)

2008-02-11 22:57:38

[permalink] [raw]

Subject: Re: [PATCH] x86 (Linux Tiny): configure out support for some processors

On Mon, 2008-02-11 at 23:42 +0100, Michael Opdenacker wrote:
> /* Specific CPU type init functions */
> -int intel_cpu_init(void);
> -int amd_init_cpu(void);
> -int cyrix_init_cpu(void);
> -int nsc_init_cpu(void);
> -int centaur_init_cpu(void);
> -int transmeta_init_cpu(void);
> -int nexgen_init_cpu(void);
> -int umc_init_cpu(void);
> +
> +#ifdef CONFIG_CPU_SUP_INTEL
> +int __cpuinit __ppro_with_ram_bug(void);
> +static inline int __cpuinit ppro_with_ram_bug(void)
> +{
> + return __ppro_with_ram_bug();
> +}

I know Ingo said to do this, but I think he was flat-out wrong. If the
tradeoff is between having a dozen ifdefs contained in a single function
in one .c file vs wrapping a dozen function in a .h file, I say stick
them in the .c file.

Best would be to have no ifdefs and do it all with linker magic, of
course. But that's trickier.

Now the patch is 90% fiddling with wrappers and it's impossible to find
the interesting bits anymore..

--
Mathematics is the supreme nostalgia of our time.

2008-02-11 23:01:57

[permalink] [raw]

Subject: Re: [PATCH] x86 (Linux Tiny): configure out support for some processors

Michael Opdenacker wrote:
>
> Also note that for the sake of convenience and consistency
> between 32 and 64 bit, I move the declaration of force_mwait
> from kernel/cpu/amd.c to kernel/setup_32.c
> (force_mwait is already defined in kernel/setup_64.c).
>

NAK NAK NAK NAK NAK NAK NAK

This is the totally wrong thing to do. The CPU initialization stuff
should move *into* the CPU directory (unification based on the
cleaned-up 32-bit code, not on the 64-bit code which is based on a fork
of the pre-cleanup 32-bit code.)

-hpa

2008-02-11 23:02:45

[permalink] [raw]

Subject: Re: [PATCH] x86 (Linux Tiny): configure out support for some processors

Matt Mackall wrote:
>
> Best would be to have no ifdefs and do it all with linker magic, of
> course. But that's trickier.
>

I concur with this, definitely.

-hpa

2008-02-11 23:36:15

[permalink] [raw]

Subject: Re: [PATCH] x86 (Linux Tiny): configure out support for some processors

On Mon, 2008-02-11 at 15:01 -0800, H. Peter Anvin wrote:
> Matt Mackall wrote:
> >
> > Best would be to have no ifdefs and do it all with linker magic, of
> > course. But that's trickier.
> >
>
> I concur with this, definitely.

Ok, so let's come up with a plan. We can:

a) use weak symbols, ala cond_syscall
b) use a special section
c) use early_init code (is it early enough?)
c) have some sort of registration list

Having a generic cond_call of some sort might be nice for this sort of
thing.

--
Mathematics is the supreme nostalgia of our time.

2008-02-12 01:01:56

[permalink] [raw]

Subject: Re: [PATCH] x86 (Linux Tiny): configure out support for some processors

Matt Mackall wrote:
> On Mon, 2008-02-11 at 15:01 -0800, H. Peter Anvin wrote:
>> Matt Mackall wrote:
>>> Best would be to have no ifdefs and do it all with linker magic, of
>>> course. But that's trickier.
>>>
>> I concur with this, definitely.
>
> Ok, so let's come up with a plan. We can:
>
> a) use weak symbols, ala cond_syscall
> b) use a special section
> c) use early_init code (is it early enough?)
> c) have some sort of registration list
>
> Having a generic cond_call of some sort might be nice for this sort of
> thing.
>

c) is out, because this has to be executed after the early generic code
and before the late generic code.

b) would be my first choice, and yes, it would be a good thing to have a
generalized mechanism for this. For the registrant, it's pretty easy:
just add a macro that adds a pointer to a named section. We then need a
way to get the base address and length of each such section in order to
be able to execute each function in sequence.

-hpa

2008-02-12 01:20:23

[permalink] [raw]

Subject: Re: [PATCH] x86 (Linux Tiny): configure out support for some processors

On Mon, 2008-02-11 at 16:54 -0800, H. Peter Anvin wrote:
> Matt Mackall wrote:
> > On Mon, 2008-02-11 at 15:01 -0800, H. Peter Anvin wrote:
> >> Matt Mackall wrote:
> >>> Best would be to have no ifdefs and do it all with linker magic, of
> >>> course. But that's trickier.
> >>>
> >> I concur with this, definitely.
> >
> > Ok, so let's come up with a plan. We can:
> >
> > a) use weak symbols, ala cond_syscall
> > b) use a special section
> > c) use early_init code (is it early enough?)
> > c) have some sort of registration list
> >
> > Having a generic cond_call of some sort might be nice for this sort of
> > thing.
> >
>
> c) is out, because this has to be executed after the early generic code
> and before the late generic code.
>
> b) would be my first choice, and yes, it would be a good thing to have a
> generalized mechanism for this. For the registrant, it's pretty easy:
> just add a macro that adds a pointer to a named section. We then need a
> way to get the base address and length of each such section in order to
> be able to execute each function in sequence.

I like the idea of making a generalized hook section. But this is a bit
burdensome for Michael's little patch (unless you have time to whip
something up) so I think we should probably explore it separately.

--
Mathematics is the supreme nostalgia of our time.

2008-02-15 11:00:48

by Thomas Petazzoni

[permalink] [raw]

Subject: [RFC] [PATCH] x86: Use ELF section to list CPU vendor specific code (Linux Tiny)

Hi,

Le Mon, 11 Feb 2008 16:54:30 -0800,
"H. Peter Anvin" <[email protected]> a écrit :

> b) would be my first choice, and yes, it would be a good thing to
> have a generalized mechanism for this. For the registrant, it's
> pretty easy: just add a macro that adds a pointer to a named
> section. We then need a way to get the base address and length of
> each such section in order to be able to execute each function in
> sequence.

You'll find below a tentative patch that implements this. Tuple
(vendor, pointer to cpu_dev structure) are stored in a
x86cpuvendor.init section of the kernel, which is then read by the
generic CPU code in arch/x86/kernel/cpu/common.c to fill the cpu_devs[]
function.

Moreover the early_init_...() calls are integrated into that mechanism
using a new c_early_init() member of the cpu_dev structure.

The patch is for review only at the moment. Disabling compilation of
unused CPU support code will be done in a separate patch (I've taken
over Michael Opdenacker's work for the moment, with his agreement).

Thanks for your review and comments,

Thomas

---

Replace the hardcoded list of initialization functions for each CPU
vendor by a list in an ELF section, which is read at initialization in
arch/x86/kernel/cpu/cpu.c to fill the cpu_devs[] array. The ELF
section, named .x86cpuvendor.init, is reclaimed after boot, and
contains entries of type "struct cpu_vendor_dev" which associates a
vendor number with a pointer to a "struct cpu_dev" structure.

This first modification allows to remove all the VENDOR_init_cpu()
functions.

This patch also removes the hardcoded calls to early_init_amd() and
early_init_intel(). Instead, we add a "c_early_init" member to the
cpu_dev structure, which is then called if not NULL by the generic CPU
initialization code. Unfortunately, in early_cpu_detect(), this_cpu is
not yet set, so we have to use the cpu_devs[] array directly.

This patch is part of the Linux Tiny project, and is needed for
further patch that will allow to disable compilation of unused CPU
support code.

Signed-off-by: Thomas Petazzoni <[email protected]>

---
arch/x86/kernel/cpu/amd.c | 5 ++++-
arch/x86/kernel/cpu/centaur.c | 6 +-----
arch/x86/kernel/cpu/common.c | 33 ++++++++++-----------------------
arch/x86/kernel/cpu/cpu.h | 26 +++++++++++++-------------
arch/x86/kernel/cpu/cyrix.c | 13 ++-----------
arch/x86/kernel/cpu/intel.c | 9 +++------
arch/x86/kernel/cpu/transmeta.c | 6 +-----
arch/x86/kernel/cpu/umc.c | 7 ++-----
arch/x86/kernel/vmlinux_32.lds.S | 5 +++++
arch/x86/kernel/vmlinux_64.lds.S | 5 +++++
10 files changed, 46 insertions(+), 69 deletions(-)

Index: linux/arch/x86/kernel/cpu/amd.c
===================================================================
--- linux.orig/arch/x86/kernel/cpu/amd.c
+++ linux/arch/x86/kernel/cpu/amd.c
@@ -63,7 +63,7 @@

int force_mwait __cpuinitdata;

-void __cpuinit early_init_amd(struct cpuinfo_x86 *c)
+static void __cpuinit early_init_amd(struct cpuinfo_x86 *c)
{
if (cpuid_eax(0x80000000) >= 0x80000007) {
c->x86_power = cpuid_edx(0x80000007);
@@ -336,6 +336,7 @@
}
},
},
+ .c_early_init = early_init_amd,
.c_init = init_amd,
.c_size_cache = amd_size_cache,
};
@@ -345,3 +346,5 @@
cpu_devs[X86_VENDOR_AMD] = &amd_cpu_dev;
return 0;
}
+
+cpu_vendor_dev_register(X86_VENDOR_AMD, &amd_cpu_dev);
Index: linux/arch/x86/kernel/cpu/centaur.c
===================================================================
--- linux.orig/arch/x86/kernel/cpu/centaur.c
+++ linux/arch/x86/kernel/cpu/centaur.c
@@ -464,8 +464,4 @@
.c_size_cache = centaur_size_cache,
};

-int __init centaur_init_cpu(void)
-{
- cpu_devs[X86_VENDOR_CENTAUR] = &centaur_cpu_dev;
- return 0;
-}
+cpu_vendor_dev_register(X86_VENDOR_CENTAUR, &centaur_cpu_dev);
Index: linux/arch/x86/kernel/cpu/common.c
===================================================================
--- linux.orig/arch/x86/kernel/cpu/common.c
+++ linux/arch/x86/kernel/cpu/common.c
@@ -328,14 +328,9 @@

get_cpu_vendor(c, 1);

- switch (c->x86_vendor) {
- case X86_VENDOR_AMD:
- early_init_amd(c);
- break;
- case X86_VENDOR_INTEL:
- early_init_intel(c);
- break;
- }
+ if (c->x86_vendor != X86_VENDOR_UNKNOWN &&
+ cpu_devs[c->x86_vendor]->c_early_init)
+ cpu_devs[c->x86_vendor]->c_early_init(c);

early_get_cap(c);
}
@@ -616,23 +611,15 @@

cpumask_t cpu_initialized __cpuinitdata = CPU_MASK_NONE;

-/* This is hacky. :)
- * We're emulating future behavior.
- * In the future, the cpu-specific init functions will be called implicitly
- * via the magic of initcalls.
- * They will insert themselves into the cpu_devs structure.
- * Then, when cpu_init() is called, we can just iterate over that array.
- */
void __init early_cpu_init(void)
{
- intel_cpu_init();
- cyrix_init_cpu();
- nsc_init_cpu();
- amd_init_cpu();
- centaur_init_cpu();
- transmeta_init_cpu();
- nexgen_init_cpu();
- umc_init_cpu();
+ struct cpu_vendor_dev *cvdev;
+
+ for (cvdev = __x86cpuvendor_start ;
+ cvdev < __x86cpuvendor_end ;
+ cvdev++)
+ cpu_devs[cvdev->vendor] = cvdev->cpu_dev;
+
early_cpu_detect();
}

Index: linux/arch/x86/kernel/cpu/cpu.h
===================================================================
--- linux.orig/arch/x86/kernel/cpu/cpu.h
+++ linux/arch/x86/kernel/cpu/cpu.h
@@ -14,6 +14,7 @@

struct cpu_model_info c_models[4];

+ void (*c_early_init)(struct cpuinfo_x86 *c);
void (*c_init)(struct cpuinfo_x86 * c);
void (*c_identify)(struct cpuinfo_x86 * c);
unsigned int (*c_size_cache)(struct cpuinfo_x86 * c, unsigned int size);
@@ -21,18 +22,17 @@

extern struct cpu_dev * cpu_devs [X86_VENDOR_NUM];

-extern int get_model_name(struct cpuinfo_x86 *c);
-extern void display_cacheinfo(struct cpuinfo_x86 *c);
+struct cpu_vendor_dev {
+ int vendor;
+ struct cpu_dev *cpu_dev;
+};

-extern void early_init_intel(struct cpuinfo_x86 *c);
-extern void early_init_amd(struct cpuinfo_x86 *c);
+#define cpu_vendor_dev_register(cpu_vendor_id, cpu_dev) \
+ static struct cpu_vendor_dev __cpu_vendor_dev_##cpu_vendor_id __used \
+ __attribute__((__section__(".x86cpuvendor.init"))) = \
+ { cpu_vendor_id, cpu_dev }

-/* Specific CPU type init functions */
-int intel_cpu_init(void);
-int amd_init_cpu(void);
-int cyrix_init_cpu(void);
-int nsc_init_cpu(void);
-int centaur_init_cpu(void);
-int transmeta_init_cpu(void);
-int nexgen_init_cpu(void);
-int umc_init_cpu(void);
+extern struct cpu_vendor_dev __x86cpuvendor_start[], __x86cpuvendor_end[];
+
+extern int get_model_name(struct cpuinfo_x86 *c);
+extern void display_cacheinfo(struct cpuinfo_x86 *c);
Index: linux/arch/x86/kernel/cpu/cyrix.c
===================================================================
--- linux.orig/arch/x86/kernel/cpu/cyrix.c
+++ linux/arch/x86/kernel/cpu/cyrix.c
@@ -439,11 +439,7 @@
.c_identify = cyrix_identify,
};

-int __init cyrix_init_cpu(void)
-{
- cpu_devs[X86_VENDOR_CYRIX] = &cyrix_cpu_dev;
- return 0;
-}
+cpu_vendor_dev_register(X86_VENDOR_CYRIX, &cyrix_cpu_dev);

static struct cpu_dev nsc_cpu_dev __cpuinitdata = {
.c_vendor = "NSC",
@@ -451,9 +447,4 @@
.c_init = init_nsc,
};

-int __init nsc_init_cpu(void)
-{
- cpu_devs[X86_VENDOR_NSC] = &nsc_cpu_dev;
- return 0;
-}
-
+cpu_vendor_dev_register(X86_VENDOR_NSC, &nsc_cpu_dev);
Index: linux/arch/x86/kernel/cpu/intel.c
===================================================================
--- linux.orig/arch/x86/kernel/cpu/intel.c
+++ linux/arch/x86/kernel/cpu/intel.c
@@ -30,7 +30,7 @@
struct movsl_mask movsl_mask __read_mostly;
#endif

-void __cpuinit early_init_intel(struct cpuinfo_x86 *c)
+static void __cpuinit early_init_intel(struct cpuinfo_x86 *c)
{
/* Netburst reports 64 bytes clflush size, but does IO in 128 bytes */
if (c->x86 == 15 && c->x86_cache_alignment == 64)
@@ -290,15 +290,12 @@
}
},
},
+ .c_early_init = early_init_intel,
.c_init = init_intel,
.c_size_cache = intel_size_cache,
};

-__init int intel_cpu_init(void)
-{
- cpu_devs[X86_VENDOR_INTEL] = &intel_cpu_dev;
- return 0;
-}
+cpu_vendor_dev_register(X86_VENDOR_INTEL, &intel_cpu_dev);

#ifndef CONFIG_X86_CMPXCHG
unsigned long cmpxchg_386_u8(volatile void *ptr, u8 old, u8 new)
Index: linux/arch/x86/kernel/cpu/transmeta.c
===================================================================
--- linux.orig/arch/x86/kernel/cpu/transmeta.c
+++ linux/arch/x86/kernel/cpu/transmeta.c
@@ -109,8 +109,4 @@
.c_identify = transmeta_identify,
};

-int __init transmeta_init_cpu(void)
-{
- cpu_devs[X86_VENDOR_TRANSMETA] = &transmeta_cpu_dev;
- return 0;
-}
+cpu_vendor_dev_register(X86_VENDOR_TRANSMETA, &transmeta_cpu_dev);
Index: linux/arch/x86/kernel/cpu/umc.c
===================================================================
--- linux.orig/arch/x86/kernel/cpu/umc.c
+++ linux/arch/x86/kernel/cpu/umc.c
@@ -19,8 +19,5 @@
},
};

-int __init umc_init_cpu(void)
-{
- cpu_devs[X86_VENDOR_UMC] = &umc_cpu_dev;
- return 0;
-}
+cpu_vendor_dev_register(X86_VENDOR_UMC, &umc_cpu_dev);
+
Index: linux/arch/x86/kernel/vmlinux_32.lds.S
===================================================================
--- linux.orig/arch/x86/kernel/vmlinux_32.lds.S
+++ linux/arch/x86/kernel/vmlinux_32.lds.S
@@ -149,6 +149,11 @@
*(.con_initcall.init)
__con_initcall_end = .;
}
+ .x86cpuvendor.init : AT(ADDR(.x86cpuvendor.init) - LOAD_OFFSET) {
+ __x86cpuvendor_start = .;
+ *(.x86cpuvendor.init)
+ __x86cpuvendor_end = .;
+ }
SECURITY_INIT
. = ALIGN(4);
.altinstructions : AT(ADDR(.altinstructions) - LOAD_OFFSET) {
Index: linux/arch/x86/kernel/vmlinux_64.lds.S
===================================================================
--- linux.orig/arch/x86/kernel/vmlinux_64.lds.S
+++ linux/arch/x86/kernel/vmlinux_64.lds.S
@@ -177,6 +177,11 @@
*(.con_initcall.init)
}
__con_initcall_end = .;
+ __x86cpuvendor_start = .;
+ .x86cpuvendor.init : AT(ADDR(.x86cpuvendor.init) - LOAD_OFFSET) {
+ *(.x86cpuvendor.init)
+ }
+ __x86cpuvendor_end = .;
SECURITY_INIT

. = ALIGN(8);

--
Thomas Petazzoni, Free Electrons
Free Embedded Linux Training Materials
on http://free-electrons.com/training
(More than 1500 pages!)

Attachments:

signature.asc (189.00 B)

2008-02-17 18:15:05

by Ingo Molnar

[permalink] [raw]

Subject: Re: [RFC] [PATCH] x86: Use ELF section to list CPU vendor specific code (Linux Tiny)

* Thomas Petazzoni <[email protected]> wrote:

> Le Mon, 11 Feb 2008 16:54:30 -0800,
> "H. Peter Anvin" <[email protected]> a ?crit :
>
> > b) would be my first choice, and yes, it would be a good thing to
> > have a generalized mechanism for this. For the registrant, it's
> > pretty easy: just add a macro that adds a pointer to a named
> > section. We then need a way to get the base address and length of
> > each such section in order to be able to execute each function in
> > sequence.
>
> You'll find below a tentative patch that implements this. Tuple
> (vendor, pointer to cpu_dev structure) are stored in a
> x86cpuvendor.init section of the kernel, which is then read by the
> generic CPU code in arch/x86/kernel/cpu/common.c to fill the
> cpu_devs[] function.

thanks, i've picked this up into x86.git. It all looks much cleaner and
much more maintainable now. Peter, any objections?

Ingo

2008-02-17 19:47:36

[permalink] [raw]

Subject: Re: [RFC] [PATCH] x86: Use ELF section to list CPU vendor specific code (Linux Tiny)

Ingo Molnar wrote:
> * Thomas Petazzoni <[email protected]> wrote:
>
>> Le Mon, 11 Feb 2008 16:54:30 -0800,
>> "H. Peter Anvin" <[email protected]> a ?crit :
>>
>>> b) would be my first choice, and yes, it would be a good thing to
>>> have a generalized mechanism for this. For the registrant, it's
>>> pretty easy: just add a macro that adds a pointer to a named
>>> section. We then need a way to get the base address and length of
>>> each such section in order to be able to execute each function in
>>> sequence.
>> You'll find below a tentative patch that implements this. Tuple
>> (vendor, pointer to cpu_dev structure) are stored in a
>> x86cpuvendor.init section of the kernel, which is then read by the
>> generic CPU code in arch/x86/kernel/cpu/common.c to fill the
>> cpu_devs[] function.
>
> thanks, i've picked this up into x86.git. It all looks much cleaner and
> much more maintainable now. Peter, any objections?
>

Looks great to me.

-hpa

2008-02-23 02:44:32

[permalink] [raw]

Subject: Re: [RFC] [PATCH] x86: Use ELF section to list CPU vendor specific code (Linux Tiny)

On Fri, 2008-02-15 at 12:00 +0100, Thomas Petazzoni wrote:
> Hi,
>
> Le Mon, 11 Feb 2008 16:54:30 -0800,
> "H. Peter Anvin" <[email protected]> a écrit :
>
> > b) would be my first choice, and yes, it would be a good thing to
> > have a generalized mechanism for this. For the registrant, it's
> > pretty easy: just add a macro that adds a pointer to a named
> > section. We then need a way to get the base address and length of
> > each such section in order to be able to execute each function in
> > sequence.
>
> You'll find below a tentative patch that implements this. Tuple
> (vendor, pointer to cpu_dev structure) are stored in a
> x86cpuvendor.init section of the kernel, which is then read by the
> generic CPU code in arch/x86/kernel/cpu/common.c to fill the cpu_devs[]
> function.

This is not quite what Peter and I were thinking of, I think. It's not
at all generic. How about a section that simply contains a set of
function pointers, a macro to add things to that section, and a function
that calls all the pointers in that section. Eg:

CALLBACK_SECTION(init_cpu_amd, "cpuvendor.init");
invoke_callback_section("cpuvendor.init");

..which would give us a generic facility we could use in various places.

--
Mathematics is the supreme nostalgia of our time.

2008-02-23 03:20:39

[permalink] [raw]

Subject: Re: [RFC] [PATCH] x86: Use ELF section to list CPU vendor specific code (Linux Tiny)

Matt Mackall wrote:
>
> This is not quite what Peter and I were thinking of, I think. It's not
> at all generic. How about a section that simply contains a set of
> function pointers, a macro to add things to that section, and a function
> that calls all the pointers in that section. Eg:
>
> CALLBACK_SECTION(init_cpu_amd, "cpuvendor.init");
> invoke_callback_section("cpuvendor.init");
>
> ..which would give us a generic facility we could use in various places.
>

Indeed, we already have enough instances of this kind of stuff.

-hpa

2008-02-25 08:30:08

by Thomas Petazzoni

[permalink] [raw]

Subject: Re: [RFC] [PATCH] x86: Use ELF section to list CPU vendor specific code (Linux Tiny)

Le Sat, 23 Feb 2008 10:43:37 +0800,
Matt Mackall <[email protected]> a écrit :

> This is not quite what Peter and I were thinking of, I think. It's not
> at all generic. How about a section that simply contains a set of
> function pointers, a macro to add things to that section, and a
> function that calls all the pointers in that section. Eg:
>
> CALLBACK_SECTION(init_cpu_amd, "cpuvendor.init");
> invoke_callback_section("cpuvendor.init");
>
> ..which would give us a generic facility we could use in various
> places.

I see. Probably doable. How would it work in the LD script file ? Your
mechanism allows to specify any section name, but AFAIK, the sections
must be explicitly listed in the kernel LD script in order to be
included in the final kernel image. Am I missing something ?

Sincerly,

Thomas
--
Thomas Petazzoni, Free Electrons
Free Embedded Linux Training Materials
on http://free-electrons.com/training
(More than 1500 pages!)

Attachments:

signature.asc (189.00 B)

2008-02-25 17:07:16

[permalink] [raw]

Subject: Re: [RFC] [PATCH] x86: Use ELF section to list CPU vendor specific code (Linux Tiny)

On Mon, 2008-02-25 at 09:29 +0100, Thomas Petazzoni wrote:
> Le Sat, 23 Feb 2008 10:43:37 +0800,
> Matt Mackall <[email protected]> a écrit :
>
> > This is not quite what Peter and I were thinking of, I think. It's not
> > at all generic. How about a section that simply contains a set of
> > function pointers, a macro to add things to that section, and a
> > function that calls all the pointers in that section. Eg:
> >
> > CALLBACK_SECTION(init_cpu_amd, "cpuvendor.init");
> > invoke_callback_section("cpuvendor.init");
> >
> > ..which would give us a generic facility we could use in various
> > places.
>
> I see. Probably doable. How would it work in the LD script file ? Your
> mechanism allows to specify any section name, but AFAIK, the sections
> must be explicitly listed in the kernel LD script in order to be
> included in the final kernel image. Am I missing something ?

I can't see any way to avoid it, but we can leave it to future
generations to come up with something more clever.

--
Mathematics is the supreme nostalgia of our time.

2008-02-25 17:54:35

by Thomas Petazzoni

[permalink] [raw]

Subject: Re: [RFC] [PATCH] x86: Use ELF section to list CPU vendor specific code (Linux Tiny)

Le Mon, 25 Feb 2008 09:03:12 -0800,
Matt Mackall <[email protected]> a écrit :

> > > This is not quite what Peter and I were thinking of, I think.
> > > It's not at all generic. How about a section that simply contains
> > > a set of function pointers, a macro to add things to that
> > > section, and a function that calls all the pointers in that
> > > section. Eg:
> > >
> > > CALLBACK_SECTION(init_cpu_amd, "cpuvendor.init");
> > > invoke_callback_section("cpuvendor.init");
> > >
> > > ..which would give us a generic facility we could use in various
> > > places.
> >
> > I see. Probably doable. How would it work in the LD script file ?
> > Your mechanism allows to specify any section name, but AFAIK, the
> > sections must be explicitly listed in the kernel LD script in order
> > to be included in the final kernel image. Am I missing something ?
>
> I can't see any way to avoid it, but we can leave it to future
> generations to come up with something more clever.

After a quick look at the LD documentation, it seems that wildcards are
supported in the input section names of the linker script. So that the
CALLBACK_SECTION() macro could add the function pointer to a section
named:

gcm. ## name

(gcm standing for "generic callback mechanism") and then, in the linker
script, do:

*(gcm.*)

I'm going to try that.

Sincerly,

Thomas
--
Thomas Petazzoni, Free Electrons
Free Embedded Linux Training Materials
on http://free-electrons.com/training
(More than 1500 pages!)

Attachments:

signature.asc (189.00 B)

2008-02-25 17:59:33