All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2] arm: Fix memory attribute inconsistencies when using fixmap
@ 2017-03-21 17:30 Jon Medhurst
  2017-03-21 17:36 ` Jon Medhurst (Tixy)
  0 siblings, 1 reply; 12+ messages in thread
From: Jon Medhurst @ 2017-03-21 17:30 UTC (permalink / raw)
  To: linux-arm-kernel

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 at vger.kernel.org # v4.3+
---

Changes since v1:
- Completely rewritten based on Russell's and Ard's sugestions
- Subject changed from "arm: Fix cache inconsistency when using fixmap"


 arch/arm/include/asm/fixmap.h |  7 +------
 arch/arm/include/asm/mmu.h    |  2 ++
 arch/arm/kernel/setup.c       |  5 +----
 arch/arm/mm/mmu.c             | 12 ++++++++++--
 4 files changed, 14 insertions(+), 12 deletions(-)

diff --git a/arch/arm/include/asm/fixmap.h b/arch/arm/include/asm/fixmap.h
index 5c17d2dec777..c54f06ace2a1 100644
--- a/arch/arm/include/asm/fixmap.h
+++ b/arch/arm/include/asm/fixmap.h
@@ -41,7 +41,7 @@ static const enum fixed_addresses __end_of_fixed_addresses =
 
 #define FIXMAP_PAGE_COMMON	(L_PTE_YOUNG | L_PTE_PRESENT | L_PTE_XN | L_PTE_DIRTY)
 
-#define FIXMAP_PAGE_NORMAL	(FIXMAP_PAGE_COMMON | L_PTE_MT_WRITEBACK)
+#define FIXMAP_PAGE_NORMAL	(pgprot_kernel | L_PTE_XN)
 #define FIXMAP_PAGE_RO		(FIXMAP_PAGE_NORMAL | L_PTE_RDONLY)
 
 /* Used by set_fixmap_(io|nocache), both meant for mapping a device */
@@ -53,13 +53,8 @@ static const enum fixed_addresses __end_of_fixed_addresses =
 #ifdef CONFIG_MMU
 
 void __set_fixmap(enum fixed_addresses idx, phys_addr_t phys, pgprot_t prot);
-void __init early_fixmap_init(void);
 
 #include <asm-generic/fixmap.h>
 
-#else
-
-static inline void early_fixmap_init(void) { }
-
 #endif
 #endif
diff --git a/arch/arm/include/asm/mmu.h b/arch/arm/include/asm/mmu.h
index a5b47421059d..920d29ca5c3b 100644
--- a/arch/arm/include/asm/mmu.h
+++ b/arch/arm/include/asm/mmu.h
@@ -24,6 +24,8 @@ typedef struct {
 #define ASID(mm)	(0)
 #endif
 
+void early_mm_init(void);
+
 #else
 
 /*
diff --git a/arch/arm/kernel/setup.c b/arch/arm/kernel/setup.c
index f4e54503afa9..37c59589aac3 100644
--- a/arch/arm/kernel/setup.c
+++ b/arch/arm/kernel/setup.c
@@ -40,7 +40,6 @@
 #include <asm/efi.h>
 #include <asm/elf.h>
 #include <asm/early_ioremap.h>
-#include <asm/fixmap.h>
 #include <asm/procinfo.h>
 #include <asm/psci.h>
 #include <asm/sections.h>
@@ -1082,12 +1081,10 @@ void __init setup_arch(char **cmdline_p)
 	strlcpy(cmd_line, boot_command_line, COMMAND_LINE_SIZE);
 	*cmdline_p = cmd_line;
 
-	early_fixmap_init();
-	early_ioremap_init();
-
 	parse_early_param();
 
 #ifdef CONFIG_MMU
+	early_mm_init();
 	early_paging_init(mdesc);
 #endif
 	setup_dma_zone(mdesc);
diff --git a/arch/arm/mm/mmu.c b/arch/arm/mm/mmu.c
index 4e016d7f37b3..f81809e6bd10 100644
--- a/arch/arm/mm/mmu.c
+++ b/arch/arm/mm/mmu.c
@@ -22,6 +22,7 @@
 #include <asm/cputype.h>
 #include <asm/sections.h>
 #include <asm/cachetype.h>
+#include <asm/early_ioremap.h>
 #include <asm/fixmap.h>
 #include <asm/sections.h>
 #include <asm/setup.h>
@@ -382,7 +383,7 @@ static inline pmd_t * __init fixmap_pmd(unsigned long addr)
 	return pmd;
 }
 
-void __init early_fixmap_init(void)
+static void __init early_fixmap_init(void)
 {
 	pmd_t *pmd;
 
@@ -1616,7 +1617,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 +1636,11 @@ 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(void)
+{
+	build_mem_type_table();
+
+	early_fixmap_init();
+	early_ioremap_init();
+}
-- 
2.11.0

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH v2] arm: Fix memory attribute inconsistencies when using fixmap
  2017-03-21 17:30 [PATCH v2] arm: Fix memory attribute inconsistencies when using fixmap Jon Medhurst
@ 2017-03-21 17:36 ` Jon Medhurst (Tixy)
  2017-03-21 19:00   ` Ard Biesheuvel
  0 siblings, 1 reply; 12+ messages in thread
From: Jon Medhurst (Tixy) @ 2017-03-21 17:36 UTC (permalink / raw)
  To: linux-arm-kernel

On Tue, 2017-03-21 at 17:30 +0000, Jon Medhurst wrote:
> 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 at vger.kernel.org # v4.3+

Sorry, this should have...
Signed-off-by: Jon Medhurst <tixy@linaro.org>

> ---
> 
> Changes since v1:
> - Completely rewritten based on Russell's and Ard's sugestions
> - Subject changed from "arm: Fix cache inconsistency when using fixmap"
> 
> 
>  arch/arm/include/asm/fixmap.h |  7 +------
>  arch/arm/include/asm/mmu.h    |  2 ++
>  arch/arm/kernel/setup.c       |  5 +----
>  arch/arm/mm/mmu.c             | 12 ++++++++++--
>  4 files changed, 14 insertions(+), 12 deletions(-)
> 
> diff --git a/arch/arm/include/asm/fixmap.h b/arch/arm/include/asm/fixmap.h
> index 5c17d2dec777..c54f06ace2a1 100644
> --- a/arch/arm/include/asm/fixmap.h
> +++ b/arch/arm/include/asm/fixmap.h
> @@ -41,7 +41,7 @@ static const enum fixed_addresses __end_of_fixed_addresses =
>  
>  #define FIXMAP_PAGE_COMMON	(L_PTE_YOUNG | L_PTE_PRESENT | L_PTE_XN | L_PTE_DIRTY)
>  
> -#define FIXMAP_PAGE_NORMAL	(FIXMAP_PAGE_COMMON | L_PTE_MT_WRITEBACK)
> +#define FIXMAP_PAGE_NORMAL	(pgprot_kernel | L_PTE_XN)
>  #define FIXMAP_PAGE_RO		(FIXMAP_PAGE_NORMAL | L_PTE_RDONLY)
>  
>  /* Used by set_fixmap_(io|nocache), both meant for mapping a device */
> @@ -53,13 +53,8 @@ static const enum fixed_addresses __end_of_fixed_addresses =
>  #ifdef CONFIG_MMU
>  
>  void __set_fixmap(enum fixed_addresses idx, phys_addr_t phys, pgprot_t prot);
> -void __init early_fixmap_init(void);
>  
>  #include <asm-generic/fixmap.h>
>  
> -#else
> -
> -static inline void early_fixmap_init(void) { }
> -
>  #endif
>  #endif
> diff --git a/arch/arm/include/asm/mmu.h b/arch/arm/include/asm/mmu.h
> index a5b47421059d..920d29ca5c3b 100644
> --- a/arch/arm/include/asm/mmu.h
> +++ b/arch/arm/include/asm/mmu.h
> @@ -24,6 +24,8 @@ typedef struct {
>  #define ASID(mm)	(0)
>  #endif
>  
> +void early_mm_init(void);
> +
>  #else
>  
>  /*
> diff --git a/arch/arm/kernel/setup.c b/arch/arm/kernel/setup.c
> index f4e54503afa9..37c59589aac3 100644
> --- a/arch/arm/kernel/setup.c
> +++ b/arch/arm/kernel/setup.c
> @@ -40,7 +40,6 @@
>  #include <asm/efi.h>
>  #include <asm/elf.h>
>  #include <asm/early_ioremap.h>
> -#include <asm/fixmap.h>
>  #include <asm/procinfo.h>
>  #include <asm/psci.h>
>  #include <asm/sections.h>
> @@ -1082,12 +1081,10 @@ void __init setup_arch(char **cmdline_p)
>  	strlcpy(cmd_line, boot_command_line, COMMAND_LINE_SIZE);
>  	*cmdline_p = cmd_line;
>  
> -	early_fixmap_init();
> -	early_ioremap_init();
> -
>  	parse_early_param();
>  
>  #ifdef CONFIG_MMU
> +	early_mm_init();
>  	early_paging_init(mdesc);
>  #endif
>  	setup_dma_zone(mdesc);
> diff --git a/arch/arm/mm/mmu.c b/arch/arm/mm/mmu.c
> index 4e016d7f37b3..f81809e6bd10 100644
> --- a/arch/arm/mm/mmu.c
> +++ b/arch/arm/mm/mmu.c
> @@ -22,6 +22,7 @@
>  #include <asm/cputype.h>
>  #include <asm/sections.h>
>  #include <asm/cachetype.h>
> +#include <asm/early_ioremap.h>
>  #include <asm/fixmap.h>
>  #include <asm/sections.h>
>  #include <asm/setup.h>
> @@ -382,7 +383,7 @@ static inline pmd_t * __init fixmap_pmd(unsigned long addr)
>  	return pmd;
>  }
>  
> -void __init early_fixmap_init(void)
> +static void __init early_fixmap_init(void)
>  {
>  	pmd_t *pmd;
>  
> @@ -1616,7 +1617,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 +1636,11 @@ 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(void)
> +{
> +	build_mem_type_table();
> +
> +	early_fixmap_init();
> +	early_ioremap_init();
> +}

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH v2] arm: Fix memory attribute inconsistencies when using fixmap
  2017-03-21 17:36 ` Jon Medhurst (Tixy)
