Re: [PATCH v3 00/70] TCI fixes and cleanups

[Alex Bennée]

Alex Bennée <alex.bennee@linaro.org> writes:

> Alex Bennée <alex.bennee@linaro.org> writes:
>
>> Richard Henderson <richard.henderson@linaro.org> writes:
>>
>>> Changes since v2:
>>>   * 20-something patches are now upstream.
>>>   * Increase testing timeout for tci.
>>>   * Gitlab testing for tci w/ 32-bit host.
>>
>> Hmm that fails with this applied on the current master:
>>
>> cc -Ilibqemu-i386-linux-user.fa.p -I. -I.. -Itarget/i386 -I../target/i386 
>> -I../linux-user/host/i386 -Ilinux-user -I../linux-user -Ilinux-user/i386 
>> -I../linux-user/i386 -I../capstone/include/capstone -Itrace -Iqapi -Iui 
>> -Iui/shader -I/usr/include/glib-2.0 -I/usr/lib/glib-2.0/include 
>> -fdiagnostics-color=auto -pipe -Wall -Winvalid-pch -Werror -std=gnu99 -O2 -g 
>> -isystem /builds/stsquad/qemu/linux-headers -isystem linux-headers -iquote . 
>> -iquote /builds/stsquad/qemu -iquote /builds/stsquad/qemu/include -iquote 
>> /builds/stsquad/qemu/disas/libvixl -iquote /builds/stsquad/qemu/tcg/tci 
>> -iquote /builds/stsquad/qemu/accel/tcg -pthread -U_FORTIFY_SOURCE 
>> -D_FORTIFY_SOURCE=2 -m32 -D_GNU_SOURCE -D_FILE_OFFSET_BITS=64 
>> -D_LARGEFILE_SOURCE -Wstrict-prototypes -Wredundant-decls -Wundef 
>> -Wwrite-strings -Wmissing-prototypes -fno-strict-aliasing -fno-common 
>> -fwrapv -m32 -Wold-style-declaration -Wold-style-definition -Wtype-limits 
>> -Wformat-security -Wformat-y2k -Winit-self -Wignored-qualifiers -Wempty-body 
>> -Wnested-externs -Wendif-labels -Wexpansion-to-defined 
>> -Wimplicit-fallthrough=2 -Wno-missing-include-dirs -Wno-shift-negative-value 
>> -Wno-psabi -fstack-protector-strong -fPIC -isystem../linux-headers 
>> -isystemlinux-headers -DNEED_CPU_H 
>> '-DCONFIG_TARGET="i386-linux-user-config-target.h"' 
>> '-DCONFIG_DEVICES="i386-linux-user-config-devices.h"' -MD -MQ 
>> libqemu-i386-linux-user.fa.p/tcg_tci.c.o -MF 
>> libqemu-i386-linux-user.fa.p/tcg_tci.c.o.d -o 
>> libqemu-i386-linux-user.fa.p/tcg_tci.c.o -c ../tcg/tci.c
>> ../tcg/tci.c: In function 'tcg_qemu_tb_exec':
>> ../tcg/tci.c:317:37: error: passing argument 1 of 'g2h' makes pointer from 
>> integer without a cast [-Werror=int-conversion]
>>   317 | # define qemu_ld_ub      ldub_p(g2h(taddr))
>>       |                                     ^~~~~
>>       |                                     |
>>       |                                     target_ulong {aka unsigned int}
>> ../tcg/tci.c:923:25: note: in expansion of macro 'qemu_ld_ub'
>>   923 |                 tmp32 = qemu_ld_ub;
>>       |                         ^~~~~~~~~~
>>
>> and more:
>>
>> https://gitlab.com/stsquad/qemu/-/jobs/1076231320
>
> Looks like it was broken by:
>
> 3e8f1628e864201692aa28996f8f64f9761555af is the first bad commit
> commit 3e8f1628e864201692aa28996f8f64f9761555af
> Author: Richard Henderson <richard.henderson@linaro.org>
> Date:   Fri Feb 12 10:48:43 2021 -0800
>
>     exec: Use cpu_untagged_addr in g2h; split out g2h_untagged
>
>     Use g2h_untagged in contexts that have no cpu, e.g. the binary
>     loaders that operate before the primary cpu is created.  As a
>     colollary, target_mmap and friends must use untagged addresses,
>     since they are used by the loaders.
>
>     Use g2h_untagged on values returned from target_mmap, as the
>     kernel never applies a tag itself.
>
>     Use g2h_untagged on all pc values.  The only current user of
>     tags, aarch64, removes tags from code addresses upon branch,
>     so "pc" is always untagged.
>
>     Use g2h with the cpu context on hand wherever possible.
>
>     Use g2h_untagged in lock_user, which will be updated soon.
>
>     Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
>     Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
>     Message-id: 20210212184902.1251044-13-richard.henderson@linaro.org
>     Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
<snip>

I will now move to v4 which I missed and has a fix for this ;-)

-- 
Alex Bennée