Re: [PATCH 16/15] KVM: X86: Update mmu->pdptrs only when it is changed

Sean Christopherson
2021-12-07T23:43:53Z

On Thu, Nov 11, 2021, Lai Jiangshan wrote:
> From: Lai Jiangshan <laijs@linux.alibaba.com>
>
> It is unchanged in most cases.
>
> Signed-off-by: Lai Jiangshan <laijs@linux.alibaba.com>
> ---
> arch/x86/kvm/x86.c | 11 +++++++----
> 1 file changed, 7 insertions(+), 4 deletions(-)
>
> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> index 6ca19cac4aff..0176eaa86a35 100644
> --- a/arch/x86/kvm/x86.c
> +++ b/arch/x86/kvm/x86.c
> @@ -828,10 +828,13 @@ int load_pdptrs(struct kvm_vcpu *vcpu, struct kvm_mmu *mmu, unsigned long cr3)
> }
> }
>
> - memcpy(mmu->pdptrs, pdpte, sizeof(mmu->pdptrs));
> - kvm_register_mark_dirty(vcpu, VCPU_EXREG_PDPTR);
> - /* Ensure the dirty PDPTEs to be loaded. */
> - kvm_make_request(KVM_REQ_LOAD_MMU_PGD, vcpu);
> + kvm_register_mark_available(vcpu, VCPU_EXREG_PDPTR);
> + if (memcmp(mmu->pdptrs, pdpte, sizeof(mmu->pdptrs))) {
> + memcpy(mmu->pdptrs, pdpte, sizeof(mmu->pdptrs));
> + kvm_register_mark_dirty(vcpu, VCPU_EXREG_PDPTR);
> + /* Ensure the dirty PDPTEs to be loaded. */
> + kvm_make_request(KVM_REQ_LOAD_MMU_PGD, vcpu);
> + }

Can this be unqueued until there's sufficient justification that (a) this is
correct and (b) actually provides a meaningful performance optimization? There
have been far too many PDPTR caching bugs to make this change without an analysis
of why it's safe, e.g. what guarantees the that PDPTRs in the VMCS are sync'd
with mmu->pdptrs? I'm not saying they aren't, I just want the changelog to prove
that they are.

The next patch does add a fairly heavy unload of the current root for !TDP, but
that's a bug fix and should be ordered before any optimizations anyways.

> vcpu->arch.pdptrs_from_userspace = false;
>
> return 1;
> --
> 2.19.1.6.gb485710b
>