[PATCH] x86: hpet: Don't default CONFIG_HPET_TIMER to be y for X86_64

[Feng Tang]

On many new phone/tablet platforms like Baytrail/Merrifield etc,
the HPET are either defeatured or has some problem to be used
as a reliable timer. As these platforms also have X86_64, we
should not make HPET_TIMER default y for all X86_64.

Signed-off-by: Feng Tang <feng...@intel.com>
---
arch/x86/Kconfig | 3 +--
1 files changed, 1 insertions(+), 2 deletions(-)

diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
index 0af5250..269bd47 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -695,8 +695,7 @@ config X86_CYCLONE_TIMER
source "arch/x86/Kconfig.cpu"

config HPET_TIMER
- def_bool X86_64
- prompt "HPET Timer Support" if X86_32
+ prompt "HPET Timer Support"
---help---
Use the IA-PC HPET (High Precision Event Timer) to manage
time in preference to the PIT and RTC, if a HPET is
--
1.7.0.4

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majo...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

[Clemens Ladisch]

Feng Tang wrote:
> On many new phone/tablet platforms like Baytrail/Merrifield etc,
> the HPET are either defeatured or has some problem to be used
> as a reliable timer. As these platforms also have X86_64, we
> should not make HPET_TIMER default y for all X86_64.

The help text still says:
| You can safely choose Y here. [...]
| Choose N to continue using the legacy 8254 timer.

Are these statements still true for those platforms?


Regards,
Clemens

[Andy Lutomirski]

On 03/27/2014 04:02 AM, Clemens Ladisch wrote:
> Feng Tang wrote:
>> On many new phone/tablet platforms like Baytrail/Merrifield etc,
>> the HPET are either defeatured or has some problem to be used
>> as a reliable timer. As these platforms also have X86_64, we
>> should not make HPET_TIMER default y for all X86_64.

Wouldn't it be better to fix the hpet driver so that the hpet doesn't
used on these platforms?

>
> The help text still says:
> | You can safely choose Y here. [...]
> | Choose N to continue using the legacy 8254 timer.
>
> Are these statements still true for those platforms?

They aren't true for modern desktop and server platforms -- the TSC is
used regardless of hpet availability.

[John Stultz]

On Thu, Mar 27, 2014 at 12:52 PM, Andy Lutomirski <lu...@amacapital.net> wrote:
> On 03/27/2014 04:02 AM, Clemens Ladisch wrote:
>> Feng Tang wrote:
>> The help text still says:
>> | You can safely choose Y here. [...]
>> | Choose N to continue using the legacy 8254 timer.
>>
>> Are these statements still true for those platforms?
>
> They aren't true for modern desktop and server platforms -- the TSC is
> used regardless of hpet availability.

While I suspect the comment above is in relation to the non-apic
timer. But with respect to timekeeping, our point is true assuming the
TSC isn't mucked up by the BIOS. My 1yr old i7-3930k single socket
system still has some wonky BIOS bug that offsets the boot core's TSC.
And that's intel's bios, so I can only imagine other vendors have
found other ways to cause trouble.

So yea, the hpet availability for timekeeping is still important, as
the TSC can still be problematic.

thanks
-john

[Andy Lutomirski]

On Thu, Mar 27, 2014 at 2:27 PM, John Stultz <john....@linaro.org> wrote:
> On Thu, Mar 27, 2014 at 12:52 PM, Andy Lutomirski <lu...@amacapital.net> wrote:
>> On 03/27/2014 04:02 AM, Clemens Ladisch wrote:
>>> Feng Tang wrote:
>>> The help text still says:
>>> | You can safely choose Y here. [...]
>>> | Choose N to continue using the legacy 8254 timer.
>>>
>>> Are these statements still true for those platforms?
>>
>> They aren't true for modern desktop and server platforms -- the TSC is
>> used regardless of hpet availability.
>
> While I suspect the comment above is in relation to the non-apic
> timer. But with respect to timekeeping, our point is true assuming the
> TSC isn't mucked up by the BIOS. My 1yr old i7-3930k single socket
> system still has some wonky BIOS bug that offsets the boot core's TSC.
> And that's intel's bios, so I can only imagine other vendors have
> found other ways to cause trouble.

