Re: [PATCH v2] x86/svm: Intercept CR0 writes selectively

[Ross Lagerwall <ross.lagerwall@citrix.com>]

On 9/8/26 2:57 PM, Andrew Cooper wrote:
> On 08/09/2026 9:44 am, Jan Beulich wrote:
>> On 04.09.2026 14:23, Ross Lagerwall wrote:
>>> @@ -2517,6 +2518,7 @@ void asmlinkage svm_vmexit_handler(void)
>>>       hvm_sanitize_regs_fields(
>>>           regs, !(vmcb_get_efer(vmcb) & EFER_LMA) || !(vmcb->cs.l));
>>>   
>>> +    v->arch.hvm.guest_cr[0] = vmcb_get_cr0(vmcb);
>>>       v->arch.hvm.guest_cr[2] = vmcb_get_cr2(vmcb);
>>>       if ( paging_mode_hap(v->domain) )
>>>           v->arch.hvm.guest_cr[3] = v->arch.hvm.hw_cr[3] = vmcb_get_cr3(vmcb);
>> gitlab-CI says no to this patch [1], and I think it's the change above which
>> gets in the way of running guests in shadow mode (as is the case for the two
>> qemu-smoke-x86_64-*-pvh tests). svm_update_guest_cr() has
>>
>>          value = v->arch.hvm.guest_cr[0];
>>          if ( paging_mode_shadow(v->domain) )
>>              value |= X86_CR0_PG | X86_CR0_WP;
>>          vmcb_set_cr0(vmcb, value);
>>
>> i.e. upon reading back the guest's CR0 value we would wrongly record it
>> having PG (and WP) enabled, at which point shadow code would try to use the
>> still-zero CR3 as page table base. I think values need merging here, with
>> only TS and MP taken from what vmcb_get_cr0() returns.
> 
> Yes this is broken for Shadow.
> 
> Shadow shouldn't be trying to use CR0_WRITE_SEL.  What's in the VMCB is
> unrelated to the guest's choice of value, and reverse engineering it is
> fragile.
> 
> Use full CR0_WRITE for Shadow guests, and use CR0_WRITE_SEL for HAP only.

I did try Jan's suggestion which fixes the initial problem but crashes when
dom0 brings up its second vCPU.

If keeping Shadow guests using full CR0_WRITE is acceptable, I'll update the
patch to do that since there are surely better things to do than spending time
making Shadow improvements.

Ross