On Thu, 10 Jul 2025 at 13:42, Xiaoyao Li <[email protected]> wrote:
>
> On 7/10/2025 8:27 PM, Peter Maydell wrote:
> > 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 ?
>
> It's future-proven. So that this code will still work correctly even if
> CPUID_XSTATE_XCR0_MASK/CPUID_XSTATE_XSS_MASK has higher bit (above 32)
> set in the future.

OK; I have marked the report as a false-positive.

-- PMM

Reply via email to