| Message ID | 20220815141027.780659-1-ardb@kernel.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | ARM: mm: shrink permanent FDT mapping to avoid mismatched attributes | expand |
On Mon, Aug 15, 2022 at 04:10:27PM +0200, Ard Biesheuvel wrote: > diff --git a/arch/arm/include/asm/memory.h b/arch/arm/include/asm/memory.h > index a55a9038abc8..aeb83eb5d251 100644 > --- a/arch/arm/include/asm/memory.h > +++ b/arch/arm/include/asm/memory.h > @@ -79,7 +79,6 @@ > #define XIP_VIRT_ADDR(physaddr) (MODULES_VADDR + ((physaddr) & 0x000fffff)) > > #define FDT_FIXED_BASE UL(0xff800000) > -#define FDT_FIXED_SIZE (2 * SECTION_SIZE) > #define FDT_VIRT_BASE(physbase) ((void *)(FDT_FIXED_BASE | (physbase) % SECTION_SIZE)) Should this now use PAGE_SIZE rather than SECTION_SIZE since we won't be using a section mapping?
On Mon, 15 Aug 2022 at 16:29, Russell King (Oracle) <linux@armlinux.org.uk> wrote: > > On Mon, Aug 15, 2022 at 04:10:27PM +0200, Ard Biesheuvel wrote: > > diff --git a/arch/arm/include/asm/memory.h b/arch/arm/include/asm/memory.h > > index a55a9038abc8..aeb83eb5d251 100644 > > --- a/arch/arm/include/asm/memory.h > > +++ b/arch/arm/include/asm/memory.h > > @@ -79,7 +79,6 @@ > > #define XIP_VIRT_ADDR(physaddr) (MODULES_VADDR + ((physaddr) & 0x000fffff)) > > > > #define FDT_FIXED_BASE UL(0xff800000) > > -#define FDT_FIXED_SIZE (2 * SECTION_SIZE) > > #define FDT_VIRT_BASE(physbase) ((void *)(FDT_FIXED_BASE | (physbase) % SECTION_SIZE)) > > Should this now use PAGE_SIZE rather than SECTION_SIZE since we won't > be using a section mapping? > The early boot code will still be using a section mapping, and so it implicitly performs the same conversion as FDT_VIRT_BASE does now. If we change this definition, the virtual mapping of the DT may shift around in memory when we switch from the early to the permanent mapping, which means we'll have to fix up initial_boot_params too. So I think it is easier just to leave it as is.
diff --git a/arch/arm/include/asm/memory.h b/arch/arm/include/asm/memory.h index a55a9038abc8..aeb83eb5d251 100644 --- a/arch/arm/include/asm/memory.h +++ b/arch/arm/include/asm/memory.h @@ -79,7 +79,6 @@ #define XIP_VIRT_ADDR(physaddr) (MODULES_VADDR + ((physaddr) & 0x000fffff)) #define FDT_FIXED_BASE UL(0xff800000) -#define FDT_FIXED_SIZE (2 * SECTION_SIZE) #define FDT_VIRT_BASE(physbase) ((void *)(FDT_FIXED_BASE | (physbase) % SECTION_SIZE)) #if !defined(CONFIG_SMP) && !defined(CONFIG_ARM_LPAE) diff --git a/arch/arm/kernel/setup.c b/arch/arm/kernel/setup.c index 1e8a50a97edf..fe07086f7e56 100644 --- a/arch/arm/kernel/setup.c +++ b/arch/arm/kernel/setup.c @@ -1092,6 +1092,8 @@ static struct notifier_block arm_restart_nb = { .priority = 128, }; +unsigned int __initdata dtsize; + void __init setup_arch(char **cmdline_p) { const struct machine_desc *mdesc = NULL; @@ -1103,9 +1105,10 @@ void __init setup_arch(char **cmdline_p) setup_processor(); if (atags_vaddr) { mdesc = setup_machine_fdt(atags_vaddr); - if (mdesc) - memblock_reserve(__atags_pointer, - fdt_totalsize(atags_vaddr)); + if (mdesc) { + dtsize = fdt_totalsize(atags_vaddr); + memblock_reserve(__atags_pointer, dtsize); + } } if (!mdesc) mdesc = setup_machine_tags(atags_vaddr, __machine_arch_type); diff --git a/arch/arm/mm/mmu.c b/arch/arm/mm/mmu.c index a49f0b9c0f75..ffe87df966ab 100644 --- a/arch/arm/mm/mmu.c +++ b/arch/arm/mm/mmu.c @@ -39,7 +39,7 @@ #include "mm.h" #include "tcm.h" -extern unsigned long __atags_pointer; +extern void *initial_boot_params; /* * empty_zero_page is a special page that is used for @@ -1388,11 +1388,14 @@ static void __init devicemaps_init(const struct machine_desc *mdesc) for (addr = VMALLOC_START; addr < (FIXADDR_TOP & PMD_MASK); addr += PMD_SIZE) pmd_clear(pmd_off_k(addr)); - if (__atags_pointer) { + if (IS_ENABLED(CONFIG_OF_FLATTREE) && initial_boot_params) { /* create a read-only mapping of the device tree */ - map.pfn = __phys_to_pfn(__atags_pointer & SECTION_MASK); - map.virtual = FDT_FIXED_BASE; - map.length = FDT_FIXED_SIZE; + extern unsigned long __atags_pointer; + extern unsigned int dtsize; + + map.pfn = __phys_to_pfn(__atags_pointer); + map.virtual = (unsigned long)initial_boot_params; + map.length = dtsize; map.type = MT_MEMORY_RO; create_mapping(&map); }
Zhen Lei writes in commit 598f0a99fa8a ("ARM: 9210/1: Mark the FDT_FIXED sections as shareable"): Commit 7a1be318f579 ("ARM: 9012/1: move device tree mapping out of linear region") uses FDT_FIXED_BASE to map the whole FDT_FIXED_SIZE memory area which contains fdt. But it only reserves the exact physical memory that fdt occupied. Unfortunately, this mapping is non-shareable. An illegal or speculative read access can bring the RAM content from non-fdt zone into cache, PIPT makes it to be hit by subsequently read access through shareable mapping (such as linear mapping), and the cache consistency between cores is lost due to non-shareable property. |<---------FDT_FIXED_SIZE------>| | | ------------------------------- | <non-fdt> | <fdt> | <non-fdt> | ------------------------------- 1. CoreA read <non-fdt> through MT_ROM mapping, the old data is loaded into the cache. 2. CoreB write <non-fdt> to update data through linear mapping. CoreA received the notification to invalid the corresponding cachelines, but the property non-shareable makes it to be ignored. 3. CoreA read <non-fdt> through linear mapping, cache hit, the old data is read. However, the resulting fix is incomplete, as mismatched shareability attributes are not the only potential problem vector here: the non-fdt regions might also be covered by a no-map memory reservation, or be mapped with non-cacheable attributes for, e.g., firmware calls or non-coherent DMA. This means, in order to eliminate any potential mismatched attribute mappings, we must reduce the size of the FDT mapping to match its memblock reservation, and eliminate the non-fdt regions altogether. The permanent FDT region will no longer cover the ATAGS when booting a non-DT system, but this mapping was never used or exposed after boot anyway. (The ATAGS are copied into a separate buffer by the early ATAGS processing code) Fixes: 7a1be318f579 ("ARM: 9012/1: move device tree mapping out of linear region") Reported-by: Zhen Lei <thunder.leizhen@huawei.com> Signed-off-by: Ard Biesheuvel <ardb@kernel.org> --- arch/arm/include/asm/memory.h | 1 - arch/arm/kernel/setup.c | 9 ++++++--- arch/arm/mm/mmu.c | 13 ++++++++----- 3 files changed, 14 insertions(+), 9 deletions(-)