* [PATCH 0/2] target/riscv: Fix mstatus.MPP related support @ 2023-03-30 13:58 Weiwei Li 2023-03-30 13:58 ` [PATCH 1/2] target/riscv: Fix the mstatus.MPP value after executing MRET Weiwei Li 2023-03-30 13:58 ` [PATCH 2/2] target/riscv: Legalize MPP value in write_mstatus Weiwei Li 0 siblings, 2 replies; 13+ messages in thread From: Weiwei Li @ 2023-03-30 13:58 UTC (permalink / raw) To: qemu-riscv, qemu-devel Cc: palmer, alistair.francis, bin.meng, dbarboza, zhiwei_liu, wangjunqiang, lazyparser, Weiwei Li This patchset tries to fix some problems in current implementation for mstatus.MPP. - set to the least-privileged supported mode (U if U-mode is implemented, else M) after executing mret - legalize mpp when write to mstatus The port is available here: https://github.com/plctlab/plct-qemu/tree/plct-mpp-fix Weiwei Li (2): target/riscv: Fix the mstatus.MPP value after executing MRET target/riscv: Legalize MPP value in write_mstatus target/riscv/cpu_helper.c | 5 +---- target/riscv/csr.c | 14 ++++++++++++++ target/riscv/op_helper.c | 3 ++- 3 files changed, 17 insertions(+), 5 deletions(-) -- 2.25.1 ^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH 1/2] target/riscv: Fix the mstatus.MPP value after executing MRET 2023-03-30 13:58 [PATCH 0/2] target/riscv: Fix mstatus.MPP related support Weiwei Li @ 2023-03-30 13:58 ` Weiwei Li 2023-04-06 0:43 ` Alistair Francis 2023-03-30 13:58 ` [PATCH 2/2] target/riscv: Legalize MPP value in write_mstatus Weiwei Li 1 sibling, 1 reply; 13+ messages in thread From: Weiwei Li @ 2023-03-30 13:58 UTC (permalink / raw) To: qemu-riscv, qemu-devel Cc: palmer, alistair.francis, bin.meng, dbarboza, zhiwei_liu, wangjunqiang, lazyparser, Weiwei Li The MPP will be set to the least-privileged supported mode (U if U-mode is implemented, else M). 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 ^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH 1/2] target/riscv: Fix the mstatus.MPP value after executing MRET 2023-03-30 13:58 ` [PATCH 1/2] target/riscv: Fix the mstatus.MPP value after executing MRET Weiwei Li @ 2023-04-06 0:43 ` Alistair Francis 2023-04-06 0:56 ` liweiwei 0 siblings, 1 reply; 13+ messages in thread From: Alistair Francis @ 2023-04-06 0:43 UTC (permalink / raw) To: Weiwei Li Cc: qemu-riscv, qemu-devel, palmer, alistair.francis, bin.meng, dbarboza, zhiwei_liu, wangjunqiang, lazyparser 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 > > ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 1/2] target/riscv: Fix the mstatus.MPP value after executing MRET 2023-04-06 0:43 ` Alistair Francis @ 2023-04-06 0:56 ` liweiwei 2023-04-06 1:46 ` Alistair Francis 0 siblings, 1 reply; 13+ messages in thread From: liweiwei @ 2023-04-06 0:56 UTC (permalink / raw) To: Alistair Francis, Weiwei Li Cc: qemu-riscv, qemu-devel, palmer, alistair.francis, bin.meng, dbarboza, zhiwei_liu, wangjunqiang, lazyparser [-- Attachment #1: Type: text/plain, Size: 2110 bytes --] 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 >> >> [-- Attachment #2: Type: text/html, Size: 9810 bytes --] ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 1/2] target/riscv: Fix the mstatus.MPP value after executing MRET 2023-04-06 0:56 ` liweiwei @ 2023-04-06 1:46 ` Alistair Francis 2023-04-06 2:14 ` liweiwei 0 siblings, 1 reply; 13+ messages in thread From: Alistair Francis @ 2023-04-06 1:46 UTC (permalink / raw) To: liweiwei Cc: qemu-riscv, qemu-devel, palmer, alistair.francis, bin.meng, dbarboza, zhiwei_liu, wangjunqiang, lazyparser 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 > > ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 1/2] target/riscv: Fix the mstatus.MPP value after executing MRET 2023-04-06 1:46 ` Alistair Francis @ 2023-04-06 2:14 ` liweiwei 2023-04-06 2:24 ` Alistair Francis 0 siblings, 1 reply; 13+ messages in thread From: liweiwei @ 2023-04-06 2:14 UTC (permalink / raw) To: Alistair Francis Cc: liweiwei, qemu-riscv, qemu-devel, palmer, alistair.francis, bin.meng, dbarboza, zhiwei_liu, wangjunqiang, lazyparser 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 >> >> ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 1/2] target/riscv: Fix the mstatus.MPP value after executing MRET 2023-04-06 2:14 ` liweiwei @ 2023-04-06 2:24 ` Alistair Francis 2023-04-06 2:39 ` liweiwei 2023-04-06 3:01 ` liweiwei 0 siblings, 2 replies; 13+ messages in thread From: Alistair Francis @ 2023-04-06 2:24 UTC (permalink / raw) To: liweiwei Cc: qemu-riscv, qemu-devel, palmer, alistair.francis, bin.meng, dbarboza, zhiwei_liu, wangjunqiang, lazyparser 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 > >> > >> > ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 1/2] target/riscv: Fix the mstatus.MPP value after executing MRET 2023-04-06 2:24 ` Alistair Francis @ 2023-04-06 2:39 ` liweiwei 2023-04-06 3:01 ` liweiwei 1 sibling, 0 replies; 13+ messages in thread From: liweiwei @ 2023-04-06 2:39 UTC (permalink / raw) To: Alistair Francis Cc: liweiwei, qemu-riscv, qemu-devel, palmer, alistair.francis, bin.meng, dbarboza, zhiwei_liu, wangjunqiang, lazyparser 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 >>>> >>>> ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 1/2] target/riscv: Fix the mstatus.MPP value after executing MRET 2023-04-06 2:24 ` Alistair Francis 2023-04-06 2:39 ` liweiwei @ 2023-04-06 3:01 ` liweiwei 2023-04-06 3:55 ` Alistair Francis 1 sibling, 1 reply; 13+ messages in thread From: liweiwei @ 2023-04-06 3:01 UTC (permalink / raw) To: Alistair Francis, liweiwei Cc: qemu-riscv, qemu-devel, palmer, alistair.francis, bin.meng, dbarboza, zhiwei_liu, wangjunqiang, lazyparser 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 >>>> >>>> ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 1/2] target/riscv: Fix the mstatus.MPP value after executing MRET 2023-04-06 3:01 ` liweiwei @ 2023-04-06 3:55 ` Alistair Francis 0 siblings, 0 replies; 13+ messages in thread From: Alistair Francis @ 2023-04-06 3:55 UTC (permalink / raw) To: liweiwei Cc: qemu-riscv, qemu-devel, palmer, alistair.francis, bin.meng, dbarboza, zhiwei_liu, wangjunqiang, lazyparser 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 > >>>> > >>>> > ^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH 2/2] target/riscv: Legalize MPP value in write_mstatus 2023-03-30 13:58 [PATCH 0/2] target/riscv: Fix mstatus.MPP related support Weiwei Li 2023-03-30 13:58 ` [PATCH 1/2] target/riscv: Fix the mstatus.MPP value after executing MRET Weiwei Li @ 2023-03-30 13:58 ` Weiwei Li 2023-04-06 1:26 ` Alistair Francis 1 sibling, 1 reply; 13+ messages in thread From: Weiwei Li @ 2023-03-30 13:58 UTC (permalink / raw) To: qemu-riscv, qemu-devel Cc: palmer, alistair.francis, bin.meng, dbarboza, zhiwei_liu, wangjunqiang, lazyparser, Weiwei Li mstatus.MPP field is a WARL field, so we remain it unchanged if an 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 ^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH 2/2] target/riscv: Legalize MPP value in write_mstatus 2023-03-30 13:58 ` [PATCH 2/2] target/riscv: Legalize MPP value in write_mstatus Weiwei Li @ 2023-04-06 1:26 ` Alistair Francis 2023-04-06 1:35 ` liweiwei 0 siblings, 1 reply; 13+ messages in thread From: Alistair Francis @ 2023-04-06 1:26 UTC (permalink / raw) To: Weiwei Li Cc: qemu-riscv, qemu-devel, palmer, alistair.francis, bin.meng, dbarboza, zhiwei_liu, wangjunqiang, lazyparser 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. 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 > > ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 2/2] target/riscv: Legalize MPP value in write_mstatus 2023-04-06 1:26 ` Alistair Francis @ 2023-04-06 1:35 ` liweiwei 0 siblings, 0 replies; 13+ messages in thread From: liweiwei @ 2023-04-06 1:35 UTC (permalink / raw) To: Alistair Francis Cc: liweiwei, qemu-riscv, qemu-devel, palmer, alistair.francis, bin.meng, dbarboza, zhiwei_liu, wangjunqiang, lazyparser 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 >> >> ^ permalink raw reply [flat|nested] 13+ messages in thread
end of thread, other threads:[~2023-04-06 3:56 UTC | newest] Thread overview: 13+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2023-03-30 13:58 [PATCH 0/2] target/riscv: Fix mstatus.MPP related support Weiwei Li 2023-03-30 13:58 ` [PATCH 1/2] target/riscv: Fix the mstatus.MPP value after executing MRET Weiwei Li 2023-04-06 0:43 ` Alistair Francis 2023-04-06 0:56 ` liweiwei 2023-04-06 1:46 ` Alistair Francis 2023-04-06 2:14 ` liweiwei 2023-04-06 2:24 ` Alistair Francis 2023-04-06 2:39 ` liweiwei 2023-04-06 3:01 ` liweiwei 2023-04-06 3:55 ` Alistair Francis 2023-03-30 13:58 ` [PATCH 2/2] target/riscv: Legalize MPP value in write_mstatus Weiwei Li 2023-04-06 1:26 ` Alistair Francis 2023-04-06 1:35 ` liweiwei
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).