All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 1/9] KVM: iommu: fix releasing unmapped page
@ 2012-07-29  8:12 Xiao Guangrong
  2012-07-29  8:12 ` [PATCH 2/9] KVM: define kvm_fault_pfn statically Xiao Guangrong
                   ` (7 more replies)
  0 siblings, 8 replies; 18+ messages in thread
From: Xiao Guangrong @ 2012-07-29  8:12 UTC (permalink / raw)
  To: Avi Kivity; +Cc: Marcelo Tosatti, LKML, KVM

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

^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH 2/9] KVM: define kvm_fault_pfn statically
  2012-07-29  8:12 [PATCH 1/9] KVM: iommu: fix releasing unmapped page Xiao Guangrong
@ 2012-07-29  8:12 ` Xiao Guangrong
  2012-08-02 12:51   ` Marcelo Tosatti
  2012-07-29  8:13 ` [PATCH 3/9] KVM: define kvm_hwpoison_pfn statically Xiao Guangrong
                   ` (6 subsequent siblings)
  7 siblings, 1 reply; 18+ messages in thread
From: Xiao Guangrong @ 2012-07-29  8:12 UTC (permalink / raw)
  To: Xiao Guangrong; +Cc: Avi Kivity, Marcelo Tosatti, LKML, KVM

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


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH 3/9] KVM: define kvm_hwpoison_pfn statically
  2012-07-29  8:12 [PATCH 1/9] KVM: iommu: fix releasing unmapped page Xiao Guangrong
  2012-07-29  8:12 ` [PATCH 2/9] KVM: define kvm_fault_pfn statically Xiao Guangrong
@ 2012-07-29  8:13 ` Xiao Guangrong
  2012-08-02 12:52   ` Marcelo Tosatti
  2012-07-29  8:14 ` [PATCH 4/9] KVM: define kvm_bad_pfn statically Xiao Guangrong
                   ` (5 subsequent siblings)
  7 siblings, 1 reply; 18+ messages in thread
From: Xiao Guangrong @ 2012-07-29  8:13 UTC (permalink / raw)
  To: Xiao Guangrong; +Cc: Avi Kivity, Marcelo Tosatti, LKML, KVM

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

^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH 4/9] KVM: define kvm_bad_pfn statically
  2012-07-29  8:12 [PATCH 1/9] KVM: iommu: fix releasing unmapped page Xiao Guangrong
  2012-07-29  8:12 ` [PATCH 2/9] KVM: define kvm_fault_pfn statically Xiao Guangrong
  2012-07-29  8:13 ` [PATCH 3/9] KVM: define kvm_hwpoison_pfn statically Xiao Guangrong
@ 2012-07-29  8:14 ` Xiao Guangrong
  2012-08-02 13:15   ` Marcelo Tosatti
  2012-07-29  8:15 ` [PATCH 5/9] KVM: inline is_*_pfn functions Xiao Guangrong
                   ` (4 subsequent siblings)
  7 siblings, 1 reply; 18+ messages in thread
From: Xiao Guangrong @ 2012-07-29  8:14 UTC (permalink / raw)
  To: Xiao Guangrong; +Cc: Avi Kivity, Marcelo Tosatti, LKML, KVM

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

^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH 5/9] KVM: inline is_*_pfn functions
  2012-07-29  8:12 [PATCH 1/9] KVM: iommu: fix releasing unmapped page Xiao Guangrong
                   ` (2 preceding siblings ...)
  2012-07-29  8:14 ` [PATCH 4/9] KVM: define kvm_bad_pfn statically Xiao Guangrong
@ 2012-07-29  8:15 ` Xiao Guangrong
  2012-07-29  8:15 ` [PATCH 6/9] KVM: remove the unused declare Xiao Guangrong
                   ` (3 subsequent siblings)
  7 siblings, 0 replies; 18+ messages in thread
From: Xiao Guangrong @ 2012-07-29  8:15 UTC (permalink / raw)
  To: Xiao Guangrong; +Cc: Avi Kivity, Marcelo Tosatti, LKML, KVM

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

^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH 6/9] KVM: remove the unused declare
  2012-07-29  8:12 [PATCH 1/9] KVM: iommu: fix releasing unmapped page Xiao Guangrong
                   ` (3 preceding siblings ...)
  2012-07-29  8:15 ` [PATCH 5/9] KVM: inline is_*_pfn functions Xiao Guangrong
