Fix the PCPU access macros. It was found that the PCPU pointer, when

held in register r13, is used outside the bounds of critical_enter()
and critical_exit() by virtue of optimizations performed by the
compiler. The net effect being that address computations of fields
in the PCPU structure could be relative to the PCPU structure of the
CPU on which the address computation was performed and not related
to the CPU that executes the actual load or store operation.
The typical failure mode being that the per-CPU cache of UMA got
corrupted due to accesses from other CPUs.

Adding more volatile decorating to the register expression does not
help. The thinking being that volatile is assumed to work on memory
references and not register references. Thus, the fix is to perform
the address computation using a volatile inline assembly statement.

Additionally, since the reference is fundamentally non-atomic on ia64
by virtue of have a distinct address computation followed by the
actual load or store operation, it is required to wrap the entire
PCPU access in a critical section.

With PCPU_GET and friends requiring curthread now that they're in a
critical section, low-level use of these macros in functions like
cpu_switch() is not possible anymore. Consequently, a second order
set of changes is needed to avoid using PCPU_GET and friends where
curthread is either not set yet, or in the process of being changed.
In those cases, explicit dereferencing of pcpup is needed. In those
cases it is also possible to do that.

This is a direct commit to stable/10.

Approved by:	re@ (marius)
This commit is contained in:
Marcel Moolenaar 2014-09-06 22:17:54 +00:00
parent a9efc14bb1
commit e31ece45b7
Notes: svn2git 2020-12-20 02:59:44 +00:00
svn path=/stable/10/; revision=271211
5 changed files with 53 additions and 20 deletions

View File

@ -171,8 +171,8 @@ ia64_highfp_save_ipi(void)
td->td_pcb->pcb_fpcpu = NULL;
PCPU_SET(fpcurthread, NULL);
}
wakeup(PCPU_PTR(fpcurthread));
mtx_unlock_spin(&ia64_highfp_mtx);
wakeup(&PCPU_GET(fpcurthread));
return ((td != NULL) ? 1 : 0);
}

View File

@ -458,7 +458,7 @@ cpu_switch(struct thread *old, struct thread *new, struct mtx *mtx)
#ifdef COMPAT_FREEBSD32
ia32_savectx(oldpcb);
#endif
if (PCPU_GET(fpcurthread) == old)
if (pcpup->pc_fpcurthread == old)
old->td_frame->tf_special.psr |= IA64_PSR_DFH;
if (!savectx(oldpcb)) {
newpcb = new->td_pcb;
@ -472,13 +472,13 @@ cpu_switch(struct thread *old, struct thread *new, struct mtx *mtx)
cpu_spinwait();
#endif
PCPU_SET(curthread, new);
pcpup->pc_curthread = new;
#ifdef COMPAT_FREEBSD32
ia32_restorectx(newpcb);
#endif
if (PCPU_GET(fpcurthread) == new)
if (pcpup->pc_fpcurthread == new)
new->td_frame->tf_special.psr &= ~IA64_PSR_DFH;
restorectx(newpcb);
/* We should not get here. */
@ -500,7 +500,7 @@ cpu_throw(struct thread *old __unused, struct thread *new)
cpu_spinwait();
#endif
PCPU_SET(curthread, new);
pcpup->pc_curthread = new;
#ifdef COMPAT_FREEBSD32
ia32_restorectx(newpcb);
@ -833,7 +833,7 @@ ia64_init(void)
pcpu_init(pcpup, 0, sizeof(pcpu0));
dpcpu_init(ia64_physmem_alloc(DPCPU_SIZE, PAGE_SIZE), 0);
cpu_pcpu_setup(pcpup, ~0U, ia64_get_lid());
PCPU_SET(curthread, &thread0);
pcpup->pc_curthread = &thread0;
/*
* Initialize the console before we print anything out.

View File

@ -222,7 +222,7 @@ ia64_ap_startup(void)
ia64_ap_state.as_trace = 0x108;
vhpt = PCPU_GET(md.vhpt);
vhpt = pcpup->pc_md.vhpt;
map_vhpt(vhpt);
ia64_set_pta(vhpt + (1 << 8) + (pmap_vhpt_log2size << 2) + 1);
ia64_srlz_i();
@ -246,8 +246,8 @@ ia64_ap_startup(void)
cpu_spinwait();
/* Initialize curthread. */
KASSERT(PCPU_GET(idlethread) != NULL, ("no idle thread"));
PCPU_SET(curthread, PCPU_GET(idlethread));
KASSERT(pcpup->pc_idlethread != NULL, ("no idle thread"));
pcpup->pc_curthread = pcpup->pc_idlethread;
pmap_invalidate_all();

View File

@ -94,7 +94,7 @@ ia64_xtrace_init_common(vm_paddr_t pa)
__asm __volatile("mov psr.l=%0" :: "r" (psr));
ia64_srlz_i();
PCPU_SET(md.xtrace_tail, ia64_xtrace_base);
pcpup->pc_md.xtrace_tail = ia64_xtrace_base;
ia64_set_k3(ia64_xtrace_base);
}
@ -119,7 +119,7 @@ ia64_xtrace_init_ap(void *buf)
ia64_set_k3(0);
return;
}
PCPU_SET(md.xtrace_buffer, buf);
pcpup->pc_md.xtrace_buffer = buf;
pa = ia64_tpa((uintptr_t)buf);
ia64_xtrace_init_common(pa);
}
@ -140,7 +140,7 @@ ia64_xtrace_init_bsp(void)
ia64_set_k3(0);
return;
}
PCPU_SET(md.xtrace_buffer, buf);
pcpup->pc_md.xtrace_buffer = buf;
pa = IA64_RR_MASK((uintptr_t)buf);
ia64_xtrace_init_common(pa);
}

