Re: [Qemu-devel] Actual TB code doesn't look like what was intended (TCG issue)?

[Peter Maydell]

On 24 June 2011 09:34, Max Filippov <address@hidden> wrote:
> I thought it doesn't matter. It's target-xtensa that I've been
> developing

Ah...

>> My first guess is that the target's front end might have a bug
>> where it wrongly bakes in assumptions about bits of the CPUState.
>> QEMU will occasionally retranslate-in-place a TB (if a load in
>> the TB causes an exception) so if the frontend generates different
>> code the second time around things will go wrong...
>>
>> You should be able to find out what's stomping on the code
>> with the aid of a debugger and some watchpoints.
>
> I just thought that "lea    -0x10(%rbx),%esp" may not appear
> in generated code at all, and in the OUT section (which is for
> different MMU mode, as I can see now) it is
> "lea    -0x10(%rbx),%r12d".
> The instruction itself looks odd: it writes to esp and the sizes
> of the registers it operates on are different.

Yes, typically when we retranslate code in place then the second
time around we will stop before the end of the TB, so if there
is a mismatch in the generated code this usually manifests as
a corrupted instruction where half of it got rewritten.

Here are my rules of thumb for generating code where the code
generated might change based on some bit of CPU state:

When you are generating code, if the code you generate will
change based on the contents of something in the CPUState struct,
then the bit of CPUState you are looking at has to be one of:
 (1) encoded in the TB flags (or tb->pc or tb->cs_base)
      (and gen_intermediate_code_internal() must read and
      use the value in tb->tb_flags, not the one in env)
 (2) always constant for the life of the simulation
      (eg env->features if you have some sort of
      "target feature support" flags)
 (3) specially handled so that when it changes then
      all TBs or at least all affected TBs are flushed
      (env->breakpoints is in this category), and also
      if the change is the result of some instruction then
      you must terminate the TB after that instruction.
      This is often used for things that rarely change and/or
      where you can't fit the whole value into tb_flags.

The reason for this is that the CPUState you're passed in
is not guaranteed to be the state of the CPU at the PC
which you are starting translation from.

This is the xtensa port at
http://jcmvbkbc.spb.ru/git/?p=dumb/qemu-xtensa.git;a=shortlog;h=refs/heads/xtensa
right?

It looks like you're breaking these rules with a lot of
the fields in your DisasContext. (Most obviously, you
need to translate code from tb->pc, not env->pc, and
xtensa_get_ring() and xtensa_get_cring() should not read
from env->sregs[PS]. But you should be clear for every
field in DisasContext which category it falls into.)

-- PMM