On Thu, Apr 6, 2023 at 1:02 PM liweiwei <liwei...@iscas.ac.cn> wrote: > > > On 2023/4/6 10:24, Alistair Francis wrote: > > On Thu, Apr 6, 2023 at 12:14 PM liweiwei <liwei...@iscas.ac.cn> wrote: > >> > >> On 2023/4/6 09:46, Alistair Francis wrote: > >>> On Thu, Apr 6, 2023 at 10:56 AM liweiwei <liwei...@iscas.ac.cn> wrote: > >>>> On 2023/4/6 08:43, Alistair Francis wrote: > >>>> > >>>> On Thu, Mar 30, 2023 at 11:59 PM Weiwei Li <liwei...@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 > >> Yeah. I see it. However, this is a little different from the description > >> in section 3.1.6.1. > > They seem to be in conflict. It's probably worth opening an issue > > against the spec to get some clarification here. > > I have sent an issue for > it(https://github.com/riscv/riscv-isa-manual/issues/1006). > > However, I just find it may be not a conflict. Section 9.6.4 is the spec > for hypervisor. And when hypervisor is supported, > > S-mode, then U-mode should be supported too.
Ah, I didn't think to check the actual section! Good call. I think you are right then. In which case this patch is the correct way to go :) Feel free to close the issue if you want to. Reviewed-by: Alistair Francis <alistair.fran...@wdc.com> Alistair > > Regards, > > Weiwei Li > > > > >> And MPP is WARL field. PRV_U will be an illegal value for MPP if U-mode > >> is not implemented. > > Yeah, I think you are right. It just directly goes against the mret > > section. I suspect the mret section is wrong and needs to be updated > > > >> So I think description in section 3.1.6.1 seems more reasonable. > >> > >>>> (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 > >> Sorry. I don't get your problem. I replied by Thunderbird. Above is the > > Have a look at your previous email, it's a HTML email. If I view the > > source of the email I see this: > > > > Content-Type: text/html; charset=UTF-8 > > > > and the formatting is a little off. > > > > This email that I'm replying to is a plain text email. I'm not sure > > what happened, but try to check that your responses are plain text. I > > think there is a setting in Thunderbird to just open and reply to all > > emails as plain text, which is probably worth turning on > > > > Alistair > > > >> title for the latest release version of the spec in riscv-isa-manual > >> github > >> (https://github.com/riscv/riscv-isa-manual/releases/tag/draft-20230131-c0b298a). > >> > >> Regards, > >> > >> Weiwei Li > >> > >>> 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 <liwei...@iscas.ac.cn> > >>>> Signed-off-by: Junqiang Wang <wangjunqi...@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 > >>>> > >>>> >
