DF-5420 / fix.diff
1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 32 33 34 35 36 37 38 39 40 41 42 43 44 45 46 47 48 49 50 51 52 53 54 55 56 57 58 59 60 61 62 63 64 65 66 67 68 69 70 71 72 73 74 75 76 77 78 79 80 81 82 83 84 85 86 87 88 89 90 91 92 93 94 95 96 97 98 99 100 101 102 103 104 105 106 107 108 109 110 111 112 113 114 115 116 117 118 119 120 121 122 123 124 125 126 127 128 129 130 131 132 133 134 135 136 137 138 139 140 141 142 143 144 145 146 147 148 149 150 151 152 153 154 155 156 157 158 159 160 161 162 163 164 165 166 167 168 169 170 171 172 173 174 175 176 | --- a/sys/platform/pc64/x86_64/pmap_inval.c +++ b/sys/platform/pc64/x86_64/pmap_inval.c @@ -277,16 +277,32 @@ pmap = kernel_pmap; /* + * DF-5420 (validated fix): + * (1) Acquire the interlock BEFORE sampling pm_active. The old + * code sampled the mask first, so a cpu concurrently switching + * into the pmap could slip its CPULOCK_EXCL test in before we + * set it and never see the invalidation. + * (2) Never use the single-cpu shortcut for user pmaps when we + * are modifying a PTE: pm_active is not a sound indicator that + * other cpus have no live TLB entries for the pmap (a + * descheduled/preempted cpu retains its TLB; the protocol + * relies on its reactivation %cr3 reload to flush, which is + * not guaranteed under virtualization and is too fragile to + * hang PTE teardown correctness on). + */ + pmap_inval_init(pmap); + + /* * Shortcut single-cpu case if possible. */ + if (pmap != kernel_pmap && ptep != NULL) + goto noshortcut; if (CPUMASK_CMPMASKEQ(pmap->pm_active, gd->gd_cpumask) && pmap_inval_force_nonopt == 0) { /* * Convert to invltlb if there are too many pages to * invlpg on. */ - if (pmap->pm_flags & PMAP_MULTI) - pmap_inval_init(pmap); if (npgs == 1) { if (ptep) opte = atomic_swap_long(ptep, npte); @@ -314,11 +330,11 @@ --npgs; } } - if (pmap->pm_flags & PMAP_MULTI) - pmap_inval_done(pmap); + pmap_inval_done(pmap); return opte; } +noshortcut: /* * We need a critical section to prevent getting preempted while @@ -327,8 +343,9 @@ * * tsc_target is our watchdog timeout that will attempt to recover * from a lost IPI. Set to 1/16 second for now. + * + * (DF-5420: interlock already acquired above.) */ - pmap_inval_init(pmap); info = &invinfo[cpu]; /* @@ -372,8 +389,15 @@ info->mode = INVSTORE; tmpmask = pmap->pm_active; /* volatile (bits may be cleared) */ - if (pmap_inval_force_allcpus) + if (pmap_inval_force_allcpus || + (pmap != kernel_pmap && ptep != NULL)) { + /* + * DF-5420: PTE-modifying user-pmap operations must not + * trust pm_active (descheduled cpus retain live TLB + * entries); broadcast to all active cpus instead. + */ tmpmask = smp_active_mask; + } cpu_ccfence(); CPUMASK_ANDMASK(tmpmask, smp_active_mask); @@ -426,6 +450,23 @@ */ ATOMIC_CPUMASK_NANDBIT(smp_invmask, cpu); write_rflags(rflags); + + /* + * DF-5420: wait for target cpus to finish their trailing + * invalidations before returning. The old code returned after + * clearing only our own done bit, leaving a window where the PTE + * was already torn down but target TLBs still held the old + * translation. + */ + { + int spins = 0; + + while (CPUMASK_TESTNZERO(info->done)) { + if (++spins > 100000000) + break; /* defensive */ + cpu_pause(); + } + } pmap_inval_done(pmap); return opte; @@ -455,31 +496,9 @@ pmap = kernel_pmap; /* - * Shortcut single-cpu case if possible. - */ - if (CPUMASK_CMPMASKEQ(pmap->pm_active, gd->gd_cpumask) && - pmap_inval_force_nonopt == 0) { - if (pmap->pm_flags & PMAP_MULTI) - pmap_inval_init(pmap); - if (atomic_cmpset_long(ptep, opte, npte)) { - if (va == (vm_offset_t)-1) - cpu_invltlb(); - else - cpu_invlpg((void *)va); - if (pmap->pm_flags & PMAP_MULTI) - pmap_inval_done(pmap); - return 1; - } else { - if (pmap->pm_flags & PMAP_MULTI) - pmap_inval_done(pmap); - return 0; - } - } - - /* - * We need a critical section to prevent getting preempted while - * we setup our command. A preemption might execute its own - * pmap_inval*() command and create confusion below. + * DF-5420 (validated fix): acquire the interlock BEFORE sampling + * pm_active, and do not use the single-cpu shortcut for user pmaps + * (see pmap_inval_smp). */ pmap_inval_init(pmap); info = &invinfo[cpu]; @@ -522,8 +541,15 @@ info->success = 0; tmpmask = pmap->pm_active; /* volatile */ - if (pmap_inval_force_allcpus) + if (pmap_inval_force_allcpus || + (pmap != kernel_pmap && ptep != NULL)) { + /* + * DF-5420: PTE-modifying user-pmap operations must not + * trust pm_active (descheduled cpus retain live TLB + * entries); broadcast to all active cpus instead. + */ tmpmask = smp_active_mask; + } cpu_ccfence(); CPUMASK_ANDMASK(tmpmask, smp_active_mask); CPUMASK_ORBIT(tmpmask, cpu); @@ -555,6 +581,20 @@ ATOMIC_CPUMASK_NANDBIT(smp_invmask, cpu); write_rflags(rflags); + + /* + * DF-5420: wait for target cpus to finish their trailing + * invalidations before returning (see pmap_inval_smp). + */ + { + int spins = 0; + + while (CPUMASK_TESTNZERO(info->done)) { + if (++spins > 100000000) + break; /* defensive */ + cpu_pause(); + } + } pmap_inval_done(pmap); return success; |