* [Qemu-devel] [PATCH 1/9] target-mips: Add CP0_Ebase.WG (write gate) support
2016-09-06 11:03 [Qemu-devel] [PATCH 0/9] target-mips: Add Enhanced Virtual Addressing (EVA) support James Hogan
@ 2016-09-06 11:03 ` James Hogan
2016-10-07 13:42 ` Yongbok Kim
2016-09-06 11:03 ` [Qemu-devel] [PATCH 2/9] target-mips: Prepare loads/stores for EVA James Hogan
` (8 subsequent siblings)
9 siblings, 1 reply; 30+ messages in thread
From: James Hogan @ 2016-09-06 11:03 UTC (permalink / raw)
To: Leon Alrae; +Cc: qemu-devel, James Hogan, Aurelien Jarno
Add support for the CP0_EBase.WG bit, which allows upper bits to be
written (bits 31:30 on MIPS32, or bits 63:30 on MIPS64), along with the
CP0_Config5.CV bit to control whether the exception vector for Cache
Error exceptions is forced into KSeg1.
This is necessary on MIPS32 to support Segmentation Control and Enhanced
Virtual Addressing (EVA) extensions (where KSeg1 addresses may not
represent an unmapped uncached segment).
It is also useful on MIPS64 to allow the exception base to reside in
XKPhys, and possibly out of range of KSEG0 and KSEG1.
Signed-off-by: James Hogan <james.hogan@imgtec.com>
Cc: Leon Alrae <leon.alrae@imgtec.com>
Cc: Aurelien Jarno <aurelien@aurel32.net>
---
target-mips/cpu.h | 4 +++-
target-mips/helper.c | 13 +++++++------
target-mips/machine.c | 6 +++---
target-mips/op_helper.c | 13 +++++++++++--
target-mips/translate.c | 8 +++++---
target-mips/translate_init.c | 1 +
6 files changed, 30 insertions(+), 15 deletions(-)
diff --git a/target-mips/cpu.h b/target-mips/cpu.h
index 5182dc74ffa3..6ea2bf14c817 100644
--- a/target-mips/cpu.h
+++ b/target-mips/cpu.h
@@ -399,7 +399,9 @@ struct CPUMIPSState {
#define CP0Ca_EC 2
target_ulong CP0_EPC;
int32_t CP0_PRid;
- int32_t CP0_EBase;
+ target_ulong CP0_EBase;
+ target_ulong CP0_EBase_rw_bitmask;
+#define CP0EBase_WG 11
target_ulong CP0_CMGCRBase;
int32_t CP0_Config0;
#define CP0C0_M 31
diff --git a/target-mips/helper.c b/target-mips/helper.c
index c864b15b97a8..29ebf391cb94 100644
--- a/target-mips/helper.c
+++ b/target-mips/helper.c
@@ -821,11 +821,7 @@ void mips_cpu_do_interrupt(CPUState *cs)
goto set_EPC;
case EXCP_CACHE:
cause = 30;
- if (env->CP0_Status & (1 << CP0St_BEV)) {
- offset = 0x100;
- } else {
- offset = 0x20000100;
- }
+ offset = 0x100;
set_EPC:
if (!(env->CP0_Status & (1 << CP0St_EXL))) {
env->CP0_EPC = exception_resume_pc(env);
@@ -851,9 +847,14 @@ void mips_cpu_do_interrupt(CPUState *cs)
env->hflags &= ~MIPS_HFLAG_BMASK;
if (env->CP0_Status & (1 << CP0St_BEV)) {
env->active_tc.PC = env->exception_base + 0x200;
+ } else if (cause == 30 && !(env->CP0_Config5 & (1 << CP0C5_CV))) {
+ /* Force KSeg1 for cache errors */
+ env->active_tc.PC = (int32_t)KSEG1_BASE |
+ (env->CP0_EBase & 0x1FFFF000);
} else {
- env->active_tc.PC = (int32_t)(env->CP0_EBase & ~0x3ff);
+ env->active_tc.PC = env->CP0_EBase & ~0xfff;
}
+
env->active_tc.PC += offset;
set_hflags_for_handler(env);
env->CP0_Cause = (env->CP0_Cause & ~(0x1f << CP0Ca_EC)) | (cause << CP0Ca_EC);
diff --git a/target-mips/machine.c b/target-mips/machine.c
index a27f2f156da9..e795fecaa0d6 100644
--- a/target-mips/machine.c
+++ b/target-mips/machine.c
@@ -206,8 +206,8 @@ const VMStateDescription vmstate_tlb = {
const VMStateDescription vmstate_mips_cpu = {
.name = "cpu",
- .version_id = 8,
- .minimum_version_id = 8,
+ .version_id = 9,
+ .minimum_version_id = 9,
.post_load = cpu_post_load,
.fields = (VMStateField[]) {
/* Active TC */
@@ -267,7 +267,7 @@ const VMStateDescription vmstate_mips_cpu = {
VMSTATE_INT32(env.CP0_Cause, MIPSCPU),
VMSTATE_UINTTL(env.CP0_EPC, MIPSCPU),
VMSTATE_INT32(env.CP0_PRid, MIPSCPU),
- VMSTATE_INT32(env.CP0_EBase, MIPSCPU),
+ VMSTATE_UINTTL(env.CP0_EBase, MIPSCPU),
VMSTATE_INT32(env.CP0_Config0, MIPSCPU),
VMSTATE_INT32(env.CP0_Config1, MIPSCPU),
VMSTATE_INT32(env.CP0_Config2, MIPSCPU),
diff --git a/target-mips/op_helper.c b/target-mips/op_helper.c
index ea2f2abe198c..71ad16e41dd4 100644
--- a/target-mips/op_helper.c
+++ b/target-mips/op_helper.c
@@ -1526,14 +1526,23 @@ target_ulong helper_mftc0_ebase(CPUMIPSState *env)
void helper_mtc0_ebase(CPUMIPSState *env, target_ulong arg1)
{
- env->CP0_EBase = (env->CP0_EBase & ~0x3FFFF000) | (arg1 & 0x3FFFF000);
+ if (arg1 & (1 << CP0EBase_WG) & env->CP0_EBase_rw_bitmask) {
+ env->CP0_EBase = (env->CP0_EBase & 0x7ff) | (arg1 & ~0x7ff);
+ } else {
+ env->CP0_EBase = (env->CP0_EBase & ~0x3FFFF800) | (arg1 & 0x3FFFF800);
+ }
}
void helper_mttc0_ebase(CPUMIPSState *env, target_ulong arg1)
{
int other_tc = env->CP0_VPEControl & (0xff << CP0VPECo_TargTC);
CPUMIPSState *other = mips_cpu_map_tc(env, &other_tc);
- other->CP0_EBase = (other->CP0_EBase & ~0x3FFFF000) | (arg1 & 0x3FFFF000);
+ if (arg1 & (1 << CP0EBase_WG) & env->CP0_EBase_rw_bitmask) {
+ other->CP0_EBase = (other->CP0_EBase & 0x7ff) | (arg1 & ~0x7ff);
+ } else {
+ other->CP0_EBase = (other->CP0_EBase & ~0x3FFFF800) |
+ (arg1 & 0x3FFFF800);
+ }
}
target_ulong helper_mftc0_configx(CPUMIPSState *env, target_ulong idx)
diff --git a/target-mips/translate.c b/target-mips/translate.c
index bab52cb25498..e224c2f09af4 100644
--- a/target-mips/translate.c
+++ b/target-mips/translate.c
@@ -5316,7 +5316,8 @@ static void gen_mfc0(DisasContext *ctx, TCGv arg, int reg, int sel)
break;
case 1:
check_insn(ctx, ISA_MIPS32R2);
- gen_mfc0_load32(arg, offsetof(CPUMIPSState, CP0_EBase));
+ tcg_gen_ld_tl(arg, cpu_env, offsetof(CPUMIPSState, CP0_EBase));
+ tcg_gen_ext32s_tl(arg, arg);
rn = "EBase";
break;
case 3:
@@ -6630,7 +6631,7 @@ static void gen_dmfc0(DisasContext *ctx, TCGv arg, int reg, int sel)
break;
case 1:
check_insn(ctx, ISA_MIPS32R2);
- gen_mfc0_load32(arg, offsetof(CPUMIPSState, CP0_EBase));
+ tcg_gen_ld_tl(arg, cpu_env, offsetof(CPUMIPSState, CP0_EBase));
rn = "EBase";
break;
case 3:
@@ -20247,6 +20248,7 @@ void cpu_state_reset(CPUMIPSState *env)
env->CP0_SRSConf4 = env->cpu_model->CP0_SRSConf4;
env->CP0_PageGrain_rw_bitmask = env->cpu_model->CP0_PageGrain_rw_bitmask;
env->CP0_PageGrain = env->cpu_model->CP0_PageGrain;
+ env->CP0_EBase_rw_bitmask = env->cpu_model->CP0_EBase_rw_bitmask;
env->active_fpu.fcr0 = env->cpu_model->CP1_fcr0;
env->active_fpu.fcr31_rw_bitmask = env->cpu_model->CP1_fcr31_rw_bitmask;
env->active_fpu.fcr31 = env->cpu_model->CP1_fcr31;
@@ -20297,7 +20299,7 @@ void cpu_state_reset(CPUMIPSState *env)
if (kvm_enabled()) {
env->CP0_EBase |= 0x40000000;
} else {
- env->CP0_EBase |= 0x80000000;
+ env->CP0_EBase |= (int32_t)0x80000000;
}
if (env->CP0_Config3 & (1 << CP0C3_CMGCR)) {
env->CP0_CMGCRBase = 0x1fbf8000 >> 4;
diff --git a/target-mips/translate_init.c b/target-mips/translate_init.c
index 39ed5c4c1b12..f327e6e7de6c 100644
--- a/target-mips/translate_init.c
+++ b/target-mips/translate_init.c
@@ -101,6 +101,7 @@ struct mips_def_t {
int32_t CP0_SRSConf4;
int32_t CP0_PageGrain_rw_bitmask;
int32_t CP0_PageGrain;
+ target_ulong CP0_EBase_rw_bitmask;
int insn_flags;
enum mips_mmu_types mmu_type;
};
--
git-series 0.8.10
^ permalink raw reply related [flat|nested] 30+ messages in thread* Re: [Qemu-devel] [PATCH 1/9] target-mips: Add CP0_Ebase.WG (write gate) support
2016-09-06 11:03 ` [Qemu-devel] [PATCH 1/9] target-mips: Add CP0_Ebase.WG (write gate) support James Hogan
@ 2016-10-07 13:42 ` Yongbok Kim
2016-10-07 15:37 ` James Hogan
0 siblings, 1 reply; 30+ messages in thread
From: Yongbok Kim @ 2016-10-07 13:42 UTC (permalink / raw)
To: James Hogan, qemu-devel@nongnu.org; +Cc: Aurelien Jarno
On 06/09/2016 12:03, James Hogan wrote:
> Add support for the CP0_EBase.WG bit, which allows upper bits to be
> written (bits 31:30 on MIPS32, or bits 63:30 on MIPS64), along with the
> CP0_Config5.CV bit to control whether the exception vector for Cache
> Error exceptions is forced into KSeg1.
>
> This is necessary on MIPS32 to support Segmentation Control and Enhanced
> Virtual Addressing (EVA) extensions (where KSeg1 addresses may not
> represent an unmapped uncached segment).
>
> It is also useful on MIPS64 to allow the exception base to reside in
> XKPhys, and possibly out of range of KSEG0 and KSEG1.
>
> Signed-off-by: James Hogan <james.hogan@imgtec.com>
> Cc: Leon Alrae <leon.alrae@imgtec.com>
> Cc: Aurelien Jarno <aurelien@aurel32.net>
> ---
> target-mips/cpu.h | 4 +++-
> target-mips/helper.c | 13 +++++++------
> target-mips/machine.c | 6 +++---
> target-mips/op_helper.c | 13 +++++++++++--
> target-mips/translate.c | 8 +++++---
> target-mips/translate_init.c | 1 +
> 6 files changed, 30 insertions(+), 15 deletions(-)
>
> diff --git a/target-mips/cpu.h b/target-mips/cpu.h
> index 5182dc74ffa3..6ea2bf14c817 100644
> --- a/target-mips/cpu.h
> +++ b/target-mips/cpu.h
> @@ -399,7 +399,9 @@ struct CPUMIPSState {
> #define CP0Ca_EC 2
> target_ulong CP0_EPC;
> int32_t CP0_PRid;
> - int32_t CP0_EBase;
> + target_ulong CP0_EBase;
> + target_ulong CP0_EBase_rw_bitmask;
> +#define CP0EBase_WG 11
> target_ulong CP0_CMGCRBase;
> int32_t CP0_Config0;
> #define CP0C0_M 31
> diff --git a/target-mips/helper.c b/target-mips/helper.c
> index c864b15b97a8..29ebf391cb94 100644
> --- a/target-mips/helper.c
> +++ b/target-mips/helper.c
> @@ -821,11 +821,7 @@ void mips_cpu_do_interrupt(CPUState *cs)
> goto set_EPC;
> case EXCP_CACHE:
> cause = 30;
> - if (env->CP0_Status & (1 << CP0St_BEV)) {
> - offset = 0x100;
> - } else {
> - offset = 0x20000100;
> - }
> + offset = 0x100;
> set_EPC:
> if (!(env->CP0_Status & (1 << CP0St_EXL))) {
> env->CP0_EPC = exception_resume_pc(env);
> @@ -851,9 +847,14 @@ void mips_cpu_do_interrupt(CPUState *cs)
> env->hflags &= ~MIPS_HFLAG_BMASK;
> if (env->CP0_Status & (1 << CP0St_BEV)) {
> env->active_tc.PC = env->exception_base + 0x200;
> + } else if (cause == 30 && !(env->CP0_Config5 & (1 << CP0C5_CV))) {
&& !Config3SC? per MIPS32PRA 4.10.1.6.
It is not able to check Config3SC yet as it has not been defined but it can
be added later after the patch #7
> + /* Force KSeg1 for cache errors */
> + env->active_tc.PC = (int32_t)KSEG1_BASE |
> + (env->CP0_EBase & 0x1FFFF000);
> } else {
> - env->active_tc.PC = (int32_t)(env->CP0_EBase & ~0x3ff);
> + env->active_tc.PC = env->CP0_EBase & ~0xfff;
> }
> +
> env->active_tc.PC += offset;
> set_hflags_for_handler(env);
> env->CP0_Cause = (env->CP0_Cause & ~(0x1f << CP0Ca_EC)) | (cause << CP0Ca_EC);
> diff --git a/target-mips/machine.c b/target-mips/machine.c
> index a27f2f156da9..e795fecaa0d6 100644
> --- a/target-mips/machine.c
> +++ b/target-mips/machine.c
> @@ -206,8 +206,8 @@ const VMStateDescription vmstate_tlb = {
>
> const VMStateDescription vmstate_mips_cpu = {
> .name = "cpu",
> - .version_id = 8,
> - .minimum_version_id = 8,
> + .version_id = 9,
> + .minimum_version_id = 9,
> .post_load = cpu_post_load,
> .fields = (VMStateField[]) {
> /* Active TC */
> @@ -267,7 +267,7 @@ const VMStateDescription vmstate_mips_cpu = {
> VMSTATE_INT32(env.CP0_Cause, MIPSCPU),
> VMSTATE_UINTTL(env.CP0_EPC, MIPSCPU),
> VMSTATE_INT32(env.CP0_PRid, MIPSCPU),
> - VMSTATE_INT32(env.CP0_EBase, MIPSCPU),
> + VMSTATE_UINTTL(env.CP0_EBase, MIPSCPU),
> VMSTATE_INT32(env.CP0_Config0, MIPSCPU),
> VMSTATE_INT32(env.CP0_Config1, MIPSCPU),
> VMSTATE_INT32(env.CP0_Config2, MIPSCPU),
> diff --git a/target-mips/op_helper.c b/target-mips/op_helper.c
> index ea2f2abe198c..71ad16e41dd4 100644
> --- a/target-mips/op_helper.c
> +++ b/target-mips/op_helper.c
> @@ -1526,14 +1526,23 @@ target_ulong helper_mftc0_ebase(CPUMIPSState *env)
>
> void helper_mtc0_ebase(CPUMIPSState *env, target_ulong arg1)
> {
> - env->CP0_EBase = (env->CP0_EBase & ~0x3FFFF000) | (arg1 & 0x3FFFF000);
> + if (arg1 & (1 << CP0EBase_WG) & env->CP0_EBase_rw_bitmask) {
I guess it has to check the WG bit of EBase register rather than the new WG
bit value. "To ensure backward compatibility with MIPS32, the write-gate
bit must be set before the upper bits can be changed."
CP0_EBase_rw_bitmask is now added but it is not really used other than WG
bit. Perhaps rename it into something like CP0_EBaseWG_rw_bitmask would be
better to avoid any confusion. Otherwise assign those hard-coded mask into
the bitmask variable in the cpu_state_reset().
> + env->CP0_EBase = (env->CP0_EBase & 0x7ff) | (arg1 & ~0x7ff);
> + } else {
> + env->CP0_EBase = (env->CP0_EBase & ~0x3FFFF800) | (arg1 & 0x3FFFF800);
> + }
> }
>
> void helper_mttc0_ebase(CPUMIPSState *env, target_ulong arg1)
> {
> int other_tc = env->CP0_VPEControl & (0xff << CP0VPECo_TargTC);
> CPUMIPSState *other = mips_cpu_map_tc(env, &other_tc);
> - other->CP0_EBase = (other->CP0_EBase & ~0x3FFFF000) | (arg1 & 0x3FFFF000);
> + if (arg1 & (1 << CP0EBase_WG) & env->CP0_EBase_rw_bitmask) {
Same here.
> + other->CP0_EBase = (other->CP0_EBase & 0x7ff) | (arg1 & ~0x7ff);
> + } else {
> + other->CP0_EBase = (other->CP0_EBase & ~0x3FFFF800) |
> + (arg1 & 0x3FFFF800);
> + }
> }
>
> target_ulong helper_mftc0_configx(CPUMIPSState *env, target_ulong idx)
> diff --git a/target-mips/translate.c b/target-mips/translate.c
> index bab52cb25498..e224c2f09af4 100644
> --- a/target-mips/translate.c
> +++ b/target-mips/translate.c
> @@ -5316,7 +5316,8 @@ static void gen_mfc0(DisasContext *ctx, TCGv arg, int reg, int sel)
> break;
> case 1:
> check_insn(ctx, ISA_MIPS32R2);
> - gen_mfc0_load32(arg, offsetof(CPUMIPSState, CP0_EBase));
> + tcg_gen_ld_tl(arg, cpu_env, offsetof(CPUMIPSState, CP0_EBase));
> + tcg_gen_ext32s_tl(arg, arg);
> rn = "EBase";
> break;
> case 3:
> @@ -6630,7 +6631,7 @@ static void gen_dmfc0(DisasContext *ctx, TCGv arg, int reg, int sel)
> break;
> case 1:
> check_insn(ctx, ISA_MIPS32R2);
> - gen_mfc0_load32(arg, offsetof(CPUMIPSState, CP0_EBase));
> + tcg_gen_ld_tl(arg, cpu_env, offsetof(CPUMIPSState, CP0_EBase));
> rn = "EBase";
> break;
> case 3:
> @@ -20247,6 +20248,7 @@ void cpu_state_reset(CPUMIPSState *env)
> env->CP0_SRSConf4 = env->cpu_model->CP0_SRSConf4;
> env->CP0_PageGrain_rw_bitmask = env->cpu_model->CP0_PageGrain_rw_bitmask;
> env->CP0_PageGrain = env->cpu_model->CP0_PageGrain;
> + env->CP0_EBase_rw_bitmask = env->cpu_model->CP0_EBase_rw_bitmask;
> env->active_fpu.fcr0 = env->cpu_model->CP1_fcr0;
> env->active_fpu.fcr31_rw_bitmask = env->cpu_model->CP1_fcr31_rw_bitmask;
> env->active_fpu.fcr31 = env->cpu_model->CP1_fcr31;
> @@ -20297,7 +20299,7 @@ void cpu_state_reset(CPUMIPSState *env)
> if (kvm_enabled()) {
> env->CP0_EBase |= 0x40000000;
> } else {
> - env->CP0_EBase |= 0x80000000;
> + env->CP0_EBase |= (int32_t)0x80000000;
> }
> if (env->CP0_Config3 & (1 << CP0C3_CMGCR)) {
> env->CP0_CMGCRBase = 0x1fbf8000 >> 4;
> diff --git a/target-mips/translate_init.c b/target-mips/translate_init.c
> index 39ed5c4c1b12..f327e6e7de6c 100644
> --- a/target-mips/translate_init.c
> +++ b/target-mips/translate_init.c
> @@ -101,6 +101,7 @@ struct mips_def_t {
> int32_t CP0_SRSConf4;
> int32_t CP0_PageGrain_rw_bitmask;
> int32_t CP0_PageGrain;
> + target_ulong CP0_EBase_rw_bitmask;
> int insn_flags;
> enum mips_mmu_types mmu_type;
> };
>
Regards,
Yongbok
^ permalink raw reply [flat|nested] 30+ messages in thread* Re: [Qemu-devel] [PATCH 1/9] target-mips: Add CP0_Ebase.WG (write gate) support
2016-10-07 13:42 ` Yongbok Kim
@ 2016-10-07 15:37 ` James Hogan
0 siblings, 0 replies; 30+ messages in thread
From: James Hogan @ 2016-10-07 15:37 UTC (permalink / raw)
To: Yongbok Kim; +Cc: qemu-devel@nongnu.org, Aurelien Jarno
[-- Attachment #1: Type: text/plain, Size: 9567 bytes --]
On Fri, Oct 07, 2016 at 02:42:15PM +0100, Yongbok Kim wrote:
>
>
> On 06/09/2016 12:03, James Hogan wrote:
> > Add support for the CP0_EBase.WG bit, which allows upper bits to be
> > written (bits 31:30 on MIPS32, or bits 63:30 on MIPS64), along with the
> > CP0_Config5.CV bit to control whether the exception vector for Cache
> > Error exceptions is forced into KSeg1.
> >
> > This is necessary on MIPS32 to support Segmentation Control and Enhanced
> > Virtual Addressing (EVA) extensions (where KSeg1 addresses may not
> > represent an unmapped uncached segment).
> >
> > It is also useful on MIPS64 to allow the exception base to reside in
> > XKPhys, and possibly out of range of KSEG0 and KSEG1.
> >
> > Signed-off-by: James Hogan <james.hogan@imgtec.com>
> > Cc: Leon Alrae <leon.alrae@imgtec.com>
> > Cc: Aurelien Jarno <aurelien@aurel32.net>
> > ---
> > target-mips/cpu.h | 4 +++-
> > target-mips/helper.c | 13 +++++++------
> > target-mips/machine.c | 6 +++---
> > target-mips/op_helper.c | 13 +++++++++++--
> > target-mips/translate.c | 8 +++++---
> > target-mips/translate_init.c | 1 +
> > 6 files changed, 30 insertions(+), 15 deletions(-)
> >
> > diff --git a/target-mips/cpu.h b/target-mips/cpu.h
> > index 5182dc74ffa3..6ea2bf14c817 100644
> > --- a/target-mips/cpu.h
> > +++ b/target-mips/cpu.h
> > @@ -399,7 +399,9 @@ struct CPUMIPSState {
> > #define CP0Ca_EC 2
> > target_ulong CP0_EPC;
> > int32_t CP0_PRid;
> > - int32_t CP0_EBase;
> > + target_ulong CP0_EBase;
> > + target_ulong CP0_EBase_rw_bitmask;
> > +#define CP0EBase_WG 11
> > target_ulong CP0_CMGCRBase;
> > int32_t CP0_Config0;
> > #define CP0C0_M 31
> > diff --git a/target-mips/helper.c b/target-mips/helper.c
> > index c864b15b97a8..29ebf391cb94 100644
> > --- a/target-mips/helper.c
> > +++ b/target-mips/helper.c
> > @@ -821,11 +821,7 @@ void mips_cpu_do_interrupt(CPUState *cs)
> > goto set_EPC;
> > case EXCP_CACHE:
> > cause = 30;
> > - if (env->CP0_Status & (1 << CP0St_BEV)) {
> > - offset = 0x100;
> > - } else {
> > - offset = 0x20000100;
> > - }
> > + offset = 0x100;
> > set_EPC:
> > if (!(env->CP0_Status & (1 << CP0St_EXL))) {
> > env->CP0_EPC = exception_resume_pc(env);
> > @@ -851,9 +847,14 @@ void mips_cpu_do_interrupt(CPUState *cs)
> > env->hflags &= ~MIPS_HFLAG_BMASK;
> > if (env->CP0_Status & (1 << CP0St_BEV)) {
> > env->active_tc.PC = env->exception_base + 0x200;
> > + } else if (cause == 30 && !(env->CP0_Config5 & (1 << CP0C5_CV))) {
>
> && !Config3SC? per MIPS32PRA 4.10.1.6.
> It is not able to check Config3SC yet as it has not been defined but it can
> be added later after the patch #7
Okay, though CV is controlled via .CP0_Config5_rw_bitmask already, but
no harm adding the Config3.SC check too.
>
> > + /* Force KSeg1 for cache errors */
> > + env->active_tc.PC = (int32_t)KSEG1_BASE |
> > + (env->CP0_EBase & 0x1FFFF000);
> > } else {
> > - env->active_tc.PC = (int32_t)(env->CP0_EBase & ~0x3ff);
> > + env->active_tc.PC = env->CP0_EBase & ~0xfff;
> > }
> > +
> > env->active_tc.PC += offset;
> > set_hflags_for_handler(env);
> > env->CP0_Cause = (env->CP0_Cause & ~(0x1f << CP0Ca_EC)) | (cause << CP0Ca_EC);
> > diff --git a/target-mips/machine.c b/target-mips/machine.c
> > index a27f2f156da9..e795fecaa0d6 100644
> > --- a/target-mips/machine.c
> > +++ b/target-mips/machine.c
> > @@ -206,8 +206,8 @@ const VMStateDescription vmstate_tlb = {
> >
> > const VMStateDescription vmstate_mips_cpu = {
> > .name = "cpu",
> > - .version_id = 8,
> > - .minimum_version_id = 8,
> > + .version_id = 9,
> > + .minimum_version_id = 9,
> > .post_load = cpu_post_load,
> > .fields = (VMStateField[]) {
> > /* Active TC */
> > @@ -267,7 +267,7 @@ const VMStateDescription vmstate_mips_cpu = {
> > VMSTATE_INT32(env.CP0_Cause, MIPSCPU),
> > VMSTATE_UINTTL(env.CP0_EPC, MIPSCPU),
> > VMSTATE_INT32(env.CP0_PRid, MIPSCPU),
> > - VMSTATE_INT32(env.CP0_EBase, MIPSCPU),
> > + VMSTATE_UINTTL(env.CP0_EBase, MIPSCPU),
> > VMSTATE_INT32(env.CP0_Config0, MIPSCPU),
> > VMSTATE_INT32(env.CP0_Config1, MIPSCPU),
> > VMSTATE_INT32(env.CP0_Config2, MIPSCPU),
> > diff --git a/target-mips/op_helper.c b/target-mips/op_helper.c
> > index ea2f2abe198c..71ad16e41dd4 100644
> > --- a/target-mips/op_helper.c
> > +++ b/target-mips/op_helper.c
> > @@ -1526,14 +1526,23 @@ target_ulong helper_mftc0_ebase(CPUMIPSState *env)
> >
> > void helper_mtc0_ebase(CPUMIPSState *env, target_ulong arg1)
> > {
> > - env->CP0_EBase = (env->CP0_EBase & ~0x3FFFF000) | (arg1 & 0x3FFFF000);
> > + if (arg1 & (1 << CP0EBase_WG) & env->CP0_EBase_rw_bitmask) {
>
> I guess it has to check the WG bit of EBase register rather than the new WG
> bit value. "To ensure backward compatibility with MIPS32, the write-gate
> bit must be set before the upper bits can be changed."
Yeh, that is misleadingly worded. The way real hardware behaves is that
the upper bits are only writable when the WG bit is set as part of the
same write.
The description of WG bit itself backs that up:
"Bits 31..30 are unchanged on writes to EBase when WG=0 in the value
being written. The WG bit must be set true in the written value to
change the values of bits 31..30"
>
> CP0_EBase_rw_bitmask is now added but it is not really used other than WG
> bit. Perhaps rename it into something like CP0_EBaseWG_rw_bitmask would be
> better to avoid any confusion. Otherwise assign those hard-coded mask into
> the bitmask variable in the cpu_state_reset().
Since the mask depends on WG, i'll rename it.
Thanks for the review.
Cheers
James
>
> > + env->CP0_EBase = (env->CP0_EBase & 0x7ff) | (arg1 & ~0x7ff);
> > + } else {
> > + env->CP0_EBase = (env->CP0_EBase & ~0x3FFFF800) | (arg1 & 0x3FFFF800);
> > + }
> > }
> >
> > void helper_mttc0_ebase(CPUMIPSState *env, target_ulong arg1)
> > {
> > int other_tc = env->CP0_VPEControl & (0xff << CP0VPECo_TargTC);
> > CPUMIPSState *other = mips_cpu_map_tc(env, &other_tc);
> > - other->CP0_EBase = (other->CP0_EBase & ~0x3FFFF000) | (arg1 & 0x3FFFF000);
> > + if (arg1 & (1 << CP0EBase_WG) & env->CP0_EBase_rw_bitmask) {
>
> Same here.
>
> > + other->CP0_EBase = (other->CP0_EBase & 0x7ff) | (arg1 & ~0x7ff);
> > + } else {
> > + other->CP0_EBase = (other->CP0_EBase & ~0x3FFFF800) |
> > + (arg1 & 0x3FFFF800);
> > + }
> > }
> >
> > target_ulong helper_mftc0_configx(CPUMIPSState *env, target_ulong idx)
> > diff --git a/target-mips/translate.c b/target-mips/translate.c
> > index bab52cb25498..e224c2f09af4 100644
> > --- a/target-mips/translate.c
> > +++ b/target-mips/translate.c
> > @@ -5316,7 +5316,8 @@ static void gen_mfc0(DisasContext *ctx, TCGv arg, int reg, int sel)
> > break;
> > case 1:
> > check_insn(ctx, ISA_MIPS32R2);
> > - gen_mfc0_load32(arg, offsetof(CPUMIPSState, CP0_EBase));
> > + tcg_gen_ld_tl(arg, cpu_env, offsetof(CPUMIPSState, CP0_EBase));
> > + tcg_gen_ext32s_tl(arg, arg);
> > rn = "EBase";
> > break;
> > case 3:
> > @@ -6630,7 +6631,7 @@ static void gen_dmfc0(DisasContext *ctx, TCGv arg, int reg, int sel)
> > break;
> > case 1:
> > check_insn(ctx, ISA_MIPS32R2);
> > - gen_mfc0_load32(arg, offsetof(CPUMIPSState, CP0_EBase));
> > + tcg_gen_ld_tl(arg, cpu_env, offsetof(CPUMIPSState, CP0_EBase));
> > rn = "EBase";
> > break;
> > case 3:
> > @@ -20247,6 +20248,7 @@ void cpu_state_reset(CPUMIPSState *env)
> > env->CP0_SRSConf4 = env->cpu_model->CP0_SRSConf4;
> > env->CP0_PageGrain_rw_bitmask = env->cpu_model->CP0_PageGrain_rw_bitmask;
> > env->CP0_PageGrain = env->cpu_model->CP0_PageGrain;
> > + env->CP0_EBase_rw_bitmask = env->cpu_model->CP0_EBase_rw_bitmask;
> > env->active_fpu.fcr0 = env->cpu_model->CP1_fcr0;
> > env->active_fpu.fcr31_rw_bitmask = env->cpu_model->CP1_fcr31_rw_bitmask;
> > env->active_fpu.fcr31 = env->cpu_model->CP1_fcr31;
> > @@ -20297,7 +20299,7 @@ void cpu_state_reset(CPUMIPSState *env)
> > if (kvm_enabled()) {
> > env->CP0_EBase |= 0x40000000;
> > } else {
> > - env->CP0_EBase |= 0x80000000;
> > + env->CP0_EBase |= (int32_t)0x80000000;
> > }
> > if (env->CP0_Config3 & (1 << CP0C3_CMGCR)) {
> > env->CP0_CMGCRBase = 0x1fbf8000 >> 4;
> > diff --git a/target-mips/translate_init.c b/target-mips/translate_init.c
> > index 39ed5c4c1b12..f327e6e7de6c 100644
> > --- a/target-mips/translate_init.c
> > +++ b/target-mips/translate_init.c
> > @@ -101,6 +101,7 @@ struct mips_def_t {
> > int32_t CP0_SRSConf4;
> > int32_t CP0_PageGrain_rw_bitmask;
> > int32_t CP0_PageGrain;
> > + target_ulong CP0_EBase_rw_bitmask;
> > int insn_flags;
> > enum mips_mmu_types mmu_type;
> > };
> >
>
>
> Regards,
> Yongbok
>
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply [flat|nested] 30+ messages in thread
* [Qemu-devel] [PATCH 2/9] target-mips: Prepare loads/stores for EVA
2016-09-06 11:03 [Qemu-devel] [PATCH 0/9] target-mips: Add Enhanced Virtual Addressing (EVA) support James Hogan
2016-09-06 11:03 ` [Qemu-devel] [PATCH 1/9] target-mips: Add CP0_Ebase.WG (write gate) support James Hogan
@ 2016-09-06 11:03 ` James Hogan
2016-10-07 15:32 ` Yongbok Kim
2016-09-06 11:03 ` [Qemu-devel] [PATCH 3/9] target-mips: Decode EVA load & store instructions James Hogan
` (7 subsequent siblings)
9 siblings, 1 reply; 30+ messages in thread
From: James Hogan @ 2016-09-06 11:03 UTC (permalink / raw)
To: Leon Alrae; +Cc: qemu-devel, James Hogan, Aurelien Jarno
EVA load and store instructions access the user mode address map, so
they need to use mem_idx of MIPS_HFLAG_UM. Update the various utility
functions to allow mem_idx to be more easily overridden from the
decoding logic.
Specifically we add a mem_idx argument to the op_ld/st_* helpers used
for atomics, and a mem_idx local variable to gen_ld(), gen_st(), and
gen_st_cond().
Signed-off-by: James Hogan <james.hogan@imgtec.com>
Cc: Leon Alrae <leon.alrae@imgtec.com>
Cc: Aurelien Jarno <aurelien@aurel32.net>
---
target-mips/translate.c | 77 ++++++++++++++++++++++--------------------
1 file changed, 42 insertions(+), 35 deletions(-)
diff --git a/target-mips/translate.c b/target-mips/translate.c
index e224c2f09af4..df2befbd5294 100644
--- a/target-mips/translate.c
+++ b/target-mips/translate.c
@@ -2028,7 +2028,8 @@ FOP_CONDNS(s, FMT_S, 32, gen_store_fpr32(ctx, fp0, fd))
/* load/store instructions. */
#ifdef CONFIG_USER_ONLY
#define OP_LD_ATOMIC(insn,fname) \
-static inline void op_ld_##insn(TCGv ret, TCGv arg1, DisasContext *ctx) \
+static inline void op_ld_##insn(TCGv ret, TCGv arg1, int mem_idx, \
+ DisasContext *ctx) \
{ \
TCGv t0 = tcg_temp_new(); \
tcg_gen_mov_tl(t0, arg1); \
@@ -2039,9 +2040,10 @@ static inline void op_ld_##insn(TCGv ret, TCGv arg1, DisasContext *ctx) \
}
#else
#define OP_LD_ATOMIC(insn,fname) \
-static inline void op_ld_##insn(TCGv ret, TCGv arg1, DisasContext *ctx) \
+static inline void op_ld_##insn(TCGv ret, TCGv arg1, int mem_idx, \
+ DisasContext *ctx) \
{ \
- gen_helper_1e1i(insn, ret, arg1, ctx->mem_idx); \
+ gen_helper_1e1i(insn, ret, arg1, mem_idx); \
}
#endif
OP_LD_ATOMIC(ll,ld32s);
@@ -2052,7 +2054,8 @@ OP_LD_ATOMIC(lld,ld64);
#ifdef CONFIG_USER_ONLY
#define OP_ST_ATOMIC(insn,fname,ldname,almask) \
-static inline void op_st_##insn(TCGv arg1, TCGv arg2, int rt, DisasContext *ctx) \
+static inline void op_st_##insn(TCGv arg1, TCGv arg2, int rt, int mem_idx, \
+ DisasContext *ctx) \
{ \
TCGv t0 = tcg_temp_new(); \
TCGLabel *l1 = gen_new_label(); \
@@ -2076,10 +2079,11 @@ static inline void op_st_##insn(TCGv arg1, TCGv arg2, int rt, DisasContext *ctx)
}
#else
#define OP_ST_ATOMIC(insn,fname,ldname,almask) \
-static inline void op_st_##insn(TCGv arg1, TCGv arg2, int rt, DisasContext *ctx) \
+static inline void op_st_##insn(TCGv arg1, TCGv arg2, int rt, int mem_idx, \
+ DisasContext *ctx) \
{ \
TCGv t0 = tcg_temp_new(); \
- gen_helper_1e2i(insn, t0, arg1, arg2, ctx->mem_idx); \
+ gen_helper_1e2i(insn, t0, arg1, arg2, mem_idx); \
gen_store_gpr(t0, rt); \
tcg_temp_free(t0); \
}
@@ -2122,6 +2126,7 @@ static void gen_ld(DisasContext *ctx, uint32_t opc,
int rt, int base, int16_t offset)
{
TCGv t0, t1, t2;
+ int mem_idx = ctx->mem_idx;
if (rt == 0 && ctx->insn_flags & (INSN_LOONGSON2E | INSN_LOONGSON2F)) {
/* Loongson CPU uses a load to zero register for prefetch.
@@ -2136,32 +2141,32 @@ static void gen_ld(DisasContext *ctx, uint32_t opc,
switch (opc) {
#if defined(TARGET_MIPS64)
case OPC_LWU:
- tcg_gen_qemu_ld_tl(t0, t0, ctx->mem_idx, MO_TEUL |
+ tcg_gen_qemu_ld_tl(t0, t0, mem_idx, MO_TEUL |
ctx->default_tcg_memop_mask);
gen_store_gpr(t0, rt);
break;
case OPC_LD:
- tcg_gen_qemu_ld_tl(t0, t0, ctx->mem_idx, MO_TEQ |
+ tcg_gen_qemu_ld_tl(t0, t0, mem_idx, MO_TEQ |
ctx->default_tcg_memop_mask);
gen_store_gpr(t0, rt);
break;
case OPC_LLD:
case R6_OPC_LLD:
- op_ld_lld(t0, t0, ctx);
+ op_ld_lld(t0, t0, mem_idx, ctx);
gen_store_gpr(t0, rt);
break;
case OPC_LDL:
t1 = tcg_temp_new();
/* Do a byte access to possibly trigger a page
fault with the unaligned address. */
- tcg_gen_qemu_ld_tl(t1, t0, ctx->mem_idx, MO_UB);
+ tcg_gen_qemu_ld_tl(t1, t0, mem_idx, MO_UB);
tcg_gen_andi_tl(t1, t0, 7);
#ifndef TARGET_WORDS_BIGENDIAN
tcg_gen_xori_tl(t1, t1, 7);
#endif
tcg_gen_shli_tl(t1, t1, 3);
tcg_gen_andi_tl(t0, t0, ~7);
- tcg_gen_qemu_ld_tl(t0, t0, ctx->mem_idx, MO_TEQ);
+ tcg_gen_qemu_ld_tl(t0, t0, mem_idx, MO_TEQ);
tcg_gen_shl_tl(t0, t0, t1);
t2 = tcg_const_tl(-1);
tcg_gen_shl_tl(t2, t2, t1);
@@ -2176,14 +2181,14 @@ static void gen_ld(DisasContext *ctx, uint32_t opc,
t1 = tcg_temp_new();
/* Do a byte access to possibly trigger a page
fault with the unaligned address. */
- tcg_gen_qemu_ld_tl(t1, t0, ctx->mem_idx, MO_UB);
+ tcg_gen_qemu_ld_tl(t1, t0, mem_idx, MO_UB);
tcg_gen_andi_tl(t1, t0, 7);
#ifdef TARGET_WORDS_BIGENDIAN
tcg_gen_xori_tl(t1, t1, 7);
#endif
tcg_gen_shli_tl(t1, t1, 3);
tcg_gen_andi_tl(t0, t0, ~7);
- tcg_gen_qemu_ld_tl(t0, t0, ctx->mem_idx, MO_TEQ);
+ tcg_gen_qemu_ld_tl(t0, t0, mem_idx, MO_TEQ);
tcg_gen_shr_tl(t0, t0, t1);
tcg_gen_xori_tl(t1, t1, 63);
t2 = tcg_const_tl(0xfffffffffffffffeull);
@@ -2199,7 +2204,7 @@ static void gen_ld(DisasContext *ctx, uint32_t opc,
t1 = tcg_const_tl(pc_relative_pc(ctx));
gen_op_addr_add(ctx, t0, t0, t1);
tcg_temp_free(t1);
- tcg_gen_qemu_ld_tl(t0, t0, ctx->mem_idx, MO_TEQ);
+ tcg_gen_qemu_ld_tl(t0, t0, mem_idx, MO_TEQ);
gen_store_gpr(t0, rt);
break;
#endif
@@ -2207,44 +2212,44 @@ static void gen_ld(DisasContext *ctx, uint32_t opc,
t1 = tcg_const_tl(pc_relative_pc(ctx));
gen_op_addr_add(ctx, t0, t0, t1);
tcg_temp_free(t1);
- tcg_gen_qemu_ld_tl(t0, t0, ctx->mem_idx, MO_TESL);
+ tcg_gen_qemu_ld_tl(t0, t0, mem_idx, MO_TESL);
gen_store_gpr(t0, rt);
break;
case OPC_LW:
- tcg_gen_qemu_ld_tl(t0, t0, ctx->mem_idx, MO_TESL |
+ tcg_gen_qemu_ld_tl(t0, t0, mem_idx, MO_TESL |
ctx->default_tcg_memop_mask);
gen_store_gpr(t0, rt);
break;
case OPC_LH:
- tcg_gen_qemu_ld_tl(t0, t0, ctx->mem_idx, MO_TESW |
+ tcg_gen_qemu_ld_tl(t0, t0, mem_idx, MO_TESW |
ctx->default_tcg_memop_mask);
gen_store_gpr(t0, rt);
break;
case OPC_LHU:
- tcg_gen_qemu_ld_tl(t0, t0, ctx->mem_idx, MO_TEUW |
+ tcg_gen_qemu_ld_tl(t0, t0, mem_idx, MO_TEUW |
ctx->default_tcg_memop_mask);
gen_store_gpr(t0, rt);
break;
case OPC_LB:
- tcg_gen_qemu_ld_tl(t0, t0, ctx->mem_idx, MO_SB);
+ tcg_gen_qemu_ld_tl(t0, t0, mem_idx, MO_SB);
gen_store_gpr(t0, rt);
break;
case OPC_LBU:
- tcg_gen_qemu_ld_tl(t0, t0, ctx->mem_idx, MO_UB);
+ tcg_gen_qemu_ld_tl(t0, t0, mem_idx, MO_UB);
gen_store_gpr(t0, rt);
break;
case OPC_LWL:
t1 = tcg_temp_new();
/* Do a byte access to possibly trigger a page
fault with the unaligned address. */
- tcg_gen_qemu_ld_tl(t1, t0, ctx->mem_idx, MO_UB);
+ tcg_gen_qemu_ld_tl(t1, t0, mem_idx, MO_UB);
tcg_gen_andi_tl(t1, t0, 3);
#ifndef TARGET_WORDS_BIGENDIAN
tcg_gen_xori_tl(t1, t1, 3);
#endif
tcg_gen_shli_tl(t1, t1, 3);
tcg_gen_andi_tl(t0, t0, ~3);
- tcg_gen_qemu_ld_tl(t0, t0, ctx->mem_idx, MO_TEUL);
+ tcg_gen_qemu_ld_tl(t0, t0, mem_idx, MO_TEUL);
tcg_gen_shl_tl(t0, t0, t1);
t2 = tcg_const_tl(-1);
tcg_gen_shl_tl(t2, t2, t1);
@@ -2260,14 +2265,14 @@ static void gen_ld(DisasContext *ctx, uint32_t opc,
t1 = tcg_temp_new();
/* Do a byte access to possibly trigger a page
fault with the unaligned address. */
- tcg_gen_qemu_ld_tl(t1, t0, ctx->mem_idx, MO_UB);
+ tcg_gen_qemu_ld_tl(t1, t0, mem_idx, MO_UB);
tcg_gen_andi_tl(t1, t0, 3);
#ifdef TARGET_WORDS_BIGENDIAN
tcg_gen_xori_tl(t1, t1, 3);
#endif
tcg_gen_shli_tl(t1, t1, 3);
tcg_gen_andi_tl(t0, t0, ~3);
- tcg_gen_qemu_ld_tl(t0, t0, ctx->mem_idx, MO_TEUL);
+ tcg_gen_qemu_ld_tl(t0, t0, mem_idx, MO_TEUL);
tcg_gen_shr_tl(t0, t0, t1);
tcg_gen_xori_tl(t1, t1, 31);
t2 = tcg_const_tl(0xfffffffeull);
@@ -2282,7 +2287,7 @@ static void gen_ld(DisasContext *ctx, uint32_t opc,
break;
case OPC_LL:
case R6_OPC_LL:
- op_ld_ll(t0, t0, ctx);
+ op_ld_ll(t0, t0, mem_idx, ctx);
gen_store_gpr(t0, rt);
break;
}
@@ -2295,38 +2300,39 @@ static void gen_st (DisasContext *ctx, uint32_t opc, int rt,
{
TCGv t0 = tcg_temp_new();
TCGv t1 = tcg_temp_new();
+ int mem_idx = ctx->mem_idx;
gen_base_offset_addr(ctx, t0, base, offset);
gen_load_gpr(t1, rt);
switch (opc) {
#if defined(TARGET_MIPS64)
case OPC_SD:
- tcg_gen_qemu_st_tl(t1, t0, ctx->mem_idx, MO_TEQ |
+ tcg_gen_qemu_st_tl(t1, t0, mem_idx, MO_TEQ |
ctx->default_tcg_memop_mask);
break;
case OPC_SDL:
- gen_helper_0e2i(sdl, t1, t0, ctx->mem_idx);
+ gen_helper_0e2i(sdl, t1, t0, mem_idx);
break;
case OPC_SDR:
- gen_helper_0e2i(sdr, t1, t0, ctx->mem_idx);
+ gen_helper_0e2i(sdr, t1, t0, mem_idx);
break;
#endif
case OPC_SW:
- tcg_gen_qemu_st_tl(t1, t0, ctx->mem_idx, MO_TEUL |
+ tcg_gen_qemu_st_tl(t1, t0, mem_idx, MO_TEUL |
ctx->default_tcg_memop_mask);
break;
case OPC_SH:
- tcg_gen_qemu_st_tl(t1, t0, ctx->mem_idx, MO_TEUW |
+ tcg_gen_qemu_st_tl(t1, t0, mem_idx, MO_TEUW |
ctx->default_tcg_memop_mask);
break;
case OPC_SB:
- tcg_gen_qemu_st_tl(t1, t0, ctx->mem_idx, MO_8);
+ tcg_gen_qemu_st_tl(t1, t0, mem_idx, MO_8);
break;
case OPC_SWL:
- gen_helper_0e2i(swl, t1, t0, ctx->mem_idx);
+ gen_helper_0e2i(swl, t1, t0, mem_idx);
break;
case OPC_SWR:
- gen_helper_0e2i(swr, t1, t0, ctx->mem_idx);
+ gen_helper_0e2i(swr, t1, t0, mem_idx);
break;
}
tcg_temp_free(t0);
@@ -2339,6 +2345,7 @@ static void gen_st_cond (DisasContext *ctx, uint32_t opc, int rt,
int base, int16_t offset)
{
TCGv t0, t1;
+ int mem_idx = ctx->mem_idx;
#ifdef CONFIG_USER_ONLY
t0 = tcg_temp_local_new();
@@ -2353,12 +2360,12 @@ static void gen_st_cond (DisasContext *ctx, uint32_t opc, int rt,
#if defined(TARGET_MIPS64)
case OPC_SCD:
case R6_OPC_SCD:
- op_st_scd(t1, t0, rt, ctx);
+ op_st_scd(t1, t0, rt, mem_idx, ctx);
break;
#endif
case OPC_SC:
case R6_OPC_SC:
- op_st_sc(t1, t0, rt, ctx);
+ op_st_sc(t1, t0, rt, mem_idx, ctx);
break;
}
tcg_temp_free(t1);
--
git-series 0.8.10
^ permalink raw reply related [flat|nested] 30+ messages in thread* Re: [Qemu-devel] [PATCH 2/9] target-mips: Prepare loads/stores for EVA
2016-09-06 11:03 ` [Qemu-devel] [PATCH 2/9] target-mips: Prepare loads/stores for EVA James Hogan
@ 2016-10-07 15:32 ` Yongbok Kim
0 siblings, 0 replies; 30+ messages in thread
From: Yongbok Kim @ 2016-10-07 15:32 UTC (permalink / raw)
To: James Hogan, qemu-devel; +Cc: Aurelien Jarno
On 06/09/2016 12:03, James Hogan wrote:
> EVA load and store instructions access the user mode address map, so
> they need to use mem_idx of MIPS_HFLAG_UM. Update the various utility
> functions to allow mem_idx to be more easily overridden from the
> decoding logic.
>
> Specifically we add a mem_idx argument to the op_ld/st_* helpers used
> for atomics, and a mem_idx local variable to gen_ld(), gen_st(), and
> gen_st_cond().
>
> Signed-off-by: James Hogan <james.hogan@imgtec.com>
> Cc: Leon Alrae <leon.alrae@imgtec.com>
> Cc: Aurelien Jarno <aurelien@aurel32.net>
> ---
> target-mips/translate.c | 77 ++++++++++++++++++++++--------------------
> 1 file changed, 42 insertions(+), 35 deletions(-)
>
> diff --git a/target-mips/translate.c b/target-mips/translate.c
> index e224c2f09af4..df2befbd5294 100644
> --- a/target-mips/translate.c
> +++ b/target-mips/translate.c
> @@ -2028,7 +2028,8 @@ FOP_CONDNS(s, FMT_S, 32, gen_store_fpr32(ctx, fp0, fd))
> /* load/store instructions. */
> #ifdef CONFIG_USER_ONLY
> #define OP_LD_ATOMIC(insn,fname) \
> -static inline void op_ld_##insn(TCGv ret, TCGv arg1, DisasContext *ctx) \
> +static inline void op_ld_##insn(TCGv ret, TCGv arg1, int mem_idx, \
> + DisasContext *ctx) \
> { \
> TCGv t0 = tcg_temp_new(); \
> tcg_gen_mov_tl(t0, arg1); \
> @@ -2039,9 +2040,10 @@ static inline void op_ld_##insn(TCGv ret, TCGv arg1, DisasContext *ctx) \
> }
> #else
> #define OP_LD_ATOMIC(insn,fname) \
> -static inline void op_ld_##insn(TCGv ret, TCGv arg1, DisasContext *ctx) \
> +static inline void op_ld_##insn(TCGv ret, TCGv arg1, int mem_idx, \
> + DisasContext *ctx) \
> { \
> - gen_helper_1e1i(insn, ret, arg1, ctx->mem_idx); \
> + gen_helper_1e1i(insn, ret, arg1, mem_idx); \
> }
> #endif
> OP_LD_ATOMIC(ll,ld32s);
> @@ -2052,7 +2054,8 @@ OP_LD_ATOMIC(lld,ld64);
>
> #ifdef CONFIG_USER_ONLY
> #define OP_ST_ATOMIC(insn,fname,ldname,almask) \
> -static inline void op_st_##insn(TCGv arg1, TCGv arg2, int rt, DisasContext *ctx) \
> +static inline void op_st_##insn(TCGv arg1, TCGv arg2, int rt, int mem_idx, \
> + DisasContext *ctx) \
> { \
> TCGv t0 = tcg_temp_new(); \
> TCGLabel *l1 = gen_new_label(); \
> @@ -2076,10 +2079,11 @@ static inline void op_st_##insn(TCGv arg1, TCGv arg2, int rt, DisasContext *ctx)
> }
> #else
> #define OP_ST_ATOMIC(insn,fname,ldname,almask) \
> -static inline void op_st_##insn(TCGv arg1, TCGv arg2, int rt, DisasContext *ctx) \
> +static inline void op_st_##insn(TCGv arg1, TCGv arg2, int rt, int mem_idx, \
> + DisasContext *ctx) \
> { \
> TCGv t0 = tcg_temp_new(); \
> - gen_helper_1e2i(insn, t0, arg1, arg2, ctx->mem_idx); \
> + gen_helper_1e2i(insn, t0, arg1, arg2, mem_idx); \
> gen_store_gpr(t0, rt); \
> tcg_temp_free(t0); \
> }
> @@ -2122,6 +2126,7 @@ static void gen_ld(DisasContext *ctx, uint32_t opc,
> int rt, int base, int16_t offset)
> {
> TCGv t0, t1, t2;
> + int mem_idx = ctx->mem_idx;
>
> if (rt == 0 && ctx->insn_flags & (INSN_LOONGSON2E | INSN_LOONGSON2F)) {
> /* Loongson CPU uses a load to zero register for prefetch.
> @@ -2136,32 +2141,32 @@ static void gen_ld(DisasContext *ctx, uint32_t opc,
> switch (opc) {
> #if defined(TARGET_MIPS64)
> case OPC_LWU:
> - tcg_gen_qemu_ld_tl(t0, t0, ctx->mem_idx, MO_TEUL |
> + tcg_gen_qemu_ld_tl(t0, t0, mem_idx, MO_TEUL |
> ctx->default_tcg_memop_mask);
> gen_store_gpr(t0, rt);
> break;
> case OPC_LD:
> - tcg_gen_qemu_ld_tl(t0, t0, ctx->mem_idx, MO_TEQ |
> + tcg_gen_qemu_ld_tl(t0, t0, mem_idx, MO_TEQ |
> ctx->default_tcg_memop_mask);
> gen_store_gpr(t0, rt);
> break;
> case OPC_LLD:
> case R6_OPC_LLD:
> - op_ld_lld(t0, t0, ctx);
> + op_ld_lld(t0, t0, mem_idx, ctx);
> gen_store_gpr(t0, rt);
> break;
> case OPC_LDL:
> t1 = tcg_temp_new();
> /* Do a byte access to possibly trigger a page
> fault with the unaligned address. */
> - tcg_gen_qemu_ld_tl(t1, t0, ctx->mem_idx, MO_UB);
> + tcg_gen_qemu_ld_tl(t1, t0, mem_idx, MO_UB);
> tcg_gen_andi_tl(t1, t0, 7);
> #ifndef TARGET_WORDS_BIGENDIAN
> tcg_gen_xori_tl(t1, t1, 7);
> #endif
> tcg_gen_shli_tl(t1, t1, 3);
> tcg_gen_andi_tl(t0, t0, ~7);
> - tcg_gen_qemu_ld_tl(t0, t0, ctx->mem_idx, MO_TEQ);
> + tcg_gen_qemu_ld_tl(t0, t0, mem_idx, MO_TEQ);
> tcg_gen_shl_tl(t0, t0, t1);
> t2 = tcg_const_tl(-1);
> tcg_gen_shl_tl(t2, t2, t1);
> @@ -2176,14 +2181,14 @@ static void gen_ld(DisasContext *ctx, uint32_t opc,
> t1 = tcg_temp_new();
> /* Do a byte access to possibly trigger a page
> fault with the unaligned address. */
> - tcg_gen_qemu_ld_tl(t1, t0, ctx->mem_idx, MO_UB);
> + tcg_gen_qemu_ld_tl(t1, t0, mem_idx, MO_UB);
> tcg_gen_andi_tl(t1, t0, 7);
> #ifdef TARGET_WORDS_BIGENDIAN
> tcg_gen_xori_tl(t1, t1, 7);
> #endif
> tcg_gen_shli_tl(t1, t1, 3);
> tcg_gen_andi_tl(t0, t0, ~7);
> - tcg_gen_qemu_ld_tl(t0, t0, ctx->mem_idx, MO_TEQ);
> + tcg_gen_qemu_ld_tl(t0, t0, mem_idx, MO_TEQ);
> tcg_gen_shr_tl(t0, t0, t1);
> tcg_gen_xori_tl(t1, t1, 63);
> t2 = tcg_const_tl(0xfffffffffffffffeull);
> @@ -2199,7 +2204,7 @@ static void gen_ld(DisasContext *ctx, uint32_t opc,
> t1 = tcg_const_tl(pc_relative_pc(ctx));
> gen_op_addr_add(ctx, t0, t0, t1);
> tcg_temp_free(t1);
> - tcg_gen_qemu_ld_tl(t0, t0, ctx->mem_idx, MO_TEQ);
> + tcg_gen_qemu_ld_tl(t0, t0, mem_idx, MO_TEQ);
> gen_store_gpr(t0, rt);
> break;
> #endif
> @@ -2207,44 +2212,44 @@ static void gen_ld(DisasContext *ctx, uint32_t opc,
> t1 = tcg_const_tl(pc_relative_pc(ctx));
> gen_op_addr_add(ctx, t0, t0, t1);
> tcg_temp_free(t1);
> - tcg_gen_qemu_ld_tl(t0, t0, ctx->mem_idx, MO_TESL);
> + tcg_gen_qemu_ld_tl(t0, t0, mem_idx, MO_TESL);
> gen_store_gpr(t0, rt);
> break;
> case OPC_LW:
> - tcg_gen_qemu_ld_tl(t0, t0, ctx->mem_idx, MO_TESL |
> + tcg_gen_qemu_ld_tl(t0, t0, mem_idx, MO_TESL |
> ctx->default_tcg_memop_mask);
> gen_store_gpr(t0, rt);
> break;
> case OPC_LH:
> - tcg_gen_qemu_ld_tl(t0, t0, ctx->mem_idx, MO_TESW |
> + tcg_gen_qemu_ld_tl(t0, t0, mem_idx, MO_TESW |
> ctx->default_tcg_memop_mask);
> gen_store_gpr(t0, rt);
> break;
> case OPC_LHU:
> - tcg_gen_qemu_ld_tl(t0, t0, ctx->mem_idx, MO_TEUW |
> + tcg_gen_qemu_ld_tl(t0, t0, mem_idx, MO_TEUW |
> ctx->default_tcg_memop_mask);
> gen_store_gpr(t0, rt);
> break;
> case OPC_LB:
> - tcg_gen_qemu_ld_tl(t0, t0, ctx->mem_idx, MO_SB);
> + tcg_gen_qemu_ld_tl(t0, t0, mem_idx, MO_SB);
> gen_store_gpr(t0, rt);
> break;
> case OPC_LBU:
> - tcg_gen_qemu_ld_tl(t0, t0, ctx->mem_idx, MO_UB);
> + tcg_gen_qemu_ld_tl(t0, t0, mem_idx, MO_UB);
> gen_store_gpr(t0, rt);
> break;
> case OPC_LWL:
> t1 = tcg_temp_new();
> /* Do a byte access to possibly trigger a page
> fault with the unaligned address. */
> - tcg_gen_qemu_ld_tl(t1, t0, ctx->mem_idx, MO_UB);
> + tcg_gen_qemu_ld_tl(t1, t0, mem_idx, MO_UB);
> tcg_gen_andi_tl(t1, t0, 3);
> #ifndef TARGET_WORDS_BIGENDIAN
> tcg_gen_xori_tl(t1, t1, 3);
> #endif
> tcg_gen_shli_tl(t1, t1, 3);
> tcg_gen_andi_tl(t0, t0, ~3);
> - tcg_gen_qemu_ld_tl(t0, t0, ctx->mem_idx, MO_TEUL);
> + tcg_gen_qemu_ld_tl(t0, t0, mem_idx, MO_TEUL);
> tcg_gen_shl_tl(t0, t0, t1);
> t2 = tcg_const_tl(-1);
> tcg_gen_shl_tl(t2, t2, t1);
> @@ -2260,14 +2265,14 @@ static void gen_ld(DisasContext *ctx, uint32_t opc,
> t1 = tcg_temp_new();
> /* Do a byte access to possibly trigger a page
> fault with the unaligned address. */
> - tcg_gen_qemu_ld_tl(t1, t0, ctx->mem_idx, MO_UB);
> + tcg_gen_qemu_ld_tl(t1, t0, mem_idx, MO_UB);
> tcg_gen_andi_tl(t1, t0, 3);
> #ifdef TARGET_WORDS_BIGENDIAN
> tcg_gen_xori_tl(t1, t1, 3);
> #endif
> tcg_gen_shli_tl(t1, t1, 3);
> tcg_gen_andi_tl(t0, t0, ~3);
> - tcg_gen_qemu_ld_tl(t0, t0, ctx->mem_idx, MO_TEUL);
> + tcg_gen_qemu_ld_tl(t0, t0, mem_idx, MO_TEUL);
> tcg_gen_shr_tl(t0, t0, t1);
> tcg_gen_xori_tl(t1, t1, 31);
> t2 = tcg_const_tl(0xfffffffeull);
> @@ -2282,7 +2287,7 @@ static void gen_ld(DisasContext *ctx, uint32_t opc,
> break;
> case OPC_LL:
> case R6_OPC_LL:
> - op_ld_ll(t0, t0, ctx);
> + op_ld_ll(t0, t0, mem_idx, ctx);
> gen_store_gpr(t0, rt);
> break;
> }
> @@ -2295,38 +2300,39 @@ static void gen_st (DisasContext *ctx, uint32_t opc, int rt,
> {
> TCGv t0 = tcg_temp_new();
> TCGv t1 = tcg_temp_new();
> + int mem_idx = ctx->mem_idx;
>
> gen_base_offset_addr(ctx, t0, base, offset);
> gen_load_gpr(t1, rt);
> switch (opc) {
> #if defined(TARGET_MIPS64)
> case OPC_SD:
> - tcg_gen_qemu_st_tl(t1, t0, ctx->mem_idx, MO_TEQ |
> + tcg_gen_qemu_st_tl(t1, t0, mem_idx, MO_TEQ |
> ctx->default_tcg_memop_mask);
> break;
> case OPC_SDL:
> - gen_helper_0e2i(sdl, t1, t0, ctx->mem_idx);
> + gen_helper_0e2i(sdl, t1, t0, mem_idx);
> break;
> case OPC_SDR:
> - gen_helper_0e2i(sdr, t1, t0, ctx->mem_idx);
> + gen_helper_0e2i(sdr, t1, t0, mem_idx);
> break;
> #endif
> case OPC_SW:
> - tcg_gen_qemu_st_tl(t1, t0, ctx->mem_idx, MO_TEUL |
> + tcg_gen_qemu_st_tl(t1, t0, mem_idx, MO_TEUL |
> ctx->default_tcg_memop_mask);
> break;
> case OPC_SH:
> - tcg_gen_qemu_st_tl(t1, t0, ctx->mem_idx, MO_TEUW |
> + tcg_gen_qemu_st_tl(t1, t0, mem_idx, MO_TEUW |
> ctx->default_tcg_memop_mask);
> break;
> case OPC_SB:
> - tcg_gen_qemu_st_tl(t1, t0, ctx->mem_idx, MO_8);
> + tcg_gen_qemu_st_tl(t1, t0, mem_idx, MO_8);
> break;
> case OPC_SWL:
> - gen_helper_0e2i(swl, t1, t0, ctx->mem_idx);
> + gen_helper_0e2i(swl, t1, t0, mem_idx);
> break;
> case OPC_SWR:
> - gen_helper_0e2i(swr, t1, t0, ctx->mem_idx);
> + gen_helper_0e2i(swr, t1, t0, mem_idx);
> break;
> }
> tcg_temp_free(t0);
> @@ -2339,6 +2345,7 @@ static void gen_st_cond (DisasContext *ctx, uint32_t opc, int rt,
> int base, int16_t offset)
> {
> TCGv t0, t1;
> + int mem_idx = ctx->mem_idx;
>
> #ifdef CONFIG_USER_ONLY
> t0 = tcg_temp_local_new();
> @@ -2353,12 +2360,12 @@ static void gen_st_cond (DisasContext *ctx, uint32_t opc, int rt,
> #if defined(TARGET_MIPS64)
> case OPC_SCD:
> case R6_OPC_SCD:
> - op_st_scd(t1, t0, rt, ctx);
> + op_st_scd(t1, t0, rt, mem_idx, ctx);
> break;
> #endif
> case OPC_SC:
> case R6_OPC_SC:
> - op_st_sc(t1, t0, rt, ctx);
> + op_st_sc(t1, t0, rt, mem_idx, ctx);
> break;
> }
> tcg_temp_free(t1);
>
Reviewed-by: Yongbok Kim <yongbok.kim@imgtec.com>
Regards,
Yongbok
^ permalink raw reply [flat|nested] 30+ messages in thread
* [Qemu-devel] [PATCH 3/9] target-mips: Decode EVA load & store instructions
2016-09-06 11:03 [Qemu-devel] [PATCH 0/9] target-mips: Add Enhanced Virtual Addressing (EVA) support James Hogan
2016-09-06 11:03 ` [Qemu-devel] [PATCH 1/9] target-mips: Add CP0_Ebase.WG (write gate) support James Hogan
2016-09-06 11:03 ` [Qemu-devel] [PATCH 2/9] target-mips: Prepare loads/stores for EVA James Hogan
@ 2016-09-06 11:03 ` James Hogan
2016-10-07 15:34 ` Yongbok Kim
2016-09-06 11:03 ` [Qemu-devel] [PATCH 4/9] target-mips: Check memory permissions with mem_idx James Hogan
` (6 subsequent siblings)
9 siblings, 1 reply; 30+ messages in thread
From: James Hogan @ 2016-09-06 11:03 UTC (permalink / raw)
To: Leon Alrae; +Cc: qemu-devel, James Hogan, Aurelien Jarno
Implement decoding of EVA loads and stores. These access the user
address space from kernel mode when implemented, so for each instruction
we need to check that EVA is available from Config5.EVA & check for
sufficient COP0 privelege (with the new check_eva()), and then override
the mem_idx used for the operation.
Unfortunately some Loongson 2E instructions use overlapping encodings,
so we must be careful not to prevent those from being decoded when EVA
is absent.
Signed-off-by: James Hogan <james.hogan@imgtec.com>
Cc: Leon Alrae <leon.alrae@imgtec.com>
Cc: Aurelien Jarno <aurelien@aurel32.net>
---
target-mips/translate.c | 106 +++++++++++++++++++++++++++++++++++++++++-
1 file changed, 106 insertions(+), 0 deletions(-)
diff --git a/target-mips/translate.c b/target-mips/translate.c
index df2befbd5294..8506c39a359c 100644
--- a/target-mips/translate.c
+++ b/target-mips/translate.c
@@ -426,6 +426,24 @@ enum {
OPC_EXTR_W_DSP = 0x38 | OPC_SPECIAL3,
OPC_DEXTR_W_DSP = 0x3C | OPC_SPECIAL3,
+ /* EVA */
+ OPC_LWLE = 0x19 | OPC_SPECIAL3,
+ OPC_LWRE = 0x1A | OPC_SPECIAL3,
+ OPC_CACHEE = 0x1B | OPC_SPECIAL3,
+ OPC_SBE = 0x1C | OPC_SPECIAL3,
+ OPC_SHE = 0x1D | OPC_SPECIAL3,
+ OPC_SCE = 0x1E | OPC_SPECIAL3,
+ OPC_SWE = 0x1F | OPC_SPECIAL3,
+ OPC_SWLE = 0x21 | OPC_SPECIAL3,
+ OPC_SWRE = 0x22 | OPC_SPECIAL3,
+ OPC_PREFE = 0x23 | OPC_SPECIAL3,
+ OPC_LBUE = 0x28 | OPC_SPECIAL3,
+ OPC_LHUE = 0x29 | OPC_SPECIAL3,
+ OPC_LBE = 0x2C | OPC_SPECIAL3,
+ OPC_LHE = 0x2D | OPC_SPECIAL3,
+ OPC_LLE = 0x2E | OPC_SPECIAL3,
+ OPC_LWE = 0x2F | OPC_SPECIAL3,
+
/* R6 */
R6_OPC_PREF = 0x35 | OPC_SPECIAL3,
R6_OPC_CACHE = 0x25 | OPC_SPECIAL3,
@@ -1430,6 +1448,7 @@ typedef struct DisasContext {
bool bp;
uint64_t PAMask;
bool mvh;
+ bool eva;
int CP0_LLAddr_shift;
bool ps;
bool vp;
@@ -2215,29 +2234,47 @@ static void gen_ld(DisasContext *ctx, uint32_t opc,
tcg_gen_qemu_ld_tl(t0, t0, mem_idx, MO_TESL);
gen_store_gpr(t0, rt);
break;
+ case OPC_LWE:
+ mem_idx = MIPS_HFLAG_UM;
+ /* fall through */
case OPC_LW:
tcg_gen_qemu_ld_tl(t0, t0, mem_idx, MO_TESL |
ctx->default_tcg_memop_mask);
gen_store_gpr(t0, rt);
break;
+ case OPC_LHE:
+ mem_idx = MIPS_HFLAG_UM;
+ /* fall through */
case OPC_LH:
tcg_gen_qemu_ld_tl(t0, t0, mem_idx, MO_TESW |
ctx->default_tcg_memop_mask);
gen_store_gpr(t0, rt);
break;
+ case OPC_LHUE:
+ mem_idx = MIPS_HFLAG_UM;
+ /* fall through */
case OPC_LHU:
tcg_gen_qemu_ld_tl(t0, t0, mem_idx, MO_TEUW |
ctx->default_tcg_memop_mask);
gen_store_gpr(t0, rt);
break;
+ case OPC_LBE:
+ mem_idx = MIPS_HFLAG_UM;
+ /* fall through */
case OPC_LB:
tcg_gen_qemu_ld_tl(t0, t0, mem_idx, MO_SB);
gen_store_gpr(t0, rt);
break;
+ case OPC_LBUE:
+ mem_idx = MIPS_HFLAG_UM;
+ /* fall through */
case OPC_LBU:
tcg_gen_qemu_ld_tl(t0, t0, mem_idx, MO_UB);
gen_store_gpr(t0, rt);
break;
+ case OPC_LWLE:
+ mem_idx = MIPS_HFLAG_UM;
+ /* fall through */
case OPC_LWL:
t1 = tcg_temp_new();
/* Do a byte access to possibly trigger a page
@@ -2261,6 +2298,9 @@ static void gen_ld(DisasContext *ctx, uint32_t opc,
tcg_gen_ext32s_tl(t0, t0);
gen_store_gpr(t0, rt);
break;
+ case OPC_LWRE:
+ mem_idx = MIPS_HFLAG_UM;
+ /* fall through */
case OPC_LWR:
t1 = tcg_temp_new();
/* Do a byte access to possibly trigger a page
@@ -2285,6 +2325,9 @@ static void gen_ld(DisasContext *ctx, uint32_t opc,
tcg_gen_ext32s_tl(t0, t0);
gen_store_gpr(t0, rt);
break;
+ case OPC_LLE:
+ mem_idx = MIPS_HFLAG_UM;
+ /* fall through */
case OPC_LL:
case R6_OPC_LL:
op_ld_ll(t0, t0, mem_idx, ctx);
@@ -2317,20 +2360,35 @@ static void gen_st (DisasContext *ctx, uint32_t opc, int rt,
gen_helper_0e2i(sdr, t1, t0, mem_idx);
break;
#endif
+ case OPC_SWE:
+ mem_idx = MIPS_HFLAG_UM;
+ /* fall through */
case OPC_SW:
tcg_gen_qemu_st_tl(t1, t0, mem_idx, MO_TEUL |
ctx->default_tcg_memop_mask);
break;
+ case OPC_SHE:
+ mem_idx = MIPS_HFLAG_UM;
+ /* fall through */
case OPC_SH:
tcg_gen_qemu_st_tl(t1, t0, mem_idx, MO_TEUW |
ctx->default_tcg_memop_mask);
break;
+ case OPC_SBE:
+ mem_idx = MIPS_HFLAG_UM;
+ /* fall through */
case OPC_SB:
tcg_gen_qemu_st_tl(t1, t0, mem_idx, MO_8);
break;
+ case OPC_SWLE:
+ mem_idx = MIPS_HFLAG_UM;
+ /* fall through */
case OPC_SWL:
gen_helper_0e2i(swl, t1, t0, mem_idx);
break;
+ case OPC_SWRE:
+ mem_idx = MIPS_HFLAG_UM;
+ /* fall through */
case OPC_SWR:
gen_helper_0e2i(swr, t1, t0, mem_idx);
break;
@@ -2363,6 +2421,9 @@ static void gen_st_cond (DisasContext *ctx, uint32_t opc, int rt,
op_st_scd(t1, t0, rt, mem_idx, ctx);
break;
#endif
+ case OPC_SCE:
+ mem_idx = MIPS_HFLAG_UM;
+ /* fall through */
case OPC_SC:
case R6_OPC_SC:
op_st_sc(t1, t0, rt, mem_idx, ctx);
@@ -17981,13 +18042,57 @@ static void decode_opc_special3(CPUMIPSState *env, DisasContext *ctx)
{
int rs, rt, rd, sa;
uint32_t op1, op2;
+ int16_t imm;
rs = (ctx->opcode >> 21) & 0x1f;
rt = (ctx->opcode >> 16) & 0x1f;
rd = (ctx->opcode >> 11) & 0x1f;
sa = (ctx->opcode >> 6) & 0x1f;
+ imm = (int16_t)ctx->opcode >> 7;
op1 = MASK_SPECIAL3(ctx->opcode);
+
+ /*
+ * EVA loads and stores overlap Loongson 2E instructions decoded by
+ * decode_opc_special3_legacy(), so be careful to allow their decoding when
+ * EVA is absent.
+ */
+ if (ctx->eva) {
+ switch (op1) {
+ case OPC_LWLE ... OPC_LWRE:
+ check_insn_opc_removed(ctx, ISA_MIPS32R6);
+ /* fall through */
+ case OPC_LBUE ... OPC_LHUE:
+ case OPC_LBE ... OPC_LWE:
+ check_cp0_enabled(ctx);
+ gen_ld(ctx, op1, rt, rs, imm);
+ return;
+ case OPC_SWLE ... OPC_SWRE:
+ check_insn_opc_removed(ctx, ISA_MIPS32R6);
+ /* fall through */
+ case OPC_SBE ... OPC_SHE:
+ case OPC_SWE:
+ check_cp0_enabled(ctx);
+ gen_st(ctx, op1, rt, rs, imm);
+ return;
+ case OPC_SCE:
+ check_cp0_enabled(ctx);
+ gen_st_cond(ctx, op1, rt, rs, imm);
+ return;
+ case OPC_CACHEE:
+ check_cp0_enabled(ctx);
+ if (ctx->hflags & MIPS_HFLAG_ITC_CACHE) {
+ gen_cache_operation(ctx, rt, rs, imm);
+ }
+ /* Treat as NOP. */
+ return;
+ case OPC_PREFE:
+ check_cp0_enabled(ctx);
+ /* Treat as NOP. */
+ return;
+ }
+ }
+
switch (op1) {
case OPC_EXT:
case OPC_INS:
@@ -19883,6 +19988,7 @@ void gen_intermediate_code(CPUMIPSState *env, struct TranslationBlock *tb)
ctx.bp = (env->CP0_Config3 >> CP0C3_BP) & 1;
ctx.PAMask = env->PAMask;
ctx.mvh = (env->CP0_Config5 >> CP0C5_MVH) & 1;
+ ctx.eva = (env->CP0_Config5 >> CP0C5_EVA) & 1;
ctx.CP0_LLAddr_shift = env->CP0_LLAddr_shift;
ctx.cmgcr = (env->CP0_Config3 >> CP0C3_CMGCR) & 1;
/* Restore delay slot state from the tb context. */
--
git-series 0.8.10
^ permalink raw reply related [flat|nested] 30+ messages in thread* Re: [Qemu-devel] [PATCH 3/9] target-mips: Decode EVA load & store instructions
2016-09-06 11:03 ` [Qemu-devel] [PATCH 3/9] target-mips: Decode EVA load & store instructions James Hogan
@ 2016-10-07 15:34 ` Yongbok Kim
2016-10-07 15:48 ` James Hogan
0 siblings, 1 reply; 30+ messages in thread
From: Yongbok Kim @ 2016-10-07 15:34 UTC (permalink / raw)
To: James Hogan, qemu-devel; +Cc: Aurelien Jarno
On 06/09/2016 12:03, James Hogan wrote:
> Implement decoding of EVA loads and stores. These access the user
> address space from kernel mode when implemented, so for each instruction
> we need to check that EVA is available from Config5.EVA & check for
> sufficient COP0 privelege (with the new check_eva()), and then override
privilege
> the mem_idx used for the operation.
>
> Unfortunately some Loongson 2E instructions use overlapping encodings,
> so we must be careful not to prevent those from being decoded when EVA
> is absent.
>
> Signed-off-by: James Hogan <james.hogan@imgtec.com>
> Cc: Leon Alrae <leon.alrae@imgtec.com>
> Cc: Aurelien Jarno <aurelien@aurel32.net>
> ---
> target-mips/translate.c | 106 +++++++++++++++++++++++++++++++++++++++++-
> 1 file changed, 106 insertions(+), 0 deletions(-)
>
> diff --git a/target-mips/translate.c b/target-mips/translate.c
> index df2befbd5294..8506c39a359c 100644
> --- a/target-mips/translate.c
> +++ b/target-mips/translate.c
> @@ -426,6 +426,24 @@ enum {
> OPC_EXTR_W_DSP = 0x38 | OPC_SPECIAL3,
> OPC_DEXTR_W_DSP = 0x3C | OPC_SPECIAL3,
>
> + /* EVA */
> + OPC_LWLE = 0x19 | OPC_SPECIAL3,
> + OPC_LWRE = 0x1A | OPC_SPECIAL3,
> + OPC_CACHEE = 0x1B | OPC_SPECIAL3,
> + OPC_SBE = 0x1C | OPC_SPECIAL3,
> + OPC_SHE = 0x1D | OPC_SPECIAL3,
> + OPC_SCE = 0x1E | OPC_SPECIAL3,
> + OPC_SWE = 0x1F | OPC_SPECIAL3,
> + OPC_SWLE = 0x21 | OPC_SPECIAL3,
> + OPC_SWRE = 0x22 | OPC_SPECIAL3,
> + OPC_PREFE = 0x23 | OPC_SPECIAL3,
> + OPC_LBUE = 0x28 | OPC_SPECIAL3,
> + OPC_LHUE = 0x29 | OPC_SPECIAL3,
> + OPC_LBE = 0x2C | OPC_SPECIAL3,
> + OPC_LHE = 0x2D | OPC_SPECIAL3,
> + OPC_LLE = 0x2E | OPC_SPECIAL3,
> + OPC_LWE = 0x2F | OPC_SPECIAL3,
> +
EVA for MIPS32 only. It is OK but it's worth mentioning it in the commit
messages.
> /* R6 */
> R6_OPC_PREF = 0x35 | OPC_SPECIAL3,
> R6_OPC_CACHE = 0x25 | OPC_SPECIAL3,
> @@ -1430,6 +1448,7 @@ typedef struct DisasContext {
> bool bp;
> uint64_t PAMask;
> bool mvh;
> + bool eva;
> int CP0_LLAddr_shift;
> bool ps;
> bool vp;
> @@ -2215,29 +2234,47 @@ static void gen_ld(DisasContext *ctx, uint32_t opc,
> tcg_gen_qemu_ld_tl(t0, t0, mem_idx, MO_TESL);
> gen_store_gpr(t0, rt);
> break;
> + case OPC_LWE:
> + mem_idx = MIPS_HFLAG_UM;
> + /* fall through */
> case OPC_LW:
> tcg_gen_qemu_ld_tl(t0, t0, mem_idx, MO_TESL |
> ctx->default_tcg_memop_mask);
> gen_store_gpr(t0, rt);
> break;
> + case OPC_LHE:
> + mem_idx = MIPS_HFLAG_UM;
> + /* fall through */
> case OPC_LH:
> tcg_gen_qemu_ld_tl(t0, t0, mem_idx, MO_TESW |
> ctx->default_tcg_memop_mask);
> gen_store_gpr(t0, rt);
> break;
> + case OPC_LHUE:
> + mem_idx = MIPS_HFLAG_UM;
> + /* fall through */
> case OPC_LHU:
> tcg_gen_qemu_ld_tl(t0, t0, mem_idx, MO_TEUW |
> ctx->default_tcg_memop_mask);
> gen_store_gpr(t0, rt);
> break;
> + case OPC_LBE:
> + mem_idx = MIPS_HFLAG_UM;
> + /* fall through */
> case OPC_LB:
> tcg_gen_qemu_ld_tl(t0, t0, mem_idx, MO_SB);
> gen_store_gpr(t0, rt);
> break;
> + case OPC_LBUE:
> + mem_idx = MIPS_HFLAG_UM;
> + /* fall through */
> case OPC_LBU:
> tcg_gen_qemu_ld_tl(t0, t0, mem_idx, MO_UB);
> gen_store_gpr(t0, rt);
> break;
> + case OPC_LWLE:
> + mem_idx = MIPS_HFLAG_UM;
> + /* fall through */
> case OPC_LWL:
> t1 = tcg_temp_new();
> /* Do a byte access to possibly trigger a page
> @@ -2261,6 +2298,9 @@ static void gen_ld(DisasContext *ctx, uint32_t opc,
> tcg_gen_ext32s_tl(t0, t0);
> gen_store_gpr(t0, rt);
> break;
> + case OPC_LWRE:
> + mem_idx = MIPS_HFLAG_UM;
> + /* fall through */
> case OPC_LWR:
> t1 = tcg_temp_new();
> /* Do a byte access to possibly trigger a page
> @@ -2285,6 +2325,9 @@ static void gen_ld(DisasContext *ctx, uint32_t opc,
> tcg_gen_ext32s_tl(t0, t0);
> gen_store_gpr(t0, rt);
> break;
> + case OPC_LLE:
> + mem_idx = MIPS_HFLAG_UM;
> + /* fall through */
> case OPC_LL:
> case R6_OPC_LL:
> op_ld_ll(t0, t0, mem_idx, ctx);
> @@ -2317,20 +2360,35 @@ static void gen_st (DisasContext *ctx, uint32_t opc, int rt,
> gen_helper_0e2i(sdr, t1, t0, mem_idx);
> break;
> #endif
> + case OPC_SWE:
> + mem_idx = MIPS_HFLAG_UM;
> + /* fall through */
> case OPC_SW:
> tcg_gen_qemu_st_tl(t1, t0, mem_idx, MO_TEUL |
> ctx->default_tcg_memop_mask);
> break;
> + case OPC_SHE:
> + mem_idx = MIPS_HFLAG_UM;
> + /* fall through */
> case OPC_SH:
> tcg_gen_qemu_st_tl(t1, t0, mem_idx, MO_TEUW |
> ctx->default_tcg_memop_mask);
> break;
> + case OPC_SBE:
> + mem_idx = MIPS_HFLAG_UM;
> + /* fall through */
> case OPC_SB:
> tcg_gen_qemu_st_tl(t1, t0, mem_idx, MO_8);
> break;
> + case OPC_SWLE:
> + mem_idx = MIPS_HFLAG_UM;
> + /* fall through */
> case OPC_SWL:
> gen_helper_0e2i(swl, t1, t0, mem_idx);
> break;
> + case OPC_SWRE:
> + mem_idx = MIPS_HFLAG_UM;
> + /* fall through */
> case OPC_SWR:
> gen_helper_0e2i(swr, t1, t0, mem_idx);
> break;
> @@ -2363,6 +2421,9 @@ static void gen_st_cond (DisasContext *ctx, uint32_t opc, int rt,
> op_st_scd(t1, t0, rt, mem_idx, ctx);
> break;
> #endif
> + case OPC_SCE:
> + mem_idx = MIPS_HFLAG_UM;
> + /* fall through */
> case OPC_SC:
> case R6_OPC_SC:
> op_st_sc(t1, t0, rt, mem_idx, ctx);
> @@ -17981,13 +18042,57 @@ static void decode_opc_special3(CPUMIPSState *env, DisasContext *ctx)
> {
> int rs, rt, rd, sa;
> uint32_t op1, op2;
> + int16_t imm;
>
> rs = (ctx->opcode >> 21) & 0x1f;
> rt = (ctx->opcode >> 16) & 0x1f;
> rd = (ctx->opcode >> 11) & 0x1f;
> sa = (ctx->opcode >> 6) & 0x1f;
> + imm = (int16_t)ctx->opcode >> 7;
imm = (int16_t) (ctx->opcode >> 7) & 0x1ff;
>
> op1 = MASK_SPECIAL3(ctx->opcode);
> +
> + /*
> + * EVA loads and stores overlap Loongson 2E instructions decoded by
> + * decode_opc_special3_legacy(), so be careful to allow their decoding when
> + * EVA is absent.
> + */
> + if (ctx->eva) {
> + switch (op1) {
> + case OPC_LWLE ... OPC_LWRE:
> + check_insn_opc_removed(ctx, ISA_MIPS32R6);
> + /* fall through */
> + case OPC_LBUE ... OPC_LHUE:
> + case OPC_LBE ... OPC_LWE:
> + check_cp0_enabled(ctx);
> + gen_ld(ctx, op1, rt, rs, imm);
> + return;
> + case OPC_SWLE ... OPC_SWRE:
> + check_insn_opc_removed(ctx, ISA_MIPS32R6);
> + /* fall through */
> + case OPC_SBE ... OPC_SHE:
> + case OPC_SWE:
> + check_cp0_enabled(ctx);
> + gen_st(ctx, op1, rt, rs, imm);
> + return;
> + case OPC_SCE:
> + check_cp0_enabled(ctx);
> + gen_st_cond(ctx, op1, rt, rs, imm);
> + return;
> + case OPC_CACHEE:
> + check_cp0_enabled(ctx);
> + if (ctx->hflags & MIPS_HFLAG_ITC_CACHE) {
> + gen_cache_operation(ctx, rt, rs, imm);
> + }
> + /* Treat as NOP. */
this comment is not relevant any more.
> + return;
> + case OPC_PREFE:
> + check_cp0_enabled(ctx);
> + /* Treat as NOP. */
> + return;
> + }
> + }
> +
> switch (op1) {
> case OPC_EXT:
> case OPC_INS:
> @@ -19883,6 +19988,7 @@ void gen_intermediate_code(CPUMIPSState *env, struct TranslationBlock *tb)
> ctx.bp = (env->CP0_Config3 >> CP0C3_BP) & 1;
> ctx.PAMask = env->PAMask;
> ctx.mvh = (env->CP0_Config5 >> CP0C5_MVH) & 1;
> + ctx.eva = (env->CP0_Config5 >> CP0C5_EVA) & 1;
> ctx.CP0_LLAddr_shift = env->CP0_LLAddr_shift;
> ctx.cmgcr = (env->CP0_Config3 >> CP0C3_CMGCR) & 1;
> /* Restore delay slot state from the tb context. */
>
Regards,
Yongbok
^ permalink raw reply [flat|nested] 30+ messages in thread* Re: [Qemu-devel] [PATCH 3/9] target-mips: Decode EVA load & store instructions
2016-10-07 15:34 ` Yongbok Kim
@ 2016-10-07 15:48 ` James Hogan
2016-10-07 16:05 ` Yongbok Kim
0 siblings, 1 reply; 30+ messages in thread
From: James Hogan @ 2016-10-07 15:48 UTC (permalink / raw)
To: Yongbok Kim; +Cc: qemu-devel, Aurelien Jarno
[-- Attachment #1: Type: text/plain, Size: 10058 bytes --]
On Fri, Oct 07, 2016 at 04:34:27PM +0100, Yongbok Kim wrote:
>
>
> On 06/09/2016 12:03, James Hogan wrote:
> > Implement decoding of EVA loads and stores. These access the user
> > address space from kernel mode when implemented, so for each instruction
> > we need to check that EVA is available from Config5.EVA & check for
> > sufficient COP0 privelege (with the new check_eva()), and then override
>
> privilege
Good spot, thanks
>
> > the mem_idx used for the operation.
> >
> > Unfortunately some Loongson 2E instructions use overlapping encodings,
> > so we must be careful not to prevent those from being decoded when EVA
> > is absent.
> >
> > Signed-off-by: James Hogan <james.hogan@imgtec.com>
> > Cc: Leon Alrae <leon.alrae@imgtec.com>
> > Cc: Aurelien Jarno <aurelien@aurel32.net>
> > ---
> > target-mips/translate.c | 106 +++++++++++++++++++++++++++++++++++++++++-
> > 1 file changed, 106 insertions(+), 0 deletions(-)
> >
> > diff --git a/target-mips/translate.c b/target-mips/translate.c
> > index df2befbd5294..8506c39a359c 100644
> > --- a/target-mips/translate.c
> > +++ b/target-mips/translate.c
> > @@ -426,6 +426,24 @@ enum {
> > OPC_EXTR_W_DSP = 0x38 | OPC_SPECIAL3,
> > OPC_DEXTR_W_DSP = 0x3C | OPC_SPECIAL3,
> >
> > + /* EVA */
> > + OPC_LWLE = 0x19 | OPC_SPECIAL3,
> > + OPC_LWRE = 0x1A | OPC_SPECIAL3,
> > + OPC_CACHEE = 0x1B | OPC_SPECIAL3,
> > + OPC_SBE = 0x1C | OPC_SPECIAL3,
> > + OPC_SHE = 0x1D | OPC_SPECIAL3,
> > + OPC_SCE = 0x1E | OPC_SPECIAL3,
> > + OPC_SWE = 0x1F | OPC_SPECIAL3,
> > + OPC_SWLE = 0x21 | OPC_SPECIAL3,
> > + OPC_SWRE = 0x22 | OPC_SPECIAL3,
> > + OPC_PREFE = 0x23 | OPC_SPECIAL3,
> > + OPC_LBUE = 0x28 | OPC_SPECIAL3,
> > + OPC_LHUE = 0x29 | OPC_SPECIAL3,
> > + OPC_LBE = 0x2C | OPC_SPECIAL3,
> > + OPC_LHE = 0x2D | OPC_SPECIAL3,
> > + OPC_LLE = 0x2E | OPC_SPECIAL3,
> > + OPC_LWE = 0x2F | OPC_SPECIAL3,
> > +
>
> EVA for MIPS32 only. It is OK but it's worth mentioning it in the commit
> messages.
Are you referring to the lack of sde/lde instructions? My understanding
is that these were never defined since EVA is aimed at 32-bit code
(although segmentation control can still be implemented on a MIPS64
core, e.g. P6600).
>
> > /* R6 */
> > R6_OPC_PREF = 0x35 | OPC_SPECIAL3,
> > R6_OPC_CACHE = 0x25 | OPC_SPECIAL3,
> > @@ -1430,6 +1448,7 @@ typedef struct DisasContext {
> > bool bp;
> > uint64_t PAMask;
> > bool mvh;
> > + bool eva;
> > int CP0_LLAddr_shift;
> > bool ps;
> > bool vp;
> > @@ -2215,29 +2234,47 @@ static void gen_ld(DisasContext *ctx, uint32_t opc,
> > tcg_gen_qemu_ld_tl(t0, t0, mem_idx, MO_TESL);
> > gen_store_gpr(t0, rt);
> > break;
> > + case OPC_LWE:
> > + mem_idx = MIPS_HFLAG_UM;
> > + /* fall through */
> > case OPC_LW:
> > tcg_gen_qemu_ld_tl(t0, t0, mem_idx, MO_TESL |
> > ctx->default_tcg_memop_mask);
> > gen_store_gpr(t0, rt);
> > break;
> > + case OPC_LHE:
> > + mem_idx = MIPS_HFLAG_UM;
> > + /* fall through */
> > case OPC_LH:
> > tcg_gen_qemu_ld_tl(t0, t0, mem_idx, MO_TESW |
> > ctx->default_tcg_memop_mask);
> > gen_store_gpr(t0, rt);
> > break;
> > + case OPC_LHUE:
> > + mem_idx = MIPS_HFLAG_UM;
> > + /* fall through */
> > case OPC_LHU:
> > tcg_gen_qemu_ld_tl(t0, t0, mem_idx, MO_TEUW |
> > ctx->default_tcg_memop_mask);
> > gen_store_gpr(t0, rt);
> > break;
> > + case OPC_LBE:
> > + mem_idx = MIPS_HFLAG_UM;
> > + /* fall through */
> > case OPC_LB:
> > tcg_gen_qemu_ld_tl(t0, t0, mem_idx, MO_SB);
> > gen_store_gpr(t0, rt);
> > break;
> > + case OPC_LBUE:
> > + mem_idx = MIPS_HFLAG_UM;
> > + /* fall through */
> > case OPC_LBU:
> > tcg_gen_qemu_ld_tl(t0, t0, mem_idx, MO_UB);
> > gen_store_gpr(t0, rt);
> > break;
> > + case OPC_LWLE:
> > + mem_idx = MIPS_HFLAG_UM;
> > + /* fall through */
> > case OPC_LWL:
> > t1 = tcg_temp_new();
> > /* Do a byte access to possibly trigger a page
> > @@ -2261,6 +2298,9 @@ static void gen_ld(DisasContext *ctx, uint32_t opc,
> > tcg_gen_ext32s_tl(t0, t0);
> > gen_store_gpr(t0, rt);
> > break;
> > + case OPC_LWRE:
> > + mem_idx = MIPS_HFLAG_UM;
> > + /* fall through */
> > case OPC_LWR:
> > t1 = tcg_temp_new();
> > /* Do a byte access to possibly trigger a page
> > @@ -2285,6 +2325,9 @@ static void gen_ld(DisasContext *ctx, uint32_t opc,
> > tcg_gen_ext32s_tl(t0, t0);
> > gen_store_gpr(t0, rt);
> > break;
> > + case OPC_LLE:
> > + mem_idx = MIPS_HFLAG_UM;
> > + /* fall through */
> > case OPC_LL:
> > case R6_OPC_LL:
> > op_ld_ll(t0, t0, mem_idx, ctx);
> > @@ -2317,20 +2360,35 @@ static void gen_st (DisasContext *ctx, uint32_t opc, int rt,
> > gen_helper_0e2i(sdr, t1, t0, mem_idx);
> > break;
> > #endif
> > + case OPC_SWE:
> > + mem_idx = MIPS_HFLAG_UM;
> > + /* fall through */
> > case OPC_SW:
> > tcg_gen_qemu_st_tl(t1, t0, mem_idx, MO_TEUL |
> > ctx->default_tcg_memop_mask);
> > break;
> > + case OPC_SHE:
> > + mem_idx = MIPS_HFLAG_UM;
> > + /* fall through */
> > case OPC_SH:
> > tcg_gen_qemu_st_tl(t1, t0, mem_idx, MO_TEUW |
> > ctx->default_tcg_memop_mask);
> > break;
> > + case OPC_SBE:
> > + mem_idx = MIPS_HFLAG_UM;
> > + /* fall through */
> > case OPC_SB:
> > tcg_gen_qemu_st_tl(t1, t0, mem_idx, MO_8);
> > break;
> > + case OPC_SWLE:
> > + mem_idx = MIPS_HFLAG_UM;
> > + /* fall through */
> > case OPC_SWL:
> > gen_helper_0e2i(swl, t1, t0, mem_idx);
> > break;
> > + case OPC_SWRE:
> > + mem_idx = MIPS_HFLAG_UM;
> > + /* fall through */
> > case OPC_SWR:
> > gen_helper_0e2i(swr, t1, t0, mem_idx);
> > break;
> > @@ -2363,6 +2421,9 @@ static void gen_st_cond (DisasContext *ctx, uint32_t opc, int rt,
> > op_st_scd(t1, t0, rt, mem_idx, ctx);
> > break;
> > #endif
> > + case OPC_SCE:
> > + mem_idx = MIPS_HFLAG_UM;
> > + /* fall through */
> > case OPC_SC:
> > case R6_OPC_SC:
> > op_st_sc(t1, t0, rt, mem_idx, ctx);
> > @@ -17981,13 +18042,57 @@ static void decode_opc_special3(CPUMIPSState *env, DisasContext *ctx)
> > {
> > int rs, rt, rd, sa;
> > uint32_t op1, op2;
> > + int16_t imm;
> >
> > rs = (ctx->opcode >> 21) & 0x1f;
> > rt = (ctx->opcode >> 16) & 0x1f;
> > rd = (ctx->opcode >> 11) & 0x1f;
> > sa = (ctx->opcode >> 6) & 0x1f;
> > + imm = (int16_t)ctx->opcode >> 7;
>
> imm = (int16_t) (ctx->opcode >> 7) & 0x1ff;
That won't sign extend it correctly.
>
> >
> > op1 = MASK_SPECIAL3(ctx->opcode);
> > +
> > + /*
> > + * EVA loads and stores overlap Loongson 2E instructions decoded by
> > + * decode_opc_special3_legacy(), so be careful to allow their decoding when
> > + * EVA is absent.
> > + */
> > + if (ctx->eva) {
> > + switch (op1) {
> > + case OPC_LWLE ... OPC_LWRE:
> > + check_insn_opc_removed(ctx, ISA_MIPS32R6);
> > + /* fall through */
> > + case OPC_LBUE ... OPC_LHUE:
> > + case OPC_LBE ... OPC_LWE:
> > + check_cp0_enabled(ctx);
> > + gen_ld(ctx, op1, rt, rs, imm);
> > + return;
> > + case OPC_SWLE ... OPC_SWRE:
> > + check_insn_opc_removed(ctx, ISA_MIPS32R6);
> > + /* fall through */
> > + case OPC_SBE ... OPC_SHE:
> > + case OPC_SWE:
> > + check_cp0_enabled(ctx);
> > + gen_st(ctx, op1, rt, rs, imm);
> > + return;
> > + case OPC_SCE:
> > + check_cp0_enabled(ctx);
> > + gen_st_cond(ctx, op1, rt, rs, imm);
> > + return;
> > + case OPC_CACHEE:
> > + check_cp0_enabled(ctx);
> > + if (ctx->hflags & MIPS_HFLAG_ITC_CACHE) {
> > + gen_cache_operation(ctx, rt, rs, imm);
> > + }
> > + /* Treat as NOP. */
>
> this comment is not relevant any more.
Well its relevant in the sense that the CACHEE operation doesn't
actually do anything of any significance. It won't even trigger a TLB
exception (which technically it should).
I can drop it though if you prefer.
>
> > + return;
> > + case OPC_PREFE:
> > + check_cp0_enabled(ctx);
> > + /* Treat as NOP. */
> > + return;
> > + }
> > + }
> > +
> > switch (op1) {
> > case OPC_EXT:
> > case OPC_INS:
> > @@ -19883,6 +19988,7 @@ void gen_intermediate_code(CPUMIPSState *env, struct TranslationBlock *tb)
> > ctx.bp = (env->CP0_Config3 >> CP0C3_BP) & 1;
> > ctx.PAMask = env->PAMask;
> > ctx.mvh = (env->CP0_Config5 >> CP0C5_MVH) & 1;
> > + ctx.eva = (env->CP0_Config5 >> CP0C5_EVA) & 1;
> > ctx.CP0_LLAddr_shift = env->CP0_LLAddr_shift;
> > ctx.cmgcr = (env->CP0_Config3 >> CP0C3_CMGCR) & 1;
> > /* Restore delay slot state from the tb context. */
> >
>
> Regards,
> Yongbok
Thanks for reviewing,
Cheers
James
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply [flat|nested] 30+ messages in thread* Re: [Qemu-devel] [PATCH 3/9] target-mips: Decode EVA load & store instructions
2016-10-07 15:48 ` James Hogan
@ 2016-10-07 16:05 ` Yongbok Kim
2016-10-07 16:16 ` James Hogan
0 siblings, 1 reply; 30+ messages in thread
From: Yongbok Kim @ 2016-10-07 16:05 UTC (permalink / raw)
To: James Hogan; +Cc: qemu-devel, Aurelien Jarno
On 07/10/2016 16:48, James Hogan wrote:
> On Fri, Oct 07, 2016 at 04:34:27PM +0100, Yongbok Kim wrote:
>>
>>
>> On 06/09/2016 12:03, James Hogan wrote:
>>> Implement decoding of EVA loads and stores. These access the user
>>> address space from kernel mode when implemented, so for each instruction
>>> we need to check that EVA is available from Config5.EVA & check for
>>> sufficient COP0 privelege (with the new check_eva()), and then override
>>
>> privilege
>
> Good spot, thanks
>
>>
>>> the mem_idx used for the operation.
>>>
>>> Unfortunately some Loongson 2E instructions use overlapping encodings,
>>> so we must be careful not to prevent those from being decoded when EVA
>>> is absent.
>>>
>>> Signed-off-by: James Hogan <james.hogan@imgtec.com>
>>> Cc: Leon Alrae <leon.alrae@imgtec.com>
>>> Cc: Aurelien Jarno <aurelien@aurel32.net>
>>> ---
>>> target-mips/translate.c | 106 +++++++++++++++++++++++++++++++++++++++++-
>>> 1 file changed, 106 insertions(+), 0 deletions(-)
>>>
>>> diff --git a/target-mips/translate.c b/target-mips/translate.c
>>> index df2befbd5294..8506c39a359c 100644
>>> --- a/target-mips/translate.c
>>> +++ b/target-mips/translate.c
>>> @@ -426,6 +426,24 @@ enum {
>>> OPC_EXTR_W_DSP = 0x38 | OPC_SPECIAL3,
>>> OPC_DEXTR_W_DSP = 0x3C | OPC_SPECIAL3,
>>>
>>> + /* EVA */
>>> + OPC_LWLE = 0x19 | OPC_SPECIAL3,
>>> + OPC_LWRE = 0x1A | OPC_SPECIAL3,
>>> + OPC_CACHEE = 0x1B | OPC_SPECIAL3,
>>> + OPC_SBE = 0x1C | OPC_SPECIAL3,
>>> + OPC_SHE = 0x1D | OPC_SPECIAL3,
>>> + OPC_SCE = 0x1E | OPC_SPECIAL3,
>>> + OPC_SWE = 0x1F | OPC_SPECIAL3,
>>> + OPC_SWLE = 0x21 | OPC_SPECIAL3,
>>> + OPC_SWRE = 0x22 | OPC_SPECIAL3,
>>> + OPC_PREFE = 0x23 | OPC_SPECIAL3,
>>> + OPC_LBUE = 0x28 | OPC_SPECIAL3,
>>> + OPC_LHUE = 0x29 | OPC_SPECIAL3,
>>> + OPC_LBE = 0x2C | OPC_SPECIAL3,
>>> + OPC_LHE = 0x2D | OPC_SPECIAL3,
>>> + OPC_LLE = 0x2E | OPC_SPECIAL3,
>>> + OPC_LWE = 0x2F | OPC_SPECIAL3,
>>> +
>>
>> EVA for MIPS32 only. It is OK but it's worth mentioning it in the commit
>> messages.
>
> Are you referring to the lack of sde/lde instructions? My understanding
> is that these were never defined since EVA is aimed at 32-bit code
> (although segmentation control can still be implemented on a MIPS64
> core, e.g. P6600).
>
Okay. What about microMIPS version of these instructions?
>>
>>> /* R6 */
>>> R6_OPC_PREF = 0x35 | OPC_SPECIAL3,
>>> R6_OPC_CACHE = 0x25 | OPC_SPECIAL3,
>>> @@ -1430,6 +1448,7 @@ typedef struct DisasContext {
>>> bool bp;
>>> uint64_t PAMask;
>>> bool mvh;
>>> + bool eva;
>>> int CP0_LLAddr_shift;
>>> bool ps;
>>> bool vp;
>>> @@ -2215,29 +2234,47 @@ static void gen_ld(DisasContext *ctx, uint32_t opc,
>>> tcg_gen_qemu_ld_tl(t0, t0, mem_idx, MO_TESL);
>>> gen_store_gpr(t0, rt);
>>> break;
>>> + case OPC_LWE:
>>> + mem_idx = MIPS_HFLAG_UM;
>>> + /* fall through */
>>> case OPC_LW:
>>> tcg_gen_qemu_ld_tl(t0, t0, mem_idx, MO_TESL |
>>> ctx->default_tcg_memop_mask);
>>> gen_store_gpr(t0, rt);
>>> break;
>>> + case OPC_LHE:
>>> + mem_idx = MIPS_HFLAG_UM;
>>> + /* fall through */
>>> case OPC_LH:
>>> tcg_gen_qemu_ld_tl(t0, t0, mem_idx, MO_TESW |
>>> ctx->default_tcg_memop_mask);
>>> gen_store_gpr(t0, rt);
>>> break;
>>> + case OPC_LHUE:
>>> + mem_idx = MIPS_HFLAG_UM;
>>> + /* fall through */
>>> case OPC_LHU:
>>> tcg_gen_qemu_ld_tl(t0, t0, mem_idx, MO_TEUW |
>>> ctx->default_tcg_memop_mask);
>>> gen_store_gpr(t0, rt);
>>> break;
>>> + case OPC_LBE:
>>> + mem_idx = MIPS_HFLAG_UM;
>>> + /* fall through */
>>> case OPC_LB:
>>> tcg_gen_qemu_ld_tl(t0, t0, mem_idx, MO_SB);
>>> gen_store_gpr(t0, rt);
>>> break;
>>> + case OPC_LBUE:
>>> + mem_idx = MIPS_HFLAG_UM;
>>> + /* fall through */
>>> case OPC_LBU:
>>> tcg_gen_qemu_ld_tl(t0, t0, mem_idx, MO_UB);
>>> gen_store_gpr(t0, rt);
>>> break;
>>> + case OPC_LWLE:
>>> + mem_idx = MIPS_HFLAG_UM;
>>> + /* fall through */
>>> case OPC_LWL:
>>> t1 = tcg_temp_new();
>>> /* Do a byte access to possibly trigger a page
>>> @@ -2261,6 +2298,9 @@ static void gen_ld(DisasContext *ctx, uint32_t opc,
>>> tcg_gen_ext32s_tl(t0, t0);
>>> gen_store_gpr(t0, rt);
>>> break;
>>> + case OPC_LWRE:
>>> + mem_idx = MIPS_HFLAG_UM;
>>> + /* fall through */
>>> case OPC_LWR:
>>> t1 = tcg_temp_new();
>>> /* Do a byte access to possibly trigger a page
>>> @@ -2285,6 +2325,9 @@ static void gen_ld(DisasContext *ctx, uint32_t opc,
>>> tcg_gen_ext32s_tl(t0, t0);
>>> gen_store_gpr(t0, rt);
>>> break;
>>> + case OPC_LLE:
>>> + mem_idx = MIPS_HFLAG_UM;
>>> + /* fall through */
>>> case OPC_LL:
>>> case R6_OPC_LL:
>>> op_ld_ll(t0, t0, mem_idx, ctx);
>>> @@ -2317,20 +2360,35 @@ static void gen_st (DisasContext *ctx, uint32_t opc, int rt,
>>> gen_helper_0e2i(sdr, t1, t0, mem_idx);
>>> break;
>>> #endif
>>> + case OPC_SWE:
>>> + mem_idx = MIPS_HFLAG_UM;
>>> + /* fall through */
>>> case OPC_SW:
>>> tcg_gen_qemu_st_tl(t1, t0, mem_idx, MO_TEUL |
>>> ctx->default_tcg_memop_mask);
>>> break;
>>> + case OPC_SHE:
>>> + mem_idx = MIPS_HFLAG_UM;
>>> + /* fall through */
>>> case OPC_SH:
>>> tcg_gen_qemu_st_tl(t1, t0, mem_idx, MO_TEUW |
>>> ctx->default_tcg_memop_mask);
>>> break;
>>> + case OPC_SBE:
>>> + mem_idx = MIPS_HFLAG_UM;
>>> + /* fall through */
>>> case OPC_SB:
>>> tcg_gen_qemu_st_tl(t1, t0, mem_idx, MO_8);
>>> break;
>>> + case OPC_SWLE:
>>> + mem_idx = MIPS_HFLAG_UM;
>>> + /* fall through */
>>> case OPC_SWL:
>>> gen_helper_0e2i(swl, t1, t0, mem_idx);
>>> break;
>>> + case OPC_SWRE:
>>> + mem_idx = MIPS_HFLAG_UM;
>>> + /* fall through */
>>> case OPC_SWR:
>>> gen_helper_0e2i(swr, t1, t0, mem_idx);
>>> break;
>>> @@ -2363,6 +2421,9 @@ static void gen_st_cond (DisasContext *ctx, uint32_t opc, int rt,
>>> op_st_scd(t1, t0, rt, mem_idx, ctx);
>>> break;
>>> #endif
>>> + case OPC_SCE:
>>> + mem_idx = MIPS_HFLAG_UM;
>>> + /* fall through */
>>> case OPC_SC:
>>> case R6_OPC_SC:
>>> op_st_sc(t1, t0, rt, mem_idx, ctx);
>>> @@ -17981,13 +18042,57 @@ static void decode_opc_special3(CPUMIPSState *env, DisasContext *ctx)
>>> {
>>> int rs, rt, rd, sa;
>>> uint32_t op1, op2;
>>> + int16_t imm;
>>>
>>> rs = (ctx->opcode >> 21) & 0x1f;
>>> rt = (ctx->opcode >> 16) & 0x1f;
>>> rd = (ctx->opcode >> 11) & 0x1f;
>>> sa = (ctx->opcode >> 6) & 0x1f;
>>> + imm = (int16_t)ctx->opcode >> 7;
>>
>> imm = (int16_t) (ctx->opcode >> 7) & 0x1ff;
>
> That won't sign extend it correctly.
That's true :) Use sextract32() then?
>
>>
>>>
>>> op1 = MASK_SPECIAL3(ctx->opcode);
>>> +
>>> + /*
>>> + * EVA loads and stores overlap Loongson 2E instructions decoded by
>>> + * decode_opc_special3_legacy(), so be careful to allow their decoding when
>>> + * EVA is absent.
>>> + */
>>> + if (ctx->eva) {
>>> + switch (op1) {
>>> + case OPC_LWLE ... OPC_LWRE:
>>> + check_insn_opc_removed(ctx, ISA_MIPS32R6);
>>> + /* fall through */
>>> + case OPC_LBUE ... OPC_LHUE:
>>> + case OPC_LBE ... OPC_LWE:
>>> + check_cp0_enabled(ctx);
>>> + gen_ld(ctx, op1, rt, rs, imm);
>>> + return;
>>> + case OPC_SWLE ... OPC_SWRE:
>>> + check_insn_opc_removed(ctx, ISA_MIPS32R6);
>>> + /* fall through */
>>> + case OPC_SBE ... OPC_SHE:
>>> + case OPC_SWE:
>>> + check_cp0_enabled(ctx);
>>> + gen_st(ctx, op1, rt, rs, imm);
>>> + return;
>>> + case OPC_SCE:
>>> + check_cp0_enabled(ctx);
>>> + gen_st_cond(ctx, op1, rt, rs, imm);
>>> + return;
>>> + case OPC_CACHEE:
>>> + check_cp0_enabled(ctx);
>>> + if (ctx->hflags & MIPS_HFLAG_ITC_CACHE) {
>>> + gen_cache_operation(ctx, rt, rs, imm);
>>> + }
>>> + /* Treat as NOP. */
>>
>> this comment is not relevant any more.
>
> Well its relevant in the sense that the CACHEE operation doesn't
> actually do anything of any significance. It won't even trigger a TLB
> exception (which technically it should).
>
> I can drop it though if you prefer.
>
Understood. That's fine. Let's leave it.
>>
>>> + return;
>>> + case OPC_PREFE:
>>> + check_cp0_enabled(ctx);
>>> + /* Treat as NOP. */
>>> + return;
>>> + }
>>> + }
>>> +
>>> switch (op1) {
>>> case OPC_EXT:
>>> case OPC_INS:
>>> @@ -19883,6 +19988,7 @@ void gen_intermediate_code(CPUMIPSState *env, struct TranslationBlock *tb)
>>> ctx.bp = (env->CP0_Config3 >> CP0C3_BP) & 1;
>>> ctx.PAMask = env->PAMask;
>>> ctx.mvh = (env->CP0_Config5 >> CP0C5_MVH) & 1;
>>> + ctx.eva = (env->CP0_Config5 >> CP0C5_EVA) & 1;
>>> ctx.CP0_LLAddr_shift = env->CP0_LLAddr_shift;
>>> ctx.cmgcr = (env->CP0_Config3 >> CP0C3_CMGCR) & 1;
>>> /* Restore delay slot state from the tb context. */
>>>
>>
>> Regards,
>> Yongbok
>
> Thanks for reviewing,
>
> Cheers
> James
>
^ permalink raw reply [flat|nested] 30+ messages in thread* Re: [Qemu-devel] [PATCH 3/9] target-mips: Decode EVA load & store instructions
2016-10-07 16:05 ` Yongbok Kim
@ 2016-10-07 16:16 ` James Hogan
0 siblings, 0 replies; 30+ messages in thread
From: James Hogan @ 2016-10-07 16:16 UTC (permalink / raw)
To: Yongbok Kim; +Cc: qemu-devel, Aurelien Jarno
[-- Attachment #1: Type: text/plain, Size: 2686 bytes --]
On Fri, Oct 07, 2016 at 05:05:30PM +0100, Yongbok Kim wrote:
> On 07/10/2016 16:48, James Hogan wrote:
> > On Fri, Oct 07, 2016 at 04:34:27PM +0100, Yongbok Kim wrote:
> >>> diff --git a/target-mips/translate.c b/target-mips/translate.c
> >>> index df2befbd5294..8506c39a359c 100644
> >>> --- a/target-mips/translate.c
> >>> +++ b/target-mips/translate.c
> >>> @@ -426,6 +426,24 @@ enum {
> >>> OPC_EXTR_W_DSP = 0x38 | OPC_SPECIAL3,
> >>> OPC_DEXTR_W_DSP = 0x3C | OPC_SPECIAL3,
> >>>
> >>> + /* EVA */
> >>> + OPC_LWLE = 0x19 | OPC_SPECIAL3,
> >>> + OPC_LWRE = 0x1A | OPC_SPECIAL3,
> >>> + OPC_CACHEE = 0x1B | OPC_SPECIAL3,
> >>> + OPC_SBE = 0x1C | OPC_SPECIAL3,
> >>> + OPC_SHE = 0x1D | OPC_SPECIAL3,
> >>> + OPC_SCE = 0x1E | OPC_SPECIAL3,
> >>> + OPC_SWE = 0x1F | OPC_SPECIAL3,
> >>> + OPC_SWLE = 0x21 | OPC_SPECIAL3,
> >>> + OPC_SWRE = 0x22 | OPC_SPECIAL3,
> >>> + OPC_PREFE = 0x23 | OPC_SPECIAL3,
> >>> + OPC_LBUE = 0x28 | OPC_SPECIAL3,
> >>> + OPC_LHUE = 0x29 | OPC_SPECIAL3,
> >>> + OPC_LBE = 0x2C | OPC_SPECIAL3,
> >>> + OPC_LHE = 0x2D | OPC_SPECIAL3,
> >>> + OPC_LLE = 0x2E | OPC_SPECIAL3,
> >>> + OPC_LWE = 0x2F | OPC_SPECIAL3,
> >>> +
> >>
> >> EVA for MIPS32 only. It is OK but it's worth mentioning it in the commit
> >> messages.
> >
> > Are you referring to the lack of sde/lde instructions? My understanding
> > is that these were never defined since EVA is aimed at 32-bit code
> > (although segmentation control can still be implemented on a MIPS64
> > core, e.g. P6600).
> >
>
> Okay. What about microMIPS version of these instructions?
True. Should be easy enough to add (though whether Linux has ever been
tested with microMIPS + EVA is a whole other question... it should work
I'd think even without the unaligned access emulation of the microMIPS
versions implemented).
> >>> @@ -17981,13 +18042,57 @@ static void decode_opc_special3(CPUMIPSState *env, DisasContext *ctx)
> >>> {
> >>> int rs, rt, rd, sa;
> >>> uint32_t op1, op2;
> >>> + int16_t imm;
> >>>
> >>> rs = (ctx->opcode >> 21) & 0x1f;
> >>> rt = (ctx->opcode >> 16) & 0x1f;
> >>> rd = (ctx->opcode >> 11) & 0x1f;
> >>> sa = (ctx->opcode >> 6) & 0x1f;
> >>> + imm = (int16_t)ctx->opcode >> 7;
> >>
> >> imm = (int16_t) (ctx->opcode >> 7) & 0x1ff;
> >
> > That won't sign extend it correctly.
>
> That's true :) Use sextract32() then?
Okay
Cheers
James
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply [flat|nested] 30+ messages in thread
* [Qemu-devel] [PATCH 4/9] target-mips: Check memory permissions with mem_idx
2016-09-06 11:03 [Qemu-devel] [PATCH 0/9] target-mips: Add Enhanced Virtual Addressing (EVA) support James Hogan
` (2 preceding siblings ...)
2016-09-06 11:03 ` [Qemu-devel] [PATCH 3/9] target-mips: Decode EVA load & store instructions James Hogan
@ 2016-09-06 11:03 ` James Hogan
2016-10-07 15:48 ` Yongbok Kim
2016-09-06 11:03 ` [Qemu-devel] [PATCH 5/9] target-mips: Abstract mmu_idx from hflags James Hogan
` (5 subsequent siblings)
9 siblings, 1 reply; 30+ messages in thread
From: James Hogan @ 2016-09-06 11:03 UTC (permalink / raw)
To: Leon Alrae; +Cc: qemu-devel, James Hogan, Aurelien Jarno
When performing virtual to physical address translation, check the
required privilege level based on the mem_idx rather than the mode in
the hflags. This will allow EVA loads & stores to operate safely only on
user memory from kernel mode.
For the cases where the mmu_idx doesn't need to be overridden
(mips_cpu_get_phys_page_debug() and cpu_mips_translate_address()), we
calculate the required mmu_idx using cpu_mmu_index(). Note that this
only tests the MIPS_HFLAG_KSU bits rather than MIPS_HFLAG_MODE, so we
don't test the debug mode hflag MIPS_HFLAG_DM any longer. This should be
fine as get_physical_address() only compares against MIPS_HFLAG_UM and
MIPS_HFLAG_SM, neither of which should get set by compute_hflags() when
MIPS_HFLAG_DM is set.
Signed-off-by: James Hogan <james.hogan@imgtec.com>
Cc: Leon Alrae <leon.alrae@imgtec.com>
Cc: Aurelien Jarno <aurelien@aurel32.net>
---
target-mips/helper.c | 17 +++++++++--------
1 file changed, 9 insertions(+), 8 deletions(-)
diff --git a/target-mips/helper.c b/target-mips/helper.c
index 29ebf391cb94..2065fc3ec119 100644
--- a/target-mips/helper.c
+++ b/target-mips/helper.c
@@ -109,11 +109,11 @@ int r4k_map_address (CPUMIPSState *env, hwaddr *physical, int *prot,
static int get_physical_address (CPUMIPSState *env, hwaddr *physical,
int *prot, target_ulong real_address,
- int rw, int access_type)
+ int rw, int access_type, int mmu_idx)
{
/* User mode can only access useg/xuseg */
- int user_mode = (env->hflags & MIPS_HFLAG_MODE) == MIPS_HFLAG_UM;
- int supervisor_mode = (env->hflags & MIPS_HFLAG_MODE) == MIPS_HFLAG_SM;
+ int user_mode = mmu_idx == MIPS_HFLAG_UM;
+ int supervisor_mode = mmu_idx == MIPS_HFLAG_SM;
int kernel_mode = !user_mode && !supervisor_mode;
#if defined(TARGET_MIPS64)
int UX = (env->CP0_Status & (1 << CP0St_UX)) != 0;
@@ -413,11 +413,12 @@ static void raise_mmu_exception(CPUMIPSState *env, target_ulong address,
hwaddr mips_cpu_get_phys_page_debug(CPUState *cs, vaddr addr)
{
MIPSCPU *cpu = MIPS_CPU(cs);
+ CPUMIPSState *env = &cpu->env;
hwaddr phys_addr;
int prot;
- if (get_physical_address(&cpu->env, &phys_addr, &prot, addr, 0,
- ACCESS_INT) != 0) {
+ if (get_physical_address(env, &phys_addr, &prot, addr, 0, ACCESS_INT,
+ cpu_mmu_index(env, false)) != 0) {
return -1;
}
return phys_addr;
@@ -449,7 +450,7 @@ int mips_cpu_handle_mmu_fault(CPUState *cs, vaddr address, int rw,
correctly */
access_type = ACCESS_INT;
ret = get_physical_address(env, &physical, &prot,
- address, rw, access_type);
+ address, rw, access_type, mmu_idx);
qemu_log_mask(CPU_LOG_MMU,
"%s address=%" VADDR_PRIx " ret %d physical " TARGET_FMT_plx
" prot %d\n",
@@ -479,8 +480,8 @@ hwaddr cpu_mips_translate_address(CPUMIPSState *env, target_ulong address, int r
/* data access */
access_type = ACCESS_INT;
- ret = get_physical_address(env, &physical, &prot,
- address, rw, access_type);
+ ret = get_physical_address(env, &physical, &prot, address, rw, access_type,
+ cpu_mmu_index(env, false));
if (ret != TLBRET_MATCH) {
raise_mmu_exception(env, address, rw, ret);
return -1LL;
--
git-series 0.8.10
^ permalink raw reply related [flat|nested] 30+ messages in thread* Re: [Qemu-devel] [PATCH 4/9] target-mips: Check memory permissions with mem_idx
2016-09-06 11:03 ` [Qemu-devel] [PATCH 4/9] target-mips: Check memory permissions with mem_idx James Hogan
@ 2016-10-07 15:48 ` Yongbok Kim
2017-07-06 20:50 ` James Hogan
0 siblings, 1 reply; 30+ messages in thread
From: Yongbok Kim @ 2016-10-07 15:48 UTC (permalink / raw)
To: James Hogan, qemu-devel; +Cc: Aurelien Jarno
On 06/09/2016 12:03, James Hogan wrote:
> When performing virtual to physical address translation, check the
> required privilege level based on the mem_idx rather than the mode in
> the hflags. This will allow EVA loads & stores to operate safely only on
> user memory from kernel mode.
>
> For the cases where the mmu_idx doesn't need to be overridden
> (mips_cpu_get_phys_page_debug() and cpu_mips_translate_address()), we
> calculate the required mmu_idx using cpu_mmu_index(). Note that this
> only tests the MIPS_HFLAG_KSU bits rather than MIPS_HFLAG_MODE, so we
> don't test the debug mode hflag MIPS_HFLAG_DM any longer. This should be
> fine as get_physical_address() only compares against MIPS_HFLAG_UM and
> MIPS_HFLAG_SM, neither of which should get set by compute_hflags() when
> MIPS_HFLAG_DM is set.
>
> Signed-off-by: James Hogan <james.hogan@imgtec.com>
> Cc: Leon Alrae <leon.alrae@imgtec.com>
> Cc: Aurelien Jarno <aurelien@aurel32.net>
> ---
> target-mips/helper.c | 17 +++++++++--------
> 1 file changed, 9 insertions(+), 8 deletions(-)
>
> diff --git a/target-mips/helper.c b/target-mips/helper.c
> index 29ebf391cb94..2065fc3ec119 100644
> --- a/target-mips/helper.c
> +++ b/target-mips/helper.c
> @@ -109,11 +109,11 @@ int r4k_map_address (CPUMIPSState *env, hwaddr *physical, int *prot,
>
> static int get_physical_address (CPUMIPSState *env, hwaddr *physical,
> int *prot, target_ulong real_address,
> - int rw, int access_type)
> + int rw, int access_type, int mmu_idx)
> {
> /* User mode can only access useg/xuseg */
> - int user_mode = (env->hflags & MIPS_HFLAG_MODE) == MIPS_HFLAG_UM;
> - int supervisor_mode = (env->hflags & MIPS_HFLAG_MODE) == MIPS_HFLAG_SM;
> + int user_mode = mmu_idx == MIPS_HFLAG_UM;
> + int supervisor_mode = mmu_idx == MIPS_HFLAG_SM;
> int kernel_mode = !user_mode && !supervisor_mode;
> #if defined(TARGET_MIPS64)
> int UX = (env->CP0_Status & (1 << CP0St_UX)) != 0;
> @@ -413,11 +413,12 @@ static void raise_mmu_exception(CPUMIPSState *env, target_ulong address,
> hwaddr mips_cpu_get_phys_page_debug(CPUState *cs, vaddr addr)
> {
> MIPSCPU *cpu = MIPS_CPU(cs);
> + CPUMIPSState *env = &cpu->env;
Not really useful change as it is used only once but it is ok.
> hwaddr phys_addr;
> int prot;
>
> - if (get_physical_address(&cpu->env, &phys_addr, &prot, addr, 0,
> - ACCESS_INT) != 0) {
> + if (get_physical_address(env, &phys_addr, &prot, addr, 0, ACCESS_INT,
> + cpu_mmu_index(env, false)) != 0) {
> return -1;
> }
> return phys_addr;
> @@ -449,7 +450,7 @@ int mips_cpu_handle_mmu_fault(CPUState *cs, vaddr address, int rw,
> correctly */
> access_type = ACCESS_INT;
> ret = get_physical_address(env, &physical, &prot,
> - address, rw, access_type);
> + address, rw, access_type, mmu_idx);
> qemu_log_mask(CPU_LOG_MMU,
> "%s address=%" VADDR_PRIx " ret %d physical " TARGET_FMT_plx
> " prot %d\n",
> @@ -479,8 +480,8 @@ hwaddr cpu_mips_translate_address(CPUMIPSState *env, target_ulong address, int r
>
> /* data access */
> access_type = ACCESS_INT;
> - ret = get_physical_address(env, &physical, &prot,
> - address, rw, access_type);
> + ret = get_physical_address(env, &physical, &prot, address, rw, access_type,
> + cpu_mmu_index(env, false));
> if (ret != TLBRET_MATCH) {
> raise_mmu_exception(env, address, rw, ret);
> return -1LL;
>
Otherwise,
Reviewed-by: Yongbok Kim <yongbok.kim@imgtec.com>
Regards,
Yongbok
^ permalink raw reply [flat|nested] 30+ messages in thread* Re: [Qemu-devel] [PATCH 4/9] target-mips: Check memory permissions with mem_idx
2016-10-07 15:48 ` Yongbok Kim
@ 2017-07-06 20:50 ` James Hogan
0 siblings, 0 replies; 30+ messages in thread
From: James Hogan @ 2017-07-06 20:50 UTC (permalink / raw)
To: Yongbok Kim; +Cc: qemu-devel, Aurelien Jarno
[-- Attachment #1: Type: text/plain, Size: 1124 bytes --]
On Fri, Oct 07, 2016 at 04:48:31PM +0100, Yongbok Kim wrote:
> On 06/09/2016 12:03, James Hogan wrote:
> > diff --git a/target-mips/helper.c b/target-mips/helper.c
> > index 29ebf391cb94..2065fc3ec119 100644
> > --- a/target-mips/helper.c
> > +++ b/target-mips/helper.c
> > @@ -413,11 +413,12 @@ static void raise_mmu_exception(CPUMIPSState *env, target_ulong address,
> > hwaddr mips_cpu_get_phys_page_debug(CPUState *cs, vaddr addr)
> > {
> > MIPSCPU *cpu = MIPS_CPU(cs);
> > + CPUMIPSState *env = &cpu->env;
>
> Not really useful change as it is used only once but it is ok.
Its used twice in the code below ...
>
> > hwaddr phys_addr;
> > int prot;
> >
> > - if (get_physical_address(&cpu->env, &phys_addr, &prot, addr, 0,
> > - ACCESS_INT) != 0) {
> > + if (get_physical_address(env, &phys_addr, &prot, addr, 0, ACCESS_INT,
> > + cpu_mmu_index(env, false)) != 0) {
... though I acknowledge it has marginal value
> Otherwise,
>
> Reviewed-by: Yongbok Kim <yongbok.kim@imgtec.com>
Thanks
James
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 30+ messages in thread
* [Qemu-devel] [PATCH 5/9] target-mips: Abstract mmu_idx from hflags
2016-09-06 11:03 [Qemu-devel] [PATCH 0/9] target-mips: Add Enhanced Virtual Addressing (EVA) support James Hogan
` (3 preceding siblings ...)
2016-09-06 11:03 ` [Qemu-devel] [PATCH 4/9] target-mips: Check memory permissions with mem_idx James Hogan
@ 2016-09-06 11:03 ` James Hogan
2016-10-07 16:08 ` Yongbok Kim
2016-09-06 11:03 ` [Qemu-devel] [PATCH 6/9] target-mips: Add an MMU mode for ERL James Hogan
` (4 subsequent siblings)
9 siblings, 1 reply; 30+ messages in thread
From: James Hogan @ 2016-09-06 11:03 UTC (permalink / raw)
To: Leon Alrae; +Cc: qemu-devel, James Hogan, Aurelien Jarno
The MIPS mmu_idx is sometimes calculated from hflags without an env
pointer available as cpu_mmu_index() requires.
Create a common hflags_mmu_index() for the purpose of this calculation
which can operate on any hflags, not just with an env pointer, and
update cpu_mmu_index() itself and gen_intermediate_code() to use it.
This will also allow the logic to be more easily updated when a new MMU
mode is added.
Signed-off-by: James Hogan <james.hogan@imgtec.com>
Cc: Leon Alrae <leon.alrae@imgtec.com>
Cc: Aurelien Jarno <aurelien@aurel32.net>
---
target-mips/cpu.h | 8 +++++++-
target-mips/translate.c | 2 +-
2 files changed, 8 insertions(+), 2 deletions(-)
diff --git a/target-mips/cpu.h b/target-mips/cpu.h
index 6ea2bf14c817..8ddc965e4735 100644
--- a/target-mips/cpu.h
+++ b/target-mips/cpu.h
@@ -695,9 +695,15 @@ extern uint32_t cpu_rddsp(uint32_t mask_num, CPUMIPSState *env);
#define MMU_MODE1_SUFFIX _super
#define MMU_MODE2_SUFFIX _user
#define MMU_USER_IDX 2
+
+static inline int hflags_mmu_index(uint32_t hflags)
+{
+ return hflags & MIPS_HFLAG_KSU;
+}
+
static inline int cpu_mmu_index (CPUMIPSState *env, bool ifetch)
{
- return env->hflags & MIPS_HFLAG_KSU;
+ return hflags_mmu_index(env->hflags);
}
static inline bool cpu_mips_hw_interrupts_enabled(CPUMIPSState *env)
diff --git a/target-mips/translate.c b/target-mips/translate.c
index 8506c39a359c..af17fc95eb8f 100644
--- a/target-mips/translate.c
+++ b/target-mips/translate.c
@@ -20004,7 +20004,7 @@ void gen_intermediate_code(CPUMIPSState *env, struct TranslationBlock *tb)
#ifdef CONFIG_USER_ONLY
ctx.mem_idx = MIPS_HFLAG_UM;
#else
- ctx.mem_idx = ctx.hflags & MIPS_HFLAG_KSU;
+ ctx.mem_idx = hflags_mmu_index(ctx.hflags);
#endif
ctx.default_tcg_memop_mask = (ctx.insn_flags & ISA_MIPS32R6) ?
MO_UNALN : MO_ALIGN;
--
git-series 0.8.10
^ permalink raw reply related [flat|nested] 30+ messages in thread* Re: [Qemu-devel] [PATCH 5/9] target-mips: Abstract mmu_idx from hflags
2016-09-06 11:03 ` [Qemu-devel] [PATCH 5/9] target-mips: Abstract mmu_idx from hflags James Hogan
@ 2016-10-07 16:08 ` Yongbok Kim
2017-07-06 20:55 ` James Hogan
0 siblings, 1 reply; 30+ messages in thread
From: Yongbok Kim @ 2016-10-07 16:08 UTC (permalink / raw)
To: James Hogan, qemu-devel; +Cc: Aurelien Jarno
On 06/09/2016 12:03, James Hogan wrote:
> The MIPS mmu_idx is sometimes calculated from hflags without an env
> pointer available as cpu_mmu_index() requires.
>
> Create a common hflags_mmu_index() for the purpose of this calculation
> which can operate on any hflags, not just with an env pointer, and
> update cpu_mmu_index() itself and gen_intermediate_code() to use it.
>
> This will also allow the logic to be more easily updated when a new MMU
> mode is added.
>
> Signed-off-by: James Hogan <james.hogan@imgtec.com>
> Cc: Leon Alrae <leon.alrae@imgtec.com>
> Cc: Aurelien Jarno <aurelien@aurel32.net>
> ---
> target-mips/cpu.h | 8 +++++++-
> target-mips/translate.c | 2 +-
> 2 files changed, 8 insertions(+), 2 deletions(-)
>
> diff --git a/target-mips/cpu.h b/target-mips/cpu.h
> index 6ea2bf14c817..8ddc965e4735 100644
> --- a/target-mips/cpu.h
> +++ b/target-mips/cpu.h
> @@ -695,9 +695,15 @@ extern uint32_t cpu_rddsp(uint32_t mask_num, CPUMIPSState *env);
> #define MMU_MODE1_SUFFIX _super
> #define MMU_MODE2_SUFFIX _user
> #define MMU_USER_IDX 2
> +
> +static inline int hflags_mmu_index(uint32_t hflags)
> +{
> + return hflags & MIPS_HFLAG_KSU;
> +}
> +
> static inline int cpu_mmu_index (CPUMIPSState *env, bool ifetch)
> {
> - return env->hflags & MIPS_HFLAG_KSU;
> + return hflags_mmu_index(env->hflags);
> }
>
> static inline bool cpu_mips_hw_interrupts_enabled(CPUMIPSState *env)
> diff --git a/target-mips/translate.c b/target-mips/translate.c
> index 8506c39a359c..af17fc95eb8f 100644
> --- a/target-mips/translate.c
> +++ b/target-mips/translate.c
> @@ -20004,7 +20004,7 @@ void gen_intermediate_code(CPUMIPSState *env, struct TranslationBlock *tb)
> #ifdef CONFIG_USER_ONLY
> ctx.mem_idx = MIPS_HFLAG_UM;
> #else
> - ctx.mem_idx = ctx.hflags & MIPS_HFLAG_KSU;
> + ctx.mem_idx = hflags_mmu_index(ctx.hflags);
> #endif
> ctx.default_tcg_memop_mask = (ctx.insn_flags & ISA_MIPS32R6) ?
> MO_UNALN : MO_ALIGN;
>
There are two other lines in the op_helper.c.
1466: switch (env->hflags & MIPS_HFLAG_KSU) {
2242: switch (env->hflags & MIPS_HFLAG_KSU) {
Using the function in the cpu.h also can be considered.
Otherwise,
Reviewed-by: Yongbok Kim <yongbok.kim@imgtec.com>
Regards,
Yongbok
^ permalink raw reply [flat|nested] 30+ messages in thread* Re: [Qemu-devel] [PATCH 5/9] target-mips: Abstract mmu_idx from hflags
2016-10-07 16:08 ` Yongbok Kim
@ 2017-07-06 20:55 ` James Hogan
0 siblings, 0 replies; 30+ messages in thread
From: James Hogan @ 2017-07-06 20:55 UTC (permalink / raw)
To: Yongbok Kim; +Cc: qemu-devel, Aurelien Jarno
[-- Attachment #1: Type: text/plain, Size: 2955 bytes --]
On Fri, Oct 07, 2016 at 05:08:46PM +0100, Yongbok Kim wrote:
> On 06/09/2016 12:03, James Hogan wrote:
> > The MIPS mmu_idx is sometimes calculated from hflags without an env
> > pointer available as cpu_mmu_index() requires.
> >
> > Create a common hflags_mmu_index() for the purpose of this calculation
> > which can operate on any hflags, not just with an env pointer, and
> > update cpu_mmu_index() itself and gen_intermediate_code() to use it.
> >
> > This will also allow the logic to be more easily updated when a new MMU
> > mode is added.
> >
> > Signed-off-by: James Hogan <james.hogan@imgtec.com>
> > Cc: Leon Alrae <leon.alrae@imgtec.com>
> > Cc: Aurelien Jarno <aurelien@aurel32.net>
> > ---
> > target-mips/cpu.h | 8 +++++++-
> > target-mips/translate.c | 2 +-
> > 2 files changed, 8 insertions(+), 2 deletions(-)
> >
> > diff --git a/target-mips/cpu.h b/target-mips/cpu.h
> > index 6ea2bf14c817..8ddc965e4735 100644
> > --- a/target-mips/cpu.h
> > +++ b/target-mips/cpu.h
> > @@ -695,9 +695,15 @@ extern uint32_t cpu_rddsp(uint32_t mask_num, CPUMIPSState *env);
> > #define MMU_MODE1_SUFFIX _super
> > #define MMU_MODE2_SUFFIX _user
> > #define MMU_USER_IDX 2
> > +
> > +static inline int hflags_mmu_index(uint32_t hflags)
> > +{
> > + return hflags & MIPS_HFLAG_KSU;
> > +}
> > +
> > static inline int cpu_mmu_index (CPUMIPSState *env, bool ifetch)
> > {
> > - return env->hflags & MIPS_HFLAG_KSU;
> > + return hflags_mmu_index(env->hflags);
> > }
> >
> > static inline bool cpu_mips_hw_interrupts_enabled(CPUMIPSState *env)
> > diff --git a/target-mips/translate.c b/target-mips/translate.c
> > index 8506c39a359c..af17fc95eb8f 100644
> > --- a/target-mips/translate.c
> > +++ b/target-mips/translate.c
> > @@ -20004,7 +20004,7 @@ void gen_intermediate_code(CPUMIPSState *env, struct TranslationBlock *tb)
> > #ifdef CONFIG_USER_ONLY
> > ctx.mem_idx = MIPS_HFLAG_UM;
> > #else
> > - ctx.mem_idx = ctx.hflags & MIPS_HFLAG_KSU;
> > + ctx.mem_idx = hflags_mmu_index(ctx.hflags);
> > #endif
> > ctx.default_tcg_memop_mask = (ctx.insn_flags & ISA_MIPS32R6) ?
> > MO_UNALN : MO_ALIGN;
> >
>
> There are two other lines in the op_helper.c.
> 1466: switch (env->hflags & MIPS_HFLAG_KSU) {
> 2242: switch (env->hflags & MIPS_HFLAG_KSU) {
>
> Using the function in the cpu.h also can be considered.
Yeh, I avoided changing these as it wasn't clear if they were interested
in just logging the mode from KSU, or specifically the mmu index (i.e.
to answer "what address space am I looking at?"), and it looked more
like the former given the reference to MIPS_HFLAG_{UM,SM,KM}
definitions.
I've now changed them both to effecitvely log the mmu index, changing
the MIPS_HFLAG_{UM,SM,KM} references to literal numbers as used
elsewhere when comparing MMU indicies.
Cheers
James
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 30+ messages in thread
* [Qemu-devel] [PATCH 6/9] target-mips: Add an MMU mode for ERL
2016-09-06 11:03 [Qemu-devel] [PATCH 0/9] target-mips: Add Enhanced Virtual Addressing (EVA) support James Hogan
` (4 preceding siblings ...)
2016-09-06 11:03 ` [Qemu-devel] [PATCH 5/9] target-mips: Abstract mmu_idx from hflags James Hogan
@ 2016-09-06 11:03 ` James Hogan
2016-10-13 13:18 ` Yongbok Kim
2016-09-06 11:03 ` [Qemu-devel] [PATCH 7/9] target-mips: Add segmentation control registers James Hogan
` (3 subsequent siblings)
9 siblings, 1 reply; 30+ messages in thread
From: James Hogan @ 2016-09-06 11:03 UTC (permalink / raw)
To: Leon Alrae; +Cc: qemu-devel, James Hogan, Aurelien Jarno
The segmentation control feature allows a legacy memory segment to
become unmapped uncached at error level (according to CP0_Status.ERL),
and in fact the user segment is already treated in this way by QEMU.
Add a new MMU mode for this state so that QEMU's mappings don't persist
between ERL=0 and ERL=1.
Signed-off-by: James Hogan <james.hogan@imgtec.com>
Cc: Leon Alrae <leon.alrae@imgtec.com>
Cc: Aurelien Jarno <aurelien@aurel32.net>
---
target-mips/cpu.h | 17 +++++++++++++----
target-mips/op_helper.c | 2 ++
2 files changed, 15 insertions(+), 4 deletions(-)
diff --git a/target-mips/cpu.h b/target-mips/cpu.h
index 8ddc965e4735..2abb330272d9 100644
--- a/target-mips/cpu.h
+++ b/target-mips/cpu.h
@@ -134,7 +134,7 @@ struct CPUMIPSFPUContext {
#define FP_UNIMPLEMENTED 32
};
-#define NB_MMU_MODES 3
+#define NB_MMU_MODES 4
#define TARGET_INSN_START_EXTRA_WORDS 2
typedef struct CPUMIPSMVPContext CPUMIPSMVPContext;
@@ -550,7 +550,7 @@ struct CPUMIPSState {
#define EXCP_INST_NOTAVAIL 0x2 /* No valid instruction word for BadInstr */
uint32_t hflags; /* CPU State */
/* TMASK defines different execution modes */
-#define MIPS_HFLAG_TMASK 0xF5807FF
+#define MIPS_HFLAG_TMASK 0x1F5807FF
#define MIPS_HFLAG_MODE 0x00007 /* execution modes */
/* The KSU flags must be the lowest bits in hflags. The flag order
must be the same as defined for CP0 Status. This allows to use
@@ -600,6 +600,7 @@ struct CPUMIPSState {
#define MIPS_HFLAG_FRE 0x2000000 /* FRE enabled */
#define MIPS_HFLAG_ELPA 0x4000000
#define MIPS_HFLAG_ITC_CACHE 0x8000000 /* CACHE instr. operates on ITC tag */
+#define MIPS_HFLAG_ERL 0x10000000 /* error level flag */
target_ulong btarget; /* Jump / branch target */
target_ulong bcond; /* Branch condition (if needed) */
@@ -694,11 +695,16 @@ extern uint32_t cpu_rddsp(uint32_t mask_num, CPUMIPSState *env);
#define MMU_MODE0_SUFFIX _kernel
#define MMU_MODE1_SUFFIX _super
#define MMU_MODE2_SUFFIX _user
+#define MMU_MODE3_SUFFIX _error
#define MMU_USER_IDX 2
static inline int hflags_mmu_index(uint32_t hflags)
{
- return hflags & MIPS_HFLAG_KSU;
+ if (hflags & MIPS_HFLAG_ERL) {
+ return 3; /* ERL */
+ } else {
+ return hflags & MIPS_HFLAG_KSU;
+ }
}
static inline int cpu_mmu_index (CPUMIPSState *env, bool ifetch)
@@ -966,7 +972,10 @@ static inline void compute_hflags(CPUMIPSState *env)
MIPS_HFLAG_F64 | MIPS_HFLAG_FPU | MIPS_HFLAG_KSU |
MIPS_HFLAG_AWRAP | MIPS_HFLAG_DSP | MIPS_HFLAG_DSPR2 |
MIPS_HFLAG_SBRI | MIPS_HFLAG_MSA | MIPS_HFLAG_FRE |
- MIPS_HFLAG_ELPA);
+ MIPS_HFLAG_ELPA | MIPS_HFLAG_ERL);
+ if (env->CP0_Status & (1 << CP0St_ERL)) {
+ env->hflags |= MIPS_HFLAG_ERL;
+ }
if (!(env->CP0_Status & (1 << CP0St_EXL)) &&
!(env->CP0_Status & (1 << CP0St_ERL)) &&
!(env->hflags & MIPS_HFLAG_DM)) {
diff --git a/target-mips/op_helper.c b/target-mips/op_helper.c
index 71ad16e41dd4..829ab0bc3cca 100644
--- a/target-mips/op_helper.c
+++ b/target-mips/op_helper.c
@@ -66,6 +66,7 @@ static inline type do_##name(CPUMIPSState *env, target_ulong addr, \
case 1: return (type) cpu_##insn##_super_ra(env, addr, retaddr); \
default: \
case 2: return (type) cpu_##insn##_user_ra(env, addr, retaddr); \
+ case 3: return (type) cpu_##insn##_error_ra(env, addr, retaddr); \
} \
}
#endif
@@ -93,6 +94,7 @@ static inline void do_##name(CPUMIPSState *env, target_ulong addr, \
case 1: cpu_##insn##_super_ra(env, addr, val, retaddr); break; \
default: \
case 2: cpu_##insn##_user_ra(env, addr, val, retaddr); break; \
+ case 3: cpu_##insn##_error_ra(env, addr, val, retaddr); break; \
} \
}
#endif
--
git-series 0.8.10
^ permalink raw reply related [flat|nested] 30+ messages in thread* Re: [Qemu-devel] [PATCH 6/9] target-mips: Add an MMU mode for ERL
2016-09-06 11:03 ` [Qemu-devel] [PATCH 6/9] target-mips: Add an MMU mode for ERL James Hogan
@ 2016-10-13 13:18 ` Yongbok Kim
2017-07-06 20:58 ` James Hogan
0 siblings, 1 reply; 30+ messages in thread
From: Yongbok Kim @ 2016-10-13 13:18 UTC (permalink / raw)
To: James Hogan, Leon Alrae; +Cc: qemu-devel, Aurelien Jarno
On 06/09/2016 12:03, James Hogan wrote:
> The segmentation control feature allows a legacy memory segment to
> become unmapped uncached at error level (according to CP0_Status.ERL),
> and in fact the user segment is already treated in this way by QEMU.
>
> Add a new MMU mode for this state so that QEMU's mappings don't persist
> between ERL=0 and ERL=1.
>
> Signed-off-by: James Hogan <james.hogan@imgtec.com>
> Cc: Leon Alrae <leon.alrae@imgtec.com>
> Cc: Aurelien Jarno <aurelien@aurel32.net>
> ---
> target-mips/cpu.h | 17 +++++++++++++----
> target-mips/op_helper.c | 2 ++
> 2 files changed, 15 insertions(+), 4 deletions(-)
>
> diff --git a/target-mips/cpu.h b/target-mips/cpu.h
> index 8ddc965e4735..2abb330272d9 100644
> --- a/target-mips/cpu.h
> +++ b/target-mips/cpu.h
> @@ -134,7 +134,7 @@ struct CPUMIPSFPUContext {
> #define FP_UNIMPLEMENTED 32
> };
>
> -#define NB_MMU_MODES 3
> +#define NB_MMU_MODES 4
> #define TARGET_INSN_START_EXTRA_WORDS 2
>
> typedef struct CPUMIPSMVPContext CPUMIPSMVPContext;
> @@ -550,7 +550,7 @@ struct CPUMIPSState {
> #define EXCP_INST_NOTAVAIL 0x2 /* No valid instruction word for BadInstr */
> uint32_t hflags; /* CPU State */
> /* TMASK defines different execution modes */
> -#define MIPS_HFLAG_TMASK 0xF5807FF
> +#define MIPS_HFLAG_TMASK 0x1F5807FF
> #define MIPS_HFLAG_MODE 0x00007 /* execution modes */
> /* The KSU flags must be the lowest bits in hflags. The flag order
> must be the same as defined for CP0 Status. This allows to use
> @@ -600,6 +600,7 @@ struct CPUMIPSState {
> #define MIPS_HFLAG_FRE 0x2000000 /* FRE enabled */
> #define MIPS_HFLAG_ELPA 0x4000000
> #define MIPS_HFLAG_ITC_CACHE 0x8000000 /* CACHE instr. operates on ITC tag */
> +#define MIPS_HFLAG_ERL 0x10000000 /* error level flag */
> target_ulong btarget; /* Jump / branch target */
> target_ulong bcond; /* Branch condition (if needed) */
>
> @@ -694,11 +695,16 @@ extern uint32_t cpu_rddsp(uint32_t mask_num, CPUMIPSState *env);
> #define MMU_MODE0_SUFFIX _kernel
> #define MMU_MODE1_SUFFIX _super
> #define MMU_MODE2_SUFFIX _user
> +#define MMU_MODE3_SUFFIX _error
> #define MMU_USER_IDX 2
>
> static inline int hflags_mmu_index(uint32_t hflags)
> {
> - return hflags & MIPS_HFLAG_KSU;
> + if (hflags & MIPS_HFLAG_ERL) {
> + return 3; /* ERL */
Not really useful but consider to macro it.
> + } else {
> + return hflags & MIPS_HFLAG_KSU;
> + }
> }
>
> static inline int cpu_mmu_index (CPUMIPSState *env, bool ifetch)
> @@ -966,7 +972,10 @@ static inline void compute_hflags(CPUMIPSState *env)
> MIPS_HFLAG_F64 | MIPS_HFLAG_FPU | MIPS_HFLAG_KSU |
> MIPS_HFLAG_AWRAP | MIPS_HFLAG_DSP | MIPS_HFLAG_DSPR2 |
> MIPS_HFLAG_SBRI | MIPS_HFLAG_MSA | MIPS_HFLAG_FRE |
> - MIPS_HFLAG_ELPA);
> + MIPS_HFLAG_ELPA | MIPS_HFLAG_ERL);
> + if (env->CP0_Status & (1 << CP0St_ERL)) {
> + env->hflags |= MIPS_HFLAG_ERL;
> + }
> if (!(env->CP0_Status & (1 << CP0St_EXL)) &&
> !(env->CP0_Status & (1 << CP0St_ERL)) &&
> !(env->hflags & MIPS_HFLAG_DM)) {
> diff --git a/target-mips/op_helper.c b/target-mips/op_helper.c
> index 71ad16e41dd4..829ab0bc3cca 100644
> --- a/target-mips/op_helper.c
> +++ b/target-mips/op_helper.c
> @@ -66,6 +66,7 @@ static inline type do_##name(CPUMIPSState *env, target_ulong addr, \
> case 1: return (type) cpu_##insn##_super_ra(env, addr, retaddr); \
> default: \
> case 2: return (type) cpu_##insn##_user_ra(env, addr, retaddr); \
> + case 3: return (type) cpu_##insn##_error_ra(env, addr, retaddr); \
> } \
> }
> #endif
> @@ -93,6 +94,7 @@ static inline void do_##name(CPUMIPSState *env, target_ulong addr, \
> case 1: cpu_##insn##_super_ra(env, addr, val, retaddr); break; \
> default: \
> case 2: cpu_##insn##_user_ra(env, addr, val, retaddr); break; \
> + case 3: cpu_##insn##_error_ra(env, addr, val, retaddr); break; \
> } \
> }
> #endif
>
Otherwise,
Reviewed-by: Yongbok Kim <yongbok.kim@imgtec.com>
Regards,
Yongbok
^ permalink raw reply [flat|nested] 30+ messages in thread* Re: [Qemu-devel] [PATCH 6/9] target-mips: Add an MMU mode for ERL
2016-10-13 13:18 ` Yongbok Kim
@ 2017-07-06 20:58 ` James Hogan
0 siblings, 0 replies; 30+ messages in thread
From: James Hogan @ 2017-07-06 20:58 UTC (permalink / raw)
To: Yongbok Kim; +Cc: Leon Alrae, qemu-devel, Aurelien Jarno
[-- Attachment #1: Type: text/plain, Size: 5055 bytes --]
On Thu, Oct 13, 2016 at 02:18:31PM +0100, Yongbok Kim wrote:
>
>
> On 06/09/2016 12:03, James Hogan wrote:
> > The segmentation control feature allows a legacy memory segment to
> > become unmapped uncached at error level (according to CP0_Status.ERL),
> > and in fact the user segment is already treated in this way by QEMU.
> >
> > Add a new MMU mode for this state so that QEMU's mappings don't persist
> > between ERL=0 and ERL=1.
> >
> > Signed-off-by: James Hogan <james.hogan@imgtec.com>
> > Cc: Leon Alrae <leon.alrae@imgtec.com>
> > Cc: Aurelien Jarno <aurelien@aurel32.net>
> > ---
> > target-mips/cpu.h | 17 +++++++++++++----
> > target-mips/op_helper.c | 2 ++
> > 2 files changed, 15 insertions(+), 4 deletions(-)
> >
> > diff --git a/target-mips/cpu.h b/target-mips/cpu.h
> > index 8ddc965e4735..2abb330272d9 100644
> > --- a/target-mips/cpu.h
> > +++ b/target-mips/cpu.h
> > @@ -134,7 +134,7 @@ struct CPUMIPSFPUContext {
> > #define FP_UNIMPLEMENTED 32
> > };
> >
> > -#define NB_MMU_MODES 3
> > +#define NB_MMU_MODES 4
> > #define TARGET_INSN_START_EXTRA_WORDS 2
> >
> > typedef struct CPUMIPSMVPContext CPUMIPSMVPContext;
> > @@ -550,7 +550,7 @@ struct CPUMIPSState {
> > #define EXCP_INST_NOTAVAIL 0x2 /* No valid instruction word for BadInstr */
> > uint32_t hflags; /* CPU State */
> > /* TMASK defines different execution modes */
> > -#define MIPS_HFLAG_TMASK 0xF5807FF
> > +#define MIPS_HFLAG_TMASK 0x1F5807FF
> > #define MIPS_HFLAG_MODE 0x00007 /* execution modes */
> > /* The KSU flags must be the lowest bits in hflags. The flag order
> > must be the same as defined for CP0 Status. This allows to use
> > @@ -600,6 +600,7 @@ struct CPUMIPSState {
> > #define MIPS_HFLAG_FRE 0x2000000 /* FRE enabled */
> > #define MIPS_HFLAG_ELPA 0x4000000
> > #define MIPS_HFLAG_ITC_CACHE 0x8000000 /* CACHE instr. operates on ITC tag */
> > +#define MIPS_HFLAG_ERL 0x10000000 /* error level flag */
> > target_ulong btarget; /* Jump / branch target */
> > target_ulong bcond; /* Branch condition (if needed) */
> >
> > @@ -694,11 +695,16 @@ extern uint32_t cpu_rddsp(uint32_t mask_num, CPUMIPSState *env);
> > #define MMU_MODE0_SUFFIX _kernel
> > #define MMU_MODE1_SUFFIX _super
> > #define MMU_MODE2_SUFFIX _user
> > +#define MMU_MODE3_SUFFIX _error
> > #define MMU_USER_IDX 2
> >
> > static inline int hflags_mmu_index(uint32_t hflags)
> > {
> > - return hflags & MIPS_HFLAG_KSU;
> > + if (hflags & MIPS_HFLAG_ERL) {
> > + return 3; /* ERL */
>
> Not really useful but consider to macro it.
Yeh, I mainly didn't do this as the other MMU indices don't have
definitions either (see switch of literal numbers below).
Cheers
James
>
> > + } else {
> > + return hflags & MIPS_HFLAG_KSU;
> > + }
> > }
> >
> > static inline int cpu_mmu_index (CPUMIPSState *env, bool ifetch)
> > @@ -966,7 +972,10 @@ static inline void compute_hflags(CPUMIPSState *env)
> > MIPS_HFLAG_F64 | MIPS_HFLAG_FPU | MIPS_HFLAG_KSU |
> > MIPS_HFLAG_AWRAP | MIPS_HFLAG_DSP | MIPS_HFLAG_DSPR2 |
> > MIPS_HFLAG_SBRI | MIPS_HFLAG_MSA | MIPS_HFLAG_FRE |
> > - MIPS_HFLAG_ELPA);
> > + MIPS_HFLAG_ELPA | MIPS_HFLAG_ERL);
> > + if (env->CP0_Status & (1 << CP0St_ERL)) {
> > + env->hflags |= MIPS_HFLAG_ERL;
> > + }
> > if (!(env->CP0_Status & (1 << CP0St_EXL)) &&
> > !(env->CP0_Status & (1 << CP0St_ERL)) &&
> > !(env->hflags & MIPS_HFLAG_DM)) {
> > diff --git a/target-mips/op_helper.c b/target-mips/op_helper.c
> > index 71ad16e41dd4..829ab0bc3cca 100644
> > --- a/target-mips/op_helper.c
> > +++ b/target-mips/op_helper.c
> > @@ -66,6 +66,7 @@ static inline type do_##name(CPUMIPSState *env, target_ulong addr, \
> > case 1: return (type) cpu_##insn##_super_ra(env, addr, retaddr); \
> > default: \
> > case 2: return (type) cpu_##insn##_user_ra(env, addr, retaddr); \
> > + case 3: return (type) cpu_##insn##_error_ra(env, addr, retaddr); \
> > } \
> > }
> > #endif
> > @@ -93,6 +94,7 @@ static inline void do_##name(CPUMIPSState *env, target_ulong addr, \
> > case 1: cpu_##insn##_super_ra(env, addr, val, retaddr); break; \
> > default: \
> > case 2: cpu_##insn##_user_ra(env, addr, val, retaddr); break; \
> > + case 3: cpu_##insn##_error_ra(env, addr, val, retaddr); break; \
> > } \
> > }
> > #endif
> >
>
> Otherwise,
>
> Reviewed-by: Yongbok Kim <yongbok.kim@imgtec.com>
>
> Regards,
> Yongbok
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 30+ messages in thread
* [Qemu-devel] [PATCH 7/9] target-mips: Add segmentation control registers
2016-09-06 11:03 [Qemu-devel] [PATCH 0/9] target-mips: Add Enhanced Virtual Addressing (EVA) support James Hogan
` (5 preceding siblings ...)
2016-09-06 11:03 ` [Qemu-devel] [PATCH 6/9] target-mips: Add an MMU mode for ERL James Hogan
@ 2016-09-06 11:03 ` James Hogan
2016-10-10 14:57 ` Yongbok Kim
2016-09-06 11:03 ` [Qemu-devel] [PATCH 8/9] target-mips: Implement segmentation control James Hogan
` (2 subsequent siblings)
9 siblings, 1 reply; 30+ messages in thread
From: James Hogan @ 2016-09-06 11:03 UTC (permalink / raw)
To: Leon Alrae; +Cc: qemu-devel, James Hogan, Aurelien Jarno
The optional segmentation control registers CP0_SegCtl0, CP0_SegCtl1 &
CP0_SegCtl2 control the behaviour and required privilege of the legacy
virtual memory segments.
Add them to the CP0 interface so they can be read and written when
CP0_Config3.SC=1, and initialise them to describe the standard legacy
layout so they can be used in future patches regardless of whether they
are exposed to the guest.
Signed-off-by: James Hogan <james.hogan@imgtec.com>
Cc: Leon Alrae <leon.alrae@imgtec.com>
Cc: Aurelien Jarno <aurelien@aurel32.net>
---
target-mips/cpu.h | 31 ++++++++++++++++-
target-mips/helper.h | 3 ++-
target-mips/machine.c | 7 ++--
target-mips/op_helper.c | 24 ++++++++++++-
target-mips/translate.c | 84 ++++++++++++++++++++++++++++++++++++++++++-
5 files changed, 147 insertions(+), 2 deletions(-)
diff --git a/target-mips/cpu.h b/target-mips/cpu.h
index 2abb330272d9..fd8292eaa9c3 100644
--- a/target-mips/cpu.h
+++ b/target-mips/cpu.h
@@ -306,6 +306,36 @@ struct CPUMIPSState {
#define CP0PG_XIE 30
#define CP0PG_ELPA 29
#define CP0PG_IEC 27
+ target_ulong CP0_SegCtl0;
+ target_ulong CP0_SegCtl1;
+ target_ulong CP0_SegCtl2;
+#define CP0SC_PA 9
+#define CP0SC_PA_MASK (0x7FULL << CP0SC_PA)
+#define CP0SC_PA_1GMASK (0x7EULL << CP0SC_PA)
+#define CP0SC_AM 4
+#define CP0SC_AM_MASK (0x7ULL << CP0SC_AM)
+#define CP0SC_AM_UK 0ULL
+#define CP0SC_AM_MK 1ULL
+#define CP0SC_AM_MSK 2ULL
+#define CP0SC_AM_MUSK 3ULL
+#define CP0SC_AM_MUSUK 4ULL
+#define CP0SC_AM_USK 5ULL
+#define CP0SC_AM_UUSK 7ULL
+#define CP0SC_EU 3
+#define CP0SC_EU_MASK (1ULL << CP0SC_EU)
+#define CP0SC_C 0
+#define CP0SC_C_MASK (0x7ULL << CP0SC_C)
+#define CP0SC_MASK (CP0SC_C_MASK | CP0SC_EU_MASK | CP0SC_AM_MASK | \
+ CP0SC_PA_MASK)
+#define CP0SC_1GMASK (CP0SC_C_MASK | CP0SC_EU_MASK | CP0SC_AM_MASK | \
+ CP0SC_PA_1GMASK)
+#define CP0SC0_MASK (CP0SC_MASK | (CP0SC_MASK << 16))
+#define CP0SC1_XAM 59
+#define CP0SC1_XAM_MASK (0x7ULL << CP0SC1_XAM)
+#define CP0SC1_MASK (CP0SC_MASK | (CP0SC_MASK << 16) | CP0SC1_XAM_MASK)
+#define CP0SC2_XR 56
+#define CP0SC2_XR_MASK (0xFFULL << CP0SC2_XR)
+#define CP0SC2_MASK (CP0SC_1GMASK | (CP0SC_1GMASK << 16) | CP0SC2_XR_MASK)
int32_t CP0_Wired;
int32_t CP0_SRSConf0_rw_bitmask;
int32_t CP0_SRSConf0;
@@ -449,6 +479,7 @@ struct CPUMIPSState {
#define CP0C3_MSAP 28
#define CP0C3_BP 27
#define CP0C3_BI 26
+#define CP0C3_SC 25
#define CP0C3_IPLW 21
#define CP0C3_MMAR 18
#define CP0C3_MCU 17
diff --git a/target-mips/helper.h b/target-mips/helper.h
index 666936c81bfe..961741370ad2 100644
--- a/target-mips/helper.h
+++ b/target-mips/helper.h
@@ -122,6 +122,9 @@ DEF_HELPER_2(mtc0_entrylo1, void, env, tl)
DEF_HELPER_2(mtc0_context, void, env, tl)
DEF_HELPER_2(mtc0_pagemask, void, env, tl)
DEF_HELPER_2(mtc0_pagegrain, void, env, tl)
+DEF_HELPER_2(mtc0_segctl0, void, env, tl)
+DEF_HELPER_2(mtc0_segctl1, void, env, tl)
+DEF_HELPER_2(mtc0_segctl2, void, env, tl)
DEF_HELPER_2(mtc0_wired, void, env, tl)
DEF_HELPER_2(mtc0_srsconf0, void, env, tl)
DEF_HELPER_2(mtc0_srsconf1, void, env, tl)
diff --git a/target-mips/machine.c b/target-mips/machine.c
index e795fecaa0d6..d90f217c3fe5 100644
--- a/target-mips/machine.c
+++ b/target-mips/machine.c
@@ -206,8 +206,8 @@ const VMStateDescription vmstate_tlb = {
const VMStateDescription vmstate_mips_cpu = {
.name = "cpu",
- .version_id = 9,
- .minimum_version_id = 9,
+ .version_id = 10,
+ .minimum_version_id = 10,
.post_load = cpu_post_load,
.fields = (VMStateField[]) {
/* Active TC */
@@ -247,6 +247,9 @@ const VMStateDescription vmstate_mips_cpu = {
VMSTATE_UINTTL(env.CP0_Context, MIPSCPU),
VMSTATE_INT32(env.CP0_PageMask, MIPSCPU),
VMSTATE_INT32(env.CP0_PageGrain, MIPSCPU),
+ VMSTATE_UINTTL(env.CP0_SegCtl0, MIPSCPU),
+ VMSTATE_UINTTL(env.CP0_SegCtl1, MIPSCPU),
+ VMSTATE_UINTTL(env.CP0_SegCtl2, MIPSCPU),
VMSTATE_INT32(env.CP0_Wired, MIPSCPU),
VMSTATE_INT32(env.CP0_SRSConf0, MIPSCPU),
VMSTATE_INT32(env.CP0_SRSConf1, MIPSCPU),
diff --git a/target-mips/op_helper.c b/target-mips/op_helper.c
index 829ab0bc3cca..983de785b318 100644
--- a/target-mips/op_helper.c
+++ b/target-mips/op_helper.c
@@ -1337,6 +1337,30 @@ void helper_mtc0_pagegrain(CPUMIPSState *env, target_ulong arg1)
restore_pamask(env);
}
+void helper_mtc0_segctl0(CPUMIPSState *env, target_ulong arg1)
+{
+ CPUState *cs = CPU(mips_env_get_cpu(env));
+
+ env->CP0_SegCtl0 = arg1 & CP0SC0_MASK;
+ tlb_flush(cs, 1);
+}
+
+void helper_mtc0_segctl1(CPUMIPSState *env, target_ulong arg1)
+{
+ CPUState *cs = CPU(mips_env_get_cpu(env));
+
+ env->CP0_SegCtl1 = arg1 & CP0SC1_MASK;
+ tlb_flush(cs, 1);
+}
+
+void helper_mtc0_segctl2(CPUMIPSState *env, target_ulong arg1)
+{
+ CPUState *cs = CPU(mips_env_get_cpu(env));
+
+ env->CP0_SegCtl2 = arg1 & CP0SC2_MASK;
+ tlb_flush(cs, 1);
+}
+
void helper_mtc0_wired(CPUMIPSState *env, target_ulong arg1)
{
if (env->insn_flags & ISA_MIPS32R6) {
diff --git a/target-mips/translate.c b/target-mips/translate.c
index af17fc95eb8f..28d93bb56cd0 100644
--- a/target-mips/translate.c
+++ b/target-mips/translate.c
@@ -1449,6 +1449,7 @@ typedef struct DisasContext {
uint64_t PAMask;
bool mvh;
bool eva;
+ bool sc;
int CP0_LLAddr_shift;
bool ps;
bool vp;
@@ -5216,6 +5217,21 @@ static void gen_mfc0(DisasContext *ctx, TCGv arg, int reg, int sel)
gen_mfc0_load32(arg, offsetof(CPUMIPSState, CP0_PageGrain));
rn = "PageGrain";
break;
+ case 2:
+ CP0_CHECK(ctx->sc);
+ tcg_gen_ld32s_tl(arg, cpu_env, offsetof(CPUMIPSState, CP0_SegCtl0));
+ rn = "SegCtl0";
+ break;
+ case 3:
+ CP0_CHECK(ctx->sc);
+ tcg_gen_ld32s_tl(arg, cpu_env, offsetof(CPUMIPSState, CP0_SegCtl1));
+ rn = "SegCtl1";
+ break;
+ case 4:
+ CP0_CHECK(ctx->sc);
+ tcg_gen_ld32s_tl(arg, cpu_env, offsetof(CPUMIPSState, CP0_SegCtl2));
+ rn = "SegCtl2";
+ break;
default:
goto cp0_unimplemented;
}
@@ -5872,6 +5888,21 @@ static void gen_mtc0(DisasContext *ctx, TCGv arg, int reg, int sel)
rn = "PageGrain";
ctx->bstate = BS_STOP;
break;
+ case 2:
+ CP0_CHECK(ctx->sc);
+ gen_helper_mtc0_segctl0(cpu_env, arg);
+ rn = "SegCtl0";
+ break;
+ case 3:
+ CP0_CHECK(ctx->sc);
+ gen_helper_mtc0_segctl1(cpu_env, arg);
+ rn = "SegCtl1";
+ break;
+ case 4:
+ CP0_CHECK(ctx->sc);
+ gen_helper_mtc0_segctl2(cpu_env, arg);
+ rn = "SegCtl2";
+ break;
default:
goto cp0_unimplemented;
}
@@ -6534,6 +6565,20 @@ static void gen_dmfc0(DisasContext *ctx, TCGv arg, int reg, int sel)
gen_mfc0_load32(arg, offsetof(CPUMIPSState, CP0_PageGrain));
rn = "PageGrain";
break;
+ case 2:
+ CP0_CHECK(ctx->sc);
+ tcg_gen_ld_tl(arg, cpu_env, offsetof(CPUMIPSState, CP0_SegCtl0));
+ rn = "SegCtl0";
+ break;
+ case 3:
+ CP0_CHECK(ctx->sc);
+ tcg_gen_ld_tl(arg, cpu_env, offsetof(CPUMIPSState, CP0_SegCtl1));
+ rn = "SegCtl1";
+ break;
+ case 4:
+ CP0_CHECK(ctx->sc);
+ tcg_gen_ld_tl(arg, cpu_env, offsetof(CPUMIPSState, CP0_SegCtl2));
+ rn = "SegCtl2";
default:
goto cp0_unimplemented;
}
@@ -7172,6 +7217,21 @@ static void gen_dmtc0(DisasContext *ctx, TCGv arg, int reg, int sel)
gen_helper_mtc0_pagegrain(cpu_env, arg);
rn = "PageGrain";
break;
+ case 2:
+ CP0_CHECK(ctx->sc);
+ gen_helper_mtc0_segctl0(cpu_env, arg);
+ rn = "SegCtl0";
+ break;
+ case 3:
+ CP0_CHECK(ctx->sc);
+ gen_helper_mtc0_segctl1(cpu_env, arg);
+ rn = "SegCtl1";
+ break;
+ case 4:
+ CP0_CHECK(ctx->sc);
+ gen_helper_mtc0_segctl2(cpu_env, arg);
+ rn = "SegCtl2";
+ break;
default:
goto cp0_unimplemented;
}
@@ -19989,6 +20049,7 @@ void gen_intermediate_code(CPUMIPSState *env, struct TranslationBlock *tb)
ctx.PAMask = env->PAMask;
ctx.mvh = (env->CP0_Config5 >> CP0C5_MVH) & 1;
ctx.eva = (env->CP0_Config5 >> CP0C5_EVA) & 1;
+ ctx.sc = (env->CP0_Config3 >> CP0C3_SC) & 1;
ctx.CP0_LLAddr_shift = env->CP0_LLAddr_shift;
ctx.cmgcr = (env->CP0_Config3 >> CP0C3_CMGCR) & 1;
/* Restore delay slot state from the tb context. */
@@ -20463,6 +20524,29 @@ void cpu_state_reset(CPUMIPSState *env)
env->tcs[0].CP0_TCStatus = (1 << CP0TCSt_A);
}
}
+
+ /*
+ * Configure default legacy segmentation control. We use this regardless of
+ * whether segmentation control is presented to the guest.
+ */
+ /* KSeg3 (seg0 0xE0000000..0xFFFFFFFF) */
+ env->CP0_SegCtl0 = (CP0SC_AM_MK << CP0SC_AM);
+ /* KSeg2 (seg1 0xC0000000..0xDFFFFFFF) */
+ env->CP0_SegCtl0 |= ((CP0SC_AM_MSK << CP0SC_AM)) << 16;
+ /* KSeg1 (seg2 0xA0000000..0x9FFFFFFF) */
+ env->CP0_SegCtl1 = (0 << CP0SC_PA) | (CP0SC_AM_UK << CP0SC_AM) |
+ (2 << CP0SC_C);
+ /* KSeg0 (seg3 0x80000000..0x9FFFFFFF) */
+ env->CP0_SegCtl1 |= ((0 << CP0SC_PA) | (CP0SC_AM_UK << CP0SC_AM) |
+ (3 << CP0SC_C)) << 16;
+ /* USeg (seg4 0x40000000..0x7FFFFFFF) */
+ env->CP0_SegCtl2 = (2 << CP0SC_PA) | (CP0SC_AM_MUSK << CP0SC_AM) |
+ (1 << CP0SC_EU) | (2 << CP0SC_C);
+ /* USeg (seg5 0x00000000..0x3FFFFFFF) */
+ env->CP0_SegCtl2 |= ((0 << CP0SC_PA) | (CP0SC_AM_MUSK << CP0SC_AM) |
+ (1 << CP0SC_EU) | (2 << CP0SC_C)) << 16;
+ /* XKPhys (note, SegCtl2.XR = 0, so XAM won't be used) */
+ env->CP0_SegCtl1 |= (CP0SC_AM_UK << CP0SC1_XAM);
#endif
if ((env->insn_flags & ISA_MIPS32R6) &&
(env->active_fpu.fcr0 & (1 << FCR0_F64))) {
--
git-series 0.8.10
^ permalink raw reply related [flat|nested] 30+ messages in thread* Re: [Qemu-devel] [PATCH 7/9] target-mips: Add segmentation control registers
2016-09-06 11:03 ` [Qemu-devel] [PATCH 7/9] target-mips: Add segmentation control registers James Hogan
@ 2016-10-10 14:57 ` Yongbok Kim
2017-07-06 22:05 ` James Hogan
0 siblings, 1 reply; 30+ messages in thread
From: Yongbok Kim @ 2016-10-10 14:57 UTC (permalink / raw)
To: James Hogan, qemu-devel@nongnu.org; +Cc: Aurelien Jarno
On 06/09/2016 12:03, James Hogan wrote:
> The optional segmentation control registers CP0_SegCtl0, CP0_SegCtl1 &
> CP0_SegCtl2 control the behaviour and required privilege of the legacy
> virtual memory segments.
>
> Add them to the CP0 interface so they can be read and written when
> CP0_Config3.SC=1, and initialise them to describe the standard legacy
> layout so they can be used in future patches regardless of whether they
> are exposed to the guest.
>
> Signed-off-by: James Hogan <james.hogan@imgtec.com>
> Cc: Leon Alrae <leon.alrae@imgtec.com>
> Cc: Aurelien Jarno <aurelien@aurel32.net>
> ---
> target-mips/cpu.h | 31 ++++++++++++++++-
> target-mips/helper.h | 3 ++-
> target-mips/machine.c | 7 ++--
> target-mips/op_helper.c | 24 ++++++++++++-
> target-mips/translate.c | 84 ++++++++++++++++++++++++++++++++++++++++++-
> 5 files changed, 147 insertions(+), 2 deletions(-)
>
> diff --git a/target-mips/cpu.h b/target-mips/cpu.h
> index 2abb330272d9..fd8292eaa9c3 100644
> --- a/target-mips/cpu.h
> +++ b/target-mips/cpu.h
> @@ -306,6 +306,36 @@ struct CPUMIPSState {
> #define CP0PG_XIE 30
> #define CP0PG_ELPA 29
> #define CP0PG_IEC 27
> + target_ulong CP0_SegCtl0;
> + target_ulong CP0_SegCtl1;
> + target_ulong CP0_SegCtl2;
> +#define CP0SC_PA 9
> +#define CP0SC_PA_MASK (0x7FULL << CP0SC_PA)
> +#define CP0SC_PA_1GMASK (0x7EULL << CP0SC_PA)
> +#define CP0SC_AM 4
> +#define CP0SC_AM_MASK (0x7ULL << CP0SC_AM)
> +#define CP0SC_AM_UK 0ULL
> +#define CP0SC_AM_MK 1ULL
> +#define CP0SC_AM_MSK 2ULL
> +#define CP0SC_AM_MUSK 3ULL
> +#define CP0SC_AM_MUSUK 4ULL
> +#define CP0SC_AM_USK 5ULL
> +#define CP0SC_AM_UUSK 7ULL
> +#define CP0SC_EU 3
> +#define CP0SC_EU_MASK (1ULL << CP0SC_EU)
> +#define CP0SC_C 0
> +#define CP0SC_C_MASK (0x7ULL << CP0SC_C)
> +#define CP0SC_MASK (CP0SC_C_MASK | CP0SC_EU_MASK | CP0SC_AM_MASK | \
> + CP0SC_PA_MASK)
> +#define CP0SC_1GMASK (CP0SC_C_MASK | CP0SC_EU_MASK | CP0SC_AM_MASK | \
> + CP0SC_PA_1GMASK)
> +#define CP0SC0_MASK (CP0SC_MASK | (CP0SC_MASK << 16))
Defining 16 as a macro would be preferable.
> +#define CP0SC1_XAM 59
> +#define CP0SC1_XAM_MASK (0x7ULL << CP0SC1_XAM)
> +#define CP0SC1_MASK (CP0SC_MASK | (CP0SC_MASK << 16) | CP0SC1_XAM_MASK)
> +#define CP0SC2_XR 56
> +#define CP0SC2_XR_MASK (0xFFULL << CP0SC2_XR)
> +#define CP0SC2_MASK (CP0SC_1GMASK | (CP0SC_1GMASK << 16) | CP0SC2_XR_MASK)
> int32_t CP0_Wired;
> int32_t CP0_SRSConf0_rw_bitmask;
> int32_t CP0_SRSConf0;
> @@ -449,6 +479,7 @@ struct CPUMIPSState {
> #define CP0C3_MSAP 28
> #define CP0C3_BP 27
> #define CP0C3_BI 26
> +#define CP0C3_SC 25
> #define CP0C3_IPLW 21
> #define CP0C3_MMAR 18
> #define CP0C3_MCU 17
> diff --git a/target-mips/helper.h b/target-mips/helper.h
> index 666936c81bfe..961741370ad2 100644
> --- a/target-mips/helper.h
> +++ b/target-mips/helper.h
> @@ -122,6 +122,9 @@ DEF_HELPER_2(mtc0_entrylo1, void, env, tl)
> DEF_HELPER_2(mtc0_context, void, env, tl)
> DEF_HELPER_2(mtc0_pagemask, void, env, tl)
> DEF_HELPER_2(mtc0_pagegrain, void, env, tl)
> +DEF_HELPER_2(mtc0_segctl0, void, env, tl)
> +DEF_HELPER_2(mtc0_segctl1, void, env, tl)
> +DEF_HELPER_2(mtc0_segctl2, void, env, tl)
> DEF_HELPER_2(mtc0_wired, void, env, tl)
> DEF_HELPER_2(mtc0_srsconf0, void, env, tl)
> DEF_HELPER_2(mtc0_srsconf1, void, env, tl)
> diff --git a/target-mips/machine.c b/target-mips/machine.c
> index e795fecaa0d6..d90f217c3fe5 100644
> --- a/target-mips/machine.c
> +++ b/target-mips/machine.c
> @@ -206,8 +206,8 @@ const VMStateDescription vmstate_tlb = {
>
> const VMStateDescription vmstate_mips_cpu = {
> .name = "cpu",
> - .version_id = 9,
> - .minimum_version_id = 9,
> + .version_id = 10,
> + .minimum_version_id = 10,
> .post_load = cpu_post_load,
> .fields = (VMStateField[]) {
> /* Active TC */
> @@ -247,6 +247,9 @@ const VMStateDescription vmstate_mips_cpu = {
> VMSTATE_UINTTL(env.CP0_Context, MIPSCPU),
> VMSTATE_INT32(env.CP0_PageMask, MIPSCPU),
> VMSTATE_INT32(env.CP0_PageGrain, MIPSCPU),
> + VMSTATE_UINTTL(env.CP0_SegCtl0, MIPSCPU),
> + VMSTATE_UINTTL(env.CP0_SegCtl1, MIPSCPU),
> + VMSTATE_UINTTL(env.CP0_SegCtl2, MIPSCPU),
> VMSTATE_INT32(env.CP0_Wired, MIPSCPU),
> VMSTATE_INT32(env.CP0_SRSConf0, MIPSCPU),
> VMSTATE_INT32(env.CP0_SRSConf1, MIPSCPU),
> diff --git a/target-mips/op_helper.c b/target-mips/op_helper.c
> index 829ab0bc3cca..983de785b318 100644
> --- a/target-mips/op_helper.c
> +++ b/target-mips/op_helper.c
> @@ -1337,6 +1337,30 @@ void helper_mtc0_pagegrain(CPUMIPSState *env, target_ulong arg1)
> restore_pamask(env);
> }
>
> +void helper_mtc0_segctl0(CPUMIPSState *env, target_ulong arg1)
> +{
> + CPUState *cs = CPU(mips_env_get_cpu(env));
> +
> + env->CP0_SegCtl0 = arg1 & CP0SC0_MASK;
Writes of unsupported values to the C field require to leave the field
unmodified in the R6. Permit 2 (Uncached) only?
> + tlb_flush(cs, 1);
cpu_mips_tlb_flush()?
> +}
> +
> +void helper_mtc0_segctl1(CPUMIPSState *env, target_ulong arg1)
> +{
> + CPUState *cs = CPU(mips_env_get_cpu(env));
> +
> + env->CP0_SegCtl1 = arg1 & CP0SC1_MASK;
> + tlb_flush(cs, 1);
> +}
> +
> +void helper_mtc0_segctl2(CPUMIPSState *env, target_ulong arg1)
> +{
> + CPUState *cs = CPU(mips_env_get_cpu(env));
> +
> + env->CP0_SegCtl2 = arg1 & CP0SC2_MASK;
> + tlb_flush(cs, 1);
> +}
> +
> void helper_mtc0_wired(CPUMIPSState *env, target_ulong arg1)
> {
> if (env->insn_flags & ISA_MIPS32R6) {
> diff --git a/target-mips/translate.c b/target-mips/translate.c
> index af17fc95eb8f..28d93bb56cd0 100644
> --- a/target-mips/translate.c
> +++ b/target-mips/translate.c
> @@ -1449,6 +1449,7 @@ typedef struct DisasContext {
> uint64_t PAMask;
> bool mvh;
> bool eva;
> + bool sc;
> int CP0_LLAddr_shift;
> bool ps;
> bool vp;
> @@ -5216,6 +5217,21 @@ static void gen_mfc0(DisasContext *ctx, TCGv arg, int reg, int sel)
> gen_mfc0_load32(arg, offsetof(CPUMIPSState, CP0_PageGrain));
> rn = "PageGrain";
> break;
> + case 2:
> + CP0_CHECK(ctx->sc);
> + tcg_gen_ld32s_tl(arg, cpu_env, offsetof(CPUMIPSState, CP0_SegCtl0));
This might cause problem in 64-bit guest with big-endian host. Replace it
with tcg_gen_ld_tl() and tcg_gen_ext32s_tl().
> + rn = "SegCtl0";
> + break;
> + case 3:
> + CP0_CHECK(ctx->sc);
> + tcg_gen_ld32s_tl(arg, cpu_env, offsetof(CPUMIPSState, CP0_SegCtl1));
same as above
> + rn = "SegCtl1";
> + break;
> + case 4:
> + CP0_CHECK(ctx->sc);
> + tcg_gen_ld32s_tl(arg, cpu_env, offsetof(CPUMIPSState, CP0_SegCtl2));
and here.
> + rn = "SegCtl2";
> + break;
> default:
> goto cp0_unimplemented;
> }
> @@ -5872,6 +5888,21 @@ static void gen_mtc0(DisasContext *ctx, TCGv arg, int reg, int sel)
> rn = "PageGrain";
> ctx->bstate = BS_STOP;
> break;
> + case 2:
> + CP0_CHECK(ctx->sc);
> + gen_helper_mtc0_segctl0(cpu_env, arg);
> + rn = "SegCtl0";
> + break;
> + case 3:
> + CP0_CHECK(ctx->sc);
> + gen_helper_mtc0_segctl1(cpu_env, arg);
> + rn = "SegCtl1";
> + break;
> + case 4:
> + CP0_CHECK(ctx->sc);
> + gen_helper_mtc0_segctl2(cpu_env, arg);
> + rn = "SegCtl2";
> + break;
> default:
> goto cp0_unimplemented;
> }
> @@ -6534,6 +6565,20 @@ static void gen_dmfc0(DisasContext *ctx, TCGv arg, int reg, int sel)
> gen_mfc0_load32(arg, offsetof(CPUMIPSState, CP0_PageGrain));
> rn = "PageGrain";
> break;
> + case 2:
> + CP0_CHECK(ctx->sc);
> + tcg_gen_ld_tl(arg, cpu_env, offsetof(CPUMIPSState, CP0_SegCtl0));
> + rn = "SegCtl0";
> + break;
> + case 3:
> + CP0_CHECK(ctx->sc);
> + tcg_gen_ld_tl(arg, cpu_env, offsetof(CPUMIPSState, CP0_SegCtl1));
> + rn = "SegCtl1";
> + break;
> + case 4:
> + CP0_CHECK(ctx->sc);
> + tcg_gen_ld_tl(arg, cpu_env, offsetof(CPUMIPSState, CP0_SegCtl2));
> + rn = "SegCtl2";
> default:
> goto cp0_unimplemented;
> }
> @@ -7172,6 +7217,21 @@ static void gen_dmtc0(DisasContext *ctx, TCGv arg, int reg, int sel)
> gen_helper_mtc0_pagegrain(cpu_env, arg);
> rn = "PageGrain";
> break;
> + case 2:
> + CP0_CHECK(ctx->sc);
> + gen_helper_mtc0_segctl0(cpu_env, arg);
> + rn = "SegCtl0";
> + break;
> + case 3:
> + CP0_CHECK(ctx->sc);
> + gen_helper_mtc0_segctl1(cpu_env, arg);
> + rn = "SegCtl1";
> + break;
> + case 4:
> + CP0_CHECK(ctx->sc);
> + gen_helper_mtc0_segctl2(cpu_env, arg);
> + rn = "SegCtl2";
> + break;
> default:
> goto cp0_unimplemented;
> }
> @@ -19989,6 +20049,7 @@ void gen_intermediate_code(CPUMIPSState *env, struct TranslationBlock *tb)
> ctx.PAMask = env->PAMask;
> ctx.mvh = (env->CP0_Config5 >> CP0C5_MVH) & 1;
> ctx.eva = (env->CP0_Config5 >> CP0C5_EVA) & 1;
> + ctx.sc = (env->CP0_Config3 >> CP0C3_SC) & 1;
> ctx.CP0_LLAddr_shift = env->CP0_LLAddr_shift;
> ctx.cmgcr = (env->CP0_Config3 >> CP0C3_CMGCR) & 1;
> /* Restore delay slot state from the tb context. */
> @@ -20463,6 +20524,29 @@ void cpu_state_reset(CPUMIPSState *env)
> env->tcs[0].CP0_TCStatus = (1 << CP0TCSt_A);
> }
> }
> +
> + /*
> + * Configure default legacy segmentation control. We use this regardless of
> + * whether segmentation control is presented to the guest.
> + */
> + /* KSeg3 (seg0 0xE0000000..0xFFFFFFFF) */
> + env->CP0_SegCtl0 = (CP0SC_AM_MK << CP0SC_AM);
> + /* KSeg2 (seg1 0xC0000000..0xDFFFFFFF) */
> + env->CP0_SegCtl0 |= ((CP0SC_AM_MSK << CP0SC_AM)) << 16;
> + /* KSeg1 (seg2 0xA0000000..0x9FFFFFFF) */
> + env->CP0_SegCtl1 = (0 << CP0SC_PA) | (CP0SC_AM_UK << CP0SC_AM) |
> + (2 << CP0SC_C);
> + /* KSeg0 (seg3 0x80000000..0x9FFFFFFF) */
> + env->CP0_SegCtl1 |= ((0 << CP0SC_PA) | (CP0SC_AM_UK << CP0SC_AM) |
> + (3 << CP0SC_C)) << 16;
> + /* USeg (seg4 0x40000000..0x7FFFFFFF) */
> + env->CP0_SegCtl2 = (2 << CP0SC_PA) | (CP0SC_AM_MUSK << CP0SC_AM) |
> + (1 << CP0SC_EU) | (2 << CP0SC_C);
Reset value of C is Undefined in the spec?
> + /* USeg (seg5 0x00000000..0x3FFFFFFF) */
> + env->CP0_SegCtl2 |= ((0 << CP0SC_PA) | (CP0SC_AM_MUSK << CP0SC_AM) |
> + (1 << CP0SC_EU) | (2 << CP0SC_C)) << 16;
Same here
> + /* XKPhys (note, SegCtl2.XR = 0, so XAM won't be used) */
> + env->CP0_SegCtl1 |= (CP0SC_AM_UK << CP0SC1_XAM);
SegCtl1.XAM is undefined too?
> #endif
> if ((env->insn_flags & ISA_MIPS32R6) &&
> (env->active_fpu.fcr0 & (1 << FCR0_F64))) {
>
Regards,
Yongbok
^ permalink raw reply [flat|nested] 30+ messages in thread* Re: [Qemu-devel] [PATCH 7/9] target-mips: Add segmentation control registers
2016-10-10 14:57 ` Yongbok Kim
@ 2017-07-06 22:05 ` James Hogan
0 siblings, 0 replies; 30+ messages in thread
From: James Hogan @ 2017-07-06 22:05 UTC (permalink / raw)
To: Yongbok Kim; +Cc: qemu-devel@nongnu.org, Aurelien Jarno
[-- Attachment #1: Type: text/plain, Size: 14832 bytes --]
On Mon, Oct 10, 2016 at 03:57:08PM +0100, Yongbok Kim wrote:
>
>
> On 06/09/2016 12:03, James Hogan wrote:
> > The optional segmentation control registers CP0_SegCtl0, CP0_SegCtl1 &
> > CP0_SegCtl2 control the behaviour and required privilege of the legacy
> > virtual memory segments.
> >
> > Add them to the CP0 interface so they can be read and written when
> > CP0_Config3.SC=1, and initialise them to describe the standard legacy
> > layout so they can be used in future patches regardless of whether they
> > are exposed to the guest.
> >
> > Signed-off-by: James Hogan <james.hogan@imgtec.com>
> > Cc: Leon Alrae <leon.alrae@imgtec.com>
> > Cc: Aurelien Jarno <aurelien@aurel32.net>
> > ---
> > target-mips/cpu.h | 31 ++++++++++++++++-
> > target-mips/helper.h | 3 ++-
> > target-mips/machine.c | 7 ++--
> > target-mips/op_helper.c | 24 ++++++++++++-
> > target-mips/translate.c | 84 ++++++++++++++++++++++++++++++++++++++++++-
> > 5 files changed, 147 insertions(+), 2 deletions(-)
> >
> > diff --git a/target-mips/cpu.h b/target-mips/cpu.h
> > index 2abb330272d9..fd8292eaa9c3 100644
> > --- a/target-mips/cpu.h
> > +++ b/target-mips/cpu.h
> > @@ -306,6 +306,36 @@ struct CPUMIPSState {
> > #define CP0PG_XIE 30
> > #define CP0PG_ELPA 29
> > #define CP0PG_IEC 27
> > + target_ulong CP0_SegCtl0;
> > + target_ulong CP0_SegCtl1;
> > + target_ulong CP0_SegCtl2;
> > +#define CP0SC_PA 9
> > +#define CP0SC_PA_MASK (0x7FULL << CP0SC_PA)
> > +#define CP0SC_PA_1GMASK (0x7EULL << CP0SC_PA)
> > +#define CP0SC_AM 4
> > +#define CP0SC_AM_MASK (0x7ULL << CP0SC_AM)
> > +#define CP0SC_AM_UK 0ULL
> > +#define CP0SC_AM_MK 1ULL
> > +#define CP0SC_AM_MSK 2ULL
> > +#define CP0SC_AM_MUSK 3ULL
> > +#define CP0SC_AM_MUSUK 4ULL
> > +#define CP0SC_AM_USK 5ULL
> > +#define CP0SC_AM_UUSK 7ULL
> > +#define CP0SC_EU 3
> > +#define CP0SC_EU_MASK (1ULL << CP0SC_EU)
> > +#define CP0SC_C 0
> > +#define CP0SC_C_MASK (0x7ULL << CP0SC_C)
> > +#define CP0SC_MASK (CP0SC_C_MASK | CP0SC_EU_MASK | CP0SC_AM_MASK | \
> > + CP0SC_PA_MASK)
> > +#define CP0SC_1GMASK (CP0SC_C_MASK | CP0SC_EU_MASK | CP0SC_AM_MASK | \
> > + CP0SC_PA_1GMASK)
> > +#define CP0SC0_MASK (CP0SC_MASK | (CP0SC_MASK << 16))
>
> Defining 16 as a macro would be preferable.
Given that the information about each segment occupies half of a
register, and that it is described that way in the PRA (i.e. as 16bit
segment configurations, with the fields within that described in a
single place), I think the use of literal 16 is more readable in this
case.
>
> > +#define CP0SC1_XAM 59
> > +#define CP0SC1_XAM_MASK (0x7ULL << CP0SC1_XAM)
> > +#define CP0SC1_MASK (CP0SC_MASK | (CP0SC_MASK << 16) | CP0SC1_XAM_MASK)
> > +#define CP0SC2_XR 56
> > +#define CP0SC2_XR_MASK (0xFFULL << CP0SC2_XR)
> > +#define CP0SC2_MASK (CP0SC_1GMASK | (CP0SC_1GMASK << 16) | CP0SC2_XR_MASK)
> > int32_t CP0_Wired;
> > int32_t CP0_SRSConf0_rw_bitmask;
> > int32_t CP0_SRSConf0;
> > @@ -449,6 +479,7 @@ struct CPUMIPSState {
> > #define CP0C3_MSAP 28
> > #define CP0C3_BP 27
> > #define CP0C3_BI 26
> > +#define CP0C3_SC 25
> > #define CP0C3_IPLW 21
> > #define CP0C3_MMAR 18
> > #define CP0C3_MCU 17
> > diff --git a/target-mips/helper.h b/target-mips/helper.h
> > index 666936c81bfe..961741370ad2 100644
> > --- a/target-mips/helper.h
> > +++ b/target-mips/helper.h
> > @@ -122,6 +122,9 @@ DEF_HELPER_2(mtc0_entrylo1, void, env, tl)
> > DEF_HELPER_2(mtc0_context, void, env, tl)
> > DEF_HELPER_2(mtc0_pagemask, void, env, tl)
> > DEF_HELPER_2(mtc0_pagegrain, void, env, tl)
> > +DEF_HELPER_2(mtc0_segctl0, void, env, tl)
> > +DEF_HELPER_2(mtc0_segctl1, void, env, tl)
> > +DEF_HELPER_2(mtc0_segctl2, void, env, tl)
> > DEF_HELPER_2(mtc0_wired, void, env, tl)
> > DEF_HELPER_2(mtc0_srsconf0, void, env, tl)
> > DEF_HELPER_2(mtc0_srsconf1, void, env, tl)
> > diff --git a/target-mips/machine.c b/target-mips/machine.c
> > index e795fecaa0d6..d90f217c3fe5 100644
> > --- a/target-mips/machine.c
> > +++ b/target-mips/machine.c
> > @@ -206,8 +206,8 @@ const VMStateDescription vmstate_tlb = {
> >
> > const VMStateDescription vmstate_mips_cpu = {
> > .name = "cpu",
> > - .version_id = 9,
> > - .minimum_version_id = 9,
> > + .version_id = 10,
> > + .minimum_version_id = 10,
> > .post_load = cpu_post_load,
> > .fields = (VMStateField[]) {
> > /* Active TC */
> > @@ -247,6 +247,9 @@ const VMStateDescription vmstate_mips_cpu = {
> > VMSTATE_UINTTL(env.CP0_Context, MIPSCPU),
> > VMSTATE_INT32(env.CP0_PageMask, MIPSCPU),
> > VMSTATE_INT32(env.CP0_PageGrain, MIPSCPU),
> > + VMSTATE_UINTTL(env.CP0_SegCtl0, MIPSCPU),
> > + VMSTATE_UINTTL(env.CP0_SegCtl1, MIPSCPU),
> > + VMSTATE_UINTTL(env.CP0_SegCtl2, MIPSCPU),
> > VMSTATE_INT32(env.CP0_Wired, MIPSCPU),
> > VMSTATE_INT32(env.CP0_SRSConf0, MIPSCPU),
> > VMSTATE_INT32(env.CP0_SRSConf1, MIPSCPU),
> > diff --git a/target-mips/op_helper.c b/target-mips/op_helper.c
> > index 829ab0bc3cca..983de785b318 100644
> > --- a/target-mips/op_helper.c
> > +++ b/target-mips/op_helper.c
> > @@ -1337,6 +1337,30 @@ void helper_mtc0_pagegrain(CPUMIPSState *env, target_ulong arg1)
> > restore_pamask(env);
> > }
> >
> > +void helper_mtc0_segctl0(CPUMIPSState *env, target_ulong arg1)
> > +{
> > + CPUState *cs = CPU(mips_env_get_cpu(env));
> > +
> > + env->CP0_SegCtl0 = arg1 & CP0SC0_MASK;
>
> Writes of unsupported values to the C field require to leave the field
> unmodified in the R6. Permit 2 (Uncached) only?
Good point.
Though its worth noting that we don't do that for Config0.K0 either:
void helper_mtc0_config0(CPUMIPSState *env, target_ulong arg1)
{
env->CP0_Config0 = (env->CP0_Config0 & 0x81FFFFF8) | (arg1 & 0x00000007);
}
>
> > + tlb_flush(cs, 1);
>
> cpu_mips_tlb_flush()?
Hmm. My reasoning was that this doesn't in any way affect TLB content or
give away the eviction of TLB entries into the shadow half of the TLB.
It only affects the virtual segments that determine whether the TLB is
used for lookup or not.
So if entries are evicted into the shadow part of the TLB by TLBWR,
there is no reason they should be invalidated just because the segments
were changed.
AFAIU the same logic applies to two other uses of cpu_mips_tlb_flush():
- When status.UX/SX/KX is cleared (mappings still persist and nothing is
given away about evicted TLB entries).
- When the ASID is changed (ASID stored in tlb entries so mappings
persist, with nothing given away about evicted TLB entries).
Whereas the remaining uses of cpu_mips_tlb_flush() make absolute sense:
- tlbinv and tlbinvf would legitimately be expected to ensure mappings
are no longer accessible.
- tlbr calls r4k_mips_tlb_flush_extra() unconditionally anyway.
As do the uses of r4k_mips_tlb_flush_extra():
- tlbwi to revoke permissions would legitimately be expected to ensure
the old mapping is no longer accessible.
erm, BUG? what about XI, RI, and EHINV bits changing? I'll check/fix
if I get the time.
- tlbp gives away that a mapping has been evicted.
- tlbr could be used to deduce that a mapping has been evicted.
>
> > +}
> > +
> > +void helper_mtc0_segctl1(CPUMIPSState *env, target_ulong arg1)
> > +{
> > + CPUState *cs = CPU(mips_env_get_cpu(env));
> > +
> > + env->CP0_SegCtl1 = arg1 & CP0SC1_MASK;
> > + tlb_flush(cs, 1);
> > +}
> > +
> > +void helper_mtc0_segctl2(CPUMIPSState *env, target_ulong arg1)
> > +{
> > + CPUState *cs = CPU(mips_env_get_cpu(env));
> > +
> > + env->CP0_SegCtl2 = arg1 & CP0SC2_MASK;
> > + tlb_flush(cs, 1);
> > +}
> > +
> > void helper_mtc0_wired(CPUMIPSState *env, target_ulong arg1)
> > {
> > if (env->insn_flags & ISA_MIPS32R6) {
> > diff --git a/target-mips/translate.c b/target-mips/translate.c
> > index af17fc95eb8f..28d93bb56cd0 100644
> > --- a/target-mips/translate.c
> > +++ b/target-mips/translate.c
> > @@ -1449,6 +1449,7 @@ typedef struct DisasContext {
> > uint64_t PAMask;
> > bool mvh;
> > bool eva;
> > + bool sc;
> > int CP0_LLAddr_shift;
> > bool ps;
> > bool vp;
> > @@ -5216,6 +5217,21 @@ static void gen_mfc0(DisasContext *ctx, TCGv arg, int reg, int sel)
> > gen_mfc0_load32(arg, offsetof(CPUMIPSState, CP0_PageGrain));
> > rn = "PageGrain";
> > break;
> > + case 2:
> > + CP0_CHECK(ctx->sc);
> > + tcg_gen_ld32s_tl(arg, cpu_env, offsetof(CPUMIPSState, CP0_SegCtl0));
>
> This might cause problem in 64-bit guest with big-endian host. Replace it
> with tcg_gen_ld_tl() and tcg_gen_ext32s_tl().
Hmm yes, MFC0 UserLocal looks suspect too (how did that ever work...).
>
> > + rn = "SegCtl0";
> > + break;
> > + case 3:
> > + CP0_CHECK(ctx->sc);
> > + tcg_gen_ld32s_tl(arg, cpu_env, offsetof(CPUMIPSState, CP0_SegCtl1));
>
> same as above
>
> > + rn = "SegCtl1";
> > + break;
> > + case 4:
> > + CP0_CHECK(ctx->sc);
> > + tcg_gen_ld32s_tl(arg, cpu_env, offsetof(CPUMIPSState, CP0_SegCtl2));
>
> and here.
>
> > + rn = "SegCtl2";
> > + break;
> > default:
> > goto cp0_unimplemented;
> > }
> > @@ -5872,6 +5888,21 @@ static void gen_mtc0(DisasContext *ctx, TCGv arg, int reg, int sel)
> > rn = "PageGrain";
> > ctx->bstate = BS_STOP;
> > break;
> > + case 2:
> > + CP0_CHECK(ctx->sc);
> > + gen_helper_mtc0_segctl0(cpu_env, arg);
> > + rn = "SegCtl0";
> > + break;
> > + case 3:
> > + CP0_CHECK(ctx->sc);
> > + gen_helper_mtc0_segctl1(cpu_env, arg);
> > + rn = "SegCtl1";
> > + break;
> > + case 4:
> > + CP0_CHECK(ctx->sc);
> > + gen_helper_mtc0_segctl2(cpu_env, arg);
> > + rn = "SegCtl2";
> > + break;
> > default:
> > goto cp0_unimplemented;
> > }
> > @@ -6534,6 +6565,20 @@ static void gen_dmfc0(DisasContext *ctx, TCGv arg, int reg, int sel)
> > gen_mfc0_load32(arg, offsetof(CPUMIPSState, CP0_PageGrain));
> > rn = "PageGrain";
> > break;
> > + case 2:
> > + CP0_CHECK(ctx->sc);
> > + tcg_gen_ld_tl(arg, cpu_env, offsetof(CPUMIPSState, CP0_SegCtl0));
> > + rn = "SegCtl0";
> > + break;
> > + case 3:
> > + CP0_CHECK(ctx->sc);
> > + tcg_gen_ld_tl(arg, cpu_env, offsetof(CPUMIPSState, CP0_SegCtl1));
> > + rn = "SegCtl1";
> > + break;
> > + case 4:
> > + CP0_CHECK(ctx->sc);
> > + tcg_gen_ld_tl(arg, cpu_env, offsetof(CPUMIPSState, CP0_SegCtl2));
> > + rn = "SegCtl2";
> > default:
> > goto cp0_unimplemented;
> > }
> > @@ -7172,6 +7217,21 @@ static void gen_dmtc0(DisasContext *ctx, TCGv arg, int reg, int sel)
> > gen_helper_mtc0_pagegrain(cpu_env, arg);
> > rn = "PageGrain";
> > break;
> > + case 2:
> > + CP0_CHECK(ctx->sc);
> > + gen_helper_mtc0_segctl0(cpu_env, arg);
> > + rn = "SegCtl0";
> > + break;
> > + case 3:
> > + CP0_CHECK(ctx->sc);
> > + gen_helper_mtc0_segctl1(cpu_env, arg);
> > + rn = "SegCtl1";
> > + break;
> > + case 4:
> > + CP0_CHECK(ctx->sc);
> > + gen_helper_mtc0_segctl2(cpu_env, arg);
> > + rn = "SegCtl2";
> > + break;
> > default:
> > goto cp0_unimplemented;
> > }
> > @@ -19989,6 +20049,7 @@ void gen_intermediate_code(CPUMIPSState *env, struct TranslationBlock *tb)
> > ctx.PAMask = env->PAMask;
> > ctx.mvh = (env->CP0_Config5 >> CP0C5_MVH) & 1;
> > ctx.eva = (env->CP0_Config5 >> CP0C5_EVA) & 1;
> > + ctx.sc = (env->CP0_Config3 >> CP0C3_SC) & 1;
> > ctx.CP0_LLAddr_shift = env->CP0_LLAddr_shift;
> > ctx.cmgcr = (env->CP0_Config3 >> CP0C3_CMGCR) & 1;
> > /* Restore delay slot state from the tb context. */
> > @@ -20463,6 +20524,29 @@ void cpu_state_reset(CPUMIPSState *env)
> > env->tcs[0].CP0_TCStatus = (1 << CP0TCSt_A);
> > }
> > }
> > +
> > + /*
> > + * Configure default legacy segmentation control. We use this regardless of
> > + * whether segmentation control is presented to the guest.
> > + */
> > + /* KSeg3 (seg0 0xE0000000..0xFFFFFFFF) */
> > + env->CP0_SegCtl0 = (CP0SC_AM_MK << CP0SC_AM);
> > + /* KSeg2 (seg1 0xC0000000..0xDFFFFFFF) */
> > + env->CP0_SegCtl0 |= ((CP0SC_AM_MSK << CP0SC_AM)) << 16;
> > + /* KSeg1 (seg2 0xA0000000..0x9FFFFFFF) */
> > + env->CP0_SegCtl1 = (0 << CP0SC_PA) | (CP0SC_AM_UK << CP0SC_AM) |
> > + (2 << CP0SC_C);
> > + /* KSeg0 (seg3 0x80000000..0x9FFFFFFF) */
> > + env->CP0_SegCtl1 |= ((0 << CP0SC_PA) | (CP0SC_AM_UK << CP0SC_AM) |
> > + (3 << CP0SC_C)) << 16;
> > + /* USeg (seg4 0x40000000..0x7FFFFFFF) */
> > + env->CP0_SegCtl2 = (2 << CP0SC_PA) | (CP0SC_AM_MUSK << CP0SC_AM) |
> > + (1 << CP0SC_EU) | (2 << CP0SC_C);
>
> Reset value of C is Undefined in the spec?
Well, it says the reset state of the SegCtl registers is Implementation
Dependent (since some cores can be hardware configured to boot in EVA
mode).
>
> > + /* USeg (seg5 0x00000000..0x3FFFFFFF) */
> > + env->CP0_SegCtl2 |= ((0 << CP0SC_PA) | (CP0SC_AM_MUSK << CP0SC_AM) |
> > + (1 << CP0SC_EU) | (2 << CP0SC_C)) << 16;
>
> Same here
>
> > + /* XKPhys (note, SegCtl2.XR = 0, so XAM won't be used) */
> > + env->CP0_SegCtl1 |= (CP0SC_AM_UK << CP0SC1_XAM);
>
> SegCtl1.XAM is undefined too?
Erm, it indeed does say that. UK is effectively the default since
SegCtl2.XR resets to 0.
What are you suggesting though, that we put a random rather than
sensible value in there?
>
> > #endif
> > if ((env->insn_flags & ISA_MIPS32R6) &&
> > (env->active_fpu.fcr0 & (1 << FCR0_F64))) {
> >
>
>
> Regards,
> Yongbok
Thanks for the reviews,
Cheers
James
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 30+ messages in thread
* [Qemu-devel] [PATCH 8/9] target-mips: Implement segmentation control
2016-09-06 11:03 [Qemu-devel] [PATCH 0/9] target-mips: Add Enhanced Virtual Addressing (EVA) support James Hogan
` (6 preceding siblings ...)
2016-09-06 11:03 ` [Qemu-devel] [PATCH 7/9] target-mips: Add segmentation control registers James Hogan
@ 2016-09-06 11:03 ` James Hogan
2016-10-13 13:06 ` Yongbok Kim
2016-09-06 11:03 ` [Qemu-devel] [PATCH 9/9] target-mips: Add EVA support to P5600 James Hogan
2016-09-06 11:18 ` [Qemu-devel] [PATCH 0/9] target-mips: Add Enhanced Virtual Addressing (EVA) support no-reply
9 siblings, 1 reply; 30+ messages in thread
From: James Hogan @ 2016-09-06 11:03 UTC (permalink / raw)
To: Leon Alrae; +Cc: qemu-devel, James Hogan, Aurelien Jarno
Implement the optional segmentation control feature in the virtual to
physical address translation code.
The fixed legacy segment and XKPhys handling is replaced with a dynamic
layout based on the segmentation control registers (which should be set
up even when the feature is not exposed to the guest).
Signed-off-by: James Hogan <james.hogan@imgtec.com>
Cc: Leon Alrae <leon.alrae@imgtec.com>
Cc: Aurelien Jarno <aurelien@aurel32.net>
---
target-mips/helper.c | 154 +++++++++++++++++++++++++++++++++++---------
1 file changed, 123 insertions(+), 31 deletions(-)
diff --git a/target-mips/helper.c b/target-mips/helper.c
index 2065fc3ec119..63d709bd620f 100644
--- a/target-mips/helper.c
+++ b/target-mips/helper.c
@@ -107,15 +107,107 @@ int r4k_map_address (CPUMIPSState *env, hwaddr *physical, int *prot,
return TLBRET_NOMATCH;
}
+static int is_seg_am_mapped(unsigned int am, bool eu, int mmu_idx)
+{
+ /*
+ * Interpret access control mode and mmu_idx.
+ * AdE? TLB?
+ * AM K S U E K S U E
+ * UK 0 0 1 1 0 0 - - 0
+ * MK 1 0 1 1 0 1 - - !eu
+ * MSK 2 0 0 1 0 1 1 - !eu
+ * MUSK 3 0 0 0 0 1 1 1 !eu
+ * MUSUK 4 0 0 0 0 0 1 1 0
+ * USK 5 0 0 1 0 0 0 - 0
+ * - 6 - - - - - - - -
+ * UUSK 7 0 0 0 0 0 0 0 0
+ */
+ int32_t adetlb_mask;
+
+ switch (mmu_idx) {
+ case 3 /* ERL */:
+ /* If EU is set, always unmapped */
+ if (eu) {
+ return 0;
+ }
+ /* fall through */
+ case MIPS_HFLAG_KM:
+ /* Never AdE, TLB mapped if AM={1,2,3} */
+ adetlb_mask = 0x70000000;
+ goto check_tlb;
+
+ case MIPS_HFLAG_SM:
+ /* AdE if AM={0,1}, TLB mapped if AM={2,3,4} */
+ adetlb_mask = 0xc0380000;
+ goto check_ade;
+
+ case MIPS_HFLAG_UM:
+ /* AdE if AM={0,1,2,5}, TLB mapped if AM={3,4} */
+ adetlb_mask = 0xe4180000;
+ /* fall through */
+ check_ade:
+ /* does this AM cause AdE in current execution mode */
+ if ((adetlb_mask << am) < 0) {
+ return TLBRET_BADADDR;
+ }
+ adetlb_mask <<= 8;
+ /* fall through */
+ check_tlb:
+ /* is this AM mapped in current execution mode */
+ return ((adetlb_mask << am) < 0);
+ default:
+ assert(0);
+ return TLBRET_BADADDR;
+ };
+}
+
+static int get_seg_physical_address(CPUMIPSState *env, hwaddr *physical,
+ int *prot, target_ulong real_address,
+ int rw, int access_type, int mmu_idx,
+ unsigned int am, bool eu,
+ target_ulong segmask,
+ target_ulong physical_base)
+{
+ int mapped = is_seg_am_mapped(am, eu, mmu_idx);
+
+ if (mapped < 0) {
+ /* is_seg_am_mapped can report TLBRET_BADADDR */
+ return mapped;
+ } else if (mapped) {
+ /* The segment is TLB mapped */
+ return env->tlb->map_address(env, physical, prot, real_address, rw,
+ access_type);
+ } else {
+ /* The segment is unmapped */
+ *physical = physical_base | (real_address & segmask);
+ *prot = PAGE_READ | PAGE_WRITE;
+ return TLBRET_MATCH;
+ }
+}
+
+static int get_segctl_physical_address(CPUMIPSState *env, hwaddr *physical,
+ int *prot, target_ulong real_address,
+ int rw, int access_type, int mmu_idx,
+ uint16_t segctl, target_ulong segmask)
+{
+ unsigned int am = (segctl & CP0SC_AM_MASK) >> CP0SC_AM;
+ bool eu = (segctl >> CP0SC_EU) & 1;
+ target_ulong pa = ((target_ulong)segctl & CP0SC_PA_MASK) << 20;
+
+ return get_seg_physical_address(env, physical, prot, real_address, rw,
+ access_type, mmu_idx, am, eu, segmask,
+ pa & ~segmask);
+}
+
static int get_physical_address (CPUMIPSState *env, hwaddr *physical,
int *prot, target_ulong real_address,
int rw, int access_type, int mmu_idx)
{
/* User mode can only access useg/xuseg */
+#if defined(TARGET_MIPS64)
int user_mode = mmu_idx == MIPS_HFLAG_UM;
int supervisor_mode = mmu_idx == MIPS_HFLAG_SM;
int kernel_mode = !user_mode && !supervisor_mode;
-#if defined(TARGET_MIPS64)
int UX = (env->CP0_Status & (1 << CP0St_UX)) != 0;
int SX = (env->CP0_Status & (1 << CP0St_SX)) != 0;
int KX = (env->CP0_Status & (1 << CP0St_KX)) != 0;
@@ -148,12 +240,16 @@ static int get_physical_address (CPUMIPSState *env, hwaddr *physical,
if (address <= USEG_LIMIT) {
/* useg */
- if (env->CP0_Status & (1 << CP0St_ERL)) {
- *physical = address & 0xFFFFFFFF;
- *prot = PAGE_READ | PAGE_WRITE;
+ uint16_t segctl;
+
+ if (address >= 0x40000000UL) {
+ segctl = env->CP0_SegCtl2;
} else {
- ret = env->tlb->map_address(env, physical, prot, real_address, rw, access_type);
+ segctl = env->CP0_SegCtl2 >> 16;
}
+ ret = get_segctl_physical_address(env, physical, prot, real_address, rw,
+ access_type, mmu_idx, segctl,
+ 0x3FFFFFFF);
#if defined(TARGET_MIPS64)
} else if (address < 0x4000000000000000ULL) {
/* xuseg */
@@ -172,10 +268,16 @@ static int get_physical_address (CPUMIPSState *env, hwaddr *physical,
}
} else if (address < 0xC000000000000000ULL) {
/* xkphys */
- if (kernel_mode && KX &&
- (address & 0x07FFFFFFFFFFFFFFULL) <= env->PAMask) {
- *physical = address & env->PAMask;
- *prot = PAGE_READ | PAGE_WRITE;
+ if (KX && (address & 0x07FFFFFFFFFFFFFFULL) <= env->PAMask) {
+ unsigned int am = CP0SC_AM_UK;
+ unsigned int xr = (env->CP0_SegCtl2 & CP0SC2_XR_MASK) >> CP0SC2_XR;
+
+ if (xr & (1 << ((address >> 59) & 0x7))) {
+ am = (env->CP0_SegCtl1 & CP0SC1_XAM_MASK) >> CP0SC1_XAM;
+ }
+ ret = get_seg_physical_address(env, physical, prot, real_address,
+ rw, access_type, mmu_idx, am, false,
+ env->PAMask, 0);
} else {
ret = TLBRET_BADADDR;
}
@@ -190,35 +292,25 @@ static int get_physical_address (CPUMIPSState *env, hwaddr *physical,
#endif
} else if (address < (int32_t)KSEG1_BASE) {
/* kseg0 */
- if (kernel_mode) {
- *physical = address - (int32_t)KSEG0_BASE;
- *prot = PAGE_READ | PAGE_WRITE;
- } else {
- ret = TLBRET_BADADDR;
- }
+ ret = get_segctl_physical_address(env, physical, prot, real_address, rw,
+ access_type, mmu_idx,
+ env->CP0_SegCtl1 >> 16, 0x1FFFFFFF);
} else if (address < (int32_t)KSEG2_BASE) {
/* kseg1 */
- if (kernel_mode) {
- *physical = address - (int32_t)KSEG1_BASE;
- *prot = PAGE_READ | PAGE_WRITE;
- } else {
- ret = TLBRET_BADADDR;
- }
+ ret = get_segctl_physical_address(env, physical, prot, real_address, rw,
+ access_type, mmu_idx,
+ env->CP0_SegCtl1, 0x1FFFFFFF);
} else if (address < (int32_t)KSEG3_BASE) {
/* sseg (kseg2) */
- if (supervisor_mode || kernel_mode) {
- ret = env->tlb->map_address(env, physical, prot, real_address, rw, access_type);
- } else {
- ret = TLBRET_BADADDR;
- }
+ ret = get_segctl_physical_address(env, physical, prot, real_address, rw,
+ access_type, mmu_idx,
+ env->CP0_SegCtl0 >> 16, 0x1FFFFFFF);
} else {
/* kseg3 */
/* XXX: debug segment is not emulated */
- if (kernel_mode) {
- ret = env->tlb->map_address(env, physical, prot, real_address, rw, access_type);
- } else {
- ret = TLBRET_BADADDR;
- }
+ ret = get_segctl_physical_address(env, physical, prot, real_address, rw,
+ access_type, mmu_idx,
+ env->CP0_SegCtl0, 0x1FFFFFFF);
}
return ret;
}
--
git-series 0.8.10
^ permalink raw reply related [flat|nested] 30+ messages in thread* Re: [Qemu-devel] [PATCH 8/9] target-mips: Implement segmentation control
2016-09-06 11:03 ` [Qemu-devel] [PATCH 8/9] target-mips: Implement segmentation control James Hogan
@ 2016-10-13 13:06 ` Yongbok Kim
2017-07-06 23:15 ` James Hogan
0 siblings, 1 reply; 30+ messages in thread
From: Yongbok Kim @ 2016-10-13 13:06 UTC (permalink / raw)
To: James Hogan, qemu-devel; +Cc: Aurelien Jarno
On 06/09/2016 12:03, James Hogan wrote:
> Implement the optional segmentation control feature in the virtual to
> physical address translation code.
>
> The fixed legacy segment and XKPhys handling is replaced with a dynamic
> layout based on the segmentation control registers (which should be set
> up even when the feature is not exposed to the guest).
>
> Signed-off-by: James Hogan <james.hogan@imgtec.com>
> Cc: Leon Alrae <leon.alrae@imgtec.com>
> Cc: Aurelien Jarno <aurelien@aurel32.net>
> ---
> target-mips/helper.c | 154 +++++++++++++++++++++++++++++++++++---------
> 1 file changed, 123 insertions(+), 31 deletions(-)
>
> diff --git a/target-mips/helper.c b/target-mips/helper.c
> index 2065fc3ec119..63d709bd620f 100644
> --- a/target-mips/helper.c
> +++ b/target-mips/helper.c
> @@ -107,15 +107,107 @@ int r4k_map_address (CPUMIPSState *env, hwaddr *physical, int *prot,
> return TLBRET_NOMATCH;
> }
>
> +static int is_seg_am_mapped(unsigned int am, bool eu, int mmu_idx)
> +{
Nice :) I like this simplified logic to decode access controls.
> + /*
> + * Interpret access control mode and mmu_idx.
> + * AdE? TLB?
> + * AM K S U E K S U E
> + * UK 0 0 1 1 0 0 - - 0
> + * MK 1 0 1 1 0 1 - - !eu
> + * MSK 2 0 0 1 0 1 1 - !eu
> + * MUSK 3 0 0 0 0 1 1 1 !eu
> + * MUSUK 4 0 0 0 0 0 1 1 0
> + * USK 5 0 0 1 0 0 0 - 0
> + * - 6 - - - - - - - -
> + * UUSK 7 0 0 0 0 0 0 0 0
> + */
> + int32_t adetlb_mask;
> +
> + switch (mmu_idx) {
> + case 3 /* ERL */:
> + /* If EU is set, always unmapped */
> + if (eu) {
> + return 0;
> + }
I guess checking ERL and EU has to be outside of the switch case.
If ERL = 1 and EU = 0 then it has to check with the current Privilege level
but in this case it falls through and always checks against Kernel mode.
> + /* fall through */
> + case MIPS_HFLAG_KM:
> + /* Never AdE, TLB mapped if AM={1,2,3} */
> + adetlb_mask = 0x70000000;
> + goto check_tlb;
> +
> + case MIPS_HFLAG_SM:
> + /* AdE if AM={0,1}, TLB mapped if AM={2,3,4} */
> + adetlb_mask = 0xc0380000;
> + goto check_ade;
> +
> + case MIPS_HFLAG_UM:
> + /* AdE if AM={0,1,2,5}, TLB mapped if AM={3,4} */
> + adetlb_mask = 0xe4180000;
> + /* fall through */
> + check_ade:
> + /* does this AM cause AdE in current execution mode */
> + if ((adetlb_mask << am) < 0) {
> + return TLBRET_BADADDR;
> + }
> + adetlb_mask <<= 8;
> + /* fall through */
> + check_tlb:
> + /* is this AM mapped in current execution mode */
> + return ((adetlb_mask << am) < 0);
> + default:
> + assert(0);
> + return TLBRET_BADADDR;
> + };
> +}
> +
> +static int get_seg_physical_address(CPUMIPSState *env, hwaddr *physical,
> + int *prot, target_ulong real_address,
> + int rw, int access_type, int mmu_idx,
> + unsigned int am, bool eu,
> + target_ulong segmask,
> + target_ulong physical_base)
uint64_t for physical_base. see below comment.
> +{
> + int mapped = is_seg_am_mapped(am, eu, mmu_idx);
> +
> + if (mapped < 0) {
> + /* is_seg_am_mapped can report TLBRET_BADADDR */
> + return mapped;
> + } else if (mapped) {
> + /* The segment is TLB mapped */
> + return env->tlb->map_address(env, physical, prot, real_address, rw,
> + access_type);
> + } else {
> + /* The segment is unmapped */
> + *physical = physical_base | (real_address & segmask);
> + *prot = PAGE_READ | PAGE_WRITE;
> + return TLBRET_MATCH;
> + }
> +}
> +
> +static int get_segctl_physical_address(CPUMIPSState *env, hwaddr *physical,
> + int *prot, target_ulong real_address,
> + int rw, int access_type, int mmu_idx,
> + uint16_t segctl, target_ulong segmask)
> +{
> + unsigned int am = (segctl & CP0SC_AM_MASK) >> CP0SC_AM;
> + bool eu = (segctl >> CP0SC_EU) & 1;
> + target_ulong pa = ((target_ulong)segctl & CP0SC_PA_MASK) << 20;
According to the architecture spec, PA supports mapping of up to a 36-bit
physical address. uint64_t would be better for PA and physical_base.
> +
> + return get_seg_physical_address(env, physical, prot, real_address, rw,
> + access_type, mmu_idx, am, eu, segmask,
> + pa & ~segmask);
> +}
> +
> static int get_physical_address (CPUMIPSState *env, hwaddr *physical,
> int *prot, target_ulong real_address,
> int rw, int access_type, int mmu_idx)
> {
> /* User mode can only access useg/xuseg */
> +#if defined(TARGET_MIPS64)
> int user_mode = mmu_idx == MIPS_HFLAG_UM;
> int supervisor_mode = mmu_idx == MIPS_HFLAG_SM;
> int kernel_mode = !user_mode && !supervisor_mode;
> -#if defined(TARGET_MIPS64)
> int UX = (env->CP0_Status & (1 << CP0St_UX)) != 0;
> int SX = (env->CP0_Status & (1 << CP0St_SX)) != 0;
> int KX = (env->CP0_Status & (1 << CP0St_KX)) != 0;
> @@ -148,12 +240,16 @@ static int get_physical_address (CPUMIPSState *env, hwaddr *physical,
>
> if (address <= USEG_LIMIT) {
> /* useg */
> - if (env->CP0_Status & (1 << CP0St_ERL)) {
> - *physical = address & 0xFFFFFFFF;
> - *prot = PAGE_READ | PAGE_WRITE;
> + uint16_t segctl;
> +
> + if (address >= 0x40000000UL) {
> + segctl = env->CP0_SegCtl2;
> } else {
> - ret = env->tlb->map_address(env, physical, prot, real_address, rw, access_type);
> + segctl = env->CP0_SegCtl2 >> 16;
> }
> + ret = get_segctl_physical_address(env, physical, prot, real_address, rw,
> + access_type, mmu_idx, segctl,
> + 0x3FFFFFFF);
> #if defined(TARGET_MIPS64)
> } else if (address < 0x4000000000000000ULL) {
> /* xuseg */
> @@ -172,10 +268,16 @@ static int get_physical_address (CPUMIPSState *env, hwaddr *physical,
> }
> } else if (address < 0xC000000000000000ULL) {
> /* xkphys */
> - if (kernel_mode && KX &&
> - (address & 0x07FFFFFFFFFFFFFFULL) <= env->PAMask) {
> - *physical = address & env->PAMask;
> - *prot = PAGE_READ | PAGE_WRITE;
> + if (KX && (address & 0x07FFFFFFFFFFFFFFULL) <= env->PAMask) {
KX should be dropped from here. More over, we need to check KX, SX and UX
whether it meets minimum privilege level according to AM.
> + unsigned int am = CP0SC_AM_UK;
When the region is disabled, it is valid for Kernel mode only.
> + unsigned int xr = (env->CP0_SegCtl2 & CP0SC2_XR_MASK) >> CP0SC2_XR;
> +
> + if (xr & (1 << ((address >> 59) & 0x7))) {
> + am = (env->CP0_SegCtl1 & CP0SC1_XAM_MASK) >> CP0SC1_XAM;
> + }
> + ret = get_seg_physical_address(env, physical, prot, real_address,
> + rw, access_type, mmu_idx, am, false,
> + env->PAMask, 0);
> } else {
> ret = TLBRET_BADADDR;
> }
> @@ -190,35 +292,25 @@ static int get_physical_address (CPUMIPSState *env, hwaddr *physical,
> #endif
> } else if (address < (int32_t)KSEG1_BASE) {
> /* kseg0 */
> - if (kernel_mode) {
> - *physical = address - (int32_t)KSEG0_BASE;
> - *prot = PAGE_READ | PAGE_WRITE;
> - } else {
> - ret = TLBRET_BADADDR;
> - }
> + ret = get_segctl_physical_address(env, physical, prot, real_address, rw,
> + access_type, mmu_idx,
> + env->CP0_SegCtl1 >> 16, 0x1FFFFFFF);
> } else if (address < (int32_t)KSEG2_BASE) {
> /* kseg1 */
> - if (kernel_mode) {
> - *physical = address - (int32_t)KSEG1_BASE;
> - *prot = PAGE_READ | PAGE_WRITE;
> - } else {
> - ret = TLBRET_BADADDR;
> - }
> + ret = get_segctl_physical_address(env, physical, prot, real_address, rw,
> + access_type, mmu_idx,
> + env->CP0_SegCtl1, 0x1FFFFFFF);
> } else if (address < (int32_t)KSEG3_BASE) {
> /* sseg (kseg2) */
> - if (supervisor_mode || kernel_mode) {
> - ret = env->tlb->map_address(env, physical, prot, real_address, rw, access_type);
> - } else {
> - ret = TLBRET_BADADDR;
> - }
> + ret = get_segctl_physical_address(env, physical, prot, real_address, rw,
> + access_type, mmu_idx,
> + env->CP0_SegCtl0 >> 16, 0x1FFFFFFF);
> } else {
> /* kseg3 */
> /* XXX: debug segment is not emulated */
> - if (kernel_mode) {
> - ret = env->tlb->map_address(env, physical, prot, real_address, rw, access_type);
> - } else {
> - ret = TLBRET_BADADDR;
> - }
> + ret = get_segctl_physical_address(env, physical, prot, real_address, rw,
> + access_type, mmu_idx,
> + env->CP0_SegCtl0, 0x1FFFFFFF);
> }
> return ret;
> }
>
Regards,
Yongbok
^ permalink raw reply [flat|nested] 30+ messages in thread* Re: [Qemu-devel] [PATCH 8/9] target-mips: Implement segmentation control
2016-10-13 13:06 ` Yongbok Kim
@ 2017-07-06 23:15 ` James Hogan
0 siblings, 0 replies; 30+ messages in thread
From: James Hogan @ 2017-07-06 23:15 UTC (permalink / raw)
To: Yongbok Kim; +Cc: qemu-devel, Aurelien Jarno
[-- Attachment #1: Type: text/plain, Size: 10958 bytes --]
On Thu, Oct 13, 2016 at 02:06:25PM +0100, Yongbok Kim wrote:
>
>
> On 06/09/2016 12:03, James Hogan wrote:
> > Implement the optional segmentation control feature in the virtual to
> > physical address translation code.
> >
> > The fixed legacy segment and XKPhys handling is replaced with a dynamic
> > layout based on the segmentation control registers (which should be set
> > up even when the feature is not exposed to the guest).
> >
> > Signed-off-by: James Hogan <james.hogan@imgtec.com>
> > Cc: Leon Alrae <leon.alrae@imgtec.com>
> > Cc: Aurelien Jarno <aurelien@aurel32.net>
> > ---
> > target-mips/helper.c | 154 +++++++++++++++++++++++++++++++++++---------
> > 1 file changed, 123 insertions(+), 31 deletions(-)
> >
> > diff --git a/target-mips/helper.c b/target-mips/helper.c
> > index 2065fc3ec119..63d709bd620f 100644
> > --- a/target-mips/helper.c
> > +++ b/target-mips/helper.c
> > @@ -107,15 +107,107 @@ int r4k_map_address (CPUMIPSState *env, hwaddr *physical, int *prot,
> > return TLBRET_NOMATCH;
> > }
> >
> > +static int is_seg_am_mapped(unsigned int am, bool eu, int mmu_idx)
> > +{
>
> Nice :) I like this simplified logic to decode access controls.
>
> > + /*
> > + * Interpret access control mode and mmu_idx.
> > + * AdE? TLB?
> > + * AM K S U E K S U E
> > + * UK 0 0 1 1 0 0 - - 0
> > + * MK 1 0 1 1 0 1 - - !eu
> > + * MSK 2 0 0 1 0 1 1 - !eu
> > + * MUSK 3 0 0 0 0 1 1 1 !eu
> > + * MUSUK 4 0 0 0 0 0 1 1 0
> > + * USK 5 0 0 1 0 0 0 - 0
> > + * - 6 - - - - - - - -
> > + * UUSK 7 0 0 0 0 0 0 0 0
> > + */
> > + int32_t adetlb_mask;
> > +
> > + switch (mmu_idx) {
> > + case 3 /* ERL */:
> > + /* If EU is set, always unmapped */
> > + if (eu) {
> > + return 0;
> > + }
>
> I guess checking ERL and EU has to be outside of the switch case.
> If ERL = 1 and EU = 0 then it has to check with the current Privilege level
> but in this case it falls through and always checks against Kernel mode.
No, "The processor is in Kernel Mode when the DM bit in the Debug
register is 0... and any of the following is true... The ERL bit in the
Status register is 1"
See how compute_hflags() leaves hflags & KSU == KM in the specified
cases.
>
> > + /* fall through */
> > + case MIPS_HFLAG_KM:
> > + /* Never AdE, TLB mapped if AM={1,2,3} */
> > + adetlb_mask = 0x70000000;
> > + goto check_tlb;
> > +
> > + case MIPS_HFLAG_SM:
> > + /* AdE if AM={0,1}, TLB mapped if AM={2,3,4} */
> > + adetlb_mask = 0xc0380000;
> > + goto check_ade;
> > +
> > + case MIPS_HFLAG_UM:
> > + /* AdE if AM={0,1,2,5}, TLB mapped if AM={3,4} */
> > + adetlb_mask = 0xe4180000;
> > + /* fall through */
> > + check_ade:
> > + /* does this AM cause AdE in current execution mode */
> > + if ((adetlb_mask << am) < 0) {
> > + return TLBRET_BADADDR;
> > + }
> > + adetlb_mask <<= 8;
> > + /* fall through */
> > + check_tlb:
> > + /* is this AM mapped in current execution mode */
> > + return ((adetlb_mask << am) < 0);
> > + default:
> > + assert(0);
> > + return TLBRET_BADADDR;
> > + };
> > +}
> > +
> > +static int get_seg_physical_address(CPUMIPSState *env, hwaddr *physical,
> > + int *prot, target_ulong real_address,
> > + int rw, int access_type, int mmu_idx,
> > + unsigned int am, bool eu,
> > + target_ulong segmask,
> > + target_ulong physical_base)
>
> uint64_t for physical_base. see below comment.
>
> > +{
> > + int mapped = is_seg_am_mapped(am, eu, mmu_idx);
> > +
> > + if (mapped < 0) {
> > + /* is_seg_am_mapped can report TLBRET_BADADDR */
> > + return mapped;
> > + } else if (mapped) {
> > + /* The segment is TLB mapped */
> > + return env->tlb->map_address(env, physical, prot, real_address, rw,
> > + access_type);
> > + } else {
> > + /* The segment is unmapped */
> > + *physical = physical_base | (real_address & segmask);
> > + *prot = PAGE_READ | PAGE_WRITE;
> > + return TLBRET_MATCH;
> > + }
> > +}
> > +
> > +static int get_segctl_physical_address(CPUMIPSState *env, hwaddr *physical,
> > + int *prot, target_ulong real_address,
> > + int rw, int access_type, int mmu_idx,
> > + uint16_t segctl, target_ulong segmask)
> > +{
> > + unsigned int am = (segctl & CP0SC_AM_MASK) >> CP0SC_AM;
> > + bool eu = (segctl >> CP0SC_EU) & 1;
> > + target_ulong pa = ((target_ulong)segctl & CP0SC_PA_MASK) << 20;
>
> According to the architecture spec, PA supports mapping of up to a 36-bit
> physical address. uint64_t would be better for PA and physical_base.
Yes, good catch.
>
> > +
> > + return get_seg_physical_address(env, physical, prot, real_address, rw,
> > + access_type, mmu_idx, am, eu, segmask,
> > + pa & ~segmask);
> > +}
> > +
> > static int get_physical_address (CPUMIPSState *env, hwaddr *physical,
> > int *prot, target_ulong real_address,
> > int rw, int access_type, int mmu_idx)
> > {
> > /* User mode can only access useg/xuseg */
> > +#if defined(TARGET_MIPS64)
> > int user_mode = mmu_idx == MIPS_HFLAG_UM;
> > int supervisor_mode = mmu_idx == MIPS_HFLAG_SM;
> > int kernel_mode = !user_mode && !supervisor_mode;
> > -#if defined(TARGET_MIPS64)
> > int UX = (env->CP0_Status & (1 << CP0St_UX)) != 0;
> > int SX = (env->CP0_Status & (1 << CP0St_SX)) != 0;
> > int KX = (env->CP0_Status & (1 << CP0St_KX)) != 0;
> > @@ -148,12 +240,16 @@ static int get_physical_address (CPUMIPSState *env, hwaddr *physical,
> >
> > if (address <= USEG_LIMIT) {
> > /* useg */
> > - if (env->CP0_Status & (1 << CP0St_ERL)) {
> > - *physical = address & 0xFFFFFFFF;
> > - *prot = PAGE_READ | PAGE_WRITE;
> > + uint16_t segctl;
> > +
> > + if (address >= 0x40000000UL) {
> > + segctl = env->CP0_SegCtl2;
> > } else {
> > - ret = env->tlb->map_address(env, physical, prot, real_address, rw, access_type);
> > + segctl = env->CP0_SegCtl2 >> 16;
> > }
> > + ret = get_segctl_physical_address(env, physical, prot, real_address, rw,
> > + access_type, mmu_idx, segctl,
> > + 0x3FFFFFFF);
> > #if defined(TARGET_MIPS64)
> > } else if (address < 0x4000000000000000ULL) {
> > /* xuseg */
> > @@ -172,10 +268,16 @@ static int get_physical_address (CPUMIPSState *env, hwaddr *physical,
> > }
> > } else if (address < 0xC000000000000000ULL) {
> > /* xkphys */
> > - if (kernel_mode && KX &&
> > - (address & 0x07FFFFFFFFFFFFFFULL) <= env->PAMask) {
> > - *physical = address & env->PAMask;
> > - *prot = PAGE_READ | PAGE_WRITE;
> > + if (KX && (address & 0x07FFFFFFFFFFFFFFULL) <= env->PAMask) {
>
> KX should be dropped from here.
> More over, we need to check KX, SX and UX whether it meets minimum
> privilege level according to AM.
Oh, hmm... good point!
Cheers
James
>
> > + unsigned int am = CP0SC_AM_UK;
>
> When the region is disabled, it is valid for Kernel mode only.
>
> > + unsigned int xr = (env->CP0_SegCtl2 & CP0SC2_XR_MASK) >> CP0SC2_XR;
> > +
> > + if (xr & (1 << ((address >> 59) & 0x7))) {
> > + am = (env->CP0_SegCtl1 & CP0SC1_XAM_MASK) >> CP0SC1_XAM;
> > + }
> > + ret = get_seg_physical_address(env, physical, prot, real_address,
> > + rw, access_type, mmu_idx, am, false,
> > + env->PAMask, 0);
> > } else {
> > ret = TLBRET_BADADDR;
> > }
> > @@ -190,35 +292,25 @@ static int get_physical_address (CPUMIPSState *env, hwaddr *physical,
> > #endif
> > } else if (address < (int32_t)KSEG1_BASE) {
> > /* kseg0 */
> > - if (kernel_mode) {
> > - *physical = address - (int32_t)KSEG0_BASE;
> > - *prot = PAGE_READ | PAGE_WRITE;
> > - } else {
> > - ret = TLBRET_BADADDR;
> > - }
> > + ret = get_segctl_physical_address(env, physical, prot, real_address, rw,
> > + access_type, mmu_idx,
> > + env->CP0_SegCtl1 >> 16, 0x1FFFFFFF);
> > } else if (address < (int32_t)KSEG2_BASE) {
> > /* kseg1 */
> > - if (kernel_mode) {
> > - *physical = address - (int32_t)KSEG1_BASE;
> > - *prot = PAGE_READ | PAGE_WRITE;
> > - } else {
> > - ret = TLBRET_BADADDR;
> > - }
> > + ret = get_segctl_physical_address(env, physical, prot, real_address, rw,
> > + access_type, mmu_idx,
> > + env->CP0_SegCtl1, 0x1FFFFFFF);
> > } else if (address < (int32_t)KSEG3_BASE) {
> > /* sseg (kseg2) */
> > - if (supervisor_mode || kernel_mode) {
> > - ret = env->tlb->map_address(env, physical, prot, real_address, rw, access_type);
> > - } else {
> > - ret = TLBRET_BADADDR;
> > - }
> > + ret = get_segctl_physical_address(env, physical, prot, real_address, rw,
> > + access_type, mmu_idx,
> > + env->CP0_SegCtl0 >> 16, 0x1FFFFFFF);
> > } else {
> > /* kseg3 */
> > /* XXX: debug segment is not emulated */
> > - if (kernel_mode) {
> > - ret = env->tlb->map_address(env, physical, prot, real_address, rw, access_type);
> > - } else {
> > - ret = TLBRET_BADADDR;
> > - }
> > + ret = get_segctl_physical_address(env, physical, prot, real_address, rw,
> > + access_type, mmu_idx,
> > + env->CP0_SegCtl0, 0x1FFFFFFF);
> > }
> > return ret;
> > }
> >
>
>
> Regards,
> Yongbok
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 30+ messages in thread
* [Qemu-devel] [PATCH 9/9] target-mips: Add EVA support to P5600
2016-09-06 11:03 [Qemu-devel] [PATCH 0/9] target-mips: Add Enhanced Virtual Addressing (EVA) support James Hogan
` (7 preceding siblings ...)
2016-09-06 11:03 ` [Qemu-devel] [PATCH 8/9] target-mips: Implement segmentation control James Hogan
@ 2016-09-06 11:03 ` James Hogan
2016-10-13 13:15 ` Yongbok Kim
2016-09-06 11:18 ` [Qemu-devel] [PATCH 0/9] target-mips: Add Enhanced Virtual Addressing (EVA) support no-reply
9 siblings, 1 reply; 30+ messages in thread
From: James Hogan @ 2016-09-06 11:03 UTC (permalink / raw)
To: Leon Alrae; +Cc: qemu-devel, James Hogan, Aurelien Jarno
Add the Enhanced Virtual Addressing (EVA) feature to the P5600 core
configuration, along with the related Segmentation Control (SC) feature
and writable CP0_EBase.WG bit.
This allows it to run Malta EVA kernels.
Signed-off-by: James Hogan <james.hogan@imgtec.com>
Cc: Leon Alrae <leon.alrae@imgtec.com>
Cc: Aurelien Jarno <aurelien@aurel32.net>
---
target-mips/translate_init.c | 14 ++++++++------
1 file changed, 8 insertions(+), 6 deletions(-)
diff --git a/target-mips/translate_init.c b/target-mips/translate_init.c
index f327e6e7de6c..58d5d429941a 100644
--- a/target-mips/translate_init.c
+++ b/target-mips/translate_init.c
@@ -399,9 +399,9 @@ static const mips_def_t mips_defs[] =
},
{
/* FIXME:
- * Config3: CMGCR, SC, PW, VZ, CTXTC, CDMM, TL
+ * Config3: CMGCR, PW, VZ, CTXTC, CDMM, TL
* Config4: MMUExtDef
- * Config5: EVA, MRP
+ * Config5: MRP
* FIR(FCR0): Has2008
* */
.name = "P5600",
@@ -414,13 +414,14 @@ static const mips_def_t mips_defs[] =
(1 << CP0C1_PC) | (1 << CP0C1_FP),
.CP0_Config2 = MIPS_CONFIG2,
.CP0_Config3 = MIPS_CONFIG3 | (1U << CP0C3_M) | (1 << CP0C3_MSAP) |
- (1 << CP0C3_BP) | (1 << CP0C3_BI) | (1 << CP0C3_ULRI) |
- (1 << CP0C3_RXI) | (1 << CP0C3_LPA) | (1 << CP0C3_VInt),
+ (1 << CP0C3_BP) | (1 << CP0C3_BI) | (1 << CP0C3_SC) |
+ (1 << CP0C3_ULRI) | (1 << CP0C3_RXI) | (1 << CP0C3_LPA) |
+ (1 << CP0C3_VInt),
.CP0_Config4 = MIPS_CONFIG4 | (1U << CP0C4_M) | (2 << CP0C4_IE) |
(0x1c << CP0C4_KScrExist),
.CP0_Config4_rw_bitmask = 0,
- .CP0_Config5 = MIPS_CONFIG5 | (1 << CP0C5_MVH) | (1 << CP0C5_LLB) |
- (1 << CP0C5_MRP),
+ .CP0_Config5 = MIPS_CONFIG5 | (1 << CP0C5_EVA) | (1 << CP0C5_MVH) |
+ (1 << CP0C5_LLB) | (1 << CP0C5_MRP),
.CP0_Config5_rw_bitmask = (1 << CP0C5_K) | (1 << CP0C5_CV) |
(1 << CP0C5_MSAEn) | (1 << CP0C5_UFE) |
(1 << CP0C5_FRE) | (1 << CP0C5_UFR),
@@ -431,6 +432,7 @@ static const mips_def_t mips_defs[] =
.CP0_Status_rw_bitmask = 0x3C68FF1F,
.CP0_PageGrain_rw_bitmask = (1U << CP0PG_RIE) | (1 << CP0PG_XIE) |
(1 << CP0PG_ELPA) | (1 << CP0PG_IEC),
+ .CP0_EBase_rw_bitmask = (1 << CP0EBase_WG),
.CP1_fcr0 = (1 << FCR0_FREP) | (1 << FCR0_UFRP) | (1 << FCR0_HAS2008) |
(1 << FCR0_F64) | (1 << FCR0_L) | (1 << FCR0_W) |
(1 << FCR0_D) | (1 << FCR0_S) | (0x03 << FCR0_PRID),
--
git-series 0.8.10
^ permalink raw reply related [flat|nested] 30+ messages in thread* Re: [Qemu-devel] [PATCH 9/9] target-mips: Add EVA support to P5600
2016-09-06 11:03 ` [Qemu-devel] [PATCH 9/9] target-mips: Add EVA support to P5600 James Hogan
@ 2016-10-13 13:15 ` Yongbok Kim
0 siblings, 0 replies; 30+ messages in thread
From: Yongbok Kim @ 2016-10-13 13:15 UTC (permalink / raw)
To: James Hogan, qemu-devel; +Cc: Aurelien Jarno
On 06/09/2016 12:03, James Hogan wrote:
> Add the Enhanced Virtual Addressing (EVA) feature to the P5600 core
> configuration, along with the related Segmentation Control (SC) feature
> and writable CP0_EBase.WG bit.
>
> This allows it to run Malta EVA kernels.
>
> Signed-off-by: James Hogan <james.hogan@imgtec.com>
> Cc: Leon Alrae <leon.alrae@imgtec.com>
> Cc: Aurelien Jarno <aurelien@aurel32.net>
> ---
> target-mips/translate_init.c | 14 ++++++++------
> 1 file changed, 8 insertions(+), 6 deletions(-)
>
> diff --git a/target-mips/translate_init.c b/target-mips/translate_init.c
> index f327e6e7de6c..58d5d429941a 100644
> --- a/target-mips/translate_init.c
> +++ b/target-mips/translate_init.c
> @@ -399,9 +399,9 @@ static const mips_def_t mips_defs[] =
> },
> {
> /* FIXME:
> - * Config3: CMGCR, SC, PW, VZ, CTXTC, CDMM, TL
> + * Config3: CMGCR, PW, VZ, CTXTC, CDMM, TL
> * Config4: MMUExtDef
> - * Config5: EVA, MRP
> + * Config5: MRP
> * FIR(FCR0): Has2008
> * */
> .name = "P5600",
> @@ -414,13 +414,14 @@ static const mips_def_t mips_defs[] =
> (1 << CP0C1_PC) | (1 << CP0C1_FP),
> .CP0_Config2 = MIPS_CONFIG2,
> .CP0_Config3 = MIPS_CONFIG3 | (1U << CP0C3_M) | (1 << CP0C3_MSAP) |
> - (1 << CP0C3_BP) | (1 << CP0C3_BI) | (1 << CP0C3_ULRI) |
> - (1 << CP0C3_RXI) | (1 << CP0C3_LPA) | (1 << CP0C3_VInt),
> + (1 << CP0C3_BP) | (1 << CP0C3_BI) | (1 << CP0C3_SC) |
> + (1 << CP0C3_ULRI) | (1 << CP0C3_RXI) | (1 << CP0C3_LPA) |
> + (1 << CP0C3_VInt),
> .CP0_Config4 = MIPS_CONFIG4 | (1U << CP0C4_M) | (2 << CP0C4_IE) |
> (0x1c << CP0C4_KScrExist),
> .CP0_Config4_rw_bitmask = 0,
> - .CP0_Config5 = MIPS_CONFIG5 | (1 << CP0C5_MVH) | (1 << CP0C5_LLB) |
> - (1 << CP0C5_MRP),
> + .CP0_Config5 = MIPS_CONFIG5 | (1 << CP0C5_EVA) | (1 << CP0C5_MVH) |
> + (1 << CP0C5_LLB) | (1 << CP0C5_MRP),
> .CP0_Config5_rw_bitmask = (1 << CP0C5_K) | (1 << CP0C5_CV) |
> (1 << CP0C5_MSAEn) | (1 << CP0C5_UFE) |
> (1 << CP0C5_FRE) | (1 << CP0C5_UFR),
> @@ -431,6 +432,7 @@ static const mips_def_t mips_defs[] =
> .CP0_Status_rw_bitmask = 0x3C68FF1F,
> .CP0_PageGrain_rw_bitmask = (1U << CP0PG_RIE) | (1 << CP0PG_XIE) |
> (1 << CP0PG_ELPA) | (1 << CP0PG_IEC),
> + .CP0_EBase_rw_bitmask = (1 << CP0EBase_WG),
> .CP1_fcr0 = (1 << FCR0_FREP) | (1 << FCR0_UFRP) | (1 << FCR0_HAS2008) |
> (1 << FCR0_F64) | (1 << FCR0_L) | (1 << FCR0_W) |
> (1 << FCR0_D) | (1 << FCR0_S) | (0x03 << FCR0_PRID),
>
Reviewed-by: Yongbok Kim <yongbok.kim@imgtec.com>
Regards,
Yongbok
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [Qemu-devel] [PATCH 0/9] target-mips: Add Enhanced Virtual Addressing (EVA) support
2016-09-06 11:03 [Qemu-devel] [PATCH 0/9] target-mips: Add Enhanced Virtual Addressing (EVA) support James Hogan
` (8 preceding siblings ...)
2016-09-06 11:03 ` [Qemu-devel] [PATCH 9/9] target-mips: Add EVA support to P5600 James Hogan
@ 2016-09-06 11:18 ` no-reply
2016-09-06 12:09 ` James Hogan
9 siblings, 1 reply; 30+ messages in thread
From: no-reply @ 2016-09-06 11:18 UTC (permalink / raw)
To: james.hogan; +Cc: famz, leon.alrae, qemu-devel, aurelien
Hi,
Your series seems to have some coding style problems. See output below for
more information:
Subject: [Qemu-devel] [PATCH 0/9] target-mips: Add Enhanced Virtual Addressing (EVA) support
Type: series
Message-id: cover.bcfb6a7121f5fd92c9ebda9f199ff06cb0d4bc05.1473159543.git-series.james.hogan@imgtec.com
=== TEST SCRIPT BEGIN ===
#!/bin/bash
BASE=base
n=1
total=$(git log --oneline $BASE.. | wc -l)
failed=0
# Useful git options
git config --local diff.renamelimit 0
git config --local diff.renames True
commits="$(git log --format=%H --reverse $BASE..)"
for c in $commits; do
echo "Checking PATCH $n/$total: $(git show --no-patch --format=%s $c)..."
if ! git show $c --format=email | ./scripts/checkpatch.pl --mailback -; then
failed=1
echo
fi
n=$((n+1))
done
exit $failed
=== TEST SCRIPT END ===
Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
From https://github.com/patchew-project/qemu
* [new tag] patchew/cover.bcfb6a7121f5fd92c9ebda9f199ff06cb0d4bc05.1473159543.git-series.james.hogan@imgtec.com -> patchew/cover.bcfb6a7121f5fd92c9ebda9f199ff06cb0d4bc05.1473159543.git-series.james.hogan@imgtec.com
Switched to a new branch 'test'
d8e2b4a target-mips: Add EVA support to P5600
41640a0 target-mips: Implement segmentation control
1b812b6 target-mips: Add segmentation control registers
e68a781 target-mips: Add an MMU mode for ERL
a4d378b target-mips: Abstract mmu_idx from hflags
4331ffd target-mips: Check memory permissions with mem_idx
0b20e7b target-mips: Decode EVA load & store instructions
f813a87 target-mips: Prepare loads/stores for EVA
75eb9a9 target-mips: Add CP0_Ebase.WG (write gate) support
=== OUTPUT BEGIN ===
Checking PATCH 1/9: target-mips: Add CP0_Ebase.WG (write gate) support...
ERROR: space prohibited after that '&' (ctx:WxW)
#104: FILE: target-mips/op_helper.c:1529:
+ if (arg1 & (1 << CP0EBase_WG) & env->CP0_EBase_rw_bitmask) {
^
ERROR: space prohibited after that '&' (ctx:WxW)
#116: FILE: target-mips/op_helper.c:1540:
+ if (arg1 & (1 << CP0EBase_WG) & env->CP0_EBase_rw_bitmask) {
^
total: 2 errors, 0 warnings, 119 lines checked
Your patch has style problems, please review. If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
Checking PATCH 2/9: target-mips: Prepare loads/stores for EVA...
Checking PATCH 3/9: target-mips: Decode EVA load & store instructions...
Checking PATCH 4/9: target-mips: Check memory permissions with mem_idx...
Checking PATCH 5/9: target-mips: Abstract mmu_idx from hflags...
Checking PATCH 6/9: target-mips: Add an MMU mode for ERL...
ERROR: trailing statements should be on next line
#94: FILE: target-mips/op_helper.c:97:
+ case 3: cpu_##insn##_error_ra(env, addr, val, retaddr); break; \
total: 1 errors, 0 warnings, 65 lines checked
Your patch has style problems, please review. If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
Checking PATCH 7/9: target-mips: Add segmentation control registers...
Checking PATCH 8/9: target-mips: Implement segmentation control...
Checking PATCH 9/9: target-mips: Add EVA support to P5600...
=== OUTPUT END ===
Test command exited with code: 1
---
Email generated automatically by Patchew [http://patchew.org/].
Please send your feedback to patchew-devel@freelists.org
^ permalink raw reply [flat|nested] 30+ messages in thread* Re: [Qemu-devel] [PATCH 0/9] target-mips: Add Enhanced Virtual Addressing (EVA) support
2016-09-06 11:18 ` [Qemu-devel] [PATCH 0/9] target-mips: Add Enhanced Virtual Addressing (EVA) support no-reply
@ 2016-09-06 12:09 ` James Hogan
0 siblings, 0 replies; 30+ messages in thread
From: James Hogan @ 2016-09-06 12:09 UTC (permalink / raw)
To: qemu-devel; +Cc: famz, leon.alrae, aurelien
[-- Attachment #1: Type: text/plain, Size: 1011 bytes --]
On Tue, Sep 06, 2016 at 04:18:38AM -0700, no-reply@patchew.org wrote:
> Checking PATCH 1/9: target-mips: Add CP0_Ebase.WG (write gate) support...
> ERROR: space prohibited after that '&' (ctx:WxW)
> #104: FILE: target-mips/op_helper.c:1529:
> + if (arg1 & (1 << CP0EBase_WG) & env->CP0_EBase_rw_bitmask) {
> ^
>
> ERROR: space prohibited after that '&' (ctx:WxW)
> #116: FILE: target-mips/op_helper.c:1540:
> + if (arg1 & (1 << CP0EBase_WG) & env->CP0_EBase_rw_bitmask) {
> ^
its a bitwise and operator, not an address-of operator, so I presumed
this was a false positive.
> Checking PATCH 6/9: target-mips: Add an MMU mode for ERL...
> ERROR: trailing statements should be on next line
> #94: FILE: target-mips/op_helper.c:97:
> + case 3: cpu_##insn##_error_ra(env, addr, val, retaddr); break; \
And this is merely consistent with the surrounding lines so I didn't
bother fixing it.
Cheers
James
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply [flat|nested] 30+ messages in thread