[Qemu-devel] [PATCH] Added more entries to the LEON processor configuration register

[Qemu-devel] [PATCH] Added more entries to the LEON processor configuration register

[Ronald Hecht]

Signed-off-by: Ronald Hecht <ronald.hecht@gmx.de>
---
 target-sparc/helper.h      |    1 +
 target-sparc/ldst_helper.c |    6 ++++++
 target-sparc/translate.c   |   10 +++-------
 3 files changed, 10 insertions(+), 7 deletions(-)

diff --git a/target-sparc/helper.h b/target-sparc/helper.h
index 74ecad1..23a2ad4 100644
--- a/target-sparc/helper.h
+++ b/target-sparc/helper.h
@@ -5,6 +5,7 @@ DEF_HELPER_1(rett, void, env)
 DEF_HELPER_2(wrpsr, void, env, tl)
 DEF_HELPER_1(rdpsr, tl, env)
 DEF_HELPER_1(power_down, void, env)
+DEF_HELPER_1(rdasr17, tl, env)
 #else
 DEF_HELPER_2(wrpil, void, env, tl)
 DEF_HELPER_2(wrpstate, void, env, tl)
diff --git a/target-sparc/ldst_helper.c b/target-sparc/ldst_helper.c
index bb5016c..d620d8f 100644
--- a/target-sparc/ldst_helper.c
+++ b/target-sparc/ldst_helper.c
@@ -2326,6 +2326,12 @@ void helper_power_down(CPUSPARCState *env)
     cpu_loop_exit(env);
 }
 
