Skip to contentSkip to conversation

Add noclflush option and related cleanup

PATCH In review Andi Kleen 4 participants 14 messages Jan 18, 2008 – Jan 19, 2008

Subsystems (5)
lore
Share & tools
lore

Andi Kleen

author
#0 lore raw

cover Add noclflush option and related cleanup

I added a noclflush option because Ingo asked for it to make it possible to disable parts of the new CPA code.

While I was at it I did some generic cleanup in the area of cpuid disable options and generalized them a bit. That is where the other patches come from.

-Andi

author
#1 lore raw

PATCH 1/7 Add frame work to disable CPUID bits on the command line

There are already various options to disable specific cpuid bits on the command line. They all use their own variable. Add a generic mask to make this easier in the future.

---
 arch/x86/kernel/cpu/common.c |    6 ++++++
 arch/x86/kernel/setup_64.c   |    6 ++++++
 include/asm-x86/cpufeature.h |    4 ++++
 include/asm-x86/processor.h  |    1 +
 4 files changed, 17 insertions(+)
arch/x86/kernel/cpu/common.c +6 -0
Index: linux/arch/x86/kernel/cpu/common.c
===================================================================
@@ -57,6 +57,8 @@ DEFINE_PER_CPU(struct gdt_page, gdt_page
 } };
 EXPORT_PER_CPU_SYMBOL_GPL(gdt_page);
 
+__u32 cleared_cpu_caps[NCAPINTS] __cpuinitdata;
+
 static int cachesize_override __cpuinitdata = -1;
 static int disable_x86_fxsr __cpuinitdata;
 static int disable_x86_serial_nr __cpuinitdata = 1;
@@ -497,6 +499,10 @@ void __cpuinit identify_cpu(struct cpuin
 			boot_cpu_data.x86_capability[i] &= c->x86_capability[i];
 	}
 
+	/* Clear all flags overriden by options */
+	for (i = 0; i < NCAPINTS; i++)
+		c->x86_capability[i] ^= cleared_cpu_caps[i];
+
 	/* Init Machine Check Exception if available. */
 	mcheck_init(c);
 
include/asm-x86/cpufeature.h +4 -0
Index: linux/include/asm-x86/cpufeature.h
===================================================================
@@ -132,6 +132,10 @@
 
 #define set_cpu_cap(c, bit)	set_bit(bit, (unsigned long *)((c)->x86_capability))
 #define clear_cpu_cap(c, bit)	clear_bit(bit, (unsigned long *)((c)->x86_capability))
+#define setup_clear_cpu_cap(bit) do { \
+	clear_cpu_cap(&boot_cpu_data, bit);	\
+	set_bit(bit, cleared_cpu_caps); 	\
+} while (0)
 
 #define cpu_has_fpu		boot_cpu_has(X86_FEATURE_FPU)
 #define cpu_has_vme		boot_cpu_has(X86_FEATURE_VME)
