From: tixy@linaro.org (Jon Medhurst (Tixy))
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH v3] arm: Fix memory attribute inconsistencies when using fixmap
Date: Tue, 04 Apr 2017 09:48:50 +0100 [thread overview]
Message-ID: <1491295730.2995.1.camel@linaro.org> (raw)
In-Reply-To: <1491291087-17450-1-git-send-email-ard.biesheuvel@linaro.org>
On Tue, 2017-04-04 at 08:31 +0100, Ard Biesheuvel wrote:
> From: Jon Medhurst <tixy@linaro.org>
>
> 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: <stable@vger.kernel.org> # v4.3+
> Signed-off-by: Jon Medhurst <tixy@linaro.org>
> [ardb: keep early_fixmap_init() before param parsing, for earlycon]
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> ---
[...]
> 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;
next prev parent reply other threads:[~2017-04-04 8:48 UTC|newest]
Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-04-04 7:31 [PATCH v3] arm: Fix memory attribute inconsistencies when using fixmap Ard Biesheuvel
2017-04-04 8:48 ` Jon Medhurst (Tixy) [this message]
2017-04-04 9:10 ` Ard Biesheuvel
2017-04-04 16:01 ` Jon Medhurst (Tixy)
2017-04-05 22:42 ` Russell King - ARM Linux
2017-04-06 13:46 ` Jon Medhurst (Tixy)
2017-04-06 13:49 ` Ard Biesheuvel
2017-04-06 16:03 ` [PATCH v4] " Jon Medhurst (Tixy)
2017-04-08 16:51 ` afzal mohammed
2017-04-10 10:16 ` Jon Medhurst (Tixy)
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=1491295730.2995.1.camel@linaro.org \
--to=tixy@linaro.org \
--cc=linux-arm-kernel@lists.infradead.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).