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.
Signature · 1 line
error compiling committee.c: too many arguments to function
Quoted 4 lines · Avi Kivity
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 <[email protected]> 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
Signature · 1 line
error compiling committee.c: too many arguments to function
Quoted 18 lines · Avi Kivity
Quoted 4 lines · Avi Kivity
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
9a01a9c3eb336eca37c17fd74c79806ee0bda05bAuthor: Kevin O'Connor <[email protected]> Date: Wed Aug 25 21:07:48 2010 -0400Only 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.
Quoted 13 lines
git bisect start 'rel-0.6.1' '
17d3e46511aeedc9f09a8216d194d749187b80aa'
# good: [b4525a0ec176426788f293cce92160e6573e86b6] Handle unaligned sizes iniomemcpy(). git bisect goodb4525a0ec176426788f293cce92160e6573e86b6
# good: [e2074bf6ec2956e1d803e62dcb052b7c88c214f0] Add ACPI SSDT/DSDT support for CPU hotplug. git bisect goode2074bf6ec2956e1d803e62dcb052b7c88c214f0
# bad: [7ce09ae6542f0f4187024ae3267b61a0cf6ebd39] Maketools/transdump.pymore resilient to unknown input. git bisect bad7ce09ae6542f0f4187024ae3267b61a0cf6ebd39
# good: [5feb83c8a55744397b4dd208fb4016a5c051222e] add write support to virtio-blk
git bisect good5feb83c8a55744397b4dd208fb4016a5c051222e
# bad: [6039fc55274deb7202060d08e0f23b9f3dcface4] Updateqemu_cfg_readto use "rep insb". git bisect bad6039fc55274deb7202060d08e0f23b9f3dcface4
That was premature, the real culprit (if I didn't mess up the bisect) is:
commit 6039fc55274deb7202060d08e0f23b9f3dcface4
Author: Kevin O'Connor <[email protected]> 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.
Signature · 1 line
error compiling committee.c: too many arguments to function
Quoted 7 lines · Avi Kivity
That was premature, the real culprit (if I didn't mess up the bisect) is:
commit
6039fc55274deb7202060d08e0f23b9f3dcface4Author: Kevin O'Connor <[email protected]> Date: Wed Aug 25 21:43:19 2010 -0400
Quoted 8 lines
Update
qemu_cfg_readto 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.
Signature · 1 line
error compiling committee.c: too many arguments to function
Quoted 10 lines · Avi Kivity
Quoted 9 lines · Avi Kivity
That was premature, the real culprit (if I didn't mess up the bisect) is:
commit
6039fc55274deb7202060d08e0f23b9f3dcface4Author: Kevin O'Connor <[email protected]> Date: Wed Aug 25 21:43:19 2010 -0400Update
qemu_cfg_readto use "rep insb".
Quoted 9 lines
Quoted 6 lines
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.
Signature · 1 line
error compiling committee.c: too many arguments to function
Quoted 9 lines · Avi Kivity
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.
Signature · 1 line
error compiling committee.c: too many arguments to function
Quoted 4 lines · Avi Kivity
Quoted 3 lines · Avi Kivity
I think it's a miscompile.
Quoted 9 lines
Quoted 6 lines
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.
Signature · 1 line
error compiling committee.c: too many arguments to function
Quoted 9 lines · Avi Kivity
Quoted 8 lines · Avi Kivity
Quoted 7 lines · Avi Kivity
I think it's a miscompile.
out/code16.o:1a4: 3e ds 1a5: 6c insb (%dx),%es:(%edi)
Quoted 8 lines
Quoted 5 lines
Quoted 2 lines
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
Quoted 5 lines · Kevin O'Connor
Quoted 3 lines
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.
Quoted 3 lines
It sounds
like we really do want this on the main branch though for the speed benefit it provides.
Yes.
Signature · 1 line
error compiling committee.c: too many arguments to function
Quoted 9 lines · Avi Kivity
Quoted 8 lines · Avi Kivity
Quoted 7 lines · Avi Kivity
I think it's a miscompile.
out/code16.o:1a4: 3e ds 1a5: 6c insb (%dx),%es:(%edi)
Quoted 8 lines
Quoted 5 lines
Quoted 2 lines
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).
Signature · 1 line
error compiling committee.c: too many arguments to function
Quoted 13 lines · Avi Kivity
Quoted 12 lines · Avi Kivity
Quoted 11 lines · Avi Kivity
Quoted 9 lines · Avi Kivity
I think it's a miscompile.
out/code16.o:1a4: 3e ds 1a5: 6c insb (%dx),%es:(%edi)Note no 66 prefix.
Quoted 7 lines
Quoted 5 lines
Quoted 2 lines
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
Quoted 8 lines · Alexander Graf
Quoted 6 lines
Quoted 4 lines
Quoted 1 line
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.
Signature · 1 line
error compiling committee.c: too many arguments to function
Quoted 7 lines · Avi Kivity
Quoted 6 lines · Alexander Graf
Quoted 6 lines
Quoted 4 lines
Quoted 1 line
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.
^^
Quoted 5 lines
Quoted 3 lines
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
Quoted 12 lines · Alexander Graf
Quoted 7 lines · Avi Kivity
Quoted 6 lines · Alexander Graf
Quoted 6 lines
Quoted 4 lines
Quoted 1 line
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.
^^
Quoted 8 lines
Quoted 5 lines
Quoted 3 lines
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.
Signature · 1 line
error compiling committee.c: too many arguments to function