Is this, perhaps, an MSI X79A-GD65 (8D) (MS-7760)? If so, there's a fixed BIOS.

>
> So yea, the hpet availability for timekeeping is still important, as
> the TSC can still be problematic.

Is HPET really that much better than acpi_pm? I can read my HPET in
~584ns (vdso) or ~649ns (syscall) and my acpi_pm in 753ns. So it's
better, but not by a whole lot.

But yes, I see no good reason to disable it, except specifically on
systems where there are known bugs.

>
> thanks
> -john



--
Andy Lutomirski
AMA Capital Management, LLC

[John Stultz]

On 03/27/2014 02:33 PM, Andy Lutomirski wrote:
> On Thu, Mar 27, 2014 at 2:27 PM, John Stultz <john....@linaro.org> wrote:
>> On Thu, Mar 27, 2014 at 12:52 PM, Andy Lutomirski <lu...@amacapital.net> wrote:
>>> On 03/27/2014 04:02 AM, Clemens Ladisch wrote:
>>>> Feng Tang wrote:
>>>> The help text still says:
>>>> | You can safely choose Y here. [...]
>>>> | Choose N to continue using the legacy 8254 timer.
>>>>
>>>> Are these statements still true for those platforms?
>>> They aren't true for modern desktop and server platforms -- the TSC is
>>> used regardless of hpet availability.
>> While I suspect the comment above is in relation to the non-apic
>> timer. But with respect to timekeeping, our point is true assuming the
>> TSC isn't mucked up by the BIOS. My 1yr old i7-3930k single socket
>> system still has some wonky BIOS bug that offsets the boot core's TSC.
>> And that's intel's bios, so I can only imagine other vendors have
>> found other ways to cause trouble.
> Is this, perhaps, an MSI X79A-GD65 (8D) (MS-7760)? If so, there's a fixed BIOS.

Actually, an Intel DX79TO. I've harangued some folks I know, but no
fixes for the BIOS have been released.

>> So yea, the hpet availability for timekeeping is still important, as
>> the TSC can still be problematic.
> Is HPET really that much better than acpi_pm? I can read my HPET in
> ~584ns (vdso) or ~649ns (syscall) and my acpi_pm in 753ns. So it's
> better, but not by a whole lot.

I know on older hardware it was a bigger win. I'd have to look at what
my current system does.

> But yes, I see no good reason to disable it, except specifically on
> systems where there are known bugs.

Agreed.

thanks
-john

[Feng Tang]

On Thu, Mar 27, 2014 at 02:27:26PM -0700, John Stultz wrote:
> On Thu, Mar 27, 2014 at 12:52 PM, Andy Lutomirski <lu...@amacapital.net> wrote:
> > On 03/27/2014 04:02 AM, Clemens Ladisch wrote:
> >> Feng Tang wrote:
> >> The help text still says:
> >> | You can safely choose Y here. [...]
> >> | Choose N to continue using the legacy 8254 timer.
> >>
> >> Are these statements still true for those platforms?
> >
> > They aren't true for modern desktop and server platforms -- the TSC is
> > used regardless of hpet availability.
>
> While I suspect the comment above is in relation to the non-apic
> timer. But with respect to timekeeping, our point is true assuming the
> TSC isn't mucked up by the BIOS. My 1yr old i7-3930k single socket
> system still has some wonky BIOS bug that offsets the boot core's TSC.
> And that's intel's bios, so I can only imagine other vendors have
> found other ways to cause trouble.
>
> So yea, the hpet availability for timekeeping is still important, as
> the TSC can still be problematic.

