From: Alistair Francis <alistair23@gmail.com>
To: Bin Meng <bmeng.cn@gmail.com>
Cc: "open list:RISC-V" <qemu-riscv@nongnu.org>,
Palmer Dabbelt <palmer@dabbelt.com>,
Alistair Francis <alistair.francis@wdc.com>,
"qemu-devel@nongnu.org Developers" <qemu-devel@nongnu.org>
Subject: Re: [PATCH v1 12/16] target/riscv: cpu_helper: Remove compile time XLEN checks
Date: Tue, 27 Oct 2020 13:25:47 -0700 [thread overview]
Message-ID: <CAKmqyKM5bNqK0K4f9by0vQFbmm7C0Gxqy_V=2Ao8i=HB8k=EPg@mail.gmail.com> (raw)
In-Reply-To: <CAEUhbmVtYUro2QBKKhDkQNF3go7HjNr-QBj0=msvG9_hhhHYgg@mail.gmail.com>
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
WARNING: multiple messages have this Message-ID (diff)
From: Alistair Francis <alistair23@gmail.com>
To: Bin Meng <bmeng.cn@gmail.com>
Cc: Alistair Francis <alistair.francis@wdc.com>,
"qemu-devel@nongnu.org Developers" <qemu-devel@nongnu.org>,
"open list:RISC-V" <qemu-riscv@nongnu.org>,
Palmer Dabbelt <palmer@dabbelt.com>
Subject: Re: [PATCH v1 12/16] target/riscv: cpu_helper: Remove compile time XLEN checks
Date: Tue, 27 Oct 2020 13:25:47 -0700 [thread overview]
Message-ID: <CAKmqyKM5bNqK0K4f9by0vQFbmm7C0Gxqy_V=2Ao8i=HB8k=EPg@mail.gmail.com> (raw)
In-Reply-To: <CAEUhbmVtYUro2QBKKhDkQNF3go7HjNr-QBj0=msvG9_hhhHYgg@mail.gmail.com>
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
next prev parent reply other threads:[~2020-10-27 20:38 UTC|newest]
Thread overview: 76+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-10-23 15:33 [PATCH v1 00/16] RISC-V: Start to remove xlen preprocess Alistair Francis
2020-10-23 15:33 ` Alistair Francis
2020-10-23 15:33 ` [PATCH v1 01/16] target/riscv: Add a TYPE_RISCV_CPU_BASE CPU Alistair Francis
2020-10-23 15:33 ` Alistair Francis
2020-10-26 8:55 ` Bin Meng
2020-10-26 8:55 ` Bin Meng
2020-10-23 15:33 ` [PATCH v1 02/16] riscv: spike: Remove target macro conditionals Alistair Francis
2020-10-23 15:33 ` Alistair Francis
2020-10-26 8:55 ` Bin Meng
2020-10-26 8:55 ` Bin Meng
2020-10-23 15:33 ` [PATCH v1 03/16] riscv: virt: " Alistair Francis
2020-10-23 15:33 ` Alistair Francis
2020-10-26 8:55 ` Bin Meng
2020-10-26 8:55 ` Bin Meng
2020-10-23 15:33 ` [PATCH v1 04/16] hw/riscv: boot: Remove compile time XLEN checks Alistair Francis
2020-10-23 15:33 ` Alistair Francis
2020-10-26 8:55 ` Bin Meng
2020-10-26 8:55 ` Bin Meng
2020-10-23 15:33 ` [PATCH v1 05/16] hw/riscv: virt: " Alistair Francis
2020-10-23 15:33 ` Alistair Francis
2020-10-26 8:55 ` Bin Meng
2020-10-26 8:55 ` Bin Meng
2020-10-23 15:33 ` [PATCH v1 06/16] hw/riscv: spike: " Alistair Francis
2020-10-23 15:33 ` Alistair Francis
2020-10-26 8:55 ` Bin Meng
2020-10-26 8:55 ` Bin Meng
2020-10-23 15:33 ` [PATCH v1 07/16] hw/riscv: sifive_u: " Alistair Francis
2020-10-23 15:33 ` Alistair Francis
2020-10-26 8:56 ` Bin Meng
2020-10-26 8:56 ` Bin Meng
2020-10-26 15:12 ` Alistair Francis
2020-10-26 15:12 ` Alistair Francis
2020-10-23 15:33 ` [PATCH v1 08/16] target/riscv: fpu_helper: Match function defs in HELPER macros Alistair Francis
2020-10-23 15:33 ` Alistair Francis
2020-10-26 8:56 ` Bin Meng
2020-10-26 8:56 ` Bin Meng
2020-10-26 15:16 ` Alistair Francis
2020-10-26 15:16 ` Alistair Francis
2020-10-23 15:33 ` [PATCH v1 09/16] target/riscv: Add a riscv_cpu_is_32bit() helper function Alistair Francis
2020-10-23 15:33 ` Alistair Francis
2020-10-26 8:56 ` Bin Meng
2020-10-26 8:56 ` Bin Meng
2020-10-23 15:33 ` [PATCH v1 10/16] target/riscv: Specify the XLEN for CPUs Alistair Francis
2020-10-23 15:33 ` Alistair Francis
2020-10-26 8:56 ` Bin Meng
2020-10-26 8:56 ` Bin Meng
2020-10-23 15:33 ` [PATCH v1 11/16] target/riscv: cpu: Remove compile time XLEN checks Alistair Francis
2020-10-23 15:33 ` Alistair Francis
2020-10-26 8:56 ` Bin Meng
2020-10-26 8:56 ` Bin Meng
2020-10-23 15:33 ` [PATCH v1 12/16] target/riscv: cpu_helper: " Alistair Francis
2020-10-23 15:33 ` Alistair Francis
2020-10-26 8:56 ` Bin Meng
2020-10-26 8:56 ` Bin Meng
2020-10-27 20:25 ` Alistair Francis [this message]
2020-10-27 20:25 ` Alistair Francis
2020-10-23 15:33 ` [PATCH v1 13/16] target/riscv: csr: " Alistair Francis
2020-10-23 15:33 ` Alistair Francis
2020-10-26 8:56 ` Bin Meng
2020-10-26 8:56 ` Bin Meng
2020-10-23 15:33 ` [PATCH v1 14/16] target/riscv: cpu: Set XLEN independently from target Alistair Francis
2020-10-23 15:33 ` Alistair Francis
2020-10-26 8:56 ` Bin Meng
2020-10-26 8:56 ` Bin Meng
2020-10-23 15:33 ` [PATCH v1 15/16] target/riscv: Convert the get/set_field() to support 64-bit values Alistair Francis
2020-10-23 15:33 ` Alistair Francis
2020-10-23 15:33 ` [PATCH v1 16/16] target/riscv: Consolidate *statush registers Alistair Francis
2020-10-23 15:33 ` Alistair Francis
2020-10-23 16:55 ` Richard Henderson
2020-10-23 16:55 ` Richard Henderson
2020-10-26 8:56 ` Bin Meng
2020-10-26 8:56 ` Bin Meng
2020-10-26 8:55 ` [PATCH v1 00/16] RISC-V: Start to remove xlen preprocess Bin Meng
2020-10-26 8:55 ` Bin Meng
2020-10-26 16:32 ` Alistair Francis
2020-10-26 16:32 ` Alistair Francis
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to='CAKmqyKM5bNqK0K4f9by0vQFbmm7C0Gxqy_V=2Ao8i=HB8k=EPg@mail.gmail.com' \
--to=alistair23@gmail.com \
--cc=alistair.francis@wdc.com \
--cc=bmeng.cn@gmail.com \
--cc=palmer@dabbelt.com \
--cc=qemu-devel@nongnu.org \
--cc=qemu-riscv@nongnu.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.