Boot failure in 2.5.31 BK with new TLS patch

Boot failure in 2.5.31 BK with new TLS patch

[James Bottomley]

This is probably local to me since I've got a box which takes quad CPU cards 
that has always been very picky about the GDT layout at boot.  However, it's 
been failing miserably with the new padding the TLS stuff introduces into the 
boot time GDT.

I attach a patch that leaves the boot time %cs and %ds at their old values 
(0x10 and 0x18), and shifts to the new GDT layout after the switch to 
protected mode has been accomplished.

For those who don't have this GDT boot problem, it reduces the size of the 
boot code by about 64 bytes.

James

You can import this changeset into BK by piping this whole message to:
'| bk receive [path to repository]' or apply the patch as usual.

===================================================================


ChangeSet@1.486, 2002-08-16 18:23:02-05:00, jejb@mulgrave.(none)
  Separate boot GDT from runtime GDT.  Allow small compact GDT at boot
  for those PCs which have problems with big ones during the protected
  mode transition.


 arch/i386/boot/compressed/head.S |    8 ++++----
 arch/i386/boot/compressed/misc.c |    2 +-
 arch/i386/boot/setup.S           |   15 +++++++--------
 arch/i386/kernel/head.S          |   15 +++++++++++++--
 arch/i386/kernel/trampoline.S    |    6 +++---
 include/asm-i386/segment.h       |    8 ++++++++
 6 files changed, 36 insertions(+), 18 deletions(-)


diff -Nru a/arch/i386/boot/compressed/head.S b/arch/i386/boot/compressed/head.S
--- a/arch/i386/boot/compressed/head.S	Fri Aug 16 19:29:20 2002
+++ b/arch/i386/boot/compressed/head.S	Fri Aug 16 19:29:20 2002
@@ -31,7 +31,7 @@
 startup_32:
 	cld
 	cli
-	movl $(__KERNEL_DS),%eax
+	movl $(__BOOT_DS),%eax
 	movl %eax,%ds
 	movl %eax,%es
 	movl %eax,%fs
@@ -74,7 +74,7 @@
 	popl %esi	# discard address
 	popl %esi	# real mode pointer
 	xorl %ebx,%ebx
-	ljmp $(__KERNEL_CS), $0x100000
+	ljmp $(__BOOT_CS), $0x100000
 
 /*
  * We come here, if we were loaded high.
@@ -101,7 +101,7 @@
 	popl %eax	# hcount
 	movl $0x100000,%edi
 	cli		# make sure we don't get interrupted
-	ljmp $(__KERNEL_CS), $0x1000 # and jump to the move routine
+	ljmp $(__BOOT_CS), $0x1000 # and jump to the move routine
 
 /*
  * Routine (template) for moving the decompressed kernel in place,
@@ -124,5 +124,5 @@
 	movsl
 	movl %ebx,%esi	# Restore setup pointer
 	xorl %ebx,%ebx
-	ljmp $(__KERNEL_CS), $0x100000
+	ljmp $(__BOOT_CS), $0x100000
 move_routine_end:
diff -Nru a/arch/i386/boot/compressed/misc.c b/arch/i386/boot/compressed/misc.c
--- a/arch/i386/boot/compressed/misc.c	Fri Aug 16 19:29:20 2002
+++ b/arch/i386/boot/compressed/misc.c	Fri Aug 16 19:29:20 2002
@@ -299,7 +299,7 @@
 struct {
 	long * a;
 	short b;
-	} stack_start = { & user_stack [STACK_SIZE] , __KERNEL_DS };
+	} stack_start = { & user_stack [STACK_SIZE] , __BOOT_DS };
 
 static void setup_normal_output_buffer(void)
 {
diff -Nru a/arch/i386/boot/setup.S b/arch/i386/boot/setup.S
--- a/arch/i386/boot/setup.S	Fri Aug 16 19:29:20 2002
+++ b/arch/i386/boot/setup.S	Fri Aug 16 19:29:20 2002
@@ -801,7 +801,7 @@
 	subw	$DELTA_INITSEG, %si
 	shll	$4, %esi			# Convert to 32-bit pointer
 # NOTE: For high loaded big kernels we need a
-#	jmpi    0x100000,__KERNEL_CS
+#	jmpi    0x100000,__BOOT_CS
 #
 #	but we yet haven't reloaded the CS register, so the default size 
 #	of the target offset still is 16 bit.
@@ -812,7 +812,7 @@
 	.byte 0x66, 0xea			# prefix + jmpi-opcode
 code32:	.long	0x1000				# will be set to 0x100000
 						# for big kernels
-	.word	__KERNEL_CS
+	.word	__BOOT_CS
 
 # Here's a bunch of information about your current kernel..
 kernel_version:	.ascii	UTS_RELEASE
@@ -1007,12 +1007,12 @@
 # Descriptor tables
 #
 # NOTE: if you think the GDT is large, you can make it smaller by just
-# defining the KERNEL_CS and KERNEL_DS entries and shifting the gdt
-# address down by GDT_ENTRY_KERNEL_CS*8. This puts bogus entries into
+# defining the BOOT_CS and BOOT_DS entries and shifting the gdt
+# address down by GDT_ENTRY_BOOT_CS*8. This puts bogus entries into
 # the GDT, but those wont be used so it's not a problem.
 #
 gdt:
-	.fill GDT_ENTRY_KERNEL_CS,8,0
+	.fill GDT_ENTRY_BOOT_CS,8,0
 
 	.word	0xFFFF				# 4Gb - (0x100000*0x1000 = 4Gb)
 	.word	0				# base address = 0
@@ -1025,13 +1025,12 @@
 	.word	0x9200				# data read/write
 	.word	0x00CF				# granularity = 4096, 386
 						#  (+5th nibble of limit)
+gdt_end:
 idt_48:
 	.word	0				# idt limit = 0
 	.word	0, 0				# idt base = 0L
 gdt_48:
-	.word	0x8000				# gdt limit=2048,
-						#  256 GDT entries
-
+	.word	gdt_end - gdt - 1		# gdt limit
 	.word	0, 0				# gdt base (filled in later)
 
 # Include video setup & detection code
diff -Nru a/arch/i386/kernel/head.S b/arch/i386/kernel/head.S
--- a/arch/i386/kernel/head.S	Fri Aug 16 19:29:20 2002
+++ b/arch/i386/kernel/head.S	Fri Aug 16 19:29:20 2002
@@ -46,7 +46,7 @@
  * Set segments to known values
  */
 	cld
