[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v1 12/16] target/riscv: cpu_helper: Remove compile time XLEN
From: |
Alistair Francis |
Subject: |
Re: [PATCH v1 12/16] target/riscv: cpu_helper: Remove compile time XLEN checks |
Date: |
Tue, 27 Oct 2020 13:25:47 -0700 |
On Mon, Oct 26, 2020 at 1:56 AM Bin Meng <bmeng.cn@gmail.com> wrote:
>
> 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 | 6 ++---
> > target/riscv/cpu_helper.c | 52 ++++++++++++++++++++-------------------
> > 2 files changed, 29 insertions(+), 29 deletions(-)
> >
> > diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
> > index 6096243aed..8bde15544d 100644
> > --- a/target/riscv/cpu.h
> > +++ b/target/riscv/cpu.h
> > @@ -194,9 +194,8 @@ struct CPURISCVState {
> > target_ulong vscause;
> > target_ulong vstval;
> > target_ulong vsatp;
> > -#ifdef TARGET_RISCV32
> > + /* This is RV32 only */
> > target_ulong vsstatush;
>
> nits: could we move the definition to the line just below where
> vsstatus is defined in this structure, like other similar *h members?
This has been removed in the latest rebase.
Alistair
>
> > -#endif
> >
> > target_ulong mtval2;
> > target_ulong mtinst;
> > @@ -209,9 +208,8 @@ struct CPURISCVState {
> > target_ulong stval_hs;
> > target_ulong satp_hs;
> > target_ulong mstatus_hs;
> > -#ifdef TARGET_RISCV32
> > + /* This is RV32 only */
> > target_ulong mstatush_hs;
> > -#endif
> >
> > target_ulong scounteren;
> > target_ulong mcounteren;
> > diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c
> > index 4652082df1..62aed24feb 100644
> > --- a/target/riscv/cpu_helper.c
> > +++ b/target/riscv/cpu_helper.c
> > @@ -126,10 +126,10 @@ void riscv_cpu_swap_hypervisor_regs(CPURISCVState
> > *env)
> > env->mstatus &= ~mstatus_mask;
> > env->mstatus |= env->mstatus_hs;
> >
> > -#if defined(TARGET_RISCV32)
> > - env->vsstatush = env->mstatush;
> > - env->mstatush |= env->mstatush_hs;
> > -#endif
> > + 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,10 +154,10 @@ void riscv_cpu_swap_hypervisor_regs(CPURISCVState
> > *env)
> > env->mstatus &= ~mstatus_mask;
> > env->mstatus |= env->vsstatus;
> >
> > -#if defined(TARGET_RISCV32)
> > - env->mstatush_hs = env->mstatush;
> > - env->mstatush |= env->vsstatush;
> > -#endif
> > + if (riscv_cpu_is_32bit(env)) {
> > + env->mstatush_hs = env->mstatush;
> > + env->mstatush |= env->vsstatush;
> > + }
> >
> > env->stvec_hs = env->stvec;
> > env->stvec = env->vstvec;
> > @@ -472,11 +472,13 @@ restart:
> > return TRANSLATE_PMP_FAIL;
> > }
> >
> > -#if defined(TARGET_RISCV32)
> > - target_ulong pte = address_space_ldl(cs->as, pte_addr, attrs,
> > &res);
> > -#elif defined(TARGET_RISCV64)
> > - target_ulong pte = address_space_ldq(cs->as, pte_addr, attrs,
> > &res);
> > -#endif
> > + target_ulong pte;
> > + if (riscv_cpu_is_32bit(env)) {
> > + pte = address_space_ldl(cs->as, pte_addr, attrs, &res);
> > + } else {
> > + pte = address_space_ldq(cs->as, pte_addr, attrs, &res);
> > + }
> > +
> > if (res != MEMTX_OK) {
> > return TRANSLATE_FAIL;
> > }
> > @@ -995,19 +997,19 @@ void riscv_cpu_do_interrupt(CPUState *cs)
> > if (riscv_cpu_virt_enabled(env)) {
> > riscv_cpu_swap_hypervisor_regs(env);
> > }
> > -#ifdef TARGET_RISCV32
> > - 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);
> > + if (riscv_cpu_is_32bit(env)) {
> > + env->mstatush = set_field(env->mstatush, MSTATUS_MPV,
> > + riscv_cpu_virt_enabled(env));
>
> nits: looks the alignment is not on the left parenthesis
>
> > + 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));
>
> ditto
>
> > + if (riscv_cpu_virt_enabled(env) && tval) {
> > + env->mstatus = set_field(env->mstatus, MSTATUS_GVA, 1);
> > + }
> > }
> > -#endif
> >
> > mtval2 = env->guest_phys_fault_addr;
> >
>
> Regards,
> Bin
- Re: [PATCH v1 08/16] target/riscv: fpu_helper: Match function defs in HELPER macros, (continued)
- [PATCH v1 09/16] target/riscv: Add a riscv_cpu_is_32bit() helper function, Alistair Francis, 2020/10/23
- [PATCH v1 10/16] target/riscv: Specify the XLEN for CPUs, Alistair Francis, 2020/10/23
- [PATCH v1 11/16] target/riscv: cpu: Remove compile time XLEN checks, Alistair Francis, 2020/10/23
- [PATCH v1 12/16] target/riscv: cpu_helper: Remove compile time XLEN checks, Alistair Francis, 2020/10/23
- [PATCH v1 15/16] target/riscv: Convert the get/set_field() to support 64-bit values, Alistair Francis, 2020/10/23
- [PATCH v1 13/16] target/riscv: csr: Remove compile time XLEN checks, Alistair Francis, 2020/10/23
- [PATCH v1 16/16] target/riscv: Consolidate *statush registers, Alistair Francis, 2020/10/23
- [PATCH v1 14/16] target/riscv: cpu: Set XLEN independently from target, Alistair Francis, 2020/10/23
- Re: [PATCH v1 00/16] RISC-V: Start to remove xlen preprocess, Bin Meng, 2020/10/26