[PATCH] shorten the x86_64 boot setup GDT to what the comment says

[PATCH] shorten the x86_64 boot setup GDT to what the comment says

[Steven Rostedt]

Andi,

Stephen Tweedie, Herbert Xu, and myself have been struggling with a very
nasty bug in Xen.  But it also pointed out a small bug in the x86_64
kernel boot setup.

The GDT limit being setup by the initial bzImage code when entering into
protected mode is way too big.  The comment by the code states that the
size of the GDT is 2048, but the actual size being set up is much bigger
(32768). This happens simply because of one extra '0'.

Instead of setting up a 0x800 size, 0x8000 is set up.  On bare metal this
is fine because the CPU wont load any segments unless  they are
explicitly used.  But unfortunately, this breaks Xen on vmx FV, since it
(for now) blindly loads all the segments into the VMCS if they are less
than the gdt limit. Since the real mode segments are around 0x3000, we are
getting junk into the VMCS and that later causes an exception.

Stephen Tweedie has written up a patch to fix the Xen side and will be
submitting that to those folks. But that doesn't excuse the GDT limit
being a magnitude too big.

Signed-off-by: Steven Rostedt <rostedt@goodmis.org>

Index: linux-2.6.19-rc2/arch/x86_64/boot/setup.S
===================================================================
--- linux-2.6.19-rc2.orig/arch/x86_64/boot/setup.S	2006-11-08 21:37:58.000000000 -0500
+++ linux-2.6.19-rc2/arch/x86_64/boot/setup.S	2006-11-08 21:38:16.000000000 -0500
@@ -840,7 +840,7 @@ idt_48:
 	.word	0				# idt limit = 0
 	.word	0, 0				# idt base = 0L
 gdt_48:
-	.word	0x8000				# gdt limit=2048,
+	.word	0x800				# gdt limit=2048,
 						#  256 GDT entries

 	.word	0, 0				# gdt base (filled in later)

Re: [PATCH] shorten the x86_64 boot setup GDT to what the comment says

[Andi Kleen]

> Stephen Tweedie has written up a patch to fix the Xen side and will be
> submitting that to those folks. But that doesn't excuse the GDT limit
> being a magnitude too big.

Added thanks

-Andi

[Xen-devel] Re: [PATCH] shorten the x86_64 boot setup GDT to what the comment says

[Jan Beulich]

>>> Andi Kleen <ak@suse.de> 09.11.06 14:13 >>>
>
>> Stephen Tweedie has written up a patch to fix the Xen side and will be
>> submitting that to those folks. But that doesn't excuse the GDT limit
>> being a magnitude too big.
>
>Added thanks

Once at this - why not set it to the *correct* value, just like i386 does,
and update the comment at once? Namely, why would you expect to
never run into the original problem again if there are still possible
selectors pointing into invalid, yet within limits parts of the GDT?

Jan

Re: [Xen-devel] Re: [PATCH] shorten the x86_64 boot setup GDT to what the comment says

[Andi Kleen]

On Thursday 09 November 2006 16:31, Jan Beulich wrote:
> >>> Andi Kleen <ak@suse.de> 09.11.06 14:13 >>>
> >>
> >> Stephen Tweedie has written up a patch to fix the Xen side and will be
> >> submitting that to those folks. But that doesn't excuse the GDT limit
> >> being a magnitude too big.
> >
> >Added thanks
>
> Once at this - why not set it to the *correct* value, just like i386 does,
> and update the comment at once? Namely, why would you expect to
> never run into the original problem again if there are still possible
> selectors pointing into invalid, yet within limits parts of the GDT?

Ok I will use an offset.

-Andi

Re: [PATCH] shorten the x86_64 boot setup GDT to what the comment says

[Alexander van Heukelum]

> Andi,
> 
> Stephen Tweedie, Herbert Xu, and myself have been struggling with a very
> nasty bug in Xen.  But it also pointed out a small bug in the x86_64
> kernel boot setup.
> 
> The GDT limit being setup by the initial bzImage code when entering into
> protected mode is way too big.  The comment by the code states that the
> size of the GDT is 2048, but the actual size being set up is much bigger
> (32768). This happens simply because of one extra '0'.
> 
> Instead of setting up a 0x800 size, 0x8000 is set up.  On bare metal this
> is fine because the CPU wont load any segments unless  they are
> explicitly used.  But unfortunately, this breaks Xen on vmx FV, since it
> (for now) blindly loads all the segments into the VMCS if they are less
> than the gdt limit. Since the real mode segments are around 0x3000, we
> are
> getting junk into the VMCS and that later causes an exception.
> 
> Stephen Tweedie has written up a patch to fix the Xen side and will be
> submitting that to those folks. But that doesn't excuse the GDT limit
> being a magnitude too big.
> 
> Signed-off-by: Steven Rostedt <rostedt@goodmis.org>
> 
> Index: linux-2.6.19-rc2/arch/x86_64/boot/setup.S
> ===================================================================
> --- linux-2.6.19-rc2.orig/arch/x86_64/boot/setup.S      2006-11-08
> 21:37:58.000000000 -0500
> +++ linux-2.6.19-rc2/arch/x86_64/boot/setup.S   2006-11-08
> 21:38:16.000000000 -0500
> @@ -840,7 +840,7 @@ idt_48:
>  	.word	0				# idt limit = 0
>  	.word	0, 0				# idt base = 0L
>  gdt_48:
> -       .word   0x8000                          # gdt limit=2048,
> +       .word   0x800                           # gdt limit=2048,
>  						#  256 GDT entries
> 
>  	.word	0, 0				# gdt base (filled in later)

