x86 setup code rewrite in C

H. Peter Anvin

unread,

Jul 11, 2007, 3:21:32 PM7/11/07

to torv...@linux-foundation.org, an...@firstfloor.org, linux-...@vger.kernel.org

This patch set replaces the x86 setup code, which is currently all in
assembly, with a version written in C, using the ".code16gcc" feature
of binutils (which has been present since at least 2001.)

The new code is vastly easier to read, and, I hope, debug. It should
be noted that I found a fair number of minor bugs while going through
this code, and have attempted to correct them.

In the process of doing so, it introduces several cleanups, in
particular:

- Obsoletes the hd_info field in the boot_params structure; they are
only ever used for ST-506 (pre-IDE) drives and are pretty much
guaranteed to be wrong on current BIOSes;
- Unifies the CPU feature bits between i386 and x86-64. In the
future, it should be possible to use arch/i386/boot/cpucheck.c to do
the post-invocation CPU check currently done in
arch/x86_64/kernel/trampoline.S, although this patch set doesn't
introduce that change.
- boot_params is now a proper structure.

This patchset incorporates all feedback received by 2007-07-11 12:00 PDT.

Jeff Garzik

unread,

Jul 11, 2007, 4:08:50 PM7/11/07

to H. Peter Anvin, torv...@linux-foundation.org, an...@firstfloor.org, linux-...@vger.kernel.org

H. Peter Anvin wrote:
> This patch set replaces the x86 setup code, which is currently all in
> assembly, with a version written in C, using the ".code16gcc" feature
> of binutils (which has been present since at least 2001.)
>
> The new code is vastly easier to read, and, I hope, debug. It should
> be noted that I found a fair number of minor bugs while going through
> this code, and have attempted to correct them.
>
> In the process of doing so, it introduces several cleanups, in
> particular:
>
> - Obsoletes the hd_info field in the boot_params structure; they are
> only ever used for ST-506 (pre-IDE) drives and are pretty much
> guaranteed to be wrong on current BIOSes;
> - Unifies the CPU feature bits between i386 and x86-64. In the
> future, it should be possible to use arch/i386/boot/cpucheck.c to do
> the post-invocation CPU check currently done in
> arch/x86_64/kernel/trampoline.S, although this patch set doesn't
> introduce that change.
> - boot_params is now a proper structure.
>
> This patchset incorporates all feedback received by 2007-07-11 12:00 PDT.

I'm sure you know what's changed since the last revision, but nobody
everybody does :) It would be nice to know what's different from the
last posting.

Jeff

H. Peter Anvin

unread,

Jul 11, 2007, 4:30:36 PM7/11/07

to Jeff Garzik, torv...@linux-foundation.org, an...@firstfloor.org, linux-...@vger.kernel.org

Jeff Garzik wrote:
>
> I'm sure you know what's changed since the last revision, but nobody
> everybody does :) It would be nice to know what's different from the
> last posting.
>

- Added -fno-toplevel-reorder/-fno-unit-at-a-time to the Makefile; this
is necessary for .code16gcc to be safe on all current versions of gcc.
Added comment to that effect to code16gcc.h.
- Moved strnlen, atou and isdigit out of printf.c.
- Corrected title of cpu.c and cpucheck.c.
- Fixed "paragraps" typo in pm.c.
- Removed bogus <linux/edd.h> in string.c.
- Removed #include <linux/mmzone.h> from asm-x86/e820.h; according to
Andi it isn't needed even for non-_SETUP, and my compile testing seems
to confirm that.

Full diff attached.

-hpa

diff

Etienne Lorrain

unread,

Jul 12, 2007, 9:27:29 AM7/12/07

to linux-...@vger.kernel.org, h...@zytor.com

> This patch set replaces the x86 setup code, which is currently all in
> assembly, with a version written in C, using the ".code16gcc" feature
> of binutils (which has been present since at least 2001.)

".code16gcc" is useable since a bit earlier than that, in fact since:
http://sourceware.org/ml/binutils/2000-04/msg00021.html
has been fixed, I am using it every other day.

> The new code is vastly easier to read, and, I hope, debug.

Yes it is, and there is a lot of work in those 33 patch; in very few of
them I would have liked to read something like "inspired by ..." in one of
the C comments (no need to add any E-mail, I already receive enough spam).

