public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Zachary Amsden <zach@vmware.com>
To: Chris Wright <chrisw@sous-sol.org>
Cc: Andrew Morton <akpm@osdl.org>, Linus Torvalds <torvalds@osdl.org>,
	Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
	pazke@donpac.ru, James.Bottomley@HansenPartnership.com
Subject: Re: [PATCH] Cleanup subarch definitions in Linux/i386
Date: Thu, 30 Mar 2006 11:25:24 -0800	[thread overview]
Message-ID: <442C30A4.3010207@vmware.com> (raw)
In-Reply-To: <20060330080025.GC14724@sorel.sous-sol.org>

[-- Attachment #1: Type: text/plain, Size: 4090 bytes --]

Chris Wright wrote:
> * Zachary Amsden (zach@vmware.com) wrote:
>   
>> Comments, suggestions, anything welcome.  I think this is a much cleaner 
>> approach, and both new and existing sub-architectures will benefit.  I 
>> am sorry this patch is so large, but it is very difficult to separate 
>> into multiple steps that still allow all the subarches to compile.
>>     

Thanks for looking over these.  There is a bit more cleanup on top of 
this, mostly due to the include mess.  I have attached another patch.

> Zach, looks nice.  Saves Xen a partial copy of setup.c.  Did you have
> further/similar consolidations in mind?
>   

Exactly.
>   
>> --- linux-2.6.16.1.orig/arch/i386/Makefile	2006-03-29 19:38:47.000000000 -0800
>> +++ linux-2.6.16.1/arch/i386/Makefile	2006-03-29 19:38:54.000000000 -0800
>> @@ -45,37 +45,32 @@ CFLAGS				+= $(shell if [ $(call cc-vers
>>  
>>  CFLAGS += $(cflags-y)
>>  
>> -# Default subarch .c files
>> -mcore-y  := mach-default
>> +# Default subarch .c files (none)
>> +mcore-y  := 
>>  
>>  # Voyager subarch support
>>  mflags-$(CONFIG_X86_VOYAGER)	:= -Iinclude/asm-i386/mach-voyager
>> -mcore-$(CONFIG_X86_VOYAGER)	:= mach-voyager
>> +mcore-$(CONFIG_X86_VOYAGER)	:= arch/i386/mach-voyager/
>>     
>
> Is this intended to make way for possible fine tuning?  Smth like:
>
> mcore-$(CONFIG_X86_VOYAGER)	+= arch/i386/another_default.o
> (hmm, not sure if that would even work)
>
> Or just an aesthetic change?
>   

No - it was required in order to leave the subarch blank for the default 
case.  Including "arch/i386//" wasn't so happy in the makefile.

>   
>> --- linux-2.6.16.1.orig/include/asm-i386/acpi.h	2006-03-29 19:38:47.000000000 -0800
>> +++ linux-2.6.16.1/include/asm-i386/acpi.h	2006-03-29 19:38:54.000000000 -0800
>> @@ -31,6 +31,7 @@
>>  #include <acpi/pdc_intel.h>
>>  
>>  #include <asm/system.h>		/* defines cmpxchg */
>> +#include <asm/processor.h>	/* defines boot_cpu_data */
>>     
>
> that one necessary?
>   

Needed for mach-generic subarch to compile with CPU defined as I386 -- 
the definition of cmpxchg on older CPUs depends on boot_cpu_data.  
Removing this opens a giant rat hole.  This ginourmous rat hole is quite 
annoying.  What goes in system.h vs. processor.h is so poorly defined.  
I had to hoist boot_cpu_data into system.h as the cleanest way to fix 
this without introducing circular dependencies - processor.h already 
depends on system.h for alternative_input(), read_cr4(), write_cr4(), 
write_cr3(), and there is no end to this mess.  Trying to move dependent 
processor.h pieces into system.h didn't work, since the include/linux 
headers make the assumption that they can include asm/processor.h to 
determine if the architecture already has a prefetch instruction 
defined.  What a nightmare.  Trying to move system.h pieces into 
processor.h also doesn't work, since you basically need to move the 
whole file.

Maybe that is the way to go, but I would rather not go there now.


>>  #define COMPILER_DEPENDENT_INT64   long long
>>  #define COMPILER_DEPENDENT_UINT64  unsigned long long
>> Index: linux-2.6.16.1/include/asm-i386/arch_hooks.h
>> ===================================================================
>> --- linux-2.6.16.1.orig/include/asm-i386/arch_hooks.h	2006-03-29 19:38:47.000000000 -0800
>> +++ linux-2.6.16.1/include/asm-i386/arch_hooks.h	2006-03-29 19:38:54.000000000 -0800
>> @@ -1,7 +1,13 @@
>>  #ifndef _ASM_ARCH_HOOKS_H
>>  #define _ASM_ARCH_HOOKS_H
>>  
>> +#include <linux/config.h>
>> +#include <linux/smp.h>
>> +#include <linux/init.h>
>>  #include <linux/interrupt.h>
>> +#include <asm/acpi.h>
>> +#include <asm/arch_hooks.h>
>>     
>
> extraneous include
>   

Yes, somewhat extraneous.

>   
>> --- linux-2.6.16.1.orig/include/asm-i386/mach-default/mach_hooks.h	2006-03-29 19:38:54.000000000 -0800
>> +++ linux-2.6.16.1/include/asm-i386/mach-default/mach_hooks.h	2006-03-29 19:38:54.000000000 -0800
>> @@ -0,0 +1,6 @@
>> +#ifndef _MACH_HOOKS_H
>> +#define _MACH_HOOKS_H
>>     
>
> should probably be consistent (_MACH_HOOKS_H vs. MACH_HOOKS_H)
>   
Patch attached.

[-- Attachment #2: i386-tidy-subarch-cleanup --]
[-- Type: text/plain, Size: 8242 bytes --]

Tidy up the subarch cleanup patch with nits noticed by Chris Wright.

Signed-off-by: Zachary Amsden <zach@vmware.com>

Index: linux-2.6.16.1/include/asm-i386/arch_hooks.h
===================================================================
--- linux-2.6.16.1.orig/include/asm-i386/arch_hooks.h	2006-03-30 10:53:45.000000000 -0800
+++ linux-2.6.16.1/include/asm-i386/arch_hooks.h	2006-03-30 10:54:27.000000000 -0800
@@ -1,13 +1,9 @@
 #ifndef _ASM_ARCH_HOOKS_H
 #define _ASM_ARCH_HOOKS_H
 
-#include <linux/config.h>
-#include <linux/smp.h>
-#include <linux/init.h>
 #include <linux/interrupt.h>
 #include <asm/acpi.h>
 #include <asm/arch_hooks.h>
-#include <asm/desc.h>
 
 /*
  *	linux/include/asm/arch_hooks.h
Index: linux-2.6.16.1/include/asm-i386/acpi.h
===================================================================
--- linux-2.6.16.1.orig/include/asm-i386/acpi.h	2006-03-30 10:53:45.000000000 -0800
+++ linux-2.6.16.1/include/asm-i386/acpi.h	2006-03-30 10:54:27.000000000 -0800
@@ -31,7 +31,6 @@
 #include <acpi/pdc_intel.h>
 
 #include <asm/system.h>		/* defines cmpxchg */
-#include <asm/processor.h>	/* defines boot_cpu_data */
 
 #define COMPILER_DEPENDENT_INT64   long long
 #define COMPILER_DEPENDENT_UINT64  unsigned long long
Index: linux-2.6.16.1/include/asm-i386/processor.h
===================================================================
--- linux-2.6.16.1.orig/include/asm-i386/processor.h	2006-03-30 10:53:45.000000000 -0800
+++ linux-2.6.16.1/include/asm-i386/processor.h	2006-03-30 11:03:16.000000000 -0800
@@ -16,7 +16,6 @@
 #include <asm/cpufeature.h>
 #include <asm/msr.h>
 #include <asm/system.h>
-#include <linux/cache.h>
 #include <linux/config.h>
 #include <linux/threads.h>
 #include <asm/percpu.h>
@@ -39,82 +38,13 @@ struct desc_struct {
  */
 #define current_text_addr() ({ void *pc; __asm__("movl $1f,%0\n1:":"=g" (pc)); pc; })
 
-/*
- *  CPU type and hardware bug flags. Kept separately for each CPU.
- *  Members of this structure are referenced in head.S, so think twice
- *  before touching them. [mj]
- */
-
-struct cpuinfo_x86 {
-	__u8	x86;		/* CPU family */
-	__u8	x86_vendor;	/* CPU vendor */
-	__u8	x86_model;
-	__u8	x86_mask;
-	char	wp_works_ok;	/* It doesn't on 386's */
-	char	hlt_works_ok;	/* Problems on some 486Dx4's and old 386's */
-	char	hard_math;
-	char	rfu;
-       	int	cpuid_level;	/* Maximum supported CPUID level, -1=no CPUID */
-	unsigned long	x86_capability[NCAPINTS];
-	char	x86_vendor_id[16];
-	char	x86_model_id[64];
-	int 	x86_cache_size;  /* in KB - valid for CPUS which support this
-				    call  */
-	int 	x86_cache_alignment;	/* In bytes */
-	char	fdiv_bug;
-	char	f00f_bug;
-	char	coma_bug;
-	char	pad0;
-	int	x86_power;
-	unsigned long loops_per_jiffy;
-	unsigned char x86_max_cores;	/* cpuid returned max cores value */
-	unsigned char booted_cores;	/* number of cores as seen by OS */
-	unsigned char apicid;
-} __attribute__((__aligned__(SMP_CACHE_BYTES)));
-
-#define X86_VENDOR_INTEL 0
-#define X86_VENDOR_CYRIX 1
-#define X86_VENDOR_AMD 2
-#define X86_VENDOR_UMC 3
-#define X86_VENDOR_NEXGEN 4
-#define X86_VENDOR_CENTAUR 5
-#define X86_VENDOR_RISE 6
-#define X86_VENDOR_TRANSMETA 7
-#define X86_VENDOR_NSC 8
-#define X86_VENDOR_NUM 9
-#define X86_VENDOR_UNKNOWN 0xff
-
-/*
- * capabilities of CPUs
- */
-
-extern struct cpuinfo_x86 boot_cpu_data;
-extern struct cpuinfo_x86 new_cpu_data;
 extern struct tss_struct doublefault_tss;
 DECLARE_PER_CPU(struct tss_struct, init_tss);
 
-#ifdef CONFIG_SMP
-extern struct cpuinfo_x86 cpu_data[];
-#define current_cpu_data cpu_data[smp_processor_id()]
-#else
-#define cpu_data (&boot_cpu_data)
-#define current_cpu_data boot_cpu_data
-#endif
-
 extern	int phys_proc_id[NR_CPUS];
 extern	int cpu_core_id[NR_CPUS];
 extern char ignore_fpu_irq;
 
-extern void identify_cpu(struct cpuinfo_x86 *);
-extern void print_cpu_info(struct cpuinfo_x86 *);
-extern unsigned int init_intel_cacheinfo(struct cpuinfo_x86 *c);
-
-#ifdef CONFIG_X86_HT
-extern void detect_ht(struct cpuinfo_x86 *c);
-#else
-static inline void detect_ht(struct cpuinfo_x86 *c) {}
-#endif
-
 /*
  * EFLAGS bits
  */
@@ -720,8 +650,6 @@ static inline void prefetchw(const void 
 
 extern void select_idle_routine(const struct cpuinfo_x86 *c);
 
-#define cache_line_size() (boot_cpu_data.x86_cache_alignment)
-
 extern unsigned long boot_option_idle_override;
 extern void enable_sep_cpu(void);
 extern int sysenter_setup(void);
Index: linux-2.6.16.1/include/asm-i386/system.h
===================================================================
--- linux-2.6.16.1.orig/include/asm-i386/system.h	2006-03-30 10:53:45.000000000 -0800
+++ linux-2.6.16.1/include/asm-i386/system.h	2006-03-30 11:04:28.000000000 -0800
@@ -5,12 +5,80 @@
 #include <linux/kernel.h>
 #include <asm/segment.h>
 #include <asm/cpufeature.h>
+#include <linux/cache.h>
 #include <linux/bitops.h> /* for LOCK_PREFIX */
 
 #ifdef __KERNEL__
 
-struct task_struct;	/* one of the stranger aspects of C forward declarations.. */
-extern struct task_struct * FASTCALL(__switch_to(struct task_struct *prev, struct task_struct *next));
+/*
+ *  CPU type and hardware bug flags. Kept separately for each CPU.
+ *  Members of this structure are referenced in head.S, so think twice
+ *  before touching them. [mj]
+ */
+
+struct cpuinfo_x86 {
+	__u8	x86;		/* CPU family */
+	__u8	x86_vendor;	/* CPU vendor */
+	__u8	x86_model;
+	__u8	x86_mask;
+	char	wp_works_ok;	/* It doesn't on 386's */
+	char	hlt_works_ok;	/* Problems on some 486Dx4's and old 386's */
+	char	hard_math;
+	char	rfu;
+       	int	cpuid_level;	/* Maximum supported CPUID level, -1=no CPUID */
+	unsigned long	x86_capability[NCAPINTS];
+	char	x86_vendor_id[16];
+	char	x86_model_id[64];
+	int 	x86_cache_size;  /* in KB - valid for CPUS which support this
+				    call  */
+	int 	x86_cache_alignment;	/* In bytes */
+	char	fdiv_bug;
+	char	f00f_bug;
+	char	coma_bug;
+	char	pad0;
+	int	x86_power;
+	unsigned long loops_per_jiffy;
+	unsigned char x86_max_cores;	/* cpuid returned max cores value */
+	unsigned char booted_cores;	/* number of cores as seen by OS */
+	unsigned char apicid;
+} __attribute__((__aligned__(SMP_CACHE_BYTES)));
+
+#define X86_VENDOR_INTEL 0
+#define X86_VENDOR_CYRIX 1
+#define X86_VENDOR_AMD 2
+#define X86_VENDOR_UMC 3
+#define X86_VENDOR_NEXGEN 4
+#define X86_VENDOR_CENTAUR 5
+#define X86_VENDOR_RISE 6
+#define X86_VENDOR_TRANSMETA 7
+#define X86_VENDOR_NSC 8
+#define X86_VENDOR_NUM 9
+#define X86_VENDOR_UNKNOWN 0xff
+
+/*
+ * capabilities of CPUs
+ */
+extern struct cpuinfo_x86 boot_cpu_data;
+extern struct cpuinfo_x86 new_cpu_data;
+#define cache_line_size() (boot_cpu_data.x86_cache_alignment)
+
+#ifdef CONFIG_SMP
+extern struct cpuinfo_x86 cpu_data[];
+#define current_cpu_data cpu_data[smp_processor_id()]
+#else
+#define cpu_data (&boot_cpu_data)
+#define current_cpu_data boot_cpu_data
+#endif
+
+extern void identify_cpu(struct cpuinfo_x86 *);
+extern void print_cpu_info(struct cpuinfo_x86 *);
+extern unsigned int init_intel_cacheinfo(struct cpuinfo_x86 *c);
+
+#ifdef CONFIG_X86_HT
+extern void detect_ht(struct cpuinfo_x86 *c);
+#else
+static inline void detect_ht(struct cpuinfo_x86 *c) {}
+#endif
 
 #define switch_to(prev,next,last) do {					\
 	unsigned long esi,edi;						\
Index: linux-2.6.16.1/include/asm-i386/mach-visws/mach_hooks.h
===================================================================
--- linux-2.6.16.1.orig/include/asm-i386/mach-visws/mach_hooks.h	2006-03-30 10:53:45.000000000 -0800
+++ linux-2.6.16.1/include/asm-i386/mach-visws/mach_hooks.h	2006-03-30 10:54:27.000000000 -0800
@@ -1,5 +1,5 @@
-#ifndef MACH_HOOKS_H
-#define MACH_HOOKS_H
+#ifndef _MACH_HOOKS_H
+#define _MACH_HOOKS_H
 
 #define pre_intr_init_hook() init_VISWS_APIC_irqs()
 
Index: linux-2.6.16.1/include/asm-i386/mach-voyager/mach_hooks.h
===================================================================
--- linux-2.6.16.1.orig/include/asm-i386/mach-voyager/mach_hooks.h	2006-03-30 10:53:45.000000000 -0800
+++ linux-2.6.16.1/include/asm-i386/mach-voyager/mach_hooks.h	2006-03-30 10:54:27.000000000 -0800
@@ -1,5 +1,5 @@
-#ifndef MACH_HOOKS_H
-#define MACH_HOOKS_H
+#ifndef _MACH_HOOKS_H
+#define _MACH_HOOKS_H
 
 /**
  * intr_init_hook - post gate setup interrupt initialisation

      reply	other threads:[~2006-03-30 19:25 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2006-03-30  4:18 [PATCH] Cleanup subarch definitions in Linux/i386 Zachary Amsden
2006-03-30  8:00 ` Chris Wright
2006-03-30 19:25   ` Zachary Amsden [this message]

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=442C30A4.3010207@vmware.com \
    --to=zach@vmware.com \
    --cc=James.Bottomley@HansenPartnership.com \
    --cc=akpm@osdl.org \
    --cc=chrisw@sous-sol.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=pazke@donpac.ru \
    --cc=torvalds@osdl.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox