Steven Rostedt
2006-Nov-09 03:01 UTC
[Xen-devel] [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 <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) _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Andi Kleen
2006-Nov-09 13:13 UTC
[Xen-devel] 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 _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Andi Kleen
2006-Nov-09 13:31 UTC
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 <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 _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Andi Kleen
2006-Nov-09 13:33 UTC
[Xen-devel] 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 _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Alexander van Heukelum
2006-Nov-09 14:54 UTC
[Xen-devel] 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 <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Â… _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Steven Rostedt
2006-Nov-09 15:18 UTC
[Xen-devel] 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 _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Jan Beulich
2006-Nov-09 15:31 UTC
[Xen-devel] Re: [PATCH] shorten the x86_64 boot setup GDT to what the comment says
>>> 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 thanksOnce 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 _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Alexander van Heukelum
2006-Nov-09 15:44 UTC
[Xen-devel] 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_______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Alexander van Heukelum
2006-Nov-09 18:31 UTC
[Xen-devel] 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 _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Andi Kleen
2006-Nov-10 14:01 UTC
[Xen-devel] 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 _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Alexander van Heukelum
2006-Nov-10 15:46 UTC
[Xen-devel] 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 _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Steven Rostedt
2006-Nov-11 05:17 UTC
[Xen-devel] [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 <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 _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Andi Kleen
2006-Nov-11 06:42 UTC
[Xen-devel] 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 _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Alexander van Heukelum
2006-Nov-12 13:47 UTC
[Xen-devel] 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" <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Â… _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Steven Rostedt
2006-Nov-13 15:37 UTC
[Xen-devel] 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 _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Andi Kleen
2006-Nov-13 16:47 UTC
[Xen-devel] 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 _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel