Re: [PATCH v7 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 optimize or hoist the register reads incorrectly.
--

commit 95fe69711afd71a902fad394278581f460dbab32
Author: Chang S. Bae <chang.seok.bae@intel.com>

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

This commit adds testing for APX-specific behavior and ABI interactions.
It verifies that EGPR state is correctly saved and retained across VM entry,
and tests its exposure via the XSAVE ABI.

> diff --git a/tools/testing/selftests/kvm/include/x86/processor.h b/tools/testing/selftests/kvm/include/x86/processor.h
> index 6e6f70035508a..6e3e7d96c571e 100644
> --- a/tools/testing/selftests/kvm/include/x86/processor.h
> +++ b/tools/testing/selftests/kvm/include/x86/processor.h

[ ... ]

> @@ -917,6 +919,124 @@ static inline void write_sse_reg(int reg, const sse128_t *data)
>  	}
>  }
>  
> +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]
Is it possible for the compiler to hoist these register reads outside of
their intended execution points?

According to GCC and Clang semantics, an asm statement with output operands
but no input operands and no volatile qualifier is treated as a pure
computation. 

Could the optimizer move these reads to execute before the GUEST_SYNC()
calls, causing the test to validate stale register states (from before the
VM exit) instead of the intended restored states upon VM entry? 

Adding the volatile qualifier would prevent the compiler from reordering
these hardware register reads.

[ ... ]

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