Re: [PATCH v2 1/1] target/i386: Add support for KVM APERF/MPERF passthrough

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

Hi Anderson,

> +    [FEAT_6_ECX] = {
> +        .type = CPUID_FEATURE_WORD,
> +        .feat_names = {
> +            "aperfmperf", NULL, NULL, NULL,
> +            NULL, NULL, NULL, NULL,
> +            NULL, NULL, NULL, NULL,
> +            NULL, NULL, NULL, NULL,
> +            NULL, NULL, NULL, NULL,
> +            NULL, NULL, NULL, NULL,
> +            NULL, NULL, NULL, NULL,
> +            NULL, NULL, NULL, NULL,
> +        },
> +        .cpuid = { .eax = 6, .reg = R_ECX, },
> +        .tcg_features = 0,
> +        .unmigratable_flags = CPUID_6_ECX_APERFMPERF,

Considerring APER/MPERF passthrough have many specific restrictions
(listed in KVM's api.rst, "7.13 KVM_CAP_X86_DISABLE_EXITS"), I think
we also need not to enable it by default for max/host CPUs:

.no_autoenable_flags = CPUID_6_ECX_APERFMPERF

This indicates that users must manually enable this feature, and it is
assumed that they are aware of its limitations and have implemented
appropriate policies (such as no preemption, vCPU affinity, no idle or
C state in the guest, etc.).

> +    },

...

>  /* CPUID[0x80000007].EDX flags: */
>  #define CPUID_APM_INVTSC       (1U << 8)
> diff --git a/target/i386/kvm/kvm.c b/target/i386/kvm/kvm.c
> index 9e352882c8..ca722ff9e9 100644
> --- a/target/i386/kvm/kvm.c
> +++ b/target/i386/kvm/kvm.c
> @@ -498,6 +498,8 @@ uint32_t kvm_arch_get_supported_cpuid(KVMState *s, uint32_t function,
>          }
>      } else if (function == 6 && reg == R_EAX) {
>          ret |= CPUID_6_EAX_ARAT; /* safe to allow because of emulated APIC */
> +    } else if (function == 6 && reg == R_ECX) {
> +        ret |= CPUID_6_ECX_APERFMPERF;

if we bind this feature to cpu-pm, then we should check enable_cpu_pm
before setting CPUID. Otherwise, if this CPUID is present but
cpu-pm=off, Guest may try to access related MSRs and triggers #GP.

        if (enable_cpu_pm) {
            int disable_exits = kvm_check_extension(s,
                                                    KVM_CAP_X86_DISABLE_EXITS);

            if (disable_exits & KVM_X86_DISABLE_EXITS_APERFMPERF) {
                ret |= CPUID_6_ECX_APERFMPERF;
            }
        }

But on the other hande, maybe we don't need to bind this feature to
cpu-pm?

For example, a compute-intensive workload doesn't allow the Guest to
enter idle, then it still seems make sense to estimate performance
using aperf and mperf?

Thanks,
Zhao