@ 2017-03-21 19:00   ` Ard Biesheuvel
  2017-03-22  9:46     ` Jon Medhurst (Tixy)
  2017-04-03 17:18     ` Ard Biesheuvel
  0 siblings, 2 replies; 12+ messages in thread
From: Ard Biesheuvel @ 2017-03-21 19:00 UTC (permalink / raw)
  To: linux-arm-kernel

On 21 March 2017 at 17:36, Jon Medhurst (Tixy) <tixy@linaro.org> wrote:
> On Tue, 2017-03-21 at 17:30 +0000, Jon Medhurst wrote:
>> 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 at vger.kernel.org # v4.3+
>
> Sorry, this should have...
> Signed-off-by: Jon Medhurst <tixy@linaro.org>
>

Reviewed-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>

UEFI boot and earlycon both still work for me, so

Tested-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>

It looks like nommu should be unaffected, but it is worth giving it a
spin as well


Thanks,
Ard.


>> ---
>>
>> Changes since v1:
>> - Completely rewritten based on Russell's and Ard's sugestions
>> - Subject changed from "arm: Fix cache inconsistency when using fixmap"
>>
>>
>>  arch/arm/include/asm/fixmap.h |  7 +------
>>  arch/arm/include/asm/mmu.h    |  2 ++
>>  arch/arm/kernel/setup.c       |  5 +----
>>  arch/arm/mm/mmu.c             | 12 ++++++++++--
>>  4 files changed, 14 insertions(+), 12 deletions(-)
>>
>> diff --git a/arch/arm/include/asm/fixmap.h b/arch/arm/include/asm/fixmap.h
>> index 5c17d2dec777..c54f06ace2a1 100644
>> --- a/arch/arm/include/asm/fixmap.h
>> +++ b/arch/arm/include/asm/fixmap.h
>> @@ -41,7 +41,7 @@ static const enum fixed_addresses __end_of_fixed_addresses =
>>
>>  #define FIXMAP_PAGE_COMMON   (L_PTE_YOUNG | L_PTE_PRESENT | L_PTE_XN | L_PTE_DIRTY)
>>
>> -#define FIXMAP_PAGE_NORMAL   (FIXMAP_PAGE_COMMON | L_PTE_MT_WRITEBACK)
>> +#define FIXMAP_PAGE_NORMAL   (pgprot_kernel | L_PTE_XN)
>>  #define FIXMAP_PAGE_RO               (FIXMAP_PAGE_NORMAL | L_PTE_RDONLY)
>>
>>  /* Used by set_fixmap_(io|nocache), both meant for mapping a device */
>> @@ -53,13 +53,8 @@ static const enum fixed_addresses __end_of_fixed_addresses =
>>  #ifdef CONFIG_MMU
>>
>>  void __set_fixmap(enum fixed_addresses idx, phys_addr_t phys, pgprot_t prot);
>> -void __init early_fixmap_init(void);
>>
>>  #include <asm-generic/fixmap.h>
>>
>> -#else
>> -
>> -static inline void early_fixmap_init(void) { }
>> -
>>  #endif
>>  #endif
>> diff --git a/arch/arm/include/asm/mmu.h b/arch/arm/include/asm/mmu.h
>> index a5b47421059d..920d29ca5c3b 100644
>> --- a/arch/arm/include/asm/mmu.h
>> +++ b/arch/arm/include/asm/mmu.h
>> @@ -24,6 +24,8 @@ typedef struct {
>>  #define ASID(mm)     (0)
>>  #endif
>>
>> +void early_mm_init(void);
>> +
>>  #else
>>
>>  /*
>> diff --git a/arch/arm/kernel/setup.c b/arch/arm/kernel/setup.c
>> index f4e54503afa9..37c59589aac3 100644
>> --- a/arch/arm/kernel/setup.c
>> +++ b/arch/arm/kernel/setup.c
>> @@ -40,7 +40,6 @@
>>  #include <asm/efi.h>
>>  #include <asm/elf.h>
>>  #include <asm/early_ioremap.h>
>> -#include <asm/fixmap.h>
>>  #include <asm/procinfo.h>
>>  #include <asm/psci.h>
>>  #include <asm/sections.h>
>> @@ -1082,12 +1081,10 @@ void __init setup_arch(char **cmdline_p)
>>       strlcpy(cmd_line, boot_command_line, COMMAND_LINE_SIZE);
>>       *cmdline_p = cmd_line;
>>
>> -     early_fixmap_init();
>> -     early_ioremap_init();
>> -
>>       parse_early_param();
>>
>>  #ifdef CONFIG_MMU
>> +     early_mm_init();
>>       early_paging_init(mdesc);
>>  #endif
>>       setup_dma_zone(mdesc);
>> diff --git a/arch/arm/mm/mmu.c b/arch/arm/mm/mmu.c
>> index 4e016d7f37b3..f81809e6bd10 100644
>> --- a/arch/arm/mm/mmu.c
>> +++ b/arch/arm/mm/mmu.c
>> @@ -22,6 +22,7 @@
>>  #include <asm/cputype.h>
>>  #include <asm/sections.h>
>>  #include <asm/cachetype.h>
>> +#include <asm/early_ioremap.h>
>>  #include <asm/fixmap.h>
>>  #include <asm/sections.h>
>>  #include <asm/setup.h>
>> @@ -382,7 +383,7 @@ static inline pmd_t * __init fixmap_pmd(unsigned long addr)
>>       return pmd;
>>  }
>>
>> -void __init early_fixmap_init(void)
>> +static void __init early_fixmap_init(void)
>>  {
>>       pmd_t *pmd;
>>
>> @@ -1616,7 +1617,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 +1636,11 @@ 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(void)
>> +{
>> +     build_mem_type_table();
>> +
>> +     early_fixmap_init();
>> +     early_ioremap_init();
>> +}

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH v2] arm: Fix memory attribute inconsistencies when using fixmap
  2017-03-21 19:00   ` Ard Biesheuvel
