Re: [PATCH V16 1/6] target/mips: Fix PageMask with variable page size

[Huacai Chen]

Hi, Phillippe,

On Tue, Nov 3, 2020 at 10:53 PM Philippe Mathieu-Daudé <f4bug@amsat.org> wrote:
>
> On 10/30/20 11:25 AM, Huacai Chen wrote:
> > From: Jiaxun Yang <jiaxun.yang@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.yang@flygoat.com>
> > Signed-off-by: Huacai Chen <chenhc@lemote.com>
> > ---
> >  target/mips/cp0_helper.c | 36 +++++++++++++++++++++++++++++-------
> >  target/mips/cpu.h        |  1 +
> >  2 files changed, 30 insertions(+), 7 deletions(-)
> >
> > diff --git a/target/mips/cp0_helper.c b/target/mips/cp0_helper.c
> > index 12143ac..d90ddd9 100644
> > --- a/target/mips/cp0_helper.c
> > +++ b/target/mips/cp0_helper.c
> > @@ -892,13 +892,31 @@ 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;
> > +
> > +    if (env->insn_flags & ISA_MIPS32R6) {
> > +        return;
> > +    }
> > +    /* Don't care MASKX as we don't support 1KB page */
> > +    mask = extract32((uint32_t)arg1, CP0PM_MASK, 16);
> > +    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) {
> > +        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;
> >  }
>
> I was going to queue this patch for 5.2-rc1, but it fails all
> I6400 tests...
>
>   Linux version 4.7.0-rc1-dirty (root@2e66df87a9ff) (gcc version 6.3.0
> 20170516 (Debian 6.3.0-18) ) #1 SMP Sat Feb 1 18:38:13 UTC 2020
>   GCRs appear to have been moved (expected them at 0x1fbf8000)!
>   earlycon: uart8250 at I/O port 0x3f8 (options '38400n8')
>   bootconsole [uart8250] enabled
>   MIPS CPS SMP unable to proceed without a CM
>   CPU0 revision is: 0001a900 (MIPS I6400)
>   FPU revision is: 20f30300
>   MSA revision is: 00000300
>   MIPS: machine is mti,malta
>   Software DMA cache coherency enabled
>   Determined physical RAM map:
>   memory: 0000000008000000 @ 0000000000000000 (usable)
>   Zone ranges:
>   DMA      [mem 0x0000000000000000-0x0000000000ffffff]
>   DMA32    [mem 0x0000000001000000-0x00000000ffffffff]
>   Normal   empty
>   Movable zone start for each node
>   Early memory node ranges
>   node   0: [mem 0x0000000000000000-0x0000000007ffffff]
>   Initmem setup node 0 [mem 0x0000000000000000-0x0000000007ffffff]
>   Primary instruction cache 64kB, VIPT, 4-way, linesize 64 bytes.
>   Primary data cache 64kB, 4-way, VIPT, no aliases, linesize 64 bytes
>   percpu: Embedded 5 pages/cpu @980000000107c000 s29664 r8192 d44064 u81920
>   Built 1 zonelists in Zone order, mobility grouping on.  Total pages: 8163
>   Kernel command line: clocksource=GIC console=tty0 console=ttyS0
>   PID hash table entries: 512 (order: -2, 4096 bytes)
>   Dentry cache hash table entries: 16384 (order: 3, 131072 bytes)
>   Inode-cache hash table entries: 8192 (order: 2, 65536 bytes)
>   Kernel panic - not syncing: MMU doesn't support PAGE_SIZE=0x4000
OK, let's investigate it.

Huacai