On Tue, Aug 13, 2024 at 05:43:07PM GMT, Richard Henderson wrote:
> On 8/13/24 17:13, Andrew Jones 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
> > revert to using target_long, since a signed type will get extended.
> >
> > Fixes: af3fc195e3c8 ("target/riscv: Change the TLB page size depends on PMP
> > entries.")
> > Signed-off-by: Andrew Jones <ajo...@ventanamicro.com>
> > ---
> > target/riscv/cpu_helper.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c
> > index 395a1d914061..dfef1b20d1e8 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;
> > + target_long tlb_size = TARGET_PAGE_SIZE;
>
> If rv32 physical addresses are 34 bits, then you probably didn't want
> target_*long at all.
Yes, just using hwaddr for everything that only touches physical addresses
would probably be best, but, ifaict, it's pretty common to use target_long
for masks used on both virtual and physical addresses (TARGET_PAGE_MASK,
for example). This 'tlb_size' variable is used on both as well.
Thanks,
drew