The limit should be the offset of the last byte of the gdt table. So
I think what was meant was really 0x7ff. Comparing this code with the 
i386-version, why does x86_64 need 256 entries here, while i386 is happy
with just the code-segment and data-segment descriptors?

Greetings,
    Alexander
-- 
  Alexander van Heukelum
  heukelum@fastmail.fm

-- 
http://www.fastmail.fm - And now for something completely different…

Re: [PATCH] shorten the x86_64 boot setup GDT to what the comment says

[Steven Rostedt]

On Thu, 9 Nov 2006, Alexander van Heukelum wrote:

> >  gdt_48:
> > -       .word   0x8000                          # gdt limit=2048,
> > +       .word   0x800                           # gdt limit=2048,
> >  						#  256 GDT entries
> >
> >  	.word	0, 0				# gdt base (filled in later)
>
> The limit should be the offset of the last byte of the gdt table. So
> I think what was meant was really 0x7ff. Comparing this code with the
> i386-version, why does x86_64 need 256 entries here, while i386 is happy
> with just the code-segment and data-segment descriptors?
>


Hmm, Andi,

Should this be more like what is done in x86? Although this isn't a major
bug or anything, would it be cleaner. For example doing:

@@ -836,11 +836,15 @@ gdt:
        .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,
+       .word   gdt_end - gdt - 1               # gdt limit=2048,
                                                #  256 GDT entries

        .word   0, 0                            # gdt base (filled in

instead?

If so, I can send you another patch that does this. Will need to test it
first.

-- Steve

Re: [PATCH] shorten the x86_64 boot setup GDT to what the comment says

[Alexander van Heukelum]

On Thu, Nov 09, 2006 at 10:18:53AM -0500, Steven Rostedt wrote:
> Hmm, Andi,
> 
> Should this be more like what is done in x86? Although this isn't a major
> bug or anything, would it be cleaner. For example doing:
> 
> @@ -836,11 +836,15 @@ gdt:
>         .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,
> +       .word   gdt_end - gdt - 1               # gdt limit=2048,
>                                                 #  256 GDT entries
> 
>         .word   0, 0                            # gdt base (filled in
> 
> instead?

Hi!

Maybe you should consider 16-byte aligning the gdt table too, like
i386 does? It doesn't hurt, and as per the comment in the i386-file
"16 byte aligment is recommended by intel."

Greetings,
	Alexander van Heukelum

> If so, I can send you another patch that does this. Will need to test it
> first.
> 
> -- Steve

Re: [PATCH] shorten the x86_64 boot setup GDT to what the comment says

[Andi Kleen]

> Maybe you should consider 16-byte aligning the gdt table too, like
> i386 does? It doesn't hurt, and as per the comment in the i386-file
> "16 byte aligment is recommended by intel."

It already is.

-Andi

Re: [PATCH] shorten the x86_64 boot setup GDT to what the comment says

[Alexander van Heukelum]

On Thu, Nov 09, 2006 at 02:33:08PM +0100, Andi Kleen wrote:
> 
> > Maybe you should consider 16-byte aligning the gdt table too, like
> > i386 does? It doesn't hurt, and as per the comment in the i386-file
> > "16 byte aligment is recommended by intel."
> 
> It already is.

Hi Andi,

(Assuming you mean: "The gdt table already is 16-byte aligned.")

Hmm. Not in the most recent version of Linus' tree, not even by
concidence, and none of the patches in your quilt-current/patches touch
x86_64's version of setup.S. Am I missing something?

Alexander

> -Andi
> 

-- 
Alexander van Heukelum

Re: [PATCH] shorten the x86_64 boot setup GDT to what the comment says

[Andi Kleen]

> Hi Andi,
> 
> (Assuming you mean: "The gdt table already is 16-byte aligned.")
> 
> Hmm. Not in the most recent version of Linus' tree, not even by
> concidence, and none of the patches in your quilt-current/patches touch
> x86_64's version of setup.S. Am I missing something?

The main GDT is. The boot GDT isn't, but it doesn't matter because
it is only used for a very short time.

-Andi

Re: [PATCH] shorten the x86_64 boot setup GDT to what the comment says

[Alexander van Heukelum]

