> Date: Mon, 19 Feb 2018 13:49:48 +0100 (CET)
> From: Mark Kettenis <mark.kette...@xs4all.nl>
>
> The diff below attempts to make the arm64 pmap "mpsafe" and enables MP
> support. This diff survived a full build on my Firefly-RK3399 board.
> It also seems to work on the Overdrive 1000. It might even work on
> the Raspberry Pi 3. I'd appreciate it if people could play with this
> on the Raspberry Pi and other arm64 hardware they have.
>
> I tried to follow a strategy for making the pmap "mpsafe" that more
> closely resembles what we did for our other architectures, although I
> looked at Dale Rahn's diff as well. It seems I fixed at least some
> bugs in the process. But I'm sure there are bugs remaining. I may
> even have introduced new ones. So do expect this to crash and burn,
> or at least lock up.
>
> I'm also interested in people testing the normal GENERIC kernel with
> this diff, especially building ports. There are some changes in there
> that could affect non-MP as well.
Here is a slightly better diff that removes a potential lock ordering
problem, removes a redundant splvm and moves some code around to avoid
a level of indentation. Gaining confidence, so I'm looking for ok's
now.
Index: arch/arm64/arm64/cpu.c
===================================================================
RCS file: /cvs/src/sys/arch/arm64/arm64/cpu.c,v
retrieving revision 1.13
diff -u -p -r1.13 cpu.c
--- arch/arm64/arm64/cpu.c 31 Jan 2018 10:52:12 -0000 1.13
+++ arch/arm64/arm64/cpu.c 19 Feb 2018 12:28:05 -0000
@@ -454,13 +454,8 @@ cpu_start_secondary(struct cpu_info *ci)
spllower(IPL_NONE);
-#ifdef notyet
SCHED_LOCK(s);
cpu_switchto(NULL, sched_chooseproc());
-#else
- for (;;)
- __asm volatile("wfe");
-#endif
}
void
Index: arch/arm64/arm64/pmap.c
===================================================================
RCS file: /cvs/src/sys/arch/arm64/arm64/pmap.c,v
retrieving revision 1.47
diff -u -p -r1.47 pmap.c
--- arch/arm64/arm64/pmap.c 31 Jan 2018 23:23:16 -0000 1.47
+++ arch/arm64/arm64/pmap.c 19 Feb 2018 12:28:05 -0000
@@ -165,6 +165,19 @@ struct mem_region *pmap_allocated = &pma
int pmap_cnt_avail, pmap_cnt_allocated;
uint64_t pmap_avail_kvo;
+static inline void
+pmap_lock(struct pmap *pmap)
+{
+ if (pmap != pmap_kernel())
+ mtx_enter(&pmap->pm_mtx);
+}
+
+static inline void
+pmap_unlock(struct pmap *pmap)
+{
+ if (pmap != pmap_kernel())
+ mtx_leave(&pmap->pm_mtx);
+}
/* virtual to physical helpers */
static inline int
@@ -362,6 +375,7 @@ pmap_vp_page_alloc(struct pool *pp, int
void *v;
kd.kd_waitok = ISSET(flags, PR_WAITOK);
+ kd.kd_trylock = ISSET(flags, PR_NOWAIT);
kd.kd_slowdown = slowdown;
KERNEL_LOCK();
@@ -439,14 +453,20 @@ pmap_enter_pv(struct pte_desc *pted, str
if (__predict_false(!pmap_initialized))
return;
+ mtx_enter(&pg->mdpage.pv_mtx);
LIST_INSERT_HEAD(&(pg->mdpage.pv_list), pted, pted_pv_list);
pted->pted_va |= PTED_VA_MANAGED_M;
+ mtx_leave(&pg->mdpage.pv_mtx);
}
void
pmap_remove_pv(struct pte_desc *pted)
{
+ struct vm_page *pg = PHYS_TO_VM_PAGE(pted->pted_pte & PTE_RPGN);
+
+ mtx_enter(&pg->mdpage.pv_mtx);
LIST_REMOVE(pted, pted_pv_list);
+ mtx_leave(&pg->mdpage.pv_mtx);
}
int
@@ -454,7 +474,7 @@ pmap_enter(pmap_t pm, vaddr_t va, paddr_
{
struct pte_desc *pted;
struct vm_page *pg;
- int s, error;
+ int error;
int cache = PMAP_CACHE_WB;
int need_sync = 0;
@@ -464,9 +484,7 @@ pmap_enter(pmap_t pm, vaddr_t va, paddr_
cache = PMAP_CACHE_DEV;
pg = PHYS_TO_VM_PAGE(pa);
- /* MP - Acquire lock for this pmap */
-
- s = splvm();
+ pmap_lock(pm);
pted = pmap_vp_lookup(pm, va, NULL);
if (pted && PTED_VALID(pted)) {
pmap_remove_pted(pm, pted);
@@ -535,8 +553,7 @@ pmap_enter(pmap_t pm, vaddr_t va, paddr_
error = 0;
out:
- splx(s);
- /* MP - free pmap lock */
+ pmap_unlock(pm);
return error;
}
@@ -550,6 +567,7 @@ pmap_remove(pmap_t pm, vaddr_t sva, vadd
struct pte_desc *pted;
vaddr_t va;
+ pmap_lock(pm);
for (va = sva; va < eva; va += PAGE_SIZE) {
pted = pmap_vp_lookup(pm, va, NULL);
@@ -564,6 +582,7 @@ pmap_remove(pmap_t pm, vaddr_t sva, vadd
if (PTED_VALID(pted))
pmap_remove_pted(pm, pted);
}
+ pmap_unlock(pm);
}
/*
@@ -590,11 +609,12 @@ pmap_remove_pted(pmap_t pm, struct pte_d
pted->pted_va &= ~PTED_VA_EXEC_M;
}
- pted->pted_pte = 0;
-
if (PTED_MANAGED(pted))
pmap_remove_pv(pted);
+ pted->pted_pte = 0;
+ pted->pted_va = 0;
+
if (pm != pmap_kernel())
pool_put(&pmap_pted_pool, pted);
splx(s);
@@ -615,10 +635,6 @@ _pmap_kenter_pa(vaddr_t va, paddr_t pa,
{
pmap_t pm = pmap_kernel();
struct pte_desc *pted;
- int s;
-
- /* MP - lock pmap. */
- s = splvm();
pted = pmap_vp_lookup(pm, va, NULL);
@@ -645,8 +661,6 @@ _pmap_kenter_pa(vaddr_t va, paddr_t pa,
pmap_pte_insert(pted);
ttlb_flush(pm, va & ~PAGE_MASK);
-
- splx(s);
}
void
@@ -839,6 +853,9 @@ pmap_create(void)
pmap_t pmap;
pmap = pool_get(&pmap_pmap_pool, PR_WAITOK | PR_ZERO);
+
+ mtx_init(&pmap->pm_mtx, IPL_VM);
+
pmap_pinit(pmap);
if (pmap_vp_poolcache == 0) {
pool_setlowat(&pmap_vp_pool, 20);
@@ -853,7 +870,7 @@ pmap_create(void)
void
pmap_reference(pmap_t pm)
{
- pm->pm_refs++;
+ atomic_inc_int(&pm->pm_refs);
}
/*
@@ -865,7 +882,7 @@ pmap_destroy(pmap_t pm)
{
int refs;
- refs = --pm->pm_refs;
+ refs = atomic_dec_int_nv(&pm->pm_refs);
if (refs > 0)
return;
@@ -1384,41 +1401,67 @@ pmap_page_ro(pmap_t pm, vaddr_t va, vm_p
void
pmap_page_protect(struct vm_page *pg, vm_prot_t prot)
{
- int s;
struct pte_desc *pted;
-
- /* need to lock for this pv */
- s = splvm();
+ struct pmap *pm;
if (prot == PROT_NONE) {
- while (!LIST_EMPTY(&(pg->mdpage.pv_list))) {
+ mtx_enter(&pg->mdpage.pv_mtx);
+ while ((pted = LIST_FIRST(&(pg->mdpage.pv_list))) != NULL) {
+ pmap_reference(pted->pted_pmap);
+ pm = pted->pted_pmap;
+ mtx_leave(&pg->mdpage.pv_mtx);
+
+ pmap_lock(pm);
+
+ /*
+ * We dropped the pvlist lock before grabbing
+ * the pmap lock to avoid lock ordering
+ * problems. This means we have to check the
+ * pvlist again since somebody else might have
+ * modified it. All we care about is that the
+ * pvlist entry matches the pmap we just
+ * locked. If it doesn't, unlock the pmap and
+ * try again.
+ */
+ mtx_enter(&pg->mdpage.pv_mtx);
pted = LIST_FIRST(&(pg->mdpage.pv_list));
- pmap_remove_pted(pted->pted_pmap, pted);
+ if (pted == NULL || pted->pted_pmap != pm) {
+ mtx_leave(&pg->mdpage.pv_mtx);
+ pmap_unlock(pm);
+ pmap_destroy(pm);
+ mtx_enter(&pg->mdpage.pv_mtx);
+ continue;
+ }
+ mtx_leave(&pg->mdpage.pv_mtx);
+ pmap_remove_pted(pm, pted);
+ pmap_unlock(pm);
+ pmap_destroy(pm);
+
+ mtx_enter(&pg->mdpage.pv_mtx);
}
/* page is being reclaimed, sync icache next use */
atomic_clearbits_int(&pg->pg_flags, PG_PMAP_EXE);
- splx(s);
+ mtx_leave(&pg->mdpage.pv_mtx);
return;
}
+ mtx_enter(&pg->mdpage.pv_mtx);
LIST_FOREACH(pted, &(pg->mdpage.pv_list), pted_pv_list) {
pmap_page_ro(pted->pted_pmap, pted->pted_va, prot);
}
- splx(s);
+ mtx_leave(&pg->mdpage.pv_mtx);
}
void
pmap_protect(pmap_t pm, vaddr_t sva, vaddr_t eva, vm_prot_t prot)
{
- int s;
-
if (prot & (PROT_READ | PROT_EXEC)) {
- s = splvm();
+ pmap_lock(pm);
while (sva < eva) {
pmap_page_ro(pm, sva, 0);
sva += PAGE_SIZE;
}
- splx(s);
+ pmap_unlock(pm);
return;
}
pmap_remove(pm, sva, eva);
@@ -1446,8 +1489,8 @@ pmap_init(void)
pool_init(&pmap_pted_pool, sizeof(struct pte_desc), 0, IPL_VM, 0,
"pted", NULL);
pool_setlowat(&pmap_pted_pool, 20);
- pool_init(&pmap_vp_pool, sizeof(struct pmapvp0), PAGE_SIZE, IPL_VM,
- PR_WAITOK, "vp", &pmap_vp_allocator);
+ pool_init(&pmap_vp_pool, sizeof(struct pmapvp0), PAGE_SIZE, IPL_VM, 0,
+ "vp", &pmap_vp_allocator);
/* pool_setlowat(&pmap_vp_pool, 20); */
pmap_initialized = 1;
@@ -1692,12 +1735,10 @@ pmap_clear_modify(struct vm_page *pg)
{
struct pte_desc *pted;
uint64_t *pl3 = NULL;
- int s;
- s = splvm();
-
- pg->pg_flags &= ~PG_PMAP_MOD;
+ atomic_clearbits_int(&pg->pg_flags, PG_PMAP_MOD);
+ mtx_enter(&pg->mdpage.pv_mtx);
LIST_FOREACH(pted, &(pg->mdpage.pv_list), pted_pv_list) {
if (pmap_vp_lookup(pted->pted_pmap, pted->pted_va & ~PAGE_MASK,
&pl3) == NULL)
panic("failed to look up pte\n");
@@ -1706,7 +1747,7 @@ pmap_clear_modify(struct vm_page *pg)
ttlb_flush(pted->pted_pmap, pted->pted_va & ~PAGE_MASK);
}
- splx(s);
+ mtx_leave(&pg->mdpage.pv_mtx);
return 0;
}
@@ -1719,18 +1760,16 @@ int
pmap_clear_reference(struct vm_page *pg)
{
struct pte_desc *pted;
- int s;
-
- s = splvm();
- pg->pg_flags &= ~PG_PMAP_REF;
+ atomic_clearbits_int(&pg->pg_flags, PG_PMAP_REF);
+ mtx_enter(&pg->mdpage.pv_mtx);
LIST_FOREACH(pted, &(pg->mdpage.pv_list), pted_pv_list) {
pted->pted_pte &= ~PROT_MASK;
pmap_pte_insert(pted);
ttlb_flush(pted->pted_pmap, pted->pted_va & ~PAGE_MASK);
}
- splx(s);
+ mtx_leave(&pg->mdpage.pv_mtx);
return 0;
}
Index: arch/arm64/conf/GENERIC.MP
===================================================================
RCS file: arch/arm64/conf/GENERIC.MP
diff -N arch/arm64/conf/GENERIC.MP
--- /dev/null 1 Jan 1970 00:00:00 -0000
+++ arch/arm64/conf/GENERIC.MP 19 Feb 2018 12:28:05 -0000
@@ -0,0 +1,8 @@
+# $OpenBSD$
+
+include "arch/arm64/conf/GENERIC"
+
+option MULTIPROCESSOR
+#option MP_LOCKDEBUG
+
+cpu* at mainbus?
Index: arch/arm64/include/pmap.h
===================================================================
RCS file: /cvs/src/sys/arch/arm64/include/pmap.h,v
retrieving revision 1.9
diff -u -p -r1.9 pmap.h
--- arch/arm64/include/pmap.h 12 Jan 2018 14:52:55 -0000 1.9
+++ arch/arm64/include/pmap.h 19 Feb 2018 12:28:05 -0000
@@ -17,7 +17,9 @@
#ifndef _ARM64_PMAP_H_
#define _ARM64_PMAP_H_
-#include <arm64/pte.h>
+#include <sys/mutex.h>
+
+#include <machine/pte.h>
#define PMAP_PA_MASK ~((paddr_t)PAGE_MASK) /* to remove the flags */
#define PMAP_NOCACHE 0x1 /* non-cacheable memory */
@@ -66,6 +68,7 @@ void pagezero_cache(vaddr_t);
* Pmap stuff
*/
struct pmap {
+ struct mutex pm_mtx;
union {
struct pmapvp0 *l0; /* virtual to physical table 4 lvl */
struct pmapvp1 *l1; /* virtual to physical table 3 lvl */
@@ -104,12 +107,14 @@ void pmap_map_early(paddr_t, psize_t);
#define __HAVE_VM_PAGE_MD
struct vm_page_md {
+ struct mutex pv_mtx;
LIST_HEAD(,pte_desc) pv_list;
int pvh_attrs; /* page attributes */
};
#define VM_MDPAGE_INIT(pg) do { \
- LIST_INIT(&((pg)->mdpage.pv_list)); \
+ mtx_init(&(pg)->mdpage.pv_mtx, IPL_VM); \
+ LIST_INIT(&((pg)->mdpage.pv_list)); \
(pg)->mdpage.pvh_attrs = 0; \
} while (0)
