subject:"Re\: \[PATCH v2 2\/9\] target\/riscv\: Don't overwrite the reset vector"

Re: [PATCH v2 2/9] target/riscv: Don't overwrite the reset vector

2020-05-19 Thread Alistair Francis

On Sat, May 16, 2020 at 2:03 AM Bin Meng  wrote:
>
> On Sat, May 16, 2020 at 3:51 AM Alistair Francis  wrote:
> >
> > On Thu, May 14, 2020 at 9:54 PM Bin Meng  wrote:
> > >
> > > On Fri, May 15, 2020 at 5:51 AM Alistair Francis  
> > > wrote:
> > > >
> > > > On Thu, May 14, 2020 at 10:54 AM Philippe Mathieu-Daudé
> > > >  wrote:
> > > > >
> > > > > On 5/7/20 9:13 PM, Alistair Francis wrote:
> > > > > > If the reset vector is set in the init function don't set it again 
> > > > > > in
> > > > > > realise.
> > > > >
> > > > > typo "realize".
> > > >
> > > > It's not a typo, just correct English :)
> > > >
> > > > I have changed it.
> > > >
> > > > >
> > > > > >
> > > > > > Signed-off-by: Alistair Francis 
> > > > > > ---
> > > > > >   target/riscv/cpu.c | 20 +++-
> > > > > >   1 file changed, 11 insertions(+), 9 deletions(-)
> > > > > >
> > > > > > diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
> > > > > > index 059d71f2c7..8f837edf8d 100644
> > > > > > --- a/target/riscv/cpu.c
> > > > > > +++ b/target/riscv/cpu.c
> > > > > > @@ -111,6 +111,14 @@ static void set_feature(CPURISCVState *env, 
> > > > > > int feature)
> > > > > >   env->features |= (1ULL << feature);
> > > > > >   }
> > > > > >
> > > > > > +static int get_resetvec(CPURISCVState *env)
> > > > > > +{
> > > > > > +#ifndef CONFIG_USER_ONLY
> > > > > > +return env->resetvec;
> > > > > > +#endif
> > > > > > +return 0;
> > > > >
> > > > > Don't you get an error about double return? Maybe use #else?
> > > >
> > > > Apparently not, I have changed it though.
> > > >
> > > > Alistair
> > > >
> > > > >
> > > > > > +}
> > > > > > +
> > > > > >   static void set_resetvec(CPURISCVState *env, int resetvec)
> > > > > >   {
> > > > > >   #ifndef CONFIG_USER_ONLY
> > > > > > @@ -123,7 +131,6 @@ static void riscv_any_cpu_init(Object *obj)
> > > > > >   CPURISCVState *env = _CPU(obj)->env;
> > > > > >   set_misa(env, RVXLEN | RVI | RVM | RVA | RVF | RVD | RVC | 
> > > > > > RVU);
> > > > > >   set_priv_version(env, PRIV_VERSION_1_11_0);
> > > > > > -set_resetvec(env, DEFAULT_RSTVEC);
> > > > > >   }
> > > > > >
> > > > > >   #if defined(TARGET_RISCV32)
> > > > > > @@ -140,7 +147,6 @@ static void rv32gcsu_priv1_09_1_cpu_init(Object 
> > > > > > *obj)
> > > > > >   CPURISCVState *env = _CPU(obj)->env;
> > > > > >   set_misa(env, RV32 | RVI | RVM | RVA | RVF | RVD | RVC | RVS 
> > > > > > | RVU);
> > > > > >   set_priv_version(env, PRIV_VERSION_1_09_1);
> > > > > > -set_resetvec(env, DEFAULT_RSTVEC);
> > > > > >   set_feature(env, RISCV_FEATURE_MMU);
> > > > > >   set_feature(env, RISCV_FEATURE_PMP);
> > > > > >   }
> > > > > > @@ -150,7 +156,6 @@ static void rv32gcsu_priv1_10_0_cpu_init(Object 
> > > > > > *obj)
> > > > > >   CPURISCVState *env = _CPU(obj)->env;
> > > > > >   set_misa(env, RV32 | RVI | RVM | RVA | RVF | RVD | RVC | RVS 
> > > > > > | RVU);
> > > > > >   set_priv_version(env, PRIV_VERSION_1_10_0);
> > > > > > -set_resetvec(env, DEFAULT_RSTVEC);
> > > > > >   set_feature(env, RISCV_FEATURE_MMU);
> > > > > >   set_feature(env, RISCV_FEATURE_PMP);
> > > > > >   }
> > > > > > @@ -160,7 +165,6 @@ static void rv32imacu_nommu_cpu_init(Object 
> > > > > > *obj)
> > > > > >   CPURISCVState *env = _CPU(obj)->env;
> > > > > >   set_misa(env, RV32 | RVI | RVM | RVA | RVC | RVU);
> > > > > >   set_priv_version(env, PRIV_VERSION_1_10_0);
> > > > > > -set_resetvec(env, DEFAULT_RSTVEC);
> > > > > >   set_feature(env, RISCV_FEATURE_PMP);
> > > > > >   }
> > > > > >
> > > > > > @@ -169,7 +173,6 @@ static void rv32imafcu_nommu_cpu_init(Object 
> > > > > > *obj)
> > > > > >   CPURISCVState *env = _CPU(obj)->env;
> > > > > >   set_misa(env, RV32 | RVI | RVM | RVA | RVF | RVC | RVU);
> > > > > >   set_priv_version(env, PRIV_VERSION_1_10_0);
> > > > > > -set_resetvec(env, DEFAULT_RSTVEC);
> > > > > >   set_feature(env, RISCV_FEATURE_PMP);
> > > > > >   }
> > > > > >
> > > > > > @@ -187,7 +190,6 @@ static void rv64gcsu_priv1_09_1_cpu_init(Object 
> > > > > > *obj)
> > > > > >   CPURISCVState *env = _CPU(obj)->env;
> > > > > >   set_misa(env, RV64 | RVI | RVM | RVA | RVF | RVD | RVC | RVS 
> > > > > > | RVU);
> > > > > >   set_priv_version(env, PRIV_VERSION_1_09_1);
> > > > > > -set_resetvec(env, DEFAULT_RSTVEC);
> > > > > >   set_feature(env, RISCV_FEATURE_MMU);
> > > > > >   set_feature(env, RISCV_FEATURE_PMP);
> > > > > >   }
> > > > > > @@ -197,7 +199,6 @@ static void rv64gcsu_priv1_10_0_cpu_init(Object 
> > > > > > *obj)
> > > > > >   CPURISCVState *env = _CPU(obj)->env;
> > > > > >   set_misa(env, RV64 | RVI | RVM | RVA | RVF | RVD | RVC | RVS 
> > > > > > | RVU);
> > > > > >   set_priv_version(env, PRIV_VERSION_1_10_0);
> > > > > > -set_resetvec(env, DEFAULT_RSTVEC);
> > > > > >   set_feature(env, RISCV_FEATURE_MMU);
> > > > > >   set_feature(env, RISCV_FEATURE_PMP);
> > > > > >   }
>

