Re: [SeaBIOS] [RFC PATCH v3 0/3] Add segment limit checks to emulator
On Sat, Jul 24, 2010 at 11:45:22AM -0400, Kevin O'Connor wrote:
On Mon, Jul 12, 2010 at 04:13:06PM +0300, Avi Kivity wrote:
Does SeaBIOS use big real mode now?
SeaBIOS calls option roms in big real mode. This is required by the relevant specs.
See the transition16big function in src/romlayout.S. It briefly jumps to an address at 0xffxxx during the transition to real-mode. At a quick glance, it looks like it could probably be changed to not use a code address >64K.
I put together a SeaBIOS patch so it does not use code addresses >64K in big real mode - in case anyone wants to test it. Note, this only reduces the use of code addresses >64K - SeaBIOS will still try to use data addresses >64K (eg, in option rom PMM code). -Kevin diff --git a/src/misc.c b/src/misc.c index 354df87..108c332 100644 --- a/src/misc.c +++ b/src/misc.c @@ -156,8 +156,8 @@ u64 rombios32_gdt[] VAR16VISIBLE __aligned(8) = { GDT_LIMIT(BUILD_BIOS_SIZE-1) | GDT_CODE | GDT_BASE(BUILD_BIOS_ADDR), // 16 bit data segment base=0x0 limit=0xffff (SEG32_MODE16_DS) GDT_LIMIT(0x0ffff) | GDT_DATA, - // 16 bit code segment base=0 limit=0xffffffff (SEG32_MODE16BIG_CS) - GDT_LIMIT(0xfffff) | GDT_CODE | GDT_G, + // 16 bit code segment base=0xf0000 limit=0xffffffff (SEG32_MODE16BIG_CS) + GDT_LIMIT(0xfffff) | GDT_CODE | GDT_G | GDT_BASE(BUILD_BIOS_ADDR), // 16 bit data segment base=0 limit=0xffffffff (SEG32_MODE16BIG_DS) GDT_LIMIT(0xfffff) | GDT_DATA | GDT_G, }; diff --git a/src/romlayout.S b/src/romlayout.S index 54e5a4d..a469596 100644 --- a/src/romlayout.S +++ b/src/romlayout.S @@ -105,7 +105,7 @@ transition16big: movw %ax, %fs movw %ax, %gs - ljmpl $SEG32_MODE16BIG_CS, $(BUILD_BIOS_ADDR + 1f) + ljmpw $SEG32_MODE16BIG_CS, $1f .code16gcc 1:
On 07/24/2010 07:16 PM, Kevin O'Connor wrote:
On Sat, Jul 24, 2010 at 11:45:22AM -0400, Kevin O'Connor wrote:
On Mon, Jul 12, 2010 at 04:13:06PM +0300, Avi Kivity wrote:
Does SeaBIOS use big real mode now? SeaBIOS calls option roms in big real mode. This is required by the relevant specs.
See the transition16big function in src/romlayout.S. It briefly jumps to an address at 0xffxxx during the transition to real-mode. At a quick glance, it looks like it could probably be changed to not use a code address>64K. I put together a SeaBIOS patch so it does not use code addresses>64K in big real mode - in case anyone wants to test it. Note, this only reduces the use of code addresses>64K - SeaBIOS will still try to use data addresses>64K (eg, in option rom PMM code).
What conditions are needed to trigger this path? This can't occur under normal operation, since it will fail badly with kvm on Intel.
-Kevin
diff --git a/src/misc.c b/src/misc.c index 354df87..108c332 100644 --- a/src/misc.c +++ b/src/misc.c @@ -156,8 +156,8 @@ u64 rombios32_gdt[] VAR16VISIBLE __aligned(8) = { GDT_LIMIT(BUILD_BIOS_SIZE-1) | GDT_CODE | GDT_BASE(BUILD_BIOS_ADDR), // 16 bit data segment base=0x0 limit=0xffff (SEG32_MODE16_DS) GDT_LIMIT(0x0ffff) | GDT_DATA, - // 16 bit code segment base=0 limit=0xffffffff (SEG32_MODE16BIG_CS) - GDT_LIMIT(0xfffff) | GDT_CODE | GDT_G, + // 16 bit code segment base=0xf0000 limit=0xffffffff (SEG32_MODE16BIG_CS) + GDT_LIMIT(0xfffff) | GDT_CODE | GDT_G | GDT_BASE(BUILD_BIOS_ADDR), // 16 bit data segment base=0 limit=0xffffffff (SEG32_MODE16BIG_DS) GDT_LIMIT(0xfffff) | GDT_DATA | GDT_G, }; diff --git a/src/romlayout.S b/src/romlayout.S index 54e5a4d..a469596 100644 --- a/src/romlayout.S +++ b/src/romlayout.S @@ -105,7 +105,7 @@ transition16big: movw %ax, %fs movw %ax, %gs
- ljmpl $SEG32_MODE16BIG_CS, $(BUILD_BIOS_ADDR + 1f) + ljmpw $SEG32_MODE16BIG_CS, $1f
.code16gcc 1: -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
-- error compiling committee.c: too many arguments to function
On Sun, Jul 25, 2010 at 11:55:47AM +0300, Avi Kivity wrote:
On 07/24/2010 07:16 PM, Kevin O'Connor wrote:
On Sat, Jul 24, 2010 at 11:45:22AM -0400, Kevin O'Connor wrote:
On Mon, Jul 12, 2010 at 04:13:06PM +0300, Avi Kivity wrote:
Does SeaBIOS use big real mode now? SeaBIOS calls option roms in big real mode. This is required by the relevant specs.
See the transition16big function in src/romlayout.S. It briefly jumps to an address at 0xffxxx during the transition to real-mode. At a quick glance, it looks like it could probably be changed to not use a code address>64K. I put together a SeaBIOS patch so it does not use code addresses>64K in big real mode - in case anyone wants to test it. Note, this only reduces the use of code addresses>64K - SeaBIOS will still try to use data addresses>64K (eg, in option rom PMM code).
What conditions are needed to trigger this path? This can't occur under normal operation, since it will fail badly with kvm on Intel.
It's called on every boot. I've personally only tested kvm on amd, but I'd have to assume something must be allowing this to work on intel. On option rom execution (eg, video rom), there is a call to optionrom.c:__callrom() which calls util.c:call16big() which calls romlayout.S:__transition16big. This has been in place since SeaBIOS-0.4.0 - well before the integration with kvm. Is the kvm restriction just on the code address, or is it also for data accesses? -Kevin
On Sun, Jul 25, 2010 at 12:42:46PM -0400, Kevin O'Connor wrote:
On Sun, Jul 25, 2010 at 11:55:47AM +0300, Avi Kivity wrote:
What conditions are needed to trigger this path? This can't occur under normal operation, since it will fail badly with kvm on Intel.
It's called on every boot. I've personally only tested kvm on amd, but I'd have to assume something must be allowing this to work on intel.
BTW, the transition16big code does: ljmpl $SEG32_MODE16BIG_CS, $(0xf0000 + 1f) .code16gcc 1: // Disable protected mode movl %cr0, %eax andl $~CR0_PE, %eax movl %eax, %cr0 // far jump to flush CPU queue after transition to real mode ljmpw $0xf000, $2f 2: Only the ljmpw is in big real mode with a code address >64K - the "Disable protected mode" code is technically in 16bit protected mode. I'm not sure if that helps explain why it works. -Kevin
On 07/25/2010 08:19 PM, Kevin O'Connor wrote:
On Sun, Jul 25, 2010 at 12:42:46PM -0400, Kevin O'Connor wrote:
On Sun, Jul 25, 2010 at 11:55:47AM +0300, Avi Kivity wrote:
What conditions are needed to trigger this path? This can't occur under normal operation, since it will fail badly with kvm on Intel. It's called on every boot. I've personally only tested kvm on amd, but I'd have to assume something must be allowing this to work on intel. BTW, the transition16big code does:
ljmpl $SEG32_MODE16BIG_CS, $(0xf0000 + 1f)
.code16gcc 1: // Disable protected mode movl %cr0, %eax andl $~CR0_PE, %eax movl %eax, %cr0
// far jump to flush CPU queue after transition to real mode ljmpw $0xf000, $2f 2:
Only the ljmpw is in big real mode with a code address>64K - the "Disable protected mode" code is technically in 16bit protected mode. I'm not sure if that helps explain why it works.
What happens is kvm enters real mode with cs.limit=0xffff, the guest #GPs due to segment limit violation, and enters the emulator, which emulates the far jump correctly. So this works, and will continue to work even after we fix limit checking. It's still cleaner IMO to use normal code segments. -- I have a truly marvellous patch that fixes the bug which this signature is too narrow to contain.
On Sun, Jul 25, 2010 at 09:34:38PM +0300, Avi Kivity wrote:
On 07/25/2010 08:19 PM, Kevin O'Connor wrote:
Only the ljmpw is in big real mode with a code address>64K - the "Disable protected mode" code is technically in 16bit protected mode. I'm not sure if that helps explain why it works.
What happens is kvm enters real mode with cs.limit=0xffff, the guest #GPs due to segment limit violation, and enters the emulator, which emulates the far jump correctly.
So this works, and will continue to work even after we fix limit checking. It's still cleaner IMO to use normal code segments.
Makes sense. I committed the patch that avoids this behavior to SeaBIOS git. -Kevin
participants (2)
-
Avi Kivity -
Kevin O'Connor