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

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

[Peter Maydell]

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

An ideal long term approach would be to rearrange the CPU state
so that the tlb_table was not so far along it, but this is tricky
because it would move it from the "not cleared on CPU reset" part
of the struct to the "cleared on CPU reset" part. As a simple fix
for the 2.12 release, make the MIPS TCG target handle an arbitrary
offset by emitting more add instructions. This will mean an extra
instruction in the fastpath for TCG loads and stores for the
affected guests (currently just aarch64-softmmu).

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
This is sufficient that on MIPS host we can now at least build
and run an aarch64 guest kernel. I haven't tried 'make check'
because the only MIPS system I have access to is way too slow...

 tcg/mips/tcg-target.inc.c | 11 ++++-------
 1 file changed, 4 insertions(+), 7 deletions(-)

diff --git a/tcg/mips/tcg-target.inc.c b/tcg/mips/tcg-target.inc.c
index 4b55ab8856..ca5f1d4894 100644
--- a/tcg/mips/tcg-target.inc.c
+++ b/tcg/mips/tcg-target.inc.c
@@ -1229,13 +1229,10 @@ static void tcg_out_tlb_load(TCGContext *s, TCGReg base, TCGReg addrl,
     tcg_out_opc_reg(s, ALIAS_PADD, TCG_REG_A0, TCG_REG_A0, TCG_AREG0);
 
     /* Compensate for very large offsets.  */
-    if (add_off >= 0x8000) {
-        /* Most target env are smaller than 32k; none are larger than 64k.
-           Simplify the logic here merely to offset by 0x7ff0, giving us a
-           range just shy of 64k.  Check this assumption.  */
-        QEMU_BUILD_BUG_ON(offsetof(CPUArchState,
-                                   tlb_table[NB_MMU_MODES - 1][1])
-                          > 0x7ff0 + 0x7fff);
+    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;
-- 
2.16.2

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

[Michael S. Tsirkin]

On Fri, Apr 13, 2018 at 03:23:36PM +0100, 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
> 
> An ideal long term approach would be to rearrange the CPU state
> so that the tlb_table was not so far along it, but this is tricky
> because it would move it from the "not cleared on CPU reset" part
> of the struct to the "cleared on CPU reset" part. As a simple fix
> for the 2.12 release, make the MIPS TCG target handle an arbitrary
> offset by emitting more add instructions. This will mean an extra
> instruction in the fastpath for TCG loads and stores for the
> affected guests (currently just aarch64-softmmu).
> 
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>

Acked-by: Michael S. Tsirkin <mst@redhat.com>

> ---
> This is sufficient that on MIPS host we can now at least build
> and run an aarch64 guest kernel. I haven't tried 'make check'
> because the only MIPS system I have access to is way too slow...
> 
>  tcg/mips/tcg-target.inc.c | 11 ++++-------
>  1 file changed, 4 insertions(+), 7 deletions(-)
> 
> diff --git a/tcg/mips/tcg-target.inc.c b/tcg/mips/tcg-target.inc.c
> index 4b55ab8856..ca5f1d4894 100644
> --- a/tcg/mips/tcg-target.inc.c
> +++ b/tcg/mips/tcg-target.inc.c
> @@ -1229,13 +1229,10 @@ static void tcg_out_tlb_load(TCGContext *s, TCGReg base, TCGReg addrl,
>      tcg_out_opc_reg(s, ALIAS_PADD, TCG_REG_A0, TCG_REG_A0, TCG_AREG0);
>  
>      /* Compensate for very large offsets.  */
> -    if (add_off >= 0x8000) {
> -        /* Most target env are smaller than 32k; none are larger than 64k.
> -           Simplify the logic here merely to offset by 0x7ff0, giving us a
> -           range just shy of 64k.  Check this assumption.  */
> -        QEMU_BUILD_BUG_ON(offsetof(CPUArchState,
> -                                   tlb_table[NB_MMU_MODES - 1][1])
> -                          > 0x7ff0 + 0x7fff);
> +    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;
> -- 
> 2.16.2

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

[Richard Henderson]

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 <richard.henderson@linaro.org>


r~

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 <rth@twiddle.net> 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 <richard.henderson@linaro.org>

Thanks; applied to master.

-- PMM

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

[Alex Bennée]

Peter Maydell <peter.maydell@linaro.org> writes:

> 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
>
> An ideal long term approach would be to rearrange the CPU state
> so that the tlb_table was not so far along it, but this is tricky
> because it would move it from the "not cleared on CPU reset" part
> of the struct to the "cleared on CPU reset" part.

Is that really a problem? Doesn't it mean we'll just reload the TLB
after a reset?

--
Alex Bennée

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

[Peter Maydell]

On 30 April 2018 at 18:44, Alex Bennée <alex.bennee@linaro.org> wrote:
> Peter Maydell <peter.maydell@linaro.org> writes:
>> An ideal long term approach would be to rearrange the CPU state
>> so that the tlb_table was not so far along it, but this is tricky
>> because it would move it from the "not cleared on CPU reset" part
>> of the struct to the "cleared on CPU reset" part.
>
> Is that really a problem? Doesn't it mean we'll just reload the TLB
> after a reset?

It was the sort of "this subtly changes the semantics on all
hosts for the sake of an issue that's mips-hosts-only" change
that I didn't want to try to reason through and make very close
to release when we didn't have a lot of time to test the results...

thanks
-- PMM