>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.
The hypercall range is fixed, the registration is for modules that
dynamically
linked and for making sure the KVM module itself won't get unlinked in
the middle.
>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
>
The logic behind doing KVM peripheral stuff in a module is to let
KVM work with older kernel with only module compilation.
We have people wanting to run RHEL5 or even RHEL4.X.
Making the extra code in a different module provide extra functionality
for
older kernels.
We can merge them with the KVM module but people might want to have
the KVM with the stable feature set and without the additional stuff.
btw: the balloon driver also does the above, I'll send the code
in the following weeks.
>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
