| Message ID | 20221027221752.1683510-9-oliver.upton@linux.dev (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | KVM: arm64: Parallel stage-2 fault handling | expand |
On Thu, Oct 27, 2022 at 10:17:45PM +0000, 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> > --- > arch/arm64/include/asm/kvm_pgtable.h | 41 ++++++++++++++++++++++++++++ > arch/arm64/kvm/hyp/pgtable.c | 10 ++++++- > 2 files changed, 50 insertions(+), 1 deletion(-) > > diff --git a/arch/arm64/include/asm/kvm_pgtable.h b/arch/arm64/include/asm/kvm_pgtable.h > index e70cf57b719e..d1859e8550df 100644 > --- a/arch/arm64/include/asm/kvm_pgtable.h > +++ b/arch/arm64/include/asm/kvm_pgtable.h > @@ -37,6 +37,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) > @@ -44,6 +51,40 @@ 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) {} > + > +static inline bool kvm_pgtable_walk_lock_held(void) > +{ > + return true; > +} > + > +#else > + > +typedef kvm_pte_t __rcu *kvm_pteref_t; > + > +static inline kvm_pte_t *kvm_dereference_pteref(kvm_pteref_t pteref, bool shared) > +{ > + return rcu_dereference_check(pteref, shared); I accidentally squashed the fix for !shared into 9/15, not this patch. Fix ready for v4. -- Thanks, Oliver
On Thu, Oct 27, 2022, Oliver Upton wrote: > The use of RCU is necessary to safely change the stage-2 page tables in > parallel. RCU isn't strictly necessary, it's simply _a_ way to ensure readers have gone away prior to freeing memory. E.g. another method (used on x86 at least), is to disable IRQs when walking page tables without holding mmu_lock, and then requiring paths that free page tables to effectively do an IPI shootdown of all CPUs that are reading page tables. And "safely change" is misleading. RCU doesn't allow safely _changing_ page tables; if RCU did, then patch 10 wouldn't need to implement atomic updates. Protecting page table walks with RCU is purely about ensuring readers have gone away prior to freeing the backing memory, an entirely different mechanism is needed to allow parallel updates, e.g. the CMPXCHG + KVM_INVALID_PTE_LOCKED approach to ensure only one walker "owns" the detached page table.
diff --git a/arch/arm64/include/asm/kvm_pgtable.h b/arch/arm64/include/asm/kvm_pgtable.h index e70cf57b719e..d1859e8550df 100644 --- a/arch/arm64/include/asm/kvm_pgtable.h +++ b/arch/arm64/include/asm/kvm_pgtable.h @@ -37,6 +37,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) @@ -44,6 +51,40 @@ 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) {} + +static inline bool kvm_pgtable_walk_lock_held(void) +{ + return true; +} + +#else + +typedef kvm_pte_t __rcu *kvm_pteref_t; + +static inline kvm_pte_t *kvm_dereference_pteref(kvm_pteref_t pteref, bool shared) +{ + return rcu_dereference_check(pteref, shared); +} + +static inline void kvm_pgtable_walk_begin(void) +{ + rcu_read_lock(); +} + +static inline void kvm_pgtable_walk_end(void) +{ + rcu_read_unlock(); +} + +static inline bool kvm_pgtable_walk_lock_held(void) +{ + return rcu_read_lock_held(); +} + +#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 c4d103889a62..5bab932aeaec 100644 --- a/arch/arm64/kvm/hyp/pgtable.c +++ b/arch/arm64/kvm/hyp/pgtable.c @@ -171,6 +171,9 @@ static int kvm_pgtable_visitor_cb(struct kvm_pgtable_walk_data *data, enum kvm_pgtable_walk_flags visit) { struct kvm_pgtable_walker *walker = data->walker; + + /* Ensure the appropriate lock is held (e.g. RCU lock for stage-2 MMU) */ + WARN_ON_ONCE(kvm_pgtable_walk_shared(ctx) && !kvm_pgtable_walk_lock_held()); return walker->cb(ctx, visit); } @@ -281,8 +284,13 @@ int kvm_pgtable_walk(struct kvm_pgtable *pgt, u64 addr, u64 size, .end = PAGE_ALIGN(walk_data.addr + size), .walker = walker, }; + int r; + + kvm_pgtable_walk_begin(); + r = _kvm_pgtable_walk(pgt, &walk_data); + kvm_pgtable_walk_end(); - return _kvm_pgtable_walk(pgt, &walk_data); + return r; } struct leaf_walk_data {
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 | 41 ++++++++++++++++++++++++++++ arch/arm64/kvm/hyp/pgtable.c | 10 ++++++- 2 files changed, 50 insertions(+), 1 deletion(-)