From 24056ac060e38af16e27672864bbc90e35cb2e83 Mon Sep 17 00:00:00 2001 From: guenther Date: Fri, 18 Jun 2021 06:17:28 +0000 Subject: [PATCH] The pmap needs to know which CPUs to send IPIs when TLB entries need to be invalidated. Instead of keeping a bitset of CPUs in each pmap, have each cpu_info track which pmap it has loaded: replace pmap->pm_cpus with cpu_info->ci_proc_pmap. This reduces the atomic operations (and cache thrashing) and simplifies cpu_switchto() Also, fix a defect in cpu_switchto()'s "am I loading the same cr3?" test: ignore the CR3_REUSE_PCID bit when checking that. This makes switching between kernel threads slightly less costly. over a week in snaps with no complaints looks ok to mlarkin@ kettenis@ mpi@ --- sys/arch/amd64/amd64/genassym.cf | 5 +-- sys/arch/amd64/amd64/locore.S | 56 ++++++++++++---------------- sys/arch/amd64/amd64/pmap.c | 64 ++++++++++++++------------------ sys/arch/amd64/include/cpu.h | 3 +- sys/arch/amd64/include/pmap.h | 3 +- 5 files changed, 57 insertions(+), 74 deletions(-) diff --git a/sys/arch/amd64/amd64/genassym.cf b/sys/arch/amd64/amd64/genassym.cf index 2486d5cfccc..0db65e55cb6 100644 --- a/sys/arch/amd64/amd64/genassym.cf +++ b/sys/arch/amd64/amd64/genassym.cf @@ -1,4 +1,4 @@ -# $OpenBSD: genassym.cf,v 1.41 2019/06/11 15:23:41 mpi Exp $ +# $OpenBSD: genassym.cf,v 1.42 2021/06/18 06:17:28 guenther Exp $ # Written by Artur Grabowski art@openbsd.org, Public Domain include @@ -95,8 +95,6 @@ member pcb_pmap member pcb_savefpu struct pmap -member pm_cpus -member pm_pdirpa member pm_pdirpa_intel struct x86_64_tss @@ -109,6 +107,7 @@ member CPU_INFO_CPUID ci_cpuid member CPU_INFO_APICID ci_apicid member CPU_INFO_RESCHED ci_want_resched member CPU_INFO_CURPROC ci_curproc +member CPU_INFO_PROC_PMAP ci_proc_pmap member CPU_INFO_CURPCB ci_curpcb member CPU_INFO_IDLE_PCB ci_idle_pcb member CPU_INFO_ILEVEL ci_ilevel diff --git a/sys/arch/amd64/amd64/locore.S b/sys/arch/amd64/amd64/locore.S index 4ad7c137601..ec15e95f90f 100644 --- a/sys/arch/amd64/amd64/locore.S +++ b/sys/arch/amd64/amd64/locore.S @@ -1,4 +1,4 @@ -/* $OpenBSD: locore.S,v 1.124 2021/06/01 21:12:11 guenther Exp $ */ +/* $OpenBSD: locore.S,v 1.125 2021/06/18 06:17:28 guenther Exp $ */ /* $NetBSD: locore.S,v 1.13 2004/03/25 18:33:17 drochner Exp $ */ /* @@ -445,30 +445,24 @@ restore_saved: RETGUARD_SETUP_OFF(cpu_switchto, r11, 6*8) /* don't switch cr3 to the same thing it already was */ - movq %cr3,%rax - cmpq PCB_CR3(%r13),%rax - movq PCB_CR3(%r13),%rax /* flags from cmpq unchanged */ + movq PCB_CR3(%r13),%rax + movq %cr3,%rdi + xorq %rax,%rdi + btrq $63,%rdi /* ignore CR3_REUSE_PCID */ + testq %rdi,%rdi jz .Lsame_cr3 - /* set the new pmap's bit for the cpu */ - testq %rbx,%rbx - jz .Lno_new_pmap - lock - btsq %r9,PM_CPUS(%rbx) #ifdef DIAGNOSTIC - jc _C_LABEL(switch_pmcpu_set) + /* verify ci_proc_pmap had been updated properly */ + cmpq %rcx,CPUVAR(PROC_PMAP) + jnz .Lbogus_proc_pmap #endif -.Lno_new_pmap: + /* record which pmap this CPU should get IPIs for */ + movq %rbx,CPUVAR(PROC_PMAP) +.Lset_cr3: movq %rax,%cr3 /* %rax used below too */ - /* clear the old pmap's bit for the cpu */ - testq %rcx,%rcx - jz .Lno_old_pmap - lock - btrq %r9,PM_CPUS(%rcx) -.Lno_old_pmap: - .Lsame_cr3: /* * If we switched from a userland thread with a shallow call stack @@ -514,6 +508,18 @@ switch_restored: popq %rbx RETGUARD_CHECK(cpu_switchto, r11) ret + +#ifdef DIAGNOSTIC + .globl _C_LABEL(panic) +.Lbogus_proc_pmap: + leaq bogus_proc_pmap,%rdi + call _C_LABEL(panic) + int3 /* NOTREACHED */ + .pushsection .rodata +bogus_proc_pmap: + .asciz "curcpu->ci_proc_pmap didn't point to previous pmap" + .popsection +#endif /* DIAGNOSTIC */ END(cpu_switchto) ENTRY(cpu_idle_enter) @@ -539,20 +545,6 @@ ENTRY(cpu_idle_cycle) ret END(cpu_idle_cycle) - .globl _C_LABEL(panic) - -#ifdef DIAGNOSTIC -NENTRY(switch_pmcpu_set) - leaq switch_active(%rip),%rdi - call _C_LABEL(panic) -END(switch_pmcpu_set) - /* NOTREACHED */ - - .section .rodata -switch_active: - .asciz "activate already active pmap" - .text -#endif /* DIAGNOSTIC */ /* * savectx(struct pcb *pcb); * Update pcb, saving current processor state. diff --git a/sys/arch/amd64/amd64/pmap.c b/sys/arch/amd64/amd64/pmap.c index 7141ebb0d7c..6ad63ffb278 100644 --- a/sys/arch/amd64/amd64/pmap.c +++ b/sys/arch/amd64/amd64/pmap.c @@ -1,4 +1,4 @@ -/* $OpenBSD: pmap.c,v 1.144 2021/06/16 09:02:21 mpi Exp $ */ +/* $OpenBSD: pmap.c,v 1.145 2021/06/18 06:17:28 guenther Exp $ */ /* $NetBSD: pmap.c,v 1.3 2003/05/08 18:13:13 thorpej Exp $ */ /* @@ -312,7 +312,7 @@ void pmap_free_ptp(struct pmap *, struct vm_page *, vaddr_t, struct pg_to_free *); void pmap_freepage(struct pmap *, struct vm_page *, int, struct pg_to_free *); #ifdef MULTIPROCESSOR -static int pmap_is_active(struct pmap *, int); +static int pmap_is_active(struct pmap *, struct cpu_info *); #endif paddr_t pmap_map_ptes(struct pmap *); struct pv_entry *pmap_remove_pv(struct vm_page *, struct pmap *, vaddr_t); @@ -353,7 +353,7 @@ void pmap_tlb_shootwait(void); * of course the kernel is always loaded */ -static __inline int +static inline int pmap_is_curpmap(struct pmap *pmap) { return((pmap == pmap_kernel()) || @@ -365,15 +365,14 @@ pmap_is_curpmap(struct pmap *pmap) */ #ifdef MULTIPROCESSOR -static __inline int -pmap_is_active(struct pmap *pmap, int cpu_id) +static inline int +pmap_is_active(struct pmap *pmap, struct cpu_info *ci) { - return (pmap == pmap_kernel() || - (pmap->pm_cpus & (1ULL << cpu_id)) != 0); + return pmap == pmap_kernel() || pmap == ci->ci_proc_pmap; } #endif -static __inline u_int +static inline u_int pmap_pte2flags(u_long pte) { return (((pte & PG_U) ? PG_PMAP_REF : 0) | @@ -1313,7 +1312,6 @@ pmap_create(void) } pmap->pm_stats.wired_count = 0; pmap->pm_stats.resident_count = 1; /* count the PDP allocd below */ - pmap->pm_cpus = 0; pmap->pm_type = PMAP_TYPE_NORMAL; /* allocate PDP */ @@ -1371,16 +1369,6 @@ pmap_destroy(struct pmap *pmap) return; } - /* - * reference count is zero, free pmap resources and then free pmap. - */ - -#ifdef DIAGNOSTIC - if (__predict_false(pmap->pm_cpus != 0)) - printf("%s: pmap %p cpus=0x%llx\n", __func__, - (void *)pmap, pmap->pm_cpus); -#endif - /* * remove it from global list of pmaps */ @@ -1440,23 +1428,24 @@ pmap_activate(struct proc *p) pcb->pcb_cr3 |= (pmap != pmap_kernel()) ? cr3_pcid_proc : (PCID_KERN | cr3_reuse_pcid); - if (p == curproc) { - lcr3(pcb->pcb_cr3); + if (p != curproc) + return; + + if ((p->p_flag & P_SYSTEM) == 0) { + struct cpu_info *self = curcpu(); + + /* mark the pmap in use by this processor */ + self->ci_proc_pmap = pmap; /* in case we return to userspace without context switching */ if (cpu_meltdown) { - struct cpu_info *self = curcpu(); - self->ci_kern_cr3 = pcb->pcb_cr3 | cr3_reuse_pcid; self->ci_user_cr3 = pmap->pm_pdirpa_intel | cr3_pcid_proc_intel; } - - /* - * mark the pmap in use by this processor. - */ - x86_atomic_setbits_u64(&pmap->pm_cpus, (1ULL << cpu_number())); } + + lcr3(pcb->pcb_cr3); } /* @@ -1466,12 +1455,15 @@ pmap_activate(struct proc *p) void pmap_deactivate(struct proc *p) { - struct pmap *pmap = p->p_vmspace->vm_map.pmap; + if ((p->p_flag & P_SYSTEM) == 0) { + struct cpu_info *self = curcpu(); - /* - * mark the pmap no longer in use by this processor. - */ - x86_atomic_clearbits_u64(&pmap->pm_cpus, (1ULL << cpu_number())); + /* + * mark the pmap no longer in use by this processor. + */ + KASSERT(self->ci_proc_pmap == p->p_vmspace->vm_map.pmap); + self->ci_proc_pmap = NULL; + } } /* @@ -3168,7 +3160,7 @@ pmap_tlb_shootpage(struct pmap *pm, vaddr_t va, int shootself) CPU_INFO_FOREACH(cii, ci) { if (ci == self || !(ci->ci_flags & CPUF_RUNNING)) continue; - if (!is_kva && !pmap_is_active(pm, ci->ci_cpuid)) + if (!is_kva && !pmap_is_active(pm, ci)) continue; mask |= (1ULL << ci->ci_cpuid); wait++; @@ -3214,7 +3206,7 @@ pmap_tlb_shootrange(struct pmap *pm, vaddr_t sva, vaddr_t eva, int shootself) CPU_INFO_FOREACH(cii, ci) { if (ci == self || !(ci->ci_flags & CPUF_RUNNING)) continue; - if (!is_kva && !pmap_is_active(pm, ci->ci_cpuid)) + if (!is_kva && !pmap_is_active(pm, ci)) continue; mask |= (1ULL << ci->ci_cpuid); wait++; @@ -3269,7 +3261,7 @@ pmap_tlb_shoottlb(struct pmap *pm, int shootself) KASSERT(pm != pmap_kernel()); CPU_INFO_FOREACH(cii, ci) { - if (ci == self || !pmap_is_active(pm, ci->ci_cpuid) || + if (ci == self || !pmap_is_active(pm, ci) || !(ci->ci_flags & CPUF_RUNNING)) continue; mask |= (1ULL << ci->ci_cpuid); diff --git a/sys/arch/amd64/include/cpu.h b/sys/arch/amd64/include/cpu.h index 4a53dd0724b..cc5012b7ee7 100644 --- a/sys/arch/amd64/include/cpu.h +++ b/sys/arch/amd64/include/cpu.h @@ -1,4 +1,4 @@ -/* $OpenBSD: cpu.h,v 1.138 2021/06/02 00:39:26 cheloha Exp $ */ +/* $OpenBSD: cpu.h,v 1.139 2021/06/18 06:17:28 guenther Exp $ */ /* $NetBSD: cpu.h,v 1.1 2003/04/26 18:39:39 fvdl Exp $ */ /*- @@ -121,6 +121,7 @@ struct cpu_info { char ci_mds_tmp[32]; /* 32byte aligned */ void *ci_mds_buf; + struct pmap *ci_proc_pmap; /* last userspace pmap */ struct pcb *ci_curpcb; struct pcb *ci_idle_pcb; diff --git a/sys/arch/amd64/include/pmap.h b/sys/arch/amd64/include/pmap.h index bcce28080ac..25c5f07c9f8 100644 --- a/sys/arch/amd64/include/pmap.h +++ b/sys/arch/amd64/include/pmap.h @@ -1,4 +1,4 @@ -/* $OpenBSD: pmap.h,v 1.77 2020/01/24 05:27:32 kettenis Exp $ */ +/* $OpenBSD: pmap.h,v 1.78 2021/06/18 06:17:28 guenther Exp $ */ /* $NetBSD: pmap.h,v 1.1 2003/04/26 18:39:46 fvdl Exp $ */ /* @@ -313,7 +313,6 @@ struct pmap { /* pointer to a PTP in our pmap */ struct pmap_statistics pm_stats; /* pmap stats (lck by object lock) */ - u_int64_t pm_cpus; /* mask of CPUs using pmap */ int pm_type; /* Type of pmap this is (PMAP_TYPE_x) */ uint64_t eptp; /* cached EPTP (used by vmm) */ }; -- 2.20.1