[PATCH 0/3] Fix cr0.wp=0 emulation

[PATCH 0/3] Fix cr0.wp=0 emulation

[Avi Kivity]

Xiao didn't find the nice cr0.wp=0 trick because it wasn't there, removed
accidentally by b70ccb0b3fd. Restore the trick and document it.

Avi Kivity (3):
  KVM: MMU: Allow spte.w=1 for gpte.w=0 and cr0.wp=0 only in shadow
    mode
  KVM: MMU: Remove user access when allowing kernel access to gpte.w=0
    page
  KVM: MMU: Document cr0.wp emulation

 Documentation/kvm/mmu.txt |   18 ++++++++++++++++++
 arch/x86/kvm/mmu.c        |    6 +++++-
 2 files changed, 23 insertions(+), 1 deletions(-)

[PATCH 1/3] KVM: MMU: Allow spte.w=1 for gpte.w=0 and cr0.wp=0 only in shadow mode

[Avi Kivity]

When tdp is enabled, the guest's cr0.wp shouldn't have any effect on spte
permissions.

Signed-off-by: Avi Kivity <avi@redhat.com>
---
 arch/x86/kvm/mmu.c |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/arch/x86/kvm/mmu.c b/arch/x86/kvm/mmu.c
index 136e160..39dd8d3 100644
--- a/arch/x86/kvm/mmu.c
+++ b/arch/x86/kvm/mmu.c
@@ -1881,7 +1881,8 @@ static int set_spte(struct kvm_vcpu *vcpu, u64 *sptep,
 	spte |= (u64)pfn << PAGE_SHIFT;
 
 	if ((pte_access & ACC_WRITE_MASK)
-	    || (write_fault && !is_write_protection(vcpu) && !user_fault)) {
+	    || (!tdp_enabled && write_fault && !is_write_protection(vcpu)
+		&& !user_fault)) {
 
 		if (level > PT_PAGE_TABLE_LEVEL &&
 		    has_wrprotected_page(vcpu->kvm, gfn, level)) {
-- 
1.7.1

[PATCH 2/3] KVM: MMU: Remove user access when allowing kernel access to gpte.w=0 page

[Avi Kivity]

If cr0.wp=0, we have to allow the guest kernel access to a page with pte.w=0.
We do that by setting spte.w=1, since the host cr0.wp must remain set so the
host can write protect pages.  Once we allow write access, we must remove
user access otherwise we mistakenly allow the user to write the page.

Signed-off-by: Avi Kivity <avi@redhat.com>
---
 arch/x86/kvm/mmu.c |    3 +++
 1 files changed, 3 insertions(+), 0 deletions(-)

diff --git a/arch/x86/kvm/mmu.c b/arch/x86/kvm/mmu.c
index 39dd8d3..56f8c3c 100644
--- a/arch/x86/kvm/mmu.c
+++ b/arch/x86/kvm/mmu.c
@@ -1894,6 +1894,9 @@ static int set_spte(struct kvm_vcpu *vcpu, u64 *sptep,
 
 		spte |= PT_WRITABLE_MASK;
 
+		if (!tdp_enabled && !(pte_access & ACC_WRITE_MASK))
+			spte &= ~PT_USER_MASK;
+
 		/*
 		 * Optimization: for pte sync, if spte was writable the hash
 		 * lookup is unnecessary (and expensive). Write protection
-- 
1.7.1

[PATCH 3/3] KVM: MMU: Document cr0.wp emulation

[Avi Kivity]

Signed-off-by: Avi Kivity <avi@redhat.com>
---
 Documentation/kvm/mmu.txt |   18 ++++++++++++++++++
 1 files changed, 18 insertions(+), 0 deletions(-)

diff --git a/Documentation/kvm/mmu.txt b/Documentation/kvm/mmu.txt
index 2201dcb..6528ad9 100644
--- a/Documentation/kvm/mmu.txt
+++ b/Documentation/kvm/mmu.txt
@@ -298,6 +298,24 @@ Host translation updates:
   - look up affected sptes through reverse map
   - drop (or update) translations
 
+Emulating cr0.wp
+================
+
+If tdp is not enabled, the host must keep cr0.wp=1 so page write protection
+works for the guest kernel, not guest guest userspace.  When the guest
+cr0.wp=1, this does not present a problem.  However when the guest cr0.wp=0,
+we cannot map the permissions for gpte.u=1, gpte.w=0 to any spte.
+
+We handle this my mapping the permissions to two possible gptes, depending
+on fault type:
+
+- kernel write fault: spte.u=0, spte.w=1 (allows full kernel access,
+  disallows user access)
+- read fault: spte.u=1, spte.w=0 (allows full read access, disallows kernel
+  write access)
+
+(user write faults generate a #PF)
+
 Further reading
 ===============
 
-- 
1.7.1

Re: [PATCH 2/3] KVM: MMU: Remove user access when allowing kernel access to gpte.w=0 page

[Xiao Guangrong]

Avi Kivity wrote:
> If cr0.wp=0, we have to allow the guest kernel access to a page with pte.w=0.
> We do that by setting spte.w=1, since the host cr0.wp must remain set so the
> host can write protect pages.  Once we allow write access, we must remove
> user access otherwise we mistakenly allow the user to write the page.
> 

Yeah, it's really a nice way :-)

Reviewed-by: Xiao Guangrong <xiaoguangrong@cn.fujitsu.com>

> Signed-off-by: Avi Kivity <avi@redhat.com>
> ---
>  arch/x86/kvm/mmu.c |    3 +++
>  1 files changed, 3 insertions(+), 0 deletions(-)
> 
> diff --git a/arch/x86/kvm/mmu.c b/arch/x86/kvm/mmu.c
> index 39dd8d3..56f8c3c 100644
> --- a/arch/x86/kvm/mmu.c
> +++ b/arch/x86/kvm/mmu.c
> @@ -1894,6 +1894,9 @@ static int set_spte(struct kvm_vcpu *vcpu, u64 *sptep,
>  
>  		spte |= PT_WRITABLE_MASK;
>  
> +		if (!tdp_enabled && !(pte_access & ACC_WRITE_MASK))
> +			spte &= ~PT_USER_MASK;
> +
>  		/*
>  		 * Optimization: for pte sync, if spte was writable the hash
>  		 * lookup is unnecessary (and expensive). Write protection