diff mbox

[PULL,7/7] tcg-i386: Use new return-argument ld/st helpers

Message ID 53861B26.7040400@weilnetz.de
State Not Applicable
Headers show

Commit Message

Stefan Weil May 28, 2014, 5:21 p.m. UTC
Am 28.05.2014 19:12, schrieb Richard Henderson:
> On 05/27/2014 03:37 PM, Stefan Weil wrote:
>> Hi Richard,
>>
>> this patch has broken the 64 bit version of QEMU for Windows: a Linux
>> guest starts booting, but hangs after "Booting the kernel.". I got a bug
>> report from a user and did a "git bisect" with a Tiny Core Linux guest /
>> cross build with default options / cross test with wine64 and default
>> options. Git reported this commit:
>>
>> 401c227b0a1134245ec61c6c5a9997cfc963c8e4 is the first bad commit
>> commit 401c227b0a1134245ec61c6c5a9997cfc963c8e4
>> Author: Richard Henderson <rth@twiddle.net>
>> Date:   Thu Jul 25 07:16:52 2013 -1000
>>
>>     tcg-i386: Use new return-argument ld/st helpers
>>    
>>     Discontinue the jump-around-jump-to-jump scheme, trading it for a single
>>     immediate move instruction.  The two extra jumps always consume 7 bytes,
>>     whereas the immediate move is either 5 or 7 bytes depending on where the
>>     code_gen_buffer gets located.
>>    
>>     Signed-off-by: Richard Henderson <rth@twiddle.net>
>>
>> :040000 040000 dfd9a66c85713cd1886a3342de1e9ac95d7ea43f
>> df8673dea69bc89cc2cc979aa24415e3fea4ed53 M    include
>> :040000 040000 1f7cd5291f2c69b4126c63bd567c6b106eb332c9
>> 87e7ece766168dda860b513dc97fe5af28ec2c4b M    tcg
>>
>> 32 bit versions of QEMU for Windows don't show this problem.
> 
> I'm having problem booting any iso with wine at the moment:
> 
> $ wine64 ./x86_64-softmmu/qemu-system-x86_64.exe -L ./pc-bios \
>     -vnc :1 -cdrom ../../../Downloads/TinyCore-current.iso
> Assertion failed!
> 
> Program: Z:\home\rth\work\qemu\bld-w64\x86_64-softmmu\qemu-system-x86_64.exe
> File: /home/rth/work/qemu/qemu/qemu-coroutine-lock.c, Line 91
> 
> Expression: qemu_in_coroutine()
> 
> abnormal program termination
> 
> Naturally, this isn't happening with a native linux boot with the same arguments.
> 
> But I can boot an alpha rom:
> 
> $ wine64 ./alpha-softmmu/qemu-system-alpha.exe -L ./pc-bios/ -nographic
> PCI: 00:00:0 class 0300 id 1013:00b8
> PCI:   region 0: 10000000
> PCI:   region 1: 12000000
> PCI: 00:01:0 class 0200 id 8086:100e
> PCI:   region 0: 12020000
> PCI:   region 1: 0000c000
> PCI: 00:02:0 class 0101 id 1095:0646
> PCI:   region 0: 0000c040
> PCI:   region 1: 0000c048
> PCI:   region 3: 0000c04c
>>>>
> 
> Which says to me that it's rather unlikely that this basic load/store patch
> could be the problem.
> 
> 
> r~
> 


The current coroutine implementation for Windows does not work (compiler
problem?) and triggers that assertion. Here is one of the workarounds
which help:

     to->action = action;

So we have the paradox situation that adding one more assertion helps to
avoid triggering another assertion.

The alpha ROM obviously does not trigger coroutines. Block I/O usually does.

Stefan

Comments