-	movl $(__KERNEL_DS),%eax
+	movl $(__BOOT_DS),%eax
 	movl %eax,%ds
 	movl %eax,%es
 	movl %eax,%fs
@@ -306,7 +306,7 @@
 
 ENTRY(stack_start)
 	.long init_thread_union+8192
-	.long __KERNEL_DS
+	.long __BOOT_DS
 
 /* This is the default interrupt "handler" :-) */
 int_msg:
@@ -409,6 +409,17 @@
 /*
  * The Global Descriptor Table contains 28 quadwords, per-CPU.
  */
+#ifdef CONFIG_SMP
+/*
+ * The boot_gdt_table must mirror the equivalent in setup.S and is
+ * used only by the trampoline for booting other CPUs
+ */
+ENTRY(boot_gdt_table)
+	.fill GDT_ENTRY_BOOT_CS,8,0
+	.quad 0x00cf9a000000ffff	/* kernel 4GB code at 0x00000000 */
+	.quad 0x00cf92000000ffff	/* kernel 4GB data at 0x00000000 */
+#endif
+	
 ENTRY(cpu_gdt_table)
 	.quad 0x0000000000000000	/* NULL descriptor */
 	.quad 0x0000000000000000	/* 0x0b reserved */
diff -Nru a/arch/i386/kernel/trampoline.S b/arch/i386/kernel/trampoline.S
--- a/arch/i386/kernel/trampoline.S	Fri Aug 16 19:29:20 2002
+++ b/arch/i386/kernel/trampoline.S	Fri Aug 16 19:29:20 2002
@@ -56,7 +56,7 @@
 	lmsw	%ax		# into protected mode
 	jmp	flush_instr
 flush_instr:
-	ljmpl	$__KERNEL_CS, $0x00100000
+	ljmpl	$__BOOT_CS, $0x00100000
 			# jump to startup_32 in arch/i386/kernel/head.S
 
 idt_48:
@@ -69,8 +69,8 @@
 #
 
 gdt_48:
-	.word	0x0800			# gdt limit = 2048, 256 GDT entries
-	.long	cpu_gdt_table-__PAGE_OFFSET	# gdt base = gdt (first SMP CPU)
+	.word	__BOOT_DS + 7			# gdt limit
+	.long	boot_gdt_table-__PAGE_OFFSET	# gdt base = gdt (first SMP CPU)
 
 .globl trampoline_end
 trampoline_end:
diff -Nru a/include/asm-i386/segment.h b/include/asm-i386/segment.h
--- a/include/asm-i386/segment.h	Fri Aug 16 19:29:20 2002
+++ b/include/asm-i386/segment.h	Fri Aug 16 19:29:20 2002
@@ -69,6 +69,14 @@
 
 #define GDT_SIZE (GDT_ENTRIES * 8)
 
