Add the hook kvm_arch_set_memory region to hold mmu-specific changes

Add the hook kvm_arch_set_memory region to hold mmu-specific changes

[Zhang, Xiantao]

[-- Attachment #1: Type: text/plain, Size: 186 bytes --]

Patch [3/3] Add the hook kvm_arch_set_memory_region.  In this patch,
introduce a function kvm_caculate_mmu_pages to compute mmu pages in
total, once memory region changes.

Xiantao

[-- Attachment #2: 0003-KVM-Portability-Split-kvm_set_memory_region.patch --]
[-- Type: application/octet-stream, Size: 3272 bytes --]

From 87259f087801b93aa516e76f6a303c54eb8ddb10 Mon Sep 17 00:00:00 2001
From: Zhang Xiantao <xiantao.zhang@intel.com>
Date: Mon, 19 Nov 2007 13:13:44 +0800
Subject: [PATCH] KVM Portability: Split kvm_set_memory_region.
Add arch support for kvm_set_memory_region, and mov mmu-specific code to
kvm_arch_set_memory_region.
Signed-off-by: Zhang Xiantao <xiantao.zhang@intel.com>
---
 drivers/kvm/kvm.h      |    2 ++
 drivers/kvm/kvm_main.c |   22 ++--------------------
 drivers/kvm/x86.c      |   29 +++++++++++++++++++++++++++++
 3 files changed, 33 insertions(+), 20 deletions(-)

diff --git a/drivers/kvm/kvm.h b/drivers/kvm/kvm.h
index c1aa84f..7544e90 100644
--- a/drivers/kvm/kvm.h
+++ b/drivers/kvm/kvm.h
@@ -539,6 +539,8 @@ int kvm_is_error_hva(unsigned long addr);
 int kvm_set_memory_region(struct kvm *kvm,
 			  struct kvm_userspace_memory_region *mem,
 			  int user_alloc);
+void kvm_arch_set_memory_region(struct kvm *kvm,
+			  struct kvm_userspace_memory_region *mem);
 int __kvm_set_memory_region(struct kvm *kvm,
 			    struct kvm_userspace_memory_region *mem,
 			    int user_alloc);
diff --git a/drivers/kvm/kvm_main.c b/drivers/kvm/kvm_main.c
index bda733a..a4a32bd 100644
--- a/drivers/kvm/kvm_main.c
+++ b/drivers/kvm/kvm_main.c
@@ -332,29 +332,9 @@ int __kvm_set_memory_region(struct kvm *kvm,
 	if (mem->slot >= kvm->nmemslots)
 		kvm->nmemslots = mem->slot + 1;
 
-	if (!kvm->n_requested_mmu_pages) {
-		unsigned int n_pages;
-
-		if (npages) {
-			n_pages = npages * KVM_PERMILLE_MMU_PAGES / 1000;
-			kvm_mmu_change_mmu_pages(kvm, kvm->n_alloc_mmu_pages +
-						 n_pages);
-		} else {
-			unsigned int nr_mmu_pages;
-
-			n_pages = old.npages * KVM_PERMILLE_MMU_PAGES / 1000;
-			nr_mmu_pages = kvm->n_alloc_mmu_pages - n_pages;
-			nr_mmu_pages = max(nr_mmu_pages,
-				        (unsigned int) KVM_MIN_ALLOC_MMU_PAGES);
-			kvm_mmu_change_mmu_pages(kvm, nr_mmu_pages);
-		}
-	}
 
 	*memslot = new;
 
-	kvm_mmu_slot_remove_write_access(kvm, mem->slot);
-	kvm_flush_remote_tlbs(kvm);
-
 	kvm_free_physmem_slot(&old, &new);
 	return 0;
 
@@ -374,6 +354,8 @@ int kvm_set_memory_region(struct kvm *kvm,
 
 	mutex_lock(&kvm->lock);
 	r = __kvm_set_memory_region(kvm, mem, user_alloc);
+	if (r == 0)
+		kvm_arch_set_memory_region(kvm, mem);
 	mutex_unlock(&kvm->lock);
 	return r;
 }
diff --git a/drivers/kvm/x86.c b/drivers/kvm/x86.c
index 0ec0eee..89268b5 100644
--- a/drivers/kvm/x86.c
+++ b/drivers/kvm/x86.c
@@ -2646,3 +2646,32 @@ void kvm_arch_destroy_vm(struct kvm *kvm)
 	kvm_free_physmem(kvm);
 	kfree(kvm);
 }
+
+static unsigned int kvm_calculate_mmu_pages(struct kvm *kvm)
+{
+	int i;
+	unsigned int nr_mmu_pages;
+	unsigned int  nr_pages = 0;
+
+	for (i = 0; i < kvm->nmemslots; i++)
+		nr_pages += kvm->memslots[i].npages;
+
+	nr_mmu_pages = nr_pages * KVM_PERMILLE_MMU_PAGES / 1000;
+	nr_mmu_pages = max(nr_mmu_pages,
+			(unsigned int) KVM_MIN_ALLOC_MMU_PAGES);
+
+	return nr_mmu_pages;
+}
+
+void kvm_arch_set_memory_region(struct kvm *kvm,
+			  struct kvm_userspace_memory_region *mem)
+{
+	if (!kvm->n_requested_mmu_pages) {
+		int nr_mmu_pages = kvm_calculate_mmu_pages(kvm);
+		kvm_mmu_change_mmu_pages(kvm, nr_mmu_pages);
+	}
+
+	kvm_mmu_slot_remove_write_access(kvm, mem->slot);
+	kvm_flush_remote_tlbs(kvm);
+}
+
-- 
1.5.1.2


[-- Attachment #3: Type: text/plain, Size: 228 bytes --]

-------------------------------------------------------------------------
This SF.net email is sponsored by: Microsoft
Defy all challenges. Microsoft(R) Visual Studio 2005.
http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/

[-- Attachment #4: Type: text/plain, Size: 186 bytes --]

_______________________________________________
kvm-devel mailing list
kvm-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org
https://lists.sourceforge.net/lists/listinfo/kvm-devel

Re: Add the hook kvm_arch_set_memory region to hold mmu-specific changes

[Carsten Otte]

Zhang, Xiantao wrote:
> Patch [3/3] Add the hook kvm_arch_set_memory_region.  In this patch,
> introduce a function kvm_caculate_mmu_pages to compute mmu pages in
> total, once memory region changes.diff --git a/drivers/kvm/kvm_main.c b/drivers/kvm/kvm_main.c

index bda733a..a4a32bd 100644
--- a/drivers/kvm/kvm_main.c
+++ b/drivers/kvm/kvm_main.c
@@ -332,29 +332,9 @@ int __kvm_set_memory_region(struct kvm *kvm,
  	if (mem->slot >= kvm->nmemslots)
  		kvm->nmemslots = mem->slot + 1;

-	if (!kvm->n_requested_mmu_pages) {
-		unsigned int n_pages;
-
-		if (npages) {
-			n_pages = npages * KVM_PERMILLE_MMU_PAGES / 1000;
-			kvm_mmu_change_mmu_pages(kvm, kvm->n_alloc_mmu_pages +
-						 n_pages);
-		} else {
-			unsigned int nr_mmu_pages;
-
-			n_pages = old.npages * KVM_PERMILLE_MMU_PAGES / 1000;
-			nr_mmu_pages = kvm->n_alloc_mmu_pages - n_pages;
-			nr_mmu_pages = max(nr_mmu_pages,
-				        (unsigned int) KVM_MIN_ALLOC_MMU_PAGES);
-			kvm_mmu_change_mmu_pages(kvm, nr_mmu_pages);
-		}
-	}

  	*memslot = new;

-	kvm_mmu_slot_remove_write_access(kvm, mem->slot);
-	kvm_flush_remote_tlbs(kvm);
-
  	kvm_free_physmem_slot(&old, &new);
  	return 0;

@@ -374,6 +354,8 @@ int kvm_set_memory_region(struct kvm *kvm,

  	mutex_lock(&kvm->lock);
  	r = __kvm_set_memory_region(kvm, mem, user_alloc);
+	if (r == 0)
+		kvm_arch_set_memory_region(kvm, mem);
  	mutex_unlock(&kvm->lock);
  	return r;
  }
This split of __kvm_set_memory_region assumes that all architectures 
have physmem slots. I support Avi's suggestion to have user allocated 
memory as new portable approach, and this old kernel-allocation based 
one x86 only. If that's where we're heading, this split is the wrong 
approach. I think we can come up with something smarter then "move the 
whole thing to x86" in this case, but all cases that have user_alloc 
== 1 should really be handled by x86.c code.

-------------------------------------------------------------------------
This SF.net email is sponsored by: Microsoft
Defy all challenges. Microsoft(R) Visual Studio 2005.
http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/

Re: Add the hook kvm_arch_set_memory region to hold mmu-specific changes

[Zhang, Xiantao]

Carsten Otte wrote:
> Zhang, Xiantao wrote:
>> Patch [3/3] Add the hook kvm_arch_set_memory_region.  In this patch,
>> introduce a function kvm_caculate_mmu_pages to compute mmu pages in
>> total, once memory region changes.diff --git
>> a/drivers/kvm/kvm_main.c b/drivers/kvm/kvm_main.c 
> 

>   	mutex_lock(&kvm->lock);
>   	r = __kvm_set_memory_region(kvm, mem, user_alloc);
> +	if (r == 0)
> +		kvm_arch_set_memory_region(kvm, mem);
>   	mutex_unlock(&kvm->lock);
>   	return r;
>   }
> This split of __kvm_set_memory_region assumes that all architectures
> have physmem slots. I support Avi's suggestion to have user allocated
> memory as new portable approach, and this old kernel-allocation based
> one x86 only. If that's where we're heading, this split is the wrong
> approach. I think we can come up with something smarter then "move the
> whole thing to x86" in this case, but all cases that have user_alloc
> == 1 should really be handled by x86.c code.

User-allocation should be what we are heading. But considering
compatibility with old user-space support, I think kernel-allocation
approach should exist for a long time. I think we don't need to consider
this case now. Once the kernel-allocation approach is abandoned in
future, as you say, we can move them all into x86. 
Xiantao

-------------------------------------------------------------------------
This SF.net email is sponsored by: Microsoft
Defy all challenges. Microsoft(R) Visual Studio 2005.
http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/

Re: Add the hook kvm_arch_set_memory region to hold mmu-specific changes

[Carsten Otte]

Zhang, Xiantao wrote:
> User-allocation should be what we are heading. But considering
> compatibility with old user-space support, I think kernel-allocation
> approach should exist for a long time. 
That's right. This is why I would prefer to have the corresponding 
code out of kvm_main.c: it may exist for a long time for x86.

> I think we don't need to consider
> this case now. Once the kernel-allocation approach is abandoned in
> future, as you say, we can move them all into x86. 
I'd rather prefer to move it upfront. Otherwise, I'd have to consider 
that case for s390 as long as it remains in common. At least I'd have 
to make sure it does'nt get used on s390 using an if() or #ifdef.

-------------------------------------------------------------------------
This SF.net email is sponsored by: Microsoft
Defy all challenges. Microsoft(R) Visual Studio 2005.
http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/

Re: Add the hook kvm_arch_set_memory region to hold mmu-specific changes

[Avi Kivity]

Carsten Otte wrote:
> Zhang, Xiantao wrote:
>> User-allocation should be what we are heading. But considering
>> compatibility with old user-space support, I think kernel-allocation
>> approach should exist for a long time. 
> That's right. This is why I would prefer to have the corresponding 
> code out of kvm_main.c: it may exist for a long time for x86.
>
>> I think we don't need to consider
>> this case now. Once the kernel-allocation approach is abandoned in
>> future, as you say, we can move them all into x86. 
> I'd rather prefer to move it upfront. Otherwise, I'd have to consider 
> that case for s390 as long as it remains in common. At least I'd have 
> to make sure it does'nt get used on s390 using an if() or #ifdef.

I agree, other archs shouldn't have to suffer.

-- 
error compiling committee.c: too many arguments to function


-------------------------------------------------------------------------
This SF.net email is sponsored by: Microsoft
Defy all challenges. Microsoft(R) Visual Studio 2005.
http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/

Re: Add the hook kvm_arch_set_memory region to hold mmu-specific changes

[Zhang, Xiantao]

Avi Kivity wrote:
> Carsten Otte wrote:
>> Zhang, Xiantao wrote:
>>> User-allocation should be what we are heading. But considering
>>> compatibility with old user-space support, I think kernel-allocation
>>> approach should exist for a long time.
>> That's right. This is why I would prefer to have the corresponding
>> code out of kvm_main.c: it may exist for a long time for x86.
>> 
>>> I think we don't need to consider
>>> this case now. Once the kernel-allocation approach is abandoned in
>>> future, as you say, we can move them all into x86.
>> I'd rather prefer to move it upfront. Otherwise, I'd have to consider
>> that case for s390 as long as it remains in common. At least I'd have
>> to make sure it does'nt get used on s390 using an if() or #ifdef.
> 
> I agree, other archs shouldn't have to suffer.

So,  now we move the whole thing(__kvm_set_memory_region) into arch ? :)
Xiantao

-------------------------------------------------------------------------
This SF.net email is sponsored by: Microsoft
Defy all challenges. Microsoft(R) Visual Studio 2005.
http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/

Re: Add the hook kvm_arch_set_memory region to hold mmu-specific changes

[Avi Kivity]

Zhang, Xiantao wrote:
> Avi Kivity wrote:
>   
>> Carsten Otte wrote:
>>     
>>> Zhang, Xiantao wrote:
>>>       
>>>> User-allocation should be what we are heading. But considering
>>>> compatibility with old user-space support, I think kernel-allocation
>>>> approach should exist for a long time.
>>>>         
>>> That's right. This is why I would prefer to have the corresponding
>>> code out of kvm_main.c: it may exist for a long time for x86.
>>>
>>>       
>>>> I think we don't need to consider
>>>> this case now. Once the kernel-allocation approach is abandoned in
>>>> future, as you say, we can move them all into x86.
>>>>         
>>> I'd rather prefer to move it upfront. Otherwise, I'd have to consider
>>> that case for s390 as long as it remains in common. At least I'd have
>>> to make sure it does'nt get used on s390 using an if() or #ifdef.
>>>       
>> I agree, other archs shouldn't have to suffer.
>>     
>
> So,  now we move the whole thing(__kvm_set_memory_region) into arch ? :)
> Xiantao
>   

It's fairly generic.  Only the if (!user_alloc) parts need to move.


-- 
error compiling committee.c: too many arguments to function


-------------------------------------------------------------------------
This SF.net email is sponsored by: Microsoft
Defy all challenges. Microsoft(R) Visual Studio 2005.
http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/