2009-06-08 10:30:04

by Harald Welte

[permalink] [raw]
Subject: [PATCH 2/2] CPUFREQ: Mark e_powersaver driver as EXPERIMENTAL and DANGEROUS

The e_powersaver driver for VIA's C7 CPU's needs to be marked as
DANGEROUS as it configures the CPU to power states that are out
of specification.

According to Centaur, all systems with C7 and Nano CPU's support
the ACPI p-state method. Thus, the acpi-cpufreq driver should
be used instead.

Signed-off-by: Harald Welte <[email protected]>
---
arch/x86/kernel/cpu/cpufreq/Kconfig | 9 ++++++---
1 files changed, 6 insertions(+), 3 deletions(-)

diff --git a/arch/x86/kernel/cpu/cpufreq/Kconfig b/arch/x86/kernel/cpu/cpufreq/Kconfig
index 52c8398..f138c6c 100644
--- a/arch/x86/kernel/cpu/cpufreq/Kconfig
+++ b/arch/x86/kernel/cpu/cpufreq/Kconfig
@@ -220,11 +220,14 @@ config X86_LONGHAUL
If in doubt, say N.

config X86_E_POWERSAVER
- tristate "VIA C7 Enhanced PowerSaver"
+ tristate "VIA C7 Enhanced PowerSaver (DANGEROUS)"
select CPU_FREQ_TABLE
- depends on X86_32
+ depends on X86_32 && EXPERIMENTAL
help
- This adds the CPUFreq driver for VIA C7 processors.
+ This adds the CPUFreq driver for VIA C7 processors. However, this driver
+ does not have any safeguards to prevent operating the CPU out of spec
+ and is thus considered dangerous. Please use the regular ACPI cpufreq
+ driver, enabled by CONFIG_X86_ACPI_CPUFREQ.

If in doubt, say N.

--
1.6.2.4


2009-06-08 12:09:16

by Matthew Garrett

[permalink] [raw]
Subject: Re: [PATCH 2/2] CPUFREQ: Mark e_powersaver driver as EXPERIMENTAL and DANGEROUS

On Mon, Jun 08, 2009 at 06:29:36PM +0800, Harald Welte wrote:
> The e_powersaver driver for VIA's C7 CPU's needs to be marked as
> DANGEROUS as it configures the CPU to power states that are out
> of specification.
>
> According to Centaur, all systems with C7 and Nano CPU's support
> the ACPI p-state method. Thus, the acpi-cpufreq driver should
> be used instead.

Do we know if vendors are actually shipping with the appropriate BIOS
tables? The number of people using e_powersaver seems to be suspiciously
large, though perhaps that's just because the help text implied it was
the right choice for C7.

--
Matthew Garrett | [email protected]

2009-06-08 13:00:25

by Michael S. Zick

[permalink] [raw]
Subject: Re: [PATCH 2/2] CPUFREQ: Mark e_powersaver driver as EXPERIMENTAL and DANGEROUS

On Mon June 8 2009, Matthew Garrett wrote:
> On Mon, Jun 08, 2009 at 06:29:36PM +0800, Harald Welte wrote:
> > The e_powersaver driver for VIA's C7 CPU's needs to be marked as
> > DANGEROUS as it configures the CPU to power states that are out
> > of specification.
> >
> > According to Centaur, all systems with C7 and Nano CPU's support
> > the ACPI p-state method. Thus, the acpi-cpufreq driver should
> > be used instead.
>
> Do we know if vendors are actually shipping with the appropriate BIOS
> tables? The number of people using e_powersaver seems to be suspiciously
> large, though perhaps that's just because the help text implied it was
> the right choice for C7.
>

In part, it was because acpi_freq would not load - H.W. in another post
has proposed a fix for that.

My test machine is a "first production" of the Everex Cloudbook - -
About as early as you can get of something in the public hands -
If there is something lacking in a machine's BIOS - it should be
missing in this one - I can test for that condition.
(This one is hitting user code with the silicon still setup
in the state set by "#reset" - untouched by the BIOS.)

Of course, that says nothing about other and/or newer machines -
At the moment I am toying with the idea of a cpu-quirks routine
that could check/correct for an incompletely setup chip.
On the Model-D chips, that could also switch the processor over
to its on-silicon thermal/power/freq adaptive controller.

All of the Netbooks that I know of have shipped with the Model-D
processor (ULV) - so the above cpu-quirks routine would "fix" those.

Machines running other C7 models...
They will just have to trust the BIOS and/or a repaired e_powersaver.

Mike

2009-06-08 13:06:29

by Michael S. Zick

[permalink] [raw]
Subject: Re: [PATCH 2/2] CPUFREQ: Mark e_powersaver driver as EXPERIMENTAL and DANGEROUS

