Re: [Qemu-devel] [PATCH for-2.12] tcg/mips: Handle large offsets from target env to tlb_table

[Peter Maydell]

On 13 April 2018 at 20:09, Richard Henderson <address@hidden> wrote:
> On 04/13/2018 04:23 AM, Peter Maydell wrote:
>> The MIPS TCG target makes the assumption that the offset from the
>> target env pointer to the tlb_table is less than about 64K. This
>> used to be true, but gradual addition of features to the Arm
>> target means that it's no longer true there. This results in
>> the build-time assertion failing:
>>
>> In file included from /home/pm215/qemu/include/qemu/osdep.h:36:0,
>>                  from /home/pm215/qemu/tcg/tcg.c:28:
>> /home/pm215/qemu/tcg/mips/tcg-target.inc.c: In function ‘tcg_out_tlb_load’:
>> /home/pm215/qemu/include/qemu/compiler.h:90:36: error: static assertion 
>> failed: "not expecting: offsetof(CPUArchState, tlb_table[NB_MMU_MODES - 
>> 1][1]) > 0x7ff0 + 0x7fff"
>>  #define QEMU_BUILD_BUG_MSG(x, msg) _Static_assert(!(x), msg)
>>                                     ^
>> /home/pm215/qemu/include/qemu/compiler.h:98:30: note: in expansion of macro 
>> ‘QEMU_BUILD_BUG_MSG’
>>  #define QEMU_BUILD_BUG_ON(x) QEMU_BUILD_BUG_MSG(x, "not expecting: " #x)
>>                               ^
>> /home/pm215/qemu/tcg/mips/tcg-target.inc.c:1236:9: note: in expansion of 
>> macro ‘QEMU_BUILD_BUG_ON’
>>          QEMU_BUILD_BUG_ON(offsetof(CPUArchState,
>>          ^
>> /home/pm215/qemu/rules.mak:66: recipe for target 'tcg/tcg.o' failed
>
> Yes, I asked for help on this from the MIPS folks back in January when I 
> posted
> patches for ppc and arm(32) hosts.  And then of course forgot about it in the
> interim.
>
>> +    while (add_off >= 0x8000) {
>> +        /* Most target env are smaller than 32k, but a few are larger than 
>> 64k,
>> +         * so handle an arbitrarily large offset.
>> +         */
>>          tcg_out_opc_imm(s, ALIAS_PADDI, TCG_REG_A0, TCG_REG_A0, 0x7ff0);
>>          cmp_off -= 0x7ff0;
>>          add_off -= 0x7ff0;
>
> This is a pretty darned good solution, really.  I should have thought of it
> myself at the time.  The new AArch64 offset is about 80k, so there will be two
> adds emitted.  Loading C<<16 into a temp register and then adding would be no
> better in the end (one can only add C<<16 directly with mipsr6 extensions).
>
> Reviewed-by: Richard Henderson <address@hidden>

Thanks; applied to master.

-- PMM