Etienne.

http://gujin.org

___________________________________________________________________________
Découvrez une nouvelle façon d'obtenir des réponses à toutes vos questions !
Profitez des connaissances, des opinions et des expériences des internautes sur Yahoo! Questions/Réponses
http://fr.answers.yahoo.com

Linus Torvalds

unread,

Jul 12, 2007, 1:25:44 PM7/12/07

to H. Peter Anvin, an...@firstfloor.org, Linux Kernel Mailing List, Andrew Morton

On Wed, 11 Jul 2007, H. Peter Anvin wrote:
>
> This patch set replaces the x86 setup code, which is currently all in
> assembly, with a version written in C, using the ".code16gcc" feature
> of binutils (which has been present since at least 2001.)
>

> 76 files changed, 4606 insertions(+), 5209 deletions(-)

I can't really argue against this on any sane grounds - not only is it
removing more lines than it adds, but moving from mostly unreadable
assembly to C seems a good idea.

How does this impact the size of that code? Do we even care?

But as to how to integrate it, I'm not sure I really want to just merge
it. I suspect we would want to have it in some public tree that people
actually test at least to some degree first, and the -mm tree seems to
make most sense.

I didn't see anything objectionable in the series, although I do think the
explanations need to be re-done for a number of them. You seem to have
violated the "a single line to explain the patch at the top" rule, and as
a result they make no sense for some of them (the explanation for patch
05/33 doesn't parse for me and 07/33 seems to have the single-line
problem)

So let's just get this merged. But the question is, do we put it in
2.6.23-rc1, or do we put it in -mm for a few weeks, which would imply
waiting for the next merge window? Andrew?

Linus

Andrew Morton

unread,

Jul 12, 2007, 1:32:09 PM7/12/07

to Linus Torvalds, H. Peter Anvin, an...@firstfloor.org, Linux Kernel Mailing List

On Thu, 12 Jul 2007 10:24:48 -0700 (PDT)
Linus Torvalds <torv...@linux-foundation.org> wrote:

>
>
> On Wed, 11 Jul 2007, H. Peter Anvin wrote:
> >
> > This patch set replaces the x86 setup code, which is currently all in
> > assembly, with a version written in C, using the ".code16gcc" feature
> > of binutils (which has been present since at least 2001.)
> >
> > 76 files changed, 4606 insertions(+), 5209 deletions(-)
>
> I can't really argue against this on any sane grounds - not only is it
> removing more lines than it adds, but moving from mostly unreadable
> assembly to C seems a good idea.
>
> How does this impact the size of that code? Do we even care?
>
> But as to how to integrate it, I'm not sure I really want to just merge
> it. I suspect we would want to have it in some public tree that people
> actually test at least to some degree first, and the -mm tree seems to
> make most sense.
>
> I didn't see anything objectionable in the series, although I do think the
> explanations need to be re-done for a number of them. You seem to have
> violated the "a single line to explain the patch at the top" rule, and as
> a result they make no sense for some of them (the explanation for patch
> 05/33 doesn't parse for me and 07/33 seems to have the single-line
> problem)
>
> So let's just get this merged. But the question is, do we put it in
> 2.6.23-rc1, or do we put it in -mm for a few weeks, which would imply
> waiting for the next merge window? Andrew?
>

This code has been in -mm since 11 May, as git-newsetup.patch. It has
caused (for what it is) astonishingly few problems. Maybe a couple of
build glitches and one runtime failure, all quickly fixed.

I'd say it's ready.

Linus Torvalds

unread,

Jul 12, 2007, 1:50:41 PM7/12/07

to Andrew Morton, H. Peter Anvin, an...@firstfloor.org, Linux Kernel Mailing List

On Thu, 12 Jul 2007, Andrew Morton wrote:
>
> This code has been in -mm since 11 May, as git-newsetup.patch. It has
> caused (for what it is) astonishingly few problems. Maybe a couple of
> build glitches and one runtime failure, all quickly fixed.
>
> I'd say it's ready.

Ok. That makes it easy. I'll just merge it.

Linus

Andi Kleen

unread,

Jul 12, 2007, 3:38:38 PM7/12/07

to Linus Torvalds, H. Peter Anvin, an...@firstfloor.org, Linux Kernel Mailing List, Andrew Morton

