[PATCH 1/9] KVM: iommu: fix releasing unmapped page

[PATCH 1/9] KVM: iommu: fix releasing unmapped page

[Xiao Guangrong]

There are two bugs:
- the 'error page' is forgot to be released
  [ it is unneeded after commit a2766325cf9f9, for backport, we
    still do kvm_release_pfn_clean for the error pfn ]

- guest pages are always released regardless of the unmapped page
  (e,g, caused by hwpoison)

Signed-off-by: Xiao Guangrong <xiaoguangrong@linux.vnet.ibm.com>
---
 virt/kvm/iommu.c |    7 +++++++
 1 files changed, 7 insertions(+), 0 deletions(-)

diff --git a/virt/kvm/iommu.c b/virt/kvm/iommu.c
index c03f1fb..6a67bea 100644
--- a/virt/kvm/iommu.c
+++ b/virt/kvm/iommu.c
@@ -107,6 +107,7 @@ int kvm_iommu_map_pages(struct kvm *kvm, struct kvm_memory_slot *slot)
 		 */
 		pfn = kvm_pin_pages(slot, gfn, page_size);
 		if (is_error_pfn(pfn)) {
+			kvm_release_pfn_clean(pfn);
 			gfn += 1;
 			continue;
 		}
@@ -300,6 +301,12 @@ static void kvm_iommu_put_pages(struct kvm *kvm,

 		/* Get physical address */
 		phys = iommu_iova_to_phys(domain, gfn_to_gpa(gfn));
+
+		if (!phys) {
+			gfn++;
+			continue;
+		}
+
 		pfn  = phys >> PAGE_SHIFT;

 		/* Unmap address from IO address space */
-- 
1.7.7.6

[PATCH 2/9] KVM: define kvm_fault_pfn statically

[Xiao Guangrong]

After that, the exported and un-inline function, get_fault_pfn,
can be removed

Signed-off-by: Xiao Guangrong <xiaoguangrong@linux.vnet.ibm.com>
---
 arch/x86/kvm/mmu.c       |    2 +-
 include/linux/kvm_host.h |    3 ++-
 virt/kvm/kvm_main.c      |   12 +++---------
 3 files changed, 6 insertions(+), 11 deletions(-)

diff --git a/arch/x86/kvm/mmu.c b/arch/x86/kvm/mmu.c
index a9a2052..19bac91 100644
--- a/arch/x86/kvm/mmu.c
+++ b/arch/x86/kvm/mmu.c
@@ -2514,7 +2514,7 @@ static pfn_t pte_prefetch_gfn_to_pfn(struct kvm_vcpu *vcpu, gfn_t gfn,

 	slot = gfn_to_memslot_dirty_bitmap(vcpu, gfn, no_dirty_log);
 	if (!slot)
-		return get_fault_pfn();
+		return kvm_fault_pfn;

 	hva = gfn_to_hva_memslot(slot, gfn);

diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h
index dbc65f9..7cd6871 100644
--- a/include/linux/kvm_host.h
+++ b/include/linux/kvm_host.h
@@ -48,6 +48,8 @@
 #define KVM_MAX_MMIO_FRAGMENTS \
 	(KVM_MMIO_SIZE / KVM_USER_MMIO_SIZE + KVM_EXTRA_MMIO_FRAGMENTS)

+#define kvm_fault_pfn	(-EFAULT)
+
 /*
  * vcpu->requests bit members
  */
@@ -444,7 +446,6 @@ void kvm_release_pfn_clean(pfn_t pfn);
 void kvm_set_pfn_dirty(pfn_t pfn);
 void kvm_set_pfn_accessed(pfn_t pfn);
 void kvm_get_pfn(pfn_t pfn);
-pfn_t get_fault_pfn(void);

 int kvm_read_guest_page(struct kvm *kvm, gfn_t gfn, void *data, int offset,
 			int len);
diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
index bcf973e..2117aa8 100644
--- a/virt/kvm/kvm_main.c
+++ b/virt/kvm/kvm_main.c
@@ -948,12 +948,6 @@ static pfn_t get_bad_pfn(void)
 	return -ENOENT;
 }

-pfn_t get_fault_pfn(void)
-{
-	return -EFAULT;
-}
-EXPORT_SYMBOL_GPL(get_fault_pfn);
-
 static pfn_t get_hwpoison_pfn(void)
 {
 	return -EHWPOISON;
@@ -1124,7 +1118,7 @@ static pfn_t hva_to_pfn(unsigned long addr, bool atomic, bool *async,
 		struct vm_area_struct *vma;

 		if (atomic)
-			return get_fault_pfn();
+			return kvm_fault_pfn;

 		down_read(&current->mm->mmap_sem);
 		if (npages == -EHWPOISON ||
@@ -1136,7 +1130,7 @@ static pfn_t hva_to_pfn(unsigned long addr, bool atomic, bool *async,
 		vma = find_vma_intersection(current->mm, addr, addr+1);

 		if (vma == NULL)
-			pfn = get_fault_pfn();
+			pfn = kvm_fault_pfn;
 		else if ((vma->vm_flags & VM_PFNMAP)) {
 			pfn = ((addr - vma->vm_start) >> PAGE_SHIFT) +
 				vma->vm_pgoff;
@@ -1144,7 +1138,7 @@ static pfn_t hva_to_pfn(unsigned long addr, bool atomic, bool *async,
 		} else {
 			if (async && (vma->vm_flags & VM_WRITE))
 				*async = true;
-			pfn = get_fault_pfn();
+			pfn = kvm_fault_pfn;
 		}
 		up_read(&current->mm->mmap_sem);
 	} else
-- 
1.7.7.6

Re: [PATCH 2/9] KVM: define kvm_fault_pfn statically

[Marcelo Tosatti]

On Sun, Jul 29, 2012 at 04:12:58PM +0800, Xiao Guangrong wrote:
> After that, the exported and un-inline function, get_fault_pfn,
> can be removed
> 
> Signed-off-by: Xiao Guangrong <xiaoguangrong@linux.vnet.ibm.com>
> ---
>  arch/x86/kvm/mmu.c       |    2 +-
>  include/linux/kvm_host.h |    3 ++-
>  virt/kvm/kvm_main.c      |   12 +++---------
>  3 files changed, 6 insertions(+), 11 deletions(-)
> 
> diff --git a/arch/x86/kvm/mmu.c b/arch/x86/kvm/mmu.c
> index a9a2052..19bac91 100644
> --- a/arch/x86/kvm/mmu.c
> +++ b/arch/x86/kvm/mmu.c
> @@ -2514,7 +2514,7 @@ static pfn_t pte_prefetch_gfn_to_pfn(struct kvm_vcpu *vcpu, gfn_t gfn,
> 
>  	slot = gfn_to_memslot_dirty_bitmap(vcpu, gfn, no_dirty_log);
>  	if (!slot)
> -		return get_fault_pfn();
> +		return kvm_fault_pfn;
> 
>  	hva = gfn_to_hva_memslot(slot, gfn);
> 
> diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h
> index dbc65f9..7cd6871 100644
> --- a/include/linux/kvm_host.h
> +++ b/include/linux/kvm_host.h
> @@ -48,6 +48,8 @@
>  #define KVM_MAX_MMIO_FRAGMENTS \
>  	(KVM_MMIO_SIZE / KVM_USER_MMIO_SIZE + KVM_EXTRA_MMIO_FRAGMENTS)
> 
> +#define kvm_fault_pfn	(-EFAULT)
> +

This is confusing, it appears to be a variable not a definition. 
Use "#define KVM_FAULT_PFN_ERR" please.

[PATCH 3/9] KVM: define kvm_hwpoison_pfn statically

[Xiao Guangrong]

Then, get_hwpoison_pfn and is_hwpoison_pfn can be removed

Signed-off-by: Xiao Guangrong <xiaoguangrong@linux.vnet.ibm.com>
---
 arch/x86/kvm/mmu.c       |    2 +-
 include/linux/kvm_host.h |    4 ++--
 virt/kvm/kvm_main.c      |   13 +------------
 3 files changed, 4 insertions(+), 15 deletions(-)

diff --git a/arch/x86/kvm/mmu.c b/arch/x86/kvm/mmu.c
index 19bac91..320a781 100644
--- a/arch/x86/kvm/mmu.c
+++ b/arch/x86/kvm/mmu.c
@@ -2651,7 +2651,7 @@ static void kvm_send_hwpoison_signal(unsigned long address, struct task_struct *
 static int kvm_handle_bad_page(struct kvm_vcpu *vcpu, gfn_t gfn, pfn_t pfn)
 {
 	kvm_release_pfn_clean(pfn);
-	if (is_hwpoison_pfn(pfn)) {
+	if (pfn == kvm_hwpoison_pfn) {
 		kvm_send_hwpoison_signal(gfn_to_hva(vcpu->kvm, gfn), current);
 		return 0;
 	}
diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h
index 7cd6871..4de56cd 100644
--- a/include/linux/kvm_host.h
+++ b/include/linux/kvm_host.h
@@ -48,7 +48,8 @@
 #define KVM_MAX_MMIO_FRAGMENTS \
 	(KVM_MMIO_SIZE / KVM_USER_MMIO_SIZE + KVM_EXTRA_MMIO_FRAGMENTS)

-#define kvm_fault_pfn	(-EFAULT)
+#define kvm_fault_pfn		(-EFAULT)
+#define kvm_hwpoison_pfn	(-EHWPOISON)

 /*
  * vcpu->requests bit members
@@ -396,7 +397,6 @@ extern struct page *bad_page;

 int is_error_page(struct page *page);
 int is_error_pfn(pfn_t pfn);
-int is_hwpoison_pfn(pfn_t pfn);
 int is_noslot_pfn(pfn_t pfn);
 int is_invalid_pfn(pfn_t pfn);
 int kvm_is_error_hva(unsigned long addr);
diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
index 2117aa8..390a03d 100644
--- a/virt/kvm/kvm_main.c
+++ b/virt/kvm/kvm_main.c
@@ -948,17 +948,6 @@ static pfn_t get_bad_pfn(void)
 	return -ENOENT;
 }

-static pfn_t get_hwpoison_pfn(void)
-{
-	return -EHWPOISON;
-}
-
-int is_hwpoison_pfn(pfn_t pfn)
-{
-	return pfn == -EHWPOISON;
-}
-EXPORT_SYMBOL_GPL(is_hwpoison_pfn);
-
 int is_noslot_pfn(pfn_t pfn)
 {
 	return pfn == -ENOENT;
@@ -1124,7 +1113,7 @@ static pfn_t hva_to_pfn(unsigned long addr, bool atomic, bool *async,
 		if (npages == -EHWPOISON ||
 			(!async && check_user_page_hwpoison(addr))) {
 			up_read(&current->mm->mmap_sem);
-			return get_hwpoison_pfn();
+			return kvm_hwpoison_pfn;
 		}

 		vma = find_vma_intersection(current->mm, addr, addr+1);
-- 
1.7.7.6

Re: [PATCH 3/9] KVM: define kvm_hwpoison_pfn statically

[Marcelo Tosatti]

On Sun, Jul 29, 2012 at 04:13:41PM +0800, Xiao Guangrong wrote:
> Then, get_hwpoison_pfn and is_hwpoison_pfn can be removed
> 
> Signed-off-by: Xiao Guangrong <xiaoguangrong@linux.vnet.ibm.com>
> ---
>  arch/x86/kvm/mmu.c       |    2 +-
>  include/linux/kvm_host.h |    4 ++--
>  virt/kvm/kvm_main.c      |   13 +------------
>  3 files changed, 4 insertions(+), 15 deletions(-)
> 
> diff --git a/arch/x86/kvm/mmu.c b/arch/x86/kvm/mmu.c
> index 19bac91..320a781 100644
> --- a/arch/x86/kvm/mmu.c
> +++ b/arch/x86/kvm/mmu.c
> @@ -2651,7 +2651,7 @@ static void kvm_send_hwpoison_signal(unsigned long address, struct task_struct *
>  static int kvm_handle_bad_page(struct kvm_vcpu *vcpu, gfn_t gfn, pfn_t pfn)
>  {
>  	kvm_release_pfn_clean(pfn);
> -	if (is_hwpoison_pfn(pfn)) {
> +	if (pfn == kvm_hwpoison_pfn) {
>  		kvm_send_hwpoison_signal(gfn_to_hva(vcpu->kvm, gfn), current);
>  		return 0;
>  	}
> diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h
> index 7cd6871..4de56cd 100644
> --- a/include/linux/kvm_host.h
> +++ b/include/linux/kvm_host.h
> @@ -48,7 +48,8 @@
>  #define KVM_MAX_MMIO_FRAGMENTS \
>  	(KVM_MMIO_SIZE / KVM_USER_MMIO_SIZE + KVM_EXTRA_MMIO_FRAGMENTS)
> 
> -#define kvm_fault_pfn	(-EFAULT)
> +#define kvm_fault_pfn		(-EFAULT)
> +#define kvm_hwpoison_pfn	(-EHWPOISON)

Same here as kvm_fault_pfn.

[PATCH 4/9] KVM: define kvm_bad_pfn statically

[Xiao Guangrong]

Then, remove get_bad_pfn

Signed-off-by: Xiao Guangrong <xiaoguangrong@linux.vnet.ibm.com>
---
 include/linux/kvm_host.h |    1 +
 virt/kvm/kvm_main.c      |    7 +------
 2 files changed, 2 insertions(+), 6 deletions(-)

diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h
index 4de56cd..b02203f 100644
--- a/include/linux/kvm_host.h
+++ b/include/linux/kvm_host.h
@@ -50,6 +50,7 @@

 #define kvm_fault_pfn		(-EFAULT)
 #define kvm_hwpoison_pfn	(-EHWPOISON)
+#define kvm_bad_pfn		(-ENOENT)

 /*
  * vcpu->requests bit members
diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
index 390a03d..da16191 100644
--- a/virt/kvm/kvm_main.c
+++ b/virt/kvm/kvm_main.c
@@ -943,11 +943,6 @@ int is_error_pfn(pfn_t pfn)
 }
 EXPORT_SYMBOL_GPL(is_error_pfn);

-static pfn_t get_bad_pfn(void)
-{
-	return -ENOENT;
-}
-
 int is_noslot_pfn(pfn_t pfn)
 {
 	return pfn == -ENOENT;
@@ -1152,7 +1147,7 @@ static pfn_t __gfn_to_pfn(struct kvm *kvm, gfn_t gfn, bool atomic, bool *async,

 	addr = gfn_to_hva(kvm, gfn);
 	if (kvm_is_error_hva(addr))
-		return get_bad_pfn();
+		return kvm_bad_pfn;

 	return hva_to_pfn(addr, atomic, async, write_fault, writable);
 }
-- 
1.7.7.6

Re: [PATCH 4/9] KVM: define kvm_bad_pfn statically

[Marcelo Tosatti]

On Sun, Jul 29, 2012 at 04:14:25PM +0800, Xiao Guangrong wrote:
> Then, remove get_bad_pfn
> 
> Signed-off-by: Xiao Guangrong <xiaoguangrong@linux.vnet.ibm.com>
> ---
>  include/linux/kvm_host.h |    1 +
>  virt/kvm/kvm_main.c      |    7 +------
>  2 files changed, 2 insertions(+), 6 deletions(-)
> 
> diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h
> index 4de56cd..b02203f 100644
> --- a/include/linux/kvm_host.h
> +++ b/include/linux/kvm_host.h
> @@ -50,6 +50,7 @@
> 
>  #define kvm_fault_pfn		(-EFAULT)
>  #define kvm_hwpoison_pfn	(-EHWPOISON)
> +#define kvm_bad_pfn		(-ENOENT)

Remind me what is the guarantee that -Exxx does not clash with
a valid pfn number?

Re: [PATCH 4/9] KVM: define kvm_bad_pfn statically

[Paul Mackerras]

On Thu, Aug 02, 2012 at 10:15:27AM -0300, Marcelo Tosatti wrote:

> Remind me what is the guarantee that -Exxx does not clash with
> a valid pfn number?

A pfn number is an address >> PAGE_SHIFT, so it will have the top 12
(at least) bits clear, whereas -Exxx will have the top bit set.

Paul.

Re: [PATCH 4/9] KVM: define kvm_bad_pfn statically

[Xiao Guangrong]

Marcelo, Paul,

Thanks for your review!

On 08/03/2012 08:01 AM, Paul Mackerras wrote:
> On Thu, Aug 02, 2012 at 10:15:27AM -0300, Marcelo Tosatti wrote:
> 
>> Remind me what is the guarantee that -Exxx does not clash with
>> a valid pfn number?
> 
> A pfn number is an address >> PAGE_SHIFT, so it will have the top 12
> (at least) bits clear, whereas -Exxx will have the top bit set.
> 

Yes.

As this way is hard to understand and it will break huge memory support
on PAE 32bit cpu, i have used a new way in the v2:

http://marc.info/?l=linux-kernel&m=134398012027025&w=2

Please review the new version.

[PATCH 5/9] KVM: inline is_*_pfn functions

[Xiao Guangrong]

These functions are exported and can not inline, move them
to kvm_host.h to eliminate the overload of function call

Signed-off-by: Xiao Guangrong <xiaoguangrong@linux.vnet.ibm.com>
---
 include/linux/kvm_host.h |   19 ++++++++++++++++---
 virt/kvm/kvm_main.c      |   18 ------------------
 2 files changed, 16 insertions(+), 21 deletions(-)

diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h
index b02203f..98255ce 100644
--- a/include/linux/kvm_host.h
+++ b/include/linux/kvm_host.h
@@ -21,6 +21,7 @@
 #include <linux/slab.h>
 #include <linux/rcupdate.h>
 #include <linux/ratelimit.h>
+#include <linux/err.h>
 #include <asm/signal.h>

 #include <linux/kvm.h>
@@ -52,6 +53,21 @@
 #define kvm_hwpoison_pfn	(-EHWPOISON)
 #define kvm_bad_pfn		(-ENOENT)

+static inline int is_error_pfn(pfn_t pfn)
+{
+	return IS_ERR_VALUE(pfn);
+}
+
+static inline int is_noslot_pfn(pfn_t pfn)
+{
+	return pfn == -ENOENT;
+}
+
+static inline int is_invalid_pfn(pfn_t pfn)
+{
+	return !is_noslot_pfn(pfn) && is_error_pfn(pfn);
+}
+
 /*
  * vcpu->requests bit members
  */
@@ -397,9 +413,6 @@ id_to_memslot(struct kvm_memslots *slots, int id)
 extern struct page *bad_page;

 int is_error_page(struct page *page);
-int is_error_pfn(pfn_t pfn);
-int is_noslot_pfn(pfn_t pfn);
-int is_invalid_pfn(pfn_t pfn);
 int kvm_is_error_hva(unsigned long addr);
 int kvm_set_memory_region(struct kvm *kvm,
 			  struct kvm_userspace_memory_region *mem,
diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
index da16191..51aaba4 100644
--- a/virt/kvm/kvm_main.c
+++ b/virt/kvm/kvm_main.c
@@ -937,24 +937,6 @@ int is_error_page(struct page *page)
 }
 EXPORT_SYMBOL_GPL(is_error_page);

-int is_error_pfn(pfn_t pfn)
-{
-	return IS_ERR_VALUE(pfn);
-}
-EXPORT_SYMBOL_GPL(is_error_pfn);
-
-int is_noslot_pfn(pfn_t pfn)
-{
-	return pfn == -ENOENT;
-}
-EXPORT_SYMBOL_GPL(is_noslot_pfn);
-
-int is_invalid_pfn(pfn_t pfn)
-{
-	return !is_noslot_pfn(pfn) && is_error_pfn(pfn);
-}
-EXPORT_SYMBOL_GPL(is_invalid_pfn);
-
 struct page *get_bad_page(void)
 {
 	return ERR_PTR(-ENOENT);
-- 
1.7.7.6

[PATCH 6/9] KVM: remove the unused declare

[Xiao Guangrong]

Remove it since it is not used anymore

Signed-off-by: Xiao Guangrong <xiaoguangrong@linux.vnet.ibm.com>
---
 include/linux/kvm_host.h |    2 --
 1 files changed, 0 insertions(+), 2 deletions(-)

diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h
index 98255ce..387ecc5 100644
--- a/include/linux/kvm_host.h
+++ b/include/linux/kvm_host.h
@@ -410,8 +410,6 @@ id_to_memslot(struct kvm_memslots *slots, int id)
 	return slot;
 }

-extern struct page *bad_page;
-
 int is_error_page(struct page *page);
 int kvm_is_error_hva(unsigned long addr);
 int kvm_set_memory_region(struct kvm *kvm,
-- 
1.7.7.6

[PATCH 7/9] KVM: define kvm_bad_page statically

[Xiao Guangrong]

It is used to eliminate the overload of function call and cleanup
the code

Signed-off-by: Xiao Guangrong <xiaoguangrong@linux.vnet.ibm.com>
---
 include/linux/kvm_host.h |    9 +++++++--
 virt/kvm/async_pf.c      |    2 +-
 virt/kvm/kvm_main.c      |   13 +------------
 3 files changed, 9 insertions(+), 15 deletions(-)

diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h
index 387ecc5..08a9da9 100644
--- a/include/linux/kvm_host.h
+++ b/include/linux/kvm_host.h
@@ -68,6 +68,13 @@ static inline int is_invalid_pfn(pfn_t pfn)
 	return !is_noslot_pfn(pfn) && is_error_pfn(pfn);
 }

+#define kvm_bad_page	(ERR_PTR(-ENOENT))
+
+static inline int is_error_page(struct page *page)
+{
+	return IS_ERR(page);
+}
+
 /*
  * vcpu->requests bit members
  */
@@ -410,7 +417,6 @@ id_to_memslot(struct kvm_memslots *slots, int id)
 	return slot;
 }

-int is_error_page(struct page *page);
 int kvm_is_error_hva(unsigned long addr);
 int kvm_set_memory_region(struct kvm *kvm,
 			  struct kvm_userspace_memory_region *mem,
@@ -437,7 +443,6 @@ void kvm_arch_flush_shadow(struct kvm *kvm);
 int gfn_to_page_many_atomic(struct kvm *kvm, gfn_t gfn, struct page **pages,
 			    int nr_pages);

-struct page *get_bad_page(void);
 struct page *gfn_to_page(struct kvm *kvm, gfn_t gfn);
 unsigned long gfn_to_hva(struct kvm *kvm, gfn_t gfn);
 void kvm_release_page_clean(struct page *page);
diff --git a/virt/kvm/async_pf.c b/virt/kvm/async_pf.c
index 7972278..aa38af6 100644
--- a/virt/kvm/async_pf.c
+++ b/virt/kvm/async_pf.c
@@ -203,7 +203,7 @@ int kvm_async_pf_wakeup_all(struct kvm_vcpu *vcpu)
 	if (!work)
 		return -ENOMEM;

-	work->page = get_bad_page();
+	work->page = kvm_bad_page;
 	INIT_LIST_HEAD(&work->queue); /* for list_del to work */

 	spin_lock(&vcpu->async_pf.lock);
diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
index 51aaba4..f09f48a 100644
--- a/virt/kvm/kvm_main.c
+++ b/virt/kvm/kvm_main.c
@@ -931,17 +931,6 @@ void kvm_disable_largepages(void)
 }
 EXPORT_SYMBOL_GPL(kvm_disable_largepages);

-int is_error_page(struct page *page)
-{
-	return IS_ERR(page);
-}
-EXPORT_SYMBOL_GPL(is_error_page);
-
-struct page *get_bad_page(void)
-{
-	return ERR_PTR(-ENOENT);
-}
-
 static inline unsigned long bad_hva(void)
 {
 	return PAGE_OFFSET;
@@ -1188,7 +1177,7 @@ static struct page *kvm_pfn_to_page(pfn_t pfn)
 	WARN_ON(kvm_is_mmio_pfn(pfn));

 	if (is_error_pfn(pfn) || kvm_is_mmio_pfn(pfn))
-		return get_bad_page();
+		return kvm_bad_page;

 	return pfn_to_page(pfn);
 }
-- 
1.7.7.6

Re: [PATCH 7/9] KVM: define kvm_bad_page statically

[Marcelo Tosatti]

On Sun, Jul 29, 2012 at 04:16:28PM +0800, Xiao Guangrong wrote:
> It is used to eliminate the overload of function call and cleanup
> the code
> 
> Signed-off-by: Xiao Guangrong <xiaoguangrong@linux.vnet.ibm.com>
> ---
>  include/linux/kvm_host.h |    9 +++++++--
>  virt/kvm/async_pf.c      |    2 +-
>  virt/kvm/kvm_main.c      |   13 +------------
>  3 files changed, 9 insertions(+), 15 deletions(-)
> 
> diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h
> index 387ecc5..08a9da9 100644
> --- a/include/linux/kvm_host.h
> +++ b/include/linux/kvm_host.h
> @@ -68,6 +68,13 @@ static inline int is_invalid_pfn(pfn_t pfn)
>  	return !is_noslot_pfn(pfn) && is_error_pfn(pfn);
>  }
> 
> +#define kvm_bad_page	(ERR_PTR(-ENOENT))

KVM_ERR_PTR_BAD_PAGE ...

[PATCH 8/9] KVM: do not release the error pfn

[Xiao Guangrong]

After commit a2766325cf9f9, the error pfn is replaced by the
error code, it need not be released anymore

[ The patch is compiling tested for powerpc ]

Signed-off-by: Xiao Guangrong <xiaoguangrong@linux.vnet.ibm.com>
---
 arch/powerpc/kvm/e500_tlb.c |    1 -
 arch/x86/kvm/mmu.c          |    6 +++---
 arch/x86/kvm/mmu_audit.c    |    4 +---
 arch/x86/kvm/paging_tmpl.h  |    8 ++------
 virt/kvm/iommu.c            |    1 -
 virt/kvm/kvm_main.c         |   14 ++++++++------
 6 files changed, 14 insertions(+), 20 deletions(-)

diff --git a/arch/powerpc/kvm/e500_tlb.c b/arch/powerpc/kvm/e500_tlb.c
index c8f6c58..09ce5ac 100644
--- a/arch/powerpc/kvm/e500_tlb.c
+++ b/arch/powerpc/kvm/e500_tlb.c
@@ -524,7 +524,6 @@ static inline void kvmppc_e500_shadow_map(struct kvmppc_vcpu_e500 *vcpu_e500,
 		if (is_error_pfn(pfn)) {
 			printk(KERN_ERR "Couldn't get real page for gfn %lx!\n",
 					(long)gfn);
-			kvm_release_pfn_clean(pfn);
 			return;
 		}

diff --git a/arch/x86/kvm/mmu.c b/arch/x86/kvm/mmu.c
index 320a781..949a5b8 100644
--- a/arch/x86/kvm/mmu.c
+++ b/arch/x86/kvm/mmu.c
@@ -2498,7 +2498,9 @@ static void mmu_set_spte(struct kvm_vcpu *vcpu, u64 *sptep,
 				rmap_recycle(vcpu, sptep, gfn);
 		}
 	}
-	kvm_release_pfn_clean(pfn);
+
+	if (!is_error_pfn(pfn))
+		kvm_release_pfn_clean(pfn);
 }

 static void nonpaging_new_cr3(struct kvm_vcpu *vcpu)
@@ -3275,8 +3277,6 @@ static bool try_async_pf(struct kvm_vcpu *vcpu, bool prefault, gfn_t gfn,
 	if (!async)
 		return false; /* *pfn has correct page already */

-	kvm_release_pfn_clean(*pfn);
-
 	if (!prefault && can_do_async_pf(vcpu)) {
 		trace_kvm_try_async_get_page(gva, gfn);
 		if (kvm_find_async_pf_gfn(vcpu, gfn)) {
diff --git a/arch/x86/kvm/mmu_audit.c b/arch/x86/kvm/mmu_audit.c
index 7d7d0b9..bac5fa4 100644
--- a/arch/x86/kvm/mmu_audit.c
+++ b/arch/x86/kvm/mmu_audit.c
@@ -116,10 +116,8 @@ static void audit_mappings(struct kvm_vcpu *vcpu, u64 *sptep, int level)
 	gfn = kvm_mmu_page_get_gfn(sp, sptep - sp->spt);
 	pfn = gfn_to_pfn_atomic(vcpu->kvm, gfn);

-	if (is_error_pfn(pfn)) {
-		kvm_release_pfn_clean(pfn);
+	if (is_error_pfn(pfn))
 		return;
-	}

 	hpa =  pfn << PAGE_SHIFT;
 	if ((*sptep & PT64_BASE_ADDR_MASK) != hpa)
diff --git a/arch/x86/kvm/paging_tmpl.h b/arch/x86/kvm/paging_tmpl.h
index bb7cf01..bf8c42b 100644
--- a/arch/x86/kvm/paging_tmpl.h
+++ b/arch/x86/kvm/paging_tmpl.h
@@ -370,10 +370,8 @@ static void FNAME(update_pte)(struct kvm_vcpu *vcpu, struct kvm_mmu_page *sp,
 	pgprintk("%s: gpte %llx spte %p\n", __func__, (u64)gpte, spte);
 	pte_access = sp->role.access & FNAME(gpte_access)(vcpu, gpte, true);
 	pfn = gfn_to_pfn_atomic(vcpu->kvm, gpte_to_gfn(gpte));
-	if (mmu_invalid_pfn(pfn)) {
-		kvm_release_pfn_clean(pfn);
+	if (mmu_invalid_pfn(pfn))
 		return;
-	}

 	/*
 	 * we call mmu_set_spte() with host_writable = true because that
@@ -448,10 +446,8 @@ static void FNAME(pte_prefetch)(struct kvm_vcpu *vcpu, struct guest_walker *gw,
 		gfn = gpte_to_gfn(gpte);
 		pfn = pte_prefetch_gfn_to_pfn(vcpu, gfn,
 				      pte_access & ACC_WRITE_MASK);
-		if (mmu_invalid_pfn(pfn)) {
-			kvm_release_pfn_clean(pfn);
+		if (mmu_invalid_pfn(pfn))
 			break;
-		}

 		mmu_set_spte(vcpu, spte, sp->role.access, pte_access, 0, 0,
 			     NULL, PT_PAGE_TABLE_LEVEL, gfn,
diff --git a/virt/kvm/iommu.c b/virt/kvm/iommu.c
index 6a67bea..037cb67 100644
--- a/virt/kvm/iommu.c
+++ b/virt/kvm/iommu.c
@@ -107,7 +107,6 @@ int kvm_iommu_map_pages(struct kvm *kvm, struct kvm_memory_slot *slot)
 		 */
 		pfn = kvm_pin_pages(slot, gfn, page_size);
 		if (is_error_pfn(pfn)) {
-			kvm_release_pfn_clean(pfn);
 			gfn += 1;
 			continue;
 		}
diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
index f09f48a..0c29714 100644
--- a/virt/kvm/kvm_main.c
+++ b/virt/kvm/kvm_main.c
@@ -102,9 +102,6 @@ static bool largepages_enabled = true;

 bool kvm_is_mmio_pfn(pfn_t pfn)
 {
-	if (is_error_pfn(pfn))
-		return false;
-
 	if (pfn_valid(pfn)) {
 		int reserved;
 		struct page *tail = pfn_to_page(pfn);
@@ -1174,10 +1171,13 @@ EXPORT_SYMBOL_GPL(gfn_to_page_many_atomic);

 static struct page *kvm_pfn_to_page(pfn_t pfn)
 {
-	WARN_ON(kvm_is_mmio_pfn(pfn));
+	if (is_error_pfn(pfn))
+		return kvm_bad_page;

-	if (is_error_pfn(pfn) || kvm_is_mmio_pfn(pfn))
+	if (kvm_is_mmio_pfn(pfn)) {
+		WARN_ON(1);
 		return kvm_bad_page;
+	}

 	return pfn_to_page(pfn);
 }
@@ -1202,7 +1202,9 @@ EXPORT_SYMBOL_GPL(kvm_release_page_clean);

 void kvm_release_pfn_clean(pfn_t pfn)
 {
-	if (!is_error_pfn(pfn) && !kvm_is_mmio_pfn(pfn))
+	WARN_ON(is_error_pfn(pfn));
+
+	if (!kvm_is_mmio_pfn(pfn))
 		put_page(pfn_to_page(pfn));
 }
 EXPORT_SYMBOL_GPL(kvm_release_pfn_clean);
-- 
1.7.7.6

Re: [PATCH 8/9] KVM: do not release the error pfn

[Xiao Guangrong]

kvm_release_pfn_clean in kvm_handle_bad_page() also can be removed, please
review this one instead.

Changelog:
   remove kvm_release_pfn_clean in kvm_handle_bad_page()


From: Xiao Guangrong <xiaoguangrong@linux.vnet.ibm.com>
Subject: [PATCH 08/21] KVM: do not release the error pfn

After commit a2766325cf9f9, the error pfn is replaced by the
error code, it need not be released anymore

[ The patch is compiling tested for powerpc ]

Signed-off-by: Xiao Guangrong <xiaoguangrong@linux.vnet.ibm.com>
---
 arch/powerpc/kvm/e500_tlb.c |    1 -
 arch/x86/kvm/mmu.c          |    7 +++----
 arch/x86/kvm/mmu_audit.c    |    4 +---
 arch/x86/kvm/paging_tmpl.h  |    8 ++------
 virt/kvm/iommu.c            |    1 -
 virt/kvm/kvm_main.c         |   14 ++++++++------
 6 files changed, 14 insertions(+), 21 deletions(-)

diff --git a/arch/powerpc/kvm/e500_tlb.c b/arch/powerpc/kvm/e500_tlb.c
index c8f6c58..09ce5ac 100644
--- a/arch/powerpc/kvm/e500_tlb.c
+++ b/arch/powerpc/kvm/e500_tlb.c
@@ -524,7 +524,6 @@ static inline void kvmppc_e500_shadow_map(struct kvmppc_vcpu_e500 *vcpu_e500,
 		if (is_error_pfn(pfn)) {
 			printk(KERN_ERR "Couldn't get real page for gfn %lx!\n",
 					(long)gfn);
-			kvm_release_pfn_clean(pfn);
 			return;
 		}

diff --git a/arch/x86/kvm/mmu.c b/arch/x86/kvm/mmu.c
index 320a781..d9a73d8 100644
--- a/arch/x86/kvm/mmu.c
+++ b/arch/x86/kvm/mmu.c
@@ -2498,7 +2498,9 @@ static void mmu_set_spte(struct kvm_vcpu *vcpu, u64 *sptep,
 				rmap_recycle(vcpu, sptep, gfn);
 		}
 	}
-	kvm_release_pfn_clean(pfn);
+
+	if (!is_error_pfn(pfn))
+		kvm_release_pfn_clean(pfn);
 }

 static void nonpaging_new_cr3(struct kvm_vcpu *vcpu)
@@ -2650,7 +2652,6 @@ static void kvm_send_hwpoison_signal(unsigned long address, struct task_struct *

 static int kvm_handle_bad_page(struct kvm_vcpu *vcpu, gfn_t gfn, pfn_t pfn)
 {
-	kvm_release_pfn_clean(pfn);
 	if (pfn == kvm_hwpoison_pfn) {
 		kvm_send_hwpoison_signal(gfn_to_hva(vcpu->kvm, gfn), current);
 		return 0;
@@ -3275,8 +3276,6 @@ static bool try_async_pf(struct kvm_vcpu *vcpu, bool prefault, gfn_t gfn,
 	if (!async)
 		return false; /* *pfn has correct page already */

-	kvm_release_pfn_clean(*pfn);
-
 	if (!prefault && can_do_async_pf(vcpu)) {
 		trace_kvm_try_async_get_page(gva, gfn);
 		if (kvm_find_async_pf_gfn(vcpu, gfn)) {
diff --git a/arch/x86/kvm/mmu_audit.c b/arch/x86/kvm/mmu_audit.c
index 7d7d0b9..bac5fa4 100644
--- a/arch/x86/kvm/mmu_audit.c
+++ b/arch/x86/kvm/mmu_audit.c
@@ -116,10 +116,8 @@ static void audit_mappings(struct kvm_vcpu *vcpu, u64 *sptep, int level)
 	gfn = kvm_mmu_page_get_gfn(sp, sptep - sp->spt);
 	pfn = gfn_to_pfn_atomic(vcpu->kvm, gfn);

-	if (is_error_pfn(pfn)) {
-		kvm_release_pfn_clean(pfn);
+	if (is_error_pfn(pfn))
 		return;
-	}

 	hpa =  pfn << PAGE_SHIFT;
 	if ((*sptep & PT64_BASE_ADDR_MASK) != hpa)
diff --git a/arch/x86/kvm/paging_tmpl.h b/arch/x86/kvm/paging_tmpl.h
index bb7cf01..bf8c42b 100644
--- a/arch/x86/kvm/paging_tmpl.h
+++ b/arch/x86/kvm/paging_tmpl.h
@@ -370,10 +370,8 @@ static void FNAME(update_pte)(struct kvm_vcpu *vcpu, struct kvm_mmu_page *sp,
 	pgprintk("%s: gpte %llx spte %p\n", __func__, (u64)gpte, spte);
 	pte_access = sp->role.access & FNAME(gpte_access)(vcpu, gpte, true);
 	pfn = gfn_to_pfn_atomic(vcpu->kvm, gpte_to_gfn(gpte));
-	if (mmu_invalid_pfn(pfn)) {
-		kvm_release_pfn_clean(pfn);
+	if (mmu_invalid_pfn(pfn))
 		return;
-	}

 	/*
 	 * we call mmu_set_spte() with host_writable = true because that
@@ -448,10 +446,8 @@ static void FNAME(pte_prefetch)(struct kvm_vcpu *vcpu, struct guest_walker *gw,
 		gfn = gpte_to_gfn(gpte);
 		pfn = pte_prefetch_gfn_to_pfn(vcpu, gfn,
 				      pte_access & ACC_WRITE_MASK);
-		if (mmu_invalid_pfn(pfn)) {
-			kvm_release_pfn_clean(pfn);
+		if (mmu_invalid_pfn(pfn))
 			break;
-		}

 		mmu_set_spte(vcpu, spte, sp->role.access, pte_access, 0, 0,
 			     NULL, PT_PAGE_TABLE_LEVEL, gfn,
diff --git a/virt/kvm/iommu.c b/virt/kvm/iommu.c
index 6a67bea..037cb67 100644
--- a/virt/kvm/iommu.c
+++ b/virt/kvm/iommu.c
@@ -107,7 +107,6 @@ int kvm_iommu_map_pages(struct kvm *kvm, struct kvm_memory_slot *slot)
 		 */
 		pfn = kvm_pin_pages(slot, gfn, page_size);
 		if (is_error_pfn(pfn)) {
-			kvm_release_pfn_clean(pfn);
 			gfn += 1;
 			continue;
 		}
diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
index f09f48a..0c29714 100644
--- a/virt/kvm/kvm_main.c
+++ b/virt/kvm/kvm_main.c
@@ -102,9 +102,6 @@ static bool largepages_enabled = true;

 bool kvm_is_mmio_pfn(pfn_t pfn)
 {
-	if (is_error_pfn(pfn))
-		return false;
-
 	if (pfn_valid(pfn)) {
 		int reserved;
 		struct page *tail = pfn_to_page(pfn);
@@ -1174,10 +1171,13 @@ EXPORT_SYMBOL_GPL(gfn_to_page_many_atomic);

 static struct page *kvm_pfn_to_page(pfn_t pfn)
 {
-	WARN_ON(kvm_is_mmio_pfn(pfn));
+	if (is_error_pfn(pfn))
+		return kvm_bad_page;

-	if (is_error_pfn(pfn) || kvm_is_mmio_pfn(pfn))
+	if (kvm_is_mmio_pfn(pfn)) {
+		WARN_ON(1);
 		return kvm_bad_page;
+	}

 	return pfn_to_page(pfn);
 }
@@ -1202,7 +1202,9 @@ EXPORT_SYMBOL_GPL(kvm_release_page_clean);

 void kvm_release_pfn_clean(pfn_t pfn)
 {
-	if (!is_error_pfn(pfn) && !kvm_is_mmio_pfn(pfn))
+	WARN_ON(is_error_pfn(pfn));
+
+	if (!kvm_is_mmio_pfn(pfn))
 		put_page(pfn_to_page(pfn));
 }
 EXPORT_SYMBOL_GPL(kvm_release_pfn_clean);
-- 
1.7.7.6

Re: [PATCH 8/9] KVM: do not release the error pfn

[Marcelo Tosatti]

On Sun, Jul 29, 2012 at 04:18:58PM +0800, Xiao Guangrong wrote:
> After commit a2766325cf9f9, the error pfn is replaced by the
> error code, it need not be released anymore
> 
> [ The patch is compiling tested for powerpc ]
> 
> Signed-off-by: Xiao Guangrong <xiaoguangrong@linux.vnet.ibm.com>
> ---
>  arch/powerpc/kvm/e500_tlb.c |    1 -
>  arch/x86/kvm/mmu.c          |    6 +++---
>  arch/x86/kvm/mmu_audit.c    |    4 +---
>  arch/x86/kvm/paging_tmpl.h  |    8 ++------
>  virt/kvm/iommu.c            |    1 -
>  virt/kvm/kvm_main.c         |   14 ++++++++------
>  6 files changed, 14 insertions(+), 20 deletions(-)
> 
> diff --git a/arch/powerpc/kvm/e500_tlb.c b/arch/powerpc/kvm/e500_tlb.c
> index c8f6c58..09ce5ac 100644
> --- a/arch/powerpc/kvm/e500_tlb.c
> +++ b/arch/powerpc/kvm/e500_tlb.c
> @@ -524,7 +524,6 @@ static inline void kvmppc_e500_shadow_map(struct kvmppc_vcpu_e500 *vcpu_e500,
>  		if (is_error_pfn(pfn)) {
>  			printk(KERN_ERR "Couldn't get real page for gfn %lx!\n",
>  					(long)gfn);
> -			kvm_release_pfn_clean(pfn);
>  			return;
>  		}
> 
> diff --git a/arch/x86/kvm/mmu.c b/arch/x86/kvm/mmu.c
> index 320a781..949a5b8 100644
> --- a/arch/x86/kvm/mmu.c
> +++ b/arch/x86/kvm/mmu.c
> @@ -2498,7 +2498,9 @@ static void mmu_set_spte(struct kvm_vcpu *vcpu, u64 *sptep,
>  				rmap_recycle(vcpu, sptep, gfn);
>  		}
>  	}
> -	kvm_release_pfn_clean(pfn);
> +
> +	if (!is_error_pfn(pfn))
> +		kvm_release_pfn_clean(pfn);
>  }

Can it ever be error_pfn? Seems a problem if so.

Re: [PATCH 8/9] KVM: do not release the error pfn

[Xiao Guangrong]

On 08/02/2012 09:14 PM, Marcelo Tosatti wrote:
> On Sun, Jul 29, 2012 at 04:18:58PM +0800, Xiao Guangrong wrote:
>> After commit a2766325cf9f9, the error pfn is replaced by the
>> error code, it need not be released anymore
>>
>> [ The patch is compiling tested for powerpc ]
>>
>> Signed-off-by: Xiao Guangrong <xiaoguangrong@linux.vnet.ibm.com>
>> ---
>>  arch/powerpc/kvm/e500_tlb.c |    1 -
>>  arch/x86/kvm/mmu.c          |    6 +++---
>>  arch/x86/kvm/mmu_audit.c    |    4 +---
>>  arch/x86/kvm/paging_tmpl.h  |    8 ++------
>>  virt/kvm/iommu.c            |    1 -
>>  virt/kvm/kvm_main.c         |   14 ++++++++------
>>  6 files changed, 14 insertions(+), 20 deletions(-)
>>
>> diff --git a/arch/powerpc/kvm/e500_tlb.c b/arch/powerpc/kvm/e500_tlb.c
>> index c8f6c58..09ce5ac 100644
>> --- a/arch/powerpc/kvm/e500_tlb.c
>> +++ b/arch/powerpc/kvm/e500_tlb.c
>> @@ -524,7 +524,6 @@ static inline void kvmppc_e500_shadow_map(struct kvmppc_vcpu_e500 *vcpu_e500,
>>  		if (is_error_pfn(pfn)) {
>>  			printk(KERN_ERR "Couldn't get real page for gfn %lx!\n",
>>  					(long)gfn);
>> -			kvm_release_pfn_clean(pfn);
>>  			return;
>>  		}
>>
>> diff --git a/arch/x86/kvm/mmu.c b/arch/x86/kvm/mmu.c
>> index 320a781..949a5b8 100644
>> --- a/arch/x86/kvm/mmu.c
>> +++ b/arch/x86/kvm/mmu.c
>> @@ -2498,7 +2498,9 @@ static void mmu_set_spte(struct kvm_vcpu *vcpu, u64 *sptep,
>>  				rmap_recycle(vcpu, sptep, gfn);
>>  		}
>>  	}
>> -	kvm_release_pfn_clean(pfn);
>> +
>> +	if (!is_error_pfn(pfn))
>> +		kvm_release_pfn_clean(pfn);
>>  }
> 
> Can it ever be error_pfn? Seems a problem if so.
> 

Yes, the no-slot-pfn, we will cache the mmio access into spte.

[PATCH 9/9] KVM: do not release the error page

[Xiao Guangrong]

After commit a2766325cf9f9, the error page is replaced by the
error code, it need not be released anymore

[ The patch is compiling tested for powerpc ]

Signed-off-by: Xiao Guangrong <xiaoguangrong@linux.vnet.ibm.com>
---
 arch/powerpc/kvm/44x_tlb.c   |    1 -
 arch/powerpc/kvm/book3s_pr.c |    4 +---
 arch/x86/kvm/svm.c           |    1 -
 arch/x86/kvm/vmx.c           |    5 ++---
 arch/x86/kvm/x86.c           |    9 +++------
 include/linux/kvm_host.h     |    2 +-
 virt/kvm/async_pf.c          |    4 ++--
 virt/kvm/kvm_main.c          |    5 +++--
 8 files changed, 12 insertions(+), 19 deletions(-)

diff --git a/arch/powerpc/kvm/44x_tlb.c b/arch/powerpc/kvm/44x_tlb.c
index 33aa715..5dd3ab4 100644
--- a/arch/powerpc/kvm/44x_tlb.c
+++ b/arch/powerpc/kvm/44x_tlb.c
@@ -319,7 +319,6 @@ void kvmppc_mmu_map(struct kvm_vcpu *vcpu, u64 gvaddr, gpa_t gpaddr,
 	if (is_error_page(new_page)) {
 		printk(KERN_ERR "Couldn't get guest page for gfn %llx!\n",
 			(unsigned long long)gfn);
-		kvm_release_page_clean(new_page);
 		return;
 	}
 	hpaddr = page_to_phys(new_page);
diff --git a/arch/powerpc/kvm/book3s_pr.c b/arch/powerpc/kvm/book3s_pr.c
index a1baec3..05c28f5 100644
--- a/arch/powerpc/kvm/book3s_pr.c
+++ b/arch/powerpc/kvm/book3s_pr.c
@@ -242,10 +242,8 @@ static void kvmppc_patch_dcbz(struct kvm_vcpu *vcpu, struct kvmppc_pte *pte)
 	int i;

 	hpage = gfn_to_page(vcpu->kvm, pte->raddr >> PAGE_SHIFT);
-	if (is_error_page(hpage)) {
-		kvm_release_page_clean(hpage);
+	if (is_error_page(hpage))
 		return;
-	}

 	hpage_offset = pte->raddr & ~PAGE_MASK;
 	hpage_offset &= ~0xFFFULL;
diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c
index 687d0c3..31be4a5 100644
--- a/arch/x86/kvm/svm.c
+++ b/arch/x86/kvm/svm.c
@@ -2105,7 +2105,6 @@ static void *nested_svm_map(struct vcpu_svm *svm, u64 gpa, struct page **_page)
 	return kmap(page);

 error:
-	kvm_release_page_clean(page);
 	kvm_inject_gp(&svm->vcpu, 0);

 	return NULL;
diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
index 2300e53..4b6e809 100644
--- a/arch/x86/kvm/vmx.c
+++ b/arch/x86/kvm/vmx.c
@@ -596,10 +596,9 @@ static inline struct vmcs12 *get_vmcs12(struct kvm_vcpu *vcpu)
 static struct page *nested_get_page(struct kvm_vcpu *vcpu, gpa_t addr)
 {
 	struct page *page = gfn_to_page(vcpu->kvm, addr >> PAGE_SHIFT);
-	if (is_error_page(page)) {
-		kvm_release_page_clean(page);
+	if (is_error_page(page))
 		return NULL;
-	}
+
 	return page;
 }

diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index b6379e5..3c94d80 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -1635,10 +1635,9 @@ int kvm_set_msr_common(struct kvm_vcpu *vcpu, u32 msr, u64 data)
 		vcpu->arch.time_page =
 				gfn_to_page(vcpu->kvm, data >> PAGE_SHIFT);

-		if (is_error_page(vcpu->arch.time_page)) {
-			kvm_release_page_clean(vcpu->arch.time_page);
+		if (is_error_page(vcpu->arch.time_page))
 			vcpu->arch.time_page = NULL;
-		}
+
 		break;
 	}
 	case MSR_KVM_ASYNC_PF_EN:
@@ -3941,10 +3940,8 @@ static int emulator_cmpxchg_emulated(struct x86_emulate_ctxt *ctxt,
 		goto emul_write;

 	page = gfn_to_page(vcpu->kvm, gpa >> PAGE_SHIFT);
-	if (is_error_page(page)) {
-		kvm_release_page_clean(page);
+	if (is_error_page(page))
 		goto emul_write;
-	}

 	kaddr = kmap_atomic(page);
 	kaddr += offset_in_page(gpa);
diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h
index 08a9da9..76242a1 100644
--- a/include/linux/kvm_host.h
+++ b/include/linux/kvm_host.h
@@ -458,7 +458,7 @@ pfn_t gfn_to_pfn(struct kvm *kvm, gfn_t gfn);
 pfn_t gfn_to_pfn_prot(struct kvm *kvm, gfn_t gfn, bool write_fault,
 		      bool *writable);
 pfn_t gfn_to_pfn_memslot(struct kvm_memory_slot *slot, gfn_t gfn);
-void kvm_release_pfn_dirty(pfn_t);
+void kvm_release_pfn_dirty(pfn_t pfn);
 void kvm_release_pfn_clean(pfn_t pfn);
 void kvm_set_pfn_dirty(pfn_t pfn);
 void kvm_set_pfn_accessed(pfn_t pfn);
diff --git a/virt/kvm/async_pf.c b/virt/kvm/async_pf.c
index aa38af6..1f434ef 100644
--- a/virt/kvm/async_pf.c
+++ b/virt/kvm/async_pf.c
@@ -111,7 +111,7 @@ void kvm_clear_async_pf_completion_queue(struct kvm_vcpu *vcpu)
 			list_entry(vcpu->async_pf.done.next,
 				   typeof(*work), link);
 		list_del(&work->link);
-		if (work->page)
+		if (!is_error_page(work->page))
 			kvm_release_page_clean(work->page);
 		kmem_cache_free(async_pf_cache, work);
 	}
@@ -138,7 +138,7 @@ void kvm_check_async_pf_completion(struct kvm_vcpu *vcpu)

 		list_del(&work->queue);
 		vcpu->async_pf.queued--;
-		if (work->page)
+		if (!is_error_page(work->page))
 			kvm_release_page_clean(work->page);
 		kmem_cache_free(async_pf_cache, work);
 	}
diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
index 0c29714..9eadb58 100644
--- a/virt/kvm/kvm_main.c
+++ b/virt/kvm/kvm_main.c
@@ -1195,8 +1195,9 @@ EXPORT_SYMBOL_GPL(gfn_to_page);

 void kvm_release_page_clean(struct page *page)
 {
-	if (!is_error_page(page))
-		kvm_release_pfn_clean(page_to_pfn(page));
+	WARN_ON(is_error_page(page));
+
+	kvm_release_pfn_clean(page_to_pfn(page));
 }
 EXPORT_SYMBOL_GPL(kvm_release_page_clean);

-- 
1.7.7.6