Nitin A Kamble wrote:
@ -1194,36 +1194,14 @@ static void do_cpuid_1_ent(struct kvm_cpuid_entry2
*entry, u32 function,
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);
- 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
+ const u32 kvm_unsupported_word0_x86_features = bit(X86_FEATURE_ACC);
+ const u32 kvm_unsupported_word1_x86_features = bit(X86_FEATURE_RDTSCP) |
+#ifndef CONFIG_X86_64
What's the motivation for this change?
A potential problem is that a newer cpu might define new bits, which we
don't support, yet we report them as supported.
The motivation is: IMO There would be very few features which KVM will
not be able to support by default. Also lots of cpuid bits are being
blocked unnecessarily by the current code.
If there is a new feature which would break KVM implementation then it
will easy to notice, and easy to block that bit in the KVM at that time.
But if the new feature is not affecting KVM then it will not be noticed,
and KVM will end up unsupporting that cpu feature.
Having an extra cpuid bit means that a guest will crash, while missing
one slows it down a bit. We need to be conservative and not risk
passing incorrect data.
get_cpu() before
@@ -1246,6 +1224,7 @@ static void do_cpuid_ent(struct kvm_cpuid_entry2 *entry,
u32 function,
int t, times = entry->eax & 0xff;
entry->flags |= KVM_CPUID_FLAG_STATEFUL_FUNC;
+ entry->flags |= KVM_CPUID_FLAG_STATE_READ_NEXT;
for (t = 1; t < times && *nent < maxnent; ++t) {
do_cpuid_1_ent(&entry[t], function, 0);
entry[t].flags |= KVM_CPUID_FLAG_STATEFUL_FUNC;
Why?
This is a bug fix. Without this change the code does not find a leaf 2
entry in the list.
Please send it as a separate patch, with a changelog entry describing
why it is needed.
@@ -1276,7 +1255,7 @@ static void do_cpuid_ent(struct kvm_cpuid_entry2 *entry,
u32 function,
entry->flags |= KVM_CPUID_FLAG_SIGNIFCANT_INDEX;
/* read more entries until level_type is zero */
for (i = 1; *nent < maxnent; ++i) {
- level_type = entry[i - 1].ecx & 0xff;
+ level_type = entry[i - 1].ecx & 0xff00;
if (!level_type)
break;
do_cpuid_1_ent(&entry[i], function, i);
Why?
This is also a bugfix. The type field is in bits 8-15. As per the code,
the bits 0-7 would not define the end of counting. You can look at IA32
processor Software developer's manual for this.
Separate patch, please.
@@ -1302,10 +1281,14 @@ static int kvm_dev_ioctl_get_supported_cpuid(struct
kvm_cpuid2 *cpuid,
{
struct kvm_cpuid_entry2 *cpuid_entries;
int limit, nent = 0, r = -E2BIG;
+ int sizer = 0;
u32 func;
- if (cpuid->nent < 1)
- goto out;
+ if (cpuid->nent == 0) {
+ sizer = 1;
+ cpuid->nent = KVM_MAX_CPUID_ENTRIES;
+ }
+
That's an ABI change. New userspace could call an older kernel with
nent = 0, and get an unexpected response.
That is right. What do you suggest for solution? Currently this ABI is
not utilized. Would adding another ioctl would work?
We could have the response to KVM_CHECK_EXTENSION(KVM_CAP_CPUID2 (or
however it's called)) return the number of entries, and document that 1
means "unknown" (well, we could have done it with your proposal too).
@@ -2860,6 +2845,13 @@ void kvm_emulate_cpuid(struct kvm_vcpu *vcpu)
kvm_register_write(vcpu, VCPU_REGS_RCX, best->ecx);
kvm_register_write(vcpu, VCPU_REGS_RDX, best->edx);
}
+ if (function == 0x1) { /* set the per-cpu apicid */
+ u32 ebx = kvm_register_read(vcpu, VCPU_REGS_RBX);
+ ebx &= 0x00ffffff;
+ ebx |= (vcpu->vcpu_id << 24);
+ kvm_register_write(vcpu, VCPU_REGS_RBX, ebx);
+ }
+
ABI change again. This is userspace's job.
This does not look like an ABI change to me.
Right, it isn't an ABI change. But we should avoid situations where the
same call does different things in different kernel versions.
I will look if vcpu_id is
available in the userspace, if yes, then this code can be moved into
userspace.
Yes, it's available in userspace.
--
I have a truly marvellous patch that fixes the bug which this
signature is too narrow to contain.
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
