Commit d81bbe6d authored by Mark Rutland's avatar Mark Rutland Committed by Will Deacon

Revert "arm64: mm: set the contiguous bit for kernel mappings where appropriate"

This reverts commit 0bfc445d.

When we change the permissions of regions mapped using contiguous
entries, the architecture requires us to follow a Break-Before-Make
strategy, breaking *all* associated entries before we can change any of
the following properties from the entries:

 - presence of the contiguous bit
 - output address
 - attributes
 - permissiones

Failure to do so can result in a number of problems (e.g. TLB conflict
aborts and/or erroneous results from TLB lookups).

See ARM DDI 0487A.k_iss10775, "Misprogramming of the Contiguous bit",
page D4-1762.

We do not take this into account when altering the permissions of kernel
segments in mark_rodata_ro(), where we change the permissions of live
contiguous entires one-by-one, leaving them transiently inconsistent.
This has been observed to result in failures on some fast model
configurations.

Unfortunately, we cannot follow Break-Before-Make here as we'd have to
unmap kernel text and data used to perform the sequence.

For the timebeing, revert commit 0bfc445d so as to avoid issues
resulting from this misuse of the contiguous bit.
Signed-off-by: default avatarMark Rutland <mark.rutland@arm.com>
Acked-by: default avatarArd Biesheuvel <ard.biesheuvel@linaro.org>
Reported-by: default avatarJean-Philippe Brucker <jean-philippe.brucker@arm.com>
Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: Will Deacon <Will.Deacon@arm.com>
Cc: stable@vger.kernel.org # v4.10
Signed-off-by: default avatarWill Deacon <will.deacon@arm.com>
parent ea6eac90
...@@ -109,10 +109,8 @@ static bool pgattr_change_is_safe(u64 old, u64 new) ...@@ -109,10 +109,8 @@ static bool pgattr_change_is_safe(u64 old, u64 new)
static void alloc_init_pte(pmd_t *pmd, unsigned long addr, static void alloc_init_pte(pmd_t *pmd, unsigned long addr,
unsigned long end, unsigned long pfn, unsigned long end, unsigned long pfn,
pgprot_t prot, pgprot_t prot,
phys_addr_t (*pgtable_alloc)(void), phys_addr_t (*pgtable_alloc)(void))
bool page_mappings_only)
{ {
pgprot_t __prot = prot;
pte_t *pte; pte_t *pte;
BUG_ON(pmd_sect(*pmd)); BUG_ON(pmd_sect(*pmd));
...@@ -130,18 +128,7 @@ static void alloc_init_pte(pmd_t *pmd, unsigned long addr, ...@@ -130,18 +128,7 @@ static void alloc_init_pte(pmd_t *pmd, unsigned long addr,
do { do {
pte_t old_pte = *pte; pte_t old_pte = *pte;
/* set_pte(pte, pfn_pte(pfn, prot));
* Set the contiguous bit for the subsequent group of PTEs if
* its size and alignment are appropriate.
*/
if (((addr | PFN_PHYS(pfn)) & ~CONT_PTE_MASK) == 0) {
if (end - addr >= CONT_PTE_SIZE && !page_mappings_only)
__prot = __pgprot(pgprot_val(prot) | PTE_CONT);
else
__prot = prot;
}
set_pte(pte, pfn_pte(pfn, __prot));
pfn++; pfn++;
/* /*
...@@ -160,7 +147,6 @@ static void alloc_init_pmd(pud_t *pud, unsigned long addr, unsigned long end, ...@@ -160,7 +147,6 @@ static void alloc_init_pmd(pud_t *pud, unsigned long addr, unsigned long end,
phys_addr_t (*pgtable_alloc)(void), phys_addr_t (*pgtable_alloc)(void),
bool page_mappings_only) bool page_mappings_only)
{ {
pgprot_t __prot = prot;
pmd_t *pmd; pmd_t *pmd;
unsigned long next; unsigned long next;
...@@ -187,18 +173,7 @@ static void alloc_init_pmd(pud_t *pud, unsigned long addr, unsigned long end, ...@@ -187,18 +173,7 @@ static void alloc_init_pmd(pud_t *pud, unsigned long addr, unsigned long end,
/* try section mapping first */ /* try section mapping first */
if (((addr | next | phys) & ~SECTION_MASK) == 0 && if (((addr | next | phys) & ~SECTION_MASK) == 0 &&
!page_mappings_only) { !page_mappings_only) {
/* pmd_set_huge(pmd, phys, prot);
* Set the contiguous bit for the subsequent group of
* PMDs if its size and alignment are appropriate.
*/
if (((addr | phys) & ~CONT_PMD_MASK) == 0) {
if (end - addr >= CONT_PMD_SIZE)
__prot = __pgprot(pgprot_val(prot) |
PTE_CONT);
else
__prot = prot;
}
pmd_set_huge(pmd, phys, __prot);
/* /*
* After the PMD entry has been populated once, we * After the PMD entry has been populated once, we
...@@ -208,8 +183,7 @@ static void alloc_init_pmd(pud_t *pud, unsigned long addr, unsigned long end, ...@@ -208,8 +183,7 @@ static void alloc_init_pmd(pud_t *pud, unsigned long addr, unsigned long end,
pmd_val(*pmd))); pmd_val(*pmd)));
} else { } else {
alloc_init_pte(pmd, addr, next, __phys_to_pfn(phys), alloc_init_pte(pmd, addr, next, __phys_to_pfn(phys),
prot, pgtable_alloc, prot, pgtable_alloc);
page_mappings_only);
BUG_ON(pmd_val(old_pmd) != 0 && BUG_ON(pmd_val(old_pmd) != 0 &&
pmd_val(old_pmd) != pmd_val(*pmd)); pmd_val(old_pmd) != pmd_val(*pmd));
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment