Re: [PATCH] target/i386: fix direction of "32-bit MMU" test

[Paolo Bonzini]

On Tue, Apr 9, 2024 at 12:59 PM Zhao Liu <zhao1.liu@intel.com> wrote:
>
> Hi Michael & Paolo,
>
> On Fri, Apr 05, 2024 at 08:30:43PM +0300, Michael Tokarev wrote:
> > Date: Fri, 5 Apr 2024 20:30:43 +0300
> > From: Michael Tokarev <mjt@tls.msk.ru>
> > Subject: Re: [PATCH] target/i386: fix direction of "32-bit MMU" test
> >
> > 01.04.2024 09:02, Michael Tokarev:
> >
> > > Anyone can guess why this rather trivial and obviously correct patch 
> > > causes segfaults
> > > in a few tests in staging-7.2 - when run in tcg mode, namely:
> > >
> > >    pxe-test
> > >    migration-test
> > >    boot-serial-test
> > >    bios-tables-test
> > >    vmgenid-test
> > >    cdrom-test
> > >
> > > When reverting this single commit from staging-7.2, it all works fine 
> > > again.
> >
> > It sigsegvs in probe_access_internal():
> >
> >   CPUTLBEntry *entry = tlb_entry(env, mmu_idx, addr); -- this one returns 
> > NULL,
> >
> > and next there's a call
> >
> >   tlb_addr = tlb_read_ofs(entry, elt_ofs);
> >
> > which fails.
> >
> > #0  0x0000555555c5de8a in tlb_read_ofs (ofs=8, entry=0x0) at 
> > 7.2/accel/tcg/cputlb.c:1455
> > #1  probe_access_internal
> >     (env=0x555556a862a0, addr=4294967280, fault_size=fault_size@entry=1,
> > access_type=access_type@entry=MMU_INST_FETCH, mmu_idx=5,
> > nonfault=nonfault@entry=false, phost=0x7fffea4d32a0, pfull=0x7fffea4d3298,
> > retaddr=0)
> >     at 7.2/accel/tcg/cputlb.c:1555
> > #2  0x0000555555c62aba in get_page_addr_code_hostp
> >     (env=<optimized out>, addr=addr@entry=4294967280, hostp=hostp@entry=0x0)
> >     at 7.2/accel/tcg/cputlb.c:1691
> > #3  0x0000555555c52b54 in get_page_addr_code (addr=4294967280, 
> > env=<optimized out>)
> >     at 7.2/include/exec/exec-all.h:714
> > #4  tb_htable_lookup
> >     (cpu=cpu@entry=0x555556a85530, pc=pc@entry=4294967280,
> > cs_base=cs_base@entry=4294901760, flags=flags@entry=64,
> > cflags=cflags@entry=4278190080) at 7.2/accel/tcg/cpu-exec.c:236
> > #5  0x0000555555c53e8e in tb_lookup
> >     (cflags=4278190080, flags=64, cs_base=4294901760, pc=4294967280, 
> > cpu=0x555556a85530)
> >     at 7.2/accel/tcg/cpu-exec.c:270
> > #6  cpu_exec (cpu=cpu@entry=0x555556a85530) at 7.2/accel/tcg/cpu-exec.c:1001
> > #7  0x0000555555c75d2f in tcg_cpus_exec (cpu=cpu@entry=0x555556a85530)
> >     at 7.2/accel/tcg/tcg-accel-ops.c:69
> > #8  0x0000555555c75e80 in mttcg_cpu_thread_fn (arg=arg@entry=0x555556a85530)
> >     at 7.2/accel/tcg/tcg-accel-ops-mttcg.c:95
> > #9  0x0000555555ded098 in qemu_thread_start (args=0x555556adac40)
> >     at 7.2/util/qemu-thread-posix.c:505
> > #10 0x00007ffff5793134 in start_thread (arg=<optimized out>)
> > #11 0x00007ffff58137dc in clone3 ()
> >
>
> I debugged it manually, and found the problem occurs in tlb_index() with
> mmu_idx=5.
>
> For v7.2, the maximum mmu index supported by i386 is 4 (since
> NB_MMU_MODES = 5 defined in target/i386/cpu-param.h).
>
> On Michael's 7.2-i386-mmu-idx tree, the commit 9fc3a7828d25 ("target/i386:
> use separate MMU indexes for 32-bit accesses") introduced more indexes
> without relaxing the NB_MMU_MODES for i386.
>
> Before this fix, probe_access_internal() just got the wrong mmu_idx as 4,
> and it's not out of bounds. After this fix, the right mmu_idx=5 is truly
> out of bounds.
>
> On the master branch, there's no such issue since the commits ffd824f3f32d
> ("include/exec: Set default NB_MMU_MODES to 16") and 6787318a5d86
> ("target/i386: Remove NB_MMU_MODES define") relaxed upper limit of MMU
> index for i386.

Thanks Zhao! Alternatively, it's enough to set NB_MMU_MODES to 8 in
commit 9fc3a7828d25.

Paolo