Message ID | 20201104063713.3039035-1-anup.patel@wdc.com (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
Series | RISC-V: Use non-PGD mappings for early DTB access | expand |
On Tue, Nov 3, 2020 at 10:37 PM Anup Patel <anup.patel@wdc.com> wrote: > > Currently, we use PGD mappings for early DTB mapping in early_pgd > but this breaks Linux kernel on SiFive Unleashed because on SiFive > Unleashed PMP checks don't work correctly for PGD mappings. > Not sure why this is an issue with Unleashed only. Microchip Polarfire SoC seems to work fine with early DTB pa as 0x88000000. However, unleashed fails to boot if the DTB is loaded any address below 0xC0000000 and it is mapped via PGD. > To fix early DTB mappings on SiFive Unleashed, we use non-PGD > mappings (i.e. PMD) for early DTB access. > > Fixes: 8f3a2b4a96dc ("RISC-V: Move DT mapping outof fixmap") > Signed-off-by: Anup Patel <anup.patel@wdc.com> > --- > arch/riscv/mm/init.c | 14 ++++++++++++++ > 1 file changed, 14 insertions(+) > > diff --git a/arch/riscv/mm/init.c b/arch/riscv/mm/init.c > index ea933b789a88..0d13d0c36a7d 100644 > --- a/arch/riscv/mm/init.c > +++ b/arch/riscv/mm/init.c > @@ -297,6 +297,7 @@ pmd_t fixmap_pmd[PTRS_PER_PMD] __page_aligned_bss; > #define NUM_EARLY_PMDS (1UL + MAX_EARLY_MAPPING_SIZE / PGDIR_SIZE) > #endif > pmd_t early_pmd[PTRS_PER_PMD * NUM_EARLY_PMDS] __initdata __aligned(PAGE_SIZE); > +pmd_t early_dtb_pmd[PTRS_PER_PMD] __initdata __aligned(PAGE_SIZE); > > static pmd_t *__init get_pmd_virt_early(phys_addr_t pa) > { > @@ -494,6 +495,18 @@ asmlinkage void __init setup_vm(uintptr_t dtb_pa) > load_pa + (va - PAGE_OFFSET), > map_size, PAGE_KERNEL_EXEC); > > +#ifndef __PAGETABLE_PMD_FOLDED > + /* Setup early PMD for DTB */ > + create_pgd_mapping(early_pg_dir, DTB_EARLY_BASE_VA, > + (uintptr_t)early_dtb_pmd, PGDIR_SIZE, PAGE_TABLE); > + /* Create two consecutive PMD mappings for FDT early scan */ > + pa = dtb_pa & ~(PMD_SIZE - 1); > + create_pmd_mapping(early_dtb_pmd, DTB_EARLY_BASE_VA, > + pa, PMD_SIZE, PAGE_KERNEL); > + create_pmd_mapping(early_dtb_pmd, DTB_EARLY_BASE_VA + PMD_SIZE, > + pa + PMD_SIZE, PMD_SIZE, PAGE_KERNEL); > + dtb_early_va = (void *)DTB_EARLY_BASE_VA + (dtb_pa & (PMD_SIZE - 1)); > +#else > /* Create two consecutive PGD mappings for FDT early scan */ > pa = dtb_pa & ~(PGDIR_SIZE - 1); > create_pgd_mapping(early_pg_dir, DTB_EARLY_BASE_VA, > @@ -501,6 +514,7 @@ asmlinkage void __init setup_vm(uintptr_t dtb_pa) > create_pgd_mapping(early_pg_dir, DTB_EARLY_BASE_VA + PGDIR_SIZE, > pa + PGDIR_SIZE, PGDIR_SIZE, PAGE_KERNEL); > dtb_early_va = (void *)DTB_EARLY_BASE_VA + (dtb_pa & (PGDIR_SIZE - 1)); > +#endif > dtb_early_pa = dtb_pa; > > /* > -- > 2.25.1 > > > _______________________________________________ > linux-riscv mailing list > linux-riscv@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-riscv FWIW: Tested on Unleashed & Polarfire SoC. Tested-by: Atish Patra <atish.patra@wdc.com> Reviewed-by: Atish Patra <atish.patra@wdc.com> @palmer: Can you consider this patch in next rcX PR ?
On Tue, 03 Nov 2020 22:37:13 PST (-0800), Anup Patel wrote: > Currently, we use PGD mappings for early DTB mapping in early_pgd > but this breaks Linux kernel on SiFive Unleashed because on SiFive > Unleashed PMP checks don't work correctly for PGD mappings. > > To fix early DTB mappings on SiFive Unleashed, we use non-PGD > mappings (i.e. PMD) for early DTB access. > > Fixes: 8f3a2b4a96dc ("RISC-V: Move DT mapping outof fixmap") > Signed-off-by: Anup Patel <anup.patel@wdc.com> > --- > arch/riscv/mm/init.c | 14 ++++++++++++++ > 1 file changed, 14 insertions(+) > > diff --git a/arch/riscv/mm/init.c b/arch/riscv/mm/init.c > index ea933b789a88..0d13d0c36a7d 100644 > --- a/arch/riscv/mm/init.c > +++ b/arch/riscv/mm/init.c > @@ -297,6 +297,7 @@ pmd_t fixmap_pmd[PTRS_PER_PMD] __page_aligned_bss; > #define NUM_EARLY_PMDS (1UL + MAX_EARLY_MAPPING_SIZE / PGDIR_SIZE) > #endif > pmd_t early_pmd[PTRS_PER_PMD * NUM_EARLY_PMDS] __initdata __aligned(PAGE_SIZE); > +pmd_t early_dtb_pmd[PTRS_PER_PMD] __initdata __aligned(PAGE_SIZE); > > static pmd_t *__init get_pmd_virt_early(phys_addr_t pa) > { > @@ -494,6 +495,18 @@ asmlinkage void __init setup_vm(uintptr_t dtb_pa) > load_pa + (va - PAGE_OFFSET), > map_size, PAGE_KERNEL_EXEC); > > +#ifndef __PAGETABLE_PMD_FOLDED > + /* Setup early PMD for DTB */ > + create_pgd_mapping(early_pg_dir, DTB_EARLY_BASE_VA, > + (uintptr_t)early_dtb_pmd, PGDIR_SIZE, PAGE_TABLE); > + /* Create two consecutive PMD mappings for FDT early scan */ > + pa = dtb_pa & ~(PMD_SIZE - 1); > + create_pmd_mapping(early_dtb_pmd, DTB_EARLY_BASE_VA, > + pa, PMD_SIZE, PAGE_KERNEL); > + create_pmd_mapping(early_dtb_pmd, DTB_EARLY_BASE_VA + PMD_SIZE, > + pa + PMD_SIZE, PMD_SIZE, PAGE_KERNEL); > + dtb_early_va = (void *)DTB_EARLY_BASE_VA + (dtb_pa & (PMD_SIZE - 1)); > +#else > /* Create two consecutive PGD mappings for FDT early scan */ > pa = dtb_pa & ~(PGDIR_SIZE - 1); > create_pgd_mapping(early_pg_dir, DTB_EARLY_BASE_VA, > @@ -501,6 +514,7 @@ asmlinkage void __init setup_vm(uintptr_t dtb_pa) > create_pgd_mapping(early_pg_dir, DTB_EARLY_BASE_VA + PGDIR_SIZE, > pa + PGDIR_SIZE, PGDIR_SIZE, PAGE_KERNEL); > dtb_early_va = (void *)DTB_EARLY_BASE_VA + (dtb_pa & (PGDIR_SIZE - 1)); > +#endif > dtb_early_pa = dtb_pa; > > /* We're starting to build up a handful of workarounds for these sorts of things. The PMP trap vs WARL one was the last I could remember, but that's a bit different as both of those behaviors were allowed by specifications at some point. IIRC there were also some TLB shootdown issues floating The best I can come up with is to add both some sort of "minimum support specification version" Kconfig entry and an "quirks" set of Kconfig entries. That would allow us to gradually jettison old ISAs as well as more cleanly add support for broken hardware like this. Do you have a pointer to some datasheet type document that describes the issue? That'd probably be the line I'd like to draw for adding workarounds like this, as otherwise we can't really be sure something is a hardware issue. That said, it's better to have the fix so this is on fixes. I'll try to remember it for the list of workarounds. Thanks!
On Fri, Nov 6, 2020 at 1:30 PM Palmer Dabbelt <palmerdabbelt@google.com> wrote: > > On Tue, 03 Nov 2020 22:37:13 PST (-0800), Anup Patel wrote: > > Currently, we use PGD mappings for early DTB mapping in early_pgd > > but this breaks Linux kernel on SiFive Unleashed because on SiFive > > Unleashed PMP checks don't work correctly for PGD mappings. > > > > To fix early DTB mappings on SiFive Unleashed, we use non-PGD > > mappings (i.e. PMD) for early DTB access. > > > > Fixes: 8f3a2b4a96dc ("RISC-V: Move DT mapping outof fixmap") > > Signed-off-by: Anup Patel <anup.patel@wdc.com> > > --- > > arch/riscv/mm/init.c | 14 ++++++++++++++ > > 1 file changed, 14 insertions(+) > > > > diff --git a/arch/riscv/mm/init.c b/arch/riscv/mm/init.c > > index ea933b789a88..0d13d0c36a7d 100644 > > --- a/arch/riscv/mm/init.c > > +++ b/arch/riscv/mm/init.c > > @@ -297,6 +297,7 @@ pmd_t fixmap_pmd[PTRS_PER_PMD] __page_aligned_bss; > > #define NUM_EARLY_PMDS (1UL + MAX_EARLY_MAPPING_SIZE / PGDIR_SIZE) > > #endif > > pmd_t early_pmd[PTRS_PER_PMD * NUM_EARLY_PMDS] __initdata __aligned(PAGE_SIZE); > > +pmd_t early_dtb_pmd[PTRS_PER_PMD] __initdata __aligned(PAGE_SIZE); > > > > static pmd_t *__init get_pmd_virt_early(phys_addr_t pa) > > { > > @@ -494,6 +495,18 @@ asmlinkage void __init setup_vm(uintptr_t dtb_pa) > > load_pa + (va - PAGE_OFFSET), > > map_size, PAGE_KERNEL_EXEC); > > > > +#ifndef __PAGETABLE_PMD_FOLDED > > + /* Setup early PMD for DTB */ > > + create_pgd_mapping(early_pg_dir, DTB_EARLY_BASE_VA, > > + (uintptr_t)early_dtb_pmd, PGDIR_SIZE, PAGE_TABLE); > > + /* Create two consecutive PMD mappings for FDT early scan */ > > + pa = dtb_pa & ~(PMD_SIZE - 1); > > + create_pmd_mapping(early_dtb_pmd, DTB_EARLY_BASE_VA, > > + pa, PMD_SIZE, PAGE_KERNEL); > > + create_pmd_mapping(early_dtb_pmd, DTB_EARLY_BASE_VA + PMD_SIZE, > > + pa + PMD_SIZE, PMD_SIZE, PAGE_KERNEL); > > + dtb_early_va = (void *)DTB_EARLY_BASE_VA + (dtb_pa & (PMD_SIZE - 1)); > > +#else > > /* Create two consecutive PGD mappings for FDT early scan */ > > pa = dtb_pa & ~(PGDIR_SIZE - 1); > > create_pgd_mapping(early_pg_dir, DTB_EARLY_BASE_VA, > > @@ -501,6 +514,7 @@ asmlinkage void __init setup_vm(uintptr_t dtb_pa) > > create_pgd_mapping(early_pg_dir, DTB_EARLY_BASE_VA + PGDIR_SIZE, > > pa + PGDIR_SIZE, PGDIR_SIZE, PAGE_KERNEL); > > dtb_early_va = (void *)DTB_EARLY_BASE_VA + (dtb_pa & (PGDIR_SIZE - 1)); > > +#endif > > dtb_early_pa = dtb_pa; > > > > /* > > We're starting to build up a handful of workarounds for these sorts of things. > The PMP trap vs WARL one was the last I could remember, but that's a bit > different as both of those behaviors were allowed by specifications at some > point. IIRC there were also some TLB shootdown issues floating > > The best I can come up with is to add both some sort of "minimum support > specification version" Kconfig entry and an "quirks" set of Kconfig entries. > That would allow us to gradually jettison old ISAs as well as more cleanly add > support for broken hardware like this. > > Do you have a pointer to some datasheet type document that describes the issue? Unfortunately, we still don't know where all SiFive erratums are documented. Maybe you know the right person who can publish this ?? The PMP checks not working correctly for PGD-mappings was discovered independently by Bin Meng on some other OS (almost 1.5 years back). Few discussions about this erratum have happened on OpenSBI mailing list as well although in a very different context. In fact, Andrew had confirmed about this erratum in sw-dev google groups. (Refer, https://groups.google.com/a/groups.riscv.org/g/sw-dev/c/CfyT5Los5uA/m/yY0ES0dZBAAJ?pli=1) (Refer, https://github.com/riscv/opensbi/issues/103) Till now we have encountered two erratums for SiFive U54 cores: 1. SFENCE.VMA not working with "virtual address" parameter 2. PMP checks not working for PGD mappings The Microchip ICICLE kit has U54 cores but the PMP checks issue is only seen on SiFive Unleashed so I guess this issue is fixed in-between but we can't be sure without detailed documentation. > That'd probably be the line I'd like to draw for adding workarounds like this, > as otherwise we can't really be sure something is a hardware issue. > > That said, it's better to have the fix so this is on fixes. I'll try to > remember it for the list of workarounds. I agree. At the moment, 5.10-rc2 does not boot on SiFive Unleashed but it boots on Microchip icicle kit and everywhere else because of the PMP checks issue. Regards, Anup
On Fri, Nov 6, 2020 at 4:52 PM Anup Patel <anup@brainfault.org> wrote: > > On Fri, Nov 6, 2020 at 1:30 PM Palmer Dabbelt <palmerdabbelt@google.com> wrote: > > > > On Tue, 03 Nov 2020 22:37:13 PST (-0800), Anup Patel wrote: > > > Currently, we use PGD mappings for early DTB mapping in early_pgd > > > but this breaks Linux kernel on SiFive Unleashed because on SiFive > > > Unleashed PMP checks don't work correctly for PGD mappings. > > > > > > To fix early DTB mappings on SiFive Unleashed, we use non-PGD > > > mappings (i.e. PMD) for early DTB access. > > > > > > Fixes: 8f3a2b4a96dc ("RISC-V: Move DT mapping outof fixmap") > > > Signed-off-by: Anup Patel <anup.patel@wdc.com> > > > --- > > > arch/riscv/mm/init.c | 14 ++++++++++++++ > > > 1 file changed, 14 insertions(+) > > > > > > diff --git a/arch/riscv/mm/init.c b/arch/riscv/mm/init.c > > > index ea933b789a88..0d13d0c36a7d 100644 > > > --- a/arch/riscv/mm/init.c > > > +++ b/arch/riscv/mm/init.c > > > @@ -297,6 +297,7 @@ pmd_t fixmap_pmd[PTRS_PER_PMD] __page_aligned_bss; > > > #define NUM_EARLY_PMDS (1UL + MAX_EARLY_MAPPING_SIZE / PGDIR_SIZE) > > > #endif > > > pmd_t early_pmd[PTRS_PER_PMD * NUM_EARLY_PMDS] __initdata __aligned(PAGE_SIZE); > > > +pmd_t early_dtb_pmd[PTRS_PER_PMD] __initdata __aligned(PAGE_SIZE); > > > > > > static pmd_t *__init get_pmd_virt_early(phys_addr_t pa) > > > { > > > @@ -494,6 +495,18 @@ asmlinkage void __init setup_vm(uintptr_t dtb_pa) > > > load_pa + (va - PAGE_OFFSET), > > > map_size, PAGE_KERNEL_EXEC); > > > > > > +#ifndef __PAGETABLE_PMD_FOLDED > > > + /* Setup early PMD for DTB */ > > > + create_pgd_mapping(early_pg_dir, DTB_EARLY_BASE_VA, > > > + (uintptr_t)early_dtb_pmd, PGDIR_SIZE, PAGE_TABLE); > > > + /* Create two consecutive PMD mappings for FDT early scan */ > > > + pa = dtb_pa & ~(PMD_SIZE - 1); > > > + create_pmd_mapping(early_dtb_pmd, DTB_EARLY_BASE_VA, > > > + pa, PMD_SIZE, PAGE_KERNEL); > > > + create_pmd_mapping(early_dtb_pmd, DTB_EARLY_BASE_VA + PMD_SIZE, > > > + pa + PMD_SIZE, PMD_SIZE, PAGE_KERNEL); > > > + dtb_early_va = (void *)DTB_EARLY_BASE_VA + (dtb_pa & (PMD_SIZE - 1)); > > > +#else > > > /* Create two consecutive PGD mappings for FDT early scan */ > > > pa = dtb_pa & ~(PGDIR_SIZE - 1); > > > create_pgd_mapping(early_pg_dir, DTB_EARLY_BASE_VA, > > > @@ -501,6 +514,7 @@ asmlinkage void __init setup_vm(uintptr_t dtb_pa) > > > create_pgd_mapping(early_pg_dir, DTB_EARLY_BASE_VA + PGDIR_SIZE, > > > pa + PGDIR_SIZE, PGDIR_SIZE, PAGE_KERNEL); > > > dtb_early_va = (void *)DTB_EARLY_BASE_VA + (dtb_pa & (PGDIR_SIZE - 1)); > > > +#endif > > > dtb_early_pa = dtb_pa; > > > > > > /* > > > > We're starting to build up a handful of workarounds for these sorts of things. > > The PMP trap vs WARL one was the last I could remember, but that's a bit > > different as both of those behaviors were allowed by specifications at some > > point. IIRC there were also some TLB shootdown issues floating > > > > The best I can come up with is to add both some sort of "minimum support > > specification version" Kconfig entry and an "quirks" set of Kconfig entries. > > That would allow us to gradually jettison old ISAs as well as more cleanly add > > support for broken hardware like this. > > > > Do you have a pointer to some datasheet type document that describes the issue? > > Unfortunately, we still don't know where all SiFive erratums are documented. > Maybe you know the right person who can publish this ?? Last time Andrew said this erratum would be published but did not give a name of whom from SiFive in charge .. > > The PMP checks not working correctly for PGD-mappings was discovered > independently by Bin Meng on some other OS (almost 1.5 years back). Few Yep, this undocumented erratum was initially discovered during our VxWorks RISC-V port. > discussions about this erratum have happened on OpenSBI mailing list as well > although in a very different context. In fact, Andrew had confirmed about this > erratum in sw-dev google groups. > (Refer, https://groups.google.com/a/groups.riscv.org/g/sw-dev/c/CfyT5Los5uA/m/yY0ES0dZBAAJ?pli=1) > (Refer, https://github.com/riscv/opensbi/issues/103) The issue is not just about gigapages. Using megapages also could trigger this issue. To me this erratum seems like: if any PTEs are set up to overlap with PMP protected memory, FU540 will trigger an instruction access exception as soon as SATP is written. > > Till now we have encountered two erratums for SiFive U54 cores: > 1. SFENCE.VMA not working with "virtual address" parameter > 2. PMP checks not working for PGD mappings > > The Microchip ICICLE kit has U54 cores but the PMP checks issue > is only seen on SiFive Unleashed so I guess this issue is fixed > in-between but we can't be sure without detailed documentation. I believe Microchip PolarFire SoC suffers this same erratum but I did not verify that. Currently PMP is configured to protect the eNVM range on PolarFire, and this range is not mapped by any PTE yet in S-mode. > > > That'd probably be the line I'd like to draw for adding workarounds like this, > > as otherwise we can't really be sure something is a hardware issue. > > > > That said, it's better to have the fix so this is on fixes. I'll try to > > remember it for the list of workarounds. > > I agree. At the moment, 5.10-rc2 does not boot on SiFive Unleashed > but it boots on Microchip icicle kit and everywhere else because of > the PMP checks issue. Regards, Bin
diff --git a/arch/riscv/mm/init.c b/arch/riscv/mm/init.c index ea933b789a88..0d13d0c36a7d 100644 --- a/arch/riscv/mm/init.c +++ b/arch/riscv/mm/init.c @@ -297,6 +297,7 @@ pmd_t fixmap_pmd[PTRS_PER_PMD] __page_aligned_bss; #define NUM_EARLY_PMDS (1UL + MAX_EARLY_MAPPING_SIZE / PGDIR_SIZE) #endif pmd_t early_pmd[PTRS_PER_PMD * NUM_EARLY_PMDS] __initdata __aligned(PAGE_SIZE); +pmd_t early_dtb_pmd[PTRS_PER_PMD] __initdata __aligned(PAGE_SIZE); static pmd_t *__init get_pmd_virt_early(phys_addr_t pa) { @@ -494,6 +495,18 @@ asmlinkage void __init setup_vm(uintptr_t dtb_pa) load_pa + (va - PAGE_OFFSET), map_size, PAGE_KERNEL_EXEC); +#ifndef __PAGETABLE_PMD_FOLDED + /* Setup early PMD for DTB */ + create_pgd_mapping(early_pg_dir, DTB_EARLY_BASE_VA, + (uintptr_t)early_dtb_pmd, PGDIR_SIZE, PAGE_TABLE); + /* Create two consecutive PMD mappings for FDT early scan */ + pa = dtb_pa & ~(PMD_SIZE - 1); + create_pmd_mapping(early_dtb_pmd, DTB_EARLY_BASE_VA, + pa, PMD_SIZE, PAGE_KERNEL); + create_pmd_mapping(early_dtb_pmd, DTB_EARLY_BASE_VA + PMD_SIZE, + pa + PMD_SIZE, PMD_SIZE, PAGE_KERNEL); + dtb_early_va = (void *)DTB_EARLY_BASE_VA + (dtb_pa & (PMD_SIZE - 1)); +#else /* Create two consecutive PGD mappings for FDT early scan */ pa = dtb_pa & ~(PGDIR_SIZE - 1); create_pgd_mapping(early_pg_dir, DTB_EARLY_BASE_VA, @@ -501,6 +514,7 @@ asmlinkage void __init setup_vm(uintptr_t dtb_pa) create_pgd_mapping(early_pg_dir, DTB_EARLY_BASE_VA + PGDIR_SIZE, pa + PGDIR_SIZE, PGDIR_SIZE, PAGE_KERNEL); dtb_early_va = (void *)DTB_EARLY_BASE_VA + (dtb_pa & (PGDIR_SIZE - 1)); +#endif dtb_early_pa = dtb_pa; /*
Currently, we use PGD mappings for early DTB mapping in early_pgd but this breaks Linux kernel on SiFive Unleashed because on SiFive Unleashed PMP checks don't work correctly for PGD mappings. To fix early DTB mappings on SiFive Unleashed, we use non-PGD mappings (i.e. PMD) for early DTB access. Fixes: 8f3a2b4a96dc ("RISC-V: Move DT mapping outof fixmap") Signed-off-by: Anup Patel <anup.patel@wdc.com> --- arch/riscv/mm/init.c | 14 ++++++++++++++ 1 file changed, 14 insertions(+)