include/asm-x86/processor.h +1 -0
Index: linux/include/asm-x86/processor.h
===================================================================
@@ -118,6 +118,7 @@ struct cpuinfo_x86 {
 extern struct cpuinfo_x86 boot_cpu_data;
 extern struct cpuinfo_x86 new_cpu_data;
 extern struct tss_struct doublefault_tss;
+extern __u32 cleared_cpu_caps[NCAPINTS];
 
 #ifdef CONFIG_SMP
 DECLARE_PER_CPU(struct cpuinfo_x86, cpu_info);
arch/x86/kernel/setup_64.c +6 -0
Index: linux/arch/x86/kernel/setup_64.c
===================================================================
@@ -82,6 +82,8 @@
 struct cpuinfo_x86 boot_cpu_data __read_mostly;
 EXPORT_SYMBOL(boot_cpu_data);
 
+__u32 cleared_cpu_caps[NCAPINTS] __cpuinitdata;
+
 unsigned long mmu_cr4_features;
 
 /* Boot loader ID as an integer, for the benefit of proc_dointvec */
@@ -1012,6 +1014,10 @@ void __cpuinit identify_cpu(struct cpuin
 			boot_cpu_data.x86_capability[i] &= c->x86_capability[i];
 	}
 
+	/* Clear all flags overriden by options */
+	for (i = 0; i < NCAPINTS; i++)
+		c->x86_capability[i] ^= cleared_cpu_caps[i];
+
 #ifdef CONFIG_X86_MCE
 	mcheck_init(c);
 #endif
author
#2 lore raw

PATCH 2/7 Convert some existing cpuid disable options to new generic bitmap

In reply to Andi Kleen

This convers nofxsr, mem=nopentium and nosep to use the new generic cpuid disable bitmap instead of using own variables.

---
 arch/x86/kernel/cpu/common.c |   34 ++++------------------------------
 arch/x86/kernel/setup_32.c   |    5 +----
 2 files changed, 5 insertions(+), 34 deletions(-)
arch/x86/kernel/cpu/common.c +4 -30
Index: linux/arch/x86/kernel/cpu/common.c
===================================================================
@@ -60,14 +60,10 @@ EXPORT_PER_CPU_SYMBOL_GPL(gdt_page);
 __u32 cleared_cpu_caps[NCAPINTS] __cpuinitdata;
 
 static int cachesize_override __cpuinitdata = -1;
-static int disable_x86_fxsr __cpuinitdata;
 static int disable_x86_serial_nr __cpuinitdata = 1;
-static int disable_x86_sep __cpuinitdata;
 
 struct cpu_dev * cpu_devs[X86_VENDOR_NUM] = {};
 
-extern int disable_pse;
-
 static void __cpuinit default_init(struct cpuinfo_x86 * c)
 {
 	/* Not much we can do here... */
@@ -216,16 +212,8 @@ static void __cpuinit get_cpu_vendor(str
 
 static int __init x86_fxsr_setup(char * s)
 {
-	/* Tell all the other CPUs to not use it... */
-	disable_x86_fxsr = 1;
-
-	/*
-	 * ... and clear the bits early in the boot_cpu_data
-	 * so that the bootup process doesn't try to do this
-	 * either.
-	 */
-	clear_bit(X86_FEATURE_FXSR, boot_cpu_data.x86_capability);
-	clear_bit(X86_FEATURE_XMM, boot_cpu_data.x86_capability);
+	setup_clear_cpu_cap(X86_FEATURE_FXSR);
+	setup_clear_cpu_cap(X86_FEATURE_XMM);
 	return 1;
 }
 __setup("nofxsr", x86_fxsr_setup);
@@ -233,7 +221,7 @@ __setup("nofxsr", x86_fxsr_setup);
 
 static int __init x86_sep_setup(char * s)
 {
-	disable_x86_sep = 1;
+	setup_clear_cpu_cap(X86_FEATURE_SEP);
 	return 1;
 }
 __setup("nosep", x86_sep_setup);
@@ -462,19 +450,6 @@ void __cpuinit identify_cpu(struct cpuin
 	if ( tsc_disable )
 		clear_bit(X86_FEATURE_TSC, c->x86_capability);
 
-	/* FXSR disabled? */
-	if (disable_x86_fxsr) {
-		clear_bit(X86_FEATURE_FXSR, c->x86_capability);
-		clear_bit(X86_FEATURE_XMM, c->x86_capability);
-	}
-
-	/* SEP disabled? */
-	if (disable_x86_sep)
-		clear_bit(X86_FEATURE_SEP, c->x86_capability);
-
-	if (disable_pse)
-		clear_bit(X86_FEATURE_PSE, c->x86_capability);
-
 	/* If the model name is still unset, do table lookup. */
 	if ( !c->x86_model_id[0] ) {
 		char *p;
@@ -629,8 +604,7 @@ void __init early_cpu_init(void)
 	/* pse is not compatible with on-the-fly unmapping,
 	 * disable it even if the cpus claim to support it.
 	 */
-	clear_bit(X86_FEATURE_PSE, boot_cpu_data.x86_capability);
-	disable_pse = 1;
+	setup_clear_cpu_cap(X86_FEATURE_PSE);
 #endif
 }
 
arch/x86/kernel/setup_32.c +1 -4
Index: linux/arch/x86/kernel/setup_32.c
===================================================================
@@ -70,8 +70,6 @@
    address, and must not be in the .bss segment! */
 unsigned long init_pg_tables_end __initdata = ~0UL;
 
-int disable_pse __cpuinitdata = 0;
-
 /*
  * Machine setup..
  */
@@ -245,8 +243,7 @@ static int __init parse_mem(char *arg)
 		return -EINVAL;
 
 	if (strcmp(arg, "nopentium") == 0) {
-		clear_cpu_cap(&boot_cpu_data, X86_FEATURE_PSE);
-		disable_pse = 1;
+		setup_clear_cpu_cap(X86_FEATURE_PSE);
 	} else {
 		/* If the user specifies memory size, we
 		 * limit the BIOS-provided memory map to
author
#3 lore raw

PATCH 3/7 Don't disable RDTSC in userland for 32bit notsc

In reply to Andi Kleen

Modern 32bit userland doesn't even boot when the TSC is disabled because ld.so tends to contain RDTSCs. So make notsc only effective for the kernel, similar to 64bit.

---
 arch/x86/kernel/cpu/common.c |    1 -
 1 file changed, 1 deletion(-)
arch/x86/kernel/cpu/common.c +0 -1
Index: linux/arch/x86/kernel/cpu/common.c
===================================================================
@@ -654,7 +654,6 @@ void __cpuinit cpu_init(void)
 		printk(KERN_NOTICE "Disabling TSC...\n");
 		/**** FIX-HPA: DOES THIS REALLY BELONG HERE? ****/
 		clear_bit(X86_FEATURE_TSC, boot_cpu_data.x86_capability);
-		set_in_cr4(X86_CR4_TSD);
 	}
 
 	load_idt(&idt_descr);
author
#4 lore raw

PATCH 4/7 Convert TSC disabling to generic cpuid disable bitmap

In reply to Andi Kleen

This cleans up quite a lot of code.

I think I test compiled all the affected variants (voyager, numaq), but didn't test them, but the change is pretty straight forward for them.

This means NUMAQ didn't compile, but I don't think it was related to my patches.

---
 arch/x86/kernel/cpu/bugs.c    |    2 +-
 arch/x86/kernel/cpu/common.c  |    9 ---------
 arch/x86/kernel/numaq_32.c    |    2 +-
 arch/x86/kernel/tsc_32.c      |   14 +++-----------
 arch/x86/mach-voyager/setup.c |    2 +-
 arch/x86/xen/time.c           |    2 +-
 include/asm-x86/tsc.h         |    2 --
 7 files changed, 7 insertions(+), 26 deletions(-)
arch/x86/kernel/cpu/bugs.c +1 -1
Index: linux/arch/x86/kernel/cpu/bugs.c
===================================================================
@@ -154,7 +154,7 @@ static void __init check_config(void)
  * If we configured ourselves for a TSC, we'd better have one!
  */
 #ifdef CONFIG_X86_TSC
-	if (!cpu_has_tsc && !tsc_disable)
+	if (!cpu_has_tsc)
 		panic("Kernel compiled for Pentium+, requires TSC feature!");
 #endif
 
arch/x86/kernel/cpu/common.c +0 -9
Index: linux/arch/x86/kernel/cpu/common.c
===================================================================
@@ -446,10 +446,6 @@ void __cpuinit identify_cpu(struct cpuin
 	 * we do "generic changes."
 	 */
 
-	/* TSC disabled? */
-	if ( tsc_disable )
-		clear_bit(X86_FEATURE_TSC, c->x86_capability);
-
 	/* If the model name is still unset, do table lookup. */
 	if ( !c->x86_model_id[0] ) {
 		char *p;
@@ -650,11 +646,6 @@ void __cpuinit cpu_init(void)
 
 	if (cpu_has_vme || cpu_has_tsc || cpu_has_de)
 		clear_in_cr4(X86_CR4_VME|X86_CR4_PVI|X86_CR4_TSD|X86_CR4_DE);
-	if (tsc_disable && cpu_has_tsc) {
-		printk(KERN_NOTICE "Disabling TSC...\n");
-		/**** FIX-HPA: DOES THIS REALLY BELONG HERE? ****/
-		clear_bit(X86_FEATURE_TSC, boot_cpu_data.x86_capability);
-	}
 
 	load_idt(&idt_descr);
 	switch_to_new_gdt();
arch/x86/kernel/numaq_32.c +1 -1
Index: linux/arch/x86/kernel/numaq_32.c
===================================================================
@@ -82,7 +82,7 @@ static int __init numaq_tsc_disable(void
 {
 	if (num_online_nodes() > 1) {
 		printk(KERN_DEBUG "NUMAQ: disabling TSC\n");
-		tsc_disable = 1;
+		setup_clear_cpu_cap(X86_FEATURE_TSC);
 	}
 	return 0;
 }
arch/x86/kernel/tsc_32.c +3 -11
Index: linux/arch/x86/kernel/tsc_32.c
===================================================================
@@ -24,8 +24,6 @@ static int tsc_enabled;
 unsigned int tsc_khz;
 EXPORT_SYMBOL_GPL(tsc_khz);
 
-int tsc_disable;
-
 #ifdef CONFIG_X86_TSC
 static int __init tsc_setup(char *str)
 {
@@ -40,8 +38,7 @@ static int __init tsc_setup(char *str)
  */
 static int __init tsc_setup(char *str)
 {
-	tsc_disable = 1;
-
+	setup_clear_cpu_cap(X86_FEATURE_TSC);
 	return 1;
 }
 #endif
@@ -395,7 +392,7 @@ void __init tsc_init(void)
 {
 	int cpu;
 
-	if (!cpu_has_tsc || tsc_disable)
+	if (!cpu_has_tsc)
 		goto out_no_tsc;
 
 	cpu_khz = calculate_cpu_khz();
@@ -439,10 +436,5 @@ void __init tsc_init(void)
 	return;
 
 out_no_tsc:
-	/*
-	 * Set the tsc_disable flag if there's no TSC support, this
-	 * makes it a fast flag for the kernel to see whether it
-	 * should be using the TSC.
-	 */
-	tsc_disable = 1;
+	setup_clear_cpu_cap(X86_FEATURE_TSC);
 }
arch/x86/mach-voyager/setup.c +1 -1
Index: linux/arch/x86/mach-voyager/setup.c
===================================================================
@@ -37,7 +37,7 @@ void __init pre_setup_arch_hook(void)
 {
 	/* Voyagers run their CPUs from independent clocks, so disable
 	 * the TSC code because we can't sync them */
-	tsc_disable = 1;
+	setup_clear_cpu_cap(X86_FEATURE_TSC);
 }
 
 void __init trap_init_hook(void)
arch/x86/xen/time.c +1 -1
Index: linux/arch/x86/xen/time.c
===================================================================
@@ -592,7 +592,7 @@ __init void xen_time_init(void)
 	set_normalized_timespec(&wall_to_monotonic,
 				-xtime.tv_sec, -xtime.tv_nsec);
 
-	tsc_disable = 0;
+	setup_clear_cpu_cap(X86_FEATURE_TSC);
 
 	xen_setup_timer(cpu);
 	xen_setup_cpu_clockevents();
include/asm-x86/tsc.h +0 -2
Index: linux/include/asm-x86/tsc.h
===================================================================
@@ -16,8 +16,6 @@ typedef unsigned long long cycles_t;
 
 extern unsigned int cpu_khz;
 extern unsigned int tsc_khz;
-/* flag for disabling the tsc */
-extern int tsc_disable;
 
 extern void disable_TSC(void);
 
author
#9 lore raw

PATCH 5/7 Remove CONFIG_X86_TSC for 32bit

In reply to Andi Kleen

The config option protects so little code that it is fairly pointless. Also a lot of its code was related to itself only (as in panicing without TSC). And TSC less CPUs are completely handled at runtime anyways.

This makes 32bit behaviour match x86-64.

I also removed an #if defined(CONFIG_X86_GENERIC) in get_cycles. Not sure what that was good for, it didn't make any sense.

---
 arch/x86/Kconfig.cpu          |    4 ----
 arch/x86/kernel/cpu/bugs.c    |    8 --------
 arch/x86/kernel/tsc_32.c      |   10 ----------
 drivers/acpi/processor_idle.c |   10 +++++-----
 include/asm-x86/tsc.h         |    6 ------
 5 files changed, 5 insertions(+), 33 deletions(-)
arch/x86/kernel/cpu/bugs.c +0 -8
Index: linux/arch/x86/kernel/cpu/bugs.c
===================================================================
@@ -151,14 +151,6 @@ static void __init check_config(void)
 #endif
 
 /*
- * If we configured ourselves for a TSC, we'd better have one!
- */
-#ifdef CONFIG_X86_TSC
-	if (!cpu_has_tsc)
-		panic("Kernel compiled for Pentium+, requires TSC feature!");
-#endif
-
-/*
  * If we were told we had a good local APIC, check for buggy Pentia,
  * i.e. all B steppings and the C2 stepping of P54C when using their
  * integrated APIC (see 11AP erratum in "Pentium Processor
arch/x86/kernel/tsc_32.c +0 -10
Index: linux/arch/x86/kernel/tsc_32.c
===================================================================
@@ -24,14 +24,6 @@ static int tsc_enabled;
 unsigned int tsc_khz;
 EXPORT_SYMBOL_GPL(tsc_khz);
 
-#ifdef CONFIG_X86_TSC
-static int __init tsc_setup(char *str)
-{
-	printk(KERN_WARNING "notsc: Kernel compiled with CONFIG_X86_TSC, "
-				"cannot disable TSC.\n");
-	return 1;
-}
-#else
 /*
  * disable flag for tsc. Takes effect by clearing the TSC cpu flag
  * in cpu/common.c
@@ -41,8 +33,6 @@ static int __init tsc_setup(char *str)
 	setup_clear_cpu_cap(X86_FEATURE_TSC);
 	return 1;
 }
-#endif
-
 __setup("notsc", tsc_setup);
 
 /*
drivers/acpi/processor_idle.c +5 -5
Index: linux/drivers/acpi/processor_idle.c
===================================================================
@@ -357,7 +357,7 @@ int acpi_processor_resume(struct acpi_de
 	return 0;
 }
 
-#if defined (CONFIG_GENERIC_TIME) && defined (CONFIG_X86_TSC)
+#if defined (CONFIG_GENERIC_TIME)
 static int tsc_halts_in_c(int state)
 {
 	switch (boot_cpu_data.x86_vendor) {
@@ -534,7 +534,7 @@ static void acpi_processor_idle(void)
 		/* Get end time (ticks) */
 		t2 = inl(acpi_gbl_FADT.xpm_timer_block.address);
 
-#if defined (CONFIG_GENERIC_TIME) && defined (CONFIG_X86_TSC)
+#if defined (CONFIG_GENERIC_TIME)
 		/* TSC halts in C2, so notify users */
 		if (tsc_halts_in_c(ACPI_STATE_C2))
 			mark_tsc_unstable("possible TSC halt in C2");
@@ -599,7 +599,7 @@ static void acpi_processor_idle(void)
 			acpi_set_register(ACPI_BITREG_ARB_DISABLE, 0);
 		}
 
-#if defined (CONFIG_GENERIC_TIME) && defined (CONFIG_X86_TSC)
+#if defined (CONFIG_GENERIC_TIME)
 		/* TSC halts in C3, so notify users */
 		if (tsc_halts_in_c(ACPI_STATE_C3))
 			mark_tsc_unstable("TSC halts in C3");
@@ -1465,7 +1465,7 @@ static int acpi_idle_enter_simple(struct
 	acpi_idle_do_entry(cx);
 	t2 = inl(acpi_gbl_FADT.xpm_timer_block.address);
 
-#if defined (CONFIG_GENERIC_TIME) && defined (CONFIG_X86_TSC)
+#if defined (CONFIG_GENERIC_TIME)
 	/* TSC could halt in idle, so notify users */
 	if (tsc_halts_in_c(cx->type))
 		mark_tsc_unstable("TSC halts in idle");;
@@ -1577,7 +1577,7 @@ static int acpi_idle_enter_bm(struct cpu
 		spin_unlock(&c3_lock);
 	}
 
-#if defined (CONFIG_GENERIC_TIME) && defined (CONFIG_X86_TSC)
+#if defined (CONFIG_GENERIC_TIME)
 	/* TSC could halt in idle, so notify users */
 	if (tsc_halts_in_c(ACPI_STATE_C3))
 		mark_tsc_unstable("TSC halts in idle");
include/asm-x86/tsc.h +0 -6
Index: linux/include/asm-x86/tsc.h
===================================================================
@@ -23,14 +23,10 @@ static inline cycles_t get_cycles(void)
 {
 	unsigned long long ret = 0;
 
-#ifndef CONFIG_X86_TSC
 	if (!cpu_has_tsc)
 		return 0;
-#endif
 
-#if defined(CONFIG_X86_GENERIC) || defined(CONFIG_X86_TSC)
 	rdtscll(ret);
-#endif
 	return ret;
 }
 
@@ -40,10 +36,8 @@ static inline cycles_t vget_cycles(void)
 	 * We only do VDSOs on TSC capable CPUs, so this shouldnt
 	 * access boot_cpu_data (which is not VDSO-safe):
 	 */
-#ifndef CONFIG_X86_TSC
 	if (!cpu_has_tsc)
 		return 0;
-#endif
 	return (cycles_t) __native_read_tsc();
 }
 
arch/x86/Kconfig.cpu +0 -4
Index: linux/arch/x86/Kconfig.cpu
===================================================================
@@ -377,10 +377,6 @@ config X86_OOSTORE
 	def_bool y
 	depends on (MWINCHIP3D || MWINCHIP2 || MWINCHIPC6) && MTRR
 
-config X86_TSC
-	def_bool y
-	depends on ((MWINCHIP3D || MWINCHIP2 || MCRUSOE || MEFFICEON || MCYRIXIII || MK7 || MK6 || MPENTIUM4 || MPENTIUMM || MPENTIUMIII || MPENTIUMII || M686 || M586MMX || M586TSC || MK8 || MVIAC3_2 || MVIAC7 || MGEODEGX1 || MGEODE_LX || MCORE2) && !X86_NUMAQ) || X86_64
-
 # this should be set for all -march=.. options where the compiler
 # generates cmov.
 config X86_CMOV
author
#12 lore raw

PATCH 6/7 Add noclflush option

In reply to Andi Kleen

To disable CLFLUSH usage, especially in change_page_attr().

Ingo asked for this.

---
 Documentation/kernel-parameters.txt |    2 ++
 arch/x86/kernel/cpu/common.c        |    7 +++++++
 arch/x86/kernel/setup_64.c          |    7 +++++++
 3 files changed, 16 insertions(+)
arch/x86/kernel/cpu/common.c +7 -0
Index: linux/arch/x86/kernel/cpu/common.c
===================================================================
@@ -542,6 +542,13 @@ void __cpuinit detect_ht(struct cpuinfo_
 }
 #endif
 
+static __init int setup_noclflush(char *arg)
+{
+	setup_clear_cpu_cap(X86_FEATURE_CLFLSH);
+	return 1;
+}
+__setup("noclflush", setup_noclflush);
+
 void __cpuinit print_cpu_info(struct cpuinfo_x86 *c)
 {
 	char *vendor = NULL;
arch/x86/kernel/setup_64.c +7 -0
Index: linux/arch/x86/kernel/setup_64.c
===================================================================
@@ -1042,6 +1042,13 @@ void __cpuinit identify_cpu(struct cpuin
 	}
 }
 
+static __init int setup_noclflush(char *arg)
+{
+	setup_clear_cpu_cap(X86_FEATURE_CLFLSH);
+	return 1;
+}
+__setup("noclflush", setup_noclflush);
+
 void __cpuinit print_cpu_info(struct cpuinfo_x86 *c)
 {
 	if (c->x86_model_id[0])
Documentation/kernel-parameters.txt +2 -0
Index: linux/Documentation/kernel-parameters.txt
===================================================================
@@ -1173,6 +1173,8 @@ and is between 256 and 4096 characters. 
 			register save and restore. The kernel will only save
 			legacy floating-point registers on task switch.
 
+	noclflush	[BUGS=X86] Don't use the CLFLUSH instruction
+
 	nohlt		[BUGS=ARM]
 
 	no-hlt		[BUGS=X86-32] Tells the kernel that the hlt
author
#13 lore raw

PATCH 7/7 Add generic clearcpuid=... option

In reply to Andi Kleen

Add a generic option to clear any cpuid bit. I added it because it was very easy to add with the new generic cpuid disable bitmap and perhaps it will be useful in the future.

---
 Documentation/kernel-parameters.txt |   13 +++++++++++++
 arch/x86/kernel/cpu/common.c        |   11 +++++++++++
 arch/x86/kernel/setup_64.c          |   11 +++++++++++
 3 files changed, 35 insertions(+)
arch/x86/kernel/cpu/common.c +11 -0
Index: linux/arch/x86/kernel/cpu/common.c
===================================================================
@@ -572,6 +572,17 @@ void __cpuinit print_cpu_info(struct cpu
 		printk("\n");
 }
 
+static __init int setup_disablecpuid(char *arg)
+{
+	int bit;
+	if (get_option(&arg, &bit) && bit < NCAPINTS*32)
+		setup_clear_cpu_cap(bit);
+	else
+		return 0;
+	return 1;
+}
+__setup("clearcpuid=", setup_disablecpuid);
+
 cpumask_t cpu_initialized __cpuinitdata = CPU_MASK_NONE;
 
 /* This is hacky. :)
arch/x86/kernel/setup_64.c +11 -0
Index: linux/arch/x86/kernel/setup_64.c
===================================================================
@@ -1060,6 +1060,17 @@ void __cpuinit print_cpu_info(struct cpu
 		printk(KERN_CONT "\n");
 }
 
+static __init int setup_disablecpuid(char *arg)
+{
+	int bit;
+	if (get_option(&arg, &bit) && bit < NCAPINTS*32)
+		setup_clear_cpu_cap(bit);
+	else
+		return 0;
+	return 1;
+}
+__setup("clearcpuid=", setup_disablecpuid);
+
 /*
  *	Get CPU information for use by the procfs.
  */
Documentation/kernel-parameters.txt +13 -0
Index: linux/Documentation/kernel-parameters.txt
===================================================================
@@ -408,6 +408,19 @@ and is between 256 and 4096 characters. 
 			[SPARC64] tick
 			[X86-64] hpet,tsc
 
+	clearcpuid=BITNUM [X86]
+			Disable CPUID feature X for the kernel. See
+			include/asm-x86/cpufeature.h for the valid bit numbers.
+			Note the Linux specific bits are not necessarily
+			stable over kernel options, but the vendor specific
+			ones should be.
+			Also note that user programs calling CPUID directly
+			or using the feature without checking anything
+			will still see it. This just prevents it from
+			being used by the kernel or shown in /proc/cpuinfo.
+			Also note the kernel might malfunction if you disable
+			some critical bits.
+
 	code_bytes	[IA32/X86_64] How many bytes of object code to print
 			in an oops report.
 			Range: 0 - 8192
#10 lore raw

Re: [PATCH] [5/7] Remove CONFIG_X86_TSC for 32bit II

In reply to Andi Kleen
Quoted 8 lines · Andi Kleen

The config option protects so little code that it is fairly pointless. Also a lot of its code was related to itself only (as in panicing without TSC). And TSC less CPUs are completely handled at runtime anyways.

This makes 32bit behaviour match x86-64.

I also removed an #if defined(CONFIG_X86_GENERIC) in get_cycles. Not sure what that was good for, it didn't make any sense.

Hmm, during further testing i found that the patch causes boot failures in some configuration. I withdraw this one for now.

The other patches in the series are independent and should be still considered for merging.

-Andi

#11 lore raw

Re: [PATCH] [5/7] Remove CONFIG_X86_TSC for 32bit II

Quoted 6 lines · Andi Kleen
Quoted 4 lines · Andi Kleen

The config option protects so little code that it is fairly pointless. Also a lot of its code was related to itself only (as in panicing without TSC). And TSC less CPUs are completely handled at runtime anyways.

Quoted 7 lines
Quoted 4 lines

This makes 32bit behaviour match x86-64.

I also removed an #if defined(CONFIG_X86_GENERIC) in get_cycles. Not sure what that was good for, it didn't make any sense.

Hmm, during further testing i found that the patch causes boot failures in some configuration. I withdraw this one for now.

ok, pulled. In what situations did it fail? It worked fine on a (small) number of systems here.

Quoted 2 lines

The other patches in the series are independent and should be still considered for merging.

yep, they are in and have caused no problems so far.

Ingo

#5 lore raw

Re: [PATCH] [4/7] Convert TSC disabling to generic cpuid disable bitmap

In reply to Andi Kleen

arch/x86/xen/time.c __init void xen_time_init(void) · line 598

Quoted 13 lines · Andi Kleen

Index: linux/arch/x86/xen/time.c
===================================================================

arch/x86/xen/time.c +1 -1
@@ -592,7 +592,7 @@ __init void xen_time_init(void)
        set_normalized_timespec(&wall_to_monotonic,
                                -xtime.tv_sec, -xtime.tv_nsec);
 
-       tsc_disable = 0;
+       setup_clear_cpu_cap(X86_FEATURE_TSC);
 
        xen_setup_timer(cpu);
        xen_setup_cpu_clockevents();

That inverts the meaning, doesn't it? Previously the code force the TSC to be on and now it forcefully disables it. Now when booting a Xen guest I get:

        Kernel panic - not syncing: Kernel compiled for Pentium+, requires TSC feature!

I think you want something like the following
---

Quoted 1 line

From eab0f03f2659670673496b48c03c0f79151e7bbf Mon Sep 17 00:00:00 2001

From: Ian Campbell <[email protected]>
Date: Sat, 19 Jan 2008 18:08:23 +0000
Subject: [PATCH] x86: Xen requires TSC support to be forced on, not disabled.

---
 arch/x86/xen/time.c          |    2 +-
 include/asm-x86/cpufeature.h |    4 ++++
 2 files changed, 5 insertions(+), 1 deletions(-)
arch/x86/xen/time.c +1 -1
index 6f5c74a..b3721fd 100644
@@ -592,7 +592,7 @@ __init void xen_time_init(void)
 	set_normalized_timespec(&wall_to_monotonic,
 				-xtime.tv_sec, -xtime.tv_nsec);
 
-	setup_clear_cpu_cap(X86_FEATURE_TSC);
+	setup_force_cpu_cap(X86_FEATURE_TSC);
 
 	xen_setup_timer(cpu);
 	xen_setup_cpu_clockevents();
include/asm-x86/cpufeature.h +4 -0
index 91a7665..b6f969c 100644
@@ -135,6 +135,10 @@
 	clear_cpu_cap(&boot_cpu_data, bit);	\
 	set_bit(bit, cleared_cpu_caps); 	\
 } while (0)
+#define setup_force_cpu_cap(bit) do { \
+	set_cpu_cap(&boot_cpu_data, bit);	\
+	clear_bit(bit, cleared_cpu_caps); 	\
+} while (0)
 
 #define cpu_has_fpu		boot_cpu_has(X86_FEATURE_FPU)
 #define cpu_has_vme		boot_cpu_has(X86_FEATURE_VME)
Signature · 7 lines
1.5.3.8


-- 
Ian Campbell

All that glitters is not gold; all that wander are not lost.
#8 lore raw

Re: [PATCH] [4/7] Convert TSC disabling to generic cpuid disable bitmap

In reply to Andi Kleen

Should have added Jeremy to CC, sorry for the duplicate message.

arch/x86/xen/time.c __init void xen_time_init(void) · line 598

Quoted 13 lines · Andi Kleen

Index: linux/arch/x86/xen/time.c
===================================================================

arch/x86/xen/time.c +1 -1
@@ -592,7 +592,7 @@ __init void xen_time_init(void)
        set_normalized_timespec(&wall_to_monotonic,
                                -xtime.tv_sec, -xtime.tv_nsec);
 
-       tsc_disable = 0;
+       setup_clear_cpu_cap(X86_FEATURE_TSC);
 
        xen_setup_timer(cpu);
        xen_setup_cpu_clockevents();

That inverts the meaning, doesn't it? Previously the code force the TSC to be on and now it forcefully disables it. Now when booting a Xen guest I get:

        Kernel panic - not syncing: Kernel compiled for Pentium+, requires TSC feature!

I think you want something like the following
---

Quoted 1 line

From eab0f03f2659670673496b48c03c0f79151e7bbf Mon Sep 17 00:00:00 2001

From: Ian Campbell <[email protected]>
Date: Sat, 19 Jan 2008 18:08:23 +0000
Subject: [PATCH] x86: Xen requires TSC support to be forced on, not disabled.

---
 arch/x86/xen/time.c          |    2 +-
 include/asm-x86/cpufeature.h |    4 ++++
 2 files changed, 5 insertions(+), 1 deletions(-)
arch/x86/xen/time.c +1 -1
index 6f5c74a..b3721fd 100644
@@ -592,7 +592,7 @@ __init void xen_time_init(void)
 	set_normalized_timespec(&wall_to_monotonic,
 				-xtime.tv_sec, -xtime.tv_nsec);
 
-	setup_clear_cpu_cap(X86_FEATURE_TSC);
+	setup_force_cpu_cap(X86_FEATURE_TSC);
 
 	xen_setup_timer(cpu);
 	xen_setup_cpu_clockevents();
include/asm-x86/cpufeature.h +4 -0
index 91a7665..b6f969c 100644
@@ -135,6 +135,10 @@
 	clear_cpu_cap(&boot_cpu_data, bit);	\
 	set_bit(bit, cleared_cpu_caps); 	\
 } while (0)
+#define setup_force_cpu_cap(bit) do { \
+	set_cpu_cap(&boot_cpu_data, bit);	\
+	clear_bit(bit, cleared_cpu_caps); 	\
+} while (0)
 
 #define cpu_has_fpu		boot_cpu_has(X86_FEATURE_FPU)
 #define cpu_has_vme		boot_cpu_has(X86_FEATURE_VME)
Signature · 7 lines
1.5.3.8


-- 
Ian Campbell

All that glitters is not gold; all that wander are not lost.
author
#6 lore raw

Re: [PATCH] [4/7] Convert TSC disabling to generic cpuid disable bitmap

In reply to Ian Campbell
Quoted 12 lines · On Saturday 19 January 2008 19:15:48 Ian Campbell wrote:
Quoted 11 lines · Andi Kleen

Index: linux/arch/x86/xen/time.c
===================================================================

arch/x86/xen/time.c +1 -1
@@ -592,7 +592,7 @@ __init void xen_time_init(void)
        set_normalized_timespec(&wall_to_monotonic,
                                -xtime.tv_sec, -xtime.tv_nsec);
 
-       tsc_disable = 0;
+       setup_clear_cpu_cap(X86_FEATURE_TSC);
 
Quoted 7 lines

arch/x86/xen/time.c __init void xen_time_init(void) · line 598

Quoted 2 lines
arch/x86/xen/time.c +0 -0
        xen_setup_timer(cpu);
        xen_setup_cpu_clockevents();

That inverts the meaning, doesn't it? Previously the code force the TSC to be on and now it forcefully disables it. Now when booting a Xen guest I get:

        Kernel panic - not syncing: Kernel compiled for Pentium+, requires TSC feature!

You're right -- i was a bit overzealous in search'n'replace with that one. It's the only place who force enables TSC.

arch/x86/xen/time.c __init void xen_time_init(void) · line 595

Quoted 10 lines
arch/x86/xen/time.c +1 -1
index 6f5c74a..b3721fd 100644
@@ -592,7 +592,7 @@ __init void xen_time_init(void)
 	set_normalized_timespec(&wall_to_monotonic,
 				-xtime.tv_sec, -xtime.tv_nsec);
 
-	setup_clear_cpu_cap(X86_FEATURE_TSC);
+	setup_force_cpu_cap(X86_FEATURE_TSC);

Actually that would be only needed if someone else disabled TSC explicitely before.

Simply deleting this should be sufficient. Does this patch work for you?

It would break if someone passes notsc to a Xen kernel, but then a lot of options make the kernel break if you don't know what you're doing so that doesn't seem like a big issue.

-Andi

---

Don't disable TSC in Xen boot

That was a typo in the previous TSC option changes.

TSC shouldn't be disabled at this point, so just don't do anything.

arch/x86/xen/time.c +0 -2
Index: linux/arch/x86/xen/time.c
===================================================================
@@ -592,8 +592,6 @@ __init void xen_time_init(void)
 	set_normalized_timespec(&wall_to_monotonic,
 				-xtime.tv_sec, -xtime.tv_nsec);
 
-	setup_clear_cpu_cap(X86_FEATURE_TSC);
-
 	xen_setup_timer(cpu);
 	xen_setup_cpu_clockevents();
 }
#7 lore raw

Re: [PATCH] [4/7] Convert TSC disabling to generic cpuid disable bitmap

Quoted 9 lines · Andi Kleen
Quoted 8 lines · On Saturday 19 January 2008 19:15:48 Ian Campbell wrote:
arch/x86/xen/time.c +0 -0
index 6f5c74a..b3721fd 100644
@@ -592,7 +592,7 @@ __init void xen_time_init(void)
 	set_normalized_timespec(&wall_to_monotonic,
 				-xtime.tv_sec, -xtime.tv_nsec);
 
Quoted 6 lines

arch/x86/xen/time.c __init void xen_time_init(void) · line 595

Quoted 2 lines
arch/x86/xen/time.c +1 -1
-	setup_clear_cpu_cap(X86_FEATURE_TSC);
+	setup_force_cpu_cap(X86_FEATURE_TSC);

Actually that would be only needed if someone else disabled TSC explicitely before.

Simply deleting this should be sufficient. Does this patch work for you?

Yes.

Quoted 2 lines

It would break if someone passes notsc to a Xen kernel, but then a lot of options make the kernel break if you don't know what you're doing so that doesn't seem like a big issue.

I'm happy either way. I think the Xen paravirt subsystem will use TSC where it has to due to the Xen architecture regardless of this setting.

Ian.

Signature · 3 lines
Ian Campbell

Good day for overcoming obstacles.  Try a steeplechase.