| Message ID | 20211115131837.195527-3-mlevitsk@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | VMX: nested migration fixes for 32 bit nested guests | expand |
On Mon, Nov 15, 2021, Maxim Levitsky wrote: > When the host is running with normal TDP mmu (EPT/NPT), > and it is running a nested 32 bit guest, then after a migration, > the host mmu (aka root_mmu) is first initialized with > nested guest's IA32_EFER, due to the way userspace restores > the nested state. Please try to avoid unnecessary newlines, I find it quite difficult to read as my eyeballs need to jump around more. E.g. wrapping at 75 chars yields When the host is running with normal TDP mmu (EPT/NPT), and it is running a nested 32 bit guest, then after a migration, the host mmu (aka root_mmu) is first initialized with nested guest's IA32_EFER, due to the way userspace restores the nested state. When later, this is corrected on first nested VM exit to the host, when host EFER is loaded from vmcs12, the root_mmu is not reset, because the role.base.level in this case, reflects the level of the TDP mmu which is always 4 (or 5) on EPT, and usually 4 or even 5 on AMD (when we have 64-bit host). Since most of the paging state is already captured in the extended mmu role, just add the EFER.LMA there to force that reset. > When later, this is corrected on first nested VM exit to the host, > when host EFER is loaded from vmcs12, > the root_mmu is not reset, because the role.base.level > in this case, reflects the level of the TDP mmu which is > always 4 (or 5) on EPT, and usually 4 or even 5 on AMD > (when we have 64 bit host). > > Since most of the paging state is already captured in > the extended mmu role, just add the EFER.LMA there to > force that reset. Similar to patch 1, I'd like to word the changelog to make it very clear that this fix is _necessary_, not just a hack to fudge around QEMU behavior. I've spent far too much time deciphering historical KVM changelogs along the lines of "QEMU does XYZ, change KVM to handle that", and in more than one case the "fix" has been wrong and/or incomplete. Incorporate EFER.LMA into kvm_mmu_extended_role, as it used to compute the guest root level and is not reflected in kvm_mmu_page_role.level when TDP is in use. When simply running the guest, it is impossible for EFER.LMA and kvm_mmu.root_level to get out of sync, as the guest cannot transition from PAE paging to 64-bit paging without toggling CR0.PG, i.e. without first bouncing through a different MMU context. And stuffing guest state via KVM_SET_SREGS{2} also ensures a full MMU context reset. However, if KVM_SET_SREGS{2} is followed by KVM_SET_NESTED_STATE, e.g. to set guest state when migrating the VM while L2 is active, the vCPU state will reflect L2, not L1. If L1 is using TDP for L2, then root_mmu will have been configured using L2's state, despite not being used for L2. If L2.EFER.LMA != L1.EFER.LMA, and L2 is using PAE paging, then root_mmu will be configured for guest PAE paging, but will match the mmu_role for 64-bit paging and cause KVM to not reconfigured root_mmu on the next nested VM-Exit. And after typing that up, it's probably also worth adding a blurb to call out (and argue against) the alternative. Alternatively, the root_mmu's role could be invalidated after a successful KVM_SET_NESTED_STATE that yields vcpu->arch.mmu != vcpu->arch.root_mmu, i.e. that switches the active mmu to guest_mmu, but doing so would force KVM to reconfigure the root_mmu in the common case where L1 and L2 have the same EFER, e.g. are both 64-bit guests. > Suggested-by: Sean Christopherson <seanjc@google.com> > Signed-off-by: Maxim Levitsky <mlevitsk@redhat.com> > --- > arch/x86/include/asm/kvm_host.h | 1 + > arch/x86/kvm/mmu/mmu.c | 1 + > 2 files changed, 2 insertions(+) > > diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h > index 88fce6ab4bbd7..a44b9eb7d4d6d 100644 > --- a/arch/x86/include/asm/kvm_host.h > +++ b/arch/x86/include/asm/kvm_host.h > @@ -364,6 +364,7 @@ union kvm_mmu_extended_role { > unsigned int cr4_smap:1; > unsigned int cr4_smep:1; > unsigned int cr4_la57:1; > + unsigned int efer_lma:1; > }; > }; > > diff --git a/arch/x86/kvm/mmu/mmu.c b/arch/x86/kvm/mmu/mmu.c > index 354d2ca92df4d..5c4a41697a717 100644 > --- a/arch/x86/kvm/mmu/mmu.c > +++ b/arch/x86/kvm/mmu/mmu.c > @@ -4682,6 +4682,7 @@ static union kvm_mmu_extended_role kvm_calc_mmu_role_ext(struct kvm_vcpu *vcpu, > /* PKEY and LA57 are active iff long mode is active. */ > ext.cr4_pke = ____is_efer_lma(regs) && ____is_cr4_pke(regs); > ext.cr4_la57 = ____is_efer_lma(regs) && ____is_cr4_la57(regs); > + ext.efer_lma = ____is_efer_lma(regs); > } > > ext.valid = 1; > -- > 2.26.3 >
diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h index 88fce6ab4bbd7..a44b9eb7d4d6d 100644 --- a/arch/x86/include/asm/kvm_host.h +++ b/arch/x86/include/asm/kvm_host.h @@ -364,6 +364,7 @@ union kvm_mmu_extended_role { unsigned int cr4_smap:1; unsigned int cr4_smep:1; unsigned int cr4_la57:1; + unsigned int efer_lma:1; }; }; diff --git a/arch/x86/kvm/mmu/mmu.c b/arch/x86/kvm/mmu/mmu.c index 354d2ca92df4d..5c4a41697a717 100644 --- a/arch/x86/kvm/mmu/mmu.c +++ b/arch/x86/kvm/mmu/mmu.c @@ -4682,6 +4682,7 @@ static union kvm_mmu_extended_role kvm_calc_mmu_role_ext(struct kvm_vcpu *vcpu, /* PKEY and LA57 are active iff long mode is active. */ ext.cr4_pke = ____is_efer_lma(regs) && ____is_cr4_pke(regs); ext.cr4_la57 = ____is_efer_lma(regs) && ____is_cr4_la57(regs); + ext.efer_lma = ____is_efer_lma(regs); } ext.valid = 1;
When the host is running with normal TDP mmu (EPT/NPT), and it is running a nested 32 bit guest, then after a migration, the host mmu (aka root_mmu) is first initialized with nested guest's IA32_EFER, due to the way userspace restores the nested state. When later, this is corrected on first nested VM exit to the host, when host EFER is loaded from vmcs12, the root_mmu is not reset, because the role.base.level in this case, reflects the level of the TDP mmu which is always 4 (or 5) on EPT, and usually 4 or even 5 on AMD (when we have 64 bit host). Since most of the paging state is already captured in the extended mmu role, just add the EFER.LMA there to force that reset. Suggested-by: Sean Christopherson <seanjc@google.com> Signed-off-by: Maxim Levitsky <mlevitsk@redhat.com> --- arch/x86/include/asm/kvm_host.h | 1 + arch/x86/kvm/mmu/mmu.c | 1 + 2 files changed, 2 insertions(+)