Re: [PATCH v2 2/9] target/riscv: Don't overwrite the reset vector

2020-05-16 Thread Bin Meng

On Sat, May 16, 2020 at 3:51 AM Alistair Francis  wrote:
>
> On Thu, May 14, 2020 at 9:54 PM Bin Meng  wrote:
> >
> > On Fri, May 15, 2020 at 5:51 AM Alistair Francis  
> > wrote:
> > >
> > > On Thu, May 14, 2020 at 10:54 AM Philippe Mathieu-Daudé
> > >  wrote:
> > > >
> > > > On 5/7/20 9:13 PM, Alistair Francis wrote:
> > > > > If the reset vector is set in the init function don't set it again in
> > > > > realise.
> > > >
> > > > typo "realize".
> > >
> > > It's not a typo, just correct English :)
> > >
> > > I have changed it.
> > >
> > > >
> > > > >
> > > > > Signed-off-by: Alistair Francis 
> > > > > ---
> > > > >   target/riscv/cpu.c | 20 +++-
> > > > >   1 file changed, 11 insertions(+), 9 deletions(-)
> > > > >
> > > > > diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
> > > > > index 059d71f2c7..8f837edf8d 100644
> > > > > --- a/target/riscv/cpu.c
> > > > > +++ b/target/riscv/cpu.c
> > > > > @@ -111,6 +111,14 @@ static void set_feature(CPURISCVState *env, int 
> > > > > feature)
> > > > >   env->features |= (1ULL << feature);
> > > > >   }
> > > > >
> > > > > +static int get_resetvec(CPURISCVState *env)
> > > > > +{
> > > > > +#ifndef CONFIG_USER_ONLY
> > > > > +return env->resetvec;
> > > > > +#endif
> > > > > +return 0;
> > > >
> > > > Don't you get an error about double return? Maybe use #else?
> > >
> > > Apparently not, I have changed it though.
> > >
> > > Alistair
> > >
> > > >
> > > > > +}
> > > > > +
> > > > >   static void set_resetvec(CPURISCVState *env, int resetvec)
> > > > >   {
> > > > >   #ifndef CONFIG_USER_ONLY
> > > > > @@ -123,7 +131,6 @@ static void riscv_any_cpu_init(Object *obj)
> > > > >   CPURISCVState *env = _CPU(obj)->env;
> > > > >   set_misa(env, RVXLEN | RVI | RVM | RVA | RVF | RVD | RVC | RVU);
> > > > >   set_priv_version(env, PRIV_VERSION_1_11_0);
> > > > > -set_resetvec(env, DEFAULT_RSTVEC);
> > > > >   }
> > > > >
> > > > >   #if defined(TARGET_RISCV32)
> > > > > @@ -140,7 +147,6 @@ static void rv32gcsu_priv1_09_1_cpu_init(Object 
> > > > > *obj)
> > > > >   CPURISCVState *env = _CPU(obj)->env;
> > > > >   set_misa(env, RV32 | RVI | RVM | RVA | RVF | RVD | RVC | RVS | 
> > > > > RVU);
> > > > >   set_priv_version(env, PRIV_VERSION_1_09_1);
> > > > > -set_resetvec(env, DEFAULT_RSTVEC);
> > > > >   set_feature(env, RISCV_FEATURE_MMU);
> > > > >   set_feature(env, RISCV_FEATURE_PMP);
> > > > >   }
> > > > > @@ -150,7 +156,6 @@ static void rv32gcsu_priv1_10_0_cpu_init(Object 
> > > > > *obj)
> > > > >   CPURISCVState *env = _CPU(obj)->env;
> > > > >   set_misa(env, RV32 | RVI | RVM | RVA | RVF | RVD | RVC | RVS | 
> > > > > RVU);
> > > > >   set_priv_version(env, PRIV_VERSION_1_10_0);
> > > > > -set_resetvec(env, DEFAULT_RSTVEC);
> > > > >   set_feature(env, RISCV_FEATURE_MMU);
> > > > >   set_feature(env, RISCV_FEATURE_PMP);
> > > > >   }
> > > > > @@ -160,7 +165,6 @@ static void rv32imacu_nommu_cpu_init(Object *obj)
> > > > >   CPURISCVState *env = _CPU(obj)->env;
> > > > >   set_misa(env, RV32 | RVI | RVM | RVA | RVC | RVU);
> > > > >   set_priv_version(env, PRIV_VERSION_1_10_0);
> > > > > -set_resetvec(env, DEFAULT_RSTVEC);
> > > > >   set_feature(env, RISCV_FEATURE_PMP);
> > > > >   }
> > > > >
> > > > > @@ -169,7 +173,6 @@ static void rv32imafcu_nommu_cpu_init(Object *obj)
> > > > >   CPURISCVState *env = _CPU(obj)->env;
> > > > >   set_misa(env, RV32 | RVI | RVM | RVA | RVF | RVC | RVU);
> > > > >   set_priv_version(env, PRIV_VERSION_1_10_0);
> > > > > -set_resetvec(env, DEFAULT_RSTVEC);
> > > > >   set_feature(env, RISCV_FEATURE_PMP);
> > > > >   }
> > > > >
> > > > > @@ -187,7 +190,6 @@ static void rv64gcsu_priv1_09_1_cpu_init(Object 
> > > > > *obj)
> > > > >   CPURISCVState *env = _CPU(obj)->env;
> > > > >   set_misa(env, RV64 | RVI | RVM | RVA | RVF | RVD | RVC | RVS | 
> > > > > RVU);
> > > > >   set_priv_version(env, PRIV_VERSION_1_09_1);
> > > > > -set_resetvec(env, DEFAULT_RSTVEC);
> > > > >   set_feature(env, RISCV_FEATURE_MMU);
> > > > >   set_feature(env, RISCV_FEATURE_PMP);
> > > > >   }
> > > > > @@ -197,7 +199,6 @@ static void rv64gcsu_priv1_10_0_cpu_init(Object 
> > > > > *obj)
> > > > >   CPURISCVState *env = _CPU(obj)->env;
> > > > >   set_misa(env, RV64 | RVI | RVM | RVA | RVF | RVD | RVC | RVS | 
> > > > > RVU);
> > > > >   set_priv_version(env, PRIV_VERSION_1_10_0);
> > > > > -set_resetvec(env, DEFAULT_RSTVEC);
> > > > >   set_feature(env, RISCV_FEATURE_MMU);
> > > > >   set_feature(env, RISCV_FEATURE_PMP);
> > > > >   }
> > > > > @@ -207,7 +208,6 @@ static void rv64imacu_nommu_cpu_init(Object *obj)
> > > > >   CPURISCVState *env = _CPU(obj)->env;
> > > > >   set_misa(env, RV64 | RVI | RVM | RVA | RVC | RVU);
> > > > >   set_priv_version(env, PRIV_VERSION_1_10_0);
> > > > > -set_resetvec(env, DEFAULT_RSTVEC);
> > > > >   set_feature(env,

Re: [PATCH v2 2/9] target/riscv: Don't overwrite the reset vector

2020-05-15 Thread Alistair Francis

On Thu, May 14, 2020 at 9:54 PM Bin Meng  wrote:
>
> On Fri, May 15, 2020 at 5:51 AM Alistair Francis  wrote:
> >
> > On Thu, May 14, 2020 at 10:54 AM Philippe Mathieu-Daudé
> >  wrote:
> > >
> > > On 5/7/20 9:13 PM, Alistair Francis wrote:
> > > > If the reset vector is set in the init function don't set it again in
> > > > realise.
> > >
> > > typo "realize".
> >
> > It's not a typo, just correct English :)
> >
> > I have changed it.
> >
> > >
> > > >
> > > > Signed-off-by: Alistair Francis 
> > > > ---
> > > >   target/riscv/cpu.c | 20 +++-
> > > >   1 file changed, 11 insertions(+), 9 deletions(-)
> > > >
> > > > diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
> > > > index 059d71f2c7..8f837edf8d 100644
> > > > --- a/target/riscv/cpu.c
> > > > +++ b/target/riscv/cpu.c
> > > > @@ -111,6 +111,14 @@ static void set_feature(CPURISCVState *env, int 
> > > > feature)
> > > >   env->features |= (1ULL << feature);
> > > >   }
> > > >
> > > > +static int get_resetvec(CPURISCVState *env)
> > > > +{
> > > > +#ifndef CONFIG_USER_ONLY
> > > > +return env->resetvec;
> > > > +#endif
> > > > +return 0;
> > >
> > > Don't you get an error about double return? Maybe use #else?
> >
> > Apparently not, I have changed it though.
> >
> > Alistair
> >
> > >
> > > > +}
> > > > +
> > > >   static void set_resetvec(CPURISCVState *env, int resetvec)
> > > >   {
> > > >   #ifndef CONFIG_USER_ONLY
> > > > @@ -123,7 +131,6 @@ static void riscv_any_cpu_init(Object *obj)
> > > >   CPURISCVState *env = _CPU(obj)->env;
> > > >   set_misa(env, RVXLEN | RVI | RVM | RVA | RVF | RVD | RVC | RVU);
> > > >   set_priv_version(env, PRIV_VERSION_1_11_0);
> > > > -set_resetvec(env, DEFAULT_RSTVEC);
> > > >   }
> > > >
> > > >   #if defined(TARGET_RISCV32)
> > > > @@ -140,7 +147,6 @@ static void rv32gcsu_priv1_09_1_cpu_init(Object 
> > > > *obj)
> > > >   CPURISCVState *env = _CPU(obj)->env;
> > > >   set_misa(env, RV32 | RVI | RVM | RVA | RVF | RVD | RVC | RVS | 
> > > > RVU);
> > > >   set_priv_version(env, PRIV_VERSION_1_09_1);
> > > > -set_resetvec(env, DEFAULT_RSTVEC);
> > > >   set_feature(env, RISCV_FEATURE_MMU);
> > > >   set_feature(env, RISCV_FEATURE_PMP);
> > > >   }
> > > > @@ -150,7 +156,6 @@ static void rv32gcsu_priv1_10_0_cpu_init(Object 
> > > > *obj)
> > > >   CPURISCVState *env = _CPU(obj)->env;
> > > >   set_misa(env, RV32 | RVI | RVM | RVA | RVF | RVD | RVC | RVS | 
> > > > RVU);
> > > >   set_priv_version(env, PRIV_VERSION_1_10_0);
> > > > -set_resetvec(env, DEFAULT_RSTVEC);
> > > >   set_feature(env, RISCV_FEATURE_MMU);
> > > >   set_feature(env, RISCV_FEATURE_PMP);
> > > >   }
> > > > @@ -160,7 +165,6 @@ static void rv32imacu_nommu_cpu_init(Object *obj)
> > > >   CPURISCVState *env = _CPU(obj)->env;
> > > >   set_misa(env, RV32 | RVI | RVM | RVA | RVC | RVU);
> > > >   set_priv_version(env, PRIV_VERSION_1_10_0);
> > > > -set_resetvec(env, DEFAULT_RSTVEC);
> > > >   set_feature(env, RISCV_FEATURE_PMP);
> > > >   }
> > > >
> > > > @@ -169,7 +173,6 @@ static void rv32imafcu_nommu_cpu_init(Object *obj)
> > > >   CPURISCVState *env = _CPU(obj)->env;
> > > >   set_misa(env, RV32 | RVI | RVM | RVA | RVF | RVC | RVU);
> > > >   set_priv_version(env, PRIV_VERSION_1_10_0);
> > > > -set_resetvec(env, DEFAULT_RSTVEC);
> > > >   set_feature(env, RISCV_FEATURE_PMP);
> > > >   }
> > > >
> > > > @@ -187,7 +190,6 @@ static void rv64gcsu_priv1_09_1_cpu_init(Object 
> > > > *obj)
> > > >   CPURISCVState *env = _CPU(obj)->env;
> > > >   set_misa(env, RV64 | RVI | RVM | RVA | RVF | RVD | RVC | RVS | 
> > > > RVU);
> > > >   set_priv_version(env, PRIV_VERSION_1_09_1);
> > > > -set_resetvec(env, DEFAULT_RSTVEC);
> > > >   set_feature(env, RISCV_FEATURE_MMU);
> > > >   set_feature(env, RISCV_FEATURE_PMP);
> > > >   }
> > > > @@ -197,7 +199,6 @@ static void rv64gcsu_priv1_10_0_cpu_init(Object 
> > > > *obj)
> > > >   CPURISCVState *env = _CPU(obj)->env;
> > > >   set_misa(env, RV64 | RVI | RVM | RVA | RVF | RVD | RVC | RVS | 
> > > > RVU);
> > > >   set_priv_version(env, PRIV_VERSION_1_10_0);
> > > > -set_resetvec(env, DEFAULT_RSTVEC);
> > > >   set_feature(env, RISCV_FEATURE_MMU);
> > > >   set_feature(env, RISCV_FEATURE_PMP);
> > > >   }
> > > > @@ -207,7 +208,6 @@ static void rv64imacu_nommu_cpu_init(Object *obj)
> > > >   CPURISCVState *env = _CPU(obj)->env;
> > > >   set_misa(env, RV64 | RVI | RVM | RVA | RVC | RVU);
> > > >   set_priv_version(env, PRIV_VERSION_1_10_0);
> > > > -set_resetvec(env, DEFAULT_RSTVEC);
> > > >   set_feature(env, RISCV_FEATURE_PMP);
> > > >   }
> > > >
> > > > @@ -399,7 +399,9 @@ static void riscv_cpu_realize(DeviceState *dev, 
> > > > Error **errp)
> > > >   }
> > > >
> > > >   set_priv_version(env, priv_version);
> > > > -set_resetvec(env, DEFAULT_RSTVEC);
> > > > +if (!get_resetvec(env)) {
>
>

Re: [PATCH v2 2/9] target/riscv: Don't overwrite the reset vector

2020-05-14 Thread Bin Meng

On Fri, May 15, 2020 at 5:51 AM Alistair Francis  wrote:
>
> On Thu, May 14, 2020 at 10:54 AM Philippe Mathieu-Daudé
>  wrote:
> >
> > On 5/7/20 9:13 PM, Alistair Francis wrote:
> > > If the reset vector is set in the init function don't set it again in
> > > realise.
> >
> > typo "realize".
>
> It's not a typo, just correct English :)
>
> I have changed it.
>
> >
> > >
> > > Signed-off-by: Alistair Francis 
> > > ---
> > >   target/riscv/cpu.c | 20 +++-
> > >   1 file changed, 11 insertions(+), 9 deletions(-)
> > >
> > > diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
> > > index 059d71f2c7..8f837edf8d 100644
> > > --- a/target/riscv/cpu.c
> > > +++ b/target/riscv/cpu.c
> > > @@ -111,6 +111,14 @@ static void set_feature(CPURISCVState *env, int 
> > > feature)
> > >   env->features |= (1ULL << feature);
> > >   }
> > >
> > > +static int get_resetvec(CPURISCVState *env)
> > > +{
> > > +#ifndef CONFIG_USER_ONLY
> > > +return env->resetvec;
> > > +#endif
> > > +return 0;
> >
> > Don't you get an error about double return? Maybe use #else?
>
> Apparently not, I have changed it though.
>
> Alistair
>
> >
> > > +}
> > > +
> > >   static void set_resetvec(CPURISCVState *env, int resetvec)
> > >   {
> > >   #ifndef CONFIG_USER_ONLY
> > > @@ -123,7 +131,6 @@ static void riscv_any_cpu_init(Object *obj)
> > >   CPURISCVState *env = _CPU(obj)->env;
> > >   set_misa(env, RVXLEN | RVI | RVM | RVA | RVF | RVD | RVC | RVU);
> > >   set_priv_version(env, PRIV_VERSION_1_11_0);
> > > -set_resetvec(env, DEFAULT_RSTVEC);
> > >   }
> > >
> > >   #if defined(TARGET_RISCV32)
> > > @@ -140,7 +147,6 @@ static void rv32gcsu_priv1_09_1_cpu_init(Object *obj)
> > >   CPURISCVState *env = _CPU(obj)->env;
> > >   set_misa(env, RV32 | RVI | RVM | RVA | RVF | RVD | RVC | RVS | RVU);
> > >   set_priv_version(env, PRIV_VERSION_1_09_1);
> > > -set_resetvec(env, DEFAULT_RSTVEC);
> > >   set_feature(env, RISCV_FEATURE_MMU);
> > >   set_feature(env, RISCV_FEATURE_PMP);
> > >   }
> > > @@ -150,7 +156,6 @@ static void rv32gcsu_priv1_10_0_cpu_init(Object *obj)
> > >   CPURISCVState *env = _CPU(obj)->env;
> > >   set_misa(env, RV32 | RVI | RVM | RVA | RVF | RVD | RVC | RVS | RVU);
> > >   set_priv_version(env, PRIV_VERSION_1_10_0);
> > > -set_resetvec(env, DEFAULT_RSTVEC);
> > >   set_feature(env, RISCV_FEATURE_MMU);
> > >   set_feature(env, RISCV_FEATURE_PMP);
> > >   }
> > > @@ -160,7 +165,6 @@ static void rv32imacu_nommu_cpu_init(Object *obj)
> > >   CPURISCVState *env = _CPU(obj)->env;
> > >   set_misa(env, RV32 | RVI | RVM | RVA | RVC | RVU);
> > >   set_priv_version(env, PRIV_VERSION_1_10_0);
> > > -set_resetvec(env, DEFAULT_RSTVEC);
> > >   set_feature(env, RISCV_FEATURE_PMP);
> > >   }
> > >
> > > @@ -169,7 +173,6 @@ static void rv32imafcu_nommu_cpu_init(Object *obj)
> > >   CPURISCVState *env = _CPU(obj)->env;
> > >   set_misa(env, RV32 | RVI | RVM | RVA | RVF | RVC | RVU);
> > >   set_priv_version(env, PRIV_VERSION_1_10_0);
> > > -set_resetvec(env, DEFAULT_RSTVEC);
> > >   set_feature(env, RISCV_FEATURE_PMP);
> > >   }
> > >
> > > @@ -187,7 +190,6 @@ static void rv64gcsu_priv1_09_1_cpu_init(Object *obj)
> > >   CPURISCVState *env = _CPU(obj)->env;
> > >   set_misa(env, RV64 | RVI | RVM | RVA | RVF | RVD | RVC | RVS | RVU);
> > >   set_priv_version(env, PRIV_VERSION_1_09_1);
> > > -set_resetvec(env, DEFAULT_RSTVEC);
> > >   set_feature(env, RISCV_FEATURE_MMU);
> > >   set_feature(env, RISCV_FEATURE_PMP);
> > >   }
> > > @@ -197,7 +199,6 @@ static void rv64gcsu_priv1_10_0_cpu_init(Object *obj)
> > >   CPURISCVState *env = _CPU(obj)->env;
> > >   set_misa(env, RV64 | RVI | RVM | RVA | RVF | RVD | RVC | RVS | RVU);
> > >   set_priv_version(env, PRIV_VERSION_1_10_0);
> > > -set_resetvec(env, DEFAULT_RSTVEC);
> > >   set_feature(env, RISCV_FEATURE_MMU);
> > >   set_feature(env, RISCV_FEATURE_PMP);
> > >   }
> > > @@ -207,7 +208,6 @@ static void rv64imacu_nommu_cpu_init(Object *obj)
> > >   CPURISCVState *env = _CPU(obj)->env;
> > >   set_misa(env, RV64 | RVI | RVM | RVA | RVC | RVU);
> > >   set_priv_version(env, PRIV_VERSION_1_10_0);
> > > -set_resetvec(env, DEFAULT_RSTVEC);
> > >   set_feature(env, RISCV_FEATURE_PMP);
> > >   }
> > >
> > > @@ -399,7 +399,9 @@ static void riscv_cpu_realize(DeviceState *dev, Error 
> > > **errp)
> > >   }
> > >
> > >   set_priv_version(env, priv_version);
> > > -set_resetvec(env, DEFAULT_RSTVEC);
> > > +if (!get_resetvec(env)) {

What if we have a RISC-V CPU whose reset vector is at address 0?

> > > +set_resetvec(env, DEFAULT_RSTVEC);
> > > +}
> > >
> > >   if (cpu->cfg.mmu) {
> > >   set_feature(env, RISCV_FEATURE_MMU);
> > >

Regards,
Bin

Re: [PATCH v2 2/9] target/riscv: Don't overwrite the reset vector

2020-05-14 Thread Alistair Francis

On Thu, May 14, 2020 at 10:54 AM Philippe Mathieu-Daudé
 wrote:
>
> On 5/7/20 9:13 PM, Alistair Francis wrote:
> > If the reset vector is set in the init function don't set it again in
> > realise.
>
> typo "realize".

It's not a typo, just correct English :)

I have changed it.

>
> >
> > Signed-off-by: Alistair Francis 
> > ---
> >   target/riscv/cpu.c | 20 +++-
> >   1 file changed, 11 insertions(+), 9 deletions(-)
> >
> > diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
> > index 059d71f2c7..8f837edf8d 100644
> > --- a/target/riscv/cpu.c
> > +++ b/target/riscv/cpu.c
> > @@ -111,6 +111,14 @@ static void set_feature(CPURISCVState *env, int 
> > feature)
> >   env->features |= (1ULL << feature);
> >   }
> >
> > +static int get_resetvec(CPURISCVState *env)
> > +{
> > +#ifndef CONFIG_USER_ONLY
> > +return env->resetvec;
> > +#endif
> > +return 0;
>
> Don't you get an error about double return? Maybe use #else?

Apparently not, I have changed it though.

Alistair

>
> > +}
> > +
> >   static void set_resetvec(CPURISCVState *env, int resetvec)
> >   {
> >   #ifndef CONFIG_USER_ONLY
> > @@ -123,7 +131,6 @@ static void riscv_any_cpu_init(Object *obj)
> >   CPURISCVState *env = _CPU(obj)->env;
> >   set_misa(env, RVXLEN | RVI | RVM | RVA | RVF | RVD | RVC | RVU);
> >   set_priv_version(env, PRIV_VERSION_1_11_0);
> > -set_resetvec(env, DEFAULT_RSTVEC);
> >   }
> >
> >   #if defined(TARGET_RISCV32)
> > @@ -140,7 +147,6 @@ static void rv32gcsu_priv1_09_1_cpu_init(Object *obj)
> >   CPURISCVState *env = _CPU(obj)->env;
> >   set_misa(env, RV32 | RVI | RVM | RVA | RVF | RVD | RVC | RVS | RVU);
> >   set_priv_version(env, PRIV_VERSION_1_09_1);
> > -set_resetvec(env, DEFAULT_RSTVEC);
> >   set_feature(env, RISCV_FEATURE_MMU);
> >   set_feature(env, RISCV_FEATURE_PMP);
> >   }
> > @@ -150,7 +156,6 @@ static void rv32gcsu_priv1_10_0_cpu_init(Object *obj)
> >   CPURISCVState *env = _CPU(obj)->env;
> >   set_misa(env, RV32 | RVI | RVM | RVA | RVF | RVD | RVC | RVS | RVU);
> >   set_priv_version(env, PRIV_VERSION_1_10_0);
> > -set_resetvec(env, DEFAULT_RSTVEC);
> >   set_feature(env, RISCV_FEATURE_MMU);
> >   set_feature(env, RISCV_FEATURE_PMP);
> >   }
> > @@ -160,7 +165,6 @@ static void rv32imacu_nommu_cpu_init(Object *obj)
> >   CPURISCVState *env = _CPU(obj)->env;
> >   set_misa(env, RV32 | RVI | RVM | RVA | RVC | RVU);
> >   set_priv_version(env, PRIV_VERSION_1_10_0);
> > -set_resetvec(env, DEFAULT_RSTVEC);
> >   set_feature(env, RISCV_FEATURE_PMP);
> >   }
> >
> > @@ -169,7 +173,6 @@ static void rv32imafcu_nommu_cpu_init(Object *obj)
> >   CPURISCVState *env = _CPU(obj)->env;
> >   set_misa(env, RV32 | RVI | RVM | RVA | RVF | RVC | RVU);
> >   set_priv_version(env, PRIV_VERSION_1_10_0);
> > -set_resetvec(env, DEFAULT_RSTVEC);
> >   set_feature(env, RISCV_FEATURE_PMP);
> >   }
> >
> > @@ -187,7 +190,6 @@ static void rv64gcsu_priv1_09_1_cpu_init(Object *obj)
> >   CPURISCVState *env = _CPU(obj)->env;
> >   set_misa(env, RV64 | RVI | RVM | RVA | RVF | RVD | RVC | RVS | RVU);
> >   set_priv_version(env, PRIV_VERSION_1_09_1);
> > -set_resetvec(env, DEFAULT_RSTVEC);
> >   set_feature(env, RISCV_FEATURE_MMU);
> >   set_feature(env, RISCV_FEATURE_PMP);
> >   }
> > @@ -197,7 +199,6 @@ static void rv64gcsu_priv1_10_0_cpu_init(Object *obj)
> >   CPURISCVState *env = _CPU(obj)->env;
> >   set_misa(env, RV64 | RVI | RVM | RVA | RVF | RVD | RVC | RVS | RVU);
> >   set_priv_version(env, PRIV_VERSION_1_10_0);
> > -set_resetvec(env, DEFAULT_RSTVEC);
> >   set_feature(env, RISCV_FEATURE_MMU);
> >   set_feature(env, RISCV_FEATURE_PMP);
> >   }
> > @@ -207,7 +208,6 @@ static void rv64imacu_nommu_cpu_init(Object *obj)
> >   CPURISCVState *env = _CPU(obj)->env;
> >   set_misa(env, RV64 | RVI | RVM | RVA | RVC | RVU);
> >   set_priv_version(env, PRIV_VERSION_1_10_0);
> > -set_resetvec(env, DEFAULT_RSTVEC);
> >   set_feature(env, RISCV_FEATURE_PMP);
> >   }
> >
> > @@ -399,7 +399,9 @@ static void riscv_cpu_realize(DeviceState *dev, Error 
> > **errp)
> >   }
> >
> >   set_priv_version(env, priv_version);
> > -set_resetvec(env, DEFAULT_RSTVEC);
> > +if (!get_resetvec(env)) {
> > +set_resetvec(env, DEFAULT_RSTVEC);
> > +}
> >
> >   if (cpu->cfg.mmu) {
> >   set_feature(env, RISCV_FEATURE_MMU);
> >
>

Re: [PATCH v2 2/9] target/riscv: Don't overwrite the reset vector

2020-05-14 Thread Philippe Mathieu-Daudé


On 5/7/20 9:13 PM, Alistair Francis wrote:

If the reset vector is set in the init function don't set it again in
realise.


typo "realize".



Signed-off-by: Alistair Francis 
---
  target/riscv/cpu.c | 20 +++-
  1 file changed, 11 insertions(+), 9 deletions(-)

diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
index 059d71f2c7..8f837edf8d 100644
--- a/target/riscv/cpu.c
+++ b/target/riscv/cpu.c
@@ -111,6 +111,14 @@ static void set_feature(CPURISCVState *env, int feature)
  env->features |= (1ULL << feature);
  }
  
