From: Mayuresh Chitale <mchitale@ventanamicro.com>
To: Weiwei Li <liweiwei@iscas.ac.cn>
Cc: alistair.francis@wdc.com, qemu-devel@nongnu.org, qemu-riscv@nongnu.org
Subject: Re: [PATCH v6 2/5] target/riscv: smstateen check for h/senvcfg
Date: Thu, 28 Jul 2022 12:11:44 +0530 [thread overview]
Message-ID: <81f0e2ed116a7c39eb0981ed165b00feb815817f.camel@ventanamicro.com> (raw)
In-Reply-To: <150ceeab-85b7-5bd6-1024-328881a22d74@iscas.ac.cn>
On Fri, 2022-07-22 at 08:45 +0800, Weiwei Li wrote:
> 在 2022/7/21 下午11:31, Mayuresh Chitale 写道:
> > Accesses to henvcfg, henvcfgh and senvcfg are allowed only if
> > corresponding bit in mstateen0/hstateen0 is enabled. Otherwise an
> > illegal instruction trap is generated.
> >
> > Signed-off-by: Mayuresh Chitale <mchitale@ventanamicro.com>
> > ---
> > target/riscv/csr.c | 100
> > +++++++++++++++++++++++++++++++++++++++++----
> > 1 file changed, 93 insertions(+), 7 deletions(-)
> >
> > diff --git a/target/riscv/csr.c b/target/riscv/csr.c
> > index 27032a416c..ab06b117f9 100644
> > --- a/target/riscv/csr.c
> > +++ b/target/riscv/csr.c
> > @@ -40,6 +40,55 @@ void riscv_set_csr_ops(int csrno,
> > riscv_csr_operations *ops)
> > }
> >
> > /* Predicates */
> > +#if !defined(CONFIG_USER_ONLY)
> > +static RISCVException smstateen_acc_ok(CPURISCVState *env, int
> > index,
> > + uint64_t bit)
> > +{
> > + bool virt = riscv_cpu_virt_enabled(env);
> > + CPUState *cs = env_cpu(env);
> > + RISCVCPU *cpu = RISCV_CPU(cs);
> > + uint64_t hstateen = env->hstateen[index];
> > + uint64_t sstateen = env->sstateen[index];
> > +
> > + if (env->priv == PRV_M || !cpu->cfg.ext_smstateen) {
> > + return RISCV_EXCP_NONE;
> > + }
> > +
> > + if (!(env->mstateen[index] & bit)) {
> > + return RISCV_EXCP_ILLEGAL_INST;
> > + }
> > +
> > + /*
> > + * Treat hstateen and sstateen as read-only zero if
> > mstateen0.staten
> > + * is clear.
> > + */
> > + if (!(env->mstateen[index] & SMSTATEEN_STATEN)) {
> > + hstateen = 0;
> > + sstateen = 0;
> > + }
> > +
> > + if (virt) {
> > + if (!(hstateen & bit)) {
> > + return RISCV_EXCP_VIRT_INSTRUCTION_FAULT;
> > + }
> > + /*
> > + * Treat sstateen as read-only zero if hstateen0.staten is
> > clear.
> > + */
> > + if (!(hstateen & SMSTATEEN_STATEN)) {
> > + sstateen = 0;
> > + }
> > + }
> > +
> > + if (env->priv == PRV_U && riscv_has_ext(env, RVS)) {
> > + if (!(sstateen & bit)) {
> > + return RISCV_EXCP_ILLEGAL_INST;
> > + }
> > + }
> > +
> > + return RISCV_EXCP_NONE;
> > +}
> > +#endif
> > +
> > static RISCVException fs(CPURISCVState *env, int csrno)
> > {
> > #if !defined(CONFIG_USER_ONLY)
> > @@ -1708,6 +1757,13 @@ static RISCVException
> > write_menvcfgh(CPURISCVState *env, int csrno,
> > static RISCVException read_senvcfg(CPURISCVState *env, int csrno,
> > target_ulong *val)
> > {
> > + RISCVException ret;
> > +
> > + ret = smstateen_acc_ok(env, 0, SMSTATEEN0_HSENVCFG);
>
> I think it's better to add this check into the predicate.
>
> By the way, sharing the same function for all related csrs is
> easily
> misunderstood. However, It seems correct.
We use the default global predicates ie hmode/smode etc for the envcfg
registers and the global predicates cant be modified to include
additional checks for envcfg registers. We could implement new
predicates for envcfg but I think the current approach is simpler.
>
> Regards,
>
> Weiwei Li
>
> > + if (ret != RISCV_EXCP_NONE) {
> > + return ret;
> > + }
> > +
> > *val = env->senvcfg;
> > return RISCV_EXCP_NONE;
> > }
> > @@ -1716,15 +1772,27 @@ static RISCVException
> > write_senvcfg(CPURISCVState *env, int csrno,
> > target_ulong val)
> > {
> > uint64_t mask = SENVCFG_FIOM | SENVCFG_CBIE | SENVCFG_CBCFE |
> > SENVCFG_CBZE;
> > + RISCVException ret;
> >
> > - env->senvcfg = (env->senvcfg & ~mask) | (val & mask);
> > + ret = smstateen_acc_ok(env, 0, SMSTATEEN0_HSENVCFG);
> > + if (ret != RISCV_EXCP_NONE) {
> > + return ret;
> > + }
> >
> > + env->senvcfg = (env->senvcfg & ~mask) | (val & mask);
> > return RISCV_EXCP_NONE;
> > }
> >
> > static RISCVException read_henvcfg(CPURISCVState *env, int csrno,
> > target_ulong *val)
> > {
> > + RISCVException ret;
> > +
> > + ret = smstateen_acc_ok(env, 0, SMSTATEEN0_HSENVCFG);
> > + if (ret != RISCV_EXCP_NONE) {
> > + return ret;
> > + }
> > +
> > *val = env->henvcfg;
> > return RISCV_EXCP_NONE;
> > }
> > @@ -1733,6 +1801,12 @@ static RISCVException
> > write_henvcfg(CPURISCVState *env, int csrno,
> > target_ulong val)
> > {
> > uint64_t mask = HENVCFG_FIOM | HENVCFG_CBIE | HENVCFG_CBCFE |
> > HENVCFG_CBZE;
> > + RISCVException ret;
> > +
> > + ret = smstateen_acc_ok(env, 0, SMSTATEEN0_HSENVCFG);
> > + if (ret != RISCV_EXCP_NONE) {
> > + return ret;
> > + }
> >
> > if (riscv_cpu_mxl(env) == MXL_RV64) {
> > mask |= HENVCFG_PBMTE | HENVCFG_STCE;
> > @@ -1746,6 +1820,13 @@ static RISCVException
> > write_henvcfg(CPURISCVState *env, int csrno,
> > static RISCVException read_henvcfgh(CPURISCVState *env, int
> > csrno,
> > target_ulong *val)
> > {
> > + RISCVException ret;
> > +
> > + ret = smstateen_acc_ok(env, 0, SMSTATEEN0_HSENVCFG);
> > + if (ret != RISCV_EXCP_NONE) {
> > + return ret;
> > + }
> > +
> > *val = env->henvcfg >> 32;
> > return RISCV_EXCP_NONE;
> > }
> > @@ -1755,9 +1836,14 @@ static RISCVException
> > write_henvcfgh(CPURISCVState *env, int csrno,
> > {
> > uint64_t mask = HENVCFG_PBMTE | HENVCFG_STCE;
> > uint64_t valh = (uint64_t)val << 32;
> > + RISCVException ret;
> >
> > - env->henvcfg = (env->henvcfg & ~mask) | (valh & mask);
> > + ret = smstateen_acc_ok(env, 0, SMSTATEEN0_HSENVCFG);
> > + if (ret != RISCV_EXCP_NONE) {
> > + return ret;
> > + }
> >
> > + env->henvcfg = (env->henvcfg & ~mask) | (valh & mask);
> > return RISCV_EXCP_NONE;
> > }
> >
> > @@ -1789,7 +1875,7 @@ static RISCVException
> > write_mstateen(CPURISCVState *env, int csrno,
> > static RISCVException write_mstateen0(CPURISCVState *env, int
> > csrno,
> > target_ulong new_val)
> > {
> > - uint64_t wr_mask = SMSTATEEN_STATEN;
> > + uint64_t wr_mask = SMSTATEEN_STATEN | SMSTATEEN0_HSENVCFG;
> >
> > return write_mstateen(env, csrno, wr_mask, new_val);
> > }
> > @@ -1836,7 +1922,7 @@ static RISCVException
> > write_mstateenh(CPURISCVState *env, int csrno,
> > static RISCVException write_mstateen0h(CPURISCVState *env, int
> > csrno,
> > target_ulong new_val)
> > {
> > - uint64_t wr_mask = SMSTATEEN_STATEN;
> > + uint64_t wr_mask = SMSTATEEN_STATEN | SMSTATEEN0_HSENVCFG;
> >
> > return write_mstateenh(env, csrno, wr_mask, new_val);
> > }
> > @@ -1885,7 +1971,7 @@ static RISCVException
> > write_hstateen(CPURISCVState *env, int csrno,
> > static RISCVException write_hstateen0(CPURISCVState *env, int
> > csrno,
> > target_ulong new_val)
> > {
> > - uint64_t wr_mask = SMSTATEEN_STATEN;
> > + uint64_t wr_mask = SMSTATEEN_STATEN | SMSTATEEN0_HSENVCFG;
> >
> > return write_hstateen(env, csrno, wr_mask, new_val);
> > }
> > @@ -1936,7 +2022,7 @@ static RISCVException
> > write_hstateenh(CPURISCVState *env, int csrno,
> > static RISCVException write_hstateen0h(CPURISCVState *env, int
> > csrno,
> > target_ulong new_val)
> > {
> > - uint64_t wr_mask = SMSTATEEN_STATEN;
> > + uint64_t wr_mask = SMSTATEEN_STATEN | SMSTATEEN0_HSENVCFG;
> >
> > return write_hstateenh(env, csrno, wr_mask, new_val);
> > }
> > @@ -1995,7 +2081,7 @@ static RISCVException
> > write_sstateen(CPURISCVState *env, int csrno,
> > static RISCVException write_sstateen0(CPURISCVState *env, int
> > csrno,
> > target_ulong new_val)
> > {
> > - uint64_t wr_mask = SMSTATEEN_STATEN;
> > + uint64_t wr_mask = SMSTATEEN_STATEN | SMSTATEEN0_HSENVCFG;
> >
> > return write_sstateen(env, csrno, wr_mask, new_val);
> > }
next prev parent reply other threads:[~2022-07-28 6:41 UTC|newest]
Thread overview: 22+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-07-21 15:31 [PATCH v6 0/5] RISC-V Smstateen support Mayuresh Chitale
2022-07-21 15:31 ` [PATCH v6 1/5] target/riscv: Add smstateen support Mayuresh Chitale
2022-07-22 0:31 ` Weiwei Li
2022-07-24 15:39 ` Mayuresh Chitale
2022-07-25 7:11 ` Weiwei Li
2022-07-28 5:39 ` Mayuresh Chitale
2022-07-21 15:31 ` [PATCH v6 2/5] target/riscv: smstateen check for h/senvcfg Mayuresh Chitale
2022-07-22 0:45 ` Weiwei Li
2022-07-28 6:41 ` Mayuresh Chitale [this message]
2022-07-28 7:59 ` Weiwei Li
2022-07-21 15:31 ` [PATCH v6 3/5] target/riscv: smstateen check for fcsr Mayuresh Chitale
2022-07-22 1:42 ` Weiwei Li
2022-07-24 15:49 ` Mayuresh Chitale
2022-07-25 7:23 ` Weiwei Li
2022-07-28 6:15 ` Mayuresh Chitale
2022-07-28 7:38 ` Weiwei Li
2022-07-28 8:09 ` Ben Dooks
2022-07-29 12:29 ` Mayuresh Chitale
2022-07-21 15:31 ` [PATCH v6 4/5] target/riscv: smstateen check for AIA/IMSIC Mayuresh Chitale
2022-07-22 1:45 ` Weiwei Li
2022-07-21 15:31 ` [PATCH v6 5/5] target/riscv: smstateen knobs Mayuresh Chitale
2022-07-22 1:47 ` Weiwei Li
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=81f0e2ed116a7c39eb0981ed165b00feb815817f.camel@ventanamicro.com \
--to=mchitale@ventanamicro.com \
--cc=alistair.francis@wdc.com \
--cc=liweiwei@iscas.ac.cn \
--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.