seabios 0.6.1 regression
Installing Windows XP with seabios 0.6.1, immediately after the first reboot, Windows hangs in protected mode instead of proceeding with installation. I'm bisecting this, but if anyone can point to a likely culprit, I can try it first. -- error compiling committee.c: too many arguments to function
On 11/15/2010 03:39 PM, Avi Kivity wrote:
Installing Windows XP with seabios 0.6.1, immediately after the first reboot, Windows hangs in protected mode instead of proceeding with installation.
I'm bisecting this, but if anyone can point to a likely culprit, I can try it first.
Bisect says: commit 9a01a9c3eb336eca37c17fd74c79806ee0bda05b Author: Kevin O'Connor <kevin@koconnor.net> Date: Wed Aug 25 21:07:48 2010 -0400 Only show bootsplash during boot menu. When the bootsplash picture is shown, it's not possible to see text. So, only display the picture while prompting the user for the boot menu. git bisect start 'rel-0.6.1' '17d3e46511aeedc9f09a8216d194d749187b80aa' # good: [b4525a0ec176426788f293cce92160e6573e86b6] Handle unaligned sizes in iomemcpy(). git bisect good b4525a0ec176426788f293cce92160e6573e86b6 # good: [e2074bf6ec2956e1d803e62dcb052b7c88c214f0] Add ACPI SSDT/DSDT support for CPU hotplug. git bisect good e2074bf6ec2956e1d803e62dcb052b7c88c214f0 # bad: [7ce09ae6542f0f4187024ae3267b61a0cf6ebd39] Make tools/transdump.py more resilient to unknown input. git bisect bad 7ce09ae6542f0f4187024ae3267b61a0cf6ebd39 # good: [5feb83c8a55744397b4dd208fb4016a5c051222e] add write support to virtio-blk git bisect good 5feb83c8a55744397b4dd208fb4016a5c051222e # bad: [6039fc55274deb7202060d08e0f23b9f3dcface4] Update qemu_cfg_read to use "rep insb". git bisect bad 6039fc55274deb7202060d08e0f23b9f3dcface4 -- error compiling committee.c: too many arguments to function
On 11/15/2010 05:04 PM, Avi Kivity wrote:
On 11/15/2010 03:39 PM, Avi Kivity wrote:
Installing Windows XP with seabios 0.6.1, immediately after the first reboot, Windows hangs in protected mode instead of proceeding with installation.
I'm bisecting this, but if anyone can point to a likely culprit, I can try it first.
Bisect says:
commit 9a01a9c3eb336eca37c17fd74c79806ee0bda05b Author: Kevin O'Connor <kevin@koconnor.net> Date: Wed Aug 25 21:07:48 2010 -0400
Only show bootsplash during boot menu.
When the bootsplash picture is shown, it's not possible to see text. So, only display the picture while prompting the user for the boot menu.
git bisect start 'rel-0.6.1' '17d3e46511aeedc9f09a8216d194d749187b80aa' # good: [b4525a0ec176426788f293cce92160e6573e86b6] Handle unaligned sizes in iomemcpy(). git bisect good b4525a0ec176426788f293cce92160e6573e86b6 # good: [e2074bf6ec2956e1d803e62dcb052b7c88c214f0] Add ACPI SSDT/DSDT support for CPU hotplug. git bisect good e2074bf6ec2956e1d803e62dcb052b7c88c214f0 # bad: [7ce09ae6542f0f4187024ae3267b61a0cf6ebd39] Make tools/transdump.py more resilient to unknown input. git bisect bad 7ce09ae6542f0f4187024ae3267b61a0cf6ebd39 # good: [5feb83c8a55744397b4dd208fb4016a5c051222e] add write support to virtio-blk git bisect good 5feb83c8a55744397b4dd208fb4016a5c051222e # bad: [6039fc55274deb7202060d08e0f23b9f3dcface4] Update qemu_cfg_read to use "rep insb". git bisect bad 6039fc55274deb7202060d08e0f23b9f3dcface4
That was premature, the real culprit (if I didn't mess up the bisect) is: commit 6039fc55274deb7202060d08e0f23b9f3dcface4 Author: Kevin O'Connor <kevin@koconnor.net> Date: Wed Aug 25 21:43:19 2010 -0400 Update qemu_cfg_read to use "rep insb". Use rep insb instead of manual loop - the host may be able to optimize the rep insb instruction. Which doesn't make any sense - will do more testing. -- error compiling committee.c: too many arguments to function
On 11/15/2010 05:12 PM, Avi Kivity wrote:
That was premature, the real culprit (if I didn't mess up the bisect) is:
commit 6039fc55274deb7202060d08e0f23b9f3dcface4 Author: Kevin O'Connor <kevin@koconnor.net> Date: Wed Aug 25 21:43:19 2010 -0400
Update qemu_cfg_read to use "rep insb".
Use rep insb instead of manual loop - the host may be able to optimize the rep insb instruction.
Which doesn't make any sense - will do more testing.
Confirmed with multiple tests. -- error compiling committee.c: too many arguments to function
On 11/15/2010 05:34 PM, Avi Kivity wrote:
On 11/15/2010 05:12 PM, Avi Kivity wrote:
That was premature, the real culprit (if I didn't mess up the bisect) is:
commit 6039fc55274deb7202060d08e0f23b9f3dcface4 Author: Kevin O'Connor <kevin@koconnor.net> Date: Wed Aug 25 21:43:19 2010 -0400
Update qemu_cfg_read to use "rep insb".
Use rep insb instead of manual loop - the host may be able to optimize the rep insb instruction.
Which doesn't make any sense - will do more testing.
Confirmed with multiple tests.
I think it's a miscompile. out/code16.o: 1a4: 3e ds 1a5: 6c insb (%dx),%es:(%edi) Note no 66 prefix. -- error compiling committee.c: too many arguments to function
On 11/15/2010 05:41 PM, Avi Kivity wrote:
I think it's a miscompile.
out/code16.o: 1a4: 3e ds 1a5: 6c insb (%dx),%es:(%edi)
Note no 66 prefix.
It isn't, that was random crap. All the insb() code is 32-bit. -- error compiling committee.c: too many arguments to function
On 11/15/2010 05:49 PM, Avi Kivity wrote:
On 11/15/2010 05:41 PM, Avi Kivity wrote:
I think it's a miscompile.
out/code16.o: 1a4: 3e ds 1a5: 6c insb (%dx),%es:(%edi)
Note no 66 prefix.
It isn't, that was random crap. All the insb() code is 32-bit.
Rewriting it to use inb / stos works (jecxz ; insb; loop doesn't) so it looks like a kernel bug in insb emulation. -- error compiling committee.c: too many arguments to function
On Mon, Nov 15, 2010 at 06:09:45PM +0200, Avi Kivity wrote:
On 11/15/2010 05:49 PM, Avi Kivity wrote:
On 11/15/2010 05:41 PM, Avi Kivity wrote:
I think it's a miscompile.
out/code16.o: 1a4: 3e ds 1a5: 6c insb (%dx),%es:(%edi)
Note no 66 prefix.
It isn't, that was random crap. All the insb() code is 32-bit.
Rewriting it to use inb / stos works (jecxz ; insb; loop doesn't) so it looks like a kernel bug in insb emulation.
Ughh. I can revert that change on stable-0.6.1 if needed. It sounds like we really do want this on the main branch though for the speed benefit it provides. -Kevin
On 11/16/2010 02:18 AM, Kevin O'Connor wrote:
Rewriting it to use inb / stos works (jecxz ; insb; loop doesn't) so it looks like a kernel bug in insb emulation.
Ughh. I can revert that change on stable-0.6.1 if needed.
kernel bug -> kernel fix. SeaBIOS should feel free to use any documented hardware capability, esp. something as basic as rep insb which dates back to the good old 8086 days. As an exception I ask that you stay away from big real mode and SMM.
It sounds like we really do want this on the main branch though for the speed benefit it provides.
Yes. -- error compiling committee.c: too many arguments to function
On 11/15/2010 06:09 PM, Avi Kivity wrote:
On 11/15/2010 05:49 PM, Avi Kivity wrote:
On 11/15/2010 05:41 PM, Avi Kivity wrote:
I think it's a miscompile.
out/code16.o: 1a4: 3e ds 1a5: 6c insb (%dx),%es:(%edi)
Note no 66 prefix.
It isn't, that was random crap. All the insb() code is 32-bit.
Rewriting it to use inb / stos works (jecxz ; insb; loop doesn't) so it looks like a kernel bug in insb emulation.
Turns out is was a subtle bug in the tpr optimization we do for Windows XP. The problem happens when we load the vapic option rom from the firmware config interface. With inb / movb, writing the vapic area happens in guest context, which the kernel is prepared to handle. With insb, the write happens from kvm, which is then undone on the next entry, leading to the tpr being set to a high value. I'll fix it temporarily by clearing the vapic area from the option rom. Later I'd like to add a proper reset to kvm tpr optimization within qemu-kvm (helps when we dual boot Windows XP and some other OS). -- error compiling committee.c: too many arguments to function
On 16.11.2010, at 11:47, Avi Kivity wrote:
On 11/15/2010 06:09 PM, Avi Kivity wrote:
On 11/15/2010 05:49 PM, Avi Kivity wrote:
On 11/15/2010 05:41 PM, Avi Kivity wrote:
I think it's a miscompile.
out/code16.o: 1a4: 3e ds 1a5: 6c insb (%dx),%es:(%edi)
Note no 66 prefix.
It isn't, that was random crap. All the insb() code is 32-bit.
Rewriting it to use inb / stos works (jecxz ; insb; loop doesn't) so it looks like a kernel bug in insb emulation.
Turns out is was a subtle bug in the tpr optimization we do for Windows XP. The problem happens when we load the vapic option rom from the firmware config interface. With inb / movb, writing the vapic area happens in guest context, which the kernel is prepared to handle. With insb, the write happens from kvm, which is then undone on the next entry, leading to the tpr being set to a high value.
Shouldn't the vapic area be mapped in on demand? Then we could map it on option rom init time and everyone's happy. Alex
On 11/16/2010 03:19 PM, Alexander Graf wrote:
Rewriting it to use inb / stos works (jecxz ; insb; loop doesn't) so it looks like a kernel bug in insb emulation.
Turns out is was a subtle bug in the tpr optimization we do for Windows XP. The problem happens when we load the vapic option rom from the firmware config interface. With inb / movb, writing the vapic area happens in guest context, which the kernel is prepared to handle. With insb, the write happens from kvm, which is then undone on the next entry, leading to the tpr being set to a high value.
Shouldn't the vapic area be mapped in on demand? Then we could map it on option rom init time and everyone's happy.
Mapped in? It's an option rom. -- error compiling committee.c: too many arguments to function
On 16.11.2010, at 15:15, Avi Kivity wrote:
On 11/16/2010 03:19 PM, Alexander Graf wrote:
Rewriting it to use inb / stos works (jecxz ; insb; loop doesn't) so it looks like a kernel bug in insb emulation.
Turns out is was a subtle bug in the tpr optimization we do for Windows XP. The problem happens when we load the vapic option rom from the firmware config interface. With inb / movb, writing the vapic area happens in guest context, which the kernel is prepared to handle.
^^
With insb, the write happens from kvm, which is then undone on the next entry, leading to the tpr being set to a high value.
Shouldn't the vapic area be mapped in on demand? Then we could map it on option rom init time and everyone's happy.
Mapped in? It's an option rom.
According to your wording, the kernel handles writes to the vapic area. If the vapic area is only made special after initialization of the option rom, we wouldn't have this issue. Alex
On 11/16/2010 04:17 PM, Alexander Graf wrote:
On 16.11.2010, at 15:15, Avi Kivity wrote:
On 11/16/2010 03:19 PM, Alexander Graf wrote:
Rewriting it to use inb / stos works (jecxz ; insb; loop doesn't) so it looks like a kernel bug in insb emulation.
Turns out is was a subtle bug in the tpr optimization we do for Windows XP. The problem happens when we load the vapic option rom from the firmware config interface. With inb / movb, writing the vapic area happens in guest context, which the kernel is prepared to handle.
^^
With insb, the write happens from kvm, which is then undone on the next entry, leading to the tpr being set to a high value.
Shouldn't the vapic area be mapped in on demand? Then we could map it on option rom init time and everyone's happy.
Mapped in? It's an option rom.
According to your wording, the kernel handles writes to the vapic area. If the vapic area is only made special after initialization of the option rom, we wouldn't have this issue.
Ah, yes, and I'm even now writing a reset handler to make this happen. -- error compiling committee.c: too many arguments to function
Avi Kivity wrote:
Confirmed with multiple tests.
I think it's a miscompile.
As a data point you could try building SeaBIOS with a "vanilla" toolchain. In coreboot we've seen significant problems with most distribution toolchains, because they are tweaked for user space. A script to build the coreboot reference toolchain is in the coreboot source tree, under util/crossgcc. //Peter
participants (4)
-
Alexander Graf -
Avi Kivity -
Kevin O'Connor -
Peter Stuge