KVM: Extend Eager Page Splitting to the shadow MMU
Subsystems (10)
v5 cover KVM: Extend Eager Page Splitting to the shadow MMU
This series extends KVM's Eager Page Splitting to also split huge pages mapped by the shadow MMU, specifically **nested MMUs**.
For background on Eager Page Splitting, see:
- Proposal: https://lore.kernel.org/kvm/CALzav=dV_U4r1K9oDq4esb4mpBQDQ2ROQ5zH5wV3KpOaZrRW-A at mail.gmail.com/
- TDP MMU support: https://lore.kernel.org/kvm/20220119230739.2234394-1-dmatlack at google.com/
Splitting huge pages mapped by the shadow MMU is more complicated than the TDP MMU, but it is also more important for performance as the shadow MMU handles huge page write-protection faults under the write lock. See the Performance section for more details.
The extra complexity of splitting huge pages mapped by the shadow MMU comes from a few places:
(1) The shadow MMU has a limit on the number of shadow pages that are allowed to be allocated. So, as a policy, Eager Page Splitting refuses to split if there are KVM_MIN_FREE_MMU_PAGES or fewer pages available.
(2) Huge pages may be mapped by indirect shadow pages which may have access permission constraints from the guest (unlike the TDP MMU which is ACC_ALL by default).
(3) Splitting a huge page may end up re-using an existing lower level shadow page tables. This is unlike the TDP MMU which always allocates new shadow page tables when splitting.
(4) When installing the lower level SPTEs, they must be added to the rmap which may require allocating additional pte_list_desc structs.
In Google's internal implementation of Eager Page Splitting, we do not handle cases (3) and (4), and intstead opts to skip splitting entirely (case 3) or only partially splitting (case 4). This series handles the additional cases, which requires an additional 4KiB of memory per VM to store the extra pte_list_desc cache. However it does also avoids the need for TLB flushes in most cases and allows KVM to split more pages mapped by shadow paging.
The bulk of this series is just refactoring the existing MMU code in preparation for splitting, specifically to make it possible to operate on the MMU outside of a vCPU context.
Motivation
----------
During dirty logging, VMs using the shadow MMU suffer from:
(1) Write-protection faults on huge pages that take the MMU lock to unmap the huge page, map a 4KiB page, and update the dirty log.
(2) Non-present faults caused by (1) that take the MMU lock to map in the missing page.
(3) Write-protection faults on 4KiB pages that take the MMU lock to make the page writable and update the dirty log. [Note: These faults only take the MMU lock during shadow paging.]
The lock contention from (1), (2) and (3) can severely degrade application performance to the point of failure. Eager page splitting eliminates (1) by moving the splitting of huge pages off the vCPU threads onto the thread invoking VM-ioctls to configure dirty logging, and eliminates (2) by fully splitting each huge page into its constituent small pages. (3) is still a concern for shadow paging workloads (e.g. nested virtualization) but is not addressed by this series.
Splitting in the VM-ioctl thread is useful because it can run in the background without interrupting vCPU execution. However, it does take the MMU lock so it may introduce some extra contention if vCPUs are hammering the MMU lock. This is offset by the fact that eager page splitting drops the MMU lock after splitting each SPTE if there is any contention, and the fact that eager page splitting is reducing the MMU lock contention from (1) and (2) above. Even workloads that only write to 5% of their memory see massive MMU lock contention reduction during dirty logging thanks to Eager Page Splitting (see Performance data below).
A downside of Eager Page Splitting is that it splits all huge pages, which may include ranges of memory that are never written to by the guest and thus could theoretically stay huge. Workloads that write to only a fraction of their memory may see higher TLB miss costs with Eager Page Splitting enabled. However, that is secondary to the application failure that otherwise may occur without Eager Page Splitting.
Further work is necessary to improve the TLB miss performance for read-heavy workoads, such as dirty logging at 2M instead of 4K.
Performance
-----------
To measure the performance impact of Eager Page Splitting I ran dirty_log_perf_test with support for a new flag, -n, that causes each vCPU thread to run in L2 instead of L1. This support will be sent out in a separate series.
To measure the imapct of customer performance, we can look at the time it takes all vCPUs to dirty memory after dirty logging has been enabled. Without Eager Page Splitting enabled, such dirtying must take faults to split huge pages and bottleneck on the MMU lock.
For write-heavy workloads, there is not as much benefit since nested MMUs still have to take the write-lock when resolving 4K write-protection faults (case (3) in the Motivation section). But ready-heavy workloads greatly benefit.
| Config: tdp_mmu=Y, nested, 100% writes |
| Iteration 1 dirty memory time |
| ------------------------------------------------------- |
vCPU Count | eager_page_split=N | eager_page_split=Y |
------------ | -------------------------- | -------------------------- |
2 | 0.367445635s | 0.359880160s |
4 | 0.503976497s | 0.418760595s |
8 | 1.328792652s | 1.442455382s |
16 | 4.609457301s | 3.649754574s |
32 | 8.751328485s | 7.659014140s |
64 | 20.438482174s | 17.890019577s | | Config: tdp_mmu=Y, nested, 50% writes |
| Iteration 1 dirty memory time |
| ------------------------------------------------------- |
vCPU Count | eager_page_split=N | eager_page_split=Y |
------------ | -------------------------- | -------------------------- |
2 | 0.374082549s | 0.189881327s |
4 | 0.498175012s | 0.216221200s |
8 | 1.848155856s | 0.525316794s |
16 | 4.387725630s | 1.844867390s |
32 | 9.153260046s | 4.061645844s |
64 | 20.077600588s | 8.825413269s | | Config: tdp_mmu=Y, nested, 5% writes |
| Iteration 1 dirty memory time |
| ------------------------------------------------------- |
vCPU Count | eager_page_split=N | eager_page_split=Y |
------------ | -------------------------- | -------------------------- |
2 | 0.386395635s | 0.023315599s |
4 | 0.495352933s | 0.024971794s |
8 | 1.568730321s | 0.052010563s |
16 | 4.258323166s | 0.174402708s |
32 | 9.260176347s | 0.377929203s |
64 | 19.861473882s | 0.905998574s |Eager Page Splitting does increase the time it takes to enable dirty logging when not using initially-all-set, since that's when KVM splits huge pages. However, this runs in parallel with vCPU execution and drops the MMU lock whenever there is contention.
| Config: tdp_mmu=Y, nested, 100% writes |
| Enabling dirty logging time |
| ------------------------------------------------------- |
vCPU Count | eager_page_split=N | eager_page_split=Y |
------------ | -------------------------- | -------------------------- |
2 | 0.001330088s | 0.018624938s |
4 | 0.002763111s | 0.037247815s |
8 | 0.005220762s | 0.074637543s |
16 | 0.010381925s | 0.149096917s |
32 | 0.022109466s | 0.307983859s |
64 | 0.085547182s | 0.854228170s |Similarly, Eager Page Splitting increases the time it takes to clear the dirty log for when using initially-all-set. The first time userspace clears the dirty log, KVM will split huge pages:
| Config: tdp_mmu=Y, nested, 100% writes initially-all-set |
| Iteration 1 clear dirty log time |
| ------------------------------------------------------- |
vCPU Count | eager_page_split=N | eager_page_split=Y |
------------ | -------------------------- | -------------------------- |
2 | 0.001947098s | 0.019836052s |
4 | 0.003817996s | 0.039574178s |
8 | 0.007673616s | 0.079118964s |
16 | 0.015733003s | 0.158006697s |
32 | 0.031728367s | 0.330793049s |
64 | 0.108699714s | 0.891762988s |Subsequent calls to clear the dirty log incur almost no additional cost since KVM can very quickly determine there are no more huge pages to split via the RMAP. This is unlike the TDP MMU which must re-traverse the entire page table to check for huge pages.
| Config: tdp_mmu=Y, nested, 100% writes initially-all-set |
| Iteration 2 clear dirty log time |
| ------------------------------------------------------- |
vCPU Count | eager_page_split=N | eager_page_split=Y |
------------ | -------------------------- | -------------------------- |
2 | 0.009585296s | 0.009931437s |
4 | 0.019188984s | 0.019842738s |
8 | 0.038568630s | 0.039951832s |
16 | 0.077188525s | 0.079536780s |
32 | 0.156728329s | 0.163612725s |
64 | 0.418679324s | 0.337336844s |Testing
-------
- Ran all kvm-unit-tests and KVM selftests.
- Booted a 32-bit non-PAE kernel with shadow paging to verify the quadrant change.
- Ran dirty_log_perf_test with support for a new flag, -n, that causes each vCPU thread to run in L2 instead of L1. This support will be sent out in a separate series.
- Tested VM live migration with nested MMUs and huge pages. The live migration setup consisted of an 8 vCPU 8 GiB VM running on an Intel Cascade Lake host and backed by 1GiB HugeTLBFS memory. The VM was running Debian 10. Inside a VM was a 6 vCPU 4Gib nested VM also Debian 10 and backed by 2M HugeTLBFS. Inside the nested VM ran a workload that aggressively accessed memory across 6 threads. Tracepoints during the migration confirmes eager page splitting occurred, both for the direct TDP MMU mappings, and the nested MMU mappings.
Version Log
-----------
v5:
- Rebase on top of latest kvm/queue.
- Collected R-b tags from Sean and Lai.
- Add another patch to stop passing non-zero quadrant [Sean]
- Drop vcpu_or_null and __kvm_sync_page() [Sean]
- Formatting and wording changes [Sean]
- Pass role instead of sp when making huge split SPTEs [Sean]
- Fix riscv compilation error [kernel test robot]
- Document split caches protected by slots_lock [Lai]
v4:
- Limit eager page splitting to nested MMUs [Sean]
- Use memory caches for SP allocation [Sean]
- Use kvm_mmu_get_page() with NULL vCPU for EPS [Sean]
- Use u64 instead of bit field for shadow translation entry [Sean]
- Add Sean's R-b to "Use a bool" patch.
- Fix printf warning in "Cache access bits" patch.
- Fix asymmentrical pr_err_ratelimit() + WARN() [Sean]
- Drop unnecessary unsync check for huge pages [Sean]
- Eliminate use of we in comments and change logs [Sean]
- Allocate objects arrays dynamically [Ben]
v3: https://lore.kernel.org/kvm/20220401175554.1931568-1-dmatlack at google.com/
- Add R-b tags from Peter.
- Explain direct SPs in indirect MMUs in commit message [Peter]
- Change BUG_ON() to WARN_ON_ONCE() in quadrant calculation [me]
- Eliminate unnecessary gotos [Peter]
- Drop mmu_alloc_pte_list_desc() [Peter]
- Also update access cache in mmu_set_spte() if was_rmapped [Peter]
- Fix number of gfn bits in shadowed_translation cache [Peter]
- Pass sp to make_huge_page_split_spte() to derive level and exec [me]
- Eliminate flush var in kvm_rmap_zap_collapsible_sptes() [Peter]
- Drop NULL pte_list_desc cache fallback [Peter]
- Fix get_access to return sp->role.access. [me]
- Re-use split cache across calls to CLEAR_DIRTY_LOG for better perf [me]
- Top-up the split cache outside of the MMU lock when possible [me]
- Refactor prepare_to_split_huge_page() into try_split_huge_page() [me]
- Collapse PATCH 20, 23, and 24 avoid intermediate complexity [Peter]
- Update the RISC-V function stage2_ioremap() [Anup]
v2: https://lore.kernel.org/kvm/20220311002528.2230172-1-dmatlack at google.com/
- Add performance data for workloads that mix reads and writes [Peter]
- Collect R-b tags from Ben and Sean.
- Fix quadrant calculation when deriving role from parent [Sean]
- Tweak new shadow page function names [Sean]
- Move set_page_private() to allocation functions [Ben]
- Only zap collapsible SPTEs up to MAX_LEVEL-1 [Ben]
- Always top-up pte_list_desc cache to reduce complexity [Ben]
- Require mmu cache capacity field to be initialized and add WARN() to reduce chance of programmer error [Marc]
- Fix up kvm_mmu_memory_cache struct initialization in arm64 [Marc]
v1: https://lore.kernel.org/kvm/20220203010051.2813563-1-dmatlack at google.com/
David Matlack (21):
KVM: x86/mmu: Optimize MMU page cache lookup for all direct SPs
KVM: x86/mmu: Use a bool for direct
KVM: x86/mmu: Derive shadow MMU page role from parent
KVM: x86/mmu: Always pass 0 for @quadrant when gptes are 8 bytes
KVM: x86/mmu: Decompose kvm_mmu_get_page() into separate functions
KVM: x86/mmu: Consolidate shadow page allocation and initialization
KVM: x86/mmu: Rename shadow MMU functions that deal with shadow pages
KVM: x86/mmu: Move guest PT write-protection to account_shadowed()
KVM: x86/mmu: Pass memory caches to allocate SPs separately
KVM: x86/mmu: Replace vcpu with kvm in kvm_mmu_alloc_shadow_page()
KVM: x86/mmu: Pass kvm pointer separately from vcpu to
kvm_mmu_find_shadow_page()
KVM: x86/mmu: Allow NULL @vcpu in kvm_mmu_find_shadow_page()
KVM: x86/mmu: Pass const memslot to rmap_add()
KVM: x86/mmu: Decouple rmap_add() and link_shadow_page() from kvm_vcpu
KVM: x86/mmu: Update page stats in __rmap_add()
KVM: x86/mmu: Cache the access bits of shadowed translations
KVM: x86/mmu: Extend make_huge_page_split_spte() for the shadow MMU
KVM: x86/mmu: Zap collapsible SPTEs in shadow MMU at all possible
levels
KVM: x86/mmu: Refactor drop_large_spte()
KVM: Allow for different capacities in kvm_mmu_memory_cache structs
KVM: x86/mmu: Extend Eager Page Splitting to nested MMUs
.../admin-guide/kernel-parameters.txt | 3 +- arch/arm64/kvm/arm.c | 1 + arch/arm64/kvm/mmu.c | 5 +- arch/mips/kvm/mips.c | 2 + arch/riscv/kvm/mmu.c | 8 +- arch/riscv/kvm/vcpu.c | 1 + arch/x86/include/asm/kvm_host.h | 26 +- arch/x86/kvm/mmu/mmu.c | 704 ++++++++++++++---- arch/x86/kvm/mmu/mmu_internal.h | 17 +- arch/x86/kvm/mmu/paging_tmpl.h | 17 +- arch/x86/kvm/mmu/spte.c | 16 +- arch/x86/kvm/mmu/spte.h | 2 +- arch/x86/kvm/mmu/tdp_mmu.c | 2 +- arch/x86/kvm/x86.c | 6 + include/linux/kvm_types.h | 9 +- virt/kvm/kvm_main.c | 20 +- 16 files changed, 671 insertions(+), 168 deletions(-)
base-commit: a3808d88461270c71d3fece5e51cc486ecdac7d0
Signature · 1 line
2.36.0.550.gb090851708-goog
PATCH v5 1/21 KVM: x86/mmu: Optimize MMU page cache lookup for all direct SPs
Commit fb58a9c345f6 ("KVM: x86/mmu: Optimize MMU page cache lookup for fully direct MMUs") skipped the unsync checks and write flood clearing for full direct MMUs. We can extend this further to skip the checks for all direct shadow pages. Direct shadow pages in indirect MMUs (i.e. shadow paging) are used when shadowing a guest huge page with smaller pages. Such direct shadow pages, like their counterparts in fully direct MMUs, are never marked unsynced or have a non-zero write-flooding count.
Checking sp->role.direct also generates better code than checking direct_map because, due to register pressure, direct_map has to get shoved onto the stack and then pulled back off.
No functional change intended.
--- arch/x86/kvm/mmu/mmu.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
arch/x86/kvm/mmu/mmu.c +2 -2
@@ -2026,7 +2026,6 @@ static struct kvm_mmu_page *kvm_mmu_get_page(struct kvm_vcpu *vcpu, int direct, unsigned int access) { - bool direct_mmu = vcpu->arch.mmu->root_role.direct;union kvm_mmu_page_role role;struct hlist_head *sp_list;unsigned quadrant;
@@ -2070,7 +2069,8 @@ static struct kvm_mmu_page *kvm_mmu_get_page(struct kvm_vcpu *vcpu,continue ; } -if (direct_mmu) +/* unsync and write-flooding only apply to indirect SPs. */ +if (sp->role.direct) goto trace_get_page;if (sp->unsync) {
Signature · 1 line
2.36.0.550.gb090851708-goog
PATCH v5 2/21 KVM: x86/mmu: Use a bool for direct
In reply to David MatlackThe parameter "direct" can either be true or false, and all of the callers pass in a bool variable or true/false literal, so just use the type bool.
No functional change intended.
--- arch/x86/kvm/mmu/mmu.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
arch/x86/kvm/mmu/mmu.c +2 -2
@@ -1690,7 +1690,7 @@ static void drop_parent_pte(struct kvm_mmu_page *sp,mmu_spte_clear_no_track (parent_pte); } -static struct kvm_mmu_page *kvm_mmu_alloc_page (struct kvm_vcpu *vcpu,int direct) +static struct kvm_mmu_page *kvm_mmu_alloc_page (struct kvm_vcpu *vcpu,bool direct) {struct kvm_mmu_page *sp;
@@ -2023,7 +2023,7 @@ static struct kvm_mmu_page *kvm_mmu_get_page(struct kvm_vcpu *vcpu,gfn_t gfn, gva_t gaddr, unsigned level, - int direct, + bool direct, unsigned int access) { union kvm_mmu_page_role role;
Signature · 1 line
2.36.0.550.gb090851708-goog
PATCH v5 3/21 KVM: x86/mmu: Derive shadow MMU page role from parent
In reply to David MatlackInstead of computing the shadow page role from scratch for every new page, derive most of the information from the parent shadow page. This eliminates the dependency on the vCPU root role to allocate shadow page tables, and reduces the number of parameters to kvm_mmu_get_page().
Preemptively split out the role calculation to a separate function for use in a following commit.
No functional change intended.
--- arch/x86/kvm/mmu/mmu.c | 103 +++++++++++++++++++++++---------- arch/x86/kvm/mmu/paging_tmpl.h | 9 +-- 2 files changed, 77 insertions(+), 35 deletions(-)
arch/x86/kvm/mmu/mmu.c +72 -31
@@ -2019,33 +2019,15 @@ static void clear_sp_write_flooding_count(u64 *spte)__clear_sp_write_flooding_count (sptep_to_sp (spte)); } -static struct kvm_mmu_page *kvm_mmu_get_page (struct kvm_vcpu *vcpu, -gfn_t gfn, -gva_t gaddr, -unsigned level, -bool direct, -unsigned int access) +static struct kvm_mmu_page *kvm_mmu_get_page (struct kvm_vcpu *vcpu,gfn_t gfn, +union kvm_mmu_page_role role) { -union kvm_mmu_page_role role;struct hlist_head *sp_list; -unsigned quadrant;struct kvm_mmu_page *sp;int ret;int collisions =0 ;LIST_HEAD (invalid_list); - role = vcpu->arch.mmu->root_role; - role.level = level; - role.direct = direct; - role.access = access; -if (role.has_4_byte_gpte) { - quadrant = gaddr >> (PAGE_SHIFT + (PT64_PT_BITS * level)); - quadrant &= (1 << ((PT32_PT_BITS -PT64_PT_BITS ) * level)) -1 ; - role.quadrant = quadrant; - } -if (level <= vcpu->arch.mmu->cpu_role.base.level) - role.passthrough =0 ; - sp_list = &vcpu->kvm->arch.mmu_page_hash[kvm_page_table_hashfn (gfn)];for_each_valid_sp (vcpu->kvm, sp, sp_list) {if (sp->gfn != gfn) {
@@ -2063,7 +2045,7 @@ static struct kvm_mmu_page *kvm_mmu_get_page(struct kvm_vcpu *vcpu,* Unsync pages must not be left as is, because the new * upper-level page will be write-protected. */ -if (level >PG_LEVEL_4K && sp->unsync) +if (role.level >PG_LEVEL_4K && sp->unsync)kvm_mmu_prepare_zap_page (vcpu->kvm, sp, &invalid_list);continue ;
@@ -2104,14 +2086,14 @@ static struct kvm_mmu_page *kvm_mmu_get_page(struct kvm_vcpu *vcpu, ++vcpu->kvm->stat.mmu_cache_miss; - sp =kvm_mmu_alloc_page (vcpu, direct); + sp =kvm_mmu_alloc_page (vcpu, role.direct); sp->gfn = gfn; sp->role = role;hlist_add_head (&sp->hash_link, sp_list);if (sp_has_gptes (sp)) {account_shadowed (vcpu->kvm, sp); -if (level ==PG_LEVEL_4K &&kvm_vcpu_write_protect_gfn (vcpu, gfn)) +if (role.level ==PG_LEVEL_4K &&kvm_vcpu_write_protect_gfn (vcpu, gfn))kvm_flush_remote_tlbs_with_address (vcpu->kvm, gfn,1 ); }trace_kvm_mmu_get_page (sp, true);
@@ -2123,6 +2105,55 @@ static struct kvm_mmu_page *kvm_mmu_get_page(struct kvm_vcpu *vcpu,return sp; } +static union kvm_mmu_page_role kvm_mmu_child_role (u64 *sptep,bool direct,u32 access) +{ +struct kvm_mmu_page *parent_sp =sptep_to_sp (sptep); +union kvm_mmu_page_role role; + + role = parent_sp->role; + role.level--; + role.access = access; + role.direct = direct; + role.passthrough =0 ; + +/* +* If the guest has 4-byte PTEs then that means it's using 32-bit, +* 2-level, non-PAE paging. KVM shadows such guests with PAE paging +* (i.e. 8-byte PTEs). The difference in PTE size means that KVM must +* shadow each guest page table with multiple shadow page tables, which +* requires extra bookkeeping in the role. +* +* Specifically, to shadow the guest's page directory (which covers a +* 4GiB address space), KVM uses 4 PAE page directories, each mapping +* 1GiB of the address space. @role.quadrant encodes which quarter of +* the address space each maps. +* +* To shadow the guest's page tables (which each map a 4MiB region), KVM +* uses 2 PAE page tables, each mapping a 2MiB region. For these, +* @role.quadrant encodes which half of the region they map. +* +* Note, the 4 PAE page directories are pre-allocated and the quadrant +* assigned in mmu_alloc_root(). So only page tables need to be handled +* here. +*/ +if (role.has_4_byte_gpte) { +WARN_ON_ONCE (role.level !=PG_LEVEL_4K ); + role.quadrant = (sptep - parent_sp->spt) %2 ; + } + +return role; +} + +static struct kvm_mmu_page *kvm_mmu_get_child_sp (struct kvm_vcpu *vcpu, +u64 *sptep,gfn_t gfn, +bool direct,u32 access) +{ +union kvm_mmu_page_role role; + + role =kvm_mmu_child_role (sptep, direct, access); +return kvm_mmu_get_page (vcpu, gfn, role); +} +static void shadow_walk_init_using_root (struct kvm_shadow_walk_iterator *iterator,struct kvm_vcpu *vcpu,hpa_t root,u64 addr)
@@ -2965,8 +2996,7 @@ static int __direct_map(struct kvm_vcpu *vcpu, struct kvm_page_fault *fault)if (is_shadow_present_pte (*it.sptep))continue ; - sp =kvm_mmu_get_page (vcpu, base_gfn, it.addr, - it.level -1 , true,ACC_ALL ); + sp =kvm_mmu_get_child_sp (vcpu, it.sptep, base_gfn, true,ACC_ALL );link_shadow_page (vcpu, it.sptep, sp); if (fault->is_tdp && fault->huge_page_disallowed &&
@@ -3369,12 +3399,24 @@ static int mmu_check_root(struct kvm_vcpu *vcpu, gfn_t root_gfn)return ret; } -static hpa_t mmu_alloc_root (struct kvm_vcpu *vcpu,gfn_t gfn,gva_t gva, +static hpa_t mmu_alloc_root (struct kvm_vcpu *vcpu,gfn_t gfn,int quadrant,u8 level,bool direct) { +union kvm_mmu_page_role role;struct kvm_mmu_page *sp; - sp =kvm_mmu_get_page (vcpu, gfn, gva, level, direct,ACC_ALL ); + role = vcpu->arch.mmu->root_role; + role.level = level; + role.direct = direct; + role.access =ACC_ALL ; + +if (role.has_4_byte_gpte) + role.quadrant = quadrant; + +if (level <= vcpu->arch.mmu->cpu_role.base.level) + role.passthrough =0 ; + + sp =kvm_mmu_get_page (vcpu, gfn, role); ++sp->root_count;return __pa (sp->spt);
@@ -3408,8 +3450,8 @@ static int mmu_alloc_direct_roots(struct kvm_vcpu *vcpu)for (i =0 ; i <4 ; ++i) {WARN_ON_ONCE (IS_VALID_PAE_ROOT (mmu->pae_root[i])); - root =mmu_alloc_root (vcpu, i << (30 -PAGE_SHIFT ), - i <<30 ,PT32_ROOT_LEVEL , true); + root =mmu_alloc_root (vcpu, i << (30 -PAGE_SHIFT ), i, +PT32_ROOT_LEVEL , true); mmu->pae_root[i] = root |PT_PRESENT_MASK | shadow_me_mask; }
@@ -3578,8 +3620,7 @@ static int mmu_alloc_shadow_roots(struct kvm_vcpu *vcpu) root_gfn = pdptrs[i] >>PAGE_SHIFT ; } - root =mmu_alloc_root (vcpu, root_gfn, i <<30 , -PT32_ROOT_LEVEL , false); + root =mmu_alloc_root (vcpu, root_gfn, i,PT32_ROOT_LEVEL , false); mmu->pae_root[i] = root | pm_mask; }
arch/x86/kvm/mmu/paging_tmpl.h +5 -4
@@ -648,8 +648,9 @@ static int FNAME(fetch)(struct kvm_vcpu *vcpu, struct kvm_page_fault *fault,if (!is_shadow_present_pte (*it.sptep)) { table_gfn = gw->table_gfn[it.level -2 ]; access = gw->pt_access[it.level -2 ]; - sp =kvm_mmu_get_page (vcpu, table_gfn, fault->addr, - it.level-1 , false, access); + sp =kvm_mmu_get_child_sp (vcpu, it.sptep, table_gfn, + false, access); +/* * We must synchronize the pagetable before linking it * because the guest doesn't need to flush tlb when
@@ -705,8 +706,8 @@ static int FNAME(fetch)(struct kvm_vcpu *vcpu, struct kvm_page_fault *fault,drop_large_spte (vcpu, it.sptep);if (!is_shadow_present_pte (*it.sptep)) { - sp =kvm_mmu_get_page (vcpu, base_gfn, fault->addr, - it.level -1 , true, direct_access); + sp =kvm_mmu_get_child_sp (vcpu, it.sptep, base_gfn, + true, direct_access);link_shadow_page (vcpu, it.sptep, sp);if (fault->huge_page_disallowed && fault->req_level >= it.level)
Signature · 1 line
2.36.0.550.gb090851708-goog
Re: [PATCH v5 03/21] KVM: x86/mmu: Derive shadow MMU page role from parent
Quoted 9 lines · David Matlack
arch/x86/kvm/mmu/mmu.c +3
-2-static hpa_t mmu_alloc_root (struct kvm_vcpu *vcpu,gfn_t gfn,gva_t gva, +static hpa_t mmu_alloc_root (struct kvm_vcpu *vcpu,gfn_t gfn,int quadrant,u8 level,bool direct) { +union kvm_mmu_page_role role;struct kvm_mmu_page *sp; - sp =kvm_mmu_get_page (vcpu, gfn, gva, level, direct,ACC_ALL ); + role = vcpu->arch.mmu->root_role;
arch/x86/kvm/mmu/mmu.c static int mmu_check_root(struct kvm_vcpu *vcpu, gfn_t root_gfn) · line 3418
Quoted 10 lines
arch/x86/kvm/mmu/mmu.c +10
-0+ role.level = level; + role.direct = direct; + role.access =ACC_ALL ; + +if (role.has_4_byte_gpte) + role.quadrant = quadrant; + +if (level <= vcpu->arch.mmu->cpu_role.base.level) + role.passthrough =0 ; +
+ role.level = level;
+
+ if (role.has_4_byte_gpte)
+ role.quadrant = quadrant;
Only these lines are needed because of mmu->pae_root, others are the same as vcpu->arch.mmu->root_role.
The argument @direct is vcpu->arch.mmu->root_role.direct. vcpu->arch.mmu->root_role.access is always set to be ACC_ALL.
vcpu->arch.mmu->root_role.passthrough is 0 when mmu->pae_root is used. Or if vcpu->arch.mmu->root_role.passthrough is 1, @level must be 5 and vcpu->arch.mmu->cpu_role.base.level must be 4, the code here is useless.
Re: [PATCH v5 03/21] KVM: x86/mmu: Derive shadow MMU page role from parent
Quoted 33 lines · Lai Jiangshan
Quoted 19 lines · David Matlack
arch/x86/kvm/mmu/mmu.c +13
-2-static hpa_t mmu_alloc_root (struct kvm_vcpu *vcpu,gfn_t gfn,gva_t gva, +static hpa_t mmu_alloc_root (struct kvm_vcpu *vcpu,gfn_t gfn,int quadrant,u8 level,bool direct) { +union kvm_mmu_page_role role;struct kvm_mmu_page *sp; - sp =kvm_mmu_get_page (vcpu, gfn, gva, level, direct,ACC_ALL ); + role = vcpu->arch.mmu->root_role; + role.level = level; + role.direct = direct; + role.access =ACC_ALL ; + +if (role.has_4_byte_gpte) + role.quadrant = quadrant; + +if (level <= vcpu->arch.mmu->cpu_role.base.level) + role.passthrough =0 ; +arch/x86/kvm/mmu/mmu.c +4
-0+ role.level = level; + +if (role.has_4_byte_gpte) + role.quadrant = quadrant;Only these lines are needed because of
mmu->pae_root, others are the same asvcpu->arch.mmu->root_role.
Quoted 7 lines
The argument @direct is
vcpu->arch.mmu->root_role.direct.vcpu->arch.mmu->root_role.accessis always set to beACC_ALL.
vcpu->arch.mmu->root_role.passthroughis 0 whenmmu->pae_rootis used. Or ifvcpu->arch.mmu->root_role.passthroughis 1, @level must be 5 andvcpu->arch.mmu->cpu_role.base.levelmust be 4, the code here is useless.
Ah, thank you for the tip. That is cleaner.
PATCH v5 4/21 KVM: x86/mmu: Always pass 0 for @quadrant when gptes are 8 bytes
In reply to David MatlackThe quadrant is only used when gptes are 4 bytes, but mmu_alloc_{direct,shadow}_roots() pass in a non-zero quadrant for PAE page directories regardless. Make this less confusing by only passing in a non-zero quadrant when it is actually necessary.
--- arch/x86/kvm/mmu/mmu.c | 18 ++++++++++++++---- 1 file changed, 14 insertions(+), 4 deletions(-)
arch/x86/kvm/mmu/mmu.c +14 -4
@@ -3409,9 +3409,10 @@ static hpa_t mmu_alloc_root(struct kvm_vcpu *vcpu, gfn_t gfn, int quadrant, role.level = level; role.direct = direct; role.access =ACC_ALL ; + role.quadrant = quadrant; -if (role.has_4_byte_gpte) - role.quadrant = quadrant; +WARN_ON_ONCE (quadrant && !role.has_4_byte_gpte); +WARN_ON_ONCE (direct && role.has_4_byte_gpte);if (level <= vcpu->arch.mmu->cpu_role.base.level) role.passthrough =0 ;
@@ -3450,7 +3451,7 @@ static int mmu_alloc_direct_roots(struct kvm_vcpu *vcpu)for (i =0 ; i <4 ; ++i) {WARN_ON_ONCE (IS_VALID_PAE_ROOT (mmu->pae_root[i])); - root =mmu_alloc_root (vcpu, i << (30 -PAGE_SHIFT ), i, + root =mmu_alloc_root (vcpu, i << (30 -PAGE_SHIFT ),0 ,PT32_ROOT_LEVEL , true); mmu->pae_root[i] = root |PT_PRESENT_MASK | shadow_me_mask;
@@ -3535,6 +3536,7 @@ static int mmu_alloc_shadow_roots(struct kvm_vcpu *vcpu)struct kvm_mmu *mmu = vcpu->arch.mmu;u64 pdptrs[4 ], pm_mask;gfn_t root_gfn, root_pgd; +unsigned int quadrant;hpa_t root;unsigned i;int r;
@@ -3620,7 +3622,15 @@ static int mmu_alloc_shadow_roots(struct kvm_vcpu *vcpu) root_gfn = pdptrs[i] >>PAGE_SHIFT ; } - root =mmu_alloc_root (vcpu, root_gfn, i,PT32_ROOT_LEVEL , false); +/* +* If shadowing 32-bit non-PAE page tables, each PAE page +* directory maps one quarter of the guest's non-PAE page +* directory. Othwerise each PAE page direct shadows one guest +* PAE page directory so that quadrant should be 0. +*/ + quadrant = (mmu->cpu_role.base.level ==PT32_ROOT_LEVEL ) ? i :0 ; + + root =mmu_alloc_root (vcpu, root_gfn, quadrant,PT32_ROOT_LEVEL , false); mmu->pae_root[i] = root | pm_mask; }
Signature · 1 line
2.36.0.550.gb090851708-goog
PATCH v5 5/21 KVM: x86/mmu: Decompose kvm_mmu_get_page() into separate functions
In reply to David MatlackDecompose kvm_mmu_get_page() into separate helper functions to increase readability and prepare for allocating shadow pages without a vcpu pointer.
Specifically, pull the guts of kvm_mmu_get_page() into 2 helper functions:
kvm_mmu_find_shadow_page() -
Walks the page hash checking for any existing mmu pages that match the given gfn and role.
kvm_mmu_alloc_shadow_page()
Allocates and initializes an entirely new kvm_mmu_page. This currently requries a vcpu pointer for allocation and looking up the memslot but that will be removed in a future commit.
No functional change intended.
--- arch/x86/kvm/mmu/mmu.c | 52 +++++++++++++++++++++++++++++++----------- 1 file changed, 39 insertions(+), 13 deletions(-)
arch/x86/kvm/mmu/mmu.c +39 -13
@@ -2019,16 +2019,16 @@ static void clear_sp_write_flooding_count(u64 *spte)__clear_sp_write_flooding_count (sptep_to_sp (spte)); } -static struct kvm_mmu_page *kvm_mmu_get_page (struct kvm_vcpu *vcpu,gfn_t gfn, -union kvm_mmu_page_role role) +static struct kvm_mmu_page *kvm_mmu_find_shadow_page (struct kvm_vcpu *vcpu, +gfn_t gfn, +struct hlist_head *sp_list, +union kvm_mmu_page_role role) { -struct hlist_head *sp_list;struct kvm_mmu_page *sp;int ret;int collisions =0 ;LIST_HEAD (invalid_list); - sp_list = &vcpu->kvm->arch.mmu_page_hash[kvm_page_table_hashfn (gfn)];for_each_valid_sp (vcpu->kvm, sp, sp_list) {if (sp->gfn != gfn) { collisions++;
@@ -2053,7 +2053,7 @@ static struct kvm_mmu_page *kvm_mmu_get_page(struct kvm_vcpu *vcpu, gfn_t gfn,/* unsync and write-flooding only apply to indirect SPs. */ if (sp->role.direct) - goto trace_get_page; + goto out;if (sp->unsync) {/*
@@ -2079,14 +2079,26 @@ static struct kvm_mmu_page *kvm_mmu_get_page(struct kvm_vcpu *vcpu, gfn_t gfn,__clear_sp_write_flooding_count (sp); -trace_get_page: -trace_kvm_mmu_get_page (sp, false); goto out; } + sp =NULL ; ++vcpu->kvm->stat.mmu_cache_miss; - sp =kvm_mmu_alloc_page (vcpu, role.direct); +out: +kvm_mmu_commit_zap_page (vcpu->kvm, &invalid_list); + +if (collisions > vcpu->kvm->stat.max_mmu_page_hash_collisions) + vcpu->kvm->stat.max_mmu_page_hash_collisions = collisions; +return sp; +} + +static struct kvm_mmu_page *kvm_mmu_alloc_shadow_page (struct kvm_vcpu *vcpu, +gfn_t gfn, +struct hlist_head *sp_list, +union kvm_mmu_page_role role) +{ +struct kvm_mmu_page *sp =kvm_mmu_alloc_page (vcpu, role.direct); sp->gfn = gfn; sp->role = role;
@@ -2096,12 +2108,26 @@ static struct kvm_mmu_page *kvm_mmu_get_page(struct kvm_vcpu *vcpu, gfn_t gfn,if (role.level ==PG_LEVEL_4K &&kvm_vcpu_write_protect_gfn (vcpu, gfn))kvm_flush_remote_tlbs_with_address (vcpu->kvm, gfn,1 ); } -trace_kvm_mmu_get_page (sp, true); -out: -kvm_mmu_commit_zap_page (vcpu->kvm, &invalid_list); -if (collisions > vcpu->kvm->stat.max_mmu_page_hash_collisions) - vcpu->kvm->stat.max_mmu_page_hash_collisions = collisions; +return sp; +} + +static struct kvm_mmu_page *kvm_mmu_get_page (struct kvm_vcpu *vcpu,gfn_t gfn, +union kvm_mmu_page_role role) +{ +struct hlist_head *sp_list; +struct kvm_mmu_page *sp; +bool created = false; + + sp_list = &vcpu->kvm->arch.mmu_page_hash[kvm_page_table_hashfn (gfn)]; + + sp =kvm_mmu_find_shadow_page (vcpu, gfn, sp_list, role); +if (!sp) { + created = true; + sp =kvm_mmu_alloc_shadow_page (vcpu, gfn, sp_list, role); + } + +trace_kvm_mmu_get_page (sp, created);return sp; }
Signature · 1 line
2.36.0.550.gb090851708-goog
PATCH v5 6/21 KVM: x86/mmu: Consolidate shadow page allocation and initialization
In reply to David MatlackConsolidate kvm_mmu_alloc_page() and kvm_mmu_alloc_shadow_page() under the latter so that all shadow page allocation and initialization happens in one place.
No functional change intended.
--- arch/x86/kvm/mmu/mmu.c | 39 +++++++++++++++++---------------------- 1 file changed, 17 insertions(+), 22 deletions(-)
arch/x86/kvm/mmu/mmu.c +17 -22
@@ -1690,27 +1690,6 @@ static void drop_parent_pte(struct kvm_mmu_page *sp,mmu_spte_clear_no_track (parent_pte); } -static struct kvm_mmu_page *kvm_mmu_alloc_page (struct kvm_vcpu *vcpu,bool direct) -{ -struct kvm_mmu_page *sp; - - sp =kvm_mmu_memory_cache_alloc (&vcpu->arch.mmu_page_header_cache); - sp->spt =kvm_mmu_memory_cache_alloc (&vcpu->arch.mmu_shadow_page_cache); -if (!direct) - sp->gfns =kvm_mmu_memory_cache_alloc (&vcpu->arch.mmu_gfn_array_cache); -set_page_private (virt_to_page (sp->spt), (unsigned long )sp); - -/* -* active_mmu_pages must be a FIFO list, as kvm_zap_obsolete_pages() -* depends on valid pages being added to the head of the list. See -* comments in kvm_zap_obsolete_pages(). -*/ - sp->mmu_valid_gen = vcpu->kvm->arch.mmu_valid_gen; -list_add (&sp->link, &vcpu->kvm->arch.active_mmu_pages); -kvm_mod_used_mmu_pages (vcpu->kvm,+1 ); -return sp; -} -static void mark_unsync (u64 *spte);static void kvm_mmu_mark_parents_unsync (struct kvm_mmu_page *sp) {
@@ -2098,7 +2077,23 @@ static struct kvm_mmu_page *kvm_mmu_alloc_shadow_page(struct kvm_vcpu *vcpu, struct hlist_head *sp_list, union kvm_mmu_page_role role) { -struct kvm_mmu_page *sp =kvm_mmu_alloc_page (vcpu, role.direct); +struct kvm_mmu_page *sp; + + sp =kvm_mmu_memory_cache_alloc (&vcpu->arch.mmu_page_header_cache); + sp->spt =kvm_mmu_memory_cache_alloc (&vcpu->arch.mmu_shadow_page_cache); +if (!role.direct) + sp->gfns =kvm_mmu_memory_cache_alloc (&vcpu->arch.mmu_gfn_array_cache); + +set_page_private (virt_to_page (sp->spt), (unsigned long )sp); + +/* +* active_mmu_pages must be a FIFO list, as kvm_zap_obsolete_pages() +* depends on valid pages being added to the head of the list. See +* comments in kvm_zap_obsolete_pages(). +*/ + sp->mmu_valid_gen = vcpu->kvm->arch.mmu_valid_gen; +list_add (&sp->link, &vcpu->kvm->arch.active_mmu_pages); +kvm_mod_used_mmu_pages (vcpu->kvm,+1 ); sp->gfn = gfn; sp->role = role;
Signature · 1 line
2.36.0.550.gb090851708-goog
PATCH v5 7/21 KVM: x86/mmu: Rename shadow MMU functions that deal with shadow pages
In reply to David MatlackRename 2 functions:
kvm_mmu_get_page() -> kvm_mmu_get_shadow_page() kvm_mmu_free_page() -> kvm_mmu_free_shadow_page()
This change makes it clear that these functions deal with shadow pages rather than struct pages. It also aligns these functions with the naming scheme for kvm_mmu_find_shadow_page() and kvm_mmu_alloc_shadow_page().
Prefer "shadow_page" over the shorter "sp" since these are core functions and the line lengths aren't terrible.
No functional change intended.
--- arch/x86/kvm/mmu/mmu.c | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-)
arch/x86/kvm/mmu/mmu.c +7 -6
@@ -1652,7 +1652,7 @@ static inline void kvm_mod_used_mmu_pages(struct kvm *kvm, long nr)percpu_counter_add (&kvm_total_used_mmu_pages, nr); } -static void kvm_mmu_free_page (struct kvm_mmu_page *sp) +static void kvm_mmu_free_shadow_page (struct kvm_mmu_page *sp) {MMU_WARN_ON (!is_empty_shadow_page (sp->spt));hlist_del (&sp->hash_link);
@@ -2107,8 +2107,9 @@ static struct kvm_mmu_page *kvm_mmu_alloc_shadow_page(struct kvm_vcpu *vcpu,return sp; } -static struct kvm_mmu_page *kvm_mmu_get_page (struct kvm_vcpu *vcpu,gfn_t gfn, -union kvm_mmu_page_role role) +static struct kvm_mmu_page *kvm_mmu_get_shadow_page (struct kvm_vcpu *vcpu, +gfn_t gfn, +union kvm_mmu_page_role role) {struct hlist_head *sp_list;struct kvm_mmu_page *sp;
@@ -2172,7 +2173,7 @@ static struct kvm_mmu_page *kvm_mmu_get_child_sp(struct kvm_vcpu *vcpu,union kvm_mmu_page_role role; role =kvm_mmu_child_role (sptep, direct, access); -return kvm_mmu_get_page (vcpu, gfn, role); +return kvm_mmu_get_shadow_page (vcpu, gfn, role); } static void shadow_walk_init_using_root(struct kvm_shadow_walk_iterator *iterator,
@@ -2448,7 +2449,7 @@ static void kvm_mmu_commit_zap_page(struct kvm *kvm,list_for_each_entry_safe (sp, nsp, invalid_list, link) {WARN_ON (!sp->role.invalid || sp->root_count); -kvm_mmu_free_page (sp); +kvm_mmu_free_shadow_page (sp); } }
@@ -3438,7 +3439,7 @@ static hpa_t mmu_alloc_root(struct kvm_vcpu *vcpu, gfn_t gfn, int quadrant,if (level <= vcpu->arch.mmu->cpu_role.base.level) role.passthrough =0 ; - sp =kvm_mmu_get_page (vcpu, gfn, role); + sp =kvm_mmu_get_shadow_page (vcpu, gfn, role); ++sp->root_count;return __pa (sp->spt);
Signature · 1 line
2.36.0.550.gb090851708-goog
PATCH v5 8/21 KVM: x86/mmu: Move guest PT write-protection to account_shadowed()
In reply to David MatlackMove the code that write-protects newly-shadowed guest page tables into account_shadowed(). This avoids a extra gfn-to-memslot lookup and is a more logical place for this code to live. But most importantly, this reduces kvm_mmu_alloc_shadow_page()'s reliance on having a struct kvm_vcpu pointer, which will be necessary when creating new shadow pages during VM ioctls for eager page splitting.
Note, it is safe to drop the role.level == PG_LEVEL_4K check since account_shadowed() returns early if role.level > PG_LEVEL_4K.
No functional change intended.
--- arch/x86/kvm/mmu/mmu.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-)
arch/x86/kvm/mmu/mmu.c +4 -4
@@ -792,6 +792,9 @@ static void account_shadowed(struct kvm *kvm, struct kvm_mmu_page *sp) KVM_PAGE_TRACK_WRITE);kvm_mmu_gfn_disallow_lpage (slot, gfn); + +if (kvm_mmu_slot_gfn_write_protect (kvm, slot, gfn,PG_LEVEL_4K )) +kvm_flush_remote_tlbs_with_address (kvm, gfn,1 ); }void account_huge_nx_page (struct kvm *kvm,struct kvm_mmu_page *sp)
@@ -2098,11 +2101,8 @@ static struct kvm_mmu_page *kvm_mmu_alloc_shadow_page(struct kvm_vcpu *vcpu, sp->gfn = gfn; sp->role = role;hlist_add_head (&sp->hash_link, sp_list); -if (sp_has_gptes (sp)) { +if (sp_has_gptes (sp))account_shadowed (vcpu->kvm, sp); -if (role.level ==PG_LEVEL_4K &&kvm_vcpu_write_protect_gfn (vcpu, gfn)) -kvm_flush_remote_tlbs_with_address (vcpu->kvm, gfn,1 ); - }return sp; }
Signature · 1 line
2.36.0.550.gb090851708-goog
PATCH v5 9/21 KVM: x86/mmu: Pass memory caches to allocate SPs separately
In reply to David MatlackRefactor kvm_mmu_alloc_shadow_page() to receive the caches from which it will allocate the various pieces of memory for shadow pages as a parameter, rather than deriving them from the vcpu pointer. This will be useful in a future commit where shadow pages are allocated during VM ioctls for eager page splitting, and thus will use a different set of caches.
Preemptively pull the caches out all the way to kvm_mmu_get_shadow_page() since eager page splitting will not be calling kvm_mmu_alloc_shadow_page() directly.
No functional change intended.
--- arch/x86/kvm/mmu/mmu.c | 36 +++++++++++++++++++++++++++++------- 1 file changed, 29 insertions(+), 7 deletions(-)
arch/x86/kvm/mmu/mmu.c +29 -7
@@ -2075,17 +2075,25 @@ static struct kvm_mmu_page *kvm_mmu_find_shadow_page(struct kvm_vcpu *vcpu,return sp; } +/* Caches used when allocating a new shadow page. */ +struct shadow_page_caches { +struct kvm_mmu_memory_cache *page_header_cache; +struct kvm_mmu_memory_cache *shadow_page_cache; +struct kvm_mmu_memory_cache *gfn_array_cache; +}; +static struct kvm_mmu_page *kvm_mmu_alloc_shadow_page (struct kvm_vcpu *vcpu, +struct shadow_page_caches *caches,gfn_t gfn,struct hlist_head *sp_list,union kvm_mmu_page_role role) {struct kvm_mmu_page *sp; - sp =kvm_mmu_memory_cache_alloc (&vcpu->arch.mmu_page_header_cache); - sp->spt =kvm_mmu_memory_cache_alloc (&vcpu->arch.mmu_shadow_page_cache); + sp =kvm_mmu_memory_cache_alloc (caches->page_header_cache); + sp->spt =kvm_mmu_memory_cache_alloc (caches->shadow_page_cache);if (!role.direct) - sp->gfns =kvm_mmu_memory_cache_alloc (&vcpu->arch.mmu_gfn_array_cache); + sp->gfns =kvm_mmu_memory_cache_alloc (caches->gfn_array_cache);set_page_private (virt_to_page (sp->spt), (unsigned long )sp);
@@ -2107,9 +2115,10 @@ static struct kvm_mmu_page *kvm_mmu_alloc_shadow_page(struct kvm_vcpu *vcpu,return sp; } -static struct kvm_mmu_page *kvm_mmu_get_shadow_page (struct kvm_vcpu *vcpu, -gfn_t gfn, -union kvm_mmu_page_role role) +static struct kvm_mmu_page *__kvm_mmu_get_shadow_page (struct kvm_vcpu *vcpu, +struct shadow_page_caches *caches, +gfn_t gfn, +union kvm_mmu_page_role role) {struct hlist_head *sp_list;struct kvm_mmu_page *sp;
@@ -2120,13 +2129,26 @@ static struct kvm_mmu_page *kvm_mmu_get_shadow_page(struct kvm_vcpu *vcpu, sp =kvm_mmu_find_shadow_page (vcpu, gfn, sp_list, role);if (!sp) { created = true; - sp =kvm_mmu_alloc_shadow_page (vcpu, gfn, sp_list, role); + sp =kvm_mmu_alloc_shadow_page (vcpu, caches, gfn, sp_list, role); }trace_kvm_mmu_get_page (sp, created);return sp; } +static struct kvm_mmu_page *kvm_mmu_get_shadow_page (struct kvm_vcpu *vcpu, +gfn_t gfn, +union kvm_mmu_page_role role) +{ +struct shadow_page_caches caches = { + .page_header_cache = &vcpu->arch.mmu_page_header_cache, + .shadow_page_cache = &vcpu->arch.mmu_shadow_page_cache, + .gfn_array_cache = &vcpu->arch.mmu_gfn_array_cache, + }; + +return __kvm_mmu_get_shadow_page (vcpu, &caches, gfn, role); +} +static union kvm_mmu_page_role kvm_mmu_child_role (u64 *sptep,bool direct,u32 access) {struct kvm_mmu_page *parent_sp =sptep_to_sp (sptep);
Signature · 1 line
2.36.0.550.gb090851708-goog
PATCH v5 10/21 KVM: x86/mmu: Replace vcpu with kvm in kvm_mmu_alloc_shadow_page()
In reply to David MatlackThe vcpu pointer in kvm_mmu_alloc_shadow_page() is only used to get the kvm pointer. So drop the vcpu pointer and just pass in the kvm pointer.
No functional change intended.
--- arch/x86/kvm/mmu/mmu.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-)
arch/x86/kvm/mmu/mmu.c +6 -6
@@ -2082,7 +2082,7 @@ struct shadow_page_caches {struct kvm_mmu_memory_cache *gfn_array_cache; }; -static struct kvm_mmu_page *kvm_mmu_alloc_shadow_page(struct kvm_vcpu *vcpu, +static struct kvm_mmu_page *kvm_mmu_alloc_shadow_page(struct kvm *kvm, struct shadow_page_caches *caches, gfn_t gfn, struct hlist_head *sp_list,
@@ -2102,15 +2102,15 @@ static struct kvm_mmu_page *kvm_mmu_alloc_shadow_page(struct kvm_vcpu *vcpu,* depends on valid pages being added to the head of the list. See * comments in kvm_zap_obsolete_pages(). */ - sp->mmu_valid_gen = vcpu->kvm->arch.mmu_valid_gen; -list_add (&sp->link, &vcpu->kvm->arch.active_mmu_pages); -kvm_mod_used_mmu_pages (vcpu->kvm,+1 ); + sp->mmu_valid_gen = kvm->arch.mmu_valid_gen; +list_add (&sp->link, &kvm->arch.active_mmu_pages); +kvm_mod_used_mmu_pages (kvm,+1 ); sp->gfn = gfn; sp->role = role;hlist_add_head (&sp->hash_link, sp_list);if (sp_has_gptes (sp)) -account_shadowed (vcpu->kvm, sp); +account_shadowed (kvm, sp);return sp; }
@@ -2129,7 +2129,7 @@ static struct kvm_mmu_page *__kvm_mmu_get_shadow_page(struct kvm_vcpu *vcpu, sp =kvm_mmu_find_shadow_page (vcpu, gfn, sp_list, role);if (!sp) { created = true; - sp =kvm_mmu_alloc_shadow_page (vcpu, caches, gfn, sp_list, role); + sp =kvm_mmu_alloc_shadow_page (vcpu->kvm, caches, gfn, sp_list, role); }trace_kvm_mmu_get_page (sp, created);
Signature · 1 line
2.36.0.550.gb090851708-goog
PATCH v5 11/21 KVM: x86/mmu: Pass kvm pointer separately from vcpu to kvm_mmu_find_shadow_page()
In reply to David MatlackGet the kvm pointer from the caller, rather than deriving it from vcpu->kvm, and plumb the kvm pointer all the way from kvm_mmu_get_shadow_page(). With this change in place, the vcpu pointer is only needed to sync indirect shadow pages. In other words, __kvm_mmu_get_shadow_page() can now be used to get *direct* shadow pages without a vcpu pointer. This enables eager page splitting, which needs to allocate direct shadow pages during VM ioctls.
No functional change intended.
--- arch/x86/kvm/mmu/mmu.c | 28 +++++++++++++++------------- 1 file changed, 15 insertions(+), 13 deletions(-)
arch/x86/kvm/mmu/mmu.c +15 -13
@@ -2001,7 +2001,8 @@ static void clear_sp_write_flooding_count(u64 *spte)__clear_sp_write_flooding_count (sptep_to_sp (spte)); } -static struct kvm_mmu_page *kvm_mmu_find_shadow_page (struct kvm_vcpu *vcpu, +static struct kvm_mmu_page *kvm_mmu_find_shadow_page (struct kvm *kvm, +struct kvm_vcpu *vcpu,gfn_t gfn,struct hlist_head *sp_list,union kvm_mmu_page_role role)
@@ -2011,7 +2012,7 @@ static struct kvm_mmu_page *kvm_mmu_find_shadow_page(struct kvm_vcpu *vcpu,int collisions =0 ;LIST_HEAD (invalid_list); -for_each_valid_sp (vcpu->kvm, sp, sp_list) { +for_each_valid_sp (kvm, sp, sp_list) {if (sp->gfn != gfn) { collisions++;continue ;
@@ -2028,7 +2029,7 @@ static struct kvm_mmu_page *kvm_mmu_find_shadow_page(struct kvm_vcpu *vcpu,* upper-level page will be write-protected. */ if (role.level >PG_LEVEL_4K && sp->unsync) -kvm_mmu_prepare_zap_page (vcpu->kvm, sp, +kvm_mmu_prepare_zap_page (kvm, sp, &invalid_list);continue ; }
@@ -2056,7 +2057,7 @@ static struct kvm_mmu_page *kvm_mmu_find_shadow_page(struct kvm_vcpu *vcpu,WARN_ON (!list_empty (&invalid_list));if (ret >0 ) -kvm_flush_remote_tlbs (vcpu->kvm); +kvm_flush_remote_tlbs (kvm); }__clear_sp_write_flooding_count (sp);
@@ -2065,13 +2066,13 @@ static struct kvm_mmu_page *kvm_mmu_find_shadow_page(struct kvm_vcpu *vcpu, } sp =NULL ; - ++vcpu->kvm->stat.mmu_cache_miss; + ++kvm->stat.mmu_cache_miss; out: -kvm_mmu_commit_zap_page (vcpu->kvm, &invalid_list); +kvm_mmu_commit_zap_page (kvm, &invalid_list); -if (collisions > vcpu->kvm->stat.max_mmu_page_hash_collisions) - vcpu->kvm->stat.max_mmu_page_hash_collisions = collisions; +if (collisions > kvm->stat.max_mmu_page_hash_collisions) + kvm->stat.max_mmu_page_hash_collisions = collisions;return sp; }
@@ -2115,7 +2116,8 @@ static struct kvm_mmu_page *kvm_mmu_alloc_shadow_page(struct kvm *kvm,return sp; } -static struct kvm_mmu_page *__kvm_mmu_get_shadow_page (struct kvm_vcpu *vcpu, +static struct kvm_mmu_page *__kvm_mmu_get_shadow_page (struct kvm *kvm, +struct kvm_vcpu *vcpu,struct shadow_page_caches *caches,gfn_t gfn,union kvm_mmu_page_role role)
@@ -2124,12 +2126,12 @@ static struct kvm_mmu_page *__kvm_mmu_get_shadow_page(struct kvm_vcpu *vcpu,struct kvm_mmu_page *sp;bool created = false; - sp_list = &vcpu->kvm->arch.mmu_page_hash[kvm_page_table_hashfn (gfn)]; + sp_list = &kvm->arch.mmu_page_hash[kvm_page_table_hashfn (gfn)]; - sp =kvm_mmu_find_shadow_page (vcpu, gfn, sp_list, role); + sp =kvm_mmu_find_shadow_page (kvm, vcpu, gfn, sp_list, role);if (!sp) { created = true; - sp =kvm_mmu_alloc_shadow_page (vcpu->kvm, caches, gfn, sp_list, role); + sp =kvm_mmu_alloc_shadow_page (kvm, caches, gfn, sp_list, role); }trace_kvm_mmu_get_page (sp, created);
@@ -2146,7 +2148,7 @@ static struct kvm_mmu_page *kvm_mmu_get_shadow_page(struct kvm_vcpu *vcpu, .gfn_array_cache = &vcpu->arch.mmu_gfn_array_cache, }; -return __kvm_mmu_get_shadow_page (vcpu, &caches, gfn, role); +return __kvm_mmu_get_shadow_page (vcpu->kvm, vcpu, &caches, gfn, role); }static union kvm_mmu_page_role kvm_mmu_child_role (u64 *sptep,bool direct,u32 access)
Signature · 1 line
2.36.0.550.gb090851708-goog
PATCH v5 12/21 KVM: x86/mmu: Allow NULL @vcpu in kvm_mmu_find_shadow_page()
In reply to David MatlackAllow @vcpu to be NULL in kvm_mmu_find_shadow_page() (and its only caller __kvm_mmu_get_shadow_page()). @vcpu is only required to sync indirect shadow pages, so it's safe to pass in NULL when looking up direct shadow pages.
This will be used for doing eager page splitting, which allocates direct shadow pages from the context of a VM ioctl without access to a vCPU pointer.
--- arch/x86/kvm/mmu/mmu.c | 13 +++++++++++++ 1 file changed, 13 insertions(+)
arch/x86/kvm/mmu/mmu.c +13 -0
@@ -1850,6 +1850,7 @@ static int kvm_sync_page(struct kvm_vcpu *vcpu, struct kvm_mmu_page *sp,if (ret <0 )kvm_mmu_prepare_zap_page (vcpu->kvm, sp, invalid_list); +return ret; }
@@ -2001,6 +2002,7 @@ static void clear_sp_write_flooding_count(u64 *spte)__clear_sp_write_flooding_count (sptep_to_sp (spte)); } +/* Note, @vcpu may be NULL if @role.direct is true. */ static struct kvm_mmu_page *kvm_mmu_find_shadow_page(struct kvm *kvm, struct kvm_vcpu *vcpu, gfn_t gfn,
@@ -2039,6 +2041,16 @@ static struct kvm_mmu_page *kvm_mmu_find_shadow_page(struct kvm *kvm, goto out;if (sp->unsync) { +/* +* A vCPU pointer should always be provided when finding +* indirect shadow pages, as that shadow page may +* already exist and need to be synced using the vCPU +* pointer. Direct shadow pages are never unsync and +* thus do not require a vCPU pointer. +*/ +if (KVM_BUG_ON (!vcpu, kvm)) +break ; +/* * The page is good, but is stale. kvm_sync_page does * get the latest guest state, but (unlike mmu_unsync_children)
@@ -2116,6 +2128,7 @@ static struct kvm_mmu_page *kvm_mmu_alloc_shadow_page(struct kvm *kvm,return sp; } +/* Note, @vcpu may be NULL if @role.direct is true. */ static struct kvm_mmu_page *__kvm_mmu_get_shadow_page(struct kvm *kvm, struct kvm_vcpu *vcpu, struct shadow_page_caches *caches,
Signature · 1 line
2.36.0.550.gb090851708-goog
PATCH v5 13/21 KVM: x86/mmu: Pass const memslot to rmap_add()
In reply to David Matlackrmap_add() only uses the slot to call gfn_to_rmap() which takes a const memslot.
No functional change intended.
--- arch/x86/kvm/mmu/mmu.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
arch/x86/kvm/mmu/mmu.c +1 -1
@@ -1582,7 +1582,7 @@ static bool kvm_test_age_rmapp(struct kvm *kvm, struct kvm_rmap_head *rmap_head,#define RMAP_RECYCLE_THRESHOLD 1000 -static void rmap_add (struct kvm_vcpu *vcpu,struct kvm_memory_slot *slot, +static void rmap_add (struct kvm_vcpu *vcpu,const struct kvm_memory_slot *slot,u64 *spte,gfn_t gfn) {struct kvm_mmu_page *sp;
Signature · 1 line
2.36.0.550.gb090851708-goog
PATCH v5 14/21 KVM: x86/mmu: Decouple rmap_add() and link_shadow_page() from kvm_vcpu
In reply to David MatlackAllow adding new entries to the rmap and linking shadow pages without a struct kvm_vcpu pointer by moving the implementation of rmap_add() and link_shadow_page() into inner helper functions.
No functional change intended.
--- arch/x86/kvm/mmu/mmu.c | 45 +++++++++++++++++++++++++----------------- 1 file changed, 27 insertions(+), 18 deletions(-)
arch/x86/kvm/mmu/mmu.c +27 -18
@@ -699,11 +699,6 @@ static void mmu_free_memory_caches(struct kvm_vcpu *vcpu)kvm_mmu_free_memory_cache (&vcpu->arch.mmu_page_header_cache); } -static struct pte_list_desc *mmu_alloc_pte_list_desc (struct kvm_vcpu *vcpu) -{ -return kvm_mmu_memory_cache_alloc (&vcpu->arch.mmu_pte_list_desc_cache); -} -static void mmu_free_pte_list_desc (struct pte_list_desc *pte_list_desc) {kmem_cache_free (pte_list_desc_cache, pte_list_desc);
@@ -858,7 +853,7 @@ gfn_to_memslot_dirty_bitmap(struct kvm_vcpu *vcpu, gfn_t gfn,/* * Returns the number of pointers in the rmap chain, not counting the new one. */ -static int pte_list_add (struct kvm_vcpu *vcpu,u64 *spte, +static int pte_list_add (struct kvm_mmu_memory_cache *cache,u64 *spte,struct kvm_rmap_head *rmap_head) {struct pte_list_desc *desc;
@@ -869,7 +864,7 @@ static int pte_list_add(struct kvm_vcpu *vcpu, u64 *spte, rmap_head->val = (unsigned long )spte; }else if (!(rmap_head->val &1 )) {rmap_printk ("%p %llx 1->many\n" , spte, *spte); - desc =mmu_alloc_pte_list_desc (vcpu); + desc =kvm_mmu_memory_cache_alloc (cache); desc->sptes[0 ] = (u64 *)rmap_head->val; desc->sptes[1 ] = spte; desc->spte_count =2 ;
@@ -881,7 +876,7 @@ static int pte_list_add(struct kvm_vcpu *vcpu, u64 *spte,while (desc->spte_count ==PTE_LIST_EXT ) { count +=PTE_LIST_EXT ;if (!desc->more) { - desc->more =mmu_alloc_pte_list_desc (vcpu); + desc->more =kvm_mmu_memory_cache_alloc (cache); desc = desc->more; desc->spte_count =0 ;break ;
@@ -1582,8 +1577,10 @@ static bool kvm_test_age_rmapp(struct kvm *kvm, struct kvm_rmap_head *rmap_head,#define RMAP_RECYCLE_THRESHOLD 1000 -static void rmap_add (struct kvm_vcpu *vcpu,const struct kvm_memory_slot *slot, -u64 *spte,gfn_t gfn) +static void __rmap_add (struct kvm *kvm, +struct kvm_mmu_memory_cache *cache, +const struct kvm_memory_slot *slot, +u64 *spte,gfn_t gfn) {struct kvm_mmu_page *sp;struct kvm_rmap_head *rmap_head;
@@ -1592,15 +1589,21 @@ static void rmap_add(struct kvm_vcpu *vcpu, const struct kvm_memory_slot *slot, sp =sptep_to_sp (spte);kvm_mmu_page_set_gfn (sp, spte - sp->spt, gfn); rmap_head =gfn_to_rmap (gfn, sp->role.level, slot); - rmap_count =pte_list_add (vcpu, spte, rmap_head); + rmap_count =pte_list_add (cache, spte, rmap_head);if (rmap_count >RMAP_RECYCLE_THRESHOLD ) { -kvm_unmap_rmapp (vcpu->kvm, rmap_head,NULL , gfn, sp->role.level,__pte (0 )); +kvm_unmap_rmapp (kvm, rmap_head,NULL , gfn, sp->role.level,__pte (0 ));kvm_flush_remote_tlbs_with_address ( - vcpu->kvm, sp->gfn,KVM_PAGES_PER_HPAGE (sp->role.level)); + kvm, sp->gfn,KVM_PAGES_PER_HPAGE (sp->role.level)); } } +static void rmap_add (struct kvm_vcpu *vcpu,const struct kvm_memory_slot *slot, +u64 *spte,gfn_t gfn) +{ +__rmap_add (vcpu->kvm, &vcpu->arch.mmu_pte_list_desc_cache, slot, spte, gfn); +} +bool kvm_age_gfn (struct kvm *kvm,struct kvm_gfn_range *range) {bool young = false;
@@ -1671,13 +1674,13 @@ static unsigned kvm_page_table_hashfn(gfn_t gfn)return hash_64 (gfn,KVM_MMU_HASH_SHIFT ); } -static void mmu_page_add_parent_pte (struct kvm_vcpu *vcpu, +static void mmu_page_add_parent_pte (struct kvm_mmu_memory_cache *cache,struct kvm_mmu_page *sp,u64 *parent_pte) {if (!parent_pte)return ; -pte_list_add (vcpu, parent_pte, &sp->parent_ptes); +pte_list_add (cache, parent_pte, &sp->parent_ptes); } static void mmu_page_remove_parent_pte(struct kvm_mmu_page *sp,
@@ -2276,8 +2279,8 @@ static void shadow_walk_next(struct kvm_shadow_walk_iterator *iterator)__shadow_walk_next (iterator, *iterator->sptep); } -static void link_shadow_page (struct kvm_vcpu *vcpu,u64 *sptep, -struct kvm_mmu_page *sp) +static void __link_shadow_page (struct kvm_mmu_memory_cache *cache,u64 *sptep, +struct kvm_mmu_page *sp) {u64 spte;
@@ -2287,12 +2290,18 @@ static void link_shadow_page(struct kvm_vcpu *vcpu, u64 *sptep,mmu_spte_set (sptep, spte); -mmu_page_add_parent_pte (vcpu, sp, sptep); +mmu_page_add_parent_pte (cache, sp, sptep);if (sp->unsync_children || sp->unsync)mark_unsync (sptep); } +static void link_shadow_page (struct kvm_vcpu *vcpu,u64 *sptep, +struct kvm_mmu_page *sp) +{ +__link_shadow_page (&vcpu->arch.mmu_pte_list_desc_cache, sptep, sp); +} +static void validate_direct_spte (struct kvm_vcpu *vcpu,u64 *sptep,unsigned direct_access) {
Signature · 1 line
2.36.0.550.gb090851708-goog
PATCH v5 15/21 KVM: x86/mmu: Update page stats in __rmap_add()
In reply to David MatlackUpdate the page stats in __rmap_add() rather than at the call site. This will avoid having to manually update page stats when splitting huge pages in a subsequent commit.
No functional change intended.
--- arch/x86/kvm/mmu/mmu.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
arch/x86/kvm/mmu/mmu.c +2 -1
@@ -1588,6 +1588,8 @@ static void __rmap_add(struct kvm *kvm, sp =sptep_to_sp (spte);kvm_mmu_page_set_gfn (sp, spte - sp->spt, gfn); +kvm_update_page_stats (kvm, sp->role.level,1 ); + rmap_head =gfn_to_rmap (gfn, sp->role.level, slot); rmap_count =pte_list_add (cache, spte, rmap_head);
@@ -2810,7 +2812,6 @@ static int mmu_set_spte(struct kvm_vcpu *vcpu, struct kvm_memory_slot *slot,if (!was_rmapped) {WARN_ON_ONCE (ret ==RET_PF_SPURIOUS ); -kvm_update_page_stats (vcpu->kvm, level,1 );rmap_add (vcpu, slot, sptep, gfn); }
Signature · 1 line
2.36.0.550.gb090851708-goog
PATCH v5 16/21 KVM: x86/mmu: Cache the access bits of shadowed translations
In reply to David MatlackSplitting huge pages requires allocating/finding shadow pages to replace the huge page. Shadow pages are keyed, in part, off the guest access permissions they are shadowing. For fully direct MMUs, there is no shadowing so the access bits in the shadow page role are always ACC_ALL. But during shadow paging, the guest can enforce whatever access permissions it wants.
When KVM is resolving a fault, it walks the guest pages tables to determine the guest access permissions. But that is difficult to plumb when splitting huge pages outside of a fault context, e.g. for eager page splitting.
To enable eager page splitting, KVM can cache the shadowed (guest) access permissions whenever it updates the shadow page tables (e.g. during fault, or FNAME(sync_page)). In fact KVM already does this to cache the shadowed GFN using the gfns array in the shadow page. The access bits only take up 3 bits, which leaves 61 bits left over for gfns, which is more than enough. So this change does not require any additional memory.
Now that the gfns array caches more information than just GFNs, rename it to shadowed_translation.
While here, preemptively fix up the WARN_ON() that detects gfn mismatches in direct SPs. The WARN_ON() was paired with a pr_err_ratelimited(), which means that users could sometimes see the WARN without the accompanying error message. Fix this by outputting the error message as part of the WARN splat.
--- arch/x86/include/asm/kvm_host.h | 2 +- arch/x86/kvm/mmu/mmu.c | 85 +++++++++++++++++++++++---------- arch/x86/kvm/mmu/mmu_internal.h | 17 ++++++- arch/x86/kvm/mmu/paging_tmpl.h | 8 +++- 4 files changed, 83 insertions(+), 29 deletions(-)
arch/x86/include/asm/kvm_host.h +1 -1
@@ -710,7 +710,7 @@ struct kvm_vcpu_arch {struct kvm_mmu_memory_cache mmu_pte_list_desc_cache;struct kvm_mmu_memory_cache mmu_shadow_page_cache; -struct kvm_mmu_memory_cache mmu_gfn_array_cache; +struct kvm_mmu_memory_cache mmu_shadowed_info_cache;struct kvm_mmu_memory_cache mmu_page_header_cache;/*
arch/x86/kvm/mmu/mmu.c +61 -24
167 lines not on this page · raw message
arch/x86/kvm/mmu/mmu_internal.h +15 -2
@@ -53,8 +53,21 @@ struct kvm_mmu_page {gfn_t gfn;u64 *spt; -/* hold the gfn of each spte inside spt */ -gfn_t *gfns; + +/* +* Stores the result of the guest translation being shadowed by each +* SPTE. KVM shadows two types of guest translations: nGPA -> GPA +* (shadow EPT/NPT) and GVA -> GPA (traditional shadow paging). In both +* cases the result of the translation is a GPA and a set of access +* constraints. +* +* The GFN is stored in the upper bits (PAGE_SHIFT) and the shadowed +* access permissions are stored in the lower bits. Note, for +* convenience and uniformity across guests, the access permissions are +* stored in KVM format (e.g. ACC_EXEC_MASK) not the raw guest format. +*/ +u64 *shadowed_translation; +/* Currently serving as active root */ union {int root_count;
arch/x86/kvm/mmu/paging_tmpl.h +6 -2
@@ -979,7 +979,8 @@ static gpa_t FNAME(gva_to_gpa)(struct kvm_vcpu *vcpu, struct kvm_mmu *mmu, }/* -* Using the cached information from sp->gfns is safe because: +* Using the information in sp->shadowed_translation (kvm_mmu_page_get_gfn()) is +* safe because: * - The spte has a reference to the struct page, so the pfn for a given gfn * can't change unless all sptes pointing to it are nuked first. *
@@ -1054,12 +1055,15 @@ static int FNAME(sync_page)(struct kvm_vcpu *vcpu, struct kvm_mmu_page *sp)if (sync_mmio_spte (vcpu, &sp->spt[i], gfn, pte_access))continue ; -if (gfn != sp->gfns[i]) { +if (gfn !=kvm_mmu_page_get_gfn (sp, i)) {drop_spte (vcpu->kvm, &sp->spt[i]); flush = true;continue ; } +/* Update the shadowed access bits in case they changed. */ +kvm_mmu_page_set_access (sp, i, pte_access); + sptep = &sp->spt[i]; spte = *sptep; host_writable = spte & shadow_host_writable_mask;
Signature · 1 line
2.36.0.550.gb090851708-goog
PATCH v5 17/21 KVM: x86/mmu: Extend make_huge_page_split_spte() for the shadow MMU
In reply to David MatlackCurrently make_huge_page_split_spte() assumes execute permissions can be granted to any 4K SPTE when splitting huge pages. This is true for the TDP MMU but is not necessarily true for the shadow MMU, since KVM may be shadowing a non-executable huge page.
To fix this, pass in the role of the child shadow page where the huge page will be split and derive the execution permission from that. This is correct because huge pages are always split with direct shadow page and thus the shadow page role contains the correct access permissions.
No functional change intended.
--- arch/x86/kvm/mmu/spte.c | 16 ++++++++-------- arch/x86/kvm/mmu/spte.h | 2 +- arch/x86/kvm/mmu/tdp_mmu.c | 2 +- 3 files changed, 10 insertions(+), 10 deletions(-)
arch/x86/kvm/mmu/spte.c +8 -8
@@ -244,10 +244,10 @@ static u64 make_spte_executable(u64 spte)* This is used during huge page splitting to build the SPTEs that make up the * new page table. */ -u64 make_huge_page_split_spte (u64 huge_spte,int huge_level,int index) +u64 make_huge_page_split_spte (u64 huge_spte,union kvm_mmu_page_role role, +int index) {u64 child_spte; -int child_level;if (WARN_ON_ONCE (!is_shadow_present_pte (huge_spte)))return 0 ;
@@ -256,23 +256,23 @@ u64 make_huge_page_split_spte(u64 huge_spte, int huge_level, int index)return 0 ; child_spte = huge_spte; - child_level = huge_level -1 ;/* * The child_spte already has the base address of the huge page being * split. So we just have to OR in the offset to the page at the next * lower level for the given index. */ - child_spte |= (index *KVM_PAGES_PER_HPAGE (child_level)) <<PAGE_SHIFT ; + child_spte |= (index *KVM_PAGES_PER_HPAGE (role.level)) <<PAGE_SHIFT ; -if (child_level ==PG_LEVEL_4K ) { +if (role.level ==PG_LEVEL_4K ) { child_spte &= ~PT_PAGE_SIZE_MASK ;/* -* When splitting to a 4K page, mark the page executable as the -* NX hugepage mitigation no longer applies. +* When splitting to a 4K page where execution is allowed, mark +* the page executable as the NX hugepage mitigation no longer +* applies. */ -if (is_nx_huge_page_enabled ()) +if ((role.access &ACC_EXEC_MASK ) &&is_nx_huge_page_enabled ()) child_spte =make_spte_executable (child_spte); }
arch/x86/kvm/mmu/spte.h +1 -1
@@ -425,7 +425,7 @@ bool make_spte(struct kvm_vcpu *vcpu, struct kvm_mmu_page *sp, unsigned int pte_access, gfn_t gfn, kvm_pfn_t pfn, u64 old_spte, bool prefetch, bool can_unsync, bool host_writable, u64 *new_spte); -u64 make_huge_page_split_spte (u64 huge_spte,int huge_level,int index); +u64 make_huge_page_split_spte (u64 huge_spte,union kvm_mmu_page_role role,int index);u64 make_nonleaf_spte (u64 *child_pt,bool ad_disabled);u64 make_mmio_spte (struct kvm_vcpu *vcpu,u64 gfn,unsigned int access);u64 mark_spte_for_access_track (u64 spte);
arch/x86/kvm/mmu/tdp_mmu.c +1 -1
@@ -1488,7 +1488,7 @@ static int tdp_mmu_split_huge_page(struct kvm *kvm, struct tdp_iter *iter,* not been linked in yet and thus is not reachable from any other CPU. */ for (i =0 ; i <PT64_ENT_PER_PAGE ; i++) - sp->spt[i] =make_huge_page_split_spte (huge_spte, level, i); + sp->spt[i] =make_huge_page_split_spte (huge_spte, sp->role, i);/* * Replace the huge spte with a pointer to the populated lower level
Signature · 1 line
2.36.0.550.gb090851708-goog
PATCH v5 18/21 KVM: x86/mmu: Zap collapsible SPTEs in shadow MMU at all possible levels
In reply to David MatlackCurrently KVM only zaps collapsible 4KiB SPTEs in the shadow MMU. This is fine for now since KVM never creates intermediate huge pages during dirty logging. In other words, KVM always replaces 1GiB pages directly with 4KiB pages, so there is no reason to look for collapsible 2MiB pages.
However, this will stop being true once the shadow MMU participates in eager page splitting. During eager page splitting, each 1GiB is first split into 2MiB pages and then those are split into 4KiB pages. The intermediate 2MiB pages may be left behind if an error condition causes eager page splitting to bail early.
No functional change intended.
--- arch/x86/kvm/mmu/mmu.c | 21 ++++++++++++++------- 1 file changed, 14 insertions(+), 7 deletions(-)
arch/x86/kvm/mmu/mmu.c +14 -7
@@ -6183,18 +6183,25 @@ static bool kvm_mmu_zap_collapsible_spte(struct kvm *kvm,return need_tlb_flush; } +static void kvm_rmap_zap_collapsible_sptes (struct kvm *kvm, +const struct kvm_memory_slot *slot) +{ +/* +* Note, use KVM_MAX_HUGEPAGE_LEVEL - 1 since there's no need to zap +* pages that are already mapped at the maximum possible level. +*/ +if (slot_handle_level (kvm, slot, kvm_mmu_zap_collapsible_spte, +PG_LEVEL_4K ,KVM_MAX_HUGEPAGE_LEVEL -1 , + true)) +kvm_arch_flush_remote_tlbs_memslot (kvm, slot); +} +void kvm_mmu_zap_collapsible_sptes (struct kvm *kvm,const struct kvm_memory_slot *slot) {if (kvm_memslots_have_rmaps (kvm)) {write_lock (&kvm->mmu_lock); -/* -* Zap only 4k SPTEs since the legacy MMU only supports dirty -* logging at a 4k granularity and never creates collapsible -* 2m SPTEs during dirty logging. -*/ -if (slot_handle_level_4k (kvm, slot, kvm_mmu_zap_collapsible_spte, true)) -kvm_arch_flush_remote_tlbs_memslot (kvm, slot); +kvm_rmap_zap_collapsible_sptes (kvm, slot);write_unlock (&kvm->mmu_lock); }
Signature · 1 line
2.36.0.550.gb090851708-goog
PATCH v5 19/21 KVM: x86/mmu: Refactor drop_large_spte()
In reply to David Matlackdrop_large_spte() drops a large SPTE if it exists and then flushes TLBs. Its helper function, __drop_large_spte(), does the drop without the flush.
In preparation for eager page splitting, which will need to sometimes flush when dropping large SPTEs (and sometimes not), push the flushing logic down into __drop_large_spte() and add a bool parameter to control it.
No functional change intended.
--- arch/x86/kvm/mmu/mmu.c | 28 ++++++++++++++-------------- 1 file changed, 14 insertions(+), 14 deletions(-)
arch/x86/kvm/mmu/mmu.c +14 -14
@@ -1161,26 +1161,26 @@ static void drop_spte(struct kvm *kvm, u64 *sptep)rmap_remove (kvm, sptep); } - -static bool __drop_large_spte (struct kvm *kvm,u64 *sptep) +static void __drop_large_spte (struct kvm *kvm,u64 *sptep,bool flush) { -if (is_large_pte (*sptep)) { -WARN_ON (sptep_to_sp (sptep)->role.level ==PG_LEVEL_4K ); -drop_spte (kvm, sptep); -return true; - } +struct kvm_mmu_page *sp; -return false; +if (!is_large_pte (*sptep)) +return ; + + sp =sptep_to_sp (sptep); +WARN_ON (sp->role.level ==PG_LEVEL_4K ); + +drop_spte (kvm, sptep); + +if (flush) +kvm_flush_remote_tlbs_with_address (kvm, sp->gfn, +KVM_PAGES_PER_HPAGE (sp->role.level)); }static void drop_large_spte (struct kvm_vcpu *vcpu,u64 *sptep) { -if (__drop_large_spte (vcpu->kvm, sptep)) { -struct kvm_mmu_page *sp =sptep_to_sp (sptep); - -kvm_flush_remote_tlbs_with_address (vcpu->kvm, sp->gfn, -KVM_PAGES_PER_HPAGE (sp->role.level)); - } +return __drop_large_spte (vcpu->kvm, sptep, true); }/*
Signature · 1 line
2.36.0.550.gb090851708-goog
PATCH v5 20/21 KVM: Allow for different capacities in kvm_mmu_memory_cache structs
In reply to David MatlackAllow the capacity of the kvm_mmu_memory_cache struct to be chosen at declaration time rather than being fixed for all declarations. This will be used in a follow-up commit to declare an cache in x86 with a capacity of 512+ objects without having to increase the capacity of all caches in KVM.
This change requires each cache now specify its capacity at runtime, since the cache struct itself no longer has a fixed capacity known at compile time. To protect against someone accidentally defining a kvm_mmu_memory_cache struct directly (without the extra storage), this commit includes a WARN_ON() in kvm_mmu_topup_memory_cache().
In order to support different capacities, this commit changes the objects pointer array to be dynamically allocated the first time the cache is topped-up.
An alternative would be to lay out the objects array after the kvm_mmu_memory_cache struct, which can be done at compile time. But that change, unfortunately, adds some grottiness to arm64 and riscv, which uses a function-local (i.e. stack-allocated) kvm_mmu_memory_cache struct. Since C does not allow anonymous structs in functions, the new wrapper struct that contains kvm_mmu_memory_cache and the objects pointer array, must be named, which means dealing with an outer and inner struct. The outer struct can't be dropped since then there would be no guarantee the kvm_mmu_memory_cache struct and objects array would be laid out consecutively on the stack.
No functional change intended.
--- arch/arm64/kvm/arm.c | 1 + arch/arm64/kvm/mmu.c | 5 ++++- arch/mips/kvm/mips.c | 2 ++ arch/riscv/kvm/mmu.c | 8 ++++---- arch/riscv/kvm/vcpu.c | 1 + arch/x86/kvm/mmu/mmu.c | 9 +++++++++ include/linux/kvm_types.h | 9 +++++++-- virt/kvm/kvm_main.c | 20 ++++++++++++++++++-- 8 files changed, 46 insertions(+), 9 deletions(-)
arch/arm64/kvm/arm.c +1 -0
@@ -320,6 +320,7 @@ int kvm_arch_vcpu_create(struct kvm_vcpu *vcpu) vcpu->arch.target =-1 ;bitmap_zero (vcpu->arch.features,KVM_VCPU_MAX_FEATURES ); + vcpu->arch.mmu_page_cache.capacity =KVM_ARCH_NR_OBJS_PER_MEMORY_CACHE ; vcpu->arch.mmu_page_cache.gfp_zero = __GFP_ZERO;/* Set up the timer */
arch/arm64/kvm/mmu.c +4 -1
@@ -764,7 +764,10 @@ int kvm_phys_addr_ioremap(struct kvm *kvm, phys_addr_t guest_ipa, {phys_addr_t addr;int ret =0 ; -struct kvm_mmu_memory_cache cache = {0 , __GFP_ZERO,NULL , }; +struct kvm_mmu_memory_cache cache = { + .capacity =KVM_ARCH_NR_OBJS_PER_MEMORY_CACHE , + .gfp_zero = __GFP_ZERO, + };struct kvm_pgtable *pgt = kvm->arch.mmu.pgt;enum kvm_pgtable_prot prot =KVM_PGTABLE_PROT_DEVICE | KVM_PGTABLE_PROT_R |
arch/mips/kvm/mips.c +2 -0
@@ -387,6 +387,8 @@ int kvm_arch_vcpu_create(struct kvm_vcpu *vcpu)if (err) goto out_free_gebase; + vcpu->arch.mmu_page_cache.capacity =KVM_ARCH_NR_OBJS_PER_MEMORY_CACHE ; +return 0 ; out_free_gebase:
arch/riscv/kvm/mmu.c +4 -4
@@ -347,10 +347,10 @@ static int stage2_ioremap(struct kvm *kvm, gpa_t gpa, phys_addr_t hpa,int ret =0 ;unsigned long pfn;phys_addr_t addr, end; -struct kvm_mmu_memory_cache pcache; - -memset (&pcache,0 ,sizeof (pcache)); - pcache.gfp_zero = __GFP_ZERO; +struct kvm_mmu_memory_cache pcache = { + .capacity =KVM_ARCH_NR_OBJS_PER_MEMORY_CACHE , + .gfp_zero = __GFP_ZERO, + }; end = (gpa + size +PAGE_SIZE -1 ) &PAGE_MASK ; pfn =__phys_to_pfn (hpa);
arch/riscv/kvm/vcpu.c +1 -0
@@ -94,6 +94,7 @@ int kvm_arch_vcpu_create(struct kvm_vcpu *vcpu)/* Mark this VCPU never ran */ vcpu->arch.ran_atleast_once = false; + vcpu->arch.mmu_page_cache.capacity =KVM_ARCH_NR_OBJS_PER_MEMORY_CACHE ; vcpu->arch.mmu_page_cache.gfp_zero = __GFP_ZERO;/* Setup ISA features available to VCPU */
arch/x86/kvm/mmu/mmu.c +9 -0
@@ -5803,12 +5803,21 @@ int kvm_mmu_create(struct kvm_vcpu *vcpu) {int ret; + vcpu->arch.mmu_pte_list_desc_cache.capacity = +KVM_ARCH_NR_OBJS_PER_MEMORY_CACHE ; vcpu->arch.mmu_pte_list_desc_cache.kmem_cache = pte_list_desc_cache; vcpu->arch.mmu_pte_list_desc_cache.gfp_zero = __GFP_ZERO; + vcpu->arch.mmu_page_header_cache.capacity = +KVM_ARCH_NR_OBJS_PER_MEMORY_CACHE ; vcpu->arch.mmu_page_header_cache.kmem_cache = mmu_page_header_cache; vcpu->arch.mmu_page_header_cache.gfp_zero = __GFP_ZERO; + vcpu->arch.mmu_shadowed_info_cache.capacity = +KVM_ARCH_NR_OBJS_PER_MEMORY_CACHE ; + + vcpu->arch.mmu_shadow_page_cache.capacity = +KVM_ARCH_NR_OBJS_PER_MEMORY_CACHE ; vcpu->arch.mmu_shadow_page_cache.gfp_zero = __GFP_ZERO; vcpu->arch.mmu = &vcpu->arch.root_mmu;
include/linux/kvm_types.h +7 -2
@@ -83,14 +83,19 @@ struct gfn_to_pfn_cache {* MMU flows is problematic, as is triggering reclaim, I/O, etc... while * holding MMU locks. Note, these caches act more like prefetch buffers than * classical caches, i.e. objects are not returned to the cache on being freed. +* +* The storage for the cache object pointers is allocated dynamically when the +* cache is topped-up. The capacity field defines the number of object pointers +* available after the struct. */ struct kvm_mmu_memory_cache {int nobjs; +int capacity;gfp_t gfp_zero;struct kmem_cache *kmem_cache; -void *objects[KVM_ARCH_NR_OBJS_PER_MEMORY_CACHE ]; +void **objects; }; -#endif +#endif /* KVM_ARCH_NR_OBJS_PER_MEMORY_CACHE */ #define HALT_POLL_HIST_COUNT 32
virt/kvm/kvm_main.c +18 -2
@@ -371,12 +371,23 @@ static inline void *mmu_memory_cache_alloc_obj(struct kvm_mmu_memory_cache *mc,int kvm_mmu_topup_memory_cache (struct kvm_mmu_memory_cache *mc,int min) { +gfp_t gfp =GFP_KERNEL_ACCOUNT ;void *obj;if (mc->nobjs >= min)return 0 ; -while (mc->nobjs <ARRAY_SIZE (mc->objects)) { - obj =mmu_memory_cache_alloc_obj (mc,GFP_KERNEL_ACCOUNT ); + +if (WARN_ON (mc->capacity ==0 )) +return -EINVAL ; + +if (!mc->objects) { + mc->objects =kvmalloc_array (sizeof (void *), mc->capacity, gfp); +if (!mc->objects) +return -ENOMEM ; + } + +while (mc->nobjs < mc->capacity) { + obj =mmu_memory_cache_alloc_obj (mc, gfp);if (!obj)return mc->nobjs >= min ?0 : -ENOMEM ; mc->objects[mc->nobjs++] = obj;
@@ -397,6 +408,11 @@ void kvm_mmu_free_memory_cache(struct kvm_mmu_memory_cache *mc)else free_page ((unsigned long )mc->objects[--mc->nobjs]); } + +kvfree (mc->objects); + +/* Note, must set to NULL to avoid use-after-free in the next top-up. */ + mc->objects =NULL ; }void *kvm_mmu_memory_cache_alloc (struct kvm_mmu_memory_cache *mc)
Signature · 1 line
2.36.0.550.gb090851708-goog
Re: [PATCH v5 20/21] KVM: Allow for different capacities in kvm_mmu_memory_cache structs
Quoted 17 lines · David Matlack
Allow the capacity of the
kvm_mmu_memory_cachestruct to be chosen at declaration time rather than being fixed for all declarations. This will be used in a follow-up commit to declare an cache in x86 with a capacity of 512+ objects without having to increase the capacity of all caches in KVM.This change requires each cache now specify its capacity at runtime, since the cache struct itself no longer has a fixed capacity known at compile time. To protect against someone accidentally defining a
kvm_mmu_memory_cachestruct directly (without the extra storage), this commit includes aWARN_ON()inkvm_mmu_topup_memory_cache().In order to support different capacities, this commit changes the objects pointer array to be dynamically allocated the first time the cache is topped-up.
Quoted 10 lines
An alternative would be to lay out the objects array after the
kvm_mmu_memory_cachestruct, which can be done at compile time. But that change, unfortunately, adds some grottiness to arm64 and riscv, which uses a function-local (i.e. stack-allocated)kvm_mmu_memory_cachestruct. Since C does not allow anonymous structs in functions, the new wrapper struct that containskvm_mmu_memory_cacheand the objects pointer array, must be named, which means dealing with an outer and inner struct. The outer struct can't be dropped since then there would be no guarantee thekvm_mmu_memory_cachestruct and objects array would be laid out consecutively on the stack.
You may want to drop this paragraph. Someone interested in the history can find it on the list.
Quoted 126 lines
No functional change intended.
Signed-off-by David Matlack <[email protected]>--- arch/arm64/kvm/arm.c | 1 + arch/arm64/kvm/mmu.c | 5 ++++- arch/mips/kvm/mips.c | 2 ++ arch/riscv/kvm/mmu.c | 8 ++++---- arch/riscv/kvm/vcpu.c | 1 + arch/x86/kvm/mmu/mmu.c | 9 +++++++++ include/linux/kvm_types.h | 9 +++++++-- virt/kvm/kvm_main.c | 20 ++++++++++++++++++-- 8 files changed, 46 insertions(+), 9 deletions(-)arch/arm64/kvm/arm.c +1
-0@@ -320,6 +320,7 @@ int kvm_arch_vcpu_create(struct kvm_vcpu *vcpu) vcpu->arch.target =-1 ;bitmap_zero (vcpu->arch.features,KVM_VCPU_MAX_FEATURES ); + vcpu->arch.mmu_page_cache.capacity =KVM_ARCH_NR_OBJS_PER_MEMORY_CACHE ; vcpu->arch.mmu_page_cache.gfp_zero = __GFP_ZERO;/* Set up the timer */ arch/arm64/kvm/mmu.c +4
-1@@ -764,7 +764,10 @@ int kvm_phys_addr_ioremap(struct kvm *kvm, phys_addr_t guest_ipa, {phys_addr_t addr;int ret =0 ; -struct kvm_mmu_memory_cache cache = {0 , __GFP_ZERO,NULL , }; +struct kvm_mmu_memory_cache cache = { + .capacity =KVM_ARCH_NR_OBJS_PER_MEMORY_CACHE , + .gfp_zero = __GFP_ZERO, + };struct kvm_pgtable *pgt = kvm->arch.mmu.pgt;enum kvm_pgtable_prot prot =KVM_PGTABLE_PROT_DEVICE | KVM_PGTABLE_PROT_R |arch/mips/kvm/mips.c +2
-0@@ -387,6 +387,8 @@ int kvm_arch_vcpu_create(struct kvm_vcpu *vcpu)if (err) goto out_free_gebase; + vcpu->arch.mmu_page_cache.capacity =KVM_ARCH_NR_OBJS_PER_MEMORY_CACHE ; +return 0 ; out_free_gebase:arch/riscv/kvm/mmu.c +4
-4@@ -347,10 +347,10 @@ static int stage2_ioremap(struct kvm *kvm, gpa_t gpa, phys_addr_t hpa,int ret =0 ;unsigned long pfn;phys_addr_t addr, end; -struct kvm_mmu_memory_cache pcache; - -memset (&pcache,0 ,sizeof (pcache)); - pcache.gfp_zero = __GFP_ZERO; +struct kvm_mmu_memory_cache pcache = { + .capacity =KVM_ARCH_NR_OBJS_PER_MEMORY_CACHE , + .gfp_zero = __GFP_ZERO, + }; end = (gpa + size +PAGE_SIZE -1 ) &PAGE_MASK ; pfn =__phys_to_pfn (hpa);arch/riscv/kvm/vcpu.c +1
-0@@ -94,6 +94,7 @@ int kvm_arch_vcpu_create(struct kvm_vcpu *vcpu)/* Mark this VCPU never ran */ vcpu->arch.ran_atleast_once = false; + vcpu->arch.mmu_page_cache.capacity =KVM_ARCH_NR_OBJS_PER_MEMORY_CACHE ; vcpu->arch.mmu_page_cache.gfp_zero = __GFP_ZERO;/* Setup ISA features available to VCPU */ arch/x86/kvm/mmu/mmu.c +9
-0@@ -5803,12 +5803,21 @@ int kvm_mmu_create(struct kvm_vcpu *vcpu) {int ret; + vcpu->arch.mmu_pte_list_desc_cache.capacity = +KVM_ARCH_NR_OBJS_PER_MEMORY_CACHE ; vcpu->arch.mmu_pte_list_desc_cache.kmem_cache = pte_list_desc_cache; vcpu->arch.mmu_pte_list_desc_cache.gfp_zero = __GFP_ZERO; + vcpu->arch.mmu_page_header_cache.capacity = +KVM_ARCH_NR_OBJS_PER_MEMORY_CACHE ; vcpu->arch.mmu_page_header_cache.kmem_cache = mmu_page_header_cache; vcpu->arch.mmu_page_header_cache.gfp_zero = __GFP_ZERO; + vcpu->arch.mmu_shadowed_info_cache.capacity = +KVM_ARCH_NR_OBJS_PER_MEMORY_CACHE ; + + vcpu->arch.mmu_shadow_page_cache.capacity = +KVM_ARCH_NR_OBJS_PER_MEMORY_CACHE ; vcpu->arch.mmu_shadow_page_cache.gfp_zero = __GFP_ZERO; vcpu->arch.mmu = &vcpu->arch.root_mmu;include/linux/kvm_types.h +4
-0@@ -83,14 +83,19 @@ struct gfn_to_pfn_cache {* MMU flows is problematic, as is triggering reclaim, I/O, etc... while * holding MMU locks. Note, these caches act more like prefetch buffers than * classical caches, i.e. objects are not returned to the cache on being freed. +* +* The storage for the cache object pointers is allocated dynamically when the +* cache is topped-up. The capacity field defines the number of object pointers +* available after the struct. */
include/linux/kvm_types.h struct gfn_to_pfn_cache { · line 98
Quoted 10 lines
include/linux/kvm_types.h +3
-2struct kvm_mmu_memory_cache {int nobjs; +int capacity;gfp_t gfp_zero;struct kmem_cache *kmem_cache; -void *objects[KVM_ARCH_NR_OBJS_PER_MEMORY_CACHE ]; +void **objects; }; -#endif +#endif /* KVM_ARCH_NR_OBJS_PER_MEMORY_CACHE */
One thing that is missing here (and was already missing) is to make it plain that kvm_mmu_memory_cache can only be used in contexts where there are no concurrent accesses to the cache.
Quoted 35 lines
include/linux/kvm_types.h +0
-0#define HALT_POLL_HIST_COUNT 32virt/kvm/kvm_main.c +13
-2@@ -371,12 +371,23 @@ static inline void *mmu_memory_cache_alloc_obj(struct kvm_mmu_memory_cache *mc,int kvm_mmu_topup_memory_cache (struct kvm_mmu_memory_cache *mc,int min) { +gfp_t gfp =GFP_KERNEL_ACCOUNT ;void *obj;if (mc->nobjs >= min)return 0 ; -while (mc->nobjs <ARRAY_SIZE (mc->objects)) { - obj =mmu_memory_cache_alloc_obj (mc,GFP_KERNEL_ACCOUNT ); + +if (WARN_ON (mc->capacity ==0 )) +return -EINVAL ; + +if (!mc->objects) { + mc->objects =kvmalloc_array (sizeof (void *), mc->capacity, gfp); +if (!mc->objects) +return -ENOMEM ; + } + +while (mc->nobjs < mc->capacity) { + obj =mmu_memory_cache_alloc_obj (mc, gfp);if (!obj)return mc->nobjs >= min ?0 : -ENOMEM ; mc->objects[mc->nobjs++] = obj;@@ -397,6 +408,11 @@ void kvm_mmu_free_memory_cache(struct kvm_mmu_memory_cache *mc)else
virt/kvm/kvm_main.c void kvm_mmu_free_memory_cache(struct kvm_mmu_memory_cache *mc) · line 418
Quoted 10 lines
virt/kvm/kvm_main.c +5
-0free_page ((unsigned long )mc->objects[--mc->nobjs]); } + +kvfree (mc->objects); + +/* Note, must set to NULL to avoid use-after-free in the next top-up. */ + mc->objects =NULL ; }void *kvm_mmu_memory_cache_alloc (struct kvm_mmu_memory_cache *mc)
Otherwise:
M.
Signature · 1 line
Without deviation from the norm, progress is not possible.
Anup Patel
maintainerRe: [PATCH v5 20/21] KVM: Allow for different capacities in kvm_mmu_memory_cache structs
In reply to David MatlackQuoted 28 lines · David Matlack
Allow the capacity of the
kvm_mmu_memory_cachestruct to be chosen at declaration time rather than being fixed for all declarations. This will be used in a follow-up commit to declare an cache in x86 with a capacity of 512+ objects without having to increase the capacity of all caches in KVM.This change requires each cache now specify its capacity at runtime, since the cache struct itself no longer has a fixed capacity known at compile time. To protect against someone accidentally defining a
kvm_mmu_memory_cachestruct directly (without the extra storage), this commit includes aWARN_ON()inkvm_mmu_topup_memory_cache().In order to support different capacities, this commit changes the objects pointer array to be dynamically allocated the first time the cache is topped-up.
An alternative would be to lay out the objects array after the
kvm_mmu_memory_cachestruct, which can be done at compile time. But that change, unfortunately, adds some grottiness to arm64 and riscv, which uses a function-local (i.e. stack-allocated)kvm_mmu_memory_cachestruct. Since C does not allow anonymous structs in functions, the new wrapper struct that containskvm_mmu_memory_cacheand the objects pointer array, must be named, which means dealing with an outer and inner struct. The outer struct can't be dropped since then there would be no guarantee thekvm_mmu_memory_cachestruct and objects array would be laid out consecutively on the stack.
Quoted 3 lines
No functional change intended.
Signed-off-by David Matlack <[email protected]>
Overall, this looks good to me. I wanted to try this patch with KVM RISC-V but there are conflicts in include/linux/kvm_types.h and virt/kvm/kvm_main.c
Please take a look.
Regards,
Anup
Quoted 180 lines
--- arch/arm64/kvm/arm.c | 1 + arch/arm64/kvm/mmu.c | 5 ++++- arch/mips/kvm/mips.c | 2 ++ arch/riscv/kvm/mmu.c | 8 ++++---- arch/riscv/kvm/vcpu.c | 1 + arch/x86/kvm/mmu/mmu.c | 9 +++++++++ include/linux/kvm_types.h | 9 +++++++-- virt/kvm/kvm_main.c | 20 ++++++++++++++++++-- 8 files changed, 46 insertions(+), 9 deletions(-)arch/arm64/kvm/arm.c +1
-0@@ -320,6 +320,7 @@ int kvm_arch_vcpu_create(struct kvm_vcpu *vcpu) vcpu->arch.target =-1 ;bitmap_zero (vcpu->arch.features,KVM_VCPU_MAX_FEATURES ); + vcpu->arch.mmu_page_cache.capacity =KVM_ARCH_NR_OBJS_PER_MEMORY_CACHE ; vcpu->arch.mmu_page_cache.gfp_zero = __GFP_ZERO;/* Set up the timer */ arch/arm64/kvm/mmu.c +4
-1@@ -764,7 +764,10 @@ int kvm_phys_addr_ioremap(struct kvm *kvm, phys_addr_t guest_ipa, {phys_addr_t addr;int ret =0 ; -struct kvm_mmu_memory_cache cache = {0 , __GFP_ZERO,NULL , }; +struct kvm_mmu_memory_cache cache = { + .capacity =KVM_ARCH_NR_OBJS_PER_MEMORY_CACHE , + .gfp_zero = __GFP_ZERO, + };struct kvm_pgtable *pgt = kvm->arch.mmu.pgt;enum kvm_pgtable_prot prot =KVM_PGTABLE_PROT_DEVICE | KVM_PGTABLE_PROT_R |arch/mips/kvm/mips.c +2
-0@@ -387,6 +387,8 @@ int kvm_arch_vcpu_create(struct kvm_vcpu *vcpu)if (err) goto out_free_gebase; + vcpu->arch.mmu_page_cache.capacity =KVM_ARCH_NR_OBJS_PER_MEMORY_CACHE ; +return 0 ; out_free_gebase:arch/riscv/kvm/mmu.c +4
-4@@ -347,10 +347,10 @@ static int stage2_ioremap(struct kvm *kvm, gpa_t gpa, phys_addr_t hpa,int ret =0 ;unsigned long pfn;phys_addr_t addr, end; -struct kvm_mmu_memory_cache pcache; - -memset (&pcache,0 ,sizeof (pcache)); - pcache.gfp_zero = __GFP_ZERO; +struct kvm_mmu_memory_cache pcache = { + .capacity =KVM_ARCH_NR_OBJS_PER_MEMORY_CACHE , + .gfp_zero = __GFP_ZERO, + }; end = (gpa + size +PAGE_SIZE -1 ) &PAGE_MASK ; pfn =__phys_to_pfn (hpa);arch/riscv/kvm/vcpu.c +1
-0@@ -94,6 +94,7 @@ int kvm_arch_vcpu_create(struct kvm_vcpu *vcpu)/* Mark this VCPU never ran */ vcpu->arch.ran_atleast_once = false; + vcpu->arch.mmu_page_cache.capacity =KVM_ARCH_NR_OBJS_PER_MEMORY_CACHE ; vcpu->arch.mmu_page_cache.gfp_zero = __GFP_ZERO;/* Setup ISA features available to VCPU */ arch/x86/kvm/mmu/mmu.c +9
-0@@ -5803,12 +5803,21 @@ int kvm_mmu_create(struct kvm_vcpu *vcpu) {int ret; + vcpu->arch.mmu_pte_list_desc_cache.capacity = +KVM_ARCH_NR_OBJS_PER_MEMORY_CACHE ; vcpu->arch.mmu_pte_list_desc_cache.kmem_cache = pte_list_desc_cache; vcpu->arch.mmu_pte_list_desc_cache.gfp_zero = __GFP_ZERO; + vcpu->arch.mmu_page_header_cache.capacity = +KVM_ARCH_NR_OBJS_PER_MEMORY_CACHE ; vcpu->arch.mmu_page_header_cache.kmem_cache = mmu_page_header_cache; vcpu->arch.mmu_page_header_cache.gfp_zero = __GFP_ZERO; + vcpu->arch.mmu_shadowed_info_cache.capacity = +KVM_ARCH_NR_OBJS_PER_MEMORY_CACHE ; + + vcpu->arch.mmu_shadow_page_cache.capacity = +KVM_ARCH_NR_OBJS_PER_MEMORY_CACHE ; vcpu->arch.mmu_shadow_page_cache.gfp_zero = __GFP_ZERO; vcpu->arch.mmu = &vcpu->arch.root_mmu;include/linux/kvm_types.h +7
-2@@ -83,14 +83,19 @@ struct gfn_to_pfn_cache {* MMU flows is problematic, as is triggering reclaim, I/O, etc... while * holding MMU locks. Note, these caches act more like prefetch buffers than * classical caches, i.e. objects are not returned to the cache on being freed. +* +* The storage for the cache object pointers is allocated dynamically when the +* cache is topped-up. The capacity field defines the number of object pointers +* available after the struct. */ struct kvm_mmu_memory_cache {int nobjs; +int capacity;gfp_t gfp_zero;struct kmem_cache *kmem_cache; -void *objects[KVM_ARCH_NR_OBJS_PER_MEMORY_CACHE ]; +void **objects; }; -#endif +#endif /* KVM_ARCH_NR_OBJS_PER_MEMORY_CACHE */ #define HALT_POLL_HIST_COUNT 32virt/kvm/kvm_main.c +18
-3@@ -371,12 +371,23 @@ static inline void *mmu_memory_cache_alloc_obj(struct kvm_mmu_memory_cache *mc,int kvm_mmu_topup_memory_cache (struct kvm_mmu_memory_cache *mc,int min) { +gfp_t gfp =GFP_KERNEL_ACCOUNT ;void *obj;if (mc->nobjs >= min)return 0 ; -while (mc->nobjs <ARRAY_SIZE (mc->objects)) { - obj =mmu_memory_cache_alloc_obj (mc,GFP_KERNEL_ACCOUNT ); + +if (WARN_ON (mc->capacity ==0 )) +return -EINVAL ; + +if (!mc->objects) { + mc->objects =kvmalloc_array (sizeof (void *), mc->capacity, gfp); +if (!mc->objects) +return -ENOMEM ; + } + +while (mc->nobjs < mc->capacity) { + obj =mmu_memory_cache_alloc_obj (mc, gfp);if (!obj)return mc->nobjs >= min ?0 : -ENOMEM ; mc->objects[mc->nobjs++] = obj;@@ -397,6 +408,11 @@ void kvm_mmu_free_memory_cache(struct kvm_mmu_memory_cache *mc)else free_page ((unsigned long )mc->objects[--mc->nobjs]); } + +kvfree (mc->objects); + +/* Note, must set to NULL to avoid use-after-free in the next top-up. */ + mc->objects =NULL ; }void *kvm_mmu_memory_cache_alloc (struct kvm_mmu_memory_cache *mc) --2.36.0.550.gb090851708-goog
Re: [PATCH v5 20/21] KVM: Allow for different capacities in kvm_mmu_memory_cache structs
Quoted 30 lines · Anup Patel
Quoted 28 lines · David Matlack
Allow the capacity of the
kvm_mmu_memory_cachestruct to be chosen at declaration time rather than being fixed for all declarations. This will be used in a follow-up commit to declare an cache in x86 with a capacity of 512+ objects without having to increase the capacity of all caches in KVM.This change requires each cache now specify its capacity at runtime, since the cache struct itself no longer has a fixed capacity known at compile time. To protect against someone accidentally defining a
kvm_mmu_memory_cachestruct directly (without the extra storage), this commit includes aWARN_ON()inkvm_mmu_topup_memory_cache().In order to support different capacities, this commit changes the objects pointer array to be dynamically allocated the first time the cache is topped-up.
An alternative would be to lay out the objects array after the
kvm_mmu_memory_cachestruct, which can be done at compile time. But that change, unfortunately, adds some grottiness to arm64 and riscv, which uses a function-local (i.e. stack-allocated)kvm_mmu_memory_cachestruct. Since C does not allow anonymous structs in functions, the new wrapper struct that containskvm_mmu_memory_cacheand the objects pointer array, must be named, which means dealing with an outer and inner struct. The outer struct can't be dropped since then there would be no guarantee thekvm_mmu_memory_cachestruct and objects array would be laid out consecutively on the stack.
Quoted 6 lines
Quoted 3 lines
No functional change intended.
Signed-off-by David Matlack <[email protected]>Overall, this looks good to me. I wanted to try this patch with KVM RISC-V but there are conflicts in
include/linux/kvm_types.handvirt/kvm/kvm_main.c
Hi Anup,
I just sent another version rebased on top of the latest kvm/queue. Let me know if you are still seeing merge conflicts.
Quoted 186 lines
Please take a look.
Regards,
AnupQuoted 180 lines
--- arch/arm64/kvm/arm.c | 1 + arch/arm64/kvm/mmu.c | 5 ++++- arch/mips/kvm/mips.c | 2 ++ arch/riscv/kvm/mmu.c | 8 ++++---- arch/riscv/kvm/vcpu.c | 1 + arch/x86/kvm/mmu/mmu.c | 9 +++++++++ include/linux/kvm_types.h | 9 +++++++-- virt/kvm/kvm_main.c | 20 ++++++++++++++++++-- 8 files changed, 46 insertions(+), 9 deletions(-)arch/arm64/kvm/arm.c +1
-0@@ -320,6 +320,7 @@ int kvm_arch_vcpu_create(struct kvm_vcpu *vcpu) vcpu->arch.target =-1 ;bitmap_zero (vcpu->arch.features,KVM_VCPU_MAX_FEATURES ); + vcpu->arch.mmu_page_cache.capacity =KVM_ARCH_NR_OBJS_PER_MEMORY_CACHE ; vcpu->arch.mmu_page_cache.gfp_zero = __GFP_ZERO;/* Set up the timer */ arch/arm64/kvm/mmu.c +4
-1@@ -764,7 +764,10 @@ int kvm_phys_addr_ioremap(struct kvm *kvm, phys_addr_t guest_ipa, {phys_addr_t addr;int ret =0 ; -struct kvm_mmu_memory_cache cache = {0 , __GFP_ZERO,NULL , }; +struct kvm_mmu_memory_cache cache = { + .capacity =KVM_ARCH_NR_OBJS_PER_MEMORY_CACHE , + .gfp_zero = __GFP_ZERO, + };struct kvm_pgtable *pgt = kvm->arch.mmu.pgt;enum kvm_pgtable_prot prot =KVM_PGTABLE_PROT_DEVICE | KVM_PGTABLE_PROT_R |arch/mips/kvm/mips.c +2
-0@@ -387,6 +387,8 @@ int kvm_arch_vcpu_create(struct kvm_vcpu *vcpu)if (err) goto out_free_gebase; + vcpu->arch.mmu_page_cache.capacity =KVM_ARCH_NR_OBJS_PER_MEMORY_CACHE ; +return 0 ; out_free_gebase:arch/riscv/kvm/mmu.c +4
-4@@ -347,10 +347,10 @@ static int stage2_ioremap(struct kvm *kvm, gpa_t gpa, phys_addr_t hpa,int ret =0 ;unsigned long pfn;phys_addr_t addr, end; -struct kvm_mmu_memory_cache pcache; - -memset (&pcache,0 ,sizeof (pcache)); - pcache.gfp_zero = __GFP_ZERO; +struct kvm_mmu_memory_cache pcache = { + .capacity =KVM_ARCH_NR_OBJS_PER_MEMORY_CACHE , + .gfp_zero = __GFP_ZERO, + }; end = (gpa + size +PAGE_SIZE -1 ) &PAGE_MASK ; pfn =__phys_to_pfn (hpa);arch/riscv/kvm/vcpu.c +1
-0@@ -94,6 +94,7 @@ int kvm_arch_vcpu_create(struct kvm_vcpu *vcpu)/* Mark this VCPU never ran */ vcpu->arch.ran_atleast_once = false; + vcpu->arch.mmu_page_cache.capacity =KVM_ARCH_NR_OBJS_PER_MEMORY_CACHE ; vcpu->arch.mmu_page_cache.gfp_zero = __GFP_ZERO;/* Setup ISA features available to VCPU */ arch/x86/kvm/mmu/mmu.c +9
-0@@ -5803,12 +5803,21 @@ int kvm_mmu_create(struct kvm_vcpu *vcpu) {int ret; + vcpu->arch.mmu_pte_list_desc_cache.capacity = +KVM_ARCH_NR_OBJS_PER_MEMORY_CACHE ; vcpu->arch.mmu_pte_list_desc_cache.kmem_cache = pte_list_desc_cache; vcpu->arch.mmu_pte_list_desc_cache.gfp_zero = __GFP_ZERO; + vcpu->arch.mmu_page_header_cache.capacity = +KVM_ARCH_NR_OBJS_PER_MEMORY_CACHE ; vcpu->arch.mmu_page_header_cache.kmem_cache = mmu_page_header_cache; vcpu->arch.mmu_page_header_cache.gfp_zero = __GFP_ZERO; + vcpu->arch.mmu_shadowed_info_cache.capacity = +KVM_ARCH_NR_OBJS_PER_MEMORY_CACHE ; + + vcpu->arch.mmu_shadow_page_cache.capacity = +KVM_ARCH_NR_OBJS_PER_MEMORY_CACHE ; vcpu->arch.mmu_shadow_page_cache.gfp_zero = __GFP_ZERO; vcpu->arch.mmu = &vcpu->arch.root_mmu;include/linux/kvm_types.h +7
-2@@ -83,14 +83,19 @@ struct gfn_to_pfn_cache {* MMU flows is problematic, as is triggering reclaim, I/O, etc... while * holding MMU locks. Note, these caches act more like prefetch buffers than * classical caches, i.e. objects are not returned to the cache on being freed. +* +* The storage for the cache object pointers is allocated dynamically when the +* cache is topped-up. The capacity field defines the number of object pointers +* available after the struct. */ struct kvm_mmu_memory_cache {int nobjs; +int capacity;gfp_t gfp_zero;struct kmem_cache *kmem_cache; -void *objects[KVM_ARCH_NR_OBJS_PER_MEMORY_CACHE ]; +void **objects; }; -#endif +#endif /* KVM_ARCH_NR_OBJS_PER_MEMORY_CACHE */ #define HALT_POLL_HIST_COUNT 32virt/kvm/kvm_main.c +18
-3@@ -371,12 +371,23 @@ static inline void *mmu_memory_cache_alloc_obj(struct kvm_mmu_memory_cache *mc,int kvm_mmu_topup_memory_cache (struct kvm_mmu_memory_cache *mc,int min) { +gfp_t gfp =GFP_KERNEL_ACCOUNT ;void *obj;if (mc->nobjs >= min)return 0 ; -while (mc->nobjs <ARRAY_SIZE (mc->objects)) { - obj =mmu_memory_cache_alloc_obj (mc,GFP_KERNEL_ACCOUNT ); + +if (WARN_ON (mc->capacity ==0 )) +return -EINVAL ; + +if (!mc->objects) { + mc->objects =kvmalloc_array (sizeof (void *), mc->capacity, gfp); +if (!mc->objects) +return -ENOMEM ; + } + +while (mc->nobjs < mc->capacity) { + obj =mmu_memory_cache_alloc_obj (mc, gfp);if (!obj)return mc->nobjs >= min ?0 : -ENOMEM ; mc->objects[mc->nobjs++] = obj;@@ -397,6 +408,11 @@ void kvm_mmu_free_memory_cache(struct kvm_mmu_memory_cache *mc)else free_page ((unsigned long )mc->objects[--mc->nobjs]); } + +kvfree (mc->objects); + +/* Note, must set to NULL to avoid use-after-free in the next top-up. */ + mc->objects =NULL ; }void *kvm_mmu_memory_cache_alloc (struct kvm_mmu_memory_cache *mc) --2.36.0.550.gb090851708-goog
Sean Christopherson
maintainerRe: [PATCH v5 20/21] KVM: Allow for different capacities in kvm_mmu_memory_cache structs
In reply to David Matlackarch/arm64/kvm/mmu.c int kvm_phys_addr_ioremap(struct kvm *kvm, phys_addr_t guest_ipa, · line 769
Quoted 12 lines · David Matlack
arch/arm64/kvm/mmu.c +3
-1@@ -764,7 +764,10 @@ int kvm_phys_addr_ioremap(struct kvm *kvm, phys_addr_t guest_ipa, {phys_addr_t addr;int ret =0 ; -struct kvm_mmu_memory_cache cache = {0 , __GFP_ZERO,NULL , }; +struct kvm_mmu_memory_cache cache = { + .capacity =KVM_ARCH_NR_OBJS_PER_MEMORY_CACHE , + .gfp_zero = __GFP_ZERO,
I dislike requiring all users to specificy the capacity. It largely defeats the purpose of KVM_ARCH_NR_OBJS_PER_MEMORY_CACHE, and bleeds details into code that really doesn't care all that much about the details.
Rather than force the capacity to be set before topup, what about adding a custom capacity topup helper? That allows keeping a default capacity, better documents the caches that are special, and provides an opportunity to sanity check that the capacity isn't incorrectly changed by the user.
And then I believe this code becomes:
struct kvm_mmu_memory_cache cache = { .gfp_zero = __GFP_ZERO };E.g. (completely untested)
static int __kvm_mmu_topup_memory_cache(struct kvm_mmu_memory_cache *mc,
int capacity, int min)
{
gfp_t gfp = GFP_KERNEL_ACCOUNT;
void *obj;
if (mc->nobjs >= min)
return 0;
if (likely(mc->capacity)) {
if (WARN_ON_ONCE(mc->capacity != capacity || !mc->objects))
return -EIO;
} else {
mc->objects = kvmalloc_array(sizeof(void *), capacity, gfp);
if (!mc->objects)
return -ENOMEM;
mc->capacity = capacity;
}
while (mc->nobjs < mc->capacity) {
obj = mmu_memory_cache_alloc_obj(mc, gfp);
if (!obj)
return mc->nobjs >= min ? 0 : -ENOMEM;
mc->objects[mc->nobjs++] = obj;
}
return 0;
}int kvm_mmu_topup_memory_cache(struct kvm_mmu_memory_cache *mc, int min)
{
const int capacity = KVM_ARCH_NR_OBJS_PER_MEMORY_CACHE;
return __kvm_mmu_topup_memory_cache(mc, capacity, min);
}int kvm_mmu_topup_custom_memory_cache(struct kvm_mmu_memory_cache *mc,
int capacity)
{
return __kvm_mmu_topup_memory_cache(mc, capacity, capacity);
}Re: [PATCH v5 20/21] KVM: Allow for different capacities in kvm_mmu_memory_cache structs
Quoted 19 lines · Sean Christopherson
arch/arm64/kvm/mmu.c int kvm_phys_addr_ioremap(struct kvm *kvm, phys_addr_t guest_ipa, · line 769
Quoted 12 lines · David Matlack
arch/arm64/kvm/mmu.c +3
-1@@ -764,7 +764,10 @@ int kvm_phys_addr_ioremap(struct kvm *kvm, phys_addr_t guest_ipa, {phys_addr_t addr;int ret =0 ; -struct kvm_mmu_memory_cache cache = {0 , __GFP_ZERO,NULL , }; +struct kvm_mmu_memory_cache cache = { + .capacity =KVM_ARCH_NR_OBJS_PER_MEMORY_CACHE , + .gfp_zero = __GFP_ZERO,I dislike requiring all users to specificy the capacity. It largely defeats the purpose of
KVM_ARCH_NR_OBJS_PER_MEMORY_CACHE, and bleeds details into code that really doesn't care all that much about the details.
Quoted 4 lines
Rather than force the capacity to be set before topup, what about adding a custom capacity topup helper? That allows keeping a default capacity, better documents the caches that are special, and provides an opportunity to sanity check that the capacity isn't incorrectly changed by the user.
Even simpler: If mc->capacity is 0 in topup, set it to KVM_ARCH_NR_OBJS_PER_MEMORY_CACHE.
This is what I had before when I was laying out the storage for objects in a separate array. It was risky then because it was too easy for someone to accidentally corrupt memory (call topup with capacity==0 but without allocating the objects array). Now that topup takes care of allocation automatically, that risk is gone.
Quoted 49 lines
And then I believe this code becomes:
struct kvm_mmu_memory_cache cache = { .gfp_zero = __GFP_ZERO };E.g. (completely untested)
static int __kvm_mmu_topup_memory_cache(struct
kvm_mmu_memory_cache*mc,
int capacity, int min){ gfp_t gfp = GFP_KERNEL_ACCOUNT; void *obj; if (mc->nobjs >= min) return 0; if (likely(mc->capacity)) { if (WARN_ON_ONCE(mc->capacity != capacity || !mc->objects)) return -EIO; } else { mc->objects = kvmalloc_array(sizeof(void *), capacity, gfp); if (!mc->objects) return -ENOMEM; mc->capacity = capacity; } while (mc->nobjs < mc->capacity) { obj = mmu_memory_cache_alloc_obj(mc, gfp); if (!obj) return mc->nobjs >= min ? 0 : -ENOMEM; mc->objects[mc->nobjs++] = obj; } return 0; }int
kvm_mmu_topup_memory_cache(struct kvm_mmu_memory_cache *mc, int min){ const int capacity = KVM_ARCH_NR_OBJS_PER_MEMORY_CACHE; return __kvm_mmu_topup_memory_cache(mc, capacity, min); }int kvm_mmu_topup_custom_memory_cache(struct
kvm_mmu_memory_cache*mc,
int capacity){ return __kvm_mmu_topup_memory_cache(mc, capacity, capacity); }
Sean Christopherson
maintainerRe: [PATCH v5 20/21] KVM: Allow for different capacities in kvm_mmu_memory_cache structs
Quoted 20 lines · David Matlack
Quoted 19 lines · Sean Christopherson
arch/arm64/kvm/mmu.c int kvm_phys_addr_ioremap(struct kvm *kvm, phys_addr_t guest_ipa, · line 769
Quoted 12 lines · David Matlack
arch/arm64/kvm/mmu.c +3
-1@@ -764,7 +764,10 @@ int kvm_phys_addr_ioremap(struct kvm *kvm, phys_addr_t guest_ipa, {phys_addr_t addr;int ret =0 ; -struct kvm_mmu_memory_cache cache = {0 , __GFP_ZERO,NULL , }; +struct kvm_mmu_memory_cache cache = { + .capacity =KVM_ARCH_NR_OBJS_PER_MEMORY_CACHE , + .gfp_zero = __GFP_ZERO,I dislike requiring all users to specificy the capacity. It largely defeats the purpose of
KVM_ARCH_NR_OBJS_PER_MEMORY_CACHE, and bleeds details into code that really doesn't care all that much about the details.
Quoted 7 lines
Quoted 4 lines
Rather than force the capacity to be set before topup, what about adding a custom capacity topup helper? That allows keeping a default capacity, better documents the caches that are special, and provides an opportunity to sanity check that the capacity isn't incorrectly changed by the user.
Even simpler: If
mc->capacityis 0 in topup, set it toKVM_ARCH_NR_OBJS_PER_MEMORY_CACHE.
I slightly prefer the explicit "custom" approach as it guards against topup being called before the capacity is initialized, and against the capacity being changed after the first topup call. It's a somewhat contrived reason since we obviously rely on gfp_zero to be initialized before topup, but I like being more explicit nonetheless.
PATCH v5 21/21 KVM: x86/mmu: Extend Eager Page Splitting to nested MMUs
In reply to David MatlackAdd support for Eager Page Splitting pages that are mapped by nested MMUs. Walk through the rmap first splitting all 1GiB pages to 2MiB pages, and then splitting all 2MiB pages to 4KiB pages.
Note, Eager Page Splitting is limited to nested MMUs as a policy rather than due to any technical reason (the sp->role.guest_mode check could just be deleted and Eager Page Splitting would work correctly for all shadow MMU pages). There is really no reason to support Eager Page Splitting for tdp_mmu=N, since such support will eventually be phased out, and there is no current use case supporting Eager Page Splitting on hosts where TDP is either disabled or unavailable in hardware. Furthermore, future improvements to nested MMU scalability may diverge the code from the legacy shadow paging implementation. These improvements will be simpler to make if Eager Page Splitting does not have to worry about legacy shadow paging.
Splitting huge pages mapped by nested MMUs requires dealing with some extra complexity beyond that of the TDP MMU:
(1) The shadow MMU has a limit on the number of shadow pages that are allowed to be allocated. So, as a policy, Eager Page Splitting refuses to split if there are KVM_MIN_FREE_MMU_PAGES or fewer pages available.
(2) Splitting a huge page may end up re-using an existing lower level shadow page tables. This is unlike the TDP MMU which always allocates new shadow page tables when splitting.
(3) When installing the lower level SPTEs, they must be added to the rmap which may require allocating additional pte_list_desc structs.
Case (2) is especially interesting since it may require a TLB flush, unlike the TDP MMU which can fully split huge pages without any TLB flushes. Specifically, an existing lower level page table may point to even lower level page tables that are not fully populated, effectively unmapping a portion of the huge page, which requires a flush.
This commit performs such flushes after dropping the huge page and before installing the lower level page table. This TLB flush could instead be delayed until the MMU lock is about to be dropped, which would batch flushes for multiple splits. However these flushes should be rare in practice (a huge page must be aliased in multiple SPTEs and have been split for NX Huge Pages in only some of them). Flushing immediately is simpler to plumb and also reduces the chances of tripping over a CPU bug (e.g. see iTLB multihit).
[ This commit is based off of the original implementation of Eager Page
Splitting from Peter in Google's kernel from 2016. ]
--- .../admin-guide/kernel-parameters.txt | 3 +- arch/x86/include/asm/kvm_host.h | 24 ++ arch/x86/kvm/mmu/mmu.c | 269 +++++++++++++++++- arch/x86/kvm/x86.c | 6 + 4 files changed, 293 insertions(+), 9 deletions(-)
Documentation/admin-guide/kernel-parameters.txt +1 -2
@@ -2387,8 +2387,7 @@ the KVM_CLEAR_DIRTY ioctl, and only for the pages being cleared. - Eager page splitting currently only supports splitting - huge pages mapped by the TDP MMU. + Eager page splitting is only supported when kvm.tdp_mmu=Y. Default is Y (on).
arch/x86/include/asm/kvm_host.h +24 -0
@@ -1265,6 +1265,28 @@ struct kvm_arch {* the global KVM_MAX_VCPU_IDS may lead to significant memory waste. */ u32 max_vcpu_ids; + +/* +* Memory caches used to allocate shadow pages when performing eager +* page splitting. No need for a shadowed_info_cache since eager page +* splitting only allocates direct shadow pages. +* +* Protected by kvm->slots_lock. +*/ +struct kvm_mmu_memory_cache split_shadow_page_cache; +struct kvm_mmu_memory_cache split_page_header_cache; + +/* +* Memory cache used to allocate pte_list_desc structs while splitting +* huge pages. In the worst case, to split one huge page, 512 +* pte_list_desc structs are needed to add each lower level leaf sptep +* to the rmap plus 1 to extend the parent_ptes rmap of the lower level +* page table. +* +* Protected by kvm->slots_lock. +*/ +#define SPLIT_DESC_CACHE_CAPACITY 513 +struct kvm_mmu_memory_cache split_desc_cache; };struct kvm_vm_stat {
@@ -1639,6 +1661,8 @@ void kvm_mmu_zap_all(struct kvm *kvm);void kvm_mmu_invalidate_mmio_sptes (struct kvm *kvm,u64 gen);void kvm_mmu_change_mmu_pages (struct kvm *kvm,unsigned long kvm_nr_mmu_pages); +void free_split_caches (struct kvm *kvm); +int load_pdptrs (struct kvm_vcpu *vcpu,unsigned long cr3); int emulator_write_phys(struct kvm_vcpu *vcpu, gpa_t gpa,
arch/x86/kvm/mmu/mmu.c +262 -7
295 lines not on this page · raw message
arch/x86/kvm/x86.c +6 -0
@@ -12197,6 +12197,12 @@ static void kvm_mmu_slot_apply_flags(struct kvm *kvm,* page faults will create the large-page sptes. */ kvm_mmu_zap_collapsible_sptes (kvm, new); + +/* +* Free any memory left behind by eager page splitting. Ignore +* the module parameter since userspace might have changed it. +*/ +free_split_caches (kvm); }else {/* * Initially-all-set does not require write protecting any page,
Signature · 1 line
2.36.0.550.gb090851708-goog