All of lore.kernel.org
 help / color / mirror / Atom feed
From: Alexey Kardashevskiy <aik@ozlabs.ru>
To: Christophe Leroy <christophe.leroy@csgroup.eu>,
	linuxppc-dev@lists.ozlabs.org
Cc: Chris Packham <chris.packham@alliedtelesis.co.nz>
Subject: Re: [RFC PATCH kernel] powerpc: Fix early setup to make early_ioremap work
Date: Thu, 20 May 2021 15:52:38 +1000	[thread overview]
Message-ID: <db846a8e-bc75-244f-5e4a-513e762a875f@ozlabs.ru> (raw)
In-Reply-To: <467a5a5f-7fab-b6b1-e81f-85518378f4b8@csgroup.eu>



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 <aik@ozlabs.ru>
> 
> Reviewed-by: Christophe Leroy <christophe.leroy@csgroup.eu>
> 
>> ---
>>   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

  reply	other threads:[~2021-05-20  5:54 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-05-20  3:29 [RFC PATCH kernel] powerpc: Fix early setup to make early_ioremap work Alexey Kardashevskiy
2021-05-20  3:33 ` Alexey Kardashevskiy
2021-05-20  5:38   ` Christophe Leroy
2021-05-20  5:46 ` Christophe Leroy
2021-05-20  5:52   ` Alexey Kardashevskiy [this message]
2021-05-20  6:03     ` Christophe Leroy
2021-05-21 12:44 ` Michael Ellerman

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=db846a8e-bc75-244f-5e4a-513e762a875f@ozlabs.ru \
    --to=aik@ozlabs.ru \
    --cc=chris.packham@alliedtelesis.co.nz \
    --cc=christophe.leroy@csgroup.eu \
    --cc=linuxppc-dev@lists.ozlabs.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.