[Qemu-devel] [PATCH 0/2] targets/openrisc: Improve exception vectoring.

[Qemu-devel] [PATCH 0/2] targets/openrisc: Improve exception vectoring.

[Tim 'mithro' Ansell]

Hi,

This patch series improves the exception vectoring on the OpenRISC platform by
adding support for both the EVBAR register and EPH bit.

This is my first patch to upstream QEMU, so please do point of if I have done
anything silly.

Tim 'mithro' Ansell (2):
  target/openrisc: Implement EVBAR register
  target/openrisc: Implement EPH bit

 target/openrisc/cpu.c        | 2 ++
 target/openrisc/cpu.h        | 7 +++++++
 target/openrisc/interrupt.c  | 9 ++++++++-
 target/openrisc/sys_helper.c | 7 +++++++
 4 files changed, 24 insertions(+), 1 deletion(-)

-- 
2.12.1

[Qemu-devel] [PATCH 1/2] target/openrisc: Implement EVBAR register

[Tim 'mithro' Ansell]

Exception Vector Base Address Register (EVBAR) - This optional register
can be used to apply an offset to the exception vector addresses.

The significant bits (31-12) of the vector offset address for each
exception depend on the setting of the Supervision Register (SR)'s EPH
bit and the Exception Vector Base Address Register (EVBAR).

Its presence is indicated by the EVBARP bit in the CPU Configuration
Register (CPUCFGR).

Signed-off-by: Tim 'mithro' Ansell <mithro@mithis.com>
---
 target/openrisc/cpu.c        | 2 ++
 target/openrisc/cpu.h        | 7 +++++++
 target/openrisc/interrupt.c  | 6 +++++-
 target/openrisc/sys_helper.c | 7 +++++++
 4 files changed, 21 insertions(+), 1 deletion(-)

diff --git a/target/openrisc/cpu.c b/target/openrisc/cpu.c
index 7fd2b9a216..1524ed981a 100644
--- a/target/openrisc/cpu.c
+++ b/target/openrisc/cpu.c
@@ -134,6 +134,7 @@ static void or1200_initfn(Object *obj)
 
     set_feature(cpu, OPENRISC_FEATURE_OB32S);
     set_feature(cpu, OPENRISC_FEATURE_OF32S);
+    set_feature(cpu, OPENRISC_FEATURE_EVBAR);
 }
 
 static void openrisc_any_initfn(Object *obj)
@@ -141,6 +142,7 @@ static void openrisc_any_initfn(Object *obj)
     OpenRISCCPU *cpu = OPENRISC_CPU(obj);
 
     set_feature(cpu, OPENRISC_FEATURE_OB32S);
