* [PATCH v2 01/15] KVM: arm64: Combine visitor arguments into a context structure
2022-10-07 23:28 [PATCH v2 00/15] KVM: arm64: Parallel stage-2 fault handling Oliver Upton
@ 2022-10-07 23:28 ` Oliver Upton
2022-10-07 23:28 ` [PATCH v2 02/15] KVM: arm64: Stash observed pte value in visitor context Oliver Upton
` (13 subsequent siblings)
14 siblings, 0 replies; 27+ messages in thread
From: Oliver Upton @ 2022-10-07 23:28 UTC (permalink / raw)
To: Marc Zyngier, James Morse, Alexandru Elisei
Cc: linux-arm-kernel, kvmarm, kvm, Reiji Watanabe, Ricardo Koller,
David Matlack, Quentin Perret, Ben Gardon, Gavin Shan, Peter Xu,
Will Deacon, Sean Christopherson, kvmarm, Oliver Upton
Passing new arguments by value to the visitor callbacks is extremely
inflexible for stuffing new parameters used by only some of the
visitors. Use a context structure instead and pass the pointer through
to the visitor callback.
While at it, redefine the 'flags' parameter to the visitor to contain
the bit indicating the phase of the walk. Pass the entire set of flags
through the context structure such that the walker can communicate
additional state to the visitor callback.
No functional change intended.
Signed-off-by: Oliver Upton <oliver.upton@linux.dev>
---
arch/arm64/include/asm/kvm_pgtable.h | 15 +-
arch/arm64/kvm/hyp/nvhe/mem_protect.c | 10 +-
arch/arm64/kvm/hyp/nvhe/setup.c | 16 +-
arch/arm64/kvm/hyp/pgtable.c | 269 +++++++++++++-------------
4 files changed, 154 insertions(+), 156 deletions(-)
diff --git a/arch/arm64/include/asm/kvm_pgtable.h b/arch/arm64/include/asm/kvm_pgtable.h
index 1b098bd4cd37..829d7167ec87 100644
--- a/arch/arm64/include/asm/kvm_pgtable.h
+++ b/arch/arm64/include/asm/kvm_pgtable.h
@@ -191,10 +191,17 @@ enum kvm_pgtable_walk_flags {
KVM_PGTABLE_WALK_TABLE_POST = BIT(2),
};
-typedef int (*kvm_pgtable_visitor_fn_t)(u64 addr, u64 end, u32 level,
- kvm_pte_t *ptep,
- enum kvm_pgtable_walk_flags flag,
- void * const arg);
+struct kvm_pgtable_visit_ctx {
+ kvm_pte_t *ptep;
+ void * const arg;
+ u64 addr;
+ u64 end;
+ u32 level;
+ enum kvm_pgtable_walk_flags flags;
+};
+
+typedef int (*kvm_pgtable_visitor_fn_t)(const struct kvm_pgtable_visit_ctx *ctx,
+ enum kvm_pgtable_walk_flags visit);
/**
* struct kvm_pgtable_walker - Hook into a page-table walk.
diff --git a/arch/arm64/kvm/hyp/nvhe/mem_protect.c b/arch/arm64/kvm/hyp/nvhe/mem_protect.c
index 1e78acf9662e..8f5b6a36a039 100644
--- a/arch/arm64/kvm/hyp/nvhe/mem_protect.c
+++ b/arch/arm64/kvm/hyp/nvhe/mem_protect.c
@@ -417,13 +417,11 @@ struct check_walk_data {
enum pkvm_page_state (*get_page_state)(kvm_pte_t pte);
};
-static int __check_page_state_visitor(u64 addr, u64 end, u32 level,
- kvm_pte_t *ptep,
- enum kvm_pgtable_walk_flags flag,
- void * const arg)
+static int __check_page_state_visitor(const struct kvm_pgtable_visit_ctx *ctx,
+ enum kvm_pgtable_walk_flags visit)
{
- struct check_walk_data *d = arg;
- kvm_pte_t pte = *ptep;
+ struct check_walk_data *d = ctx->arg;
+ kvm_pte_t pte = *ctx->ptep;
if (kvm_pte_valid(pte) && !addr_is_memory(kvm_pte_to_phys(pte)))
return -EINVAL;
diff --git a/arch/arm64/kvm/hyp/nvhe/setup.c b/arch/arm64/kvm/hyp/nvhe/setup.c
index e8d4ea2fcfa0..a293cf5eba1b 100644
--- a/arch/arm64/kvm/hyp/nvhe/setup.c
+++ b/arch/arm64/kvm/hyp/nvhe/setup.c
@@ -186,15 +186,13 @@ static void hpool_put_page(void *addr)
hyp_put_page(&hpool, addr);
}
-static int finalize_host_mappings_walker(u64 addr, u64 end, u32 level,
- kvm_pte_t *ptep,
- enum kvm_pgtable_walk_flags flag,
- void * const arg)
+static int finalize_host_mappings_walker(const struct kvm_pgtable_visit_ctx *ctx,
+ enum kvm_pgtable_walk_flags visit)
{
- struct kvm_pgtable_mm_ops *mm_ops = arg;
+ struct kvm_pgtable_mm_ops *mm_ops = ctx->arg;
enum kvm_pgtable_prot prot;
enum pkvm_page_state state;
- kvm_pte_t pte = *ptep;
+ kvm_pte_t pte = *ctx->ptep;
phys_addr_t phys;
if (!kvm_pte_valid(pte))
@@ -205,11 +203,11 @@ static int finalize_host_mappings_walker(u64 addr, u64 end, u32 level,
* was unable to access the hyp_vmemmap and so the buddy allocator has
* initialised the refcount to '1'.
*/
- mm_ops->get_page(ptep);
- if (flag != KVM_PGTABLE_WALK_LEAF)
+ mm_ops->get_page(ctx->ptep);
+ if (visit != KVM_PGTABLE_WALK_LEAF)
return 0;
- if (level != (KVM_PGTABLE_MAX_LEVELS - 1))
+ if (ctx->level != (KVM_PGTABLE_MAX_LEVELS - 1))
return -EINVAL;
phys = kvm_pte_to_phys(pte);
diff --git a/arch/arm64/kvm/hyp/pgtable.c b/arch/arm64/kvm/hyp/pgtable.c
index cdf8e76b0be1..900c8b9c0cfc 100644
--- a/arch/arm64/kvm/hyp/pgtable.c
+++ b/arch/arm64/kvm/hyp/pgtable.c
@@ -64,20 +64,20 @@ static bool kvm_phys_is_valid(u64 phys)
return phys < BIT(id_aa64mmfr0_parange_to_phys_shift(ID_AA64MMFR0_EL1_PARANGE_MAX));
}
-static bool kvm_block_mapping_supported(u64 addr, u64 end, u64 phys, u32 level)
+static bool kvm_block_mapping_supported(const struct kvm_pgtable_visit_ctx *ctx, u64 phys)
{
- u64 granule = kvm_granule_size(level);
+ u64 granule = kvm_granule_size(ctx->level);
- if (!kvm_level_supports_block_mapping(level))
+ if (!kvm_level_supports_block_mapping(ctx->level))
return false;
- if (granule > (end - addr))
+ if (granule > (ctx->end - ctx->addr))
return false;
if (kvm_phys_is_valid(phys) && !IS_ALIGNED(phys, granule))
return false;
- return IS_ALIGNED(addr, granule);
+ return IS_ALIGNED(ctx->addr, granule);
}
static u32 kvm_pgtable_idx(struct kvm_pgtable_walk_data *data, u32 level)
@@ -172,12 +172,12 @@ static kvm_pte_t kvm_init_invalid_leaf_owner(u8 owner_id)
return FIELD_PREP(KVM_INVALID_PTE_OWNER_MASK, owner_id);
}
-static int kvm_pgtable_visitor_cb(struct kvm_pgtable_walk_data *data, u64 addr,
- u32 level, kvm_pte_t *ptep,
- enum kvm_pgtable_walk_flags flag)
+static int kvm_pgtable_visitor_cb(struct kvm_pgtable_walk_data *data,
+ const struct kvm_pgtable_visit_ctx *ctx,
+ enum kvm_pgtable_walk_flags visit)
{
struct kvm_pgtable_walker *walker = data->walker;
- return walker->cb(addr, data->end, level, ptep, flag, walker->arg);
+ return walker->cb(ctx, visit);
}
static int __kvm_pgtable_walk(struct kvm_pgtable_walk_data *data,
@@ -186,20 +186,24 @@ static int __kvm_pgtable_walk(struct kvm_pgtable_walk_data *data,
static inline int __kvm_pgtable_visit(struct kvm_pgtable_walk_data *data,
kvm_pte_t *ptep, u32 level)
{
+ enum kvm_pgtable_walk_flags flags = data->walker->flags;
+ struct kvm_pgtable_visit_ctx ctx = {
+ .ptep = ptep,
+ .arg = data->walker->arg,
+ .addr = data->addr,
+ .end = data->end,
+ .level = level,
+ .flags = flags,
+ };
int ret = 0;
- u64 addr = data->addr;
kvm_pte_t *childp, pte = *ptep;
bool table = kvm_pte_table(pte, level);
- enum kvm_pgtable_walk_flags flags = data->walker->flags;
- if (table && (flags & KVM_PGTABLE_WALK_TABLE_PRE)) {
- ret = kvm_pgtable_visitor_cb(data, addr, level, ptep,
- KVM_PGTABLE_WALK_TABLE_PRE);
- }
+ if (table && (ctx.flags & KVM_PGTABLE_WALK_TABLE_PRE))
+ ret = kvm_pgtable_visitor_cb(data, &ctx, KVM_PGTABLE_WALK_TABLE_PRE);
- if (!table && (flags & KVM_PGTABLE_WALK_LEAF)) {
- ret = kvm_pgtable_visitor_cb(data, addr, level, ptep,
- KVM_PGTABLE_WALK_LEAF);
+ if (!table && (ctx.flags & KVM_PGTABLE_WALK_LEAF)) {
+ ret = kvm_pgtable_visitor_cb(data, &ctx, KVM_PGTABLE_WALK_LEAF);
pte = *ptep;
table = kvm_pte_table(pte, level);
}
@@ -218,10 +222,8 @@ static inline int __kvm_pgtable_visit(struct kvm_pgtable_walk_data *data,
if (ret)
goto out;
- if (flags & KVM_PGTABLE_WALK_TABLE_POST) {
- ret = kvm_pgtable_visitor_cb(data, addr, level, ptep,
- KVM_PGTABLE_WALK_TABLE_POST);
- }
+ if (ctx.flags & KVM_PGTABLE_WALK_TABLE_POST)
+ ret = kvm_pgtable_visitor_cb(data, &ctx, KVM_PGTABLE_WALK_TABLE_POST);
out:
return ret;
@@ -292,13 +294,13 @@ struct leaf_walk_data {
u32 level;
};
-static int leaf_walker(u64 addr, u64 end, u32 level, kvm_pte_t *ptep,
- enum kvm_pgtable_walk_flags flag, void * const arg)
+static int leaf_walker(const struct kvm_pgtable_visit_ctx *ctx,
+ enum kvm_pgtable_walk_flags visit)
{
- struct leaf_walk_data *data = arg;
+ struct leaf_walk_data *data = ctx->arg;
- data->pte = *ptep;
- data->level = level;
+ data->pte = *ctx->ptep;
+ data->level = ctx->level;
return 0;
}
@@ -383,47 +385,47 @@ enum kvm_pgtable_prot kvm_pgtable_hyp_pte_prot(kvm_pte_t pte)
return prot;
}
-static bool hyp_map_walker_try_leaf(u64 addr, u64 end, u32 level,
- kvm_pte_t *ptep, struct hyp_map_data *data)
+static bool hyp_map_walker_try_leaf(const struct kvm_pgtable_visit_ctx *ctx,
+ struct hyp_map_data *data)
{
- kvm_pte_t new, old = *ptep;
- u64 granule = kvm_granule_size(level), phys = data->phys;
+ kvm_pte_t new, old = *ctx->ptep;
+ u64 granule = kvm_granule_size(ctx->level), phys = data->phys;
- if (!kvm_block_mapping_supported(addr, end, phys, level))
+ if (!kvm_block_mapping_supported(ctx, phys))
return false;
data->phys += granule;
- new = kvm_init_valid_leaf_pte(phys, data->attr, level);
+ new = kvm_init_valid_leaf_pte(phys, data->attr, ctx->level);
if (old == new)
return true;
if (!kvm_pte_valid(old))
- data->mm_ops->get_page(ptep);
+ data->mm_ops->get_page(ctx->ptep);
else if (WARN_ON((old ^ new) & ~KVM_PTE_LEAF_ATTR_HI_SW))
return false;
- smp_store_release(ptep, new);
+ smp_store_release(ctx->ptep, new);
return true;
}
-static int hyp_map_walker(u64 addr, u64 end, u32 level, kvm_pte_t *ptep,
- enum kvm_pgtable_walk_flags flag, void * const arg)
+static int hyp_map_walker(const struct kvm_pgtable_visit_ctx *ctx,
+ enum kvm_pgtable_walk_flags visit)
{
kvm_pte_t *childp;
- struct hyp_map_data *data = arg;
+ struct hyp_map_data *data = ctx->arg;
struct kvm_pgtable_mm_ops *mm_ops = data->mm_ops;
- if (hyp_map_walker_try_leaf(addr, end, level, ptep, arg))
+ if (hyp_map_walker_try_leaf(ctx, data))
return 0;
- if (WARN_ON(level == KVM_PGTABLE_MAX_LEVELS - 1))
+ if (WARN_ON(ctx->level == KVM_PGTABLE_MAX_LEVELS - 1))
return -EINVAL;
childp = (kvm_pte_t *)mm_ops->zalloc_page(NULL);
if (!childp)
return -ENOMEM;
- kvm_set_table_pte(ptep, childp, mm_ops);
- mm_ops->get_page(ptep);
+ kvm_set_table_pte(ctx->ptep, childp, mm_ops);
+ mm_ops->get_page(ctx->ptep);
return 0;
}
@@ -456,39 +458,39 @@ struct hyp_unmap_data {
struct kvm_pgtable_mm_ops *mm_ops;
};
-static int hyp_unmap_walker(u64 addr, u64 end, u32 level, kvm_pte_t *ptep,
- enum kvm_pgtable_walk_flags flag, void * const arg)
+static int hyp_unmap_walker(const struct kvm_pgtable_visit_ctx *ctx,
+ enum kvm_pgtable_walk_flags visit)
{
- kvm_pte_t pte = *ptep, *childp = NULL;
- u64 granule = kvm_granule_size(level);
- struct hyp_unmap_data *data = arg;
+ kvm_pte_t pte = *ctx->ptep, *childp = NULL;
+ u64 granule = kvm_granule_size(ctx->level);
+ struct hyp_unmap_data *data = ctx->arg;
struct kvm_pgtable_mm_ops *mm_ops = data->mm_ops;
if (!kvm_pte_valid(pte))
return -EINVAL;
- if (kvm_pte_table(pte, level)) {
+ if (kvm_pte_table(pte, ctx->level)) {
childp = kvm_pte_follow(pte, mm_ops);
if (mm_ops->page_count(childp) != 1)
return 0;
- kvm_clear_pte(ptep);
+ kvm_clear_pte(ctx->ptep);
dsb(ishst);
- __tlbi_level(vae2is, __TLBI_VADDR(addr, 0), level);
+ __tlbi_level(vae2is, __TLBI_VADDR(ctx->addr, 0), ctx->level);
} else {
- if (end - addr < granule)
+ if (ctx->end - ctx->addr < granule)
return -EINVAL;
- kvm_clear_pte(ptep);
+ kvm_clear_pte(ctx->ptep);
dsb(ishst);
- __tlbi_level(vale2is, __TLBI_VADDR(addr, 0), level);
+ __tlbi_level(vale2is, __TLBI_VADDR(ctx->addr, 0), ctx->level);
data->unmapped += granule;
}
dsb(ish);
isb();
- mm_ops->put_page(ptep);
+ mm_ops->put_page(ctx->ptep);
if (childp)
mm_ops->put_page(childp);
@@ -532,18 +534,18 @@ int kvm_pgtable_hyp_init(struct kvm_pgtable *pgt, u32 va_bits,
return 0;
}
-static int hyp_free_walker(u64 addr, u64 end, u32 level, kvm_pte_t *ptep,
- enum kvm_pgtable_walk_flags flag, void * const arg)
+static int hyp_free_walker(const struct kvm_pgtable_visit_ctx *ctx,
+ enum kvm_pgtable_walk_flags visit)
{
- struct kvm_pgtable_mm_ops *mm_ops = arg;
- kvm_pte_t pte = *ptep;
+ struct kvm_pgtable_mm_ops *mm_ops = ctx->arg;
+ kvm_pte_t pte = *ctx->ptep;
if (!kvm_pte_valid(pte))
return 0;
- mm_ops->put_page(ptep);
+ mm_ops->put_page(ctx->ptep);
- if (kvm_pte_table(pte, level))
+ if (kvm_pte_table(pte, ctx->level))
mm_ops->put_page(kvm_pte_follow(pte, mm_ops));
return 0;
@@ -682,19 +684,19 @@ static bool stage2_pte_is_counted(kvm_pte_t pte)
return !!pte;
}
-static void stage2_put_pte(kvm_pte_t *ptep, struct kvm_s2_mmu *mmu, u64 addr,
- u32 level, struct kvm_pgtable_mm_ops *mm_ops)
+static void stage2_put_pte(const struct kvm_pgtable_visit_ctx *ctx, struct kvm_s2_mmu *mmu,
+ struct kvm_pgtable_mm_ops *mm_ops)
{
/*
* Clear the existing PTE, and perform break-before-make with
* TLB maintenance if it was valid.
*/
- if (kvm_pte_valid(*ptep)) {
- kvm_clear_pte(ptep);
- kvm_call_hyp(__kvm_tlb_flush_vmid_ipa, mmu, addr, level);
+ if (kvm_pte_valid(*ctx->ptep)) {
+ kvm_clear_pte(ctx->ptep);
+ kvm_call_hyp(__kvm_tlb_flush_vmid_ipa, mmu, ctx->addr, ctx->level);
}
- mm_ops->put_page(ptep);
+ mm_ops->put_page(ctx->ptep);
}
static bool stage2_pte_cacheable(struct kvm_pgtable *pgt, kvm_pte_t pte)
@@ -708,29 +710,28 @@ static bool stage2_pte_executable(kvm_pte_t pte)
return !(pte & KVM_PTE_LEAF_ATTR_HI_S2_XN);
}
-static bool stage2_leaf_mapping_allowed(u64 addr, u64 end, u32 level,
+static bool stage2_leaf_mapping_allowed(const struct kvm_pgtable_visit_ctx *ctx,
struct stage2_map_data *data)
{
- if (data->force_pte && (level < (KVM_PGTABLE_MAX_LEVELS - 1)))
+ if (data->force_pte && (ctx->level < (KVM_PGTABLE_MAX_LEVELS - 1)))
return false;
- return kvm_block_mapping_supported(addr, end, data->phys, level);
+ return kvm_block_mapping_supported(ctx, data->phys);
}
-static int stage2_map_walker_try_leaf(u64 addr, u64 end, u32 level,
- kvm_pte_t *ptep,
+static int stage2_map_walker_try_leaf(const struct kvm_pgtable_visit_ctx *ctx,
struct stage2_map_data *data)
{
- kvm_pte_t new, old = *ptep;
- u64 granule = kvm_granule_size(level), phys = data->phys;
+ kvm_pte_t new, old = *ctx->ptep;
+ u64 granule = kvm_granule_size(ctx->level), phys = data->phys;
struct kvm_pgtable *pgt = data->mmu->pgt;
struct kvm_pgtable_mm_ops *mm_ops = data->mm_ops;
- if (!stage2_leaf_mapping_allowed(addr, end, level, data))
+ if (!stage2_leaf_mapping_allowed(ctx, data))
return -E2BIG;
if (kvm_phys_is_valid(phys))
- new = kvm_init_valid_leaf_pte(phys, data->attr, level);
+ new = kvm_init_valid_leaf_pte(phys, data->attr, ctx->level);
else
new = kvm_init_invalid_leaf_owner(data->owner_id);
@@ -744,7 +745,7 @@ static int stage2_map_walker_try_leaf(u64 addr, u64 end, u32 level,
if (!stage2_pte_needs_update(old, new))
return -EAGAIN;
- stage2_put_pte(ptep, data->mmu, addr, level, mm_ops);
+ stage2_put_pte(ctx, data->mmu, mm_ops);
}
/* Perform CMOs before installation of the guest stage-2 PTE */
@@ -755,26 +756,25 @@ static int stage2_map_walker_try_leaf(u64 addr, u64 end, u32 level,
if (mm_ops->icache_inval_pou && stage2_pte_executable(new))
mm_ops->icache_inval_pou(kvm_pte_follow(new, mm_ops), granule);
- smp_store_release(ptep, new);
+ smp_store_release(ctx->ptep, new);
if (stage2_pte_is_counted(new))
- mm_ops->get_page(ptep);
+ mm_ops->get_page(ctx->ptep);
if (kvm_phys_is_valid(phys))
data->phys += granule;
return 0;
}
-static int stage2_map_walk_table_pre(u64 addr, u64 end, u32 level,
- kvm_pte_t *ptep,
+static int stage2_map_walk_table_pre(const struct kvm_pgtable_visit_ctx *ctx,
struct stage2_map_data *data)
{
if (data->anchor)
return 0;
- if (!stage2_leaf_mapping_allowed(addr, end, level, data))
+ if (!stage2_leaf_mapping_allowed(ctx, data))
return 0;
- data->childp = kvm_pte_follow(*ptep, data->mm_ops);
- kvm_clear_pte(ptep);
+ data->childp = kvm_pte_follow(*ctx->ptep, data->mm_ops);
+ kvm_clear_pte(ctx->ptep);
/*
* Invalidate the whole stage-2, as we may have numerous leaf
@@ -782,29 +782,29 @@ static int stage2_map_walk_table_pre(u64 addr, u64 end, u32 level,
* individually.
*/
kvm_call_hyp(__kvm_tlb_flush_vmid, data->mmu);
- data->anchor = ptep;
+ data->anchor = ctx->ptep;
return 0;
}
-static int stage2_map_walk_leaf(u64 addr, u64 end, u32 level, kvm_pte_t *ptep,
+static int stage2_map_walk_leaf(const struct kvm_pgtable_visit_ctx *ctx,
struct stage2_map_data *data)
{
struct kvm_pgtable_mm_ops *mm_ops = data->mm_ops;
- kvm_pte_t *childp, pte = *ptep;
+ kvm_pte_t *childp, pte = *ctx->ptep;
int ret;
if (data->anchor) {
if (stage2_pte_is_counted(pte))
- mm_ops->put_page(ptep);
+ mm_ops->put_page(ctx->ptep);
return 0;
}
- ret = stage2_map_walker_try_leaf(addr, end, level, ptep, data);
+ ret = stage2_map_walker_try_leaf(ctx, data);
if (ret != -E2BIG)
return ret;
- if (WARN_ON(level == KVM_PGTABLE_MAX_LEVELS - 1))
+ if (WARN_ON(ctx->level == KVM_PGTABLE_MAX_LEVELS - 1))
return -EINVAL;
if (!data->memcache)
@@ -820,16 +820,15 @@ static int stage2_map_walk_leaf(u64 addr, u64 end, u32 level, kvm_pte_t *ptep,
* will be mapped lazily.
*/
if (stage2_pte_is_counted(pte))
- stage2_put_pte(ptep, data->mmu, addr, level, mm_ops);
+ stage2_put_pte(ctx, data->mmu, mm_ops);
- kvm_set_table_pte(ptep, childp, mm_ops);
- mm_ops->get_page(ptep);
+ kvm_set_table_pte(ctx->ptep, childp, mm_ops);
+ mm_ops->get_page(ctx->ptep);
return 0;
}
-static int stage2_map_walk_table_post(u64 addr, u64 end, u32 level,
- kvm_pte_t *ptep,
+static int stage2_map_walk_table_post(const struct kvm_pgtable_visit_ctx *ctx,
struct stage2_map_data *data)
{
struct kvm_pgtable_mm_ops *mm_ops = data->mm_ops;
@@ -839,17 +838,17 @@ static int stage2_map_walk_table_post(u64 addr, u64 end, u32 level,
if (!data->anchor)
return 0;
- if (data->anchor == ptep) {
+ if (data->anchor == ctx->ptep) {
childp = data->childp;
data->anchor = NULL;
data->childp = NULL;
- ret = stage2_map_walk_leaf(addr, end, level, ptep, data);
+ ret = stage2_map_walk_leaf(ctx, data);
} else {
- childp = kvm_pte_follow(*ptep, mm_ops);
+ childp = kvm_pte_follow(*ctx->ptep, mm_ops);
}
mm_ops->put_page(childp);
- mm_ops->put_page(ptep);
+ mm_ops->put_page(ctx->ptep);
return ret;
}
@@ -873,18 +872,18 @@ static int stage2_map_walk_table_post(u64 addr, u64 end, u32 level,
* the page-table, installing the block entry when it revisits the anchor
* pointer and clearing the anchor to NULL.
*/
-static int stage2_map_walker(u64 addr, u64 end, u32 level, kvm_pte_t *ptep,
- enum kvm_pgtable_walk_flags flag, void * const arg)
+static int stage2_map_walker(const struct kvm_pgtable_visit_ctx *ctx,
+ enum kvm_pgtable_walk_flags visit)
{
- struct stage2_map_data *data = arg;
+ struct stage2_map_data *data = ctx->arg;
- switch (flag) {
+ switch (visit) {
case KVM_PGTABLE_WALK_TABLE_PRE:
- return stage2_map_walk_table_pre(addr, end, level, ptep, data);
+ return stage2_map_walk_table_pre(ctx, data);
case KVM_PGTABLE_WALK_LEAF:
- return stage2_map_walk_leaf(addr, end, level, ptep, data);
+ return stage2_map_walk_leaf(ctx, data);
case KVM_PGTABLE_WALK_TABLE_POST:
- return stage2_map_walk_table_post(addr, end, level, ptep, data);
+ return stage2_map_walk_table_post(ctx, data);
}
return -EINVAL;
@@ -949,25 +948,24 @@ int kvm_pgtable_stage2_set_owner(struct kvm_pgtable *pgt, u64 addr, u64 size,
return ret;
}
-static int stage2_unmap_walker(u64 addr, u64 end, u32 level, kvm_pte_t *ptep,
- enum kvm_pgtable_walk_flags flag,
- void * const arg)
+static int stage2_unmap_walker(const struct kvm_pgtable_visit_ctx *ctx,
+ enum kvm_pgtable_walk_flags visit)
{
- struct kvm_pgtable *pgt = arg;
+ struct kvm_pgtable *pgt = ctx->arg;
struct kvm_s2_mmu *mmu = pgt->mmu;
struct kvm_pgtable_mm_ops *mm_ops = pgt->mm_ops;
- kvm_pte_t pte = *ptep, *childp = NULL;
+ kvm_pte_t pte = *ctx->ptep, *childp = NULL;
bool need_flush = false;
if (!kvm_pte_valid(pte)) {
if (stage2_pte_is_counted(pte)) {
- kvm_clear_pte(ptep);
- mm_ops->put_page(ptep);
+ kvm_clear_pte(ctx->ptep);
+ mm_ops->put_page(ctx->ptep);
}
return 0;
}
- if (kvm_pte_table(pte, level)) {
+ if (kvm_pte_table(pte, ctx->level)) {
childp = kvm_pte_follow(pte, mm_ops);
if (mm_ops->page_count(childp) != 1)
@@ -981,11 +979,11 @@ static int stage2_unmap_walker(u64 addr, u64 end, u32 level, kvm_pte_t *ptep,
* block entry and rely on the remaining portions being faulted
* back lazily.
*/
- stage2_put_pte(ptep, mmu, addr, level, mm_ops);
+ stage2_put_pte(ctx, mmu, mm_ops);
if (need_flush && mm_ops->dcache_clean_inval_poc)
mm_ops->dcache_clean_inval_poc(kvm_pte_follow(pte, mm_ops),
- kvm_granule_size(level));
+ kvm_granule_size(ctx->level));
if (childp)
mm_ops->put_page(childp);
@@ -1012,18 +1010,17 @@ struct stage2_attr_data {
struct kvm_pgtable_mm_ops *mm_ops;
};
-static int stage2_attr_walker(u64 addr, u64 end, u32 level, kvm_pte_t *ptep,
- enum kvm_pgtable_walk_flags flag,
- void * const arg)
+static int stage2_attr_walker(const struct kvm_pgtable_visit_ctx *ctx,
+ enum kvm_pgtable_walk_flags visit)
{
- kvm_pte_t pte = *ptep;
- struct stage2_attr_data *data = arg;
+ kvm_pte_t pte = *ctx->ptep;
+ struct stage2_attr_data *data = ctx->arg;
struct kvm_pgtable_mm_ops *mm_ops = data->mm_ops;
if (!kvm_pte_valid(pte))
return 0;
- data->level = level;
+ data->level = ctx->level;
data->pte = pte;
pte &= ~data->attr_clr;
pte |= data->attr_set;
@@ -1039,10 +1036,10 @@ static int stage2_attr_walker(u64 addr, u64 end, u32 level, kvm_pte_t *ptep,
* stage-2 PTE if we are going to add executable permission.
*/
if (mm_ops->icache_inval_pou &&
- stage2_pte_executable(pte) && !stage2_pte_executable(*ptep))
+ stage2_pte_executable(pte) && !stage2_pte_executable(*ctx->ptep))
mm_ops->icache_inval_pou(kvm_pte_follow(pte, mm_ops),
- kvm_granule_size(level));
- WRITE_ONCE(*ptep, pte);
+ kvm_granule_size(ctx->level));
+ WRITE_ONCE(*ctx->ptep, pte);
}
return 0;
@@ -1140,20 +1137,19 @@ int kvm_pgtable_stage2_relax_perms(struct kvm_pgtable *pgt, u64 addr,
return ret;
}
-static int stage2_flush_walker(u64 addr, u64 end, u32 level, kvm_pte_t *ptep,
- enum kvm_pgtable_walk_flags flag,
- void * const arg)
+static int stage2_flush_walker(const struct kvm_pgtable_visit_ctx *ctx,
+ enum kvm_pgtable_walk_flags visit)
{
- struct kvm_pgtable *pgt = arg;
+ struct kvm_pgtable *pgt = ctx->arg;
struct kvm_pgtable_mm_ops *mm_ops = pgt->mm_ops;
- kvm_pte_t pte = *ptep;
+ kvm_pte_t pte = *ctx->ptep;
if (!kvm_pte_valid(pte) || !stage2_pte_cacheable(pgt, pte))
return 0;
if (mm_ops->dcache_clean_inval_poc)
mm_ops->dcache_clean_inval_poc(kvm_pte_follow(pte, mm_ops),
- kvm_granule_size(level));
+ kvm_granule_size(ctx->level));
return 0;
}
@@ -1200,19 +1196,18 @@ int __kvm_pgtable_stage2_init(struct kvm_pgtable *pgt, struct kvm_s2_mmu *mmu,
return 0;
}
-static int stage2_free_walker(u64 addr, u64 end, u32 level, kvm_pte_t *ptep,
- enum kvm_pgtable_walk_flags flag,
- void * const arg)
+static int stage2_free_walker(const struct kvm_pgtable_visit_ctx *ctx,
+ enum kvm_pgtable_walk_flags visit)
{
- struct kvm_pgtable_mm_ops *mm_ops = arg;
- kvm_pte_t pte = *ptep;
+ struct kvm_pgtable_mm_ops *mm_ops = ctx->arg;
+ kvm_pte_t pte = *ctx->ptep;
if (!stage2_pte_is_counted(pte))
return 0;
- mm_ops->put_page(ptep);
+ mm_ops->put_page(ctx->ptep);
- if (kvm_pte_table(pte, level))
+ if (kvm_pte_table(pte, ctx->level))
mm_ops->put_page(kvm_pte_follow(pte, mm_ops));
return 0;
--
2.38.0.rc1.362.ged0d419d3c-goog
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 27+ messages in thread* [PATCH v2 02/15] KVM: arm64: Stash observed pte value in visitor context
2022-10-07 23:28 [PATCH v2 00/15] KVM: arm64: Parallel stage-2 fault handling Oliver Upton
2022-10-07 23:28 ` [PATCH v2 01/15] KVM: arm64: Combine visitor arguments into a context structure Oliver Upton
@ 2022-10-07 23:28 ` Oliver Upton
2022-10-07 23:28 ` [PATCH v2 03/15] KVM: arm64: Pass mm_ops through the " Oliver Upton
` (12 subsequent siblings)
14 siblings, 0 replies; 27+ messages in thread
From: Oliver Upton @ 2022-10-07 23:28 UTC (permalink / raw)
To: Marc Zyngier, James Morse, Alexandru Elisei
Cc: linux-arm-kernel, kvmarm, kvm, Reiji Watanabe, Ricardo Koller,
David Matlack, Quentin Perret, Ben Gardon, Gavin Shan, Peter Xu,
Will Deacon, Sean Christopherson, kvmarm, Oliver Upton
Rather than reading the ptep all over the shop, read the ptep once from
__kvm_pgtable_visit() and stick it in the visitor context. Reread the
ptep after visiting a leaf in case the callback installed a new table
underneath.
No functional change intended.
Signed-off-by: Oliver Upton <oliver.upton@linux.dev>
---
arch/arm64/include/asm/kvm_pgtable.h | 1 +
arch/arm64/kvm/hyp/nvhe/mem_protect.c | 5 +-
arch/arm64/kvm/hyp/nvhe/setup.c | 7 +--
arch/arm64/kvm/hyp/pgtable.c | 86 +++++++++++++--------------
4 files changed, 48 insertions(+), 51 deletions(-)
diff --git a/arch/arm64/include/asm/kvm_pgtable.h b/arch/arm64/include/asm/kvm_pgtable.h
index 829d7167ec87..38bf9f4c5906 100644
--- a/arch/arm64/include/asm/kvm_pgtable.h
+++ b/arch/arm64/include/asm/kvm_pgtable.h
@@ -193,6 +193,7 @@ enum kvm_pgtable_walk_flags {
struct kvm_pgtable_visit_ctx {
kvm_pte_t *ptep;
+ kvm_pte_t old;
void * const arg;
u64 addr;
u64 end;
diff --git a/arch/arm64/kvm/hyp/nvhe/mem_protect.c b/arch/arm64/kvm/hyp/nvhe/mem_protect.c
index 8f5b6a36a039..d21d1b08a055 100644
--- a/arch/arm64/kvm/hyp/nvhe/mem_protect.c
+++ b/arch/arm64/kvm/hyp/nvhe/mem_protect.c
@@ -421,12 +421,11 @@ static int __check_page_state_visitor(const struct kvm_pgtable_visit_ctx *ctx,
enum kvm_pgtable_walk_flags visit)
{
struct check_walk_data *d = ctx->arg;
- kvm_pte_t pte = *ctx->ptep;
- if (kvm_pte_valid(pte) && !addr_is_memory(kvm_pte_to_phys(pte)))
+ if (kvm_pte_valid(ctx->old) && !addr_is_memory(kvm_pte_to_phys(ctx->old)))
return -EINVAL;
- return d->get_page_state(pte) == d->desired ? 0 : -EPERM;
+ return d->get_page_state(ctx->old) == d->desired ? 0 : -EPERM;
}
static int check_page_state_range(struct kvm_pgtable *pgt, u64 addr, u64 size,
diff --git a/arch/arm64/kvm/hyp/nvhe/setup.c b/arch/arm64/kvm/hyp/nvhe/setup.c
index a293cf5eba1b..6af443c9d78e 100644
--- a/arch/arm64/kvm/hyp/nvhe/setup.c
+++ b/arch/arm64/kvm/hyp/nvhe/setup.c
@@ -192,10 +192,9 @@ static int finalize_host_mappings_walker(const struct kvm_pgtable_visit_ctx *ctx
struct kvm_pgtable_mm_ops *mm_ops = ctx->arg;
enum kvm_pgtable_prot prot;
enum pkvm_page_state state;
- kvm_pte_t pte = *ctx->ptep;
phys_addr_t phys;
- if (!kvm_pte_valid(pte))
+ if (!kvm_pte_valid(ctx->old))
return 0;
/*
@@ -210,7 +209,7 @@ static int finalize_host_mappings_walker(const struct kvm_pgtable_visit_ctx *ctx
if (ctx->level != (KVM_PGTABLE_MAX_LEVELS - 1))
return -EINVAL;
- phys = kvm_pte_to_phys(pte);
+ phys = kvm_pte_to_phys(ctx->old);
if (!addr_is_memory(phys))
return -EINVAL;
@@ -218,7 +217,7 @@ static int finalize_host_mappings_walker(const struct kvm_pgtable_visit_ctx *ctx
* Adjust the host stage-2 mappings to match the ownership attributes
* configured in the hypervisor stage-1.
*/
- state = pkvm_getstate(kvm_pgtable_hyp_pte_prot(pte));
+ state = pkvm_getstate(kvm_pgtable_hyp_pte_prot(ctx->old));
switch (state) {
case PKVM_PAGE_OWNED:
return host_stage2_set_owner_locked(phys, PAGE_SIZE, pkvm_hyp_id);
diff --git a/arch/arm64/kvm/hyp/pgtable.c b/arch/arm64/kvm/hyp/pgtable.c
index 900c8b9c0cfc..fb3696b3a997 100644
--- a/arch/arm64/kvm/hyp/pgtable.c
+++ b/arch/arm64/kvm/hyp/pgtable.c
@@ -189,6 +189,7 @@ static inline int __kvm_pgtable_visit(struct kvm_pgtable_walk_data *data,
enum kvm_pgtable_walk_flags flags = data->walker->flags;
struct kvm_pgtable_visit_ctx ctx = {
.ptep = ptep,
+ .old = READ_ONCE(*ptep),
.arg = data->walker->arg,
.addr = data->addr,
.end = data->end,
@@ -196,16 +197,16 @@ static inline int __kvm_pgtable_visit(struct kvm_pgtable_walk_data *data,
.flags = flags,
};
int ret = 0;
- kvm_pte_t *childp, pte = *ptep;
- bool table = kvm_pte_table(pte, level);
+ kvm_pte_t *childp;
+ bool table = kvm_pte_table(ctx.old, level);
if (table && (ctx.flags & KVM_PGTABLE_WALK_TABLE_PRE))
ret = kvm_pgtable_visitor_cb(data, &ctx, KVM_PGTABLE_WALK_TABLE_PRE);
if (!table && (ctx.flags & KVM_PGTABLE_WALK_LEAF)) {
ret = kvm_pgtable_visitor_cb(data, &ctx, KVM_PGTABLE_WALK_LEAF);
- pte = *ptep;
- table = kvm_pte_table(pte, level);
+ ctx.old = READ_ONCE(*ptep);
+ table = kvm_pte_table(ctx.old, level);
}
if (ret)
@@ -217,7 +218,7 @@ static inline int __kvm_pgtable_visit(struct kvm_pgtable_walk_data *data,
goto out;
}
- childp = kvm_pte_follow(pte, data->pgt->mm_ops);
+ childp = kvm_pte_follow(ctx.old, data->pgt->mm_ops);
ret = __kvm_pgtable_walk(data, childp, level + 1);
if (ret)
goto out;
@@ -299,7 +300,7 @@ static int leaf_walker(const struct kvm_pgtable_visit_ctx *ctx,
{
struct leaf_walk_data *data = ctx->arg;
- data->pte = *ctx->ptep;
+ data->pte = ctx->old;
data->level = ctx->level;
return 0;
@@ -388,7 +389,7 @@ enum kvm_pgtable_prot kvm_pgtable_hyp_pte_prot(kvm_pte_t pte)
static bool hyp_map_walker_try_leaf(const struct kvm_pgtable_visit_ctx *ctx,
struct hyp_map_data *data)
{
- kvm_pte_t new, old = *ctx->ptep;
+ kvm_pte_t new;
u64 granule = kvm_granule_size(ctx->level), phys = data->phys;
if (!kvm_block_mapping_supported(ctx, phys))
@@ -396,11 +397,11 @@ static bool hyp_map_walker_try_leaf(const struct kvm_pgtable_visit_ctx *ctx,
data->phys += granule;
new = kvm_init_valid_leaf_pte(phys, data->attr, ctx->level);
- if (old == new)
+ if (ctx->old == new)
return true;
- if (!kvm_pte_valid(old))
+ if (!kvm_pte_valid(ctx->old))
data->mm_ops->get_page(ctx->ptep);
- else if (WARN_ON((old ^ new) & ~KVM_PTE_LEAF_ATTR_HI_SW))
+ else if (WARN_ON((ctx->old ^ new) & ~KVM_PTE_LEAF_ATTR_HI_SW))
return false;
smp_store_release(ctx->ptep, new);
@@ -461,16 +462,16 @@ struct hyp_unmap_data {
static int hyp_unmap_walker(const struct kvm_pgtable_visit_ctx *ctx,
enum kvm_pgtable_walk_flags visit)
{
- kvm_pte_t pte = *ctx->ptep, *childp = NULL;
+ kvm_pte_t *childp = NULL;
u64 granule = kvm_granule_size(ctx->level);
struct hyp_unmap_data *data = ctx->arg;
struct kvm_pgtable_mm_ops *mm_ops = data->mm_ops;
- if (!kvm_pte_valid(pte))
+ if (!kvm_pte_valid(ctx->old))
return -EINVAL;
- if (kvm_pte_table(pte, ctx->level)) {
- childp = kvm_pte_follow(pte, mm_ops);
+ if (kvm_pte_table(ctx->old, ctx->level)) {
+ childp = kvm_pte_follow(ctx->old, mm_ops);
if (mm_ops->page_count(childp) != 1)
return 0;
@@ -538,15 +539,14 @@ static int hyp_free_walker(const struct kvm_pgtable_visit_ctx *ctx,
enum kvm_pgtable_walk_flags visit)
{
struct kvm_pgtable_mm_ops *mm_ops = ctx->arg;
- kvm_pte_t pte = *ctx->ptep;
- if (!kvm_pte_valid(pte))
+ if (!kvm_pte_valid(ctx->old))
return 0;
mm_ops->put_page(ctx->ptep);
- if (kvm_pte_table(pte, ctx->level))
- mm_ops->put_page(kvm_pte_follow(pte, mm_ops));
+ if (kvm_pte_table(ctx->old, ctx->level))
+ mm_ops->put_page(kvm_pte_follow(ctx->old, mm_ops));
return 0;
}
@@ -691,7 +691,7 @@ static void stage2_put_pte(const struct kvm_pgtable_visit_ctx *ctx, struct kvm_s
* Clear the existing PTE, and perform break-before-make with
* TLB maintenance if it was valid.
*/
- if (kvm_pte_valid(*ctx->ptep)) {
+ if (kvm_pte_valid(ctx->old)) {
kvm_clear_pte(ctx->ptep);
kvm_call_hyp(__kvm_tlb_flush_vmid_ipa, mmu, ctx->addr, ctx->level);
}
@@ -722,7 +722,7 @@ static bool stage2_leaf_mapping_allowed(const struct kvm_pgtable_visit_ctx *ctx,
static int stage2_map_walker_try_leaf(const struct kvm_pgtable_visit_ctx *ctx,
struct stage2_map_data *data)
{
- kvm_pte_t new, old = *ctx->ptep;
+ kvm_pte_t new;
u64 granule = kvm_granule_size(ctx->level), phys = data->phys;
struct kvm_pgtable *pgt = data->mmu->pgt;
struct kvm_pgtable_mm_ops *mm_ops = data->mm_ops;
@@ -735,14 +735,14 @@ static int stage2_map_walker_try_leaf(const struct kvm_pgtable_visit_ctx *ctx,
else
new = kvm_init_invalid_leaf_owner(data->owner_id);
- if (stage2_pte_is_counted(old)) {
+ if (stage2_pte_is_counted(ctx->old)) {
/*
* Skip updating the PTE if we are trying to recreate the exact
* same mapping or only change the access permissions. Instead,
* the vCPU will exit one more time from guest if still needed
* and then go through the path of relaxing permissions.
*/
- if (!stage2_pte_needs_update(old, new))
+ if (!stage2_pte_needs_update(ctx->old, new))
return -EAGAIN;
stage2_put_pte(ctx, data->mmu, mm_ops);
@@ -773,7 +773,7 @@ static int stage2_map_walk_table_pre(const struct kvm_pgtable_visit_ctx *ctx,
if (!stage2_leaf_mapping_allowed(ctx, data))
return 0;
- data->childp = kvm_pte_follow(*ctx->ptep, data->mm_ops);
+ data->childp = kvm_pte_follow(ctx->old, data->mm_ops);
kvm_clear_pte(ctx->ptep);
/*
@@ -790,11 +790,11 @@ static int stage2_map_walk_leaf(const struct kvm_pgtable_visit_ctx *ctx,
struct stage2_map_data *data)
{
struct kvm_pgtable_mm_ops *mm_ops = data->mm_ops;
- kvm_pte_t *childp, pte = *ctx->ptep;
+ kvm_pte_t *childp;
int ret;
if (data->anchor) {
- if (stage2_pte_is_counted(pte))
+ if (stage2_pte_is_counted(ctx->old))
mm_ops->put_page(ctx->ptep);
return 0;
@@ -819,7 +819,7 @@ static int stage2_map_walk_leaf(const struct kvm_pgtable_visit_ctx *ctx,
* a table. Accesses beyond 'end' that fall within the new table
* will be mapped lazily.
*/
- if (stage2_pte_is_counted(pte))
+ if (stage2_pte_is_counted(ctx->old))
stage2_put_pte(ctx, data->mmu, mm_ops);
kvm_set_table_pte(ctx->ptep, childp, mm_ops);
@@ -844,7 +844,7 @@ static int stage2_map_walk_table_post(const struct kvm_pgtable_visit_ctx *ctx,
data->childp = NULL;
ret = stage2_map_walk_leaf(ctx, data);
} else {
- childp = kvm_pte_follow(*ctx->ptep, mm_ops);
+ childp = kvm_pte_follow(ctx->old, mm_ops);
}
mm_ops->put_page(childp);
@@ -954,23 +954,23 @@ static int stage2_unmap_walker(const struct kvm_pgtable_visit_ctx *ctx,
struct kvm_pgtable *pgt = ctx->arg;
struct kvm_s2_mmu *mmu = pgt->mmu;
struct kvm_pgtable_mm_ops *mm_ops = pgt->mm_ops;
- kvm_pte_t pte = *ctx->ptep, *childp = NULL;
+ kvm_pte_t *childp = NULL;
bool need_flush = false;
- if (!kvm_pte_valid(pte)) {
- if (stage2_pte_is_counted(pte)) {
+ if (!kvm_pte_valid(ctx->old)) {
+ if (stage2_pte_is_counted(ctx->old)) {
kvm_clear_pte(ctx->ptep);
mm_ops->put_page(ctx->ptep);
}
return 0;
}
- if (kvm_pte_table(pte, ctx->level)) {
- childp = kvm_pte_follow(pte, mm_ops);
+ if (kvm_pte_table(ctx->old, ctx->level)) {
+ childp = kvm_pte_follow(ctx->old, mm_ops);
if (mm_ops->page_count(childp) != 1)
return 0;
- } else if (stage2_pte_cacheable(pgt, pte)) {
+ } else if (stage2_pte_cacheable(pgt, ctx->old)) {
need_flush = !stage2_has_fwb(pgt);
}
@@ -982,7 +982,7 @@ static int stage2_unmap_walker(const struct kvm_pgtable_visit_ctx *ctx,
stage2_put_pte(ctx, mmu, mm_ops);
if (need_flush && mm_ops->dcache_clean_inval_poc)
- mm_ops->dcache_clean_inval_poc(kvm_pte_follow(pte, mm_ops),
+ mm_ops->dcache_clean_inval_poc(kvm_pte_follow(ctx->old, mm_ops),
kvm_granule_size(ctx->level));
if (childp)
@@ -1013,11 +1013,11 @@ struct stage2_attr_data {
static int stage2_attr_walker(const struct kvm_pgtable_visit_ctx *ctx,
enum kvm_pgtable_walk_flags visit)
{
- kvm_pte_t pte = *ctx->ptep;
+ kvm_pte_t pte = ctx->old;
struct stage2_attr_data *data = ctx->arg;
struct kvm_pgtable_mm_ops *mm_ops = data->mm_ops;
- if (!kvm_pte_valid(pte))
+ if (!kvm_pte_valid(ctx->old))
return 0;
data->level = ctx->level;
@@ -1036,7 +1036,7 @@ static int stage2_attr_walker(const struct kvm_pgtable_visit_ctx *ctx,
* stage-2 PTE if we are going to add executable permission.
*/
if (mm_ops->icache_inval_pou &&
- stage2_pte_executable(pte) && !stage2_pte_executable(*ctx->ptep))
+ stage2_pte_executable(pte) && !stage2_pte_executable(ctx->old))
mm_ops->icache_inval_pou(kvm_pte_follow(pte, mm_ops),
kvm_granule_size(ctx->level));
WRITE_ONCE(*ctx->ptep, pte);
@@ -1142,13 +1142,12 @@ static int stage2_flush_walker(const struct kvm_pgtable_visit_ctx *ctx,
{
struct kvm_pgtable *pgt = ctx->arg;
struct kvm_pgtable_mm_ops *mm_ops = pgt->mm_ops;
- kvm_pte_t pte = *ctx->ptep;
- if (!kvm_pte_valid(pte) || !stage2_pte_cacheable(pgt, pte))
+ if (!kvm_pte_valid(ctx->old) || !stage2_pte_cacheable(pgt, ctx->old))
return 0;
if (mm_ops->dcache_clean_inval_poc)
- mm_ops->dcache_clean_inval_poc(kvm_pte_follow(pte, mm_ops),
+ mm_ops->dcache_clean_inval_poc(kvm_pte_follow(ctx->old, mm_ops),
kvm_granule_size(ctx->level));
return 0;
}
@@ -1200,15 +1199,14 @@ static int stage2_free_walker(const struct kvm_pgtable_visit_ctx *ctx,
enum kvm_pgtable_walk_flags visit)
{
struct kvm_pgtable_mm_ops *mm_ops = ctx->arg;
- kvm_pte_t pte = *ctx->ptep;
- if (!stage2_pte_is_counted(pte))
+ if (!stage2_pte_is_counted(ctx->old))
return 0;
mm_ops->put_page(ctx->ptep);
- if (kvm_pte_table(pte, ctx->level))
- mm_ops->put_page(kvm_pte_follow(pte, mm_ops));
+ if (kvm_pte_table(ctx->old, ctx->level))
+ mm_ops->put_page(kvm_pte_follow(ctx->old, mm_ops));
return 0;
}
--
2.38.0.rc1.362.ged0d419d3c-goog
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 27+ messages in thread* [PATCH v2 03/15] KVM: arm64: Pass mm_ops through the visitor context
2022-10-07 23:28 [PATCH v2 00/15] KVM: arm64: Parallel stage-2 fault handling Oliver Upton
2022-10-07 23:28 ` [PATCH v2 01/15] KVM: arm64: Combine visitor arguments into a context structure Oliver Upton
2022-10-07 23:28 ` [PATCH v2 02/15] KVM: arm64: Stash observed pte value in visitor context Oliver Upton
@ 2022-10-07 23:28 ` Oliver Upton
2022-10-07 23:28 ` [PATCH v2 04/15] KVM: arm64: Don't pass kvm_pgtable through kvm_pgtable_walk_data Oliver Upton
` (11 subsequent siblings)
14 siblings, 0 replies; 27+ messages in thread
From: Oliver Upton @ 2022-10-07 23:28 UTC (permalink / raw)
To: Marc Zyngier, James Morse, Alexandru Elisei
Cc: linux-arm-kernel, kvmarm, kvm, Reiji Watanabe, Ricardo Koller,
David Matlack, Quentin Perret, Ben Gardon, Gavin Shan, Peter Xu,
Will Deacon, Sean Christopherson, kvmarm, Oliver Upton
As a prerequisite for getting visitors off of struct kvm_pgtable, pass
mm_ops through the visitor context.
No functional change intended.
Signed-off-by: Oliver Upton <oliver.upton@linux.dev>
---
arch/arm64/include/asm/kvm_pgtable.h | 1 +
arch/arm64/kvm/hyp/nvhe/setup.c | 3 +-
arch/arm64/kvm/hyp/pgtable.c | 63 +++++++++++-----------------
3 files changed, 26 insertions(+), 41 deletions(-)
diff --git a/arch/arm64/include/asm/kvm_pgtable.h b/arch/arm64/include/asm/kvm_pgtable.h
index 38bf9f4c5906..51acdef1b80a 100644
--- a/arch/arm64/include/asm/kvm_pgtable.h
+++ b/arch/arm64/include/asm/kvm_pgtable.h
@@ -195,6 +195,7 @@ struct kvm_pgtable_visit_ctx {
kvm_pte_t *ptep;
kvm_pte_t old;
void * const arg;
+ struct kvm_pgtable_mm_ops *mm_ops;
u64 addr;
u64 end;
u32 level;
diff --git a/arch/arm64/kvm/hyp/nvhe/setup.c b/arch/arm64/kvm/hyp/nvhe/setup.c
index 6af443c9d78e..1068338d77f3 100644
--- a/arch/arm64/kvm/hyp/nvhe/setup.c
+++ b/arch/arm64/kvm/hyp/nvhe/setup.c
@@ -189,7 +189,7 @@ static void hpool_put_page(void *addr)
static int finalize_host_mappings_walker(const struct kvm_pgtable_visit_ctx *ctx,
enum kvm_pgtable_walk_flags visit)
{
- struct kvm_pgtable_mm_ops *mm_ops = ctx->arg;
+ struct kvm_pgtable_mm_ops *mm_ops = ctx->mm_ops;
enum kvm_pgtable_prot prot;
enum pkvm_page_state state;
phys_addr_t phys;
@@ -239,7 +239,6 @@ static int finalize_host_mappings(void)
struct kvm_pgtable_walker walker = {
.cb = finalize_host_mappings_walker,
.flags = KVM_PGTABLE_WALK_LEAF | KVM_PGTABLE_WALK_TABLE_POST,
- .arg = pkvm_pgtable.mm_ops,
};
int i, ret;
diff --git a/arch/arm64/kvm/hyp/pgtable.c b/arch/arm64/kvm/hyp/pgtable.c
index fb3696b3a997..db25e81a9890 100644
--- a/arch/arm64/kvm/hyp/pgtable.c
+++ b/arch/arm64/kvm/hyp/pgtable.c
@@ -181,9 +181,10 @@ static int kvm_pgtable_visitor_cb(struct kvm_pgtable_walk_data *data,
}
static int __kvm_pgtable_walk(struct kvm_pgtable_walk_data *data,
- kvm_pte_t *pgtable, u32 level);
+ struct kvm_pgtable_mm_ops *mm_ops, kvm_pte_t *pgtable, u32 level);
static inline int __kvm_pgtable_visit(struct kvm_pgtable_walk_data *data,
+ struct kvm_pgtable_mm_ops *mm_ops,
kvm_pte_t *ptep, u32 level)
{
enum kvm_pgtable_walk_flags flags = data->walker->flags;
@@ -191,6 +192,7 @@ static inline int __kvm_pgtable_visit(struct kvm_pgtable_walk_data *data,
.ptep = ptep,
.old = READ_ONCE(*ptep),
.arg = data->walker->arg,
+ .mm_ops = mm_ops,
.addr = data->addr,
.end = data->end,
.level = level,
@@ -218,8 +220,8 @@ static inline int __kvm_pgtable_visit(struct kvm_pgtable_walk_data *data,
goto out;
}
- childp = kvm_pte_follow(ctx.old, data->pgt->mm_ops);
- ret = __kvm_pgtable_walk(data, childp, level + 1);
+ childp = kvm_pte_follow(ctx.old, mm_ops);
+ ret = __kvm_pgtable_walk(data, mm_ops, childp, level + 1);
if (ret)
goto out;
@@ -231,7 +233,7 @@ static inline int __kvm_pgtable_visit(struct kvm_pgtable_walk_data *data,
}
static int __kvm_pgtable_walk(struct kvm_pgtable_walk_data *data,
- kvm_pte_t *pgtable, u32 level)
+ struct kvm_pgtable_mm_ops *mm_ops, kvm_pte_t *pgtable, u32 level)
{
u32 idx;
int ret = 0;
@@ -245,7 +247,7 @@ static int __kvm_pgtable_walk(struct kvm_pgtable_walk_data *data,
if (data->addr >= data->end)
break;
- ret = __kvm_pgtable_visit(data, ptep, level);
+ ret = __kvm_pgtable_visit(data, mm_ops, ptep, level);
if (ret)
break;
}
@@ -269,7 +271,7 @@ static int _kvm_pgtable_walk(struct kvm_pgtable_walk_data *data)
for (idx = kvm_pgd_page_idx(data); data->addr < data->end; ++idx) {
kvm_pte_t *ptep = &pgt->pgd[idx * PTRS_PER_PTE];
- ret = __kvm_pgtable_walk(data, ptep, pgt->start_level);
+ ret = __kvm_pgtable_walk(data, pgt->mm_ops, ptep, pgt->start_level);
if (ret)
break;
}
@@ -332,7 +334,6 @@ int kvm_pgtable_get_leaf(struct kvm_pgtable *pgt, u64 addr,
struct hyp_map_data {
u64 phys;
kvm_pte_t attr;
- struct kvm_pgtable_mm_ops *mm_ops;
};
static int hyp_set_prot_attr(enum kvm_pgtable_prot prot, kvm_pte_t *ptep)
@@ -400,7 +401,7 @@ static bool hyp_map_walker_try_leaf(const struct kvm_pgtable_visit_ctx *ctx,
if (ctx->old == new)
return true;
if (!kvm_pte_valid(ctx->old))
- data->mm_ops->get_page(ctx->ptep);
+ ctx->mm_ops->get_page(ctx->ptep);
else if (WARN_ON((ctx->old ^ new) & ~KVM_PTE_LEAF_ATTR_HI_SW))
return false;
@@ -413,7 +414,7 @@ static int hyp_map_walker(const struct kvm_pgtable_visit_ctx *ctx,
{
kvm_pte_t *childp;
struct hyp_map_data *data = ctx->arg;
- struct kvm_pgtable_mm_ops *mm_ops = data->mm_ops;
+ struct kvm_pgtable_mm_ops *mm_ops = ctx->mm_ops;
if (hyp_map_walker_try_leaf(ctx, data))
return 0;
@@ -436,7 +437,6 @@ int kvm_pgtable_hyp_map(struct kvm_pgtable *pgt, u64 addr, u64 size, u64 phys,
int ret;
struct hyp_map_data map_data = {
.phys = ALIGN_DOWN(phys, PAGE_SIZE),
- .mm_ops = pgt->mm_ops,
};
struct kvm_pgtable_walker walker = {
.cb = hyp_map_walker,
@@ -454,18 +454,13 @@ int kvm_pgtable_hyp_map(struct kvm_pgtable *pgt, u64 addr, u64 size, u64 phys,
return ret;
}
-struct hyp_unmap_data {
- u64 unmapped;
- struct kvm_pgtable_mm_ops *mm_ops;
-};
-
static int hyp_unmap_walker(const struct kvm_pgtable_visit_ctx *ctx,
enum kvm_pgtable_walk_flags visit)
{
kvm_pte_t *childp = NULL;
u64 granule = kvm_granule_size(ctx->level);
- struct hyp_unmap_data *data = ctx->arg;
- struct kvm_pgtable_mm_ops *mm_ops = data->mm_ops;
+ u64 *unmapped = ctx->arg;
+ struct kvm_pgtable_mm_ops *mm_ops = ctx->mm_ops;
if (!kvm_pte_valid(ctx->old))
return -EINVAL;
@@ -486,7 +481,7 @@ static int hyp_unmap_walker(const struct kvm_pgtable_visit_ctx *ctx,
kvm_clear_pte(ctx->ptep);
dsb(ishst);
__tlbi_level(vale2is, __TLBI_VADDR(ctx->addr, 0), ctx->level);
- data->unmapped += granule;
+ *unmapped += granule;
}
dsb(ish);
@@ -501,12 +496,10 @@ static int hyp_unmap_walker(const struct kvm_pgtable_visit_ctx *ctx,
u64 kvm_pgtable_hyp_unmap(struct kvm_pgtable *pgt, u64 addr, u64 size)
{
- struct hyp_unmap_data unmap_data = {
- .mm_ops = pgt->mm_ops,
- };
+ u64 unmapped = 0;
struct kvm_pgtable_walker walker = {
.cb = hyp_unmap_walker,
- .arg = &unmap_data,
+ .arg = &unmapped,
.flags = KVM_PGTABLE_WALK_LEAF | KVM_PGTABLE_WALK_TABLE_POST,
};
@@ -514,7 +507,7 @@ u64 kvm_pgtable_hyp_unmap(struct kvm_pgtable *pgt, u64 addr, u64 size)
return 0;
kvm_pgtable_walk(pgt, addr, size, &walker);
- return unmap_data.unmapped;
+ return unmapped;
}
int kvm_pgtable_hyp_init(struct kvm_pgtable *pgt, u32 va_bits,
@@ -538,7 +531,7 @@ int kvm_pgtable_hyp_init(struct kvm_pgtable *pgt, u32 va_bits,
static int hyp_free_walker(const struct kvm_pgtable_visit_ctx *ctx,
enum kvm_pgtable_walk_flags visit)
{
- struct kvm_pgtable_mm_ops *mm_ops = ctx->arg;
+ struct kvm_pgtable_mm_ops *mm_ops = ctx->mm_ops;
if (!kvm_pte_valid(ctx->old))
return 0;
@@ -556,7 +549,6 @@ void kvm_pgtable_hyp_destroy(struct kvm_pgtable *pgt)
struct kvm_pgtable_walker walker = {
.cb = hyp_free_walker,
.flags = KVM_PGTABLE_WALK_LEAF | KVM_PGTABLE_WALK_TABLE_POST,
- .arg = pgt->mm_ops,
};
WARN_ON(kvm_pgtable_walk(pgt, 0, BIT(pgt->ia_bits), &walker));
@@ -575,8 +567,6 @@ struct stage2_map_data {
struct kvm_s2_mmu *mmu;
void *memcache;
- struct kvm_pgtable_mm_ops *mm_ops;
-
/* Force mappings to page granularity */
bool force_pte;
};
@@ -725,7 +715,7 @@ static int stage2_map_walker_try_leaf(const struct kvm_pgtable_visit_ctx *ctx,
kvm_pte_t new;
u64 granule = kvm_granule_size(ctx->level), phys = data->phys;
struct kvm_pgtable *pgt = data->mmu->pgt;
- struct kvm_pgtable_mm_ops *mm_ops = data->mm_ops;
+ struct kvm_pgtable_mm_ops *mm_ops = ctx->mm_ops;
if (!stage2_leaf_mapping_allowed(ctx, data))
return -E2BIG;
@@ -773,7 +763,7 @@ static int stage2_map_walk_table_pre(const struct kvm_pgtable_visit_ctx *ctx,
if (!stage2_leaf_mapping_allowed(ctx, data))
return 0;
- data->childp = kvm_pte_follow(ctx->old, data->mm_ops);
+ data->childp = kvm_pte_follow(ctx->old, ctx->mm_ops);
kvm_clear_pte(ctx->ptep);
/*
@@ -789,7 +779,7 @@ static int stage2_map_walk_table_pre(const struct kvm_pgtable_visit_ctx *ctx,
static int stage2_map_walk_leaf(const struct kvm_pgtable_visit_ctx *ctx,
struct stage2_map_data *data)
{
- struct kvm_pgtable_mm_ops *mm_ops = data->mm_ops;
+ struct kvm_pgtable_mm_ops *mm_ops = ctx->mm_ops;
kvm_pte_t *childp;
int ret;
@@ -831,7 +821,7 @@ static int stage2_map_walk_leaf(const struct kvm_pgtable_visit_ctx *ctx,
static int stage2_map_walk_table_post(const struct kvm_pgtable_visit_ctx *ctx,
struct stage2_map_data *data)
{
- struct kvm_pgtable_mm_ops *mm_ops = data->mm_ops;
+ struct kvm_pgtable_mm_ops *mm_ops = ctx->mm_ops;
kvm_pte_t *childp;
int ret = 0;
@@ -898,7 +888,6 @@ int kvm_pgtable_stage2_map(struct kvm_pgtable *pgt, u64 addr, u64 size,
.phys = ALIGN_DOWN(phys, PAGE_SIZE),
.mmu = pgt->mmu,
.memcache = mc,
- .mm_ops = pgt->mm_ops,
.force_pte = pgt->force_pte_cb && pgt->force_pte_cb(addr, addr + size, prot),
};
struct kvm_pgtable_walker walker = {
@@ -929,7 +918,6 @@ int kvm_pgtable_stage2_set_owner(struct kvm_pgtable *pgt, u64 addr, u64 size,
.phys = KVM_PHYS_INVALID,
.mmu = pgt->mmu,
.memcache = mc,
- .mm_ops = pgt->mm_ops,
.owner_id = owner_id,
.force_pte = true,
};
@@ -953,7 +941,7 @@ static int stage2_unmap_walker(const struct kvm_pgtable_visit_ctx *ctx,
{
struct kvm_pgtable *pgt = ctx->arg;
struct kvm_s2_mmu *mmu = pgt->mmu;
- struct kvm_pgtable_mm_ops *mm_ops = pgt->mm_ops;
+ struct kvm_pgtable_mm_ops *mm_ops = ctx->mm_ops;
kvm_pte_t *childp = NULL;
bool need_flush = false;
@@ -1007,7 +995,6 @@ struct stage2_attr_data {
kvm_pte_t attr_clr;
kvm_pte_t pte;
u32 level;
- struct kvm_pgtable_mm_ops *mm_ops;
};
static int stage2_attr_walker(const struct kvm_pgtable_visit_ctx *ctx,
@@ -1015,7 +1002,7 @@ static int stage2_attr_walker(const struct kvm_pgtable_visit_ctx *ctx,
{
kvm_pte_t pte = ctx->old;
struct stage2_attr_data *data = ctx->arg;
- struct kvm_pgtable_mm_ops *mm_ops = data->mm_ops;
+ struct kvm_pgtable_mm_ops *mm_ops = ctx->mm_ops;
if (!kvm_pte_valid(ctx->old))
return 0;
@@ -1055,7 +1042,6 @@ static int stage2_update_leaf_attrs(struct kvm_pgtable *pgt, u64 addr,
struct stage2_attr_data data = {
.attr_set = attr_set & attr_mask,
.attr_clr = attr_clr & attr_mask,
- .mm_ops = pgt->mm_ops,
};
struct kvm_pgtable_walker walker = {
.cb = stage2_attr_walker,
@@ -1198,7 +1184,7 @@ int __kvm_pgtable_stage2_init(struct kvm_pgtable *pgt, struct kvm_s2_mmu *mmu,
static int stage2_free_walker(const struct kvm_pgtable_visit_ctx *ctx,
enum kvm_pgtable_walk_flags visit)
{
- struct kvm_pgtable_mm_ops *mm_ops = ctx->arg;
+ struct kvm_pgtable_mm_ops *mm_ops = ctx->mm_ops;
if (!stage2_pte_is_counted(ctx->old))
return 0;
@@ -1218,7 +1204,6 @@ void kvm_pgtable_stage2_destroy(struct kvm_pgtable *pgt)
.cb = stage2_free_walker,
.flags = KVM_PGTABLE_WALK_LEAF |
KVM_PGTABLE_WALK_TABLE_POST,
- .arg = pgt->mm_ops,
};
WARN_ON(kvm_pgtable_walk(pgt, 0, BIT(pgt->ia_bits), &walker));
--
2.38.0.rc1.362.ged0d419d3c-goog
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 27+ messages in thread* [PATCH v2 04/15] KVM: arm64: Don't pass kvm_pgtable through kvm_pgtable_walk_data
2022-10-07 23:28 [PATCH v2 00/15] KVM: arm64: Parallel stage-2 fault handling Oliver Upton
` (2 preceding siblings ...)
2022-10-07 23:28 ` [PATCH v2 03/15] KVM: arm64: Pass mm_ops through the " Oliver Upton
@ 2022-10-07 23:28 ` Oliver Upton
2022-10-07 23:28 ` [PATCH v2 05/15] KVM: arm64: Add a helper to tear down unlinked stage-2 subtrees Oliver Upton
` (10 subsequent siblings)
14 siblings, 0 replies; 27+ messages in thread
From: Oliver Upton @ 2022-10-07 23:28 UTC (permalink / raw)
To: Marc Zyngier, James Morse, Alexandru Elisei
Cc: linux-arm-kernel, kvmarm, kvm, Reiji Watanabe, Ricardo Koller,
David Matlack, Quentin Perret, Ben Gardon, Gavin Shan, Peter Xu,
Will Deacon, Sean Christopherson, kvmarm, Oliver Upton
In order to tear down page tables from outside the context of
kvm_pgtable (such as an RCU callback), stop passing a pointer through
kvm_pgtable_walk_data.
No functional change intended.
Signed-off-by: Oliver Upton <oliver.upton@linux.dev>
---
arch/arm64/kvm/hyp/pgtable.c | 18 +++++-------------
1 file changed, 5 insertions(+), 13 deletions(-)
diff --git a/arch/arm64/kvm/hyp/pgtable.c b/arch/arm64/kvm/hyp/pgtable.c
index db25e81a9890..93989b750a26 100644
--- a/arch/arm64/kvm/hyp/pgtable.c
+++ b/arch/arm64/kvm/hyp/pgtable.c
@@ -50,7 +50,6 @@
#define KVM_MAX_OWNER_ID 1
struct kvm_pgtable_walk_data {
- struct kvm_pgtable *pgt;
struct kvm_pgtable_walker *walker;
u64 addr;
@@ -88,7 +87,7 @@ static u32 kvm_pgtable_idx(struct kvm_pgtable_walk_data *data, u32 level)
return (data->addr >> shift) & mask;
}
-static u32 __kvm_pgd_page_idx(struct kvm_pgtable *pgt, u64 addr)
+static u32 kvm_pgd_page_idx(struct kvm_pgtable *pgt, u64 addr)
{
u64 shift = kvm_granule_shift(pgt->start_level - 1); /* May underflow */
u64 mask = BIT(pgt->ia_bits) - 1;
@@ -96,11 +95,6 @@ static u32 __kvm_pgd_page_idx(struct kvm_pgtable *pgt, u64 addr)
return (addr & mask) >> shift;
}
-static u32 kvm_pgd_page_idx(struct kvm_pgtable_walk_data *data)
-{
- return __kvm_pgd_page_idx(data->pgt, data->addr);
-}
-
static u32 kvm_pgd_pages(u32 ia_bits, u32 start_level)
{
struct kvm_pgtable pgt = {
@@ -108,7 +102,7 @@ static u32 kvm_pgd_pages(u32 ia_bits, u32 start_level)
.start_level = start_level,
};
- return __kvm_pgd_page_idx(&pgt, -1ULL) + 1;
+ return kvm_pgd_page_idx(&pgt, -1ULL) + 1;
}
static bool kvm_pte_table(kvm_pte_t pte, u32 level)
@@ -255,11 +249,10 @@ static int __kvm_pgtable_walk(struct kvm_pgtable_walk_data *data,
return ret;
}
-static int _kvm_pgtable_walk(struct kvm_pgtable_walk_data *data)
+static int _kvm_pgtable_walk(struct kvm_pgtable *pgt, struct kvm_pgtable_walk_data *data)
{
u32 idx;
int ret = 0;
- struct kvm_pgtable *pgt = data->pgt;
u64 limit = BIT(pgt->ia_bits);
if (data->addr > limit || data->end > limit)
@@ -268,7 +261,7 @@ static int _kvm_pgtable_walk(struct kvm_pgtable_walk_data *data)
if (!pgt->pgd)
return -EINVAL;
- for (idx = kvm_pgd_page_idx(data); data->addr < data->end; ++idx) {
+ for (idx = kvm_pgd_page_idx(pgt, data->addr); data->addr < data->end; ++idx) {
kvm_pte_t *ptep = &pgt->pgd[idx * PTRS_PER_PTE];
ret = __kvm_pgtable_walk(data, pgt->mm_ops, ptep, pgt->start_level);
@@ -283,13 +276,12 @@ int kvm_pgtable_walk(struct kvm_pgtable *pgt, u64 addr, u64 size,
struct kvm_pgtable_walker *walker)
{
struct kvm_pgtable_walk_data walk_data = {
- .pgt = pgt,
.addr = ALIGN_DOWN(addr, PAGE_SIZE),
.end = PAGE_ALIGN(walk_data.addr + size),
.walker = walker,
};
- return _kvm_pgtable_walk(&walk_data);
+ return _kvm_pgtable_walk(pgt, &walk_data);
}
struct leaf_walk_data {
--
2.38.0.rc1.362.ged0d419d3c-goog
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 27+ messages in thread* [PATCH v2 05/15] KVM: arm64: Add a helper to tear down unlinked stage-2 subtrees
2022-10-07 23:28 [PATCH v2 00/15] KVM: arm64: Parallel stage-2 fault handling Oliver Upton
` (3 preceding siblings ...)
2022-10-07 23:28 ` [PATCH v2 04/15] KVM: arm64: Don't pass kvm_pgtable through kvm_pgtable_walk_data Oliver Upton
@ 2022-10-07 23:28 ` Oliver Upton
2022-10-07 23:28 ` [PATCH v2 06/15] KVM: arm64: Tear down unlinked stage-2 subtree after break-before-make Oliver Upton
` (9 subsequent siblings)
14 siblings, 0 replies; 27+ messages in thread
From: Oliver Upton @ 2022-10-07 23:28 UTC (permalink / raw)
To: Marc Zyngier, James Morse, Alexandru Elisei
Cc: linux-arm-kernel, kvmarm, kvm, Reiji Watanabe, Ricardo Koller,
David Matlack, Quentin Perret, Ben Gardon, Gavin Shan, Peter Xu,
Will Deacon, Sean Christopherson, kvmarm, Oliver Upton
A subsequent change to KVM will move the tear down of an unlinked
stage-2 subtree out of the critical path of the break-before-make
sequence.
Introduce a new helper for tearing down unlinked stage-2 subtrees.
Leverage the existing stage-2 free walkers to do so, with a deep call
into __kvm_pgtable_walk() as the subtree is no longer reachable from the
root.
Signed-off-by: Oliver Upton <oliver.upton@linux.dev>
---
arch/arm64/include/asm/kvm_pgtable.h | 11 +++++++++++
arch/arm64/kvm/hyp/pgtable.c | 23 +++++++++++++++++++++++
2 files changed, 34 insertions(+)
diff --git a/arch/arm64/include/asm/kvm_pgtable.h b/arch/arm64/include/asm/kvm_pgtable.h
index 51acdef1b80a..71b7d154b78a 100644
--- a/arch/arm64/include/asm/kvm_pgtable.h
+++ b/arch/arm64/include/asm/kvm_pgtable.h
@@ -325,6 +325,17 @@ int __kvm_pgtable_stage2_init(struct kvm_pgtable *pgt, struct kvm_s2_mmu *mmu,
*/
void kvm_pgtable_stage2_destroy(struct kvm_pgtable *pgt);
+/**
+ * kvm_pgtable_stage2_free_removed() - Free a removed stage-2 paging structure.
+ * @mm_ops: Memory management callbacks.
+ * @pgtable: Unlinked stage-2 paging structure to be freed.
+ * @level: Level of the stage-2 paging structure to be freed.
+ *
+ * The page-table is assumed to be unreachable by any hardware walkers prior to
+ * freeing and therefore no TLB invalidation is performed.
+ */
+void kvm_pgtable_stage2_free_removed(struct kvm_pgtable_mm_ops *mm_ops, void *pgtable, u32 level);
+
/**
* kvm_pgtable_stage2_map() - Install a mapping in a guest stage-2 page-table.
* @pgt: Page-table structure initialised by kvm_pgtable_stage2_init*().
diff --git a/arch/arm64/kvm/hyp/pgtable.c b/arch/arm64/kvm/hyp/pgtable.c
index 93989b750a26..363a5cce7e1a 100644
--- a/arch/arm64/kvm/hyp/pgtable.c
+++ b/arch/arm64/kvm/hyp/pgtable.c
@@ -1203,3 +1203,26 @@ void kvm_pgtable_stage2_destroy(struct kvm_pgtable *pgt)
pgt->mm_ops->free_pages_exact(pgt->pgd, pgd_sz);
pgt->pgd = NULL;
}
+
+void kvm_pgtable_stage2_free_removed(struct kvm_pgtable_mm_ops *mm_ops, void *pgtable, u32 level)
+{
+ kvm_pte_t *ptep = (kvm_pte_t *)pgtable;
+ struct kvm_pgtable_walker walker = {
+ .cb = stage2_free_walker,
+ .flags = KVM_PGTABLE_WALK_LEAF |
+ KVM_PGTABLE_WALK_TABLE_POST,
+ };
+ struct kvm_pgtable_walk_data data = {
+ .walker = &walker,
+
+ /*
+ * At this point the IPA really doesn't matter, as the page
+ * table being traversed has already been removed from the stage
+ * 2. Set an appropriate range to cover the entire page table.
+ */
+ .addr = 0,
+ .end = kvm_granule_size(level),
+ };
+
+ WARN_ON(__kvm_pgtable_walk(&data, mm_ops, ptep, level));
+}
--
2.38.0.rc1.362.ged0d419d3c-goog
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 27+ messages in thread* [PATCH v2 06/15] KVM: arm64: Tear down unlinked stage-2 subtree after break-before-make
2022-10-07 23:28 [PATCH v2 00/15] KVM: arm64: Parallel stage-2 fault handling Oliver Upton
` (4 preceding siblings ...)
2022-10-07 23:28 ` [PATCH v2 05/15] KVM: arm64: Add a helper to tear down unlinked stage-2 subtrees Oliver Upton
@ 2022-10-07 23:28 ` Oliver Upton
2022-10-28 18:41 ` Ricardo Koller
2022-10-07 23:28 ` [PATCH v2 07/15] KVM: arm64: Use an opaque type for pteps Oliver Upton
` (8 subsequent siblings)
14 siblings, 1 reply; 27+ messages in thread
From: Oliver Upton @ 2022-10-07 23:28 UTC (permalink / raw)
To: Marc Zyngier, James Morse, Alexandru Elisei
Cc: linux-arm-kernel, kvmarm, kvm, Reiji Watanabe, Ricardo Koller,
David Matlack, Quentin Perret, Ben Gardon, Gavin Shan, Peter Xu,
Will Deacon, Sean Christopherson, kvmarm, Oliver Upton
The break-before-make sequence is a bit annoying as it opens a window
wherein memory is unmapped from the guest. KVM should replace the PTE
as quickly as possible and avoid unnecessary work in between.
Presently, the stage-2 map walker tears down a removed table before
installing a block mapping when coalescing a table into a block. As the
removed table is no longer visible to hardware walkers after the
DSB+TLBI, it is possible to move the remaining cleanup to happen after
installing the new PTE.
Reshuffle the stage-2 map walker to install the new block entry in
the pre-order callback. Unwire all of the teardown logic and replace
it with a call to kvm_pgtable_stage2_free_removed() after fixing
the PTE. The post-order visitor is now completely unnecessary, so drop
it. Finally, touch up the comments to better represent the now
simplified map walker.
Note that the call to tear down the unlinked stage-2 is indirected
as a subsequent change will use an RCU callback to trigger tear down.
RCU is not available to pKVM, so there is a need to use different
implementations on pKVM and non-pKVM VMs.
Signed-off-by: Oliver Upton <oliver.upton@linux.dev>
---
arch/arm64/include/asm/kvm_pgtable.h | 3 +
arch/arm64/kvm/hyp/nvhe/mem_protect.c | 6 ++
arch/arm64/kvm/hyp/pgtable.c | 84 ++++++++-------------------
arch/arm64/kvm/mmu.c | 8 +++
4 files changed, 40 insertions(+), 61 deletions(-)
diff --git a/arch/arm64/include/asm/kvm_pgtable.h b/arch/arm64/include/asm/kvm_pgtable.h
index 71b7d154b78a..c33edcf36b5b 100644
--- a/arch/arm64/include/asm/kvm_pgtable.h
+++ b/arch/arm64/include/asm/kvm_pgtable.h
@@ -77,6 +77,8 @@ static inline bool kvm_level_supports_block_mapping(u32 level)
* allocation is physically contiguous.
* @free_pages_exact: Free an exact number of memory pages previously
* allocated by zalloc_pages_exact.
+ * @free_removed_table: Free a removed paging structure by unlinking and
+ * dropping references.
* @get_page: Increment the refcount on a page.
* @put_page: Decrement the refcount on a page. When the
* refcount reaches 0 the page is automatically
@@ -95,6 +97,7 @@ struct kvm_pgtable_mm_ops {
void* (*zalloc_page)(void *arg);
void* (*zalloc_pages_exact)(size_t size);
void (*free_pages_exact)(void *addr, size_t size);
+ void (*free_removed_table)(void *addr, u32 level);
void (*get_page)(void *addr);
void (*put_page)(void *addr);
int (*page_count)(void *addr);
diff --git a/arch/arm64/kvm/hyp/nvhe/mem_protect.c b/arch/arm64/kvm/hyp/nvhe/mem_protect.c
index d21d1b08a055..735769886b55 100644
--- a/arch/arm64/kvm/hyp/nvhe/mem_protect.c
+++ b/arch/arm64/kvm/hyp/nvhe/mem_protect.c
@@ -79,6 +79,11 @@ static void host_s2_put_page(void *addr)
hyp_put_page(&host_s2_pool, addr);
}
+static void host_s2_free_removed_table(void *addr, u32 level)
+{
+ kvm_pgtable_stage2_free_removed(&host_kvm.mm_ops, addr, level);
+}
+
static int prepare_s2_pool(void *pgt_pool_base)
{
unsigned long nr_pages, pfn;
@@ -93,6 +98,7 @@ static int prepare_s2_pool(void *pgt_pool_base)
host_kvm.mm_ops = (struct kvm_pgtable_mm_ops) {
.zalloc_pages_exact = host_s2_zalloc_pages_exact,
.zalloc_page = host_s2_zalloc_page,
+ .free_removed_table = host_s2_free_removed_table,
.phys_to_virt = hyp_phys_to_virt,
.virt_to_phys = hyp_virt_to_phys,
.page_count = hyp_page_count,
diff --git a/arch/arm64/kvm/hyp/pgtable.c b/arch/arm64/kvm/hyp/pgtable.c
index 363a5cce7e1a..02c33fccb178 100644
--- a/arch/arm64/kvm/hyp/pgtable.c
+++ b/arch/arm64/kvm/hyp/pgtable.c
@@ -746,16 +746,19 @@ static int stage2_map_walker_try_leaf(const struct kvm_pgtable_visit_ctx *ctx,
return 0;
}
+static int stage2_map_walk_leaf(const struct kvm_pgtable_visit_ctx *ctx,
+ struct stage2_map_data *data);
+
static int stage2_map_walk_table_pre(const struct kvm_pgtable_visit_ctx *ctx,
struct stage2_map_data *data)
{
- if (data->anchor)
- return 0;
+ struct kvm_pgtable_mm_ops *mm_ops = ctx->mm_ops;
+ kvm_pte_t *childp = kvm_pte_follow(ctx->old, mm_ops);
+ int ret;
if (!stage2_leaf_mapping_allowed(ctx, data))
return 0;
- data->childp = kvm_pte_follow(ctx->old, ctx->mm_ops);
kvm_clear_pte(ctx->ptep);
/*
@@ -764,8 +767,13 @@ static int stage2_map_walk_table_pre(const struct kvm_pgtable_visit_ctx *ctx,
* individually.
*/
kvm_call_hyp(__kvm_tlb_flush_vmid, data->mmu);
- data->anchor = ctx->ptep;
- return 0;
+
+ ret = stage2_map_walk_leaf(ctx, data);
+
+ mm_ops->put_page(ctx->ptep);
+ mm_ops->free_removed_table(childp, ctx->level + 1);
+
+ return ret;
}
static int stage2_map_walk_leaf(const struct kvm_pgtable_visit_ctx *ctx,
@@ -775,13 +783,6 @@ static int stage2_map_walk_leaf(const struct kvm_pgtable_visit_ctx *ctx,
kvm_pte_t *childp;
int ret;
- if (data->anchor) {
- if (stage2_pte_is_counted(ctx->old))
- mm_ops->put_page(ctx->ptep);
-
- return 0;
- }
-
ret = stage2_map_walker_try_leaf(ctx, data);
if (ret != -E2BIG)
return ret;
@@ -810,49 +811,14 @@ static int stage2_map_walk_leaf(const struct kvm_pgtable_visit_ctx *ctx,
return 0;
}
-static int stage2_map_walk_table_post(const struct kvm_pgtable_visit_ctx *ctx,
- struct stage2_map_data *data)
-{
- struct kvm_pgtable_mm_ops *mm_ops = ctx->mm_ops;
- kvm_pte_t *childp;
- int ret = 0;
-
- if (!data->anchor)
- return 0;
-
- if (data->anchor == ctx->ptep) {
- childp = data->childp;
- data->anchor = NULL;
- data->childp = NULL;
- ret = stage2_map_walk_leaf(ctx, data);
- } else {
- childp = kvm_pte_follow(ctx->old, mm_ops);
- }
-
- mm_ops->put_page(childp);
- mm_ops->put_page(ctx->ptep);
-
- return ret;
-}
-
/*
- * This is a little fiddly, as we use all three of the walk flags. The idea
- * is that the TABLE_PRE callback runs for table entries on the way down,
- * looking for table entries which we could conceivably replace with a
- * block entry for this mapping. If it finds one, then it sets the 'anchor'
- * field in 'struct stage2_map_data' to point at the table entry, before
- * clearing the entry to zero and descending into the now detached table.
- *
- * The behaviour of the LEAF callback then depends on whether or not the
- * anchor has been set. If not, then we're not using a block mapping higher
- * up the table and we perform the mapping at the existing leaves instead.
- * If, on the other hand, the anchor _is_ set, then we drop references to
- * all valid leaves so that the pages beneath the anchor can be freed.
+ * The TABLE_PRE callback runs for table entries on the way down, looking
+ * for table entries which we could conceivably replace with a block entry
+ * for this mapping. If it finds one it replaces the entry and calls
+ * kvm_pgtable_mm_ops::free_removed_table() to tear down the detached table.
*
- * Finally, the TABLE_POST callback does nothing if the anchor has not
- * been set, but otherwise frees the page-table pages while walking back up
- * the page-table, installing the block entry when it revisits the anchor
- * pointer and clearing the anchor to NULL.
+ * Otherwise, the LEAF callback performs the mapping at the existing leaves
+ * instead.
*/
static int stage2_map_walker(const struct kvm_pgtable_visit_ctx *ctx,
enum kvm_pgtable_walk_flags visit)
@@ -864,11 +830,9 @@ static int stage2_map_walker(const struct kvm_pgtable_visit_ctx *ctx,
return stage2_map_walk_table_pre(ctx, data);
case KVM_PGTABLE_WALK_LEAF:
return stage2_map_walk_leaf(ctx, data);
- case KVM_PGTABLE_WALK_TABLE_POST:
- return stage2_map_walk_table_post(ctx, data);
+ default:
+ return -EINVAL;
}
-
- return -EINVAL;
}
int kvm_pgtable_stage2_map(struct kvm_pgtable *pgt, u64 addr, u64 size,
@@ -885,8 +849,7 @@ int kvm_pgtable_stage2_map(struct kvm_pgtable *pgt, u64 addr, u64 size,
struct kvm_pgtable_walker walker = {
.cb = stage2_map_walker,
.flags = KVM_PGTABLE_WALK_TABLE_PRE |
- KVM_PGTABLE_WALK_LEAF |
- KVM_PGTABLE_WALK_TABLE_POST,
+ KVM_PGTABLE_WALK_LEAF,
.arg = &map_data,
};
@@ -916,8 +879,7 @@ int kvm_pgtable_stage2_set_owner(struct kvm_pgtable *pgt, u64 addr, u64 size,
struct kvm_pgtable_walker walker = {
.cb = stage2_map_walker,
.flags = KVM_PGTABLE_WALK_TABLE_PRE |
- KVM_PGTABLE_WALK_LEAF |
- KVM_PGTABLE_WALK_TABLE_POST,
+ KVM_PGTABLE_WALK_LEAF,
.arg = &map_data,
};
diff --git a/arch/arm64/kvm/mmu.c b/arch/arm64/kvm/mmu.c
index c9a13e487187..04a25319abb0 100644
--- a/arch/arm64/kvm/mmu.c
+++ b/arch/arm64/kvm/mmu.c
@@ -102,6 +102,13 @@ static void *kvm_host_zalloc_pages_exact(size_t size)
return alloc_pages_exact(size, GFP_KERNEL_ACCOUNT | __GFP_ZERO);
}
+static struct kvm_pgtable_mm_ops kvm_s2_mm_ops;
+
+static void stage2_free_removed_table(void *addr, u32 level)
+{
+ kvm_pgtable_stage2_free_removed(&kvm_s2_mm_ops, addr, level);
+}
+
static void kvm_host_get_page(void *addr)
{
get_page(virt_to_page(addr));
@@ -627,6 +634,7 @@ static struct kvm_pgtable_mm_ops kvm_s2_mm_ops = {
.zalloc_page = stage2_memcache_zalloc_page,
.zalloc_pages_exact = kvm_host_zalloc_pages_exact,
.free_pages_exact = free_pages_exact,
+ .free_removed_table = stage2_free_removed_table,
.get_page = kvm_host_get_page,
.put_page = kvm_host_put_page,
.page_count = kvm_host_page_count,
--
2.38.0.rc1.362.ged0d419d3c-goog
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 27+ messages in thread* Re: [PATCH v2 06/15] KVM: arm64: Tear down unlinked stage-2 subtree after break-before-make
2022-10-07 23:28 ` [PATCH v2 06/15] KVM: arm64: Tear down unlinked stage-2 subtree after break-before-make Oliver Upton
@ 2022-10-28 18:41 ` Ricardo Koller
2022-10-28 18:43 ` Ricardo Koller
0 siblings, 1 reply; 27+ messages in thread
From: Ricardo Koller @ 2022-10-28 18:41 UTC (permalink / raw)
To: Oliver Upton
Cc: Marc Zyngier, James Morse, Alexandru Elisei, linux-arm-kernel,
kvmarm, kvm, Reiji Watanabe, David Matlack, Quentin Perret,
Ben Gardon, Gavin Shan, Peter Xu, Will Deacon,
Sean Christopherson, kvmarm
On Fri, Oct 07, 2022 at 11:28:09PM +0000, Oliver Upton wrote:
> The break-before-make sequence is a bit annoying as it opens a window
> wherein memory is unmapped from the guest. KVM should replace the PTE
> as quickly as possible and avoid unnecessary work in between.
>
> Presently, the stage-2 map walker tears down a removed table before
> installing a block mapping when coalescing a table into a block. As the
> removed table is no longer visible to hardware walkers after the
> DSB+TLBI, it is possible to move the remaining cleanup to happen after
> installing the new PTE.
>
> Reshuffle the stage-2 map walker to install the new block entry in
> the pre-order callback. Unwire all of the teardown logic and replace
> it with a call to kvm_pgtable_stage2_free_removed() after fixing
> the PTE. The post-order visitor is now completely unnecessary, so drop
> it. Finally, touch up the comments to better represent the now
> simplified map walker.
>
> Note that the call to tear down the unlinked stage-2 is indirected
> as a subsequent change will use an RCU callback to trigger tear down.
> RCU is not available to pKVM, so there is a need to use different
> implementations on pKVM and non-pKVM VMs.
>
> Signed-off-by: Oliver Upton <oliver.upton@linux.dev>
> ---
> arch/arm64/include/asm/kvm_pgtable.h | 3 +
> arch/arm64/kvm/hyp/nvhe/mem_protect.c | 6 ++
> arch/arm64/kvm/hyp/pgtable.c | 84 ++++++++-------------------
> arch/arm64/kvm/mmu.c | 8 +++
> 4 files changed, 40 insertions(+), 61 deletions(-)
>
> diff --git a/arch/arm64/include/asm/kvm_pgtable.h b/arch/arm64/include/asm/kvm_pgtable.h
> index 71b7d154b78a..c33edcf36b5b 100644
> --- a/arch/arm64/include/asm/kvm_pgtable.h
> +++ b/arch/arm64/include/asm/kvm_pgtable.h
> @@ -77,6 +77,8 @@ static inline bool kvm_level_supports_block_mapping(u32 level)
> * allocation is physically contiguous.
> * @free_pages_exact: Free an exact number of memory pages previously
> * allocated by zalloc_pages_exact.
> + * @free_removed_table: Free a removed paging structure by unlinking and
> + * dropping references.
> * @get_page: Increment the refcount on a page.
> * @put_page: Decrement the refcount on a page. When the
> * refcount reaches 0 the page is automatically
> @@ -95,6 +97,7 @@ struct kvm_pgtable_mm_ops {
> void* (*zalloc_page)(void *arg);
> void* (*zalloc_pages_exact)(size_t size);
> void (*free_pages_exact)(void *addr, size_t size);
> + void (*free_removed_table)(void *addr, u32 level);
> void (*get_page)(void *addr);
> void (*put_page)(void *addr);
> int (*page_count)(void *addr);
> diff --git a/arch/arm64/kvm/hyp/nvhe/mem_protect.c b/arch/arm64/kvm/hyp/nvhe/mem_protect.c
> index d21d1b08a055..735769886b55 100644
> --- a/arch/arm64/kvm/hyp/nvhe/mem_protect.c
> +++ b/arch/arm64/kvm/hyp/nvhe/mem_protect.c
> @@ -79,6 +79,11 @@ static void host_s2_put_page(void *addr)
> hyp_put_page(&host_s2_pool, addr);
> }
>
> +static void host_s2_free_removed_table(void *addr, u32 level)
> +{
> + kvm_pgtable_stage2_free_removed(&host_kvm.mm_ops, addr, level);
> +}
> +
> static int prepare_s2_pool(void *pgt_pool_base)
> {
> unsigned long nr_pages, pfn;
> @@ -93,6 +98,7 @@ static int prepare_s2_pool(void *pgt_pool_base)
> host_kvm.mm_ops = (struct kvm_pgtable_mm_ops) {
> .zalloc_pages_exact = host_s2_zalloc_pages_exact,
> .zalloc_page = host_s2_zalloc_page,
> + .free_removed_table = host_s2_free_removed_table,
> .phys_to_virt = hyp_phys_to_virt,
> .virt_to_phys = hyp_virt_to_phys,
> .page_count = hyp_page_count,
> diff --git a/arch/arm64/kvm/hyp/pgtable.c b/arch/arm64/kvm/hyp/pgtable.c
> index 363a5cce7e1a..02c33fccb178 100644
> --- a/arch/arm64/kvm/hyp/pgtable.c
> +++ b/arch/arm64/kvm/hyp/pgtable.c
> @@ -746,16 +746,19 @@ static int stage2_map_walker_try_leaf(const struct kvm_pgtable_visit_ctx *ctx,
> return 0;
> }
>
> +static int stage2_map_walk_leaf(const struct kvm_pgtable_visit_ctx *ctx,
> + struct stage2_map_data *data);
> +
> static int stage2_map_walk_table_pre(const struct kvm_pgtable_visit_ctx *ctx,
> struct stage2_map_data *data)
> {
> - if (data->anchor)
> - return 0;
> + struct kvm_pgtable_mm_ops *mm_ops = ctx->mm_ops;
> + kvm_pte_t *childp = kvm_pte_follow(ctx->old, mm_ops);
> + int ret;
>
> if (!stage2_leaf_mapping_allowed(ctx, data))
> return 0;
>
> - data->childp = kvm_pte_follow(ctx->old, ctx->mm_ops);
> kvm_clear_pte(ctx->ptep);
>
> /*
> @@ -764,8 +767,13 @@ static int stage2_map_walk_table_pre(const struct kvm_pgtable_visit_ctx *ctx,
> * individually.
> */
> kvm_call_hyp(__kvm_tlb_flush_vmid, data->mmu);
> - data->anchor = ctx->ptep;
> - return 0;
> +
> + ret = stage2_map_walk_leaf(ctx, data);
> +
> + mm_ops->put_page(ctx->ptep);
> + mm_ops->free_removed_table(childp, ctx->level + 1);
This could save some cycles:
if (stage2_pte_is_counted(ctx->old)
mm_ops->free_removed_table(childp, ctx->level + 1);
as coming back using the stage2_free_walker() requires some preparation,
and it does the exact same check anyawy.
> +
> + return ret;
> }
>
> static int stage2_map_walk_leaf(const struct kvm_pgtable_visit_ctx *ctx,
> @@ -775,13 +783,6 @@ static int stage2_map_walk_leaf(const struct kvm_pgtable_visit_ctx *ctx,
> kvm_pte_t *childp;
> int ret;
>
> - if (data->anchor) {
> - if (stage2_pte_is_counted(ctx->old))
> - mm_ops->put_page(ctx->ptep);
> -
> - return 0;
> - }
> -
> ret = stage2_map_walker_try_leaf(ctx, data);
> if (ret != -E2BIG)
> return ret;
> @@ -810,49 +811,14 @@ static int stage2_map_walk_leaf(const struct kvm_pgtable_visit_ctx *ctx,
> return 0;
> }
>
> -static int stage2_map_walk_table_post(const struct kvm_pgtable_visit_ctx *ctx,
> - struct stage2_map_data *data)
> -{
> - struct kvm_pgtable_mm_ops *mm_ops = ctx->mm_ops;
> - kvm_pte_t *childp;
> - int ret = 0;
> -
> - if (!data->anchor)
> - return 0;
> -
> - if (data->anchor == ctx->ptep) {
> - childp = data->childp;
> - data->anchor = NULL;
> - data->childp = NULL;
> - ret = stage2_map_walk_leaf(ctx, data);
> - } else {
> - childp = kvm_pte_follow(ctx->old, mm_ops);
> - }
> -
> - mm_ops->put_page(childp);
> - mm_ops->put_page(ctx->ptep);
> -
> - return ret;
> -}
> -
> /*
> - * This is a little fiddly, as we use all three of the walk flags. The idea
> - * is that the TABLE_PRE callback runs for table entries on the way down,
> - * looking for table entries which we could conceivably replace with a
> - * block entry for this mapping. If it finds one, then it sets the 'anchor'
> - * field in 'struct stage2_map_data' to point at the table entry, before
> - * clearing the entry to zero and descending into the now detached table.
> - *
> - * The behaviour of the LEAF callback then depends on whether or not the
> - * anchor has been set. If not, then we're not using a block mapping higher
> - * up the table and we perform the mapping at the existing leaves instead.
> - * If, on the other hand, the anchor _is_ set, then we drop references to
> - * all valid leaves so that the pages beneath the anchor can be freed.
> + * The TABLE_PRE callback runs for table entries on the way down, looking
> + * for table entries which we could conceivably replace with a block entry
> + * for this mapping. If it finds one it replaces the entry and calls
> + * kvm_pgtable_mm_ops::free_removed_table() to tear down the detached table.
> *
> - * Finally, the TABLE_POST callback does nothing if the anchor has not
> - * been set, but otherwise frees the page-table pages while walking back up
> - * the page-table, installing the block entry when it revisits the anchor
> - * pointer and clearing the anchor to NULL.
> + * Otherwise, the LEAF callback performs the mapping at the existing leaves
> + * instead.
> */
> static int stage2_map_walker(const struct kvm_pgtable_visit_ctx *ctx,
> enum kvm_pgtable_walk_flags visit)
> @@ -864,11 +830,9 @@ static int stage2_map_walker(const struct kvm_pgtable_visit_ctx *ctx,
> return stage2_map_walk_table_pre(ctx, data);
> case KVM_PGTABLE_WALK_LEAF:
> return stage2_map_walk_leaf(ctx, data);
> - case KVM_PGTABLE_WALK_TABLE_POST:
> - return stage2_map_walk_table_post(ctx, data);
> + default:
> + return -EINVAL;
> }
> -
> - return -EINVAL;
> }
>
> int kvm_pgtable_stage2_map(struct kvm_pgtable *pgt, u64 addr, u64 size,
> @@ -885,8 +849,7 @@ int kvm_pgtable_stage2_map(struct kvm_pgtable *pgt, u64 addr, u64 size,
> struct kvm_pgtable_walker walker = {
> .cb = stage2_map_walker,
> .flags = KVM_PGTABLE_WALK_TABLE_PRE |
> - KVM_PGTABLE_WALK_LEAF |
> - KVM_PGTABLE_WALK_TABLE_POST,
> + KVM_PGTABLE_WALK_LEAF,
> .arg = &map_data,
> };
>
> @@ -916,8 +879,7 @@ int kvm_pgtable_stage2_set_owner(struct kvm_pgtable *pgt, u64 addr, u64 size,
> struct kvm_pgtable_walker walker = {
> .cb = stage2_map_walker,
> .flags = KVM_PGTABLE_WALK_TABLE_PRE |
> - KVM_PGTABLE_WALK_LEAF |
> - KVM_PGTABLE_WALK_TABLE_POST,
> + KVM_PGTABLE_WALK_LEAF,
> .arg = &map_data,
> };
>
> diff --git a/arch/arm64/kvm/mmu.c b/arch/arm64/kvm/mmu.c
> index c9a13e487187..04a25319abb0 100644
> --- a/arch/arm64/kvm/mmu.c
> +++ b/arch/arm64/kvm/mmu.c
> @@ -102,6 +102,13 @@ static void *kvm_host_zalloc_pages_exact(size_t size)
> return alloc_pages_exact(size, GFP_KERNEL_ACCOUNT | __GFP_ZERO);
> }
>
> +static struct kvm_pgtable_mm_ops kvm_s2_mm_ops;
> +
> +static void stage2_free_removed_table(void *addr, u32 level)
> +{
> + kvm_pgtable_stage2_free_removed(&kvm_s2_mm_ops, addr, level);
> +}
> +
> static void kvm_host_get_page(void *addr)
> {
> get_page(virt_to_page(addr));
> @@ -627,6 +634,7 @@ static struct kvm_pgtable_mm_ops kvm_s2_mm_ops = {
> .zalloc_page = stage2_memcache_zalloc_page,
> .zalloc_pages_exact = kvm_host_zalloc_pages_exact,
> .free_pages_exact = free_pages_exact,
> + .free_removed_table = stage2_free_removed_table,
> .get_page = kvm_host_get_page,
> .put_page = kvm_host_put_page,
> .page_count = kvm_host_page_count,
> --
> 2.38.0.rc1.362.ged0d419d3c-goog
>
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 27+ messages in thread* Re: [PATCH v2 06/15] KVM: arm64: Tear down unlinked stage-2 subtree after break-before-make
2022-10-28 18:41 ` Ricardo Koller
@ 2022-10-28 18:43 ` Ricardo Koller
2022-10-28 18:53 ` Ricardo Koller
0 siblings, 1 reply; 27+ messages in thread
From: Ricardo Koller @ 2022-10-28 18:43 UTC (permalink / raw)
To: Oliver Upton
Cc: Marc Zyngier, James Morse, Alexandru Elisei, linux-arm-kernel,
kvmarm, kvm, Reiji Watanabe, David Matlack, Quentin Perret,
Ben Gardon, Gavin Shan, Peter Xu, Will Deacon,
Sean Christopherson, kvmarm
On Fri, Oct 28, 2022 at 11:41:57AM -0700, Ricardo Koller wrote:
> On Fri, Oct 07, 2022 at 11:28:09PM +0000, Oliver Upton wrote:
> > The break-before-make sequence is a bit annoying as it opens a window
> > wherein memory is unmapped from the guest. KVM should replace the PTE
> > as quickly as possible and avoid unnecessary work in between.
> >
> > Presently, the stage-2 map walker tears down a removed table before
> > installing a block mapping when coalescing a table into a block. As the
> > removed table is no longer visible to hardware walkers after the
> > DSB+TLBI, it is possible to move the remaining cleanup to happen after
> > installing the new PTE.
> >
> > Reshuffle the stage-2 map walker to install the new block entry in
> > the pre-order callback. Unwire all of the teardown logic and replace
> > it with a call to kvm_pgtable_stage2_free_removed() after fixing
> > the PTE. The post-order visitor is now completely unnecessary, so drop
> > it. Finally, touch up the comments to better represent the now
> > simplified map walker.
> >
> > Note that the call to tear down the unlinked stage-2 is indirected
> > as a subsequent change will use an RCU callback to trigger tear down.
> > RCU is not available to pKVM, so there is a need to use different
> > implementations on pKVM and non-pKVM VMs.
> >
> > Signed-off-by: Oliver Upton <oliver.upton@linux.dev>
> > ---
> > arch/arm64/include/asm/kvm_pgtable.h | 3 +
> > arch/arm64/kvm/hyp/nvhe/mem_protect.c | 6 ++
> > arch/arm64/kvm/hyp/pgtable.c | 84 ++++++++-------------------
> > arch/arm64/kvm/mmu.c | 8 +++
> > 4 files changed, 40 insertions(+), 61 deletions(-)
> >
> > diff --git a/arch/arm64/include/asm/kvm_pgtable.h b/arch/arm64/include/asm/kvm_pgtable.h
> > index 71b7d154b78a..c33edcf36b5b 100644
> > --- a/arch/arm64/include/asm/kvm_pgtable.h
> > +++ b/arch/arm64/include/asm/kvm_pgtable.h
> > @@ -77,6 +77,8 @@ static inline bool kvm_level_supports_block_mapping(u32 level)
> > * allocation is physically contiguous.
> > * @free_pages_exact: Free an exact number of memory pages previously
> > * allocated by zalloc_pages_exact.
> > + * @free_removed_table: Free a removed paging structure by unlinking and
> > + * dropping references.
> > * @get_page: Increment the refcount on a page.
> > * @put_page: Decrement the refcount on a page. When the
> > * refcount reaches 0 the page is automatically
> > @@ -95,6 +97,7 @@ struct kvm_pgtable_mm_ops {
> > void* (*zalloc_page)(void *arg);
> > void* (*zalloc_pages_exact)(size_t size);
> > void (*free_pages_exact)(void *addr, size_t size);
> > + void (*free_removed_table)(void *addr, u32 level);
> > void (*get_page)(void *addr);
> > void (*put_page)(void *addr);
> > int (*page_count)(void *addr);
> > diff --git a/arch/arm64/kvm/hyp/nvhe/mem_protect.c b/arch/arm64/kvm/hyp/nvhe/mem_protect.c
> > index d21d1b08a055..735769886b55 100644
> > --- a/arch/arm64/kvm/hyp/nvhe/mem_protect.c
> > +++ b/arch/arm64/kvm/hyp/nvhe/mem_protect.c
> > @@ -79,6 +79,11 @@ static void host_s2_put_page(void *addr)
> > hyp_put_page(&host_s2_pool, addr);
> > }
> >
> > +static void host_s2_free_removed_table(void *addr, u32 level)
> > +{
> > + kvm_pgtable_stage2_free_removed(&host_kvm.mm_ops, addr, level);
> > +}
> > +
> > static int prepare_s2_pool(void *pgt_pool_base)
> > {
> > unsigned long nr_pages, pfn;
> > @@ -93,6 +98,7 @@ static int prepare_s2_pool(void *pgt_pool_base)
> > host_kvm.mm_ops = (struct kvm_pgtable_mm_ops) {
> > .zalloc_pages_exact = host_s2_zalloc_pages_exact,
> > .zalloc_page = host_s2_zalloc_page,
> > + .free_removed_table = host_s2_free_removed_table,
> > .phys_to_virt = hyp_phys_to_virt,
> > .virt_to_phys = hyp_virt_to_phys,
> > .page_count = hyp_page_count,
> > diff --git a/arch/arm64/kvm/hyp/pgtable.c b/arch/arm64/kvm/hyp/pgtable.c
> > index 363a5cce7e1a..02c33fccb178 100644
> > --- a/arch/arm64/kvm/hyp/pgtable.c
> > +++ b/arch/arm64/kvm/hyp/pgtable.c
> > @@ -746,16 +746,19 @@ static int stage2_map_walker_try_leaf(const struct kvm_pgtable_visit_ctx *ctx,
> > return 0;
> > }
> >
> > +static int stage2_map_walk_leaf(const struct kvm_pgtable_visit_ctx *ctx,
> > + struct stage2_map_data *data);
> > +
> > static int stage2_map_walk_table_pre(const struct kvm_pgtable_visit_ctx *ctx,
> > struct stage2_map_data *data)
> > {
> > - if (data->anchor)
> > - return 0;
> > + struct kvm_pgtable_mm_ops *mm_ops = ctx->mm_ops;
> > + kvm_pte_t *childp = kvm_pte_follow(ctx->old, mm_ops);
> > + int ret;
> >
> > if (!stage2_leaf_mapping_allowed(ctx, data))
> > return 0;
> >
> > - data->childp = kvm_pte_follow(ctx->old, ctx->mm_ops);
> > kvm_clear_pte(ctx->ptep);
> >
> > /*
> > @@ -764,8 +767,13 @@ static int stage2_map_walk_table_pre(const struct kvm_pgtable_visit_ctx *ctx,
> > * individually.
> > */
> > kvm_call_hyp(__kvm_tlb_flush_vmid, data->mmu);
> > - data->anchor = ctx->ptep;
> > - return 0;
> > +
> > + ret = stage2_map_walk_leaf(ctx, data);
> > +
> > + mm_ops->put_page(ctx->ptep);
> > + mm_ops->free_removed_table(childp, ctx->level + 1);
>
> This could save some cycles:
>
> if (stage2_pte_is_counted(childp) // typo: should be childp
> mm_ops->free_removed_table(childp, ctx->level + 1);
>
> as coming back using the stage2_free_walker() requires some preparation,
> and it does the exact same check anyawy.
>
> > +
> > + return ret;
> > }
> >
> > static int stage2_map_walk_leaf(const struct kvm_pgtable_visit_ctx *ctx,
> > @@ -775,13 +783,6 @@ static int stage2_map_walk_leaf(const struct kvm_pgtable_visit_ctx *ctx,
> > kvm_pte_t *childp;
> > int ret;
> >
> > - if (data->anchor) {
> > - if (stage2_pte_is_counted(ctx->old))
> > - mm_ops->put_page(ctx->ptep);
> > -
> > - return 0;
> > - }
> > -
> > ret = stage2_map_walker_try_leaf(ctx, data);
> > if (ret != -E2BIG)
> > return ret;
> > @@ -810,49 +811,14 @@ static int stage2_map_walk_leaf(const struct kvm_pgtable_visit_ctx *ctx,
> > return 0;
> > }
> >
> > -static int stage2_map_walk_table_post(const struct kvm_pgtable_visit_ctx *ctx,
> > - struct stage2_map_data *data)
> > -{
> > - struct kvm_pgtable_mm_ops *mm_ops = ctx->mm_ops;
> > - kvm_pte_t *childp;
> > - int ret = 0;
> > -
> > - if (!data->anchor)
> > - return 0;
> > -
> > - if (data->anchor == ctx->ptep) {
> > - childp = data->childp;
> > - data->anchor = NULL;
> > - data->childp = NULL;
> > - ret = stage2_map_walk_leaf(ctx, data);
> > - } else {
> > - childp = kvm_pte_follow(ctx->old, mm_ops);
> > - }
> > -
> > - mm_ops->put_page(childp);
> > - mm_ops->put_page(ctx->ptep);
> > -
> > - return ret;
> > -}
> > -
> > /*
> > - * This is a little fiddly, as we use all three of the walk flags. The idea
> > - * is that the TABLE_PRE callback runs for table entries on the way down,
> > - * looking for table entries which we could conceivably replace with a
> > - * block entry for this mapping. If it finds one, then it sets the 'anchor'
> > - * field in 'struct stage2_map_data' to point at the table entry, before
> > - * clearing the entry to zero and descending into the now detached table.
> > - *
> > - * The behaviour of the LEAF callback then depends on whether or not the
> > - * anchor has been set. If not, then we're not using a block mapping higher
> > - * up the table and we perform the mapping at the existing leaves instead.
> > - * If, on the other hand, the anchor _is_ set, then we drop references to
> > - * all valid leaves so that the pages beneath the anchor can be freed.
> > + * The TABLE_PRE callback runs for table entries on the way down, looking
> > + * for table entries which we could conceivably replace with a block entry
> > + * for this mapping. If it finds one it replaces the entry and calls
> > + * kvm_pgtable_mm_ops::free_removed_table() to tear down the detached table.
> > *
> > - * Finally, the TABLE_POST callback does nothing if the anchor has not
> > - * been set, but otherwise frees the page-table pages while walking back up
> > - * the page-table, installing the block entry when it revisits the anchor
> > - * pointer and clearing the anchor to NULL.
> > + * Otherwise, the LEAF callback performs the mapping at the existing leaves
> > + * instead.
> > */
> > static int stage2_map_walker(const struct kvm_pgtable_visit_ctx *ctx,
> > enum kvm_pgtable_walk_flags visit)
> > @@ -864,11 +830,9 @@ static int stage2_map_walker(const struct kvm_pgtable_visit_ctx *ctx,
> > return stage2_map_walk_table_pre(ctx, data);
> > case KVM_PGTABLE_WALK_LEAF:
> > return stage2_map_walk_leaf(ctx, data);
> > - case KVM_PGTABLE_WALK_TABLE_POST:
> > - return stage2_map_walk_table_post(ctx, data);
> > + default:
> > + return -EINVAL;
> > }
> > -
> > - return -EINVAL;
> > }
> >
> > int kvm_pgtable_stage2_map(struct kvm_pgtable *pgt, u64 addr, u64 size,
> > @@ -885,8 +849,7 @@ int kvm_pgtable_stage2_map(struct kvm_pgtable *pgt, u64 addr, u64 size,
> > struct kvm_pgtable_walker walker = {
> > .cb = stage2_map_walker,
> > .flags = KVM_PGTABLE_WALK_TABLE_PRE |
> > - KVM_PGTABLE_WALK_LEAF |
> > - KVM_PGTABLE_WALK_TABLE_POST,
> > + KVM_PGTABLE_WALK_LEAF,
> > .arg = &map_data,
> > };
> >
> > @@ -916,8 +879,7 @@ int kvm_pgtable_stage2_set_owner(struct kvm_pgtable *pgt, u64 addr, u64 size,
> > struct kvm_pgtable_walker walker = {
> > .cb = stage2_map_walker,
> > .flags = KVM_PGTABLE_WALK_TABLE_PRE |
> > - KVM_PGTABLE_WALK_LEAF |
> > - KVM_PGTABLE_WALK_TABLE_POST,
> > + KVM_PGTABLE_WALK_LEAF,
> > .arg = &map_data,
> > };
> >
> > diff --git a/arch/arm64/kvm/mmu.c b/arch/arm64/kvm/mmu.c
> > index c9a13e487187..04a25319abb0 100644
> > --- a/arch/arm64/kvm/mmu.c
> > +++ b/arch/arm64/kvm/mmu.c
> > @@ -102,6 +102,13 @@ static void *kvm_host_zalloc_pages_exact(size_t size)
> > return alloc_pages_exact(size, GFP_KERNEL_ACCOUNT | __GFP_ZERO);
> > }
> >
> > +static struct kvm_pgtable_mm_ops kvm_s2_mm_ops;
> > +
> > +static void stage2_free_removed_table(void *addr, u32 level)
> > +{
> > + kvm_pgtable_stage2_free_removed(&kvm_s2_mm_ops, addr, level);
> > +}
> > +
> > static void kvm_host_get_page(void *addr)
> > {
> > get_page(virt_to_page(addr));
> > @@ -627,6 +634,7 @@ static struct kvm_pgtable_mm_ops kvm_s2_mm_ops = {
> > .zalloc_page = stage2_memcache_zalloc_page,
> > .zalloc_pages_exact = kvm_host_zalloc_pages_exact,
> > .free_pages_exact = free_pages_exact,
> > + .free_removed_table = stage2_free_removed_table,
> > .get_page = kvm_host_get_page,
> > .put_page = kvm_host_put_page,
> > .page_count = kvm_host_page_count,
> > --
> > 2.38.0.rc1.362.ged0d419d3c-goog
> >
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 27+ messages in thread* Re: [PATCH v2 06/15] KVM: arm64: Tear down unlinked stage-2 subtree after break-before-make
2022-10-28 18:43 ` Ricardo Koller
@ 2022-10-28 18:53 ` Ricardo Koller
0 siblings, 0 replies; 27+ messages in thread
From: Ricardo Koller @ 2022-10-28 18:53 UTC (permalink / raw)
To: Oliver Upton
Cc: Marc Zyngier, James Morse, Alexandru Elisei, linux-arm-kernel,
kvmarm, kvm, Reiji Watanabe, David Matlack, Quentin Perret,
Ben Gardon, Gavin Shan, Peter Xu, Will Deacon,
Sean Christopherson, kvmarm
On Fri, Oct 28, 2022 at 11:43:33AM -0700, Ricardo Koller wrote:
> On Fri, Oct 28, 2022 at 11:41:57AM -0700, Ricardo Koller wrote:
> > On Fri, Oct 07, 2022 at 11:28:09PM +0000, Oliver Upton wrote:
> > > The break-before-make sequence is a bit annoying as it opens a window
> > > wherein memory is unmapped from the guest. KVM should replace the PTE
> > > as quickly as possible and avoid unnecessary work in between.
> > >
> > > Presently, the stage-2 map walker tears down a removed table before
> > > installing a block mapping when coalescing a table into a block. As the
> > > removed table is no longer visible to hardware walkers after the
> > > DSB+TLBI, it is possible to move the remaining cleanup to happen after
> > > installing the new PTE.
> > >
> > > Reshuffle the stage-2 map walker to install the new block entry in
> > > the pre-order callback. Unwire all of the teardown logic and replace
> > > it with a call to kvm_pgtable_stage2_free_removed() after fixing
> > > the PTE. The post-order visitor is now completely unnecessary, so drop
> > > it. Finally, touch up the comments to better represent the now
> > > simplified map walker.
> > >
> > > Note that the call to tear down the unlinked stage-2 is indirected
> > > as a subsequent change will use an RCU callback to trigger tear down.
> > > RCU is not available to pKVM, so there is a need to use different
> > > implementations on pKVM and non-pKVM VMs.
> > >
> > > Signed-off-by: Oliver Upton <oliver.upton@linux.dev>
> > > ---
> > > arch/arm64/include/asm/kvm_pgtable.h | 3 +
> > > arch/arm64/kvm/hyp/nvhe/mem_protect.c | 6 ++
> > > arch/arm64/kvm/hyp/pgtable.c | 84 ++++++++-------------------
> > > arch/arm64/kvm/mmu.c | 8 +++
> > > 4 files changed, 40 insertions(+), 61 deletions(-)
> > >
> > > diff --git a/arch/arm64/include/asm/kvm_pgtable.h b/arch/arm64/include/asm/kvm_pgtable.h
> > > index 71b7d154b78a..c33edcf36b5b 100644
> > > --- a/arch/arm64/include/asm/kvm_pgtable.h
> > > +++ b/arch/arm64/include/asm/kvm_pgtable.h
> > > @@ -77,6 +77,8 @@ static inline bool kvm_level_supports_block_mapping(u32 level)
> > > * allocation is physically contiguous.
> > > * @free_pages_exact: Free an exact number of memory pages previously
> > > * allocated by zalloc_pages_exact.
> > > + * @free_removed_table: Free a removed paging structure by unlinking and
> > > + * dropping references.
> > > * @get_page: Increment the refcount on a page.
> > > * @put_page: Decrement the refcount on a page. When the
> > > * refcount reaches 0 the page is automatically
> > > @@ -95,6 +97,7 @@ struct kvm_pgtable_mm_ops {
> > > void* (*zalloc_page)(void *arg);
> > > void* (*zalloc_pages_exact)(size_t size);
> > > void (*free_pages_exact)(void *addr, size_t size);
> > > + void (*free_removed_table)(void *addr, u32 level);
> > > void (*get_page)(void *addr);
> > > void (*put_page)(void *addr);
> > > int (*page_count)(void *addr);
> > > diff --git a/arch/arm64/kvm/hyp/nvhe/mem_protect.c b/arch/arm64/kvm/hyp/nvhe/mem_protect.c
> > > index d21d1b08a055..735769886b55 100644
> > > --- a/arch/arm64/kvm/hyp/nvhe/mem_protect.c
> > > +++ b/arch/arm64/kvm/hyp/nvhe/mem_protect.c
> > > @@ -79,6 +79,11 @@ static void host_s2_put_page(void *addr)
> > > hyp_put_page(&host_s2_pool, addr);
> > > }
> > >
> > > +static void host_s2_free_removed_table(void *addr, u32 level)
> > > +{
> > > + kvm_pgtable_stage2_free_removed(&host_kvm.mm_ops, addr, level);
> > > +}
> > > +
> > > static int prepare_s2_pool(void *pgt_pool_base)
> > > {
> > > unsigned long nr_pages, pfn;
> > > @@ -93,6 +98,7 @@ static int prepare_s2_pool(void *pgt_pool_base)
> > > host_kvm.mm_ops = (struct kvm_pgtable_mm_ops) {
> > > .zalloc_pages_exact = host_s2_zalloc_pages_exact,
> > > .zalloc_page = host_s2_zalloc_page,
> > > + .free_removed_table = host_s2_free_removed_table,
> > > .phys_to_virt = hyp_phys_to_virt,
> > > .virt_to_phys = hyp_virt_to_phys,
> > > .page_count = hyp_page_count,
> > > diff --git a/arch/arm64/kvm/hyp/pgtable.c b/arch/arm64/kvm/hyp/pgtable.c
> > > index 363a5cce7e1a..02c33fccb178 100644
> > > --- a/arch/arm64/kvm/hyp/pgtable.c
> > > +++ b/arch/arm64/kvm/hyp/pgtable.c
> > > @@ -746,16 +746,19 @@ static int stage2_map_walker_try_leaf(const struct kvm_pgtable_visit_ctx *ctx,
> > > return 0;
> > > }
> > >
> > > +static int stage2_map_walk_leaf(const struct kvm_pgtable_visit_ctx *ctx,
> > > + struct stage2_map_data *data);
> > > +
> > > static int stage2_map_walk_table_pre(const struct kvm_pgtable_visit_ctx *ctx,
> > > struct stage2_map_data *data)
> > > {
> > > - if (data->anchor)
> > > - return 0;
> > > + struct kvm_pgtable_mm_ops *mm_ops = ctx->mm_ops;
> > > + kvm_pte_t *childp = kvm_pte_follow(ctx->old, mm_ops);
> > > + int ret;
> > >
> > > if (!stage2_leaf_mapping_allowed(ctx, data))
> > > return 0;
> > >
> > > - data->childp = kvm_pte_follow(ctx->old, ctx->mm_ops);
> > > kvm_clear_pte(ctx->ptep);
> > >
> > > /*
> > > @@ -764,8 +767,13 @@ static int stage2_map_walk_table_pre(const struct kvm_pgtable_visit_ctx *ctx,
> > > * individually.
> > > */
> > > kvm_call_hyp(__kvm_tlb_flush_vmid, data->mmu);
> > > - data->anchor = ctx->ptep;
> > > - return 0;
> > > +
> > > + ret = stage2_map_walk_leaf(ctx, data);
> > > +
> > > + mm_ops->put_page(ctx->ptep);
> > > + mm_ops->free_removed_table(childp, ctx->level + 1);
> >
> > This could save some cycles:
> >
> > if (stage2_pte_is_counted(childp) // typo: should be childp
> > mm_ops->free_removed_table(childp, ctx->level + 1);
> >
> > as coming back using the stage2_free_walker() requires some preparation,
> > and it does the exact same check anyawy.
Ignore that, it doesn't make any difference: it is always a valid table.
> >
> > > +
> > > + return ret;
> > > }
> > >
> > > static int stage2_map_walk_leaf(const struct kvm_pgtable_visit_ctx *ctx,
> > > @@ -775,13 +783,6 @@ static int stage2_map_walk_leaf(const struct kvm_pgtable_visit_ctx *ctx,
> > > kvm_pte_t *childp;
> > > int ret;
> > >
> > > - if (data->anchor) {
> > > - if (stage2_pte_is_counted(ctx->old))
> > > - mm_ops->put_page(ctx->ptep);
> > > -
> > > - return 0;
> > > - }
> > > -
> > > ret = stage2_map_walker_try_leaf(ctx, data);
> > > if (ret != -E2BIG)
> > > return ret;
> > > @@ -810,49 +811,14 @@ static int stage2_map_walk_leaf(const struct kvm_pgtable_visit_ctx *ctx,
> > > return 0;
> > > }
> > >
> > > -static int stage2_map_walk_table_post(const struct kvm_pgtable_visit_ctx *ctx,
> > > - struct stage2_map_data *data)
> > > -{
> > > - struct kvm_pgtable_mm_ops *mm_ops = ctx->mm_ops;
> > > - kvm_pte_t *childp;
> > > - int ret = 0;
> > > -
> > > - if (!data->anchor)
> > > - return 0;
> > > -
> > > - if (data->anchor == ctx->ptep) {
> > > - childp = data->childp;
> > > - data->anchor = NULL;
> > > - data->childp = NULL;
> > > - ret = stage2_map_walk_leaf(ctx, data);
> > > - } else {
> > > - childp = kvm_pte_follow(ctx->old, mm_ops);
> > > - }
> > > -
> > > - mm_ops->put_page(childp);
> > > - mm_ops->put_page(ctx->ptep);
> > > -
> > > - return ret;
> > > -}
> > > -
> > > /*
> > > - * This is a little fiddly, as we use all three of the walk flags. The idea
> > > - * is that the TABLE_PRE callback runs for table entries on the way down,
> > > - * looking for table entries which we could conceivably replace with a
> > > - * block entry for this mapping. If it finds one, then it sets the 'anchor'
> > > - * field in 'struct stage2_map_data' to point at the table entry, before
> > > - * clearing the entry to zero and descending into the now detached table.
> > > - *
> > > - * The behaviour of the LEAF callback then depends on whether or not the
> > > - * anchor has been set. If not, then we're not using a block mapping higher
> > > - * up the table and we perform the mapping at the existing leaves instead.
> > > - * If, on the other hand, the anchor _is_ set, then we drop references to
> > > - * all valid leaves so that the pages beneath the anchor can be freed.
> > > + * The TABLE_PRE callback runs for table entries on the way down, looking
> > > + * for table entries which we could conceivably replace with a block entry
> > > + * for this mapping. If it finds one it replaces the entry and calls
> > > + * kvm_pgtable_mm_ops::free_removed_table() to tear down the detached table.
> > > *
> > > - * Finally, the TABLE_POST callback does nothing if the anchor has not
> > > - * been set, but otherwise frees the page-table pages while walking back up
> > > - * the page-table, installing the block entry when it revisits the anchor
> > > - * pointer and clearing the anchor to NULL.
> > > + * Otherwise, the LEAF callback performs the mapping at the existing leaves
> > > + * instead.
> > > */
> > > static int stage2_map_walker(const struct kvm_pgtable_visit_ctx *ctx,
> > > enum kvm_pgtable_walk_flags visit)
> > > @@ -864,11 +830,9 @@ static int stage2_map_walker(const struct kvm_pgtable_visit_ctx *ctx,
> > > return stage2_map_walk_table_pre(ctx, data);
> > > case KVM_PGTABLE_WALK_LEAF:
> > > return stage2_map_walk_leaf(ctx, data);
> > > - case KVM_PGTABLE_WALK_TABLE_POST:
> > > - return stage2_map_walk_table_post(ctx, data);
> > > + default:
> > > + return -EINVAL;
> > > }
> > > -
> > > - return -EINVAL;
> > > }
> > >
> > > int kvm_pgtable_stage2_map(struct kvm_pgtable *pgt, u64 addr, u64 size,
> > > @@ -885,8 +849,7 @@ int kvm_pgtable_stage2_map(struct kvm_pgtable *pgt, u64 addr, u64 size,
> > > struct kvm_pgtable_walker walker = {
> > > .cb = stage2_map_walker,
> > > .flags = KVM_PGTABLE_WALK_TABLE_PRE |
> > > - KVM_PGTABLE_WALK_LEAF |
> > > - KVM_PGTABLE_WALK_TABLE_POST,
> > > + KVM_PGTABLE_WALK_LEAF,
> > > .arg = &map_data,
> > > };
> > >
> > > @@ -916,8 +879,7 @@ int kvm_pgtable_stage2_set_owner(struct kvm_pgtable *pgt, u64 addr, u64 size,
> > > struct kvm_pgtable_walker walker = {
> > > .cb = stage2_map_walker,
> > > .flags = KVM_PGTABLE_WALK_TABLE_PRE |
> > > - KVM_PGTABLE_WALK_LEAF |
> > > - KVM_PGTABLE_WALK_TABLE_POST,
> > > + KVM_PGTABLE_WALK_LEAF,
> > > .arg = &map_data,
> > > };
> > >
> > > diff --git a/arch/arm64/kvm/mmu.c b/arch/arm64/kvm/mmu.c
> > > index c9a13e487187..04a25319abb0 100644
> > > --- a/arch/arm64/kvm/mmu.c
> > > +++ b/arch/arm64/kvm/mmu.c
> > > @@ -102,6 +102,13 @@ static void *kvm_host_zalloc_pages_exact(size_t size)
> > > return alloc_pages_exact(size, GFP_KERNEL_ACCOUNT | __GFP_ZERO);
> > > }
> > >
> > > +static struct kvm_pgtable_mm_ops kvm_s2_mm_ops;
> > > +
> > > +static void stage2_free_removed_table(void *addr, u32 level)
> > > +{
> > > + kvm_pgtable_stage2_free_removed(&kvm_s2_mm_ops, addr, level);
> > > +}
> > > +
> > > static void kvm_host_get_page(void *addr)
> > > {
> > > get_page(virt_to_page(addr));
> > > @@ -627,6 +634,7 @@ static struct kvm_pgtable_mm_ops kvm_s2_mm_ops = {
> > > .zalloc_page = stage2_memcache_zalloc_page,
> > > .zalloc_pages_exact = kvm_host_zalloc_pages_exact,
> > > .free_pages_exact = free_pages_exact,
> > > + .free_removed_table = stage2_free_removed_table,
> > > .get_page = kvm_host_get_page,
> > > .put_page = kvm_host_put_page,
> > > .page_count = kvm_host_page_count,
> > > --
> > > 2.38.0.rc1.362.ged0d419d3c-goog
> > >
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 27+ messages in thread
* [PATCH v2 07/15] KVM: arm64: Use an opaque type for pteps
2022-10-07 23:28 [PATCH v2 00/15] KVM: arm64: Parallel stage-2 fault handling Oliver Upton
` (5 preceding siblings ...)
2022-10-07 23:28 ` [PATCH v2 06/15] KVM: arm64: Tear down unlinked stage-2 subtree after break-before-make Oliver Upton
@ 2022-10-07 23:28 ` Oliver Upton
2022-10-19 23:17 ` Sean Christopherson
2022-10-07 23:28 ` [PATCH v2 08/15] KVM: arm64: Protect stage-2 traversal with RCU Oliver Upton
` (7 subsequent siblings)
14 siblings, 1 reply; 27+ messages in thread
From: Oliver Upton @ 2022-10-07 23:28 UTC (permalink / raw)
To: Marc Zyngier, James Morse, Alexandru Elisei
Cc: linux-arm-kernel, kvmarm, kvm, Reiji Watanabe, Ricardo Koller,
David Matlack, Quentin Perret, Ben Gardon, Gavin Shan, Peter Xu,
Will Deacon, Sean Christopherson, kvmarm, Oliver Upton
Use an opaque type for pteps and require visitors explicitly dereference
the pointer before using. Protecting page table memory with RCU requires
that KVM dereferences RCU-annotated pointers before using. However, RCU
is not available for use in the nVHE hypervisor and the opaque type can
be conditionally annotated with RCU for the stage-2 MMU.
Call the type a 'pteref' to avoid a naming collision with raw pteps. No
functional change intended.
Signed-off-by: Oliver Upton <oliver.upton@linux.dev>
---
arch/arm64/include/asm/kvm_pgtable.h | 9 ++++++++-
arch/arm64/kvm/hyp/pgtable.c | 23 ++++++++++++-----------
2 files changed, 20 insertions(+), 12 deletions(-)
diff --git a/arch/arm64/include/asm/kvm_pgtable.h b/arch/arm64/include/asm/kvm_pgtable.h
index c33edcf36b5b..beb89eac155c 100644
--- a/arch/arm64/include/asm/kvm_pgtable.h
+++ b/arch/arm64/include/asm/kvm_pgtable.h
@@ -25,6 +25,13 @@ static inline u64 kvm_get_parange(u64 mmfr0)
typedef u64 kvm_pte_t;
+typedef kvm_pte_t *kvm_pteref_t;
+
+static inline kvm_pte_t *kvm_dereference_pteref(kvm_pteref_t pteref, bool shared)
+{
+ return pteref;
+}
+
#define KVM_PTE_VALID BIT(0)
#define KVM_PTE_ADDR_MASK GENMASK(47, PAGE_SHIFT)
@@ -170,7 +177,7 @@ typedef bool (*kvm_pgtable_force_pte_cb_t)(u64 addr, u64 end,
struct kvm_pgtable {
u32 ia_bits;
u32 start_level;
- kvm_pte_t *pgd;
+ kvm_pteref_t pgd;
struct kvm_pgtable_mm_ops *mm_ops;
/* Stage-2 only */
diff --git a/arch/arm64/kvm/hyp/pgtable.c b/arch/arm64/kvm/hyp/pgtable.c
index 02c33fccb178..6b6e1ed7ee2f 100644
--- a/arch/arm64/kvm/hyp/pgtable.c
+++ b/arch/arm64/kvm/hyp/pgtable.c
@@ -175,13 +175,14 @@ static int kvm_pgtable_visitor_cb(struct kvm_pgtable_walk_data *data,
}
static int __kvm_pgtable_walk(struct kvm_pgtable_walk_data *data,
- struct kvm_pgtable_mm_ops *mm_ops, kvm_pte_t *pgtable, u32 level);
+ struct kvm_pgtable_mm_ops *mm_ops, kvm_pteref_t pgtable, u32 level);
static inline int __kvm_pgtable_visit(struct kvm_pgtable_walk_data *data,
struct kvm_pgtable_mm_ops *mm_ops,
- kvm_pte_t *ptep, u32 level)
+ kvm_pteref_t pteref, u32 level)
{
enum kvm_pgtable_walk_flags flags = data->walker->flags;
+ kvm_pte_t *ptep = kvm_dereference_pteref(pteref, false);
struct kvm_pgtable_visit_ctx ctx = {
.ptep = ptep,
.old = READ_ONCE(*ptep),
@@ -193,7 +194,7 @@ static inline int __kvm_pgtable_visit(struct kvm_pgtable_walk_data *data,
.flags = flags,
};
int ret = 0;
- kvm_pte_t *childp;
+ kvm_pteref_t childp;
bool table = kvm_pte_table(ctx.old, level);
if (table && (ctx.flags & KVM_PGTABLE_WALK_TABLE_PRE))
@@ -214,7 +215,7 @@ static inline int __kvm_pgtable_visit(struct kvm_pgtable_walk_data *data,
goto out;
}
- childp = kvm_pte_follow(ctx.old, mm_ops);
+ childp = (kvm_pteref_t)kvm_pte_follow(ctx.old, mm_ops);
ret = __kvm_pgtable_walk(data, mm_ops, childp, level + 1);
if (ret)
goto out;
@@ -227,7 +228,7 @@ static inline int __kvm_pgtable_visit(struct kvm_pgtable_walk_data *data,
}
static int __kvm_pgtable_walk(struct kvm_pgtable_walk_data *data,
- struct kvm_pgtable_mm_ops *mm_ops, kvm_pte_t *pgtable, u32 level)
+ struct kvm_pgtable_mm_ops *mm_ops, kvm_pteref_t pgtable, u32 level)
{
u32 idx;
int ret = 0;
@@ -236,12 +237,12 @@ static int __kvm_pgtable_walk(struct kvm_pgtable_walk_data *data,
return -EINVAL;
for (idx = kvm_pgtable_idx(data, level); idx < PTRS_PER_PTE; ++idx) {
- kvm_pte_t *ptep = &pgtable[idx];
+ kvm_pteref_t pteref = &pgtable[idx];
if (data->addr >= data->end)
break;
- ret = __kvm_pgtable_visit(data, mm_ops, ptep, level);
+ ret = __kvm_pgtable_visit(data, mm_ops, pteref, level);
if (ret)
break;
}
@@ -262,9 +263,9 @@ static int _kvm_pgtable_walk(struct kvm_pgtable *pgt, struct kvm_pgtable_walk_da
return -EINVAL;
for (idx = kvm_pgd_page_idx(pgt, data->addr); data->addr < data->end; ++idx) {
- kvm_pte_t *ptep = &pgt->pgd[idx * PTRS_PER_PTE];
+ kvm_pteref_t pteref = &pgt->pgd[idx * PTRS_PER_PTE];
- ret = __kvm_pgtable_walk(data, pgt->mm_ops, ptep, pgt->start_level);
+ ret = __kvm_pgtable_walk(data, pgt->mm_ops, pteref, pgt->start_level);
if (ret)
break;
}
@@ -507,7 +508,7 @@ int kvm_pgtable_hyp_init(struct kvm_pgtable *pgt, u32 va_bits,
{
u64 levels = ARM64_HW_PGTABLE_LEVELS(va_bits);
- pgt->pgd = (kvm_pte_t *)mm_ops->zalloc_page(NULL);
+ pgt->pgd = (kvm_pteref_t)mm_ops->zalloc_page(NULL);
if (!pgt->pgd)
return -ENOMEM;
@@ -1119,7 +1120,7 @@ int __kvm_pgtable_stage2_init(struct kvm_pgtable *pgt, struct kvm_s2_mmu *mmu,
u32 start_level = VTCR_EL2_TGRAN_SL0_BASE - sl0;
pgd_sz = kvm_pgd_pages(ia_bits, start_level) * PAGE_SIZE;
- pgt->pgd = mm_ops->zalloc_pages_exact(pgd_sz);
+ pgt->pgd = (kvm_pteref_t)mm_ops->zalloc_pages_exact(pgd_sz);
if (!pgt->pgd)
return -ENOMEM;
--
2.38.0.rc1.362.ged0d419d3c-goog
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 27+ messages in thread* Re: [PATCH v2 07/15] KVM: arm64: Use an opaque type for pteps
2022-10-07 23:28 ` [PATCH v2 07/15] KVM: arm64: Use an opaque type for pteps Oliver Upton
@ 2022-10-19 23:17 ` Sean Christopherson
2022-10-20 8:32 ` Oliver Upton
0 siblings, 1 reply; 27+ messages in thread
From: Sean Christopherson @ 2022-10-19 23:17 UTC (permalink / raw)
To: Oliver Upton
Cc: Marc Zyngier, James Morse, Alexandru Elisei, linux-arm-kernel,
kvmarm, kvm, Reiji Watanabe, Ricardo Koller, David Matlack,
Quentin Perret, Ben Gardon, Gavin Shan, Peter Xu, Will Deacon,
kvmarm
On Fri, Oct 07, 2022, Oliver Upton wrote:
> Use an opaque type for pteps and require visitors explicitly dereference
> the pointer before using. Protecting page table memory with RCU requires
> that KVM dereferences RCU-annotated pointers before using. However, RCU
> is not available for use in the nVHE hypervisor and the opaque type can
> be conditionally annotated with RCU for the stage-2 MMU.
>
> Call the type a 'pteref' to avoid a naming collision with raw pteps. No
> functional change intended.
>
> Signed-off-by: Oliver Upton <oliver.upton@linux.dev>
> ---
> arch/arm64/include/asm/kvm_pgtable.h | 9 ++++++++-
> arch/arm64/kvm/hyp/pgtable.c | 23 ++++++++++++-----------
> 2 files changed, 20 insertions(+), 12 deletions(-)
>
> diff --git a/arch/arm64/include/asm/kvm_pgtable.h b/arch/arm64/include/asm/kvm_pgtable.h
> index c33edcf36b5b..beb89eac155c 100644
> --- a/arch/arm64/include/asm/kvm_pgtable.h
> +++ b/arch/arm64/include/asm/kvm_pgtable.h
> @@ -25,6 +25,13 @@ static inline u64 kvm_get_parange(u64 mmfr0)
>
> typedef u64 kvm_pte_t;
>
> +typedef kvm_pte_t *kvm_pteref_t;
> +
> +static inline kvm_pte_t *kvm_dereference_pteref(kvm_pteref_t pteref, bool shared)
> +{
> + return pteref;
Returning the pointer is unsafe (when it becomes RCU-protected). The full
dereference of the data needs to occur under RCU protection, not just the retrieval
of the pointer. E.g. this (straw man) would be broken
bool table = kvm_pte_table(ctx.old, level);
rcu_read_lock();
ptep = kvm_dereference_pteref(pteref, flags & KVM_PGTABLE_WALK_SHARED);
rcu_read_unlock();
if (table && (ctx.flags & KVM_PGTABLE_WALK_TABLE_PRE))
ret = kvm_pgtable_visitor_cb(data, &ctx, KVM_PGTABLE_WALK_TABLE_PRE);
if (!table && (ctx.flags & KVM_PGTABLE_WALK_LEAF)) {
ret = kvm_pgtable_visitor_cb(data, &ctx, KVM_PGTABLE_WALK_LEAF);
ctx.old = READ_ONCE(*ptep);
table = kvm_pte_table(ctx.old, level);
}
as the read of the entry pointed at by ptep could be to a page table that is freed
in an RCU callback.
The naming collision you are trying to avoid is a symptom of this bad pattern,
as there should never be "raw" pteps floating around, at least not in non-pKVM
contexts that utilize RCU.
> +}
> +
> #define KVM_PTE_VALID BIT(0)
>
> #define KVM_PTE_ADDR_MASK GENMASK(47, PAGE_SHIFT)
> @@ -170,7 +177,7 @@ typedef bool (*kvm_pgtable_force_pte_cb_t)(u64 addr, u64 end,
> struct kvm_pgtable {
> u32 ia_bits;
> u32 start_level;
> - kvm_pte_t *pgd;
> + kvm_pteref_t pgd;
> struct kvm_pgtable_mm_ops *mm_ops;
>
> /* Stage-2 only */
> diff --git a/arch/arm64/kvm/hyp/pgtable.c b/arch/arm64/kvm/hyp/pgtable.c
> index 02c33fccb178..6b6e1ed7ee2f 100644
> --- a/arch/arm64/kvm/hyp/pgtable.c
> +++ b/arch/arm64/kvm/hyp/pgtable.c
> @@ -175,13 +175,14 @@ static int kvm_pgtable_visitor_cb(struct kvm_pgtable_walk_data *data,
> }
>
> static int __kvm_pgtable_walk(struct kvm_pgtable_walk_data *data,
> - struct kvm_pgtable_mm_ops *mm_ops, kvm_pte_t *pgtable, u32 level);
> + struct kvm_pgtable_mm_ops *mm_ops, kvm_pteref_t pgtable, u32 level);
>
> static inline int __kvm_pgtable_visit(struct kvm_pgtable_walk_data *data,
> struct kvm_pgtable_mm_ops *mm_ops,
> - kvm_pte_t *ptep, u32 level)
> + kvm_pteref_t pteref, u32 level)
> {
> enum kvm_pgtable_walk_flags flags = data->walker->flags;
> + kvm_pte_t *ptep = kvm_dereference_pteref(pteref, false);
> struct kvm_pgtable_visit_ctx ctx = {
> .ptep = ptep,
> .old = READ_ONCE(*ptep),
This is where you want the protection to kick in, e.g.
typedef kvm_pte_t __rcu *kvm_ptep_t;
static inline kvm_pte_t kvm_read_pte(kvm_ptep_t ptep)
{
return READ_ONCE(*rcu_dereference(ptep));
}
.old = kvm_read_pte(ptep),
In other words, the pointer itself isn't that's protected, it's PTE that the
pointer points at that's protected.
rcu_dereference() has no overhead when CONFIG_PROVE_RCU=n, i.e. there's no reason
to "optimize" dereferences.
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 27+ messages in thread* Re: [PATCH v2 07/15] KVM: arm64: Use an opaque type for pteps
2022-10-19 23:17 ` Sean Christopherson
@ 2022-10-20 8:32 ` Oliver Upton
2022-10-27 22:31 ` Oliver Upton
0 siblings, 1 reply; 27+ messages in thread
From: Oliver Upton @ 2022-10-20 8:32 UTC (permalink / raw)
To: Sean Christopherson
Cc: Marc Zyngier, James Morse, Alexandru Elisei, linux-arm-kernel,
kvmarm, kvm, Reiji Watanabe, Ricardo Koller, David Matlack,
Quentin Perret, Ben Gardon, Gavin Shan, Peter Xu, Will Deacon,
kvmarm
On Wed, Oct 19, 2022 at 11:17:43PM +0000, Sean Christopherson wrote:
> On Fri, Oct 07, 2022, Oliver Upton wrote:
> > Use an opaque type for pteps and require visitors explicitly dereference
> > the pointer before using. Protecting page table memory with RCU requires
> > that KVM dereferences RCU-annotated pointers before using. However, RCU
> > is not available for use in the nVHE hypervisor and the opaque type can
> > be conditionally annotated with RCU for the stage-2 MMU.
> >
> > Call the type a 'pteref' to avoid a naming collision with raw pteps. No
> > functional change intended.
> >
> > Signed-off-by: Oliver Upton <oliver.upton@linux.dev>
> > ---
> > arch/arm64/include/asm/kvm_pgtable.h | 9 ++++++++-
> > arch/arm64/kvm/hyp/pgtable.c | 23 ++++++++++++-----------
> > 2 files changed, 20 insertions(+), 12 deletions(-)
> >
> > diff --git a/arch/arm64/include/asm/kvm_pgtable.h b/arch/arm64/include/asm/kvm_pgtable.h
> > index c33edcf36b5b..beb89eac155c 100644
> > --- a/arch/arm64/include/asm/kvm_pgtable.h
> > +++ b/arch/arm64/include/asm/kvm_pgtable.h
> > @@ -25,6 +25,13 @@ static inline u64 kvm_get_parange(u64 mmfr0)
> >
> > typedef u64 kvm_pte_t;
> >
> > +typedef kvm_pte_t *kvm_pteref_t;
> > +
> > +static inline kvm_pte_t *kvm_dereference_pteref(kvm_pteref_t pteref, bool shared)
> > +{
> > + return pteref;
>
> Returning the pointer is unsafe (when it becomes RCU-protected). The full
> dereference of the data needs to occur under RCU protection, not just the retrieval
> of the pointer. E.g. this (straw man) would be broken
>
> bool table = kvm_pte_table(ctx.old, level);
>
> rcu_read_lock();
> ptep = kvm_dereference_pteref(pteref, flags & KVM_PGTABLE_WALK_SHARED);
> rcu_read_unlock();
>
> if (table && (ctx.flags & KVM_PGTABLE_WALK_TABLE_PRE))
> ret = kvm_pgtable_visitor_cb(data, &ctx, KVM_PGTABLE_WALK_TABLE_PRE);
>
> if (!table && (ctx.flags & KVM_PGTABLE_WALK_LEAF)) {
> ret = kvm_pgtable_visitor_cb(data, &ctx, KVM_PGTABLE_WALK_LEAF);
> ctx.old = READ_ONCE(*ptep);
> table = kvm_pte_table(ctx.old, level);
> }
>
> as the read of the entry pointed at by ptep could be to a page table that is freed
> in an RCU callback.
>
> The naming collision you are trying to avoid is a symptom of this bad pattern,
> as there should never be "raw" pteps floating around, at least not in non-pKVM
> contexts that utilize RCU.
Fair enough, this was mostly from an attempt to avoid churn in the
visitor callbacks by adding more layering for reads/writes through RCU
pointers. The lifetime of the raw pointer is 'safe' as it sits on the
stack and we go behind the rcu lock much further up.
> > +}
> > +
> > #define KVM_PTE_VALID BIT(0)
> >
> > #define KVM_PTE_ADDR_MASK GENMASK(47, PAGE_SHIFT)
> > @@ -170,7 +177,7 @@ typedef bool (*kvm_pgtable_force_pte_cb_t)(u64 addr, u64 end,
> > struct kvm_pgtable {
> > u32 ia_bits;
> > u32 start_level;
> > - kvm_pte_t *pgd;
> > + kvm_pteref_t pgd;
> > struct kvm_pgtable_mm_ops *mm_ops;
> >
> > /* Stage-2 only */
> > diff --git a/arch/arm64/kvm/hyp/pgtable.c b/arch/arm64/kvm/hyp/pgtable.c
> > index 02c33fccb178..6b6e1ed7ee2f 100644
> > --- a/arch/arm64/kvm/hyp/pgtable.c
> > +++ b/arch/arm64/kvm/hyp/pgtable.c
> > @@ -175,13 +175,14 @@ static int kvm_pgtable_visitor_cb(struct kvm_pgtable_walk_data *data,
> > }
> >
> > static int __kvm_pgtable_walk(struct kvm_pgtable_walk_data *data,
> > - struct kvm_pgtable_mm_ops *mm_ops, kvm_pte_t *pgtable, u32 level);
> > + struct kvm_pgtable_mm_ops *mm_ops, kvm_pteref_t pgtable, u32 level);
> >
> > static inline int __kvm_pgtable_visit(struct kvm_pgtable_walk_data *data,
> > struct kvm_pgtable_mm_ops *mm_ops,
> > - kvm_pte_t *ptep, u32 level)
> > + kvm_pteref_t pteref, u32 level)
> > {
> > enum kvm_pgtable_walk_flags flags = data->walker->flags;
> > + kvm_pte_t *ptep = kvm_dereference_pteref(pteref, false);
> > struct kvm_pgtable_visit_ctx ctx = {
> > .ptep = ptep,
> > .old = READ_ONCE(*ptep),
>
> This is where you want the protection to kick in, e.g.
>
> typedef kvm_pte_t __rcu *kvm_ptep_t;
>
> static inline kvm_pte_t kvm_read_pte(kvm_ptep_t ptep)
> {
> return READ_ONCE(*rcu_dereference(ptep));
> }
>
> .old = kvm_read_pte(ptep),
>
> In other words, the pointer itself isn't that's protected, it's PTE that the
> pointer points at that's protected.
Right, but practically speaking it is the boundary at which we assert
that protection.
Anyhow, I'll look at abstracting the actual memory accesses in the
visitors without too much mess.
--
Thanks,
Oliver
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 27+ messages in thread* Re: [PATCH v2 07/15] KVM: arm64: Use an opaque type for pteps
2022-10-20 8:32 ` Oliver Upton
@ 2022-10-27 22:31 ` Oliver Upton
0 siblings, 0 replies; 27+ messages in thread
From: Oliver Upton @ 2022-10-27 22:31 UTC (permalink / raw)
To: Sean Christopherson
Cc: kvm, Marc Zyngier, Will Deacon, kvmarm, Ben Gardon, David Matlack,
kvmarm, linux-arm-kernel
On Thu, Oct 20, 2022 at 11:32:28AM +0300, Oliver Upton wrote:
> On Wed, Oct 19, 2022 at 11:17:43PM +0000, Sean Christopherson wrote:
> > On Fri, Oct 07, 2022, Oliver Upton wrote:
[...]
> > > diff --git a/arch/arm64/kvm/hyp/pgtable.c b/arch/arm64/kvm/hyp/pgtable.c
> > > index 02c33fccb178..6b6e1ed7ee2f 100644
> > > --- a/arch/arm64/kvm/hyp/pgtable.c
> > > +++ b/arch/arm64/kvm/hyp/pgtable.c
> > > @@ -175,13 +175,14 @@ static int kvm_pgtable_visitor_cb(struct kvm_pgtable_walk_data *data,
> > > }
> > >
> > > static int __kvm_pgtable_walk(struct kvm_pgtable_walk_data *data,
> > > - struct kvm_pgtable_mm_ops *mm_ops, kvm_pte_t *pgtable, u32 level);
> > > + struct kvm_pgtable_mm_ops *mm_ops, kvm_pteref_t pgtable, u32 level);
> > >
> > > static inline int __kvm_pgtable_visit(struct kvm_pgtable_walk_data *data,
> > > struct kvm_pgtable_mm_ops *mm_ops,
> > > - kvm_pte_t *ptep, u32 level)
> > > + kvm_pteref_t pteref, u32 level)
> > > {
> > > enum kvm_pgtable_walk_flags flags = data->walker->flags;
> > > + kvm_pte_t *ptep = kvm_dereference_pteref(pteref, false);
> > > struct kvm_pgtable_visit_ctx ctx = {
> > > .ptep = ptep,
> > > .old = READ_ONCE(*ptep),
> >
> > This is where you want the protection to kick in, e.g.
> >
> > typedef kvm_pte_t __rcu *kvm_ptep_t;
> >
> > static inline kvm_pte_t kvm_read_pte(kvm_ptep_t ptep)
> > {
> > return READ_ONCE(*rcu_dereference(ptep));
> > }
> >
> > .old = kvm_read_pte(ptep),
> >
> > In other words, the pointer itself isn't that's protected, it's PTE that the
> > pointer points at that's protected.
>
> Right, but practically speaking it is the boundary at which we assert
> that protection.
>
> Anyhow, I'll look at abstracting the actual memory accesses in the
> visitors without too much mess.
Took this in a slightly different direction after playing with it for a
while. Abstracting all PTE accesses adds a lot of churn to the series.
Adding in an assertion before invoking a visitor callback (i.e. when the
raw pointer is about to be used) provides a similar degree of assurance
that we are indeed RCU-safe.
--
Thanks,
Oliver
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 27+ messages in thread
* [PATCH v2 08/15] KVM: arm64: Protect stage-2 traversal with RCU
2022-10-07 23:28 [PATCH v2 00/15] KVM: arm64: Parallel stage-2 fault handling Oliver Upton
` (6 preceding siblings ...)
2022-10-07 23:28 ` [PATCH v2 07/15] KVM: arm64: Use an opaque type for pteps Oliver Upton
@ 2022-10-07 23:28 ` Oliver Upton
2022-10-19 23:29 ` Sean Christopherson
2022-10-07 23:28 ` [PATCH v2 09/15] KVM: arm64: Free removed stage-2 tables in RCU callback Oliver Upton
` (6 subsequent siblings)
14 siblings, 1 reply; 27+ messages in thread
From: Oliver Upton @ 2022-10-07 23:28 UTC (permalink / raw)
To: Marc Zyngier, James Morse, Alexandru Elisei
Cc: linux-arm-kernel, kvmarm, kvm, Reiji Watanabe, Ricardo Koller,
David Matlack, Quentin Perret, Ben Gardon, Gavin Shan, Peter Xu,
Will Deacon, Sean Christopherson, kvmarm, Oliver Upton
The use of RCU is necessary to safely change the stage-2 page tables in
parallel. Acquire and release the RCU read lock when traversing the page
tables.
Use the _raw() flavor of rcu_dereference when changes to the page tables
are otherwise protected from parallel software walkers (e.g. holding the
write lock).
Signed-off-by: Oliver Upton <oliver.upton@linux.dev>
---
arch/arm64/include/asm/kvm_pgtable.h | 34 ++++++++++++++++++++++++++++
arch/arm64/kvm/hyp/pgtable.c | 7 +++++-
2 files changed, 40 insertions(+), 1 deletion(-)
diff --git a/arch/arm64/include/asm/kvm_pgtable.h b/arch/arm64/include/asm/kvm_pgtable.h
index beb89eac155c..60c37e5e77dd 100644
--- a/arch/arm64/include/asm/kvm_pgtable.h
+++ b/arch/arm64/include/asm/kvm_pgtable.h
@@ -25,6 +25,13 @@ static inline u64 kvm_get_parange(u64 mmfr0)
typedef u64 kvm_pte_t;
+/*
+ * RCU cannot be used in a non-kernel context such as the hyp. As such, page
+ * table walkers used in hyp do not call into RCU and instead use other
+ * synchronization mechanisms (such as a spinlock).
+ */
+#if defined(__KVM_NVHE_HYPERVISOR__) || defined(__KVM_VHE_HYPERVISOR__)
+
typedef kvm_pte_t *kvm_pteref_t;
static inline kvm_pte_t *kvm_dereference_pteref(kvm_pteref_t pteref, bool shared)
@@ -32,6 +39,33 @@ static inline kvm_pte_t *kvm_dereference_pteref(kvm_pteref_t pteref, bool shared
return pteref;
}
+static inline void kvm_pgtable_walk_begin(void) {}
+static inline void kvm_pgtable_walk_end(void) {}
+
+#else
+
+typedef kvm_pte_t __rcu *kvm_pteref_t;
+
+static inline kvm_pte_t *kvm_dereference_pteref(kvm_pteref_t pteref, bool shared)
+{
+ if (shared)
+ return rcu_dereference(pteref);
+
+ return rcu_dereference_raw(pteref);
+}
+
+static inline void kvm_pgtable_walk_begin(void)
+{
+ rcu_read_lock();
+}
+
+static inline void kvm_pgtable_walk_end(void)
+{
+ rcu_read_unlock();
+}
+
+#endif
+
#define KVM_PTE_VALID BIT(0)
#define KVM_PTE_ADDR_MASK GENMASK(47, PAGE_SHIFT)
diff --git a/arch/arm64/kvm/hyp/pgtable.c b/arch/arm64/kvm/hyp/pgtable.c
index 6b6e1ed7ee2f..c2be15850497 100644
--- a/arch/arm64/kvm/hyp/pgtable.c
+++ b/arch/arm64/kvm/hyp/pgtable.c
@@ -281,8 +281,13 @@ int kvm_pgtable_walk(struct kvm_pgtable *pgt, u64 addr, u64 size,
.end = PAGE_ALIGN(walk_data.addr + size),
.walker = walker,
};
+ int r;
- return _kvm_pgtable_walk(pgt, &walk_data);
+ kvm_pgtable_walk_begin();
+ r = _kvm_pgtable_walk(pgt, &walk_data);
+ kvm_pgtable_walk_end();
+
+ return r;
}
struct leaf_walk_data {
--
2.38.0.rc1.362.ged0d419d3c-goog
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 27+ messages in thread* Re: [PATCH v2 08/15] KVM: arm64: Protect stage-2 traversal with RCU
2022-10-07 23:28 ` [PATCH v2 08/15] KVM: arm64: Protect stage-2 traversal with RCU Oliver Upton
@ 2022-10-19 23:29 ` Sean Christopherson
2022-10-20 8:34 ` Oliver Upton
0 siblings, 1 reply; 27+ messages in thread
From: Sean Christopherson @ 2022-10-19 23:29 UTC (permalink / raw)
To: Oliver Upton
Cc: Marc Zyngier, James Morse, Alexandru Elisei, linux-arm-kernel,
kvmarm, kvm, Reiji Watanabe, Ricardo Koller, David Matlack,
Quentin Perret, Ben Gardon, Gavin Shan, Peter Xu, Will Deacon,
kvmarm
On Fri, Oct 07, 2022, Oliver Upton wrote:
> The use of RCU is necessary to safely change the stage-2 page tables in
> parallel. Acquire and release the RCU read lock when traversing the page
> tables.
>
> Use the _raw() flavor of rcu_dereference when changes to the page tables
> are otherwise protected from parallel software walkers (e.g. holding the
> write lock).
>
> Signed-off-by: Oliver Upton <oliver.upton@linux.dev>
> ---
...
> @@ -32,6 +39,33 @@ static inline kvm_pte_t *kvm_dereference_pteref(kvm_pteref_t pteref, bool shared
> return pteref;
> }
>
> +static inline void kvm_pgtable_walk_begin(void) {}
> +static inline void kvm_pgtable_walk_end(void) {}
> +
> +#else
> +
> +typedef kvm_pte_t __rcu *kvm_pteref_t;
> +
> +static inline kvm_pte_t *kvm_dereference_pteref(kvm_pteref_t pteref, bool shared)
> +{
> + if (shared)
> + return rcu_dereference(pteref);
> +
> + return rcu_dereference_raw(pteref);
Rather than use raw, use rcu_dereference_check(). If you can plumb down @kvm or
@mmu_lock, the ideal check would be (apparently there's no lockdep_is_held_write()
wrapper?)
return READ_ONCE(*rcu_dereference_check(ptep, lockdep_is_held_type(mmu_lock, 0)));
If getting at mmu_lock is too difficult, this can still be
return READ_ONCE(*rcu_dereference_check(ptep, !shared);
Doubt it matters for code generation, but IMO it's cleaner overall.
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 27+ messages in thread* Re: [PATCH v2 08/15] KVM: arm64: Protect stage-2 traversal with RCU
2022-10-19 23:29 ` Sean Christopherson
@ 2022-10-20 8:34 ` Oliver Upton
0 siblings, 0 replies; 27+ messages in thread
From: Oliver Upton @ 2022-10-20 8:34 UTC (permalink / raw)
To: Sean Christopherson
Cc: Marc Zyngier, James Morse, Alexandru Elisei, linux-arm-kernel,
kvmarm, kvm, Reiji Watanabe, Ricardo Koller, David Matlack,
Quentin Perret, Ben Gardon, Gavin Shan, Peter Xu, Will Deacon,
kvmarm
On Wed, Oct 19, 2022 at 11:29:56PM +0000, Sean Christopherson wrote:
> On Fri, Oct 07, 2022, Oliver Upton wrote:
> > The use of RCU is necessary to safely change the stage-2 page tables in
> > parallel. Acquire and release the RCU read lock when traversing the page
> > tables.
> >
> > Use the _raw() flavor of rcu_dereference when changes to the page tables
> > are otherwise protected from parallel software walkers (e.g. holding the
> > write lock).
> >
> > Signed-off-by: Oliver Upton <oliver.upton@linux.dev>
> > ---
>
> ...
>
> > @@ -32,6 +39,33 @@ static inline kvm_pte_t *kvm_dereference_pteref(kvm_pteref_t pteref, bool shared
> > return pteref;
> > }
> >
> > +static inline void kvm_pgtable_walk_begin(void) {}
> > +static inline void kvm_pgtable_walk_end(void) {}
> > +
> > +#else
> > +
> > +typedef kvm_pte_t __rcu *kvm_pteref_t;
> > +
> > +static inline kvm_pte_t *kvm_dereference_pteref(kvm_pteref_t pteref, bool shared)
> > +{
> > + if (shared)
> > + return rcu_dereference(pteref);
> > +
> > + return rcu_dereference_raw(pteref);
>
> Rather than use raw, use rcu_dereference_check(). If you can plumb down @kvm or
> @mmu_lock, the ideal check would be (apparently there's no lockdep_is_held_write()
> wrapper?)
>
> return READ_ONCE(*rcu_dereference_check(ptep, lockdep_is_held_type(mmu_lock, 0)));
>
> If getting at mmu_lock is too difficult, this can still be
>
> return READ_ONCE(*rcu_dereference_check(ptep, !shared);
>
> Doubt it matters for code generation, but IMO it's cleaner overall.
As the page table walkers can be used outside of the context of a VM
(such as hyp stage-1), I think option #2 is probably a bit easier.
--
Thanks,
Oliver
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 27+ messages in thread
* [PATCH v2 09/15] KVM: arm64: Free removed stage-2 tables in RCU callback
2022-10-07 23:28 [PATCH v2 00/15] KVM: arm64: Parallel stage-2 fault handling Oliver Upton
` (7 preceding siblings ...)
2022-10-07 23:28 ` [PATCH v2 08/15] KVM: arm64: Protect stage-2 traversal with RCU Oliver Upton
@ 2022-10-07 23:28 ` Oliver Upton
2022-10-07 23:28 ` [PATCH v2 10/15] KVM: arm64: Atomically update stage 2 leaf attributes in parallel walks Oliver Upton
` (5 subsequent siblings)
14 siblings, 0 replies; 27+ messages in thread
From: Oliver Upton @ 2022-10-07 23:28 UTC (permalink / raw)
To: Marc Zyngier, James Morse, Alexandru Elisei
Cc: linux-arm-kernel, kvmarm, kvm, Reiji Watanabe, Ricardo Koller,
David Matlack, Quentin Perret, Ben Gardon, Gavin Shan, Peter Xu,
Will Deacon, Sean Christopherson, kvmarm, Oliver Upton
There is no real urgency to free a stage-2 subtree that was pruned.
Nonetheless, KVM does the tear down in the stage-2 fault path while
holding the MMU lock.
Free removed stage-2 subtrees after an RCU grace period. To guarantee
all stage-2 table pages are freed before killing a VM, add an
rcu_barrier() to the flush path.
Signed-off-by: Oliver Upton <oliver.upton@linux.dev>
---
arch/arm64/kvm/mmu.c | 14 +++++++++++++-
1 file changed, 13 insertions(+), 1 deletion(-)
diff --git a/arch/arm64/kvm/mmu.c b/arch/arm64/kvm/mmu.c
index 04a25319abb0..66eede0fbb36 100644
--- a/arch/arm64/kvm/mmu.c
+++ b/arch/arm64/kvm/mmu.c
@@ -104,9 +104,21 @@ static void *kvm_host_zalloc_pages_exact(size_t size)
static struct kvm_pgtable_mm_ops kvm_s2_mm_ops;
+static void stage2_free_removed_table_rcu_cb(struct rcu_head *head)
+{
+ struct page *page = container_of(head, struct page, rcu_head);
+ void *pgtable = page_to_virt(page);
+ u32 level = page_private(page);
+
+ kvm_pgtable_stage2_free_removed(&kvm_s2_mm_ops, pgtable, level);
+}
+
static void stage2_free_removed_table(void *addr, u32 level)
{
- kvm_pgtable_stage2_free_removed(&kvm_s2_mm_ops, addr, level);
+ struct page *page = virt_to_page(addr);
+
+ set_page_private(page, (unsigned long)level);
+ call_rcu(&page->rcu_head, stage2_free_removed_table_rcu_cb);
}
static void kvm_host_get_page(void *addr)
--
2.38.0.rc1.362.ged0d419d3c-goog
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 27+ messages in thread* [PATCH v2 10/15] KVM: arm64: Atomically update stage 2 leaf attributes in parallel walks
2022-10-07 23:28 [PATCH v2 00/15] KVM: arm64: Parallel stage-2 fault handling Oliver Upton
` (8 preceding siblings ...)
2022-10-07 23:28 ` [PATCH v2 09/15] KVM: arm64: Free removed stage-2 tables in RCU callback Oliver Upton
@ 2022-10-07 23:28 ` Oliver Upton
2022-10-07 23:30 ` [PATCH v2 11/15] KVM: arm64: Split init and set for table PTE Oliver Upton
` (4 subsequent siblings)
14 siblings, 0 replies; 27+ messages in thread
From: Oliver Upton @ 2022-10-07 23:28 UTC (permalink / raw)
To: Marc Zyngier, James Morse, Alexandru Elisei
Cc: linux-arm-kernel, kvmarm, kvm, Reiji Watanabe, Ricardo Koller,
David Matlack, Quentin Perret, Ben Gardon, Gavin Shan, Peter Xu,
Will Deacon, Sean Christopherson, kvmarm, Oliver Upton
The stage2 attr walker is already used for parallel walks. Since commit
f783ef1c0e82 ("KVM: arm64: Add fast path to handle permission relaxation
during dirty logging"), KVM acquires the read lock when
write-unprotecting a PTE. However, the walker only uses a simple store
to update the PTE. This is safe as the only possible race is with
hardware updates to the access flag, which is benign.
However, a subsequent change to KVM will allow more changes to the stage
2 page tables to be done in parallel. Prepare the stage 2 attribute
walker by performing atomic updates to the PTE when walking in parallel.
Signed-off-by: Oliver Upton <oliver.upton@linux.dev>
---
arch/arm64/include/asm/kvm_pgtable.h | 8 +++++++
arch/arm64/kvm/hyp/pgtable.c | 31 ++++++++++++++++++++--------
2 files changed, 30 insertions(+), 9 deletions(-)
diff --git a/arch/arm64/include/asm/kvm_pgtable.h b/arch/arm64/include/asm/kvm_pgtable.h
index 60c37e5e77dd..f9a80354fc8e 100644
--- a/arch/arm64/include/asm/kvm_pgtable.h
+++ b/arch/arm64/include/asm/kvm_pgtable.h
@@ -228,11 +228,14 @@ struct kvm_pgtable {
* children.
* @KVM_PGTABLE_WALK_TABLE_POST: Visit table entries after their
* children.
+ * @KVM_PGTABLE_WALK_SHARED: Indicates the page-tables may be shared
+ * with other software walkers.
*/
enum kvm_pgtable_walk_flags {
KVM_PGTABLE_WALK_LEAF = BIT(0),
KVM_PGTABLE_WALK_TABLE_PRE = BIT(1),
KVM_PGTABLE_WALK_TABLE_POST = BIT(2),
+ KVM_PGTABLE_WALK_SHARED = BIT(3),
};
struct kvm_pgtable_visit_ctx {
@@ -249,6 +252,11 @@ struct kvm_pgtable_visit_ctx {
typedef int (*kvm_pgtable_visitor_fn_t)(const struct kvm_pgtable_visit_ctx *ctx,
enum kvm_pgtable_walk_flags visit);
+static inline bool kvm_pgtable_walk_shared(const struct kvm_pgtable_visit_ctx *ctx)
+{
+ return ctx->flags & KVM_PGTABLE_WALK_SHARED;
+}
+
/**
* struct kvm_pgtable_walker - Hook into a page-table walk.
* @cb: Callback function to invoke during the walk.
diff --git a/arch/arm64/kvm/hyp/pgtable.c b/arch/arm64/kvm/hyp/pgtable.c
index c2be15850497..8aebddcdd593 100644
--- a/arch/arm64/kvm/hyp/pgtable.c
+++ b/arch/arm64/kvm/hyp/pgtable.c
@@ -182,7 +182,7 @@ static inline int __kvm_pgtable_visit(struct kvm_pgtable_walk_data *data,
kvm_pteref_t pteref, u32 level)
{
enum kvm_pgtable_walk_flags flags = data->walker->flags;
- kvm_pte_t *ptep = kvm_dereference_pteref(pteref, false);
+ kvm_pte_t *ptep = kvm_dereference_pteref(pteref, flags & KVM_PGTABLE_WALK_SHARED);
struct kvm_pgtable_visit_ctx ctx = {
.ptep = ptep,
.old = READ_ONCE(*ptep),
@@ -672,6 +672,16 @@ static bool stage2_pte_is_counted(kvm_pte_t pte)
return !!pte;
}
+static bool stage2_try_set_pte(const struct kvm_pgtable_visit_ctx *ctx, kvm_pte_t new)
+{
+ if (!kvm_pgtable_walk_shared(ctx)) {
+ WRITE_ONCE(*ctx->ptep, new);
+ return true;
+ }
+
+ return cmpxchg(ctx->ptep, ctx->old, new) == ctx->old;
+}
+
static void stage2_put_pte(const struct kvm_pgtable_visit_ctx *ctx, struct kvm_s2_mmu *mmu,
struct kvm_pgtable_mm_ops *mm_ops)
{
@@ -986,7 +996,9 @@ static int stage2_attr_walker(const struct kvm_pgtable_visit_ctx *ctx,
stage2_pte_executable(pte) && !stage2_pte_executable(ctx->old))
mm_ops->icache_inval_pou(kvm_pte_follow(pte, mm_ops),
kvm_granule_size(ctx->level));
- WRITE_ONCE(*ctx->ptep, pte);
+
+ if (!stage2_try_set_pte(ctx, pte))
+ return -EAGAIN;
}
return 0;
@@ -995,7 +1007,7 @@ static int stage2_attr_walker(const struct kvm_pgtable_visit_ctx *ctx,
static int stage2_update_leaf_attrs(struct kvm_pgtable *pgt, u64 addr,
u64 size, kvm_pte_t attr_set,
kvm_pte_t attr_clr, kvm_pte_t *orig_pte,
- u32 *level)
+ u32 *level, enum kvm_pgtable_walk_flags flags)
{
int ret;
kvm_pte_t attr_mask = KVM_PTE_LEAF_ATTR_LO | KVM_PTE_LEAF_ATTR_HI;
@@ -1006,7 +1018,7 @@ static int stage2_update_leaf_attrs(struct kvm_pgtable *pgt, u64 addr,
struct kvm_pgtable_walker walker = {
.cb = stage2_attr_walker,
.arg = &data,
- .flags = KVM_PGTABLE_WALK_LEAF,
+ .flags = flags | KVM_PGTABLE_WALK_LEAF,
};
ret = kvm_pgtable_walk(pgt, addr, size, &walker);
@@ -1025,14 +1037,14 @@ int kvm_pgtable_stage2_wrprotect(struct kvm_pgtable *pgt, u64 addr, u64 size)
{
return stage2_update_leaf_attrs(pgt, addr, size, 0,
KVM_PTE_LEAF_ATTR_LO_S2_S2AP_W,
- NULL, NULL);
+ NULL, NULL, 0);
}
kvm_pte_t kvm_pgtable_stage2_mkyoung(struct kvm_pgtable *pgt, u64 addr)
{
kvm_pte_t pte = 0;
stage2_update_leaf_attrs(pgt, addr, 1, KVM_PTE_LEAF_ATTR_LO_S2_AF, 0,
- &pte, NULL);
+ &pte, NULL, 0);
dsb(ishst);
return pte;
}
@@ -1041,7 +1053,7 @@ kvm_pte_t kvm_pgtable_stage2_mkold(struct kvm_pgtable *pgt, u64 addr)
{
kvm_pte_t pte = 0;
stage2_update_leaf_attrs(pgt, addr, 1, 0, KVM_PTE_LEAF_ATTR_LO_S2_AF,
- &pte, NULL);
+ &pte, NULL, 0);
/*
* "But where's the TLBI?!", you scream.
* "Over in the core code", I sigh.
@@ -1054,7 +1066,7 @@ kvm_pte_t kvm_pgtable_stage2_mkold(struct kvm_pgtable *pgt, u64 addr)
bool kvm_pgtable_stage2_is_young(struct kvm_pgtable *pgt, u64 addr)
{
kvm_pte_t pte = 0;
- stage2_update_leaf_attrs(pgt, addr, 1, 0, 0, &pte, NULL);
+ stage2_update_leaf_attrs(pgt, addr, 1, 0, 0, &pte, NULL, 0);
return pte & KVM_PTE_LEAF_ATTR_LO_S2_AF;
}
@@ -1077,7 +1089,8 @@ int kvm_pgtable_stage2_relax_perms(struct kvm_pgtable *pgt, u64 addr,
if (prot & KVM_PGTABLE_PROT_X)
clr |= KVM_PTE_LEAF_ATTR_HI_S2_XN;
- ret = stage2_update_leaf_attrs(pgt, addr, 1, set, clr, NULL, &level);
+ ret = stage2_update_leaf_attrs(pgt, addr, 1, set, clr, NULL, &level,
+ KVM_PGTABLE_WALK_SHARED);
if (!ret)
kvm_call_hyp(__kvm_tlb_flush_vmid_ipa, pgt->mmu, addr, level);
return ret;
--
2.38.0.rc1.362.ged0d419d3c-goog
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 27+ messages in thread* [PATCH v2 11/15] KVM: arm64: Split init and set for table PTE
2022-10-07 23:28 [PATCH v2 00/15] KVM: arm64: Parallel stage-2 fault handling Oliver Upton
` (9 preceding siblings ...)
2022-10-07 23:28 ` [PATCH v2 10/15] KVM: arm64: Atomically update stage 2 leaf attributes in parallel walks Oliver Upton
@ 2022-10-07 23:30 ` Oliver Upton
2022-10-07 23:31 ` [PATCH v2 12/15] KVM: arm64: Make block->table PTE changes parallel-aware Oliver Upton
` (3 subsequent siblings)
14 siblings, 0 replies; 27+ messages in thread
From: Oliver Upton @ 2022-10-07 23:30 UTC (permalink / raw)
To: Marc Zyngier, James Morse, Alexandru Elisei
Cc: linux-arm-kernel, kvmarm, kvm, Reiji Watanabe, Ricardo Koller,
David Matlack, Quentin Perret, Ben Gardon, Gavin Shan, Peter Xu,
Will Deacon, Sean Christopherson, kvmarm, Oliver Upton
Create a helper to initialize a table and directly call
smp_store_release() to install it (for now). Prepare for a subsequent
change that generalizes PTE writes with a helper.
Signed-off-by: Oliver Upton <oliver.upton@linux.dev>
---
arch/arm64/kvm/hyp/pgtable.c | 18 +++++++++---------
1 file changed, 9 insertions(+), 9 deletions(-)
diff --git a/arch/arm64/kvm/hyp/pgtable.c b/arch/arm64/kvm/hyp/pgtable.c
index 8aebddcdd593..f649a2251a35 100644
--- a/arch/arm64/kvm/hyp/pgtable.c
+++ b/arch/arm64/kvm/hyp/pgtable.c
@@ -136,16 +136,13 @@ static void kvm_clear_pte(kvm_pte_t *ptep)
WRITE_ONCE(*ptep, 0);
}
-static void kvm_set_table_pte(kvm_pte_t *ptep, kvm_pte_t *childp,
- struct kvm_pgtable_mm_ops *mm_ops)
+static kvm_pte_t kvm_init_table_pte(kvm_pte_t *childp, struct kvm_pgtable_mm_ops *mm_ops)
{
- kvm_pte_t old = *ptep, pte = kvm_phys_to_pte(mm_ops->virt_to_phys(childp));
+ kvm_pte_t pte = kvm_phys_to_pte(mm_ops->virt_to_phys(childp));
pte |= FIELD_PREP(KVM_PTE_TYPE, KVM_PTE_TYPE_TABLE);
pte |= KVM_PTE_VALID;
-
- WARN_ON(kvm_pte_valid(old));
- smp_store_release(ptep, pte);
+ return pte;
}
static kvm_pte_t kvm_init_valid_leaf_pte(u64 pa, kvm_pte_t attr, u32 level)
@@ -410,7 +407,7 @@ static bool hyp_map_walker_try_leaf(const struct kvm_pgtable_visit_ctx *ctx,
static int hyp_map_walker(const struct kvm_pgtable_visit_ctx *ctx,
enum kvm_pgtable_walk_flags visit)
{
- kvm_pte_t *childp;
+ kvm_pte_t *childp, new;
struct hyp_map_data *data = ctx->arg;
struct kvm_pgtable_mm_ops *mm_ops = ctx->mm_ops;
@@ -424,8 +421,10 @@ static int hyp_map_walker(const struct kvm_pgtable_visit_ctx *ctx,
if (!childp)
return -ENOMEM;
- kvm_set_table_pte(ctx->ptep, childp, mm_ops);
+ new = kvm_init_table_pte(childp, mm_ops);
mm_ops->get_page(ctx->ptep);
+ smp_store_release(ctx->ptep, new);
+
return 0;
}
@@ -821,8 +820,9 @@ static int stage2_map_walk_leaf(const struct kvm_pgtable_visit_ctx *ctx,
if (stage2_pte_is_counted(ctx->old))
stage2_put_pte(ctx, data->mmu, mm_ops);
- kvm_set_table_pte(ctx->ptep, childp, mm_ops);
+ new = kvm_init_table_pte(childp, mm_ops);
mm_ops->get_page(ctx->ptep);
+ smp_store_release(ctx->ptep, new);
return 0;
}
--
2.38.0.rc1.362.ged0d419d3c-goog
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 27+ messages in thread* [PATCH v2 12/15] KVM: arm64: Make block->table PTE changes parallel-aware
2022-10-07 23:28 [PATCH v2 00/15] KVM: arm64: Parallel stage-2 fault handling Oliver Upton
` (10 preceding siblings ...)
2022-10-07 23:30 ` [PATCH v2 11/15] KVM: arm64: Split init and set for table PTE Oliver Upton
@ 2022-10-07 23:31 ` Oliver Upton
2022-10-07 23:32 ` [PATCH v2 13/15] KVM: arm64: Make leaf->leaf " Oliver Upton
` (2 subsequent siblings)
14 siblings, 0 replies; 27+ messages in thread
From: Oliver Upton @ 2022-10-07 23:31 UTC (permalink / raw)
To: Marc Zyngier, James Morse, Alexandru Elisei
Cc: linux-arm-kernel, kvmarm, kvm, Reiji Watanabe, Ricardo Koller,
David Matlack, Quentin Perret, Ben Gardon, Gavin Shan, Peter Xu,
Will Deacon, Sean Christopherson, kvmarm, Oliver Upton
In order to service stage-2 faults in parallel, stage-2 table walkers
must take exclusive ownership of the PTE being worked on. An additional
requirement of the architecture is that software must perform a
'break-before-make' operation when changing the block size used for
mapping memory.
Roll these two concepts together into helpers for performing a
'break-before-make' sequence. Use a special PTE value to indicate a PTE
has been locked by a software walker. Additionally, use an atomic
compare-exchange to 'break' the PTE when the stage-2 page tables are
possibly shared with another software walker. Elide the DSB + TLBI if
the evicted PTE was invalid (and thus not subject to break-before-make).
All of the atomics do nothing for now, as the stage-2 walker isn't fully
ready to perform parallel walks.
Signed-off-by: Oliver Upton <oliver.upton@linux.dev>
---
arch/arm64/kvm/hyp/pgtable.c | 82 +++++++++++++++++++++++++++++++++---
1 file changed, 76 insertions(+), 6 deletions(-)
diff --git a/arch/arm64/kvm/hyp/pgtable.c b/arch/arm64/kvm/hyp/pgtable.c
index f649a2251a35..932134f0d359 100644
--- a/arch/arm64/kvm/hyp/pgtable.c
+++ b/arch/arm64/kvm/hyp/pgtable.c
@@ -49,6 +49,12 @@
#define KVM_INVALID_PTE_OWNER_MASK GENMASK(9, 2)
#define KVM_MAX_OWNER_ID 1
+/*
+ * Used to indicate a pte for which a 'break-before-make' sequence is in
+ * progress.
+ */
+#define KVM_INVALID_PTE_LOCKED BIT(10)
+
struct kvm_pgtable_walk_data {
struct kvm_pgtable_walker *walker;
@@ -671,6 +677,11 @@ static bool stage2_pte_is_counted(kvm_pte_t pte)
return !!pte;
}
+static bool stage2_pte_is_locked(kvm_pte_t pte)
+{
+ return !kvm_pte_valid(pte) && (pte & KVM_INVALID_PTE_LOCKED);
+}
+
static bool stage2_try_set_pte(const struct kvm_pgtable_visit_ctx *ctx, kvm_pte_t new)
{
if (!kvm_pgtable_walk_shared(ctx)) {
@@ -681,6 +692,64 @@ static bool stage2_try_set_pte(const struct kvm_pgtable_visit_ctx *ctx, kvm_pte_
return cmpxchg(ctx->ptep, ctx->old, new) == ctx->old;
}
+/**
+ * stage2_try_break_pte() - Invalidates a pte according to the
+ * 'break-before-make' requirements of the
+ * architecture.
+ *
+ * @ctx: context of the visited pte.
+ * @data: stage-2 map data
+ *
+ * Returns: true if the pte was successfully broken.
+ *
+ * If the removed pte was valid, performs the necessary serialization and TLB
+ * invalidation for the old value. For counted ptes, drops the reference count
+ * on the containing table page.
+ */
+static bool stage2_try_break_pte(const struct kvm_pgtable_visit_ctx *ctx,
+ struct stage2_map_data *data)
+{
+ struct kvm_pgtable_mm_ops *mm_ops = ctx->mm_ops;
+
+ if (stage2_pte_is_locked(ctx->old)) {
+ /*
+ * Should never occur if this walker has exclusive access to the
+ * page tables.
+ */
+ WARN_ON(!kvm_pgtable_walk_shared(ctx));
+ return false;
+ }
+
+ if (!stage2_try_set_pte(ctx, KVM_INVALID_PTE_LOCKED))
+ return false;
+
+ /*
+ * Perform the appropriate TLB invalidation based on the evicted pte
+ * value (if any).
+ */
+ if (kvm_pte_table(ctx->old, ctx->level))
+ kvm_call_hyp(__kvm_tlb_flush_vmid, data->mmu);
+ else if (kvm_pte_valid(ctx->old))
+ kvm_call_hyp(__kvm_tlb_flush_vmid_ipa, data->mmu, ctx->addr, ctx->level);
+
+ if (stage2_pte_is_counted(ctx->old))
+ mm_ops->put_page(ctx->ptep);
+
+ return true;
+}
+
+static void stage2_make_pte(const struct kvm_pgtable_visit_ctx *ctx, kvm_pte_t new)
+{
+ struct kvm_pgtable_mm_ops *mm_ops = ctx->mm_ops;
+
+ WARN_ON(!stage2_pte_is_locked(*ctx->ptep));
+
+ if (stage2_pte_is_counted(new))
+ mm_ops->get_page(ctx->ptep);
+
+ smp_store_release(ctx->ptep, new);
+}
+
static void stage2_put_pte(const struct kvm_pgtable_visit_ctx *ctx, struct kvm_s2_mmu *mmu,
struct kvm_pgtable_mm_ops *mm_ops)
{
@@ -795,7 +864,7 @@ static int stage2_map_walk_leaf(const struct kvm_pgtable_visit_ctx *ctx,
struct stage2_map_data *data)
{
struct kvm_pgtable_mm_ops *mm_ops = ctx->mm_ops;
- kvm_pte_t *childp;
+ kvm_pte_t *childp, new;
int ret;
ret = stage2_map_walker_try_leaf(ctx, data);
@@ -812,17 +881,18 @@ static int stage2_map_walk_leaf(const struct kvm_pgtable_visit_ctx *ctx,
if (!childp)
return -ENOMEM;
+ if (!stage2_try_break_pte(ctx, data)) {
+ mm_ops->put_page(childp);
+ return -EAGAIN;
+ }
+
/*
* If we've run into an existing block mapping then replace it with
* a table. Accesses beyond 'end' that fall within the new table
* will be mapped lazily.
*/
- if (stage2_pte_is_counted(ctx->old))
- stage2_put_pte(ctx, data->mmu, mm_ops);
-
new = kvm_init_table_pte(childp, mm_ops);
- mm_ops->get_page(ctx->ptep);
- smp_store_release(ctx->ptep, new);
+ stage2_make_pte(ctx, new);
return 0;
}
--
2.38.0.rc1.362.ged0d419d3c-goog
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 27+ messages in thread* [PATCH v2 13/15] KVM: arm64: Make leaf->leaf PTE changes parallel-aware
2022-10-07 23:28 [PATCH v2 00/15] KVM: arm64: Parallel stage-2 fault handling Oliver Upton
` (11 preceding siblings ...)
2022-10-07 23:31 ` [PATCH v2 12/15] KVM: arm64: Make block->table PTE changes parallel-aware Oliver Upton
@ 2022-10-07 23:32 ` Oliver Upton
2022-10-07 23:32 ` [PATCH v2 14/15] KVM: arm64: Make table->block " Oliver Upton
2022-10-07 23:32 ` [PATCH v2 15/15] KVM: arm64: Handle stage-2 faults in parallel Oliver Upton
14 siblings, 0 replies; 27+ messages in thread
From: Oliver Upton @ 2022-10-07 23:32 UTC (permalink / raw)
To: Marc Zyngier, James Morse, Alexandru Elisei
Cc: linux-arm-kernel, kvmarm, kvm, Reiji Watanabe, Ricardo Koller,
David Matlack, Quentin Perret, Ben Gardon, Gavin Shan, Peter Xu,
Will Deacon, Sean Christopherson, kvmarm, Oliver Upton
Convert stage2_map_walker_try_leaf() to use the new break-before-make
helpers, thereby making the handler parallel-aware. As before, avoid the
break-before-make if recreating the existing mapping. Additionally,
retry execution if another vCPU thread is modifying the same PTE.
Signed-off-by: Oliver Upton <oliver.upton@linux.dev>
---
arch/arm64/kvm/hyp/pgtable.c | 26 ++++++++++++--------------
1 file changed, 12 insertions(+), 14 deletions(-)
diff --git a/arch/arm64/kvm/hyp/pgtable.c b/arch/arm64/kvm/hyp/pgtable.c
index 932134f0d359..adeece227495 100644
--- a/arch/arm64/kvm/hyp/pgtable.c
+++ b/arch/arm64/kvm/hyp/pgtable.c
@@ -801,18 +801,17 @@ static int stage2_map_walker_try_leaf(const struct kvm_pgtable_visit_ctx *ctx,
else
new = kvm_init_invalid_leaf_owner(data->owner_id);
- if (stage2_pte_is_counted(ctx->old)) {
- /*
- * Skip updating the PTE if we are trying to recreate the exact
- * same mapping or only change the access permissions. Instead,
- * the vCPU will exit one more time from guest if still needed
- * and then go through the path of relaxing permissions.
- */
- if (!stage2_pte_needs_update(ctx->old, new))
- return -EAGAIN;
+ /*
+ * Skip updating the PTE if we are trying to recreate the exact
+ * same mapping or only change the access permissions. Instead,
+ * the vCPU will exit one more time from guest if still needed
+ * and then go through the path of relaxing permissions.
+ */
+ if (!stage2_pte_needs_update(ctx->old, new))
+ return -EAGAIN;
- stage2_put_pte(ctx, data->mmu, mm_ops);
- }
+ if (!stage2_try_break_pte(ctx, data))
+ return -EAGAIN;
/* Perform CMOs before installation of the guest stage-2 PTE */
if (mm_ops->dcache_clean_inval_poc && stage2_pte_cacheable(pgt, new))
@@ -822,9 +821,8 @@ static int stage2_map_walker_try_leaf(const struct kvm_pgtable_visit_ctx *ctx,
if (mm_ops->icache_inval_pou && stage2_pte_executable(new))
mm_ops->icache_inval_pou(kvm_pte_follow(new, mm_ops), granule);
- smp_store_release(ctx->ptep, new);
- if (stage2_pte_is_counted(new))
- mm_ops->get_page(ctx->ptep);
+ stage2_make_pte(ctx, new);
+
if (kvm_phys_is_valid(phys))
data->phys += granule;
return 0;
--
2.38.0.rc1.362.ged0d419d3c-goog
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 27+ messages in thread* [PATCH v2 14/15] KVM: arm64: Make table->block changes parallel-aware
2022-10-07 23:28 [PATCH v2 00/15] KVM: arm64: Parallel stage-2 fault handling Oliver Upton
` (12 preceding siblings ...)
2022-10-07 23:32 ` [PATCH v2 13/15] KVM: arm64: Make leaf->leaf " Oliver Upton
@ 2022-10-07 23:32 ` Oliver Upton
2022-10-07 23:32 ` [PATCH v2 15/15] KVM: arm64: Handle stage-2 faults in parallel Oliver Upton
14 siblings, 0 replies; 27+ messages in thread
From: Oliver Upton @ 2022-10-07 23:32 UTC (permalink / raw)
To: Marc Zyngier, James Morse, Alexandru Elisei
Cc: linux-arm-kernel, kvmarm, kvm, Reiji Watanabe, Ricardo Koller,
David Matlack, Quentin Perret, Ben Gardon, Gavin Shan, Peter Xu,
Will Deacon, Sean Christopherson, kvmarm, Oliver Upton
stage2_map_walk_leaf() and friends now handle stage-2 PTEs generically,
and perform the correct flush when a table PTE is removed. Additionally,
they've been made parallel-aware, using an atomic break to take
ownership of the PTE.
Stop clearing the PTE in the pre-order callback and instead let
stage2_map_walk_leaf() deal with it.
Signed-off-by: Oliver Upton <oliver.upton@linux.dev>
---
arch/arm64/kvm/hyp/pgtable.c | 15 +++------------
1 file changed, 3 insertions(+), 12 deletions(-)
diff --git a/arch/arm64/kvm/hyp/pgtable.c b/arch/arm64/kvm/hyp/pgtable.c
index adeece227495..d951829c3876 100644
--- a/arch/arm64/kvm/hyp/pgtable.c
+++ b/arch/arm64/kvm/hyp/pgtable.c
@@ -841,21 +841,12 @@ static int stage2_map_walk_table_pre(const struct kvm_pgtable_visit_ctx *ctx,
if (!stage2_leaf_mapping_allowed(ctx, data))
return 0;
- kvm_clear_pte(ctx->ptep);
-
- /*
- * Invalidate the whole stage-2, as we may have numerous leaf
- * entries below us which would otherwise need invalidating
- * individually.
- */
- kvm_call_hyp(__kvm_tlb_flush_vmid, data->mmu);
-
ret = stage2_map_walk_leaf(ctx, data);
+ if (ret)
+ return ret;
- mm_ops->put_page(ctx->ptep);
mm_ops->free_removed_table(childp, ctx->level + 1);
-
- return ret;
+ return 0;
}
static int stage2_map_walk_leaf(const struct kvm_pgtable_visit_ctx *ctx,
--
2.38.0.rc1.362.ged0d419d3c-goog
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 27+ messages in thread* [PATCH v2 15/15] KVM: arm64: Handle stage-2 faults in parallel
2022-10-07 23:28 [PATCH v2 00/15] KVM: arm64: Parallel stage-2 fault handling Oliver Upton
` (13 preceding siblings ...)
2022-10-07 23:32 ` [PATCH v2 14/15] KVM: arm64: Make table->block " Oliver Upton
@ 2022-10-07 23:32 ` Oliver Upton
[not found] ` <202210081008.A9PLyQx2-lkp@intel.com>
2022-10-19 23:32 ` Sean Christopherson
14 siblings, 2 replies; 27+ messages in thread
From: Oliver Upton @ 2022-10-07 23:32 UTC (permalink / raw)
To: Marc Zyngier, James Morse, Alexandru Elisei
Cc: linux-arm-kernel, kvmarm, kvm, Reiji Watanabe, Ricardo Koller,
David Matlack, Quentin Perret, Ben Gardon, Gavin Shan, Peter Xu,
Will Deacon, Sean Christopherson, kvmarm, Oliver Upton
The stage-2 map walker has been made parallel-aware, and as such can be
called while only holding the read side of the MMU lock. Rip out the
conditional locking in user_mem_abort() and instead grab the read lock.
Continue to take the write lock from other callsites to
kvm_pgtable_stage2_map().
Signed-off-by: Oliver Upton <oliver.upton@linux.dev>
---
arch/arm64/include/asm/kvm_pgtable.h | 3 ++-
arch/arm64/kvm/hyp/nvhe/mem_protect.c | 2 +-
arch/arm64/kvm/hyp/pgtable.c | 5 +++--
arch/arm64/kvm/mmu.c | 31 ++++++---------------------
4 files changed, 13 insertions(+), 28 deletions(-)
diff --git a/arch/arm64/include/asm/kvm_pgtable.h b/arch/arm64/include/asm/kvm_pgtable.h
index f9a80354fc8e..fcbf982ab489 100644
--- a/arch/arm64/include/asm/kvm_pgtable.h
+++ b/arch/arm64/include/asm/kvm_pgtable.h
@@ -397,6 +397,7 @@ void kvm_pgtable_stage2_free_removed(struct kvm_pgtable_mm_ops *mm_ops, void *pg
* @prot: Permissions and attributes for the mapping.
* @mc: Cache of pre-allocated and zeroed memory from which to allocate
* page-table pages.
+ * @flags: Flags to control the page-table walk (ex. a shared walk)
*
* The offset of @addr within a page is ignored, @size is rounded-up to
* the next page boundary and @phys is rounded-down to the previous page
@@ -418,7 +419,7 @@ void kvm_pgtable_stage2_free_removed(struct kvm_pgtable_mm_ops *mm_ops, void *pg
*/
int kvm_pgtable_stage2_map(struct kvm_pgtable *pgt, u64 addr, u64 size,
u64 phys, enum kvm_pgtable_prot prot,
- void *mc);
+ void *mc, enum kvm_pgtable_walk_flags flags);
/**
* kvm_pgtable_stage2_set_owner() - Unmap and annotate pages in the IPA space to
diff --git a/arch/arm64/kvm/hyp/nvhe/mem_protect.c b/arch/arm64/kvm/hyp/nvhe/mem_protect.c
index 735769886b55..64058e71d083 100644
--- a/arch/arm64/kvm/hyp/nvhe/mem_protect.c
+++ b/arch/arm64/kvm/hyp/nvhe/mem_protect.c
@@ -257,7 +257,7 @@ static inline int __host_stage2_idmap(u64 start, u64 end,
enum kvm_pgtable_prot prot)
{
return kvm_pgtable_stage2_map(&host_kvm.pgt, start, end - start, start,
- prot, &host_s2_pool);
+ prot, &host_s2_pool, false);
}
/*
diff --git a/arch/arm64/kvm/hyp/pgtable.c b/arch/arm64/kvm/hyp/pgtable.c
index d951829c3876..e947dac07ae1 100644
--- a/arch/arm64/kvm/hyp/pgtable.c
+++ b/arch/arm64/kvm/hyp/pgtable.c
@@ -912,7 +912,7 @@ static int stage2_map_walker(const struct kvm_pgtable_visit_ctx *ctx,
int kvm_pgtable_stage2_map(struct kvm_pgtable *pgt, u64 addr, u64 size,
u64 phys, enum kvm_pgtable_prot prot,
- void *mc)
+ void *mc, enum kvm_pgtable_walk_flags flags)
{
int ret;
struct stage2_map_data map_data = {
@@ -923,7 +923,8 @@ int kvm_pgtable_stage2_map(struct kvm_pgtable *pgt, u64 addr, u64 size,
};
struct kvm_pgtable_walker walker = {
.cb = stage2_map_walker,
- .flags = KVM_PGTABLE_WALK_TABLE_PRE |
+ .flags = flags |
+ KVM_PGTABLE_WALK_TABLE_PRE |
KVM_PGTABLE_WALK_LEAF,
.arg = &map_data,
};
diff --git a/arch/arm64/kvm/mmu.c b/arch/arm64/kvm/mmu.c
index 66eede0fbb36..f8e0f9b6c9e0 100644
--- a/arch/arm64/kvm/mmu.c
+++ b/arch/arm64/kvm/mmu.c
@@ -826,7 +826,7 @@ int kvm_phys_addr_ioremap(struct kvm *kvm, phys_addr_t guest_ipa,
write_lock(&kvm->mmu_lock);
ret = kvm_pgtable_stage2_map(pgt, addr, PAGE_SIZE, pa, prot,
- &cache);
+ &cache, 0);
write_unlock(&kvm->mmu_lock);
if (ret)
break;
@@ -1121,7 +1121,6 @@ static int user_mem_abort(struct kvm_vcpu *vcpu, phys_addr_t fault_ipa,
gfn_t gfn;
kvm_pfn_t pfn;
bool logging_active = memslot_is_logging(memslot);
- bool use_read_lock = false;
unsigned long fault_level = kvm_vcpu_trap_get_fault_level(vcpu);
unsigned long vma_pagesize, fault_granule;
enum kvm_pgtable_prot prot = KVM_PGTABLE_PROT_R;
@@ -1156,8 +1155,6 @@ static int user_mem_abort(struct kvm_vcpu *vcpu, phys_addr_t fault_ipa,
if (logging_active) {
force_pte = true;
vma_shift = PAGE_SHIFT;
- use_read_lock = (fault_status == FSC_PERM && write_fault &&
- fault_granule == PAGE_SIZE);
} else {
vma_shift = get_vma_page_shift(vma, hva);
}
@@ -1256,15 +1253,7 @@ static int user_mem_abort(struct kvm_vcpu *vcpu, phys_addr_t fault_ipa,
if (exec_fault && device)
return -ENOEXEC;
- /*
- * To reduce MMU contentions and enhance concurrency during dirty
- * logging dirty logging, only acquire read lock for permission
- * relaxation.
- */
- if (use_read_lock)
- read_lock(&kvm->mmu_lock);
- else
- write_lock(&kvm->mmu_lock);
+ read_lock(&kvm->mmu_lock);
pgt = vcpu->arch.hw_mmu->pgt;
if (mmu_invalidate_retry(kvm, mmu_seq))
goto out_unlock;
@@ -1308,15 +1297,12 @@ static int user_mem_abort(struct kvm_vcpu *vcpu, phys_addr_t fault_ipa,
* permissions only if vma_pagesize equals fault_granule. Otherwise,
* kvm_pgtable_stage2_map() should be called to change block size.
*/
- if (fault_status == FSC_PERM && vma_pagesize == fault_granule) {
+ if (fault_status == FSC_PERM && vma_pagesize == fault_granule)
ret = kvm_pgtable_stage2_relax_perms(pgt, fault_ipa, prot);
- } else {
- WARN_ONCE(use_read_lock, "Attempted stage-2 map outside of write lock\n");
-
+ else
ret = kvm_pgtable_stage2_map(pgt, fault_ipa, vma_pagesize,
__pfn_to_phys(pfn), prot,
- memcache);
- }
+ memcache, KVM_PGTABLE_WALK_SHARED);
/* Mark the page dirty only if the fault is handled successfully */
if (writable && !ret) {
@@ -1325,10 +1311,7 @@ static int user_mem_abort(struct kvm_vcpu *vcpu, phys_addr_t fault_ipa,
}
out_unlock:
- if (use_read_lock)
- read_unlock(&kvm->mmu_lock);
- else
- write_unlock(&kvm->mmu_lock);
+ read_unlock(&kvm->mmu_lock);
kvm_set_pfn_accessed(pfn);
kvm_release_pfn_clean(pfn);
return ret != -EAGAIN ? ret : 0;
@@ -1534,7 +1517,7 @@ bool kvm_set_spte_gfn(struct kvm *kvm, struct kvm_gfn_range *range)
*/
kvm_pgtable_stage2_map(kvm->arch.mmu.pgt, range->start << PAGE_SHIFT,
PAGE_SIZE, __pfn_to_phys(pfn),
- KVM_PGTABLE_PROT_R, NULL);
+ KVM_PGTABLE_PROT_R, NULL, KVM_PGTABLE_WALK_SHARED);
return false;
}
--
2.38.0.rc1.362.ged0d419d3c-goog
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 27+ messages in thread[parent not found: <202210081008.A9PLyQx2-lkp@intel.com>]
* Re: [PATCH v2 15/15] KVM: arm64: Handle stage-2 faults in parallel
[not found] ` <202210081008.A9PLyQx2-lkp@intel.com>
@ 2022-10-08 3:14 ` Oliver Upton
0 siblings, 0 replies; 27+ messages in thread
From: Oliver Upton @ 2022-10-08 3:14 UTC (permalink / raw)
To: kernel test robot
Cc: Marc Zyngier, James Morse, Alexandru Elisei, kbuild-all,
linux-arm-kernel, kvmarm, kvm, Reiji Watanabe, Ricardo Koller,
David Matlack, Quentin Perret, Ben Gardon, Gavin Shan, Peter Xu,
Will Deacon, Sean Christopherson, kvmarm
On Sat, Oct 08, 2022 at 11:01:44AM +0800, kernel test robot wrote:
[...]
> All warnings (new ones prefixed by >>):
>
> arch/arm64/kvm/hyp/nvhe/mem_protect.c: In function '__host_stage2_idmap':
> >> arch/arm64/kvm/hyp/nvhe/mem_protect.c:260:60: warning: implicit conversion from 'enum <anonymous>' to 'enum kvm_pgtable_walk_flags' [-Wenum-conversion]
> 260 | prot, &host_s2_pool, false);
> | ^~~~~
>
Woops, this one fell through the cracks of the flagification.
--
Thanks,
Oliver
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH v2 15/15] KVM: arm64: Handle stage-2 faults in parallel
2022-10-07 23:32 ` [PATCH v2 15/15] KVM: arm64: Handle stage-2 faults in parallel Oliver Upton
[not found] ` <202210081008.A9PLyQx2-lkp@intel.com>
@ 2022-10-19 23:32 ` Sean Christopherson
2022-10-20 8:35 ` Oliver Upton
1 sibling, 1 reply; 27+ messages in thread
From: Sean Christopherson @ 2022-10-19 23:32 UTC (permalink / raw)
To: Oliver Upton
Cc: Marc Zyngier, James Morse, Alexandru Elisei, linux-arm-kernel,
kvmarm, kvm, Reiji Watanabe, Ricardo Koller, David Matlack,
Quentin Perret, Ben Gardon, Gavin Shan, Peter Xu, Will Deacon,
kvmarm
On Fri, Oct 07, 2022, Oliver Upton wrote:
> @@ -1534,7 +1517,7 @@ bool kvm_set_spte_gfn(struct kvm *kvm, struct kvm_gfn_range *range)
> */
> kvm_pgtable_stage2_map(kvm->arch.mmu.pgt, range->start << PAGE_SHIFT,
> PAGE_SIZE, __pfn_to_phys(pfn),
> - KVM_PGTABLE_PROT_R, NULL);
> + KVM_PGTABLE_PROT_R, NULL, KVM_PGTABLE_WALK_SHARED);
All MMU notifier events acquire mmu_lock for write when invoking arch code, i.e.
this isn't a shared walk.
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 27+ messages in thread* Re: [PATCH v2 15/15] KVM: arm64: Handle stage-2 faults in parallel
2022-10-19 23:32 ` Sean Christopherson
@ 2022-10-20 8:35 ` Oliver Upton
0 siblings, 0 replies; 27+ messages in thread
From: Oliver Upton @ 2022-10-20 8:35 UTC (permalink / raw)
To: Sean Christopherson
Cc: Marc Zyngier, James Morse, Alexandru Elisei, linux-arm-kernel,
kvmarm, kvm, Reiji Watanabe, Ricardo Koller, David Matlack,
Quentin Perret, Ben Gardon, Gavin Shan, Peter Xu, Will Deacon,
kvmarm
On Wed, Oct 19, 2022 at 11:32:36PM +0000, Sean Christopherson wrote:
> On Fri, Oct 07, 2022, Oliver Upton wrote:
> > @@ -1534,7 +1517,7 @@ bool kvm_set_spte_gfn(struct kvm *kvm, struct kvm_gfn_range *range)
> > */
> > kvm_pgtable_stage2_map(kvm->arch.mmu.pgt, range->start << PAGE_SHIFT,
> > PAGE_SIZE, __pfn_to_phys(pfn),
> > - KVM_PGTABLE_PROT_R, NULL);
> > + KVM_PGTABLE_PROT_R, NULL, KVM_PGTABLE_WALK_SHARED);
>
> All MMU notifier events acquire mmu_lock for write when invoking arch code, i.e.
> this isn't a shared walk.
Derp, didn't intend to set the flag here. I'll fix it in v3.
--
Thanks,
Oliver
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 27+ messages in thread