2006-11-30 04:26:44

by Ben Collins

[permalink] [raw]
Subject: Ubuntu patch sync for 2.6.20

This is a set of patches from the Ubuntu tree that seemed suitable for
upstream sync.

[PATCH 1/4] [x86] Add command line option to enable/disable hyper-threading.

[PATCH 2/4] [APIC] Allow disabling of UP APIC/IO-APIC by default, with command line option to turn it on.

[PATCH 3/4] [ATM] Add CPPFLAGS to byteorder.h check.

[PATCH 4/4] [HVCS] Select HVC_CONSOLE if HVCS is enabled.


arch/i386/Kconfig | 13 +++++++++++++
Documentation/kernel-parameters.txt | 3 +++
arch/i386/Kconfig | 5 +++++
arch/i386/kernel/apic.c | 13 +++++++++++--
arch/i386/kernel/cpu/common.c | 30 +++++++++++++++++++++++++++++-
arch/i386/kernel/io_apic.c | 10 +++++++++-
drivers/atm/Makefile | 3 +--
drivers/char/Kconfig | 2 +-
include/asm-i386/apic.h | 6 ++++++
include/asm-i386/io_apic.h | 6 +++++-
10 files changed, 83 insertions(+), 8 deletions(-)


2006-11-30 04:26:20

by Ben Collins

[permalink] [raw]
Subject: Ubuntu patch sync for 2.6.20

This is a set of patches from the Ubuntu tree that seemed suitable for
upstream sync.

[PATCH 1/4] [x86] Add command line option to enable/disable hyper-threading.

[PATCH 2/4] [APIC] Allow disabling of UP APIC/IO-APIC by default, with command line option to turn it on.

[PATCH 3/4] [ATM] Add CPPFLAGS to byteorder.h check.

[PATCH 4/4] [HVCS] Select HVC_CONSOLE if HVCS is enabled.


arch/i386/Kconfig | 13 +++++++++++++
Documentation/kernel-parameters.txt | 3 +++
arch/i386/Kconfig | 5 +++++
arch/i386/kernel/apic.c | 13 +++++++++++--
arch/i386/kernel/cpu/common.c | 30 +++++++++++++++++++++++++++++-
arch/i386/kernel/io_apic.c | 10 +++++++++-
drivers/atm/Makefile | 3 +--
drivers/char/Kconfig | 2 +-
include/asm-i386/apic.h | 6 ++++++
include/asm-i386/io_apic.h | 6 +++++-
10 files changed, 83 insertions(+), 8 deletions(-)

2006-11-30 04:26:19

by Ben Collins

[permalink] [raw]
Subject: Ubuntu patch sync for 2.6.20

This is a set of patches from the Ubuntu tree that seemed suitable for
upstream sync.

[PATCH 1/4] [x86] Add command line option to enable/disable hyper-threading.

[PATCH 2/4] [APIC] Allow disabling of UP APIC/IO-APIC by default, with command line option to turn it on.

[PATCH 3/4] [ATM] Add CPPFLAGS to byteorder.h check.

[PATCH 4/4] [HVCS] Select HVC_CONSOLE if HVCS is enabled.


arch/i386/Kconfig | 13 +++++++++++++
Documentation/kernel-parameters.txt | 3 +++
arch/i386/Kconfig | 5 +++++
arch/i386/kernel/apic.c | 13 +++++++++++--
arch/i386/kernel/cpu/common.c | 30 +++++++++++++++++++++++++++++-
arch/i386/kernel/io_apic.c | 10 +++++++++-
drivers/atm/Makefile | 3 +--
drivers/char/Kconfig | 2 +-
include/asm-i386/apic.h | 6 ++++++
include/asm-i386/io_apic.h | 6 +++++-
10 files changed, 83 insertions(+), 8 deletions(-)

2006-11-30 04:26:43

by Ben Collins

[permalink] [raw]
Subject: [PATCH 3/4] [ATM] Add CPPFLAGS to byteorder.h check.

O= builds produced errors in the shell command because of unfound headers.