Yes, agreed. My patch only provides a way to disable HPET for some specific
platforms :)

Thanks,
Feng

[Feng Tang]

On Thu, Mar 27, 2014 at 12:52:57PM -0700, Andy Lutomirski wrote:
> On 03/27/2014 04:02 AM, Clemens Ladisch wrote:
> > Feng Tang wrote:
> >> On many new phone/tablet platforms like Baytrail/Merrifield etc,
> >> the HPET are either defeatured or has some problem to be used
> >> as a reliable timer. As these platforms also have X86_64, we
> >> should not make HPET_TIMER default y for all X86_64.
>
> Wouldn't it be better to fix the hpet driver so that the hpet doesn't
> used on these platforms?
>
> >
> > The help text still says:
> > | You can safely choose Y here. [...]
> > | Choose N to continue using the legacy 8254 timer.
> >
> > Are these statements still true for those platforms?
>
> They aren't true for modern desktop and server platforms -- the TSC is
> used regardless of hpet availability.

The way of using TSC varies depends on the generations of platforms.
Thomas Gleixner once gave a very good description, which link I personally
bookmarked, and would share here:
https://lkml.org/lkml/2013/5/8/202

Thanks,
Feng

[Feng Tang]

On many new phone/tablet platforms like Baytrail/Merrifield etc,
the HPET are either defeatured or has some problem to be used
as a reliable timer. As these platforms also have X86_64, we
should not make HPET_TIMER default y for all X86_64.

Signed-off-by: Feng Tang <feng...@intel.com>
---

arch/x86/Kconfig | 7 +++----
1 files changed, 3 insertions(+), 4 deletions(-)

diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
index 0af5250..7f2fb4b 100644

--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -695,8 +695,7 @@ config X86_CYCLONE_TIMER
source "arch/x86/Kconfig.cpu"

config HPET_TIMER
- def_bool X86_64
- prompt "HPET Timer Support" if X86_32
+ prompt "HPET Timer Support"
---help---
Use the IA-PC HPET (High Precision Event Timer) to manage
time in preference to the PIT and RTC, if a HPET is

@@ -709,9 +708,9 @@ config HPET_TIMER

You can safely choose Y here. However, HPET will only be
activated if the platform and the BIOS support this feature.
- Otherwise the 8254 will be used for timing services.

- Choose N to continue using the legacy 8254 timer.
+ Choose N to use the legacy 8254 timer or the latest
+ lapic_timer+tsc solution.

config HPET_EMULATE_RTC
def_bool y
--
1.7.0.4

[Ingo Molnar]

* Feng Tang <feng...@intel.com> wrote:

> On many new phone/tablet platforms like Baytrail/Merrifield etc, the
> HPET are either defeatured or has some problem to be used as a
> reliable timer. As these platforms also have X86_64, we should not
> make HPET_TIMER default y for all X86_64.

NAK!

If the HPET is unreliable on a specific platform then any of the
following solutions would address the problem (in order of
preference):

- the hardware should not expose it. Why waste silicon on something
that does not work?

- or the firmware should not expose it. Why expose something that
does not work?

- or the kernel should have a quirk to reliably disable it. Why
should we crash or misbehave if a driver is built into the
kernel?

tweaking a default is _NOT_ a solution for an unreliable hpet.

Thanks,

Ingo

[Feng Tang]

Hi Ingo,

Thanks for reviewing this

On Fri, Mar 28, 2014 at 08:17:16AM +0100, Ingo Molnar wrote:
>
> * Feng Tang <feng...@intel.com> wrote:
>
> > On many new phone/tablet platforms like Baytrail/Merrifield etc, the
> > HPET are either defeatured or has some problem to be used as a
> > reliable timer. As these platforms also have X86_64, we should not
> > make HPET_TIMER default y for all X86_64.
>
> NAK!
>
> If the HPET is unreliable on a specific platform then any of the
> following solutions would address the problem (in order of
> preference):
>
> - the hardware should not expose it. Why waste silicon on something
> that does not work?
>
> - or the firmware should not expose it. Why expose something that
> does not work?

