> > > +void set_cr4_smep(int smep) > > +{ > > + unsigned long cr4 = read_cr4(); > > + > > + cr4 &= ~CR4_SMEP_MASK; > > + if (smep) > > + cr4 |= CR4_SMEP_MASK; > > + write_cr4(cr4); > > +} > > + > > It can work if the box does not support SMEP? It will report unhandled exception 13 in access.out which we count as errors either. Do we need verify it before running the smep test case? > > > void set_efer_nx(int nx) > > { > > unsigned long long efer; > > @@ -176,7 +188,7 @@ void ac_test_init(ac_test_t *at, void *virt) > > > > int ac_test_bump_one(ac_test_t *at) > > { > > - for (int i = 0; i < NR_AC_FLAGS; ++i) > > + for (int i = 0; i < NR_AC_FLAGS-1; ++i) > > Why not test "SMEP" for all test case? It's actually the last question. :) The page where the current code stays is user page. If enabled SMEP, we must add some code to convert the current page into kernel page, or the page fault will occur again and again. And it seems no need to test SMEP for all test case. We just need fetch access which we do in a separate case. > > > if (!at->flags[i]) { > > at->flags[i] = 1; > > return 1; > > @@ -287,6 +299,9 @@ void ac_set_expected_status(ac_test_t *at) > > if (at->flags[AC_PDE_PSE]) { > > if (at->flags[AC_ACCESS_WRITE] && !at->expected_fault) > > at->expected_pde |= PT_DIRTY_MASK; > > + if (at->flags[AC_ACCESS_FETCH] && at->flags[AC_PDE_USER] > > + && at->flags[AC_CPU_CR4_SMEP]) > > + at->expected_fault = 1; > > goto no_pte; > > } > > > > @@ -306,7 +321,11 @@ void ac_set_expected_status(ac_test_t *at) > > && (at->flags[AC_CPU_CR0_WP] || at->flags[AC_ACCESS_USER])) > > at->expected_fault = 1; > > > > - if (at->flags[AC_ACCESS_FETCH] && at->flags[AC_PTE_NX]) > > + if (at->flags[AC_ACCESS_FETCH] > > + && (at->flags[AC_PTE_NX] > > + || (at->flags[AC_CPU_CR4_SMEP] > > + && at->flags[AC_PDE_USER] > > + && at->flags[AC_PTE_USER]))) > > at->expected_fault = 1; > > > > if (at->expected_fault) > > @@ -320,7 +339,7 @@ no_pte: > > fault: > > if (!at->expected_fault) > > at->ignore_pde = 0; > > - if (!at->flags[AC_CPU_EFER_NX]) > > + if (!at->flags[AC_CPU_EFER_NX] && !at->flags[AC_CPU_CR4_SMEP]) > > at->expected_error &= ~PFERR_FETCH_MASK; > > } > > > > You check the AC_CPU_CR4_SMEP for all case, but only set the cr4 bit for > check_smep_on_prefetch_pte(), it is better to move set_cr4_smep to > ac_test_do_access()? Yes, will do. > > > @@ -645,6 +664,72 @@ err: > > return 0; > > } > > > > +static int check_smep_on_prefetch_pte(ac_pool_t *pool) > > +{ > > + ac_test_t at1; > > + int err_smep, err_prepare_notwp, err_smep_notwp; > > + extern u64 ptl2[]; > > + > > + ac_test_init(&at1, (void *)(0x123406001000)); > > + > > + at1.flags[AC_PDE_PRESENT] = 1; > > + at1.flags[AC_PTE_PRESENT] = 1; > > + at1.flags[AC_PDE_USER] = 1; > > + at1.flags[AC_PTE_USER] = 1; > > + at1.flags[AC_PDE_ACCESSED] = 1; > > + at1.flags[AC_PTE_ACCESSED] = 1; > > + at1.flags[AC_ACCESS_FETCH] = 1; > > + at1.flags[AC_CPU_CR4_SMEP] = 1; > > + at1.flags[AC_CPU_CR0_WP] = 1; > > + ac_test_setup_pte(&at1, pool); > > + ptl2[2] -= 0x4; > > Why is it needed? :-) Reply above. :) > -- > To unsubscribe from this list: send the line "unsubscribe kvm" in > the body of a message to majordomo@xxxxxxxxxxxxxxx > More majordomo info at http://vger.kernel.org/majordomo-info.html ��.n��������+%������w��{.n�����o�^n�r������&��z�ޗ�zf���h���~����������_��+v���)ߣ