On Mon June 8 2009, Harald Welte wrote:
> The e_powersaver driver for VIA's C7 CPU's needs to be marked as
> DANGEROUS as it configures the CPU to power states that are out
> of specification.
>

I'll put this and your acpi proposal into a test build today - -
If my machine survives, I'll post the build for others to try.

The current build posted (-09159) does not have *either* control
included, leaving the machine running at silicon "#reset" conditions.
Slow - but safe.

Mike
> According to Centaur, all systems with C7 and Nano CPU's support
> the ACPI p-state method. Thus, the acpi-cpufreq driver should
> be used instead.
>
> Signed-off-by: Harald Welte <[email protected]>
> ---
> arch/x86/kernel/cpu/cpufreq/Kconfig | 9 ++++++---
> 1 files changed, 6 insertions(+), 3 deletions(-)
>
> diff --git a/arch/x86/kernel/cpu/cpufreq/Kconfig b/arch/x86/kernel/cpu/cpufreq/Kconfig
> index 52c8398..f138c6c 100644
> --- a/arch/x86/kernel/cpu/cpufreq/Kconfig
> +++ b/arch/x86/kernel/cpu/cpufreq/Kconfig
> @@ -220,11 +220,14 @@ config X86_LONGHAUL
> If in doubt, say N.
>
> config X86_E_POWERSAVER
> - tristate "VIA C7 Enhanced PowerSaver"
> + tristate "VIA C7 Enhanced PowerSaver (DANGEROUS)"
> select CPU_FREQ_TABLE
> - depends on X86_32
> + depends on X86_32 && EXPERIMENTAL
> help
> - This adds the CPUFreq driver for VIA C7 processors.
> + This adds the CPUFreq driver for VIA C7 processors. However, this driver
> + does not have any safeguards to prevent operating the CPU out of spec
> + and is thus considered dangerous. Please use the regular ACPI cpufreq
> + driver, enabled by CONFIG_X86_ACPI_CPUFREQ.
>
> If in doubt, say N.
>

2009-06-08 13:16:06

by Michael S. Zick

[permalink] [raw]
Subject: Re: [PATCH 2/2] CPUFREQ: Mark e_powersaver driver as EXPERIMENTAL and DANGEROUS

On Mon June 8 2009, Michael S. Zick wrote:
> On Mon June 8 2009, Harald Welte wrote:
> > The e_powersaver driver for VIA's C7 CPU's needs to be marked as
> > DANGEROUS as it configures the CPU to power states that are out
> > of specification.
> >
>
> I'll put this and your acpi proposal into a test build today - -
> If my machine survives, I'll post the build for others to try.
>
> The current build posted (-09159) does not have *either* control
>
Sorry - typo - today is of course the 159th day of the year - -
The previously posted build *without* either control is the -09156. ;)

Mike
> included, leaving the machine running at silicon "#reset" conditions.
> Slow - but safe.
>
> Mike
> > According to Centaur, all systems with C7 and Nano CPU's support
> > the ACPI p-state method. Thus, the acpi-cpufreq driver should
> > be used instead.
> >
> > Signed-off-by: Harald Welte <[email protected]>
> > ---
> > arch/x86/kernel/cpu/cpufreq/Kconfig | 9 ++++++---
> > 1 files changed, 6 insertions(+), 3 deletions(-)
> >
> > diff --git a/arch/x86/kernel/cpu/cpufreq/Kconfig b/arch/x86/kernel/cpu/cpufreq/Kconfig
> > index 52c8398..f138c6c 100644
> > --- a/arch/x86/kernel/cpu/cpufreq/Kconfig
> > +++ b/arch/x86/kernel/cpu/cpufreq/Kconfig
> > @@ -220,11 +220,14 @@ config X86_LONGHAUL
> > If in doubt, say N.
> >
> > config X86_E_POWERSAVER
> > - tristate "VIA C7 Enhanced PowerSaver"
> > + tristate "VIA C7 Enhanced PowerSaver (DANGEROUS)"
> > select CPU_FREQ_TABLE
> > - depends on X86_32
> > + depends on X86_32 && EXPERIMENTAL
> > help
> > - This adds the CPUFreq driver for VIA C7 processors.
> > + This adds the CPUFreq driver for VIA C7 processors. However, this driver
> > + does not have any safeguards to prevent operating the CPU out of spec
> > + and is thus considered dangerous. Please use the regular ACPI cpufreq
> > + driver, enabled by CONFIG_X86_ACPI_CPUFREQ.
> >
> > If in doubt, say N.
> >
>
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/
>
>

2009-06-08 18:20:19