+static int get_resetvec(CPURISCVState *env)

+{
+#ifndef CONFIG_USER_ONLY
+return env->resetvec;
+#endif
+return 0;


Don't you get an error about double return? Maybe use #else?


+}
+
  static void set_resetvec(CPURISCVState *env, int resetvec)
  {
  #ifndef CONFIG_USER_ONLY
@@ -123,7 +131,6 @@ static void riscv_any_cpu_init(Object *obj)
  CPURISCVState *env = _CPU(obj)->env;
  set_misa(env, RVXLEN | RVI | RVM | RVA | RVF | RVD | RVC | RVU);
  set_priv_version(env, PRIV_VERSION_1_11_0);
-set_resetvec(env, DEFAULT_RSTVEC);
  }
  
  #if defined(TARGET_RISCV32)

@@ -140,7 +147,6 @@ static void rv32gcsu_priv1_09_1_cpu_init(Object *obj)
  CPURISCVState *env = _CPU(obj)->env;
  set_misa(env, RV32 | RVI | RVM | RVA | RVF | RVD | RVC | RVS | RVU);
  set_priv_version(env, PRIV_VERSION_1_09_1);
-set_resetvec(env, DEFAULT_RSTVEC);
  set_feature(env, RISCV_FEATURE_MMU);
  set_feature(env, RISCV_FEATURE_PMP);
  }
