* [PATCH v3 1/9] KVM: nSVM: split kvm_init_shadow_npt_mmu() from kvm_init_shadow_mmu()
2020-07-09 14:53 [PATCH v3 0/9] KVM: nSVM: fixes for CR3/MMU switch upon nested guest entry/exit Vitaly Kuznetsov
@ 2020-07-09 14:53 ` Vitaly Kuznetsov
2020-07-09 14:53 ` [PATCH v3 2/9] KVM: nSVM: stop dereferencing vcpu->arch.mmu to get the context in kvm_init_shadow{,_npt}_mmu() Vitaly Kuznetsov
` (8 subsequent siblings)
9 siblings, 0 replies; 19+ messages in thread
From: Vitaly Kuznetsov @ 2020-07-09 14:53 UTC (permalink / raw)
To: kvm, Paolo Bonzini
Cc: Sean Christopherson, Wanpeng Li, Jim Mattson, Junaid Shahid,
linux-kernel
As a preparatory change for moving kvm_mmu_new_pgd() from
nested_prepare_vmcb_save() to nested_svm_init_mmu_context() split
kvm_init_shadow_npt_mmu() from kvm_init_shadow_mmu(). This also makes
the code look more like nVMX (kvm_init_shadow_ept_mmu()).
No functional change intended.
Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com>
---
arch/x86/kvm/mmu.h | 3 ++-
arch/x86/kvm/mmu/mmu.c | 31 ++++++++++++++++++++++++-------
arch/x86/kvm/svm/nested.c | 3 ++-
3 files changed, 28 insertions(+), 9 deletions(-)
diff --git a/arch/x86/kvm/mmu.h b/arch/x86/kvm/mmu.h
index 444bb9c54548..94378ef1df54 100644
--- a/arch/x86/kvm/mmu.h
+++ b/arch/x86/kvm/mmu.h
@@ -57,7 +57,8 @@ void
reset_shadow_zero_bits_mask(struct kvm_vcpu *vcpu, struct kvm_mmu *context);
void kvm_init_mmu(struct kvm_vcpu *vcpu, bool reset_roots);
-void kvm_init_shadow_mmu(struct kvm_vcpu *vcpu, u32 cr0, u32 cr4, u32 efer);
+void kvm_init_shadow_npt_mmu(struct kvm_vcpu *vcpu, u32 cr0, u32 cr4, u32 efer,
+ gpa_t nested_cr3);
void kvm_init_shadow_ept_mmu(struct kvm_vcpu *vcpu, bool execonly,
bool accessed_dirty, gpa_t new_eptp);
bool kvm_can_do_async_pf(struct kvm_vcpu *vcpu);
diff --git a/arch/x86/kvm/mmu/mmu.c b/arch/x86/kvm/mmu/mmu.c
index 2da46b4e11b5..93f18e5fa8b5 100644
--- a/arch/x86/kvm/mmu/mmu.c
+++ b/arch/x86/kvm/mmu/mmu.c
@@ -4952,14 +4952,10 @@ kvm_calc_shadow_mmu_root_page_role(struct kvm_vcpu *vcpu, bool base_only)
return role;
}
-void kvm_init_shadow_mmu(struct kvm_vcpu *vcpu, u32 cr0, u32 cr4, u32 efer)
+static void shadow_mmu_init_context(struct kvm_vcpu *vcpu, u32 cr0, u32 cr4,
+ u32 efer, union kvm_mmu_role new_role)
{
struct kvm_mmu *context = vcpu->arch.mmu;
- union kvm_mmu_role new_role =
- kvm_calc_shadow_mmu_root_page_role(vcpu, false);
-
- if (new_role.as_u64 == context->mmu_role.as_u64)
- return;
if (!(cr0 & X86_CR0_PG))
nonpaging_init_context(vcpu, context);
@@ -4973,7 +4969,28 @@ void kvm_init_shadow_mmu(struct kvm_vcpu *vcpu, u32 cr0, u32 cr4, u32 efer)
context->mmu_role.as_u64 = new_role.as_u64;
reset_shadow_zero_bits_mask(vcpu, context);
}
-EXPORT_SYMBOL_GPL(kvm_init_shadow_mmu);
+
+static void kvm_init_shadow_mmu(struct kvm_vcpu *vcpu, u32 cr0, u32 cr4, u32 efer)
+{
+ struct kvm_mmu *context = vcpu->arch.mmu;
+ union kvm_mmu_role new_role =
+ kvm_calc_shadow_mmu_root_page_role(vcpu, false);
+
+ if (new_role.as_u64 != context->mmu_role.as_u64)
+ shadow_mmu_init_context(vcpu, cr0, cr4, efer, new_role);
+}
+
+void kvm_init_shadow_npt_mmu(struct kvm_vcpu *vcpu, u32 cr0, u32 cr4, u32 efer,
+ gpa_t nested_cr3)
+{
+ struct kvm_mmu *context = vcpu->arch.mmu;
+ union kvm_mmu_role new_role =
+ kvm_calc_shadow_mmu_root_page_role(vcpu, false);
+
+ if (new_role.as_u64 != context->mmu_role.as_u64)
+ shadow_mmu_init_context(vcpu, cr0, cr4, efer, new_role);
+}
+EXPORT_SYMBOL_GPL(kvm_init_shadow_npt_mmu);
static union kvm_mmu_role
kvm_calc_shadow_ept_root_page_role(struct kvm_vcpu *vcpu, bool accessed_dirty,
diff --git a/arch/x86/kvm/svm/nested.c b/arch/x86/kvm/svm/nested.c
index 6bceafb19108..e424bce13e6c 100644
--- a/arch/x86/kvm/svm/nested.c
+++ b/arch/x86/kvm/svm/nested.c
@@ -87,7 +87,8 @@ static void nested_svm_init_mmu_context(struct kvm_vcpu *vcpu)
WARN_ON(mmu_is_nested(vcpu));
vcpu->arch.mmu = &vcpu->arch.guest_mmu;
- kvm_init_shadow_mmu(vcpu, X86_CR0_PG, hsave->save.cr4, hsave->save.efer);
+ kvm_init_shadow_npt_mmu(vcpu, X86_CR0_PG, hsave->save.cr4, hsave->save.efer,
+ svm->nested.ctl.nested_cr3);
vcpu->arch.mmu->get_guest_pgd = nested_svm_get_tdp_cr3;
vcpu->arch.mmu->get_pdptr = nested_svm_get_tdp_pdptr;
vcpu->arch.mmu->inject_page_fault = nested_svm_inject_npf_exit;
--
2.25.4
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [PATCH v3 2/9] KVM: nSVM: stop dereferencing vcpu->arch.mmu to get the context in kvm_init_shadow{,_npt}_mmu()
2020-07-09 14:53 [PATCH v3 0/9] KVM: nSVM: fixes for CR3/MMU switch upon nested guest entry/exit Vitaly Kuznetsov
2020-07-09 14:53 ` [PATCH v3 1/9] KVM: nSVM: split kvm_init_shadow_npt_mmu() from kvm_init_shadow_mmu() Vitaly Kuznetsov
@ 2020-07-09 14:53 ` Vitaly Kuznetsov
2020-07-09 17:47 ` Paolo Bonzini
2020-07-09 14:53 ` [PATCH v3 3/9] KVM: nSVM: reset nested_run_pending upon nested_svm_vmrun_msrpm() failure Vitaly Kuznetsov
` (7 subsequent siblings)
9 siblings, 1 reply; 19+ messages in thread
From: Vitaly Kuznetsov @ 2020-07-09 14:53 UTC (permalink / raw)
To: kvm, Paolo Bonzini
Cc: Sean Christopherson, Wanpeng Li, Jim Mattson, Junaid Shahid,
linux-kernel
Now as kvm_init_shadow_npt_mmu() is separated from kvm_init_shadow_mmu()
we always know the MMU context we need to use so there is no need to
dereference vcpu->arch.mmu pointer.
Suggested-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com>
---
arch/x86/kvm/mmu/mmu.c | 15 +++++++--------
1 file changed, 7 insertions(+), 8 deletions(-)
diff --git a/arch/x86/kvm/mmu/mmu.c b/arch/x86/kvm/mmu/mmu.c
index 93f18e5fa8b5..69fa51af8cbf 100644
--- a/arch/x86/kvm/mmu/mmu.c
+++ b/arch/x86/kvm/mmu/mmu.c
@@ -4952,11 +4952,10 @@ kvm_calc_shadow_mmu_root_page_role(struct kvm_vcpu *vcpu, bool base_only)
return role;
}
-static void shadow_mmu_init_context(struct kvm_vcpu *vcpu, u32 cr0, u32 cr4,
- u32 efer, union kvm_mmu_role new_role)
+static void shadow_mmu_init_context(struct kvm_vcpu *vcpu, struct kvm_mmu *context,
+ u32 cr0, u32 cr4, u32 efer,
+ union kvm_mmu_role new_role)
{
- struct kvm_mmu *context = vcpu->arch.mmu;
-
if (!(cr0 & X86_CR0_PG))
nonpaging_init_context(vcpu, context);
else if (efer & EFER_LMA)
@@ -4972,23 +4971,23 @@ static void shadow_mmu_init_context(struct kvm_vcpu *vcpu, u32 cr0, u32 cr4,
static void kvm_init_shadow_mmu(struct kvm_vcpu *vcpu, u32 cr0, u32 cr4, u32 efer)
{
- struct kvm_mmu *context = vcpu->arch.mmu;
+ struct kvm_mmu *context = &vcpu->arch.root_mmu;
union kvm_mmu_role new_role =
kvm_calc_shadow_mmu_root_page_role(vcpu, false);
if (new_role.as_u64 != context->mmu_role.as_u64)
- shadow_mmu_init_context(vcpu, cr0, cr4, efer, new_role);
+ shadow_mmu_init_context(vcpu, context, cr0, cr4, efer, new_role);
}
void kvm_init_shadow_npt_mmu(struct kvm_vcpu *vcpu, u32 cr0, u32 cr4, u32 efer,
gpa_t nested_cr3)
{
- struct kvm_mmu *context = vcpu->arch.mmu;
+ struct kvm_mmu *context = &vcpu->arch.guest_mmu;
union kvm_mmu_role new_role =
kvm_calc_shadow_mmu_root_page_role(vcpu, false);
if (new_role.as_u64 != context->mmu_role.as_u64)
- shadow_mmu_init_context(vcpu, cr0, cr4, efer, new_role);
+ shadow_mmu_init_context(vcpu, context, cr0, cr4, efer, new_role);
}
EXPORT_SYMBOL_GPL(kvm_init_shadow_npt_mmu);
--
2.25.4
^ permalink raw reply related [flat|nested] 19+ messages in thread
* Re: [PATCH v3 2/9] KVM: nSVM: stop dereferencing vcpu->arch.mmu to get the context in kvm_init_shadow{,_npt}_mmu()
2020-07-09 14:53 ` [PATCH v3 2/9] KVM: nSVM: stop dereferencing vcpu->arch.mmu to get the context in kvm_init_shadow{,_npt}_mmu() Vitaly Kuznetsov
@ 2020-07-09 17:47 ` Paolo Bonzini
0 siblings, 0 replies; 19+ messages in thread
From: Paolo Bonzini @ 2020-07-09 17:47 UTC (permalink / raw)
To: Vitaly Kuznetsov, kvm
Cc: Sean Christopherson, Wanpeng Li, Jim Mattson, Junaid Shahid,
linux-kernel
On 09/07/20 16:53, Vitaly Kuznetsov wrote:
> Now as kvm_init_shadow_npt_mmu() is separated from kvm_init_shadow_mmu()
> we always know the MMU context we need to use so there is no need to
> dereference vcpu->arch.mmu pointer.
>
> Suggested-by: Paolo Bonzini <pbonzini@redhat.com>
> Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com>
> ---
> arch/x86/kvm/mmu/mmu.c | 15 +++++++--------
> 1 file changed, 7 insertions(+), 8 deletions(-)
>
This is actually true of all init functions, so we can squash this in too
(my fault for being too concise):
diff --git a/arch/x86/kvm/mmu/mmu.c b/arch/x86/kvm/mmu/mmu.c
index 2c4fb5684782..78c88e8aecfa 100644
--- a/arch/x86/kvm/mmu/mmu.c
+++ b/arch/x86/kvm/mmu/mmu.c
@@ -4850,7 +4850,7 @@ kvm_calc_tdp_mmu_root_page_role(struct kvm_vcpu *vcpu, bool base_only)
static void init_kvm_tdp_mmu(struct kvm_vcpu *vcpu)
{
- struct kvm_mmu *context = vcpu->arch.mmu;
+ struct kvm_mmu *context = &vcpu->arch.root_mmu;
union kvm_mmu_role new_role =
kvm_calc_tdp_mmu_root_page_role(vcpu, false);
@@ -4989,7 +4989,7 @@ kvm_calc_shadow_ept_root_page_role(struct kvm_vcpu *vcpu, bool accessed_dirty,
void kvm_init_shadow_ept_mmu(struct kvm_vcpu *vcpu, bool execonly,
bool accessed_dirty, gpa_t new_eptp)
{
- struct kvm_mmu *context = vcpu->arch.mmu;
+ struct kvm_mmu *context = &vcpu->arch.guest_mmu;
u8 level = vmx_eptp_page_walk_level(new_eptp);
union kvm_mmu_role new_role =
kvm_calc_shadow_ept_root_page_role(vcpu, accessed_dirty,
@@ -5023,7 +5023,7 @@ EXPORT_SYMBOL_GPL(kvm_init_shadow_ept_mmu);
static void init_kvm_softmmu(struct kvm_vcpu *vcpu)
{
- struct kvm_mmu *context = vcpu->arch.mmu;
+ struct kvm_mmu *context = &vcpu->arch.root_mmu;
kvm_init_shadow_mmu(vcpu,
kvm_read_cr0_bits(vcpu, X86_CR0_PG),
(BTW, a patch to rename nested_mmu to nested_walk_mmu and guest_mmu to
nested_tdp_mmu would be welcome).
Paolo
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [PATCH v3 3/9] KVM: nSVM: reset nested_run_pending upon nested_svm_vmrun_msrpm() failure
2020-07-09 14:53 [PATCH v3 0/9] KVM: nSVM: fixes for CR3/MMU switch upon nested guest entry/exit Vitaly Kuznetsov
2020-07-09 14:53 ` [PATCH v3 1/9] KVM: nSVM: split kvm_init_shadow_npt_mmu() from kvm_init_shadow_mmu() Vitaly Kuznetsov
2020-07-09 14:53 ` [PATCH v3 2/9] KVM: nSVM: stop dereferencing vcpu->arch.mmu to get the context in kvm_init_shadow{,_npt}_mmu() Vitaly Kuznetsov
@ 2020-07-09 14:53 ` Vitaly Kuznetsov
2020-07-09 17:48 ` Paolo Bonzini
2020-07-09 14:53 ` [PATCH v3 4/9] KVM: nSVM: prepare to handle errors from enter_svm_guest_mode() Vitaly Kuznetsov
` (6 subsequent siblings)
9 siblings, 1 reply; 19+ messages in thread
From: Vitaly Kuznetsov @ 2020-07-09 14:53 UTC (permalink / raw)
To: kvm, Paolo Bonzini
Cc: Sean Christopherson, Wanpeng Li, Jim Mattson, Junaid Shahid,
linux-kernel
WARN_ON_ONCE(svm->nested.nested_run_pending) in nested_svm_vmexit()
will fire if nested_run_pending remains '1' but it doesn't really
need to, we are already failing and not going to run nested guest.
Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com>
---
arch/x86/kvm/svm/nested.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/arch/x86/kvm/svm/nested.c b/arch/x86/kvm/svm/nested.c
index e424bce13e6c..1cc8592b1820 100644
--- a/arch/x86/kvm/svm/nested.c
+++ b/arch/x86/kvm/svm/nested.c
@@ -468,6 +468,8 @@ int nested_svm_vmrun(struct vcpu_svm *svm)
enter_svm_guest_mode(svm, vmcb_gpa, nested_vmcb);
if (!nested_svm_vmrun_msrpm(svm)) {
+ svm->nested.nested_run_pending = 0;
+
svm->vmcb->control.exit_code = SVM_EXIT_ERR;
svm->vmcb->control.exit_code_hi = 0;
svm->vmcb->control.exit_info_1 = 0;
--
2.25.4
^ permalink raw reply related [flat|nested] 19+ messages in thread
* Re: [PATCH v3 3/9] KVM: nSVM: reset nested_run_pending upon nested_svm_vmrun_msrpm() failure
2020-07-09 14:53 ` [PATCH v3 3/9] KVM: nSVM: reset nested_run_pending upon nested_svm_vmrun_msrpm() failure Vitaly Kuznetsov
@ 2020-07-09 17:48 ` Paolo Bonzini
0 siblings, 0 replies; 19+ messages in thread
From: Paolo Bonzini @ 2020-07-09 17:48 UTC (permalink / raw)
To: Vitaly Kuznetsov, kvm
Cc: Sean Christopherson, Wanpeng Li, Jim Mattson, Junaid Shahid,
linux-kernel
On 09/07/20 16:53, Vitaly Kuznetsov wrote:
> WARN_ON_ONCE(svm->nested.nested_run_pending) in nested_svm_vmexit()
> will fire if nested_run_pending remains '1' but it doesn't really
> need to, we are already failing and not going to run nested guest.
>
> Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com>
> ---
> arch/x86/kvm/svm/nested.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/arch/x86/kvm/svm/nested.c b/arch/x86/kvm/svm/nested.c
> index e424bce13e6c..1cc8592b1820 100644
> --- a/arch/x86/kvm/svm/nested.c
> +++ b/arch/x86/kvm/svm/nested.c
> @@ -468,6 +468,8 @@ int nested_svm_vmrun(struct vcpu_svm *svm)
> enter_svm_guest_mode(svm, vmcb_gpa, nested_vmcb);
>
> if (!nested_svm_vmrun_msrpm(svm)) {
> + svm->nested.nested_run_pending = 0;
> +
> svm->vmcb->control.exit_code = SVM_EXIT_ERR;
> svm->vmcb->control.exit_code_hi = 0;
> svm->vmcb->control.exit_info_1 = 0;
>
I wouldn't complain if you added a kvm-unit-tests testcase for this...
Paolo
^ permalink raw reply [flat|nested] 19+ messages in thread
* [PATCH v3 4/9] KVM: nSVM: prepare to handle errors from enter_svm_guest_mode()
2020-07-09 14:53 [PATCH v3 0/9] KVM: nSVM: fixes for CR3/MMU switch upon nested guest entry/exit Vitaly Kuznetsov
` (2 preceding siblings ...)
2020-07-09 14:53 ` [PATCH v3 3/9] KVM: nSVM: reset nested_run_pending upon nested_svm_vmrun_msrpm() failure Vitaly Kuznetsov
@ 2020-07-09 14:53 ` Vitaly Kuznetsov
2020-07-09 14:53 ` [PATCH v3 5/9] KVM: nSVM: introduce nested_svm_load_cr3() Vitaly Kuznetsov
` (5 subsequent siblings)
9 siblings, 0 replies; 19+ messages in thread
From: Vitaly Kuznetsov @ 2020-07-09 14:53 UTC (permalink / raw)
To: kvm, Paolo Bonzini
Cc: Sean Christopherson, Wanpeng Li, Jim Mattson, Junaid Shahid,
linux-kernel
Some operations in enter_svm_guest_mode() may fail, e.g. currently
we suppress kvm_set_cr3() return value. Prepare the code to proparate
errors.
No functional change intended.
Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com>
---
arch/x86/kvm/svm/nested.c | 26 ++++++++++++++++----------
arch/x86/kvm/svm/svm.c | 6 ++++--
arch/x86/kvm/svm/svm.h | 4 ++--
3 files changed, 22 insertions(+), 14 deletions(-)
diff --git a/arch/x86/kvm/svm/nested.c b/arch/x86/kvm/svm/nested.c
index 1cc8592b1820..5e6c988a4e6b 100644
--- a/arch/x86/kvm/svm/nested.c
+++ b/arch/x86/kvm/svm/nested.c
@@ -379,7 +379,7 @@ static void nested_prepare_vmcb_control(struct vcpu_svm *svm)
mark_all_dirty(svm->vmcb);
}
-void enter_svm_guest_mode(struct vcpu_svm *svm, u64 vmcb_gpa,
+int enter_svm_guest_mode(struct vcpu_svm *svm, u64 vmcb_gpa,
struct vmcb *nested_vmcb)
{
svm->nested.vmcb = vmcb_gpa;
@@ -388,6 +388,8 @@ void enter_svm_guest_mode(struct vcpu_svm *svm, u64 vmcb_gpa,
nested_prepare_vmcb_control(svm);
svm_set_gif(svm, true);
+
+ return 0;
}
int nested_svm_vmrun(struct vcpu_svm *svm)
@@ -465,18 +467,22 @@ int nested_svm_vmrun(struct vcpu_svm *svm)
copy_vmcb_control_area(&hsave->control, &vmcb->control);
svm->nested.nested_run_pending = 1;
- enter_svm_guest_mode(svm, vmcb_gpa, nested_vmcb);
- if (!nested_svm_vmrun_msrpm(svm)) {
- svm->nested.nested_run_pending = 0;
+ if (enter_svm_guest_mode(svm, vmcb_gpa, nested_vmcb))
+ goto out_exit_err;
- svm->vmcb->control.exit_code = SVM_EXIT_ERR;
- svm->vmcb->control.exit_code_hi = 0;
- svm->vmcb->control.exit_info_1 = 0;
- svm->vmcb->control.exit_info_2 = 0;
+ if (nested_svm_vmrun_msrpm(svm))
+ goto out;
- nested_svm_vmexit(svm);
- }
+out_exit_err:
+ svm->nested.nested_run_pending = 0;
+
+ svm->vmcb->control.exit_code = SVM_EXIT_ERR;
+ svm->vmcb->control.exit_code_hi = 0;
+ svm->vmcb->control.exit_info_1 = 0;
+ svm->vmcb->control.exit_info_2 = 0;
+
+ nested_svm_vmexit(svm);
out:
kvm_vcpu_unmap(&svm->vcpu, &map, true);
diff --git a/arch/x86/kvm/svm/svm.c b/arch/x86/kvm/svm/svm.c
index c0da4dd78ac5..b8ec56fe5478 100644
--- a/arch/x86/kvm/svm/svm.c
+++ b/arch/x86/kvm/svm/svm.c
@@ -3843,6 +3843,7 @@ static int svm_pre_leave_smm(struct kvm_vcpu *vcpu, const char *smstate)
struct kvm_host_map map;
u64 guest;
u64 vmcb;
+ int ret = 0;
guest = GET_SMSTATE(u64, smstate, 0x7ed8);
vmcb = GET_SMSTATE(u64, smstate, 0x7ee0);
@@ -3851,10 +3852,11 @@ static int svm_pre_leave_smm(struct kvm_vcpu *vcpu, const char *smstate)
if (kvm_vcpu_map(&svm->vcpu, gpa_to_gfn(vmcb), &map) == -EINVAL)
return 1;
nested_vmcb = map.hva;
- enter_svm_guest_mode(svm, vmcb, nested_vmcb);
+ ret = enter_svm_guest_mode(svm, vmcb, nested_vmcb);
kvm_vcpu_unmap(&svm->vcpu, &map, true);
}
- return 0;
+
+ return ret;
}
static void enable_smi_window(struct kvm_vcpu *vcpu)
diff --git a/arch/x86/kvm/svm/svm.h b/arch/x86/kvm/svm/svm.h
index 6ac4c00a5d82..f98649af11d1 100644
--- a/arch/x86/kvm/svm/svm.h
+++ b/arch/x86/kvm/svm/svm.h
@@ -387,8 +387,8 @@ static inline bool nested_exit_on_nmi(struct vcpu_svm *svm)
return (svm->nested.ctl.intercept & (1ULL << INTERCEPT_NMI));
}
-void enter_svm_guest_mode(struct vcpu_svm *svm, u64 vmcb_gpa,
- struct vmcb *nested_vmcb);
+int enter_svm_guest_mode(struct vcpu_svm *svm, u64 vmcb_gpa,
+ struct vmcb *nested_vmcb);
void svm_leave_nested(struct vcpu_svm *svm);
int nested_svm_vmrun(struct vcpu_svm *svm);
void nested_svm_vmloadsave(struct vmcb *from_vmcb, struct vmcb *to_vmcb);
--
2.25.4
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [PATCH v3 5/9] KVM: nSVM: introduce nested_svm_load_cr3()
2020-07-09 14:53 [PATCH v3 0/9] KVM: nSVM: fixes for CR3/MMU switch upon nested guest entry/exit Vitaly Kuznetsov
` (3 preceding siblings ...)
2020-07-09 14:53 ` [PATCH v3 4/9] KVM: nSVM: prepare to handle errors from enter_svm_guest_mode() Vitaly Kuznetsov
@ 2020-07-09 14:53 ` Vitaly Kuznetsov
2020-07-09 14:53 ` [PATCH v3 6/9] KVM: nSVM: move kvm_set_cr3() after nested_svm_uninit_mmu_context() Vitaly Kuznetsov
` (4 subsequent siblings)
9 siblings, 0 replies; 19+ messages in thread
From: Vitaly Kuznetsov @ 2020-07-09 14:53 UTC (permalink / raw)
To: kvm, Paolo Bonzini
Cc: Sean Christopherson, Wanpeng Li, Jim Mattson, Junaid Shahid,
linux-kernel
As a preparatory change for implementing nested specifig PGD switch for
nSVM (following nVMX' nested_vmx_load_cr3()) instead of relying on
kvm_set_cr3() introduce nested_svm_load_cr3().
The only intended functional change for now is that errors from
kvm_set_cr3() are propagated.
Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com>
---
arch/x86/kvm/svm/nested.c | 35 +++++++++++++++++++++++++++++++----
1 file changed, 31 insertions(+), 4 deletions(-)
diff --git a/arch/x86/kvm/svm/nested.c b/arch/x86/kvm/svm/nested.c
index 5e6c988a4e6b..d0fd63e8d835 100644
--- a/arch/x86/kvm/svm/nested.c
+++ b/arch/x86/kvm/svm/nested.c
@@ -311,8 +311,25 @@ static void nested_vmcb_save_pending_event(struct vcpu_svm *svm,
nested_vmcb->control.exit_int_info = exit_int_info;
}
-static void nested_prepare_vmcb_save(struct vcpu_svm *svm, struct vmcb *nested_vmcb)
+static inline bool nested_npt_enabled(struct vcpu_svm *svm)
{
+ return svm->nested.ctl.nested_ctl & SVM_NESTED_CTL_NP_ENABLE;
+}
+
+/*
+ * Load guest's cr3 at nested entry. @nested_npt is true if we are
+ * emulating VM-Entry into a guest with NPT enabled.
+ */
+static int nested_svm_load_cr3(struct kvm_vcpu *vcpu, unsigned long cr3,
+ bool nested_npt)
+{
+ return kvm_set_cr3(vcpu, cr3);
+}
+
+static int nested_prepare_vmcb_save(struct vcpu_svm *svm, struct vmcb *nested_vmcb)
+{
+ int ret = 0;
+
/* Load the nested guest state */
svm->vmcb->save.es = nested_vmcb->save.es;
svm->vmcb->save.cs = nested_vmcb->save.cs;
@@ -324,7 +341,9 @@ static void nested_prepare_vmcb_save(struct vcpu_svm *svm, struct vmcb *nested_v
svm_set_efer(&svm->vcpu, nested_vmcb->save.efer);
svm_set_cr0(&svm->vcpu, nested_vmcb->save.cr0);
svm_set_cr4(&svm->vcpu, nested_vmcb->save.cr4);
- (void)kvm_set_cr3(&svm->vcpu, nested_vmcb->save.cr3);
+
+ ret = nested_svm_load_cr3(&svm->vcpu, nested_vmcb->save.cr3,
+ nested_npt_enabled(svm));
svm->vmcb->save.cr2 = svm->vcpu.arch.cr2 = nested_vmcb->save.cr2;
kvm_rax_write(&svm->vcpu, nested_vmcb->save.rax);
@@ -338,12 +357,14 @@ static void nested_prepare_vmcb_save(struct vcpu_svm *svm, struct vmcb *nested_v
svm->vmcb->save.dr7 = nested_vmcb->save.dr7;
svm->vcpu.arch.dr6 = nested_vmcb->save.dr6;
svm->vmcb->save.cpl = nested_vmcb->save.cpl;
+
+ return ret;
}
static void nested_prepare_vmcb_control(struct vcpu_svm *svm)
{
const u32 mask = V_INTR_MASKING_MASK | V_GIF_ENABLE_MASK | V_GIF_MASK;
- if (svm->nested.ctl.nested_ctl & SVM_NESTED_CTL_NP_ENABLE)
+ if (nested_npt_enabled(svm))
nested_svm_init_mmu_context(&svm->vcpu);
/* Guest paging mode is active - reset mmu */
@@ -382,9 +403,15 @@ static void nested_prepare_vmcb_control(struct vcpu_svm *svm)
int enter_svm_guest_mode(struct vcpu_svm *svm, u64 vmcb_gpa,
struct vmcb *nested_vmcb)
{
+ int ret;
+
svm->nested.vmcb = vmcb_gpa;
load_nested_vmcb_control(svm, &nested_vmcb->control);
- nested_prepare_vmcb_save(svm, nested_vmcb);
+
+ ret = nested_prepare_vmcb_save(svm, nested_vmcb);
+ if (ret)
+ return ret;
+
nested_prepare_vmcb_control(svm);
svm_set_gif(svm, true);
--
2.25.4
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [PATCH v3 6/9] KVM: nSVM: move kvm_set_cr3() after nested_svm_uninit_mmu_context()
2020-07-09 14:53 [PATCH v3 0/9] KVM: nSVM: fixes for CR3/MMU switch upon nested guest entry/exit Vitaly Kuznetsov
` (4 preceding siblings ...)
2020-07-09 14:53 ` [PATCH v3 5/9] KVM: nSVM: introduce nested_svm_load_cr3() Vitaly Kuznetsov
@ 2020-07-09 14:53 ` Vitaly Kuznetsov
2020-07-09 17:51 ` Paolo Bonzini
2020-07-09 14:53 ` [PATCH v3 7/9] KVM: nSVM: implement nested_svm_load_cr3() and use it for host->guest switch Vitaly Kuznetsov
` (3 subsequent siblings)
9 siblings, 1 reply; 19+ messages in thread
From: Vitaly Kuznetsov @ 2020-07-09 14:53 UTC (permalink / raw)
To: kvm, Paolo Bonzini
Cc: Sean Christopherson, Wanpeng Li, Jim Mattson, Junaid Shahid,
linux-kernel
kvm_mmu_new_pgd() refers to arch.mmu and at this point it still references
arch.guest_mmu while arch.root_mmu is expected.
Note, the change is effectively a nop: when !npt_enabled,
nested_svm_uninit_mmu_context() does nothing (as we don't do
nested_svm_init_mmu_context()) and with npt_enabled we don't
do kvm_set_cr3() but we're about to switch to nested_svm_load_cr3().
No functional change intended.
Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com>
---
arch/x86/kvm/svm/nested.c | 14 ++++++++------
1 file changed, 8 insertions(+), 6 deletions(-)
diff --git a/arch/x86/kvm/svm/nested.c b/arch/x86/kvm/svm/nested.c
index d0fd63e8d835..5f001d2c41d1 100644
--- a/arch/x86/kvm/svm/nested.c
+++ b/arch/x86/kvm/svm/nested.c
@@ -621,12 +621,6 @@ int nested_svm_vmexit(struct vcpu_svm *svm)
svm_set_efer(&svm->vcpu, hsave->save.efer);
svm_set_cr0(&svm->vcpu, hsave->save.cr0 | X86_CR0_PE);
svm_set_cr4(&svm->vcpu, hsave->save.cr4);
- if (npt_enabled) {
- svm->vmcb->save.cr3 = hsave->save.cr3;
- svm->vcpu.arch.cr3 = hsave->save.cr3;
- } else {
- (void)kvm_set_cr3(&svm->vcpu, hsave->save.cr3);
- }
kvm_rax_write(&svm->vcpu, hsave->save.rax);
kvm_rsp_write(&svm->vcpu, hsave->save.rsp);
kvm_rip_write(&svm->vcpu, hsave->save.rip);
@@ -646,6 +640,14 @@ int nested_svm_vmexit(struct vcpu_svm *svm)
kvm_vcpu_unmap(&svm->vcpu, &map, true);
nested_svm_uninit_mmu_context(&svm->vcpu);
+
+ if (npt_enabled) {
+ svm->vmcb->save.cr3 = hsave->save.cr3;
+ svm->vcpu.arch.cr3 = hsave->save.cr3;
+ } else {
+ (void)kvm_set_cr3(&svm->vcpu, hsave->save.cr3);
+ }
+
kvm_mmu_reset_context(&svm->vcpu);
kvm_mmu_load(&svm->vcpu);
--
2.25.4
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [PATCH v3 7/9] KVM: nSVM: implement nested_svm_load_cr3() and use it for host->guest switch
2020-07-09 14:53 [PATCH v3 0/9] KVM: nSVM: fixes for CR3/MMU switch upon nested guest entry/exit Vitaly Kuznetsov
` (5 preceding siblings ...)
2020-07-09 14:53 ` [PATCH v3 6/9] KVM: nSVM: move kvm_set_cr3() after nested_svm_uninit_mmu_context() Vitaly Kuznetsov
@ 2020-07-09 14:53 ` Vitaly Kuznetsov
2020-07-09 17:57 ` Paolo Bonzini
2020-07-09 14:53 ` [PATCH v3 8/9] KVM: nSVM: use nested_svm_load_cr3() on guest->host switch Vitaly Kuznetsov
` (2 subsequent siblings)
9 siblings, 1 reply; 19+ messages in thread
From: Vitaly Kuznetsov @ 2020-07-09 14:53 UTC (permalink / raw)
To: kvm, Paolo Bonzini
Cc: Sean Christopherson, Wanpeng Li, Jim Mattson, Junaid Shahid,
linux-kernel
Undesired triple fault gets injected to L1 guest on SVM when L2 is
launched with certain CR3 values. #TF is raised by mmu_check_root()
check in fast_pgd_switch() and the root cause is that when
kvm_set_cr3() is called from nested_prepare_vmcb_save() with NPT
enabled CR3 points to a nGPA so we can't check it with
kvm_is_visible_gfn().
Using generic kvm_set_cr3() when switching to nested guest is not
a great idea as we'll have to distinguish between 'real' CR3s and
'nested' CR3s to e.g. not call kvm_mmu_new_pgd() with nGPA. Following
nVMX implement nested-specific nested_svm_load_cr3() doing the job.
Note: the current implementation is sub-optimal as we always do TLB
flush/MMU sync but this is still an improvement as we at least stop doing
kvm_mmu_reset_context().
Fixes: 7c390d350f8b ("kvm: x86: Add fast CR3 switch code path")
Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com>
---
arch/x86/kvm/mmu/mmu.c | 2 ++
arch/x86/kvm/svm/nested.c | 33 +++++++++++++++++++++++++--------
2 files changed, 27 insertions(+), 8 deletions(-)
diff --git a/arch/x86/kvm/mmu/mmu.c b/arch/x86/kvm/mmu/mmu.c
index 69fa51af8cbf..1c3a231f825b 100644
--- a/arch/x86/kvm/mmu/mmu.c
+++ b/arch/x86/kvm/mmu/mmu.c
@@ -4986,6 +4986,8 @@ void kvm_init_shadow_npt_mmu(struct kvm_vcpu *vcpu, u32 cr0, u32 cr4, u32 efer,
union kvm_mmu_role new_role =
kvm_calc_shadow_mmu_root_page_role(vcpu, false);
+ __kvm_mmu_new_pgd(vcpu, nested_cr3, new_role.base, false, false);
+
if (new_role.as_u64 != context->mmu_role.as_u64)
shadow_mmu_init_context(vcpu, context, cr0, cr4, efer, new_role);
}
diff --git a/arch/x86/kvm/svm/nested.c b/arch/x86/kvm/svm/nested.c
index 5f001d2c41d1..5ddf20941cf9 100644
--- a/arch/x86/kvm/svm/nested.c
+++ b/arch/x86/kvm/svm/nested.c
@@ -323,7 +323,28 @@ static inline bool nested_npt_enabled(struct vcpu_svm *svm)
static int nested_svm_load_cr3(struct kvm_vcpu *vcpu, unsigned long cr3,
bool nested_npt)
{
- return kvm_set_cr3(vcpu, cr3);
+ if (cr3 & rsvd_bits(cpuid_maxphyaddr(vcpu), 63))
+ return -EINVAL;
+
+ if (!nested_npt && is_pae_paging(vcpu) &&
+ (cr3 != kvm_read_cr3(vcpu) || pdptrs_changed(vcpu))) {
+ if (!load_pdptrs(vcpu, vcpu->arch.walk_mmu, cr3))
+ return -EINVAL;
+ }
+
+ /*
+ * TODO: optimize unconditional TLB flush/MMU sync here and in
+ * kvm_init_shadow_npt_mmu().
+ */
+ if (!nested_npt)
+ kvm_mmu_new_pgd(vcpu, cr3, false, false);
+
+ vcpu->arch.cr3 = cr3;
+ kvm_register_mark_available(vcpu, VCPU_EXREG_CR3);
+
+ kvm_init_mmu(vcpu, false);
+
+ return 0;
}
static int nested_prepare_vmcb_save(struct vcpu_svm *svm, struct vmcb *nested_vmcb)
@@ -342,6 +363,9 @@ static int nested_prepare_vmcb_save(struct vcpu_svm *svm, struct vmcb *nested_vm
svm_set_cr0(&svm->vcpu, nested_vmcb->save.cr0);
svm_set_cr4(&svm->vcpu, nested_vmcb->save.cr4);
+ if (nested_npt_enabled(svm))
+ nested_svm_init_mmu_context(&svm->vcpu);
+
ret = nested_svm_load_cr3(&svm->vcpu, nested_vmcb->save.cr3,
nested_npt_enabled(svm));
@@ -364,13 +388,6 @@ static int nested_prepare_vmcb_save(struct vcpu_svm *svm, struct vmcb *nested_vm
static void nested_prepare_vmcb_control(struct vcpu_svm *svm)
{
const u32 mask = V_INTR_MASKING_MASK | V_GIF_ENABLE_MASK | V_GIF_MASK;
- if (nested_npt_enabled(svm))
- nested_svm_init_mmu_context(&svm->vcpu);
-
- /* Guest paging mode is active - reset mmu */
- kvm_mmu_reset_context(&svm->vcpu);
-
- svm_flush_tlb(&svm->vcpu);
svm->vmcb->control.tsc_offset = svm->vcpu.arch.tsc_offset =
svm->vcpu.arch.l1_tsc_offset + svm->nested.ctl.tsc_offset;
--
2.25.4
^ permalink raw reply related [flat|nested] 19+ messages in thread
* Re: [PATCH v3 7/9] KVM: nSVM: implement nested_svm_load_cr3() and use it for host->guest switch
2020-07-09 14:53 ` [PATCH v3 7/9] KVM: nSVM: implement nested_svm_load_cr3() and use it for host->guest switch Vitaly Kuznetsov
@ 2020-07-09 17:57 ` Paolo Bonzini
2020-07-09 17:59 ` Paolo Bonzini
0 siblings, 1 reply; 19+ messages in thread
From: Paolo Bonzini @ 2020-07-09 17:57 UTC (permalink / raw)
To: Vitaly Kuznetsov, kvm
Cc: Sean Christopherson, Wanpeng Li, Jim Mattson, Junaid Shahid,
linux-kernel
On 09/07/20 16:53, Vitaly Kuznetsov wrote:
> + if (nested_npt_enabled(svm))
> + nested_svm_init_mmu_context(&svm->vcpu);
> +
> ret = nested_svm_load_cr3(&svm->vcpu, nested_vmcb->save.cr3,
> nested_npt_enabled(svm));
This needs to be done in svm_set_nested_state, so my suggestion is that
the previous patch includes a call to nested_svm_load_cr3 in
svm_set_nested_state, and this one adds the "if" inside
nested_svm_load_cr3 itself.
Paolo
> @@ -364,13 +388,6 @@ static int nested_prepare_vmcb_save(struct vcpu_svm *svm, struct vmcb *nested_vm
> static void nested_prepare_vmcb_control(struct vcpu_svm *svm)
> {
> const u32 mask = V_INTR_MASKING_MASK | V_GIF_ENABLE_MASK | V_GIF_MASK;
> - if (nested_npt_enabled(svm))
> - nested_svm_init_mmu_context(&svm->vcpu);
> -
> - /* Guest paging mode is active - reset mmu */
> - kvm_mmu_reset_context(&svm->vcpu);
> -
> - svm_flush_tlb(&svm->vcpu);
>
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v3 7/9] KVM: nSVM: implement nested_svm_load_cr3() and use it for host->guest switch
2020-07-09 17:57 ` Paolo Bonzini
@ 2020-07-09 17:59 ` Paolo Bonzini
2020-07-10 11:40 ` Vitaly Kuznetsov
0 siblings, 1 reply; 19+ messages in thread
From: Paolo Bonzini @ 2020-07-09 17:59 UTC (permalink / raw)
To: Vitaly Kuznetsov, kvm
Cc: Sean Christopherson, Wanpeng Li, Jim Mattson, Junaid Shahid,
linux-kernel
On 09/07/20 19:57, Paolo Bonzini wrote:
> On 09/07/20 16:53, Vitaly Kuznetsov wrote:
>> + if (nested_npt_enabled(svm))
>> + nested_svm_init_mmu_context(&svm->vcpu);
>> +
>> ret = nested_svm_load_cr3(&svm->vcpu, nested_vmcb->save.cr3,
>> nested_npt_enabled(svm));
>
> This needs to be done in svm_set_nested_state, so my suggestion is that
> the previous patch includes a call to nested_svm_load_cr3 in
> svm_set_nested_state, and this one adds the "if" inside
> nested_svm_load_cr3 itself.
Actually no, that doesn't work after the next patch. So the best option
is probably to extract nested_svm_init_mmu as a separate step in
enter_svm_guest_mode. This also leaves nested_prepare_vmcb_save as a
void function.
Paolo
>
> Paolo
>
>> @@ -364,13 +388,6 @@ static int nested_prepare_vmcb_save(struct vcpu_svm *svm, struct vmcb *nested_vm
>> static void nested_prepare_vmcb_control(struct vcpu_svm *svm)
>> {
>> const u32 mask = V_INTR_MASKING_MASK | V_GIF_ENABLE_MASK | V_GIF_MASK;
>> - if (nested_npt_enabled(svm))
>> - nested_svm_init_mmu_context(&svm->vcpu);
>> -
>> - /* Guest paging mode is active - reset mmu */
>> - kvm_mmu_reset_context(&svm->vcpu);
>> -
>> - svm_flush_tlb(&svm->vcpu);
>>
>
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v3 7/9] KVM: nSVM: implement nested_svm_load_cr3() and use it for host->guest switch
2020-07-09 17:59 ` Paolo Bonzini
@ 2020-07-10 11:40 ` Vitaly Kuznetsov
2020-07-10 12:08 ` Paolo Bonzini
0 siblings, 1 reply; 19+ messages in thread
From: Vitaly Kuznetsov @ 2020-07-10 11:40 UTC (permalink / raw)
To: Paolo Bonzini, kvm
Cc: Sean Christopherson, Wanpeng Li, Jim Mattson, Junaid Shahid,
linux-kernel
Paolo Bonzini <pbonzini@redhat.com> writes:
> On 09/07/20 19:57, Paolo Bonzini wrote:
>> On 09/07/20 16:53, Vitaly Kuznetsov wrote:
>>> + if (nested_npt_enabled(svm))
>>> + nested_svm_init_mmu_context(&svm->vcpu);
>>> +
>>> ret = nested_svm_load_cr3(&svm->vcpu, nested_vmcb->save.cr3,
>>> nested_npt_enabled(svm));
>>
>> This needs to be done in svm_set_nested_state, so my suggestion is that
>> the previous patch includes a call to nested_svm_load_cr3 in
>> svm_set_nested_state, and this one adds the "if" inside
>> nested_svm_load_cr3 itself.
>
> Actually no, that doesn't work after the next patch. So the best option
> is probably to extract nested_svm_init_mmu as a separate step in
> enter_svm_guest_mode. This also leaves nested_prepare_vmcb_save as a
> void function.
>
Hm, it seems I missed svm_set_nested_state() path
completely. Surprisingly, state_test didn't fail)
I'm struggling a bit to understand why we don't have kvm_set_cr3() on
svm_set_nested_state() path: enter_svm_guest_mode() does it through
nested_prepare_vmcb_save() but it is skipped in svm_set_nested_state().
Don't we need it at least for !npt_enabled case? We'll have to extract
nested_cr3 from nested_vmcb then.
--
Vitaly
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v3 7/9] KVM: nSVM: implement nested_svm_load_cr3() and use it for host->guest switch
2020-07-10 11:40 ` Vitaly Kuznetsov
@ 2020-07-10 12:08 ` Paolo Bonzini
2020-07-10 12:30 ` Vitaly Kuznetsov
0 siblings, 1 reply; 19+ messages in thread
From: Paolo Bonzini @ 2020-07-10 12:08 UTC (permalink / raw)
To: Vitaly Kuznetsov, kvm
Cc: Sean Christopherson, Wanpeng Li, Jim Mattson, Junaid Shahid,
linux-kernel
On 10/07/20 13:40, Vitaly Kuznetsov wrote:
> Hm, it seems I missed svm_set_nested_state() path
> completely. Surprisingly, state_test didn't fail)
>
> I'm struggling a bit to understand why we don't have kvm_set_cr3() on
> svm_set_nested_state() path: enter_svm_guest_mode() does it through
> nested_prepare_vmcb_save() but it is skipped in svm_set_nested_state().
> Don't we need it at least for !npt_enabled case?
In svm_set_nested_state you'll have CR3 already set to the right value.
On the source, KVM_GET_SREGS returns the vmcb12's CR3 and it is already
restored with KVM_SET_SREGS on the destination before set_nested_state.
So, only the nested_cr3 has to be set.
Paolo
> We'll have to extract
> nested_cr3 from nested_vmcb then.
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v3 7/9] KVM: nSVM: implement nested_svm_load_cr3() and use it for host->guest switch
2020-07-10 12:08 ` Paolo Bonzini
@ 2020-07-10 12:30 ` Vitaly Kuznetsov
0 siblings, 0 replies; 19+ messages in thread
From: Vitaly Kuznetsov @ 2020-07-10 12:30 UTC (permalink / raw)
To: Paolo Bonzini, kvm
Cc: Sean Christopherson, Wanpeng Li, Jim Mattson, Junaid Shahid,
linux-kernel
Paolo Bonzini <pbonzini@redhat.com> writes:
> On 10/07/20 13:40, Vitaly Kuznetsov wrote:
>> Hm, it seems I missed svm_set_nested_state() path
>> completely. Surprisingly, state_test didn't fail)
>>
>> I'm struggling a bit to understand why we don't have kvm_set_cr3() on
>> svm_set_nested_state() path: enter_svm_guest_mode() does it through
>> nested_prepare_vmcb_save() but it is skipped in svm_set_nested_state().
>> Don't we need it at least for !npt_enabled case?
>
> In svm_set_nested_state you'll have CR3 already set to the right value.
> On the source, KVM_GET_SREGS returns the vmcb12's CR3 and it is already
> restored with KVM_SET_SREGS on the destination before set_nested_state.
>
> So, only the nested_cr3 has to be set.
>
Ah, thanks, so it seems there is no need to merge nested_svm_load_cr3()
(former kvm_set_cr3()) with nested_svm_init_mmu_context() in a new
nested_svm_init_mmu() as we'll only need the latter on
svm_set_nested_state(). We can just move nested_svm_load_cr3() out of
nested_prepare_vmcb_save() to enter_svm_guest_mode().
--
Vitaly
^ permalink raw reply [flat|nested] 19+ messages in thread
* [PATCH v3 8/9] KVM: nSVM: use nested_svm_load_cr3() on guest->host switch
2020-07-09 14:53 [PATCH v3 0/9] KVM: nSVM: fixes for CR3/MMU switch upon nested guest entry/exit Vitaly Kuznetsov
` (6 preceding siblings ...)
2020-07-09 14:53 ` [PATCH v3 7/9] KVM: nSVM: implement nested_svm_load_cr3() and use it for host->guest switch Vitaly Kuznetsov
@ 2020-07-09 14:53 ` Vitaly Kuznetsov
2020-07-09 14:53 ` [PATCH v3 9/9] KVM: x86: drop superfluous mmu_check_root() from fast_pgd_switch() Vitaly Kuznetsov
2020-07-09 18:01 ` [PATCH v3 0/9] KVM: nSVM: fixes for CR3/MMU switch upon nested guest entry/exit Paolo Bonzini
9 siblings, 0 replies; 19+ messages in thread
From: Vitaly Kuznetsov @ 2020-07-09 14:53 UTC (permalink / raw)
To: kvm, Paolo Bonzini
Cc: Sean Christopherson, Wanpeng Li, Jim Mattson, Junaid Shahid,
linux-kernel
Make nSVM code resemble nVMX where nested_vmx_load_cr3() is used on
both guest->host and host->guest transitions. Also, we can now
eliminate unconditional kvm_mmu_reset_context() and speed things up.
Note, nVMX has two different paths: load_vmcs12_host_state() and
nested_vmx_restore_host_state() and the later is used to restore from
'partial' switch to L2, it always uses kvm_mmu_reset_context().
nSVM doesn't have this yet. Also, nested_svm_vmexit()'s return value
is almost always ignored nowadays.
Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com>
---
arch/x86/kvm/svm/nested.c | 15 ++++++---------
1 file changed, 6 insertions(+), 9 deletions(-)
diff --git a/arch/x86/kvm/svm/nested.c b/arch/x86/kvm/svm/nested.c
index 5ddf20941cf9..9a137d3b3a1c 100644
--- a/arch/x86/kvm/svm/nested.c
+++ b/arch/x86/kvm/svm/nested.c
@@ -317,7 +317,7 @@ static inline bool nested_npt_enabled(struct vcpu_svm *svm)
}
/*
- * Load guest's cr3 at nested entry. @nested_npt is true if we are
+ * Load guest's/host's cr3 at nested entry. @nested_npt is true if we are
* emulating VM-Entry into a guest with NPT enabled.
*/
static int nested_svm_load_cr3(struct kvm_vcpu *vcpu, unsigned long cr3,
@@ -658,15 +658,12 @@ int nested_svm_vmexit(struct vcpu_svm *svm)
nested_svm_uninit_mmu_context(&svm->vcpu);
- if (npt_enabled) {
- svm->vmcb->save.cr3 = hsave->save.cr3;
- svm->vcpu.arch.cr3 = hsave->save.cr3;
- } else {
- (void)kvm_set_cr3(&svm->vcpu, hsave->save.cr3);
- }
+ rc = nested_svm_load_cr3(&svm->vcpu, hsave->save.cr3, false);
+ if (rc)
+ return 1;
- kvm_mmu_reset_context(&svm->vcpu);
- kvm_mmu_load(&svm->vcpu);
+ if (npt_enabled)
+ svm->vmcb->save.cr3 = hsave->save.cr3;
/*
* Drop what we picked up for L2 via svm_complete_interrupts() so it
--
2.25.4
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [PATCH v3 9/9] KVM: x86: drop superfluous mmu_check_root() from fast_pgd_switch()
2020-07-09 14:53 [PATCH v3 0/9] KVM: nSVM: fixes for CR3/MMU switch upon nested guest entry/exit Vitaly Kuznetsov
` (7 preceding siblings ...)
2020-07-09 14:53 ` [PATCH v3 8/9] KVM: nSVM: use nested_svm_load_cr3() on guest->host switch Vitaly Kuznetsov
@ 2020-07-09 14:53 ` Vitaly Kuznetsov
2020-07-09 18:01 ` [PATCH v3 0/9] KVM: nSVM: fixes for CR3/MMU switch upon nested guest entry/exit Paolo Bonzini
9 siblings, 0 replies; 19+ messages in thread
From: Vitaly Kuznetsov @ 2020-07-09 14:53 UTC (permalink / raw)
To: kvm, Paolo Bonzini
Cc: Sean Christopherson, Wanpeng Li, Jim Mattson, Junaid Shahid,
linux-kernel
The mmu_check_root() check in fast_pgd_switch() seems to be
superfluous: when GPA is outside of the visible range
cached_root_available() will fail for non-direct roots
(as we can't have a matching one on the list) and we don't
seem to care for direct ones.
Also, raising #TF immediately when a non-existent GFN is written to CR3
doesn't seem to mach architectural behavior. Drop the check.
Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com>
---
arch/x86/kvm/mmu/mmu.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/arch/x86/kvm/mmu/mmu.c b/arch/x86/kvm/mmu/mmu.c
index 1c3a231f825b..c004ef9caf8f 100644
--- a/arch/x86/kvm/mmu/mmu.c
+++ b/arch/x86/kvm/mmu/mmu.c
@@ -4277,8 +4277,7 @@ static bool fast_pgd_switch(struct kvm_vcpu *vcpu, gpa_t new_pgd,
*/
if (mmu->shadow_root_level >= PT64_ROOT_4LEVEL &&
mmu->root_level >= PT64_ROOT_4LEVEL)
- return !mmu_check_root(vcpu, new_pgd >> PAGE_SHIFT) &&
- cached_root_available(vcpu, new_pgd, new_role);
+ return cached_root_available(vcpu, new_pgd, new_role);
return false;
}
--
2.25.4
^ permalink raw reply related [flat|nested] 19+ messages in thread
* Re: [PATCH v3 0/9] KVM: nSVM: fixes for CR3/MMU switch upon nested guest entry/exit
2020-07-09 14:53 [PATCH v3 0/9] KVM: nSVM: fixes for CR3/MMU switch upon nested guest entry/exit Vitaly Kuznetsov
` (8 preceding siblings ...)
2020-07-09 14:53 ` [PATCH v3 9/9] KVM: x86: drop superfluous mmu_check_root() from fast_pgd_switch() Vitaly Kuznetsov
@ 2020-07-09 18:01 ` Paolo Bonzini
9 siblings, 0 replies; 19+ messages in thread
From: Paolo Bonzini @ 2020-07-09 18:01 UTC (permalink / raw)
To: Vitaly Kuznetsov, kvm
Cc: Sean Christopherson, Wanpeng Li, Jim Mattson, Junaid Shahid,
linux-kernel
On 09/07/20 16:53, Vitaly Kuznetsov wrote:
> This is a successor of "[PATCH v2 0/3] KVM: nSVM: fix #TF from CR3 switch
> when entering guest" and "[PATCH] KVM: x86: drop erroneous mmu_check_root()
> from fast_pgd_switch()".
>
> The snowball is growing fast! It all started with an intention to fix
> the particular 'tripple fault' issue (now fixed by PATCH7) but now we
> also get rid of unconditional kvm_mmu_reset_context() upon nested guest
> entry/exit and make the code resemble nVMX. There is still a huge room
> for further improvement (proper error propagation, removing unconditional
> MMU sync/TLB flush,...) but at least we're making some progress.
>
> Tested with kvm selftests/kvm-unit-tests and by running nested Hyper-V
> on KVM. The series doesn't seem to introduce any new issues.
Looks like a v4 is needed but we're converging at least, it won't have
27 patches. :)
> Vitaly Kuznetsov (9):
> KVM: nSVM: split kvm_init_shadow_npt_mmu() from kvm_init_shadow_mmu()
> KVM: nSVM: stop dereferencing vcpu->arch.mmu to get the context in
> kvm_init_shadow{,_npt}_mmu()
> KVM: nSVM: reset nested_run_pending upon nested_svm_vmrun_msrpm()
> failure
> KVM: nSVM: prepare to handle errors from enter_svm_guest_mode()
> KVM: nSVM: introduce nested_svm_load_cr3()
> KVM: nSVM: move kvm_set_cr3() after nested_svm_uninit_mmu_context()
> KVM: nSVM: implement nested_svm_load_cr3() and use it for host->guest
> switch
> KVM: nSVM: use nested_svm_load_cr3() on guest->host switch
> KVM: x86: drop superfluous mmu_check_root() from fast_pgd_switch()
>
> arch/x86/kvm/mmu.h | 3 +-
> arch/x86/kvm/mmu/mmu.c | 39 ++++++++++----
> arch/x86/kvm/svm/nested.c | 108 ++++++++++++++++++++++++++++----------
> arch/x86/kvm/svm/svm.c | 6 ++-
> arch/x86/kvm/svm/svm.h | 4 +-
> 5 files changed, 116 insertions(+), 44 deletions(-)
>
^ permalink raw reply [flat|nested] 19+ messages in thread