| Message ID | 20230213180215.1524938-19-bmeng@tinylab.org |
|---|---|
| State | New |
| Headers | show |
| Series | target/riscv: Various fixes to gdbstub and CSR access | expand |
On 2023/2/14 22:27, Bin Meng wrote: > At present the envcfg CSRs predicate() routines are generic one like > smode(), hmode. The configuration check is done in the read / write > routine. Create a new predicate routine to cover such check, so that > gdbstub can correctly report its existence. > > Signed-off-by: Bin Meng <bmeng@tinylab.org> > > --- > > target/riscv/csr.c | 98 +++++++++++++++++++++++++++++----------------- > 1 file changed, 61 insertions(+), 37 deletions(-) > > diff --git a/target/riscv/csr.c b/target/riscv/csr.c > index 37350b8a6d..284ccc09dd 100644 > --- a/target/riscv/csr.c > +++ b/target/riscv/csr.c > @@ -41,40 +41,6 @@ void riscv_set_csr_ops(int csrno, riscv_csr_operations *ops) > } > > /* Predicates */ > -#if !defined(CONFIG_USER_ONLY) > -static RISCVException smstateen_acc_ok(CPURISCVState *env, int index, > - uint64_t bit) > -{ > - bool virt = riscv_cpu_virt_enabled(env); > - RISCVCPU *cpu = env_archcpu(env); > - > - if (env->priv == PRV_M || !cpu->cfg.ext_smstateen) { > - return RISCV_EXCP_NONE; > - } > - > - if (!(env->mstateen[index] & bit)) { > - return RISCV_EXCP_ILLEGAL_INST; > - } > - > - if (virt) { > - if (!(env->hstateen[index] & bit)) { > - return RISCV_EXCP_VIRT_INSTRUCTION_FAULT; > - } > - > - if (env->priv == PRV_U && !(env->sstateen[index] & bit)) { > - return RISCV_EXCP_VIRT_INSTRUCTION_FAULT; > - } > - } > - > - if (env->priv == PRV_U && riscv_has_ext(env, RVS)) { > - if (!(env->sstateen[index] & bit)) { > - return RISCV_EXCP_ILLEGAL_INST; > - } > - } > - > - return RISCV_EXCP_NONE; > -} > -#endif > > static RISCVException fs(CPURISCVState *env, int csrno) > { > @@ -318,6 +284,32 @@ static RISCVException umode32(CPURISCVState *env, int csrno) > return umode(env, csrno); > } > > +static RISCVException envcfg(CPURISCVState *env, int csrno) > +{ > + RISCVCPU *cpu = env_archcpu(env); > + riscv_csr_predicate_fn predicate; > + > + if (cpu->cfg.ext_smstateen) { > + return RISCV_EXCP_ILLEGAL_INST; > + } This check seems not right here. Why ILLEGAL_INST is directly triggered if smstateen is enabled? It seems that smstateen related check will be done for senvcfg/henvcfg{h} when smstateen is enabled. Regards, Weiwei Li > + > + switch (csrno) { > + case CSR_SENVCFG: > + predicate = smode; > + break; > + case CSR_HENVCFG: > + predicate = hmode; > + break; > + case CSR_HENVCFGH: > + predicate = hmode32; > + break; > + default: > + g_assert_not_reached(); > + } > + > + return predicate(env, csrno); > +} > + > static RISCVException mstateen(CPURISCVState *env, int csrno) > { > RISCVCPU *cpu = env_archcpu(env); > @@ -1946,6 +1938,38 @@ static RISCVException write_menvcfgh(CPURISCVState *env, int csrno, > return RISCV_EXCP_NONE; > } > > +static RISCVException smstateen_acc_ok(CPURISCVState *env, int index, > + uint64_t bit) > +{ > + bool virt = riscv_cpu_virt_enabled(env); > + > + if (env->priv == PRV_M) { > + return RISCV_EXCP_NONE; > + } > + > + if (!(env->mstateen[index] & bit)) { > + return RISCV_EXCP_ILLEGAL_INST; > + } > + > + if (virt) { > + if (!(env->hstateen[index] & bit)) { > + return RISCV_EXCP_VIRT_INSTRUCTION_FAULT; > + } > + > + if (env->priv == PRV_U && !(env->sstateen[index] & bit)) { > + return RISCV_EXCP_VIRT_INSTRUCTION_FAULT; > + } > + } > + > + if (env->priv == PRV_U && riscv_has_ext(env, RVS)) { > + if (!(env->sstateen[index] & bit)) { > + return RISCV_EXCP_ILLEGAL_INST; > + } > + } > + > + return RISCV_EXCP_NONE; > +} > + > static RISCVException read_senvcfg(CPURISCVState *env, int csrno, > target_ulong *val) > { > @@ -4087,11 +4111,11 @@ riscv_csr_operations csr_ops[CSR_TABLE_SIZE] = { > .min_priv_ver = PRIV_VERSION_1_12_0 }, > [CSR_MENVCFGH] = { "menvcfgh", umode32, read_menvcfgh, write_menvcfgh, > .min_priv_ver = PRIV_VERSION_1_12_0 }, > - [CSR_SENVCFG] = { "senvcfg", smode, read_senvcfg, write_senvcfg, > + [CSR_SENVCFG] = { "senvcfg", envcfg, read_senvcfg, write_senvcfg, > .min_priv_ver = PRIV_VERSION_1_12_0 }, > - [CSR_HENVCFG] = { "henvcfg", hmode, read_henvcfg, write_henvcfg, > + [CSR_HENVCFG] = { "henvcfg", envcfg, read_henvcfg, write_henvcfg, > .min_priv_ver = PRIV_VERSION_1_12_0 }, > - [CSR_HENVCFGH] = { "henvcfgh", hmode32, read_henvcfgh, write_henvcfgh, > + [CSR_HENVCFGH] = { "henvcfgh", envcfg, read_henvcfgh, write_henvcfgh, > .min_priv_ver = PRIV_VERSION_1_12_0 }, > > /* Smstateen extension CSRs */
On Tue, Feb 14, 2023 at 10:59 PM weiwei <liweiwei@iscas.ac.cn> wrote: > > > On 2023/2/14 22:27, Bin Meng wrote: > > At present the envcfg CSRs predicate() routines are generic one like > > smode(), hmode. The configuration check is done in the read / write > > routine. Create a new predicate routine to cover such check, so that > > gdbstub can correctly report its existence. > > > > Signed-off-by: Bin Meng <bmeng@tinylab.org> > > > > --- > > > > target/riscv/csr.c | 98 +++++++++++++++++++++++++++++----------------- > > 1 file changed, 61 insertions(+), 37 deletions(-) > > > > diff --git a/target/riscv/csr.c b/target/riscv/csr.c > > index 37350b8a6d..284ccc09dd 100644 > > --- a/target/riscv/csr.c > > +++ b/target/riscv/csr.c > > @@ -41,40 +41,6 @@ void riscv_set_csr_ops(int csrno, riscv_csr_operations *ops) > > } > > > > /* Predicates */ > > -#if !defined(CONFIG_USER_ONLY) > > -static RISCVException smstateen_acc_ok(CPURISCVState *env, int index, > > - uint64_t bit) > > -{ > > - bool virt = riscv_cpu_virt_enabled(env); > > - RISCVCPU *cpu = env_archcpu(env); > > - > > - if (env->priv == PRV_M || !cpu->cfg.ext_smstateen) { > > - return RISCV_EXCP_NONE; > > - } > > - > > - if (!(env->mstateen[index] & bit)) { > > - return RISCV_EXCP_ILLEGAL_INST; > > - } > > - > > - if (virt) { > > - if (!(env->hstateen[index] & bit)) { > > - return RISCV_EXCP_VIRT_INSTRUCTION_FAULT; > > - } > > - > > - if (env->priv == PRV_U && !(env->sstateen[index] & bit)) { > > - return RISCV_EXCP_VIRT_INSTRUCTION_FAULT; > > - } > > - } > > - > > - if (env->priv == PRV_U && riscv_has_ext(env, RVS)) { > > - if (!(env->sstateen[index] & bit)) { > > - return RISCV_EXCP_ILLEGAL_INST; > > - } > > - } > > - > > - return RISCV_EXCP_NONE; > > -} > > -#endif > > > > static RISCVException fs(CPURISCVState *env, int csrno) > > { > > @@ -318,6 +284,32 @@ static RISCVException umode32(CPURISCVState *env, int csrno) > > return umode(env, csrno); > > } > > > > +static RISCVException envcfg(CPURISCVState *env, int csrno) > > +{ > > + RISCVCPU *cpu = env_archcpu(env); > > + riscv_csr_predicate_fn predicate; > > + > > + if (cpu->cfg.ext_smstateen) { > > + return RISCV_EXCP_ILLEGAL_INST; > > + } > > This check seems not right here. Why ILLEGAL_INST is directly > triggered if smstateen is enabled? This logic was there in the original codes. I was confused when I looked at this as well. Anyway, if it is an issue, it should be a separate patch. > > It seems that smstateen related check will be done for > senvcfg/henvcfg{h} when smstateen is enabled. > Regards, Bin
On 2023/2/15 10:22, Bin Meng wrote: > On Tue, Feb 14, 2023 at 10:59 PM weiwei <liweiwei@iscas.ac.cn> wrote: >> >> On 2023/2/14 22:27, Bin Meng wrote: >>> At present the envcfg CSRs predicate() routines are generic one like >>> smode(), hmode. The configuration check is done in the read / write >>> routine. Create a new predicate routine to cover such check, so that >>> gdbstub can correctly report its existence. >>> >>> Signed-off-by: Bin Meng <bmeng@tinylab.org> >>> >>> --- >>> >>> target/riscv/csr.c | 98 +++++++++++++++++++++++++++++----------------- >>> 1 file changed, 61 insertions(+), 37 deletions(-) >>> >>> diff --git a/target/riscv/csr.c b/target/riscv/csr.c >>> index 37350b8a6d..284ccc09dd 100644 >>> --- a/target/riscv/csr.c >>> +++ b/target/riscv/csr.c >>> @@ -41,40 +41,6 @@ void riscv_set_csr_ops(int csrno, riscv_csr_operations *ops) >>> } >>> >>> /* Predicates */ >>> -#if !defined(CONFIG_USER_ONLY) >>> -static RISCVException smstateen_acc_ok(CPURISCVState *env, int index, >>> - uint64_t bit) >>> -{ >>> - bool virt = riscv_cpu_virt_enabled(env); >>> - RISCVCPU *cpu = env_archcpu(env); >>> - >>> - if (env->priv == PRV_M || !cpu->cfg.ext_smstateen) { >>> - return RISCV_EXCP_NONE; >>> - } >>> - >>> - if (!(env->mstateen[index] & bit)) { >>> - return RISCV_EXCP_ILLEGAL_INST; >>> - } >>> - >>> - if (virt) { >>> - if (!(env->hstateen[index] & bit)) { >>> - return RISCV_EXCP_VIRT_INSTRUCTION_FAULT; >>> - } >>> - >>> - if (env->priv == PRV_U && !(env->sstateen[index] & bit)) { >>> - return RISCV_EXCP_VIRT_INSTRUCTION_FAULT; >>> - } >>> - } >>> - >>> - if (env->priv == PRV_U && riscv_has_ext(env, RVS)) { >>> - if (!(env->sstateen[index] & bit)) { >>> - return RISCV_EXCP_ILLEGAL_INST; >>> - } >>> - } >>> - >>> - return RISCV_EXCP_NONE; >>> -} >>> -#endif >>> >>> static RISCVException fs(CPURISCVState *env, int csrno) >>> { >>> @@ -318,6 +284,32 @@ static RISCVException umode32(CPURISCVState *env, int csrno) >>> return umode(env, csrno); >>> } >>> >>> +static RISCVException envcfg(CPURISCVState *env, int csrno) >>> +{ >>> + RISCVCPU *cpu = env_archcpu(env); >>> + riscv_csr_predicate_fn predicate; >>> + >>> + if (cpu->cfg.ext_smstateen) { >>> + return RISCV_EXCP_ILLEGAL_INST; >>> + } >> This check seems not right here. Why ILLEGAL_INST is directly >> triggered if smstateen is enabled? > This logic was there in the original codes. I was confused when I > looked at this as well. Sorry, I didn't find the original codes. Do you mean this: if (env->priv == PRV_M || !cpu->cfg.ext_smstateen) { return RISCV_EXCP_NONE; } If so, I think the check here is to make the following *stateen related check ignored when smstateen extension is disabled. Regards, Weiwei Li > Anyway, if it is an issue, it should be a separate patch. > >> It seems that smstateen related check will be done for >> senvcfg/henvcfg{h} when smstateen is enabled. >> > Regards, > Bin
On Tue, 14 Feb 2023 18:22:21 PST (-0800), Bin Meng wrote: > On Tue, Feb 14, 2023 at 10:59 PM weiwei <liweiwei@iscas.ac.cn> wrote: >> >> >> On 2023/2/14 22:27, Bin Meng wrote: >> > At present the envcfg CSRs predicate() routines are generic one like >> > smode(), hmode. The configuration check is done in the read / write >> > routine. Create a new predicate routine to cover such check, so that >> > gdbstub can correctly report its existence. >> > >> > Signed-off-by: Bin Meng <bmeng@tinylab.org> >> > >> > --- >> > >> > target/riscv/csr.c | 98 +++++++++++++++++++++++++++++----------------- >> > 1 file changed, 61 insertions(+), 37 deletions(-) >> > >> > diff --git a/target/riscv/csr.c b/target/riscv/csr.c >> > index 37350b8a6d..284ccc09dd 100644 >> > --- a/target/riscv/csr.c >> > +++ b/target/riscv/csr.c >> > @@ -41,40 +41,6 @@ void riscv_set_csr_ops(int csrno, riscv_csr_operations *ops) >> > } >> > >> > /* Predicates */ >> > -#if !defined(CONFIG_USER_ONLY) >> > -static RISCVException smstateen_acc_ok(CPURISCVState *env, int index, >> > - uint64_t bit) >> > -{ >> > - bool virt = riscv_cpu_virt_enabled(env); >> > - RISCVCPU *cpu = env_archcpu(env); >> > - >> > - if (env->priv == PRV_M || !cpu->cfg.ext_smstateen) { >> > - return RISCV_EXCP_NONE; >> > - } >> > - >> > - if (!(env->mstateen[index] & bit)) { >> > - return RISCV_EXCP_ILLEGAL_INST; >> > - } >> > - >> > - if (virt) { >> > - if (!(env->hstateen[index] & bit)) { >> > - return RISCV_EXCP_VIRT_INSTRUCTION_FAULT; >> > - } >> > - >> > - if (env->priv == PRV_U && !(env->sstateen[index] & bit)) { >> > - return RISCV_EXCP_VIRT_INSTRUCTION_FAULT; >> > - } >> > - } >> > - >> > - if (env->priv == PRV_U && riscv_has_ext(env, RVS)) { >> > - if (!(env->sstateen[index] & bit)) { >> > - return RISCV_EXCP_ILLEGAL_INST; >> > - } >> > - } >> > - >> > - return RISCV_EXCP_NONE; >> > -} >> > -#endif >> > >> > static RISCVException fs(CPURISCVState *env, int csrno) >> > { >> > @@ -318,6 +284,32 @@ static RISCVException umode32(CPURISCVState *env, int csrno) >> > return umode(env, csrno); >> > } >> > >> > +static RISCVException envcfg(CPURISCVState *env, int csrno) >> > +{ >> > + RISCVCPU *cpu = env_archcpu(env); >> > + riscv_csr_predicate_fn predicate; >> > + >> > + if (cpu->cfg.ext_smstateen) { >> > + return RISCV_EXCP_ILLEGAL_INST; >> > + } >> >> This check seems not right here. Why ILLEGAL_INST is directly >> triggered if smstateen is enabled? > > This logic was there in the original codes. I was confused when I > looked at this as well. > > Anyway, if it is an issue, it should be a separate patch. Seems reasonable to me, it's always nice to split up the refactoring types. So I queued this up as 4ac6c32224 ("Merge patch series "target/riscv: Various fixes to gdbstub and CSR access""). I had to fix up the From address on the patch you re-sent and there was a minor merge conflict, but otherwise things look sane to me. I'll hold off on sending anything for a bit just in case, though. Thanks! > >> >> It seems that smstateen related check will be done for >> senvcfg/henvcfg{h} when smstateen is enabled. >> > > Regards, > Bin
Hi Palmer, On Fri, Feb 17, 2023 at 12:40 AM Palmer Dabbelt <palmer@dabbelt.com> wrote: > > On Tue, 14 Feb 2023 18:22:21 PST (-0800), Bin Meng wrote: > > On Tue, Feb 14, 2023 at 10:59 PM weiwei <liweiwei@iscas.ac.cn> wrote: > >> > >> > >> On 2023/2/14 22:27, Bin Meng wrote: > >> > At present the envcfg CSRs predicate() routines are generic one like > >> > smode(), hmode. The configuration check is done in the read / write > >> > routine. Create a new predicate routine to cover such check, so that > >> > gdbstub can correctly report its existence. > >> > > >> > Signed-off-by: Bin Meng <bmeng@tinylab.org> > >> > > >> > --- > >> > > >> > target/riscv/csr.c | 98 +++++++++++++++++++++++++++++----------------- > >> > 1 file changed, 61 insertions(+), 37 deletions(-) > >> > > >> > diff --git a/target/riscv/csr.c b/target/riscv/csr.c > >> > index 37350b8a6d..284ccc09dd 100644 > >> > --- a/target/riscv/csr.c > >> > +++ b/target/riscv/csr.c > >> > @@ -41,40 +41,6 @@ void riscv_set_csr_ops(int csrno, riscv_csr_operations *ops) > >> > } > >> > > >> > /* Predicates */ > >> > -#if !defined(CONFIG_USER_ONLY) > >> > -static RISCVException smstateen_acc_ok(CPURISCVState *env, int index, > >> > - uint64_t bit) > >> > -{ > >> > - bool virt = riscv_cpu_virt_enabled(env); > >> > - RISCVCPU *cpu = env_archcpu(env); > >> > - > >> > - if (env->priv == PRV_M || !cpu->cfg.ext_smstateen) { > >> > - return RISCV_EXCP_NONE; > >> > - } > >> > - > >> > - if (!(env->mstateen[index] & bit)) { > >> > - return RISCV_EXCP_ILLEGAL_INST; > >> > - } > >> > - > >> > - if (virt) { > >> > - if (!(env->hstateen[index] & bit)) { > >> > - return RISCV_EXCP_VIRT_INSTRUCTION_FAULT; > >> > - } > >> > - > >> > - if (env->priv == PRV_U && !(env->sstateen[index] & bit)) { > >> > - return RISCV_EXCP_VIRT_INSTRUCTION_FAULT; > >> > - } > >> > - } > >> > - > >> > - if (env->priv == PRV_U && riscv_has_ext(env, RVS)) { > >> > - if (!(env->sstateen[index] & bit)) { > >> > - return RISCV_EXCP_ILLEGAL_INST; > >> > - } > >> > - } > >> > - > >> > - return RISCV_EXCP_NONE; > >> > -} > >> > -#endif > >> > > >> > static RISCVException fs(CPURISCVState *env, int csrno) > >> > { > >> > @@ -318,6 +284,32 @@ static RISCVException umode32(CPURISCVState *env, int csrno) > >> > return umode(env, csrno); > >> > } > >> > > >> > +static RISCVException envcfg(CPURISCVState *env, int csrno) > >> > +{ > >> > + RISCVCPU *cpu = env_archcpu(env); > >> > + riscv_csr_predicate_fn predicate; > >> > + > >> > + if (cpu->cfg.ext_smstateen) { > >> > + return RISCV_EXCP_ILLEGAL_INST; > >> > + } > >> > >> This check seems not right here. Why ILLEGAL_INST is directly > >> triggered if smstateen is enabled? > > > > This logic was there in the original codes. I was confused when I > > looked at this as well. > > > > Anyway, if it is an issue, it should be a separate patch. > > Seems reasonable to me, it's always nice to split up the refactoring types. So > I queued this up as 4ac6c32224 ("Merge patch series "target/riscv: Various > fixes to gdbstub and CSR access""). > > I had to fix up the From address on the patch you re-sent and there was a minor > merge conflict, but otherwise things look sane to me. I'll hold off on sending > anything for a bit just in case, though. > There are some open comments in this series I need to address. Please drop this v1. I will send v2 soon. Regards, Bin
On Thu, 16 Feb 2023 17:59:42 PST (-0800), Bin Meng wrote: > Hi Palmer, > > On Fri, Feb 17, 2023 at 12:40 AM Palmer Dabbelt <palmer@dabbelt.com> wrote: >> >> On Tue, 14 Feb 2023 18:22:21 PST (-0800), Bin Meng wrote: >> > On Tue, Feb 14, 2023 at 10:59 PM weiwei <liweiwei@iscas.ac.cn> wrote: >> >> >> >> >> >> On 2023/2/14 22:27, Bin Meng wrote: >> >> > At present the envcfg CSRs predicate() routines are generic one like >> >> > smode(), hmode. The configuration check is done in the read / write >> >> > routine. Create a new predicate routine to cover such check, so that >> >> > gdbstub can correctly report its existence. >> >> > >> >> > Signed-off-by: Bin Meng <bmeng@tinylab.org> >> >> > >> >> > --- >> >> > >> >> > target/riscv/csr.c | 98 +++++++++++++++++++++++++++++----------------- >> >> > 1 file changed, 61 insertions(+), 37 deletions(-) >> >> > >> >> > diff --git a/target/riscv/csr.c b/target/riscv/csr.c >> >> > index 37350b8a6d..284ccc09dd 100644 >> >> > --- a/target/riscv/csr.c >> >> > +++ b/target/riscv/csr.c >> >> > @@ -41,40 +41,6 @@ void riscv_set_csr_ops(int csrno, riscv_csr_operations *ops) >> >> > } >> >> > >> >> > /* Predicates */ >> >> > -#if !defined(CONFIG_USER_ONLY) >> >> > -static RISCVException smstateen_acc_ok(CPURISCVState *env, int index, >> >> > - uint64_t bit) >> >> > -{ >> >> > - bool virt = riscv_cpu_virt_enabled(env); >> >> > - RISCVCPU *cpu = env_archcpu(env); >> >> > - >> >> > - if (env->priv == PRV_M || !cpu->cfg.ext_smstateen) { >> >> > - return RISCV_EXCP_NONE; >> >> > - } >> >> > - >> >> > - if (!(env->mstateen[index] & bit)) { >> >> > - return RISCV_EXCP_ILLEGAL_INST; >> >> > - } >> >> > - >> >> > - if (virt) { >> >> > - if (!(env->hstateen[index] & bit)) { >> >> > - return RISCV_EXCP_VIRT_INSTRUCTION_FAULT; >> >> > - } >> >> > - >> >> > - if (env->priv == PRV_U && !(env->sstateen[index] & bit)) { >> >> > - return RISCV_EXCP_VIRT_INSTRUCTION_FAULT; >> >> > - } >> >> > - } >> >> > - >> >> > - if (env->priv == PRV_U && riscv_has_ext(env, RVS)) { >> >> > - if (!(env->sstateen[index] & bit)) { >> >> > - return RISCV_EXCP_ILLEGAL_INST; >> >> > - } >> >> > - } >> >> > - >> >> > - return RISCV_EXCP_NONE; >> >> > -} >> >> > -#endif >> >> > >> >> > static RISCVException fs(CPURISCVState *env, int csrno) >> >> > { >> >> > @@ -318,6 +284,32 @@ static RISCVException umode32(CPURISCVState *env, int csrno) >> >> > return umode(env, csrno); >> >> > } >> >> > >> >> > +static RISCVException envcfg(CPURISCVState *env, int csrno) >> >> > +{ >> >> > + RISCVCPU *cpu = env_archcpu(env); >> >> > + riscv_csr_predicate_fn predicate; >> >> > + >> >> > + if (cpu->cfg.ext_smstateen) { >> >> > + return RISCV_EXCP_ILLEGAL_INST; >> >> > + } >> >> >> >> This check seems not right here. Why ILLEGAL_INST is directly >> >> triggered if smstateen is enabled? >> > >> > This logic was there in the original codes. I was confused when I >> > looked at this as well. >> > >> > Anyway, if it is an issue, it should be a separate patch. >> >> Seems reasonable to me, it's always nice to split up the refactoring types. So >> I queued this up as 4ac6c32224 ("Merge patch series "target/riscv: Various >> fixes to gdbstub and CSR access""). >> >> I had to fix up the From address on the patch you re-sent and there was a minor >> merge conflict, but otherwise things look sane to me. I'll hold off on sending >> anything for a bit just in case, though. >> > > There are some open comments in this series I need to address. Please > drop this v1. I will send v2 soon. Sorry aobut that, I'd thought they were all reviewed. I've dropped it from the queue. Thanks! > > Regards, > Bin
diff --git a/target/riscv/csr.c b/target/riscv/csr.c index 37350b8a6d..284ccc09dd 100644 --- a/target/riscv/csr.c +++ b/target/riscv/csr.c @@ -41,40 +41,6 @@ void riscv_set_csr_ops(int csrno, riscv_csr_operations *ops) } /* Predicates */ -#if !defined(CONFIG_USER_ONLY) -static RISCVException smstateen_acc_ok(CPURISCVState *env, int index, - uint64_t bit) -{ - bool virt = riscv_cpu_virt_enabled(env); - RISCVCPU *cpu = env_archcpu(env); - - if (env->priv == PRV_M || !cpu->cfg.ext_smstateen) { - return RISCV_EXCP_NONE; - } - - if (!(env->mstateen[index] & bit)) { - return RISCV_EXCP_ILLEGAL_INST; - } - - if (virt) { - if (!(env->hstateen[index] & bit)) { - return RISCV_EXCP_VIRT_INSTRUCTION_FAULT; - } - - if (env->priv == PRV_U && !(env->sstateen[index] & bit)) { - return RISCV_EXCP_VIRT_INSTRUCTION_FAULT; - } - } - - if (env->priv == PRV_U && riscv_has_ext(env, RVS)) { - if (!(env->sstateen[index] & bit)) { - return RISCV_EXCP_ILLEGAL_INST; - } - } - - return RISCV_EXCP_NONE; -} -#endif static RISCVException fs(CPURISCVState *env, int csrno) { @@ -318,6 +284,32 @@ static RISCVException umode32(CPURISCVState *env, int csrno) return umode(env, csrno); } +static RISCVException envcfg(CPURISCVState *env, int csrno) +{ + RISCVCPU *cpu = env_archcpu(env); + riscv_csr_predicate_fn predicate; + + if (cpu->cfg.ext_smstateen) { + return RISCV_EXCP_ILLEGAL_INST; + } + + switch (csrno) { + case CSR_SENVCFG: + predicate = smode; + break; + case CSR_HENVCFG: + predicate = hmode; + break; + case CSR_HENVCFGH: + predicate = hmode32; + break; + default: + g_assert_not_reached(); + } + + return predicate(env, csrno); +} + static RISCVException mstateen(CPURISCVState *env, int csrno) { RISCVCPU *cpu = env_archcpu(env); @@ -1946,6 +1938,38 @@ static RISCVException write_menvcfgh(CPURISCVState *env, int csrno, return RISCV_EXCP_NONE; } +static RISCVException smstateen_acc_ok(CPURISCVState *env, int index, + uint64_t bit) +{ + bool virt = riscv_cpu_virt_enabled(env); + + if (env->priv == PRV_M) { + return RISCV_EXCP_NONE; + } + + if (!(env->mstateen[index] & bit)) { + return RISCV_EXCP_ILLEGAL_INST; + } + + if (virt) { + if (!(env->hstateen[index] & bit)) { + return RISCV_EXCP_VIRT_INSTRUCTION_FAULT; + } + + if (env->priv == PRV_U && !(env->sstateen[index] & bit)) { + return RISCV_EXCP_VIRT_INSTRUCTION_FAULT; + } + } + + if (env->priv == PRV_U && riscv_has_ext(env, RVS)) { + if (!(env->sstateen[index] & bit)) { + return RISCV_EXCP_ILLEGAL_INST; + } + } + + return RISCV_EXCP_NONE; +} + static RISCVException read_senvcfg(CPURISCVState *env, int csrno, target_ulong *val) { @@ -4087,11 +4111,11 @@ riscv_csr_operations csr_ops[CSR_TABLE_SIZE] = { .min_priv_ver = PRIV_VERSION_1_12_0 }, [CSR_MENVCFGH] = { "menvcfgh", umode32, read_menvcfgh, write_menvcfgh, .min_priv_ver = PRIV_VERSION_1_12_0 }, - [CSR_SENVCFG] = { "senvcfg", smode, read_senvcfg, write_senvcfg, + [CSR_SENVCFG] = { "senvcfg", envcfg, read_senvcfg, write_senvcfg, .min_priv_ver = PRIV_VERSION_1_12_0 }, - [CSR_HENVCFG] = { "henvcfg", hmode, read_henvcfg, write_henvcfg, + [CSR_HENVCFG] = { "henvcfg", envcfg, read_henvcfg, write_henvcfg, .min_priv_ver = PRIV_VERSION_1_12_0 }, - [CSR_HENVCFGH] = { "henvcfgh", hmode32, read_henvcfgh, write_henvcfgh, + [CSR_HENVCFGH] = { "henvcfgh", envcfg, read_henvcfgh, write_henvcfgh, .min_priv_ver = PRIV_VERSION_1_12_0 }, /* Smstateen extension CSRs */
At present the envcfg CSRs predicate() routines are generic one like smode(), hmode. The configuration check is done in the read / write routine. Create a new predicate routine to cover such check, so that gdbstub can correctly report its existence. Signed-off-by: Bin Meng <bmeng@tinylab.org> --- target/riscv/csr.c | 98 +++++++++++++++++++++++++++++----------------- 1 file changed, 61 insertions(+), 37 deletions(-)