On Fri, Nov 10, 2006 at 03:01:39PM +0100, Andi Kleen wrote:
> > Hi Andi,
> > 
> > (Assuming you mean: "The gdt table already is 16-byte aligned.")
> > 
> > Hmm. Not in the most recent version of Linus' tree, not even by
> > concidence, and none of the patches in your quilt-current/patches touch
> > x86_64's version of setup.S. Am I missing something?
> 
> The main GDT is. The boot GDT isn't, but it doesn't matter because
> it is only used for a very short time.

Aha, thanks for clearing that up. I agree it is not important to have
the boot GDT aligned, but I think it is preferable to make parts of the
two versions of setup.S equal if possible.

Let's see what Steven Rostedt comes up with.

I find the relocatable image patches interesting. I wonder if one can
get such a kernel 'running' using bochs, freedos, and loadlin ;).

Alexander

Re: [PATCH] shorten the x86_64 boot setup GDT to what the comment says

[Alexander van Heukelum]

On Fri, 10 Nov 2006 16:46:00 +0100, "Alexander van Heukelum"
<heukelum@mailshack.com> said:
> On Fri, Nov 10, 2006 at 03:01:39PM +0100, Andi Kleen wrote:
> > > Hi Andi,
> > > 
> > > (Assuming you mean: "The gdt table already is 16-byte aligned.")
> > > 
> > > Hmm. Not in the most recent version of Linus' tree, not even by
> > > concidence, and none of the patches in your quilt-current/patches touch
> > > x86_64's version of setup.S. Am I missing something?
> > 
> > The main GDT is. The boot GDT isn't, but it doesn't matter because
> > it is only used for a very short time.
> 
> Aha, thanks for clearing that up. I agree it is not important to have
> the boot GDT aligned, but I think it is preferable to make parts of the
> two versions of setup.S equal if possible.
> 
> Let's see what Steven Rostedt comes up with.
> 
> I find the relocatable image patches interesting. I wonder if one can
> get such a kernel 'running' using bochs, freedos, and loadlin ;).

Was it clear that I was sceptical about this still working? Oh well,
I tried it, and it did not break. Freedos' versions of himem and emm386 
loaded, DOS=HIGH,UMB.

Alexander
-- 
  Alexander van Heukelum
  heukelum@fastmail.fm

-- 
http://www.fastmail.fm - Same, same, but different…

[PATCH] make x86_64 boot gdt size exact (like x86).

[Steven Rostedt]

Andi,

Here's another patch that is basically a copy from x86's boot/setup.S.
It makes the GDT limit the exact size that is needed.  I tested this with
the same Xen test that broke the original 0x8000 size, and it booted just
fine.

Note, If you already pushed my previous patch. This patch should easily be
applied by manually removing the extra zero.

-- Steve

Signed-off-by: Steven Rostedt <rostedt@goodmis.org>

Index: linux-2.6.18-hack/arch/x86_64/boot/setup.S
===================================================================
--- linux-2.6.18-hack.orig/arch/x86_64/boot/setup.S
+++ linux-2.6.18-hack/arch/x86_64/boot/setup.S
@@ -836,13 +836,15 @@ gdt:
 	.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	gdt_end - gdt - 1		# gdt limit
 	.word	0, 0				# gdt base (filled in later)

 # Include video setup & detection code

Re: [PATCH] make x86_64 boot gdt size exact (like x86).

[Andi Kleen]

On Saturday 11 November 2006 06:17, Steven Rostedt wrote:
> 
> Andi,
> 
> Here's another patch that is basically a copy from x86's boot/setup.S.
> It makes the GDT limit the exact size that is needed.  I tested this with
> the same Xen test that broke the original 0x8000 size, and it booted just
> fine.

I had already changed the previous patch to be like that

(except for the - 1)

-Andi

Re: [PATCH] make x86_64 boot gdt size exact (like x86).

[Steven Rostedt]

On Sat, 11 Nov 2006, Andi Kleen wrote:

> On Saturday 11 November 2006 06:17, Steven Rostedt wrote:
> >
> > Andi,
> >
> > Here's another patch that is basically a copy from x86's boot/setup.S.
> > It makes the GDT limit the exact size that is needed.  I tested this with
> > the same Xen test that broke the original 0x8000 size, and it booted just
> > fine.
>
> I had already changed the previous patch to be like that
>
> (except for the - 1)
>

Andi,

Do you have the exact patch that you applied somewhere public?  A git repo
or something. I'd like to match what will be going upstream exactly.

Thanks.

-- Steve

Re: [PATCH] make x86_64 boot gdt size exact (like x86).

[Andi Kleen]

> Do you have the exact patch that you applied somewhere public?  A git repo
> or something. I'd like to match what will be going upstream exactly.


ftp://ftp.firstfloor.org/pub/ak/x86_64/quilt/patches/fix-boot-gdt-limit

But I still need to add the - 1 ... Will be up soon

-Andi