On 07/19/2018 05:54 AM, Stefan Markovic wrote:
> From: Stefan Markovic <smarko...@wavecomp.com>
>
> Add testing Config0.WR bit into watch exception handling logic.
Config1, here and in the subject.
>
> Signed-off-by: Aleksandar Markovic <amarko...@wavecomp.com>
> Signed-off-by: Stefan Markovic <smarko...@wavecomp.com>
> ---
> target/mips/helper.c | 12 +++++++++++-
> target/mips/translate.c | 22 ++++++++++++++++------
> 2 files changed, 27 insertions(+), 7 deletions(-)
>
> diff --git a/target/mips/helper.c b/target/mips/helper.c
> index 9535131..dc8f2a5 100644
> --- a/target/mips/helper.c
> +++ b/target/mips/helper.c
> @@ -749,6 +749,14 @@ void mips_cpu_do_interrupt(CPUState *cs)
> (env->hflags & MIPS_HFLAG_DM)) {
> cs->exception_index = EXCP_DINT;
> }
> +
> + if ((cs->exception_index == EXCP_DWATCH ||
> + cs->exception_index == EXCP_DFWATCH ||
> + cs->exception_index == EXCP_IWATCH) &&
> + (env->CP0_Config1 & (1 << CP0C1_WR))) {
> + cs->exception_index = EXCP_NONE;
> + }
This will cause the switch below to abort.
In any case, I think this test is a mistake -- you should have (and probably
did, given a presumed lack of abort during testing) prevent these exceptions
from being triggered in the first place.
> case EXCP_SRESET:
> env->CP0_Status |= (1 << CP0St_SR);
> - memset(env->CP0_WatchLo, 0, sizeof(env->CP0_WatchLo));
> + if (env->CP0_Config1 & (1 << CP0C1_WR)) {
> + memset(env->CP0_WatchLo, 0, sizeof(env->CP0_WatchLo));
> + }
If it's unused/missing, why does it hurt to reset to 0?
> --- a/target/mips/translate.c
> +++ b/target/mips/translate.c
> @@ -5622,6 +5622,7 @@ static void gen_mfc0(DisasContext *ctx, TCGv arg, int
> reg, int sel)
> case 5:
> case 6:
> case 7:
> + CP0_CHECK(ctx->CP0_Config1 & (1 << CP0C1_WR));
> gen_helper_1e0i(mfc0_watchlo, arg, sel);
> rn = "WatchLo";
> break;
> @@ -5639,6 +5640,7 @@ static void gen_mfc0(DisasContext *ctx, TCGv arg, int
> reg, int sel)
> case 5:
> case 6:
> case 7:
> + CP0_CHECK(ctx->CP0_Config1 & (1 << CP0C1_WR));
> gen_helper_1e0i(mfc0_watchhi, arg, sel);
> rn = "WatchHi";
> break;
> @@ -6321,6 +6323,7 @@ static void gen_mtc0(DisasContext *ctx, TCGv arg, int
> reg, int sel)
> case 5:
> case 6:
> case 7:
> + CP0_CHECK(ctx->CP0_Config1 & (1 << CP0C1_WR));
> gen_helper_0e1i(mtc0_watchlo, arg, sel);
> rn = "WatchLo";
> break;
> @@ -6338,6 +6341,7 @@ static void gen_mtc0(DisasContext *ctx, TCGv arg, int
> reg, int sel)
> case 5:
> case 6:
> case 7:
> + CP0_CHECK(ctx->CP0_Config1 & (1 << CP0C1_WR));
> gen_helper_0e1i(mtc0_watchhi, arg, sel);
> rn = "WatchHi";
> break;
> @@ -7024,6 +7028,7 @@ static void gen_dmfc0(DisasContext *ctx, TCGv arg, int
> reg, int sel)
> case 5:
> case 6:
> case 7:
> + CP0_CHECK(ctx->CP0_Config1 & (1 << CP0C1_WR));
> gen_helper_1e0i(dmfc0_watchlo, arg, sel);
> rn = "WatchLo";
> break;
> @@ -7041,6 +7046,7 @@ static void gen_dmfc0(DisasContext *ctx, TCGv arg, int
> reg, int sel)
> case 5:
> case 6:
> case 7:
> + CP0_CHECK(ctx->CP0_Config1 & (1 << CP0C1_WR));
> gen_helper_1e0i(mfc0_watchhi, arg, sel);
> rn = "WatchHi";
> break;
> @@ -7705,6 +7711,7 @@ static void gen_dmtc0(DisasContext *ctx, TCGv arg, int
> reg, int sel)
> case 5:
> case 6:
> case 7:
> + CP0_CHECK(ctx->CP0_Config1 & (1 << CP0C1_WR));
> gen_helper_0e1i(mtc0_watchlo, arg, sel);
> rn = "WatchLo";
> break;
> @@ -7722,6 +7729,7 @@ static void gen_dmtc0(DisasContext *ctx, TCGv arg, int
> reg, int sel)
> case 5:
> case 6:
> case 7:
> + CP0_CHECK(ctx->CP0_Config1 & (1 << CP0C1_WR));
> gen_helper_0e1i(mtc0_watchhi, arg, sel);
> rn = "WatchHi";
> break;
These are probably the only changes required -- preventing the missing
registers from being set at runtime.
> @@ -25281,14 +25289,16 @@ void cpu_state_reset(CPUMIPSState *env)
> no performance counters. */
> env->CP0_IntCtl = 0xe0000000;
> {
> - int i;
> + if (env->CP0_Config1 & (1 << CP0C1_WR)) {
> + int i;
>
> - for (i = 0; i < 7; i++) {
> - env->CP0_WatchLo[i] = 0;
> - env->CP0_WatchHi[i] = 0x80000000;
> + for (i = 0; i < 7; i++) {
> + env->CP0_WatchLo[i] = 0;
> + env->CP0_WatchHi[i] = 0x80000000;
> + }
> + env->CP0_WatchLo[7] = 0;
> + env->CP0_WatchHi[7] = 0;
> }
> - env->CP0_WatchLo[7] = 0;
> - env->CP0_WatchHi[7] = 0;
> }
Again, what difference does the reset value make?
r~
