| Message ID | 20250415080254.3667878-2-ziqiaokong@gmail.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | Fix endless translation loop of riscv | expand |
On 4/15/25 01:02, Ziqiao Kong wrote: > On big endian systems, pte and updated_pte hold big endian host data > while pte_pa points to little endian target data. This means the branch > at cpu_helper.c:1669 will be always satisfied and restart translation, > causing an endless translation loop. > > The correctness of this patch can be deduced by: > > old_pte will hold value either from cpu_to_le32/64(pte) or > cpu_to_le32/64(updated_pte), both of wich is litte endian. After that, > an in-place conversion by le32/64_to_cpu(old_pte) ensures that old_pte > now is in native endian, same with pte. Therefore, the endianness of the > both side of if (old_pte != pte) is correct. > > Signed-off-by: Ziqiao Kong<ziqiaokong@gmail.com> > --- > target/riscv/cpu_helper.c | 6 ++++-- > 1 file changed, 4 insertions(+), 2 deletions(-) Reviewed-by: Richard Henderson <richard.henderson@linaro.org> r~
On Tue, Apr 15, 2025 at 6:06 PM Ziqiao Kong <ziqiaokong@gmail.com> wrote: > > On big endian systems, pte and updated_pte hold big endian host data > while pte_pa points to little endian target data. This means the branch > at cpu_helper.c:1669 will be always satisfied and restart translation, > causing an endless translation loop. > > The correctness of this patch can be deduced by: > > old_pte will hold value either from cpu_to_le32/64(pte) or > cpu_to_le32/64(updated_pte), both of wich is litte endian. After that, > an in-place conversion by le32/64_to_cpu(old_pte) ensures that old_pte > now is in native endian, same with pte. Therefore, the endianness of the > both side of if (old_pte != pte) is correct. > > Signed-off-by: Ziqiao Kong <ziqiaokong@gmail.com> Reviewed-by: Alistair Francis <alistair.francis@wdc.com> Alistair > --- > target/riscv/cpu_helper.c | 6 ++++-- > 1 file changed, 4 insertions(+), 2 deletions(-) > > diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c > index 6c4391d96b..3233b66e7e 100644 > --- a/target/riscv/cpu_helper.c > +++ b/target/riscv/cpu_helper.c > @@ -1662,9 +1662,11 @@ static int get_physical_address(CPURISCVState *env, hwaddr *physical, > target_ulong *pte_pa = qemu_map_ram_ptr(mr->ram_block, addr1); > target_ulong old_pte; > if (riscv_cpu_sxl(env) == MXL_RV32) { > - old_pte = qatomic_cmpxchg((uint32_t *)pte_pa, pte, updated_pte); > + old_pte = qatomic_cmpxchg((uint32_t *)pte_pa, cpu_to_le32(pte), cpu_to_le32(updated_pte)); > + old_pte = le32_to_cpu(old_pte); > } else { > - old_pte = qatomic_cmpxchg(pte_pa, pte, updated_pte); > + old_pte = qatomic_cmpxchg(pte_pa, cpu_to_le64(pte), cpu_to_le64(updated_pte)); > + old_pte = le64_to_cpu(old_pte); > } > if (old_pte != pte) { > goto restart; > -- > 2.34.1 > >
diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c index 6c4391d96b..3233b66e7e 100644 --- a/target/riscv/cpu_helper.c +++ b/target/riscv/cpu_helper.c @@ -1662,9 +1662,11 @@ static int get_physical_address(CPURISCVState *env, hwaddr *physical, target_ulong *pte_pa = qemu_map_ram_ptr(mr->ram_block, addr1); target_ulong old_pte; if (riscv_cpu_sxl(env) == MXL_RV32) { - old_pte = qatomic_cmpxchg((uint32_t *)pte_pa, pte, updated_pte); + old_pte = qatomic_cmpxchg((uint32_t *)pte_pa, cpu_to_le32(pte), cpu_to_le32(updated_pte)); + old_pte = le32_to_cpu(old_pte); } else { - old_pte = qatomic_cmpxchg(pte_pa, pte, updated_pte); + old_pte = qatomic_cmpxchg(pte_pa, cpu_to_le64(pte), cpu_to_le64(updated_pte)); + old_pte = le64_to_cpu(old_pte); } if (old_pte != pte) { goto restart;
On big endian systems, pte and updated_pte hold big endian host data while pte_pa points to little endian target data. This means the branch at cpu_helper.c:1669 will be always satisfied and restart translation, causing an endless translation loop. The correctness of this patch can be deduced by: old_pte will hold value either from cpu_to_le32/64(pte) or cpu_to_le32/64(updated_pte), both of wich is litte endian. After that, an in-place conversion by le32/64_to_cpu(old_pte) ensures that old_pte now is in native endian, same with pte. Therefore, the endianness of the both side of if (old_pte != pte) is correct. Signed-off-by: Ziqiao Kong <ziqiaokong@gmail.com> --- target/riscv/cpu_helper.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-)