Re: [PATCH] KVM: MMU: Fix guest stuck during boot due to read/write emulation against GVA
2017-08-11 14:40 GMT+08:00 David Hildenbrand : > On 11.08.2017 08:13, Paolo Bonzini wrote: >> On 11/08/2017 07:59, Wanpeng Li wrote: >>> From: Wanpeng Li >>> >>> Commit c016004494b0 (KVM: x86: Avoid guest page table walk when >>> gpa_available >>> is set) avoids the page table walk when cr2 has already contained a valid >>> GPA. >>> However, that is not the truth if ept == 0 and shadow page table is used. In >>> this scenario cr2 can just contains a valid GVA. The commit results in guest >>> stuck during boot due to read/write emulation against GVA instead of GPA of >>> the guest. >>> >>> This patch fixes it by not setting the gpa_available flag if the shadow >>> page table >>> is used. >> >> Yup, I got these on my overnight tests too. A better fix that I was >> going to commit today is: >> >> if (vcpu->arch.mmu.direct_map) { >> vcpu->arch.gpa_available = true; >> vcpu->arch.gpa_val = fault_address; >> } >> > > Makes sense to me. Can we add a comment? > > /* with shadow page tables, cr2 could contain a GVA, not a GPA */ Yeah, thanks for the review. Regards, Wanpeng Li
Re: [PATCH] KVM: MMU: Fix guest stuck during boot due to read/write emulation against GVA
On 11.08.2017 08:13, Paolo Bonzini wrote: > On 11/08/2017 07:59, Wanpeng Li wrote: >> From: Wanpeng Li >> >> Commit c016004494b0 (KVM: x86: Avoid guest page table walk when >> gpa_available >> is set) avoids the page table walk when cr2 has already contained a valid >> GPA. >> However, that is not the truth if ept == 0 and shadow page table is used. In >> this scenario cr2 can just contains a valid GVA. The commit results in guest >> stuck during boot due to read/write emulation against GVA instead of GPA of >> the guest. >> >> This patch fixes it by not setting the gpa_available flag if the shadow page >> table >> is used. > > Yup, I got these on my overnight tests too. A better fix that I was > going to commit today is: > > if (vcpu->arch.mmu.direct_map) { > vcpu->arch.gpa_available = true; > vcpu->arch.gpa_val = fault_address; > } > Makes sense to me. Can we add a comment? /* with shadow page tables, cr2 could contain a GVA, not a GPA */ > because AMD gets here even if npt=1. > > Paolo -- Thanks, David
Re: [PATCH] KVM: MMU: Fix guest stuck during boot due to read/write emulation against GVA
2017-08-11 14:13 GMT+08:00 Paolo Bonzini : > On 11/08/2017 07:59, Wanpeng Li wrote: >> From: Wanpeng Li >> >> Commit c016004494b0 (KVM: x86: Avoid guest page table walk when gpa_available >> is set) avoids the page table walk when cr2 has already contained a valid >> GPA. >> However, that is not the truth if ept == 0 and shadow page table is used. In >> this scenario cr2 can just contains a valid GVA. The commit results in guest >> stuck during boot due to read/write emulation against GVA instead of GPA of >> the guest. >> >> This patch fixes it by not setting the gpa_available flag if the shadow page >> table >> is used. > > Yup, I got these on my overnight tests too. A better fix that I was > going to commit today is: > > if (vcpu->arch.mmu.direct_map) { > vcpu->arch.gpa_available = true; > vcpu->arch.gpa_val = fault_address; > } > > because AMD gets here even if npt=1. Good point, could I post a patch for this? Regards, Wanpeng Li > > Paolo > >> Cc: Paolo Bonzini >> Cc: Radim Krčmář >> Cc: Brijesh Singh >> Signed-off-by: Wanpeng Li >> --- >> arch/x86/kvm/mmu.c | 2 -- >> 1 file changed, 2 deletions(-) >> >> diff --git a/arch/x86/kvm/mmu.c b/arch/x86/kvm/mmu.c >> index 849312d..e4ce20b 100644 >> --- a/arch/x86/kvm/mmu.c >> +++ b/arch/x86/kvm/mmu.c >> @@ -3791,8 +3791,6 @@ int kvm_handle_page_fault(struct kvm_vcpu *vcpu, u64 >> error_code, >> >> if (need_unprotect && kvm_event_needs_reinjection(vcpu)) >> kvm_mmu_unprotect_page_virt(vcpu, fault_address); >> - vcpu->arch.gpa_available = true; >> - vcpu->arch.gpa_val = fault_address; >> r = kvm_mmu_page_fault(vcpu, fault_address, error_code, insn, >> insn_len); >> break; >> >
Re: [PATCH] KVM: MMU: Fix guest stuck during boot due to read/write emulation against GVA
On 11/08/2017 07:59, Wanpeng Li wrote: > From: Wanpeng Li > > Commit c016004494b0 (KVM: x86: Avoid guest page table walk when gpa_available > is set) avoids the page table walk when cr2 has already contained a valid > GPA. > However, that is not the truth if ept == 0 and shadow page table is used. In > this scenario cr2 can just contains a valid GVA. The commit results in guest > stuck during boot due to read/write emulation against GVA instead of GPA of > the guest. > > This patch fixes it by not setting the gpa_available flag if the shadow page > table > is used. Yup, I got these on my overnight tests too. A better fix that I was going to commit today is: if (vcpu->arch.mmu.direct_map) { vcpu->arch.gpa_available = true; vcpu->arch.gpa_val = fault_address; } because AMD gets here even if npt=1. Paolo > Cc: Paolo Bonzini > Cc: Radim Krčmář > Cc: Brijesh Singh > Signed-off-by: Wanpeng Li > --- > arch/x86/kvm/mmu.c | 2 -- > 1 file changed, 2 deletions(-) > > diff --git a/arch/x86/kvm/mmu.c b/arch/x86/kvm/mmu.c > index 849312d..e4ce20b 100644 > --- a/arch/x86/kvm/mmu.c > +++ b/arch/x86/kvm/mmu.c > @@ -3791,8 +3791,6 @@ int kvm_handle_page_fault(struct kvm_vcpu *vcpu, u64 > error_code, > > if (need_unprotect && kvm_event_needs_reinjection(vcpu)) > kvm_mmu_unprotect_page_virt(vcpu, fault_address); > - vcpu->arch.gpa_available = true; > - vcpu->arch.gpa_val = fault_address; > r = kvm_mmu_page_fault(vcpu, fault_address, error_code, insn, > insn_len); > break; >
[PATCH] KVM: MMU: Fix guest stuck during boot due to read/write emulation against GVA
From: Wanpeng Li Commit c016004494b0 (KVM: x86: Avoid guest page table walk when gpa_available is set) avoids the page table walk when cr2 has already contained a valid GPA. However, that is not the truth if ept == 0 and shadow page table is used. In this scenario cr2 can just contains a valid GVA. The commit results in guest stuck during boot due to read/write emulation against GVA instead of GPA of the guest. This patch fixes it by not setting the gpa_available flag if the shadow page table is used. Cc: Paolo Bonzini Cc: Radim Krčmář Cc: Brijesh Singh Signed-off-by: Wanpeng Li --- arch/x86/kvm/mmu.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/arch/x86/kvm/mmu.c b/arch/x86/kvm/mmu.c index 849312d..e4ce20b 100644 --- a/arch/x86/kvm/mmu.c +++ b/arch/x86/kvm/mmu.c @@ -3791,8 +3791,6 @@ int kvm_handle_page_fault(struct kvm_vcpu *vcpu, u64 error_code, if (need_unprotect && kvm_event_needs_reinjection(vcpu)) kvm_mmu_unprotect_page_virt(vcpu, fault_address); - vcpu->arch.gpa_available = true; - vcpu->arch.gpa_val = fault_address; r = kvm_mmu_page_fault(vcpu, fault_address, error_code, insn, insn_len); break; -- 2.7.4