| Message ID | 20230330135818.68417-2-liweiwei@iscas.ac.cn (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | target/riscv: Fix mstatus.MPP related support | expand |
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: "MRET then in mstatus/mstatush sets MPV=0, MPP=0, MIE=MPIE, and MPIE=1" 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 > >
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 (draft-20230131-c0b298a: Clarify WFI trapping behavior (#972)). > > "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 anxRET instruction, supposingxPP holds the valuey,xIE is set toxPIE; the privilege mode is changed toy;xPIE is set to 1; andxPP is set to the least-privileged supported mode (U if U-mode is implemented, else M). Ify̸=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 >> >>
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 > >
On 2023/4/6 09:46, Alistair Francis wrote: > 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 Yeah. I see it. However, this is a little different from the description in section 3.1.6.1. And MPP is WARL field. PRV_U will be an illegal value for MPP if U-mode is not implemented. 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 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 <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 >> >>
On Thu, Apr 6, 2023 at 12:14 PM liweiwei <liweiwei@iscas.ac.cn> wrote: > > > On 2023/4/6 09:46, Alistair Francis wrote: > > 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 > > 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. > > 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 <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 > >> > >> >
On 2023/4/6 10:24, Alistair Francis wrote: > On Thu, Apr 6, 2023 at 12:14 PM liweiwei <liweiwei@iscas.ac.cn> wrote: >> >> On 2023/4/6 09:46, Alistair Francis wrote: >>> 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 >> 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. OK. I'll send an issue for it. > >> 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 OK . Thanks! I'll try to set it later. Regards, Weiwei Li > > 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 <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 >>>> >>>>
On 2023/4/6 10:24, Alistair Francis wrote: > On Thu, Apr 6, 2023 at 12:14 PM liweiwei <liweiwei@iscas.ac.cn> wrote: >> >> On 2023/4/6 09:46, Alistair Francis wrote: >>> 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 >> 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. 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 <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 >>>> >>>>
On Thu, Apr 6, 2023 at 1:02 PM liweiwei <liweiwei@iscas.ac.cn> wrote: > > > On 2023/4/6 10:24, Alistair Francis wrote: > > On Thu, Apr 6, 2023 at 12:14 PM liweiwei <liweiwei@iscas.ac.cn> wrote: > >> > >> On 2023/4/6 09:46, Alistair Francis wrote: > >>> 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 > >> 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.francis@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 <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 > >>>> > >>>> >
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);