@@ -150,7 +156,6 @@ static void rv32gcsu_priv1_10_0_cpu_init(Object *obj)
  CPURISCVState *env = _CPU(obj)->env;
  set_misa(env, RV32 | RVI | RVM | RVA | RVF | RVD | RVC | RVS | RVU);
  set_priv_version(env, PRIV_VERSION_1_10_0);
-set_resetvec(env, DEFAULT_RSTVEC);
  set_feature(env, RISCV_FEATURE_MMU);
  set_feature(env, RISCV_FEATURE_PMP);
  }
@@ -160,7 +165,6 @@ static void rv32imacu_nommu_cpu_init(Object *obj)
  CPURISCVState *env = _CPU(obj)->env;
  set_misa(env, RV32 | RVI | RVM | RVA | RVC | RVU);
  set_priv_version(env, PRIV_VERSION_1_10_0);
-set_resetvec(env, DEFAULT_RSTVEC);
  set_feature(env, RISCV_FEATURE_PMP);
  }
  
@@ -169,7 +173,6 @@ static void rv32imafcu_nommu_cpu_init(Object *obj)

  CPURISCVState *env = _CPU(obj)->env;
  set_misa(env, RV32 | RVI | RVM | RVA | RVF | RVC | RVU);
  set_priv_version(env, PRIV_VERSION_1_10_0);
