[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH] target/arm: fix IL bit for data abort exceptions
From: |
Richard Henderson |
Subject: |
Re: [PATCH] target/arm: fix IL bit for data abort exceptions |
Date: |
Tue, 17 Dec 2019 15:03:16 -1000 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.2.2 |
On 12/17/19 11:02 AM, Jeff Kubascik wrote:
> diff --git a/target/arm/tlb_helper.c b/target/arm/tlb_helper.c
> index 5feb312941..e63f8bda29 100644
> --- a/target/arm/tlb_helper.c
> +++ b/target/arm/tlb_helper.c
> @@ -44,7 +44,7 @@ static inline uint32_t merge_syn_data_abort(uint32_t
> template_syn,
> syn = syn_data_abort_with_iss(same_el,
> 0, 0, 0, 0, 0,
> ea, 0, s1ptw, is_write, fsc,
> - false);
> + true);
> /* Merge the runtime syndrome with the template syndrome. */
> syn |= template_syn;
This doesn't look correct. Surely the IL bit should come from template_syn?
> diff --git a/target/arm/translate-a64.c b/target/arm/translate-a64.c
> index d4bebbe629..a3c618fdd9 100644
> --- a/target/arm/translate-a64.c
> +++ b/target/arm/translate-a64.c
> @@ -14045,6 +14045,7 @@ static void disas_a64_insn(CPUARMState *env,
> DisasContext *s)
> s->pc_curr = s->base.pc_next;
> insn = arm_ldl_code(env, s->base.pc_next, s->sctlr_b);
> s->insn = insn;
> + s->is_16bit = false;
> s->base.pc_next += 4;
Should not be necessary, as the field is not read along any a64 path. (Also,
while it's not yet in master, there's a patch on list that zero initializes the
entire structure.)
> diff --git a/target/arm/translate.c b/target/arm/translate.c
> index 2b6c1f91bf..300480f1b7 100644
> --- a/target/arm/translate.c
> +++ b/target/arm/translate.c
> @@ -8555,7 +8555,7 @@ static ISSInfo make_issinfo(DisasContext *s, int rd,
> bool p, bool w)
>
> /* ISS not valid if writeback */
> if (p && !w) {
> - ret = rd;
> + ret = rd | (s->is_16bit ? ISSIs16Bit : 0);
> } else {
> ret = ISSInvalid;
> }
> @@ -11057,6 +11057,7 @@ static void arm_tr_translate_insn(DisasContextBase
> *dcbase, CPUState *cpu)
> dc->pc_curr = dc->base.pc_next;
> insn = arm_ldl_code(env, dc->base.pc_next, dc->sctlr_b);
> dc->insn = insn;
> + dc->is_16bit = false;
> dc->base.pc_next += 4;
> disas_arm_insn(dc, insn);
>
> @@ -11126,6 +11127,7 @@ static void thumb_tr_translate_insn(DisasContextBase
> *dcbase, CPUState *cpu)
> dc->pc_curr = dc->base.pc_next;
> insn = arm_lduw_code(env, dc->base.pc_next, dc->sctlr_b);
> is_16bit = thumb_insn_is_16bit(dc, dc->base.pc_next, insn);
> + dc->is_16bit = is_16bit;
> dc->base.pc_next += 2;
> if (!is_16bit) {
> uint32_t insn2 = arm_lduw_code(env, dc->base.pc_next, dc->sctlr_b);
> diff --git a/target/arm/translate.h b/target/arm/translate.h
> index b837b7fcbf..c16f434477 100644
> --- a/target/arm/translate.h
> +++ b/target/arm/translate.h
> @@ -14,6 +14,8 @@ typedef struct DisasContext {
> target_ulong pc_curr;
> target_ulong page_start;
> uint32_t insn;
> + /* 16-bit instruction flag */
> + bool is_16bit;
> /* Nonzero if this instruction has been conditionally skipped. */
> int condjmp;
> /* The label that will be jumped to when the instruction is skipped. */
The rest of this looks both correct and necessary.
r~