Re: [Qemu-devel] [PATCH 0/4] new Blackfin QEMU port

[Alexander Graf]

On 01.02.2011, at 05:19, Mike Frysinger wrote:

> On Mon, Jan 31, 2011 at 09:00, Edgar E. Iglesias wrote:
>> * Some operations seem to translate to large amounts of tcg code,
>>  maybe you should consider using helpers for those. An example
>>  is gen_rot_tl.
> 
> seems like generated native code is better even if it is larger

Is this true even when you declare the helper function PURE and CONST? I'm not 
saying it isn't, but it's certainly worth giving it a shot since you seem to 
have implemented the helper way now anyways :)

> 
>> * Maybe you should rename target-bfin/opcode/bfin.h into
>>  target-bfin/opcodes.h? or similiar..
> 
> i prefer existing usage since it mirrors existing sourceware tree
> where this file comes from
> 
>> * gen_hwloop_check() seems to be looking at env->lbreg from the
>>  translator, is that OK? What happens if env->lbreg changes at
>>  runtime?
> 
> this is taken care of already
> 
>> * cpu_get_tb_cpu_state() doesn't define any tb flags?
> 
> no idea what this func is supposed to do

Qemu uses the tb flags as identifier for a translation block. So a tb is only 
reused, when the physical address, flags and code segment (x86 thing) are equal.

This can be used to have privileged operation checks in translated code and 
just put the privileged state flag in the tb flags. Then the same code address 
run in user mode is different from when it's run in kernel mode.

> 
>> I think QEMU should aim to be fast and correct. There is no need to
>> be cycle accurate but emulation should IMO always aim to produce the
>> correct results.
> 
> it does produce the correct results with correct insns.  it is
> referring to invalid insns.  the hardware itself sometimes produces
> undefined behavior with some invalid insns.
> 
>> "- transactional parallel instructions
>>         - on the hardware, if a load/store causes an exception, the other
>>           insns do not change register states either.  in qemu, they do,
>>           but since those exceptions will kill the program anyways, who
>>           cares.  no intermediate store buffers!"
>> 
>> This might be true for user emulation but I assume you'd want to fix this
>> for system emulation (at some point in time). Maybe you should just note
>> that it's not supported and leave it at that.
> 
> i dont see the point there either.  in order to handle this, the
> majority of insns would need to store all their results in
> intermediates and then move them to their final destination.

I guess you could just save off the register state on transaction start and 
restore it on transaction abort (page fault)? But yes, this is only really 
useful for system emulation, so you don't really have to care for now.


Alex