On Mon, Feb 21, 2022, Paolo Bonzini wrote:
> Most of the time, calls to get_guest_pgd result in calling kvm_read_cr3
> (the exception is only nested TDP). Check if that is the case if
> retpolines are enabled, thus avoiding an expensive indirect call.
>
> Signed-off-by: Paolo Bonzini <pbonzini@xxxxxxxxxx>
> ---
> arch/x86/kvm/mmu.h | 10 ++++++++++
> arch/x86/kvm/mmu/mmu.c | 15 ++++++++-------
> arch/x86/kvm/mmu/paging_tmpl.h | 2 +-
> arch/x86/kvm/x86.c | 2 +-
> 4 files changed, 20 insertions(+), 9 deletions(-)
>
> diff --git a/arch/x86/kvm/mmu.h b/arch/x86/kvm/mmu.h
> index 1d0c1904d69a..6ee4436e46f1 100644
> --- a/arch/x86/kvm/mmu.h
> +++ b/arch/x86/kvm/mmu.h
> @@ -116,6 +116,16 @@ static inline void kvm_mmu_load_pgd(struct kvm_vcpu *vcpu)
> vcpu->arch.mmu->shadow_root_level);
> }
>
> +extern unsigned long kvm_get_guest_cr3(struct kvm_vcpu *vcpu);
No extern please, it's superfluous and against KVM style. Moot point though, see
below.
> +static inline unsigned long kvm_mmu_get_guest_pgd(struct kvm_vcpu *vcpu, struct kvm_mmu *mmu)
Wrap the params, no reason to make this line so long.
> +{
> +#ifdef CONFIG_RETPOLINE
> + if (mmu->get_guest_pgd == kvm_get_guest_cr3)
> + return kvm_read_cr3(vcpu);
This is unnecessarily fragile and confusing at first glance. Compilers are smart
enough to generate a non-inline version of functions if they're used for function
pointers, while still inlining where appropriate. In other words, just drop
kvm_get_guest_cr3() entirely, a al get_pdptr => kvm_pdptr_read().
---
arch/x86/kvm/mmu.h | 6 +++---
arch/x86/kvm/mmu/mmu.c | 11 +++--------
2 files changed, 6 insertions(+), 11 deletions(-)
diff --git a/arch/x86/kvm/mmu.h b/arch/x86/kvm/mmu.h
index 3af66b9df640..50528d39de8d 100644
--- a/arch/x86/kvm/mmu.h
+++ b/arch/x86/kvm/mmu.h
@@ -117,11 +117,11 @@ static inline void kvm_mmu_load_pgd(struct kvm_vcpu *vcpu)
vcpu->arch.mmu->shadow_root_level);
}
-extern unsigned long kvm_get_guest_cr3(struct kvm_vcpu *vcpu);
-static inline unsigned long kvm_mmu_get_guest_pgd(struct kvm_vcpu *vcpu, struct kvm_mmu *mmu)
+static inline unsigned long kvm_mmu_get_guest_pgd(struct kvm_vcpu *vcpu,
+ struct kvm_mmu *mmu)
{
#ifdef CONFIG_RETPOLINE
- if (mmu->get_guest_pgd == kvm_get_guest_cr3)
+ if (mmu->get_guest_pgd == kvm_read_cr3)
return kvm_read_cr3(vcpu);
#endif
return mmu->get_guest_pgd(vcpu);
diff --git a/arch/x86/kvm/mmu/mmu.c b/arch/x86/kvm/mmu/mmu.c
index 995c3450c20f..cc2414397e4b 100644
--- a/arch/x86/kvm/mmu/mmu.c
+++ b/arch/x86/kvm/mmu/mmu.c
@@ -4234,11 +4234,6 @@ void kvm_mmu_new_pgd(struct kvm_vcpu *vcpu, gpa_t new_pgd)
}
EXPORT_SYMBOL_GPL(kvm_mmu_new_pgd);
-unsigned long kvm_get_guest_cr3(struct kvm_vcpu *vcpu)
-{
- return kvm_read_cr3(vcpu);
-}
-
static bool sync_mmio_spte(struct kvm_vcpu *vcpu, u64 *sptep, gfn_t gfn,
unsigned int access)
{
@@ -4793,7 +4788,7 @@ static void init_kvm_tdp_mmu(struct kvm_vcpu *vcpu)
context->invlpg = NULL;
context->shadow_root_level = kvm_mmu_get_tdp_level(vcpu);
context->direct_map = true;
- context->get_guest_pgd = kvm_get_guest_cr3;
+ context->get_guest_pgd = kvm_read_cr3;
context->get_pdptr = kvm_pdptr_read;
context->inject_page_fault = kvm_inject_page_fault;
context->root_level = role_regs_to_root_level(®s);
@@ -4968,7 +4963,7 @@ static void init_kvm_softmmu(struct kvm_vcpu *vcpu)
kvm_init_shadow_mmu(vcpu, ®s);
- context->get_guest_pgd = kvm_get_guest_cr3;
+ context->get_guest_pgd = kvm_read_cr3;
context->get_pdptr = kvm_pdptr_read;
context->inject_page_fault = kvm_inject_page_fault;
}
@@ -5000,7 +4995,7 @@ static void init_kvm_nested_mmu(struct kvm_vcpu *vcpu)
return;
g_context->mmu_role.as_u64 = new_role.as_u64;
- g_context->get_guest_pgd = kvm_get_guest_cr3;
+ g_context->get_guest_pgd = kvm_read_cr3;
g_context->get_pdptr = kvm_pdptr_read;
g_context->inject_page_fault = kvm_inject_page_fault;
g_context->root_level = new_role.base.level;
base-commit: c31df3e63672c14d8b52e34606c823e2166024b8
--