+/* Simple and small GDT entries for booting only */
+
+#define GDT_ENTRY_BOOT_CS		2
+#define __BOOT_CS	(GDT_ENTRY_BOOT_CS * 8)
+
+#define GDT_ENTRY_BOOT_DS		(GDT_ENTRY_BOOT_CS + 1)
+#define __BOOT_DS	(GDT_ENTRY_BOOT_DS * 8)
+
 /*
  * The interrupt descriptor table has room for 256 idt's,
  * the global descriptor table is dependent on the number

===================================================================


This BitKeeper patch contains the following changesets:
+
## Wrapped with gzip_uu ##


begin 664 bkpatch6639
M'XL(`."873T``[58:W/;MA+]3/Z*G7%NQTXL"@#?NN..'<OQS:2M/9'SH?<Q
M&H@$1<9\N"04-W/5_]X%J*<M6Y9:,QG"0RZ6"^"<L[LZ@"^-J'O&5_%U9![`
MOZI&]HQBDH]K_DU8AV55BB-\_KFJ\'DWK0K15:;=]Y^Z(WS6&<>RPRS71)MK
M+J,4OHFZZ1G4LA=/Y/<[T3,^7UQ^^>GLLVF>G,!YRLNQ&`@))R>FK.IO/(^;
M4R[3O"HM6?.R*83D5E05TX7IE!'"\)]+?9NXWI1ZQ/&G$8TIY0X5,6%.X#E+
M;RK4YWT%U*.AXU$R=8GKN&8?J(4^@+`N";K4`QKTF-TCK$/<'B&@UGWZ8&O@
MG0<=8KZ'OW<5YV8$`W'':RX%J'V&R_X-)'550#TI958(]<`".,OSZAZ:@N<Y
MX(?N>-2:<JFGH9NDJD&F52/@^KR!^S3#$TEQ`7!75Z-<%/@LDRF,LC'@@AJ(
M)W56CG&*MI`BDB)&-T45"]"+RF2&ZS,_@<M\:IO7R],T.SM>IDDX,7_<LGF\
MCM)N9@>>1EQ7+;,632/B;BIX;`U6]M3!84I=ZK`I'_DDX)0RYD8A<_C&XWNA
M;T0*L]%1.+5=U_%VB/A6U*7(-P9*IPYC@3]-\-@1@#CX@9.(<%N@&UPNXW,"
M&OB[QX>OB[LJSTKQ.$K7"WW<5)]QGX_L41@+UW/%"Z-\['AE+RG9*=:')U1D
M361%CT[?(Y1-F6"N<"(_"7@8!T&\Z^FO^EY&3*CKVELCSLHHG\2BRYNBHYTW
M8ER(4EKI*OM#QYDRVW,1`$Y,_)!&E-LV#WVV.=9M7I=1LM!W=\&H7GLCY.1N
M`Y>HYSI3C\0!'7'&_$#X"4M>M)MK'E>BHR%&I[+`-NYM3PY_CS*8HS'F+'D:
M9W$IXTET:U7U^(6^'1:J+[CVU/8H)3J'V.L9A/7<\-D,XD#'>94,$NDYZ!@F
MC=+T>2)![6YU[`HZ];W^CUI\O?5`]M#WONT`-3_JNU%4WW)X<S@<OK^ZNAGV
M!T?'_Q#\=[/O^\I&WXW\:W&WM#E'&WA#?J=$76:?$NVN'9ZQA0/@)9)^@@:X
M?)7,\.,"ZFHB48O0$=/?;(?G/_H\5%NA^"M0W47&S!@9?QI5S7>K&8TM'EE<
MOM"U3UTU(OX)]4)'(S78$:D4.O15D-I('MTB2K'^6`&I5ML78[1=[%X8)4R#
M5`_&'VTT0[S76*#"_^$'%5D];(/\S^#F[/S3</#QWQ?_@V-8@!G^^.<FJ,Q4
M<&>`[*3'9H'TKDY%+H653K8(,6.4^,2?XFR_A0%U=L2!#YW@57#0EK.+@A>I
M.Q(09TDB:@1^6P"758WE[@PB;39Y'B.S]>^#C*#5FW8X,%`F,L!KK@W'"\5`
M4^IJ4ST8UGU5Q\;*:THH`::DBU(<#R`625;.*^V9F1:M.9YPP76&A%#/FC1+
MY-P86RZ<S^-8`1_BZKZ$T7>U'\.+7VX^_SK_YMO`@ILT:^!N(A6MQI-FX3,K
M9:5C<ELYG06=9'G^V-%Q<$R4D=9*_/A0E'%/S;89V.I%J^[MDF?OH:/"Q#LU
MC`/]9YX5F7S`D+52=A>*[%%6/\F1C?7TDB1!X+4D\7852QLZ[+74LD9(#W'_
M1V*<E<TLOZOS:GF3BK+%3*,XM&2,I3G65`H]2YI%&)9H&\5%N8[<:GN))[FU
MMF_[D,L)%?CT_<G2P"9A*\W:RL)`QTO)Q<E()HK,S!*D$YQ?_?+AX^5P\/.U
MV7UKPEN$?]L]#Q4J)<=V%XI)(Z'(ZEJWQ0+$;Y,,CT>I2X9[-E-KQ;FL42Y0
M^6-LC//OBF-JPG*+](XI]VKO*WQ7P_GU%S6K:VH"':Y_^^AYAAG6;Q,>H[80
M$B4AUP)#$KR,[EMH-QN<R_?8YF,GCOV],FPO]<'UV>S)V3&7_/'L`Z1LEIC&
M9GZN-G%[L'3WYG+&U5K$*9<O=>HS%UGK.UB*JZR@2>ONR%FDK/TJE"WX[1IT
M$%8/^8?PFW$)RW/=&F^CWNH>[$-`5W-+WW4)G!MO%AE+5\"$S`MO[$HQ>?DV
MWM=S&R:J=^`;ZRK?$M58AW]G.+P^N[P87GWX,+BXF=F/.&[$B?[S,,FP"P-D
MKZ+1D8;BTTWO=AS^U3;\0<+8WG^W.2.84MNVV5Z=8/!:OR6>H=S/<_]<M>85
ME/ZUX`'4GE[L'CC[Z%,(4)%AD"'&1%O0Z%\L%?@?AJ7%5.DMRM)_S0-=)HG'
MDFD8;/%R`5KC\)$=2GAP]+2C/CK:,.D=T*.'[ON/W?<7[A<_=D>IB&Z;27$2
/T0@!Y4;FGUG`S`5B%P``
`
end

Re: Boot failure in 2.5.31 BK with new TLS patch

[Ingo Molnar]

On Fri, 16 Aug 2002, James Bottomley wrote:

> This is probably local to me since I've got a box which takes quad CPU
> cards that has always been very picky about the GDT layout at boot.  
> However, it's been failing miserably with the new padding the TLS stuff
> introduces into the boot time GDT.
> 
> I attach a patch that leaves the boot time %cs and %ds at their old
> values (0x10 and 0x18), and shifts to the new GDT layout after the
> switch to protected mode has been accomplished.
> 
> For those who don't have this GDT boot problem, it reduces the size of
> the boot code by about 64 bytes.

while your patch looks OK, it would be *really* interesting to find out
why the previous layout failed. Does the BIOS somehow corrupt the GDT? You
are using the stock SMP code otherwise, correct? And this part of the
patch:

 gdt_48:
-       .word   0x8000                          # gdt limit=2048,
-                                               #  256 GDT entries
-
+       .word   gdt_end - gdt - 1               # gdt limit
        .word   0, 0                            # gdt base (filled in later)

perhaps this bit alone makes the difference?

	Ingo

Re: Boot failure in 2.5.31 BK with new TLS patch

[James Bottomley]

mingo@elte.hu said:
> while your patch looks OK, it would be *really* interesting to find
> out why the previous layout failed. Does the BIOS somehow corrupt the
> GDT? You are using the stock SMP code otherwise, correct? And this
> part of the patch: 

Well, I should say, this is the voyager MCA box again...

The boot problem only happens with my quad pentium cards, the dyad pentium and 
486 are fine.  Originally, a voyager system with quad cards just wouldn't boot 
(this was in the 2.2.x days).  Eventually, by trial and error and long debug 
of the boot process I discovered it would boot if the GDT was 8 bytes aligned 
(actually, the manuals say it should be 16 byte aligned, so perhaps we should 
also add this to the Linux setup.S?).  SUS (the voyager BIOS equivalent) 
reports that the CPU took a Trap 6 at FFF38466 in the boot sequence, but I 
knew there wasn't an illegal instruction, and the memory address isn't in the 
boot code. I suspect that the quad cards have some real mode instruction 
emulation and that's where the trap is occuring.

Unfortunately, all the people at NCR who could explain what is going on have 
long since departed, so I'm afraid I can only guess.

However, the general point that we should keep the boot sequence as simple as 
possible (just in case we run across any other wierd quirks even in modern 
PCs) still remains.

James

Re: Boot failure in 2.5.31 BK with new TLS patch

[Ingo Molnar]

On Sat, 17 Aug 2002, James Bottomley wrote:

> The boot problem only happens with my quad pentium cards, the dyad
> pentium and 486 are fine.  Originally, a voyager system with quad cards
> just wouldn't boot (this was in the 2.2.x days).  Eventually, by trial
> and error and long debug of the boot process I discovered it would boot
> if the GDT was 8 bytes aligned (actually, the manuals say it should be
> 16 byte aligned, so perhaps we should also add this to the Linux
> setup.S?). [...]

indeed it's not aligned:

	c010025c T cpu_gdt_descr

could you align it by adding this line replacing the ALIGN line that
preceeds the cpu_gdt_descr definition in head.S:

	.align 32

we want to align the GDT to 32 bytes anyway, we have optimized it for
cache layout, and didnt realize that it wasnt aligned to begin with ...

> However, the general point that we should keep the boot sequence as
> simple as possible (just in case we run across any other wierd quirks
> even in modern PCs) still remains.

i agree, so unless you can find the source of the problem and we can fix
the generic GDT, your patch is the right solution.

but, the right GDT layout might affect things like APM or PNP BIOS
compatibility, so it would be useful to figure out what's wrong with the
main GDT nevertheless.

	Ingo

Re: Boot failure in 2.5.31 BK with new TLS patch

[Pavel Machek]

Hi!

> > The boot problem only happens with my quad pentium cards, the dyad
> > pentium and 486 are fine.  Originally, a voyager system with quad cards
> > just wouldn't boot (this was in the 2.2.x days).  Eventually, by trial
> > and error and long debug of the boot process I discovered it would boot
> > if the GDT was 8 bytes aligned (actually, the manuals say it should be
> > 16 byte aligned, so perhaps we should also add this to the Linux
> > setup.S?). [...]
> 
> indeed it's not aligned:
> 
> 	c010025c T cpu_gdt_descr
> 
> could you align it by adding this line replacing the ALIGN line that
> preceeds the cpu_gdt_descr definition in head.S:
> 
> 	.align 32
> 
> we want to align the GDT to 32 bytes anyway, we have optimized it for
> cache layout, and didnt realize that it wasnt aligned to begin with ...

You might want to .align L1_CACHE_SIZE (or something), IIRC P4s have bigger
cachelines than 32.
								Pave
-- 
Philips Velo 1: 1"x4"x8", 300gram, 60, 12MB, 40bogomips, linux, mutt,
details at http://atrey.karlin.mff.cuni.cz/~pavel/velo/index.html.

Re: Boot failure in 2.5.31 BK with new TLS patch

[Gabriel Paubert]

Ingo Molnar wrote:
> On Sat, 17 Aug 2002, James Bottomley wrote:
> 
> 
>>The boot problem only happens with my quad pentium cards, the dyad
>>pentium and 486 are fine.  Originally, a voyager system with quad cards
>>just wouldn't boot (this was in the 2.2.x days).  Eventually, by trial
>>and error and long debug of the boot process I discovered it would boot
>>if the GDT was 8 bytes aligned (actually, the manuals say it should be
>>16 byte aligned, so perhaps we should also add this to the Linux
>>setup.S?). [...]
> 
> 
> indeed it's not aligned:
> 
> 	c010025c T cpu_gdt_descr

Hey no, it's cpu_gdt_table that must be aligned. That one does not matter, 
it's only used once for the lgdt instruction...

Ingo, for the layout of the gdt also, the location of the TSS descriptor
is irrelevant AFAICT. It's only used when doing the initial LTR, after 
that it's never referenced by the CPU.

	Gabriel.

Re: Boot failure in 2.5.31 BK with new TLS patch

[Ingo Molnar]

On Sat, 17 Aug 2002, Gabriel Paubert wrote:

> Hey no, it's cpu_gdt_table that must be aligned. That one does not
> matter, it's only used once for the lgdt instruction...

you are right, i misread the System.map - cpu_gdt_table is aligned
properly:

  c02a39e0 D cpu_gdt_table

so it must be something else that prevents booting on those boxes. Does 
the boot BIOS code perhaps assume a certain GDT layout? A certain size? 
Does it overwrite certain GDT entries perhaps?

> Ingo, for the layout of the gdt also, the location of the TSS descriptor
> is irrelevant AFAICT. It's only used when doing the initial LTR, after
> that it's never referenced by the CPU.

yes. Fortunately this makes no difference, the LDT and the default DS/CS
are in a single cacheline still.

	Ingo

Re: Boot failure in 2.5.31 BK with new TLS patch

[James Bottomley]

paubert@iram.es said:
> Hey no, it's cpu_gdt_table that must be aligned. That one does not
> matter,  it's only used once for the lgdt instruction... 

Actually, the intel manual recommends (but doesn't require) a wierd alignment 
for the descriptors.  It recommends aligning them at an address which is 2 MOD 
4 to avoid possible alignment check faults in user mode.  Not that I think we 
can ever run into the problem, but we should probably obey the recommendation. 
 I'll fix this up as well.

James

Re: Boot failure in 2.5.31 BK with new TLS patch

[Gabriel Paubert]

James Bottomley wrote:

> 
> 
> Actually, the intel manual recommends (but doesn't require) a wierd alignment 
> for the descriptors.  It recommends aligning them at an address which is 2 MOD 
> 4 to avoid possible alignment check faults in user mode.  Not that I think we 
> can ever run into the problem, but we should probably obey the recommendation. 
>  I'll fix this up as well.

This is already done for the IDT descriptor, but not (yet) for the gdt 
descriptor(s).

Alignment checks are only done when CPL==3. And lidt/lgdt are (obviously) 
privileged, although sidt/sgdt (and sldt/str for that matter) are not,
but I can't see how application could take benefit or make malicious use
of this capability.

	Gabriel.

Re: Boot failure in 2.5.31 BK with new TLS patch

[Linus Torvalds]

The gdt descriptor alignment really shouldn't matter, but that bogus GDT 
_size_ thing in the descriptor might do it. 

Right now it's set to be 0x8000, which is not a legal GDT size (it should 
be of the form n*8-1), and is nonsensical anyway (the comment says 2048 
entries, but the fact is, we don't _have_ 2048 entries in there).

		Linus

Re: Boot failure in 2.5.31 BK with new TLS patch

[Ingo Molnar]

On Sat, 17 Aug 2002, Linus Torvalds wrote:

> The gdt descriptor alignment really shouldn't matter, but that bogus GDT
> _size_ thing in the descriptor might do it.
> 
> Right now it's set to be 0x8000, which is not a legal GDT size (it
> should be of the form n*8-1), and is nonsensical anyway (the comment
> says 2048 entries, but the fact is, we don't _have_ 2048 entries in
> there).

hm, in BK-curr it's set to:

SYMBOL_NAME(cpu_gdt_descr):
        .word GDT_ENTRIES*8-1
        .long SYMBOL_NAME(cpu_gdt_table)

this should be the correct value, right? Where do we have a 0x8000 size
value?

and the per-CPU GDT gets set up before being loaded.

	Ingo

Re: Boot failure in 2.5.31 BK with new TLS patch

[Gabriel Paubert]

Linus Torvalds wrote:
 > The gdt descriptor alignment really shouldn't matter, but that bogus
 > GDT _size_ thing in the descriptor might do it.

Indeed.


 > Right now it's set to be 0x8000, which is not a legal GDT size (it
 > should be of the form n*8-1), and is nonsensical anyway (the comment
 > says 2048 entries, but the fact is, we don't _have_ 2048 entries in
 > there).

In my source tree pulled about at the end of last night here (10pm in 
California) it is still GDT_ENTRIES*8-1. Time to pull :-)

	Gabriel.

P.S: since the first entry is not used, the first 8 bytes could be filled
with
	1:	.word 0
gdt_table_desc: .word GDT_ENTRIES*8-1
		.long 1b

to eliminate the array of GDT descriptors and save a few bytes.

Re: Boot failure in 2.5.31 BK with new TLS patch

[Ingo Molnar]

oh, setup.S. nasty indeed. (yet) untested patch attached, booting into the
new kernel shortly.

	Ingo

--- linux/arch/i386/boot/setup.S.orig2	Sat Aug 17 20:53:41 2002
+++ linux/arch/i386/boot/setup.S	Sat Aug 17 20:54:40 2002
@@ -1029,8 +1029,7 @@
 	.word	0				# idt limit = 0
 	.word	0, 0				# idt base = 0L
 gdt_48:
-	.word	0x8000				# gdt limit=2048,
-						#  256 GDT entries
+	.word	GDT_ENTRY_KERNEL_CS*8 + 16 - 1	# gdt limit
 
 	.word	0, 0				# gdt base (filled in later)
 

Re: Boot failure in 2.5.31 BK with new TLS patch

[Ingo Molnar]

On Sat, 17 Aug 2002, Ingo Molnar wrote:

> oh, setup.S. nasty indeed. (yet) untested patch attached, booting into
> the new kernel shortly.

works for me.

	Ingo

Re: Boot failure in 2.5.31 BK with new TLS patch

[Linus Torvalds]

In article <Pine.LNX.4.44.0208172051280.17227-100000@localhost.localdomain>,
Ingo Molnar  <mingo@elte.hu> wrote:
>
>oh, setup.S. nasty indeed. (yet) untested patch attached, booting into the
>new kernel shortly.

Ingo, this only fixes the gdt size, it doesn't fix the fact that the gdt
itself doesn't seem to be aligned at all (and to clarify, I'm talking
very much about the boot-time entry.S gdt, not the "real" run-time gdt). 

Mind doing that part too?

(I can well imagine that some CPU's may not even have the low 4 bits of
the gdt register wired up at all, since they should always be zero. So
doing a lgdt or lidt with the base not being 16-byte aligned could well
result in basically loading crap into the LDT, causing the system not to
work at all).

This is also true of "bootsect_gdt", I think. Altough I have no idea
what the bios "int 15" interfaces actually do.

		Linus

Re: Boot failure in 2.5.31 BK with new TLS patch

[James Bottomley]

[-- Attachment #1: Type: text/plain, Size: 1384 bytes --]

torvalds@transmeta.com said:
> The gdt descriptor alignment really shouldn't matter, but that bogus
> GDT  _size_ thing in the descriptor might do it. 

I knowit shouldn't, all I can say is that it does for me.

> Right now it's set to be 0x8000, which is not a legal GDT size (it
> should  be of the form n*8-1), and is nonsensical anyway (the comment
> says 2048  entries, but the fact is, we don't _have_ 2048 entries in
> there). 

I fixed this as part of my cleanups, but it doesn't actually make a difference 
to the voyagers.  What kills them is either gdt not 8 bytes aligned in setup.S 
or %cs above about 0x30 when going from real to protected mode (once in 
protected mode, it will happily accept arbitrary descriptor values).

The attached patch should fix all of the issues in this thread

- Align boot time GDT on 16 byte boundary (per intel recommendation)
- Use minimal boot time GDT and switch to complex one after protected mode.
- intel recommended aligment of gdt_desc and idt_desc in boot/setup.S
- make the boot time gdt_desc have the correct length.
- intel recommended aligment of cpu_gdt_descr
- make cpu_gdt_table align exactly on a cache line

I really only care about the first two of these to boot my voyager systems, 
but fixing all the others may help future proof us for later intel chips (at 
least now we follow all of the intel guidelines).

James


[-- Attachment #2: tmp.BK --]
[-- Type: text/plain , Size: 11124 bytes --]

You can import this changeset into BK by piping this whole message to:
'| bk receive [path to repository]' or apply the patch as usual.

===================================================================


ChangeSet@1.488, 2002-08-17 15:24:14-05:00, jejb@mulgrave.(none)
  Alter Boot to use minimal GDT different from protected mode run time
  GDT.   Also correct a variety of aligment issues to be in line with
  intel recommendations.


 arch/i386/boot/compressed/head.S |    8 ++++----
 arch/i386/boot/compressed/misc.c |    2 +-
 arch/i386/boot/setup.S           |   26 ++++++++++++++++----------
 arch/i386/kernel/head.S          |   22 +++++++++++++++++-----
 arch/i386/kernel/trampoline.S    |    6 +++---
 include/asm-i386/desc.h          |    1 +
 include/asm-i386/segment.h       |    8 ++++++++
 7 files changed, 50 insertions(+), 23 deletions(-)


diff -Nru a/arch/i386/boot/compressed/head.S b/arch/i386/boot/compressed/head.S
--- a/arch/i386/boot/compressed/head.S	Sat Aug 17 15:26:52 2002
+++ b/arch/i386/boot/compressed/head.S	Sat Aug 17 15:26:52 2002
@@ -31,7 +31,7 @@
 startup_32:
 	cld
 	cli
-	movl $(__KERNEL_DS),%eax
+	movl $(__BOOT_DS),%eax
 	movl %eax,%ds
 	movl %eax,%es
 	movl %eax,%fs
@@ -74,7 +74,7 @@
 	popl %esi	# discard address
 	popl %esi	# real mode pointer
 	xorl %ebx,%ebx
-	ljmp $(__KERNEL_CS), $0x100000
+	ljmp $(__BOOT_CS), $0x100000
 
 /*
  * We come here, if we were loaded high.
@@ -101,7 +101,7 @@
 	popl %eax	# hcount
 	movl $0x100000,%edi
 	cli		# make sure we don't get interrupted
-	ljmp $(__KERNEL_CS), $0x1000 # and jump to the move routine
+	ljmp $(__BOOT_CS), $0x1000 # and jump to the move routine
 
 /*
  * Routine (template) for moving the decompressed kernel in place,
@@ -124,5 +124,5 @@
 	movsl
 	movl %ebx,%esi	# Restore setup pointer
 	xorl %ebx,%ebx
-	ljmp $(__KERNEL_CS), $0x100000
+	ljmp $(__BOOT_CS), $0x100000
 move_routine_end:
diff -Nru a/arch/i386/boot/compressed/misc.c b/arch/i386/boot/compressed/misc.c
--- a/arch/i386/boot/compressed/misc.c	Sat Aug 17 15:26:52 2002
+++ b/arch/i386/boot/compressed/misc.c	Sat Aug 17 15:26:52 2002
@@ -299,7 +299,7 @@
 struct {
 	long * a;
 	short b;
-	} stack_start = { & user_stack [STACK_SIZE] , __KERNEL_DS };
+	} stack_start = { & user_stack [STACK_SIZE] , __BOOT_DS };
 
 static void setup_normal_output_buffer(void)
 {
diff -Nru a/arch/i386/boot/setup.S b/arch/i386/boot/setup.S
--- a/arch/i386/boot/setup.S	Sat Aug 17 15:26:52 2002
+++ b/arch/i386/boot/setup.S	Sat Aug 17 15:26:52 2002
@@ -801,7 +801,7 @@
 	subw	$DELTA_INITSEG, %si
 	shll	$4, %esi			# Convert to 32-bit pointer
 # NOTE: For high loaded big kernels we need a
-#	jmpi    0x100000,__KERNEL_CS
+#	jmpi    0x100000,__BOOT_CS
 #
 #	but we yet haven't reloaded the CS register, so the default size 
 #	of the target offset still is 16 bit.
@@ -812,7 +812,7 @@
 	.byte 0x66, 0xea			# prefix + jmpi-opcode
 code32:	.long	0x1000				# will be set to 0x100000
 						# for big kernels
-	.word	__KERNEL_CS
+	.word	__BOOT_CS
 
 # Here's a bunch of information about your current kernel..
 kernel_version:	.ascii	UTS_RELEASE
@@ -1006,13 +1006,15 @@
 
 # Descriptor tables
 #
-# NOTE: if you think the GDT is large, you can make it smaller by just
-# defining the KERNEL_CS and KERNEL_DS entries and shifting the gdt
-# address down by GDT_ENTRY_KERNEL_CS*8. This puts bogus entries into
-# the GDT, but those wont be used so it's not a problem.
+# NOTE: The intel manual says gdt should be sixteen bytes aligned for
+# efficiency reasons.  However, there are machines which are known not
+# to boot with misaligned GDTs, so alter this at your peril!  If you alter
+# GDT_ENTRY_BOOT_CS (in asm/segment.h) remember to leave at least two
+# empty GDT entries (one for NULL and one reserved).
 #
+	.align 16
 gdt:
-	.fill GDT_ENTRY_KERNEL_CS,8,0
+	.fill GDT_ENTRY_BOOT_CS,8,0
 
 	.word	0xFFFF				# 4Gb - (0x100000*0x1000 = 4Gb)
 	.word	0				# base address = 0
@@ -1025,13 +1027,17 @@
 	.word	0x9200				# data read/write
 	.word	0x00CF				# granularity = 4096, 386
 						#  (+5th nibble of limit)
+gdt_end:
+	.align	4
+	
+	.word	0				# alignment byte
 idt_48:
 	.word	0				# idt limit = 0
 	.word	0, 0				# idt base = 0L
-gdt_48:
-	.word	0x8000				# gdt limit=2048,
-						#  256 GDT entries
 
+	.word	0				# alignment byte
+gdt_48:
+	.word	gdt_end - gdt - 1		# gdt limit
 	.word	0, 0				# gdt base (filled in later)
 
 # Include video setup & detection code
diff -Nru a/arch/i386/kernel/head.S b/arch/i386/kernel/head.S
--- a/arch/i386/kernel/head.S	Sat Aug 17 15:26:52 2002
+++ b/arch/i386/kernel/head.S	Sat Aug 17 15:26:52 2002
@@ -15,6 +15,7 @@
 #include <asm/page.h>
 #include <asm/pgtable.h>
 #include <asm/desc.h>
+#include <asm/cache.h>
 
 #define OLD_CL_MAGIC_ADDR	0x90020
 #define OLD_CL_MAGIC		0xA33F
@@ -46,7 +47,7 @@
  * Set segments to known values
  */
 	cld