Stefan Weil May 28, 2014, 5:42 p.m. UTC | #1
Am 28.05.2014 19:21, schrieb Stefan Weil:
> Am 28.05.2014 19:12, schrieb Richard Henderson:
>> On 05/27/2014 03:37 PM, Stefan Weil wrote:
>>> Hi Richard,
>>>
>>> this patch has broken the 64 bit version of QEMU for Windows: a Linux
>>> guest starts booting, but hangs after "Booting the kernel.". I got a bug
>>> report from a user and did a "git bisect" with a Tiny Core Linux guest /
>>> cross build with default options / cross test with wine64 and default
>>> options. Git reported this commit:
>>>
>>> 401c227b0a1134245ec61c6c5a9997cfc963c8e4 is the first bad commit
>>> commit 401c227b0a1134245ec61c6c5a9997cfc963c8e4
>>> Author: Richard Henderson <rth@twiddle.net>
>>> Date:   Thu Jul 25 07:16:52 2013 -1000
>>>
>>>     tcg-i386: Use new return-argument ld/st helpers
>>>    
>>>     Discontinue the jump-around-jump-to-jump scheme, trading it for a single
>>>     immediate move instruction.  The two extra jumps always consume 7 bytes,
>>>     whereas the immediate move is either 5 or 7 bytes depending on where the
>>>     code_gen_buffer gets located.
>>>    
>>>     Signed-off-by: Richard Henderson <rth@twiddle.net>
>>>
>>> :040000 040000 dfd9a66c85713cd1886a3342de1e9ac95d7ea43f
>>> df8673dea69bc89cc2cc979aa24415e3fea4ed53 M    include
>>> :040000 040000 1f7cd5291f2c69b4126c63bd567c6b106eb332c9
>>> 87e7ece766168dda860b513dc97fe5af28ec2c4b M    tcg
>>>
>>> 32 bit versions of QEMU for Windows don't show this problem.
>>
>> I'm having problem booting any iso with wine at the moment:
>>
>> $ wine64 ./x86_64-softmmu/qemu-system-x86_64.exe -L ./pc-bios \
>>     -vnc :1 -cdrom ../../../Downloads/TinyCore-current.iso
>> Assertion failed!
>>
>> Program: Z:\home\rth\work\qemu\bld-w64\x86_64-softmmu\qemu-system-x86_64.exe
>> File: /home/rth/work/qemu/qemu/qemu-coroutine-lock.c, Line 91
>>
>> Expression: qemu_in_coroutine()
>>
>> abnormal program termination
>>
>> Naturally, this isn't happening with a native linux boot with the same arguments.
>>
>> But I can boot an alpha rom:
>>
>> $ wine64 ./alpha-softmmu/qemu-system-alpha.exe -L ./pc-bios/ -nographic
>> PCI: 00:00:0 class 0300 id 1013:00b8
>> PCI:   region 0: 10000000
>> PCI:   region 1: 12000000
>> PCI: 00:01:0 class 0200 id 8086:100e
>> PCI:   region 0: 12020000
>> PCI:   region 1: 0000c000
>> PCI: 00:02:0 class 0101 id 1095:0646
>> PCI:   region 0: 0000c040
>> PCI:   region 1: 0000c048
>> PCI:   region 3: 0000c04c
>>>>>
>>
>> Which says to me that it's rather unlikely that this basic load/store patch
>> could be the problem.


Well, git bisect found it, and it definitely makes a difference whether
this patch is applied or not.

Typical problems with w64 are pointer / long size (pointers are 8 byte,
long integers are only 4 byte) or calling conventions (they differ from
w32 or Linux). See
https://en.wikipedia.org/wiki/X86_calling_conventions#Microsoft_x64_calling_convention
or http://msdn.microsoft.com/en-us/library/ms235286.aspx.

Stefan
Richard Henderson May 28, 2014, 5:43 p.m. UTC | #2
On 05/28/2014 10:21 AM, Stefan Weil wrote:
> The alpha ROM obviously does not trigger coroutines. Block I/O usually does.

Sure.  But obviously it also contains load/store operations.


r~
Stefan Weil May 28, 2014, 5:47 p.m. UTC | #3
Am 28.05.2014 19:43, schrieb Richard Henderson:
> On 05/28/2014 10:21 AM, Stefan Weil wrote:
>> The alpha ROM obviously does not trigger coroutines. Block I/O usually does.
> 
> Sure.  But obviously it also contains load/store operations.
> 
> 
> r~
> 

My remark on coroutines should explain why you get an assertion in one
case. Alpha ROM works, and so does the PC BIOS which also works fine
with w64. All reported and observed problems with w64 are after "Booting
the kernel".

Stefan
diff mbox

Patch

diff --git a/coroutine-win32.c b/coroutine-win32.c
index edc1f72..d4c40d3 100644
--- a/coroutine-win32.c
+++ b/coroutine-win32.c
@@ -42,6 +42,7 @@  CoroutineAction qemu_coroutine_switch(Coroutine
*from_, Coroutine *to_,
     CoroutineWin32 *from = DO_UPCAST(CoroutineWin32, base, from_);
     CoroutineWin32 *to = DO_UPCAST(CoroutineWin32, base, to_);

+    g_assert(current == from_);
     current = to_;