x86: use explicit timing delay for pit accesses in kernel and pcspkr driver
pit accesses in i8253.c and pcspkr driver use outb_p for timing.
Fix them to use explicit timing delay for access to PIT,
rather than inb_p/outb_p calls, which use insufficiently explicit
delays (defaulting to port 80 writes) that can cause freeze problems
on some machines, such as Quanta moterboard machines using ENE EC's.
Since the pcspkr driver accesses PIT registers directly, it should also
use outb_pit, which is inlined, so does not need bo exported.
Explicit timing delay is only needed in pcspkr for accesses to the 8253 PIT.
Fix pcspkr driver to use the new outb_pic call properly, use
named PIC port values rather than hex constants, and drop its use of
inb_p and outb_p in accessing port 61h where it has never been needed.
Signed-off-by: David P. Reed <[email protected]>
Index: linux-2.6/drivers/input/misc/pcspkr.c
===================================================================
--- linux-2.6.orig/drivers/input/misc/pcspkr.c
+++ linux-2.6/drivers/input/misc/pcspkr.c
@@ -36,6 +36,7 @@ static int pcspkr_event(struct input_dev
{
unsigned int count = 0;
unsigned long flags;
+ unsigned char port61;
if (type != EV_SND)
return -1;
@@ -51,17 +52,18 @@ static int pcspkr_event(struct input_dev
spin_lock_irqsave(&i8253_lock, flags);
+ port61 = inb(0x61);
if (count) {
/* enable counter 2 */
- outb_p(inb_p(0x61) | 3, 0x61);
+ outb(port61 | 3, 0x61);
/* set command for counter 2, 2 byte write */
- outb_p(0xB6, 0x43);
+ outb_pit(0xB6, PIT_MODE);
/* select desired HZ */
- outb_p(count & 0xff, 0x42);
- outb((count >> 8) & 0xff, 0x42);
+ outb_pit(count & 0xff, PIT_CH2);
+ outb((count >> 8) & 0xff, PIT_CH2);
} else {
/* disable counter 2 */
- outb(inb_p(0x61) & 0xFC, 0x61);
+ outb(port61 & 0xFC, 0x61);
}
spin_unlock_irqrestore(&i8253_lock, flags);
Index: linux-2.6/include/asm-x86/i8253.h
===================================================================
--- linux-2.6.orig/include/asm-x86/i8253.h
+++ linux-2.6/include/asm-x86/i8253.h
@@ -12,7 +12,25 @@ extern struct clock_event_device *global
extern void setup_pit_timer(void);
-#define inb_pit inb_p
-#define outb_pit outb_p
+/* accesses to PIT registers need careful delays on some platforms. Define
+ them here in a common place */
+static inline unsigned char inb_pit(unsigned int port)
+{
+ /* delay for some accesses to PIT on motherboard or in chipset must be
+ at least one microsecond, but be safe here. */
+ unsigned char value = inb(port);
+ udelay(2);
+ return value;
+}
+
+static inline void outb_pit(unsigned char value, unsigned int port)
+{
+ /* delay for some accesses to PIT on motherboard or in chipset must be
+ at least one microsecond, but be safe here. */
+ outb(value, port);
+ udelay(2);
+}
+
+
#endif /* __ASM_I8253_H__ */
On Mon, 18 Feb 2008 13:58:41 -0500 (EST)
"David P. Reed" <[email protected]> wrote:
> x86: use explicit timing delay for pit accesses in kernel and pcspkr driver
Both look good to me now
On 18-02-08 19:58, David P. Reed wrote:
> --- linux-2.6.orig/include/asm-x86/i8253.h
> +++ linux-2.6/include/asm-x86/i8253.h
> @@ -12,7 +12,25 @@ extern struct clock_event_device *global
>
> extern void setup_pit_timer(void);
>
> -#define inb_pit inb_p
> -#define outb_pit outb_p
> +/* accesses to PIT registers need careful delays on some platforms. Define
> + them here in a common place */
> +static inline unsigned char inb_pit(unsigned int port)
> +{
> + /* delay for some accesses to PIT on motherboard or in chipset must be
> + at least one microsecond, but be safe here. */
> + unsigned char value = inb(port);
> + udelay(2);
> + return value;
> +}
With the remark that (at least) the PIT is accessed at a time that
microseconds and hence udelay are still a total fiction, this looks obvious
otherwise.
Now with respect to the original pre port 80 "jmp $+2" I/O delay (which the
Pentium obsoleted) I suppose it'll probably be okay even without fixing that
specifically but do note such -- it's a vital part of the problem.
Rene.
Rene Herman wrote:
>
> Now with respect to the original pre port 80 "jmp $+2" I/O delay (which
> the Pentium obsoleted) I suppose it'll probably be okay even without
> fixing that specifically but do note such -- it's a vital part of the
> problem.
>
Sorry, that paragraph didn't parse for me.
-hpa
On 18-02-08 21:43, H. Peter Anvin wrote:
> Rene Herman wrote:
>>
>> Now with respect to the original pre port 80 "jmp $+2" I/O delay
>> (which the Pentium obsoleted) I suppose it'll probably be okay even
>> without fixing that specifically but do note such -- it's a vital part
>> of the problem.
>>
>
> Sorry, that paragraph didn't parse for me.
I mean that before the linux kernel used a port 0x80 write as an I/O delay
it used a short jump (two in a row actually...) as such and this was at the
time that it actually ran on the old legacy stuff that is of most concern here.
No, if I'm not mistaken, those two jumps are actually what the udelay() is
going to do anyway as part of delay_loop() at that early stage so that even
before loops_per_jiffy calibration, I believe we should still be okay.
Yes, it's a bit of a "well, hrrm" thing, but, well... loops_per_jiffy can be
initialised a bit more conservatively then today as well (and as discussed
earlier, possibly per CPU family) but I believe it's actually sort of fine
not too worry much about it...
Rene.
On 18-02-08 22:04, Rene Herman wrote:
> On 18-02-08 21:43, H. Peter Anvin wrote:
>
>> Rene Herman wrote:
>>>
>>> Now with respect to the original pre port 80 "jmp $+2" I/O delay
>>> (which the Pentium obsoleted) I suppose it'll probably be okay even
>>> without fixing that specifically but do note such -- it's a vital
>>> part of the problem.
>>>
>>
>> Sorry, that paragraph didn't parse for me.
>
> I mean that before the linux kernel used a port 0x80 write as an I/O
> delay it used a short jump (two in a row actually...) as such and this
> was at the time that it actually ran on the old legacy stuff that is of
> most concern here.
>
> No, if I'm not mistaken, those two jumps are actually what the udelay()
_Now_, if I'm ...
> is going to do anyway as part of delay_loop() at that early stage so
> that even before loops_per_jiffy calibration, I believe we should still
> be okay.
>
> Yes, it's a bit of a "well, hrrm" thing, but, well... loops_per_jiffy
> can be initialised a bit more conservatively then today as well (and as
> discussed earlier, possibly per CPU family) but I believe it's actually
> sort of fine not too worry much about it...
Rene.
Rene Herman wrote:
>
> I mean that before the linux kernel used a port 0x80 write as an I/O
> delay it used a short jump (two in a row actually...) as such and this
> was at the time that it actually ran on the old legacy stuff that is of
> most concern here.
>
> No, if I'm not mistaken, those two jumps are actually what the udelay()
> is going to do anyway as part of delay_loop() at that early stage so
> that even before loops_per_jiffy calibration, I believe we should still
> be okay.
>
That doesn't make any sense at all. The whole point why the two jumps
were obsoleted with the P5 (or even late P4, if I'm not mistaken) was
because they were utterly insufficient when the CPU ran at something
much higher than the external speed.
> Yes, it's a bit of a "well, hrrm" thing, but, well... loops_per_jiffy
> can be initialised a bit more conservatively then today as well (and as
> discussed earlier, possibly per CPU family) but I believe it's actually
> sort of fine not too worry much about it...
Uhm... no. Quite the contrary, I would say.
-hpa
On 18-02-08 22:44, H. Peter Anvin wrote:
> Rene Herman wrote:
>>
>> I mean that before the linux kernel used a port 0x80 write as an I/O
>> delay it used a short jump (two in a row actually...) as such and this
>> was at the time that it actually ran on the old legacy stuff that is
>> of most concern here.
>>
>> No, if I'm not mistaken, those two jumps are actually what the
>> udelay() is going to do anyway as part of delay_loop() at that early
>> stage so that even before loops_per_jiffy calibration, I believe we
>> should still be okay.
>>
>
> That doesn't make any sense at all. The whole point why the two jumps
> were obsoleted with the P5 (or even late P4, if I'm not mistaken) was
> because they were utterly insufficient when the CPU ran at something
> much higher than the external speed.
Yes, but generally not any P5+ system is going to need the PIT delay in the
first place meaning it just doesn't matter. There were the VIA issues with
the PIC but unless I missed it not with the PIT.
That's the point. It's fairly unclean to say udelay(2) and then not delay
for 2 microseconds but you _have_ done the two short jumps meaning 386 and
486 systems are okay and later systems were okay to start with.
Rene.
Rene Herman wrote:
>
> Yes, but generally not any P5+ system is going to need the PIT delay in
> the first place meaning it just doesn't matter. There were the VIA
> issues with the PIC but unless I missed it not with the PIT.
>
Uhm, I'm not sure I believe that's safe.
The PIT is particularly pissy in this case -- the semantics of the PIT
are ill-defined if there hasn't been a PIT clock between two adjacent
accesses, so I fully expect that there are chipsets out there which will
do very bad things in this case.
-hpa
On 18-02-08 23:01, H. Peter Anvin wrote:
> Rene Herman wrote:
>>
>> Yes, but generally not any P5+ system is going to need the PIT delay
>> in the first place meaning it just doesn't matter. There were the VIA
>> issues with the PIC but unless I missed it not with the PIT.
>>
>
> Uhm, I'm not sure I believe that's safe.
>
> The PIT is particularly pissy in this case -- the semantics of the PIT
> are ill-defined if there hasn't been a PIT clock between two adjacent
> accesses, so I fully expect that there are chipsets out there which will
> do very bad things in this case.
Okay. Now that they're isolated, do you have a suggestion for {in,out}b_pit?
You say a PIT clock, so do you think we can bounce of the PIT iself in this
case after all?
Rene.
On 18-02-08 23:07, Rene Herman wrote:
> On 18-02-08 23:01, H. Peter Anvin wrote:
>
>> Rene Herman wrote:
>>>
>>> Yes, but generally not any P5+ system is going to need the PIT delay
>>> in the first place meaning it just doesn't matter. There were the VIA
>>> issues with the PIC but unless I missed it not with the PIT.
>>>
>>
>> Uhm, I'm not sure I believe that's safe.
>>
>> The PIT is particularly pissy in this case -- the semantics of the PIT
>> are ill-defined if there hasn't been a PIT clock between two adjacent
>> accesses, so I fully expect that there are chipsets out there which
>> will do very bad things in this case.
>
> Okay. Now that they're isolated, do you have a suggestion for
> {in,out}b_pit? You say a PIT clock, so do you think we can bounce of the
> PIT iself in this case after all?
Am I correct that channel 1 is never used? A simple read from 0x41?
Rene.
Rene Herman wrote:
>>
>> Uhm, I'm not sure I believe that's safe.
>>
>> The PIT is particularly pissy in this case -- the semantics of the PIT
>> are ill-defined if there hasn't been a PIT clock between two adjacent
>> accesses, so I fully expect that there are chipsets out there which
>> will do very bad things in this case.
>
> Okay. Now that they're isolated, do you have a suggestion for
> {in,out}b_pit? You say a PIT clock, so do you think we can bounce of the
> PIT iself in this case after all?
No, I don't think so.
-hpa
Rene Herman wrote:
> On 18-02-08 23:07, Rene Herman wrote:
>
>> On 18-02-08 23:01, H. Peter Anvin wrote:
>>
>>> Rene Herman wrote:
>>>>
>>>> Yes, but generally not any P5+ system is going to need the PIT delay
>>>> in the first place meaning it just doesn't matter. There were the
>>>> VIA issues with the PIC but unless I missed it not with the PIT.
>>>>
>>>
>>> Uhm, I'm not sure I believe that's safe.
>>>
>>> The PIT is particularly pissy in this case -- the semantics of the
>>> PIT are ill-defined if there hasn't been a PIT clock between two
>>> adjacent accesses, so I fully expect that there are chipsets out
>>> there which will do very bad things in this case.
>>
>> Okay. Now that they're isolated, do you have a suggestion for
>> {in,out}b_pit? You say a PIT clock, so do you think we can bounce of
>> the PIT iself in this case after all?
>
> Am I correct that channel 1 is never used? A simple read from 0x41?
>
Channel 1 is available for the system. In modern systems, it's pretty
much available for the OS, although that's never formally stated (in the
original PC, it was used for DRAM refresh.)
However, I could very easily see a chipset have issues with that kind of
stuff.
-hpa
* David P. Reed <[email protected]> wrote:
> x86: use explicit timing delay for pit accesses in kernel and pcspkr
> driver
thanks, applied.
Ingo
commit 9c679215248e837b34242632d5a22adf9a247021
Author: Rene Herman <[email protected]>
Date: Wed Feb 20 12:52:30 2008 +0100
x86: per CPU family loops_per_jiffy initialization
Following the current port 0x80 I/O delay replacements we need
microseconds to be somewhat usefully defined pre calibration.
Initialize 386, 486 and Pentium 1 as fastest in their families
and higher CPUs (including 64-bit) at 1 Ghz. Note that trouble
should be absent past family 5 systems anyway.
Signed-off-by: Rene Herman <[email protected]>
diff --git a/arch/x86/kernel/time_32.c b/arch/x86/kernel/time_32.c
index 1a89e93..e33e70b 100644
--- a/arch/x86/kernel/time_32.c
+++ b/arch/x86/kernel/time_32.c
@@ -32,6 +32,7 @@
#include <linux/interrupt.h>
#include <linux/time.h>
#include <linux/mca.h>
+#include <linux/delay.h>
#include <asm/arch_hooks.h>
#include <asm/hpet.h>
@@ -134,6 +135,17 @@ void __init hpet_time_init(void)
*/
void __init time_init(void)
{
+ switch (boot_cpu_data.x86) {
+ case 3:
+ loops_per_jiffy = LOOPS_PER_JIFFY_386;
+ break;
+ case 4:
+ loops_per_jiffy = LOOPS_PER_JIFFY_486;
+ break;
+ case 5:
+ loops_per_jiffy = LOOPS_PER_JIFFY_586;
+ break;
+ }
tsc_init();
late_time_init = choose_time_init();
}
diff --git a/include/asm-x86/delay.h b/include/asm-x86/delay.h
index 409a649..d0fbaf6 100644
--- a/include/asm-x86/delay.h
+++ b/include/asm-x86/delay.h
@@ -7,6 +7,11 @@
* Delay routines calling functions in arch/x86/lib/delay.c
*/
+#define LOOPS_PER_JIFFY_386 (4000000 / HZ) /* 386 at 40 Mhz */
+#define LOOPS_PER_JIFFY_486 (30000000 / HZ) /* 486 at 120 MHz */
+#define LOOPS_PER_JIFFY_586 (233000000 / HZ) /* Pentium 1 at 233 Mhz */
+#define LOOPS_PER_JIFFY (1000000000 / HZ) /* P6+ at 1 GHz */
+
/* Undefined functions to get compile-time errors */
extern void __bad_udelay(void);
extern void __bad_ndelay(void);
diff --git a/init/main.c b/init/main.c
index 8b19820..94862c8 100644
--- a/init/main.c
+++ b/init/main.c
@@ -228,12 +228,11 @@ static int __init obsolete_checksetup(char *line)
return had_early_param;
}
-/*
- * This should be approx 2 Bo*oMips to start (note initial shift), and will
- * still work even if initially too large, it will just take slightly longer
- */
-unsigned long loops_per_jiffy = (1<<12);
+#ifndef LOOPS_PER_JIFFY
+#define LOOPS_PER_JIFFY (1 << 12)
+#endif
+unsigned long loops_per_jiffy = LOOPS_PER_JIFFY;
EXPORT_SYMBOL(loops_per_jiffy);
static int __init debug_kernel(char *str)
Rene Herman wrote:
>
> _Something_ like this would seem to be the only remaining option. It
> seems fairly unuseful to #ifdef around that switch statement for kernels
> without support for the earlier families, but if you insist...
>
"Only remaining option" other than the one we've had all along. Even on
the one idiotic set of systems which break, it only breaks post-ACPI
intialization, IIRC.
-hpa
On 20-02-08 18:05, H. Peter Anvin wrote:
> Rene Herman wrote:
>>
>> _Something_ like this would seem to be the only remaining option. It
>> seems fairly unuseful to #ifdef around that switch statement for
>> kernels without support for the earlier families, but if you insist...
>>
>
> "Only remaining option" other than the one we've had all along. Even on
> the one idiotic set of systems which break, it only breaks post-ACPI
> intialization, IIRC.
Linus vetoed the DMI switch.
Rene.
Actually, disparaging things as "one idiotic system" doesn't seem like a
long-term thoughtful process - it's not even accurate. There are more
such systems that are running code today than the total number of 486
systems ever manufactured. The production rate is $1M/month.
a) ENE chips are "documented" to receive port 80, and also it is the
case that modern chipsets will happily diagnose writes to non-existent
ports as MCE's. Using side effects that depend on non-existent ports
just creates a brittle failure mode down the road. And it's not just
post ACPI "initialization". The pcspkr use of port 80 caused solid
freezes if you typed "tab" to complete a command line and there were
more than one choice, leading to beeps.
b) sad to say, Linux is not what hardware vendors use as the system that
their BIOSes MUST work with. That's Windows, and Windows, whether we
like it or not does not require hardware vendors to stay away from port 80.
IMHO, calling something "idiotic" is hardly evidence-based decision
making. Maybe you love to hate Microsoft, but until Intel writes an
architecture standard that says explicitly that a "standard PC" must not
use port 80 for any peripheral, the port 80 thing is folklore, and one
that is solely Linux-defined.
Rene Herman wrote:
> On 20-02-08 18:05, H. Peter Anvin wrote:
>
>> Rene Herman wrote:
>>>
>>> _Something_ like this would seem to be the only remaining option. It
>>> seems fairly unuseful to #ifdef around that switch statement for
>>> kernels without support for the earlier families, but if you insist...
>>>
>>
>> "Only remaining option" other than the one we've had all along. Even
>> on the one idiotic set of systems which break, it only breaks
>> post-ACPI intialization, IIRC.
>
> Linus vetoed the DMI switch.
>
> Rene.
>
commit 9c679215248e837b34242632d5a22adf9a247021
Author: Rene Herman <[email protected]>
Date: Wed Feb 20 12:52:30 2008 +0100
x86: per CPU family loops_per_jiffy initialization
Following the current port 0x80 I/O delay replacements we need
microseconds to be somewhat usefully defined pre calibration.
Initialize 386, 486 and Pentium 1 as fastest in their families
and higher CPUs (including 64-bit) at 1 Ghz. Note that trouble
should be absent past family 5 systems anyway.
Signed-off-by: Rene Herman <[email protected]>
diff --git a/arch/x86/kernel/time_32.c b/arch/x86/kernel/time_32.c
index 1a89e93..e33e70b 100644
--- a/arch/x86/kernel/time_32.c
+++ b/arch/x86/kernel/time_32.c
@@ -32,6 +32,7 @@
#include <linux/interrupt.h>
#include <linux/time.h>
#include <linux/mca.h>
+#include <linux/delay.h>
#include <asm/arch_hooks.h>
#include <asm/hpet.h>
@@ -134,6 +135,17 @@ void __init hpet_time_init(void)
*/
void __init time_init(void)
{
+ switch (boot_cpu_data.x86) {
+ case 3:
+ loops_per_jiffy = LOOPS_PER_JIFFY_386;
+ break;
+ case 4:
+ loops_per_jiffy = LOOPS_PER_JIFFY_486;
+ break;
+ case 5:
+ loops_per_jiffy = LOOPS_PER_JIFFY_586;
+ break;
+ }
tsc_init();
late_time_init = choose_time_init();
}
diff --git a/include/asm-x86/delay.h b/include/asm-x86/delay.h
index 409a649..d0fbaf6 100644
--- a/include/asm-x86/delay.h
+++ b/include/asm-x86/delay.h
@@ -7,6 +7,11 @@
* Delay routines calling functions in arch/x86/lib/delay.c
*/
+#define LOOPS_PER_JIFFY_386 (4000000 / HZ) /* 386 at 40 Mhz */
+#define LOOPS_PER_JIFFY_486 (30000000 / HZ) /* 486 at 120 MHz */
+#define LOOPS_PER_JIFFY_586 (233000000 / HZ) /* Pentium 1 at 233 Mhz */
+#define LOOPS_PER_JIFFY (1000000000 / HZ) /* P6+ at 1 GHz */
+
/* Undefined functions to get compile-time errors */
extern void __bad_udelay(void);
extern void __bad_ndelay(void);
diff --git a/init/main.c b/init/main.c
index 8b19820..94862c8 100644
--- a/init/main.c
+++ b/init/main.c
@@ -228,12 +228,11 @@ static int __init obsolete_checksetup(char *line)
return had_early_param;
}
-/*
- * This should be approx 2 Bo*oMips to start (note initial shift), and will
- * still work even if initially too large, it will just take slightly longer
- */
-unsigned long loops_per_jiffy = (1<<12);
+#ifndef LOOPS_PER_JIFFY
+#define LOOPS_PER_JIFFY (1 << 12)
+#endif
+unsigned long loops_per_jiffy = LOOPS_PER_JIFFY;
EXPORT_SYMBOL(loops_per_jiffy);
static int __init debug_kernel(char *str)