Mailing List Archive

[PATCH 08/11] nEPT: Use minimal permission for nested p2m.
From: Zhang Xiantao <xiantao.zhang@intel.com>

Emulate permission check for the nested p2m. Current solution is to
use minimal permission, and once meet permission violation in L0, then
determin whether it is caused by guest EPT or host EPT

Signed-off-by: Zhang Xiantao <xiantao.zhang@intel.com>
---
xen/arch/x86/hvm/svm/nestedsvm.c | 2 +-
xen/arch/x86/hvm/vmx/vvmx.c | 4 ++--
xen/arch/x86/mm/hap/nested_ept.c | 9 +++++----
xen/arch/x86/mm/hap/nested_hap.c | 22 +++++++++++++---------
xen/include/asm-x86/hvm/hvm.h | 2 +-
xen/include/asm-x86/hvm/svm/nestedsvm.h | 2 +-
xen/include/asm-x86/hvm/vmx/vvmx.h | 6 +++---
7 files changed, 26 insertions(+), 21 deletions(-)

diff --git a/xen/arch/x86/hvm/svm/nestedsvm.c b/xen/arch/x86/hvm/svm/nestedsvm.c
index 5dcb354..ab455a9 100644
--- a/xen/arch/x86/hvm/svm/nestedsvm.c
+++ b/xen/arch/x86/hvm/svm/nestedsvm.c
@@ -1177,7 +1177,7 @@ nsvm_vmcb_hap_enabled(struct vcpu *v)
*/
int
nsvm_hap_walk_L1_p2m(struct vcpu *v, paddr_t L2_gpa, paddr_t *L1_gpa,
- unsigned int *page_order,
+ unsigned int *page_order, uint8_t *p2m_acc,
bool_t access_r, bool_t access_w, bool_t access_x)
{
uint32_t pfec;
diff --git a/xen/arch/x86/hvm/vmx/vvmx.c b/xen/arch/x86/hvm/vmx/vvmx.c
index 3fc128b..41779bc 100644
--- a/xen/arch/x86/hvm/vmx/vvmx.c
+++ b/xen/arch/x86/hvm/vmx/vvmx.c
@@ -1494,7 +1494,7 @@ int nvmx_msr_write_intercept(unsigned int msr, u64 msr_content)
*/
int
nvmx_hap_walk_L1_p2m(struct vcpu *v, paddr_t L2_gpa, paddr_t *L1_gpa,
- unsigned int *page_order,
+ unsigned int *page_order, uint8_t *p2m_acc,
bool_t access_r, bool_t access_w, bool_t access_x)
{
uint64_t exit_qual = __vmread(EXIT_QUALIFICATION);
@@ -1504,7 +1504,7 @@ nvmx_hap_walk_L1_p2m(struct vcpu *v, paddr_t L2_gpa, paddr_t *L1_gpa,
uint32_t rwx_rights = (access_x << 2) | (access_w << 1) | access_r;
struct nestedvmx *nvmx = &vcpu_2_nvmx(v);

- rc = nept_translate_l2ga(v, L2_gpa, page_order, rwx_rights, &gfn,
+ rc = nept_translate_l2ga(v, L2_gpa, page_order, rwx_rights, &gfn, p2m_acc,
&exit_qual, &exit_reason);
switch ( rc ) {
case EPT_TRANSLATE_SUCCEED:
diff --git a/xen/arch/x86/mm/hap/nested_ept.c b/xen/arch/x86/mm/hap/nested_ept.c
index 2d733a8..637db1a 100644
--- a/xen/arch/x86/mm/hap/nested_ept.c
+++ b/xen/arch/x86/mm/hap/nested_ept.c
@@ -286,8 +286,8 @@ bool_t nept_permission_check(uint32_t rwx_acc, uint32_t rwx_bits)

int nept_translate_l2ga(struct vcpu *v, paddr_t l2ga,
unsigned int *page_order, uint32_t rwx_acc,
- unsigned long *l1gfn, uint64_t *exit_qual,
- uint32_t *exit_reason)
+ unsigned long *l1gfn, uint8_t *p2m_acc,
+ uint64_t *exit_qual, uint32_t *exit_reason)
{
uint32_t rc, rwx_bits = 0;
walk_t gw;
@@ -317,8 +317,9 @@ int nept_translate_l2ga(struct vcpu *v, paddr_t l2ga,
}
if ( nept_permission_check(rwx_acc, rwx_bits) )
{
- *l1gfn = guest_l1e_get_paddr(gw.l1e) >> PAGE_SHIFT;
- break;
+ *l1gfn = guest_l1e_get_paddr(gw.l1e) >> PAGE_SHIFT;
+ *p2m_acc = (uint8_t)rwx_bits;
+ break;
}
rc = EPT_TRANSLATE_VIOLATION;
/* Fall through to EPT violation if permission check fails. */
diff --git a/xen/arch/x86/mm/hap/nested_hap.c b/xen/arch/x86/mm/hap/nested_hap.c
index 6d1264b..9c1654d 100644
--- a/xen/arch/x86/mm/hap/nested_hap.c
+++ b/xen/arch/x86/mm/hap/nested_hap.c
@@ -142,12 +142,12 @@ nestedhap_fix_p2m(struct vcpu *v, struct p2m_domain *p2m,
*/
static int
nestedhap_walk_L1_p2m(struct vcpu *v, paddr_t L2_gpa, paddr_t *L1_gpa,
- unsigned int *page_order,
+ unsigned int *page_order, uint8_t *p2m_acc,
bool_t access_r, bool_t access_w, bool_t access_x)
{
ASSERT(hvm_funcs.nhvm_hap_walk_L1_p2m);

- return hvm_funcs.nhvm_hap_walk_L1_p2m(v, L2_gpa, L1_gpa, page_order,
+ return hvm_funcs.nhvm_hap_walk_L1_p2m(v, L2_gpa, L1_gpa, page_order, p2m_acc,
access_r, access_w, access_x);
}

@@ -158,16 +158,15 @@ nestedhap_walk_L1_p2m(struct vcpu *v, paddr_t L2_gpa, paddr_t *L1_gpa,
*/
static int
nestedhap_walk_L0_p2m(struct p2m_domain *p2m, paddr_t L1_gpa, paddr_t *L0_gpa,
- p2m_type_t *p2mt,
+ p2m_type_t *p2mt, p2m_access_t *p2ma,
unsigned int *page_order,
bool_t access_r, bool_t access_w, bool_t access_x)
{
mfn_t mfn;
- p2m_access_t p2ma;
int rc;

/* walk L0 P2M table */
- mfn = get_gfn_type_access(p2m, L1_gpa >> PAGE_SHIFT, p2mt, &p2ma,
+ mfn = get_gfn_type_access(p2m, L1_gpa >> PAGE_SHIFT, p2mt, p2ma,
0, page_order);

rc = NESTEDHVM_PAGEFAULT_MMIO;
@@ -206,12 +205,14 @@ nestedhvm_hap_nested_page_fault(struct vcpu *v, paddr_t *L2_gpa,
struct p2m_domain *p2m, *nested_p2m;
unsigned int page_order_21, page_order_10, page_order_20;
p2m_type_t p2mt_10;
+ p2m_access_t p2ma_10;
+ uint8_t p2ma_21;

p2m = p2m_get_hostp2m(d); /* L0 p2m */
nested_p2m = p2m_get_nestedp2m(v, nhvm_vcpu_p2m_base(v));

/* walk the L1 P2M table */
- rv = nestedhap_walk_L1_p2m(v, *L2_gpa, &L1_gpa, &page_order_21,
+ rv = nestedhap_walk_L1_p2m(v, *L2_gpa, &L1_gpa, &page_order_21, &p2ma_21,
access_r, access_w, access_x);

/* let caller to handle these two cases */
@@ -229,7 +230,7 @@ nestedhvm_hap_nested_page_fault(struct vcpu *v, paddr_t *L2_gpa,

/* ==> we have to walk L0 P2M */
rv = nestedhap_walk_L0_p2m(p2m, L1_gpa, &L0_gpa,
- &p2mt_10, &page_order_10,
+ &p2mt_10, &p2ma_10, &page_order_10,
access_r, access_w, access_x);

/* let upper level caller to handle these two cases */
@@ -250,10 +251,13 @@ nestedhvm_hap_nested_page_fault(struct vcpu *v, paddr_t *L2_gpa,

page_order_20 = min(page_order_21, page_order_10);

+ if (p2ma_10 > p2m_access_rwx)
+ p2ma_10 = p2m_access_rwx;
+ p2ma_10 &= (p2m_access_t)p2ma_21; /* Use minimal permission for nested p2m. */
+
/* fix p2m_get_pagetable(nested_p2m) */
nestedhap_fix_p2m(v, nested_p2m, *L2_gpa, L0_gpa, page_order_20,
- p2mt_10,
- p2m_access_rwx /* FIXME: Should use minimum permission. */);
+ p2mt_10, p2ma_10);

return NESTEDHVM_PAGEFAULT_DONE;
}
diff --git a/xen/include/asm-x86/hvm/hvm.h b/xen/include/asm-x86/hvm/hvm.h
index 80f07e9..889e3c9 100644
--- a/xen/include/asm-x86/hvm/hvm.h
+++ b/xen/include/asm-x86/hvm/hvm.h
@@ -186,7 +186,7 @@ struct hvm_function_table {

/*Walk nested p2m */
int (*nhvm_hap_walk_L1_p2m)(struct vcpu *v, paddr_t L2_gpa, paddr_t *L1_gpa,
- unsigned int *page_order,
+ unsigned int *page_order, uint8_t *p2m_acc,
bool_t access_r, bool_t access_w, bool_t access_x);
};

diff --git a/xen/include/asm-x86/hvm/svm/nestedsvm.h b/xen/include/asm-x86/hvm/svm/nestedsvm.h
index 0c90f30..748cc04 100644
--- a/xen/include/asm-x86/hvm/svm/nestedsvm.h
+++ b/xen/include/asm-x86/hvm/svm/nestedsvm.h
@@ -134,7 +134,7 @@ void svm_vmexit_do_clgi(struct cpu_user_regs *regs, struct vcpu *v);
void svm_vmexit_do_stgi(struct cpu_user_regs *regs, struct vcpu *v);
bool_t nestedsvm_gif_isset(struct vcpu *v);
int nsvm_hap_walk_L1_p2m(struct vcpu *v, paddr_t L2_gpa, paddr_t *L1_gpa,
- unsigned int *page_order,
+ unsigned int *page_order, uint8_t *p2m_acc,
bool_t access_r, bool_t access_w, bool_t access_x);

#define NSVM_INTR_NOTHANDLED 3
diff --git a/xen/include/asm-x86/hvm/vmx/vvmx.h b/xen/include/asm-x86/hvm/vmx/vvmx.h
index 661cd8a..55c0ad1 100644
--- a/xen/include/asm-x86/hvm/vmx/vvmx.h
+++ b/xen/include/asm-x86/hvm/vmx/vvmx.h
@@ -124,7 +124,7 @@ int nvmx_handle_vmxoff(struct cpu_user_regs *regs);

int
nvmx_hap_walk_L1_p2m(struct vcpu *v, paddr_t L2_gpa, paddr_t *L1_gpa,
- unsigned int *page_order,
+ unsigned int *page_order, uint8_t *p2m_acc,
bool_t access_r, bool_t access_w, bool_t access_x);
/*
* Virtual VMCS layout
@@ -207,7 +207,7 @@ int nvmx_n2_vmexit_handler(struct cpu_user_regs *regs,

int nept_translate_l2ga(struct vcpu *v, paddr_t l2ga,
unsigned int *page_order, uint32_t rwx_acc,
- unsigned long *l1gfn, uint64_t *exit_qual,
- uint32_t *exit_reason);
+ unsigned long *l1gfn, uint8_t *p2m_acc,
+ uint64_t *exit_qual, uint32_t *exit_reason);
#endif /* __ASM_X86_HVM_VVMX_H__ */

--
1.7.1


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
Re: [PATCH 08/11] nEPT: Use minimal permission for nested p2m. [ In reply to ]
At 01:57 +0800 on 11 Dec (1355191040), xiantao.zhang@intel.com wrote:
> From: Zhang Xiantao <xiantao.zhang@intel.com>
>
> Emulate permission check for the nested p2m. Current solution is to
> use minimal permission, and once meet permission violation in L0, then
> determin whether it is caused by guest EPT or host EPT
>
> Signed-off-by: Zhang Xiantao <xiantao.zhang@intel.com>

> --- a/xen/arch/x86/hvm/svm/nestedsvm.c
> +++ b/xen/arch/x86/hvm/svm/nestedsvm.c
> @@ -1177,7 +1177,7 @@ nsvm_vmcb_hap_enabled(struct vcpu *v)
> */
> int
> nsvm_hap_walk_L1_p2m(struct vcpu *v, paddr_t L2_gpa, paddr_t *L1_gpa,
> - unsigned int *page_order,
> + unsigned int *page_order, uint8_t *p2m_acc,
> bool_t access_r, bool_t access_w, bool_t access_x)

I don't like these interface changes (see below) but if we do have them,
at least make the SVM version use p2m_access_rwx, to match the old
behaviour, rather than letting it use an uninitialised stack variable. :)

> @@ -250,10 +251,13 @@ nestedhvm_hap_nested_page_fault(struct vcpu *v, paddr_t *L2_gpa,
>
> page_order_20 = min(page_order_21, page_order_10);
>
> + if (p2ma_10 > p2m_access_rwx)
> + p2ma_10 = p2m_access_rwx;

That's plain wrong. If the access type is p2m_access_rx2rw, this will
give the guest write access to what ought to be a read-only page.

I think it would be best to leave the p2m-access stuff to the p2m
walkers, and not add all those extra p2ma arguments. Instead, just use
the _actual_ access permissions of this fault as the p2ma. That way
you know you have something that's acceptabel to both p2m tables.

I guess that will mean some extra faults on read-then-write behaviour.
If those are measurable, we could look at pulling the p2m-access types
out like this, but you'll have to explicitly handle all the special
types.

Cheers,

Tim.

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel