From mboxrd@z Thu Jan 1 00:00:00 1970 From: tixy@linaro.org (Jon Medhurst (Tixy)) Date: Tue, 04 Apr 2017 09:48:50 +0100 Subject: [PATCH v3] arm: Fix memory attribute inconsistencies when using fixmap In-Reply-To: <1491291087-17450-1-git-send-email-ard.biesheuvel@linaro.org> References: <1491291087-17450-1-git-send-email-ard.biesheuvel@linaro.org> Message-ID: <1491295730.2995.1.camel@linaro.org> To: linux-arm-kernel@lists.infradead.org List-Id: linux-arm-kernel.lists.infradead.org On Tue, 2017-04-04 at 08:31 +0100, Ard Biesheuvel wrote: > From: Jon Medhurst > > To cope with the variety in ARM architectures and configurations, the > pagetable attributes for kernel memory are generated at runtime to match > the system the kernel finds itself on. This calculated value is stored > in pgprot_kernel. > > However, when early fixmap support was added for ARM (commit > a5f4c561b3b1) the attributes used for mappings were hard coded because > pgprot_kernel is not set up early enough. Unfortunately, when fixmap is > used after early boot this means the memory being mapped can have > different attributes to existing mappings, potentially leading to > unpredictable behaviour. A specific problem also exists due to the hard > coded values not include the 'shareable' attribute which means on > systems where this matters (e.g. those with multiple CPU clusters) the > cache contents for a memory location can become inconsistent between > CPUs. > > To resolve these issues we change fixmap to use the same memory > attributes (from pgprot_kernel) that the rest of the kernel uses. To > enable this we need to refactor the initialisation code so > build_mem_type_table() is called early enough. Note, that relies on early > param parsing for memory type overrides passed via the kernel command > line, so we need to make sure this call is still after > parse_early_params(). > > Fixes: a5f4c561b3b1 ("ARM: 8415/1: early fixmap support for earlycon") > Cc: # v4.3+ > Signed-off-by: Jon Medhurst > [ardb: keep early_fixmap_init() before param parsing, for earlycon] > Signed-off-by: Ard Biesheuvel > --- [...] > diff --git a/arch/arm/mm/mmu.c b/arch/arm/mm/mmu.c > index 4e016d7f37b3..ec81a30479aa 100644 > --- a/arch/arm/mm/mmu.c > +++ b/arch/arm/mm/mmu.c > @@ -414,6 +414,11 @@ void __set_fixmap(enum fixed_addresses idx, phys_addr_t phys, pgprot_t prot) > FIXADDR_END); > BUG_ON(idx >= __end_of_fixed_addresses); > > + /* we only support device mappings until pgprot_kernel has been set */ > + if (WARN_ON(pgprot_val(prot) != pgprot_val(FIXMAP_PAGE_IO) && > + pgprot_val(pgprot_kernel) == 0)) > + return; > + Hmm, on return from this function, isn't the caller then going to try and access the memory at the fixmap address we didn't map, and so get a page fault? If so, would a BUG_ON here be better? > if (pgprot_val(prot)) > set_pte_at(NULL, vaddr, pte, > pfn_pte(phys >> PAGE_SHIFT, prot)); > @@ -1616,7 +1621,6 @@ void __init paging_init(const struct machine_desc *mdesc) > { > void *zero_page; > > - build_mem_type_table(); > prepare_page_table(); > map_lowmem(); > memblock_set_current_limit(arm_lowmem_limit); > @@ -1636,3 +1640,9 @@ void __init paging_init(const struct machine_desc *mdesc) > empty_zero_page = virt_to_page(zero_page); > __flush_dcache_page(NULL, empty_zero_page); > } > + > +void __init early_mm_init(const struct machine_desc *mdesc) > +{ > + build_mem_type_table(); > + early_paging_init(mdesc); > +} I tested this change with kprobes tests on TC2 (the setup that showed the original bug) and compile tested for a nommu device (I will boot test that in a bit as that's not quick to setup). Also, with this patch we can now make?early_paging_init() static now, and remove the extern... diff --git a/arch/arm/kernel/setup.c b/arch/arm/kernel/setup.c index f5db5548ad42..8ad1e4414024 100644 --- a/arch/arm/kernel/setup.c +++ b/arch/arm/kernel/setup.c @@ -80,7 +80,6 @@ __setup("fpe=", fpe_setup); extern void init_default_cache_policy(unsigned long); extern void paging_init(const struct machine_desc *desc); -extern void early_paging_init(const struct machine_desc *); extern void adjust_lowmem_bounds(void); extern enum reboot_mode reboot_mode; extern void setup_dma_zone(const struct machine_desc *desc); diff --git a/arch/arm/mm/mmu.c b/arch/arm/mm/mmu.c index ec81a30479aa..347cca965783 100644 --- a/arch/arm/mm/mmu.c +++ b/arch/arm/mm/mmu.c @@ -1497,7 +1497,7 @@ pgtables_remap lpae_pgtables_remap_asm; * early_paging_init() recreates boot time page table setup, allowing machines * to switch over to a high (>4G) address space on LPAE systems */ -void __init early_paging_init(const struct machine_desc *mdesc) +static void __init early_paging_init(const struct machine_desc *mdesc) { pgtables_remap *lpae_pgtables_remap; unsigned long pa_pgd; @@ -1565,7 +1565,7 @@ void __init early_paging_init(const struct machine_desc *mdesc) #else -void __init early_paging_init(const struct machine_desc *mdesc) +static void __init early_paging_init(const struct machine_desc *mdesc) { long long offset;