On Tue, Nov 22, 2022 at 12:06 PM Sean Christopherson seanjc@google.com wrote:
On Tue, Nov 22, 2022, Chao Peng wrote:
diff --git a/arch/x86/kvm/mmu/mmu.c b/arch/x86/kvm/mmu/mmu.c index 10017a9f26ee..b3118d00b284 100644 --- a/arch/x86/kvm/mmu/mmu.c +++ b/arch/x86/kvm/mmu/mmu.c @@ -4280,6 +4280,10 @@ static int direct_page_fault(struct kvm_vcpu *vcpu, struct kvm_page_fault *fault
fault->gfn = fault->addr >> PAGE_SHIFT; fault->slot = kvm_vcpu_gfn_to_memslot(vcpu, fault->gfn);+#ifdef CONFIG_HAVE_KVM_PRIVATE_MEM_TESTING
- fault->is_private = kvm_slot_can_be_private(fault->slot) &&
kvm_mem_is_private(vcpu->kvm, fault->gfn);+#endif
if (page_fault_handle_page_track(vcpu, fault)) return RET_PF_EMULATE;diff --git a/arch/x86/kvm/mmu/mmu_internal.h b/arch/x86/kvm/mmu/mmu_internal.h index 5cdff5ca546c..2e759f39c2c5 100644 --- a/arch/x86/kvm/mmu/mmu_internal.h +++ b/arch/x86/kvm/mmu/mmu_internal.h @@ -188,7 +188,6 @@ struct kvm_page_fault {
/* Derived from mmu and global state. */ const bool is_tdp;
const bool is_private; const bool nx_huge_page_workaround_enabled;
/*
@@ -221,6 +220,9 @@ struct kvm_page_fault { /* The memslot containing gfn. May be NULL. */ struct kvm_memory_slot *slot;
- /* Derived from encryption bits of the faulting GPA for CVMs. */
- bool is_private;
Either we can wrap it with the CONFIG_HAVE_KVM_PRIVATE_MEM_TESTING or if it looks ugly I can remove the "const" in my code.
Hmm, I think we can keep the const. Similar to the bug in kvm_faultin_pfn()[*], the kvm_slot_can_be_private() is bogus. A fault should be considered private if it's marked as private, whether or not userspace has configured the slot to be private is irrelevant. I.e. the xarray is the single source of truth, memslots are just plumbing.
If we incorporate Sean's suggestion and use xarray as the single source of truth, then can we get rid of the HAVE_KVM_PRIVATE_MEM_TESTING config?
Specifically, the self test can call the KVM_MEMORY_ENCRYPT_REG_REGION ioctl which will set the bits for the private FD within KVM's xarray.
(Maybe this was part of the point that Sean was making; but his feedback seemed focused on the discussion about keeping `is_private` const, whereas I've been staring at this trying to figure out if we can run the UPM selftests on a non-TDX/SNP VM WITHOUT a special test-only config. And Sean's idea seems to eliminate the need for the awkward CONFIG.)