View File

@ -31,6 +31,7 @@
#define _MACHINE_PCPU_H_
#include <sys/sysctl.h>
#include <sys/systm.h>
#include <machine/pcb.h>
struct pcpu_stats {
@ -85,16 +86,48 @@ __curthread(void)
}
#define curthread (__curthread())
#define PCPU_GET(member) (pcpup->pc_ ## member)
#define __pcpu_offset(name) __offsetof(struct pcpu, name)
#define __pcpu_type(name) __typeof(((struct pcpu *)0)->name)
#define PCPU_ADD(name, val) \
do { \
__pcpu_type(pc_ ## name) *nmp; \
critical_enter(); \
__asm __volatile("add %0=%1,r13;;" : \
"=r"(nmp) : "i"(__pcpu_offset(pc_ ## name))); \
*nmp += val; \
critical_exit(); \
} while (0)
#define PCPU_GET(name) \
({ __pcpu_type(pc_ ## name) *nmp; \
__pcpu_type(pc_ ## name) res; \
critical_enter(); \
__asm __volatile("add %0=%1,r13;;" : \
"=r"(nmp) : "i"(__pcpu_offset(pc_ ## name))); \
res = *nmp; \
critical_exit(); \
res; \
})
/*
* XXX The implementation of this operation should be made atomic
* with respect to preemption.
*/
#define PCPU_ADD(member, value) (pcpup->pc_ ## member += (value))
#define PCPU_INC(member) PCPU_ADD(member, 1)
#define PCPU_PTR(member) (&pcpup->pc_ ## member)
#define PCPU_SET(member,value) (pcpup->pc_ ## member = (value))
#define PCPU_PTR(name) \
({ __pcpu_type(pc_ ## name) *nmp; \
__asm __volatile("add %0=%1,r13;;" : \
"=r"(nmp) : "i"(__pcpu_offset(pc_ ## name))); \
nmp; \
})
#define PCPU_SET(name, val) \
do { \
__pcpu_type(pc_ ## name) *nmp; \
critical_enter(); \
__asm __volatile("add %0=%1,r13;;" : \
"=r"(nmp) : "i"(__pcpu_offset(pc_ ## name))); \
*nmp = val; \
critical_exit(); \
} while (0)
#endif /* _KERNEL */