@ 2017-03-22  9:46     ` Jon Medhurst (Tixy)
  2017-03-22 12:13       ` Ard Biesheuvel
  2017-04-03 17:18     ` Ard Biesheuvel
  1 sibling, 1 reply; 12+ messages in thread
From: Jon Medhurst (Tixy) @ 2017-03-22  9:46 UTC (permalink / raw)
  To: linux-arm-kernel

On Tue, 2017-03-21 at 19:00 +0000, Ard Biesheuvel wrote:
> On 21 March 2017 at 17:36, Jon Medhurst (Tixy) <tixy@linaro.org> wrote:
> > On Tue, 2017-03-21 at 17:30 +0000, Jon Medhurst wrote:
> > > 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 at vger.kernel.org # v4.3+
> > 
> > Sorry, this should have...
> > Signed-off-by: Jon Medhurst <tixy@linaro.org>
> > 
> 
> Reviewed-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> 
> UEFI boot and earlycon both still work for me, so
> 
> Tested-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> 
> It looks like nommu should be unaffected, but it is worth giving it a
> spin as well

I did build test a couple of nommu platforms. I'll try and work out how
to get Linux on the only nommu hardware I have (ARM's MPS2) but if that
kernel doesn't work before my applying my changes I'm not going to
debug, I've opened enough cans of worms for now.

-- 
Tixy

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH v2] arm: Fix memory attribute inconsistencies when using fixmap
  2017-03-22  9:46     ` Jon Medhurst (Tixy)
