All of lore.kernel.org
 help / color / mirror / Atom feed
From: Huaitong Han <huaitong.han@intel.com>
To: jbeulich@suse.com, andrew.cooper3@citrix.com,
	george.dunlap@eu.citrix.com, tim@xen.org, keir@xen.org
Cc: Huaitong Han <huaitong.han@intel.com>,
	George Dunlap <george.dunlap@citrix.com>,
	xen-devel@lists.xen.org
Subject: [PATCH V8 4/5] xen/mm: Clean up pfec handling in gva_to_gfn
Date: Tue,  2 Feb 2016 15:35:41 +0800	[thread overview]
Message-ID: <1454398542-4815-5-git-send-email-huaitong.han@intel.com> (raw)
In-Reply-To: <1454398542-4815-1-git-send-email-huaitong.han@intel.com>

From: George Dunlap <george.dunlap@citrix.com>

At the moment, the pfec argument to gva_to_gfn has two functions:

* To inform guest_walk what kind of access is happenind

* As a value to pass back into the guest in the event of a fault.

Unfortunately this is not quite treated consistently: the hvm_fetch_*
function will "pre-clear" the PFEC_insn_fetch flag before calling
gva_to_gfn; meaning guest_walk doesn't actually know whether a given
access is an instruction fetch or not.  This works now, but will cause
issues when pkeys are introduced, since guest_walk will need to know
whether an access is an instruction fetch even if it doesn't return
PFEC_insn_fetch.

Fix this by making a clean separation for in and out functionalities
of the pfec argument:

1. Always pass in the access type to gva_to_gfn

2. Filter out inappropriate access flags before returning from gva_to_gfn.

(The PFEC_insn_fetch flag should only be passed to the guest if either NX or
SMEP is enabled.  See Intel 64 Developer's Manual, Volume 3, Chapter Paging,
PAGE-FAULT EXCEPTIONS)

Signed-off-by: George Dunlap <george.dunlap@citrix.com>
Signed-off-by: Huaitong Han <huaitong.han@intel.com>
Acked-by: Jan Beulich <jbeulich@suse.com>
---
Changes in v8:
*Add the comment describing for paging_gva_to_gfn.

Changes in v7:
*Update SDM chapter comments.
*Add hvm_vcpu check in sh_gva_to_gfn.

 xen/arch/x86/hvm/hvm.c           |  8 ++------
 xen/arch/x86/mm/hap/guest_walk.c | 10 +++++++++-
 xen/arch/x86/mm/shadow/multi.c   |  6 ++++++
 xen/include/asm-x86/paging.h     |  6 +++++-
 4 files changed, 22 insertions(+), 8 deletions(-)

diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c
index 674feea..5ec2ae1 100644
--- a/xen/arch/x86/hvm/hvm.c
+++ b/xen/arch/x86/hvm/hvm.c
@@ -4438,11 +4438,9 @@ enum hvm_copy_result hvm_copy_from_guest_virt(
 enum hvm_copy_result hvm_fetch_from_guest_virt(
     void *buf, unsigned long vaddr, int size, uint32_t pfec)
 {
-    if ( hvm_nx_enabled(current) || hvm_smep_enabled(current) )
-        pfec |= PFEC_insn_fetch;
     return __hvm_copy(buf, vaddr, size,
                       HVMCOPY_from_guest | HVMCOPY_fault | HVMCOPY_virt,
-                      PFEC_page_present | pfec);
+                      PFEC_page_present | PFEC_insn_fetch | pfec);
 }
 
 enum hvm_copy_result hvm_copy_to_guest_virt_nofault(
@@ -4464,11 +4462,9 @@ enum hvm_copy_result hvm_copy_from_guest_virt_nofault(
 enum hvm_copy_result hvm_fetch_from_guest_virt_nofault(
     void *buf, unsigned long vaddr, int size, uint32_t pfec)
 {
-    if ( hvm_nx_enabled(current) || hvm_smep_enabled(current) )
-        pfec |= PFEC_insn_fetch;
     return __hvm_copy(buf, vaddr, size,
                       HVMCOPY_from_guest | HVMCOPY_no_fault | HVMCOPY_virt,
-                      PFEC_page_present | pfec);
+                      PFEC_page_present | PFEC_insn_fetch | pfec);
 }
 
 unsigned long copy_to_user_hvm(void *to, const void *from, unsigned int len)
diff --git a/xen/arch/x86/mm/hap/guest_walk.c b/xen/arch/x86/mm/hap/guest_walk.c
index 49d0328..d2716f9 100644
--- a/xen/arch/x86/mm/hap/guest_walk.c
+++ b/xen/arch/x86/mm/hap/guest_walk.c
@@ -82,7 +82,7 @@ unsigned long hap_p2m_ga_to_gfn(GUEST_PAGING_LEVELS)(
     if ( !top_page )
     {
         pfec[0] &= ~PFEC_page_present;
-        return INVALID_GFN;
+        goto out_tweak_pfec;
     }
     top_mfn = _mfn(page_to_mfn(top_page));
 
@@ -139,6 +139,14 @@ unsigned long hap_p2m_ga_to_gfn(GUEST_PAGING_LEVELS)(
     if ( missing & _PAGE_SHARED )
         pfec[0] = PFEC_page_shared;
 
+ out_tweak_pfec:
+    /*
+     * SDM Intel 64 Volume 3, Chapter Paging, PAGE-FAULT EXCEPTIONS:
+     * The PFEC_insn_fetch flag is set only when NX or SMEP are enabled.
+     */
+    if ( !hvm_nx_enabled(v) && !hvm_smep_enabled(v) )
+        pfec[0] &= ~PFEC_insn_fetch;
+
     return INVALID_GFN;
 }
 
diff --git a/xen/arch/x86/mm/shadow/multi.c b/xen/arch/x86/mm/shadow/multi.c
index 162c06f..d42597c 100644
--- a/xen/arch/x86/mm/shadow/multi.c
+++ b/xen/arch/x86/mm/shadow/multi.c
@@ -3669,6 +3669,12 @@ sh_gva_to_gfn(struct vcpu *v, struct p2m_domain *p2m,
             pfec[0] &= ~PFEC_page_present;
         if ( missing & _PAGE_INVALID_BITS )
             pfec[0] |= PFEC_reserved_bit;
+        /*
+         * SDM Intel 64 Volume 3, Chapter Paging, PAGE-FAULT EXCEPTIONS:
+         * The PFEC_insn_fetch flag is set only when NX or SMEP are enabled.
+         */
+        if ( is_hvm_vcpu(v) && !hvm_nx_enabled(v) && !hvm_smep_enabled(v) )
+            pfec[0] &= ~PFEC_insn_fetch;
         return INVALID_GFN;
     }
     gfn = guest_walk_to_gfn(&gw);
diff --git a/xen/include/asm-x86/paging.h b/xen/include/asm-x86/paging.h
index 9a8653d..195fe8f 100644
--- a/xen/include/asm-x86/paging.h
+++ b/xen/include/asm-x86/paging.h
@@ -255,7 +255,11 @@ static inline bool_t paging_invlpg(struct vcpu *v, unsigned long va)
  * tables don't map this address for this kind of access.
  * pfec[0] is used to determine which kind of access this is when
  * walking the tables.  The caller should set the PFEC_page_present bit
- * in pfec[0]; in the failure case, that bit will be cleared if appropriate. */
+ * in pfec[0]; in the failure case, that bit will be cleared if appropriate.
+ *
+ * SDM Intel 64 Volume 3, Chapter Paging, PAGE-FAULT EXCEPTIONS:
+ * The PFEC_insn_fetch flag is set only when NX or SMEP are enabled.
+ */
 unsigned long paging_gva_to_gfn(struct vcpu *v,
                                 unsigned long va,
                                 uint32_t *pfec);
-- 
2.4.3

  parent reply	other threads:[~2016-02-02  7:35 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-02-02  7:35 [PATCH V8 0/5] x86/hvm: pkeys, add memory protection-key support Huaitong Han
2016-02-02  7:35 ` [PATCH V8 1/5] x86/hvm: pkeys, disable pkeys for guests in non-paging mode Huaitong Han
2016-02-02  7:35 ` [PATCH V8 2/5] x86/hvm: pkeys, add pkeys support for guest_walk_tables Huaitong Han
2016-02-03  9:43   ` Jan Beulich
2016-02-03 10:05     ` Han, Huaitong
2016-02-02  7:35 ` [PATCH V8 3/5] x86/hvm: pkeys, add xstate support for pkeys Huaitong Han
2016-02-02  7:35 ` Huaitong Han [this message]
2016-02-02  7:35 ` [PATCH V8 5/5] x86/hvm: pkeys, add pkeys support for cpuid handling Huaitong Han
2016-02-03  9:50   ` Jan Beulich
2016-02-03 10:04     ` Han, Huaitong
2016-02-03 11:05       ` Jan Beulich

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1454398542-4815-5-git-send-email-huaitong.han@intel.com \
    --to=huaitong.han@intel.com \
    --cc=andrew.cooper3@citrix.com \
    --cc=george.dunlap@citrix.com \
    --cc=george.dunlap@eu.citrix.com \
    --cc=jbeulich@suse.com \
    --cc=keir@xen.org \
    --cc=tim@xen.org \
    --cc=xen-devel@lists.xen.org \
    --subject='Re: [PATCH V8 4/5] xen/mm: Clean up pfec handling in gva_to_gfn' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.