The pmap needs to know which CPUs to send IPIs when TLB entries
authorguenther <guenther@openbsd.org>
Fri, 18 Jun 2021 06:17:28 +0000 (06:17 +0000)
committerguenther <guenther@openbsd.org>
Fri, 18 Jun 2021 06:17:28 +0000 (06:17 +0000)
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
sys/arch/amd64/amd64/locore.S
sys/arch/amd64/amd64/pmap.c
sys/arch/amd64/include/cpu.h
sys/arch/amd64/include/pmap.h

index 2486d5c..0db65e5 100644 (file)
@@ -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 <sys/param.h>
@@ -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
index 4ad7c13..ec15e95 100644 (file)
@@ -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.
index 7141ebb..6ad63ff 100644 (file)
@@ -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);
index 4a53dd0..cc5012b 100644 (file)
@@ -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;
 
index bcce280..25c5f07 100644 (file)
@@ -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) */
 };