Re: [RFC PATCH kernel] powerpc: Fix early setup to make early_ioremap work
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
Re: [RFC PATCH kernel] powerpc: Fix early setup to make early_ioremap work
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
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_SIZESZ_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
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
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: RCPT TO: RCPT TO: RCPT TO: RCPT TO: From: Alexey Kardashevskiy To: linuxppc-dev@lists.ozlabs.org Cc: Alexey Kardashevskiy , Michael Ellerman , Christophe Leroy 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
Hm, my thunderbird says it is not cc:'ed but git sendmail says it did cc: Server: localhost MAIL FROM: RCPT TO: RCPT TO: RCPT TO: RCPT TO: From: Alexey Kardashevskiy To: linuxppc-dev@lists.ozlabs.org Cc: Alexey Kardashevskiy , Michael Ellerman , Christophe Leroy 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
[RFC PATCH kernel] powerpc: Fix early setup to make early_ioremap work
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