@ 2012-07-29  8:15 ` Xiao Guangrong
  2012-07-29  8:16 ` [PATCH 7/9] KVM: define kvm_bad_page statically Xiao Guangrong
                   ` (2 subsequent siblings)
  7 siblings, 0 replies; 18+ messages in thread
From: Xiao Guangrong @ 2012-07-29  8:15 UTC (permalink / raw)
  To: Xiao Guangrong; +Cc: Avi Kivity, Marcelo Tosatti, LKML, KVM

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

^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH 7/9] KVM: define kvm_bad_page statically
  2012-07-29  8:12 [PATCH 1/9] KVM: iommu: fix releasing unmapped page Xiao Guangrong
                   ` (4 preceding siblings ...)
  2012-07-29  8:15 ` [PATCH 6/9] KVM: remove the unused declare Xiao Guangrong
@ 2012-07-29  8:16 ` Xiao Guangrong
  2012-08-02 12:54   ` Marcelo Tosatti
  2012-07-29  8:18 ` [PATCH 8/9] KVM: do not release the error pfn Xiao Guangrong
  2012-07-29  8:20 ` [PATCH 9/9] KVM: do not release the error page Xiao Guangrong
  7 siblings, 1 reply; 18+ messages in thread
From: Xiao Guangrong @ 2012-07-29  8:16 UTC (permalink / raw)
  To: Xiao Guangrong; +Cc: Avi Kivity, Marcelo Tosatti, LKML, KVM

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

^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH 8/9] KVM: do not release the error pfn
  2012-07-29  8:12 [PATCH 1/9] KVM: iommu: fix releasing unmapped page Xiao Guangrong
                   ` (5 preceding siblings ...)
  2012-07-29  8:16 ` [PATCH 7/9] KVM: define kvm_bad_page statically Xiao Guangrong
@ 2012-07-29  8:18 ` Xiao Guangrong
  2012-07-29 11:33   ` Xiao Guangrong
  2012-08-02 13:14   ` Marcelo Tosatti
  2012-07-29  8:20 ` [PATCH 9/9] KVM: do not release the error page Xiao Guangrong
  7 siblings, 2 replies; 18+ messages in thread
From: Xiao Guangrong @ 2012-07-29  8:18 UTC (permalink / raw)
  To: Xiao Guangrong; +Cc: Avi Kivity, Marcelo Tosatti, LKML, KVM

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

^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH 9/9] KVM: do not release the error page
  2012-07-29  8:12 [PATCH 1/9] KVM: iommu: fix releasing unmapped page Xiao Guangrong
                   ` (6 preceding siblings ...)
  2012-07-29  8:18 ` [PATCH 8/9] KVM: do not release the error pfn Xiao Guangrong
@ 2012-07-29  8:20 ` Xiao Guangrong
  7 siblings, 0 replies; 18+ messages in thread
From: Xiao Guangrong @ 2012-07-29  8:20 UTC (permalink / raw)
  To: Xiao Guangrong; +Cc: Avi Kivity, Marcelo Tosatti, LKML, KVM

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

^ permalink raw reply related	[flat|nested] 18+ messages in thread

* Re: [PATCH 8/9] KVM: do not release the error pfn
  2012-07-29  8:18 ` [PATCH 8/9] KVM: do not release the error pfn Xiao Guangrong
@ 2012-07-29 11:33   ` Xiao Guangrong
  2012-08-02 13:14   ` Marcelo Tosatti
  1 sibling, 0 replies; 18+ messages in thread
From: Xiao Guangrong @ 2012-07-29 11:33 UTC (permalink / raw)
  To: Xiao Guangrong; +Cc: Avi Kivity, Marcelo Tosatti, LKML, KVM

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

^ permalink raw reply related	[flat|nested] 18+ messages in thread

* Re: [PATCH 2/9] KVM: define kvm_fault_pfn statically
  2012-07-29  8:12 ` [PATCH 2/9] KVM: define kvm_fault_pfn statically Xiao Guangrong
@ 2012-08-02 12:51   ` Marcelo Tosatti
  0 siblings, 0 replies; 18+ messages in thread
From: Marcelo Tosatti @ 2012-08-02 12:51 UTC (permalink / raw)
  To: Xiao Guangrong; +Cc: Avi Kivity, LKML, KVM

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.


^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 3/9] KVM: define kvm_hwpoison_pfn statically
  2012-07-29  8:13 ` [PATCH 3/9] KVM: define kvm_hwpoison_pfn statically Xiao Guangrong
@ 2012-08-02 12:52   ` Marcelo Tosatti
  0 siblings, 0 replies; 18+ messages in thread
From: Marcelo Tosatti @ 2012-08-02 12:52 UTC (permalink / raw)
  To: Xiao Guangrong; +Cc: Avi Kivity, LKML, KVM

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.

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 7/9] KVM: define kvm_bad_page statically
  2012-07-29  8:16 ` [PATCH 7/9] KVM: define kvm_bad_page statically Xiao Guangrong
