| Message ID | 20230228104035.1879882-6-bmeng@tinylab.org |
|---|---|
| State | New |
| Headers | show |
| Series | target/riscv: Various fixes to gdbstub and CSR access | expand |
On 2023/2/28 18:40, Bin Meng wrote: > There is no need to generate the CSR XML if the Zicsr extension > is not enabled. Should we generate the FPU XML or Vector XML when Zicsr is not enabled? Zhiwei > > Signed-off-by: Bin Meng <bmeng@tinylab.org> > Reviewed-by: Weiwei Li <liweiwei@iscas.ac.cn> > Reviewed-by: LIU Zhiwei <zhiwei_liu@linux.alibaba.com> > --- > > (no changes since v1) > > target/riscv/gdbstub.c | 9 ++++++--- > 1 file changed, 6 insertions(+), 3 deletions(-) > > diff --git a/target/riscv/gdbstub.c b/target/riscv/gdbstub.c > index 704f3d6922..294f0ceb1c 100644 > --- a/target/riscv/gdbstub.c > +++ b/target/riscv/gdbstub.c > @@ -406,7 +406,10 @@ void riscv_cpu_register_gdb_regs_for_features(CPUState *cs) > g_assert_not_reached(); > } > > - gdb_register_coprocessor(cs, riscv_gdb_get_csr, riscv_gdb_set_csr, > - riscv_gen_dynamic_csr_xml(cs, cs->gdb_num_regs), > - "riscv-csr.xml", 0); > + if (cpu->cfg.ext_icsr) { > + int base_reg = cs->gdb_num_regs; > + gdb_register_coprocessor(cs, riscv_gdb_get_csr, riscv_gdb_set_csr, > + riscv_gen_dynamic_csr_xml(cs, base_reg), > + "riscv-csr.xml", 0); > + } > }
On Wed, Mar 1, 2023 at 5:52 PM LIU Zhiwei <zhiwei_liu@linux.alibaba.com> wrote: > > > On 2023/2/28 18:40, Bin Meng wrote: > > There is no need to generate the CSR XML if the Zicsr extension > > is not enabled. > > Should we generate the FPU XML or Vector XML when Zicsr is not enabled? Good point. I think we should disable that too. > > Zhiwei > Regards, Bin
On Wed, 01 Mar 2023 01:55:34 PST (-0800), Bin Meng wrote: > On Wed, Mar 1, 2023 at 5:52 PM LIU Zhiwei <zhiwei_liu@linux.alibaba.com> wrote: >> >> >> On 2023/2/28 18:40, Bin Meng wrote: >> > There is no need to generate the CSR XML if the Zicsr extension >> > is not enabled. >> >> Should we generate the FPU XML or Vector XML when Zicsr is not enabled? > > Good point. I think we should disable that too. Seems reasonable. Did you want to do that as part of a v3, or just as a follow-on fix? >> Zhiwei >> > > Regards, > Bin
On Thu, Mar 2, 2023 at 7:43 AM Palmer Dabbelt <palmer@dabbelt.com> wrote: > > On Wed, 01 Mar 2023 01:55:34 PST (-0800), Bin Meng wrote: > > On Wed, Mar 1, 2023 at 5:52 PM LIU Zhiwei <zhiwei_liu@linux.alibaba.com> wrote: > >> > >> > >> On 2023/2/28 18:40, Bin Meng wrote: > >> > There is no need to generate the CSR XML if the Zicsr extension > >> > is not enabled. > >> > >> Should we generate the FPU XML or Vector XML when Zicsr is not enabled? > > > > Good point. I think we should disable that too. > > Seems reasonable. Did you want to do that as part of a v3, or just as a > follow-on fix? > I looked at this further. The FPU / Vector XML is guarded by the " env->misa_ext" check. If Zicsr is disabled while F or V extension is off, QEMU will error out in riscv_cpu_realize() earlier before the gdbstub init. So current patch should be fine. Regards, Bin
On Wed, 01 Mar 2023 16:30:52 PST (-0800), Bin Meng wrote: > On Thu, Mar 2, 2023 at 7:43 AM Palmer Dabbelt <palmer@dabbelt.com> wrote: >> >> On Wed, 01 Mar 2023 01:55:34 PST (-0800), Bin Meng wrote: >> > On Wed, Mar 1, 2023 at 5:52 PM LIU Zhiwei <zhiwei_liu@linux.alibaba.com> wrote: >> >> >> >> >> >> On 2023/2/28 18:40, Bin Meng wrote: >> >> > There is no need to generate the CSR XML if the Zicsr extension >> >> > is not enabled. >> >> >> >> Should we generate the FPU XML or Vector XML when Zicsr is not enabled? >> > >> > Good point. I think we should disable that too. >> >> Seems reasonable. Did you want to do that as part of a v3, or just as a >> follow-on fix? >> > > I looked at this further. > > The FPU / Vector XML is guarded by the " env->misa_ext" check. If > Zicsr is disabled while F or V extension is off, QEMU will error out > in riscv_cpu_realize() earlier before the gdbstub init. > > So current patch should be fine. There's a merge conflict that git auto-resolved as diff --cc target/riscv/csr.c index a1ecf62305,3a7e0217e2..a2cf3536f0 --- a/target/riscv/csr.c +++ b/target/riscv/csr.c @@@ -90,10 -53,10 +53,9 @@@ static RISCVException fs(CPURISCVState static RISCVException vs(CPURISCVState *env, int csrno) { - CPUState *cs = env_cpu(env); - RISCVCPU *cpu = RISCV_CPU(cs); + RISCVCPU *cpu = env_archcpu(env); - if (env->misa_ext & RVV || - cpu->cfg.ext_zve32f || cpu->cfg.ext_zve64f) { + if (cpu->cfg.ext_zve32f) { #if !defined(CONFIG_USER_ONLY) if (!env->debugger && !riscv_cpu_vector_enabled(env)) { return RISCV_EXCP_ILLEGAL_INST; which looks correct to me. It's passing my tests and queued up, but LMK if something looks wrong. Thanks!
On 2023/3/2 8:30, Bin Meng wrote: > On Thu, Mar 2, 2023 at 7:43 AM Palmer Dabbelt <palmer@dabbelt.com> wrote: >> On Wed, 01 Mar 2023 01:55:34 PST (-0800), Bin Meng wrote: >>> On Wed, Mar 1, 2023 at 5:52 PM LIU Zhiwei <zhiwei_liu@linux.alibaba.com> wrote: >>>> >>>> On 2023/2/28 18:40, Bin Meng wrote: >>>>> There is no need to generate the CSR XML if the Zicsr extension >>>>> is not enabled. >>>> Should we generate the FPU XML or Vector XML when Zicsr is not enabled? >>> Good point. I think we should disable that too. >> Seems reasonable. Did you want to do that as part of a v3, or just as a >> follow-on fix? >> > I looked at this further. > > The FPU / Vector XML is guarded by the " env->misa_ext" check. If > Zicsr is disabled while F or V extension is off, QEMU will error out > in riscv_cpu_realize() earlier before the gdbstub init. Make sense. Zhiwei > > So current patch should be fine. > > Regards, > Bin
diff --git a/target/riscv/gdbstub.c b/target/riscv/gdbstub.c index 704f3d6922..294f0ceb1c 100644 --- a/target/riscv/gdbstub.c +++ b/target/riscv/gdbstub.c @@ -406,7 +406,10 @@ void riscv_cpu_register_gdb_regs_for_features(CPUState *cs) g_assert_not_reached(); } - gdb_register_coprocessor(cs, riscv_gdb_get_csr, riscv_gdb_set_csr, - riscv_gen_dynamic_csr_xml(cs, cs->gdb_num_regs), - "riscv-csr.xml", 0); + if (cpu->cfg.ext_icsr) { + int base_reg = cs->gdb_num_regs; + gdb_register_coprocessor(cs, riscv_gdb_get_csr, riscv_gdb_set_csr, + riscv_gen_dynamic_csr_xml(cs, base_reg), + "riscv-csr.xml", 0); + } }