I don't really like the idea of having other modules register
hypercalls. The hypercall address space is fixed and should be
maintained by KVM.
I think things like virtio are rare and important enough, that it
warrants having kvm.ko know enough about them to provide a higher level
registration mechanism. The virtio network driver should register the
fact that it's a virtio device, not which hypercalls it cares about.
I'm not sure you gain anything by doing the virtio infrastructure in a
module either. If anything, a config option in kvm.ko would suffice.
Regards,
Anthony Liguori
On Fri, 2007-08-24 at 16:59 -0700, Dor Laor wrote:
> New kernel modules for KVM are upcoming soon, these module
> will need to use hypercalls. Before calling the hypercall function,
> the kvm_main core module has to make sure it won't get unloaded.
> So hypercall register/unregister are added.
>
> Except that the kernel hypercalls handlers are numberes
> 0-KVM_NR_HYPERCALLS.
> All the userspace handlers are above KVM_NR_HYPERCALLS.
>
> Signed-off-by: Dor Laor <[EMAIL PROTECTED]>
> ---
> drivers/kvm/kvm.h | 9 ++
> drivers/kvm/kvm_main.c | 223
> ++++++++++++++++++++++++++++++++++++++--------
> include/linux/kvm_para.h | 4 +-
> 3 files changed, 197 insertions(+), 39 deletions(-)
> mode change 100644 => 100755 drivers/kvm/kvm_main.c
>
> diff --git a/drivers/kvm/kvm.h b/drivers/kvm/kvm.h
> index a42a6f3..37240cf 100644
> --- a/drivers/kvm/kvm.h
> +++ b/drivers/kvm/kvm.h
> @@ -472,6 +472,12 @@ struct kvm_arch_ops {
> unsigned char *hypercall_addr);
> };
>
> +struct kvm_hypercall {
> + unsigned long (*hypercall)(struct kvm_vcpu*, unsigned long
> args[]);
> + struct module *module;
> + int idx;
> +};
> +
> extern struct kvm_arch_ops *kvm_arch_ops;
>
> /* The guest did something we don't support. */
> @@ -588,6 +594,9 @@ int kvm_mmu_load(struct kvm_vcpu *vcpu);
> void kvm_mmu_unload(struct kvm_vcpu *vcpu);
>
> int kvm_hypercall(struct kvm_vcpu *vcpu, struct kvm_run *run);
> +int kvm_register_hypercall(struct module *module, struct kvm_hypercall
> *hypercall);
> +int kvm_unregister_hypercall(struct kvm_hypercall *hypercall);
> +
>
> static inline int kvm_mmu_page_fault(struct kvm_vcpu *vcpu, gva_t gva,
> u32 error_code)
> diff --git a/drivers/kvm/kvm_main.c b/drivers/kvm/kvm_main.c
> old mode 100644
> new mode 100755
> index abd7498..051b47a
> --- a/drivers/kvm/kvm_main.c
> +++ b/drivers/kvm/kvm_main.c
> @@ -20,6 +20,7 @@
> #include "segment_descriptor.h"
>
> #include <linux/kvm.h>
> +#include <linux/kvm_para.h>
> #include <linux/module.h>
> #include <linux/errno.h>
> #include <linux/percpu.h>
> @@ -84,6 +85,23 @@ static struct kvm_stats_debugfs_item {
>
> static struct dentry *debugfs_dir;
>
> +static struct kvm_hypercall hypercalls[KVM_KERNEL_NR_HYPERCALLS];
> +
> +static int test_hypercall(struct kvm_vcpu *vcpu, unsigned long args[])
> +{
> + printk(KERN_DEBUG "%s: hypercall invoked\n", __FUNCTION__);
> +
> + return 0;
> +}
> +
> +static struct kvm_hypercall __hypercall_test = {
> + (unsigned long (*)(struct kvm_vcpu*, unsigned long
> args[]))test_hypercall,
> + THIS_MODULE,
> + __NR_hypercall_test,
> +};
> +
> +static atomic_t dev_kvm_open_count = ATOMIC_INIT(0);
> +
> #define MAX_IO_MSRS 256
>
> #define CR0_RESERVED_BITS
> \
> @@ -302,6 +320,24 @@ static struct kvm *kvm_create_vm(void)
> return kvm;
> }
>
> +static int kvm_dev_open(struct inode *inode, struct file *filp)
> +{
> + int ret = 0;
> +
> + if (atomic_inc_return(&dev_kvm_open_count) == 1) {
> + ret = kvm_register_hypercall(THIS_MODULE,
> &__hypercall_test);
> + if (ret < 0) {
> + printk(KERN_DEBUG "%s: kvm_register_hypercall
> error, "
> + "hypercall: %s\n",
> + __FUNCTION__, "test");
> + goto out_test;
> + }
> + }
> +
> +out_test:
> + return ret;
> +}
> +
> /*
> * Free any memory in @free but not in @dont.
> */
> @@ -371,6 +407,20 @@ static void kvm_free_vcpus(struct kvm *kvm)
>
> }
>
> +static int kvm_dev_release(struct inode *inode, struct file *filp)
> +{
> + int ret = 0;
> +
> + atomic_dec(&dev_kvm_open_count);
> + ret = kvm_unregister_hypercall(&__hypercall_test);
> + if (ret < 0) {
> + printk(KERN_DEBUG "%s:kvm_unregister_hypercall error "
> + "hypercall: %s\n", __FUNCTION__,
> "hypercall_test");
> + }
> +
> + return ret;
> +}
> +
> static void kvm_destroy_vm(struct kvm *kvm)
> {
> spin_lock(&kvm_lock);
> @@ -1267,50 +1317,145 @@ int kvm_emulate_halt(struct kvm_vcpu *vcpu)
> }
> EXPORT_SYMBOL_GPL(kvm_emulate_halt);
>
> -int kvm_hypercall(struct kvm_vcpu *vcpu, struct kvm_run *run)
> +int kvm_register_hypercall(struct module* module, struct kvm_hypercall
> *hypercall)
> {
> - unsigned long nr, a0, a1, a2, a3, a4, a5, ret;
> + int r = 0;
>
> - kvm_arch_ops->cache_regs(vcpu);
> - ret = -KVM_EINVAL;
> -#ifdef CONFIG_X86_64
> - if (is_long_mode(vcpu)) {
> - nr = vcpu->regs[VCPU_REGS_RAX];
> - a0 = vcpu->regs[VCPU_REGS_RDI];
> - a1 = vcpu->regs[VCPU_REGS_RSI];
> - a2 = vcpu->regs[VCPU_REGS_RDX];
> - a3 = vcpu->regs[VCPU_REGS_RCX];
> - a4 = vcpu->regs[VCPU_REGS_R8];
> - a5 = vcpu->regs[VCPU_REGS_R9];
> - } else
> -#endif
> - {
> + if (hypercall->idx >= KVM_KERNEL_NR_HYPERCALLS ||
> + hypercall->idx < 0) {
> + printk(KERN_DEBUG "%s:hypercall registration idx(%d)\n",
> + __FUNCTION__, hypercall->idx);
> + return -EINVAL;
> + }
> +
> + spin_lock(&kvm_lock);
> +
> + if (hypercalls[hypercall->idx].hypercall) {
> + printk(KERN_DEBUG "%s:hypercall idx(%d) already
> taken\n",
> + __FUNCTION__, hypercall->idx);
> + r = -EEXIST;
> + goto out;
> + }
> +
> + if (try_module_get(module) < 0) {
> + printk(KERN_DEBUG "%s: module reference count++
> failed\n",
> + __FUNCTION__);
> + r = -EINVAL;
> + goto out;
> + }
> +
> + hypercalls[hypercall->idx].hypercall = hypercall->hypercall;
> + hypercalls[hypercall->idx].module = module;
> +
> +out:
> + spin_unlock(&kvm_lock);
> +
> + return r;
> +}
> +EXPORT_SYMBOL_GPL(kvm_register_hypercall);
> +
> +int kvm_unregister_hypercall(struct kvm_hypercall *hypercall)
> +{
> + if (hypercall->idx >= KVM_KERNEL_NR_HYPERCALLS ||
> + hypercall->idx < 0) {
> + printk(KERN_DEBUG "%s:hypercall unregistration
> idx(%d)\n",
> + __FUNCTION__, hypercall->idx);
> + return -EINVAL;
> + }
> +
> + spin_lock(&kvm_lock);
> + if (!hypercalls[hypercall->idx].hypercall) {
> + printk(KERN_DEBUG "%s:hypercall idx(%d) was not
> registered\n",
> + __FUNCTION__, hypercall->idx);
> + spin_unlock(&kvm_lock);
> + return -EEXIST;
> + }
> +
> + hypercalls[hypercall->idx].hypercall = 0;
> + module_put(hypercalls[hypercall->idx].module);
> + spin_unlock(&kvm_lock);
> +
> + return 0;
> +}
> +EXPORT_SYMBOL_GPL(kvm_unregister_hypercall);
> +
> +/*
> + * Generic hypercall dispatcher routine.
> + * Returns 0 for user space handling, 1 on success handling
> + */
> + int kvm_hypercall(struct kvm_vcpu *vcpu, struct kvm_run *run)
> + {
> + unsigned long nr, ret;
> + unsigned long args[6];
> +
> + kvm_arch_ops->cache_regs(vcpu);
> + ret = -KVM_EINVAL;
> + #ifdef CONFIG_X86_64
> + if (is_long_mode(vcpu)) {
> + nr = vcpu->regs[VCPU_REGS_RAX];
> + args[0] = vcpu->regs[VCPU_REGS_RDI];
> + args[1] = vcpu->regs[VCPU_REGS_RSI];
> + args[2] = vcpu->regs[VCPU_REGS_RDX];
> + args[3] = vcpu->regs[VCPU_REGS_RCX];
> + args[4] = vcpu->regs[VCPU_REGS_R8];
> + args[5] = vcpu->regs[VCPU_REGS_R9];
> + } else
> + #endif
> + {
> nr = vcpu->regs[VCPU_REGS_RBX] & -1u;
> - a0 = vcpu->regs[VCPU_REGS_RAX] & -1u;
> - a1 = vcpu->regs[VCPU_REGS_RCX] & -1u;
> - a2 = vcpu->regs[VCPU_REGS_RDX] & -1u;
> - a3 = vcpu->regs[VCPU_REGS_RSI] & -1u;
> - a4 = vcpu->regs[VCPU_REGS_RDI] & -1u;
> - a5 = vcpu->regs[VCPU_REGS_RBP] & -1u;
> - }
> - switch (nr) {
> - default:
> - run->hypercall.nr = nr;
> - run->hypercall.args[0] = a0;
> - run->hypercall.args[1] = a1;
> - run->hypercall.args[2] = a2;
> - run->hypercall.args[3] = a3;
> - run->hypercall.args[4] = a4;
> - run->hypercall.args[5] = a5;
> - run->hypercall.ret = ret;
> - run->hypercall.longmode = is_long_mode(vcpu);
> - kvm_arch_ops->decache_regs(vcpu);
> + args[0] = vcpu->regs[VCPU_REGS_RAX] & -1u;
> + args[1] = vcpu->regs[VCPU_REGS_RCX] & -1u;
> + args[2] = vcpu->regs[VCPU_REGS_RDX] & -1u;
> + args[3] = vcpu->regs[VCPU_REGS_RSI] & -1u;
> + args[4] = vcpu->regs[VCPU_REGS_RDI] & -1u;
> + args[5] = vcpu->regs[VCPU_REGS_RBP] & -1u;
> + }
> +
> + if (nr >= KVM_KERNEL_NR_HYPERCALLS || nr < 0) {
> + run->hypercall.nr = nr;
> + memcpy(run->hypercall.args, args, sizeof(args));
> + run->hypercall.ret = 0;
> + run->hypercall.longmode = is_long_mode(vcpu);
> + kvm_arch_ops->decache_regs(vcpu);
> run->exit_reason = KVM_EXIT_HYPERCALL;
>
> - return 0;
> + return 0;
> + }
> +
> + /* The hypercall might block or do intensive work */
> + vcpu_put(vcpu);
> +
> + /*
> + * Increase the hypercall's module ref count assures
> + * that no one will remove the module while a hypercall
> + * is executing.
> + * Theoretically we need to lock to get coherent
> module-hypercall
> + * view but practically there's almost no users this mechanism
> now.
> + */
> + if (try_module_get(hypercalls[nr].module) < 0) {
> + printk(KERN_DEBUG "%s: module reference count++
> failed\n",
> + __FUNCTION__);
> + ret = -EINVAL;
> + goto out;
> }
> - vcpu->regs[VCPU_REGS_RAX] = ret;
> - kvm_arch_ops->decache_regs(vcpu);
> +
> + if (!hypercalls[nr].hypercall) {
> + printk(KERN_ERR "%s: hypercall nr(%lx) was not yet
> registered\n",
> + __FUNCTION__, nr);
> + ret = -EINVAL;
> + goto out_put;
> + }
> +
> + ret = hypercalls[nr].hypercall(vcpu, args);
> +
> +out_put:
> + module_put(hypercalls[nr].module);
> +out:
> + vcpu_load(vcpu);
> +
> + vcpu->regs[VCPU_REGS_RAX] = ret;
> + kvm_arch_ops->decache_regs(vcpu);
> +
> return 1;
> }
> EXPORT_SYMBOL_GPL(kvm_hypercall);
> @@ -2847,6 +2992,8 @@ out:
> }
>
> static struct file_operations kvm_chardev_ops = {
> + .open = kvm_dev_open,
> + .release = kvm_dev_release,
> .unlocked_ioctl = kvm_dev_ioctl,
> .compat_ioctl = kvm_dev_ioctl,
> };
> diff --git a/include/linux/kvm_para.h b/include/linux/kvm_para.h
> index 754e29d..0cf516a 100644
> --- a/include/linux/kvm_para.h
> +++ b/include/linux/kvm_para.h
> @@ -71,6 +71,8 @@ struct kvm_vcpu_para_state {
> * No registers are clobbered by the hypercall, except that the
> * return value is in RAX.
> */
> -#define __NR_hypercalls 0
> +
> +#define KVM_KERNEL_NR_HYPERCALLS 1
> +#define __NR_hypercall_test 0
>
> #endif
>
>
> -----
> In simplicity there is elegance.
> Dor Laor ;)
>
>
> -------------------------------------------------------------------------
> This SF.net email is sponsored by: Splunk Inc.
> Still grepping through log files to find problems? Stop.
> Now Search log events and configuration files using AJAX and a browser.
> Download your FREE copy of Splunk now >> http://get.splunk.com/
> _______________________________________________
> kvm-devel mailing list
> kvm-devel@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/kvm-devel
-------------------------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems? Stop.
Now Search log events and configuration files using AJAX and a browser.
Download your FREE copy of Splunk now >> http://get.splunk.com/
_______________________________________________
kvm-devel mailing list
kvm-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/kvm-devel
