| Message ID | 20221208151159.1155471-1-christoph.muellner@vrull.eu |
|---|---|
| State | New |
| Headers | show |
| Series | [RFC] RISC-V: Save mmu_idx using FIELD_DP32 not OR | expand |
On 2022/12/8 23:11, Christoph Muellner wrote: > From: Christoph Müllner <christoph.muellner@vrull.eu> > > Setting flags using OR might work, but is not optimal > for a couple of reasons: > * No way grep for stores to the field MEM_IDX. > * The return value of cpu_mmu_index() is not masked > (not a real problem as long as cpu_mmu_index() returns only valid values). > * If the offset of MEM_IDX would get moved to non-0, then this code > would not work anymore. > > Let's use the FIELD_DP32() macro instead of the OR, which is already > used for most other flags. > > Signed-off-by: Christoph Müllner <christoph.muellner@vrull.eu> > --- > target/riscv/cpu_helper.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c > index 278d163803..d68b6b351d 100644 > --- a/target/riscv/cpu_helper.c > +++ b/target/riscv/cpu_helper.c > @@ -80,7 +80,8 @@ void cpu_get_tb_cpu_state(CPURISCVState *env, target_ulong *pc, > flags |= TB_FLAGS_MSTATUS_FS; > flags |= TB_FLAGS_MSTATUS_VS; > #else > - flags |= cpu_mmu_index(env, 0); > + flags = FIELD_DP32(flags, TB_FLAGS, MEM_IDX, cpu_mmu_index(env, 0)); > + > if (riscv_cpu_fp_enabled(env)) { > flags |= env->mstatus & MSTATUS_FS; > } We may should rename cpu_mmu_index to cpu_mem_idx and TB_FLAGS_PRIV_MMU_MASK to TB_FLAGS_PRIV_MEM_MASK. We can also remove the TB_FLAGS_PRIV_MMU_MASK as the position of MEM_IDX in tb_flags may change in the future. Otherwise, this patch looks good to me, Reviewed-by: LIU Zhiwei <zhiwei_liu@linux.alibaba.com>
On Fri, Dec 9, 2022 at 1:12 AM Christoph Muellner <christoph.muellner@vrull.eu> wrote: > > From: Christoph Müllner <christoph.muellner@vrull.eu> > > Setting flags using OR might work, but is not optimal > for a couple of reasons: > * No way grep for stores to the field MEM_IDX. > * The return value of cpu_mmu_index() is not masked > (not a real problem as long as cpu_mmu_index() returns only valid values). > * If the offset of MEM_IDX would get moved to non-0, then this code > would not work anymore. > > Let's use the FIELD_DP32() macro instead of the OR, which is already > used for most other flags. > > Signed-off-by: Christoph Müllner <christoph.muellner@vrull.eu> Reviewed-by: Alistair Francis <alistair.francis@wdc.com> Alistair > --- > target/riscv/cpu_helper.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c > index 278d163803..d68b6b351d 100644 > --- a/target/riscv/cpu_helper.c > +++ b/target/riscv/cpu_helper.c > @@ -80,7 +80,8 @@ void cpu_get_tb_cpu_state(CPURISCVState *env, target_ulong *pc, > flags |= TB_FLAGS_MSTATUS_FS; > flags |= TB_FLAGS_MSTATUS_VS; > #else > - flags |= cpu_mmu_index(env, 0); > + flags = FIELD_DP32(flags, TB_FLAGS, MEM_IDX, cpu_mmu_index(env, 0)); > + > if (riscv_cpu_fp_enabled(env)) { > flags |= env->mstatus & MSTATUS_FS; > } > -- > 2.38.1 > >
diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c index 278d163803..d68b6b351d 100644 --- a/target/riscv/cpu_helper.c +++ b/target/riscv/cpu_helper.c @@ -80,7 +80,8 @@ void cpu_get_tb_cpu_state(CPURISCVState *env, target_ulong *pc, flags |= TB_FLAGS_MSTATUS_FS; flags |= TB_FLAGS_MSTATUS_VS; #else - flags |= cpu_mmu_index(env, 0); + flags = FIELD_DP32(flags, TB_FLAGS, MEM_IDX, cpu_mmu_index(env, 0)); + if (riscv_cpu_fp_enabled(env)) { flags |= env->mstatus & MSTATUS_FS; }