@ 2012-08-02 12:54   ` Marcelo Tosatti
  0 siblings, 0 replies; 18+ messages in thread
From: Marcelo Tosatti @ 2012-08-02 12:54 UTC (permalink / raw)
  To: Xiao Guangrong; +Cc: Avi Kivity, LKML, KVM

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 ...

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 8/9] KVM: do not release the error pfn
  2012-07-29  8:18 ` [PATCH 8/9] KVM: do not release the error pfn Xiao Guangrong
  2012-07-29 11:33   ` Xiao Guangrong
@ 2012-08-02 13:14   ` Marcelo Tosatti
  2012-08-03  7:46     ` Xiao Guangrong
  1 sibling, 1 reply; 18+ messages in thread
From: Marcelo Tosatti @ 2012-08-02 13:14 UTC (permalink / raw)
  To: Xiao Guangrong; +Cc: Avi Kivity, LKML, KVM

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.

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 4/9] KVM: define kvm_bad_pfn statically
  2012-07-29  8:14 ` [PATCH 4/9] KVM: define kvm_bad_pfn statically Xiao Guangrong
@ 2012-08-02 13:15   ` Marcelo Tosatti
  2012-08-03  0:01     ` Paul Mackerras
  0 siblings, 1 reply; 18+ messages in thread
From: Marcelo Tosatti @ 2012-08-02 13:15 UTC (permalink / raw)
  To: Xiao Guangrong; +Cc: Avi Kivity, LKML, KVM

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?

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 4/9] KVM: define kvm_bad_pfn statically
  2012-08-02 13:15   ` Marcelo Tosatti
@ 2012-08-03  0:01     ` Paul Mackerras
  2012-08-03  8:13       ` Xiao Guangrong
  0 siblings, 1 reply; 18+ messages in thread
From: Paul Mackerras @ 2012-08-03  0:01 UTC (permalink / raw)
  To: Marcelo Tosatti; +Cc: Xiao Guangrong, Avi Kivity, LKML, KVM

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.

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 8/9] KVM: do not release the error pfn
  2012-08-02 13:14   ` Marcelo Tosatti
@ 2012-08-03  7:46     ` Xiao Guangrong
  0 siblings, 0 replies; 18+ messages in thread
From: Xiao Guangrong @ 2012-08-03  7:46 UTC (permalink / raw)
  To: Marcelo Tosatti; +Cc: Avi Kivity, LKML, KVM

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.

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 4/9] KVM: define kvm_bad_pfn statically
  2012-08-03  0:01     ` Paul Mackerras
@ 2012-08-03  8:13       ` Xiao Guangrong
  0 siblings, 0 replies; 18+ messages in thread
From: Xiao Guangrong @ 2012-08-03  8:13 UTC (permalink / raw)
  To: Paul Mackerras; +Cc: Marcelo Tosatti, Avi Kivity, LKML, KVM

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.

^ permalink raw reply	[flat|nested] 18+ messages in thread

end of thread, other threads:[~2012-08-03  8:13 UTC | newest]

Thread overview: 18+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-07-29  8:12 [PATCH 1/9] KVM: iommu: fix releasing unmapped page Xiao Guangrong
2012-07-29  8:12 ` [PATCH 2/9] KVM: define kvm_fault_pfn statically Xiao Guangrong
2012-08-02 12:51   ` Marcelo Tosatti
2012-07-29  8:13 ` [PATCH 3/9] KVM: define kvm_hwpoison_pfn statically Xiao Guangrong
2012-08-02 12:52   ` Marcelo Tosatti
2012-07-29  8:14 ` [PATCH 4/9] KVM: define kvm_bad_pfn statically Xiao Guangrong
2012-08-02 13:15   ` Marcelo Tosatti
2012-08-03  0:01     ` Paul Mackerras
2012-08-03  8:13       ` Xiao Guangrong
2012-07-29  8:15 ` [PATCH 5/9] KVM: inline is_*_pfn functions Xiao Guangrong
2012-07-29  8:15 ` [PATCH 6/9] KVM: remove the unused declare Xiao Guangrong
2012-07-29  8:16 ` [PATCH 7/9] KVM: define kvm_bad_page statically Xiao Guangrong
2012-08-02 12:54   ` Marcelo Tosatti
2012-07-29  8:18 ` [PATCH 8/9] KVM: do not release the error pfn Xiao Guangrong
2012-07-29 11:33   ` Xiao Guangrong
2012-08-02 13:14   ` Marcelo Tosatti
2012-08-03  7:46     ` Xiao Guangrong
2012-07-29  8:20 ` [PATCH 9/9] KVM: do not release the error page Xiao Guangrong

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.