Re: [PATCH v2 05/10] target/i386/kvm: extract unrelated code out of kvm_x86_build_cpuid()

[Zhao Liu <zhao1.liu@intel.com>]

> +static void kvm_init_pmu_info(CPUX86State *env)
> +{
> +    uint32_t eax, edx;
> +    uint32_t unused;
> +    uint32_t limit;
> +
> +    cpu_x86_cpuid(env, 0, 0, &limit, &unused, &unused, &unused);

At this stage, CPUID has already been filled and we should not use
cpu_x86_cpuid() to get the "raw" CPUID info.

Instead, after kvm_x86_build_cpuid(), the cpuid_find_entry() helper
should be preferred.

With cpuid_find_entry(), we don't even need to check the limit again.

> +
> +    if (limit < 0x0a) {
> +        return;
> +    }

...

>  int kvm_arch_init_vcpu(CPUState *cs)
>  {
>      struct {
> @@ -2267,6 +2277,8 @@ int kvm_arch_init_vcpu(CPUState *cs)
>      cpuid_i = kvm_x86_build_cpuid(env, cpuid_data.entries, cpuid_i);
>      cpuid_data.cpuid.nent = cpuid_i;
>  
> +    kvm_init_pmu_info(env);
> +

Referring what has_msr_feature_control did, what about the following
change?

 int kvm_arch_init_vcpu(CPUState *cs)
 {
     struct {
@@ -2277,8 +2240,6 @@ int kvm_arch_init_vcpu(CPUState *cs)
     cpuid_i = kvm_x86_build_cpuid(env, cpuid_data.entries, cpuid_i);
     cpuid_data.cpuid.nent = cpuid_i;

-    kvm_init_pmu_info(env);
-
     if (((env->cpuid_version >> 8)&0xF) >= 6
         && (env->features[FEAT_1_EDX] & (CPUID_MCE | CPUID_MCA)) ==
            (CPUID_MCE | CPUID_MCA)) {
@@ -2329,6 +2290,31 @@ int kvm_arch_init_vcpu(CPUState *cs)
         has_msr_feature_control = true;
     }

+    c = cpuid_find_entry(&cpuid_data.cpuid, 0xa, 0);
+    if (c) {
+        has_architectural_pmu_version = c->eax & 0xff;
+        if (has_architectural_pmu_version > 0) {
+            num_architectural_pmu_gp_counters = (c->eax & 0xff00) >> 8;
+
+            /*
+             * Shouldn't be more than 32, since that's the number of bits
+             * available in EBX to tell us _which_ counters are available.
+             * Play it safe.
+             */
+            if (num_architectural_pmu_gp_counters > MAX_GP_COUNTERS) {
+                num_architectural_pmu_gp_counters = MAX_GP_COUNTERS;
+            }
+
+            if (has_architectural_pmu_version > 1) {
+                num_architectural_pmu_fixed_counters = c->edx & 0x1f;
+
+                if (num_architectural_pmu_fixed_counters > MAX_FIXED_COUNTERS) {
+                    num_architectural_pmu_fixed_counters = MAX_FIXED_COUNTERS;
+                }
+            }
+        }
+    }
+
     if (env->mcg_cap & MCG_LMCE_P) {
         has_msr_mcg_ext_ctl = has_msr_feature_control = true;
     }
---

The above codes check 0xa after 0x1 and 0x7, and uses the local variable
`c`, so that it doesn't need to wrap another new function.

Regards,
Zhao