On Thu, 17 Mar 2022 at 05:23, Richard Henderson
<richard.hender...@linaro.org> wrote:
>
> The 4 lower bits, D, PERM, BAD, DBL, are unconditionally set on any
> exception with EH=0, or so says Table 42 (Processor Status After
> Taking Exception).
>
> We currently do not set PERM or BAD at all, and only set/clear
> DBL for tlb miss, and do not clear DBL for any other exception.
>
> It is a bit confusing to set D in tlb_fill and the rest during
> do_interrupt, so move the setting of D to do_interrupt as well.
> To do this, split EXP_TLBD into two cases, EXCP_TLB_X and EXCP_TLB_D,
> which allows us to distinguish them during do_interrupt. Choose
> a value for EXCP_TLB_D such that when truncated it produces the
> correct value for exception.CAUSE.
>
> Rename EXCP_TLB[RWX] to EXCP_PERM_[RWX], to emphasize that the
> exception is permissions related. Rename EXCP_SUPER[AD] to
> EXCP_SUPERA_[DX] to emphasize that they are both "supervisor
> address" exceptions, data and execute.
>
> Retain the setting of tlbmisc.WE for the fast-tlb-miss path, as it
> is being relied upon, but remove it from the permission path.
>
> Signed-off-by: Richard Henderson <richard.hender...@linaro.org>
> ---
> target/nios2/cpu.h | 13 +++---
> target/nios2/helper.c | 102 +++++++++++++++++++++++++++++-------------
> 2 files changed, 77 insertions(+), 38 deletions(-)
>
> diff --git a/target/nios2/cpu.h b/target/nios2/cpu.h
> index d003af5afc..c925cdd8e3 100644
> --- a/target/nios2/cpu.h
> +++ b/target/nios2/cpu.h
> @@ -166,13 +166,14 @@ FIELD(CR_TLBMISC, EE, 24, 1)
> #define EXCP_UNALIGN 6
> #define EXCP_UNALIGND 7
> #define EXCP_DIV 8
> -#define EXCP_SUPERA 9
> +#define EXCP_SUPERA_X 9
> #define EXCP_SUPERI 10
> -#define EXCP_SUPERD 11
> -#define EXCP_TLBD 12
> -#define EXCP_TLBX 13
> -#define EXCP_TLBR 14
> -#define EXCP_TLBW 15
> +#define EXCP_SUPERA_D 11
> +#define EXCP_TLB_X 12
> +#define EXCP_TLB_D (0x1000 | EXCP_TLB_X)
> +#define EXCP_PERM_X 13
> +#define EXCP_PERM_R 14
> +#define EXCP_PERM_W 15
> #define EXCP_MPUI 16
> #define EXCP_MPUD 17
>
> diff --git a/target/nios2/helper.c b/target/nios2/helper.c
> index afbafd1fdc..8b69918ba3 100644
> --- a/target/nios2/helper.c
> +++ b/target/nios2/helper.c
> @@ -49,7 +49,8 @@ void nios2_cpu_record_sigsegv(CPUState *cs, vaddr addr,
>
> #else /* !CONFIG_USER_ONLY */
>
> -static void do_exception(Nios2CPU *cpu, uint32_t exception_addr, bool
> is_break)
> +static void do_exception(Nios2CPU *cpu, uint32_t exception_addr,
> + uint32_t tlbmisc_set, bool is_break)
> {
> CPUNios2State *env = &cpu->env;
> CPUState *cs = CPU(cpu);
> @@ -68,6 +69,16 @@ static void do_exception(Nios2CPU *cpu, uint32_t
> exception_addr, bool is_break)
>
> if (cpu->mmu_present) {
> new_status |= CR_STATUS_EH;
> +
> + /*
> + * There are 4 bits that are always written.
> + * Explicitly clear them, to be set via the argument.
> + */
> + env->ctrl[CR_TLBMISC] &= ~(CR_TLBMISC_D |
> + CR_TLBMISC_PERM |
> + CR_TLBMISC_BAD |
> + CR_TLBMISC_DBL);
> + env->ctrl[CR_TLBMISC] |= tlbmisc_set;
> }
> }
>
> @@ -83,13 +94,14 @@ static void do_exception(Nios2CPU *cpu, uint32_t
> exception_addr, bool is_break)
>
> static void do_iic_irq(Nios2CPU *cpu)
> {
> - do_exception(cpu, cpu->exception_addr, false);
> + do_exception(cpu, cpu->exception_addr, 0, false);
> }
>
> void nios2_cpu_do_interrupt(CPUState *cs)
> {
> Nios2CPU *cpu = NIOS2_CPU(cs);
> CPUNios2State *env = &cpu->env;
> + uint32_t tlbmisc_set = 0;
>
> if (qemu_loglevel_mask(CPU_LOG_INT)) {
> const char *name = NULL;
> @@ -98,20 +110,21 @@ void nios2_cpu_do_interrupt(CPUState *cs)
> case EXCP_IRQ:
> name = "interrupt";
> break;
> - case EXCP_TLBD:
> + case EXCP_TLB_X:
> + case EXCP_TLB_D:
> if (env->ctrl[CR_STATUS] & CR_STATUS_EH) {
> name = "TLB MISS (double)";
> } else {
> name = "TLB MISS (fast)";
> }
> break;
> - case EXCP_TLBR:
> - case EXCP_TLBW:
> - case EXCP_TLBX:
> + case EXCP_PERM_R:
> + case EXCP_PERM_W:
> + case EXCP_PERM_X:
> name = "TLB PERM";
> break;
> - case EXCP_SUPERA:
> - case EXCP_SUPERD:
> + case EXCP_SUPERA_X:
> + case EXCP_SUPERA_D:
> name = "SUPERVISOR (address)";
> break;
> case EXCP_SUPERI:
> @@ -149,38 +162,60 @@ void nios2_cpu_do_interrupt(CPUState *cs)
> do_iic_irq(cpu);
> break;
>
> - case EXCP_TLBD:
> - if ((env->ctrl[CR_STATUS] & CR_STATUS_EH) == 0) {
> - env->ctrl[CR_TLBMISC] &= ~CR_TLBMISC_DBL;
> - env->ctrl[CR_TLBMISC] |= CR_TLBMISC_WE;
> - do_exception(cpu, cpu->fast_tlb_miss_addr, false);
> + case EXCP_TLB_D:
> + tlbmisc_set = CR_TLBMISC_D;
> + /* fall through */
> + case EXCP_TLB_X:
> + if (env->ctrl[CR_STATUS] & CR_STATUS_EH) {
> + tlbmisc_set |= CR_TLBMISC_DBL;
> + /*
> + * Normally, we don't write to tlbmisc unless !EH,
> + * so do it manually for the double-tlb miss exception.
> + */
> + env->ctrl[CR_TLBMISC] &= ~(CR_TLBMISC_D |
> + CR_TLBMISC_PERM |
> + CR_TLBMISC_BAD);
> + env->ctrl[CR_TLBMISC] |= tlbmisc_set;
> + do_exception(cpu, cpu->exception_addr, 0, false);
> } else {
> - env->ctrl[CR_TLBMISC] |= CR_TLBMISC_DBL;
> - do_exception(cpu, cpu->exception_addr, false);
> + /*
> + * ??? Implicitly setting tlbmisc.WE for the fast-tlb-miss
> + * handler appears to be out of spec. But, the linux kernel
> + * handler relies on it, writing to tlbacc without first
> + * setting tlbmisc.WE.
> + */
On page 3-21 of
https://www.intel.com/content/dam/www/programmable/us/en/pdfs/literature/hb/nios2/n2cpu_nii5v1_01.pdf
the description of the WE flag says
"Hardware sets the WE flag to one on a TLB permission violation
exception, and on a TLB miss exception when status.EH = 0."
This is the second of those two cases, isn't it ?
> + tlbmisc_set |= CR_TLBMISC_WE;
> + do_exception(cpu, cpu->fast_tlb_miss_addr, tlbmisc_set, false);
> }
> break;
> - case EXCP_TLBR:
> - case EXCP_TLBW:
> - case EXCP_TLBX:
> - if ((env->ctrl[CR_STATUS] & CR_STATUS_EH) == 0) {
> - env->ctrl[CR_TLBMISC] |= CR_TLBMISC_WE;
> - }
> - do_exception(cpu, cpu->exception_addr, false);
> + case EXCP_PERM_R:
> + case EXCP_PERM_W:
> + tlbmisc_set = CR_TLBMISC_D;
> + /* fall through */
> + case EXCP_PERM_X:
> + tlbmisc_set |= CR_TLBMISC_PERM;
> + do_exception(cpu, cpu->exception_addr, tlbmisc_set, false);
And this change is incorrectly dropping the "sets WE on
TLB permission violation exception" part.
Other than that,
Reviewed-by: Peter Maydell <peter.mayd...@linaro.org>
thanks
-- PMM
