Hollis Blanchard wrote:
On Tue, 2008-08-19 at 12:36 +0200, ehrhardt@xxxxxxxxxxxxxxxxxx wrote:
diff --git a/arch/powerpc/kvm/booke_guest.c b/arch/powerpc/kvm/booke_guest.c
--- a/arch/powerpc/kvm/booke_guest.c
+++ b/arch/powerpc/kvm/booke_guest.c
@@ -21,6 +21,7 @@
#include <linux/errno.h>
#include <linux/err.h>
#include <linux/kvm_host.h>
+#include <linux/kvm_para.h>
#include <linux/module.h>
#include <linux/vmalloc.h>
#include <linux/fs.h>
@@ -43,6 +44,7 @@
{ "itlb_v", VCPU_STAT(itlb_virt_miss_exits) },
{ "dtlb_r", VCPU_STAT(dtlb_real_miss_exits) },
{ "dtlb_v", VCPU_STAT(dtlb_virt_miss_exits) },
+ { "dtlb_pv", VCPU_STAT(dtlb_pvmem_miss_exits) },
{ "sysc", VCPU_STAT(syscall_exits) },
{ "isi", VCPU_STAT(isi_exits) },
{ "dsi", VCPU_STAT(dsi_exits) },
@@ -337,6 +339,16 @@
unsigned long eaddr = vcpu->arch.fault_dear;
gfn_t gfn;
+
+ if (vcpu->arch.pvmem && kvmppc_is_pvmem(vcpu, eaddr)) {
+ kvmppc_mmu_map(vcpu, eaddr,
+ vcpu->arch.pvmem_gpaddr >> KVM_PPCPV_MAGIC_PAGE_SHIFT,
+ 0, KVM_PPCPV_MAGIC_PAGE_FLAGS);
+ vcpu->stat.dtlb_pvmem_miss_exits++;
+ r = RESUME_GUEST;
+ break;
+ }
+
/* Check the guest TLB. */
gtlbe = kvmppc_44x_dtlb_search(vcpu, eaddr);
if (!gtlbe) {
By the way, when this code is running, what's the rate of this new
counter? How does it compare to the reduction in instruction emulation?
@@ -488,6 +500,8 @@
vcpu->arch.shadow_pid = 1;
+ vcpu->arch.pvmem = NULL;
Isn't the whole structure initialized to 0? I don't think this is
needed.
Even if not necessary I would like to keep it. We might change arch
allocation or something else.
This initialization doesn't hurt anyone and is not performance critical.
But it's up to you - if you want me to remove it I'll do that. please
drop me a mail if I should do that.
*Like win popup - "do you really want to delete this line?" - yes/no/maybe
[...]
static void kvmppc_emul_rfi(struct kvm_vcpu *vcpu)
@@ -207,8 +208,18 @@
static int kvmppc_do_hypercall(struct kvm_vcpu *vcpu)
{
int ret = 0;
+ struct page *pvmem_page;
switch (vcpu->arch.gpr[0]) {
+ case KVM_HCALL_RESERVE_MAGICPAGE:
+ vcpu->arch.pvmem_gvaddr = vcpu->arch.gpr[3];
+ vcpu->arch.pvmem_gpaddr = vcpu->arch.gpr[4];
+ down_read(¤t->mm->mmap_sem);
+ pvmem_page = gfn_to_page(vcpu->kvm,
+ vcpu->arch.pvmem_gpaddr >> KVM_PPCPV_MAGIC_PAGE_SHIFT);
+ up_read(¤t->mm->mmap_sem);
+ vcpu->arch.pvmem = kmap(pvmem_page);
+ break;
default:
printk(KERN_ERR "unknown hypercall %d\n", vcpu->arch.gpr[0]);
kvmppc_dump_vcpu(vcpu);
Where is vcpu->arch.pvmem unmapped?
atm nowhere - it is persistent once it is registered
What happens if the guest makes repeated KVM_HCALL_RESERVE_MAGICPAGE
hypercalls? Looks like a good way to leak host memory.
Also, if we migrate a guest which has a page registered, the new host
won't have vcpu->arch.pvmem set because the guest doesn't re-invoke the
KVM_HCALL_RESERVE_MAGICPAGE hypercall.
yeah, but I stored gvaddr/gpaddr. If the migration keeps those (and it
should or a lot other things break) we can fix that.
This is bringing me back to our discussion that I wanted to have a
"migration starts to run on new host" hook.
I need to look for that.
I think we need to put a little more thought into all the corner cases
here.
yeah, lets do that in a brainstorm session next week if you have some time.
It's better to clean corners interactively ;-)
--
Grüsse / regards,
Christian Ehrhardt
IBM Linux Technology Center, Open Virtualization
--
To unsubscribe from this list: send the line "unsubscribe kvm-ppc" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
