Re: [Qemu-devel] xtensa: new target architecture for qemu

[Blue Swirl]

On Sat, Apr 30, 2011 at 11:24 AM, Max Filippov <address@hidden> wrote:
>>> Essential ISA parts (like core opcodes, special registers, windowed
>>> registers, exceptions and interrupts) are implemented, other (like
>>> TLB, MMU, caches, coprocessors, rare opcodes) are not, although I'm
>>> planning to implement them if/when needed.
>>
>> Nice work. What is the status, can the emulator boot Linux for example?
>
> Rather complex applications based on ThreadX [1] realtime OS
> successfully run on it, but Linux requires at least MMU, which I
> haven't got to yet. But that's one of my goals.
>
>>> I'm wondering if this target could be eligible for inclusion into qemu 
>>> mainline.
>>> If it is, could anyone please review the code [2]?
>>
>> Please send the patch set for easier reviewing to the list, use
>> scripts/checkpatch.pl to avoid some predictable issues.
>
> Is it worth keeping patches as small as they are now (as they only
> reflect development chronology), or maybe it's better to consolidate
> them on some other basis?

Small patches are nicer for review.

>> - options field does not necessarily belong to CPUState, see for
>> example Sparc sparc_def_t how the different models are handled
>
> My intent was to be able to support AMP configuration with cores that
> has different options.
>
>> - env->options or env->singlestep shouldn't be used in disas_insn (it
>> shouldn't take env parameter at all) but the fields should be copied
>> to DisasContext and used from there
>
> What is the reason for that? DisasContext itself has a pointer to env.
> Can it be used instead?

DisasContext shouldn't have that pointer. Some of the decisions during
translation are made based on CPU model/features which don't change
during execution. Other decisions should be made based on the current
CPU state, however usually here we should not rely on env but on
tb_flags. Not using env during disas_insn tries to enforce that.
Please take a look at other targets how this should be handled.

>> - macros should be replaced by inline functions (or enums) when possible
>> - pointers to CPU and chipset docs should be added
>
> Is target-xtensa/README a good place for it?

I'd put CPU doc pointers to translate.c and chipset pointers to
corresponding device file.

>> - commenting out code is a no-no
>> - there is no disassembler for target, can you check if binutils
>> before GPLv3 switch contains one? It should be easy to add.
>
> You mean that there's no support for -d in_asm?

Yes, that would be nice but it can be added later.

>> - if possible, simcall should become a linux-user target instead
>
> simcall is used in freestanding libc provided by Tensilica, I guess
> primarily to ease OS-less development. I can make it an additional
> option, but it seems impractical to remove it completely.

Does that still emulate devices? In that case it would be like
semihosting mode in ARM, so it would be OK to stay where you put it.