@ 2017-03-22 12:13       ` Ard Biesheuvel
  2017-03-22 12:18         ` Russell King - ARM Linux
  2017-03-22 15:26         ` Jon Medhurst (Tixy)
  0 siblings, 2 replies; 12+ messages in thread
From: Ard Biesheuvel @ 2017-03-22 12:13 UTC (permalink / raw)
  To: linux-arm-kernel

On 22 March 2017 at 09:46, Jon Medhurst (Tixy) <tixy@linaro.org> wrote:
> On Tue, 2017-03-21 at 19:00 +0000, Ard Biesheuvel wrote:
>> On 21 March 2017 at 17:36, Jon Medhurst (Tixy) <tixy@linaro.org> wrote:
>> > On Tue, 2017-03-21 at 17:30 +0000, Jon Medhurst wrote:
>> > > 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 at vger.kernel.org # v4.3+
>> >
>> > Sorry, this should have...
>> > Signed-off-by: Jon Medhurst <tixy@linaro.org>
>> >
>>
>> Reviewed-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
>>
>> UEFI boot and earlycon both still work for me, so
>>
>> Tested-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
>>
>> It looks like nommu should be unaffected, but it is worth giving it a
>> spin as well
>
> I did build test a couple of nommu platforms. I'll try and work out how
> to get Linux on the only nommu hardware I have (ARM's MPS2) but if that
> kernel doesn't work before my applying my changes I'm not going to
> debug, I've opened enough cans of worms for now.
>

I think build testing is reasonable: the routines we call are all
stubbed out for nommu anyway, and most changes are local to mmu.c, so
I don't see how this would change behavior on nommu (famous last
words)

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH v2] arm: Fix memory attribute inconsistencies when using fixmap
  2017-03-22 12:13       ` Ard Biesheuvel