On Thu, Jul 12, 2007 at 10:24:48AM -0700, Linus Torvalds wrote:
>
>
> On Wed, 11 Jul 2007, H. Peter Anvin wrote:
> >
> > This patch set replaces the x86 setup code, which is currently all in
> > assembly, with a version written in C, using the ".code16gcc" feature
> > of binutils (which has been present since at least 2001.)
> >
> > 76 files changed, 4606 insertions(+), 5209 deletions(-)
>
> I can't really argue against this on any sane grounds - not only is it
> removing more lines than it adds, but moving from mostly unreadable
> assembly to C seems a good idea.

The only thing questionable is that .code16gcc is arguably quite
an abuse of gcc. I even checked with some gcc developers
and they weren't too happy about it. e.g. it's not regression
tested at all so we would be basically on our own with it.

But yes the code looks good.

-Andi

Tilman Schmidt

unread,

Jul 13, 2007, 5:28:08 AM7/13/07

to H. Peter Anvin, torv...@linux-foundation.org, an...@firstfloor.org, linux-...@vger.kernel.org

H. Peter Anvin schrieb:
> From: H. Peter Anvin <h...@zytor.com>
>
> This patch hooks the new x86 setup code into the Makefile machinery. It
> also adapts boot/tools/build.c to a two-file (as opposed to three-file)
> universe, and simplifies it substantially.

[...]

