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

[Binbin Wu <binbin.wu@linux.intel.com>]

On 9/12/2026 5:36 AM, Sohil Mehta wrote:
> From: Xin Li <xin@zytor.com>
> 
> Add CR4.FRED to the emulator's CR4 write mask.  Historically, CR4 was
> restricted to its lower 32 bits in the x86 architecture.  With the
> introduction of FRED, CR4 is expanded to use its higher 32 bits to
> accommodate the FRED enablement bit at position 32.  Update the mask
> to ensure the emulator correctly handles FRED initialization and
> prevents the inadvertent truncation of high-order bits.
> 
> Signed-off-by: Xin Li <xin@zytor.com>
> Signed-off-by: Sohil Mehta <sohil.mehta@intel.com>
> ---
> v10:
>  - New patch
> ---
>  arch/x86/kvm/regs.h |  2 +-
>  arch/x86/kvm/x86.c  | 12 +++++++++---
>  2 files changed, 10 insertions(+), 4 deletions(-)
> 
> diff --git a/arch/x86/kvm/regs.h b/arch/x86/kvm/regs.h
> index 4e641a6c20f9..ef46918daf43 100644
> --- a/arch/x86/kvm/regs.h
> +++ b/arch/x86/kvm/regs.h
> @@ -8,7 +8,7 @@
>  #define KVM_POSSIBLE_CR4_GUEST_BITS				  \
>  	(X86_CR4_PVI | X86_CR4_DE | X86_CR4_PCE | X86_CR4_OSFXSR  \
>  	 | X86_CR4_OSXMMEXCPT | X86_CR4_PGE | X86_CR4_TSD | X86_CR4_FSGSBASE \
> -	 | X86_CR4_CET)
> +	 | X86_CR4_CET | X86_CR4_FRED)
>  
>  #define X86_CR0_PDPTR_BITS    (X86_CR0_CD | X86_CR0_NW | X86_CR0_PG)
>  #define X86_CR4_TLBFLUSH_BITS (X86_CR4_PGE | X86_CR4_PCIDE | X86_CR4_PAE | X86_CR4_SMEP)
> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> index e10ac6b280f6..f264bc9c35f9 100644
> --- 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;
> +}

It's a bit weird to do it based on specific features.

Also, I am not sure this is needed to do the special handling to preserve
the higher bits.
emulator has zero extended the higher bits in fetch_register_operand() if
in non-64bit mode. I think we can just take the full 64-bit new value?
Did I miss anything?

And also for CR0, is the special handling needed?

> +
>  static unsigned long emulator_get_cr(struct x86_emulate_ctxt *ctxt, int cr)
>  {
>  	struct kvm_vcpu *vcpu = emul_to_vcpu(ctxt);
> @@ -5593,7 +5599,7 @@ static int emulator_set_cr(struct x86_emulate_ctxt *ctxt, int cr, ulong val)
>  
>  	switch (cr) {
>  	case 0:
> -		res = kvm_set_cr0(vcpu, mk_cr_64(kvm_read_cr0(vcpu), val));
> +		res = kvm_set_cr0(vcpu, mk_cr0_64(kvm_read_cr0(vcpu), val));
>  		break;
>  	case 2:
>  		vcpu->arch.cr2 = val;
> @@ -5602,7 +5608,7 @@ static int emulator_set_cr(struct x86_emulate_ctxt *ctxt, int cr, ulong val)
>  		res = kvm_set_cr3(vcpu, val);
>  		break;
>  	case 4:
> -		res = kvm_set_cr4(vcpu, mk_cr_64(kvm_read_cr4(vcpu), val));
> +		res = kvm_set_cr4(vcpu, mk_cr4_64(vcpu, kvm_read_cr4(vcpu), val));
>  		break;
>  	case 8:
>  		res = kvm_set_cr8(vcpu, val);