# \[RFC PATCH kernel\] powerpc: Fix early setup to make early\_ioremap work - Subsystems: LINUX FOR POWERPC (32-BIT AND 64-BIT), THE REST - Maintainers: Madhavan Srinivasan, Linus Torvalds - Status: LANDED (e2f5efd0f0e2 · 2021-05-20 06:43) - Landed in mainline as e2f5efd0f0e2 on 2021-05-20. - List: linuxppc-dev - 7 messages, 3 authors - Page: https://ratatoskr.run/linuxppc-dev/2021/05/9522814/t ## \[RFC PATCH kernel\] powerpc: Fix early setup to make early\_ioremap work - From: Alexey Kardashevskiy \ - Date: 2021-05-20 03:29:54 - Message: https://ratatoskr.run/linuxppc-dev/2021/05/9522814.md ```text The immediate problem is that after 0bd3f9e953bd ("powerpc/legacy_serial: Use early_ioremap()") the kernel silently reboots. The reason is that early_ioremap() returns broken addresses as it uses slot_virt[] array which initialized with offsets from FIXADDR_TOP == IOREMAP_END+FIXADDR_SIZE == KERN_IO_END- FIXADDR_SIZ + FIXADDR_SIZE == __kernel_io_end which is 0 when early_ioremap_setup() is called. __kernel_io_end is initialized little bit later in early_init_mmu(). This fixes the initialization by swapping early_ioremap_setup and early_init_mmu. This also fixes IOREMAP_END to use FIXADDR_SIZE defined just next to it, seems to make sense, unless there is some weird logic with redefining FIXADDR_SIZE as the compiling goes. Signed-off-by: Alexey Kardashevskiy --- arch/powerpc/include/asm/book3s/64/pgtable.h | 2 +- arch/powerpc/kernel/setup_64.c | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/arch/powerpc/include/asm/book3s/64/pgtable.h b/arch/powerpc/include/asm/book3s/64/pgtable.h index a666d561b44d..54a06129794b 100644 --- a/arch/powerpc/include/asm/book3s/64/pgtable.h +++ b/arch/powerpc/include/asm/book3s/64/pgtable.h @@ -325,8 +325,8 @@ extern unsigned long pci_io_base; #define PHB_IO_END (KERN_IO_START + FULL_IO_SIZE) #define IOREMAP_BASE (PHB_IO_END) #define IOREMAP_START (ioremap_bot) -#define IOREMAP_END (KERN_IO_END - FIXADDR_SIZE) #define FIXADDR_SIZE SZ_32M +#define IOREMAP_END (KERN_IO_END - FIXADDR_SIZE) /* Advertise special mapping type for AGP */ #define HAVE_PAGE_AGP diff --git a/arch/powerpc/kernel/setup_64.c b/arch/powerpc/kernel/setup_64.c index b779d25761cf..ce09fe5debf4 100644 --- a/arch/powerpc/kernel/setup_64.c +++ b/arch/powerpc/kernel/setup_64.c @@ -369,11 +369,12 @@ void __init early_setup(unsigned long dt_ptr) apply_feature_fixups(); setup_feature_keys(); - early_ioremap_setup(); /* Initialize the hash table or TLB handling */ early_init_mmu(); + early_ioremap_setup(); + /* * After firmware and early platform setup code has set things up, * we note the SPR values for configurable control/performance -- 2.30.2 ``` ## Re: \[RFC PATCH kernel\] powerpc: Fix early setup to make early\_ioremap work - From: Alexey Kardashevskiy \ - Date: 2021-05-20 03:34:04 - Message: https://ratatoskr.run/linuxppc-dev/2021/05/9522815.md ```text Hm, my thunderbird says it is not cc:'ed but git sendmail says it did cc: Server: localhost MAIL FROM:[ref] RCPT TO:[ref] RCPT TO:[ref] RCPT TO:[ref] RCPT TO:[ref] From: Alexey Kardashevskiy To: Cc: Alexey Kardashevskiy , Michael Ellerman [ref], Christophe Leroy [ref] Subject: [RFC PATCH kernel] powerpc: Fix early setup to make early_ioremap work Not sure what to believe. On 20/05/2021 13:29, Alexey Kardashevskiy wrote: > The immediate problem is that after > 0bd3f9e953bd ("powerpc/legacy_serial: Use early_ioremap()") > the kernel silently reboots. The reason is that early_ioremap() returns > broken addresses as it uses slot_virt[] array which initialized with > offsets from FIXADDR_TOP == IOREMAP_END+FIXADDR_SIZE == > KERN_IO_END- FIXADDR_SIZ + FIXADDR_SIZE == __kernel_io_end which is 0 > when early_ioremap_setup() is called. __kernel_io_end is initialized > little bit later in early_init_mmu(). > > This fixes the initialization by swapping early_ioremap_setup and > early_init_mmu. > > This also fixes IOREMAP_END to use FIXADDR_SIZE defined just next to it, > seems to make sense, unless there is some weird logic with redefining > FIXADDR_SIZE as the compiling goes. > > Signed-off-by: Alexey Kardashevskiy > --- > arch/powerpc/include/asm/book3s/64/pgtable.h | 2 +- > arch/powerpc/kernel/setup_64.c | 3 ++- > 2 files changed, 3 insertions(+), 2 deletions(-) > > diff --git a/arch/powerpc/include/asm/book3s/64/pgtable.h b/arch/powerpc/include/asm/book3s/64/pgtable.h > index a666d561b44d..54a06129794b 100644 > --- a/arch/powerpc/include/asm/book3s/64/pgtable.h > +++ b/arch/powerpc/include/asm/book3s/64/pgtable.h > @@ -325,8 +325,8 @@ extern unsigned long pci_io_base; > #define PHB_IO_END (KERN_IO_START + FULL_IO_SIZE) > #define IOREMAP_BASE (PHB_IO_END) > #define IOREMAP_START (ioremap_bot) > -#define IOREMAP_END (KERN_IO_END - FIXADDR_SIZE) > #define FIXADDR_SIZE SZ_32M > +#define IOREMAP_END (KERN_IO_END - FIXADDR_SIZE) > > /* Advertise special mapping type for AGP */ > #define HAVE_PAGE_AGP > diff --git a/arch/powerpc/kernel/setup_64.c b/arch/powerpc/kernel/setup_64.c > index b779d25761cf..ce09fe5debf4 100644 > --- a/arch/powerpc/kernel/setup_64.c > +++ b/arch/powerpc/kernel/setup_64.c > @@ -369,11 +369,12 @@ void __init early_setup(unsigned long dt_ptr) > apply_feature_fixups(); > setup_feature_keys(); > > - early_ioremap_setup(); > > /* Initialize the hash table or TLB handling */ > early_init_mmu(); > > + early_ioremap_setup(); > + > /* > * After firmware and early platform setup code has set things up, > * we note the SPR values for configurable control/performance > -- Alexey ``` ## Re: \[RFC PATCH kernel\] powerpc: Fix early setup to make early\_ioremap work - From: Christophe Leroy \ - Date: 2021-05-20 05:39:25 - Message: https://ratatoskr.run/linuxppc-dev/2021/05/9522821.md ```text Le 20/05/2021 à 05:33, Alexey Kardashevskiy a écrit : > Hm, my thunderbird says it is not cc:'ed but git sendmail says it did cc: > > > Server: localhost > MAIL FROM:[ref] > RCPT TO:[ref] > RCPT TO:[ref] > RCPT TO:[ref] > RCPT TO:[ref] > From: Alexey Kardashevskiy > To: > Cc: Alexey Kardashevskiy , >         Michael Ellerman [ref], >         Christophe Leroy [ref] > Subject: [RFC PATCH kernel] powerpc: Fix early setup to make early_ioremap work > > > Not sure what to believe. I got the patch. If you are looking at the mail you received from the ppc list, I think the list removes the members of the list from the Cc: in order to avoid the mail being received multiple times. Christophe > > > On 20/05/2021 13:29, Alexey Kardashevskiy wrote: >> The immediate problem is that after >> 0bd3f9e953bd ("powerpc/legacy_serial: Use early_ioremap()") >> the kernel silently reboots. The reason is that early_ioremap() returns >> broken addresses as it uses slot_virt[] array which initialized with >> offsets from FIXADDR_TOP == IOREMAP_END+FIXADDR_SIZE == >> KERN_IO_END- FIXADDR_SIZ + FIXADDR_SIZE == __kernel_io_end which is 0 >> when early_ioremap_setup() is called. __kernel_io_end is initialized >> little bit later in early_init_mmu(). >> >> This fixes the initialization by swapping early_ioremap_setup and >> early_init_mmu. >> >> This also fixes IOREMAP_END to use FIXADDR_SIZE defined just next to it, >> seems to make sense, unless there is some weird logic with redefining >> FIXADDR_SIZE as the compiling goes. >> >> Signed-off-by: Alexey Kardashevskiy >> --- >>   arch/powerpc/include/asm/book3s/64/pgtable.h | 2 +- >>   arch/powerpc/kernel/setup_64.c               | 3 ++- >>   2 files changed, 3 insertions(+), 2 deletions(-) >> >> diff --git a/arch/powerpc/include/asm/book3s/64/pgtable.h b/arch/powerpc/include/asm/book3s/64/pgtable.h >> index a666d561b44d..54a06129794b 100644 >> --- a/arch/powerpc/include/asm/book3s/64/pgtable.h >> +++ b/arch/powerpc/include/asm/book3s/64/pgtable.h >> @@ -325,8 +325,8 @@ extern unsigned long pci_io_base; >>   #define  PHB_IO_END    (KERN_IO_START + FULL_IO_SIZE) >>   #define IOREMAP_BASE    (PHB_IO_END) >>   #define IOREMAP_START    (ioremap_bot) >> -#define IOREMAP_END    (KERN_IO_END - FIXADDR_SIZE) >>   #define FIXADDR_SIZE    SZ_32M >> +#define IOREMAP_END    (KERN_IO_END - FIXADDR_SIZE) >>   /* Advertise special mapping type for AGP */ >>   #define HAVE_PAGE_AGP >> diff --git a/arch/powerpc/kernel/setup_64.c b/arch/powerpc/kernel/setup_64.c >> index b779d25761cf..ce09fe5debf4 100644 >> --- a/arch/powerpc/kernel/setup_64.c >> +++ b/arch/powerpc/kernel/setup_64.c >> @@ -369,11 +369,12 @@ void __init early_setup(unsigned long dt_ptr) >>       apply_feature_fixups(); >>       setup_feature_keys(); >> -    early_ioremap_setup(); >>       /* Initialize the hash table or TLB handling */ >>       early_init_mmu(); >> +    early_ioremap_setup(); >> + >>       /* >>        * After firmware and early platform setup code has set things up, >>        * we note the SPR values for configurable control/performance >> > ``` ## Re: \[RFC PATCH kernel\] powerpc: Fix early setup to make early\_ioremap work - From: Christophe Leroy \ - Date: 2021-05-20 05:46:49 - Message: https://ratatoskr.run/linuxppc-dev/2021/05/9522822.md ```text Le 20/05/2021 à 05:29, Alexey Kardashevskiy a écrit : > The immediate problem is that after > 0bd3f9e953bd ("powerpc/legacy_serial: Use early_ioremap()") > the kernel silently reboots. The reason is that early_ioremap() returns > broken addresses as it uses slot_virt[] array which initialized with > offsets from FIXADDR_TOP == IOREMAP_END+FIXADDR_SIZE == > KERN_IO_END- FIXADDR_SIZ + FIXADDR_SIZE == __kernel_io_end which is 0 > when early_ioremap_setup() is called. __kernel_io_end is initialized > little bit later in early_init_mmu(). > > This fixes the initialization by swapping early_ioremap_setup and > early_init_mmu. Hum ... Chris tested it on a T2080RDB, that must be a book3e. So we missed it. I guess your fix is right. > > This also fixes IOREMAP_END to use FIXADDR_SIZE defined just next to it, > seems to make sense, unless there is some weird logic with redefining > FIXADDR_SIZE as the compiling goes. Well, I don't think the order of defines matters, the change should be kept out of the fix. But if you want it anyway, then I'd suggest to move it before IOREMAP_BASE in order to keep the 3 IOREMAP_xxx together. > > Signed-off-by: Alexey Kardashevskiy Reviewed-by: Christophe Leroy > --- > arch/powerpc/include/asm/book3s/64/pgtable.h | 2 +- > arch/powerpc/kernel/setup_64.c | 3 ++- > 2 files changed, 3 insertions(+), 2 deletions(-) > > diff --git a/arch/powerpc/include/asm/book3s/64/pgtable.h b/arch/powerpc/include/asm/book3s/64/pgtable.h > index a666d561b44d..54a06129794b 100644 > --- a/arch/powerpc/include/asm/book3s/64/pgtable.h > +++ b/arch/powerpc/include/asm/book3s/64/pgtable.h > @@ -325,8 +325,8 @@ extern unsigned long pci_io_base; > #define PHB_IO_END (KERN_IO_START + FULL_IO_SIZE) > #define IOREMAP_BASE (PHB_IO_END) > #define IOREMAP_START (ioremap_bot) > -#define IOREMAP_END (KERN_IO_END - FIXADDR_SIZE) > #define FIXADDR_SIZE SZ_32M > +#define IOREMAP_END (KERN_IO_END - FIXADDR_SIZE) > > /* Advertise special mapping type for AGP */ > #define HAVE_PAGE_AGP > diff --git a/arch/powerpc/kernel/setup_64.c b/arch/powerpc/kernel/setup_64.c > index b779d25761cf..ce09fe5debf4 100644 > --- a/arch/powerpc/kernel/setup_64.c > +++ b/arch/powerpc/kernel/setup_64.c > @@ -369,11 +369,12 @@ void __init early_setup(unsigned long dt_ptr) > apply_feature_fixups(); > setup_feature_keys(); > > - early_ioremap_setup(); > > /* Initialize the hash table or TLB handling */ > early_init_mmu(); > > + early_ioremap_setup(); > + > /* > * After firmware and early platform setup code has set things up, > * we note the SPR values for configurable control/performance > ``` ## Re: \[RFC PATCH kernel\] powerpc: Fix early setup to make early\_ioremap work - From: Alexey Kardashevskiy \ - Date: 2021-05-20 05:54:38 - Message: https://ratatoskr.run/linuxppc-dev/2021/05/9522824.md ```text On 20/05/2021 15:46, Christophe Leroy wrote: > > > Le 20/05/2021 à 05:29, Alexey Kardashevskiy a écrit : >> The immediate problem is that after >> 0bd3f9e953bd ("powerpc/legacy_serial: Use early_ioremap()") >> the kernel silently reboots. The reason is that early_ioremap() returns >> broken addresses as it uses slot_virt[] array which initialized with >> offsets from FIXADDR_TOP == IOREMAP_END+FIXADDR_SIZE == >> KERN_IO_END- FIXADDR_SIZ + FIXADDR_SIZE == __kernel_io_end which is 0 >> when early_ioremap_setup() is called. __kernel_io_end is initialized >> little bit later in early_init_mmu(). >> >> This fixes the initialization by swapping early_ioremap_setup and >> early_init_mmu. > > Hum ... Chris tested it on a T2080RDB, that must be a book3e. > > So we missed it. I guess your fix is right. Oh cool. >> >> This also fixes IOREMAP_END to use FIXADDR_SIZE defined just next to it, >> seems to make sense, unless there is some weird logic with redefining >> FIXADDR_SIZE as the compiling goes. > > Well, I don't think the order of defines matters, the change should be kept out of the fix. When I see this: #define IOREMAP_END (KERN_IO_END - FIXADDR_SIZE) #define FIXADDR_SIZE SZ_32M ... I have to think harder what FIXADDR_SIZE was in the first macro and in what order the preprocessor expands them. > But if you want it anyway, then I'd suggest to move it before IOREMAP_BASE in order to keep the 3 IOREMAP_xxx together. Up to Michael, I guess. > >> >> Signed-off-by: Alexey Kardashevskiy > > Reviewed-by: Christophe Leroy > >> --- >>   arch/powerpc/include/asm/book3s/64/pgtable.h | 2 +- >>   arch/powerpc/kernel/setup_64.c               | 3 ++- >>   2 files changed, 3 insertions(+), 2 deletions(-) >> >> diff --git a/arch/powerpc/include/asm/book3s/64/pgtable.h b/arch/powerpc/include/asm/book3s/64/pgtable.h >> index a666d561b44d..54a06129794b 100644 >> --- a/arch/powerpc/include/asm/book3s/64/pgtable.h >> +++ b/arch/powerpc/include/asm/book3s/64/pgtable.h >> @@ -325,8 +325,8 @@ extern unsigned long pci_io_base; >>   #define  PHB_IO_END    (KERN_IO_START + FULL_IO_SIZE) >>   #define IOREMAP_BASE    (PHB_IO_END) >>   #define IOREMAP_START    (ioremap_bot) >> -#define IOREMAP_END    (KERN_IO_END - FIXADDR_SIZE) >>   #define FIXADDR_SIZE    SZ_32M >> +#define IOREMAP_END    (KERN_IO_END - FIXADDR_SIZE) >>   /* Advertise special mapping type for AGP */ >>   #define HAVE_PAGE_AGP >> diff --git a/arch/powerpc/kernel/setup_64.c b/arch/powerpc/kernel/setup_64.c >> index b779d25761cf..ce09fe5debf4 100644 >> --- a/arch/powerpc/kernel/setup_64.c >> +++ b/arch/powerpc/kernel/setup_64.c >> @@ -369,11 +369,12 @@ void __init early_setup(unsigned long dt_ptr) >>       apply_feature_fixups(); >>       setup_feature_keys(); >> -    early_ioremap_setup(); >>       /* Initialize the hash table or TLB handling */ >>       early_init_mmu(); >> +    early_ioremap_setup(); >> + >>       /* >>        * After firmware and early platform setup code has set things up, >>        * we note the SPR values for configurable control/performance >> -- Alexey ``` ## Re: \[RFC PATCH kernel\] powerpc: Fix early setup to make early\_ioremap work - From: Christophe Leroy \ - Date: 2021-05-20 06:03:48 - Message: https://ratatoskr.run/linuxppc-dev/2021/05/9522825.md ```text Le 20/05/2021 à 07:52, Alexey Kardashevskiy a écrit : > > > On 20/05/2021 15:46, Christophe Leroy wrote: >> >> >> Le 20/05/2021 à 05:29, Alexey Kardashevskiy a écrit : >>> The immediate problem is that after >>> 0bd3f9e953bd ("powerpc/legacy_serial: Use early_ioremap()") >>> the kernel silently reboots. The reason is that early_ioremap() returns >>> broken addresses as it uses slot_virt[] array which initialized with >>> offsets from FIXADDR_TOP == IOREMAP_END+FIXADDR_SIZE == >>> KERN_IO_END- FIXADDR_SIZ + FIXADDR_SIZE == __kernel_io_end which is 0 >>> when early_ioremap_setup() is called. __kernel_io_end is initialized >>> little bit later in early_init_mmu(). >>> >>> This fixes the initialization by swapping early_ioremap_setup and >>> early_init_mmu. >> >> Hum ... Chris tested it on a T2080RDB, that must be a book3e. >> >> So we missed it. I guess your fix is right. > > > Oh cool. I forgot, your patch should include Fixes: tags. Fixes: 265c3491c4bc ("powerpc: Add support for GENERIC_EARLY_IOREMAP") If you still change the FIXADDR_SIZE, then it must also have: Fixes: 9ccba66d4d2a ("powerpc/64: Fix the definition of the fixmap area") > >>> >>> This also fixes IOREMAP_END to use FIXADDR_SIZE defined just next to it, >>> seems to make sense, unless there is some weird logic with redefining >>> FIXADDR_SIZE as the compiling goes. >> >> Well, I don't think the order of defines matters, the change should be kept out of the fix. > > When I see this: > > #define IOREMAP_END    (KERN_IO_END - FIXADDR_SIZE) > #define FIXADDR_SIZE    SZ_32M > > > ... I have to think harder what FIXADDR_SIZE was in the first macro and in what order the preprocessor expands them. > > >> But if you want it anyway, then I'd suggest to move it before IOREMAP_BASE in order to keep the 3 IOREMAP_xxx together. > > Up to Michael, I guess. > > >> >>> >>> Signed-off-by: Alexey Kardashevskiy >> >> Reviewed-by: Christophe Leroy >> >>> --- >>>   arch/powerpc/include/asm/book3s/64/pgtable.h | 2 +- >>>   arch/powerpc/kernel/setup_64.c               | 3 ++- >>>   2 files changed, 3 insertions(+), 2 deletions(-) >>> >>> diff --git a/arch/powerpc/include/asm/book3s/64/pgtable.h b/arch/powerpc/include/asm/book3s/64/pgtable.h >>> index a666d561b44d..54a06129794b 100644 >>> --- a/arch/powerpc/include/asm/book3s/64/pgtable.h >>> +++ b/arch/powerpc/include/asm/book3s/64/pgtable.h >>> @@ -325,8 +325,8 @@ extern unsigned long pci_io_base; >>>   #define  PHB_IO_END    (KERN_IO_START + FULL_IO_SIZE) >>>   #define IOREMAP_BASE    (PHB_IO_END) >>>   #define IOREMAP_START    (ioremap_bot) >>> -#define IOREMAP_END    (KERN_IO_END - FIXADDR_SIZE) >>>   #define FIXADDR_SIZE    SZ_32M >>> +#define IOREMAP_END    (KERN_IO_END - FIXADDR_SIZE) >>>   /* Advertise special mapping type for AGP */ >>>   #define HAVE_PAGE_AGP >>> diff --git a/arch/powerpc/kernel/setup_64.c b/arch/powerpc/kernel/setup_64.c >>> index b779d25761cf..ce09fe5debf4 100644 >>> --- a/arch/powerpc/kernel/setup_64.c >>> +++ b/arch/powerpc/kernel/setup_64.c >>> @@ -369,11 +369,12 @@ void __init early_setup(unsigned long dt_ptr) >>>       apply_feature_fixups(); >>>       setup_feature_keys(); >>> -    early_ioremap_setup(); >>>       /* Initialize the hash table or TLB handling */ >>>       early_init_mmu(); >>> +    early_ioremap_setup(); >>> + >>>       /* >>>        * After firmware and early platform setup code has set things up, >>>        * we note the SPR values for configurable control/performance >>> > ``` ## Re: \[RFC PATCH kernel\] powerpc: Fix early setup to make early\_ioremap work - From: Michael Ellerman \ - Date: 2021-05-21 12:45:36 - Message: https://ratatoskr.run/linuxppc-dev/2021/05/9522860.md ```text On Thu, 20 May 2021 13:29:19 +1000, Alexey Kardashevskiy wrote: > The immediate problem is that after > 0bd3f9e953bd ("powerpc/legacy_serial: Use early_ioremap()") > the kernel silently reboots. The reason is that early_ioremap() returns > broken addresses as it uses slot_virt[] array which initialized with > offsets from FIXADDR_TOP == IOREMAP_END+FIXADDR_SIZE == > KERN_IO_END- FIXADDR_SIZ + FIXADDR_SIZE == __kernel_io_end which is 0 > when early_ioremap_setup() is called. __kernel_io_end is initialized > little bit later in early_init_mmu(). > > [...] Applied to powerpc/fixes. [1/1] powerpc: Fix early setup to make early_ioremap work https://git.kernel.org/powerpc/c/e2f5efd0f0e229bd110eab513e7c0331d61a4649 cheers ```