| Message ID | 1406948861-11322-2-git-send-email-scottwood@freescale.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
On Fri, 2014-08-01 at 22:07 -0500, Scott Wood wrote: > > -void __init early_init_mmu(void) > -{ > - __early_init_mmu(1); > -} > - > void early_init_mmu_secondary(void) > { > - __early_init_mmu(0); > + early_init_mmu_allcpus(); > } Small nit, it took me 30s too long to figure out what you were doing due to the naming above :) Call the latter early_init_this_mmu() and keep the global one separate such that early_init_mmu() does: early_init_mmu_common(); early_init_this_mmu(); Cheers, Ben.
On Tue, 2014-08-05 at 14:36 +1000, Benjamin Herrenschmidt wrote: > On Fri, 2014-08-01 at 22:07 -0500, Scott Wood wrote: > > > > > -void __init early_init_mmu(void) > > -{ > > - __early_init_mmu(1); > > -} > > - > > void early_init_mmu_secondary(void) > > { > > - __early_init_mmu(0); > > + early_init_mmu_allcpus(); > > } > > Small nit, it took me 30s too long to figure out what you were doing due > to the naming above :) > > Call the latter early_init_this_mmu() and keep the global one separate > such that early_init_mmu() does: > > early_init_mmu_common(); > early_init_this_mmu(); I'll do s/mmu_allcpus/this_mmu/ but early_init_mmu() needs to do things both before and after early_init_mmu_common(). Do you want two new functions (before and after) or is it OK to just rename early_init_mmu_allcpus() and put a comment before early_init_mmu() saying it's just for the boot cpu? -Scott
On Tue, 2014-08-05 at 19:48 -0500, Scott Wood wrote: > I'll do s/mmu_allcpus/this_mmu/ but early_init_mmu() needs to do things > both before and after early_init_mmu_common(). Do you want two new > functions (before and after) or is it OK to just rename > early_init_mmu_allcpus() and put a comment before early_init_mmu() > saying it's just for the boot cpu? Do we really need that before/after ? The "after" code is the linear mapping setup but does it rely on the MAS4 setting done above ? Otherwise you can do before/after using a separate function mmu_set_linear_map() Always nicer to break down too large functions anyway. Cheers, Ben.
On Wed, 2014-08-06 at 14:58 +1000, Benjamin Herrenschmidt wrote: > On Tue, 2014-08-05 at 19:48 -0500, Scott Wood wrote: > > I'll do s/mmu_allcpus/this_mmu/ but early_init_mmu() needs to do things > > both before and after early_init_mmu_common(). Do you want two new > > functions (before and after) or is it OK to just rename > > early_init_mmu_allcpus() and put a comment before early_init_mmu() > > saying it's just for the boot cpu? > > Do we really need that before/after ? The "after" code is the linear > mapping setup but does it rely on the MAS4 setting done above ? No, but it relies on the call to map_mem_in_cams() to determine how much memory was able to be mapped. > Otherwise you can do before/after using a separate function > mmu_set_linear_map() OK. > Always nicer to break down too large functions anyway. Usually yes, but it doesn't necessarily help when it's an arbitrary division of a function that is a list of simple things (as opposed to e.g. factoring out the #ifdef CONFIG_PPC_FSL_BOOK3E block into its own function). BTW, is there a particular reason why we need to use memblock_enforce_memory_limit() on FSL_BOOK3E, rather than relying on memblock_set_current_limit()? I see that when memblock_enforce_memory_limit() was added to __early_init_mmu(), memblock_set_current_limit() did not exist. On 32-bit fsl booke uses memblock_set_current_limit() for this. -Scott
On Wed, 2014-08-06 at 15:09 -0500, Scott Wood wrote: > BTW, is there a particular reason why we need to use > memblock_enforce_memory_limit() on FSL_BOOK3E, rather than relying on > memblock_set_current_limit()? I see that when > memblock_enforce_memory_limit() was added to __early_init_mmu(), > memblock_set_current_limit() did not exist. On 32-bit fsl booke uses > memblock_set_current_limit() for this. I don't remember. I don't even know what the difference is, I can have a look later if you want but I'd rather you did :-) Cheers, Ben.
diff --git a/arch/powerpc/mm/tlb_nohash.c b/arch/powerpc/mm/tlb_nohash.c index 92cb18d..11ece11 100644 --- a/arch/powerpc/mm/tlb_nohash.c +++ b/arch/powerpc/mm/tlb_nohash.c @@ -581,42 +581,10 @@ static void setup_mmu_htw(void) /* * Early initialization of the MMU TLB code */ -static void __early_init_mmu(int boot_cpu) +static void early_init_mmu_allcpus(void) { unsigned int mas4; - /* XXX This will have to be decided at runtime, but right - * now our boot and TLB miss code hard wires it. Ideally - * we should find out a suitable page size and patch the - * TLB miss code (either that or use the PACA to store - * the value we want) - */ - mmu_linear_psize = MMU_PAGE_1G; - - /* XXX This should be decided at runtime based on supported - * page sizes in the TLB, but for now let's assume 16M is - * always there and a good fit (which it probably is) - * - * Freescale booke only supports 4K pages in TLB0, so use that. - */ - if (mmu_has_feature(MMU_FTR_TYPE_FSL_E)) - mmu_vmemmap_psize = MMU_PAGE_4K; - else - mmu_vmemmap_psize = MMU_PAGE_16M; - - /* XXX This code only checks for TLB 0 capabilities and doesn't - * check what page size combos are supported by the HW. It - * also doesn't handle the case where a separate array holds - * the IND entries from the array loaded by the PT. - */ - if (boot_cpu) { - /* Look for supported page sizes */ - setup_page_sizes(); - - /* Look for HW tablewalk support */ - setup_mmu_htw(); - } - /* Set MAS4 based on page table setting */ mas4 = 0x4 << MAS4_WIMGED_SHIFT; @@ -650,11 +618,6 @@ static void __early_init_mmu(int boot_cpu) } mtspr(SPRN_MAS4, mas4); - /* Set the global containing the top of the linear mapping - * for use by the TLB miss code - */ - linear_map_top = memblock_end_of_DRAM(); - #ifdef CONFIG_PPC_FSL_BOOK3E if (mmu_has_feature(MMU_FTR_TYPE_FSL_E)) { unsigned int num_cams; @@ -662,7 +625,51 @@ static void __early_init_mmu(int boot_cpu) /* use a quarter of the TLBCAM for bolted linear map */ num_cams = (mfspr(SPRN_TLB1CFG) & TLBnCFG_N_ENTRY) / 4; linear_map_top = map_mem_in_cams(linear_map_top, num_cams); + } +#endif +} +void __init early_init_mmu(void) +{ + /* XXX This will have to be decided at runtime, but right + * now our boot and TLB miss code hard wires it. Ideally + * we should find out a suitable page size and patch the + * TLB miss code (either that or use the PACA to store + * the value we want) + */ + mmu_linear_psize = MMU_PAGE_1G; + + /* XXX This should be decided at runtime based on supported + * page sizes in the TLB, but for now let's assume 16M is + * always there and a good fit (which it probably is) + * + * Freescale booke only supports 4K pages in TLB0, so use that. + */ + if (mmu_has_feature(MMU_FTR_TYPE_FSL_E)) + mmu_vmemmap_psize = MMU_PAGE_4K; + else + mmu_vmemmap_psize = MMU_PAGE_16M; + + /* XXX This code only checks for TLB 0 capabilities and doesn't + * check what page size combos are supported by the HW. It + * also doesn't handle the case where a separate array holds + * the IND entries from the array loaded by the PT. + */ + /* Look for supported page sizes */ + setup_page_sizes(); + + /* Look for HW tablewalk support */ + setup_mmu_htw(); + + /* Set the global containing the top of the linear mapping + * for use by the TLB miss code + */ + linear_map_top = memblock_end_of_DRAM(); + + early_init_mmu_allcpus(); + +#ifdef CONFIG_PPC_FSL_BOOK3E + if (mmu_has_feature(MMU_FTR_TYPE_FSL_E)) { /* limit memory so we dont have linear faults */ memblock_enforce_memory_limit(linear_map_top); @@ -683,14 +690,9 @@ static void __early_init_mmu(int boot_cpu) memblock_set_current_limit(linear_map_top); } -void __init early_init_mmu(void) -{ - __early_init_mmu(1); -} - void early_init_mmu_secondary(void) { - __early_init_mmu(0); + early_init_mmu_allcpus(); } void setup_initial_memory_limit(phys_addr_t first_memblock_base,
__early_init_mmu() does some things that are really only needed by the boot cpu. On FSL booke, This includes calling memblock_enforce_memory_limit(), which is labelled __init. Secondary cpu init code can't be __init as that would break CPU hotplug. While it's probably a bug that memblock_enforce_memory_limit() isn't __init_memblock instead, there's no reason why we should be doing this stuff for secondary cpus in the first place. Signed-off-by: Scott Wood <scottwood@freescale.com> --- arch/powerpc/mm/tlb_nohash.c | 90 ++++++++++++++++++++++---------------------- 1 file changed, 46 insertions(+), 44 deletions(-)