1. Patchew
  2. QEMU
  3. [PATCH v1 00/16] RISC-V: Start to remove xlen preprocess
  4. View patch

[PATCH v1 12/16] target/riscv: cpu_helper: Remove compile time XLEN checks

Alistair Francis posted 16 patches 5 years, 1 month ago
There is a newer version of this series
[PATCH v1 12/16] target/riscv: cpu_helper: Remove compile time XLEN checks
Posted by Alistair Francis 5 years, 1 month ago 
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;
-#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));
+                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);
+                }
             }
-#endif
 
             mtval2 = env->guest_phys_fault_addr;
 
-- 
2.28.0
Re: [PATCH v1 12/16] target/riscv: cpu_helper: Remove compile time XLEN checks
Posted by Bin Meng 5 years, 1 month ago 
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?

> -#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 12/16] target/riscv: cpu_helper: Remove compile time XLEN checks
Posted by Alistair Francis 5 years, 1 month ago 
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

Patchew 2.1-devel-b67e4c2

© 2016 - 2025 Red Hat, Inc.