Signed-off-by: Ben Collins <[email protected]>
---
drivers/atm/Makefile | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/atm/Makefile b/drivers/atm/Makefile
index b5077ce..1b16f81 100644
--- a/drivers/atm/Makefile
+++ b/drivers/atm/Makefile
@@ -41,7 +41,7 @@ ifeq ($(CONFIG_ATM_FORE200E_PCA),y)
# guess the target endianess to choose the right PCA-200E firmware image
ifeq ($(CONFIG_ATM_FORE200E_PCA_DEFAULT_FW),y)
byteorder.h := include$(if $(patsubst $(srctree),,$(objtree)),2)/asm/byteorder.h
- CONFIG_ATM_FORE200E_PCA_FW := $(obj)/pca200e$(if $(shell $(CC) -E -dM $(byteorder.h) | grep ' __LITTLE_ENDIAN '),.bin,_ecd.bin2)
+ CONFIG_ATM_FORE200E_PCA_FW := $(obj)/pca200e$(if $(shell $(CC) $(CPPFLAGS) -E -dM $(byteorder.h) | grep ' __LITTLE_ENDIAN '),.bin,_ecd.bin2)
endif
endif

--
1.4.1

2006-11-30 04:26:44

by Ben Collins

[permalink] [raw]
Subject: [PATCH 1/4] [x86] Add command line option to enable/disable hyper-threading.

This patch adds a config option to allow disabling hyper-threading by
default, and a kernel command line option to changes this default at
boot time.

Signed-off-by: Ben Collins <[email protected]>
---
Documentation/kernel-parameters.txt | 3 +++
arch/i386/Kconfig | 5 +++++
arch/i386/kernel/cpu/common.c | 29 +++++++++++++++++++++++++++++
3 files changed, 37 insertions(+), 0 deletions(-)

diff --git a/Documentation/kernel-parameters.txt b/Documentation/kernel-parameters.txt
index 6747384..2b68d6e 100644
--- a/Documentation/kernel-parameters.txt
+++ b/Documentation/kernel-parameters.txt
@@ -600,6 +600,9 @@ and is between 256 and 4096 characters.
hisax= [HW,ISDN]
See Documentation/isdn/README.HiSax.

+ ht= [HW,IA-32,SMP] Enable or disable hyper-threading.
+ Format: <on|off>
+
hugepages= [HW,IA-32,IA-64] Maximal number of HugeTLB pages.

noirqbalance [IA-32,SMP,KNL] Disable kernel irq balancing
diff --git a/arch/i386/Kconfig b/arch/i386/Kconfig
index 8ff1c6f..b4a2461 100644
--- a/arch/i386/Kconfig
+++ b/arch/i386/Kconfig
@@ -1185,6 +1185,11 @@ config X86_HT
depends on SMP && !(X86_VISWS || X86_VOYAGER)
default y

+config X86_HT_DISABLE
+ bool "Disable Hyper-Threading by default"
+ depends on X86_HT
+ default n
+
config X86_BIOS_REBOOT
bool
depends on !(X86_VISWS || X86_VOYAGER)
diff --git a/arch/i386/kernel/cpu/common.c b/arch/i386/kernel/cpu/common.c
index d9f3e3c..42d2361 100644
--- a/arch/i386/kernel/cpu/common.c
+++ b/arch/i386/kernel/cpu/common.c
@@ -482,6 +482,29 @@ void __cpuinit identify_cpu(struct cpuin
}

