Re: [PATCH v3 3/9] KVM: x86/cpuid: Add a KVM-only leaf for IBS capabilities

[Manali Shukla <manali.shukla@amd.com>]

Hi Jim,

Thank you for reviewing this series.

On 10/8/2026 11:14 PM, Jim Mattson wrote:
> On Tue, Mar 10, 2026 at 06:00:15AM +0000, Manali Shukla wrote:
>> Add a KVM-only leaf for AMD's Instruction Based Sampling capabilities.
>> Multiple IBS related capabilities are added to KVM-only leaf, so that KVM
>> can set these capabilities for the guest, when IBS feature bit is
>> enabled on the guest.
>>
>> Signed-off-by: Manali Shukla <manali.shukla@amd.com>
>> ---
>>  arch/x86/include/asm/kvm_host.h |  1 +
>>  arch/x86/kvm/reverse_cpuid.h    | 16 ++++++++++++++++
>>  2 files changed, 17 insertions(+)
>>
>> diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
>> index 32dd2d55e6f0..01abdf7f112b 100644
>> --- a/arch/x86/include/asm/kvm_host.h
>> +++ b/arch/x86/include/asm/kvm_host.h
>> @@ -782,6 +782,7 @@ enum kvm_only_cpuid_leafs {
>>  	CPUID_12_EAX	 = NCAPINTS,
>>  	CPUID_7_1_EDX,
>>  	CPUID_8000_0007_EDX,
>> +	CPUID_8000_001B_EAX,
>>  	CPUID_8000_0022_EAX,
>>  	CPUID_7_2_EDX,
>>  	CPUID_24_0_EBX,
> 
> The other KVM-only leaves are in the order in which they were added, and
> reverse_cpuid[] uses the same order. This patch puts the new leaf in the
> middle of the enum, but near the end of reverse_cpuid[].  Please add it at
> the end of the enum, after the last existing leaf, and in the same place in
> reverse_cpuid[].

Sure. I will move CPUID_8000_001B_EAX in V4.

> 
>> diff --git a/arch/x86/kvm/reverse_cpuid.h b/arch/x86/kvm/reverse_cpuid.h
>> index 657f5f743ed9..22cfdb331e9e 100644
>> --- a/arch/x86/kvm/reverse_cpuid.h
>> +++ b/arch/x86/kvm/reverse_cpuid.h
>> @@ -76,6 +76,21 @@
>>  #define KVM_X86_FEATURE_TSA_SQ_NO	KVM_X86_FEATURE(CPUID_8000_0021_ECX, 1)
>>  #define KVM_X86_FEATURE_TSA_L1_NO	KVM_X86_FEATURE(CPUID_8000_0021_ECX, 2)
>>
>> +/* AMD defined Instruction-base Sampling capabilities. CPUID level 0x8000001B (EAX). */
> 
> Nit: "Instruction-base" should be "Instruction-Based."

Ack.

> 
>> +#define X86_FEATURE_IBS_AVAIL			KVM_X86_FEATURE(CPUID_8000_001B_EAX, 0)
>> +#define X86_FEATURE_IBS_FETCHSAM		KVM_X86_FEATURE(CPUID_8000_001B_EAX, 1)
>> +#define X86_FEATURE_IBS_OPSAM			KVM_X86_FEATURE(CPUID_8000_001B_EAX, 2)
>> +#define X86_FEATURE_IBS_RDWROPCNT		KVM_X86_FEATURE(CPUID_8000_001B_EAX, 3)
>> +#define X86_FEATURE_IBS_OPCNT			KVM_X86_FEATURE(CPUID_8000_001B_EAX, 4)
>> +#define X86_FEATURE_IBS_BRNTRGT			KVM_X86_FEATURE(CPUID_8000_001B_EAX, 5)
>> +#define X86_FEATURE_IBS_OPCNTEXT		KVM_X86_FEATURE(CPUID_8000_001B_EAX, 6)
>> +#define X86_FEATURE_IBS_RIPINVALIDCHK		KVM_X86_FEATURE(CPUID_8000_001B_EAX, 7)
>> +#define X86_FEATURE_IBS_OPBRNFUSE		KVM_X86_FEATURE(CPUID_8000_001B_EAX, 8)
>> +#define X86_FEATURE_IBS_FETCHCTLEXTD		KVM_X86_FEATURE(CPUID_8000_001B_EAX, 9)
>> +#define X86_FEATURE_IBS_ZEN4_EXT		KVM_X86_FEATURE(CPUID_8000_001B_EAX, 11)
>> +#define X86_FEATURE_IBS_LOADLATFIL		KVM_X86_FEATURE(CPUID_8000_001B_EAX, 12)
>> +#define X86_FEATURE_IBS_ZEN4_DTLBSTAT		KVM_X86_FEATURE(CPUID_8000_001B_EAX, 19)
> 
> Bit 10 (IBS_CAPS_OPDATA4 in asm/perf_event.h) is not defined here or later
> in the series. Is that an oversight?
> 

No, it was left out on purpose. CPUID Fn8000_001B_EAX[10] (IbsOpData4)
is not set on any platform that supports IBS virtualization, so there
is nothing for KVM to advertise that to the guest.

> Three of these names are different from the IBS_CAPS_* names for the same
> bits in asm/perf_event.h: ZEN4_EXT (IBS_CAPS_ZEN4), LOADLATFIL
> (IBS_CAPS_OPLDLAT), and ZEN4_DTLBSTAT (IBS_CAPS_OPDTLBPGSIZE). Please
> standardize on the existing names.
> 

Agreed. I will rename them to existing name in V4.

>> +
>>  struct cpuid_reg {
>>  	u32 function;
>>  	u32 index;
>> @@ -105,6 +120,7 @@ static const struct cpuid_reg reverse_cpuid[] = {
>>  	[CPUID_8000_0022_EAX] = {0x80000022, 0, CPUID_EAX},
>>  	[CPUID_7_2_EDX]       = {         7, 2, CPUID_EDX},
>>  	[CPUID_24_0_EBX]      = {      0x24, 0, CPUID_EBX},
>> +	[CPUID_8000_001B_EAX] = {0x8000001b, 0, CPUID_EAX},
>>  	[CPUID_8000_0021_ECX] = {0x80000021, 0, CPUID_ECX},
>>  	[CPUID_7_1_ECX]       = {         7, 1, CPUID_ECX},
>>  	[CPUID_1E_1_EAX]      = {      0x1e, 1, CPUID_EAX},

-Manali