On Wed, Oct 7, 2026 at 9:54 PM Philippe Mathieu-Daudé <[email protected]> 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] >
Hi Phil >From my point of view, it should be handled in the function target_long_bits instead for the future heterogeneous simulation. So in future target_long_bits can even return 128bit instead. We can ensure a single cpu can run on a single thread. That is, a single thread can not running two different CPU architectures, so we can use thread-local-storage for target_long_bits() to retrieve what the current thread's cpu bits are. Otherwise, heterogeneous simulation would be rather hard(my point of view). > 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; > >>>> > >>> > >>> > >> > > > > > -- 此致 礼 罗勇刚 Yours sincerely, Yonggang Luo
