Hi, Philippe, On Sat, Nov 7, 2020 at 8:11 PM Philippe Mathieu-Daudé <f4...@amsat.org> wrote: > > Hi Huacai, > > On 11/6/20 5:21 AM, Huacai Chen wrote: > > From: Jiaxun Yang <jiaxun.y...@flygoat.com> > > > > Our current code assumed the target page size is always 4k > > when handling PageMask and VPN2, however, variable page size > > was just added to mips target and that's no longer true. > > > > Fixes: ee3863b9d414 ("target/mips: Support variable page size") > > Signed-off-by: Jiaxun Yang <jiaxun.y...@flygoat.com> > > Signed-off-by: Huacai Chen <che...@lemote.com> > > --- > > target/mips/cp0_helper.c | 27 +++++++++++++++++++++------ > > target/mips/cpu.h | 1 + > > 2 files changed, 22 insertions(+), 6 deletions(-) > > Tested-by: Philippe Mathieu-Daudé <f4...@amsat.org> > > > > > diff --git a/target/mips/cp0_helper.c b/target/mips/cp0_helper.c > > index 709cc9a..92bf14f 100644 > > --- a/target/mips/cp0_helper.c > > +++ b/target/mips/cp0_helper.c > > @@ -892,13 +892,28 @@ void helper_mtc0_memorymapid(CPUMIPSState *env, > > target_ulong arg1) > > > > void update_pagemask(CPUMIPSState *env, target_ulong arg1, int32_t > > *pagemask) > > { > > - uint64_t mask = arg1 >> (TARGET_PAGE_BITS + 1); > > - if (!(env->insn_flags & ISA_MIPS32R6) || (arg1 == ~0) || > > - (mask == 0x0000 || mask == 0x0003 || mask == 0x000F || > > - mask == 0x003F || mask == 0x00FF || mask == 0x03FF || > > - mask == 0x0FFF || mask == 0x3FFF || mask == 0xFFFF)) { > > - env->CP0_PageMask = arg1 & (0x1FFFFFFF & (TARGET_PAGE_MASK << 1)); > > + unsigned long mask; > > + int maskbits; > > + > > + /* Don't care MASKX as we don't support 1KB page */ > > + mask = extract32((uint32_t)arg1, CP0PM_MASK, 16); > > I'd simply use extract64(). > > > + maskbits = find_first_zero_bit(&mask, 32); > > + > > + /* Ensure no more set bit after first zero */ > > + if (mask >> maskbits) { > > + goto invalid; > > + } > > + /* We don't support VTLB entry smaller than target page */ > > + if ((maskbits + 12) < TARGET_PAGE_BITS) { > > I suppose the magic 12 is TARGET_PAGE_BITS_MIN, right? > > If you confirm I can do the change when applying (no need to > repost the whole series). Yes, 12 is TARGET_PAGE_BITS_MIN.
Huacai > > > + goto invalid; > > } > > + env->CP0_PageMask = mask << CP0PM_MASK; > > + > > + return; > > + > > +invalid: > > + /* When invalid, set to default target page size. */ > > + env->CP0_PageMask = (~TARGET_PAGE_MASK >> 12) << CP0PM_MASK; > > Ditto. > > > } > > > > void helper_mtc0_pagemask(CPUMIPSState *env, target_ulong arg1) > > diff --git a/target/mips/cpu.h b/target/mips/cpu.h > > index d41579d..23f8c6f 100644 > > --- a/target/mips/cpu.h > > +++ b/target/mips/cpu.h > > @@ -619,6 +619,7 @@ struct CPUMIPSState { > > * CP0 Register 5 > > */ > > int32_t CP0_PageMask; > > +#define CP0PM_MASK 13 > > int32_t CP0_PageGrain_rw_bitmask; > > int32_t CP0_PageGrain; > > #define CP0PG_RIE 31 > >