2022-11-11 10:32:18

by Like Xu

Subject: [PATCH v3 5/8] KVM: x86/cpuid: Add X86_FEATURE_AMD_PMU_V2 as a KVM-only leaf entry

From: Like Xu <[email protected]>

Alias X86_FEATURE_AMD_PMU_V2 for feature AMD_PMU_V2 in KVM-only leafs that
aren't scattered by cpufeatures.h so that it can be used in KVM, e.g. to
query guest CPUID. As a bonus, no translation is needed for these features
in __feature_translate().

Suggested-by: Sean Christopherson <[email protected]>
Signed-off-by: Like Xu <[email protected]>
---
arch/x86/kvm/reverse_cpuid.h | 10 ++++++++++
1 file changed, 10 insertions(+)

diff --git a/arch/x86/kvm/reverse_cpuid.h b/arch/x86/kvm/reverse_cpuid.h
index a19d473d0184..7cfedb3e47c0 100644
--- a/arch/x86/kvm/reverse_cpuid.h
+++ b/arch/x86/kvm/reverse_cpuid.h
@@ -13,6 +13,7 @@
*/
enum kvm_only_cpuid_leafs {
CPUID_12_EAX = NCAPINTS,
+ CPUID_8000_0022_EAX,
NR_KVM_CPU_CAPS,

NKVMCAPINTS = NR_KVM_CPU_CAPS - NCAPINTS,
@@ -23,7 +24,15 @@ enum kvm_only_cpuid_leafs {
/* Intel-defined SGX sub-features, CPUID level 0x12 (EAX). */
#define KVM_X86_FEATURE_SGX1 KVM_X86_FEATURE(CPUID_12_EAX, 0)
#define KVM_X86_FEATURE_SGX2 KVM_X86_FEATURE(CPUID_12_EAX, 1)
+#define KVM_X86_FEATURE_AMD_PMU_V2 KVM_X86_FEATURE(CPUID_8000_0022_EAX, 0)

+/*
+ * Alias X86_FEATURE_* to the KVM variant for features in KVM-only leafs that
+ * aren't scattered by cpufeatures.h so that X86_FEATURE_* can be used in KVM,
+ * e.g. to query guest CPUID. As a bonus, no translation is needed for these
+ * features in __feature_translate().
+ */
+#define X86_FEATURE_AMD_PMU_V2 KVM_X86_FEATURE_AMD_PMU_V2
struct cpuid_reg {
u32 function;
u32 index;
@@ -48,6 +57,7 @@ static const struct cpuid_reg reverse_cpuid[] = {
[CPUID_7_1_EAX] = { 7, 1, CPUID_EAX},
[CPUID_12_EAX] = {0x00000012, 0, CPUID_EAX},
[CPUID_8000_001F_EAX] = {0x8000001f, 0, CPUID_EAX},
+ [CPUID_8000_0022_EAX] = {0x80000022, 0, CPUID_EAX},
};

/*
--
2.38.1

2023-01-24 19:47:54

by Sean Christopherson

Subject: Re: [PATCH v3 5/8] KVM: x86/cpuid: Add X86_FEATURE_AMD_PMU_V2 as a KVM-only leaf entry

On Fri, Nov 11, 2022, Like Xu wrote:
> From: Like Xu <[email protected]>
>
> Alias X86_FEATURE_AMD_PMU_V2 for feature AMD_PMU_V2 in KVM-only leafs that
> aren't scattered by cpufeatures.h so that it can be used in KVM, e.g. to
> query guest CPUID. As a bonus, no translation is needed for these features
> in __feature_translate().
>
> Suggested-by: Sean Christopherson <[email protected]>
> Signed-off-by: Like Xu <[email protected]>
> ---
> arch/x86/kvm/reverse_cpuid.h | 10 ++++++++++
> 1 file changed, 10 insertions(+)
>
> diff --git a/arch/x86/kvm/reverse_cpuid.h b/arch/x86/kvm/reverse_cpuid.h
> index a19d473d0184..7cfedb3e47c0 100644
> --- a/arch/x86/kvm/reverse_cpuid.h
> +++ b/arch/x86/kvm/reverse_cpuid.h
> @@ -13,6 +13,7 @@
> */
> enum kvm_only_cpuid_leafs {
> CPUID_12_EAX = NCAPINTS,
> + CPUID_8000_0022_EAX,
> NR_KVM_CPU_CAPS,
>
> NKVMCAPINTS = NR_KVM_CPU_CAPS - NCAPINTS,
> @@ -23,7 +24,15 @@ enum kvm_only_cpuid_leafs {
> /* Intel-defined SGX sub-features, CPUID level 0x12 (EAX). */
> #define KVM_X86_FEATURE_SGX1 KVM_X86_FEATURE(CPUID_12_EAX, 0)
> #define KVM_X86_FEATURE_SGX2 KVM_X86_FEATURE(CPUID_12_EAX, 1)
> +#define KVM_X86_FEATURE_AMD_PMU_V2 KVM_X86_FEATURE(CPUID_8000_0022_EAX, 0)
>
> +/*
> + * Alias X86_FEATURE_* to the KVM variant for features in KVM-only leafs that
> + * aren't scattered by cpufeatures.h so that X86_FEATURE_* can be used in KVM,
> + * e.g. to query guest CPUID. As a bonus, no translation is needed for these
> + * features in __feature_translate().
> + */
> +#define X86_FEATURE_AMD_PMU_V2 KVM_X86_FEATURE_AMD_PMU_V2

I gave you bad input earlier, for purely KVM-defined flags there's no need for an
intermediate KVM_X86_FEATURE_AMD_PMU_V2, this could simply be:

#define X86_FEATURE_AMD_PMU_V2 KVM_X86_FEATURE(CPUID_8000_0022_EAX, 0)

That's a moot point though because, after much searching because I had a very hard
time believing the kernel wouldn't want to know about this flag, I found commit

d6d0c7f681fd ("x86/cpufeatures: Add PerfMonV2 feature bit")

from nearly a year ago. I.e. to avoid confusiong, this needs to be a scattered
flag, not a purely KVM flag.

---
arch/x86/kvm/reverse_cpuid.h | 7 +++++++
1 file changed, 7 insertions(+)

diff --git a/arch/x86/kvm/reverse_cpuid.h b/arch/x86/kvm/reverse_cpuid.h
index 4945456fd646..333e28b0a13c 100644
--- a/arch/x86/kvm/reverse_cpuid.h
+++ b/arch/x86/kvm/reverse_cpuid.h
@@ -15,6 +15,7 @@ enum kvm_only_cpuid_leafs {
CPUID_12_EAX = NCAPINTS,
CPUID_7_1_EDX,
CPUID_8000_0007_EDX,
+ CPUID_8000_0022_EAX,
NR_KVM_CPU_CAPS,

NKVMCAPINTS = NR_KVM_CPU_CAPS - NCAPINTS,
@@ -47,6 +48,9 @@ enum kvm_only_cpuid_leafs {
/* CPUID level 0x80000007 (EDX). */
#define KVM_X86_FEATURE_CONSTANT_TSC KVM_X86_FEATURE(CPUID_8000_0007_EDX, 8)

+/* CPUID level 0x80000022 (EAX) */
+#define KVM_X86_FEATURE_PERFMON_V2 KVM_X86_FEATURE(CPUID_8000_0022_EAX, 0)
+
struct cpuid_reg {
u32 function;
u32 index;
@@ -73,6 +77,7 @@ static const struct cpuid_reg reverse_cpuid[] = {
[CPUID_8000_001F_EAX] = {0x8000001f, 0, CPUID_EAX},
[CPUID_7_1_EDX] = { 7, 1, CPUID_EDX},
[CPUID_8000_0007_EDX] = {0x80000007, 0, CPUID_EDX},
+ [CPUID_8000_0022_EAX] = {0x80000022, 0, CPUID_EAX},
};

/*
@@ -107,6 +112,8 @@ static __always_inline u32 __feature_translate(int x86_feature)
return KVM_X86_FEATURE_SGX_EDECCSSA;
else if (x86_feature == X86_FEATURE_CONSTANT_TSC)
return KVM_X86_FEATURE_CONSTANT_TSC;
+ else if (x86_feature == X86_FEATURE_PERFMON_V2)
+ return KVM_X86_FEATURE_PERFMON_V2;

return x86_feature;
}

base-commit: 5f3f3cc1279cd5cd52d301b97844bd3ce40c8020
--

2023-02-06 11:47:26

by Like Xu

Subject: Re: [PATCH v3 5/8] KVM: x86/cpuid: Add X86_FEATURE_AMD_PMU_V2 as a KVM-only leaf entry

On 25/1/2023 3:47 am, Sean Christopherson wrote:
> On Fri, Nov 11, 2022, Like Xu wrote:
>> From: Like Xu <[email protected]>
>>
>> Alias X86_FEATURE_AMD_PMU_V2 for feature AMD_PMU_V2 in KVM-only leafs that
>> aren't scattered by cpufeatures.h so that it can be used in KVM, e.g. to
>> query guest CPUID. As a bonus, no translation is needed for these features
>> in __feature_translate().
>>
>> Suggested-by: Sean Christopherson <[email protected]>
>> Signed-off-by: Like Xu <[email protected]>
>> ---
>> arch/x86/kvm/reverse_cpuid.h | 10 ++++++++++
>> 1 file changed, 10 insertions(+)
>>
>> diff --git a/arch/x86/kvm/reverse_cpuid.h b/arch/x86/kvm/reverse_cpuid.h
>> index a19d473d0184..7cfedb3e47c0 100644
>> --- a/arch/x86/kvm/reverse_cpuid.h
>> +++ b/arch/x86/kvm/reverse_cpuid.h
>> @@ -13,6 +13,7 @@
>> */
>> enum kvm_only_cpuid_leafs {
>> CPUID_12_EAX = NCAPINTS,
>> + CPUID_8000_0022_EAX,
>> NR_KVM_CPU_CAPS,
>>
>> NKVMCAPINTS = NR_KVM_CPU_CAPS - NCAPINTS,
>> @@ -23,7 +24,15 @@ enum kvm_only_cpuid_leafs {
>> /* Intel-defined SGX sub-features, CPUID level 0x12 (EAX). */
>> #define KVM_X86_FEATURE_SGX1 KVM_X86_FEATURE(CPUID_12_EAX, 0)
>> #define KVM_X86_FEATURE_SGX2 KVM_X86_FEATURE(CPUID_12_EAX, 1)
>> +#define KVM_X86_FEATURE_AMD_PMU_V2 KVM_X86_FEATURE(CPUID_8000_0022_EAX, 0)
>>
>> +/*
>> + * Alias X86_FEATURE_* to the KVM variant for features in KVM-only leafs that
>> + * aren't scattered by cpufeatures.h so that X86_FEATURE_* can be used in KVM,
>> + * e.g. to query guest CPUID. As a bonus, no translation is needed for these
>> + * features in __feature_translate().
>> + */
>> +#define X86_FEATURE_AMD_PMU_V2 KVM_X86_FEATURE_AMD_PMU_V2
>
> I gave you bad input earlier, for purely KVM-defined flags there's no need for an
> intermediate KVM_X86_FEATURE_AMD_PMU_V2, this could simply be:
>
> #define X86_FEATURE_AMD_PMU_V2 KVM_X86_FEATURE(CPUID_8000_0022_EAX, 0)
>
> That's a moot point though because, after much searching because I had a very hard
> time believing the kernel wouldn't want to know about this flag, I found commit
>
> d6d0c7f681fd ("x86/cpufeatures: Add PerfMonV2 feature bit")
>
> from nearly a year ago. I.e. to avoid confusiong, this needs to be a scattered
> flag, not a purely KVM flag.
>
> ---
> arch/x86/kvm/reverse_cpuid.h | 7 +++++++
> 1 file changed, 7 insertions(+)
>
> diff --git a/arch/x86/kvm/reverse_cpuid.h b/arch/x86/kvm/reverse_cpuid.h
> index 4945456fd646..333e28b0a13c 100644
> --- a/arch/x86/kvm/reverse_cpuid.h
> +++ b/arch/x86/kvm/reverse_cpuid.h
> @@ -15,6 +15,7 @@ enum kvm_only_cpuid_leafs {
> CPUID_12_EAX = NCAPINTS,
> CPUID_7_1_EDX,
> CPUID_8000_0007_EDX,
> + CPUID_8000_0022_EAX,
> NR_KVM_CPU_CAPS,
>
> NKVMCAPINTS = NR_KVM_CPU_CAPS - NCAPINTS,
> @@ -47,6 +48,9 @@ enum kvm_only_cpuid_leafs {
> /* CPUID level 0x80000007 (EDX). */
> #define KVM_X86_FEATURE_CONSTANT_TSC KVM_X86_FEATURE(CPUID_8000_0007_EDX, 8)
>
> +/* CPUID level 0x80000022 (EAX) */
> +#define KVM_X86_FEATURE_PERFMON_V2 KVM_X86_FEATURE(CPUID_8000_0022_EAX, 0)

I'm very confused and is this the usage you want to see:

kvm_cpu_cap_set(KVM_X86_FEATURE_PERFMON_V2)
kvm_cpu_cap_has(KVM_X86_FEATURE_PERFMON_V2)
guest_cpuid_has(vcpu, X86_FEATURE_PERFMON_V2)

? then what about X86_FEATURE_PERFMON_V2 ?

> +
> struct cpuid_reg {
> u32 function;
> u32 index;
> @@ -73,6 +77,7 @@ static const struct cpuid_reg reverse_cpuid[] = {
> [CPUID_8000_001F_EAX] = {0x8000001f, 0, CPUID_EAX},
> [CPUID_7_1_EDX] = { 7, 1, CPUID_EDX},
> [CPUID_8000_0007_EDX] = {0x80000007, 0, CPUID_EDX},
> + [CPUID_8000_0022_EAX] = {0x80000022, 0, CPUID_EAX},
> };
>
> /*
> @@ -107,6 +112,8 @@ static __always_inline u32 __feature_translate(int x86_feature)
> return KVM_X86_FEATURE_SGX_EDECCSSA;
> else if (x86_feature == X86_FEATURE_CONSTANT_TSC)
> return KVM_X86_FEATURE_CONSTANT_TSC;
> + else if (x86_feature == X86_FEATURE_PERFMON_V2)
> + return KVM_X86_FEATURE_PERFMON_V2;
>
> return x86_feature;
> }
>
> base-commit: 5f3f3cc1279cd5cd52d301b97844bd3ce40c8020

2023-02-10 01:32:28

by Sean Christopherson

Subject: Re: [PATCH v3 5/8] KVM: x86/cpuid: Add X86_FEATURE_AMD_PMU_V2 as a KVM-only leaf entry

On Mon, Feb 06, 2023, Like Xu wrote:
> On 25/1/2023 3:47 am, Sean Christopherson wrote:
> > On Fri, Nov 11, 2022, Like Xu wrote:
> > > From: Like Xu <[email protected]>
> > >
> > > Alias X86_FEATURE_AMD_PMU_V2 for feature AMD_PMU_V2 in KVM-only leafs that
> > > aren't scattered by cpufeatures.h so that it can be used in KVM, e.g. to
> > > query guest CPUID. As a bonus, no translation is needed for these features
> > > in __feature_translate().
> > >
> > > Suggested-by: Sean Christopherson <[email protected]>
> > > Signed-off-by: Like Xu <[email protected]>
> > > ---
> > > arch/x86/kvm/reverse_cpuid.h | 10 ++++++++++
> > > 1 file changed, 10 insertions(+)
> > >
> > > diff --git a/arch/x86/kvm/reverse_cpuid.h b/arch/x86/kvm/reverse_cpuid.h
> > > index a19d473d0184..7cfedb3e47c0 100644
> > > --- a/arch/x86/kvm/reverse_cpuid.h
> > > +++ b/arch/x86/kvm/reverse_cpuid.h
> > > @@ -13,6 +13,7 @@
> > > */
> > > enum kvm_only_cpuid_leafs {
> > > CPUID_12_EAX = NCAPINTS,
> > > + CPUID_8000_0022_EAX,
> > > NR_KVM_CPU_CAPS,
> > > NKVMCAPINTS = NR_KVM_CPU_CAPS - NCAPINTS,
> > > @@ -23,7 +24,15 @@ enum kvm_only_cpuid_leafs {
> > > /* Intel-defined SGX sub-features, CPUID level 0x12 (EAX). */
> > > #define KVM_X86_FEATURE_SGX1 KVM_X86_FEATURE(CPUID_12_EAX, 0)
> > > #define KVM_X86_FEATURE_SGX2 KVM_X86_FEATURE(CPUID_12_EAX, 1)
> > > +#define KVM_X86_FEATURE_AMD_PMU_V2 KVM_X86_FEATURE(CPUID_8000_0022_EAX, 0)
> > > +/*
> > > + * Alias X86_FEATURE_* to the KVM variant for features in KVM-only leafs that
> > > + * aren't scattered by cpufeatures.h so that X86_FEATURE_* can be used in KVM,
> > > + * e.g. to query guest CPUID. As a bonus, no translation is needed for these
> > > + * features in __feature_translate().
> > > + */
> > > +#define X86_FEATURE_AMD_PMU_V2 KVM_X86_FEATURE_AMD_PMU_V2
> >
> > I gave you bad input earlier, for purely KVM-defined flags there's no need for an
> > intermediate KVM_X86_FEATURE_AMD_PMU_V2, this could simply be:
> >
> > #define X86_FEATURE_AMD_PMU_V2 KVM_X86_FEATURE(CPUID_8000_0022_EAX, 0)
> >
> > That's a moot point though because, after much searching because I had a very hard
> > time believing the kernel wouldn't want to know about this flag, I found commit
> >
> > d6d0c7f681fd ("x86/cpufeatures: Add PerfMonV2 feature bit")
> >
> > from nearly a year ago. I.e. to avoid confusiong, this needs to be a scattered
> > flag, not a purely KVM flag.
> >
> > ---
> > arch/x86/kvm/reverse_cpuid.h | 7 +++++++
> > 1 file changed, 7 insertions(+)
> >
> > diff --git a/arch/x86/kvm/reverse_cpuid.h b/arch/x86/kvm/reverse_cpuid.h
> > index 4945456fd646..333e28b0a13c 100644
> > --- a/arch/x86/kvm/reverse_cpuid.h
> > +++ b/arch/x86/kvm/reverse_cpuid.h
> > @@ -15,6 +15,7 @@ enum kvm_only_cpuid_leafs {
> > CPUID_12_EAX = NCAPINTS,
> > CPUID_7_1_EDX,
> > CPUID_8000_0007_EDX,
> > + CPUID_8000_0022_EAX,
> > NR_KVM_CPU_CAPS,
> > NKVMCAPINTS = NR_KVM_CPU_CAPS - NCAPINTS,
> > @@ -47,6 +48,9 @@ enum kvm_only_cpuid_leafs {
> > /* CPUID level 0x80000007 (EDX). */
> > #define KVM_X86_FEATURE_CONSTANT_TSC KVM_X86_FEATURE(CPUID_8000_0007_EDX, 8)
> > +/* CPUID level 0x80000022 (EAX) */
> > +#define KVM_X86_FEATURE_PERFMON_V2 KVM_X86_FEATURE(CPUID_8000_0022_EAX, 0)
>
> I'm very confused and is this the usage you want to see:
>
> kvm_cpu_cap_set(KVM_X86_FEATURE_PERFMON_V2)
> kvm_cpu_cap_has(KVM_X86_FEATURE_PERFMON_V2)
> guest_cpuid_has(vcpu, X86_FEATURE_PERFMON_V2)
>
> ?

No, all of those should just use X86_FEATURE_PERFMON_V2, i.e. the existing flag
from cpufeatures.h.

> then what about X86_FEATURE_PERFMON_V2 ?

Not sure I follow. As above, it's scattered, thus KVM needs to redirect it to
the hardware-defined bit position, which is the role of __feature_translate()
and KVM_X86_FEATURE_PERFMON_V2.