-	movl $(__KERNEL_DS),%eax
+	movl $(__BOOT_DS),%eax
 	movl %eax,%ds
 	movl %eax,%es
 	movl %eax,%fs
@@ -306,7 +307,7 @@
 
 ENTRY(stack_start)
 	.long init_thread_union+8192
-	.long __KERNEL_DS
+	.long __BOOT_DS
 
 /* This is the default interrupt "handler" :-) */
 int_msg:
@@ -349,12 +350,12 @@
 	.long idt_table
 
 # boot GDT descriptor (later on used by CPU#0):
-
+	.word 0				# 32 bit align gdt_desc.address
 cpu_gdt_descr:
 	.word GDT_ENTRIES*8-1
 	.long cpu_gdt_table
 
-	.fill NR_CPUS-1,6,0		# space for the other GDT descriptors
+	.fill NR_CPUS-1,8,0		# space for the other GDT descriptors
 
 /*
  * This is initialized to create an identity-mapping at 0-8M (for bootup
@@ -405,10 +406,21 @@
  */
 .data
 
-ALIGN
 /*
  * The Global Descriptor Table contains 28 quadwords, per-CPU.
  */
+#ifdef CONFIG_SMP
+/*
+ * The boot_gdt_table must mirror the equivalent in setup.S and is
+ * used only by the trampoline for booting other CPUs
+ */
+	.align L1_CACHE_BYTES
+ENTRY(boot_gdt_table)
+	.fill GDT_ENTRY_BOOT_CS,8,0
+	.quad 0x00cf9a000000ffff	/* kernel 4GB code at 0x00000000 */
+	.quad 0x00cf92000000ffff	/* kernel 4GB data at 0x00000000 */
+#endif
+	.align L1_CACHE_BYTES
 ENTRY(cpu_gdt_table)
 	.quad 0x0000000000000000	/* NULL descriptor */
 	.quad 0x0000000000000000	/* 0x0b reserved */
