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

[Michael Tokarev]

09.04.2024 14:02, Paolo Bonzini wrote:
> On Tue, Apr 9, 2024 at 12:59 PM Zhao Liu <zhao1.liu@intel.com> wrote:
> > Hi Michael & Paolo,

> > 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.

Oh well.  I starred a few times at the new MMU_foo_IDXes in 
v8.2.0-1876-g90f641531c78
"target/i386: use separate MMU indexes for 32-bit accesses" a few times, it 
looked
somehow wrong, but it didn't occur to me what exactly was wrong there :)

> > 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.

Thank you, both of you, this is exactly the prob.  I fixed it by increasing
NB_MMU_NODES in "use separate MMU indexes for 32-bit accesses", since this
is the change which introduces the new indexes.

Now 7.2 work again, and appears to be under control, which is an excellent
news! :)

/mjt