> diff --git a/arch/i386/boot/tools/build.c b/arch/i386/boot/tools/build.c
> index 0579841..886f47d 100644
> --- a/arch/i386/boot/tools/build.c
> +++ b/arch/i386/boot/tools/build.c
> @@ -1,13 +1,12 @@
> /*
> * Copyright (C) 1991, 1992 Linus Torvalds
> * Copyright (C) 1997 Martin Mares
> + * Copyright (C) 2007 H. Peter Anvin
> */
>
> /*
> * This file builds a disk-image from three different files:
> *
> - * - bootsect: compatibility mbr which prints an error message if
> - * someone tries to boot the kernel directly.
> * - setup: 8086 machine code, sets up system parm
> * - system: 80386 code for actual system
> *

You might want to change the word "three" to "two" in the first
line of that comment.

HTH
T.

--
Tilman Schmidt E-Mail: til...@imap.cc
Bonn, Germany
Diese Nachricht besteht zu 100% aus wiederverwerteten Bits.
Ungeöffnet mindestens haltbar bis: (siehe Rückseite)

signature.asc

Etienne Lorrain

unread,

Jul 13, 2007, 10:32:26 AM7/13/07

to linux-...@vger.kernel.org

On Thu, 12 Jul 2007, Linus Torvalds wrote:
> On Thu, 12 Jul 2007, Andrew Morton wrote:
> >
> > This code has been in -mm since 11 May, as git-newsetup.patch. It has
> > caused (for what it is) astonishingly few problems. Maybe a couple of
> > build glitches and one runtime failure, all quickly fixed.
> >
> > I'd say it's ready.
>
> Ok. That makes it easy. I'll just merge it.
>
> Linus

Have fun, this code:
- do not open the fast A20 gate before checking if the slow A20 gate is open or closed.
- uses in asm("") inputs which may or may not be set by the compiler in the stack,
after modifying the stack pointer in the asm block: at least has_eflag()
- The VGA recalc has the same bug as the assembly version where a VGA write protected
register is written (Overflow register) without setting the enable bit (see VGA docs).
- Does not save and restore %ds when printing a char on the screen (%ds is destroyed
only when the content of the screen scroll - only for some video cards)
- Has a "dn" for outl() which sliped in instead of "dN"
and probably few other problems - just seen those by reading the patches (the asm("")
are inlined in the C code - I find it more difficult to check).

Also, I do not know if "m" is right in here:
static inline u8 rdfs8(addr_t addr)
{
u8 v;
asm("movb %%fs:%1,%0" : "=r" (v) : "m" (*(u8 *)addr));
return v;
}

I may repeat me, but to find these kind of problems, it is very nice to have an ELF
file to do a readelf/objdump -D -m i8086 (even after final link).

Etienne.
http://gujin.org

I like that message in this context...
http://marc.info/?l=linux-kernel&m=117077712515509&w=4

_____________________________________________________________________________
Ne gardez plus qu'une seule adresse mail ! Copiez vos mails vers Yahoo! Mail

Etienne Lorrain

unread,

Jul 13, 2007, 10:42:33 AM7/13/07

to linux-...@vger.kernel.org

Andi wrote:
> The only thing questionable is that .code16gcc is arguably quite
> an abuse of gcc. I even checked with some gcc developers
> and they weren't too happy about it. e.g. it's not regression
> tested at all so we would be basically on our own with it.

On the other hand GCC just produces an assembly text file, it is
not the GCC developper problem what the user does with this text file.
Usually it goes to the assembler with standard options - .code16gcc
is a "special" option - and bugs (if there is) should be forwarded to binutils.
GCC tries to go around CPU bugs, and those bug may be different in
protected or real mode - but I still do not have one example of such
a bug.
GCC also has a base library support (multiplication & divisions of
64 bits...), and when you use .code16gcc you know you cannot touch it
(it is assembled with .code32); so that management may be what they
are refering to.

Etienne.

Chuck Ebbert

unread,

Jul 13, 2007, 12:36:38 PM7/13/07

to Etienne Lorrain, linux-...@vger.kernel.org, H. Peter Anvin, Linus Torvalds

On 07/13/2007 10:25 AM, Etienne Lorrain wrote:

[ Added back cc:'s]

H. Peter Anvin

unread,

Jul 13, 2007, 1:34:35 PM7/13/07

to Chuck Ebbert, Etienne Lorrain, linux-...@vger.kernel.org, Linus Torvalds

Chuck Ebbert wrote:
>> Have fun, this code:
>> - do not open the fast A20 gate before checking if the slow A20 gate is open or closed.

As does the current code; this is highly intentional behaviour since
there are machines (in particular a whole series of machines made by
Olivetti) which lock up if you do it differently.

>> - uses in asm("") inputs which may or may not be set by the compiler in the stack,
>> after modifying the stack pointer in the asm block: at least has_eflag()

Point. "g" should be "ri". I will send a patch.

>> - The VGA recalc has the same bug as the assembly version where a VGA write protected
>> register is written (Overflow register) without setting the enable bit (see VGA docs).

OK, that would be a bug ported directly from the assembly version. The
fact that the bug can be seen now is part of why I did this work.
Please feel free to submit a patch.

>> - Does not save and restore %ds when printing a char on the screen (%ds is destroyed
>> only when the content of the screen scroll - only for some video cards)

%ds? Aren't you confusing it with the old bug which would destroy %bp?
If you have any references to %ds being destroyed I would be very
surprised. I can guarantee that very little if any assembly code I've
ever seen that deals with INT 10h -- and I've seen a lot of it -- guards
against %ds being randomly trashed.

However, the trashing of %bp is a well-known bug (although only for
machines older than the ones that can run Linux) -- the Interrupt List has:

BUGS: some implementations (including the original IBM PC) have a bug
which destroys BP

>> - Has a "dn" for outl() which sliped in instead of "dN"

That's a bug, although currently nonmanifest -- there are no users of
outl() at the present. I will send a patch.

>> and probably few other problems - just seen those by reading the patches (the asm("")
>> are inlined in the C code - I find it more difficult to check).
>>
>> Also, I do not know if "m" is right in here:
>> static inline u8 rdfs8(addr_t addr)
>> {
>> u8 v;
>> asm("movb %%fs:%1,%0" : "=r" (v) : "m" (*(u8 *)addr));
>> return v;
>> }

The "m" is correct right there.

>> I may repeat me, but to find these kind of problems, it is very nice to have an ELF
>> file to do a readelf/objdump -D -m i8086 (even after final link).

There is such a file (arch/i386/boot/setup.elf) which is retained, for
exactly this reason.

-hpa

Etienne Lorrain

unread,

Jul 13, 2007, 4:10:57 PM7/13/07

to H. Peter Anvin, Chuck Ebbert, linux-...@vger.kernel.org, Linus Torvalds

--- "H. Peter Anvin" <h...@zytor.com> wrote:
> Chuck Ebbert wrote:

Wrong name.

> >> Have fun, this code:
> >> - do not open the fast A20 gate before checking if the slow A20 gate is open or
> closed.
>
> As does the current code; this is highly intentional behaviour since
> there are machines (in particular a whole series of machines made by
> Olivetti) which lock up if you do it differently.

There was some discussion on this list about some machine which would not wake-up
correctly if slow A20 was not closed, long time ago. I did not really follow
the code after that discussion.
I wonder if you should do a "outb(0xFF, 0x64);" after "outb(0xdf, 0x60);" like
the HIMEM.SYS driver, to force an immediate update of the I/O ports - I think
I also read that in this initial chip docs, long time ago also.

> >> - Does not save and restore %ds when printing a char on the screen (%ds is
> >> destroyed only when the content of the screen scroll - only for some video cards)
>
> %ds? Aren't you confusing it with the old bug which would destroy %bp?
> If you have any references to %ds being destroyed I would be very
> surprised. I can guarantee that very little if any assembly code I've
> ever seen that deals with INT 10h -- and I've seen a lot of it -- guards
> against %ds being randomly trashed.
>
> However, the trashing of %bp is a well-known bug (although only for
> machines older than the ones that can run Linux) -- the Interrupt List has:
>
> BUGS: some implementations (including the original IBM PC) have a bug
> which destroys BP

That is on Trident cards, old card but may still be used, and BIOS may have
been copied to other cards.
Detected and documented on Gujin (boot.c and vgabios.h scroll)

> >> and probably few other problems - just seen those by reading the patches (the
> >> asm("") are inlined in the C code - I find it more difficult to check).
> >>
> >> Also, I do not know if "m" is right in here:
> >> static inline u8 rdfs8(addr_t addr)
> >> {
> >> u8 v;
> >> asm("movb %%fs:%1,%0" : "=r" (v) : "m" (*(u8 *)addr));
> >> return v;
> >> }
>
> The "m" is correct right there.

strange, "g" would mean anything can go there - and this assembly instruction
should accept every access modes.

> >> I may repeat me, but to find these kind of problems, it is very nice to have
> >> an ELF file to do a readelf/objdump -D -m i8086 (even after final link).
>
> There is such a file (arch/i386/boot/setup.elf) which is retained, for
> exactly this reason.
>
> -hpa
>

Etienne.

___________________________________________________________________________
Découvrez une nouvelle façon d'obtenir des réponses à toutes vos questions !
Profitez des connaissances, des opinions et des expériences des internautes sur Yahoo! Questions/Réponses
http://fr.answers.yahoo.com

H. Peter Anvin

unread,

Jul 13, 2007, 5:20:06 PM7/13/07

to Etienne Lorrain, Chuck Ebbert, linux-...@vger.kernel.org, Linus Torvalds

Etienne Lorrain wrote:
> --- "H. Peter Anvin" <h...@zytor.com> wrote:
>> Chuck Ebbert wrote:
>
> Wrong name.
>
>>>> Have fun, this code:
>>>> - do not open the fast A20 gate before checking if the slow A20 gate is open or
>> closed.
>>
>> As does the current code; this is highly intentional behaviour since
>> there are machines (in particular a whole series of machines made by
>> Olivetti) which lock up if you do it differently.
>
> There was some discussion on this list about some machine which would not wake-up
> correctly if slow A20 was not closed, long time ago. I did not really follow
> the code after that discussion.
> I wonder if you should do a "outb(0xFF, 0x64);" after "outb(0xdf, 0x60);" like
> the HIMEM.SYS driver, to force an immediate update of the I/O ports - I think
> I also read that in this initial chip docs, long time ago also.

Well, the code we have now has worked for quite some years as-is. This
code is not an algorithmic change. I haven't seen any machines which
need outb(0xff, 0x64) -- I wonder if that has the same effect as our
io_delay() or if it is actually potent. I shall look into it.

>>>> - Does not save and restore %ds when printing a char on the screen (%ds is
>>>> destroyed only when the content of the screen scroll - only for some video cards)
>> %ds? Aren't you confusing it with the old bug which would destroy %bp?
>> If you have any references to %ds being destroyed I would be very
>> surprised. I can guarantee that very little if any assembly code I've
>> ever seen that deals with INT 10h -- and I've seen a lot of it -- guards
>> against %ds being randomly trashed.
>>
>> However, the trashing of %bp is a well-known bug (although only for
>> machines older than the ones that can run Linux) -- the Interrupt List has:
>>
>> BUGS: some implementations (including the original IBM PC) have a bug
>> which destroys BP
>
> That is on Trident cards, old card but may still be used, and BIOS may have
> been copied to other cards.
> Detected and documented on Gujin (boot.c and vgabios.h scroll)

Are you talking about BP or DS? As I said, the BP is well-known, and
the code accounts for it in the form of the INT10 macro.

>>>> and probably few other problems - just seen those by reading the patches (the
>>>> asm("") are inlined in the C code - I find it more difficult to check).
>>>>
>>>> Also, I do not know if "m" is right in here:
>>>> static inline u8 rdfs8(addr_t addr)
>>>> {
>>>> u8 v;
>>>> asm("movb %%fs:%1,%0" : "=r" (v) : "m" (*(u8 *)addr));
>>>> return v;
>>>> }
>> The "m" is correct right there.
>
> strange, "g" would mean anything can go there - and this assembly instruction
> should accept every access modes.

Not with an %fs: prefix. It would also allow the compiler to do a move
into a register "on its own", which would be disastrous, since it would
lack the prefix. So "m" is correct.

-hpa

H. Peter Anvin

unread,

Jul 13, 2007, 6:24:24 PM7/13/07

to Chuck Ebbert, Etienne Lorrain, linux-...@vger.kernel.org, Linus Torvalds

Chuck Ebbert wrote:
>> - The VGA recalc has the same bug as the assembly version where a VGA write protected
>> register is written (Overflow register) without setting the enable bit (see VGA docs).

I dug into this, and it turns out you're incorrect. Both the assembly
code and the C code are, in fact, 100% correct:

The only instance of writing the vertical overflow register is this code
in vga_set_480_scanlines():

out_idx(0x0c, crtc, 0x11); /* Vertical sync end, unlock CR0-7 */
out_idx(0x0b, crtc, 0x06); /* Vertical total */
out_idx(0x3e, crtc, 0x07); /* Vertical overflow */
out_idx(0xea, crtc, 0x10); /* Vertical sync start */
out_idx(end, crtc, 0x12); /* Vertical display end */
out_idx(0xe7, crtc, 0x15); /* Vertical blank start */
out_idx(0x04, crtc, 0x16); /* Vertical blank end */

