Dan Kenigsberg wrote:
> These patches expose host CPU features (that are known to work under
> KVM) to guests. It makes a couple of benchmarks run faster, and
> generally gives kvm's user better info on its host.
>
> The kernel-space patch adds KVM_GET_SUPPORTED_CPUID ioctl to obtain the
> table of cpuid functions supported by the host. The user-space patch
> allows fine-tuning this table from the command-line.
>
> I had to define struct kvm_cpuid2, KVM_SET_CPUID2 etc., because cpuid
> functions are a little more complex than just function-value pairs.
>
> commit e9775d0a16097cfb71779cb2fb985fb3e5040dc8
> Author: Dan Kenigsberg <[EMAIL PROTECTED]>
> Date: Sun Nov 18 13:55:26 2007 +0200
>
> Support -cpu host option. Negotiate cpuid table with userspace.
>
The kernel doesn't have a -cpu option. The description needs to be more
descriptive (motivation, special cases in cpuid).
>
>
> +static int is_efer_nx(void) {
> + u64 efer;
>
blank line
> + rdmsrl(MSR_EFER, efer);
> + return efer & EFER_NX;
> +}
> +
>
>
> +static int kvm_vcpu_ioctl_get_cpuid2(struct kvm_vcpu *vcpu,
> + struct kvm_cpuid2 *cpuid,
> + struct kvm_cpuid_entry2 __user *entries)
> +{
> + int r;
> +
> + r = -E2BIG;
> + if (cpuid->nent < vcpu->cpuid_nent)
> + goto out;
> + r = -EFAULT;
> + if (copy_to_user(entries, &vcpu->cpuid_entries,
> + vcpu->cpuid_nent * sizeof(struct kvm_cpuid_entry2)))
> + goto out;
> + return 0;
> +
> +out:
> + cpuid->nent = vcpu->cpuid_nent;
>
whitespace damage here
> + return r;
> +
> +static void do_cpuid_ent(struct kvm_cpuid_entry2 *entry, u32 function,
> + u32 index, int *nent, int maxnent)
> +{
> + const __u32 kvm_supported_word0_x86_features = bit(X86_FEATURE_FPU) |
> + bit(X86_FEATURE_VME) | bit(X86_FEATURE_DE) |
> + bit(X86_FEATURE_PSE) | bit(X86_FEATURE_TSC) |
> + bit(X86_FEATURE_MSR) | bit(X86_FEATURE_PAE) |
> + bit(X86_FEATURE_CX8) | bit(X86_FEATURE_APIC) |
> + bit(X86_FEATURE_SEP) | bit(X86_FEATURE_PGE) |
> + bit(X86_FEATURE_CMOV) | bit(X86_FEATURE_PSE36) |
> + bit(X86_FEATURE_CLFLSH) | bit(X86_FEATURE_MMX) |
> + bit(X86_FEATURE_FXSR) | bit(X86_FEATURE_XMM) |
> + bit(X86_FEATURE_XMM2) | bit(X86_FEATURE_SELFSNOOP);
>
u32, not __u32.
> + const __u32 kvm_supported_word1_x86_features = bit(X86_FEATURE_FPU) |
> + bit(X86_FEATURE_VME) | bit(X86_FEATURE_DE) |
> + bit(X86_FEATURE_PSE) | bit(X86_FEATURE_TSC) |
> + bit(X86_FEATURE_MSR) | bit(X86_FEATURE_PAE) |
> + bit(X86_FEATURE_CX8) | bit(X86_FEATURE_APIC) |
> + bit(X86_FEATURE_PGE) |
> + bit(X86_FEATURE_CMOV) | bit(X86_FEATURE_PSE36) |
> + bit(X86_FEATURE_MMX) | bit(X86_FEATURE_FXSR) |
> + bit(X86_FEATURE_SYSCALL) |
> + (bit(X86_FEATURE_NX) && is_efer_nx()) |
> +#ifdef CONFIG_X86_64
> + bit(X86_FEATURE_LM) |
> +#endif
> + /* TODO: make sure the following features are
> + * safe for SVM guests */
> + bit(X86_FEATURE_MMXEXT) |
> + bit(X86_FEATURE_RDTSCP) | bit(X86_FEATURE_3DNOWEXT) |
> + bit(X86_FEATURE_3DNOW);
>
rdtscp isn't, I believe.
> + const __u32 kvm_supported_word3_x86_features =
> + bit(X86_FEATURE_XMM3) | bit(X86_FEATURE_CX16);
> + const __u32 kvm_supported_word6_x86_features =
> + bit(X86_FEATURE_LAHF_LM) | bit(X86_FEATURE_CMP_LEGACY);
> +
> + /* all func 2 cpuid_count() should be called on the same cpu */
> + if (function==2)
> + get_cpu();
>
Avoid the special case, just to it unconditionally.
> + do_cpuid_1_ent(entry, function, index);
> + ++*nent;
> +
> + switch (function) {
> + case 0:
> + entry->eax = min(entry->eax, (u32)0xb);
> + break;
> + case 1:
> + entry->edx &= kvm_supported_word0_x86_features;
> + entry->ecx &= kvm_supported_word3_x86_features;
> + break;
> + /* function 2 entries are STATEFUL. That is, repeated cpuid commands
> + * may return different values. This forces us to get_cpu() before
> + * issuing the first command, and also to emulate this annoying behavior
> + * in kvm_emulate_cpuid() using KVM_CPUID_FLAG_STATE_READ_NEXT */
> + case 2: {
> + int t, times = entry->eax & 0xff;
>
Indent this normally relative to other entries.
> +
> + entry->flags |= KVM_CPUID_FLAG_STATEFUL_FUNC;
> + for (t = 1; t < times && *nent < maxnent; ++t) {
> + do_cpuid_1_ent(&entry[t], function, 0);
> + entry[t].flags |= KVM_CPUID_FLAG_STATEFUL_FUNC;
> + ++*nent;
> + }
> + break;
> + }
> + /* function 4 and 0xb have additional index. */
> + case 4: {
> + int index, cache_type;
> +
> + entry->flags |= KVM_CPUID_FLAG_SIGNIFCANT_INDEX;
> + /* read more entries until cache_type is zero */
> + for (index = 1; *nent < maxnent; ++index) {
> + cache_type = entry[index - 1].eax & 0x1f;
> + if (!cache_type)
> + break;
> + do_cpuid_1_ent(&entry[index], function, index);
> + entry[index].flags |=
> + KVM_CPUID_FLAG_SIGNIFCANT_INDEX;
> + ++*nent;
> + }
> + break;
> + }
> + case 0xb: {
> + int index, level_type;
> +
> + entry->flags |= KVM_CPUID_FLAG_SIGNIFCANT_INDEX;
> + /* read more entries until level_type is zero */
> + for (index = 1; *nent < maxnent; ++index) {
> + level_type = entry[index - 1].ecx & 0xff;
> + if (!level_type)
> + break;
> + do_cpuid_1_ent(&entry[index], function, index);
> + entry[index].flags |=
> + KVM_CPUID_FLAG_SIGNIFCANT_INDEX;
> + ++*nent;
> + }
> + break;
> + }
> + case 0x80000000:
> + entry->eax = min(entry->eax, 0x8000001a);
> + break;
> + case 0x80000001:
> + entry->edx &= kvm_supported_word1_x86_features;
> + entry->ecx &= kvm_supported_word6_x86_features;
> + break;
> + }
> + if (function==2)
> + put_cpu();
> +}
> +
> +static int kvm_vm_ioctl_get_supported_cpuid(struct kvm *kvm,
> + struct kvm_cpuid2 *cpuid,
> + struct kvm_cpuid_entry2 __user *entries)
> +{
> + struct kvm_cpuid_entry2 *cpuid_entries;
> + int limit, nent = 0, r = -E2BIG;
> + u32 func
whitespace damage
> ;
> +
> + if (cpuid->nent < 1)
> + goto out;
> + r = -ENOMEM;
> + cpuid_entries = vmalloc(sizeof(struct kvm_cpuid_entry2) * cpuid->nent);
> + if (!cpuid_entries)
> + goto out;
> +
> + do_cpuid_ent(&cpuid_entries[0], 0, 0, &nent, cpuid->nent);
> + limit = cpuid_entries[0].eax;
> + for (func = 1; func <= limit && nent < cpuid->nent; ++func)
> + do_cpuid_ent(&cpuid_entries[nent], func, 0,
> + &nent, cpuid->nent);
> +
> + if (nent >= cpuid->nent)
> + goto out_free;
>
Should exit with an E2BIG here. ENOMEM means kernel's out of memory,
which isn't the case here.
> + do_cpuid_ent(&cpuid_entries[nent], 0x80000000, 0, &nent, cpuid->nent);
> + limit = cpuid_entries[nent - 1].eax;
> + for (func = 0x80000001; func <= limit && nent < cpuid->nent; ++func)
> + do_cpuid_ent(&cpuid_entries[nent], func, 0,
> + &nent, cpuid->nent);
> + r = -EFAULT;
> + if (copy_to_user(entries, cpuid_entries,
> + nent * sizeof(struct kvm_cpuid_entry2)))
> + goto out_free;
> + cpuid->nent = nent;
> + r = 0;
> +
> +out_free:
> + vfree(cpuid_entries);
> +out:
> + return r;
> +}
> +
> static int kvm_vcpu_ioctl_get_lapic(struct kvm_vcpu *vcpu,
> struct kvm_lapic_state *s)
> {
> @@ -796,6 +1027,36 @@ long kvm_arch_vcpu_ioctl(struct file *filp,
> goto out;
> break;
> }
> + case KVM_SET_CPUID2: {
> + struct kvm_cpuid2 __user *cpuid_arg = argp;
> + struct kvm_cpuid2 cpuid;
> +
> + r = -EFAULT;
> + if (copy_from_user(&cpuid, cpuid_arg, sizeof cpuid))
> + goto out;
> + r = kvm_vcpu_ioctl_set_cpuid2(vcpu, &cpuid,
> + cpuid_arg->entries);
> + if (r)
> + goto out;
> + break;
> + }
> + case KVM_GET_CPUID2: {
> + struct kvm_cpuid2 __user *cpuid_arg = argp;
> + struct kvm_cpuid2 cpuid;
> +
> + r = -EFAULT;
> + if (copy_from_user(&cpuid, cpuid_arg, sizeof cpuid))
> + goto out;
> + r = kvm_vcpu_ioctl_get_cpuid2(vcpu, &cpuid,
> + cpuid_arg->entries);
> + if (r)
> + goto out;
> + r = -EFAULT;
> + if (copy_to_user(cpuid_arg, &cpuid, sizeof cpuid))
> + goto out;
> + r = 0;
> + break;
> + }
> case KVM_GET_MSRS:
> r = msr_io(vcpu, argp, kvm_get_msr, 1);
> break;
> @@ -1091,6 +1352,24 @@ long kvm_arch_vm_ioctl(struct file *filp,
> r = 0;
> break;
> }
> + case KVM_GET_SUPPORTED_CPUID: {
> + struct kvm_cpuid2 __user *cpuid_arg = argp;
> + struct kvm_cpuid2 cpuid;
> +
> + r = -EFAULT;
> + if (copy_from_user(&cpuid, cpuid_arg, sizeof cpuid))
> + goto out;
> + r = kvm_vm_ioctl_get_supported_cpuid(kvm, &cpuid,
> + cpuid_arg->entries);
> + if (r)
> + goto out;
> +
> + r = -EFAULT;
> + if (copy_to_user(cpuid_arg, &cpuid, sizeof cpuid))
> + goto out;
> + r = 0;
> + break;
> + }
> default:
> ;
> }
> @@ -1887,14 +2166,33 @@ void realmode_set_cr(struct kvm_vcpu *vcpu, int cr,
> unsigned long val,
> }
> }
>
> +static int move_to_next_stateful_cpuid_entry(struct kvm_vcpu *vcpu, int i)
> +{
> + struct kvm_cpuid_entry2 *e = &vcpu->cpuid_entries[i];
> + int j, nent = vcpu->cpuid_nent;
> +
> + e->flags &= ~KVM_CPUID_FLAG_STATE_READ_NEXT;
> + /* when no next entry is found, the current entry[i] is reselected */
> + for (j = i + 1; j == i; j = (j + 1) % nent) {
> + struct kvm_cpuid_entry2 *ej =
> + &vcpu->cpuid_entries[j];
>
80 column limit, not 40.
> + if (ej->function == e->function) {
> + ej->flags |= KVM_CPUID_FLAG_STATE_READ_NEXT;
> + return j;
> + }
> + }
> + return 0; /* silence gcc, even though control never reaches here */
> +}
> +
> void kvm_emulate_cpuid(struct kvm_vcpu *vcpu)
> {
> int i;
> - u32 function;
> - struct kvm_cpuid_entry *e, *best;
> + u32 function, index;
> + struct kvm_cpuid_entry2 *e, *best;
>
> kvm_x86_ops->cache_regs(vcpu);
> function = vcpu->regs[VCPU_REGS_RAX];
> + index = vcpu->regs[VCPU_REGS_RCX];
> vcpu->regs[VCPU_REGS_RAX] = 0;
> vcpu->regs[VCPU_REGS_RBX] = 0;
> vcpu->regs[VCPU_REGS_RCX] = 0;
> @@ -1902,7 +2200,15 @@ void kvm_emulate_cpuid(struct kvm_vcpu *vcpu)
> best = NULL;
> for (i = 0; i < vcpu->cpuid_nent; ++i) {
> e = &vcpu->cpuid_entries[i];
> - if (e->function == function) {
> + /* find an entry with matching function, matching index (if
> + * needed), and that should be read next (if it's stateful) */
> + if (e->function == function &&
> + (!(e->flags & KVM_CPUID_FLAG_SIGNIFCANT_INDEX)
> + || e->index == index) &&
> + (!(e->flags & KVM_CPUID_FLAG_STATEFUL_FUNC)
> + || (e->flags & KVM_CPUID_FLAG_STATE_READ_NEXT) )) {
> + if (e->flags & KVM_CPUID_FLAG_STATEFUL_FUNC)
> + move_to_next_stateful_cpuid_entry(vcpu, i);
> best = e;
>
Please move this to a function with multiple if ()s and return statements.
> ------------------------------------------------------------------------
>
> commit e5a590065ab37fb5eb0481e176d455fba98310dd
> Author: Dan Kenigsberg <[EMAIL PROTECTED]>
> Date: Sun Nov 18 13:36:41 2007 +0200
>
> Add -cpu host option. Negotiate cpuid table with kernel.
>
> Signed-off-by: Dan Kenigsberg <[EMAIL PROTECTED]>
>
> diff --git a/kernel/external-module-compat.h b/kernel/external-module-compat.h
> index e3f81fe..ef5d70b 100644
> --- a/kernel/external-module-compat.h
> +++ b/kernel/external-module-compat.h
> @@ -530,14 +530,6 @@ out:
> #define dest_ExtINT 7
> #endif
>
> -/* empty_zero_page isn't exported in all kernels */
> -#include <asm/pgtable.h>
> -
> -#define empty_zero_page kvm_empty_zero_page
> -
> -static char empty_zero_page[PAGE_SIZE];
> -
> -static inline void blahblah(void)
> -{
> - (void)empty_zero_page[0];
> -}
>
Remove this removal.
>
> +int kvm_get_supported_cpuid(kvm_context_t kvm, int *nent,
> + struct kvm_cpuid_entry2 *entries)
> +{
> + struct kvm_cpuid2 *cpuid;
> + int r = -1;
>
blank line.
> +#ifdef KVM_CAP_EXT_CPUID
> + r = ioctl(kvm->fd, KVM_CHECK_EXTENSION, KVM_CAP_EXT_CPUID);
> + if (!r)
> + return -1;
>
return -errno on errors
> +
> + cpuid = malloc(sizeof(*cpuid) + *nent * sizeof(*entries));
> + if (!cpuid)
> + return -ENOMEM;
> + cpuid->nent = *nent;
> +
> + r = ioctl(kvm->vm_fd, KVM_GET_SUPPORTED_CPUID, cpuid);
> +
>
error check
> + memcpy(entries, cpuid->entries, *nent * sizeof(*entries));
> + *nent = cpuid->nent;
> + free(cpuid);
> +#endif
> + return r;
> +}
> +
> +int kvm_get_cpuid(kvm_context_t kvm, int vcpu, int *nent,
> + struct kvm_cpuid_entry2 *entries)
> +{
> + struct kvm_cpuid2 *cpuid;
> + int r = -1;
>
blank line
> +#ifdef KVM_CAP_EXT_CPUID
> + r = ioctl(kvm->fd, KVM_CHECK_EXTENSION, KVM_CAP_EXT_CPUID);
> + if (!r)
> + return -1;
>
-errno
> +
> + cpuid = malloc(sizeof(*cpuid) + *nent * sizeof(*entries));
> + if (!cpuid)
> + return -ENOMEM;
> + cpuid->nent = *nent;
> +
> + r = ioctl(kvm->vcpu_fd[vcpu], KVM_GET_CPUID2, cpuid);
> +
>
error check
> + memcpy(entries, cpuid->entries, *nent * sizeof(*entries));
> + *nent = cpuid->nent;
> + free(cpuid);
> +#endif
> + return r;
> +}
> +
>
--
error compiling committee.c: too many arguments to function
-------------------------------------------------------------------------
This SF.net email is sponsored by: Microsoft
Defy all challenges. Microsoft(R) Visual Studio 2005.
http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/
_______________________________________________
kvm-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/kvm-devel