* [Qemu-devel] [PATCH v3] target-sparc: Add and use CPU_FEATURE_CASA
@ 2014-02-14 15:16 Sebastian Huber
2014-02-14 15:33 ` Andreas Färber
0 siblings, 1 reply; 7+ messages in thread
From: Sebastian Huber @ 2014-02-14 15:16 UTC (permalink / raw)
To: qemu-devel; +Cc: blauwirbel, Sebastian Huber, afaerber, Fabien Chouteau
The LEON3 processor has support for the CASA instruction which is
normally only available for SPARC V9 processors. Binutils 2.24
and GCC 4.9 will support this instruction for LEON3. GCC uses it to
generate C11 atomic operations.
The CAS synthetic instruction uses an ASI of 0x80. If TARGET_SPARC64 is
not defined use a supervisor data load/store for an ASI of 0x80 in
helper_ld_asi()/helper_st_asi(). The supervisor data load/store was
choosen according to the LEON3 documentation.
The ASI 0x80 is defined in the SPARC V9 manual, Table 12—Address Space
Identifiers (ASIs). Here we have: 0x80, ASI_PRIMARY, Unrestricted
access, Primary address space.
Tested with the following program:
#include <assert.h>
#include <stdatomic.h>
void test(void)
{
atomic_int a;
int e;
_Bool b;
atomic_store(&a, 1);
e = 1;
b = atomic_compare_exchange_strong(&a, &e, 2);
assert(b);
assert(atomic_load(&a) == 2);
atomic_store(&a, 3);
e = 4;
b = atomic_compare_exchange_strong(&a, &e, 5);
assert(!b);
assert(atomic_load(&a) == 3);
}
Tested also on a NGMP board with a LEON4 processor.
Signed-off-by: Sebastian Huber <sebastian.huber@embedded-brains.de>
Reviewed-by: Fabien Chouteau <chouteau@adacore.com>
---
target-sparc/cpu.c | 3 +-
target-sparc/cpu.h | 4 ++-
target-sparc/helper.h | 4 ++-
target-sparc/ldst_helper.c | 28 +++++++++++++++-----------
target-sparc/translate.c | 47 ++++++++++++++++++++++++++++---------------
5 files changed, 54 insertions(+), 32 deletions(-)
diff --git a/target-sparc/cpu.c b/target-sparc/cpu.c
index e7f878e..5806e59 100644
--- a/target-sparc/cpu.c
+++ b/target-sparc/cpu.c
@@ -458,7 +458,8 @@ static const sparc_def_t sparc_defs[] = {
.mmu_trcr_mask = 0xffffffff,
.nwindows = 8,
.features = CPU_DEFAULT_FEATURES | CPU_FEATURE_TA0_SHUTDOWN |
- CPU_FEATURE_ASR17 | CPU_FEATURE_CACHE_CTRL | CPU_FEATURE_POWERDOWN,
+ CPU_FEATURE_ASR17 | CPU_FEATURE_CACHE_CTRL | CPU_FEATURE_POWERDOWN |
+ CPU_FEATURE_CASA,
},
#endif
};
diff --git a/target-sparc/cpu.h b/target-sparc/cpu.h
index c519063..2531cf9 100644
--- a/target-sparc/cpu.h
+++ b/target-sparc/cpu.h
@@ -271,12 +271,14 @@ typedef struct sparc_def_t {
#define CPU_FEATURE_ASR17 (1 << 15)
#define CPU_FEATURE_CACHE_CTRL (1 << 16)
#define CPU_FEATURE_POWERDOWN (1 << 17)
+#define CPU_FEATURE_CASA (1 << 18)
#ifndef TARGET_SPARC64
#define CPU_DEFAULT_FEATURES (CPU_FEATURE_FLOAT | CPU_FEATURE_SWAP | \
CPU_FEATURE_MUL | CPU_FEATURE_DIV | \
CPU_FEATURE_FLUSH | CPU_FEATURE_FSQRT | \
- CPU_FEATURE_FMUL | CPU_FEATURE_FSMULD)
+ CPU_FEATURE_FMUL | CPU_FEATURE_FSMULD | \
+ CPU_FEATURE_CASA)
#else
#define CPU_DEFAULT_FEATURES (CPU_FEATURE_FLOAT | CPU_FEATURE_SWAP | \
CPU_FEATURE_MUL | CPU_FEATURE_DIV | \
diff --git a/target-sparc/helper.h b/target-sparc/helper.h
index 2a771b2..cd8d3fa 100644
--- a/target-sparc/helper.h
+++ b/target-sparc/helper.h
@@ -22,7 +22,6 @@ DEF_HELPER_1(popc, tl, tl)
DEF_HELPER_4(ldda_asi, void, env, tl, int, int)
DEF_HELPER_5(ldf_asi, void, env, tl, int, int, int)
DEF_HELPER_5(stf_asi, void, env, tl, int, int, int)
-DEF_HELPER_5(cas_asi, tl, env, tl, tl, tl, i32)
DEF_HELPER_5(casx_asi, tl, env, tl, tl, tl, i32)
DEF_HELPER_2(set_softint, void, env, i64)
DEF_HELPER_2(clear_softint, void, env, i64)
@@ -31,6 +30,9 @@ DEF_HELPER_2(tick_set_count, void, ptr, i64)
DEF_HELPER_1(tick_get_count, i64, ptr)
DEF_HELPER_2(tick_set_limit, void, ptr, i64)
#endif
+#if !defined(CONFIG_USER_ONLY) || defined(TARGET_SPARC64)
+DEF_HELPER_5(cas_asi, tl, env, tl, tl, tl, i32)
+#endif
DEF_HELPER_3(check_align, void, env, tl, i32)
DEF_HELPER_1(debug, void, env)
DEF_HELPER_1(save, void, env)
diff --git a/target-sparc/ldst_helper.c b/target-sparc/ldst_helper.c
index 92761ad..32491b4 100644
--- a/target-sparc/ldst_helper.c
+++ b/target-sparc/ldst_helper.c
@@ -584,6 +584,7 @@ uint64_t helper_ld_asi(CPUSPARCState *env, target_ulong addr, int asi, int size,
}
break;
case 0xb: /* Supervisor data access */
+ case 0x80:
switch (size) {
case 1:
ret = cpu_ldub_kernel(env, addr);
@@ -955,6 +956,7 @@ void helper_st_asi(CPUSPARCState *env, target_ulong addr, uint64_t val, int asi,
}
break;
case 0xb: /* Supervisor data access */
+ case 0x80:
switch (size) {
case 1:
cpu_stb_kernel(env, addr, val);
@@ -2232,33 +2234,35 @@ void helper_stf_asi(CPUSPARCState *env, target_ulong addr, int asi, int size,
}
}
-target_ulong helper_cas_asi(CPUSPARCState *env, target_ulong addr,
- target_ulong val1, target_ulong val2, uint32_t asi)
+target_ulong helper_casx_asi(CPUSPARCState *env, target_ulong addr,
+ target_ulong val1, target_ulong val2,
+ uint32_t asi)
{
target_ulong ret;
- val2 &= 0xffffffffUL;
- ret = helper_ld_asi(env, addr, asi, 4, 0);
- ret &= 0xffffffffUL;
+ ret = helper_ld_asi(env, addr, asi, 8, 0);
if (val2 == ret) {
- helper_st_asi(env, addr, val1 & 0xffffffffUL, asi, 4);
+ helper_st_asi(env, addr, val1, asi, 8);
}
return ret;
}
+#endif /* TARGET_SPARC64 */
-target_ulong helper_casx_asi(CPUSPARCState *env, target_ulong addr,
- target_ulong val1, target_ulong val2,
- uint32_t asi)
+#if !defined(CONFIG_USER_ONLY) || defined(TARGET_SPARC64)
+target_ulong helper_cas_asi(CPUSPARCState *env, target_ulong addr,
+ target_ulong val1, target_ulong val2, uint32_t asi)
{
target_ulong ret;
- ret = helper_ld_asi(env, addr, asi, 8, 0);
+ val2 &= 0xffffffffUL;
+ ret = helper_ld_asi(env, addr, asi, 4, 0);
+ ret &= 0xffffffffUL;
if (val2 == ret) {
- helper_st_asi(env, addr, val1, asi, 8);
+ helper_st_asi(env, addr, val1 & 0xffffffffUL, asi, 4);
}
return ret;
}
-#endif /* TARGET_SPARC64 */
+#endif /* !defined(CONFIG_USER_ONLY) || defined(TARGET_SPARC64) */
void helper_ldqf(CPUSPARCState *env, target_ulong addr, int mem_idx)
{
diff --git a/target-sparc/translate.c b/target-sparc/translate.c
index 6150b22..7481c85 100644
--- a/target-sparc/translate.c
+++ b/target-sparc/translate.c
@@ -2107,18 +2107,6 @@ static inline void gen_stda_asi(DisasContext *dc, TCGv hi, TCGv addr,
tcg_temp_free_i64(t64);
}
-static inline void gen_cas_asi(DisasContext *dc, TCGv addr,
- TCGv val2, int insn, int rd)
-{
- TCGv val1 = gen_load_gpr(dc, rd);
- TCGv dst = gen_dest_gpr(dc, rd);
- TCGv_i32 r_asi = gen_get_asi(insn, addr);
-
- gen_helper_cas_asi(dst, cpu_env, addr, val1, val2, r_asi);
- tcg_temp_free_i32(r_asi);
- gen_store_gpr(dc, rd, dst);
-}
-
static inline void gen_casx_asi(DisasContext *dc, TCGv addr,
TCGv val2, int insn, int rd)
{
@@ -2229,6 +2217,22 @@ static inline void gen_stda_asi(DisasContext *dc, TCGv hi, TCGv addr,
#endif
#if !defined(CONFIG_USER_ONLY) || defined(TARGET_SPARC64)
+static inline void gen_cas_asi(DisasContext *dc, TCGv addr,
+ TCGv val2, int insn, int rd)
+{
+ TCGv val1 = gen_load_gpr(dc, rd);
+ TCGv dst = gen_dest_gpr(dc, rd);
+#ifdef TARGET_SPARC64
+ TCGv_i32 r_asi = gen_get_asi(insn, addr);
+#else
+ TCGv_i32 r_asi = tcg_const_i32(GET_FIELD(insn, 19, 26));
+#endif
+
+ gen_helper_cas_asi(dst, cpu_env, addr, val1, val2, r_asi);
+ tcg_temp_free_i32(r_asi);
+ gen_store_gpr(dc, rd, dst);
+}
+
static inline void gen_ldstub_asi(TCGv dst, TCGv addr, int insn)
{
TCGv_i64 r_val;
@@ -5103,11 +5107,6 @@ static void disas_sparc_insn(DisasContext * dc, unsigned int insn)
}
gen_stf_asi(cpu_addr, insn, 8, DFPREG(rd));
break;
- case 0x3c: /* V9 casa */
- rs2 = GET_FIELD(insn, 27, 31);
- cpu_src2 = gen_load_gpr(dc, rs2);
- gen_cas_asi(dc, cpu_addr, cpu_src2, insn, rd);
- break;
case 0x3e: /* V9 casxa */
rs2 = GET_FIELD(insn, 27, 31);
cpu_src2 = gen_load_gpr(dc, rs2);
@@ -5120,6 +5119,20 @@ static void disas_sparc_insn(DisasContext * dc, unsigned int insn)
case 0x37: /* stdc */
goto ncp_insn;
#endif
+#if !defined(CONFIG_USER_ONLY) || defined(TARGET_SPARC64)
+ case 0x3c: /* V9 or LEON3 casa */
+ CHECK_FPU_FEATURE(dc, CASA);
+#ifndef TARGET_SPARC64
+ if (IS_IMM)
+ goto illegal_insn;
+ if (!supervisor(dc))
+ goto priv_insn;
+#endif
+ rs2 = GET_FIELD(insn, 27, 31);
+ cpu_src2 = gen_load_gpr(dc, rs2);
+ gen_cas_asi(dc, cpu_addr, cpu_src2, insn, rd);
+ break;
+#endif
default:
goto illegal_insn;
}
--
1.7.7
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [Qemu-devel] [PATCH v3] target-sparc: Add and use CPU_FEATURE_CASA
2014-02-14 15:16 [Qemu-devel] [PATCH v3] target-sparc: Add and use CPU_FEATURE_CASA Sebastian Huber
@ 2014-02-14 15:33 ` Andreas Färber
2014-02-14 15:43 ` Sebastian Huber
2014-02-14 17:27 ` Fabien Chouteau
0 siblings, 2 replies; 7+ messages in thread
From: Andreas Färber @ 2014-02-14 15:33 UTC (permalink / raw)
To: Sebastian Huber, qemu-devel; +Cc: blauwirbel, Fabien Chouteau
Am 14.02.2014 16:16, schrieb Sebastian Huber:
> The LEON3 processor has support for the CASA instruction which is
> normally only available for SPARC V9 processors. Binutils 2.24
> and GCC 4.9 will support this instruction for LEON3. GCC uses it to
> generate C11 atomic operations.
>
> The CAS synthetic instruction uses an ASI of 0x80. If TARGET_SPARC64 is
> not defined use a supervisor data load/store for an ASI of 0x80 in
> helper_ld_asi()/helper_st_asi(). The supervisor data load/store was
> choosen according to the LEON3 documentation.
>
> The ASI 0x80 is defined in the SPARC V9 manual, Table 12—Address Space
> Identifiers (ASIs). Here we have: 0x80, ASI_PRIMARY, Unrestricted
> access, Primary address space.
>
> Tested with the following program:
>
> #include <assert.h>
> #include <stdatomic.h>
>
> void test(void)
> {
> atomic_int a;
> int e;
> _Bool b;
>
> atomic_store(&a, 1);
> e = 1;
> b = atomic_compare_exchange_strong(&a, &e, 2);
> assert(b);
> assert(atomic_load(&a) == 2);
>
> atomic_store(&a, 3);
> e = 4;
> b = atomic_compare_exchange_strong(&a, &e, 5);
> assert(!b);
> assert(atomic_load(&a) == 3);
> }
>
> Tested also on a NGMP board with a LEON4 processor.
>
> Signed-off-by: Sebastian Huber <sebastian.huber@embedded-brains.de>
> Reviewed-by: Fabien Chouteau <chouteau@adacore.com>
> ---
> target-sparc/cpu.c | 3 +-
> target-sparc/cpu.h | 4 ++-
> target-sparc/helper.h | 4 ++-
> target-sparc/ldst_helper.c | 28 +++++++++++++++-----------
> target-sparc/translate.c | 47 ++++++++++++++++++++++++++++---------------
> 5 files changed, 54 insertions(+), 32 deletions(-)
>
> diff --git a/target-sparc/cpu.c b/target-sparc/cpu.c
> index e7f878e..5806e59 100644
> --- a/target-sparc/cpu.c
> +++ b/target-sparc/cpu.c
> @@ -458,7 +458,8 @@ static const sparc_def_t sparc_defs[] = {
> .mmu_trcr_mask = 0xffffffff,
> .nwindows = 8,
> .features = CPU_DEFAULT_FEATURES | CPU_FEATURE_TA0_SHUTDOWN |
> - CPU_FEATURE_ASR17 | CPU_FEATURE_CACHE_CTRL | CPU_FEATURE_POWERDOWN,
> + CPU_FEATURE_ASR17 | CPU_FEATURE_CACHE_CTRL | CPU_FEATURE_POWERDOWN |
> + CPU_FEATURE_CASA,
> },
> #endif
> };
> diff --git a/target-sparc/cpu.h b/target-sparc/cpu.h
> index c519063..2531cf9 100644
> --- a/target-sparc/cpu.h
> +++ b/target-sparc/cpu.h
> @@ -271,12 +271,14 @@ typedef struct sparc_def_t {
> #define CPU_FEATURE_ASR17 (1 << 15)
> #define CPU_FEATURE_CACHE_CTRL (1 << 16)
> #define CPU_FEATURE_POWERDOWN (1 << 17)
> +#define CPU_FEATURE_CASA (1 << 18)
>
> #ifndef TARGET_SPARC64
> #define CPU_DEFAULT_FEATURES (CPU_FEATURE_FLOAT | CPU_FEATURE_SWAP | \
> CPU_FEATURE_MUL | CPU_FEATURE_DIV | \
> CPU_FEATURE_FLUSH | CPU_FEATURE_FSQRT | \
> - CPU_FEATURE_FMUL | CPU_FEATURE_FSMULD)
> + CPU_FEATURE_FMUL | CPU_FEATURE_FSMULD | \
> + CPU_FEATURE_CASA)
> #else
> #define CPU_DEFAULT_FEATURES (CPU_FEATURE_FLOAT | CPU_FEATURE_SWAP | \
> CPU_FEATURE_MUL | CPU_FEATURE_DIV | \
> diff --git a/target-sparc/helper.h b/target-sparc/helper.h
> index 2a771b2..cd8d3fa 100644
> --- a/target-sparc/helper.h
> +++ b/target-sparc/helper.h
> @@ -22,7 +22,6 @@ DEF_HELPER_1(popc, tl, tl)
> DEF_HELPER_4(ldda_asi, void, env, tl, int, int)
> DEF_HELPER_5(ldf_asi, void, env, tl, int, int, int)
> DEF_HELPER_5(stf_asi, void, env, tl, int, int, int)
> -DEF_HELPER_5(cas_asi, tl, env, tl, tl, tl, i32)
> DEF_HELPER_5(casx_asi, tl, env, tl, tl, tl, i32)
> DEF_HELPER_2(set_softint, void, env, i64)
> DEF_HELPER_2(clear_softint, void, env, i64)
> @@ -31,6 +30,9 @@ DEF_HELPER_2(tick_set_count, void, ptr, i64)
> DEF_HELPER_1(tick_get_count, i64, ptr)
> DEF_HELPER_2(tick_set_limit, void, ptr, i64)
> #endif
> +#if !defined(CONFIG_USER_ONLY) || defined(TARGET_SPARC64)
> +DEF_HELPER_5(cas_asi, tl, env, tl, tl, tl, i32)
> +#endif
> DEF_HELPER_3(check_align, void, env, tl, i32)
> DEF_HELPER_1(debug, void, env)
> DEF_HELPER_1(save, void, env)
> diff --git a/target-sparc/ldst_helper.c b/target-sparc/ldst_helper.c
> index 92761ad..32491b4 100644
> --- a/target-sparc/ldst_helper.c
> +++ b/target-sparc/ldst_helper.c
> @@ -584,6 +584,7 @@ uint64_t helper_ld_asi(CPUSPARCState *env, target_ulong addr, int asi, int size,
> }
> break;
> case 0xb: /* Supervisor data access */
> + case 0x80:
> switch (size) {
> case 1:
> ret = cpu_ldub_kernel(env, addr);
> @@ -955,6 +956,7 @@ void helper_st_asi(CPUSPARCState *env, target_ulong addr, uint64_t val, int asi,
> }
> break;
> case 0xb: /* Supervisor data access */
> + case 0x80:
> switch (size) {
> case 1:
> cpu_stb_kernel(env, addr, val);
> @@ -2232,33 +2234,35 @@ void helper_stf_asi(CPUSPARCState *env, target_ulong addr, int asi, int size,
> }
> }
>
> -target_ulong helper_cas_asi(CPUSPARCState *env, target_ulong addr,
> - target_ulong val1, target_ulong val2, uint32_t asi)
> +target_ulong helper_casx_asi(CPUSPARCState *env, target_ulong addr,
> + target_ulong val1, target_ulong val2,
> + uint32_t asi)
> {
> target_ulong ret;
>
> - val2 &= 0xffffffffUL;
> - ret = helper_ld_asi(env, addr, asi, 4, 0);
> - ret &= 0xffffffffUL;
> + ret = helper_ld_asi(env, addr, asi, 8, 0);
> if (val2 == ret) {
> - helper_st_asi(env, addr, val1 & 0xffffffffUL, asi, 4);
> + helper_st_asi(env, addr, val1, asi, 8);
> }
> return ret;
> }
> +#endif /* TARGET_SPARC64 */
>
> -target_ulong helper_casx_asi(CPUSPARCState *env, target_ulong addr,
> - target_ulong val1, target_ulong val2,
> - uint32_t asi)
> +#if !defined(CONFIG_USER_ONLY) || defined(TARGET_SPARC64)
> +target_ulong helper_cas_asi(CPUSPARCState *env, target_ulong addr,
> + target_ulong val1, target_ulong val2, uint32_t asi)
> {
> target_ulong ret;
>
> - ret = helper_ld_asi(env, addr, asi, 8, 0);
> + val2 &= 0xffffffffUL;
> + ret = helper_ld_asi(env, addr, asi, 4, 0);
> + ret &= 0xffffffffUL;
> if (val2 == ret) {
> - helper_st_asi(env, addr, val1, asi, 8);
> + helper_st_asi(env, addr, val1 & 0xffffffffUL, asi, 4);
> }
> return ret;
> }
> -#endif /* TARGET_SPARC64 */
> +#endif /* !defined(CONFIG_USER_ONLY) || defined(TARGET_SPARC64) */
>
> void helper_ldqf(CPUSPARCState *env, target_ulong addr, int mem_idx)
> {
> diff --git a/target-sparc/translate.c b/target-sparc/translate.c
> index 6150b22..7481c85 100644
> --- a/target-sparc/translate.c
> +++ b/target-sparc/translate.c
> @@ -2107,18 +2107,6 @@ static inline void gen_stda_asi(DisasContext *dc, TCGv hi, TCGv addr,
> tcg_temp_free_i64(t64);
> }
>
> -static inline void gen_cas_asi(DisasContext *dc, TCGv addr,
> - TCGv val2, int insn, int rd)
> -{
> - TCGv val1 = gen_load_gpr(dc, rd);
> - TCGv dst = gen_dest_gpr(dc, rd);
> - TCGv_i32 r_asi = gen_get_asi(insn, addr);
> -
> - gen_helper_cas_asi(dst, cpu_env, addr, val1, val2, r_asi);
> - tcg_temp_free_i32(r_asi);
> - gen_store_gpr(dc, rd, dst);
> -}
> -
> static inline void gen_casx_asi(DisasContext *dc, TCGv addr,
> TCGv val2, int insn, int rd)
> {
> @@ -2229,6 +2217,22 @@ static inline void gen_stda_asi(DisasContext *dc, TCGv hi, TCGv addr,
> #endif
>
> #if !defined(CONFIG_USER_ONLY) || defined(TARGET_SPARC64)
> +static inline void gen_cas_asi(DisasContext *dc, TCGv addr,
> + TCGv val2, int insn, int rd)
> +{
> + TCGv val1 = gen_load_gpr(dc, rd);
> + TCGv dst = gen_dest_gpr(dc, rd);
> +#ifdef TARGET_SPARC64
> + TCGv_i32 r_asi = gen_get_asi(insn, addr);
> +#else
> + TCGv_i32 r_asi = tcg_const_i32(GET_FIELD(insn, 19, 26));
> +#endif
> +
> + gen_helper_cas_asi(dst, cpu_env, addr, val1, val2, r_asi);
> + tcg_temp_free_i32(r_asi);
> + gen_store_gpr(dc, rd, dst);
> +}
> +
> static inline void gen_ldstub_asi(TCGv dst, TCGv addr, int insn)
> {
> TCGv_i64 r_val;
> @@ -5103,11 +5107,6 @@ static void disas_sparc_insn(DisasContext * dc, unsigned int insn)
> }
> gen_stf_asi(cpu_addr, insn, 8, DFPREG(rd));
> break;
> - case 0x3c: /* V9 casa */
> - rs2 = GET_FIELD(insn, 27, 31);
> - cpu_src2 = gen_load_gpr(dc, rs2);
> - gen_cas_asi(dc, cpu_addr, cpu_src2, insn, rd);
> - break;
> case 0x3e: /* V9 casxa */
> rs2 = GET_FIELD(insn, 27, 31);
> cpu_src2 = gen_load_gpr(dc, rs2);
> @@ -5120,6 +5119,20 @@ static void disas_sparc_insn(DisasContext * dc, unsigned int insn)
> case 0x37: /* stdc */
> goto ncp_insn;
> #endif
> +#if !defined(CONFIG_USER_ONLY) || defined(TARGET_SPARC64)
> + case 0x3c: /* V9 or LEON3 casa */
> + CHECK_FPU_FEATURE(dc, CASA);
> +#ifndef TARGET_SPARC64
> + if (IS_IMM)
Did you forget to run checkpatch.pl? Missing braces here ...
> + goto illegal_insn;
> + if (!supervisor(dc))
... and here.
> + goto priv_insn;
Otherwise patch looks okay, thanks for the subject normalization.
As for the other one you'll need to sort our who sends a pull if Blue
doesn't resurface - I note that qemu-trivial is not CC'ed here and the
patch probably isn't anyway. Maybe Fabien can help out with that?
More generally, independent of this patch with test case in its commit
message, you may want to consider setting up tests/tcg/sparc/ for test
cases and/or use qtest with accel=tcg similar to what mst did for ACPI
regression testing.
Regards,
Andreas
> +#endif
> + rs2 = GET_FIELD(insn, 27, 31);
> + cpu_src2 = gen_load_gpr(dc, rs2);
> + gen_cas_asi(dc, cpu_addr, cpu_src2, insn, rd);
> + break;
> +#endif
> default:
> goto illegal_insn;
> }
>
--
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [Qemu-devel] [PATCH v3] target-sparc: Add and use CPU_FEATURE_CASA
2014-02-14 15:33 ` Andreas Färber
@ 2014-02-14 15:43 ` Sebastian Huber
2014-02-14 16:04 ` Andreas Färber
2014-02-14 17:27 ` Fabien Chouteau
1 sibling, 1 reply; 7+ messages in thread
From: Sebastian Huber @ 2014-02-14 15:43 UTC (permalink / raw)
To: Andreas Färber; +Cc: blauwirbel, qemu-devel, Fabien Chouteau
On 2014-02-14 16:33, Andreas Färber wrote:
>> @@ -5120,6 +5119,20 @@ static void disas_sparc_insn(DisasContext * dc, unsigned int insn)
>> > case 0x37: /* stdc */
>> > goto ncp_insn;
>> > #endif
>> >+#if !defined(CONFIG_USER_ONLY) || defined(TARGET_SPARC64)
>> >+ case 0x3c: /* V9 or LEON3 casa */
>> >+ CHECK_FPU_FEATURE(dc, CASA);
>> >+#ifndef TARGET_SPARC64
>> >+ if (IS_IMM)
>
> Did you forget to run checkpatch.pl? Missing braces here ...
>
>> >+ goto illegal_insn;
>> >+ if (!supervisor(dc))
> ... and here.
>
>> >+ goto priv_insn;
> Otherwise patch looks okay, thanks for the subject normalization.
I didn't know there was a checkpatch.pl, I run it next time.
This code fragment is a copy and paste move from another place in this file.
There a lot of similar style errors in this file.
>
> As for the other one you'll need to sort our who sends a pull if Blue
> doesn't resurface - I note that qemu-trivial is not CC'ed here and the
> patch probably isn't anyway. Maybe Fabien can help out with that?
>
> More generally, independent of this patch with test case in its commit
> message, you may want to consider setting up tests/tcg/sparc/ for test
> cases and/or use qtest with accel=tcg similar to what mst did for ACPI
> regression testing.
Ok, I will have a look at this. Is this a required step to get the patch
committed?
--
Sebastian Huber, embedded brains GmbH
Address : Dornierstr. 4, D-82178 Puchheim, Germany
Phone : +49 89 189 47 41-16
Fax : +49 89 189 47 41-09
E-Mail : sebastian.huber@embedded-brains.de
PGP : Public key available on request.
Diese Nachricht ist keine geschäftliche Mitteilung im Sinne des EHUG.
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [Qemu-devel] [PATCH v3] target-sparc: Add and use CPU_FEATURE_CASA
2014-02-14 15:43 ` Sebastian Huber
@ 2014-02-14 16:04 ` Andreas Färber
0 siblings, 0 replies; 7+ messages in thread
From: Andreas Färber @ 2014-02-14 16:04 UTC (permalink / raw)
To: Sebastian Huber; +Cc: blauwirbel, qemu-devel, Fabien Chouteau
Am 14.02.2014 16:43, schrieb Sebastian Huber:
> On 2014-02-14 16:33, Andreas Färber wrote:
>>> @@ -5120,6 +5119,20 @@ static void disas_sparc_insn(DisasContext *
>>> dc, unsigned int insn)
>>> > case 0x37: /* stdc */
>>> > goto ncp_insn;
>>> > #endif
>>> >+#if !defined(CONFIG_USER_ONLY) || defined(TARGET_SPARC64)
>>> >+ case 0x3c: /* V9 or LEON3 casa */
>>> >+ CHECK_FPU_FEATURE(dc, CASA);
>>> >+#ifndef TARGET_SPARC64
>>> >+ if (IS_IMM)
>>
>> Did you forget to run checkpatch.pl? Missing braces here ...
>>
>>> >+ goto illegal_insn;
>>> >+ if (!supervisor(dc))
>> ... and here.
>>
>>> >+ goto priv_insn;
>> Otherwise patch looks okay, thanks for the subject normalization.
>
> I didn't know there was a checkpatch.pl, I run it next time.
http://blog.vmsplice.net/2011/03/how-to-automatically-run-checkpatchpl.html
may be helpful then. :)
There's also scripts/get_maintainer.pl --nogit-fallback.
> This code fragment is a copy and paste move from another place in this
> file. There a lot of similar style errors in this file.
Our policy is to have new code adhere to the official Coding Style. If
you want, you can do a two-patch series (in which case we ask for a
cover letter...), with the initial one cleaning up the existing style
faults and the second one doing the actual change. But to be clear, such
cleanup of old code is entirely optional.
>> As for the other one you'll need to sort our who sends a pull if Blue
>> doesn't resurface - I note that qemu-trivial is not CC'ed here and the
>> patch probably isn't anyway. Maybe Fabien can help out with that?
>>
>> More generally, independent of this patch with test case in its commit
>> message, you may want to consider setting up tests/tcg/sparc/ for test
>> cases and/or use qtest with accel=tcg similar to what mst did for ACPI
>> regression testing.
>
> Ok, I will have a look at this. Is this a required step to get the
> patch committed?
No, that was a forward-looking suggestion, seeing you providing a test
case in the commit message.
Cheers,
Andreas
--
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [Qemu-devel] [PATCH v3] target-sparc: Add and use CPU_FEATURE_CASA
2014-02-14 15:33 ` Andreas Färber
2014-02-14 15:43 ` Sebastian Huber
@ 2014-02-14 17:27 ` Fabien Chouteau
2014-03-02 18:11 ` Andreas Färber
1 sibling, 1 reply; 7+ messages in thread
From: Fabien Chouteau @ 2014-02-14 17:27 UTC (permalink / raw)
To: Andreas Färber, Sebastian Huber, qemu-devel; +Cc: blauwirbel
On 02/14/2014 04:33 PM, Andreas Färber wrote:
> As for the other one you'll need to sort our who sends a pull if Blue
> doesn't resurface -
I didn't see any message about this. Does anyone know why he's not around?
> I note that qemu-trivial is not CC'ed here and the
> patch probably isn't anyway. Maybe Fabien can help out with that?
>
Andreas I really appreciate your help on this. There's not many patches
on SPARC/Leon3, but Sebastian's work is very important for this target.
If you are OK, we can continue with this scheme: Sebastian and I will
review the patches and you can help us to apply it. What do you think?
Thanks,
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [Qemu-devel] [PATCH v3] target-sparc: Add and use CPU_FEATURE_CASA
2014-02-14 17:27 ` Fabien Chouteau
@ 2014-03-02 18:11 ` Andreas Färber
2014-03-07 12:18 ` Fabien Chouteau
0 siblings, 1 reply; 7+ messages in thread
From: Andreas Färber @ 2014-03-02 18:11 UTC (permalink / raw)
To: Fabien Chouteau, qemu-devel
Cc: blauwirbel, Sebastian Huber, Mark Cave-Ayland, Peter Maydell
Hi Fabien,
Am 14.02.2014 18:27, schrieb Fabien Chouteau:
> On 02/14/2014 04:33 PM, Andreas Färber wrote:
>> As for the other one you'll need to sort our who sends a pull if Blue
>> doesn't resurface -
>
> I didn't see any message about this. Does anyone know why he's not around?
>
>> I note that qemu-trivial is not CC'ed here and the
>> patch probably isn't anyway. Maybe Fabien can help out with that?
>>
>
> Andreas I really appreciate your help on this. There's not many patches
> on SPARC/Leon3, but Sebastian's work is very important for this target.
> If you are OK, we can continue with this scheme: Sebastian and I will
> review the patches and you can help us to apply it. What do you think?
Sorry for the late reply. I do not have the capacity to step up as SPARC
maintainer since it is not related to my work and I am already lagging
for PReP. My suggestion was rather for you as MAINTAINERS-documented
Leon3 maintainer to send a pull to Peter once a Leon3 patch got review.
Just coordinate with Mark and Peter who does what; the key idea is
someone sending a pull that doesn't break non-sparc targets.
Cheers,
Andreas
http://wiki.qemu-project.org/Contribute/SubmitAPullRequest
--
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [Qemu-devel] [PATCH v3] target-sparc: Add and use CPU_FEATURE_CASA
2014-03-02 18:11 ` Andreas Färber
@ 2014-03-07 12:18 ` Fabien Chouteau
0 siblings, 0 replies; 7+ messages in thread
From: Fabien Chouteau @ 2014-03-07 12:18 UTC (permalink / raw)
To: Andreas Färber, qemu-devel
Cc: blauwirbel, Sebastian Huber, Mark Cave-Ayland, Peter Maydell
On 03/02/2014 07:11 PM, Andreas Färber wrote:
> Hi Fabien,
>
> Am 14.02.2014 18:27, schrieb Fabien Chouteau:
>> On 02/14/2014 04:33 PM, Andreas Färber wrote:
>>> As for the other one you'll need to sort our who sends a pull if Blue
>>> doesn't resurface -
>>
>> I didn't see any message about this. Does anyone know why he's not around?
>>
>>> I note that qemu-trivial is not CC'ed here and the
>>> patch probably isn't anyway. Maybe Fabien can help out with that?
>>>
>>
>> Andreas I really appreciate your help on this. There's not many patches
>> on SPARC/Leon3, but Sebastian's work is very important for this target.
>> If you are OK, we can continue with this scheme: Sebastian and I will
>> review the patches and you can help us to apply it. What do you think?
>
> Sorry for the late reply. I do not have the capacity to step up as SPARC
> maintainer since it is not related to my work and I am already lagging
> for PReP. My suggestion was rather for you as MAINTAINERS-documented
> Leon3 maintainer to send a pull to Peter once a Leon3 patch got review.
> Just coordinate with Mark and Peter who does what; the key idea is
> someone sending a pull that doesn't break non-sparc targets.
>
Thanks Andreas,
I guess I can be the SPARC MAINTAINERS, my only concerns are the time I
will be able to allocate for this job and the fact that I only know and
use Leon3 in the SPARC architecture. But we can git it a try.
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2014-03-07 12:18 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-02-14 15:16 [Qemu-devel] [PATCH v3] target-sparc: Add and use CPU_FEATURE_CASA Sebastian Huber
2014-02-14 15:33 ` Andreas Färber
2014-02-14 15:43 ` Sebastian Huber
2014-02-14 16:04 ` Andreas Färber
2014-02-14 17:27 ` Fabien Chouteau
2014-03-02 18:11 ` Andreas Färber
2014-03-07 12:18 ` Fabien Chouteau
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.