Re: [PATCH 2/4] i386/cpu: Add PerfMonV2 feature bit

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

On Wed, Jun 12, 2024 at 02:12:18PM -0500, Babu Moger wrote:
> Date: Wed, 12 Jun 2024 14:12:18 -0500
> From: Babu Moger <babu.moger@amd.com>
> Subject: [PATCH 2/4] i386/cpu: Add PerfMonV2 feature bit
> X-Mailer: git-send-email 2.34.1
> 
> From: Sandipan Das <sandipan.das@amd.com>
> 
> CPUID leaf 0x80000022, i.e. ExtPerfMonAndDbg, advertises new performance
> monitoring features for AMD processors. Bit 0 of EAX indicates support
> for Performance Monitoring Version 2 (PerfMonV2) features. If found to
> be set during PMU initialization, the EBX bits can be used to determine
> the number of available counters for different PMUs. It also denotes the
> availability of global control and status registers.
> 
> Add the required CPUID feature word and feature bit to allow guests to
> make use of the PerfMonV2 features.
> 
> Signed-off-by: Sandipan Das <sandipan.das@amd.com>
> Signed-off-by: Babu Moger <babu.moger@amd.com>
> ---
>  target/i386/cpu.c | 26 ++++++++++++++++++++++++++
>  target/i386/cpu.h |  4 ++++
>  2 files changed, 30 insertions(+)
> 
> diff --git a/target/i386/cpu.c b/target/i386/cpu.c
> index 86a90b1405..7f1837cdc9 100644
> --- a/target/i386/cpu.c
> +++ b/target/i386/cpu.c
> @@ -1228,6 +1228,22 @@ FeatureWordInfo feature_word_info[FEATURE_WORDS] = {
>          .tcg_features = 0,
>          .unmigratable_flags = 0,
>      },
> +    [FEAT_8000_0022_EAX] = {
> +        .type = CPUID_FEATURE_WORD,
> +        .feat_names = {
> +            "perfmon-v2", 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 = 0x80000022, .reg = R_EAX, },
> +        .tcg_features = 0,
> +        .unmigratable_flags = 0,
> +    },
>      [FEAT_XSAVE] = {
>          .type = CPUID_FEATURE_WORD,
>          .feat_names = {
> @@ -6998,6 +7014,16 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index, uint32_t count,
>              *edx = 0;
>          }
>          break;
> +    case 0x80000022:
> +        *eax = *ebx = *ecx = *edx = 0;
> +        /* AMD Extended Performance Monitoring and Debug */
> +        if (kvm_enabled() && cpu->enable_pmu &&
> +            (env->features[FEAT_8000_0022_EAX] & CPUID_8000_0022_EAX_PERFMON_V2)) {
> +            *eax = CPUID_8000_0022_EAX_PERFMON_V2;
> +            *ebx = kvm_arch_get_supported_cpuid(cs->kvm_state, index, count,
> +                                                R_EBX) & 0xf;

Although only EAX[bit 0] and EBX[bits 0-3] are supported right now, I
think it's better to use “|=” rather than just override the
original *eax and *ebx, which will prevent future mistakes or omissions.

Otherwise,

Reviewed-by: Zhao Liu <zhao1.liu@intel.com>