Messages in this thread Patch in this message | | | Date | Thu, 16 Dec 2021 00:31:18 +0800 | Subject | Re: [PATCH 11/15] KVM: VMX: Update vmcs.GUEST_CR3 only when the guest CR3 is dirty | From | Lai Jiangshan <> |
| |
On 2021/12/15 23:47, Maxim Levitsky wrote: > On Mon, 2021-11-08 at 20:44 +0800, Lai Jiangshan wrote: >> From: Lai Jiangshan <laijs@linux.alibaba.com> >> >> When vcpu->arch.cr3 is changed, it is marked dirty, so vmcs.GUEST_CR3 >> can be updated only when kvm_register_is_dirty(vcpu, VCPU_EXREG_CR3). >> >> Signed-off-by: Lai Jiangshan <laijs@linux.alibaba.com> >> --- >> arch/x86/kvm/vmx/vmx.c | 4 ++-- >> 1 file changed, 2 insertions(+), 2 deletions(-) >> >> diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c >> index d94e51e9c08f..38b65b97fb7b 100644 >> --- a/arch/x86/kvm/vmx/vmx.c >> +++ b/arch/x86/kvm/vmx/vmx.c >> @@ -3126,9 +3126,9 @@ static void vmx_load_mmu_pgd(struct kvm_vcpu *vcpu, hpa_t root_hpa, >> >> if (!enable_unrestricted_guest && !is_paging(vcpu)) >> guest_cr3 = to_kvm_vmx(kvm)->ept_identity_map_addr; >> - else if (test_bit(VCPU_EXREG_CR3, (ulong *)&vcpu->arch.regs_avail)) >> + else if (kvm_register_is_dirty(vcpu, VCPU_EXREG_CR3)) >> guest_cr3 = vcpu->arch.cr3; >> - else /* vmcs01.GUEST_CR3 is already up-to-date. */ >> + else /* vmcs.GUEST_CR3 is already up-to-date. */ >> update_guest_cr3 = false; >> vmx_ept_load_pdptrs(vcpu); >> } else { > > > I just bisected this patch to break booting a VM with ept=1 but unrestricted_guest=0 > (I needed to re-test unrestricted_guest=0 bug related to SMM, but didn't want > to boot without EPT. With ept=0,the VM boots with this patch applied). >
Thanks for reporting.
Sorry, I never tested it with unrestricted_guest=0. I can't reproduce it now shortly with unrestricted_guest=0. Maybe it can be reproduced easily if I try more guests or I write a piece of guest code to deliberate hit it if the following analyses is correct.
All the paths changing %cr3 are followed with kvm_register_mark_dirty(vcpu, VCPU_EXREG_CR3) and GUEST_CR3 will be expected to be updated.
What I missed is the case of "if (!enable_unrestricted_guest && !is_paging(vcpu))" in vmx_load_mmu_pgd() which doesn't load GUEST_CR3 but clears dirty of VCPU_EXREG_CR3 (when after next run).
So when CR0 !PG -> PG, VCPU_EXREG_CR3 dirty bit should be set.
Maybe adding the following patch on top of the original patch can work.
Thanks Lai
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c index 85127b3e3690..55b45005ebb9 100644 --- a/arch/x86/kvm/x86.c +++ b/arch/x86/kvm/x86.c @@ -858,6 +858,7 @@ void kvm_post_set_cr0(struct kvm_vcpu *vcpu, unsigned long old_cr0, unsigned lon if ((cr0 ^ old_cr0) & X86_CR0_PG) { kvm_clear_async_pf_completion_queue(vcpu); kvm_async_pf_hash_reset(vcpu); + kvm_register_mark_dirty(vcpu, VCPU_EXREG_CR3); }
if ((cr0 ^ old_cr0) & KVM_MMU_CR0_ROLE_BITS)
| |