| Message ID | 1401461279-59617-12-git-send-email-leon.alrae@imgtec.com |
|---|---|
| State | New |
| Headers | show |
On 05/30/2014 07:47 AM, Leon Alrae wrote: > - if (env->CP0_Status & (1 << CP0St_UX)) { > - env->hflags |= MIPS_HFLAG_UX; > + > + if ((((env->hflags & MIPS_HFLAG_KSU) == MIPS_HFLAG_UM) && > + (env->CP0_Status & (1 << CP0St_UX))) || > + (((env->hflags & MIPS_HFLAG_KSU) == MIPS_HFLAG_SM) && > + (env->CP0_Status & (1 << CP0St_SX))) || > + (((env->hflags & MIPS_HFLAG_KSU) == MIPS_HFLAG_KM) && > + (env->CP0_Status & (1 << CP0St_KX)))) { > + env->hflags |= MIPS_HFLAG_X; > } > #endif > if ((env->CP0_Status & (1 << CP0St_CU0)) || > diff --git a/target-mips/translate.c b/target-mips/translate.c > index 2e94375..6d294e1 100644 > --- a/target-mips/translate.c > +++ b/target-mips/translate.c > @@ -1373,17 +1373,27 @@ generate_exception (DisasContext *ctx, int excp) > gen_helper_0e0i(raise_exception, excp); > } > > +#if defined(TARGET_MIPS64) > +static inline int is_wrapping_needed(DisasContext *ctx) > +{ > + if (!(ctx->hflags & MIPS_HFLAG_X)) { > + /* If not R6 then wrap only in User Mode */ > + if ((ctx->insn_flags & ISA_MIPS64R6) || > + ((ctx->hflags & MIPS_HFLAG_KSU) == MIPS_HFLAG_UM)) { This check should go above, where you set HFLAG_X. r~
On Fri, May 30, 2014 at 03:47:49PM +0100, Leon Alrae wrote: > In R6 the special behaviour for data references is also specified for Kernel > and Supervisor mode. Therefore MIPS_HFLAG_UX is replaced by generic MIPS_HFLAG_X > indicating whether 64-bit mode is enabled in current operating mode. I haven't found any indication of that in the MIPS64R6 manual (MD00091 version 6.00). Section 4.10 still only mentions the user mode. Did I miss something? > Signed-off-by: Leon Alrae <leon.alrae@imgtec.com> > --- > target-mips/cpu.h | 14 ++++++++++---- > target-mips/translate.c | 20 +++++++++++++++----- > 2 files changed, 25 insertions(+), 9 deletions(-) > > diff --git a/target-mips/cpu.h b/target-mips/cpu.h > index 6c2014e..3dbc219 100644 > --- a/target-mips/cpu.h > +++ b/target-mips/cpu.h > @@ -447,7 +447,7 @@ struct CPUMIPSState { > and RSQRT.D. */ > #define MIPS_HFLAG_COP1X 0x00080 /* COP1X instructions enabled */ > #define MIPS_HFLAG_RE 0x00100 /* Reversed endianness */ > -#define MIPS_HFLAG_UX 0x00200 /* 64-bit user mode */ > +#define MIPS_HFLAG_X 0x00200 /* 64-bit mode enabled */ > #define MIPS_HFLAG_M16 0x00400 /* MIPS16 mode flag */ > #define MIPS_HFLAG_M16_SHIFT 10 > /* If translation is interrupted between the branch instruction and > @@ -721,7 +721,7 @@ static inline void compute_hflags(CPUMIPSState *env) > { > env->hflags &= ~(MIPS_HFLAG_COP1X | MIPS_HFLAG_64 | MIPS_HFLAG_CP0 | > MIPS_HFLAG_F64 | MIPS_HFLAG_FPU | MIPS_HFLAG_KSU | > - MIPS_HFLAG_UX | MIPS_HFLAG_DSP | MIPS_HFLAG_DSPR2); > + MIPS_HFLAG_X | MIPS_HFLAG_DSP | MIPS_HFLAG_DSPR2); > if (!(env->CP0_Status & (1 << CP0St_EXL)) && > !(env->CP0_Status & (1 << CP0St_ERL)) && > !(env->hflags & MIPS_HFLAG_DM)) { > @@ -733,8 +733,14 @@ static inline void compute_hflags(CPUMIPSState *env) > (env->CP0_Status & (1 << CP0St_UX))) { > env->hflags |= MIPS_HFLAG_64; > } > - if (env->CP0_Status & (1 << CP0St_UX)) { > - env->hflags |= MIPS_HFLAG_UX; > + > + if ((((env->hflags & MIPS_HFLAG_KSU) == MIPS_HFLAG_UM) && > + (env->CP0_Status & (1 << CP0St_UX))) || > + (((env->hflags & MIPS_HFLAG_KSU) == MIPS_HFLAG_SM) && > + (env->CP0_Status & (1 << CP0St_SX))) || > + (((env->hflags & MIPS_HFLAG_KSU) == MIPS_HFLAG_KM) && > + (env->CP0_Status & (1 << CP0St_KX)))) { > + env->hflags |= MIPS_HFLAG_X; > } > #endif > if ((env->CP0_Status & (1 << CP0St_CU0)) || > diff --git a/target-mips/translate.c b/target-mips/translate.c > index 2e94375..6d294e1 100644 > --- a/target-mips/translate.c > +++ b/target-mips/translate.c > @@ -1373,17 +1373,27 @@ generate_exception (DisasContext *ctx, int excp) > gen_helper_0e0i(raise_exception, excp); > } > > +#if defined(TARGET_MIPS64) > +static inline int is_wrapping_needed(DisasContext *ctx) > +{ > + if (!(ctx->hflags & MIPS_HFLAG_X)) { > + /* If not R6 then wrap only in User Mode */ > + if ((ctx->insn_flags & ISA_MIPS64R6) || > + ((ctx->hflags & MIPS_HFLAG_KSU) == MIPS_HFLAG_UM)) { > + return 1; > + } > + } > + return 0; > +} > +#endif As Richard said, this code should be moved above, and the HFLAG semantic should be changed to "address wrapping needed". The current code is already wrong (and I am afraid I am the author...). So this could be done by renaming the HFLAG to for exemple MIPS_HFLAG_AWRAP, and checking only for this flag in gen_op_addr_add. Then the checks have to be adapted in compute_hflags, including the R6 case. > /* Addresses computation */ > static inline void gen_op_addr_add (DisasContext *ctx, TCGv ret, TCGv arg0, TCGv arg1) > { > tcg_gen_add_tl(ret, arg0, arg1); > > #if defined(TARGET_MIPS64) > - /* For compatibility with 32-bit code, data reference in user mode > - with Status_UX = 0 should be casted to 32-bit and sign extended. > - See the MIPS64 PRA manual, section 4.10. */ > - if (((ctx->hflags & MIPS_HFLAG_KSU) == MIPS_HFLAG_UM) && > - !(ctx->hflags & MIPS_HFLAG_UX)) { > + if (is_wrapping_needed(ctx)) { > tcg_gen_ext32s_i64(ret, ret);
On 30/05/14 23:41, Aurelien Jarno wrote: >> In R6 the special behaviour for data references is also specified for Kernel >> and Supervisor mode. Therefore MIPS_HFLAG_UX is replaced by generic MIPS_HFLAG_X >> indicating whether 64-bit mode is enabled in current operating mode. > > I haven't found any indication of that in the MIPS64R6 manual (MD00091 > version 6.00). Section 4.10 still only mentions the user mode. > > Did I miss something? You can find it in the Volume-II document (MD00087): Section "2.2.2.4.3 memory_address". It seems that some parts of MD00091 document haven't been fully updated yet. >> +#if defined(TARGET_MIPS64) >> +static inline int is_wrapping_needed(DisasContext *ctx) >> +{ >> + if (!(ctx->hflags & MIPS_HFLAG_X)) { >> + /* If not R6 then wrap only in User Mode */ >> + if ((ctx->insn_flags & ISA_MIPS64R6) || >> + ((ctx->hflags & MIPS_HFLAG_KSU) == MIPS_HFLAG_UM)) { >> + return 1; >> + } >> + } >> + return 0; >> +} >> +#endif > > As Richard said, this code should be moved above, and the HFLAG semantic > should be changed to "address wrapping needed". The current code is > already wrong (and I am afraid I am the author...). > > So this could be done by renaming the HFLAG to for exemple > MIPS_HFLAG_AWRAP, and checking only for this flag in gen_op_addr_add. > Then the checks have to be adapted in compute_hflags, including the R6 > case. I'll correct this. Thanks for the suggestion. Leon
diff --git a/target-mips/cpu.h b/target-mips/cpu.h index 6c2014e..3dbc219 100644 --- a/target-mips/cpu.h +++ b/target-mips/cpu.h @@ -447,7 +447,7 @@ struct CPUMIPSState { and RSQRT.D. */ #define MIPS_HFLAG_COP1X 0x00080 /* COP1X instructions enabled */ #define MIPS_HFLAG_RE 0x00100 /* Reversed endianness */ -#define MIPS_HFLAG_UX 0x00200 /* 64-bit user mode */ +#define MIPS_HFLAG_X 0x00200 /* 64-bit mode enabled */ #define MIPS_HFLAG_M16 0x00400 /* MIPS16 mode flag */ #define MIPS_HFLAG_M16_SHIFT 10 /* If translation is interrupted between the branch instruction and @@ -721,7 +721,7 @@ static inline void compute_hflags(CPUMIPSState *env) { env->hflags &= ~(MIPS_HFLAG_COP1X | MIPS_HFLAG_64 | MIPS_HFLAG_CP0 | MIPS_HFLAG_F64 | MIPS_HFLAG_FPU | MIPS_HFLAG_KSU | - MIPS_HFLAG_UX | MIPS_HFLAG_DSP | MIPS_HFLAG_DSPR2); + MIPS_HFLAG_X | MIPS_HFLAG_DSP | MIPS_HFLAG_DSPR2); if (!(env->CP0_Status & (1 << CP0St_EXL)) && !(env->CP0_Status & (1 << CP0St_ERL)) && !(env->hflags & MIPS_HFLAG_DM)) { @@ -733,8 +733,14 @@ static inline void compute_hflags(CPUMIPSState *env) (env->CP0_Status & (1 << CP0St_UX))) { env->hflags |= MIPS_HFLAG_64; } - if (env->CP0_Status & (1 << CP0St_UX)) { - env->hflags |= MIPS_HFLAG_UX; + + if ((((env->hflags & MIPS_HFLAG_KSU) == MIPS_HFLAG_UM) && + (env->CP0_Status & (1 << CP0St_UX))) || + (((env->hflags & MIPS_HFLAG_KSU) == MIPS_HFLAG_SM) && + (env->CP0_Status & (1 << CP0St_SX))) || + (((env->hflags & MIPS_HFLAG_KSU) == MIPS_HFLAG_KM) && + (env->CP0_Status & (1 << CP0St_KX)))) { + env->hflags |= MIPS_HFLAG_X; } #endif if ((env->CP0_Status & (1 << CP0St_CU0)) || diff --git a/target-mips/translate.c b/target-mips/translate.c index 2e94375..6d294e1 100644 --- a/target-mips/translate.c +++ b/target-mips/translate.c @@ -1373,17 +1373,27 @@ generate_exception (DisasContext *ctx, int excp) gen_helper_0e0i(raise_exception, excp); } +#if defined(TARGET_MIPS64) +static inline int is_wrapping_needed(DisasContext *ctx) +{ + if (!(ctx->hflags & MIPS_HFLAG_X)) { + /* If not R6 then wrap only in User Mode */ + if ((ctx->insn_flags & ISA_MIPS64R6) || + ((ctx->hflags & MIPS_HFLAG_KSU) == MIPS_HFLAG_UM)) { + return 1; + } + } + return 0; +} +#endif + /* Addresses computation */ static inline void gen_op_addr_add (DisasContext *ctx, TCGv ret, TCGv arg0, TCGv arg1) { tcg_gen_add_tl(ret, arg0, arg1); #if defined(TARGET_MIPS64) - /* For compatibility with 32-bit code, data reference in user mode - with Status_UX = 0 should be casted to 32-bit and sign extended. - See the MIPS64 PRA manual, section 4.10. */ - if (((ctx->hflags & MIPS_HFLAG_KSU) == MIPS_HFLAG_UM) && - !(ctx->hflags & MIPS_HFLAG_UX)) { + if (is_wrapping_needed(ctx)) { tcg_gen_ext32s_i64(ret, ret); } #endif
In R6 the special behaviour for data references is also specified for Kernel and Supervisor mode. Therefore MIPS_HFLAG_UX is replaced by generic MIPS_HFLAG_X indicating whether 64-bit mode is enabled in current operating mode. Signed-off-by: Leon Alrae <leon.alrae@imgtec.com> --- target-mips/cpu.h | 14 ++++++++++---- target-mips/translate.c | 20 +++++++++++++++----- 2 files changed, 25 insertions(+), 9 deletions(-)