| Message ID | 20230529121719.179507-4-liweiwei@iscas.ac.cn |
|---|---|
| State | New |
| Headers | show |
| Series | target/riscv: Fix mstatus related problems | expand |
On 5/29/23 09:17, Weiwei Li wrote: > MPV and GVA bits are added by hypervisor extension to mstatus > and mstatush (if MXLEN=32). > > Signed-off-by: Weiwei Li <liweiwei@iscas.ac.cn> > Signed-off-by: Junqiang Wang <wangjunqiang@iscas.ac.cn> > --- Reviewed-by: Daniel Henrique Barboza <dbarboza@ventanamicro.com> > target/riscv/csr.c | 10 ++++------ > 1 file changed, 4 insertions(+), 6 deletions(-) > > diff --git a/target/riscv/csr.c b/target/riscv/csr.c > index 58499b5afc..6ac11d1f11 100644 > --- a/target/riscv/csr.c > +++ b/target/riscv/csr.c > @@ -1311,11 +1311,9 @@ static RISCVException write_mstatus(CPURISCVState *env, int csrno, > } > > if (xl != MXL_RV32 || env->debugger) { > - /* > - * RV32: MPV and GVA are not in mstatus. The current plan is to > - * add them to mstatush. For now, we just don't support it. > - */ > - mask |= MSTATUS_MPV | MSTATUS_GVA; > + if (riscv_has_ext(env, RVH)) { > + mask |= MSTATUS_MPV | MSTATUS_GVA; > + } > if ((val & MSTATUS64_UXL) != 0) { > mask |= MSTATUS64_UXL; > } > @@ -1351,7 +1349,7 @@ static RISCVException write_mstatush(CPURISCVState *env, int csrno, > target_ulong val) > { > uint64_t valh = (uint64_t)val << 32; > - uint64_t mask = MSTATUS_MPV | MSTATUS_GVA; > + uint64_t mask = riscv_has_ext(env, RVH) ? MSTATUS_MPV | MSTATUS_GVA : 0; > > env->mstatus = (env->mstatus & ~mask) | (valh & mask); >
On Mon, May 29, 2023 at 10:18 PM Weiwei Li <liweiwei@iscas.ac.cn> wrote: > > MPV and GVA bits are added by hypervisor extension to mstatus > and mstatush (if MXLEN=32). > > Signed-off-by: Weiwei Li <liweiwei@iscas.ac.cn> > Signed-off-by: Junqiang Wang <wangjunqiang@iscas.ac.cn> Reviewed-by: Alistair Francis <alistair.francis@wdc.com> Alistair > --- > target/riscv/csr.c | 10 ++++------ > 1 file changed, 4 insertions(+), 6 deletions(-) > > diff --git a/target/riscv/csr.c b/target/riscv/csr.c > index 58499b5afc..6ac11d1f11 100644 > --- a/target/riscv/csr.c > +++ b/target/riscv/csr.c > @@ -1311,11 +1311,9 @@ static RISCVException write_mstatus(CPURISCVState *env, int csrno, > } > > if (xl != MXL_RV32 || env->debugger) { > - /* > - * RV32: MPV and GVA are not in mstatus. The current plan is to > - * add them to mstatush. For now, we just don't support it. > - */ > - mask |= MSTATUS_MPV | MSTATUS_GVA; > + if (riscv_has_ext(env, RVH)) { > + mask |= MSTATUS_MPV | MSTATUS_GVA; > + } > if ((val & MSTATUS64_UXL) != 0) { > mask |= MSTATUS64_UXL; > } > @@ -1351,7 +1349,7 @@ static RISCVException write_mstatush(CPURISCVState *env, int csrno, > target_ulong val) > { > uint64_t valh = (uint64_t)val << 32; > - uint64_t mask = MSTATUS_MPV | MSTATUS_GVA; > + uint64_t mask = riscv_has_ext(env, RVH) ? MSTATUS_MPV | MSTATUS_GVA : 0; > > env->mstatus = (env->mstatus & ~mask) | (valh & mask); > > -- > 2.25.1 > >
On 2023/5/29 20:17, Weiwei Li wrote: > MPV and GVA bits are added by hypervisor extension to mstatus > and mstatush (if MXLEN=32). Have you found the CSR field specifications for them, especially for GVA. Zhiwei > > Signed-off-by: Weiwei Li <liweiwei@iscas.ac.cn> > Signed-off-by: Junqiang Wang <wangjunqiang@iscas.ac.cn> > --- > target/riscv/csr.c | 10 ++++------ > 1 file changed, 4 insertions(+), 6 deletions(-) > > diff --git a/target/riscv/csr.c b/target/riscv/csr.c > index 58499b5afc..6ac11d1f11 100644 > --- a/target/riscv/csr.c > +++ b/target/riscv/csr.c > @@ -1311,11 +1311,9 @@ static RISCVException write_mstatus(CPURISCVState *env, int csrno, > } > > if (xl != MXL_RV32 || env->debugger) { > - /* > - * RV32: MPV and GVA are not in mstatus. The current plan is to > - * add them to mstatush. For now, we just don't support it. > - */ > - mask |= MSTATUS_MPV | MSTATUS_GVA; > + if (riscv_has_ext(env, RVH)) { > + mask |= MSTATUS_MPV | MSTATUS_GVA; > + } > if ((val & MSTATUS64_UXL) != 0) { > mask |= MSTATUS64_UXL; > } > @@ -1351,7 +1349,7 @@ static RISCVException write_mstatush(CPURISCVState *env, int csrno, > target_ulong val) > { > uint64_t valh = (uint64_t)val << 32; > - uint64_t mask = MSTATUS_MPV | MSTATUS_GVA; > + uint64_t mask = riscv_has_ext(env, RVH) ? MSTATUS_MPV | MSTATUS_GVA : 0; > > env->mstatus = (env->mstatus & ~mask) | (valh & mask); >
On 2023/6/12 11:08, LIU Zhiwei wrote: > > On 2023/5/29 20:17, Weiwei Li wrote: >> MPV and GVA bits are added by hypervisor extension to mstatus >> and mstatush (if MXLEN=32). > > Have you found the CSR field specifications for them, especially for GVA. Yeah. in the section 9.4.1 of the privilege spec: "/The hypervisor extension adds two fields, MPV and GVA, to the machine-level mstatus or mstatush CSR/". Regards, Weiwei Li > > Zhiwei > >> >> Signed-off-by: Weiwei Li <liweiwei@iscas.ac.cn> >> Signed-off-by: Junqiang Wang <wangjunqiang@iscas.ac.cn> >> --- >> target/riscv/csr.c | 10 ++++------ >> 1 file changed, 4 insertions(+), 6 deletions(-) >> >> diff --git a/target/riscv/csr.c b/target/riscv/csr.c >> index 58499b5afc..6ac11d1f11 100644 >> --- a/target/riscv/csr.c >> +++ b/target/riscv/csr.c >> @@ -1311,11 +1311,9 @@ static RISCVException >> write_mstatus(CPURISCVState *env, int csrno, >> } >> if (xl != MXL_RV32 || env->debugger) { >> - /* >> - * RV32: MPV and GVA are not in mstatus. The current plan is to >> - * add them to mstatush. For now, we just don't support it. >> - */ >> - mask |= MSTATUS_MPV | MSTATUS_GVA; >> + if (riscv_has_ext(env, RVH)) { >> + mask |= MSTATUS_MPV | MSTATUS_GVA; >> + } >> if ((val & MSTATUS64_UXL) != 0) { >> mask |= MSTATUS64_UXL; >> } >> @@ -1351,7 +1349,7 @@ static RISCVException >> write_mstatush(CPURISCVState *env, int csrno, >> target_ulong val) >> { >> uint64_t valh = (uint64_t)val << 32; >> - uint64_t mask = MSTATUS_MPV | MSTATUS_GVA; >> + uint64_t mask = riscv_has_ext(env, RVH) ? MSTATUS_MPV | >> MSTATUS_GVA : 0; >> env->mstatus = (env->mstatus & ~mask) | (valh & mask);
On 2023/6/12 11:16, Weiwei Li wrote: > > On 2023/6/12 11:08, LIU Zhiwei wrote: >> >> On 2023/5/29 20:17, Weiwei Li wrote: >>> MPV and GVA bits are added by hypervisor extension to mstatus >>> and mstatush (if MXLEN=32). >> >> Have you found the CSR field specifications for them, especially for >> GVA. > > Yeah. in the section 9.4.1 of the privilege spec: > > "/The hypervisor extension adds two fields, MPV and GVA, to the > machine-level mstatus or mstatush CSR/". I mean the WARL or other CSR field specifications here. Zhiwei > > Regards, > > Weiwei Li > >> >> Zhiwei >> >>> >>> Signed-off-by: Weiwei Li <liweiwei@iscas.ac.cn> >>> Signed-off-by: Junqiang Wang <wangjunqiang@iscas.ac.cn> >>> --- >>> target/riscv/csr.c | 10 ++++------ >>> 1 file changed, 4 insertions(+), 6 deletions(-) >>> >>> diff --git a/target/riscv/csr.c b/target/riscv/csr.c >>> index 58499b5afc..6ac11d1f11 100644 >>> --- a/target/riscv/csr.c >>> +++ b/target/riscv/csr.c >>> @@ -1311,11 +1311,9 @@ static RISCVException >>> write_mstatus(CPURISCVState *env, int csrno, >>> } >>> if (xl != MXL_RV32 || env->debugger) { >>> - /* >>> - * RV32: MPV and GVA are not in mstatus. The current plan >>> is to >>> - * add them to mstatush. For now, we just don't support it. >>> - */ >>> - mask |= MSTATUS_MPV | MSTATUS_GVA; >>> + if (riscv_has_ext(env, RVH)) { >>> + mask |= MSTATUS_MPV | MSTATUS_GVA; >>> + } >>> if ((val & MSTATUS64_UXL) != 0) { >>> mask |= MSTATUS64_UXL; >>> } >>> @@ -1351,7 +1349,7 @@ static RISCVException >>> write_mstatush(CPURISCVState *env, int csrno, >>> target_ulong val) >>> { >>> uint64_t valh = (uint64_t)val << 32; >>> - uint64_t mask = MSTATUS_MPV | MSTATUS_GVA; >>> + uint64_t mask = riscv_has_ext(env, RVH) ? MSTATUS_MPV | >>> MSTATUS_GVA : 0; >>> env->mstatus = (env->mstatus & ~mask) | (valh & mask);
On 2023/6/12 11:18, LIU Zhiwei wrote: > > On 2023/6/12 11:16, Weiwei Li wrote: >> >> On 2023/6/12 11:08, LIU Zhiwei wrote: >>> >>> On 2023/5/29 20:17, Weiwei Li wrote: >>>> MPV and GVA bits are added by hypervisor extension to mstatus >>>> and mstatush (if MXLEN=32). >>> >>> Have you found the CSR field specifications for them, especially for >>> GVA. >> >> Yeah. in the section 9.4.1 of the privilege spec: >> >> "/The hypervisor extension adds two fields, MPV and GVA, to the >> machine-level mstatus or mstatush CSR/". > > I mean the WARL or other CSR field specifications here. I don't quite get your idea. The only specification for MPV and GVA I found is in section 9.4.1. The spec for most of mstatus fields can be found in Section 3.1.6 "Machine Status Registers (mstatus and mstatush)". Regards, Weiwei Li > > Zhiwei > >> >> Regards, >> >> Weiwei Li >> >>> >>> Zhiwei >>> >>>> >>>> Signed-off-by: Weiwei Li <liweiwei@iscas.ac.cn> >>>> Signed-off-by: Junqiang Wang <wangjunqiang@iscas.ac.cn> >>>> --- >>>> target/riscv/csr.c | 10 ++++------ >>>> 1 file changed, 4 insertions(+), 6 deletions(-) >>>> >>>> diff --git a/target/riscv/csr.c b/target/riscv/csr.c >>>> index 58499b5afc..6ac11d1f11 100644 >>>> --- a/target/riscv/csr.c >>>> +++ b/target/riscv/csr.c >>>> @@ -1311,11 +1311,9 @@ static RISCVException >>>> write_mstatus(CPURISCVState *env, int csrno, >>>> } >>>> if (xl != MXL_RV32 || env->debugger) { >>>> - /* >>>> - * RV32: MPV and GVA are not in mstatus. The current plan >>>> is to >>>> - * add them to mstatush. For now, we just don't support it. >>>> - */ >>>> - mask |= MSTATUS_MPV | MSTATUS_GVA; >>>> + if (riscv_has_ext(env, RVH)) { >>>> + mask |= MSTATUS_MPV | MSTATUS_GVA; >>>> + } >>>> if ((val & MSTATUS64_UXL) != 0) { >>>> mask |= MSTATUS64_UXL; >>>> } >>>> @@ -1351,7 +1349,7 @@ static RISCVException >>>> write_mstatush(CPURISCVState *env, int csrno, >>>> target_ulong val) >>>> { >>>> uint64_t valh = (uint64_t)val << 32; >>>> - uint64_t mask = MSTATUS_MPV | MSTATUS_GVA; >>>> + uint64_t mask = riscv_has_ext(env, RVH) ? MSTATUS_MPV | >>>> MSTATUS_GVA : 0; >>>> env->mstatus = (env->mstatus & ~mask) | (valh & mask);
On 2023/6/12 12:35, Weiwei Li wrote: > > On 2023/6/12 11:18, LIU Zhiwei wrote: >> >> On 2023/6/12 11:16, Weiwei Li wrote: >>> >>> On 2023/6/12 11:08, LIU Zhiwei wrote: >>>> >>>> On 2023/5/29 20:17, Weiwei Li wrote: >>>>> MPV and GVA bits are added by hypervisor extension to mstatus >>>>> and mstatush (if MXLEN=32). >>>> >>>> Have you found the CSR field specifications for them, especially >>>> for GVA. >>> >>> Yeah. in the section 9.4.1 of the privilege spec: >>> >>> "/The hypervisor extension adds two fields, MPV and GVA, to the >>> machine-level mstatus or mstatush CSR/". >> >> I mean the WARL or other CSR field specifications here. > > I don't quite get your idea. The only specification for MPV and GVA I > found is in section 9.4.1. The spec for most of mstatus fields can be > found in Section 3.1.6 > "Machine Status Registers (mstatus and mstatush)". I mean is the GVA field read only or WARL(WPRI or WLRL) for the software? It could be written by the implementation. But I am not sure whether it could be written by the software. Zhiwei > > Regards, > > Weiwei Li > >> >> Zhiwei >> >>> >>> Regards, >>> >>> Weiwei Li >>> >>>> >>>> Zhiwei >>>> >>>>> >>>>> Signed-off-by: Weiwei Li <liweiwei@iscas.ac.cn> >>>>> Signed-off-by: Junqiang Wang <wangjunqiang@iscas.ac.cn> >>>>> --- >>>>> target/riscv/csr.c | 10 ++++------ >>>>> 1 file changed, 4 insertions(+), 6 deletions(-) >>>>> >>>>> diff --git a/target/riscv/csr.c b/target/riscv/csr.c >>>>> index 58499b5afc..6ac11d1f11 100644 >>>>> --- a/target/riscv/csr.c >>>>> +++ b/target/riscv/csr.c >>>>> @@ -1311,11 +1311,9 @@ static RISCVException >>>>> write_mstatus(CPURISCVState *env, int csrno, >>>>> } >>>>> if (xl != MXL_RV32 || env->debugger) { >>>>> - /* >>>>> - * RV32: MPV and GVA are not in mstatus. The current plan >>>>> is to >>>>> - * add them to mstatush. For now, we just don't support it. >>>>> - */ >>>>> - mask |= MSTATUS_MPV | MSTATUS_GVA; >>>>> + if (riscv_has_ext(env, RVH)) { >>>>> + mask |= MSTATUS_MPV | MSTATUS_GVA; >>>>> + } >>>>> if ((val & MSTATUS64_UXL) != 0) { >>>>> mask |= MSTATUS64_UXL; >>>>> } >>>>> @@ -1351,7 +1349,7 @@ static RISCVException >>>>> write_mstatush(CPURISCVState *env, int csrno, >>>>> target_ulong val) >>>>> { >>>>> uint64_t valh = (uint64_t)val << 32; >>>>> - uint64_t mask = MSTATUS_MPV | MSTATUS_GVA; >>>>> + uint64_t mask = riscv_has_ext(env, RVH) ? MSTATUS_MPV | >>>>> MSTATUS_GVA : 0; >>>>> env->mstatus = (env->mstatus & ~mask) | (valh & mask);
On 2023/6/12 13:40, LIU Zhiwei wrote: > > On 2023/6/12 12:35, Weiwei Li wrote: >> >> On 2023/6/12 11:18, LIU Zhiwei wrote: >>> >>> On 2023/6/12 11:16, Weiwei Li wrote: >>>> >>>> On 2023/6/12 11:08, LIU Zhiwei wrote: >>>>> >>>>> On 2023/5/29 20:17, Weiwei Li wrote: >>>>>> MPV and GVA bits are added by hypervisor extension to mstatus >>>>>> and mstatush (if MXLEN=32). >>>>> >>>>> Have you found the CSR field specifications for them, especially >>>>> for GVA. >>>> >>>> Yeah. in the section 9.4.1 of the privilege spec: >>>> >>>> "/The hypervisor extension adds two fields, MPV and GVA, to the >>>> machine-level mstatus or mstatush CSR/". >>> >>> I mean the WARL or other CSR field specifications here. >> >> I don't quite get your idea. The only specification for MPV and GVA >> I found is in section 9.4.1. The spec for most of mstatus fields can >> be found in Section 3.1.6 >> "Machine Status Registers (mstatus and mstatush)". > > I mean is the GVA field read only or WARL(WPRI or WLRL) for the > software? It could be written by the implementation. But I am not sure > whether it could be written by the software. No, I didn't find any description about this. Regards, Weiwei Li > > Zhiwei > >> >> Regards, >> >> Weiwei Li >> >>> >>> Zhiwei >>> >>>> >>>> Regards, >>>> >>>> Weiwei Li >>>> >>>>> >>>>> Zhiwei >>>>> >>>>>> >>>>>> Signed-off-by: Weiwei Li <liweiwei@iscas.ac.cn> >>>>>> Signed-off-by: Junqiang Wang <wangjunqiang@iscas.ac.cn> >>>>>> --- >>>>>> target/riscv/csr.c | 10 ++++------ >>>>>> 1 file changed, 4 insertions(+), 6 deletions(-) >>>>>> >>>>>> diff --git a/target/riscv/csr.c b/target/riscv/csr.c >>>>>> index 58499b5afc..6ac11d1f11 100644 >>>>>> --- a/target/riscv/csr.c >>>>>> +++ b/target/riscv/csr.c >>>>>> @@ -1311,11 +1311,9 @@ static RISCVException >>>>>> write_mstatus(CPURISCVState *env, int csrno, >>>>>> } >>>>>> if (xl != MXL_RV32 || env->debugger) { >>>>>> - /* >>>>>> - * RV32: MPV and GVA are not in mstatus. The current >>>>>> plan is to >>>>>> - * add them to mstatush. For now, we just don't support it. >>>>>> - */ >>>>>> - mask |= MSTATUS_MPV | MSTATUS_GVA; >>>>>> + if (riscv_has_ext(env, RVH)) { >>>>>> + mask |= MSTATUS_MPV | MSTATUS_GVA; >>>>>> + } >>>>>> if ((val & MSTATUS64_UXL) != 0) { >>>>>> mask |= MSTATUS64_UXL; >>>>>> } >>>>>> @@ -1351,7 +1349,7 @@ static RISCVException >>>>>> write_mstatush(CPURISCVState *env, int csrno, >>>>>> target_ulong val) >>>>>> { >>>>>> uint64_t valh = (uint64_t)val << 32; >>>>>> - uint64_t mask = MSTATUS_MPV | MSTATUS_GVA; >>>>>> + uint64_t mask = riscv_has_ext(env, RVH) ? MSTATUS_MPV | >>>>>> MSTATUS_GVA : 0; >>>>>> env->mstatus = (env->mstatus & ~mask) | (valh & mask);
diff --git a/target/riscv/csr.c b/target/riscv/csr.c index 58499b5afc..6ac11d1f11 100644 --- a/target/riscv/csr.c +++ b/target/riscv/csr.c @@ -1311,11 +1311,9 @@ static RISCVException write_mstatus(CPURISCVState *env, int csrno, } if (xl != MXL_RV32 || env->debugger) { - /* - * RV32: MPV and GVA are not in mstatus. The current plan is to - * add them to mstatush. For now, we just don't support it. - */ - mask |= MSTATUS_MPV | MSTATUS_GVA; + if (riscv_has_ext(env, RVH)) { + mask |= MSTATUS_MPV | MSTATUS_GVA; + } if ((val & MSTATUS64_UXL) != 0) { mask |= MSTATUS64_UXL; } @@ -1351,7 +1349,7 @@ static RISCVException write_mstatush(CPURISCVState *env, int csrno, target_ulong val) { uint64_t valh = (uint64_t)val << 32; - uint64_t mask = MSTATUS_MPV | MSTATUS_GVA; + uint64_t mask = riscv_has_ext(env, RVH) ? MSTATUS_MPV | MSTATUS_GVA : 0; env->mstatus = (env->mstatus & ~mask) | (valh & mask);