From: Palmer Dabbelt <palmer@sifive.com>
To: alistair23@gmail.com
Cc: qemu-riscv@nongnu.org,
Alistair Francis <Alistair.Francis@wdc.com>,
qemu-devel@nongnu.org
Subject: Re: [Qemu-devel] [PATCH v1 9/9] target/riscv: Add Zifencei and Zicsr as command line options
Date: Tue, 25 Jun 2019 03:08:30 -0700 (PDT) [thread overview]
Message-ID: <mhng-94acab2f-b1df-4a4b-a29a-d51dd0405476@palmer-si-x1e> (raw)
In-Reply-To: <CAKmqyKPhty4-EbgXTHMhUOGVkVnxCzmRB14wSbLG0BSrdzbv1A@mail.gmail.com>
On Mon, 24 Jun 2019 16:16:30 PDT (-0700), alistair23@gmail.com wrote:
> On Mon, Jun 24, 2019 at 2:31 AM Palmer Dabbelt <palmer@sifive.com> wrote:
>>
>> On Mon, 17 Jun 2019 18:31:25 PDT (-0700), Alistair Francis wrote:
>> > For completeness let's add Zifencei and Zicsr as command line options,
>> > even though they can't be disabled at the moment.
>> >
>> > Signed-off-by: Alistair Francis <alistair.francis@wdc.com>
>> > ---
>> > target/riscv/cpu.c | 9 +++++++++
>> > target/riscv/cpu.h | 2 ++
>> > 2 files changed, 11 insertions(+)
>> >
>> > diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
>> > index 5af1c9b38c..53cf8607f7 100644
>> > --- a/target/riscv/cpu.c
>> > +++ b/target/riscv/cpu.c
>> > @@ -409,6 +409,13 @@ static void riscv_cpu_realize(DeviceState *dev, Error **errp)
>> > set_misa(env, RVXLEN | target_misa);
>> > }
>> >
>> > + if (!cpu->cfg.ext_ifencei) {
>> > + warn_report("QEMU does not support disabling Zifencei");
>> > + }
>> > + if (!cpu->cfg.ext_icsr) {
>> > + warn_report("QEMU does not support disabling Zicsr");
>> > + }
>> > +
>> > riscv_cpu_register_gdb_regs_for_features(cs);
>> >
>> > qemu_init_vcpu(cs);
>> > @@ -441,6 +448,8 @@ static Property riscv_cpu_properties[] = {
>> > DEFINE_PROP_BOOL("s", RISCVCPU, cfg.ext_s, true),
>> > DEFINE_PROP_BOOL("u", RISCVCPU, cfg.ext_u, true),
>> > DEFINE_PROP_BOOL("Counters", RISCVCPU, cfg.ext_counters, true),
>> > + DEFINE_PROP_BOOL("Zifencei", RISCVCPU, cfg.ext_ifencei, true),
>> > + DEFINE_PROP_BOOL("Zicsr", RISCVCPU, cfg.ext_icsr, true),
>> > DEFINE_PROP_STRING("priv_spec", RISCVCPU, cfg.priv_spec),
>> > DEFINE_PROP_BOOL("mmu", RISCVCPU, cfg.mmu, true),
>> > DEFINE_PROP_BOOL("pmp", RISCVCPU, cfg.pmp, true),
>> > diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
>> > index 786f620564..b4c212dfcf 100644
>> > --- a/target/riscv/cpu.h
>> > +++ b/target/riscv/cpu.h
>> > @@ -222,6 +222,8 @@ typedef struct RISCVCPU {
>> > bool ext_s;
>> > bool ext_u;
>> > bool ext_counters;
>> > + bool ext_ifencei;
>> > + bool ext_icsr;
>> >
>> > char *priv_spec;
>> > char *user_spec;
>>
>> I'd rather just support these, which seems about as hard as emitting a warning.
>> Disabling Zifencei is particularly useful, as the Linux ABI forbids it so this
>> way we can run tests that way. Unless I'm missing something, I think these two
>> should do it:
>>
>> From 6d645eb1e8ba4d16431af40bf04e5c165475bf5a Mon Sep 17 00:00:00 2001
>> From: Palmer Dabbelt <palmer@sifive.com>
>> Date: Mon, 24 Jun 2019 01:59:05 -0700
>> Subject: [PATCH 1/2] RISC-V: Add support for the Zifencei extension
>>
>> fence.i has been split out of the base ISA as part of the ratification
>> process. This patch adds a Zifencei argument, which disables the
>> fence.i instruction.
>>
>> Signed-off-by: Palmer Dabbelt <palmer@sifive.com>
>
> This looks good.
>
>> ---
>> target/riscv/cpu.c | 1 +
>> target/riscv/cpu.h | 1 +
>> target/riscv/insn_trans/trans_rvi.inc.c | 3 +++
>> target/riscv/translate.c | 3 +++
>> 4 files changed, 8 insertions(+)
>>
>> diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
>> index be90fa7d0808..bbad39a337b3 100644
>> --- a/target/riscv/cpu.c
>> +++ b/target/riscv/cpu.c
>> @@ -441,6 +441,7 @@ static Property riscv_cpu_properties[] = {
>> DEFINE_PROP_BOOL("s", RISCVCPU, cfg.ext_s, true),
>> DEFINE_PROP_BOOL("u", RISCVCPU, cfg.ext_u, true),
>> DEFINE_PROP_BOOL("Counters", RISCVCPU, cfg.ext_counters, true),
>> + DEFINE_PROP_BOOL("Zifencei", RISCVCPU, cfg.ext_ifencei, true),
>> DEFINE_PROP_STRING("priv_spec", RISCVCPU, cfg.priv_spec),
>> DEFINE_PROP_BOOL("mmu", RISCVCPU, cfg.mmu, true),
>> DEFINE_PROP_BOOL("pmp", RISCVCPU, cfg.pmp, true),
>> diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
>> index 4d4e0f89e206..ba551cd3082c 100644
>> --- a/target/riscv/cpu.h
>> +++ b/target/riscv/cpu.h
>> @@ -223,6 +223,7 @@ typedef struct RISCVCPU {
>> bool ext_s;
>> bool ext_u;
>> bool ext_counters;
>> + bool ext_ifencei;
>>
>> char *priv_spec;
>> char *user_spec;
>> diff --git a/target/riscv/insn_trans/trans_rvi.inc.c b/target/riscv/insn_trans/trans_rvi.inc.c
>> index 6cda078ed6ba..766594ba08ce 100644
>> --- a/target/riscv/insn_trans/trans_rvi.inc.c
>> +++ b/target/riscv/insn_trans/trans_rvi.inc.c
>> @@ -484,6 +484,9 @@ static bool trans_fence(DisasContext *ctx, arg_fence *a)
>>
>> static bool trans_fence_i(DisasContext *ctx, arg_fence_i *a)
>> {
>> + if (!ctx->ext_ifencei)
>> + return false;
>> +
>> /*
>> * FENCE_I is a no-op in QEMU,
>> * however we need to end the translation block
>> diff --git a/target/riscv/translate.c b/target/riscv/translate.c
>> index 313c27b70073..8d6ab732588d 100644
>> --- a/target/riscv/translate.c
>> +++ b/target/riscv/translate.c
>> @@ -54,6 +54,7 @@ typedef struct DisasContext {
>> to any system register, which includes CSR_FRM, so we do not have
>> to reset this known value. */
>> int frm;
>> + bool ext_ifencei;
>> } DisasContext;
>>
>> #ifdef TARGET_RISCV64
>> @@ -752,6 +753,7 @@ static void riscv_tr_init_disas_context(DisasContextBase *dcbase, CPUState *cs)
>> {
>> DisasContext *ctx = container_of(dcbase, DisasContext, base);
>> CPURISCVState *env = cs->env_ptr;
>> + RISCVCPU *cpu = RISCV_CPU(cs);
>>
>> ctx->pc_succ_insn = ctx->base.pc_first;
>> ctx->mem_idx = ctx->base.tb->flags & TB_FLAGS_MMU_MASK;
>> @@ -759,6 +761,7 @@ static void riscv_tr_init_disas_context(DisasContextBase *dcbase, CPUState *cs)
>> ctx->priv_ver = env->priv_ver;
>> ctx->misa = env->misa;
>> ctx->frm = -1; /* unknown rounding mode */
>> + ctx->ext_ifencei = cpu->cfg.ext_ifencei;
>> }
>>
>> static void riscv_tr_tb_start(DisasContextBase *db, CPUState *cpu)
>> --
>> 2.21.0
>>
>>
>> From 6d280048f076eb0f28ee0c4454a1ed8b130240cb Mon Sep 17 00:00:00 2001
>> From: Palmer Dabbelt <palmer@sifive.com>
>> Date: Mon, 24 Jun 2019 01:59:51 -0700
>> Subject: [PATCH 2/2] RISC-V: Add support for the Zicsr extension
>>
>> The various CSR instructions have been split out of the base ISA as part
>> of the ratification process. This patch adds a Zicsr argument, which
>> disables all the CSR instructions.
>>
>> Signed-off-by: Palmer Dabbelt <palmer@sifive.com>
>
> Couldn't we just do this in riscv_csrrw() to avoid passing it down to
> the translation?
>
> It gets called by all of these functions anyway.
Ya, that's much cleaner.
$ git show bc9e214eff71c6a5467e8311701de49d009069e9 | cat
commit bc9e214eff71c6a5467e8311701de49d009069e9
gpg: Signature made Tue 25 Jun 2019 02:59:24 AM PDT
gpg: using RSA key 00CE76D1834960DFCE886DF8EF4CA1502CCBAB41
gpg: issuer "palmer@dabbelt.com"
gpg: Good signature from "Palmer Dabbelt <palmer@dabbelt.com>" [ultimate]
gpg: aka "Palmer Dabbelt <palmer@sifive.com>" [ultimate]
Author: Palmer Dabbelt <palmer@sifive.com>
Date: Mon Jun 24 01:59:51 2019 -0700
RISC-V: Add support for the Zicsr extension
The various CSR instructions have been split out of the base ISA as part
of the ratification process. This patch adds a Zicsr argument, which
disables all the CSR instructions.
Signed-off-by: Palmer Dabbelt <palmer@sifive.com>
diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
index bbad39a337b3..915b9e77df33 100644
--- a/target/riscv/cpu.c
+++ b/target/riscv/cpu.c
@@ -442,6 +442,7 @@ static Property riscv_cpu_properties[] = {
DEFINE_PROP_BOOL("u", RISCVCPU, cfg.ext_u, true),
DEFINE_PROP_BOOL("Counters", RISCVCPU, cfg.ext_counters, true),
DEFINE_PROP_BOOL("Zifencei", RISCVCPU, cfg.ext_ifencei, true),
+ DEFINE_PROP_BOOL("Zicsr", RISCVCPU, cfg.ext_icsr, true),
DEFINE_PROP_STRING("priv_spec", RISCVCPU, cfg.priv_spec),
DEFINE_PROP_BOOL("mmu", RISCVCPU, cfg.mmu, true),
DEFINE_PROP_BOOL("pmp", RISCVCPU, cfg.pmp, true),
diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
index ba551cd3082c..0adb307f3298 100644
--- a/target/riscv/cpu.h
+++ b/target/riscv/cpu.h
@@ -224,6 +224,7 @@ typedef struct RISCVCPU {
bool ext_u;
bool ext_counters;
bool ext_ifencei;
+ bool ext_icsr;
char *priv_spec;
char *user_spec;
diff --git a/target/riscv/csr.c b/target/riscv/csr.c
index dad9ae4a147d..a2ae64bda42b 100644
--- a/target/riscv/csr.c
+++ b/target/riscv/csr.c
@@ -790,6 +790,7 @@ int riscv_csrrw(CPURISCVState *env, int csrno, target_ulong *ret_value,
{
int ret;
target_ulong old_value;
+ RISCVCPU *cpu = env_archcpu(env);
/* check privileges and return -1 if check fails */
#if !defined(CONFIG_USER_ONLY)
@@ -800,6 +801,10 @@ int riscv_csrrw(CPURISCVState *env, int csrno, target_ulong *ret_value,
}
#endif
+ /* ensure the CSR extension is enabled. */
+ if (!cpu->cfg.ext_icsr)
+ return -1;
+
/* check predicate */
if (!csr_ops[csrno].predicate || csr_ops[csrno].predicate(env, csrno) < 0) {
return -1;
>
> Alistair
>
>> ---
>> target/riscv/cpu.c | 1 +
>> target/riscv/cpu.h | 1 +
>> target/riscv/insn_trans/trans_rvi.inc.c | 18 ++++++++++++++++++
>> target/riscv/translate.c | 2 ++
>> 4 files changed, 22 insertions(+)
>>
>> diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
>> index bbad39a337b3..915b9e77df33 100644
>> --- a/target/riscv/cpu.c
>> +++ b/target/riscv/cpu.c
>> @@ -442,6 +442,7 @@ static Property riscv_cpu_properties[] = {
>> DEFINE_PROP_BOOL("u", RISCVCPU, cfg.ext_u, true),
>> DEFINE_PROP_BOOL("Counters", RISCVCPU, cfg.ext_counters, true),
>> DEFINE_PROP_BOOL("Zifencei", RISCVCPU, cfg.ext_ifencei, true),
>> + DEFINE_PROP_BOOL("Zicsr", RISCVCPU, cfg.ext_icsr, true),
>> DEFINE_PROP_STRING("priv_spec", RISCVCPU, cfg.priv_spec),
>> DEFINE_PROP_BOOL("mmu", RISCVCPU, cfg.mmu, true),
>> DEFINE_PROP_BOOL("pmp", RISCVCPU, cfg.pmp, true),
>> diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
>> index ba551cd3082c..0adb307f3298 100644
>> --- a/target/riscv/cpu.h
>> +++ b/target/riscv/cpu.h
>> @@ -224,6 +224,7 @@ typedef struct RISCVCPU {
>> bool ext_u;
>> bool ext_counters;
>> bool ext_ifencei;
>> + bool ext_icsr;
>>
>> char *priv_spec;
>> char *user_spec;
>> diff --git a/target/riscv/insn_trans/trans_rvi.inc.c b/target/riscv/insn_trans/trans_rvi.inc.c
>> index 766594ba08ce..66c5202cc5d6 100644
>> --- a/target/riscv/insn_trans/trans_rvi.inc.c
>> +++ b/target/riscv/insn_trans/trans_rvi.inc.c
>> @@ -524,6 +524,9 @@ static bool trans_fence_i(DisasContext *ctx, arg_fence_i *a)
>>
>> static bool trans_csrrw(DisasContext *ctx, arg_csrrw *a)
>> {
>> + if (!ctx->ext_icsr)
>> + return false;
>> +
>> TCGv source1, csr_store, dest, rs1_pass;
>> RISCV_OP_CSR_PRE;
>> gen_helper_csrrw(dest, cpu_env, source1, csr_store);
>> @@ -533,6 +536,9 @@ static bool trans_csrrw(DisasContext *ctx, arg_csrrw *a)
>>
>> static bool trans_csrrs(DisasContext *ctx, arg_csrrs *a)
>> {
>> + if (!ctx->ext_icsr)
>> + return false;
>> +
>> TCGv source1, csr_store, dest, rs1_pass;
>> RISCV_OP_CSR_PRE;
>> gen_helper_csrrs(dest, cpu_env, source1, csr_store, rs1_pass);
>> @@ -542,6 +548,9 @@ static bool trans_csrrs(DisasContext *ctx, arg_csrrs *a)
>>
>> static bool trans_csrrc(DisasContext *ctx, arg_csrrc *a)
>> {
>> + if (!ctx->ext_icsr)
>> + return false;
>> +
>> TCGv source1, csr_store, dest, rs1_pass;
>> RISCV_OP_CSR_PRE;
>> gen_helper_csrrc(dest, cpu_env, source1, csr_store, rs1_pass);
>> @@ -551,6 +560,9 @@ static bool trans_csrrc(DisasContext *ctx, arg_csrrc *a)
>>
>> static bool trans_csrrwi(DisasContext *ctx, arg_csrrwi *a)
>> {
>> + if (!ctx->ext_icsr)
>> + return false;
>> +
>> TCGv source1, csr_store, dest, rs1_pass;
>> RISCV_OP_CSR_PRE;
>> gen_helper_csrrw(dest, cpu_env, rs1_pass, csr_store);
>> @@ -560,6 +572,9 @@ static bool trans_csrrwi(DisasContext *ctx, arg_csrrwi *a)
>>
>> static bool trans_csrrsi(DisasContext *ctx, arg_csrrsi *a)
>> {
>> + if (!ctx->ext_icsr)
>> + return false;
>> +
>> TCGv source1, csr_store, dest, rs1_pass;
>> RISCV_OP_CSR_PRE;
>> gen_helper_csrrs(dest, cpu_env, rs1_pass, csr_store, rs1_pass);
>> @@ -569,6 +584,9 @@ static bool trans_csrrsi(DisasContext *ctx, arg_csrrsi *a)
>>
>> static bool trans_csrrci(DisasContext *ctx, arg_csrrci *a)
>> {
>> + if (!ctx->ext_icsr)
>> + return false;
>> +
>> TCGv source1, csr_store, dest, rs1_pass;
>> RISCV_OP_CSR_PRE;
>> gen_helper_csrrc(dest, cpu_env, rs1_pass, csr_store, rs1_pass);
>> diff --git a/target/riscv/translate.c b/target/riscv/translate.c
>> index 8d6ab732588d..be46435b9a7b 100644
>> --- a/target/riscv/translate.c
>> +++ b/target/riscv/translate.c
>> @@ -55,6 +55,7 @@ typedef struct DisasContext {
>> to reset this known value. */
>> int frm;
>> bool ext_ifencei;
>> + bool ext_icsr;
>> } DisasContext;
>>
>> #ifdef TARGET_RISCV64
>> @@ -762,6 +763,7 @@ static void riscv_tr_init_disas_context(DisasContextBase *dcbase, CPUState *cs)
>> ctx->misa = env->misa;
>> ctx->frm = -1; /* unknown rounding mode */
>> ctx->ext_ifencei = cpu->cfg.ext_ifencei;
>> + ctx->ext_icsr = cpu->cfg.ext_icsr;
>> }
>>
>> static void riscv_tr_tb_start(DisasContextBase *db, CPUState *cpu)
>> --
>> 2.21.0
>>
next prev parent reply other threads:[~2019-06-25 10:10 UTC|newest]
Thread overview: 24+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-06-18 1:31 [Qemu-devel] [PATCH v1 0/9] Update the RISC-V specification versions Alistair Francis
2019-06-18 1:31 ` [Qemu-devel] [PATCH v1 1/9] target/riscv: Restructure deprecatd CPUs Alistair Francis
2019-06-18 5:23 ` Philippe Mathieu-Daudé
2019-06-18 15:59 ` Alistair Francis
2019-06-18 1:31 ` [Qemu-devel] [PATCH v1 2/9] target/riscv: Add the privledge spec version 1.11.0 Alistair Francis
2019-06-18 1:31 ` [Qemu-devel] [PATCH v1 3/9] target/riscv: Comment in the mcountinhibit CSR Alistair Francis
2019-06-24 9:31 ` Palmer Dabbelt
2019-06-24 20:14 ` Alistair Francis
2019-06-18 1:31 ` [Qemu-devel] [PATCH v1 4/9] target/riscv: Set privledge spec 1.11.0 as default Alistair Francis
2019-06-18 1:31 ` [Qemu-devel] [PATCH v1 5/9] qemu-deprecated.texi: Deprecate the RISC-V privledge spec 1.09.1 Alistair Francis
2019-06-18 1:31 ` [Qemu-devel] [PATCH v1 6/9] target/riscv: Require either I or E base extension Alistair Francis
2019-06-18 1:31 ` [Qemu-devel] [PATCH v1 7/9] target/riscv: Remove user version information Alistair Francis
2019-06-18 1:31 ` [Qemu-devel] [PATCH v1 8/9] target/riscv: Add support for disabling/enabling Counters Alistair Francis
2019-06-18 1:31 ` [Qemu-devel] [PATCH v1 9/9] target/riscv: Add Zifencei and Zicsr as command line options Alistair Francis
2019-06-24 9:31 ` Palmer Dabbelt
2019-06-24 23:16 ` Alistair Francis
2019-06-25 10:08 ` Palmer Dabbelt [this message]
2019-06-19 10:58 ` [Qemu-devel] [PATCH v1 0/9] Update the RISC-V specification versions Palmer Dabbelt
2019-06-19 14:19 ` Alistair Francis
2019-06-21 2:49 ` Palmer Dabbelt
2019-06-22 0:23 ` Alistair Francis
2019-06-23 14:40 ` Palmer Dabbelt
2019-06-24 9:33 ` Palmer Dabbelt
2019-06-24 20:13 ` 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=mhng-94acab2f-b1df-4a4b-a29a-d51dd0405476@palmer-si-x1e \
--to=palmer@sifive.com \
--cc=Alistair.Francis@wdc.com \
--cc=alistair23@gmail.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).