From: Binbin Wu <binbin.wu@linux.intel.com>
To: isaku.yamahata@intel.com
Cc: kvm@vger.kernel.org, linux-kernel@vger.kernel.org,
isaku.yamahata@gmail.com, Paolo Bonzini <pbonzini@redhat.com>,
erdemaktas@google.com, Sean Christopherson <seanjc@google.com>,
Sagi Shahar <sagis@google.com>,
David Matlack <dmatlack@google.com>,
Kai Huang <kai.huang@intel.com>,
Zhi Wang <zhi.wang.linux@gmail.com>,
chen.bo@intel.com, hang.yuan@intel.com, tina.zhang@intel.com
Subject: Re: [PATCH v6 15/16] KVM: x86/mmu: Make kvm fault handler aware of large page of private memslot
Date: Wed, 22 Nov 2023 17:05:43 +0800 [thread overview]
Message-ID: <376511f8-1d84-41fc-84ad-73d2f0ed3af1@linux.intel.com> (raw)
In-Reply-To: <075de567893a2b09bdfb203ae7ecd1867e5c3d8e.1699368363.git.isaku.yamahata@intel.com>
On 11/7/2023 11:00 PM, isaku.yamahata@intel.com wrote:
> From: Isaku Yamahata <isaku.yamahata@intel.com>
>
> struct kvm_page_fault.req_level is the page level which takes care of the
> faulted-in page size. For now its calculation is only for the conventional
> kvm memslot by host_pfn_mapping_level() that traverses page table.
>
> However, host_pfn_mapping_level() cannot be used for private kvm memslot
> because pages of private kvm memlost aren't mapped into user virtual
> address space.
The description here is not accurate. A memslot can be private doesn't mean
all pages of the memslot can't be mapped into user virtual address space.
> Instead page order is given when getting pfn. Remember it
> in struct kvm_page_fault and use it.
>
> Signed-off-by: Isaku Yamahata <isaku.yamahata@intel.com>
> ---
> arch/x86/kvm/mmu/mmu.c | 34 +++++++++++++++++----------------
> arch/x86/kvm/mmu/mmu_internal.h | 12 +++++++++++-
> arch/x86/kvm/mmu/tdp_mmu.c | 2 +-
> 3 files changed, 30 insertions(+), 18 deletions(-)
>
> diff --git a/arch/x86/kvm/mmu/mmu.c b/arch/x86/kvm/mmu/mmu.c
> index 0bf043812644..0aec7c11f4e2 100644
> --- a/arch/x86/kvm/mmu/mmu.c
> +++ b/arch/x86/kvm/mmu/mmu.c
> @@ -3158,10 +3158,10 @@ static int host_pfn_mapping_level(struct kvm *kvm, gfn_t gfn,
>
> static int __kvm_mmu_max_mapping_level(struct kvm *kvm,
> const struct kvm_memory_slot *slot,
> - gfn_t gfn, int max_level, bool is_private)
> + gfn_t gfn, int max_level, int host_level,
> + bool is_private)
> {
> struct kvm_lpage_info *linfo;
> - int host_level;
>
> max_level = min(max_level, max_huge_page_level);
> for ( ; max_level > PG_LEVEL_4K; max_level--) {
> @@ -3170,24 +3170,23 @@ static int __kvm_mmu_max_mapping_level(struct kvm *kvm,
> break;
> }
>
> - if (is_private)
> - return max_level;
> -
> if (max_level == PG_LEVEL_4K)
> return PG_LEVEL_4K;
>
> - host_level = host_pfn_mapping_level(kvm, gfn, slot);
> + if (!is_private) {
> + WARN_ON_ONCE(host_level != PG_LEVEL_NONE);
> + host_level = host_pfn_mapping_level(kvm, gfn, slot);
> + }
> + WARN_ON_ONCE(host_level == PG_LEVEL_NONE);
> return min(host_level, max_level);
> }
>
> int kvm_mmu_max_mapping_level(struct kvm *kvm,
> const struct kvm_memory_slot *slot, gfn_t gfn,
> - int max_level)
> + int max_level, bool faultin_private)
When the parameter "faultin_private" is added, the only valid value is
"false". If the caller passes in "faultin_private = true", then it
would be a
problem based on this patch.
It seems meaningless and confusing to introduce the parameter
"faultin_private"
here.
> {
> - bool is_private = kvm_slot_can_be_private(slot) &&
> - kvm_mem_is_private(kvm, gfn);
> -
> - return __kvm_mmu_max_mapping_level(kvm, slot, gfn, max_level, is_private);
> + return __kvm_mmu_max_mapping_level(kvm, slot, gfn, max_level,
> + PG_LEVEL_NONE, faultin_private);
> }
>
> void kvm_mmu_hugepage_adjust(struct kvm_vcpu *vcpu, struct kvm_page_fault *fault)
> @@ -3212,7 +3211,8 @@ void kvm_mmu_hugepage_adjust(struct kvm_vcpu *vcpu, struct kvm_page_fault *fault
> */
> fault->req_level = __kvm_mmu_max_mapping_level(vcpu->kvm, slot,
> fault->gfn, fault->max_level,
> - fault->is_private);
> + fault->host_level,
> + kvm_is_faultin_private(fault));
> if (fault->req_level == PG_LEVEL_4K || fault->huge_page_disallowed)
> return;
>
> @@ -4336,6 +4336,7 @@ static int kvm_faultin_pfn_private(struct kvm_vcpu *vcpu,
> struct kvm_page_fault *fault)
> {
> int max_order, r;
> + u8 max_level;
>
> if (!kvm_slot_can_be_private(fault->slot)) {
> kvm_mmu_prepare_memory_fault_exit(vcpu, fault);
> @@ -4349,8 +4350,9 @@ static int kvm_faultin_pfn_private(struct kvm_vcpu *vcpu,
> return r;
> }
>
> - fault->max_level = min(kvm_max_level_for_order(max_order),
> - fault->max_level);
> + max_level = kvm_max_level_for_order(max_order);
> + fault->host_level = max_level;
> + fault->max_level = min(max_level, fault->max_level);
> fault->map_writable = !(fault->slot->flags & KVM_MEM_READONLY);
>
> return RET_PF_CONTINUE;
> @@ -4400,7 +4402,7 @@ static int __kvm_faultin_pfn(struct kvm_vcpu *vcpu, struct kvm_page_fault *fault
> return -EFAULT;
> }
>
> - if (fault->is_private)
> + if (kvm_is_faultin_private(fault))
> return kvm_faultin_pfn_private(vcpu, fault);
>
> async = false;
> @@ -6809,7 +6811,7 @@ static bool kvm_mmu_zap_collapsible_spte(struct kvm *kvm,
> */
> if (sp->role.direct &&
> sp->role.level < kvm_mmu_max_mapping_level(kvm, slot, sp->gfn,
> - PG_LEVEL_NUM)) {
> + PG_LEVEL_NUM, false)) {
> kvm_zap_one_rmap_spte(kvm, rmap_head, sptep);
>
> if (kvm_available_flush_remote_tlbs_range())
> diff --git a/arch/x86/kvm/mmu/mmu_internal.h b/arch/x86/kvm/mmu/mmu_internal.h
> index 653e96769956..6b540a10fd67 100644
> --- a/arch/x86/kvm/mmu/mmu_internal.h
> +++ b/arch/x86/kvm/mmu/mmu_internal.h
> @@ -357,6 +357,9 @@ struct kvm_page_fault {
> * is changing its own translation in the guest page tables.
> */
> bool write_fault_to_shadow_pgtable;
> +
> + /* valid only for private memslot && private gfn */
> + enum pg_level host_level;
> };
>
> int kvm_tdp_page_fault(struct kvm_vcpu *vcpu, struct kvm_page_fault *fault);
> @@ -451,7 +454,7 @@ static inline int kvm_mmu_do_page_fault(struct kvm_vcpu *vcpu, gpa_t cr2_or_gpa,
>
> int kvm_mmu_max_mapping_level(struct kvm *kvm,
> const struct kvm_memory_slot *slot, gfn_t gfn,
> - int max_level);
> + int max_level, bool faultin_private);
> void kvm_mmu_hugepage_adjust(struct kvm_vcpu *vcpu, struct kvm_page_fault *fault);
> void disallowed_hugepage_adjust(struct kvm_page_fault *fault, u64 spte, int cur_level);
>
> @@ -469,4 +472,11 @@ static inline bool kvm_hugepage_test_mixed(struct kvm_memory_slot *slot, gfn_t g
> }
> #endif
>
> +static inline bool kvm_is_faultin_private(const struct kvm_page_fault *fault)
> +{
> + if (IS_ENABLED(CONFIG_KVM_GENERIC_PRIVATE_MEM))
> + return fault->is_private && kvm_slot_can_be_private(fault->slot);
> + return false;
> +}
> +
> #endif /* __KVM_X86_MMU_INTERNAL_H */
> diff --git a/arch/x86/kvm/mmu/tdp_mmu.c b/arch/x86/kvm/mmu/tdp_mmu.c
> index c8a4bd052c71..173e4e9053fc 100644
> --- a/arch/x86/kvm/mmu/tdp_mmu.c
> +++ b/arch/x86/kvm/mmu/tdp_mmu.c
> @@ -2179,7 +2179,7 @@ static void zap_collapsible_spte_range(struct kvm *kvm,
> continue;
>
> max_mapping_level = kvm_mmu_max_mapping_level(kvm, slot,
> - iter.gfn, PG_LEVEL_NUM);
> + iter.gfn, PG_LEVEL_NUM, false);
> if (max_mapping_level < iter.level)
> continue;
>
next prev parent reply other threads:[~2023-11-22 9:05 UTC|newest]
Thread overview: 39+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-11-07 15:00 [PATCH v6 00/16] KVM TDX: TDP MMU: large page support isaku.yamahata
2023-11-07 15:00 ` [PATCH v6 01/16] KVM: TDP_MMU: Go to next level if smaller private mapping exists isaku.yamahata
2023-11-16 1:32 ` Binbin Wu
2023-11-17 1:05 ` Isaku Yamahata
2023-11-07 15:00 ` [PATCH v6 02/16] KVM: TDX: Pass page level to cache flush before TDX SEAMCALL isaku.yamahata
2023-11-16 5:36 ` Binbin Wu
2023-11-07 15:00 ` [PATCH v6 03/16] KVM: TDX: Pass KVM page level to tdh_mem_page_add() and tdh_mem_page_aug() isaku.yamahata
2023-11-16 8:18 ` Binbin Wu
2023-11-17 0:23 ` Isaku Yamahata
2023-11-07 15:00 ` [PATCH v6 04/16] KVM: TDX: Pass size to tdx_measure_page() isaku.yamahata
2023-11-16 8:57 ` Binbin Wu
2023-11-17 0:36 ` Isaku Yamahata
2023-11-07 15:00 ` [PATCH v6 05/16] KVM: TDX: Pass size to reclaim_page() isaku.yamahata
2023-11-19 6:42 ` Binbin Wu
2023-11-19 6:58 ` Binbin Wu
2023-11-07 15:00 ` [PATCH v6 06/16] KVM: TDX: Update tdx_sept_{set,drop}_private_spte() to support large page isaku.yamahata
2023-11-07 15:00 ` [PATCH v6 07/16] KVM: MMU: Introduce level info in PFERR code isaku.yamahata
2023-11-20 10:54 ` Binbin Wu
2023-11-21 10:02 ` Isaku Yamahata
2023-11-07 15:00 ` [PATCH v6 08/16] KVM: TDX: Pin pages via get_page() right before ADD/AUG'ed to TDs isaku.yamahata
2023-11-20 11:05 ` Binbin Wu
2023-11-21 10:04 ` Isaku Yamahata
2023-11-07 15:00 ` [PATCH v6 09/16] KVM: TDX: Pass desired page level in err code for page fault handler isaku.yamahata
2023-11-20 11:24 ` Binbin Wu
2023-11-21 10:27 ` Isaku Yamahata
2023-11-07 15:00 ` [PATCH v6 10/16] KVM: x86/tdp_mmu: Allocate private page table for large page split isaku.yamahata
2023-11-07 15:00 ` [PATCH v6 11/16] KVM: x86/tdp_mmu: Split the large page when zap leaf isaku.yamahata
2023-11-21 9:57 ` Binbin Wu
2023-11-21 11:00 ` Isaku Yamahata
2023-11-22 2:18 ` Binbin Wu
2023-11-07 15:00 ` [PATCH v6 12/16] KVM: x86/tdp_mmu, TDX: Split a large page when 4KB page within it converted to shared isaku.yamahata
2023-11-22 5:45 ` Binbin Wu
2023-11-07 15:00 ` [PATCH v6 13/16] KVM: x86/tdp_mmu: Try to merge pages into a large page isaku.yamahata
2023-11-22 7:24 ` Binbin Wu
2023-11-07 15:00 ` [PATCH v6 14/16] KVM: x86/tdp_mmu: TDX: Implement " isaku.yamahata
2023-11-22 7:50 ` Binbin Wu
2023-11-07 15:00 ` [PATCH v6 15/16] KVM: x86/mmu: Make kvm fault handler aware of large page of private memslot isaku.yamahata
2023-11-22 9:05 ` Binbin Wu [this message]
2023-11-07 15:00 ` [PATCH v6 16/16] KVM: TDX: Allow 2MB large page for TD GUEST isaku.yamahata
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=376511f8-1d84-41fc-84ad-73d2f0ed3af1@linux.intel.com \
--to=binbin.wu@linux.intel.com \
--cc=chen.bo@intel.com \
--cc=dmatlack@google.com \
--cc=erdemaktas@google.com \
--cc=hang.yuan@intel.com \
--cc=isaku.yamahata@gmail.com \
--cc=isaku.yamahata@intel.com \
--cc=kai.huang@intel.com \
--cc=kvm@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=pbonzini@redhat.com \
--cc=sagis@google.com \
--cc=seanjc@google.com \
--cc=tina.zhang@intel.com \
--cc=zhi.wang.linux@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).