| Message ID | 20180511035240.4016-5-richard.henderson@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | Honor CPU_DUMP_FPU | expand |
On 05/11/2018 12:52 AM, Richard Henderson wrote: > Cc: Michael Clark <mjc@sifive.com> > Cc: Palmer Dabbelt <palmer@sifive.com> > Cc: Sagar Karandikar <sagark@eecs.berkeley.edu> > Cc: Bastian Koppelmann <kbastian@mail.uni-paderborn.de> > Signed-off-by: Richard Henderson <richard.henderson@linaro.org> Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org> > --- > target/riscv/cpu.h | 1 + > target/riscv/fpu_helper.c | 6 ++++++ > target/riscv/op_helper.c | 3 +-- > 3 files changed, 8 insertions(+), 2 deletions(-) > > diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h > index 34abc383e3..f2bc243b95 100644 > --- a/target/riscv/cpu.h > +++ b/target/riscv/cpu.h > @@ -265,6 +265,7 @@ void QEMU_NORETURN do_raise_exception_err(CPURISCVState *env, > uint32_t exception, uintptr_t pc); > > target_ulong cpu_riscv_get_fflags(CPURISCVState *env); > +target_ulong cpu_riscv_get_fcsr(CPURISCVState *env); > void cpu_riscv_set_fflags(CPURISCVState *env, target_ulong); > > #define TB_FLAGS_MMU_MASK 3 > diff --git a/target/riscv/fpu_helper.c b/target/riscv/fpu_helper.c > index abbadead5c..41c7352115 100644 > --- a/target/riscv/fpu_helper.c > +++ b/target/riscv/fpu_helper.c > @@ -37,6 +37,12 @@ target_ulong cpu_riscv_get_fflags(CPURISCVState *env) > return hard; > } > > +target_ulong cpu_riscv_get_fcsr(CPURISCVState *env) > +{ > + return (cpu_riscv_get_fflags(env) << FSR_AEXC_SHIFT) > + | (env->frm << FSR_RD_SHIFT); > +} > + > void cpu_riscv_set_fflags(CPURISCVState *env, target_ulong hard) > { > int soft = 0; > diff --git a/target/riscv/op_helper.c b/target/riscv/op_helper.c > index 3abf52453c..fd2d8c0a9d 100644 > --- a/target/riscv/op_helper.c > +++ b/target/riscv/op_helper.c > @@ -423,8 +423,7 @@ target_ulong csr_read_helper(CPURISCVState *env, target_ulong csrno) > return env->frm; > case CSR_FCSR: > validate_mstatus_fs(env, GETPC()); > - return (cpu_riscv_get_fflags(env) << FSR_AEXC_SHIFT) > - | (env->frm << FSR_RD_SHIFT); > + return cpu_riscv_get_fcsr(env); > /* rdtime/rdtimeh is trapped and emulated by bbl in system mode */ > #ifdef CONFIG_USER_ONLY > case CSR_TIME: >
On Fri, May 11, 2018 at 3:52 PM, Richard Henderson < richard.henderson@linaro.org> wrote: > Cc: Michael Clark <mjc@sifive.com> > Cc: Palmer Dabbelt <palmer@sifive.com> > Cc: Sagar Karandikar <sagark@eecs.berkeley.edu> > Cc: Bastian Koppelmann <kbastian@mail.uni-paderborn.de> > Signed-off-by: Richard Henderson <richard.henderson@linaro.org> > I'm not against this change but it conflicts with changes in the riscv repo. I should post my patch queue to the list... We have made a somewhat medium sized change and have unraveled two monolithic switch statements out of csr_read_helper switch and csr_write_helper into clearly decomposed functions for modifying control and status registers, along with an interface to allow CPUs to hook custom control and status registers. This was done to support atomic read/modify/write CSRs which was not possible to achieve with the current helpers which separately called via the csr_read_helper followed by csr_write_helper. Given the only way to modify CSRs was via the switch statements, we needed to move them out to provide a mechanism for CSRs that wish to be truly atomic. e.g. 'mip'. The CSR functions are defined in The RISC-V Instruction Set Manual Volume I: User-Level ISA Document Version 2.2 as "atomic" instructions: - CSRRW (Atomic Read/Write CSR) - CSRRS (Atomic Read and Set Bits in CSR) - CSRRC (Atomic Read and Clear Bits in CSR) We have thus changed QEMU to allow truly atomic CSR implementations. The new implementation replaces the compiler doing compare/branch vs jump table switch codegen for a sparse CSR address space with a single array of function pointers. i.e. load, indirect jump. Along with this change we have also renamed functions in target/riscv to use riscv_ prefix and added a public interface to hook custom CSRs. The CSR changes will allow out of tree code to hook custom CSRs without needing to change target/riscv code. - riscv_cpu_ won over cpu_riscv_ given the number of functions conforming with the former riscv_ prefix and the desire for consistency in target/riscv In the riscv tree we now have riscv_csr_read(env, CSR_FCSR) and riscv_csr_write(env, CSR_FCSR, fcsr) as the method to read and write the composite. There is also a user in linux-user/riscv/signal.c that should probably use the new interface. We could change linux-user/riscv/signal.c to use your new interface however your interface only provides a read method and no write method, so the write interface remains in the (current) big CSR switch statement, leaving an inconsitency between the encapsulation of read and write. We currently have the new fcsr read and write encapsulated in static functions read_fcsr and write_fcsr in a new csr module (which should perhaps be called csr_helper.c). See: - https://github.com/riscv/riscv-qemu/commits/qemu-2.13-for-upstream - https://github.com/riscv/riscv-qemu/commit/0783ce5ea580552b1f8e2f16a3e3cc1af19db69b - https://github.com/riscv/riscv-qemu/commit/fa17549fbc726e83a3c163b1534c7465147c6718 > --- > target/riscv/cpu.h | 1 + > target/riscv/fpu_helper.c | 6 ++++++ > target/riscv/op_helper.c | 3 +-- > 3 files changed, 8 insertions(+), 2 deletions(-) > > diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h > index 34abc383e3..f2bc243b95 100644 > --- a/target/riscv/cpu.h > +++ b/target/riscv/cpu.h > @@ -265,6 +265,7 @@ void QEMU_NORETURN do_raise_exception_err(CPURISCVState > *env, > uint32_t exception, uintptr_t > pc); > > target_ulong cpu_riscv_get_fflags(CPURISCVState *env); > +target_ulong cpu_riscv_get_fcsr(CPURISCVState *env); > void cpu_riscv_set_fflags(CPURISCVState *env, target_ulong); > > #define TB_FLAGS_MMU_MASK 3 > diff --git a/target/riscv/fpu_helper.c b/target/riscv/fpu_helper.c > index abbadead5c..41c7352115 100644 > --- a/target/riscv/fpu_helper.c > +++ b/target/riscv/fpu_helper.c > @@ -37,6 +37,12 @@ target_ulong cpu_riscv_get_fflags(CPURISCVState *env) > return hard; > } > > +target_ulong cpu_riscv_get_fcsr(CPURISCVState *env) > +{ > + return (cpu_riscv_get_fflags(env) << FSR_AEXC_SHIFT) > + | (env->frm << FSR_RD_SHIFT); > +} > + > void cpu_riscv_set_fflags(CPURISCVState *env, target_ulong hard) > { > int soft = 0; > diff --git a/target/riscv/op_helper.c b/target/riscv/op_helper.c > index 3abf52453c..fd2d8c0a9d 100644 > --- a/target/riscv/op_helper.c > +++ b/target/riscv/op_helper.c > @@ -423,8 +423,7 @@ target_ulong csr_read_helper(CPURISCVState *env, > target_ulong csrno) > return env->frm; > case CSR_FCSR: > validate_mstatus_fs(env, GETPC()); > - return (cpu_riscv_get_fflags(env) << FSR_AEXC_SHIFT) > - | (env->frm << FSR_RD_SHIFT); > + return cpu_riscv_get_fcsr(env); > /* rdtime/rdtimeh is trapped and emulated by bbl in system mode */ > #ifdef CONFIG_USER_ONLY > case CSR_TIME: > -- > 2.17.0 > >
On 05/17/2018 07:46 PM, Michael Clark wrote: > > > On Fri, May 11, 2018 at 3:52 PM, Richard Henderson > <richard.henderson@linaro.org <mailto:richard.henderson@linaro.org>> wrote: > > Cc: Michael Clark <mjc@sifive.com <mailto:mjc@sifive.com>> > Cc: Palmer Dabbelt <palmer@sifive.com <mailto:palmer@sifive.com>> > Cc: Sagar Karandikar <sagark@eecs.berkeley.edu > <mailto:sagark@eecs.berkeley.edu>> > Cc: Bastian Koppelmann <kbastian@mail.uni-paderborn.de > <mailto:kbastian@mail.uni-paderborn.de>> > Signed-off-by: Richard Henderson <richard.henderson@linaro.org > <mailto:richard.henderson@linaro.org>> > > > I'm not against this change but it conflicts with changes in the riscv repo. I > should post my patch queue to the list... Ok, I'll drop this for now, and the dump of the FCSR in the next patch. To be revisited once your csr reorg is upstream... r~
diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h index 34abc383e3..f2bc243b95 100644 --- a/target/riscv/cpu.h +++ b/target/riscv/cpu.h @@ -265,6 +265,7 @@ void QEMU_NORETURN do_raise_exception_err(CPURISCVState *env, uint32_t exception, uintptr_t pc); target_ulong cpu_riscv_get_fflags(CPURISCVState *env); +target_ulong cpu_riscv_get_fcsr(CPURISCVState *env); void cpu_riscv_set_fflags(CPURISCVState *env, target_ulong); #define TB_FLAGS_MMU_MASK 3 diff --git a/target/riscv/fpu_helper.c b/target/riscv/fpu_helper.c index abbadead5c..41c7352115 100644 --- a/target/riscv/fpu_helper.c +++ b/target/riscv/fpu_helper.c @@ -37,6 +37,12 @@ target_ulong cpu_riscv_get_fflags(CPURISCVState *env) return hard; } +target_ulong cpu_riscv_get_fcsr(CPURISCVState *env) +{ + return (cpu_riscv_get_fflags(env) << FSR_AEXC_SHIFT) + | (env->frm << FSR_RD_SHIFT); +} + void cpu_riscv_set_fflags(CPURISCVState *env, target_ulong hard) { int soft = 0; diff --git a/target/riscv/op_helper.c b/target/riscv/op_helper.c index 3abf52453c..fd2d8c0a9d 100644 --- a/target/riscv/op_helper.c +++ b/target/riscv/op_helper.c @@ -423,8 +423,7 @@ target_ulong csr_read_helper(CPURISCVState *env, target_ulong csrno) return env->frm; case CSR_FCSR: validate_mstatus_fs(env, GETPC()); - return (cpu_riscv_get_fflags(env) << FSR_AEXC_SHIFT) - | (env->frm << FSR_RD_SHIFT); + return cpu_riscv_get_fcsr(env); /* rdtime/rdtimeh is trapped and emulated by bbl in system mode */ #ifdef CONFIG_USER_ONLY case CSR_TIME:
Cc: Michael Clark <mjc@sifive.com> Cc: Palmer Dabbelt <palmer@sifive.com> Cc: Sagar Karandikar <sagark@eecs.berkeley.edu> Cc: Bastian Koppelmann <kbastian@mail.uni-paderborn.de> Signed-off-by: Richard Henderson <richard.henderson@linaro.org> --- target/riscv/cpu.h | 1 + target/riscv/fpu_helper.c | 6 ++++++ target/riscv/op_helper.c | 3 +-- 3 files changed, 8 insertions(+), 2 deletions(-) -- 2.17.0