@ 2017-03-22 12:18         ` Russell King - ARM Linux
  2017-03-22 15:26         ` Jon Medhurst (Tixy)
  1 sibling, 0 replies; 12+ messages in thread
From: Russell King - ARM Linux @ 2017-03-22 12:18 UTC (permalink / raw)
  To: linux-arm-kernel

On Wed, Mar 22, 2017 at 12:13:40PM +0000, Ard Biesheuvel wrote:
> I think build testing is reasonable: the routines we call are all
> stubbed out for nommu anyway, and most changes are local to mmu.c, so
> I don't see how this would change behavior on nommu (famous last
> words)

I'm hoping to finally get some regular testing of nommu in place
during the next few weeks, so hopefully the stability of nommu from
the arch code point of view should get better.  Individual platforms
are a different matter.

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH v2] arm: Fix memory attribute inconsistencies when using fixmap
  2017-03-22 12:13       ` Ard Biesheuvel
  2017-03-22 12:18         ` Russell King - ARM Linux
@ 2017-03-22 15:26         ` Jon Medhurst (Tixy)
  2017-03-24 20:17           ` afzal mohammed
  1 sibling, 1 reply; 12+ messages in thread
From: Jon Medhurst (Tixy) @ 2017-03-22 15:26 UTC (permalink / raw)
  To: linux-arm-kernel