+target_ulong helper_rdasr17(CPUSPARCState *env)
+{
+    /* CPU ID, Meiko FPU, SPARC V8, Number of register windows */
+    return env->cpu_index << 28 | (2 << 10) | (1 << 8) | (env->nwindows - 1);
+}
+
 #if !defined(CONFIG_USER_ONLY)
 #ifndef TARGET_SPARC64
 void cpu_unassigned_access(CPUSPARCState *env, target_phys_addr_t addr,
diff --git a/target-sparc/translate.c b/target-sparc/translate.c
index 9babaa8..c0f7887 100644
--- a/target-sparc/translate.c
+++ b/target-sparc/translate.c
@@ -2590,13 +2590,9 @@ static void disas_sparc_insn(DisasContext * dc, unsigned int insn)
                                        microSPARC II */
                     /* Read Asr17 */
                     if (rs1 == 0x11 && dc->def->features & CPU_FEATURE_ASR17) {
-                        TCGv r_const;
-
-                        /* Read Asr17 for a Leon3 monoprocessor */
-                        r_const = tcg_const_tl((1 << 8)
-                                               | (dc->def->nwindows - 1));
-                        gen_movl_TN_reg(rd, r_const);
-                        tcg_temp_free(r_const);
+                        /* Read Asr17 on LEON3 */
+                        gen_helper_rdasr17(cpu_dst, cpu_env);
+                        gen_movl_TN_reg(rd, cpu_dst);
                         break;
                     }
 #endif
-- 
1.7.2.5

Re: [Qemu-devel] [PATCH] Added more entries to the LEON processor configuration register

[Fabien Chouteau]

On 09/18/2012 03:29 PM, Ronald Hecht wrote:
> Signed-off-by: Ronald Hecht <ronald.hecht@gmx.de>

Reviewed-by: Fabien Chouteau <chouteau@adacore.com>

-- 
Fabien Chouteau

Re: [Qemu-devel] [PATCH] Added more entries to the LEON processor configuration register

[Blue Swirl]

On Tue, Sep 18, 2012 at 1:29 PM, Ronald Hecht <ronald.hecht@gmx.de> wrote:
> Signed-off-by: Ronald Hecht <ronald.hecht@gmx.de>
> ---
>  target-sparc/helper.h      |    1 +
>  target-sparc/ldst_helper.c |    6 ++++++
>  target-sparc/translate.c   |   10 +++-------
>  3 files changed, 10 insertions(+), 7 deletions(-)
>
> diff --git a/target-sparc/helper.h b/target-sparc/helper.h
> index 74ecad1..23a2ad4 100644
> --- a/target-sparc/helper.h
> +++ b/target-sparc/helper.h
> @@ -5,6 +5,7 @@ DEF_HELPER_1(rett, void, env)
>  DEF_HELPER_2(wrpsr, void, env, tl)
>  DEF_HELPER_1(rdpsr, tl, env)
>  DEF_HELPER_1(power_down, void, env)
> +DEF_HELPER_1(rdasr17, tl, env)
>  #else
>  DEF_HELPER_2(wrpil, void, env, tl)
>  DEF_HELPER_2(wrpstate, void, env, tl)
> diff --git a/target-sparc/ldst_helper.c b/target-sparc/ldst_helper.c
> index bb5016c..d620d8f 100644
> --- a/target-sparc/ldst_helper.c
> +++ b/target-sparc/ldst_helper.c
> @@ -2326,6 +2326,12 @@ void helper_power_down(CPUSPARCState *env)
>      cpu_loop_exit(env);
>  }
>
> +target_ulong helper_rdasr17(CPUSPARCState *env)
> +{
> +    /* CPU ID, Meiko FPU, SPARC V8, Number of register windows */
> +    return env->cpu_index << 28 | (2 << 10) | (1 << 8) | (env->nwindows - 1);
> +}
> +
>  #if !defined(CONFIG_USER_ONLY)
>  #ifndef TARGET_SPARC64
>  void cpu_unassigned_access(CPUSPARCState *env, target_phys_addr_t addr,
> diff --git a/target-sparc/translate.c b/target-sparc/translate.c
> index 9babaa8..c0f7887 100644
> --- a/target-sparc/translate.c
> +++ b/target-sparc/translate.c
> @@ -2590,13 +2590,9 @@ static void disas_sparc_insn(DisasContext * dc, unsigned int insn)
>                                         microSPARC II */
>                      /* Read Asr17 */
>                      if (rs1 == 0x11 && dc->def->features & CPU_FEATURE_ASR17) {
> -                        TCGv r_const;
> -
> -                        /* Read Asr17 for a Leon3 monoprocessor */
> -                        r_const = tcg_const_tl((1 << 8)
> -                                               | (dc->def->nwindows - 1));
> -                        gen_movl_TN_reg(rd, r_const);
> -                        tcg_temp_free(r_const);
> +                        /* Read Asr17 on LEON3 */
> +                        gen_helper_rdasr17(cpu_dst, cpu_env);

Is the register is read very often? If it is, we could avoid the
helper call by performing the load of env->cpu_index, the shift and OR
with the above constant with TCG ops. The arithmetic could be even
avoided completely by storing precalculated values in cpu_index, then
other accesses to cpu_index need to shift right by 28 to read the
index (depends on balance of other cpu_index uses vs. this
instruction).

> +                        gen_movl_TN_reg(rd, cpu_dst);
>                          break;
>                      }
>  #endif
> --
> 1.7.2.5
>

Re: [Qemu-devel] [PATCH] Added more entries to the LEON processor configuration register

[Ronald Hecht]

On 09/19/2012 08:56 PM, Blue Swirl wrote:
>> --- a/target-sparc/translate.c
>> +++ b/target-sparc/translate.c
>> @@ -2590,13 +2590,9 @@ static void disas_sparc_insn(DisasContext * dc, unsigned int insn)
>>                                          microSPARC II */
>>                       /* Read Asr17 */
>>                       if (rs1 == 0x11&&  dc->def->features&  CPU_FEATURE_ASR17) {
>> -                        TCGv r_const;
>> -
>> -                        /* Read Asr17 for a Leon3 monoprocessor */
>> -                        r_const = tcg_const_tl((1<<  8)
>> -                                               | (dc->def->nwindows - 1));
>> -                        gen_movl_TN_reg(rd, r_const);
>> -                        tcg_temp_free(r_const);
>> +                        /* Read Asr17 on LEON3 */
>> +                        gen_helper_rdasr17(cpu_dst, cpu_env);
>>      
> Is the register is read very often? If it is, we could avoid the
> helper call by performing the load of env->cpu_index, the shift and OR
> with the above constant with TCG ops. The arithmetic could be even
> avoided completely by storing precalculated values in cpu_index, then
> other accesses to cpu_index need to shift right by 28 to read the
> index (depends on balance of other cpu_index uses vs. this
> instruction).
>    

The register is read quite often, but not as often as the PSR. It is 
normally read when entering the OS (Linux, PikeOS ...) Kernel to know on 
which CPU we are running a thread. But polluting the cpu_index in env 
sounds a bit strange and overkill to me. Do you really mean the 
cpu_index in the structure CPUSPARCState? Or could we place it in 
dc->def as a constant?

Best regards,
Ronald

Re: [Qemu-devel] [PATCH] Added more entries to the LEON processor configuration register

[Blue Swirl]

On Thu, Sep 20, 2012 at 7:15 AM, Ronald Hecht <ronald.hecht@gmx.de> wrote:
> On 09/19/2012 08:56 PM, Blue Swirl wrote:
>>>
>>> --- a/target-sparc/translate.c
>>> +++ b/target-sparc/translate.c
>>> @@ -2590,13 +2590,9 @@ static void disas_sparc_insn(DisasContext * dc,
>>> unsigned int insn)
>>>                                          microSPARC II */
>>>                       /* Read Asr17 */
>>>                       if (rs1 == 0x11&&  dc->def->features&
>>> CPU_FEATURE_ASR17) {
>>>
>>> -                        TCGv r_const;
>>> -
>>> -                        /* Read Asr17 for a Leon3 monoprocessor */
>>> -                        r_const = tcg_const_tl((1<<  8)
>>> -                                               | (dc->def->nwindows -
>>> 1));
>>> -                        gen_movl_TN_reg(rd, r_const);
>>> -                        tcg_temp_free(r_const);
>>> +                        /* Read Asr17 on LEON3 */
>>> +                        gen_helper_rdasr17(cpu_dst, cpu_env);
>>>
>>
>> Is the register is read very often? If it is, we could avoid the
>> helper call by performing the load of env->cpu_index, the shift and OR
>> with the above constant with TCG ops. The arithmetic could be even
>> avoided completely by storing precalculated values in cpu_index, then
>> other accesses to cpu_index need to shift right by 28 to read the
>> index (depends on balance of other cpu_index uses vs. this
>> instruction).
>>
>
>
> The register is read quite often, but not as often as the PSR. It is
> normally read when entering the OS (Linux, PikeOS ...) Kernel to know on
> which CPU we are running a thread. But polluting the cpu_index in env sounds
> a bit strange and overkill to me. Do you really mean the cpu_index in the
> structure CPUSPARCState? Or could we place it in dc->def as a constant?

It could be a new field specific to asr17. The field value is not
necessarily known at translation time since translation could be done
for another CPU, so dc->def would not work.

>
> Best regards,
> Ronald