Messages in this thread |  | | | Date | Mon, 02 Nov 2009 14:18:54 +0200 | | From | Avi Kivity <> | | Subject | Re: [PATCH 01/11] Add shared memory hypercall to PV Linux guest. |
| |
On 11/01/2009 01:56 PM, Gleb Natapov wrote: > Add hypercall that allows guest and host to setup per cpu shared > memory. > >
Better to set this up as an MSR (with bit zero enabling, bits 1-5 features, and 64-byte alignment). This allows auto-reset on INIT and live migration using the existing MSR save/restore infrastructure.
> arch/x86/include/asm/kvm_host.h | 3 + > arch/x86/include/asm/kvm_para.h | 11 +++++ > arch/x86/kernel/kvm.c | 82 +++++++++++++++++++++++++++++++++++++++ > arch/x86/kernel/setup.c | 1 + > arch/x86/kernel/smpboot.c | 3 + > arch/x86/kvm/x86.c | 70 +++++++++++++++++++++++++++++++++ > include/linux/kvm.h | 1 + > include/linux/kvm_para.h | 4 ++ > 8 files changed, 175 insertions(+), 0 deletions(-) >
Please separate into guest and host patches.
> +#define KVM_PV_SHM_VERSION 1 >
versions = bad, feature bits = good
> + > +#define KVM_PV_SHM_FEATURES_ASYNC_PF (1<< 0) > + > +struct kvm_vcpu_pv_shm { > + __u64 features; > + __u64 reason; > + __u64 param; > +}; > + >
Some documentation for this?
Also, the name should reflect the pv pagefault use. For other uses we can register other areas.
> #define MMU_QUEUE_SIZE 1024 > > @@ -37,6 +41,7 @@ struct kvm_para_state { > }; > > static DEFINE_PER_CPU(struct kvm_para_state, para_state); > +static DEFINE_PER_CPU(struct kvm_vcpu_pv_shm *, kvm_vcpu_pv_shm); >
Easier to put the entire structure here, not a pointer.
> + > +static int kvm_pv_reboot_notify(struct notifier_block *nb, > + unsigned long code, void *unused) > +{ > + if (code == SYS_RESTART) > + on_each_cpu(kvm_pv_unregister_shm, NULL, 1); > + return NOTIFY_DONE; > +} > + > +static struct notifier_block kvm_pv_reboot_nb = { > + .notifier_call = kvm_pv_reboot_notify, > +}; >
Is this called on kexec, or do we need another hook?
> +static int kvm_pv_setup_shm(struct kvm_vcpu *vcpu, unsigned long gpa, > + unsigned long size, unsigned long version, > + unsigned long *ret) > +{ > + addr = gfn_to_hva(vcpu->kvm, gfn); > + if (kvm_is_error_hva(addr)) > + return -EFAULT; > + > + /* pin page with pv shared memory */ > + down_read(&mm->mmap_sem); > + r = get_user_pages(current, mm, addr, 1, 1, 0,&vcpu->arch.pv_shm_page, > + NULL); > + up_read(&mm->mmap_sem); >
This fails if the memory area straddles a page boundary. Aligning would solve this. I prefer using put_user() though than a permanent get_user_pages().
-- error compiling committee.c: too many arguments to function
|  |