Hollis Blanchard wrote:
On Tue, 2008-08-19 at 12:36 +0200, [EMAIL PROTECTED] 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(&current->mm->mmap_sem);
+ pvmem_page = gfn_to_page(vcpu->kvm, + vcpu->arch.pvmem_gpaddr >> KVM_PPCPV_MAGIC_PAGE_SHIFT);
+               up_read(&current->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 [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to