Thread (8 messages) flat view 8 messages, 2 authors, 2015-12-14

[PATCH] arm64: mm: ensure that the zero page is visible to the page table walker

From: mark.rutland@arm.com (Mark Rutland)
Date: 2015-12-11 18:20:04
Also in: stable

quoted
quoted
+	/* Ensure the zero page is visible to the page table walker */
+	dsb(ishst);
I think this should live in early_alloc (likewise in late_alloc).

In the other cases we call early_alloc or late_allot we assume the
zeroing is visible to the page table walker.

For example in in alloc_init_pte we do:
	
	if (pmd_none(*pmd) || pmd_sect(*pmd)) {
		pte = alloc(PTRS_PER_PTE * sizeof(pte_t));
		if (pmd_sect(*pmd))
			split_pmd(pmd, pte);
		__pmd_populate(pmd, __pa(pte), PMD_TYPE_TABLE);
		flush_tlb_all();
	}

There's a dsb in __pmd_populate, but it's _after_ the write to the pmd
entry, so the walker might start walking the newly-allocated pte table
before the zeroing is visible.
Urgh. The reason this is a problem is because we're modifying the page
tables live (which I know that you're fixing) without using
break-before-make. Consequently, the usual ordering guarantees that we
get from the tlb flush after installing the invalid entry do not apply
and we end up with the issue you point out.
My feeling was that in these paths we usually assume all prior page
table updates have been made visible to the page table walkers. Given
that, having the allocator guarantee the zeroing was already visible
felt like the natural thing to do.

That said, having looked at mm/memory.c, we seem to follow the exact
same pattern when plumbing tables together dynamically, with only an
smp_wmb() between the zeroed allocation and plumbing a table entry in.

e.g. in __pte_alloc we have the pattern:

	pgtable_t new = pte_alloc_one(mm, address);
	smp_wmb();
	if (pmd_none(*pmd))
		pmd_populate(mm, pmd, new);
quoted
Either we need a barrier after every alloc, or we fold the barrier into
the two allocation functions.
Could you roll this into your patch that drops the size parameter from
the alloc functions please? Then we can name them {early,late}_alloc_pgtable
and have them do the dsb in there. Maybe we can drop it again when we're
doing proper break-before-make.
Sure, will do.

Thanks,
Mark.
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help