by Harald Welte

[permalink] [raw]
Subject: Re: [PATCH 2/2] CPUFREQ: Mark e_powersaver driver as EXPERIMENTAL and DANGEROUS

On Mon, Jun 08, 2009 at 01:08:46PM +0100, Matthew Garrett wrote:
> On Mon, Jun 08, 2009 at 06:29:36PM +0800, Harald Welte wrote:
> > The e_powersaver driver for VIA's C7 CPU's needs to be marked as
> > DANGEROUS as it configures the CPU to power states that are out
> > of specification.
> >
> > According to Centaur, all systems with C7 and Nano CPU's support
> > the ACPI p-state method. Thus, the acpi-cpufreq driver should
> > be used instead.
>
> Do we know if vendors are actually shipping with the appropriate BIOS
> tables?

I strongly assume so, since that is also how the Windows XP driver works (as
I've been told by Centaur). So if a vendor decides to not ship with the
respective BIOS tables, the power management would fail on Windows, too.

> The number of people using e_powersaver seems to be suspiciously
> large, though perhaps that's just because the help text implied it was
> the right choice for C7.

yes, I think that is the case. I just had a brief look at the driver earlier
today with one of the key Centaur engineers, and he was very clear on it: Use
ACPI.

There are also other reasons to use ACPI. Let's imagine you are having a
system that uses a certain cpu that might clock up to 1600 MHz. But the system
vendor decides to use the CPU passively cooled and thus wants to restrict the
frequency to 1300MHz. The standard practise (as recommended by VIA/Centaur BIOS
writers guide) is to remove the > 1300MHz p-states from the ACPI tables.

So even if e_powersaver was fixed to not drive the CPU out of spec, it might
still do that on a particular system/board, where the vendor has decided to
limit CPU clock for cooling reasons.

--
- Harald Welte <[email protected]> http://linux.via.com.tw/
============================================================================
VIA Free and Open Source Software Liaison

2009-06-08 18:38:25

by Michael S. Zick

[permalink] [raw]
Subject: Re: [PATCH 2/2] CPUFREQ: Mark e_powersaver driver as EXPERIMENTAL and DANGEROUS

On Mon June 8 2009, Harald Welte wrote:
> On Mon, Jun 08, 2009 at 01:08:46PM +0100, Matthew Garrett wrote:
> > On Mon, Jun 08, 2009 at 06:29:36PM +0800, Harald Welte wrote:
> > > The e_powersaver driver for VIA's C7 CPU's needs to be marked as
> > > DANGEROUS as it configures the CPU to power states that are out
> > > of specification.
> > >
> > > According to Centaur, all systems with C7 and Nano CPU's support
> > > the ACPI p-state method. Thus, the acpi-cpufreq driver should
> > > be used instead.
> >
> > Do we know if vendors are actually shipping with the appropriate BIOS
> > tables?
>
> I strongly assume so, since that is also how the Windows XP driver works (as
> I've been told by Centaur). So if a vendor decides to not ship with the
> respective BIOS tables, the power management would fail on Windows, too.
>
> > The number of people using e_powersaver seems to be suspiciously
> > large, though perhaps that's just because the help text implied it was
> > the right choice for C7.
>
> yes, I think that is the case. I just had a brief look at the driver earlier
> today with one of the key Centaur engineers, and he was very clear on it: Use
> ACPI.
>
> There are also other reasons to use ACPI. Let's imagine you are having a
> system that uses a certain cpu that might clock up to 1600 MHz. But the system
> vendor decides to use the CPU passively cooled and thus wants to restrict the
> frequency to 1300MHz. The standard practise (as recommended by VIA/Centaur BIOS
> writers guide) is to remove the > 1300MHz p-states from the ACPI tables.
>
> So even if e_powersaver was fixed to not drive the CPU out of spec, it might
> still do that on a particular system/board, where the vendor has decided to
> limit CPU clock for cooling reasons.
>

I have yet to run either of my machines using only the internal adaptive controller;
__BUT__
As I read the information, under a limited cooling condition it would be safe and
stable **for the silicon** - -
But for a machine that could come into contact with human skin... that might do harm.
Skin doesn't have the heat tolerance these silicon devices do. ;)

For an embedded appliance, or something else protected from human contact, perhaps
with no or limited BIOS support - the internal adaptive controller sounds like the
thing to be used.

Since the acpi driver will have to have the code to identify if the internal
controller is enabled and disable it - I am considering a acpi_cpu=internal
command line flag to let the (soon to be) acpi driver just enable the internal
controller and *not* turn over control to the acpi.
This would be of interest to builders of those embedded appliances and only
would add a few lines of code to an __init section.

Mike