Register 0x11 has the Protect (not enable!) bit in it, it is bit 7. As
you can see, it is cleared (meaning writable) at the beginning of this
sequence, and the fact that it's being done is even documented.

-hpa

Linus Torvalds

unread,

Jul 13, 2007, 7:10:02 PM7/13/07

to Etienne Lorrain, H. Peter Anvin, Chuck Ebbert, linux-...@vger.kernel.org

On Fri, 13 Jul 2007, Etienne Lorrain wrote:
>
> > >> Also, I do not know if "m" is right in here:
> > >> static inline u8 rdfs8(addr_t addr)
> > >> {
> > >> u8 v;
> > >> asm("movb %%fs:%1,%0" : "=r" (v) : "m" (*(u8 *)addr));
> > >> return v;
> > >> }
> >
> > The "m" is correct right there.
>
> strange, "g" would mean anything can go there - and this assembly instruction
> should accept every access modes.

No it damn well should NOT.

the %fs: override works only with memory ops. End of story.

Linus

Andrew Hendry

unread,

Jul 15, 2007, 2:43:51 AM7/15/07

to H. Peter Anvin, je...@steeleye.com, linux-...@vger.kernel.org

I tested the 'x86 setup code rewrite revised' patches on a few new
Pentium systems and some virtualized environments without any issues.