diff -Nru a/arch/i386/kernel/trampoline.S b/arch/i386/kernel/trampoline.S
--- a/arch/i386/kernel/trampoline.S	Sat Aug 17 15:26:52 2002
+++ b/arch/i386/kernel/trampoline.S	Sat Aug 17 15:26:52 2002
@@ -56,7 +56,7 @@
 	lmsw	%ax		# into protected mode
 	jmp	flush_instr
 flush_instr:
-	ljmpl	$__KERNEL_CS, $0x00100000
+	ljmpl	$__BOOT_CS, $0x00100000
 			# jump to startup_32 in arch/i386/kernel/head.S
 
 idt_48:
@@ -69,8 +69,8 @@
 #
 
 gdt_48:
-	.word	0x0800			# gdt limit = 2048, 256 GDT entries
-	.long	cpu_gdt_table-__PAGE_OFFSET	# gdt base = gdt (first SMP CPU)
+	.word	__BOOT_DS + 7			# gdt limit
+	.long	boot_gdt_table-__PAGE_OFFSET	# gdt base = gdt (first SMP CPU)
 
 .globl trampoline_end
 trampoline_end:
diff -Nru a/include/asm-i386/desc.h b/include/asm-i386/desc.h
--- a/include/asm-i386/desc.h	Sat Aug 17 15:26:52 2002
+++ b/include/asm-i386/desc.h	Sat Aug 17 15:26:52 2002
@@ -13,6 +13,7 @@
 struct Xgt_desc_struct {
 	unsigned short size;
 	unsigned long address __attribute__((packed));
+	unsigned short pad;
 } __attribute__ ((packed));
 
 extern struct Xgt_desc_struct idt_descr, cpu_gdt_descr[NR_CPUS];
