>From 9bd2f534f986768f1944e626e37af1c323e47dbb Mon Sep 17 00:00:00 2001
From: Crane Cai <[email protected]>
Date: Thu, 14 Aug 2008 10:31:01 +0800
Subject: [PATCH] HPET: Workaround for a BIOS workaround on AMD SB700 platform
On the AMD SB700 southbridge, between the revisions 0x30 to 0x3a, when its
spread-spectrum frequency modulation feature is enabled, the base frequency
used by the HPET will not be running on average slower than nominal 14.318
MHz.
Since there is no provision in the OS for HPET to work with properly with
slower frequency, the BIOS on this platform uses SMM to emulate accesses to
the HPET config register to supply a corrected base frequency to compensate
for it.
However, due to the implementation of the SMM BIOS code, there is a time
window after the first access to the HPET, which triggers initialization of
the SMM code, in which the HPET isn't available. Thus it's necessary to wait
until the HPET emulation is ready, and this is what the patch does on the
affected machines.
Signed-off-by: XiaoGang Zheng <[email protected]>
Signed-off-by: Crane Cai <[email protected]>
---
arch/x86/kernel/early-quirks.c | 13 +++++++++++++
arch/x86/kernel/hpet.c | 24 ++++++++++++++++++++++++
include/asm-x86/hpet.h | 2 ++
3 files changed, 39 insertions(+), 0 deletions(-)
diff --git a/arch/x86/kernel/early-quirks.c b/arch/x86/kernel/early-quirks.c
index 4353cf5..5f6f543 100644
--- a/arch/x86/kernel/early-quirks.c
+++ b/arch/x86/kernel/early-quirks.c
@@ -17,6 +17,7 @@
#include <asm/io_apic.h>
#include <asm/apic.h>
#include <asm/iommu.h>
+#include <asm/hpet.h>
static void __init fix_hypertransport_config(int num, int slot, int func)
{
@@ -95,6 +96,16 @@ static void __init nvidia_bugs(int num, int slot, int func)
}
+static void __init amd_sb700_hpet(int num, int slot, int func)
+{
+ int rev;
+ rev = read_pci_config_byte(num, slot, func, 0x08);
+ if (rev <= 0x3a && rev >= 0x30) {
+ sb700_hpet_workaround = 1;
+ printk(KERN_INFO "SB700 rev 0x3a or lower detected!\n");
+ }
+}
+
#define QFLAG_APPLY_ONCE 0x1
#define QFLAG_APPLIED 0x2
#define QFLAG_DONE (QFLAG_APPLY_ONCE|QFLAG_APPLIED)
@@ -114,6 +125,8 @@ static struct chipset early_qrk[] __initdata = {
PCI_CLASS_BRIDGE_PCI, PCI_ANY_ID, QFLAG_APPLY_ONCE, via_bugs },
{ PCI_VENDOR_ID_AMD, PCI_DEVICE_ID_AMD_K8_NB,
PCI_CLASS_BRIDGE_HOST, PCI_ANY_ID, 0, fix_hypertransport_config },
+ { PCI_VENDOR_ID_ATI, PCI_DEVICE_ID_ATI_SBX00_SMBUS,
+ PCI_CLASS_SERIAL_SMBUS, PCI_ANY_ID, 0, amd_sb700_hpet },
{}
};
diff --git a/arch/x86/kernel/hpet.c b/arch/x86/kernel/hpet.c
index ad2b15a..3d7f71d 100644
--- a/arch/x86/kernel/hpet.c
+++ b/arch/x86/kernel/hpet.c
@@ -23,6 +23,24 @@
* HPET address is set in acpi/boot.c, when an ACPI entry exists
*/
unsigned long hpet_address;
+
+/*
+ * On the AMD SB700 southbridge, between the revisions 0x30 to 0x3a, when its
+ * spread-spectrum frequency modulation feature is enabled, the base frequency
+ * used by the HPET will not be running on average slower than nominal 14.318
+ * MHz.
+ * Since there is no provision in the OS for HPET to work with properly with
+ * slower frequency, the BIOS on this platform uses SMM to emulate accesses to
+ * the HPET config register to supply a corrected base frequency to compensate
+ * for it.
+ * However, due to the implementation of the SMM BIOS code, there is a time
+ * window after the first access to the HPET, which triggers initialization of
+ * the SMM code, in which the HPET isn't available. Thus it's necessary to wait
+ * until the HPET emulation is ready, and this is what the patch does on the
+ * affected machines.
+ */
+int sb700_hpet_workaround __initdata = 0;
+
static void __iomem *hpet_virt_address;
unsigned long hpet_readl(unsigned long a)
@@ -369,6 +387,12 @@ int __init hpet_enable(void)
* Read the period and check for a sane value:
*/
hpet_period = hpet_readl(HPET_PERIOD);
+
+ if (sb700_hpet_workaround) {
+ int timeout = 1000;
+ while (0xffffffff == hpet_readl(HPET_CFG) && timeout-- != 0)
+ ;
+ }
if (hpet_period < HPET_MIN_PERIOD || hpet_period > HPET_MAX_PERIOD)
goto out_nohpet;
diff --git a/include/asm-x86/hpet.h b/include/asm-x86/hpet.h
index 82f1ac6..8f9a4e2 100644
--- a/include/asm-x86/hpet.h
+++ b/include/asm-x86/hpet.h
@@ -55,6 +55,8 @@
*/
#define HPET_MIN_PERIOD 100000UL
+extern int sb700_hpet_workaround;
+
/* hpet memory map physical address */
extern unsigned long hpet_address;
extern unsigned long force_hpet_address;
--
1.5.4.4
* crane cai <[email protected]> wrote:
> >From 9bd2f534f986768f1944e626e37af1c323e47dbb Mon Sep 17 00:00:00 2001
> From: Crane Cai <[email protected]>
> Date: Thu, 14 Aug 2008 10:31:01 +0800
> Subject: [PATCH] HPET: Workaround for a BIOS workaround on AMD SB700 platform
>
> On the AMD SB700 southbridge, between the revisions 0x30 to 0x3a, when
> its spread-spectrum frequency modulation feature is enabled, the base
> frequency used by the HPET will not be running on average slower than
> nominal 14.318 MHz.
>
> Since there is no provision in the OS for HPET to work with properly
> with slower frequency, the BIOS on this platform uses SMM to emulate
> accesses to the HPET config register to supply a corrected base
> frequency to compensate for it.
>
> However, due to the implementation of the SMM BIOS code, there is a
> time window after the first access to the HPET, which triggers
> initialization of the SMM code, in which the HPET isn't available.
> Thus it's necessary to wait until the HPET emulation is ready, and
> this is what the patch does on the affected machines.
nice fix! I've applied it to tip/x86/urgent as the quirk is limited to
this platform so it should be safe for v2.6.27 as well.
Exactly what kind of failure mode have you seen without the quirk? Do we
read out the wrong values and thus hpet_clocksource_register() is
calibrated incorrectly and you can get non-functional high-res timers?
Have you seen hangs/crashes due to that, or incorrect timings?
> Signed-off-by: XiaoGang Zheng <[email protected]>
> Signed-off-by: Crane Cai <[email protected]>
A quick question: the signoff order indicates that the patch has been
authored by XiaoGang Zheng. Or is the reverse order intended? (you wrote
the patch and XiaoGang Zheng processed it further)
Ingo
small build fix:
>From c107f81791afa9f749cf242944c90a18faf69fda Mon Sep 17 00:00:00 2001
From: Ingo Molnar <[email protected]>
Date: Thu, 14 Aug 2008 11:03:24 +0200
Subject: [PATCH] x86, hpet: workaround for a BIOS workaround on AMD SB700 platform, fix
fix:
arch/x86/kernel/early-quirks.c: In function 'amd_sb700_hpet':
arch/x86/kernel/early-quirks.c:105: error: 'sb700_hpet_workaround' undeclared (first use in this function)
arch/x86/kernel/early-quirks.c:105: error: (Each undeclared identifier is reported only once
arch/x86/kernel/early-quirks.c:105: error: for each function it appears in.)
which triggers in the !HPET + !HPET_TIMER case.
Signed-off-by: Ingo Molnar <[email protected]>
---
include/asm-x86/hpet.h | 4 ++--
1 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/include/asm-x86/hpet.h b/include/asm-x86/hpet.h
index 8f9a4e2..261fb68 100644
--- a/include/asm-x86/hpet.h
+++ b/include/asm-x86/hpet.h
@@ -55,8 +55,6 @@
*/
#define HPET_MIN_PERIOD 100000UL
-extern int sb700_hpet_workaround;
-
/* hpet memory map physical address */
extern unsigned long hpet_address;
extern unsigned long force_hpet_address;
@@ -87,6 +85,8 @@ extern void hpet_unregister_irq_handler(rtc_irq_handler handler);
#else /* CONFIG_HPET_TIMER */
+extern int sb700_hpet_workaround;
+
static inline int hpet_enable(void) { return 0; }
static inline int is_hpet_enabled(void) { return 0; }
#define hpet_readl(a) 0
On Thu, 2008-08-14 at 10:41 +0200, Ingo Molnar wrote:
> * crane cai <[email protected]> wrote:
>
> > >From 9bd2f534f986768f1944e626e37af1c323e47dbb Mon Sep 17 00:00:00 2001
> > From: Crane Cai <[email protected]>
> > Date: Thu, 14 Aug 2008 10:31:01 +0800
> > Subject: [PATCH] HPET: Workaround for a BIOS workaround on AMD SB700 platform
> >
> > On the AMD SB700 southbridge, between the revisions 0x30 to 0x3a, when
> > its spread-spectrum frequency modulation feature is enabled, the base
> > frequency used by the HPET will not be running on average slower than
> > nominal 14.318 MHz.
> >
> > Since there is no provision in the OS for HPET to work with properly
> > with slower frequency, the BIOS on this platform uses SMM to emulate
> > accesses to the HPET config register to supply a corrected base
> > frequency to compensate for it.
> >
> > However, due to the implementation of the SMM BIOS code, there is a
> > time window after the first access to the HPET, which triggers
> > initialization of the SMM code, in which the HPET isn't available.
> > Thus it's necessary to wait until the HPET emulation is ready, and
> > this is what the patch does on the affected machines.
>
> nice fix! I've applied it to tip/x86/urgent as the quirk is limited to
> this platform so it should be safe for v2.6.27 as well.
>
> Exactly what kind of failure mode have you seen without the quirk? Do we
> read out the wrong values and thus hpet_clocksource_register() is
> calibrated incorrectly and you can get non-functional high-res timers?
> Have you seen hangs/crashes due to that, or incorrect timings?
System might loop in TSC calibration in booting. System tick can not correctly set.
> > Signed-off-by: XiaoGang Zheng <[email protected]>
> > Signed-off-by: Crane Cai <[email protected]>
>
> A quick question: the signoff order indicates that the patch has been
> authored by XiaoGang Zheng. Or is the reverse order intended? (you wrote
> the patch and XiaoGang Zheng processed it further)
XiaoGang insures that HPET check method in hardware view.
I refine the method and generate the patch.
Vojtech and Zahir refine the description.
This issue is cleared by Joachim and many other persons in SUSE.
Thanks all.
>
> Ingo
>
-Crane
On Thu, 14 Aug 2008, crane cai wrote:
> >From 9bd2f534f986768f1944e626e37af1c323e47dbb Mon Sep 17 00:00:00 2001
> From: Crane Cai <[email protected]>
> Date: Thu, 14 Aug 2008 10:31:01 +0800
> Subject: [PATCH] HPET: Workaround for a BIOS workaround on AMD SB700 platform
>
> On the AMD SB700 southbridge, between the revisions 0x30 to 0x3a, when its
> spread-spectrum frequency modulation feature is enabled, the base frequency
> used by the HPET will not be running on average slower than nominal 14.318
> MHz.
> Since there is no provision in the OS for HPET to work with properly with
> slower frequency, the BIOS on this platform uses SMM to emulate accesses to
> the HPET config register to supply a corrected base frequency to compensate
> for it.
> However, due to the implementation of the SMM BIOS code, there is a time
> window after the first access to the HPET, which triggers initialization of
> the SMM code, in which the HPET isn't available. Thus it's necessary to wait
> until the HPET emulation is ready, and this is what the patch does on the
> affected machines.
Crane,
thanks for debugging this. I think we don't need a quirk for this
workaround. Checking the config register value for 0xffffffff is safe
on all machines. I simplified your patch to the one below and added a
printk in case the check times out.
One thing I'm a bit wary about is the readout of the HPET_PERIOD
register. Is the value correct even _before_ the SMM machinery is
started ?
Thanks,
tglx
-----
diff --git a/arch/x86/kernel/hpet.c b/arch/x86/kernel/hpet.c
index ad2b15a..59fd3b6 100644
--- a/arch/x86/kernel/hpet.c
+++ b/arch/x86/kernel/hpet.c
@@ -359,6 +359,7 @@ static int hpet_clocksource_register(void)
int __init hpet_enable(void)
{
unsigned long id;
+ int i;
if (!is_hpet_capable())
return 0;
@@ -369,6 +370,29 @@ int __init hpet_enable(void)
* Read the period and check for a sane value:
*/
hpet_period = hpet_readl(HPET_PERIOD);
+
+ /*
+ * AMD SB700 based systems with spread spectrum enabled use a
+ * SMM based HPET emulation to provide proper frequency
+ * setting. The SMM code is initialized with the first HPET
+ * register access and takes some time to complete. During
+ * this time the config register reads 0xffffffff. We check
+ * for max. 1000 loops whether the config register reads a non
+ * 0xffffffff value to make sure that HPET is up and running
+ * before we go further. A counting loop is safe, as the HPET
+ * access takes thousands of CPU cycles. On non SB700 based
+ * machines this check is only done once and has no side
+ * effects.
+ */
+ for (i = 0; hpet_readl(HPET_CFG) == 0xFFFFFFFF; i++) {
+ if (i == 1000) {
+ printk(KERN_WARNING
+ "HPET config register value = 0xFFFFFFFF. "
+ "Disabling HPET\n");
+ goto out_nohpet;
+ }
+ }
+
if (hpet_period < HPET_MIN_PERIOD || hpet_period > HPET_MAX_PERIOD)
goto out_nohpet;
On Thu, Aug 14, 2008 at 11:13:36AM +0800, crane cai wrote:
> >From 9bd2f534f986768f1944e626e37af1c323e47dbb Mon Sep 17 00:00:00 2001
> From: Crane Cai <[email protected]>
> Date: Thu, 14 Aug 2008 10:31:01 +0800
> Subject: [PATCH] HPET: Workaround for a BIOS workaround on AMD SB700 platform
>
> On the AMD SB700 southbridge, between the revisions 0x30 to 0x3a, when its
> spread-spectrum frequency modulation feature is enabled, the base frequency
> used by the HPET will not be running on average slower than nominal 14.318
> MHz.
Should that have read "the base frequency used by HPET will on average
be running slower than nominal 14.318 MHz"? That would be pretty much
the opposite of what the comment says, but makes more sense based on the
rest of the comment, and is a lot simpler to parse.
> Since there is no provision in the OS for HPET to work with properly with
> slower frequency, the BIOS on this platform uses SMM to emulate accesses to
> the HPET config register to supply a corrected base frequency to compensate
> for it.
Seems to have an extra "with" in front of properly.
> However, due to the implementation of the SMM BIOS code, there is a time
> window after the first access to the HPET, which triggers initialization of
> the SMM code, in which the HPET isn't available. Thus it's necessary to wait
> until the HPET emulation is ready, and this is what the patch does on the
> affected machines.
--
Len Sorensen
On Thu, Aug 14, 2008 at 10:11:56AM -0400, Lennart Sorensen wrote:
> On Thu, Aug 14, 2008 at 11:13:36AM +0800, crane cai wrote:
> > >From 9bd2f534f986768f1944e626e37af1c323e47dbb Mon Sep 17 00:00:00 2001
> > From: Crane Cai <[email protected]>
> > Date: Thu, 14 Aug 2008 10:31:01 +0800
> > Subject: [PATCH] HPET: Workaround for a BIOS workaround on AMD SB700 platform
> >
> > On the AMD SB700 southbridge, between the revisions 0x30 to 0x3a, when its
> > spread-spectrum frequency modulation feature is enabled, the base frequency
> > used by the HPET will not be running on average slower than nominal 14.318
> > MHz.
>
> Should that have read "the base frequency used by HPET will on average
> be running slower than nominal 14.318 MHz"? That would be pretty much
> the opposite of what the comment says, but makes more sense based on the
> rest of the comment, and is a lot simpler to parse.
Yes, that is the case.
> > Since there is no provision in the OS for HPET to work with properly with
> > slower frequency, the BIOS on this platform uses SMM to emulate accesses to
> > the HPET config register to supply a corrected base frequency to compensate
> > for it.
>
> Seems to have an extra "with" in front of properly.
>
> > However, due to the implementation of the SMM BIOS code, there is a time
> > window after the first access to the HPET, which triggers initialization of
> > the SMM code, in which the HPET isn't available. Thus it's necessary to wait
> > until the HPET emulation is ready, and this is what the patch does on the
> > affected machines.
--
Vojtech Pavlik
Director SuSE Labs
Hi Thomas,
Your modification has been checked OK on our SB700 platform. Thank you.
Now could you help us to merge your patch to kernel source? Or do I need
to resubmit a patch on it?
Best regards,
- Crane
On Thu, 2008-08-14 at 18:11 +0800, Thomas Gleixner wrote:
> Crane,
>
> thanks for debugging this. I think we don't need a quirk for this
> workaround. Checking the config register value for 0xffffffff is safe
> on all machines. I simplified your patch to the one below and added a
> printk in case the check times out.
>
> One thing I'm a bit wary about is the readout of the HPET_PERIOD
> register. Is the value correct even _before_ the SMM machinery is
> started ?
> Thanks,
>
> tglx
> -----
> diff --git a/arch/x86/kernel/hpet.c b/arch/x86/kernel/hpet.c
> index ad2b15a..59fd3b6 100644
> --- a/arch/x86/kernel/hpet.c
> +++ b/arch/x86/kernel/hpet.c
> @@ -359,6 +359,7 @@ static int hpet_clocksource_register(void)
> int __init hpet_enable(void)
> {
> unsigned long id;
> + int i;
>
> if (!is_hpet_capable())
> return 0;
> @@ -369,6 +370,29 @@ int __init hpet_enable(void)
> * Read the period and check for a sane value:
> */
> hpet_period = hpet_readl(HPET_PERIOD);
> +
> + /*
> + * AMD SB700 based systems with spread spectrum enabled use a
> + * SMM based HPET emulation to provide proper frequency
> + * setting. The SMM code is initialized with the first HPET
> + * register access and takes some time to complete. During
> + * this time the config register reads 0xffffffff. We check
> + * for max. 1000 loops whether the config register reads a non
> + * 0xffffffff value to make sure that HPET is up and running
> + * before we go further. A counting loop is safe, as the HPET
> + * access takes thousands of CPU cycles. On non SB700 based
> + * machines this check is only done once and has no side
> + * effects.
> + */
> + for (i = 0; hpet_readl(HPET_CFG) == 0xFFFFFFFF; i++) {
> + if (i == 1000) {
> + printk(KERN_WARNING
> + "HPET config register value = 0xFFFFFFFF. "
> + "Disabling HPET\n");
> + goto out_nohpet;
> + }
> + }
> +
> if (hpet_period < HPET_MIN_PERIOD || hpet_period > HPET_MAX_PERIOD)
> goto out_nohpet;
>
* crane cai <[email protected]> wrote:
> Hi Thomas,
>
> Your modification has been checked OK on our SB700 platform. Thank
> you. Now could you help us to merge your patch to kernel source? Or do
> I need to resubmit a patch on it?
it's all fine - it's queued up for v2.6.27 merging, in a few days. You
might want to check the end result in tip/master:
http://people.redhat.com/mingo/tip.git/README
Ingo