Agreed, I've raised problem to BIOS vendor, but the response is very slow,
and those hardware/BIOS may already hit the market as a product.

>
> - or the kernel should have a quirk to reliably disable it. Why
> should we crash or misbehave if a driver is built into the
> kernel?

I thought about this before, HPET doesn't have PCI ID like stuff, only
thing I can think of to identify them may be the CPU family/ID.
Runtime check the reliability of HPET may be difficult, as we don't
know if the 8254 or the TSC are the golden timer to check HPET.

Thanks,
Feng

[Clemens Ladisch]

Feng Tang wrote:
> On Fri, Mar 28, 2014 at 08:17:16AM +0100, Ingo Molnar wrote:
>> * Feng Tang <feng...@intel.com> wrote:
>> - or the kernel should have a quirk to reliably disable it. Why
>> should we crash or misbehave if a driver is built into the
>> kernel?
>
> I thought about this before, HPET doesn't have PCI ID like stuff,

HPET does have the PCI vendor ID in the first register.

> only thing I can think of to identify them may be the CPU family/ID.

The HPET is implemented by some actual chip, and that chip also has lots
of PCI devices. (In the case of a SoC, the CPU ID would work, too).


Regards,
Clemens

[Ingo Molnar]

* Clemens Ladisch <cle...@ladisch.de> wrote:

> Feng Tang wrote:
> > On Fri, Mar 28, 2014 at 08:17:16AM +0100, Ingo Molnar wrote:
> >> * Feng Tang <feng...@intel.com> wrote:
> >> - or the kernel should have a quirk to reliably disable it. Why
> >> should we crash or misbehave if a driver is built into the
> >> kernel?
> >
> > I thought about this before, HPET doesn't have PCI ID like stuff,
>
> HPET does have the PCI vendor ID in the first register.
>
> > only thing I can think of to identify them may be the CPU family/ID.
>
> The HPET is implemented by some actual chip, and that chip also has lots
> of PCI devices. (In the case of a SoC, the CPU ID would work, too).

Correct. See arch/x86/kernel/hpet.c, which has a large number of HPET
quirks keyed off chipset PCI IDs:

DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_ESB2_0,
ich_force_enable_hpet);
DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_ICH6_0,
ich_force_enable_hpet);
DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_ICH6_1,
ich_force_enable_hpet);
[...]

Thanks,

Ingo

[Feng Tang]

On Fri, Mar 28, 2014 at 09:11:17AM +0100, Ingo Molnar wrote:
>
> * Clemens Ladisch <cle...@ladisch.de> wrote:
>
> > Feng Tang wrote:
> > > On Fri, Mar 28, 2014 at 08:17:16AM +0100, Ingo Molnar wrote:
> > >> * Feng Tang <feng...@intel.com> wrote:
> > >> - or the kernel should have a quirk to reliably disable it. Why
> > >> should we crash or misbehave if a driver is built into the
> > >> kernel?
> > >
> > > I thought about this before, HPET doesn't have PCI ID like stuff,
> >
> > HPET does have the PCI vendor ID in the first register.
> >
> > > only thing I can think of to identify them may be the CPU family/ID.
> >
> > The HPET is implemented by some actual chip, and that chip also has lots
> > of PCI devices. (In the case of a SoC, the CPU ID would work, too).
>
> Correct. See arch/x86/kernel/hpet.c, which has a large number of HPET
> quirks keyed off chipset PCI IDs:
>
> DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_ESB2_0,
> ich_force_enable_hpet);
> DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_ICH6_0,
> ich_force_enable_hpet);
> DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_ICH6_1,
> ich_force_enable_hpet);
> [...]

Got it, thanks Ingo and Clemens for the pointer. Will try to figure it out.

