Re: [PATCH 10/16] KVM: x86/tdp_mmu: Support TDX private mapping for TDP MMU

["Edgecombe, Rick P" <rick.p.edgecombe@intel.com>]

On Mon, 2024-05-20 at 12:02 -0700, Rick Edgecombe wrote:
> 
> reflect is a nice name. I'm trying this path right now. I'll share a branch.

Here is the branch:
https://github.com/rpedgeco/linux/commit/674cd68b6ba626e48fe2446797d067e38dca80e3

TODO:
 - kvm_mmu_max_gfn() updates from iterator changes
 - kvm_flush_remote_tlbs_gfn() updates from iterator changes

The historically controversial mmu.h helpers:
static inline gfn_t kvm_gfn_direct_mask(const struct kvm *kvm)
{
	/* Only TDX sets this and it's the shared mask */
	return kvm->arch.gfn_shared_mask;
}

/* The VM keeps a mirrored copy of the private memory */
static inline bool kvm_has_mirrored_tdp(const struct kvm *kvm)
{
	return kvm->arch.vm_type == KVM_X86_TDX_VM;
}

static inline bool kvm_on_mirror(const struct kvm *kvm, enum kvm_process
process)
{
	if (!kvm_has_mirrored_tdp(kvm))
		return false;

	return process & KVM_PROCESS_PRIVATE;
}

static inline bool kvm_on_direct(const struct kvm *kvm, enum kvm_process
process)
{
	if (!kvm_has_mirrored_tdp(kvm))
		return true;

	return process & KVM_PROCESS_SHARED;
}

static inline bool kvm_zap_leafs_only(const struct kvm *kvm)
{
	return kvm->arch.vm_type == KVM_X86_TDX_VM;
}


In this solution, the tdp_mmu.c doesn't have a concept of private vs shared EPT
or GPA aliases. It just knows KVM_PROCESS_PRIVATE/SHARED, and fault->is_private.

Based on the PROCESS enums or fault->is_private, helpers in mmu.h encapsulate
whether to operate on the normal "direct" roots or the mirrored roots. When
!TDX, it always operates on direct.

The code that does PTE setting/zapping etc, calls out the mirrored "reflect"
helper and does the extra atomicity stuff when it sees the mirrored role bit.

In Isaku's code to make gfn's never have shared bits, there was still the
concept of "shared" in the TDP MMU. But now since the TDP MMU focuses on
mirrored vs direct instead, an abstraction is introduced to just ask for the
mask for the root. For TDX the direct root is for shared memory, so instead the
kvm_gfn_direct_mask() gets applied when operating on the direct root.

I think there are still some things to be polished in the branch, but overall it
does a good job of cleaning up the confusion about the connection between
private and mirrored. And also between this and the previous changes, improves
littering the generic MMU code with private/shared alias concepts.

At the same time, I think the abstractions have a small cost in clarity if you
are looking at the code from TDX's perspective. It probably wont raise any
eyebrows for people used to tracing nested EPT violations through paging_tmpl.h.
But compared to naming everything mirrored_private, there is more obfuscation of
the bits twiddled.