| Message ID | 1390236074-21533-5-git-send-email-ehabkost@redhat.com |
|---|---|
| State | New |
| Headers | show |
Am 20.01.2014 17:41, schrieb Eduardo Habkost: > There's no need to check CPU xlevel2 before calling > kvm_arch_get_supported_cpuid(s, 0xC0000001, 0, R_EDX), because: > > * The kernel won't return any entry for 0xC0000000 if host CPU vendor > is not Centaur (See kvm_dev_ioctl_get_supported_cpuid() on the kernel > code) > * Similarly, the kernel won't return any entry for 0xC0000001 if > CPUID[0xC0000000].EAX is < 0xC0000001 > * kvm_arch_get_supported_cpuid() will return 0 if no entry is returned > by the kernel for the requested leaf > > For similar reasons, we can simply set x86_cpu_def->xlevel2 directly > instead of making it conditional, because it will be set to 0 CPU vendor > is not Centaur. Commented on the wrong version: "... if CPU vendor is not ..." I guess. :) > > This will simplify the kvm_cpu_fill_host() code a little. > > Signed-off-by: Eduardo Habkost <ehabkost@redhat.com> > --- > target-i386/cpu.c | 12 +++++------- > 1 file changed, 5 insertions(+), 7 deletions(-) > > diff --git a/target-i386/cpu.c b/target-i386/cpu.c > index 1a86bcf..8bc72c2 100644 > --- a/target-i386/cpu.c > +++ b/target-i386/cpu.c > @@ -1173,13 +1173,11 @@ static void kvm_cpu_fill_host(x86_def_t *x86_cpu_def) > cpu_x86_fill_model_id(x86_cpu_def->model_id); > > /* Call Centaur's CPUID instruction. */ > - eax = kvm_arch_get_supported_cpuid(s, 0xC0000000, 0, R_EAX); > - if (eax >= 0xC0000001) { > - /* Support VIA max extended level */ > - x86_cpu_def->xlevel2 = eax; > - x86_cpu_def->features[FEAT_C000_0001_EDX] = > - kvm_arch_get_supported_cpuid(s, 0xC0000001, 0, R_EDX); > - } > + x86_cpu_def->xlevel2 = > + kvm_arch_get_supported_cpuid(s, 0xC0000000, 0, R_EAX); > + /* Support VIA max extended level */ > + x86_cpu_def->features[FEAT_C000_0001_EDX] = > + kvm_arch_get_supported_cpuid(s, 0xC0000001, 0, R_EDX); > > /* Other KVM-specific feature fields: */ > x86_cpu_def->features[FEAT_SVM] = Otherwise looks okay to me. Andreas
Il 20/01/2014 17:41, Eduardo Habkost ha scritto: > - eax = kvm_arch_get_supported_cpuid(s, 0xC0000000, 0, R_EAX); > - if (eax >= 0xC0000001) { > - /* Support VIA max extended level */ > - x86_cpu_def->xlevel2 = eax; > - x86_cpu_def->features[FEAT_C000_0001_EDX] = > - kvm_arch_get_supported_cpuid(s, 0xC0000001, 0, R_EDX); > - } > + x86_cpu_def->xlevel2 = > + kvm_arch_get_supported_cpuid(s, 0xC0000000, 0, R_EAX); > + /* Support VIA max extended level */ > + x86_cpu_def->features[FEAT_C000_0001_EDX] = > + kvm_arch_get_supported_cpuid(s, 0xC0000001, 0, R_EDX); > I'm removing this comment before applying the patch (I believe the comment was not really grammatical, and it meant something like "VIA max extended level supported"). Paolo
diff --git a/target-i386/cpu.c b/target-i386/cpu.c index 1a86bcf..8bc72c2 100644 --- a/target-i386/cpu.c +++ b/target-i386/cpu.c @@ -1173,13 +1173,11 @@ static void kvm_cpu_fill_host(x86_def_t *x86_cpu_def) cpu_x86_fill_model_id(x86_cpu_def->model_id); /* Call Centaur's CPUID instruction. */ - eax = kvm_arch_get_supported_cpuid(s, 0xC0000000, 0, R_EAX); - if (eax >= 0xC0000001) { - /* Support VIA max extended level */ - x86_cpu_def->xlevel2 = eax; - x86_cpu_def->features[FEAT_C000_0001_EDX] = - kvm_arch_get_supported_cpuid(s, 0xC0000001, 0, R_EDX); - } + x86_cpu_def->xlevel2 = + kvm_arch_get_supported_cpuid(s, 0xC0000000, 0, R_EAX); + /* Support VIA max extended level */ + x86_cpu_def->features[FEAT_C000_0001_EDX] = + kvm_arch_get_supported_cpuid(s, 0xC0000001, 0, R_EDX); /* Other KVM-specific feature fields: */ x86_cpu_def->features[FEAT_SVM] =
There's no need to check CPU xlevel2 before calling kvm_arch_get_supported_cpuid(s, 0xC0000001, 0, R_EDX), because: * The kernel won't return any entry for 0xC0000000 if host CPU vendor is not Centaur (See kvm_dev_ioctl_get_supported_cpuid() on the kernel code) * Similarly, the kernel won't return any entry for 0xC0000001 if CPUID[0xC0000000].EAX is < 0xC0000001 * kvm_arch_get_supported_cpuid() will return 0 if no entry is returned by the kernel for the requested leaf For similar reasons, we can simply set x86_cpu_def->xlevel2 directly instead of making it conditional, because it will be set to 0 CPU vendor is not Centaur. This will simplify the kvm_cpu_fill_host() code a little. Signed-off-by: Eduardo Habkost <ehabkost@redhat.com> --- target-i386/cpu.c | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-)