On Wed, 2017-03-22 at 12:13 +0000, Ard Biesheuvel wrote:
> On 22 March 2017 at 09:46, Jon Medhurst (Tixy) <tixy@linaro.org> wrote:
> > On Tue, 2017-03-21 at 19:00 +0000, Ard Biesheuvel wrote:
> > 
> > > Reviewed-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> > > 
> > > UEFI boot and earlycon both still work for me, so
> > > 
> > > Tested-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>

I forgot to say thanks?earlier, so thanks! :-)

> > > It looks like nommu should be unaffected, but it is worth giving it a
> > > spin as well
> > 
> > I did build test a couple of nommu platforms. I'll try and work out how
> > to get Linux on the only nommu hardware I have (ARM's MPS2) but if that
> > kernel doesn't work before my applying my changes I'm not going to
> > debug, I've opened enough cans of worms for now.
> > 

After much faffing around I got MPS2 booting a Linux kernel, and
somewhat to my surprise?it seems to boot fine, all the way up to
panicking because it didn't find an init. This was with $subject patch.

-- 
Tixy

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH v2] arm: Fix memory attribute inconsistencies when using fixmap
  2017-03-22 15:26         ` Jon Medhurst (Tixy)
@ 2017-03-24 20:17           ` afzal mohammed
  2017-03-27  7:29             ` Ard Biesheuvel
  0 siblings, 1 reply; 12+ messages in thread
From: afzal mohammed @ 2017-03-24 20:17 UTC (permalink / raw)
  To: linux-arm-kernel

Hi,

On Wed, Mar 22, 2017 at 03:26:28PM +0000, Jon Medhurst (Tixy) wrote:
> > > On Tue, 2017-03-21 at 19:00 +0000, Ard Biesheuvel wrote:

> > > > It looks like nommu should be unaffected, but it is worth giving it a
> > > > spin as well
> 
> After much faffing around I got MPS2 booting a Linux kernel, and
> somewhat to my surprise?it seems to boot fine, all the way up to
> panicking because it didn't find an init. This was with $subject patch.

Vybrid VF610 Cortex M4 boots to prompt w/ this patch on current mainline.

Vybrid VF610 Cortex A5 using !MMU Kernel w/ this patch too boots to
prompt (climbing onto the shoulder's of Vladimir, not Torvalds HEAD)

Regards
afzal

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH v2] arm: Fix memory attribute inconsistencies when using fixmap
  2017-03-24 20:17           ` afzal mohammed
