diff mbox

[07/10] KVM: introduce readonly_fault_pfn

Message ID 50057A49.8070400@linux.vnet.ibm.com (mailing list archive)
State New, archived
Headers show

Commit Message

Xiao Guangrong July 17, 2012, 2:44 p.m. UTC 
Introduce readonly_fault_pfn, in the later patch, it indicates failure
when we try to get a writable pfn from the readonly memslot

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

Comments

Avi Kivity July 19, 2012, 10:15 a.m. UTC | #1 
On 07/17/2012 05:44 PM, Xiao Guangrong wrote:
> Introduce readonly_fault_pfn, in the later patch, it indicates failure
> when we try to get a writable pfn from the readonly memslot
> 
> +
>  inline int kvm_is_mmio_pfn(pfn_t pfn)
>  {
>  	if (pfn_valid(pfn)) {
> @@ -949,13 +952,15 @@ EXPORT_SYMBOL_GPL(kvm_disable_largepages);
> 
>  int is_error_page(struct page *page)
>  {
> -	return page == bad_page || page == hwpoison_page || page == fault_page;
> +	return page == bad_page || page == hwpoison_page || page == fault_page
> +		|| page == readonly_fault_page;

All those checks are slow, and get_page(fault_page) etc. isn't very
scalable.

We should move to ERR_PTR() etc.  We could use ENOENT, EHWPOISON,
EFAULT, and EROFS for the above, or maybe there are better matches.

>  }
>  EXPORT_SYMBOL_GPL(is_error_page);
> 
>  int is_error_pfn(pfn_t pfn)
>  {
> -	return pfn == bad_pfn || pfn == hwpoison_pfn || pfn == fault_pfn;
> +	return pfn == bad_pfn || pfn == hwpoison_pfn || pfn == fault_pfn
> +		|| pfn == readonly_fault_pfn;
>  }

And a similar change here.
Xiao Guangrong July 20, 2012, 2:56 a.m. UTC | #2 
On 07/19/2012 06:15 PM, Avi Kivity wrote:
> On 07/17/2012 05:44 PM, Xiao Guangrong wrote:
>> Introduce readonly_fault_pfn, in the later patch, it indicates failure
>> when we try to get a writable pfn from the readonly memslot
>>
>> +
>>  inline int kvm_is_mmio_pfn(pfn_t pfn)
>>  {
>>  	if (pfn_valid(pfn)) {
>> @@ -949,13 +952,15 @@ EXPORT_SYMBOL_GPL(kvm_disable_largepages);
>>
>>  int is_error_page(struct page *page)
>>  {
>> -	return page == bad_page || page == hwpoison_page || page == fault_page;
>> +	return page == bad_page || page == hwpoison_page || page == fault_page
>> +		|| page == readonly_fault_page;
> 
> All those checks are slow, and get_page(fault_page) etc. isn't very
> scalable.
> 
> We should move to ERR_PTR() etc.  We could use ENOENT, EHWPOISON,
> EFAULT, and EROFS for the above, or maybe there are better matches.
> 

Good point. I will do it in the next version, thank you, Avi!

--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h
index e4815e9..a2302e7 100644
--- a/include/linux/kvm_host.h
+++ b/include/linux/kvm_host.h
@@ -385,6 +385,7 @@  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_readonly_fault_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 e9eab07..b70f1a4 100644
--- a/virt/kvm/kvm_main.c
+++ b/virt/kvm/kvm_main.c
@@ -109,6 +109,9 @@  static pfn_t hwpoison_pfn;
 static struct page *fault_page;
 static pfn_t fault_pfn;

+static struct page *readonly_fault_page;
+static pfn_t readonly_fault_pfn;
+
 inline int kvm_is_mmio_pfn(pfn_t pfn)
 {
 	if (pfn_valid(pfn)) {
@@ -949,13 +952,15 @@  EXPORT_SYMBOL_GPL(kvm_disable_largepages);

 int is_error_page(struct page *page)
 {
-	return page == bad_page || page == hwpoison_page || page == fault_page;
+	return page == bad_page || page == hwpoison_page || page == fault_page
+		|| page == readonly_fault_page;
 }
 EXPORT_SYMBOL_GPL(is_error_page);

 int is_error_pfn(pfn_t pfn)
 {
-	return pfn == bad_pfn || pfn == hwpoison_pfn || pfn == fault_pfn;
+	return pfn == bad_pfn || pfn == hwpoison_pfn || pfn == fault_pfn
+		|| pfn == readonly_fault_pfn;
 }
 EXPORT_SYMBOL_GPL(is_error_pfn);

@@ -965,6 +970,12 @@  int is_hwpoison_pfn(pfn_t pfn)
 }
 EXPORT_SYMBOL_GPL(is_hwpoison_pfn);

+int is_readonly_fault_pfn(pfn_t pfn)
+{
+	return pfn == readonly_fault_pfn;
+}
+EXPORT_SYMBOL_GPL(is_readonly_fault_pfn);
+
 int is_noslot_pfn(pfn_t pfn)
 {
 	return pfn == bad_pfn;
@@ -973,7 +984,8 @@  EXPORT_SYMBOL_GPL(is_noslot_pfn);

 int is_invalid_pfn(pfn_t pfn)
 {
-	return pfn == hwpoison_pfn || pfn == fault_pfn;
+	return pfn == hwpoison_pfn || pfn == fault_pfn ||
+			pfn == readonly_fault_pfn;
 }
 EXPORT_SYMBOL_GPL(is_invalid_pfn);

@@ -1076,6 +1088,12 @@  pfn_t get_fault_pfn(void)
 }
 EXPORT_SYMBOL_GPL(get_fault_pfn);

+static pfn_t get_readonly_fault_pfn(void)
+{
+	get_page(readonly_fault_page);
+	return readonly_fault_pfn;
+}
+
 int get_user_page_nowait(struct task_struct *tsk, struct mm_struct *mm,
 	unsigned long start, int write, struct page **page)
 {
@@ -2809,42 +2827,49 @@  static void kvm_sched_out(struct preempt_notifier *pn,
 	kvm_arch_vcpu_put(vcpu);
 }

-int kvm_init(void *opaque, unsigned vcpu_size, unsigned vcpu_align,
-		  struct module *module)
+static void kvm_uninit_dummy_pages(void)
 {
-	int r;
-	int cpu;
-
-	r = kvm_arch_init(opaque);
-	if (r)
-		goto out_fail;
+	if (fault_page)
+		__free_page(fault_page);
+	if (readonly_fault_page)
+		__free_page(readonly_fault_page);
+	if (hwpoison_page)
+		__free_page(hwpoison_page);
+	if (bad_page)
+		__free_page(bad_page);
+}

+static int kvm_init_dummy_pages(void)
+{
 	bad_page = alloc_page(GFP_KERNEL | __GFP_ZERO);
+	hwpoison_page = alloc_page(GFP_KERNEL | __GFP_ZERO);
+	fault_page = alloc_page(GFP_KERNEL | __GFP_ZERO);
+	readonly_fault_page = alloc_page(GFP_KERNEL | __GFP_ZERO);

-	if (bad_page == NULL) {
-		r = -ENOMEM;
-		goto out;
-	}
+	if (!bad_page || !hwpoison_page || !fault_page || !readonly_fault_page)
+		return -ENOMEM;

 	bad_pfn = page_to_pfn(bad_page);
+	hwpoison_pfn = page_to_pfn(hwpoison_page);
+	fault_pfn = page_to_pfn(fault_page);
+	readonly_fault_pfn = page_to_pfn(readonly_fault_page);

-	hwpoison_page = alloc_page(GFP_KERNEL | __GFP_ZERO);
-
-	if (hwpoison_page == NULL) {
-		r = -ENOMEM;
-		goto out_free_0;
-	}
+	return 0;
+}

-	hwpoison_pfn = page_to_pfn(hwpoison_page);
+int kvm_init(void *opaque, unsigned vcpu_size, unsigned vcpu_align,
+		  struct module *module)
+{
+	int r;
+	int cpu;

-	fault_page = alloc_page(GFP_KERNEL | __GFP_ZERO);
+	r = kvm_arch_init(opaque);
+	if (r)
+		goto out_fail;

-	if (fault_page == NULL) {
-		r = -ENOMEM;
+	r = kvm_init_dummy_pages();
+	if (r)
 		goto out_free_0;
-	}
-
-	fault_pfn = page_to_pfn(fault_page);

 	if (!zalloc_cpumask_var(&cpus_hardware_enabled, GFP_KERNEL)) {
 		r = -ENOMEM;
@@ -2920,12 +2945,7 @@  out_free_1:
 out_free_0a:
 	free_cpumask_var(cpus_hardware_enabled);
 out_free_0:
-	if (fault_page)
-		__free_page(fault_page);
-	if (hwpoison_page)
-		__free_page(hwpoison_page);
-	__free_page(bad_page);
-out:
+	kvm_uninit_dummy_pages();
 	kvm_arch_exit();
 out_fail:
 	return r;
@@ -2945,8 +2965,6 @@  void kvm_exit(void)
 	kvm_arch_hardware_unsetup();
 	kvm_arch_exit();
 	free_cpumask_var(cpus_hardware_enabled);
-	__free_page(fault_page);
-	__free_page(hwpoison_page);
-	__free_page(bad_page);
+	kvm_uninit_dummy_pages();
 }
 EXPORT_SYMBOL_GPL(kvm_exit);