| Message ID | 20170321173030.7751-1-tixy@linaro.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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@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(); > +}
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@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(); >> +}
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@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.
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@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)
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.
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.
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
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.
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
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@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.
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.
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(); +}
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+ --- 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(-)