Re: [PATCH v3] powerpc: add ioremap_early() for mapping IO regions before MMU_init()
On Mon, Jun 15, 2009 at 12:57 AM, Benjamin Herrenschmidt wrote: > On Wed, 2009-05-27 at 12:55 -0600, Grant Likely wrote: >> From: Grant Likely >> >> ioremap_early() is useful for things like mapping SoC internally registers >> and early debug output because it allows mappings to devices to be setup >> early in the boot process where they are needed. It also give a >> performance boost since BAT mapped registers don't get flushed out of >> the TLB. >> >> Without ioremap_early(), early mappings are set up in an ad-hoc manner >> and they get lost when the MMU is set up. Drivers then have to perform >> hacky fixups to transition over to new mappings. >> >> Signed-off-by: Grant Likely >> --- > > My 40x config gives me: > > /home/benh/linux-powerpc-test/drivers/video/xilinxfb.c:409: warning: > ‘dcr_host.base’ may be used uninitialized in this function > > (warning, I think, was already there, so the patch is going into -next > but we may want another one, provided we find a way to shut the idiot up > without horrible hacks since that's just gcc being stupid I believe). I'll have the final fix out to you today. g. -- Grant Likely, B.Sc., P.Eng. Secret Lab Technologies Ltd. ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH v3] powerpc: add ioremap_early() for mapping IO regions before MMU_init()
On Mon, Jun 15, 2009 at 12:55 AM, Benjamin
Herrenschmidt wrote:
> On Wed, 2009-05-27 at 12:55 -0600, Grant Likely wrote:
>> From: Grant Likely
>>
>> ioremap_early() is useful for things like mapping SoC internally registers
>> and early debug output because it allows mappings to devices to be setup
>> early in the boot process where they are needed. It also give a
>> performance boost since BAT mapped registers don't get flushed out of
>> the TLB.
>>
>> Without ioremap_early(), early mappings are set up in an ad-hoc manner
>> and they get lost when the MMU is set up. Drivers then have to perform
>> hacky fixups to transition over to new mappings.
>>
>> Signed-off-by: Grant Likely
>> ---
>
> Approach looks sane at first glance.
>
> However, I'm reluctant to but that in until we have all MMU types
> covered or we'll have "interesting" surprises.
I considered this and was originally concerned about the same thing.
However, ioremap_early() is special in that caller cannot take for
granted what it does and must understand the side effects. For
example; on 6xx ioremap_early is always going to carve out a minimum
of 128k from the virtual address space and it is likely that the range
will extend both both before and after the desired address. Plus,
because of the limited number of BATs there is real likelyhood that
ioremap_early() will fail. Code calling it must handle the failure
mode gracefully.
IMHO, I think this is only really applicable for platform code that
understands the memory layout. ie. map the entire IMMR at once, or
mapping local bus device ranges. On the 5200 I call it early in the
platform setup on the IMMR range, but I don't actually do anything
with the returned value (unless I'm doing udbg). Then, future
ioremap() calls to that range get to use the BAT mapping
transparently.
On the 440 and the 405 with the small TLB users also need to be
careful so that too many TLB entries don't get pinned to static
allocation and negate the performance improvement of doing the pinning
in the first place. Again I think it is best restricted to platform
code, and should never cause the system to fail to boot if the mapping
doesn't work.
I do want to implement it for all MMUs (when I have the bandwidth to
do so), but I don't think merging it needs to wait. If it is merged
as is and someone uses it in the wrong place (ie. non-platform code),
then 405 and 440 will fail to build, and it will get caught quickly.
Alternately I could implement stub ioremap_early() for non-6xx to just
return NULL until it can be implemented. Callers who don't handle
NULL gracefully are broken, but it won't be known until boot time.
> Also, the CPM patch
> doesn't actually fix the massive bogon in there :-)
Yeah, that was just to get it to build. I'll look at fix that too.
>> + /* Be loud and annoying if someone calls this too late.
>> + * No need to crash the kernel though */
>> + WARN_ON(mem_init_done);
>> + if (mem_init_done)
>> + return NULL;
>
> Can't we write
>
> if (WARN_ON(mem_init_done))
> return NULL;
>
> nowadays ?
I'll check.
>> + /* Make sure request is sane */
>> + if (size == 0)
>> + return NULL;
>> +
>> + /* If the region is already block mapped, then there is nothing
>> + * to do; just return the mapped address */
>> + v = p_mapped_by_bats(addr);
>> + if (v)
>> + return (void __iomem *)v;
>
> Should we check the size ?
Ugh. Yes. good catch.
>> + /* Align region size */
>> + for (bl = 128<<10; bl < (256<<20); bl <<= 1) {
>> + p = _ALIGN_DOWN(addr, bl); /* BATs align on 128k boundaries */
>> + size = ALIGN(addr - p + size, bl);
>> + if (bl >= size)
>> + break;
>> + }
>> +
>> + /* Complain loudly if too much is requested */
>> + if (bl >= (256<<20)) {
>> + WARN_ON(1);
>> + return NULL;
>> + }
>
> Do we avoid that running into the linear mapping ?
No. I'll fix.
>> + /* Allocate the aligned virtual base address. ALIGN_DOWN is used
>> + * to ensure no overlaps occur with normal 4k ioremaps. */
>> + ioremap_bot = _ALIGN_DOWN(ioremap_bot, bl) - size;
>> +
>> + /* Set up a BAT for this IO region */
>> + i = loadbat(ioremap_bot, p, size, PAGE_KERNEL_NCG);
>> + if (i < 0)
>> + return NULL;
>> +
>> + return (void __iomem *) (ioremap_bot + (addr - p));
>> }
>>
>> /*
>> diff --git a/arch/powerpc/platforms/52xx/mpc52xx_common.c
>> b/arch/powerpc/platforms/52xx/mpc52xx_common.c
>> index 8e3dd5a..2c49148 100644
>> --- a/arch/powerpc/platforms/52xx/mpc52xx_common.c
>> +++ b/arch/powerpc/platforms/52xx/mpc52xx_common.c
>> @@ -146,7 +146,20 @@ static struct of_device_id mpc52xx_cdm_ids[] __initdata
>> = {
>> void __init
>> mpc52xx_map_common_devices(void)
>> {
>> + const struct of_device_id immr_ids[] = {
>> + { .compatible = "fsl,mpc5200-immr", }
Re: [PATCH v3] powerpc: add ioremap_early() for mapping IO regions before MMU_init()
On Wed, 2009-05-27 at 12:55 -0600, Grant Likely wrote:
> From: Grant Likely
>
> ioremap_early() is useful for things like mapping SoC internally registers
> and early debug output because it allows mappings to devices to be setup
> early in the boot process where they are needed. It also give a
> performance boost since BAT mapped registers don't get flushed out of
> the TLB.
>
> Without ioremap_early(), early mappings are set up in an ad-hoc manner
> and they get lost when the MMU is set up. Drivers then have to perform
> hacky fixups to transition over to new mappings.
>
> Signed-off-by: Grant Likely
> ---
My 40x config gives me:
/home/benh/linux-powerpc-test/drivers/video/xilinxfb.c:409: warning:
‘dcr_host.base’ may be used uninitialized in this function
(warning, I think, was already there, so the patch is going into -next
but we may want another one, provided we find a way to shut the idiot up
without horrible hacks since that's just gcc being stupid I believe).
Cheers,
Ben.
> new in v3:
> - Rebased onto Ben's dma_alloc_coherent changes
> - Fixed alignment to match region size
>
> arch/powerpc/include/asm/io.h|8 +
> arch/powerpc/kernel/setup_32.c |4
> arch/powerpc/mm/init_32.c|3
> arch/powerpc/mm/mmu_decl.h |7 +
> arch/powerpc/mm/pgtable_32.c | 12 +
> arch/powerpc/mm/ppc_mmu_32.c | 210
> +++---
> arch/powerpc/platforms/52xx/mpc52xx_common.c | 13 ++
> arch/powerpc/sysdev/cpm_common.c |2
> 8 files changed, 228 insertions(+), 31 deletions(-)
>
>
> diff --git a/arch/powerpc/include/asm/io.h b/arch/powerpc/include/asm/io.h
> index 001f2f1..10183e2 100644
> --- a/arch/powerpc/include/asm/io.h
> +++ b/arch/powerpc/include/asm/io.h
> @@ -624,6 +624,12 @@ static inline void iosync(void)
> *
> * * iounmap undoes such a mapping and can be hooked
> *
> + * * ioremap_early is for setting up mapping regions during early boot.
> Useful
> + * for console devices or mapping an entire region of SoC internal
> registers.
> + * ioremap_early becomes usable at machine_init() time. Care must be taken
> + * when using this routine because it can consume limited resources like
> BAT
> + * registers.
> + *
> * * __ioremap_at (and the pending __iounmap_at) are low level functions to
> * create hand-made mappings for use only by the PCI code and cannot
> * currently be hooked. Must be page aligned.
> @@ -647,6 +653,8 @@ extern void __iomem *ioremap_flags(phys_addr_t address,
> unsigned long size,
>
> extern void iounmap(volatile void __iomem *addr);
>
> +extern void __iomem *ioremap_early(phys_addr_t addr, unsigned long size);
> +
> extern void __iomem *__ioremap(phys_addr_t, unsigned long size,
> unsigned long flags);
> extern void __iomem *__ioremap_caller(phys_addr_t, unsigned long size,
> diff --git a/arch/powerpc/kernel/setup_32.c b/arch/powerpc/kernel/setup_32.c
> index 9e1ca74..c1c0442 100644
> --- a/arch/powerpc/kernel/setup_32.c
> +++ b/arch/powerpc/kernel/setup_32.c
> @@ -41,6 +41,7 @@
> #include
>
> #include "setup.h"
> +#include "mm/mmu_decl.h"
>
> #define DBG(fmt...)
>
> @@ -118,6 +119,9 @@ notrace unsigned long __init early_init(unsigned long
> dt_ptr)
> */
> notrace void __init machine_init(unsigned long dt_ptr)
> {
> + /* Get ready to allocate IO virtual address regions */
> + ioremap_init();
> +
> /* Enable early debugging if any specified (see udbg.h) */
> udbg_early_init();
>
> diff --git a/arch/powerpc/mm/init_32.c b/arch/powerpc/mm/init_32.c
> index 3de6a0d..806c237 100644
> --- a/arch/powerpc/mm/init_32.c
> +++ b/arch/powerpc/mm/init_32.c
> @@ -168,9 +168,6 @@ void __init MMU_init(void)
> ppc_md.progress("MMU:mapin", 0x301);
> mapin_ram();
>
> - /* Initialize early top-down ioremap allocator */
> - ioremap_bot = IOREMAP_TOP;
> -
> /* Map in I/O resources */
> if (ppc_md.progress)
> ppc_md.progress("MMU:setio", 0x302);
> diff --git a/arch/powerpc/mm/mmu_decl.h b/arch/powerpc/mm/mmu_decl.h
> index d1f9c62..6be30fe 100644
> --- a/arch/powerpc/mm/mmu_decl.h
> +++ b/arch/powerpc/mm/mmu_decl.h
> @@ -86,11 +86,14 @@ struct tlbcam {
>
> extern void mapin_ram(void);
> extern int map_page(unsigned long va, phys_addr_t pa, int flags);
> -extern void setbat(int index, unsigned long virt, phys_addr_t phys,
> -unsigned int size, int flags);
> +extern int setbat(unsigned long virt, phys_addr_t phys, unsigned int size,
> + int flags);
> +extern int loadbat(unsigned long virt, phys_addr_t phys, unsigned int size,
> +int flags);
> extern void settlbcam(int index, unsigned long virt, phys_addr_t phys,
> unsigned int size, int flags, unsigned int pid);
> extern void invalidate_tlbcam_entry(int index);
> +extern void i
Re: [PATCH v3] powerpc: add ioremap_early() for mapping IO regions before MMU_init()
On Wed, 2009-05-27 at 12:55 -0600, Grant Likely wrote:
> From: Grant Likely
>
> ioremap_early() is useful for things like mapping SoC internally registers
> and early debug output because it allows mappings to devices to be setup
> early in the boot process where they are needed. It also give a
> performance boost since BAT mapped registers don't get flushed out of
> the TLB.
>
> Without ioremap_early(), early mappings are set up in an ad-hoc manner
> and they get lost when the MMU is set up. Drivers then have to perform
> hacky fixups to transition over to new mappings.
>
> Signed-off-by: Grant Likely
> ---
Approach looks sane at first glance.
However, I'm reluctant to but that in until we have all MMU types
covered or we'll have "interesting" surprises. Also, the CPM patch
doesn't actually fix the massive bogon in there :-)
> + /* Be loud and annoying if someone calls this too late.
> + * No need to crash the kernel though */
> + WARN_ON(mem_init_done);
> + if (mem_init_done)
> + return NULL;
Can't we write
if (WARN_ON(mem_init_done))
return NULL;
nowadays ?
> + /* Make sure request is sane */
> + if (size == 0)
> + return NULL;
> +
> + /* If the region is already block mapped, then there is nothing
> + * to do; just return the mapped address */
> + v = p_mapped_by_bats(addr);
> + if (v)
> + return (void __iomem *)v;
Should we check the size ?
> + /* Align region size */
> + for (bl = 128<<10; bl < (256<<20); bl <<= 1) {
> + p = _ALIGN_DOWN(addr, bl); /* BATs align on 128k boundaries */
> + size = ALIGN(addr - p + size, bl);
> + if (bl >= size)
> + break;
> + }
> +
> + /* Complain loudly if too much is requested */
> + if (bl >= (256<<20)) {
> + WARN_ON(1);
> + return NULL;
> + }
Do we avoid that running into the linear mapping ?
> + /* Allocate the aligned virtual base address. ALIGN_DOWN is used
> + * to ensure no overlaps occur with normal 4k ioremaps. */
> + ioremap_bot = _ALIGN_DOWN(ioremap_bot, bl) - size;
> +
> + /* Set up a BAT for this IO region */
> + i = loadbat(ioremap_bot, p, size, PAGE_KERNEL_NCG);
> + if (i < 0)
> + return NULL;
> +
> + return (void __iomem *) (ioremap_bot + (addr - p));
> }
>
> /*
> diff --git a/arch/powerpc/platforms/52xx/mpc52xx_common.c
> b/arch/powerpc/platforms/52xx/mpc52xx_common.c
> index 8e3dd5a..2c49148 100644
> --- a/arch/powerpc/platforms/52xx/mpc52xx_common.c
> +++ b/arch/powerpc/platforms/52xx/mpc52xx_common.c
> @@ -146,7 +146,20 @@ static struct of_device_id mpc52xx_cdm_ids[] __initdata
> = {
> void __init
> mpc52xx_map_common_devices(void)
> {
> + const struct of_device_id immr_ids[] = {
> + { .compatible = "fsl,mpc5200-immr", },
> + { .compatible = "fsl,mpc5200b-immr", },
> + { .type = "soc", .compatible = "mpc5200", }, /* lite5200 */
> + { .type = "builtin", .compatible = "mpc5200", }, /* efika */
> + {}
> + };
> struct device_node *np;
> + struct resource res;
> +
> + /* Pre-map the whole register space using a BAT entry */
> + np = of_find_matching_node(NULL, immr_ids);
> + if (np && (of_address_to_resource(np, 0, &res) == 0))
> + ioremap_early(res.start, res.end - res.start + 1);
>
> /* mpc52xx_wdt is mapped here and used in mpc52xx_restart,
>* possibly from a interrupt context. wdt is only implement
> diff --git a/arch/powerpc/sysdev/cpm_common.c
> b/arch/powerpc/sysdev/cpm_common.c
> index e4b6d66..370723e 100644
> --- a/arch/powerpc/sysdev/cpm_common.c
> +++ b/arch/powerpc/sysdev/cpm_common.c
> @@ -56,7 +56,7 @@ void __init udbg_init_cpm(void)
> {
> if (cpm_udbg_txdesc) {
> #ifdef CONFIG_CPM2
> - setbat(1, 0xf000, 0xf000, 1024*1024, PAGE_KERNEL_NCG);
> + setbat(0xf000, 0xf000, 1024*1024, PAGE_KERNEL_NCG);
> #endif
> udbg_putc = udbg_putc_cpm;
> }
That needs to be properly fixed ... maybe using ioremap_early() ? :-)
Also, make the initial call ioremap_early_init() just to make things
clear that one can't just call ioremap(), we are limited to a very
specific thing here.
For 8xx I'm not sure what the right approach is. For 40x, 440 and FSL
BookE we probably want to allow to bolt some TLB entries.
Cheers,
Ben.
Cheers,
Ben.
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
[PATCH v3] powerpc: add ioremap_early() for mapping IO regions before MMU_init()
From: Grant Likely
ioremap_early() is useful for things like mapping SoC internally registers
and early debug output because it allows mappings to devices to be setup
early in the boot process where they are needed. It also give a
performance boost since BAT mapped registers don't get flushed out of
the TLB.
Without ioremap_early(), early mappings are set up in an ad-hoc manner
and they get lost when the MMU is set up. Drivers then have to perform
hacky fixups to transition over to new mappings.
Signed-off-by: Grant Likely
---
new in v3:
- Rebased onto Ben's dma_alloc_coherent changes
- Fixed alignment to match region size
arch/powerpc/include/asm/io.h|8 +
arch/powerpc/kernel/setup_32.c |4
arch/powerpc/mm/init_32.c|3
arch/powerpc/mm/mmu_decl.h |7 +
arch/powerpc/mm/pgtable_32.c | 12 +
arch/powerpc/mm/ppc_mmu_32.c | 210 +++---
arch/powerpc/platforms/52xx/mpc52xx_common.c | 13 ++
arch/powerpc/sysdev/cpm_common.c |2
8 files changed, 228 insertions(+), 31 deletions(-)
diff --git a/arch/powerpc/include/asm/io.h b/arch/powerpc/include/asm/io.h
index 001f2f1..10183e2 100644
--- a/arch/powerpc/include/asm/io.h
+++ b/arch/powerpc/include/asm/io.h
@@ -624,6 +624,12 @@ static inline void iosync(void)
*
* * iounmap undoes such a mapping and can be hooked
*
+ * * ioremap_early is for setting up mapping regions during early boot. Useful
+ * for console devices or mapping an entire region of SoC internal registers.
+ * ioremap_early becomes usable at machine_init() time. Care must be taken
+ * when using this routine because it can consume limited resources like BAT
+ * registers.
+ *
* * __ioremap_at (and the pending __iounmap_at) are low level functions to
* create hand-made mappings for use only by the PCI code and cannot
* currently be hooked. Must be page aligned.
@@ -647,6 +653,8 @@ extern void __iomem *ioremap_flags(phys_addr_t address,
unsigned long size,
extern void iounmap(volatile void __iomem *addr);
+extern void __iomem *ioremap_early(phys_addr_t addr, unsigned long size);
+
extern void __iomem *__ioremap(phys_addr_t, unsigned long size,
unsigned long flags);
extern void __iomem *__ioremap_caller(phys_addr_t, unsigned long size,
diff --git a/arch/powerpc/kernel/setup_32.c b/arch/powerpc/kernel/setup_32.c
index 9e1ca74..c1c0442 100644
--- a/arch/powerpc/kernel/setup_32.c
+++ b/arch/powerpc/kernel/setup_32.c
@@ -41,6 +41,7 @@
#include
#include "setup.h"
+#include "mm/mmu_decl.h"
#define DBG(fmt...)
@@ -118,6 +119,9 @@ notrace unsigned long __init early_init(unsigned long
dt_ptr)
*/
notrace void __init machine_init(unsigned long dt_ptr)
{
+ /* Get ready to allocate IO virtual address regions */
+ ioremap_init();
+
/* Enable early debugging if any specified (see udbg.h) */
udbg_early_init();
diff --git a/arch/powerpc/mm/init_32.c b/arch/powerpc/mm/init_32.c
index 3de6a0d..806c237 100644
--- a/arch/powerpc/mm/init_32.c
+++ b/arch/powerpc/mm/init_32.c
@@ -168,9 +168,6 @@ void __init MMU_init(void)
ppc_md.progress("MMU:mapin", 0x301);
mapin_ram();
- /* Initialize early top-down ioremap allocator */
- ioremap_bot = IOREMAP_TOP;
-
/* Map in I/O resources */
if (ppc_md.progress)
ppc_md.progress("MMU:setio", 0x302);
diff --git a/arch/powerpc/mm/mmu_decl.h b/arch/powerpc/mm/mmu_decl.h
index d1f9c62..6be30fe 100644
--- a/arch/powerpc/mm/mmu_decl.h
+++ b/arch/powerpc/mm/mmu_decl.h
@@ -86,11 +86,14 @@ struct tlbcam {
extern void mapin_ram(void);
extern int map_page(unsigned long va, phys_addr_t pa, int flags);
-extern void setbat(int index, unsigned long virt, phys_addr_t phys,
- unsigned int size, int flags);
+extern int setbat(unsigned long virt, phys_addr_t phys, unsigned int size,
+ int flags);
+extern int loadbat(unsigned long virt, phys_addr_t phys, unsigned int size,
+ int flags);
extern void settlbcam(int index, unsigned long virt, phys_addr_t phys,
unsigned int size, int flags, unsigned int pid);
extern void invalidate_tlbcam_entry(int index);
+extern void ioremap_init(void); /* called by machine_init() */
extern int __map_without_bats;
extern unsigned long ioremap_base;
diff --git a/arch/powerpc/mm/pgtable_32.c b/arch/powerpc/mm/pgtable_32.c
index 5422169..508fb91 100644
--- a/arch/powerpc/mm/pgtable_32.c
+++ b/arch/powerpc/mm/pgtable_32.c
@@ -51,8 +51,6 @@ extern char etext[], _stext[];
#ifdef HAVE_BATS
extern phys_addr_t v_mapped_by_bats(unsigned long va);
extern unsigned long p_mapped_by_bats(phys_addr_t pa);
-void setbat(int index, unsigned long virt, phys_addr_t phys,
- unsigned int size, int flags);
#else /* !HAVE_BATS */
#define v_mapped_by_bats(x)(0UL)
