Re: [PATCH] hw/ppc/ppc405_boards: Change kernel load address

[Thomas Huth <thuth@redhat.com>]

On 08/12/2021 14.15, Cédric Le Goater wrote:
> On 12/8/21 14:07, Thomas Huth wrote:
>> On 03/12/2021 13.25, Cédric Le Goater wrote:
>>> On 12/3/21 11:40, Peter Maydell wrote:
>>>> On Fri, 3 Dec 2021 at 10:32, Thomas Huth <thuth@redhat.com> wrote:
>>>>> I guess it's an accidential NULL pointer dereference somewhere in the 
>>>>> u-boot
>>>>> code ... which will be quite hard to track down when the first page of
>>>>> memory is marked as writable... :-/
>>>>
>>>> Attach a target-arch gdb to the QEMU gdbstub and put a watchpoint on
>>>> address zero ? (Or if you suspect something inside QEMU is doing it
>>>> then run QEMU under gdb and watchpoint the host memory location
>>>> corresponding to guest address 0, but that's more painful.) Nothing
>>>> in the pre-kernel part of the boot process will have set up paging,
>>>> so the watchpointing should be pretty reliable.
>>>
>>> That's the guy:
>>>
>>> https://gitlab.com/huth/u-boot/-/blob/taihu/arch/powerpc/cpu/ppc4xx/sdram.c#L199 
>>>
>>>
>>> There must be an error in how get_ram_size() restores the RAM values :
>>>
>>>    https://gitlab.com/huth/u-boot/-/blob/taihu/common/memsize.c
>>
>> There is definitely something wrong in that function. Seems like they 
>> tried to fix it once here:
>>
>>   https://source.denx.de/u-boot/u-boot/-/commit/b8496cced856ff411f
>>
>> but that patch got later reverted without a replacement later...
> 
> 
> a fix restoring address 0, something like :
> 
> @@ -60,6 +60,9 @@ long get_ram_size(long *base, long maxsi
>           return (0);
>       }
> 
> +    addr = base;
> +    *addr = save[i];
> +
>       for (cnt = 1; cnt < maxsize / sizeof(long); cnt <<= 1) {
>           addr = base + cnt;    /* pointer arith! */
>           val = *addr;
> 
> is not enough. trap_init() will also overwrite the kernel image.
> And u-boot will complain about a wrong CRC.
> 
> The 405 series I sent improves support and latest kernel 5.16-rc4
> can be loaded without uboot. It's a start to debug user space.

Yes, your series is certainly the better way forward. I'll stop messing with 
u-boot now ... since upstream u-boot already removed the 405 support a long 
time ago, my hack was a dead end anyway (but it helped to get at least a 
kernel running again, so it was not in vain)

  Thomas