From: Paul Mackerras <hidden> Date: 2011-12-20 09:21:02
This moves gfn_to_memslot(), and the functions it calls, that is,
search_memslots() and __gfn_to_memslot(), from kvm_main.c to kvm_host.h
so that gfn_to_memslot() can be called from non-modular code even
when KVM is a module. On powerpc, the Book3S HV style of KVM has
code that is called from real mode which needs to call gfn_to_memslot()
and thus needs this. (Module code is allocated in the vmalloc region,
which can't be accessed in real mode.)
With this, we can remove builtin_gfn_to_memslot() from book3s_hv_rm_mmu.c
and thus eliminate a little bit of duplication.
Signed-off-by: Paul Mackerras <redacted>
---
arch/powerpc/kvm/book3s_hv_rm_mmu.c | 23 ++---------------------
include/linux/kvm_host.h | 25 ++++++++++++++++++++++++-
virt/kvm/kvm_main.c | 25 -------------------------
3 files changed, 26 insertions(+), 47 deletions(-)
@@ -21,25 +21,6 @@#include<asm/synch.h>#include<asm/ppc-opcode.h>-/*-*SincethisfileisbuiltinevenifKVMisamodule,weneed-*alocalcopyofthisfunctionforthecasewherekvm_main.cis-*modular.-*/-staticstructkvm_memory_slot*builtin_gfn_to_memslot(structkvm*kvm,-gfn_tgfn)-{-structkvm_memslots*slots;-structkvm_memory_slot*memslot;--slots=kvm_memslots(kvm);-kvm_for_each_memslot(memslot,slots)-if(gfn>=memslot->base_gfn&&-gfn<memslot->base_gfn+memslot->npages)-returnmemslot;-returnNULL;-}-/* Translate address of a vmalloc'd thing to a linear map address */staticvoid*real_vmalloc_addr(void*x){
@@ -171,7 +152,7 @@ long kvmppc_h_enter(struct kvm_vcpu *vcpu, unsigned long flags,/* Find the memslot (if any) for this address */gpa=(ptel&HPTE_R_RPN)&~(psize-1);gfn=gpa>>PAGE_SHIFT;-memslot=builtin_gfn_to_memslot(kvm,gfn);+memslot=gfn_to_memslot(kvm,gfn);pa=0;is_io=~0ul;rmap=NULL;
@@ -1031,18 +1018,6 @@ int kvm_is_error_hva(unsigned long addr)}EXPORT_SYMBOL_GPL(kvm_is_error_hva);-staticstructkvm_memory_slot*__gfn_to_memslot(structkvm_memslots*slots,-gfn_tgfn)-{-returnsearch_memslots(slots,gfn);-}--structkvm_memory_slot*gfn_to_memslot(structkvm*kvm,gfn_tgfn)-{-return__gfn_to_memslot(kvm_memslots(kvm),gfn);-}-EXPORT_SYMBOL_GPL(gfn_to_memslot);-intkvm_is_visible_gfn(structkvm*kvm,gfn_tgfn){structkvm_memory_slot*memslot=gfn_to_memslot(kvm,gfn);
From: Alexander Graf <hidden> Date: 2011-12-23 13:33:23
On 20.12.2011, at 10:21, Paul Mackerras wrote:
This moves gfn_to_memslot(), and the functions it calls, that is,
search_memslots() and __gfn_to_memslot(), from kvm_main.c to kvm_host.h
so that gfn_to_memslot() can be called from non-modular code even
when KVM is a module. On powerpc, the Book3S HV style of KVM has
code that is called from real mode which needs to call gfn_to_memslot()
and thus needs this. (Module code is allocated in the vmalloc region,
which can't be accessed in real mode.)
With this, we can remove builtin_gfn_to_memslot() from book3s_hv_rm_mmu.c
and thus eliminate a little bit of duplication.
Signed-off-by: Paul Mackerras <redacted>
From: Avi Kivity <hidden> Date: 2011-12-26 13:22:53
On 12/20/2011 11:21 AM, Paul Mackerras wrote:
This moves gfn_to_memslot(), and the functions it calls, that is,
search_memslots() and __gfn_to_memslot(), from kvm_main.c to kvm_host.h
so that gfn_to_memslot() can be called from non-modular code even
when KVM is a module. On powerpc, the Book3S HV style of KVM has
code that is called from real mode which needs to call gfn_to_memslot()
and thus needs this. (Module code is allocated in the vmalloc region,
which can't be accessed in real mode.)
With this, we can remove builtin_gfn_to_memslot() from book3s_hv_rm_mmu.c
and thus eliminate a little bit of duplication.
Those functions are too big to be inlined IMO. How about moving them to
another C file, and making it builtin for ppc?
The only issue is what to call it. virt/kvm/builtin-for-ppc seems silly.
Or we could move the implementation into a header file, with an extra __
prefix, and have the C stubs call those inlines, so we have exactly on
instantiation. Your real mode code can then call the inlines.
--
error compiling committee.c: too many arguments to function
From: Alexander Graf <hidden> Date: 2012-01-02 15:23:19
On 26.12.2011, at 14:22, Avi Kivity wrote:
On 12/20/2011 11:21 AM, Paul Mackerras wrote:
quoted
This moves gfn_to_memslot(), and the functions it calls, that is,
search_memslots() and __gfn_to_memslot(), from kvm_main.c to kvm_host.h
so that gfn_to_memslot() can be called from non-modular code even
when KVM is a module. On powerpc, the Book3S HV style of KVM has
code that is called from real mode which needs to call gfn_to_memslot()
and thus needs this. (Module code is allocated in the vmalloc region,
which can't be accessed in real mode.)
With this, we can remove builtin_gfn_to_memslot() from book3s_hv_rm_mmu.c
and thus eliminate a little bit of duplication.
Those functions are too big to be inlined IMO. How about moving them to
another C file, and making it builtin for ppc?
The only issue is what to call it. virt/kvm/builtin-for-ppc seems silly.
Yeah - and it makes it pretty confusing to find the functions then.
Or we could move the implementation into a header file, with an extra __
prefix, and have the C stubs call those inlines, so we have exactly on
instantiation. Your real mode code can then call the inlines.
I like this version. That way everyone should be happy :)
Alex
From: Avi Kivity <hidden> Date: 2012-01-02 16:14:01
On 01/02/2012 05:23 PM, Alexander Graf wrote:
quoted
Or we could move the implementation into a header file, with an extra __
prefix, and have the C stubs call those inlines, so we have exactly on
instantiation. Your real mode code can then call the inlines.
I like this version. That way everyone should be happy :)
Pretty much how everything is solved. Pile up another layer of
indirection (compile-time here), everyone's happy, and the code bloats.
(I'm not against this, just grumpy)
--
error compiling committee.c: too many arguments to function
From: Paul Mackerras <hidden> Date: 2012-01-12 10:41:15
This moves __gfn_to_memslot() and search_memslots() from kvm_main.c to
kvm_host.h to reduce the code duplication caused by the need for
non-modular code in arch/powerpc/kvm/book3s_hv_rm_mmu.c to call
gfn_to_memslot() in real mode.
Rather than putting gfn_to_memslot() itself in a header, which would
lead to increased code size, this puts __gfn_to_memslot() in a header.
Then, the non-modular uses of gfn_to_memslot() are changed to call
__gfn_to_memslot() instead. This way there is only one place in the
source code that needs to be changed should the gfn_to_memslot()
implementation need to be modified.
On powerpc, the Book3S HV style of KVM has code that is called from
real mode which needs to call gfn_to_memslot() and thus needs this.
(Module code is allocated in the vmalloc region, which can't be
accessed in real mode.)
With this, we can remove builtin_gfn_to_memslot() from book3s_hv_rm_mmu.c.
Signed-off-by: Paul Mackerras <redacted>
---
arch/powerpc/kvm/book3s_hv_rm_mmu.c | 23 ++---------------------
include/linux/kvm_host.h | 19 +++++++++++++++++++
virt/kvm/kvm_main.c | 19 -------------------
3 files changed, 21 insertions(+), 40 deletions(-)
@@ -21,25 +21,6 @@#include<asm/synch.h>#include<asm/ppc-opcode.h>-/*-*SincethisfileisbuiltinevenifKVMisamodule,weneed-*alocalcopyofthisfunctionforthecasewherekvm_main.cis-*modular.-*/-staticstructkvm_memory_slot*builtin_gfn_to_memslot(structkvm*kvm,-gfn_tgfn)-{-structkvm_memslots*slots;-structkvm_memory_slot*memslot;--slots=kvm_memslots(kvm);-kvm_for_each_memslot(memslot,slots)-if(gfn>=memslot->base_gfn&&-gfn<memslot->base_gfn+memslot->npages)-returnmemslot;-returnNULL;-}-/* Translate address of a vmalloc'd thing to a linear map address */staticvoid*real_vmalloc_addr(void*x){
@@ -171,7 +152,7 @@ long kvmppc_h_enter(struct kvm_vcpu *vcpu, unsigned long flags,/* Find the memslot (if any) for this address */gpa=(ptel&HPTE_R_RPN)&~(psize-1);gfn=gpa>>PAGE_SHIFT;-memslot=builtin_gfn_to_memslot(kvm,gfn);+memslot=__gfn_to_memslot(kvm_memslots(kvm),gfn);pa=0;is_io=~0ul;rmap=NULL;
@@ -1031,12 +1018,6 @@ int kvm_is_error_hva(unsigned long addr)}EXPORT_SYMBOL_GPL(kvm_is_error_hva);-staticstructkvm_memory_slot*__gfn_to_memslot(structkvm_memslots*slots,-gfn_tgfn)-{-returnsearch_memslots(slots,gfn);-}-structkvm_memory_slot*gfn_to_memslot(structkvm*kvm,gfn_tgfn){return__gfn_to_memslot(kvm_memslots(kvm),gfn);
From: Alexander Graf <hidden> Date: 2012-01-12 14:57:28
On 01/12/2012 11:41 AM, Paul Mackerras wrote:
This moves __gfn_to_memslot() and search_memslots() from kvm_main.c to
kvm_host.h to reduce the code duplication caused by the need for
non-modular code in arch/powerpc/kvm/book3s_hv_rm_mmu.c to call
gfn_to_memslot() in real mode.
Rather than putting gfn_to_memslot() itself in a header, which would
lead to increased code size, this puts __gfn_to_memslot() in a header.
Then, the non-modular uses of gfn_to_memslot() are changed to call
__gfn_to_memslot() instead. This way there is only one place in the
source code that needs to be changed should the gfn_to_memslot()
implementation need to be modified.
On powerpc, the Book3S HV style of KVM has code that is called from
real mode which needs to call gfn_to_memslot() and thus needs this.
(Module code is allocated in the vmalloc region, which can't be
accessed in real mode.)
With this, we can remove builtin_gfn_to_memslot() from book3s_hv_rm_mmu.c.
Signed-off-by: Paul Mackerras<redacted>
Confusing to review, but looks correct :). Avi, please ack.
Alex
From: Avi Kivity <hidden> Date: 2012-01-12 15:47:23
On 01/12/2012 12:41 PM, Paul Mackerras wrote:
This moves __gfn_to_memslot() and search_memslots() from kvm_main.c to
kvm_host.h to reduce the code duplication caused by the need for
non-modular code in arch/powerpc/kvm/book3s_hv_rm_mmu.c to call
gfn_to_memslot() in real mode.
Rather than putting gfn_to_memslot() itself in a header, which would
lead to increased code size, this puts __gfn_to_memslot() in a header.
Then, the non-modular uses of gfn_to_memslot() are changed to call
__gfn_to_memslot() instead. This way there is only one place in the
source code that needs to be changed should the gfn_to_memslot()
implementation need to be modified.
On powerpc, the Book3S HV style of KVM has code that is called from
real mode which needs to call gfn_to_memslot() and thus needs this.
(Module code is allocated in the vmalloc region, which can't be
accessed in real mode.)
+static inline struct kvm_memory_slot *
+search_memslots(struct kvm_memslots *slots, gfn_t gfn)
+{
+ struct kvm_memory_slot *memslot;
+
+ kvm_for_each_memslot(memslot, slots)
+ if (gfn >= memslot->base_gfn &&
+ gfn < memslot->base_gfn + memslot->npages)
+ return memslot;
+
+ return NULL;
+}
+
+static inline struct kvm_memory_slot *
+__gfn_to_memslot(struct kvm_memslots *slots, gfn_t gfn)
+{
+ return search_memslots(slots, gfn);
+}
Please add a comment here explaining why these functions are inlined.
There's also the call to kvm_gfn_to_hva_cache_init(), which should be
changed to gfn_to_memslot(), to avoid code bloat.
--
error compiling committee.c: too many arguments to function
From: Paul Mackerras <hidden> Date: 2012-01-13 06:09:51
This moves __gfn_to_memslot() and search_memslots() from kvm_main.c to
kvm_host.h to reduce the code duplication caused by the need for
non-modular code in arch/powerpc/kvm/book3s_hv_rm_mmu.c to call
gfn_to_memslot() in real mode.
Rather than putting gfn_to_memslot() itself in a header, which would
lead to increased code size, this puts __gfn_to_memslot() in a header.
Then, the non-modular uses of gfn_to_memslot() are changed to call
__gfn_to_memslot() instead. This way there is only one place in the
source code that needs to be changed should the gfn_to_memslot()
implementation need to be modified.
On powerpc, the Book3S HV style of KVM has code that is called from
real mode which needs to call gfn_to_memslot() and thus needs this.
(Module code is allocated in the vmalloc region, which can't be
accessed in real mode.)
With this, we can remove builtin_gfn_to_memslot() from book3s_hv_rm_mmu.c.
Signed-off-by: Paul Mackerras <redacted>
---
v3: Add comment, change kvm_gfn_to_hva_cache_init as requested.
arch/powerpc/kvm/book3s_hv_rm_mmu.c | 23 ++---------------------
include/linux/kvm_host.h | 25 +++++++++++++++++++++++++
virt/kvm/kvm_main.c | 21 +--------------------
3 files changed, 28 insertions(+), 41 deletions(-)
@@ -21,25 +21,6 @@#include<asm/synch.h>#include<asm/ppc-opcode.h>-/*-*SincethisfileisbuiltinevenifKVMisamodule,weneed-*alocalcopyofthisfunctionforthecasewherekvm_main.cis-*modular.-*/-staticstructkvm_memory_slot*builtin_gfn_to_memslot(structkvm*kvm,-gfn_tgfn)-{-structkvm_memslots*slots;-structkvm_memory_slot*memslot;--slots=kvm_memslots(kvm);-kvm_for_each_memslot(memslot,slots)-if(gfn>=memslot->base_gfn&&-gfn<memslot->base_gfn+memslot->npages)-returnmemslot;-returnNULL;-}-/* Translate address of a vmalloc'd thing to a linear map address */staticvoid*real_vmalloc_addr(void*x){
@@ -171,7 +152,7 @@ long kvmppc_h_enter(struct kvm_vcpu *vcpu, unsigned long flags,/* Find the memslot (if any) for this address */gpa=(ptel&HPTE_R_RPN)&~(psize-1);gfn=gpa>>PAGE_SHIFT;-memslot=builtin_gfn_to_memslot(kvm,gfn);+memslot=__gfn_to_memslot(kvm_memslots(kvm),gfn);pa=0;is_io=~0ul;rmap=NULL;
@@ -1031,12 +1018,6 @@ int kvm_is_error_hva(unsigned long addr)}EXPORT_SYMBOL_GPL(kvm_is_error_hva);-staticstructkvm_memory_slot*__gfn_to_memslot(structkvm_memslots*slots,-gfn_tgfn)-{-returnsearch_memslots(slots,gfn);-}-structkvm_memory_slot*gfn_to_memslot(structkvm*kvm,gfn_tgfn){return__gfn_to_memslot(kvm_memslots(kvm),gfn);
From: Alexander Graf <hidden> Date: 2012-01-16 13:18:54
On 13.01.2012, at 07:09, Paul Mackerras wrote:
This moves __gfn_to_memslot() and search_memslots() from kvm_main.c to
kvm_host.h to reduce the code duplication caused by the need for
non-modular code in arch/powerpc/kvm/book3s_hv_rm_mmu.c to call
gfn_to_memslot() in real mode.
Rather than putting gfn_to_memslot() itself in a header, which would
lead to increased code size, this puts __gfn_to_memslot() in a header.
Then, the non-modular uses of gfn_to_memslot() are changed to call
__gfn_to_memslot() instead. This way there is only one place in the
source code that needs to be changed should the gfn_to_memslot()
implementation need to be modified.
On powerpc, the Book3S HV style of KVM has code that is called from
real mode which needs to call gfn_to_memslot() and thus needs this.
(Module code is allocated in the vmalloc region, which can't be
accessed in real mode.)
With this, we can remove builtin_gfn_to_memslot() from book3s_hv_rm_mmu.c.
Signed-off-by: Paul Mackerras <redacted>
From: Alexander Graf <hidden> Date: 2012-01-16 13:30:46
On 13.01.2012, at 07:09, Paul Mackerras wrote:
This moves __gfn_to_memslot() and search_memslots() from kvm_main.c to
kvm_host.h to reduce the code duplication caused by the need for
non-modular code in arch/powerpc/kvm/book3s_hv_rm_mmu.c to call
gfn_to_memslot() in real mode.
=20
Rather than putting gfn_to_memslot() itself in a header, which would
lead to increased code size, this puts __gfn_to_memslot() in a header.
Then, the non-modular uses of gfn_to_memslot() are changed to call
__gfn_to_memslot() instead. This way there is only one place in the
source code that needs to be changed should the gfn_to_memslot()
implementation need to be modified.
=20
On powerpc, the Book3S HV style of KVM has code that is called from
real mode which needs to call gfn_to_memslot() and thus needs this.
(Module code is allocated in the vmalloc region, which can't be
accessed in real mode.)
=20
With this, we can remove builtin_gfn_to_memslot() from =
book3s_hv_rm_mmu.c.
Which tree is this against? I got this diff between your patch and the =
patch when applied on my tree:
-@@ -97,7 +78,7 @@ static void remove_revmap_chain(struct kvm *kvm, long =
pte_index,
- rev =3D real_vmalloc_addr(&kvm->arch.revmap[pte_index]);
- ptel =3D rev->guest_rpte;
+@@ -99,7 +80,7 @@ static void remove_revmap_chain(struct kvm *kvm, long =
pte_index,
+ rcbits =3D hpte_r & (HPTE_R_R | HPTE_R_C);
+ ptel =3D rev->guest_rpte |=3D rcbits;
Since this is completely unrelated to the actual change, I'll apply the =
patch either way. It'd just be interesting to know.
Alex
From: Paul Mackerras <hidden> Date: 2012-01-17 05:02:19
On Mon, Jan 16, 2012 at 02:30:41PM +0100, Alexander Graf wrote:
Which tree is this against? I got this diff between your patch and the patch when applied on my tree:
It's against your tree (previously) plus my first 13-patch series, but
not the second series of 5 patches, which presumably why you got the
difference. Sounds like you got it applied OK; if not let me know and
I'll rebase against your current tree.
Paul.