On Fri, 30 May 2025 at 08:23, Paolo Bonzini <[email protected]> wrote:
>
> From: Xiaoyao Li <[email protected]>
>
> Some CPUID bits are controlled by XFAM. They are not covered by
> tdx_caps.cpuid (which only contians the directly configurable bits), but
> they are actually supported when the related XFAM bit is supported.
>
> Add these XFAM controlled bits to TDX supported CPUID bits based on the
> supported_xfam.
>
> Besides, incorporate the supported_xfam into the supported CPUID leaf of
> 0xD.
>
> Signed-off-by: Xiaoyao Li <[email protected]>
> Link: https://lore.kernel.org/r/[email protected]
> Signed-off-by: Paolo Bonzini <[email protected]>
> ---
>  target/i386/cpu.h     | 16 ++++++++++
>  target/i386/cpu.c     | 12 -------
>  target/i386/kvm/tdx.c | 73 +++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 89 insertions(+), 12 deletions(-)

Hi; Coverity points out a problem with this code (CID 1610545):

> +    e = find_in_supported_entry(0xd, 0);
> +    e->eax |= (tdx_caps->supported_xfam & CPUID_XSTATE_XCR0_MASK);
> +    e->edx |= (tdx_caps->supported_xfam & CPUID_XSTATE_XCR0_MASK) >> 32;

All the bits in CPUID_XSTATE_XCR0_MASK are in the bottom half
of the word; this means that (x & CPUID_XSTATE_XCR0_MASK) >> 32
is always zero and the statement has no effect.

> +    e->ecx |= (tdx_caps->supported_xfam & CPUID_XSTATE_XSS_MASK);
> +    e->edx |= (tdx_caps->supported_xfam & CPUID_XSTATE_XSS_MASK) >> 32;

Similarly here.

What was the intention here ?

thanks
-- PMM

Reply via email to