Re: [PATCH] x86/nSVM: Save L2's CR4 on #VMEXIT, not Xen's

[Jan Beulich <jbeulich@xxxxxxxx>]

On 24.09.2026 17:50, Ross Lagerwall wrote:
> On 9/24/26 4:01 PM, Jan Beulich wrote:
>> On 23.09.2026 13:16, Ross Lagerwall wrote:
>>> On 9/23/26 10:51 AM, Lin Liu wrote:
>>>> Xen leaks the host's CR4 bits to L1.
>>>>
>>>> nsvm_vmcb_prepare4vmrun() constructs the shadow VMCB's CR4 via
>>>> hvm_set_cr4(), and svm_update_guest_cr() ORs in HVM_CR4_HOST_MASK.
>>>> nsvm_vmcb_prepare4vmexit() then copies CR4 back out of the shadow VMCB
>>>> instead of the value kept in v->arch.hvm.guest_cr[4], so L1 reads back
>>>> Xen's bits - under HAP, CR4.MCE.
>>>>
>>>> Fixes: 9a779e4fc161 ("Implement SVM specific part for Nested 
>>>> Virtualization")
>>>> Assisted-by: Claude:claude-opus-5
>>>> Signed-off-by: Lin Liu <lin.liu01@xxxxxxxxxx>
>>>> ---
>>>>    xen/arch/x86/hvm/svm/nestedsvm.c | 2 +-
>>>>    1 file changed, 1 insertion(+), 1 deletion(-)
>>>>
>>>> diff --git a/xen/arch/x86/hvm/svm/nestedsvm.c 
>>>> b/xen/arch/x86/hvm/svm/nestedsvm.c
>>>> index a8b15d6eae..8d99b0affc 100644
>>>> --- a/xen/arch/x86/hvm/svm/nestedsvm.c
>>>> +++ b/xen/arch/x86/hvm/svm/nestedsvm.c
>>>> @@ -1082,7 +1082,7 @@ nsvm_vmcb_prepare4vmexit(struct vcpu *v, struct 
>>>> cpu_user_regs *regs)
>>>>        ns_vmcb->_efer = n2vmcb->_efer;
>>>>    
>>>>        /* CRn */
>>>> -    ns_vmcb->_cr4 = n2vmcb->_cr4;
>>>> +    ns_vmcb->_cr4 = v->arch.hvm.guest_cr[4];
>>>>        ns_vmcb->_cr0 = n2vmcb->_cr0;
>>>>    
>>>>        /* DRn */
>>>
>>> Reviewed-by: Ross Lagerwall <ross.lagerwall@xxxxxxxxxx>
>>>
>>> Did you consider addressing similar issues with the other state copied from
>>> n2vmcb as well? i.e. I think something similar would apply to CR0, EFER, 
>>> etc.
>>
>> But (assuming the above code change is indeed correct) wouldn't we better 
>> deal
>> with CR0 then right away, rather that leaving things even visually 
>> inconsistent?
> 
> That's up to the maintainers to decide, though given the state of the Nested
> SVM code at the moment IMO it is fine to take valid improvements and make some
> forward progress even if they don't address all the related issues at once.

Yet moving code into more inconsistent shape isn't a very good step, when things
are meant to be truly improved.

Jan