-set_resetvec(env, DEFAULT_RSTVEC);
  set_feature(env, RISCV_FEATURE_PMP);
  }
  
@@ -187,7 +190,6 @@ static void rv64gcsu_priv1_09_1_cpu_init(Object *obj)

  CPURISCVState *env = _CPU(obj)->env;
  set_misa(env, RV64 | RVI | RVM | RVA | RVF | RVD | RVC | RVS | RVU);
  set_priv_version(env, PRIV_VERSION_1_09_1);
-set_resetvec(env, DEFAULT_RSTVEC);
  set_feature(env, RISCV_FEATURE_MMU);
  set_feature(env, RISCV_FEATURE_PMP);
  }
@@ -197,7 +199,6 @@ static void rv64gcsu_priv1_10_0_cpu_init(Object *obj)
  CPURISCVState *env = _CPU(obj)->env;
  set_misa(env, RV64 | RVI | RVM | RVA | RVF | RVD | RVC | RVS | RVU);
  set_priv_version(env, PRIV_VERSION_1_10_0);
-set_resetvec(env, DEFAULT_RSTVEC);
  set_feature(env, RISCV_FEATURE_MMU);
  set_feature(env, RISCV_FEATURE_PMP);
  }
@@ -207,7 +208,6 @@ static void rv64imacu_nommu_cpu_init(Object *obj)
  CPURISCVState *env = _CPU(obj)->env;
  set_misa(env, RV64 | RVI | RVM | RVA | RVC | RVU);
  set_priv_version(env, PRIV_VERSION_1_10_0);
-set_resetvec(env, DEFAULT_RSTVEC);
  set_feature(env, RISCV_FEATURE_PMP);
  }
  
@@ -399,7 +399,9 @@ static void riscv_cpu_realize(DeviceState *dev, Error **errp)

  }
  
  set_priv_version(env, priv_version);

-set_resetvec(env, DEFAULT_RSTVEC);
+if (!get_resetvec(env)) {
+set_resetvec(env, DEFAULT_RSTVEC);
+}
  
  if (cpu->cfg.mmu) {

  set_feature(env, RISCV_FEATURE_MMU);

Re: [PATCH v2 2/9] target/riscv: Don't overwrite the reset vector

Re: [PATCH v2 2/9] target/riscv: Don't overwrite the reset vector

Re: [PATCH v2 2/9] target/riscv: Don't overwrite the reset vector

Re: [PATCH v2 2/9] target/riscv: Don't overwrite the reset vector

Re: [PATCH v2 2/9] target/riscv: Don't overwrite the reset vector

Re: [PATCH v2 2/9] target/riscv: Don't overwrite the reset vector

6 matches

Site Navigation

Mail list logo

Footer information