public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: "H. Peter Anvin" <hpa@zytor.com>
To: Chuck Ebbert <cebbert@redhat.com>
Cc: Etienne Lorrain <etienne_lorrain@yahoo.fr>,
	linux-kernel@vger.kernel.org,
	Linus Torvalds <torvalds@linux-foundation.org>
Subject: Re: x86 setup code rewrite in C - revised
Date: Fri, 13 Jul 2007 09:51:56 -0700	[thread overview]
Message-ID: <4697ADAC.4070009@zytor.com> (raw)
In-Reply-To: <4697A9E9.3090502@redhat.com>

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

  reply	other threads:[~2007-07-13 17:34 UTC|newest]

Thread overview: 21+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2007-07-13 14:25 x86 setup code rewrite in C - revised Etienne Lorrain
2007-07-13 16:35 ` Chuck Ebbert
2007-07-13 16:51   ` H. Peter Anvin [this message]
2007-07-13 20:10     ` RE : " Etienne Lorrain
2007-07-13 21:19       ` H. Peter Anvin
2007-07-16  9:02         ` RE : " Etienne Lorrain
2007-07-16  9:15           ` H. Peter Anvin
2007-07-16 10:21             ` Etienne Lorrain
2007-07-13 23:09       ` RE : " Linus Torvalds
2007-07-13 22:23   ` H. Peter Anvin
2007-07-16 13:31     ` Etienne Lorrain
2007-07-16 17:35       ` H. Peter Anvin
  -- strict thread matches above, loose matches on Subject: below --
2007-07-13 14:42 Etienne Lorrain
2007-07-12 13:18 Etienne Lorrain
2007-07-11 19:18 H. Peter Anvin
2007-07-11 20:08 ` Jeff Garzik
2007-07-11 20:29   ` H. Peter Anvin
2007-07-12 17:24 ` Linus Torvalds
2007-07-12 17:30   ` Andrew Morton
2007-07-12 17:49     ` Linus Torvalds
2007-07-12 19:38   ` Andi Kleen

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=4697ADAC.4070009@zytor.com \
    --to=hpa@zytor.com \
    --cc=cebbert@redhat.com \
    --cc=etienne_lorrain@yahoo.fr \
    --cc=linux-kernel@vger.kernel.org \
    --cc=torvalds@linux-foundation.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox