| Message ID | cover.1603467169.git.alistair.francis@wdc.com |
|---|---|
| Headers | show |
| Series | RISC-V: Start to remove xlen preprocess | expand |
On 10/23/20 8:33 AM, Alistair Francis wrote:
> + qemu_fprintf(f, " %s %016lx\n", "vsstatus ", env->vsstatus);
You need PRIx64 here.
r~
Hi Alistair, On Fri, Oct 23, 2020 at 11:44 PM Alistair Francis <alistair.francis@wdc.com> wrote: > > The RISC-V QEMU port currently has lot of preprocessor directives that > check if we are targetting a 32-bit or 64-bit CPU. This means that the > 64-bit RISC-V target can not run 32-bit CPUs. This is different to most > other QEMU architectures and doesn't allow us to mix xlens (such as when > running Hypervisors with different xlen guests). > > This series is a step toward removing some of those to allow us to use > 32-bit CPUs on 64-bit RISC-V targets. > > Alistair Francis (16): > target/riscv: Add a TYPE_RISCV_CPU_BASE CPU > riscv: spike: Remove target macro conditionals > riscv: virt: Remove target macro conditionals > hw/riscv: boot: Remove compile time XLEN checks > hw/riscv: virt: Remove compile time XLEN checks > hw/riscv: spike: Remove compile time XLEN checks > hw/riscv: sifive_u: Remove compile time XLEN checks > target/riscv: fpu_helper: Match function defs in HELPER macros > target/riscv: Add a riscv_cpu_is_32bit() helper function > target/riscv: Specify the XLEN for CPUs > target/riscv: cpu: Remove compile time XLEN checks > target/riscv: cpu_helper: Remove compile time XLEN checks > target/riscv: csr: Remove compile time XLEN checks > target/riscv: cpu: Set XLEN independently from target > target/riscv: Convert the get/set_field() to support 64-bit values > target/riscv: Consolidate *statush registers > > include/hw/riscv/boot.h | 8 +- > include/hw/riscv/spike.h | 6 -- > include/hw/riscv/virt.h | 6 -- > target/riscv/cpu.h | 36 ++++--- > target/riscv/cpu_bits.h | 24 ++--- > hw/riscv/boot.c | 55 ++++++----- > hw/riscv/sifive_u.c | 59 ++++++------ > hw/riscv/spike.c | 50 +++++----- > hw/riscv/virt.c | 36 +++---- > target/riscv/cpu.c | 83 ++++++++++------ > target/riscv/cpu_helper.c | 47 +++------ > target/riscv/csr.c | 197 ++++++++++++++++++++------------------ > target/riscv/fpu_helper.c | 16 ++-- > target/riscv/op_helper.c | 11 +-- > 14 files changed, 328 insertions(+), 306 deletions(-) Test result: 64-bit virt & sifive_u, boots OpenSBI and kernel. 32-bit virt boots OpenSBI and kernel. 32-bit sifive_u does not boot OpenSBI. Regards, Bin
On Fri, Oct 23, 2020 at 11:44 PM Alistair Francis <alistair.francis@wdc.com> wrote: > > Signed-off-by: Alistair Francis <alistair.francis@wdc.com> > --- > target/riscv/cpu.h | 6 ++++++ > 1 file changed, 6 insertions(+) > Reviewed-by: Bin Meng <bin.meng@windriver.com> Tested-by: Bin Meng <bin.meng@windriver.com>
On Fri, Oct 23, 2020 at 11:44 PM Alistair Francis <alistair.francis@wdc.com> wrote: > > Signed-off-by: Alistair Francis <alistair.francis@wdc.com> > --- > include/hw/riscv/spike.h | 6 ------ > hw/riscv/spike.c | 2 +- > 2 files changed, 1 insertion(+), 7 deletions(-) > Reviewed-by: Bin Meng <bin.meng@windriver.com>
On Fri, Oct 23, 2020 at 11:44 PM Alistair Francis <alistair.francis@wdc.com> wrote: > > Signed-off-by: Alistair Francis <alistair.francis@wdc.com> > --- > include/hw/riscv/boot.h | 8 +++--- > hw/riscv/boot.c | 55 ++++++++++++++++++++++------------------- > hw/riscv/sifive_u.c | 2 +- > hw/riscv/spike.c | 3 ++- > hw/riscv/virt.c | 2 +- > 5 files changed, 39 insertions(+), 31 deletions(-) > Reviewed-by: Bin Meng <bin.meng@windriver.com> Tested-by: Bin Meng <bin.meng@windriver.com>
On Fri, Oct 23, 2020 at 11:45 PM Alistair Francis <alistair.francis@wdc.com> wrote: > > Signed-off-by: Alistair Francis <alistair.francis@wdc.com> > --- > target/riscv/cpu.h | 24 ++++++++++++++--------- > target/riscv/cpu_bits.h | 14 ++----------- > target/riscv/cpu.c | 7 ++----- > target/riscv/cpu_helper.c | 41 ++++++++++----------------------------- > target/riscv/csr.c | 15 ++++++++------ > target/riscv/op_helper.c | 11 ++++------- > 6 files changed, 42 insertions(+), 70 deletions(-) > > diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h > index 8bde15544d..de864a947b 100644 > --- a/target/riscv/cpu.h > +++ b/target/riscv/cpu.h > @@ -150,9 +150,11 @@ struct CPURISCVState { > target_ulong resetvec; > > target_ulong mhartid; > - target_ulong mstatus; > - /* This is RV32 only */ > - target_ulong mstatush; > + /* > + * For RV32 this is 32-bit mstatus and 32-bit mstatush. > + * For RV64 this is a 64-bit mstatus. > + */ > + uint64_t mstatus; > > target_ulong mip; > > @@ -187,15 +189,17 @@ struct CPURISCVState { > uint64_t htimedelta; > > /* Virtual CSRs */ > - target_ulong vsstatus; > target_ulong vstvec; > target_ulong vsscratch; > target_ulong vsepc; > target_ulong vscause; > target_ulong vstval; > target_ulong vsatp; > - /* This is RV32 only */ > - target_ulong vsstatush; > + /* > + * For RV32 this is 32-bit mstatus and 32-bit mstatush. > + * For RV64 this is a 64-bit mstatus. > + */ > + uint64_t vsstatus; > > target_ulong mtval2; > target_ulong mtinst; > @@ -207,9 +211,11 @@ struct CPURISCVState { > target_ulong scause_hs; > target_ulong stval_hs; > target_ulong satp_hs; > - target_ulong mstatus_hs; > - /* This is RV32 only */ > - target_ulong mstatush_hs; > + /* > + * For RV32 this is 32-bit mstatus and 32-bit mstatush. > + * For RV64 this is a 64-bit mstatus. > + */ > + uint64_t mstatus_hs; > > target_ulong scounteren; > target_ulong mcounteren; > diff --git a/target/riscv/cpu_bits.h b/target/riscv/cpu_bits.h > index db46739b1c..4b75d2766e 100644 > --- a/target/riscv/cpu_bits.h > +++ b/target/riscv/cpu_bits.h > @@ -381,19 +381,9 @@ > #define MSTATUS_TVM 0x00100000 /* since: priv-1.10 */ > #define MSTATUS_TW 0x20000000 /* since: priv-1.10 */ > #define MSTATUS_TSR 0x40000000 /* since: priv-1.10 */ > -#if defined(TARGET_RISCV64) > -#define MSTATUS_GVA 0x4000000000ULL > -#define MSTATUS_MPV 0x8000000000ULL > -#elif defined(TARGET_RISCV32) > -#define MSTATUS_GVA 0x00000040 > -#define MSTATUS_MPV 0x00000080 > -#endif > > -#ifdef TARGET_RISCV32 > -# define MSTATUS_MPV_ISSET(env) get_field(env->mstatush, MSTATUS_MPV) > -#else > -# define MSTATUS_MPV_ISSET(env) get_field(env->mstatus, MSTATUS_MPV) > -#endif > +#define MSTATUS_GVA 0x4000000000ULL > +#define MSTATUS_MPV 0x8000000000ULL nits: the indentation level is not aligned to other macros around > > #define MSTATUS64_UXL 0x0000000300000000ULL > #define MSTATUS64_SXL 0x0000000C00000000ULL > diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c > index c77687ccb2..74c5ac5777 100644 > --- a/target/riscv/cpu.c > +++ b/target/riscv/cpu.c > @@ -245,13 +245,10 @@ static void riscv_cpu_dump_state(CPUState *cs, FILE *f, int flags) > qemu_fprintf(f, " %s " TARGET_FMT_lx "\n", "pc ", env->pc); > #ifndef CONFIG_USER_ONLY > qemu_fprintf(f, " %s " TARGET_FMT_lx "\n", "mhartid ", env->mhartid); > - qemu_fprintf(f, " %s " TARGET_FMT_lx "\n", "mstatus ", env->mstatus); > - if (riscv_cpu_is_32bit(env)) { > - qemu_fprintf(f, " %s " TARGET_FMT_lx "\n", "mstatush ", env->mstatush); > - } > + qemu_fprintf(f, " %s %016lx\n", "mstatus ", env->mstatus); > if (riscv_has_ext(env, RVH)) { > qemu_fprintf(f, " %s " TARGET_FMT_lx "\n", "hstatus ", env->hstatus); > - qemu_fprintf(f, " %s " TARGET_FMT_lx "\n", "vsstatus ", env->vsstatus); > + qemu_fprintf(f, " %s %016lx\n", "vsstatus ", env->vsstatus); > } > qemu_fprintf(f, " %s " TARGET_FMT_lx "\n", "mip ", env->mip); > qemu_fprintf(f, " %s " TARGET_FMT_lx "\n", "mie ", env->mie); > diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c > index 62aed24feb..b109529ace 100644 > --- a/target/riscv/cpu_helper.c > +++ b/target/riscv/cpu_helper.c > @@ -110,27 +110,19 @@ bool riscv_cpu_fp_enabled(CPURISCVState *env) > > void riscv_cpu_swap_hypervisor_regs(CPURISCVState *env) > { > - target_ulong mstatus_mask = MSTATUS_MXR | MSTATUS_SUM | MSTATUS_FS | > - MSTATUS_SPP | MSTATUS_SPIE | MSTATUS_SIE; > + uint64_t mstatus_mask = MSTATUS_MXR | MSTATUS_SUM | MSTATUS_FS | > + MSTATUS_SPP | MSTATUS_SPIE | MSTATUS_SIE | > + MSTATUS64_UXL; > bool current_virt = riscv_cpu_virt_enabled(env); > > g_assert(riscv_has_ext(env, RVH)); > > -#if defined(TARGET_RISCV64) > - mstatus_mask |= MSTATUS64_UXL; > -#endif > - > if (current_virt) { > /* Current V=1 and we are about to change to V=0 */ > env->vsstatus = env->mstatus & mstatus_mask; > env->mstatus &= ~mstatus_mask; > env->mstatus |= env->mstatus_hs; > > - if (riscv_cpu_is_32bit(env)) { > - env->vsstatush = env->mstatush; > - env->mstatush |= env->mstatush_hs; > - } > - > env->vstvec = env->stvec; > env->stvec = env->stvec_hs; > > @@ -154,11 +146,6 @@ void riscv_cpu_swap_hypervisor_regs(CPURISCVState *env) > env->mstatus &= ~mstatus_mask; > env->mstatus |= env->vsstatus; > > - if (riscv_cpu_is_32bit(env)) { > - env->mstatush_hs = env->mstatush; > - env->mstatush |= env->vsstatush; > - } > - > env->stvec_hs = env->stvec; > env->stvec = env->vstvec; > > @@ -729,7 +716,7 @@ bool riscv_cpu_tlb_fill(CPUState *cs, vaddr address, int size, > if (riscv_has_ext(env, RVH) && env->priv == PRV_M && > access_type != MMU_INST_FETCH && > get_field(env->mstatus, MSTATUS_MPRV) && > - MSTATUS_MPV_ISSET(env)) { > + get_field(env->mstatus, MSTATUS_MPV)) { > riscv_cpu_set_two_stage_lookup(env, true); > } > > @@ -801,7 +788,7 @@ bool riscv_cpu_tlb_fill(CPUState *cs, vaddr address, int size, > if (riscv_has_ext(env, RVH) && env->priv == PRV_M && > access_type != MMU_INST_FETCH && > get_field(env->mstatus, MSTATUS_MPRV) && > - MSTATUS_MPV_ISSET(env)) { > + get_field(env->mstatus, MSTATUS_MPV)) { > riscv_cpu_set_two_stage_lookup(env, false); > } > > @@ -864,7 +851,7 @@ void riscv_cpu_do_interrupt(CPUState *cs) > RISCVCPU *cpu = RISCV_CPU(cs); > CPURISCVState *env = &cpu->env; > bool force_hs_execp = riscv_cpu_force_hs_excep_enabled(env); > - target_ulong s; > + uint64_t s; > > /* cs->exception is 32-bits wide unlike mcause which is XLEN-bits wide > * so we mask off the MSB and separate into trap type and cause. > @@ -997,18 +984,10 @@ void riscv_cpu_do_interrupt(CPUState *cs) > if (riscv_cpu_virt_enabled(env)) { > riscv_cpu_swap_hypervisor_regs(env); > } > - if (riscv_cpu_is_32bit(env)) { > - env->mstatush = set_field(env->mstatush, MSTATUS_MPV, > - riscv_cpu_virt_enabled(env)); > - if (riscv_cpu_virt_enabled(env) && tval) { > - env->mstatush = set_field(env->mstatush, MSTATUS_GVA, 1); > - } > - } else { > - env->mstatus = set_field(env->mstatus, MSTATUS_MPV, > - riscv_cpu_virt_enabled(env)); > - if (riscv_cpu_virt_enabled(env) && tval) { > - env->mstatus = set_field(env->mstatus, MSTATUS_GVA, 1); > - } > + env->mstatus = set_field(env->mstatus, MSTATUS_MPV, > + riscv_cpu_virt_enabled(env)); > + if (riscv_cpu_virt_enabled(env) && tval) { > + env->mstatus = set_field(env->mstatus, MSTATUS_GVA, 1); > } > > mtval2 = env->guest_phys_fault_addr; > diff --git a/target/riscv/csr.c b/target/riscv/csr.c > index b153cdfec3..7132a9bcd0 100644 > --- a/target/riscv/csr.c > +++ b/target/riscv/csr.c > @@ -473,8 +473,8 @@ static int validate_vm(CPURISCVState *env, target_ulong vm) > > static int write_mstatus(CPURISCVState *env, int csrno, target_ulong val) > { > - target_ulong mstatus = env->mstatus; > - target_ulong mask = 0; > + uint64_t mstatus = env->mstatus; > + uint64_t mask = 0; > int dirty; > > /* flush tlb on mstatus fields that affect VM */ > @@ -507,19 +507,22 @@ static int write_mstatus(CPURISCVState *env, int csrno, target_ulong val) > > static int read_mstatush(CPURISCVState *env, int csrno, target_ulong *val) > { > - *val = env->mstatush; > + *val = env->mstatus >> 32; > return 0; > } > > static int write_mstatush(CPURISCVState *env, int csrno, target_ulong val) > { > - if ((val ^ env->mstatush) & (MSTATUS_MPV)) { > + uint64_t val64 = (uint64_t) val << 32; > + > + if ((val64 ^ env->mstatus) & (MSTATUS_MPV)) { > tlb_flush(env_cpu(env)); > } > > - val &= MSTATUS_MPV | MSTATUS_GVA; > + val64 &= MSTATUS_MPV | MSTATUS_GVA; > > - env->mstatush = val; > + env->mstatus &= 0xFFFFFFFF; > + env->mstatus |= val64; > > return 0; > } > diff --git a/target/riscv/op_helper.c b/target/riscv/op_helper.c > index 4ce73575a7..f9361e3af8 100644 > --- a/target/riscv/op_helper.c > +++ b/target/riscv/op_helper.c > @@ -78,7 +78,8 @@ target_ulong helper_csrrc(CPURISCVState *env, target_ulong src, > > target_ulong helper_sret(CPURISCVState *env, target_ulong cpu_pc_deb) > { > - target_ulong prev_priv, prev_virt, mstatus; > + target_ulong prev_priv, prev_virt; > + uint64_t mstatus; > > if (!(env->priv >= PRV_S)) { > riscv_raise_exception(env, RISCV_EXCP_ILLEGAL_INST, GETPC()); > @@ -147,18 +148,14 @@ target_ulong helper_mret(CPURISCVState *env, target_ulong cpu_pc_deb) > riscv_raise_exception(env, RISCV_EXCP_INST_ADDR_MIS, GETPC()); > } > > - target_ulong mstatus = env->mstatus; > + uint64_t mstatus = env->mstatus; > target_ulong prev_priv = get_field(mstatus, MSTATUS_MPP); > - target_ulong prev_virt = MSTATUS_MPV_ISSET(env); > + target_ulong prev_virt = !!get_field(env->mstatus, MSTATUS_MPV); > mstatus = set_field(mstatus, MSTATUS_MIE, > get_field(mstatus, MSTATUS_MPIE)); > mstatus = set_field(mstatus, MSTATUS_MPIE, 1); > mstatus = set_field(mstatus, MSTATUS_MPP, PRV_U); > -#ifdef TARGET_RISCV32 > - env->mstatush = set_field(env->mstatush, MSTATUS_MPV, 0); > -#else > mstatus = set_field(mstatus, MSTATUS_MPV, 0); > -#endif > env->mstatus = mstatus; > riscv_cpu_set_mode(env, prev_priv); > Regards, Bin
On Mon, Oct 26, 2020 at 1:55 AM Bin Meng <bmeng.cn@gmail.com> wrote: > > Hi Alistair, > > On Fri, Oct 23, 2020 at 11:44 PM Alistair Francis > <alistair.francis@wdc.com> wrote: > > > > The RISC-V QEMU port currently has lot of preprocessor directives that > > check if we are targetting a 32-bit or 64-bit CPU. This means that the > > 64-bit RISC-V target can not run 32-bit CPUs. This is different to most > > other QEMU architectures and doesn't allow us to mix xlens (such as when > > running Hypervisors with different xlen guests). > > > > This series is a step toward removing some of those to allow us to use > > 32-bit CPUs on 64-bit RISC-V targets. > > > > Alistair Francis (16): > > target/riscv: Add a TYPE_RISCV_CPU_BASE CPU > > riscv: spike: Remove target macro conditionals > > riscv: virt: Remove target macro conditionals > > hw/riscv: boot: Remove compile time XLEN checks > > hw/riscv: virt: Remove compile time XLEN checks > > hw/riscv: spike: Remove compile time XLEN checks > > hw/riscv: sifive_u: Remove compile time XLEN checks > > target/riscv: fpu_helper: Match function defs in HELPER macros > > target/riscv: Add a riscv_cpu_is_32bit() helper function > > target/riscv: Specify the XLEN for CPUs > > target/riscv: cpu: Remove compile time XLEN checks > > target/riscv: cpu_helper: Remove compile time XLEN checks > > target/riscv: csr: Remove compile time XLEN checks > > target/riscv: cpu: Set XLEN independently from target > > target/riscv: Convert the get/set_field() to support 64-bit values > > target/riscv: Consolidate *statush registers > > > > include/hw/riscv/boot.h | 8 +- > > include/hw/riscv/spike.h | 6 -- > > include/hw/riscv/virt.h | 6 -- > > target/riscv/cpu.h | 36 ++++--- > > target/riscv/cpu_bits.h | 24 ++--- > > hw/riscv/boot.c | 55 ++++++----- > > hw/riscv/sifive_u.c | 59 ++++++------ > > hw/riscv/spike.c | 50 +++++----- > > hw/riscv/virt.c | 36 +++---- > > target/riscv/cpu.c | 83 ++++++++++------ > > target/riscv/cpu_helper.c | 47 +++------ > > target/riscv/csr.c | 197 ++++++++++++++++++++------------------ > > target/riscv/fpu_helper.c | 16 ++-- > > target/riscv/op_helper.c | 11 +-- > > 14 files changed, 328 insertions(+), 306 deletions(-) > > Test result: > > 64-bit virt & sifive_u, boots OpenSBI and kernel. > 32-bit virt boots OpenSBI and kernel. 32-bit sifive_u does not boot OpenSBI. Thanks, I have fixed the failure and added this to my automated test cases. Alistair > > Regards, > Bin