RE: [PATCH] Add minimal Hexagon target - First in a series of patches - linux-user changes + linux-user/hexagon + skeleton of target/hexagon - Files in target/hexagon/imported are from another project and therefore do not conform to qemu coding standards

[Taylor Simpson]

For the general DEBUG_HEX messages, I think the trace infrastructure isn't 
quite what I'm looking for.

Here's a sample of what it prints
Start packet: pc = 0x4002f0
Packet committed: pc = 0x4002f0
Regs written
        r18 = -69420 (0xfffef0d4)
        r19 = -69412 (0xfffef0dc)
Stores
        memd[0xfffef0b0] = 0 (0x0000000000000000)
Next PC = 0x4002f8
Exec counters: pkt = 17, insn = 24, hvx = 0

For performance, I'd like a way to disable them completely when --enable-debug 
isn't passed to configure.  It looks like #ifdef CONFIG_DEBUG_TCG will work for 
this.  Then, I'd like a command-line option to turn them on.  The "-d exec" 
seems like a reasonable place.  So, the code will look like this

#ifdef CONFIG_DEBUG_TCG
    if (qemu_loglevel_maks(CPU_LOG_EXEC)) {
        qemu_log(...);
    }
#endif

Please let me know if I'm on the right track here.

Thanks,
Taylor


-----Original Message-----
From: Laurent Vivier <address@hidden>
Sent: Tuesday, November 19, 2019 12:13 PM
To: Taylor Simpson <address@hidden>; Philippe Mathieu-Daudé <address@hidden>; 
address@hidden; address@hidden
Subject: Re: [PATCH] Add minimal Hexagon target - First in a series of patches 
- linux-user changes + linux-user/hexagon + skeleton of target/hexagon - Files 
in target/hexagon/imported are from another project and therefore do not 
conform to qemu coding standards


Le 19/11/2019 à 18:22, Taylor Simpson a écrit :
> Thanks for all the feedback on the patch.  I'll summarize my TODO list here.  
> Please let me know if there's anything I missed.
> - Add a README file in the imported directory to make it clear that the code 
> comes from another project.  Personally, I prefer keeping the name as 
> "imported".  It was suggested by Richard at the meeting.  Also as a heads-up, 
> that is a small subset of the files that will be in that directory 
> eventually.  Right now, it is the minimum needed to build the skeleton target.
> - Work on the .checkpatchignore as Philippe suggested.
> - Split out the "[__SIGRTMAX - 1] = __SIGRTMIN + 1" into a separate patch.
> - Clean up the long subject line.
> - Add license text to the new files.
> - Remove the DEBUG_HEX blocks.  In general the DEBUG_HEX macro controls a 
> bunch of debugging output as you'll see in later patches.  In the long run, I 
> think it should be replaces with a macro that is defined when configured with 
> --enable-debug and then an additional command-line argument.  I haven't 
> looked into this, so any pointers would be appreciated.

You can have a look to the trace infrastructure (docs/devel/tracing.txt). We 
have also some qemu_log() macros for low level debugging.

> - Laurent suggested I split the patch into two parts: linux-user and 
> target/hexagon.  If I do that, which one should contain the changes to common 
> files (e.g., configure)?  Also, note that we won't be able to build until 
> both patches are merged.  Is that OK?

You should add target/hexagon first, and it should not be build as we don't 
have any target (hexagon-linux-user or hexagon-softmmu), then you can add 
linux-user part that will be built and use the target/hexagone CPU. I think the 
configure part should go to the linux-user part as it enables the build.

I asked to split the patch for review purpose, but this should not break 
anything (to allow bisect).

Thanks,
Laurent

>
> Thanks,
> Taylor
>
>
> -----Original Message-----
> From: Philippe Mathieu-Daudé <address@hidden>
> Sent: Tuesday, November 19, 2019 9:19 AM
> To: Taylor Simpson <address@hidden>; address@hidden;
> address@hidden; address@hidden
> Subject: Re: [PATCH] Add minimal Hexagon target - First in a series of
> patches - linux-user changes + linux-user/hexagon + skeleton of
> target/hexagon - Files in target/hexagon/imported are from another
> project and therefore do not conform to qemu coding standards
>
> ----------------------------------------------------------------------
> ---
> CAUTION: This email originated from outside of the organization.
> ----------------------------------------------------------------------
> ---
>
> On 11/19/19 12:58 AM, Taylor Simpson wrote:
>> Signed-off-by: Taylor Simpson <address@hidden>
>> ---
> [...]
>>   target/hexagon/imported/global_types.h      |  25 +++
>>   target/hexagon/imported/iss_ver_registers.h | 183 +++++++++++++++
>>   target/hexagon/imported/max.h               |  78 +++++++
>>   target/hexagon/imported/regs.h              |  19 ++
>
> Maybe you can rename this directory as:
>
> target/hexagon/dsp-sdk/
>
> and add a README "Files under this directory are imported from the SDK 
> available once registered on developer.qualcomm.com ..."
>
>