powerpc/mm/radix: Workaround prefetch issue with KVM
Subsystems (2)
PATCH v4 1/1 powerpc/mm/radix: Workaround prefetch issue with KVM
There's a somewhat architectural issue with Radix MMU and KVM.
When coming out of a guest with AIL (ie, MMU enabled), we start executing hypervisor code with the PID register still containing whatever the guest has been using.
The problem is that the CPU can (and will) then start prefetching or speculatively load from whatever host context has that same PID (if any), thus bringing translations for that context into the TLB, which Linux doesn't know about.
This can cause stale translations and subsequent crashes.
Fixing this in a way that is neither racy nor a huge performance impact is difficult. We could just make the host invalidations always use broadcast forms but that would hurt single threaded programs for example.
We chose to fix it instead by partitioning the PID space between guest and host. This is possible because today Linux only use 19 out of the 20 bits of PID space, so existing guests will work if we make the host use the top half of the 20 bits space.
We additionally add a property to indicate to Linux the size of the PID register which will be useful if we eventually have processors with a larger PID space available.
There is still an issue with malicious guests purposefully setting the PID register to a value in the host range. Hopefully future HW can prevent that, but in the meantime, we handle it with a pair of kludges:
- On the way out of a guest, before we clear the current VCPU
in the PACA, we check the PID and if it's outside of the permitted range we flush the TLB for that PID.
- When context switching, if the mm is "new" on that CPU (the
corresponding bit was set for the first time in the mm cpumask), we check if any sibling thread is in KVM (has a non-NULL VCPU pointer in the PACA). If that is the case, we also flush the PID for that CPU (core).
This second part is needed to handle the case where a process is migrated (or starts a new pthread) on a sibling thread of the CPU coming out of KVM, as there's a window where stale translations can exist before we detect it and flush them out.
A future optimization could be added by keeping track of whether the PID has ever been used and avoid doing that for completely fresh PIDs. We could similarily mark PIDs that have been the subject of a global invalidation as "fresh". But for now this will do.
---
v2. Do the check on KVM exit *after* we've restored the host PID
v3. Properly check for radix in the exit assembly and avoid adding
yet another function to tlb-radix.c
v4. Don't add an argument to switch_mmu_context(), instead call a
workaround function directly from switch_mm_irqs_off(), which keeps the patch a lot smaller.
--- arch/powerpc/include/asm/book3s/64/mmu.h | 15 +++++----- arch/powerpc/include/asm/mmu_context.h | 18 ++++++++++-- arch/powerpc/kvm/book3s_hv_rmhandlers.S | 49 ++++++++++++++++++++++++++------ arch/powerpc/mm/mmu_context_book3s64.c | 9 ++++-- arch/powerpc/mm/pgtable-radix.c | 34 +++++++++++++++++++++- arch/powerpc/mm/tlb-radix.c | 45 +++++++++++++++++++++++++++-- 6 files changed, 148 insertions(+), 22 deletions(-)
arch/powerpc/include/asm/book3s/64/mmu.h +8 -7
@@ -59,13 +59,14 @@ extern struct patb_entry *partition_tb;#define PRTS_MASK 0x1f/* process table size field */ #define PRTB_MASK 0x0ffffffffffff000UL -/* -* Limit process table to PAGE_SIZE table. This -* also limit the max pid we can support. -* MAX_USER_CONTEXT * 16 bytes of space. -*/ -#define PRTB_SIZE_SHIFT (CONTEXT_BITS + 4) -#define PRTB_ENTRIES (1ul << CONTEXT_BITS) +/* Number of supported PID bits */ +extern unsigned int mmu_pid_bits; + +/* Base PID to allocate from */ +extern unsigned int mmu_base_pid; + +#define PRTB_SIZE_SHIFT (mmu_pid_bits + 4) +#define PRTB_ENTRIES (1ul << mmu_pid_bits)/* * Power9 currently only support 64K partition table size.
arch/powerpc/include/asm/mmu_context.h +16 -2
@@ -45,7 +45,7 @@ extern void set_context(unsigned long id, pgd_t *pgd);#ifdef CONFIG_PPC_BOOK3S_64 extern void radix__switch_mmu_context (struct mm_struct *prev, -struct mm_struct *next); +struct mm_struct *next);static inline void switch_mmu_context (struct mm_struct *prev,struct mm_struct *next,struct task_struct *tsk)
@@ -67,6 +67,12 @@ extern void __destroy_context(unsigned long context_id);extern void mmu_context_init (void );#endif +#if defined(CONFIG_KVM_BOOK3S_HV_POSSIBLE ) && defined(CONFIG_PPC_RADIX_MMU ) +extern void radix_kvm_prefetch_workaround (struct mm_struct *mm); +#else +static inline void radix_kvm_prefetch_workaround (struct mm_struct *mm) { } +#endif +extern void switch_cop (struct mm_struct *next);extern int use_cop (unsigned long acop,struct mm_struct *mm);extern void drop_cop (unsigned long acop,struct mm_struct *mm);
@@ -79,9 +85,13 @@ static inline void switch_mm_irqs_off(struct mm_struct *prev,struct mm_struct *next, struct task_struct *tsk) { + bool new_on_cpu = false; +/* Mark this context has been used on the new CPU */ -if (!cpumask_test_cpu (smp_processor_id (),mm_cpumask (next))) +if (!cpumask_test_cpu (smp_processor_id (),mm_cpumask (next))) {cpumask_set_cpu (smp_processor_id (),mm_cpumask (next)); + new_on_cpu = true; + }/* 32-bit keeps track of the current PGDIR in the thread struct */ #ifdef CONFIG_PPC32
@@ -109,6 +119,10 @@ static inline void switch_mm_irqs_off(struct mm_struct *prev,if (cpu_has_feature (CPU_FTR_ALTIVEC )) asmvolatile ("dssall" );#endif /* CONFIG_ALTIVEC */ + +if (new_on_cpu) +radix_kvm_prefetch_workaround (next); +/* * The actual HW switching method differs between the various * sub architectures. Out of line for now
arch/powerpc/kvm/book3s_hv_rmhandlers.S +41 -8
80 lines not on this page · raw message
arch/powerpc/mm/mmu_context_book3s64.c +7 -2
@@ -25,6 +25,10 @@#include <asm/mmu_context.h> #include <asm/pgalloc.h> +#ifdef CONFIG_KVM_BOOK3S_HV_POSSIBLE +#include <asm/kvm_book3s_asm.h> +#endif +#include "icswx.h" static DEFINE_SPINLOCK (mmu_context_lock);
@@ -126,9 +130,10 @@ static int hash__init_new_context(struct mm_struct *mm)static int radix__init_new_context (struct mm_struct *mm) {unsigned long rts_field; -int index; +int index, max_id; - index =alloc_context_id (1 ,PRTB_ENTRIES -1 ); + max_id = (1 << mmu_pid_bits) -1 ; + index =alloc_context_id (mmu_base_pid, max_id);if (index <0 )return index;
arch/powerpc/mm/pgtable-radix.c +33 -1
@@ -25,6 +25,9 @@#include <trace/events/thp.h> +unsigned int mmu_pid_bits; +unsigned int mmu_base_pid; +static int native_register_process_table (unsigned long base,unsigned long pg_sz,unsigned long table_size) {
@@ -243,11 +246,34 @@ static void __init radix_init_pgtable(void)for_each_memblock (memory, reg)WARN_ON (create_physical_mapping (reg->base, reg->base + reg->size)); + +/* Find out how many PID bits are supported */ +if (cpu_has_feature (CPU_FTR_HVMODE )) { +if (!mmu_pid_bits) + mmu_pid_bits =20 ; +#ifdef CONFIG_KVM_BOOK3S_HV_POSSIBLE +/* +* When KVM is possible, we only use the top half of the +* PID space to avoid collisions between host and guest PIDs +* which can cause problems due to prefetch when exiting the +* guest with AIL=3 +*/ + mmu_base_pid =1 << (mmu_pid_bits -1 ); +#else + mmu_base_pid =1 ; +#endif + }else { +/* The guest uses the bottom half of the PID space */ +if (!mmu_pid_bits) + mmu_pid_bits =19 ; + mmu_base_pid =1 ; + } +/* * Allocate Partition table and process table for the * host. */ -BUILD_BUG_ON_MSG ((PRTB_SIZE_SHIFT >36 ),"Process table size too large." ); +BUG_ON (PRTB_SIZE_SHIFT >36 ); process_tb =early_alloc_pgtable (1UL <<PRTB_SIZE_SHIFT );/* * Fill in the process table.
@@ -321,6 +347,12 @@ static int __init radix_dt_scan_page_sizes(unsigned long node,if (type ==NULL ||strcmp (type,"cpu" ) !=0 )return 0 ; +/* Find MMU PID size */ + prop =of_get_flat_dt_prop (node,"ibm,mmu-pid-bits" , &size); +if (prop && size ==4 ) + mmu_pid_bits =be32_to_cpup (prop); + +/* Grab page size encodings */ prop =of_get_flat_dt_prop (node,"ibm,processor-radix-AP-encodings" , &size);if (!prop)return 0 ;
arch/powerpc/mm/tlb-radix.c +43 -2
58 lines not on this page · raw message
Quoted 47 lines · Benjamin Herrenschmidt
There's a somewhat architectural issue with Radix MMU and KVM.
When coming out of a guest with AIL (ie, MMU enabled), we start executing hypervisor code with the PID register still containing whatever the guest has been using.
The problem is that the CPU can (and will) then start prefetching or speculatively load from whatever host context has that same PID (if any), thus bringing translations for that context into the TLB, which Linux doesn't know about.
This can cause stale translations and subsequent crashes.
Fixing this in a way that is neither racy nor a huge performance impact is difficult. We could just make the host invalidations always use broadcast forms but that would hurt single threaded programs for example.
We chose to fix it instead by partitioning the PID space between guest and host. This is possible because today Linux only use 19 out of the 20 bits of PID space, so existing guests will work if we make the host use the top half of the 20 bits space.
We additionally add a property to indicate to Linux the size of the PID register which will be useful if we eventually have processors with a larger PID space available.
There is still an issue with malicious guests purposefully setting the PID register to a value in the host range. Hopefully future HW can prevent that, but in the meantime, we handle it with a pair of kludges:
- On the way out of a guest, before we clear the current VCPU
in the PACA, we check the PID and if it's outside of the permitted range we flush the TLB for that PID.- When context switching, if the mm is "new" on that CPU (the
corresponding bit was set for the first time in the mm cpumask), we check if any sibling thread is in KVM (has a non-NULL VCPU pointer in the PACA). If that is the case, we also flush the PID for that CPU (core).This second part is needed to handle the case where a process is migrated (or starts a new pthread) on a sibling thread of the CPU coming out of KVM, as there's a window where stale translations can exist before we detect it and flush them out.
Quoted 6 lines
A future optimization could be added by keeping track of whether the PID has ever been used and avoid doing that for completely fresh PIDs. We could similarily mark PIDs that have been the subject of a global invalidation as "fresh". But for now this will do.
Signed-off-by Benjamin Herrenschmidt <[email protected]>
Mostly looks fine. I have one comment below about the tlbiels we do when we detect the case that the guest used a PID value that it shouldn't have:
[snip]
arch/powerpc/kvm/book3s_hv_rmhandlers.S BEGIN_FTR_SECTION · line 1766
Quoted 9 lines
arch/powerpc/kvm/book3s_hv_rmhandlers.S +9
-0+ /* Illegal PID, flush the TLB for this PID/LPID */ + isync + ld r6,VCPU_KVM(r9) + lwz r0,KVM_TLB_SETS(r6) + mtctr r0 + li r7,0x400 /* IS field = 0b01 */ + ptesync + sldi r0,r3,32 /* RS has PID */ +3: PPC_TLBIEL(7,0,2,1,1) /* RIC=2, PRS=1, R=1 */
The architecture says tlbiel in this form invalidates entries whose lpid matches the value in LPIDR. At this point in the code, LPIDR still contains the guest lpid, but we're trying to invalidate host entries, aren't we? In general at this point in the code we can't switch LPIDR (though on P9 we could) since we don't know at this point that the other (POWER8) threads are out of the guest.
Quoted 18 lines
arch/powerpc/kvm/book3s_hv_rmhandlers.S +6
-0+ addi r7,r7,0x1000 + bdnz 3b + ptesync + +4: /* Flush the ERAT on radix P9 DD1 guest exit */ BEGIN_FTR_SECTION PPC_INVALIDATE_ERAT END_FTR_SECTION_IFSET(CPU_FTR_POWER9_DD1) +5: /* * POWER7/POWER8 guest -> host partition switch code. * We don't have to lock against tlbies but we doarch/powerpc/mm/mmu_context_book3s64.c +0
-0
arch/powerpc/mm/mmu_context_book3s64.c line 30
Quoted 6 lines
arch/powerpc/mm/mmu_context_book3s64.c +3
-0@@ -25,6 +25,10 @@#include <asm/mmu_context.h> #include <asm/pgalloc.h> +#ifdef CONFIG_KVM_BOOK3S_HV_POSSIBLE +#include <asm/kvm_book3s_asm.h> +#endif
What is this include for? Nothing you add to this file seems to use anything from kvm_book3s_asm.h.
Paul.