diff mbox series

[for-7.1,v6,25/51] target/nios2: Clean up handling of tlbmisc in do_exception

Message ID 20220317050538.924111-26-richard.henderson@linaro.org
State Superseded
Headers show
Series target/nios2: Shadow register set, EIC and VIC | expand

Commit Message

Richard Henderson March 17, 2022, 5:05 a.m. UTC
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.henderson@linaro.org>
---
 target/nios2/cpu.h    |  13 +++---
 target/nios2/helper.c | 102 +++++++++++++++++++++++++++++-------------
 2 files changed, 77 insertions(+), 38 deletions(-)

Comments

Peter Maydell March 17, 2022, 3:41 p.m. UTC | #1
On Thu, 17 Mar 2022 at 05:23, Richard Henderson
<richard.henderson@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.henderson@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.maydell@linaro.org>

thanks
-- PMM
Richard Henderson March 17, 2022, 5:01 p.m. UTC | #2
On 3/17/22 08:41, Peter Maydell wrote:
> 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 ?

Good eyes.  I had been looking over the exception processing section.


r~
diff mbox series

Patch

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.
+             */
+            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);
+        break;
+
+    case EXCP_SUPERA_D:
+    case EXCP_UNALIGN:
+        tlbmisc_set = CR_TLBMISC_D;
+        /* fall through */
+    case EXCP_SUPERA_X:
+    case EXCP_UNALIGND:
+        tlbmisc_set |= CR_TLBMISC_BAD;
+        do_exception(cpu, cpu->exception_addr, tlbmisc_set, false);
         break;
 
-    case EXCP_SUPERA:
     case EXCP_SUPERI:
-    case EXCP_SUPERD:
     case EXCP_ILLEGAL:
     case EXCP_TRAP:
-    case EXCP_UNALIGN:
-    case EXCP_UNALIGND:
-        do_exception(cpu, cpu->exception_addr, false);
+        do_exception(cpu, cpu->exception_addr, 0, false);
         break;
 
     case EXCP_BREAK:
-        do_exception(cpu, cpu->exception_addr, true);
+        do_exception(cpu, cpu->exception_addr, 0, true);
         break;
 
     case EXCP_SEMIHOST:
@@ -235,7 +270,7 @@  bool nios2_cpu_tlb_fill(CPUState *cs, vaddr address, int size,
 {
     Nios2CPU *cpu = NIOS2_CPU(cs);
     CPUNios2State *env = &cpu->env;
-    unsigned int excp = EXCP_TLBD;
+    unsigned int excp;
     target_ulong vaddr, paddr;
     Nios2MMULookup lu;
     unsigned int hit;
@@ -262,7 +297,8 @@  bool nios2_cpu_tlb_fill(CPUState *cs, vaddr address, int size,
             if (probe) {
                 return false;
             }
-            cs->exception_index = EXCP_SUPERA;
+            cs->exception_index = (access_type == MMU_INST_FETCH
+                                   ? EXCP_SUPERA_X : EXCP_SUPERA_D);
             env->ctrl[CR_BADADDR] = address;
             cpu_loop_exit_restore(cs, retaddr);
         }
@@ -283,8 +319,10 @@  bool nios2_cpu_tlb_fill(CPUState *cs, vaddr address, int size,
         }
 
         /* Permission violation */
-        excp = (access_type == MMU_DATA_LOAD ? EXCP_TLBR :
-                access_type == MMU_DATA_STORE ? EXCP_TLBW : EXCP_TLBX);
+        excp = (access_type == MMU_DATA_LOAD ? EXCP_PERM_R :
+                access_type == MMU_DATA_STORE ? EXCP_PERM_W : EXCP_PERM_X);
+    } else {
+        excp = (access_type == MMU_INST_FETCH ? EXCP_TLB_X: EXCP_TLB_D);
     }
 
     if (probe) {