@ 2017-03-27  7:29             ` Ard Biesheuvel
  2017-03-27  8:53               ` Jon Medhurst (Tixy)
  0 siblings, 1 reply; 12+ messages in thread
From: Ard Biesheuvel @ 2017-03-27  7:29 UTC (permalink / raw)
  To: linux-arm-kernel

On 24 March 2017 at 20:17, afzal mohammed <afzal.mohd.ma@gmail.com> wrote:
> Hi,
>
> On Wed, Mar 22, 2017 at 03:26:28PM +0000, Jon Medhurst (Tixy) wrote:
>> > > On Tue, 2017-03-21 at 19:00 +0000, Ard Biesheuvel wrote:
>
>> > > > It looks like nommu should be unaffected, but it is worth giving it a
>> > > > spin as well
>>
>> After much faffing around I got MPS2 booting a Linux kernel, and
>> somewhat to my surprise it seems to boot fine, all the way up to
>> panicking because it didn't find an init. This was with $subject patch.
>
> Vybrid VF610 Cortex M4 boots to prompt w/ this patch on current mainline.
>
> Vybrid VF610 Cortex A5 using !MMU Kernel w/ this patch too boots to
> prompt (climbing onto the shoulder's of Vladimir, not Torvalds HEAD)
>

Thanks for testing!

I suppose this patch is ready to go into the patch system, with
Afzal's tested-by as well as mine.

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH v2] arm: Fix memory attribute inconsistencies when using fixmap
  2017-03-27  7:29             ` Ard Biesheuvel
@ 2017-03-27  8:53               ` Jon Medhurst (Tixy)
  0 siblings, 0 replies; 12+ messages in thread
From: Jon Medhurst (Tixy) @ 2017-03-27  8:53 UTC (permalink / raw)
  To: linux-arm-kernel

On Mon, 2017-03-27 at 08:29 +0100, Ard Biesheuvel wrote:
> On 24 March 2017 at 20:17, afzal mohammed <afzal.mohd.ma@gmail.com> wrote:
> > Hi,
> > 
> > On Wed, Mar 22, 2017 at 03:26:28PM +0000, Jon Medhurst (Tixy) wrote:
> > > > > On Tue, 2017-03-21 at 19:00 +0000, Ard Biesheuvel wrote:
> > > > > > It looks like nommu should be unaffected, but it is worth giving it a
> > > > > > spin as well
> > > 
> > > After much faffing around I got MPS2 booting a Linux kernel, and
> > > somewhat to my surprise it seems to boot fine, all the way up to
> > > panicking because it didn't find an init. This was with $subject patch.
> > 
> > Vybrid VF610 Cortex M4 boots to prompt w/ this patch on current mainline.
> > 
> > Vybrid VF610 Cortex A5 using !MMU Kernel w/ this patch too boots to
> > prompt (climbing onto the shoulder's of Vladimir, not Torvalds HEAD)
> > 
> 
> Thanks for testing!

Yes, thanks Afzal.

> 
> I suppose this patch is ready to go into the patch system, with
> Afzal's tested-by as well as mine.

Done...
http://www.armlinux.org.uk/developer/patches/viewpatch.php?id=8667/1

-- 
Tixy

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH v2] arm: Fix memory attribute inconsistencies when using fixmap
  2017-03-21 19:00   ` Ard Biesheuvel
  2017-03-22  9:46     ` Jon Medhurst (Tixy)
@ 2017-04-03 17:18     ` Ard Biesheuvel
  2017-04-03 20:00       ` Russell King - ARM Linux
  1 sibling, 1 reply; 12+ messages in thread
From: Ard Biesheuvel @ 2017-04-03 17:18 UTC (permalink / raw)
  To: linux-arm-kernel

