Message ID | 3df15946ed0c29663dc7928b31ca07576e1444f6.1580904214.git.christophe.leroy@c-s.fr (mailing list archive) |
---|---|
State | Superseded |
Headers | show |
Series | powerpc/legacy_serial: Use early_ioremap() | expand |
Context | Check | Description |
---|---|---|
snowpatch_ozlabs/apply_patch | success | Successfully applied on branch powerpc/merge (530a1cfd52af0aba1af4b1c9a7bc66a202a459b1) |
snowpatch_ozlabs/build-ppc64le | success | Build succeeded |
snowpatch_ozlabs/build-ppc64be | success | Build succeeded |
snowpatch_ozlabs/build-ppc64e | success | Build succeeded |
snowpatch_ozlabs/build-pmac32 | success | Build succeeded |
snowpatch_ozlabs/checkpatch | success | total: 0 errors, 0 warnings, 0 checks, 64 lines checked |
snowpatch_ozlabs/needsstable | success | Patch has no Fixes tags |
Hi Christophe, On Wed, 2020-02-05 at 12:03 +0000, Christophe Leroy wrote: > [ 0.000000] ioremap() called early from > find_legacy_serial_ports+0x3cc/0x474. Use early_ioremap() instead > I was just about to dig into this error message and found you patch. I applied it to a v5.5 base. > find_legacy_serial_ports() is called early from setup_arch(), before > paging_init(). vmalloc is not available yet, ioremap shouldn't be > used that early. > > Use early_ioremap() and switch to a regular ioremap() later. > > Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr> On my system (Freescale T2080 SOC) this seems to cause a crash/hang in early boot. Unfortunately because this is affecting the boot console I don't get any earlyprintk output. > --- > arch/powerpc/kernel/legacy_serial.c | 33 +++++++++++++++++++++++++ > ---- > 1 file changed, 29 insertions(+), 4 deletions(-) > > diff --git a/arch/powerpc/kernel/legacy_serial.c > b/arch/powerpc/kernel/legacy_serial.c > index f061e06e9f51..8b2c1a8553a0 100644 > --- a/arch/powerpc/kernel/legacy_serial.c > +++ b/arch/powerpc/kernel/legacy_serial.c > @@ -15,6 +15,7 @@ > #include <asm/udbg.h> > #include <asm/pci-bridge.h> > #include <asm/ppc-pci.h> > +#include <asm/early_ioremap.h> > > #undef DEBUG > > @@ -34,6 +35,7 @@ static struct legacy_serial_info { > unsigned int clock; > int irq_check_parent; > phys_addr_t taddr; > + void __iomem *early_addr; > } legacy_serial_infos[MAX_LEGACY_SERIAL_PORTS]; > > static const struct of_device_id legacy_serial_parents[] __initconst > = { > @@ -325,17 +327,16 @@ static void __init > setup_legacy_serial_console(int console) > { > struct legacy_serial_info *info = > &legacy_serial_infos[console]; > struct plat_serial8250_port *port = > &legacy_serial_ports[console]; > - void __iomem *addr; > unsigned int stride; > > stride = 1 << port->regshift; > > /* Check if a translated MMIO address has been found */ > if (info->taddr) { > - addr = ioremap(info->taddr, 0x1000); > - if (addr == NULL) > + info->early_addr = early_ioremap(info->taddr, 0x1000); > + if (info->early_addr == NULL) > return; > - udbg_uart_init_mmio(addr, stride); > + udbg_uart_init_mmio(info->early_addr, stride); > } else { > /* Check if it's PIO and we support untranslated PIO */ > if (port->iotype == UPIO_PORT && isa_io_special) > @@ -353,6 +354,30 @@ static void __init > setup_legacy_serial_console(int console) > udbg_uart_setup(info->speed, info->clock); > } > > +static int __init ioremap_legacy_serial_console(void) > +{ > + struct legacy_serial_info *info = > &legacy_serial_infos[legacy_serial_console]; > + struct plat_serial8250_port *port = > &legacy_serial_ports[legacy_serial_console]; > + void __iomem *vaddr; > + > + if (legacy_serial_console < 0) > + return 0; > + > + if (!info->early_addr) > + return 0; > + > + vaddr = ioremap(info->taddr, 0x1000); > + if (WARN_ON(!vaddr)) > + return -ENOMEM; > + > + udbg_uart_init_mmio(vaddr, 1 << port->regshift); > + early_iounmap(info->early_addr, 0x1000); > + info->early_addr = NULL; > + > + return 0; > +} > +early_initcall(ioremap_legacy_serial_console); > + > /* > * This is called very early, as part of setup_system() or > eventually > * setup_arch(), basically before anything else in this file. This > function
On 24/03/20 10:54 am, Chris Packham wrote: > Hi Christophe, > > On Wed, 2020-02-05 at 12:03 +0000, Christophe Leroy wrote: >> [ 0.000000] ioremap() called early from >> find_legacy_serial_ports+0x3cc/0x474. Use early_ioremap() instead >> > I was just about to dig into this error message and found you patch. I > applied it to a v5.5 base. > >> find_legacy_serial_ports() is called early from setup_arch(), before >> paging_init(). vmalloc is not available yet, ioremap shouldn't be >> used that early. >> >> Use early_ioremap() and switch to a regular ioremap() later. >> >> Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr> > On my system (Freescale T2080 SOC) this seems to cause a crash/hang in > early boot. Unfortunately because this is affecting the boot console I > don't get any earlyprintk output. I've been doing a bit more digging into why Christophe's patch didn't work for me. I noticed the powerpc specific early_ioremap_range() returns addresses around ioremap_bot. Yet the generic early_ioremap() uses addresses around FIXADDR_TOP. If I try the following hack I can make Christophe's patch work diff --git a/arch/powerpc/include/asm/fixmap.h b/arch/powerpc/include/asm/fixmap.h index 2ef155a3c821..7bc2f3f73c8b 100644 --- a/arch/powerpc/include/asm/fixmap.h +++ b/arch/powerpc/include/asm/fixmap.h @@ -27,7 +27,7 @@ #include <asm/kasan.h> #define FIXADDR_TOP (KASAN_SHADOW_START - PAGE_SIZE) #else -#define FIXADDR_TOP ((unsigned long)(-PAGE_SIZE)) +#define FIXADDR_TOP (IOREMAP_END - PAGE_SIZE) #endif /* I'll admit to being out of my depth. It seems that the generic early_ioremap() is not quite correctly plumbed in for powerpc. >> --- >> arch/powerpc/kernel/legacy_serial.c | 33 +++++++++++++++++++++++++ >> ---- >> 1 file changed, 29 insertions(+), 4 deletions(-) >> >> diff --git a/arch/powerpc/kernel/legacy_serial.c >> b/arch/powerpc/kernel/legacy_serial.c >> index f061e06e9f51..8b2c1a8553a0 100644 >> --- a/arch/powerpc/kernel/legacy_serial.c >> +++ b/arch/powerpc/kernel/legacy_serial.c >> @@ -15,6 +15,7 @@ >> #include <asm/udbg.h> >> #include <asm/pci-bridge.h> >> #include <asm/ppc-pci.h> >> +#include <asm/early_ioremap.h> >> >> #undef DEBUG >> >> @@ -34,6 +35,7 @@ static struct legacy_serial_info { >> unsigned int clock; >> int irq_check_parent; >> phys_addr_t taddr; >> + void __iomem *early_addr; >> } legacy_serial_infos[MAX_LEGACY_SERIAL_PORTS]; >> >> static const struct of_device_id legacy_serial_parents[] __initconst >> = { >> @@ -325,17 +327,16 @@ static void __init >> setup_legacy_serial_console(int console) >> { >> struct legacy_serial_info *info = >> &legacy_serial_infos[console]; >> struct plat_serial8250_port *port = >> &legacy_serial_ports[console]; >> - void __iomem *addr; >> unsigned int stride; >> >> stride = 1 << port->regshift; >> >> /* Check if a translated MMIO address has been found */ >> if (info->taddr) { >> - addr = ioremap(info->taddr, 0x1000); >> - if (addr == NULL) >> + info->early_addr = early_ioremap(info->taddr, 0x1000); >> + if (info->early_addr == NULL) >> return; >> - udbg_uart_init_mmio(addr, stride); >> + udbg_uart_init_mmio(info->early_addr, stride); >> } else { >> /* Check if it's PIO and we support untranslated PIO */ >> if (port->iotype == UPIO_PORT && isa_io_special) >> @@ -353,6 +354,30 @@ static void __init >> setup_legacy_serial_console(int console) >> udbg_uart_setup(info->speed, info->clock); >> } >> >> +static int __init ioremap_legacy_serial_console(void) >> +{ >> + struct legacy_serial_info *info = >> &legacy_serial_infos[legacy_serial_console]; >> + struct plat_serial8250_port *port = >> &legacy_serial_ports[legacy_serial_console]; >> + void __iomem *vaddr; >> + >> + if (legacy_serial_console < 0) >> + return 0; >> + >> + if (!info->early_addr) >> + return 0; >> + >> + vaddr = ioremap(info->taddr, 0x1000); >> + if (WARN_ON(!vaddr)) >> + return -ENOMEM; >> + >> + udbg_uart_init_mmio(vaddr, 1 << port->regshift); >> + early_iounmap(info->early_addr, 0x1000); >> + info->early_addr = NULL; >> + >> + return 0; >> +} >> +early_initcall(ioremap_legacy_serial_console); >> + >> /* >> * This is called very early, as part of setup_system() or >> eventually >> * setup_arch(), basically before anything else in this file. This >> function
Hi Chris, Le 10/08/2020 à 04:01, Chris Packham a écrit : > > On 24/03/20 10:54 am, Chris Packham wrote: >> Hi Christophe, >> >> On Wed, 2020-02-05 at 12:03 +0000, Christophe Leroy wrote: >>> [ 0.000000] ioremap() called early from >>> find_legacy_serial_ports+0x3cc/0x474. Use early_ioremap() instead >>> >> I was just about to dig into this error message and found you patch. I >> applied it to a v5.5 base. >> >>> find_legacy_serial_ports() is called early from setup_arch(), before >>> paging_init(). vmalloc is not available yet, ioremap shouldn't be >>> used that early. >>> >>> Use early_ioremap() and switch to a regular ioremap() later. >>> >>> Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr> >> On my system (Freescale T2080 SOC) this seems to cause a crash/hang in >> early boot. Unfortunately because this is affecting the boot console I >> don't get any earlyprintk output. > > I've been doing a bit more digging into why Christophe's patch didn't > work for me. I noticed the powerpc specific early_ioremap_range() > returns addresses around ioremap_bot. Yet the generic early_ioremap() > uses addresses around FIXADDR_TOP. If I try the following hack I can > make Christophe's patch work > > diff --git a/arch/powerpc/include/asm/fixmap.h > b/arch/powerpc/include/asm/fixmap.h > index 2ef155a3c821..7bc2f3f73c8b 100644 > --- a/arch/powerpc/include/asm/fixmap.h > +++ b/arch/powerpc/include/asm/fixmap.h > @@ -27,7 +27,7 @@ > #include <asm/kasan.h> > #define FIXADDR_TOP (KASAN_SHADOW_START - PAGE_SIZE) > #else > -#define FIXADDR_TOP ((unsigned long)(-PAGE_SIZE)) > +#define FIXADDR_TOP (IOREMAP_END - PAGE_SIZE) > #endif > > /* > > I'll admit to being out of my depth. It seems that the generic > early_ioremap() is not quite correctly plumbed in for powerpc. Yes that's probably true for PPC64. I see that on PPC32 I had to implement the following changes in order to enable earlier use of early_ioremap() https://github.com/torvalds/linux/commit/925ac141d106b55acbe112a9272f970631a3c082 I have the problem with QEMU with the ppce500 machine. It will allow me to investigate it a bit further.
diff --git a/arch/powerpc/kernel/legacy_serial.c b/arch/powerpc/kernel/legacy_serial.c index f061e06e9f51..8b2c1a8553a0 100644 --- a/arch/powerpc/kernel/legacy_serial.c +++ b/arch/powerpc/kernel/legacy_serial.c @@ -15,6 +15,7 @@ #include <asm/udbg.h> #include <asm/pci-bridge.h> #include <asm/ppc-pci.h> +#include <asm/early_ioremap.h> #undef DEBUG @@ -34,6 +35,7 @@ static struct legacy_serial_info { unsigned int clock; int irq_check_parent; phys_addr_t taddr; + void __iomem *early_addr; } legacy_serial_infos[MAX_LEGACY_SERIAL_PORTS]; static const struct of_device_id legacy_serial_parents[] __initconst = { @@ -325,17 +327,16 @@ static void __init setup_legacy_serial_console(int console) { struct legacy_serial_info *info = &legacy_serial_infos[console]; struct plat_serial8250_port *port = &legacy_serial_ports[console]; - void __iomem *addr; unsigned int stride; stride = 1 << port->regshift; /* Check if a translated MMIO address has been found */ if (info->taddr) { - addr = ioremap(info->taddr, 0x1000); - if (addr == NULL) + info->early_addr = early_ioremap(info->taddr, 0x1000); + if (info->early_addr == NULL) return; - udbg_uart_init_mmio(addr, stride); + udbg_uart_init_mmio(info->early_addr, stride); } else { /* Check if it's PIO and we support untranslated PIO */ if (port->iotype == UPIO_PORT && isa_io_special) @@ -353,6 +354,30 @@ static void __init setup_legacy_serial_console(int console) udbg_uart_setup(info->speed, info->clock); } +static int __init ioremap_legacy_serial_console(void) +{ + struct legacy_serial_info *info = &legacy_serial_infos[legacy_serial_console]; + struct plat_serial8250_port *port = &legacy_serial_ports[legacy_serial_console]; + void __iomem *vaddr; + + if (legacy_serial_console < 0) + return 0; + + if (!info->early_addr) + return 0; + + vaddr = ioremap(info->taddr, 0x1000); + if (WARN_ON(!vaddr)) + return -ENOMEM; + + udbg_uart_init_mmio(vaddr, 1 << port->regshift); + early_iounmap(info->early_addr, 0x1000); + info->early_addr = NULL; + + return 0; +} +early_initcall(ioremap_legacy_serial_console); + /* * This is called very early, as part of setup_system() or eventually * setup_arch(), basically before anything else in this file. This function
[ 0.000000] ioremap() called early from find_legacy_serial_ports+0x3cc/0x474. Use early_ioremap() instead find_legacy_serial_ports() is called early from setup_arch(), before paging_init(). vmalloc is not available yet, ioremap shouldn't be used that early. Use early_ioremap() and switch to a regular ioremap() later. Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr> --- arch/powerpc/kernel/legacy_serial.c | 33 +++++++++++++++++++++++++---- 1 file changed, 29 insertions(+), 4 deletions(-)