Re: [PATCH v6 19/20] KVM: x86: selftests: Add APX state and ABI test

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] The inline assembly in `read_egpr` lacks the `volatile` qualifier, allowing the compiler to cache or incorrectly reorder hardware register reads.
--

--- Patch [19]: [PATCH v6 19/20] KVM: x86: selftests: Add APX state and ABI test ---
commit b6d4e7dba224ce722415441a2fa9a8f67f8cb213
Author: Chang S. Bae <chang.seok.bae@intel.com>

KVM: x86: selftests: Add APX state and ABI test

This commit adds a selftest to verify APX-specific behavior and ABI
interactions for EGPR state. It tests state transitions across guest
updates, userspace updates, and guest APX disable scenarios.

> diff --git a/tools/testing/selftests/kvm/include/x86/processor.h b/tools/testing/selftests/kvm/include/x86/processor.h
> index 7d3a27bc0d842..5f5acc33486ab 100644
> --- a/tools/testing/selftests/kvm/include/x86/processor.h
> +++ b/tools/testing/selftests/kvm/include/x86/processor.h
[ ... ]
> +static inline unsigned long read_egpr(int reg)
> +{
> +	unsigned long data = 0;
> +
> +	/* mov %r16..%r31, %rax */
> +	switch (reg) {
> +	case 16:
> +		asm(".byte 0xd5, 0x48, 0x89, 0xc0" : "=a"(data));

[Severity: Medium]
Since the inline assembly in read_egpr() has an output operand but no
inputs, could the compiler aggressively optimize these reads?

Without the volatile qualifier, GCC treats an asm block with only outputs
as a pure function. This means the compiler might cache the result across
synchronization points, as a memory clobber in GUEST_SYNC wouldn't
invalidate pure register-based expressions.

Would adding the volatile keyword here be safer to ensure the test
always validates the actual CPU state rather than a potentially cached
compiler value?

> +		break;
> +	case 17:
> +		asm(".byte 0xd5, 0x48, 0x89, 0xc8" : "=a"(data));
[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260729191656.598771-1-chang.seok.bae@intel.com?part=19