Re: [PATCH 4/6] target/riscv: Detect sxl to set bit width for RV32 in RV64

[LIU Zhiwei]

On 2024/7/3 10:33, Alistair Francis wrote:
> On Mon, Jul 1, 2024 at 1:41 PM LIU Zhiwei <zhiwei_liu@linux.alibaba.com> wrote:
> > From: TANG Tiancheng <tangtiancheng.ttc@alibaba-inc.com>
> >
> > Ensure correct bit width based on sxl when running RV32 on RV64 QEMU.
> > This is required as MMU address translations run in S-mode.
> >
> > Signed-off-by: TANG Tiancheng <tangtiancheng.ttc@alibaba-inc.com>
> > Reviewed-by: Liu Zhiwei <zhiwei_liu@linux.alibaba.com>
> > ---
> >   target/riscv/cpu_helper.c | 15 +++++++++++----
> >   1 file changed, 11 insertions(+), 4 deletions(-)
> >
> > diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c
> > index 6709622dd3..1af83a0a36 100644
> > --- a/target/riscv/cpu_helper.c
> > +++ b/target/riscv/cpu_helper.c
> > @@ -887,12 +887,14 @@ static int get_physical_address(CPURISCVState *env, 
> > hwaddr *physical,
> >
> >       CPUState *cs = env_cpu(env);
> >       int va_bits = PGSHIFT + levels * ptidxbits + widened;
> > +    int sxlen = 16UL << riscv_cpu_sxl(env);
> > +    int sxlen_bytes = sxlen / 8;
> >
> >       if (first_stage == true) {
> >           target_ulong mask, masked_msbs;
> >
> > -        if (TARGET_LONG_BITS > (va_bits - 1)) {
> > -            mask = (1L << (TARGET_LONG_BITS - (va_bits - 1))) - 1;
> > +        if (sxlen > (va_bits - 1)) {
> > +            mask = (1L << (sxlen - (va_bits - 1))) - 1;
> >           } else {
> >               mask = 0;
> >           }
> > @@ -961,7 +963,7 @@ restart:
> >
> >           int pmp_prot;
> >           int pmp_ret = get_physical_address_pmp(env, &pmp_prot, pte_addr,
> > -                                               sizeof(target_ulong),
> > +                                               sxlen_bytes,
> >                                                  MMU_DATA_LOAD, PRV_S);
> >           if (pmp_ret != TRANSLATE_SUCCESS) {
> >               return TRANSLATE_PMP_FAIL;
> > @@ -1113,7 +1115,7 @@ restart:
> >            *   it is no longer valid and we must re-walk the page table.
> >            */
> >           MemoryRegion *mr;
> > -        hwaddr l = sizeof(target_ulong), addr1;
> > +        hwaddr l = sxlen_bytes, addr1;
> >           mr = address_space_translate(cs->as, pte_addr, &addr1, &l,
> >                                        false, MEMTXATTRS_UNSPECIFIED);
> >           if (memory_region_is_ram(mr)) {
> > @@ -1126,6 +1128,11 @@ restart:
> >               *pte_pa = pte = updated_pte;
> >   #else
> >               target_ulong old_pte = qatomic_cmpxchg(pte_pa, pte, updated_pte);
> I think you missed removing this line

Good catch.  We will fix this in v2 patch set.

Thanks,
Zhiwei

> Alistair
>
> > +            if (riscv_cpu_sxl(env) == MXL_RV32) {
> > +                old_pte = qatomic_cmpxchg((uint32_t *)pte_pa, pte, 
> > updated_pte);
> > +            } else {
> > +                old_pte = qatomic_cmpxchg(pte_pa, pte, updated_pte);
> > +            }
> >               if (old_pte != pte) {
> >                   goto restart;
> >               }
> > --
> > 2.43.0