2006-11-09 03:01:57

by Steven Rostedt

[permalink] [raw]
Subject: [PATCH] shorten the x86_64 boot setup GDT to what the comment says


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 <[email protected]>

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)


2006-11-09 14:54:33

by Alexander van Heukelum

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

> 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 <[email protected]>
>
> 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
[email protected]

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

2006-11-09 15:14:23

by Andi Kleen

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


> 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

2006-11-09 15:19:13

by Steven Rostedt

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



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

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

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

2006-11-09 15:30:11

by Jan Beulich

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

>>> Andi Kleen <[email protected]> 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

2006-11-09 15:34:05

by Andi Kleen

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

On Thursday 09 November 2006 16:31, Jan Beulich wrote:
> >>> Andi Kleen <[email protected]> 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

2006-11-09 15:34:09

by Andi Kleen

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


> 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

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

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

2006-11-10 14:01:58

by Andi Kleen

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


> 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

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

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

2006-11-11 05:18:11

by Steven Rostedt

[permalink] [raw]
Subject: [PATCH] make x86_64 boot gdt size exact (like x86).


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 <[email protected]>

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

2006-11-11 06:43:16

by Andi Kleen

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

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

2006-11-12 13:47:47

by Alexander van Heukelum

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


On Fri, 10 Nov 2006 16:46:00 +0100, "Alexander van Heukelum"
<[email protected]> 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
[email protected]

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

2006-11-13 15:38:19

by Steven Rostedt

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


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

2006-11-13 16:48:15

by Andi Kleen

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


> 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