Re: [PATCH v3 09/11] KVM: selftests: Add x86 feature and properties for AMD PMU in processor.h

From: Sean Christopherson
Date: Thu Aug 17 2023 - 19:27:02 EST


On Mon, Aug 14, 2023, Jinrong Liang wrote:
> From: Jinrong Liang <cloudliang@xxxxxxxxxxx>
>
> Add x86 feature and properties for AMD PMU so that tests don't have
> to manually retrieve the correct CPUID leaf+register, and so that the
> resulting code is self-documenting.
>
> Signed-off-by: Jinrong Liang <cloudliang@xxxxxxxxxxx>
> ---
> tools/testing/selftests/kvm/include/x86_64/processor.h | 7 +++++++
> 1 file changed, 7 insertions(+)
>
> diff --git a/tools/testing/selftests/kvm/include/x86_64/processor.h b/tools/testing/selftests/kvm/include/x86_64/processor.h
> index 6b146e1c6736..07b980b8bec2 100644
> --- a/tools/testing/selftests/kvm/include/x86_64/processor.h
> +++ b/tools/testing/selftests/kvm/include/x86_64/processor.h
> @@ -167,6 +167,7 @@ struct kvm_x86_cpu_feature {
> */
> #define X86_FEATURE_SVM KVM_X86_CPU_FEATURE(0x80000001, 0, ECX, 2)
> #define X86_FEATURE_NX KVM_X86_CPU_FEATURE(0x80000001, 0, EDX, 20)
> +#define X86_FEATURE_AMD_PMU_EXT_CORE KVM_X86_CPU_FEATURE(0x80000001, 0, ECX, 23)

I think it makes sense to follow the kernel, even though I find the kernel name
a bit funky in this case. I.e. X86_FEATURE_PERFCTR_CORE

> #define X86_FEATURE_GBPAGES KVM_X86_CPU_FEATURE(0x80000001, 0, EDX, 26)
> #define X86_FEATURE_RDTSCP KVM_X86_CPU_FEATURE(0x80000001, 0, EDX, 27)
> #define X86_FEATURE_LM KVM_X86_CPU_FEATURE(0x80000001, 0, EDX, 29)
> @@ -182,6 +183,9 @@ struct kvm_x86_cpu_feature {
> #define X86_FEATURE_VGIF KVM_X86_CPU_FEATURE(0x8000000A, 0, EDX, 16)
> #define X86_FEATURE_SEV KVM_X86_CPU_FEATURE(0x8000001F, 0, EAX, 1)
> #define X86_FEATURE_SEV_ES KVM_X86_CPU_FEATURE(0x8000001F, 0, EAX, 3)
> +#define X86_FEATURE_AMD_PERFMON_V2 KVM_X86_CPU_FEATURE(0x80000022, 0, EAX, 0)

X86_FEATURE_PERFMON_V2

> +#define X86_FEATURE_AMD_LBR_STACK KVM_X86_CPU_FEATURE(0x80000022, 0, EAX, 1)

Eh, I'd drop the "AMD".

> +#define X86_FEATURE_AMD_LBR_PMC_FREEZE KVM_X86_CPU_FEATURE(0x80000022, 0, EAX, 2)

Same here.

> /*
> * KVM defined paravirt features.
> @@ -267,6 +271,9 @@ struct kvm_x86_cpu_property {
> #define X86_PROPERTY_MAX_VIRT_ADDR KVM_X86_CPU_PROPERTY(0x80000008, 0, EAX, 8, 15)
> #define X86_PROPERTY_PHYS_ADDR_REDUCTION KVM_X86_CPU_PROPERTY(0x8000001F, 0, EBX, 6, 11)
>
> +#define X86_PROPERTY_AMD_PMU_NR_CORE_COUNTERS KVM_X86_CPU_PROPERTY(0x80000022, 0, EBX, 0, 3)
> +#define X86_PROPERTY_AMD_PMU_LBR_STACK_SIZE KVM_X86_CPU_PROPERTY(0x80000022, 0, EBX, 4, 9)

And here. I think we can get away with "GP and fixed counters means Intel, core
counters means AMD", at least as far as X86_FEATURES go. LBRs might be a different
story, but so long as there's no conflict....

Splattering AMD everywhere but not Intel everywhere bugs me :-)

> +
> #define X86_PROPERTY_MAX_CENTAUR_LEAF KVM_X86_CPU_PROPERTY(0xC0000000, 0, EAX, 0, 31)
>
> /*
> --
> 2.39.3
>