On 21 March 2017 at 19:00, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:
> On 21 March 2017 at 17:36, Jon Medhurst (Tixy) <tixy@linaro.org> wrote:
>> On Tue, 2017-03-21 at 17:30 +0000, Jon Medhurst wrote:
>>> 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 at vger.kernel.org # v4.3+
>>
>> Sorry, this should have...
>> Signed-off-by: Jon Medhurst <tixy@linaro.org>
>>
>
> Reviewed-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
>
> UEFI boot and earlycon both still work for me, so
>
> Tested-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
>

Apologies, but it appears I screwed this up: earlycon does not
actually work, it seems, due to the fact that early_fixmap_init() is
now postponed to after parse_early_params(), but earlycon
configuration (which uses set_fixmap_io()) is triggered by the early
parsing of the 'earlycon' command line parameter, and so
early_fixmap_init() occurs to late now. I am not sure how I missed
this, but I obviously did.

Since fixmaps for I/O are not affected by the issue this patch
addresses, the correct way would be to call early_fixmap_init() as we
did before, but disallow FIXMAP_PAGE_NORMAL and FIXMAP_PAGE_RO
mappings until after early_mm_init has executed.

I'm happy to code that up as a followup patch (since I am partly to
blame for this patch having ended up in -next in its current shape)
but it will have to wait until tomorrow.

Regards,
Ard.

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH v2] arm: Fix memory attribute inconsistencies when using fixmap
  2017-04-03 17:18     ` Ard Biesheuvel
@ 2017-04-03 20:00       ` Russell King - ARM Linux
  0 siblings, 0 replies; 12+ messages in thread
From: Russell King - ARM Linux @ 2017-04-03 20:00 UTC (permalink / raw)
  To: linux-arm-kernel

On Mon, Apr 03, 2017 at 06:18:47PM +0100, Ard Biesheuvel wrote:
> Apologies, but it appears I screwed this up: earlycon does not
> actually work, it seems, due to the fact that early_fixmap_init() is
> now postponed to after parse_early_params(), but earlycon
> configuration (which uses set_fixmap_io()) is triggered by the early
> parsing of the 'earlycon' command line parameter, and so
> early_fixmap_init() occurs to late now. I am not sure how I missed
> this, but I obviously did.
> 
> Since fixmaps for I/O are not affected by the issue this patch
> addresses, the correct way would be to call early_fixmap_init() as we
> did before, but disallow FIXMAP_PAGE_NORMAL and FIXMAP_PAGE_RO
> mappings until after early_mm_init has executed.
> 
> I'm happy to code that up as a followup patch (since I am partly to
> blame for this patch having ended up in -next in its current shape)
> but it will have to wait until tomorrow.

Hmm, sounds like a good thing I haven't sent it to Linus yet then... I'll
continue holding off sending that, although I would like to send what I
have queued to Linus in the next couple of days.  So, I think I'll drop
8667/1 out of fixes, and requeue as appropriate depending on how the
update arrives.

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.

^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2017-04-03 20:00 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-03-21 17:30 [PATCH v2] arm: Fix memory attribute inconsistencies when using fixmap Jon Medhurst
2017-03-21 17:36 ` Jon Medhurst (Tixy)
2017-03-21 19:00   ` Ard Biesheuvel
2017-03-22  9:46     ` Jon Medhurst (Tixy)
2017-03-22 12:13       ` Ard Biesheuvel
2017-03-22 12:18         ` Russell King - ARM Linux
2017-03-22 15:26         ` Jon Medhurst (Tixy)
2017-03-24 20:17           ` afzal mohammed
2017-03-27  7:29             ` Ard Biesheuvel
2017-03-27  8:53               ` Jon Medhurst (Tixy)
2017-04-03 17:18     ` Ard Biesheuvel
2017-04-03 20:00       ` Russell King - ARM Linux

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.