Re: [PATCH 1/2] target/riscv: Fix the mstatus.MPP value after executing MRET

[Alistair Francis]

On Thu, Apr 6, 2023 at 10:56 AM liweiwei <liweiwei@iscas.ac.cn> wrote:
>
>
> On 2023/4/6 08:43, Alistair Francis wrote:
>
> On Thu, Mar 30, 2023 at 11:59 PM Weiwei Li <liweiwei@iscas.ac.cn> wrote:
>
> The MPP will be set to the least-privileged supported mode (U if
> U-mode is implemented, else M).
>
> I don't think this is right, the spec in section 8.6.4 says this:
>
> Sorry, I didn't find this section in latest release of both privilege and 
> un-privilege spec

I updated my spec, using commit
f6b8d5c7d2dcd935b48689a337c8f5bc2be4b5e5 it's now section 9.6.4 Trap
Return

>
> (draft-20230131-c0b298a: Clarify WFI trapping behavior (#972)).

Also, you replied with a HTML email which loses the conversation
history (just see above). Can you fixup your client to reply with
plain text please

Alistair

>
> "MRET then in mstatus/mstatush sets MPV=0, MPP=0,
> MIE=MPIE, and MPIE=1"
>
> In section 3.1.6.1, the privilege spec says this:
>
> "An MRET or SRET instruction is used to return from a trap in M-mode or 
> S-mode respectively.
> When executing an xRET instruction, supposing xPP holds the value y, xIE is 
> set to xPIE; the
> privilege mode is changed to y; xPIE is set to 1; and xPP is set to the 
> least-privileged supported
> mode (U if U-mode is implemented, else M). If y̸=M, xRET also sets MPRV=0"
>
> And I think PRV_U is an illegal value for MPP if U-mode is not implemented.
>
> Regards,
>
> Weiwei Li
>
> So it should just always be 0 (PRV_U is 0)
>
> Alistair
>
> Signed-off-by: Weiwei Li <liweiwei@iscas.ac.cn>
> Signed-off-by: Junqiang Wang <wangjunqiang@iscas.ac.cn>
> ---
>  target/riscv/op_helper.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/target/riscv/op_helper.c b/target/riscv/op_helper.c
> index 84ee018f7d..991f06d98d 100644
> --- a/target/riscv/op_helper.c
> +++ b/target/riscv/op_helper.c
> @@ -339,7 +339,8 @@ target_ulong helper_mret(CPURISCVState *env)
>      mstatus = set_field(mstatus, MSTATUS_MIE,
>                          get_field(mstatus, MSTATUS_MPIE));
>      mstatus = set_field(mstatus, MSTATUS_MPIE, 1);
> -    mstatus = set_field(mstatus, MSTATUS_MPP, PRV_U);
> +    mstatus = set_field(mstatus, MSTATUS_MPP,
> +                        riscv_has_ext(env, RVU) ? PRV_U : PRV_M);
>      mstatus = set_field(mstatus, MSTATUS_MPV, 0);
>      if ((env->priv_ver >= PRIV_VERSION_1_12_0) && (prev_priv != PRV_M)) {
>          mstatus = set_field(mstatus, MSTATUS_MPRV, 0);
> --
> 2.25.1
>
>