On 07/10/26, Philippe Mathieu-Daudé wrote: > Phil. > > Regards, > > I missed it, but again Anton's comment were in this patch > description I wouldn't had barked. > > How does this will work in heterogeneous binary with one > RV32 and another RV64 cores running together? > > [we tend to avoid top-posting on technical lists]
Ah my bad I should have mentioned it here also. riscv_cpu_mxl_bits() vs target_long_bits() is, from my understanding, one of the main discrepancies between rv32 on qemu-riscv32 vs qemu-riscv64, and they are not equivalent since 32-bit code on qemu-riscv64 will still have target_long_bits() == 64, but riscv_cpu_mxl_bits() == 32. For the time being using target_long_bits() preserves the original behaviour. In a heterogeneous setting qemu-riscv64 can already run RV32 and RV64 code despite target_long_bits() == 64, but I'll take a pass over the code to check for incorrect uses of target_long_bits() and submit that separately since that would change behaviour. //Anton > > On 7/10/26 15:39, Yonggang Luo wrote: > > It's here > > https://lore.kernel.org/qemu-devel/[email protected]/ > > > > From: Anton Johansson via qemu development <[email protected]> > > > > > > I wouldn't be surprised if that's the case, although I'm personally a > > bit scared of riscv_cpu_mxl_bits() since it returns 128 for MXL_128, so > > if it's not used carefully or on clear MXL_[32|64] paths you could easily > > get UB. > > > > > > On Wed, Oct 7, 2026 at 9:26 PM Philippe Mathieu-Daudé > > <[email protected]> wrote: > > > > > > On 7/10/26 13:51, Yonggang Luo wrote: > > > > On Wed, Oct 7, 2026 at 6:25 PM Philippe Mathieu-Daudé > > > > <[email protected]> wrote: > > > > > > > > > > On 7/10/26 10:43, Anton Johansson wrote: > > > > > > Reviewed-by: Pierrick Bouvier <[email protected]> > > > > > > Signed-off-by: Anton Johansson <[email protected]> > > > > > > --- > > > > > > target/riscv/tcg/pmp.c | 5 +++-- > > > > > > target/riscv/tcg/meson.build | 2 +- > > > > > > 2 files changed, 4 insertions(+), 3 deletions(-) > > > > > > > > > > > > diff --git a/target/riscv/tcg/pmp.c b/target/riscv/tcg/pmp.c > > > > > > index 99345d63dc..2d7194de10 100644 > > > > > > --- a/target/riscv/tcg/pmp.c > > > > > > +++ b/target/riscv/tcg/pmp.c > > > > > > @@ -638,19 +638,20 @@ uint64_t pmpaddr_csr_read(CPURISCVState *env, > > > > > > uint32_t addr_index) > > > > > > if (addr_index < pmp_regions) { > > > > > > val = env->pmp_state.pmp[addr_index].addr_reg; > > > > > > int g = pmp_get_granularity_g(env); > > > > > > + const unsigned long_bits = target_long_bits(); > > > > > > > > > > We need to fetch this information from @env. Maybe > > > > > riscv_cpu_mxl_bits()? > > > > > > > > discussed before, there is possible be 128bits > > > > > > Please point me to the discussion (directly, saving us a ping / pong), > > > as: > > > > > > ''' > > > This was already discussed on v42: > > > > > > https://lore.kernel.org/qemu-devel/some-message-id-v42-20128438/ > > > ''' > > > > > > Having the discussion decision resumed in the patch description will > > > prevent further reviewers to ask again the same. > > > > > > What do you mean by "possible be 128bits"? If we want to cap the > > > value we can use MIN(64, riscv_cpu_mxl_bits(env)). > > > > > > > > > > > > > switch > > > > > > (pmp_get_a_field(env->pmp_state.pmp[addr_index].cfg_reg)) { > > > > > > case PMP_AMATCH_OFF: > > > > > > /* fallthrough */ > > > > > > case PMP_AMATCH_TOR: > > > > > > /* Bit [g-1:0] read all zero */ > > > > > > - if (g >= 1 && g < TARGET_LONG_BITS) { > > > > > > + if (g >= 1 && g < long_bits) { > > > > > > uint64_t granule = 1ULL << g; > > > > > > val = ROUND_DOWN(val, granule); > > > > > > } > > > > > > break; > > > > > > case PMP_AMATCH_NAPOT: > > > > > > /* Bit [g-2:0] read all one */ > > > > > > - if (g >= 2 && g < TARGET_LONG_BITS) { > > > > > > + if (g >= 2 && g < long_bits) { > > > > > > val = deposit64(val, 0, g - 1, -1ULL); > > > > > > } > > > > > > break; > > > > > > > > > > > > > > > > > > > > >