#ifdef CONFIG_X86_HT
+
+#ifdef CONFIG_X86_HT_DISABLE
+static int disable_ht __cpuinitdata = 1;
+#else
+static int disable_ht __cpuinitdata;
+#endif
+
+static int __init parse_ht(char *arg)
+{
+ if (!arg)
+ return -EINVAL;
+
+ if (!memcmp(arg, "on", 2))
+ disable_ht = 0;
+ else if (!memcmp(arg, "off", 3))
+ disable_ht = 1;
+ else
+ return -EINVAL;
+
+ return 0;
+}
+early_param("ht", parse_ht);
+
void __cpuinit detect_ht(struct cpuinfo_x86 *c)
{
u32 eax, ebx, ecx, edx;
@@ -492,6 +515,12 @@ void __cpuinit detect_ht(struct cpuinfo_
if (!cpu_has(c, X86_FEATURE_HT) || cpu_has(c, X86_FEATURE_CMP_LEGACY))
return;

+ if (disable_ht) {
+ printk(KERN_INFO "CPU: Hyper-Threading disabled by default. Enable with ht=on\n");
+ smp_num_siblings = 1;
+ return;
+ }
+
smp_num_siblings = (ebx & 0xff0000) >> 16;

if (smp_num_siblings == 1) {
--
1.4.1

2006-11-30 04:26:44

by Ben Collins

[permalink] [raw]
Subject: [PATCH 4/4] [HVCS] Select HVC_CONSOLE if HVCS is enabled.

If HVC_CONSOLE provides symbols that HVCS requires.

Signed-off-by: Ben Collins <[email protected]>
---
drivers/char/Kconfig | 1 +
1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/drivers/char/Kconfig b/drivers/char/Kconfig
index 2af12fc..c94ecdc 100644
--- a/drivers/char/Kconfig
+++ b/drivers/char/Kconfig
@@ -598,6 +598,7 @@ config HVC_RTAS
config HVCS
tristate "IBM Hypervisor Virtual Console Server support"
depends on PPC_PSERIES
+ select HVC_CONSOLE
help
Partitionable IBM Power5 ppc64 machines allow hosting of
firmware virtual consoles from one Linux partition by
--
1.4.1

2006-11-30 04:26:45

by Ben Collins

[permalink] [raw]
Subject: [PATCH 2/4] [APIC] Allow disabling of UP APIC/IO-APIC by default, with command line option to turn it on.

Signed-off-by: Ben Collins <[email protected]>
---
arch/i386/Kconfig | 13 +++++++++++++
arch/i386/kernel/apic.c | 13 +++++++++++--
arch/i386/kernel/io_apic.c | 10 +++++++++-
include/asm-i386/apic.h | 6 ++++++
include/asm-i386/io_apic.h | 5 +++++
5 files changed, 44 insertions(+), 3 deletions(-)

diff --git a/arch/i386/Kconfig b/arch/i386/Kconfig
index b4a2461..ef2f2db 100644
--- a/arch/i386/Kconfig
+++ b/arch/i386/Kconfig
@@ -285,6 +285,19 @@ config X86_UP_IOAPIC
to use it. If you say Y here even though your machine doesn't have
an IO-APIC, then the kernel will still run with no slowdown at all.

+config X86_UP_APIC_DEFAULT_OFF
+ bool "APIC support on uniprocessors defaults to off"
+ depends on X86_UP_APIC
+ default n
+ help
+ Some older systems have flaky APICs. Say Y to turn off APIC
+ support by default, while still allowing it to be enabled by the
+ "lapic" and "apic" command line options.
+
+ Usually this is only necessary for distro installer kernels that
+ must work with everything. Everyone else can safely say N here
+ and configure APIC support in or out as needed.
+
config X86_LOCAL_APIC
bool
depends on X86_UP_APIC || ((X86_VISWS || SMP) && !X86_VOYAGER) || X86_GENERICARCH
diff --git a/arch/i386/kernel/apic.c b/arch/i386/kernel/apic.c
index 2fd4b7d..2f2eb83 100644
--- a/arch/i386/kernel/apic.c
+++ b/arch/i386/kernel/apic.c
@@ -51,8 +51,9 @@ static cpumask_t timer_bcast_ipi;

/*
* Knob to control our willingness to enable the local APIC.
+ * -2=default-disable, -1=force-disable, 1=force-enable, 0=automatic
*/
-static int enable_local_apic __initdata = 0; /* -1=force-disable, +1=force-enable */
+static int enable_local_apic __initdata = (X86_APIC_DEFAULT_OFF ? -2 : 0);

static inline void lapic_disable(void)
{
@@ -801,7 +802,7 @@ static int __init detect_init_APIC (void
* APIC only if "lapic" specified.
*/
if (enable_local_apic <= 0) {
- printk("Local APIC disabled by BIOS -- "
+ printk("Local APIC disabled by BIOS (or by default) -- "
"you can enable it with \"lapic\"\n");
return -1;
}
@@ -1350,6 +1351,14 @@ int __init APIC_init_uniprocessor (void)
if (!smp_found_config && !cpu_has_apic)
return -1;

+ /* If local apic is off due to config_x86_apic_off option, jump
+ * out here. */
+ if (enable_local_apic < -1) {
+ printk(KERN_INFO "Local APIC disabled by default -- "
+ "use 'lapic' to enable it.\n");
+ return -1;
+ }
+
/*
* Complain if the BIOS pretends there is one.
*/
diff --git a/arch/i386/kernel/io_apic.c b/arch/i386/kernel/io_apic.c
index 3b7a63e..0122dba 100644
--- a/arch/i386/kernel/io_apic.c
+++ b/arch/i386/kernel/io_apic.c
@@ -767,7 +767,7 @@ #endif /* !CONFIG_SMP */
#define MAX_PIRQS 8
static int pirq_entries [MAX_PIRQS];
static int pirqs_enabled;
-int skip_ioapic_setup;
+int skip_ioapic_setup = X86_APIC_DEFAULT_OFF;

static int __init ioapic_setup(char *str)
{
@@ -2887,3 +2887,11 @@ static int __init parse_noapic(char *arg
return 0;
}
early_param("noapic", parse_noapic);
+
+static int __init parse_apic(char *arg)
+{
+ /* enable IO-APIC */
+ enable_ioapic_setup();
+ return 0;
+}
+early_param("apic", parse_apic);
diff --git a/include/asm-i386/apic.h b/include/asm-i386/apic.h
index b952957..a06ca3f 100644
--- a/include/asm-i386/apic.h
+++ b/include/asm-i386/apic.h
@@ -71,6 +71,12 @@ # define apic_read_around(x) apic_read(x
# define apic_write_around(x,y) apic_write_atomic((x),(y))
#endif

+#ifdef CONFIG_X86_UP_APIC_DEFAULT_OFF
+# define X86_APIC_DEFAULT_OFF 1
+#else
+# define X86_APIC_DEFAULT_OFF 0
+#endif
+
static inline void ack_APIC_irq(void)
{
/*
diff --git a/include/asm-i386/io_apic.h b/include/asm-i386/io_apic.h
index 059a9ff..ddedeec 100644
--- a/include/asm-i386/io_apic.h
+++ b/include/asm-i386/io_apic.h
@@ -126,6 +126,11 @@ static inline void disable_ioapic_setup(
skip_ioapic_setup = 1;
}

+static inline void enable_ioapic_setup(void)
+{
+ skip_ioapic_setup = 0;
+}
+
static inline int ioapic_setup_disabled(void)
{
return skip_ioapic_setup;
--
1.4.1

2006-11-30 10:59:31

by Alan

[permalink] [raw]
Subject: Re: [PATCH 1/4] [x86] Add command line option to enable/disable hyper-threading.

On Wed, 29 Nov 2006 23:26:05 -0500
Ben Collins <[email protected]> wrote:

> This patch adds a config option to allow disabling hyper-threading by
> default, and a kernel command line option to changes this default at
> boot time.
>
> Signed-off-by: Ben Collins <[email protected]>

The description is wrong - this does not disable hyperthreading it merely
leaves one thread idle. I don't believe Intel have ever published a
procedure for truely disabling HT, but if you idle a thread you may want
to adjust the cache settings on a PIV (10.5.6 in the intel docs) and set
it to shared mode. Need to play more with what the bios does I guess.

So Ack but with the proviso it should say "Ignoring" or "Not using" not
"Disabling", because it does not do the latter and there seem to be
performance differences as a result

Acked-by: Alan Cox <[email protected]>

Alan

2006-11-30 12:32:50

by Roman Zippel

[permalink] [raw]
Subject: Re: [PATCH 4/4] [HVCS] Select HVC_CONSOLE if HVCS is enabled.

Hi,

On Wed, 29 Nov 2006, Ben Collins wrote:

> If HVC_CONSOLE provides symbols that HVCS requires.
>
> Signed-off-by: Ben Collins <[email protected]>
> ---
> drivers/char/Kconfig | 1 +
> 1 files changed, 1 insertions(+), 0 deletions(-)
>
> diff --git a/drivers/char/Kconfig b/drivers/char/Kconfig
> index 2af12fc..c94ecdc 100644
> --- a/drivers/char/Kconfig
> +++ b/drivers/char/Kconfig
> @@ -598,6 +598,7 @@ config HVC_RTAS
> config HVCS
> tristate "IBM Hypervisor Virtual Console Server support"
> depends on PPC_PSERIES
> + select HVC_CONSOLE
> help
> Partitionable IBM Power5 ppc64 machines allow hosting of
> firmware virtual consoles from one Linux partition by


Why not a normal dependency?

bye, Roman

2006-11-30 15:04:49

by Ben Collins

[permalink] [raw]
Subject: Re: [PATCH 4/4] [HVCS] Select HVC_CONSOLE if HVCS is enabled.

On Thu, 2006-11-30 at 13:32 +0100, Roman Zippel wrote:
> Hi,
>
> On Wed, 29 Nov 2006, Ben Collins wrote:
>
> > If HVC_CONSOLE provides symbols that HVCS requires.
> >
> > Signed-off-by: Ben Collins <[email protected]>
> > ---
> > drivers/char/Kconfig | 1 +
> > 1 files changed, 1 insertions(+), 0 deletions(-)
> >
> > diff --git a/drivers/char/Kconfig b/drivers/char/Kconfig
> > index 2af12fc..c94ecdc 100644
> > --- a/drivers/char/Kconfig
> > +++ b/drivers/char/Kconfig
> > @@ -598,6 +598,7 @@ config HVC_RTAS
> > config HVCS
> > tristate "IBM Hypervisor Virtual Console Server support"
> > depends on PPC_PSERIES
> > + select HVC_CONSOLE
> > help
> > Partitionable IBM Power5 ppc64 machines allow hosting of
> > firmware virtual consoles from one Linux partition by
>
>
> Why not a normal dependency?

Most of the HVC options are doing select on other HVC things (like
select HVC_DRIVER). So if this one needs to be a dependency, then it
would make more sense to clean up the HVC option group to do the same. I
just did the one-liner to make it work.

2006-11-30 18:33:56

by Suresh Siddha

[permalink] [raw]
Subject: Re: [PATCH 1/4] [x86] Add command line option to enable/disable hyper-threading.

On Thu, Nov 30, 2006 at 11:06:11AM +0000, Alan wrote:
> On Wed, 29 Nov 2006 23:26:05 -0500
> Ben Collins <[email protected]> wrote:
>
> > This patch adds a config option to allow disabling hyper-threading by
> > default, and a kernel command line option to changes this default at
> > boot time.
> >
> > Signed-off-by: Ben Collins <[email protected]>
>
> The description is wrong - this does not disable hyperthreading it merely
> leaves one thread idle.

How does this patch achieve that? All this patch does is not detecting the
sibling topology. Kernel will still use all the threads and it just
forgoes the intelligence of which cpus are thread and core siblings and
thus disables the optimizations done by scheduler and doesn't export the
cpu topology to the user through sysfs and /proc.

Am I missing the point of this patch?

thanks,
suresh

> I don't believe Intel have ever published a
> procedure for truely disabling HT, but if you idle a thread you may want
> to adjust the cache settings on a PIV (10.5.6 in the intel docs) and set
> it to shared mode. Need to play more with what the bios does I guess.
>
> So Ack but with the proviso it should say "Ignoring" or "Not using" not
> "Disabling", because it does not do the latter and there seem to be
> performance differences as a result
>
> Acked-by: Alan Cox <[email protected]>

2006-12-01 08:39:36

by Ingo Molnar

[permalink] [raw]
Subject: Re: [PATCH 2/4] [APIC] Allow disabling of UP APIC/IO-APIC by default, with command line option to turn it on.


* Ben Collins <[email protected]> wrote:

> +config X86_UP_APIC_DEFAULT_OFF
> + bool "APIC support on uniprocessors defaults to off"
> + depends on X86_UP_APIC
> + default n

'n' is the default

> /*
> * Knob to control our willingness to enable the local APIC.
> + * -2=default-disable, -1=force-disable, 1=force-enable, 0=automatic
> */
> -static int enable_local_apic __initdata = 0; /* -1=force-disable, +1=force-enable */
> +static int enable_local_apic __initdata = (X86_APIC_DEFAULT_OFF ? -2 : 0);

i guess this begs for enums?

> if (enable_local_apic <= 0) {
> - printk("Local APIC disabled by BIOS -- "
> + printk("Local APIC disabled by BIOS (or by default) -- "
> "you can enable it with \"lapic\"\n");

that message should be more intelligent, depending on whether the value
is 0, -1 or -2.

> + /* If local apic is off due to config_x86_apic_off option, jump
> + * out here. */

nitpick: proper comment style for new code is:

/*
* If local APIC is off due to config_x86_apic_off option, jump
* out here.
*/

> + if (enable_local_apic < -1) {
> + printk(KERN_INFO "Local APIC disabled by default -- "
> + "use 'lapic' to enable it.\n");
> + return -1;
> + }

this should be enable_local_apic == -2. (and should use the enum)

> -int skip_ioapic_setup;
> +int skip_ioapic_setup = X86_APIC_DEFAULT_OFF;

nitpick: should be X86_IOAPIC_DEFAULT_VALUE - if the config option is
not set then this 'OFF' value will mean 'on' ...

> +static int __init parse_apic(char *arg)
> +{
> + /* enable IO-APIC */
> + enable_ioapic_setup();
> + return 0;
> +}
> +early_param("apic", parse_apic);

that should be "ioapic", not "apic". The CPU has a piece of silicon
called the "local APIC" - enabled via the 'lapic' option, and disabled
via noapic. What the option above wants to enable is the IO-APIC in the
chipset (a different piece of silicon) and the interrupt routing
capabilities attached to it. That piece is what is causing the installer
problems.

looks good in principle, but needs these cleanups.

Ingo

2006-12-01 13:29:35

by Pavel Machek

[permalink] [raw]
Subject: Re: [PATCH 1/4] [x86] Add command line option to enable/disable hyper-threading.

Hi!

> This patch adds a config option to allow disabling hyper-threading by
> default, and a kernel command line option to changes this default at
> boot time.

> +config X86_HT_DISABLE
> + bool "Disable Hyper-Threading by default"
> + depends on X86_HT
> + default n
> +

Command line options are fine, but additional config options mirroring
command line functionality look ugly to me...

Pavel
--
Thanks for all the (sleeping) penguins.

2006-12-01 13:41:45

by Ben Collins

[permalink] [raw]
Subject: Re: [PATCH 1/4] [x86] Add command line option to enable/disable hyper-threading.

On Fri, 2006-12-01 at 13:29 +0000, Pavel Machek wrote:
> Hi!
>
> > This patch adds a config option to allow disabling hyper-threading by
> > default, and a kernel command line option to changes this default at
> > boot time.
>
> > +config X86_HT_DISABLE
> > + bool "Disable Hyper-Threading by default"
> > + depends on X86_HT
> > + default n
> > +
>
> Command line options are fine, but additional config options mirroring
> command line functionality look ugly to me...

There's actually two parts to the patch. One is the kernel command line
option to allow HT to be enabled/disabled. The second is this config
option that allows the default to be off instead of the current
always-on.

The idea is that we want our users to be able to use hyper-threading,
but we don't want it on by default.

2006-12-01 14:32:25

by Arjan van de Ven

[permalink] [raw]
Subject: Re: [PATCH 1/4] [x86] Add command line option to enable/disable hyper-threading.


>
> The idea is that we want our users to be able to use hyper-threading,
> but we don't want it on by default.

Hi,

can I ask why not?

Greetings,
Arjan van de Ven
--
if you want to mail me at work (you don't), use arjan (at) linux.intel.com
Test the interaction between Linux and your BIOS via http://www.linuxfirmwarekit.org

2006-12-01 15:09:30

by Ben Collins

[permalink] [raw]
Subject: Re: [PATCH 1/4] [x86] Add command line option to enable/disable hyper-threading.

On Fri, 2006-12-01 at 15:32 +0100, Arjan van de Ven wrote:
> >
> > The idea is that we want our users to be able to use hyper-threading,
> > but we don't want it on by default.
>
> Hi,
>
> can I ask why not?

I'm just basing this on the history of the patch, which preceeds me, so
if this is incorrect, please don't blame me for misinformation :)

The original patch claims that hyper-threading opens the user up to some
sort of security risk involving hardware limitations in protecting
memory across the threads. I can't recall all the details.

If this is wrong, I'm more than happy to just drop the whole damn patch.

2006-12-01 16:10:41

by Arjan van de Ven

[permalink] [raw]
Subject: Re: [PATCH 1/4] [x86] Add command line option to enable/disable hyper-threading.


> I'm just basing this on the history of the patch, which preceeds me, so
> if this is incorrect, please don't blame me for misinformation :)
>
> The original patch claims that hyper-threading opens the user up to some
> sort of security risk involving hardware limitations in protecting
> memory across the threads. I can't recall all the details.
>
> If this is wrong, I'm more than happy to just drop the whole damn patch.

that is not correct.
I suspect what is meant is the "attack" on older openssl versions where
you could in theory get SOME information about a key in use by snooping
cache patterns in a shared cache situation. By no means is it a "direct"
leak of any kind, and openssl has since then been fixed to not have as
many key-dependent execution streams anymore.

I would suggest you drop the patch; openssl has been long fixed, and it
was only a theoretical attack in the first place...
I'm not saying the attack isn't something that should be addressed.. but
it is, and disabling hyperthreading is not the right fix.


--
if you want to mail me at work (you don't), use arjan (at) linux.intel.com
Test the interaction between Linux and your BIOS via http://www.linuxfirmwarekit.org

2006-12-01 16:14:15

by Ben Collins

[permalink] [raw]
Subject: Re: [PATCH 1/4] [x86] Add command line option to enable/disable hyper-threading.

On Fri, 2006-12-01 at 17:10 +0100, Arjan van de Ven wrote:
> > I'm just basing this on the history of the patch, which preceeds me, so
> > if this is incorrect, please don't blame me for misinformation :)
> >
> > The original patch claims that hyper-threading opens the user up to some
> > sort of security risk involving hardware limitations in protecting
> > memory across the threads. I can't recall all the details.
> >
> > If this is wrong, I'm more than happy to just drop the whole damn patch.
>
> that is not correct.
> I suspect what is meant is the "attack" on older openssl versions where
> you could in theory get SOME information about a key in use by snooping
> cache patterns in a shared cache situation. By no means is it a "direct"
> leak of any kind, and openssl has since then been fixed to not have as
> many key-dependent execution streams anymore.
>
> I would suggest you drop the patch; openssl has been long fixed, and it
> was only a theoretical attack in the first place...
> I'm not saying the attack isn't something that should be addressed.. but
> it is, and disabling hyperthreading is not the right fix.

Thanks for clearing that up. Patch withdrawn.

2006-12-01 16:20:31

by Linus Torvalds

[permalink] [raw]
Subject: Re: [PATCH 1/4] [x86] Add command line option to enable/disable hyper-threading.



On Fri, 1 Dec 2006, Arjan van de Ven wrote:
>
> I would suggest you drop the patch; openssl has been long fixed, and it
> was only a theoretical attack in the first place...
> I'm not saying the attack isn't something that should be addressed.. but
> it is, and disabling hyperthreading is not the right fix.

I concur. A lot of these "timing attacks" may be slightly easier on HT
CPU's than other CPU's, but they are still pretty damn theoretical (the
more recent branch predictor one is even more so, since it apparently
requires access to the branch predictor state itself, which you need
CPL0 to get - and once you have CPL0, why the hell bother with the branch
predictors at all, since you might as well just read the state directly
from the process..)

People are a hell of a lot better at worrying about unrealistic attacks
that they don't understand and thus sound scary, than about worrying about
the simple things ("You mean my password shouldn't be my pets name, taped
to my monitor? Really? And I wasn't supposed to give it out just because
that nice man gave me chocolate?")

So I think people have blown those SSL timing attacks _way_ out of
proportion, just because it sounds technical and cool.

Besides, most of the time you can disable HT in the BIOS, which is better
anyway if you don't want it.

Linus

2006-12-01 16:50:17

by Alan

[permalink] [raw]
Subject: Re: [PATCH 1/4] [x86] Add command line option to enable/disable hyper-threading.

> So I think people have blown those SSL timing attacks _way_ out of
> proportion, just because it sounds technical and cool.
>
> Besides, most of the time you can disable HT in the BIOS, which is better
> anyway if you don't want it.

Agreed - but the SSL thing is an irrelevance. The main reason for
disabling HT (especially on a single core CPU) is because a lot of
workloads run faster with HT *off*.

Alan

2006-12-01 17:13:32

by Mark D Rustad

[permalink] [raw]
Subject: Re: [PATCH 1/4] [x86] Add command line option to enable/disable hyper-threading.

On Dec 1, 2006, at 10:56 AM, Alan wrote:
>> So I think people have blown those SSL timing attacks _way_ out of
>> proportion, just because it sounds technical and cool.
>>
>> Besides, most of the time you can disable HT in the BIOS, which is
>> better
>> anyway if you don't want it.
>
> Agreed - but the SSL thing is an irrelevance. The main reason for
> disabling HT (especially on a single core CPU) is because a lot of
> workloads run faster with HT *off*.

Yes. Another way to effectively turn it off is to set maxcpus to the
number of physical cpus in your system. So far I have not encountered
a system that that approach does not work on.

--
Mark Rustad, [email protected]

2006-12-01 20:56:43

by Ben Collins

[permalink] [raw]
Subject: Re: [PATCH 2/4] [APIC] Allow disabling of UP APIC/IO-APIC by default, with command line option to turn it on.

On Fri, 2006-12-01 at 09:39 +0100, Ingo Molnar wrote:
> * Ben Collins <[email protected]> wrote:

> that should be "ioapic", not "apic". The CPU has a piece of silicon
> called the "local APIC" - enabled via the 'lapic' option, and disabled
> via noapic. What the option above wants to enable is the IO-APIC in the
> chipset (a different piece of silicon) and the interrupt routing
> capabilities attached to it. That piece is what is causing the installer
> problems.

I know the difference between APIC and IO-APIC :)

Thing is, the function right above the one I added was this:

static int __init parse_noapic(char *arg)
{
/* disable IO-APIC */
disable_ioapic_setup();
return 0;
}
early_param("noapic", parse_noapic);

So while "ioapic" might make more sense, it's doesn't match the opposing
command line option of "noapic".

I could include this in the diff:

+/* "noapic" is for backward compatibility */
early_param("noapic", parse_noapic);
+early_param("noioapic", parse_noapic);

And then add the "ioapic" option.

2006-12-02 08:57:09

by Ingo Molnar

[permalink] [raw]
Subject: Re: [PATCH 2/4] [APIC] Allow disabling of UP APIC/IO-APIC by default, with command line option to turn it on.


* Ben Collins <[email protected]> wrote:

> So while "ioapic" might make more sense, it's doesn't match the
> opposing command line option of "noapic".
>
> I could include this in the diff:
>
> +/* "noapic" is for backward compatibility */
> early_param("noapic", parse_noapic);
> +early_param("noioapic", parse_noapic);
>
> And then add the "ioapic" option.

makes sense i think.

Ingo

2006-12-07 22:50:21

by Peter Cordes

[permalink] [raw]
Subject: Re: [PATCH 1/4] [x86] Add command line option to enable/disable hyper-threading.

(I'm not subscribed to the list, so this message looks like a normal reply,
but it isn't. I didn't try to hack up an In-Reply-To: header...)

On Dec 1 2006, Linus Torvalds wrote:
>On Fri, 1 Dec 2006, Arjan van de Ven wrote:
>>
>> I would suggest you drop the patch; openssl has been long fixed, and it
>> was only a theoretical attack in the first place...
>> I'm not saying the attack isn't something that should be addressed.. but
>> it is, and disabling hyperthreading is not the right fix.
>
>I concur. A lot of these "timing attacks" may be slightly easier on HT
>CPU's than other CPU's, but they are still pretty damn theoretical (the
>more recent branch predictor one is even more so, since it apparently
>requires access to the branch predictor state itself, which you need
>CPL0 to get - and once you have CPL0, why the hell bother with the branch
>predictors at all, since you might as well just read the state directly
>from the process..)

I recently read the paper by Aciicmez, Koc, and Seifert.
http://eprint.iacr.org/2006/351 (full text PDF available; It's a fairly
interesting read, with some background and overview of this kind of attack).

They have a clever method for gathering information about the branch
predictor state, to attack one particular branch in the RSA code, which is
taken or not depending on the key bit.
- RSA and the attacker thread run on the same physical CPU.

- The attacker thread runs a loop that contains enough branches to ensure the
branch being attacked is flushed from the branch target buffer. Thus the
CPU has to speculate that it's not taken. If that's the case, then
nothing special happens.

- If the branch is taken, then one of the attack loop's branches is evicted
from the BTB. When the attack loop hits that misprediction, it causes a
cascade of mispredictions and BTB evictions, leading to a change in loop
execution time greater than the noise. They have plots showing some good
runs and some noisier runs...

- That gives them enough resolution to get most of the key bits in a single
run, so they don't have to average over multiple runs with the same input
data.

- They attacked 512bit keys on OpenSSL 0.9.7e, with some modifications to
make it simpler. It's unclear from their wording whether they attacked
both their simplified and the original, or if both their versions (with
and without a balanced Montgomery powering ladder) were the result of
modifying OpenSSL.

>So I think people have blown those SSL timing attacks _way_ out of
>proportion, just because it sounds technical and cool.

I think this attack is technical and cool, regardless of whether it's
useful in the real world. Obviously it requires running custom code on the
same machine as the process under attack, which can't happen in a lot of
server applications. It could be effective against SSH on a multi-user
machine, though.

>Besides, most of the time you can disable HT in the BIOS, which is better
>anyway if you don't want it.

If someone is trying to find a way to turn off HT, for security or
performance, it's better if they have to do it in a way that doesn't hurt
performance. I don't like options that look useful but when you read the
fine print turn out to be non-optimal.

If there was support for actually setting up the CPU the same way BIOSes do
when you HT is disabled that way, that would be cool. It's always nice to
be able to change things without walking down to the machine room to get a
console on our cluster that doesn't have remote access to the BIOS setup.

--
#define X(x,y) x##y
Peter Cordes ; e-mail: X(peter@cor , des.ca)

"The gods confound the man who first found out how to distinguish the hours!
Confound him, too, who in this place set up a sundial, to cut and hack
my day so wretchedly into small pieces!" -- Plautus, 200 BC