Re: [PATCH 1/6] x86: KVM: Enable CMPccXADD CPUID and expose it to guest

Jiaxi Chen
2022-10-26T03:40:49Z

On 10/19/2022 11:15 PM, Sean Christopherson wrote:
> For all the shortlogs, "expose it to guest" is technically wrong. Adding
> recognition in kvm/cpuid.c advertises KVM support to host userspace. Whether or
> not a feature is exposed to the guest is up to the userspace VMM.
>
> On Wed, Oct 19, 2022, Jiaxi Chen wrote:
>> CMPccXADD is a new set of instructions in the latest Intel platform Sierra
>> Forest. It includes a semaphore operation that can compare and add the
>
> In general, avoid pronouns in changelogs, it's not clear what "it" refers to here.
>
> And for all of these changelogs, please explicitly state that there are no VMX
> controls for these instructions, assuming that's actually true. From a KVM
> perspective, that's far more interesting than the details of the instruction(s).
>
>> operands if condition is met, which can improve database performance.
>>
>> The bit definition:
>> CPUID.(EAX=7,ECX=1):EAX[bit 7]
>>
>> This patch enables this CPUID in the kernel feature bits and expose it to
>> guest OS.
>
> Same thing here, KVM doesn't decide whether or not to expose the feature to the
> guest.
>
>> Signed-off-by: Jiaxi Chen <jiaxi.chen@linux.intel.com>
>> ---
>> arch/x86/include/asm/cpufeatures.h | 1 +
>> arch/x86/kvm/cpuid.c | 2 +-
>> 2 files changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/arch/x86/include/asm/cpufeatures.h b/arch/x86/include/asm/cpufeatures.h
>> index ef4775c6db01..445626cb5779 100644
>> --- a/arch/x86/include/asm/cpufeatures.h
>> +++ b/arch/x86/include/asm/cpufeatures.h
>> @@ -308,6 +308,7 @@
>> /* Intel-defined CPU features, CPUID level 0x00000007:1 (EAX), word 12 */
>> #define X86_FEATURE_AVX_VNNI (12*32+ 4) /* AVX VNNI instructions */
>> #define X86_FEATURE_AVX512_BF16 (12*32+ 5) /* AVX512 BFLOAT16 instructions */
>> +#define X86_FEATURE_CMPCCXADD (12*32+ 7) /* CMPccXADD instructions */
>
> Boris,
>
> What do you think about moving CPUID_7_1_EAX to be a KVM-only leaf too? AFAICT,
> KVM passthrough is the only reason the existing features are defined.
Boris,

Since CPUID_7_1_EAX has only 5 features now, it is a big waste, should we move it to
KVM-only leaf as Sean suggested. What's your opinion about this?
--
Regards,
Jiaxi