Here are some further patches ontop of yours to remove some unused
code and ifdef the MCA setup.

-Andrew

Etienne Lorrain

unread,

Jul 16, 2007, 5:02:29 AM7/16/07

to H. Peter Anvin, Chuck Ebbert, linux-...@vger.kernel.org, Linus Torvalds

--- "H. Peter Anvin" <h...@zytor.com> wrote:
> >>>> - Does not save and restore %ds when printing a char on the screen (%ds is
> >>>> destroyed only when the content of the screen scroll - only for some video cards)
> >> %ds? Aren't you confusing it with the old bug which would destroy %bp?
> >> If you have any references to %ds being destroyed I would be very
> >> surprised. I can guarantee that very little if any assembly code I've
> >> ever seen that deals with INT 10h -- and I've seen a lot of it -- guards
> >> against %ds being randomly trashed.
> >>
> >> However, the trashing of %bp is a well-known bug (although only for
> >> machines older than the ones that can run Linux) -- the Interrupt List has:
> >>
> >> BUGS: some implementations (including the original IBM PC) have a bug
> >> which destroys BP
> >
> > That is on Trident cards, old card but may still be used, and BIOS may have
> > been copied to other cards.
> > Detected and documented on Gujin (boot.c and vgabios.h scroll)
>
> Are you talking about BP or DS? As I said, the BP is well-known, and
> the code accounts for it in the form of the INT10 macro.

