Re: [PATCH v2] target/riscv32: Fix masking of physical address

[Alistair Francis <alistair23@gmail.com>]

On Mon, Sep 9, 2024 at 6:33 PM Andrew Jones <ajones@ventanamicro.com> wrote:
>
> C doesn't extend the sign bit for unsigned types since there isn't a
> sign bit to extend. This means a promotion of a u32 to a u64 results
> in the upper 32 bits of the u64 being zero. If that result is then
> used as a mask on another u64 the upper 32 bits will be cleared. rv32
> physical addresses may be up to 34 bits wide, so we don't want to
> clear the high bits while page aligning the address. The fix is to
> use hwaddr for the mask, which, even on rv32, is 64-bits wide.
>
> Fixes: af3fc195e3c8 ("target/riscv: Change the TLB page size depends on PMP entries.")
> Signed-off-by: Andrew Jones <ajones@ventanamicro.com>

Thanks!

Applied to riscv-to-apply.next

Alistair


> ---
> -v2: Switch from signed long to hwaddr
>
>  target/riscv/cpu_helper.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c
> index 395a1d914061..4b2c72780c36 100644
> --- a/target/riscv/cpu_helper.c
> +++ b/target/riscv/cpu_helper.c
> @@ -1323,7 +1323,7 @@ bool riscv_cpu_tlb_fill(CPUState *cs, vaddr address, int size,
>      int ret = TRANSLATE_FAIL;
>      int mode = mmuidx_priv(mmu_idx);
>      /* default TLB page size */
> -    target_ulong tlb_size = TARGET_PAGE_SIZE;
> +    hwaddr tlb_size = TARGET_PAGE_SIZE;
>
>      env->guest_phys_fault_addr = 0;
>
> @@ -1375,7 +1375,7 @@ bool riscv_cpu_tlb_fill(CPUState *cs, vaddr address, int size,
>
>                  qemu_log_mask(CPU_LOG_MMU,
>                                "%s PMP address=" HWADDR_FMT_plx " ret %d prot"
> -                              " %d tlb_size " TARGET_FMT_lu "\n",
> +                              " %d tlb_size %" HWADDR_PRIu "\n",
>                                __func__, pa, ret, prot_pmp, tlb_size);
>
>                  prot &= prot_pmp;
> @@ -1409,7 +1409,7 @@ bool riscv_cpu_tlb_fill(CPUState *cs, vaddr address, int size,
>
>              qemu_log_mask(CPU_LOG_MMU,
>                            "%s PMP address=" HWADDR_FMT_plx " ret %d prot"
> -                          " %d tlb_size " TARGET_FMT_lu "\n",
> +                          " %d tlb_size %" HWADDR_PRIu "\n",
>                            __func__, pa, ret, prot_pmp, tlb_size);
>
>              prot &= prot_pmp;
> --
> 2.46.0
>
>