Mark Kettenis
2016-07-31 18:03:58 UTC
The ARMv7 ARM says in B3.9 that
The architecture guarantees that a translation table entry that
generates a Translation fault or an Access flag fault is not held in
the TLB. However a translation table entry that generates a Domain
fault or a Permission fault might be held in the TLB.
and that
Any translation table entry that does not generate a Translation or
Access flag fault and is not out of context might be allocated to an
enabled TLB at any time. The only translation table entries guaranteed
not to be held in the TLB are those that generate a Translation or
Access flag fault.
So the CPU might speculatively load TLB entries. The upshot from this
is that we always have to perform a TLB flush if we modify a valid
entry. So we can't rely on PV_BEEN_REFD() to decide whether we should
flush or not. The diff below fixes thi. The diff seems to fix the
pmap_fault_fixup() messages on a Cortex A53 system. It's very likely
that this will fix them on Cortex A7 as well.
Index: arch/arm/arm/pmap7.c
===================================================================
RCS file: /cvs/src/sys/arch/arm/arm/pmap7.c,v
retrieving revision 1.29
diff -u -p -r1.29 pmap7.c
--- arch/arm/arm/pmap7.c 29 Jul 2016 06:46:15 -0000 1.29
+++ arch/arm/arm/pmap7.c 31 Jul 2016 17:55:27 -0000
@@ -373,13 +373,7 @@ struct pv_entry {
* Macro to determine if a mapping might be resident in the
* instruction cache and/or TLB
*/
-#define PV_BEEN_EXECD(f) (((f) & (PVF_REF | PVF_EXEC)) == (PVF_REF | PVF_EXEC))
-
-/*
- * Macro to determine if a mapping might be resident in the
- * data cache and/or TLB
- */
-#define PV_BEEN_REFD(f) (((f) & PVF_REF) != 0)
+#define PV_BEEN_EXECD(f) (((f) & PVF_EXEC) != 0)
/*
* Local prototypes
@@ -1034,11 +1028,12 @@ pmap_clearbit(struct vm_page *pg, u_int
*ptep = npte;
PTE_SYNC(ptep);
/* Flush the TLB entry if a current pmap. */
- if (PV_BEEN_EXECD(oflags))
- pmap_tlb_flushID_SE(pm, pv->pv_va);
- else
- if (PV_BEEN_REFD(oflags))
- pmap_tlb_flushD_SE(pm, pv->pv_va);
+ if (l2pte_valid(opte)) {
+ if (PV_BEEN_EXECD(oflags))
+ pmap_tlb_flushID_SE(pm, pv->pv_va);
+ else
+ pmap_tlb_flushD_SE(pm, pv->pv_va);
+ }
}
NPDEBUG(PDB_BITS,
@@ -1454,11 +1449,12 @@ pmap_enter(pmap_t pm, vaddr_t va, paddr_
}
}
- if (PV_BEEN_EXECD(oflags))
- pmap_tlb_flushID_SE(pm, va);
- else
- if (PV_BEEN_REFD(oflags))
- pmap_tlb_flushD_SE(pm, va);
+ if (l2pte_valid(opte)) {
+ if (PV_BEEN_EXECD(oflags))
+ pmap_tlb_flushID_SE(pm, va);
+ else
+ pmap_tlb_flushD_SE(pm, va);
+ }
}
/*
@@ -1484,7 +1480,7 @@ pmap_remove(pmap_t pm, vaddr_t sva, vadd
struct l2_bucket *l2b;
vaddr_t next_bucket;
pt_entry_t *ptep;
- u_int mappings, is_exec, is_refd;
+ u_int mappings, is_exec;
NPDEBUG(PDB_REMOVE, printf("pmap_remove: pmap=%p sva=%08lx eva=%08lx\n",
pm, sva, eva));
@@ -1525,7 +1521,6 @@ pmap_remove(pmap_t pm, vaddr_t sva, vadd
pm->pm_stats.resident_count--;
pa = l2pte_pa(pte);
is_exec = 0;
- is_refd = l2pte_valid(pte);
/*
* Update flags. In a number of circumstances,
@@ -1538,7 +1533,6 @@ pmap_remove(pmap_t pm, vaddr_t sva, vadd
pve = pmap_remove_pv(pg, pm, sva);
if (pve != NULL) {
is_exec = PV_BEEN_EXECD(pve->pv_flags);
- is_refd = PV_BEEN_REFD(pve->pv_flags);
pool_put(&pmap_pv_pool, pve);
}
}
@@ -1553,11 +1547,12 @@ pmap_remove(pmap_t pm, vaddr_t sva, vadd
*ptep = L2_TYPE_INV;
PTE_SYNC(ptep);
- if (is_exec)
- pmap_tlb_flushID_SE(pm, sva);
- else
- if (is_refd)
- pmap_tlb_flushD_SE(pm, sva);
+ if (l2pte_valid(pte)) {
+ if (is_exec)
+ pmap_tlb_flushID_SE(pm, sva);
+ else
+ pmap_tlb_flushD_SE(pm, sva);
+ }
sva += PAGE_SIZE;
ptep++;
@@ -1732,7 +1727,7 @@ void
pmap_protect(pmap_t pm, vaddr_t sva, vaddr_t eva, vm_prot_t prot)
{
struct l2_bucket *l2b;
- pt_entry_t *ptep, pte;
+ pt_entry_t *ptep, pte, opte;
vaddr_t next_bucket;
u_int flags;
int flush;
@@ -1782,19 +1777,19 @@ NPDEBUG(PDB_PROTECT, printf("\n"));
ptep = &l2b->l2b_kva[l2pte_index(sva)];
while (sva < next_bucket) {
- pte = *ptep;
+ opte = *ptep;
/* !!! not l2pte_valid */
/* XXX actually would only matter if really valid ??? */
- if (pte != 0 && l2pte_is_writeable(pte, pm)) {
+ if (opte != 0 && l2pte_is_writeable(opte, pm)) {
struct vm_page *pg;
u_int f;
- pg = PHYS_TO_VM_PAGE(l2pte_pa(pte));
+ pg = PHYS_TO_VM_PAGE(l2pte_pa(opte));
if (pg != NULL)
pmap_clean_page(pg, FALSE);
- pte = (pte & ~L2_S_PROT_MASK) |
+ pte = (opte & ~L2_S_PROT_MASK) |
L2_S_PROT(pm == pmap_kernel() ? PTE_KERNEL : PTE_USER,
- pte & L2_V7_S_XN ? PROT_READ : PROT_READ | PROT_EXEC);
+ opte & L2_V7_S_XN ? PROT_READ : PROT_READ | PROT_EXEC);
*ptep = pte;
PTE_SYNC(ptep);
@@ -1802,15 +1797,16 @@ NPDEBUG(PDB_PROTECT, printf("\n"));
f = pmap_modify_pv(pg, pm, sva,
PVF_WRITE, 0);
} else
- f = PVF_REF | PVF_EXEC;
+ f = PVF_EXEC;
if (flush >= 0) {
flush++;
- if (PV_BEEN_EXECD(f))
- cpu_tlb_flushID_SE(sva);
- else
- if (PV_BEEN_REFD(f))
- cpu_tlb_flushD_SE(sva);
+ if (l2pte_valid(opte)) {
+ if (PV_BEEN_EXECD(f))
+ cpu_tlb_flushID_SE(sva);
+ else
+ cpu_tlb_flushD_SE(sva);
+ }
} else
flags |= f;
}
@@ -1824,7 +1820,6 @@ NPDEBUG(PDB_PROTECT, printf("\n"));
if (PV_BEEN_EXECD(flags))
pmap_tlb_flushID(pm);
else
- if (PV_BEEN_REFD(flags))
pmap_tlb_flushD(pm);
}
NPDEBUG(PDB_PROTECT, printf("\n"));
The architecture guarantees that a translation table entry that
generates a Translation fault or an Access flag fault is not held in
the TLB. However a translation table entry that generates a Domain
fault or a Permission fault might be held in the TLB.
and that
Any translation table entry that does not generate a Translation or
Access flag fault and is not out of context might be allocated to an
enabled TLB at any time. The only translation table entries guaranteed
not to be held in the TLB are those that generate a Translation or
Access flag fault.
So the CPU might speculatively load TLB entries. The upshot from this
is that we always have to perform a TLB flush if we modify a valid
entry. So we can't rely on PV_BEEN_REFD() to decide whether we should
flush or not. The diff below fixes thi. The diff seems to fix the
pmap_fault_fixup() messages on a Cortex A53 system. It's very likely
that this will fix them on Cortex A7 as well.
Index: arch/arm/arm/pmap7.c
===================================================================
RCS file: /cvs/src/sys/arch/arm/arm/pmap7.c,v
retrieving revision 1.29
diff -u -p -r1.29 pmap7.c
--- arch/arm/arm/pmap7.c 29 Jul 2016 06:46:15 -0000 1.29
+++ arch/arm/arm/pmap7.c 31 Jul 2016 17:55:27 -0000
@@ -373,13 +373,7 @@ struct pv_entry {
* Macro to determine if a mapping might be resident in the
* instruction cache and/or TLB
*/
-#define PV_BEEN_EXECD(f) (((f) & (PVF_REF | PVF_EXEC)) == (PVF_REF | PVF_EXEC))
-
-/*
- * Macro to determine if a mapping might be resident in the
- * data cache and/or TLB
- */
-#define PV_BEEN_REFD(f) (((f) & PVF_REF) != 0)
+#define PV_BEEN_EXECD(f) (((f) & PVF_EXEC) != 0)
/*
* Local prototypes
@@ -1034,11 +1028,12 @@ pmap_clearbit(struct vm_page *pg, u_int
*ptep = npte;
PTE_SYNC(ptep);
/* Flush the TLB entry if a current pmap. */
- if (PV_BEEN_EXECD(oflags))
- pmap_tlb_flushID_SE(pm, pv->pv_va);
- else
- if (PV_BEEN_REFD(oflags))
- pmap_tlb_flushD_SE(pm, pv->pv_va);
+ if (l2pte_valid(opte)) {
+ if (PV_BEEN_EXECD(oflags))
+ pmap_tlb_flushID_SE(pm, pv->pv_va);
+ else
+ pmap_tlb_flushD_SE(pm, pv->pv_va);
+ }
}
NPDEBUG(PDB_BITS,
@@ -1454,11 +1449,12 @@ pmap_enter(pmap_t pm, vaddr_t va, paddr_
}
}
- if (PV_BEEN_EXECD(oflags))
- pmap_tlb_flushID_SE(pm, va);
- else
- if (PV_BEEN_REFD(oflags))
- pmap_tlb_flushD_SE(pm, va);
+ if (l2pte_valid(opte)) {
+ if (PV_BEEN_EXECD(oflags))
+ pmap_tlb_flushID_SE(pm, va);
+ else
+ pmap_tlb_flushD_SE(pm, va);
+ }
}
/*
@@ -1484,7 +1480,7 @@ pmap_remove(pmap_t pm, vaddr_t sva, vadd
struct l2_bucket *l2b;
vaddr_t next_bucket;
pt_entry_t *ptep;
- u_int mappings, is_exec, is_refd;
+ u_int mappings, is_exec;
NPDEBUG(PDB_REMOVE, printf("pmap_remove: pmap=%p sva=%08lx eva=%08lx\n",
pm, sva, eva));
@@ -1525,7 +1521,6 @@ pmap_remove(pmap_t pm, vaddr_t sva, vadd
pm->pm_stats.resident_count--;
pa = l2pte_pa(pte);
is_exec = 0;
- is_refd = l2pte_valid(pte);
/*
* Update flags. In a number of circumstances,
@@ -1538,7 +1533,6 @@ pmap_remove(pmap_t pm, vaddr_t sva, vadd
pve = pmap_remove_pv(pg, pm, sva);
if (pve != NULL) {
is_exec = PV_BEEN_EXECD(pve->pv_flags);
- is_refd = PV_BEEN_REFD(pve->pv_flags);
pool_put(&pmap_pv_pool, pve);
}
}
@@ -1553,11 +1547,12 @@ pmap_remove(pmap_t pm, vaddr_t sva, vadd
*ptep = L2_TYPE_INV;
PTE_SYNC(ptep);
- if (is_exec)
- pmap_tlb_flushID_SE(pm, sva);
- else
- if (is_refd)
- pmap_tlb_flushD_SE(pm, sva);
+ if (l2pte_valid(pte)) {
+ if (is_exec)
+ pmap_tlb_flushID_SE(pm, sva);
+ else
+ pmap_tlb_flushD_SE(pm, sva);
+ }
sva += PAGE_SIZE;
ptep++;
@@ -1732,7 +1727,7 @@ void
pmap_protect(pmap_t pm, vaddr_t sva, vaddr_t eva, vm_prot_t prot)
{
struct l2_bucket *l2b;
- pt_entry_t *ptep, pte;
+ pt_entry_t *ptep, pte, opte;
vaddr_t next_bucket;
u_int flags;
int flush;
@@ -1782,19 +1777,19 @@ NPDEBUG(PDB_PROTECT, printf("\n"));
ptep = &l2b->l2b_kva[l2pte_index(sva)];
while (sva < next_bucket) {
- pte = *ptep;
+ opte = *ptep;
/* !!! not l2pte_valid */
/* XXX actually would only matter if really valid ??? */
- if (pte != 0 && l2pte_is_writeable(pte, pm)) {
+ if (opte != 0 && l2pte_is_writeable(opte, pm)) {
struct vm_page *pg;
u_int f;
- pg = PHYS_TO_VM_PAGE(l2pte_pa(pte));
+ pg = PHYS_TO_VM_PAGE(l2pte_pa(opte));
if (pg != NULL)
pmap_clean_page(pg, FALSE);
- pte = (pte & ~L2_S_PROT_MASK) |
+ pte = (opte & ~L2_S_PROT_MASK) |
L2_S_PROT(pm == pmap_kernel() ? PTE_KERNEL : PTE_USER,
- pte & L2_V7_S_XN ? PROT_READ : PROT_READ | PROT_EXEC);
+ opte & L2_V7_S_XN ? PROT_READ : PROT_READ | PROT_EXEC);
*ptep = pte;
PTE_SYNC(ptep);
@@ -1802,15 +1797,16 @@ NPDEBUG(PDB_PROTECT, printf("\n"));
f = pmap_modify_pv(pg, pm, sva,
PVF_WRITE, 0);
} else
- f = PVF_REF | PVF_EXEC;
+ f = PVF_EXEC;
if (flush >= 0) {
flush++;
- if (PV_BEEN_EXECD(f))
- cpu_tlb_flushID_SE(sva);
- else
- if (PV_BEEN_REFD(f))
- cpu_tlb_flushD_SE(sva);
+ if (l2pte_valid(opte)) {
+ if (PV_BEEN_EXECD(f))
+ cpu_tlb_flushID_SE(sva);
+ else
+ cpu_tlb_flushD_SE(sva);
+ }
} else
flags |= f;
}
@@ -1824,7 +1820,6 @@ NPDEBUG(PDB_PROTECT, printf("\n"));
if (PV_BEEN_EXECD(flags))
pmap_tlb_flushID(pm);
else
- if (PV_BEEN_REFD(flags))
pmap_tlb_flushD(pm);
}
NPDEBUG(PDB_PROTECT, printf("\n"));