Extract of RBIL61:
INT 10 - VIDEO - SCROLL UP WINDOW
AH = 06h
AL = number of lines by which to scroll up (00h = clear entire window)
BH = attribute used to write blank lines at bottom of window
CH,CL = row,column of window's upper left corner
DH,DL = row,column of window's lower right corner
Return: nothing
Note: affects only the currently active page (see AH=05h)

BUGS: some implementations (including the original IBM PC) have a bug which
destroys BP

the Trident TVGA8900CL (BIOS dated 1992/9/8) clears DS to 0000h when
scrolling in an SVGA mode (800x600 or higher)

Scrolling is only (and automatically) done if the cursor is at bottom right.

> >>>> Also, I do not know if "m" is right in here:
> >>>> static inline u8 rdfs8(addr_t addr)
> >>>> {
> >>>> u8 v;
> >>>> asm("movb %%fs:%1,%0" : "=r" (v) : "m" (*(u8 *)addr));
> >>>> return v;
> >>>> }
> >> The "m" is correct right there.
> >
> > strange, "g" would mean anything can go there - and this assembly instruction
> > should accept every access modes.
>
> Not with an %fs: prefix. It would also allow the compiler to do a move
> into a register "on its own", which would be disastrous, since it would
> lack the prefix. So "m" is correct.

"mov %fs:(%ebx,%eax,4),%ecx" works for me.

Etienne.

_____________________________________________________________________________
Ne gardez plus qu'une seule adresse mail ! Copiez vos mails vers Yahoo! Mail

H. Peter Anvin

unread,

Jul 16, 2007, 5:15:35 AM7/16/07

to Etienne Lorrain, Chuck Ebbert, linux-...@vger.kernel.org

Etienne Lorrain wrote:

> BUGS: some implementations (including the original IBM PC) have a bug which
> destroys BP
> the Trident TVGA8900CL (BIOS dated 1992/9/8) clears DS to 0000h when
> scrolling in an SVGA mode (800x600 or higher)

"When scrolling in an SVGA mode", sounds to me like a bug when using
BIOS for text output in graphics mode. We don't do that.

>>>>>> Also, I do not know if "m" is right in here:
>>>>>> static inline u8 rdfs8(addr_t addr)
>>>>>> {
>>>>>> u8 v;
>>>>>> asm("movb %%fs:%1,%0" : "=r" (v) : "m" (*(u8 *)addr));
>>>>>> return v;
>>>>>> }
>>>> The "m" is correct right there.
>>> strange, "g" would mean anything can go there - and this assembly instruction
>>> should accept every access modes.
>> Not with an %fs: prefix. It would also allow the compiler to do a move
>> into a register "on its own", which would be disastrous, since it would
>> lack the prefix. So "m" is correct.
>
> "mov %fs:(%ebx,%eax,4),%ecx" works for me.

That's an example on what "m" can generate.

"g" could produce stuff like:

mov %fs:$1234,%ecx
mov %fs:%eax,%ecx

-hpa

Etienne Lorrain

unread,

Jul 16, 2007, 6:22:33 AM7/16/07

to H. Peter Anvin, Chuck Ebbert, linux-...@vger.kernel.org

--- "H. Peter Anvin" <h...@zytor.com> wrote:

> Etienne Lorrain wrote:
> > "mov %fs:(%ebx,%eax,4),%ecx" works for me.
>
> That's an example on what "m" can generate.
>
> "g" could produce stuff like:
>
> mov %fs:$1234,%ecx
> mov %fs:%eax,%ecx