+    set_feature(cpu, OPENRISC_FEATURE_EVBAR);
 }
 
 typedef struct OpenRISCCPUInfo {
diff --git a/target/openrisc/cpu.h b/target/openrisc/cpu.h
index 418a0e6960..1958b72718 100644
--- a/target/openrisc/cpu.h
+++ b/target/openrisc/cpu.h
@@ -111,6 +111,11 @@ enum {
     CPUCFGR_OF32S = (1 << 7),
     CPUCFGR_OF64S = (1 << 8),
     CPUCFGR_OV64S = (1 << 9),
+    /* CPUCFGR_ND = (1 << 10), */
+    /* CPUCFGR_AVRP = (1 << 11), */
+    CPUCFGR_EVBARP = (1 << 12),
+    /* CPUCFGR_ISRP = (1 << 13), */
+    /* CPUCFGR_AECSRP = (1 << 14), */
 };
 
 /* DMMU configure register */
@@ -200,6 +205,7 @@ enum {
     OPENRISC_FEATURE_OF32S = (1 << 7),
     OPENRISC_FEATURE_OF64S = (1 << 8),
     OPENRISC_FEATURE_OV64S = (1 << 9),
+    OPENRISC_FEATURE_EVBAR = (1 << 12),
 };
 
 /* Tick Timer Mode Register */
@@ -289,6 +295,7 @@ typedef struct CPUOpenRISCState {
     uint32_t dmmucfgr;        /* DMMU configure register */
     uint32_t immucfgr;        /* IMMU configure register */
     uint32_t esr;             /* Exception supervisor register */
+    uint32_t evbar;           /* Exception vector base address register */
     uint32_t fpcsr;           /* Float register */
     float_status fp_status;
 
diff --git a/target/openrisc/interrupt.c b/target/openrisc/interrupt.c
index a2eec6fb32..78f0ba9421 100644
--- a/target/openrisc/interrupt.c
+++ b/target/openrisc/interrupt.c
@@ -65,7 +65,11 @@ void openrisc_cpu_do_interrupt(CPUState *cs)
     env->lock_addr = -1;
 
     if (cs->exception_index > 0 && cs->exception_index < EXCP_NR) {
-        env->pc = (cs->exception_index << 8);
+        hwaddr vect_pc = cs->exception_index << 8;
+        if (env->cpucfgr & CPUCFGR_EVBARP) {
+            vect_pc |= env->evbar;
+        }
+        env->pc = vect_pc;
     } else {
         cpu_abort(cs, "Unhandled exception 0x%x\n", cs->exception_index);
     }
diff --git a/target/openrisc/sys_helper.c b/target/openrisc/sys_helper.c
index 60c3193656..6ba816249b 100644
--- a/target/openrisc/sys_helper.c
+++ b/target/openrisc/sys_helper.c
@@ -39,6 +39,10 @@ void HELPER(mtspr)(CPUOpenRISCState *env,
         env->vr = rb;
         break;
 
+    case TO_SPR(0, 11): /* EVBAR */
+        env->evbar = rb;
+        break;
+
     case TO_SPR(0, 16): /* NPC */
         cpu_restore_state(cs, GETPC());
         /* ??? Mirror or1ksim in not trashing delayed branch state
@@ -206,6 +210,9 @@ target_ulong HELPER(mfspr)(CPUOpenRISCState *env,
     case TO_SPR(0, 4): /* IMMUCFGR */
         return env->immucfgr;
 
+    case TO_SPR(0, 11): /* EVBAR */
+        return env->evbar;
+
     case TO_SPR(0, 16): /* NPC (equals PC) */
         cpu_restore_state(cs, GETPC());
         return env->pc;
-- 
2.12.1

[Qemu-devel] [PATCH 2/2] target/openrisc: Implement EPH bit

[Tim 'mithro' Ansell]

Exception Prefix High (EPH) control bit of the Supervision Register
(SR).

The significant bits (31-12) of the vector offset address for each
exception depend on the setting of the Supervision Register (SR)'s EPH
bit and the Exception Vector Base Address Register (EVBAR).

If SR[EPH] is set, the vector offset is logically ORed with the offset
0xF0000000.

This means if EPH is;
 * 0 - Exceptions vectors start at EVBAR
 * 1 - Exception vectors start at EVBAR | 0xF0000000

Signed-off-by: Tim 'mithro' Ansell <mithro@mithis.com>
---
 target/openrisc/interrupt.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/target/openrisc/interrupt.c b/target/openrisc/interrupt.c
index 78f0ba9421..2c91fab380 100644
--- a/target/openrisc/interrupt.c
+++ b/target/openrisc/interrupt.c
@@ -69,6 +69,9 @@ void openrisc_cpu_do_interrupt(CPUState *cs)
         if (env->cpucfgr & CPUCFGR_EVBARP) {
             vect_pc |= env->evbar;
         }
+        if (env->sr & SR_EPH) {
+            vect_pc |= 0xf0000000;
+        }
         env->pc = vect_pc;
     } else {
         cpu_abort(cs, "Unhandled exception 0x%x\n", cs->exception_index);
-- 
2.12.1

Re: [Qemu-devel] [PATCH 2/2] target/openrisc: Implement EPH bit

[Stafford Horne]

On Tue, Apr 18, 2017 at 04:15:51PM +1000, Tim 'mithro' Ansell wrote:
> Exception Prefix High (EPH) control bit of the Supervision Register
> (SR).
> 
> The significant bits (31-12) of the vector offset address for each
> exception depend on the setting of the Supervision Register (SR)'s EPH
> bit and the Exception Vector Base Address Register (EVBAR).
> 
> If SR[EPH] is set, the vector offset is logically ORed with the offset
> 0xF0000000.
> 
> This means if EPH is;
>  * 0 - Exceptions vectors start at EVBAR
>  * 1 - Exception vectors start at EVBAR | 0xF0000000
> 
> Signed-off-by: Tim 'mithro' Ansell <mithro@mithis.com>

Acked-by: Stafford Horne <shorne@gmail.com>

Thanks, I will queue this with my other changes unless anyone has any
objections.

> ---
>  target/openrisc/interrupt.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/target/openrisc/interrupt.c b/target/openrisc/interrupt.c
> index 78f0ba9421..2c91fab380 100644
> --- a/target/openrisc/interrupt.c
> +++ b/target/openrisc/interrupt.c
> @@ -69,6 +69,9 @@ void openrisc_cpu_do_interrupt(CPUState *cs)
>          if (env->cpucfgr & CPUCFGR_EVBARP) {
>              vect_pc |= env->evbar;
>          }
> +        if (env->sr & SR_EPH) {
> +            vect_pc |= 0xf0000000;
> +        }
>          env->pc = vect_pc;
>      } else {
>          cpu_abort(cs, "Unhandled exception 0x%x\n", cs->exception_index);
> -- 
> 2.12.1
> 

Re: [Qemu-devel] [PATCH 1/2] target/openrisc: Implement EVBAR register

[Stafford Horne]

On Tue, Apr 18, 2017 at 04:15:50PM +1000, Tim 'mithro' Ansell wrote:
> Exception Vector Base Address Register (EVBAR) - This optional register
> can be used to apply an offset to the exception vector addresses.
> 
> The significant bits (31-12) of the vector offset address for each
> exception depend on the setting of the Supervision Register (SR)'s EPH
> bit and the Exception Vector Base Address Register (EVBAR).
> 
> Its presence is indicated by the EVBARP bit in the CPU Configuration
> Register (CPUCFGR).
> 
> Signed-off-by: Tim 'mithro' Ansell <mithro@mithis.com>
> ---
>  target/openrisc/cpu.c        | 2 ++
>  target/openrisc/cpu.h        | 7 +++++++
>  target/openrisc/interrupt.c  | 6 +++++-
>  target/openrisc/sys_helper.c | 7 +++++++
>  4 files changed, 21 insertions(+), 1 deletion(-)
> 
> diff --git a/target/openrisc/cpu.c b/target/openrisc/cpu.c
> index 7fd2b9a216..1524ed981a 100644
> --- a/target/openrisc/cpu.c
> +++ b/target/openrisc/cpu.c
> @@ -134,6 +134,7 @@ static void or1200_initfn(Object *obj)
>  
>      set_feature(cpu, OPENRISC_FEATURE_OB32S);
>      set_feature(cpu, OPENRISC_FEATURE_OF32S);
> +    set_feature(cpu, OPENRISC_FEATURE_EVBAR);
>  }
>  
>  static void openrisc_any_initfn(Object *obj)
> @@ -141,6 +142,7 @@ static void openrisc_any_initfn(Object *obj)
>      OpenRISCCPU *cpu = OPENRISC_CPU(obj);
>  
>      set_feature(cpu, OPENRISC_FEATURE_OB32S);
> +    set_feature(cpu, OPENRISC_FEATURE_EVBAR);
>  }
>  
>  typedef struct OpenRISCCPUInfo {
> diff --git a/target/openrisc/cpu.h b/target/openrisc/cpu.h
> index 418a0e6960..1958b72718 100644
> --- a/target/openrisc/cpu.h
> +++ b/target/openrisc/cpu.h
> @@ -111,6 +111,11 @@ enum {
>      CPUCFGR_OF32S = (1 << 7),
>      CPUCFGR_OF64S = (1 << 8),
>      CPUCFGR_OV64S = (1 << 9),
> +    /* CPUCFGR_ND = (1 << 10), */
> +    /* CPUCFGR_AVRP = (1 << 11), */
> +    CPUCFGR_EVBARP = (1 << 12),
> +    /* CPUCFGR_ISRP = (1 << 13), */
> +    /* CPUCFGR_AECSRP = (1 << 14), */
>  };
>  
>  /* DMMU configure register */
> @@ -200,6 +205,7 @@ enum {
>      OPENRISC_FEATURE_OF32S = (1 << 7),
>      OPENRISC_FEATURE_OF64S = (1 << 8),
>      OPENRISC_FEATURE_OV64S = (1 << 9),
> +    OPENRISC_FEATURE_EVBAR = (1 << 12),

Does anyone know why we have to add both Features and CPUCFG bits?

It seems like duplication to me.

>  };
>  
>  /* Tick Timer Mode Register */
> @@ -289,6 +295,7 @@ typedef struct CPUOpenRISCState {
>      uint32_t dmmucfgr;        /* DMMU configure register */
>      uint32_t immucfgr;        /* IMMU configure register */
>      uint32_t esr;             /* Exception supervisor register */
> +    uint32_t evbar;           /* Exception vector base address register */

If we add something to CPUOpenRISCState, we ideally need to also add it to
machine.c vmstate definition.  However, I currently have a patch out to
rework the vmstate serialization.  I can add this to that patch.

>      uint32_t fpcsr;           /* Float register */
>      float_status fp_status;
>  
> diff --git a/target/openrisc/interrupt.c b/target/openrisc/interrupt.c
> index a2eec6fb32..78f0ba9421 100644
> --- a/target/openrisc/interrupt.c
> +++ b/target/openrisc/interrupt.c
> @@ -65,7 +65,11 @@ void openrisc_cpu_do_interrupt(CPUState *cs)
>      env->lock_addr = -1;
>  
>      if (cs->exception_index > 0 && cs->exception_index < EXCP_NR) {
> -        env->pc = (cs->exception_index << 8);
> +        hwaddr vect_pc = cs->exception_index << 8;
> +        if (env->cpucfgr & CPUCFGR_EVBARP) {
> +            vect_pc |= env->evbar;
> +        }
> +        env->pc = vect_pc;
>      } else {
>          cpu_abort(cs, "Unhandled exception 0x%x\n", cs->exception_index);
>      }
> diff --git a/target/openrisc/sys_helper.c b/target/openrisc/sys_helper.c
> index 60c3193656..6ba816249b 100644
> --- a/target/openrisc/sys_helper.c
> +++ b/target/openrisc/sys_helper.c
> @@ -39,6 +39,10 @@ void HELPER(mtspr)(CPUOpenRISCState *env,
>          env->vr = rb;
>          break;
>  
> +    case TO_SPR(0, 11): /* EVBAR */
> +        env->evbar = rb;
> +        break;
> +
>      case TO_SPR(0, 16): /* NPC */
>          cpu_restore_state(cs, GETPC());
>          /* ??? Mirror or1ksim in not trashing delayed branch state
> @@ -206,6 +210,9 @@ target_ulong HELPER(mfspr)(CPUOpenRISCState *env,
>      case TO_SPR(0, 4): /* IMMUCFGR */
>          return env->immucfgr;
>  
> +    case TO_SPR(0, 11): /* EVBAR */
> +        return env->evbar;
> +
>      case TO_SPR(0, 16): /* NPC (equals PC) */
>          cpu_restore_state(cs, GETPC());
>          return env->pc;

Reviewed-by: Stafford Horne <shorne@gmail.com>

I will pull into my branch and send PR as is unless someone has more to
say.

-Stafford

> -- 
> 2.12.1
> 

Re: [Qemu-devel] [PATCH 1/2] target/openrisc: Implement EVBAR register

[Richard Henderson]

On 04/18/2017 05:47 AM, Stafford Horne wrote:
> On Tue, Apr 18, 2017 at 04:15:50PM +1000, Tim 'mithro' Ansell wrote:
>> Exception Vector Base Address Register (EVBAR) - This optional register
>> can be used to apply an offset to the exception vector addresses.
>>
>> The significant bits (31-12) of the vector offset address for each
>> exception depend on the setting of the Supervision Register (SR)'s EPH
>> bit and the Exception Vector Base Address Register (EVBAR).
>>
>> Its presence is indicated by the EVBARP bit in the CPU Configuration
>> Register (CPUCFGR).
>>
>> Signed-off-by: Tim 'mithro' Ansell <mithro@mithis.com>
>> ---
>>  target/openrisc/cpu.c        | 2 ++
>>  target/openrisc/cpu.h        | 7 +++++++
>>  target/openrisc/interrupt.c  | 6 +++++-
>>  target/openrisc/sys_helper.c | 7 +++++++
>>  4 files changed, 21 insertions(+), 1 deletion(-)
>>
>> diff --git a/target/openrisc/cpu.c b/target/openrisc/cpu.c
>> index 7fd2b9a216..1524ed981a 100644
>> --- a/target/openrisc/cpu.c
>> +++ b/target/openrisc/cpu.c
>> @@ -134,6 +134,7 @@ static void or1200_initfn(Object *obj)
>>
>>      set_feature(cpu, OPENRISC_FEATURE_OB32S);
>>      set_feature(cpu, OPENRISC_FEATURE_OF32S);
>> +    set_feature(cpu, OPENRISC_FEATURE_EVBAR);
>>  }
>>
>>  static void openrisc_any_initfn(Object *obj)
>> @@ -141,6 +142,7 @@ static void openrisc_any_initfn(Object *obj)
>>      OpenRISCCPU *cpu = OPENRISC_CPU(obj);
>>
>>      set_feature(cpu, OPENRISC_FEATURE_OB32S);
>> +    set_feature(cpu, OPENRISC_FEATURE_EVBAR);
>>  }
>>
>>  typedef struct OpenRISCCPUInfo {
>> diff --git a/target/openrisc/cpu.h b/target/openrisc/cpu.h
>> index 418a0e6960..1958b72718 100644
>> --- a/target/openrisc/cpu.h
>> +++ b/target/openrisc/cpu.h
>> @@ -111,6 +111,11 @@ enum {
>>      CPUCFGR_OF32S = (1 << 7),
>>      CPUCFGR_OF64S = (1 << 8),
>>      CPUCFGR_OV64S = (1 << 9),
>> +    /* CPUCFGR_ND = (1 << 10), */
>> +    /* CPUCFGR_AVRP = (1 << 11), */
>> +    CPUCFGR_EVBARP = (1 << 12),
>> +    /* CPUCFGR_ISRP = (1 << 13), */
>> +    /* CPUCFGR_AECSRP = (1 << 14), */
>>  };
>>
>>  /* DMMU configure register */
>> @@ -200,6 +205,7 @@ enum {
>>      OPENRISC_FEATURE_OF32S = (1 << 7),
>>      OPENRISC_FEATURE_OF64S = (1 << 8),
>>      OPENRISC_FEATURE_OV64S = (1 << 9),
>> +    OPENRISC_FEATURE_EVBAR = (1 << 12),
>
> Does anyone know why we have to add both Features and CPUCFG bits?
>
> It seems like duplication to me.

It's probably a mistake.  I certainly see no reason for the FEATURE bits; just 
rely on the CPUCFGR definitions.

> If we add something to CPUOpenRISCState, we ideally need to also add it to
> machine.c vmstate definition.  However, I currently have a patch out to
> rework the vmstate serialization.  I can add this to that patch.

Sure.


r~

Re: [Qemu-devel] [PATCH 1/2] target/openrisc: Implement EVBAR register

[Tim Ansell]

I'm about to add support for disabling the inbuilt or1k timer peripheral
(as our SoC does not have it enabled). That isn't really a CPU feature so I
think it still makes sense to have some type of feature field? Maybe CPU
features should just be a separate category and set directly on that
register?

I also thinks it makes sense to maybe define a couple of new CPU
configurations. I think we might want,

 * or1k-all - OpenRISC CPU with all emulated features enabled.
 * or1k-minimal - Only features enabled which can't be disabled.
 * mor1k-default - Configured to match the defaults in the mor1k verilog
repo.

Then I think we also want configs for the "major users" of mor1k like,

 * mor1k-litex - Configured to match the default config used by litex/misoc
(maybe mor1k-misoc as an alias?)
 * mor1k-fusesoc - Configured to match the default of FuseSoC
 * mor1k-minsoc - Configured to match the default of minsoc (which is
different to misoc)

Thoughts?

Tim 'mithro' Ansell

On Apr 18, 2017 10:47 PM, "Stafford Horne" <shorne@gmail.com> wrote:

On Tue, Apr 18, 2017 at 04:15:50PM +1000, Tim 'mithro' Ansell wrote:
> Exception Vector Base Address Register (EVBAR) - This optional register
> can be used to apply an offset to the exception vector addresses.
>
> The significant bits (31-12) of the vector offset address for each
> exception depend on the setting of the Supervision Register (SR)'s EPH
> bit and the Exception Vector Base Address Register (EVBAR).
>
> Its presence is indicated by the EVBARP bit in the CPU Configuration
> Register (CPUCFGR).
>
> Signed-off-by: Tim 'mithro' Ansell <mithro@mithis.com>
> ---
>  target/openrisc/cpu.c        | 2 ++
>  target/openrisc/cpu.h        | 7 +++++++
>  target/openrisc/interrupt.c  | 6 +++++-
>  target/openrisc/sys_helper.c | 7 +++++++
>  4 files changed, 21 insertions(+), 1 deletion(-)
>
> diff --git a/target/openrisc/cpu.c b/target/openrisc/cpu.c
> index 7fd2b9a216..1524ed981a 100644
> --- a/target/openrisc/cpu.c
> +++ b/target/openrisc/cpu.c
> @@ -134,6 +134,7 @@ static void or1200_initfn(Object *obj)
>
>      set_feature(cpu, OPENRISC_FEATURE_OB32S);
>      set_feature(cpu, OPENRISC_FEATURE_OF32S);
> +    set_feature(cpu, OPENRISC_FEATURE_EVBAR);
>  }
>
>  static void openrisc_any_initfn(Object *obj)
> @@ -141,6 +142,7 @@ static void openrisc_any_initfn(Object *obj)
>      OpenRISCCPU *cpu = OPENRISC_CPU(obj);
>
>      set_feature(cpu, OPENRISC_FEATURE_OB32S);
> +    set_feature(cpu, OPENRISC_FEATURE_EVBAR);
>  }
>
>  typedef struct OpenRISCCPUInfo {
> diff --git a/target/openrisc/cpu.h b/target/openrisc/cpu.h
> index 418a0e6960..1958b72718 100644
> --- a/target/openrisc/cpu.h
> +++ b/target/openrisc/cpu.h
> @@ -111,6 +111,11 @@ enum {
>      CPUCFGR_OF32S = (1 << 7),
>      CPUCFGR_OF64S = (1 << 8),
>      CPUCFGR_OV64S = (1 << 9),
> +    /* CPUCFGR_ND = (1 << 10), */
> +    /* CPUCFGR_AVRP = (1 << 11), */
> +    CPUCFGR_EVBARP = (1 << 12),
> +    /* CPUCFGR_ISRP = (1 << 13), */
> +    /* CPUCFGR_AECSRP = (1 << 14), */
>  };
>
>  /* DMMU configure register */
> @@ -200,6 +205,7 @@ enum {
>      OPENRISC_FEATURE_OF32S = (1 << 7),
>      OPENRISC_FEATURE_OF64S = (1 << 8),
>      OPENRISC_FEATURE_OV64S = (1 << 9),
> +    OPENRISC_FEATURE_EVBAR = (1 << 12),

Does anyone know why we have to add both Features and CPUCFG bits?

It seems like duplication to me.

>  };
>
>  /* Tick Timer Mode Register */
> @@ -289,6 +295,7 @@ typedef struct CPUOpenRISCState {
>      uint32_t dmmucfgr;        /* DMMU configure register */
>      uint32_t immucfgr;        /* IMMU configure register */
>      uint32_t esr;             /* Exception supervisor register */
> +    uint32_t evbar;           /* Exception vector base address register
*/

If we add something to CPUOpenRISCState, we ideally need to also add it to
machine.c vmstate definition.  However, I currently have a patch out to
rework the vmstate serialization.  I can add this to that patch.

>      uint32_t fpcsr;           /* Float register */
>      float_status fp_status;
>
> diff --git a/target/openrisc/interrupt.c b/target/openrisc/interrupt.c
> index a2eec6fb32..78f0ba9421 100644
> --- a/target/openrisc/interrupt.c
> +++ b/target/openrisc/interrupt.c
> @@ -65,7 +65,11 @@ void openrisc_cpu_do_interrupt(CPUState *cs)
>      env->lock_addr = -1;
>
>      if (cs->exception_index > 0 && cs->exception_index < EXCP_NR) {
> -        env->pc = (cs->exception_index << 8);
> +        hwaddr vect_pc = cs->exception_index << 8;
> +        if (env->cpucfgr & CPUCFGR_EVBARP) {
> +            vect_pc |= env->evbar;
> +        }
> +        env->pc = vect_pc;
>      } else {
>          cpu_abort(cs, "Unhandled exception 0x%x\n", cs->exception_index);
>      }
> diff --git a/target/openrisc/sys_helper.c b/target/openrisc/sys_helper.c
> index 60c3193656..6ba816249b 100644
> --- a/target/openrisc/sys_helper.c
> +++ b/target/openrisc/sys_helper.c
> @@ -39,6 +39,10 @@ void HELPER(mtspr)(CPUOpenRISCState *env,
>          env->vr = rb;
>          break;
>
> +    case TO_SPR(0, 11): /* EVBAR */
> +        env->evbar = rb;
> +        break;
> +
>      case TO_SPR(0, 16): /* NPC */
>          cpu_restore_state(cs, GETPC());
>          /* ??? Mirror or1ksim in not trashing delayed branch state
> @@ -206,6 +210,9 @@ target_ulong HELPER(mfspr)(CPUOpenRISCState *env,
>      case TO_SPR(0, 4): /* IMMUCFGR */
>          return env->immucfgr;
>
> +    case TO_SPR(0, 11): /* EVBAR */
> +        return env->evbar;
> +
>      case TO_SPR(0, 16): /* NPC (equals PC) */
>          cpu_restore_state(cs, GETPC());
>          return env->pc;

Reviewed-by: Stafford Horne <shorne@gmail.com>

I will pull into my branch and send PR as is unless someone has more to
say.

-Stafford

> --
> 2.12.1
>

Re: [Qemu-devel] [PATCH 1/2] target/openrisc: Implement EVBAR register

[Stafford Horne]

On Thu, Apr 27, 2017 at 10:55:28AM +1000, Tim Ansell wrote:
> I'm about to add support for disabling the inbuilt or1k timer peripheral
> (as our SoC does not have it enabled). That isn't really a CPU feature so I
> think it still makes sense to have some type of feature field? Maybe CPU
> features should just be a separate category and set directly on that
> register?
> 
> I also thinks it makes sense to maybe define a couple of new CPU
> configurations. I think we might want,
> 
>  * or1k-all - OpenRISC CPU with all emulated features enabled.
>  * or1k-minimal - Only features enabled which can't be disabled.
>  * mor1k-default - Configured to match the defaults in the mor1k verilog
> repo.
> 
> Then I think we also want configs for the "major users" of mor1k like,
> 
>  * mor1k-litex - Configured to match the default config used by litex/misoc
> (maybe mor1k-misoc as an alias?)
>  * mor1k-fusesoc - Configured to match the default of FuseSoC
>  * mor1k-minsoc - Configured to match the default of minsoc (which is
> different to misoc)
> 
> Thoughts?

I think this makes sense, we have the UPR and CPUCFGR read only SPR's to
track many of these features.  Currently specifying a different model will
change the cpucfgr.  We could expand to also define UPR as well to give
something like like below.

static void or1200_initfn(Object *obj)
{
    OpenRISCCPU *cpu = OPENRISC_CPU(obj);

    cpu->env.cpucfgr = CPUCFGR_NSGF | CPUCFGR_OB32S | CPUCFGR_OF32S |
                       CPUCFGR_EVBARP;

    cpu->env.upr = UPR_UP | UPR_DMP | UPR_IMP | UPR_PICP | UPR_TTP |
                   UPR_PMP;

    /* Features not supported in SPR config */
    cpu->env.features = /* Any examples? */
}

static void or1k_minimal_initfn(Object *obj)
{
    OpenRISCCPU *cpu = OPENRISC_CPU(obj);

    /* Nothing Enabled? */
}


static const OpenRISCCPUInfo openrisc_cpus[] = {
    { .name = "or1200",      .initfn = or1200_initfn },
    { .name = "or1k-all",    .initfn = or1200_initfn },
    { .name = "or1k-minimal",.initfn = or1k_miniamal_initfn },
    ...
    { .name = "any",         .initfn = openrisc_any_initfn },
};


These could then be used in different parts of the code to track if we
actually enable those features or not.  i.e:

    if (cpu->env.upr & UPR_PICP) {
        cpu_openrisc_pic_init(cpu);
    }
    if (cpu->env.upr & UPR_TTP) {
        cpu_openrisc_clock_init(cpu);
    }

I think its possible and it will be a bit of work to get done.

Thoughts?

-Stafford

> Tim 'mithro' Ansell
> 
> On Apr 18, 2017 10:47 PM, "Stafford Horne" <shorne@gmail.com> wrote:
> 
> On Tue, Apr 18, 2017 at 04:15:50PM +1000, Tim 'mithro' Ansell wrote:
> > Exception Vector Base Address Register (EVBAR) - This optional register
> > can be used to apply an offset to the exception vector addresses.
> >
> > The significant bits (31-12) of the vector offset address for each
> > exception depend on the setting of the Supervision Register (SR)'s EPH
> > bit and the Exception Vector Base Address Register (EVBAR).
> >
> > Its presence is indicated by the EVBARP bit in the CPU Configuration
> > Register (CPUCFGR).
> >
> > Signed-off-by: Tim 'mithro' Ansell <mithro@mithis.com>
> > ---
> >  target/openrisc/cpu.c        | 2 ++
> >  target/openrisc/cpu.h        | 7 +++++++
> >  target/openrisc/interrupt.c  | 6 +++++-
> >  target/openrisc/sys_helper.c | 7 +++++++
> >  4 files changed, 21 insertions(+), 1 deletion(-)
> >
> > diff --git a/target/openrisc/cpu.c b/target/openrisc/cpu.c
> > index 7fd2b9a216..1524ed981a 100644
> > --- a/target/openrisc/cpu.c
> > +++ b/target/openrisc/cpu.c
> > @@ -134,6 +134,7 @@ static void or1200_initfn(Object *obj)
> >
> >      set_feature(cpu, OPENRISC_FEATURE_OB32S);
> >      set_feature(cpu, OPENRISC_FEATURE_OF32S);
> > +    set_feature(cpu, OPENRISC_FEATURE_EVBAR);
> >  }
> >
> >  static void openrisc_any_initfn(Object *obj)
> > @@ -141,6 +142,7 @@ static void openrisc_any_initfn(Object *obj)
> >      OpenRISCCPU *cpu = OPENRISC_CPU(obj);
> >
> >      set_feature(cpu, OPENRISC_FEATURE_OB32S);
> > +    set_feature(cpu, OPENRISC_FEATURE_EVBAR);
> >  }
> >
> >  typedef struct OpenRISCCPUInfo {
> > diff --git a/target/openrisc/cpu.h b/target/openrisc/cpu.h
> > index 418a0e6960..1958b72718 100644
> > --- a/target/openrisc/cpu.h
> > +++ b/target/openrisc/cpu.h
> > @@ -111,6 +111,11 @@ enum {
> >      CPUCFGR_OF32S = (1 << 7),
> >      CPUCFGR_OF64S = (1 << 8),
> >      CPUCFGR_OV64S = (1 << 9),
> > +    /* CPUCFGR_ND = (1 << 10), */
> > +    /* CPUCFGR_AVRP = (1 << 11), */
> > +    CPUCFGR_EVBARP = (1 << 12),
> > +    /* CPUCFGR_ISRP = (1 << 13), */
> > +    /* CPUCFGR_AECSRP = (1 << 14), */
> >  };
> >
> >  /* DMMU configure register */
> > @@ -200,6 +205,7 @@ enum {
> >      OPENRISC_FEATURE_OF32S = (1 << 7),
> >      OPENRISC_FEATURE_OF64S = (1 << 8),
> >      OPENRISC_FEATURE_OV64S = (1 << 9),
> > +    OPENRISC_FEATURE_EVBAR = (1 << 12),
> 
> Does anyone know why we have to add both Features and CPUCFG bits?
> 
> It seems like duplication to me.
> 
> >  };
> >
> >  /* Tick Timer Mode Register */
> > @@ -289,6 +295,7 @@ typedef struct CPUOpenRISCState {
> >      uint32_t dmmucfgr;        /* DMMU configure register */
> >      uint32_t immucfgr;        /* IMMU configure register */
> >      uint32_t esr;             /* Exception supervisor register */
> > +    uint32_t evbar;           /* Exception vector base address register
> */
> 
> If we add something to CPUOpenRISCState, we ideally need to also add it to
> machine.c vmstate definition.  However, I currently have a patch out to
> rework the vmstate serialization.  I can add this to that patch.
> 
> >      uint32_t fpcsr;           /* Float register */
> >      float_status fp_status;
> >
> > diff --git a/target/openrisc/interrupt.c b/target/openrisc/interrupt.c
> > index a2eec6fb32..78f0ba9421 100644
> > --- a/target/openrisc/interrupt.c
> > +++ b/target/openrisc/interrupt.c
> > @@ -65,7 +65,11 @@ void openrisc_cpu_do_interrupt(CPUState *cs)
> >      env->lock_addr = -1;
> >
> >      if (cs->exception_index > 0 && cs->exception_index < EXCP_NR) {
> > -        env->pc = (cs->exception_index << 8);
> > +        hwaddr vect_pc = cs->exception_index << 8;
> > +        if (env->cpucfgr & CPUCFGR_EVBARP) {
> > +            vect_pc |= env->evbar;
> > +        }
> > +        env->pc = vect_pc;
> >      } else {
> >          cpu_abort(cs, "Unhandled exception 0x%x\n", cs->exception_index);
> >      }
> > diff --git a/target/openrisc/sys_helper.c b/target/openrisc/sys_helper.c
> > index 60c3193656..6ba816249b 100644
> > --- a/target/openrisc/sys_helper.c
> > +++ b/target/openrisc/sys_helper.c
> > @@ -39,6 +39,10 @@ void HELPER(mtspr)(CPUOpenRISCState *env,
> >          env->vr = rb;
> >          break;
> >
> > +    case TO_SPR(0, 11): /* EVBAR */
> > +        env->evbar = rb;
> > +        break;
> > +
> >      case TO_SPR(0, 16): /* NPC */
> >          cpu_restore_state(cs, GETPC());
> >          /* ??? Mirror or1ksim in not trashing delayed branch state
> > @@ -206,6 +210,9 @@ target_ulong HELPER(mfspr)(CPUOpenRISCState *env,
> >      case TO_SPR(0, 4): /* IMMUCFGR */
> >          return env->immucfgr;
> >
> > +    case TO_SPR(0, 11): /* EVBAR */
> > +        return env->evbar;
> > +
> >      case TO_SPR(0, 16): /* NPC (equals PC) */
> >          cpu_restore_state(cs, GETPC());
> >          return env->pc;
> 
> Reviewed-by: Stafford Horne <shorne@gmail.com>
> 
> I will pull into my branch and send PR as is unless someone has more to
> say.
> 
> -Stafford
> 
> > --
> > 2.12.1
> >