- Feng

[Feng Tang]

Hi Ingo,

On Fri, Mar 28, 2014 at 09:11:17AM +0100, Ingo Molnar wrote:
>
> * Clemens Ladisch <cle...@ladisch.de> wrote:
>
> > Feng Tang wrote:
> > > On Fri, Mar 28, 2014 at 08:17:16AM +0100, Ingo Molnar wrote:
> > >> * Feng Tang <feng...@intel.com> wrote:
> > >> - or the kernel should have a quirk to reliably disable it. Why
> > >> should we crash or misbehave if a driver is built into the
> > >> kernel?
> > >
> > > I thought about this before, HPET doesn't have PCI ID like stuff,
> >
> > HPET does have the PCI vendor ID in the first register.
> >
> > > only thing I can think of to identify them may be the CPU family/ID.
> >
> > The HPET is implemented by some actual chip, and that chip also has lots
> > of PCI devices. (In the case of a SoC, the CPU ID would work, too).
>
> Correct. See arch/x86/kernel/hpet.c, which has a large number of HPET
> quirks keyed off chipset PCI IDs:
>
> DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_ESB2_0,
> ich_force_enable_hpet);
> DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_ICH6_0,
> ich_force_enable_hpet);
> DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_ICH6_1,
> ich_force_enable_hpet);
> [...]

I just gave it another thought, that the HPET on our platform currently
do have some problem to be used as clocksource/clockevent, but it may
get fixed in future version (by Silicon or BIOS).

If I add quirk to block it now, I may revert this code in future when
it get fixed, same problem applis for the future generation of platform.

So can we do a small change like below, so that we are able to disable
the HPET_TIMER in our own x86_64 .config, while all exising distributions
are not affected as it is still default "y" .

---
diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
index f67e839..a1027a5 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -711,7 +711,7 @@ source "arch/x86/Kconfig.cpu"

config HPET_TIMER

def_bool X86_64
- prompt "HPET Timer Support" if X86_32
+ prompt "HPET Timer Support"
---help---
Use the IA-PC HPET (High Precision Event Timer) to manage
time in preference to the PIT and RTC, if a HPET is

Thanks,
Feng

[Ingo Molnar]

If the hardware or BIOS gets fixed then that will be visible in the
revision level of the hardware, right?

Such kind of revision level, once it is known, can then be used to
turn the quirk on/off precisely.

Thanks,

Ingo

[Feng Tang]

AFAIK, if it is fixed in a new silicon version, we should be able to
detect it by the "stepping", but the PCI DEV ID is not likely to change.

If it is fixed by BIOS or PUNIT FW, then we may go through the DMI
info to check the BIOS version, One problem is there is some Baytrail
tablet platforms that doesn't provide DMI info as its FW is not
UEFI/Legacy BIOS compatible version.

Thanks,
Feng

[Ingo Molnar]

So just turn it off for the current hardware, via PCI ID. When you get
a fix, you can turn it back on again by refining the quirk check, for
that specific hardware.

[Feng Tang]

got it, thanks!

- Feng

[Ingo Molnar]

Btw., note that such narrow quirk refinings are pretty uncontroversial
and have a pretty fast path upstream, once you have the initial quirk
in there.

Thanks,

Ingo

[Feng Tang]

Hi All,

During implementing, I found the arch/x86/kerne/quirks.c doesn't fit
well as the DECLARE_PCI_FIXUP_HEADER/EARLY/FINAL() way of quirk happens
inside the pci subsystem init phase, while the hpet_enable() will be
called before that.

Then by searching the arch/x86/, the early-quirks.c can be used to
add such a quirk as the early quirks are checked earlier than
hpet_enable()

I'll post a RFC patch soon.

Thanks,
Feng

[Feng Tang]

Hi All,

Please help to review this RFC patch, if you are ok with it. I'll separte them into
2 and repost.
1. make the boot_hpet_disable extern
2. add quirk for baytrail HPET.
thanks!