diff -Nru a/include/asm-i386/segment.h b/include/asm-i386/segment.h
--- a/include/asm-i386/segment.h	Sat Aug 17 15:26:52 2002
+++ b/include/asm-i386/segment.h	Sat Aug 17 15:26:52 2002
@@ -69,6 +69,14 @@
 
 #define GDT_SIZE (GDT_ENTRIES * 8)
 
+/* Simple and small GDT entries for booting only */
+
+#define GDT_ENTRY_BOOT_CS		2
+#define __BOOT_CS	(GDT_ENTRY_BOOT_CS * 8)
+
+#define GDT_ENTRY_BOOT_DS		(GDT_ENTRY_BOOT_CS + 1)
+#define __BOOT_DS	(GDT_ENTRY_BOOT_DS * 8)
+
 /*
  * The interrupt descriptor table has room for 256 idt's,
  * the global descriptor table is dependent on the number

===================================================================


This BitKeeper patch contains the following changesets:
+
## Wrapped with gzip_uu ##


begin 664 bkpatch5117
M'XL(`(RQ7CT``[59^U/;2!+^6?HK^HK<%B18GAF-+)D]MGB89%-)@`)2=7N/
M<HVE$5;0PZL'Q'O>__UZ1GYB@[&SF)1$Y)E6S_37W]<][,#70N8'QC?YK6?N
MP*]941X8217?YN)>6KMIELH]?'Z59?B\V<\2V51#FR>?FCU\UK@-R@:S'!/'
M7(K2[\.]S(L#@UKV]$DY',@#X^KLP]?/QU>F>7@(IWV1WLIK6<+AH5EF^;V(
M@^)(E/TX2ZTR%VF1R%)8?I:,ID-'C!"&/PYU;>*T1K1%N#OR:4"IX%0&A'&O
MQ4UQ-TB._H@&:K(EJL?S/=IBMMUR[!'W7+MM=H!:W/.`L";QFM0%ZAPP?D!Y
M@S@'A(!:Z]&C[8!W+C2(>0)_K>>GI@_'<2ES.,&-1>-0%1*2*(T2$<.'S@T$
M41C*7*8EA'F6P"#/2NF7,H`D"R3D50IEE$@T@X,M`+169.!G>8ZC0,"]R"-9
M#B$+0<31;:(,1451R4*]K"<A2B&.4@D/4=E'*U%:RAAP<I;@V$"44986EOD)
M',HI-R]G830;&WY,DPAB_K)F!T7N]YN1[;4TU)KX:)#+HI!!LR]%8%W/;2S'
MVXBB8VPD>B[Q!*6,.7Z;<;$RAB^T[5$77\`I'=F.PUL;>'PG\U3&*QVE(\Z8
MYXY"C+U#'+RY'@]E>YVC*TS._'-:G/'-_<.ODT&FHK[LI=-JN[BI+A.NZ-F]
M=B"=EB-?Z.6RX;F]I,1S?R#Z253XEK\4_1:A;,0D<R3WW=`3[<#S@DVC/V][
MYC&ACF.O]3A*_;@*9%,424,;+Z3.,JL_3P%MSD<,*0@!P`/BMJE/A6V+MLM6
M^[K.ZLQ+UG:=]1A=LA=(7/*2BYP2VQZ%/2'=D/DN8Z+=%O2%+LZ;G///(W3C
MK"]D60U6Y#IM.7S4(H%'>X(QUU-NAB^*]H+%^0PGW-;RM(X;UJO67\-<9N\6
MQ;0\"J(@+8/*O[.R_/:%MCEKJS>@T-DMBKNNA,Y>DCGZK,QQ:/!7D3DL.>#D
MXN)&ZY52*Q25FF`OH)$_Z'\H$I=K([&%\'1L#M3\J*]&DMW'\&:WVU7>=#O7
M>_M_E^*[V7%=-49?C?A;,IB-.<4Q\(9\IT1]S`XEVEQ]>V8L[(!(D8TJ'(!J
M6_91VK-[5.VL*I$DT1#3[ZQOS[_T>8S6#/8C&-V$7\T`J>C(SXJA5?1N+>%;
MHGRA:9<ZZH[`)[35YAJBRY78\Q"ET*"O`M%J!42U"+P8H?52MT(H81JB^F;\
M"44I_+LN7G,LF.%_\)/R+N_JQ_#OZYOCTT_=ZX__.OLO[,,4RO#GSZN`,B:_
MC>&Q$0V;6+7>9D<2JUFK7ZWA7\8H<8D[PMEN#0+*-T5!"V'P.A5Y`U2;4@VF
MA?@B*-2`8RRF4XA*R%)`3WK#$BOJW575\][4(+9-7>Z!TLD\&J#;4$1_R$FQ
M'@\U(2H]>AYNXZW<!F1>35SU;<=`OHFP98`)R>Q/J0>'4D</U3?#>LCRP)C[
M&B>T@2L.I`P<;/O.+V[.#N"F+\<M1"+2"G>N$,-"+1R*?E;%@>HXBNA[*64Z
MWC/5E:38SH19CE9D&$9^)%-_B+LH"M5[`/:G#Q([S'U%H+G$[4`:%7X?&;2`
MAWZ$W:9Z=)=F#RFD68EF5&NC^BG5U&`0B\E+,'K%/F"#)'3/5?8C=*"$85;E
M,)!Y%/\-X&.H_E^/0%,XI7MV?G/UVV3M*LJ`1<^L)-M#7Q.9])3%#&*)*%56
M\9<"6[J'3*TK&6`3IOHYG($=&6(%4:P6#>=?/W_6,J$>((W(_%X&>Y;>6EOO
MO?8>4:9VO8[*^&Y8813'RR[N>_M$#4)9X:9"'6+Q8&+(X*8Q"2@Q\+-3QT"W
MABHFZC4V!5M90,6TGQ]<@_I@,FC\-D2\"GH#J)JB?HVC)"H?D=-"<[,).VW1
M:#U)3RL[K!D_>5ZKYJ?6IOSD0L-Y)7I:JJ4PZ:(R$G$\W(=>A=F&R%>G,!GX
M@ZJK@E**7BPA%@K5RL07<2<U(VDZJJ,ZHR+8'>=K+4!(<@R^7'2`@PB"O*:T
MXP"_'D@_>DR0&M:S7G".+U7^*6,^IJ_4YP[J9$%WL4]RWD)\MB"]CRI@YLZX
M78%_J-35[[?ZOY@=WE;YI*]/%H8V:=?2K$=9&,3;F>3BUTZMW/I6IP&,<\5F
MT$.-J#=WLMF6VD(L%]1,MY[ISI+Y_*I[>OGUND%5$BL;Q4#X-5.H^C%3'%@?
M#$UE!"UQXNEU4`J4X6+#0(9P>G'^_N.'[O672[/YUH2WFIT5+\X!(JF0HY(H
MS\?VY>]5A'#5QT0I3`H'14]1H4Q@$:*8"A'2&^H)LSAK'Y5Y3+2QG[@2-:LY
M);'/M'MZ?/KK6??DMYNS:U.SUNZB2WO/TYIA_5X)W.#OA/AA6VC=(B%^C.9;
MJ+$"_,,)0CG0+*P&UI_:C_G9[,G9*-UB>?8.,EL4/K68E=PV?R2R!<-M?E0S
MYKE<!GU1OM2HRQQD/)=CX\AHV]&$YVS(=S8T[%>A.\U4<S!;5:77ITOK.&1^
MX=N43X[F`7W5S5ILO)F61+I7(V32(KH,&#:3-EX7BR<LTM^!:RS*8DTJQF(B
M-+K=R^,/9]V+]^^OSV[&XWL"5W^H?]T-HQR3%]-;Y=F>QM\3AS+KD?=#!T2/
MM'7-R5"MK=Z(M:CMU=I*-N\`7Z?ROT1R^.>$J+'9RBN_5/5BB<FDJGU?I*J$
MU2PHL';$2C#/Q5`39!\]1!,HPO/<7)^N8R^!LC<F_H6R=:*K`UD3L*Z>T<QB
M$X$0KP_2'D'\B:W>2B9U15FE15TJH_ZC\`]$\/-J8$VKWRVPM>$1Z3IX+9V-
M3A!&;=MF6YV">:_VQQXLFG1KHLEK[/F"EF.D]9'NNDA/5[U-L%T*'A8%<!TA
MBTD-WR(1M>A.6Y0%/5>2CQ+X'W,'2PL%Z25Y-@PV_7)*B\;N<@/U%KR]IPUU
JT-"*2>^`[CTVWUDVWYF:G_XI$DL]_ZZHDD/BAK;G<<?\/]0"+"H`'0``
`
end

Re: Boot failure in 2.5.31 BK with new TLS patch

[Ingo Molnar]

On Sat, 17 Aug 2002, James Bottomley wrote:

> I fixed this as part of my cleanups, but it doesn't actually make a
> difference to the voyagers.  What kills them is either gdt not 8 bytes
> aligned in setup.S or %cs above about 0x30 when going from real to
> protected mode (once in protected mode, it will happily accept arbitrary
> descriptor values).

the later we cannot fix sensibly, it introduces a dependency on our main
GDT. And it's good to have a small GDT in the boot sector anyway - so your
patch definitely looks good to me.

	Ingo