On Mon, Apr 12, 2021, Yang Weijiang wrote:
> On Fri, Apr 09, 2021 at 06:59:45PM +0000, Sean Christopherson wrote:
> > On Fri, Apr 09, 2021, Paolo Bonzini wrote:
> > > On 09/04/21 09:55, Yang Weijiang wrote:
> > > > During kvm-unit-test, below failure pattern is observed, this is due to testing thread
> > > > migration + cache "lazy" flush during test, so forcely flush the cache to avoid the issue.
> > > > Pin the test app to certain physical CPU can fix the issue as well. The error report is
> > > > misleading, pke is the victim of the issue.
> > > >
> > > > test user cr4.pke: FAIL: error code 5 expected 4
> > > > Dump mapping: address: 0x123400000000
> > > > ------L4: 21ea007
> > > > ------L3: 21eb007
> > > > ------L2: 21ec000
> > > > ------L1: 2000000
> > > >
> > > > Signed-off-by: Yang Weijiang <weijiang.yang@xxxxxxxxx>
> > > > ---
> > > > x86/access.c | 2 ++
> > > > 1 file changed, 2 insertions(+)
> > > >
> > > > diff --git a/x86/access.c b/x86/access.c
> > > > index 7dc9eb6..379d533 100644
> > > > --- a/x86/access.c
> > > > +++ b/x86/access.c
> > > > @@ -211,6 +211,8 @@ static unsigned set_cr4_smep(int smep)
> > > > ptl2[2] |= PT_USER_MASK;
> > > > if (!r)
> > > > shadow_cr4 = cr4;
> > > > +
> > > > + invlpg((void *)(ptl2[2] & ~PAGE_SIZE));
> > > > return r;
> > > > }
> > > >
> > >
> > > Applied, thanks.
> >
> > Egad, I can't keep up with this new Paolo :-D
> >
> >
> > Would it also work to move the existing invlpg() into ac_test_do_access()?
> >
> Hi, Sean,
> You patch works for the app on my side, but one thing makes my confused, my patch
> invalidates the mapping for test code(ac_test_do_access), but your patch invlidates
> at->virt, they're not mapped to the same page. Why it works?
I don't know why your patch works. Best guess is that INVLPG on the PMD is
causing the CPU to flush the entire TLB, i.e. the problematic entry is collateral
damage.
> I simplified the test by only executing two patterns as below:
>
> printf("\n############# start test ############\n\n");
> at.flags = 0x8000000;
> ac_test_exec(&at, &pool);
> at.flags = 0x200000; /* or 0x10200000 */
> ac_test_exec(&at, &pool);
> printf("############# end test ############\n\n");
>
> with your patch I still got error code 5 while getting error code 4 with my patch.
> What makes it different?
Now I'm really confused. This:
at.flags = 0x8000000;
ac_test_exec(&at, &pool);
runs the test with a not-present PTE. I don't understand how you are getting
error code '5' (USER + PRESENT) when running the user test; there shouldn't be
anything for at->virt in the TLB.
Are there tests being run before this point? Even then, explicitly flushing
at->virt should work. The fact that set_cr4_smep() modifies a PMD and not the
leaf PTE should be irrelevant. Per the SDM:
INVLPG also invalidates all entries in all paging-structure caches associated
with the current PCID, regardless of the linear addresses to which they correspond.
