Re: [PATCH v10 15/28] KVM: x86: Include CR4.FRED in the emulator CR4 write mask

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] `mk_cr4_64` erroneously attempts to preserve the upper bits of CR4, contradicting x86 architecture and breaking future higher-order CR4 bits.
--

commit 3026fdc333b24532424b51db68dd9e65b0a1065c
Author: Xin Li <xin@zytor.com>

KVM: x86: Include CR4.FRED in the emulator CR4 write mask

This commit updates the emulator's CR4 write mask to accommodate the FRED
enablement bit, expanding CR4 support to use its higher 32 bits when the
feature is present.

> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> --- a/arch/x86/kvm/x86.c
> +++ b/arch/x86/kvm/x86.c

[ ... ]

> @@ -5552,11 +5552,17 @@ static int emulator_set_dr(struct x86_emulate_ctxt *ctxt, int dr,
>  	return kvm_set_dr(emul_to_vcpu(ctxt), dr, value);
>  }
>  
> -static u64 mk_cr_64(u64 curr_cr, u32 new_val)
> +static u64 mk_cr0_64(u64 curr_cr, u32 new_val)
>  {
>  	return (curr_cr & ~((1ULL << 32) - 1)) | new_val;
>  }
>  
> +static u64 mk_cr4_64(struct kvm_vcpu *vcpu, u64 curr_cr, u64 new_val)
> +{
> +	u32 shift = guest_cpu_cap_has(vcpu, X86_FEATURE_FRED) ? 33 : 32;
> +	return (curr_cr & ~((1ULL << shift) - 1)) | new_val;
> +}

[Severity: Medium]
Does this bitwise logic incorrectly preserve the upper bits of CR4?

The x86 architecture specifies that a 64-bit MOV to CR4 overwrites the entire
register, and a 32-bit MOV to CR4 clears the upper 32 bits. The val passed
into emulator_set_cr() already contains the correct architectural value
(zero-extended if 32-bit, or the full 64-bit value).

By preserving bits above the shift boundary, this could prevent a guest from
clearing future higher-order CR4 bits once they are set. Since the bitwise
AND mask will preserve the 1 from curr_cr and OR it with the new value, those
upper bits would remain permanently set if they were already present.

Could emulator_set_cr() simply pass val directly to kvm_set_cr4() without
attempting to merge it with the current CR4 value?

[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260911213659.2025974-1-sohil.mehta@intel.com?part=15