RE: [PATCH kvm-unit-tests] access: check SMEP on prefetch pte path

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



> 
> > +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���)ߣ�

[Index of Archives]     [KVM ARM]     [KVM ia64]     [KVM ppc]     [Virtualization Tools]     [Spice Development]     [Libvirt]     [Libvirt Users]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite Questions]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux