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

[Sohil Mehta <sohil.mehta@intel.com>]

On 10/29/2026 11:26 PM, Binbin Wu wrote:
> 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)
>>  

Looking at this again. I am not sure if this is accurate. Sean's comment
in v9 was that KVM should always intercept and emulate CR4.FRED writes.
IIUC, by default the bit is KVM owned. So, adding FRED to this guest
list seems opposite to that preference.

>>  #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?
> 

You are right. The value is already the full operand in 64-bit mode and
zero extended otherwise.

> And also for CR0, is the special handling needed?
> 

Yeah, this whole thing is unnecessarily complex. I think we can get rid
of mk_cr_64() and drop this patch entirely. How about something like
below (based on kvm-x86-next-2026.10.13)?

>> +
>>  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);
> 

KVM: x86: Stop truncating CR0/CR4 writes from the emulator

Pass the new value directly to kvm_set_cr0() and kvm_set_cr4() when
emulating MOV to CR, instead of taking the low 32 bits of the new value
and the upper 32 bits of the current value. Preserving the upper half is
not needed, as KVM never allows a bit above bit 31 to be set in CR0 or
CR4. In practice, mk_cr_64() only truncates the new value, which allows
an emulated write that sets a reserved bit in CR[63:32] to succeed
instead of generating #GP.

Truncating the value is unnecessary to begin with. MOV to CR is emulated
with Op3264, i.e. the emulator provides the architectural value: the
full operand in 64-bit mode, and a zero-extended 32-bit operand
otherwise. Note, only the emulator is special in this regard, VMX and
SVM pass the guest's value through as-is when handling CR access exits.

Dropping the truncation also readies the emulator for FRED. CR4.FRED
(bit 32) is the first CR4 bit defined in the upper half. It now needs no
special handling as the full value is passed through.

Signed-off-by: Sohil Mehta <sohil.mehta@intel.com>
---

diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index 32563a91fba9..a6f94595ac75 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -5541,11 +5541,6 @@ 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)
-{
-	return (curr_cr & ~((1ULL << 32) - 1)) | new_val;
-}
-
 static unsigned long emulator_get_cr(struct x86_emulate_ctxt *ctxt, int cr)
 {
 	struct kvm_vcpu *vcpu = emul_to_vcpu(ctxt);
@@ -5582,7 +5577,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, val);
 		break;
 	case 2:
 		vcpu->arch.cr2 = val;
@@ -5591,7 +5586,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, val);
 		break;
 	case 8:
 		res = kvm_set_cr8(vcpu, val);