2010-02-04 19:11:20

by Andi Kleen

[permalink] [raw]
Subject: [PATCH] HPET: Drop WARN_ON for mismatch on HPET CMP readback

HPET: Drop WARN_ON for mismatch on HPET CMP readback

At least one Intel chipset seems to always return a constant value
when reading back the HPET CMP register. This triggers the WARN_ON_ONCE
on each boot.

In addition the WARN_ON was buggy: it has a side-effect and
actually needed code could be optimized out if someone disabled
CONFIG_BUG.

So far there's no indication that miscompare on reading
points to actual problem.

So simply drop the WARN_ON_ONCE.

Based on discussions with Thomas Gleixner.

Signed-off-by: Andi Kleen <[email protected]>

diff --git a/arch/x86/kernel/hpet.c b/arch/x86/kernel/hpet.c
index ad80a1c..18cf7a6 100644
--- a/arch/x86/kernel/hpet.c
+++ b/arch/x86/kernel/hpet.c
@@ -394,14 +394,11 @@ static int hpet_next_event(unsigned long delta,
* at that point and we would wait for the next hpet interrupt
* forever. We found out that reading the CMP register back
* forces the transfer so we can rely on the comparison with
- * the counter register below. If the read back from the
- * compare register does not match the value we programmed
- * then we might have a real hardware problem. We can not do
- * much about it here, but at least alert the user/admin with
- * a prominent warning.
+ * the counter register below.
+ * But don't actually check the read-back value. Some Intel chipsets
+ * return always the same value.
*/
- WARN_ONCE(hpet_readl(HPET_Tn_CMP(timer)) != cnt,
- KERN_WARNING "hpet: compare register read back failed.\n");
+ (void)hpet_readl(HPET_Tn_CMP(timer);

return (s32)(hpet_readl(HPET_COUNTER) - cnt) >= 0 ? -ETIME : 0;
}


2010-02-04 19:22:17

by Pallipadi, Venkatesh

[permalink] [raw]
Subject: Re: [PATCH] HPET: Drop WARN_ON for mismatch on HPET CMP readback

On Thu, 2010-02-04 at 11:11 -0800, Andi Kleen wrote:
> HPET: Drop WARN_ON for mismatch on HPET CMP readback
>
> At least one Intel chipset seems to always return a constant value
> when reading back the HPET CMP register. This triggers the WARN_ON_ONCE
> on each boot.

I don't know of chipset returning a constant value. Which chipset is
this one?
There is a chipset ICH9/10 errata where it returns a previous value,
instead of most recently written value. I had a patch for that here.

http://marc.info/?l=linux-kernel&m=125849385330302&w=2

I wanted to send out final patch with a pointer to ICH9/10 spec update
which describes the errata. But, look like that spec update hasn't
happened yet. As we found issue with atleast one chipset with this
warn_on, I would prefer retaining it, with a fix like above.

Thanks,
Venki

2010-02-04 19:32:28

by Andi Kleen

[permalink] [raw]
Subject: Re: [PATCH] HPET: Drop WARN_ON for mismatch on HPET CMP readback

On Thu, Feb 04, 2010 at 11:22:15AM -0800, Pallipadi, Venkatesh wrote:
> On Thu, 2010-02-04 at 11:11 -0800, Andi Kleen wrote:
> > HPET: Drop WARN_ON for mismatch on HPET CMP readback
> >
> > At least one Intel chipset seems to always return a constant value
> > when reading back the HPET CMP register. This triggers the WARN_ON_ONCE
> > on each boot.
>
> I don't know of chipset returning a constant value. Which chipset is
> this one?

Ibex Peak. That is I'm not sure it's fully constant, but at least
it's always the same value on the HPET read triggering on boot.

>
> http://marc.info/?l=linux-kernel&m=125849385330302&w=2
>
> I wanted to send out final patch with a pointer to ICH9/10 spec update
> which describes the errata. But, look like that spec update hasn't
> happened yet. As we found issue with atleast one chipset with this
> warn_on, I would prefer retaining it, with a fix like above.

Ok fine for me. I only really care that the ugly backtrace
goes away.

-Andi

--
[email protected] -- Speaking for myself only.

2010-02-04 19:40:45

by Pallipadi, Venkatesh

[permalink] [raw]
Subject: Re: [PATCH] HPET: Drop WARN_ON for mismatch on HPET CMP readback

On Thu, 2010-02-04 at 11:32 -0800, Andi Kleen wrote:
> On Thu, Feb 04, 2010 at 11:22:15AM -0800, Pallipadi, Venkatesh wrote:
> > On Thu, 2010-02-04 at 11:11 -0800, Andi Kleen wrote:
> > > HPET: Drop WARN_ON for mismatch on HPET CMP readback
> > >
> > > At least one Intel chipset seems to always return a constant value
> > > when reading back the HPET CMP register. This triggers the WARN_ON_ONCE
> > > on each boot.
> >
> > I don't know of chipset returning a constant value. Which chipset is
> > this one?
>
> Ibex Peak. That is I'm not sure it's fully constant, but at least
> it's always the same value on the HPET read triggering on boot.

OK. Yes. Ibexpeak is the same problem as ICH9, with reads not returning
the most recent write.

Thanks,
Venki

2010-02-04 19:47:50

by Thomas Gleixner

[permalink] [raw]
Subject: Re: [PATCH] HPET: Drop WARN_ON for mismatch on HPET CMP readback

On Thu, 4 Feb 2010, Pallipadi, Venkatesh wrote:

> On Thu, 2010-02-04 at 11:32 -0800, Andi Kleen wrote:
> > On Thu, Feb 04, 2010 at 11:22:15AM -0800, Pallipadi, Venkatesh wrote:
> > > On Thu, 2010-02-04 at 11:11 -0800, Andi Kleen wrote:
> > > > HPET: Drop WARN_ON for mismatch on HPET CMP readback
> > > >
> > > > At least one Intel chipset seems to always return a constant value
> > > > when reading back the HPET CMP register. This triggers the WARN_ON_ONCE
> > > > on each boot.
> > >
> > > I don't know of chipset returning a constant value. Which chipset is
> > > this one?
> >
> > Ibex Peak. That is I'm not sure it's fully constant, but at least
> > it's always the same value on the HPET read triggering on boot.
>
> OK. Yes. Ibexpeak is the same problem as ICH9, with reads not returning
> the most recent write.

OTOH, none of the modern chipsets needs (hopefully) the read back to
avoid that missing interrupt issue which we saw on ATI.

So we could quite well do without the read back operation and save a
fair amount of cpu cycles that way. We can make it mandatory on those
ATI chips and have a command line option which helps us to debug
fallout or new problem cases.

Thanks,

tglx