* [PATCH kvm-unit-tests] access: check SMEP on prefetch pte path
@ 2011-06-24 7:16 Yang, Wei
2011-06-24 9:11 ` Xiao Guangrong
0 siblings, 1 reply; 4+ messages in thread
From: Yang, Wei @ 2011-06-24 7:16 UTC (permalink / raw)
To: avi; +Cc: kvm, xin.li, haitao.shan, Yang, Wei
This patch checks SMEP on prefetch pte path when
cr0.wp=1 and cr0.wp=0.
Signed-off-by: Yang, Wei <wei.y.yang@intel.com>
Signed-off-by: Li, Xin <xin.li@intel.com>
Signed-off-by: Shan, Haitao <haitao.shan@intel.com>
---
x86/access.c | 92 ++++++++++++++++++++++++++++++++++++++++++++++++++++++--
x86/cstart64.S | 1 +
2 files changed, 90 insertions(+), 3 deletions(-)
diff --git a/x86/access.c b/x86/access.c
index 7c8b9a5..71f04e0 100644
--- a/x86/access.c
+++ b/x86/access.c
@@ -27,6 +27,7 @@ typedef unsigned long pt_element_t;
#define PT_NX_MASK ((pt_element_t)1 << 63)
#define CR0_WP_MASK (1UL << 16)
+#define CR4_SMEP_MASK (1UL << 20)
#define PFERR_PRESENT_MASK (1U << 0)
#define PFERR_WRITE_MASK (1U << 1)
@@ -70,6 +71,7 @@ enum {
AC_CPU_EFER_NX,
AC_CPU_CR0_WP,
+ AC_CPU_CR4_SMEP,
NR_AC_FLAGS
};
@@ -140,6 +142,16 @@ void set_cr0_wp(int wp)
write_cr0(cr0);
}
+void set_cr4_smep(int smep)
+{
+ unsigned long cr4 = read_cr4();
+
+ cr4 &= ~CR4_SMEP_MASK;
+ if (smep)
+ cr4 |= CR4_SMEP_MASK;
+ write_cr4(cr4);
+}
+
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)
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;
}
@@ -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;
+ set_cr4_smep(at1.flags[AC_CPU_CR4_SMEP]);
+ err_smep = ac_test_do_access(&at1);
+ if (!err_smep) {
+ printf("%s: check SMEP on prefetch pte path with wp"
+ "fail\n", __FUNCTION__);
+ goto clean_up;
+ }
+
+ /*
+ * Here we write the ro user page when
+ * cr0.wp=0, then we execute it and SMEP
+ * fault should happen.
+ */
+ at1.flags[AC_ACCESS_WRITE] = 1;
+ at1.flags[AC_ACCESS_FETCH] = 0;
+ at1.flags[AC_CPU_CR0_WP] = 0;
+ ac_set_expected_status(&at1);
+ err_prepare_notwp = ac_test_do_access(&at1);
+ if (!err_prepare_notwp) {
+ printf("%s: SMEP prepare fail\n", __FUNCTION__);
+ goto clean_up;
+ }
+
+ at1.flags[AC_ACCESS_WRITE] = 0;
+ at1.flags[AC_ACCESS_FETCH] = 1;
+ ac_set_expected_status(&at1);
+ err_smep_notwp = ac_test_do_access(&at1);
+
+clean_up:
+ at1.flags[AC_CPU_CR4_SMEP] = 0;
+ set_cr4_smep(at1.flags[AC_CPU_CR4_SMEP]);
+ ptl2[2] += 0x4;
+
+ if (!err_smep || !err_prepare_notwp)
+ goto err;
+ if (!err_smep_notwp) {
+ printf("%s: check SMEP on prefetch pte path without wp"
+ "fail\n", __FUNCTION__);
+ __FUNCTION__);
+ goto err;
+ }
+ return 1;
+
+err:
+ return 0;
+}
+
int ac_test_exec(ac_test_t *at, ac_pool_t *pool)
{
int r;
@@ -662,6 +747,7 @@ const ac_test_fn ac_test_cases[] =
{
corrupt_hugepage_triger,
check_pfec_on_prefetch_pte,
+ check_smep_on_prefetch_pte
};
int ac_test_run(void)
diff --git a/x86/cstart64.S b/x86/cstart64.S
index 71014d8..24df5f8 100644
--- a/x86/cstart64.S
+++ b/x86/cstart64.S
@@ -21,6 +21,7 @@ ring0stacktop:
.data
.align 4096
+.globl ptl2
ptl2:
i = 0
.rept 512 * 4
--
1.7.4.1
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH kvm-unit-tests] access: check SMEP on prefetch pte path
2011-06-24 7:16 [PATCH kvm-unit-tests] access: check SMEP on prefetch pte path Yang, Wei
@ 2011-06-24 9:11 ` Xiao Guangrong
2011-06-25 2:44 ` Yang, Wei Y
0 siblings, 1 reply; 4+ messages in thread
From: Xiao Guangrong @ 2011-06-24 9:11 UTC (permalink / raw)
To: Yang, Wei; +Cc: avi, kvm, xin.li, haitao.shan
On 06/24/2011 03:16 PM, Yang, Wei wrote:
> +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?
> 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?
> 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()?
> @@ -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? :-)
^ permalink raw reply [flat|nested] 4+ messages in thread
* RE: [PATCH kvm-unit-tests] access: check SMEP on prefetch pte path
2011-06-24 9:11 ` Xiao Guangrong
@ 2011-06-25 2:44 ` Yang, Wei Y
2011-06-27 6:02 ` Xiao Guangrong
0 siblings, 1 reply; 4+ messages in thread
From: Yang, Wei Y @ 2011-06-25 2:44 UTC (permalink / raw)
To: Xiao Guangrong; +Cc: avi@redhat.com, kvm@vger.kernel.org, Li, Xin, Shan, Haitao
>
> > +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@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH kvm-unit-tests] access: check SMEP on prefetch pte path
2011-06-25 2:44 ` Yang, Wei Y
@ 2011-06-27 6:02 ` Xiao Guangrong
0 siblings, 0 replies; 4+ messages in thread
From: Xiao Guangrong @ 2011-06-27 6:02 UTC (permalink / raw)
To: Yang, Wei Y; +Cc: avi@redhat.com, kvm@vger.kernel.org, Li, Xin, Shan, Haitao
On 06/25/2011 10:44 AM, Yang, Wei Y wrote:
>
>>
>>> +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?
>
It can generate the wrong report that is not caused by the fault of KVM,
we'd better avoid it.
>>
>>> 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.
>
Oh, i see, thanks for your explanation, i think we'd better test SMEP for
all kinds of access, but i don't have strong opinion :-)
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2011-06-27 6:00 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2011-06-24 7:16 [PATCH kvm-unit-tests] access: check SMEP on prefetch pte path Yang, Wei
2011-06-24 9:11 ` Xiao Guangrong
2011-06-25 2:44 ` Yang, Wei Y
2011-06-27 6:02 ` Xiao Guangrong
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox