Re: [PATCH 2/2] target/riscv: Legalize MPP value in write_mstatus

[liweiwei]

On 2023/4/6 09:26, Alistair Francis wrote:
> On Thu, Mar 30, 2023 at 11:59 PM Weiwei Li <liweiwei@iscas.ac.cn> wrote:
> > mstatus.MPP field is a WARL field, so we remain it unchanged if an
> Only since version 1.11 of the priv spec and we do still support priv 1.10.
>
> I think it's ok to make this change for all priv versions, as it won't
> break any software running 1.10, but it's worth adding a comment or at
> least a mention in the commit message.

OK. I'll add it in next version.

Regards,

Weiwei Li

> Alistair
>
> > invalid value is written into it. And after this, RVH shouldn't be
> > passed to riscv_cpu_set_mode().
> >
> > Signed-off-by: Weiwei Li <liweiwei@iscas.ac.cn>
> > Signed-off-by: Junqiang Wang <wangjunqiang@iscas.ac.cn>
> > ---
> >   target/riscv/cpu_helper.c |  5 +----
> >   target/riscv/csr.c        | 14 ++++++++++++++
> >   2 files changed, 15 insertions(+), 4 deletions(-)
> >
> > diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c
> > index f88c503cf4..46baf3ab7c 100644
> > --- a/target/riscv/cpu_helper.c
> > +++ b/target/riscv/cpu_helper.c
> > @@ -659,12 +659,9 @@ void riscv_cpu_set_aia_ireg_rmw_fn(CPURISCVState *env, 
> > uint32_t priv,
> >
> >   void riscv_cpu_set_mode(CPURISCVState *env, target_ulong newpriv)
> >   {
> > -    if (newpriv > PRV_M) {
> > +    if (newpriv > PRV_M || newpriv == PRV_H) {
> >           g_assert_not_reached();
> >       }
> > -    if (newpriv == PRV_H) {
> > -        newpriv = PRV_U;
> > -    }
> >       if (icount_enabled() && newpriv != env->priv) {
> >           riscv_itrigger_update_priv(env);
> >       }
> > diff --git a/target/riscv/csr.c b/target/riscv/csr.c
> > index d522efc0b6..a99026c3e8 100644
> > --- a/target/riscv/csr.c
> > +++ b/target/riscv/csr.c
> > @@ -1238,6 +1238,18 @@ static bool validate_vm(CPURISCVState *env, target_ulong 
> > vm)
> >       return (vm & 0xf) <= satp_mode_max_from_map(cpu->cfg.satp_mode.map);
> >   }
> >
> > +static target_ulong legalize_mpp(CPURISCVState *env, target_ulong old_mpp,
> > +                                 target_ulong val)
> > +{
> > +    target_ulong new_mpp = get_field(val, MSTATUS_MPP);
> > +    bool mpp_invalid = (new_mpp == PRV_S && !riscv_has_ext(env, RVS)) ||
> > +                       (new_mpp == PRV_U && !riscv_has_ext(env, RVU)) ||
> > +                       (new_mpp == PRV_H);
> > +
> > +    /* Remain field unchanged if new_mpp value is invalid */
> > +    return mpp_invalid ? set_field(val, MSTATUS_MPP, old_mpp) : val;
> > +}
> > +
> >   static RISCVException write_mstatus(CPURISCVState *env, int csrno,
> >                                       target_ulong val)
> >   {
> > @@ -1245,6 +1257,8 @@ static RISCVException write_mstatus(CPURISCVState *env, 
> > int csrno,
> >       uint64_t mask = 0;
> >       RISCVMXL xl = riscv_cpu_mxl(env);
> >
> > +    val = legalize_mpp(env, get_field(mstatus, MSTATUS_MPP), val);
> > +
> >       /* flush tlb on mstatus fields that affect VM */
> >       if ((val ^ mstatus) & (MSTATUS_MXR | MSTATUS_MPP | MSTATUS_MPV |
> >               MSTATUS_MPRV | MSTATUS_SUM)) {
> > --
> > 2.25.1