- Feng



From 4b8c370649f1d2b320aa46a005b8b61eb59a539e Mon Sep 17 00:00:00 2001
From: Feng Tang <feng...@intel.com>
Date: Fri, 18 Apr 2014 15:43:22 +0800
Subject: [PATCH] x86, hpet: Add quirk to disable HPET for baytrail platform

HPET on current baytrail platform has accuracy problem to be used as
reliable clocksource/clockevent, so add a early quirk to disable it.

Signed-off-by: Feng Tang <feng...@intel.com>
---

arch/x86/include/asm/hpet.h | 1 +
arch/x86/kernel/early-quirks.c | 14 ++++++++++++++
arch/x86/kernel/hpet.c | 2 +-
3 files changed, 16 insertions(+), 1 deletions(-)

diff --git a/arch/x86/include/asm/hpet.h b/arch/x86/include/asm/hpet.h
index b18df57..36f7125 100644
--- a/arch/x86/include/asm/hpet.h
+++ b/arch/x86/include/asm/hpet.h
@@ -63,6 +63,7 @@
/* hpet memory map physical address */
extern unsigned long hpet_address;
extern unsigned long force_hpet_address;
+extern int boot_hpet_disable;
extern u8 hpet_blockid;
extern int hpet_force_user;
extern u8 hpet_msi_disable;
diff --git a/arch/x86/kernel/early-quirks.c b/arch/x86/kernel/early-quirks.c
index bc4a088..1e5f6bc 100644
--- a/arch/x86/kernel/early-quirks.c
+++ b/arch/x86/kernel/early-quirks.c
@@ -17,6 +17,7 @@
#include <asm/dma.h>
#include <asm/io_apic.h>
#include <asm/apic.h>
+#include <asm/hpet.h>
#include <asm/iommu.h>
#include <asm/gart.h>
#include <asm/irq_remapping.h>
@@ -380,6 +381,13 @@ static void __init intel_graphics_stolen(int num, int slot, int func)
}
}

+static void __init force_disable_hpet(int num, int slot, int func)
+{
+ boot_hpet_disable = 1;
+ pr_info("Will Disable HPET for this platform\n");
+}
+
+
#define QFLAG_APPLY_ONCE 0x1
#define QFLAG_APPLIED 0x2
#define QFLAG_DONE (QFLAG_APPLY_ONCE|QFLAG_APPLIED)
@@ -417,6 +425,12 @@ static struct chipset early_qrk[] __initdata = {
PCI_BASE_CLASS_BRIDGE, 0, intel_remapping_check },
{ PCI_VENDOR_ID_INTEL, PCI_ANY_ID, PCI_CLASS_DISPLAY_VGA, PCI_ANY_ID,
QFLAG_APPLY_ONCE, intel_graphics_stolen },
+ /*
+ * HPET on current version of Baytrail platform has accuracy
+ * problem, disable it for now
+ */
+ { PCI_VENDOR_ID_INTEL, 0x0f00,
+ PCI_CLASS_BRIDGE_HOST, PCI_ANY_ID, 0, force_disable_hpet},
{}
};

diff --git a/arch/x86/kernel/hpet.c b/arch/x86/kernel/hpet.c
index da85a8e..913e207 100644
--- a/arch/x86/kernel/hpet.c
+++ b/arch/x86/kernel/hpet.c
@@ -88,7 +88,7 @@ static inline void hpet_clear_mapping(void)
/*
* HPET command line enable / disable
*/
-static int boot_hpet_disable;
+int boot_hpet_disable;
int hpet_force_user;
static int hpet_verbose;

--
1.7.0.4

[Ingo Molnar]

* Feng Tang <feng...@intel.com> wrote:

As long as all those PCI IDs have a buggy HPET, this looks good to me.

hpa, tglx, any preferences?

Thanks,

Ingo