My solution was to use %fs:%a1 instead of %fs:%1 , but after checking your
solution produces better assembly.

Etienne.

_____________________________________________________________________________
Ne gardez plus qu'une seule adresse mail ! Copiez vos mails vers Yahoo! Mail

Etienne Lorrain

unread,

Jul 16, 2007, 9:32:10 AM7/16/07

to H. Peter Anvin, Chuck Ebbert, linux-...@vger.kernel.org, Linus Torvalds

--- "H. Peter Anvin" <h...@zytor.com> wrote:
> >> - The VGA recalc has the same bug as the assembly version where a VGA
> >> write protected register is written (Overflow register) without setting
> >> the enable bit (see VGA docs).
>
> I dug into this, and it turns out you're incorrect. Both the assembly
> code and the C code are, in fact, 100% correct:
>
> The only instance of writing the vertical overflow register is this code
> in vga_set_480_scanlines():
>
> out_idx(0x0c, crtc, 0x11); /* Vertical sync end, unlock CR0-7 */
> out_idx(0x0b, crtc, 0x06); /* Vertical total */
> out_idx(0x3e, crtc, 0x07); /* Vertical overflow */
> out_idx(0xea, crtc, 0x10); /* Vertical sync start */
> out_idx(end, crtc, 0x12); /* Vertical display end */
> out_idx(0xe7, crtc, 0x15); /* Vertical blank start */
> out_idx(0x04, crtc, 0x16); /* Vertical blank end */
>
> Register 0x11 has the Protect (not enable!) bit in it, it is bit 7. As
> you can see, it is cleared (meaning writable) at the beginning of this
> sequence, and the fact that it's being done is even documented.

The only time I ever needed this "end line recalculation" was when the heigh in
graphic lines was not a multiple of the character heigh - i.e. 640x350 with 8x16
or 8x8 chars - some VGA adapters do not hide the bottom graphic lines.
The function vga_set_480_scanlines() is not called, and the protect bit is never
cleared - the video BIOS leaving those low index register protected.
The function vga_recalc_vertical() (or its assembler equivalent) is probably
perfectly called but because the protect bit is never cleared, the few graphic
line are displayed during the whole Linux text session...
I have myself never seen any other problems when the graphic heigh is a multiple
of the character heigh - tested on ~40 video boards.

Etienne.

___________________________________________________________________________
Découvrez une nouvelle façon d'obtenir des réponses à toutes vos questions !
Profitez des connaissances, des opinions et des expériences des internautes sur Yahoo! Questions/Réponses
http://fr.answers.yahoo.com

H. Peter Anvin

unread,

Jul 16, 2007, 1:36:02 PM7/16/07

to Etienne Lorrain, Chuck Ebbert, linux-...@vger.kernel.org

Etienne Lorrain wrote:
>
> The only time I ever needed this "end line recalculation" was when the heigh in
> graphic lines was not a multiple of the character heigh - i.e. 640x350 with 8x16
> or 8x8 chars - some VGA adapters do not hide the bottom graphic lines.
> The function vga_set_480_scanlines() is not called, and the protect bit is never
> cleared - the video BIOS leaving those low index register protected.
> The function vga_recalc_vertical() (or its assembler equivalent) is probably
> perfectly called but because the protect bit is never cleared, the few graphic
> line are displayed during the whole Linux text session...
> I have myself never seen any other problems when the graphic heigh is a multiple
> of the character heigh - tested on ~40 video boards.
>

OK, I see what you mean. This would be a problem if:

- the VGA BIOS leaves the protected bit set
- the user enables vertical recalculation
- the size crosses a multiple of 256

It is a bug (ported from the assembly) and fortunately quite easy to
fix. I don't know why I missed this when I looked before.

-hpa

x86 setup code rewrite in C - revised

H. Peter Anvin

Jeff Garzik

H. Peter Anvin

Etienne Lorrain

Linus Torvalds

Andrew Morton

Linus Torvalds

Andi Kleen

Tilman Schmidt

Etienne Lorrain

Etienne Lorrain

Chuck Ebbert

H. Peter Anvin

Etienne Lorrain

H. Peter Anvin

H. Peter Anvin

Linus Torvalds

Andrew Hendry

Etienne Lorrain

H. Peter Anvin

Etienne Lorrain

Etienne Lorrain

H. Peter Anvin