Message ID | 20211202191446.1292125-1-clg@kaod.org |
---|---|
State | New |
Headers | show |
Series | hw/ppc/ppc405_boards: Change kernel load address | expand |
On 02/12/2021 20.14, Cédric Le Goater wrote: > The default addresses to load the kernel, fdt, initrd of AMCC boards > in U-Boot v2015.10 are : > > "kernel_addr_r=1000000\0" > "fdt_addr_r=1800000\0" > "ramdisk_addr_r=1900000\0" > > The taihu is one of these boards, the ref405ep is not but we don't > have much information on it and both boards have a very similar > address space layout. Change the kernel load address to match U-Boot > expectations and fix loading. You could additionally mention that U-Boot corrupts the kernel if it gets loaded to address 0. > Signed-off-by: Cédric Le Goater <clg@kaod.org> > --- > hw/ppc/ppc405_boards.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/hw/ppc/ppc405_boards.c b/hw/ppc/ppc405_boards.c > index 972a7a4a3e5d..b4249f4626e6 100644 > --- a/hw/ppc/ppc405_boards.c > +++ b/hw/ppc/ppc405_boards.c > @@ -45,7 +45,7 @@ > #define BIOS_FILENAME "ppc405_rom.bin" > #define BIOS_SIZE (2 * MiB) > > -#define KERNEL_LOAD_ADDR 0x00000000 > +#define KERNEL_LOAD_ADDR 0x01000000 > #define INITRD_LOAD_ADDR 0x01800000 > > #define USE_FLASH_BIOS Reviewed-by: Thomas Huth <thuth@redhat.com>
Le 02/12/2021 à 20:19, Cédric Le Goater a écrit : > On 12/2/21 20:17, Thomas Huth wrote: >> On 02/12/2021 20.14, Cédric Le Goater wrote: >>> The default addresses to load the kernel, fdt, initrd of AMCC boards >>> in U-Boot v2015.10 are : >>> >>> "kernel_addr_r=1000000\0" >>> "fdt_addr_r=1800000\0" >>> "ramdisk_addr_r=1900000\0" >>> >>> The taihu is one of these boards, the ref405ep is not but we don't >>> have much information on it and both boards have a very similar >>> address space layout. Change the kernel load address to match U-Boot >>> expectations and fix loading. >> >> You could additionally mention that U-Boot corrupts the kernel if it >> gets loaded to address 0. > > True. The first word is overwritten with zeros. I looked for it > in the code but could not find the reason. > Isn't it because U-boot unzips the kernel at address 0 ? Christophe
On 12/2/21 20:32, LEROY Christophe wrote: > > > Le 02/12/2021 à 20:19, Cédric Le Goater a écrit : >> On 12/2/21 20:17, Thomas Huth wrote: >>> On 02/12/2021 20.14, Cédric Le Goater wrote: >>>> The default addresses to load the kernel, fdt, initrd of AMCC boards >>>> in U-Boot v2015.10 are : >>>> >>>> "kernel_addr_r=1000000\0" >>>> "fdt_addr_r=1800000\0" >>>> "ramdisk_addr_r=1900000\0" >>>> >>>> The taihu is one of these boards, the ref405ep is not but we don't >>>> have much information on it and both boards have a very similar >>>> address space layout. Change the kernel load address to match U-Boot >>>> expectations and fix loading. >>> >>> You could additionally mention that U-Boot corrupts the kernel if it >>> gets loaded to address 0. >> >> True. The first word is overwritten with zeros. I looked for it >> in the code but could not find the reason. >> > > Isn't it because U-boot unzips the kernel at address 0 ? Address 0 is trashed before U-boot loads the kernel. C.
On 03/12/2021 11.27, Cédric Le Goater wrote: > On 12/2/21 20:32, LEROY Christophe wrote: >> >> >> Le 02/12/2021 à 20:19, Cédric Le Goater a écrit : >>> On 12/2/21 20:17, Thomas Huth wrote: >>>> On 02/12/2021 20.14, Cédric Le Goater wrote: >>>>> The default addresses to load the kernel, fdt, initrd of AMCC boards >>>>> in U-Boot v2015.10 are : >>>>> >>>>> "kernel_addr_r=1000000\0" >>>>> "fdt_addr_r=1800000\0" >>>>> "ramdisk_addr_r=1900000\0" >>>>> >>>>> The taihu is one of these boards, the ref405ep is not but we don't >>>>> have much information on it and both boards have a very similar >>>>> address space layout. Change the kernel load address to match U-Boot >>>>> expectations and fix loading. >>>> >>>> You could additionally mention that U-Boot corrupts the kernel if it >>>> gets loaded to address 0. >>> >>> True. The first word is overwritten with zeros. I looked for it >>> in the code but could not find the reason. >>> >> >> Isn't it because U-boot unzips the kernel at address 0 ? > > Address 0 is trashed before U-boot loads the kernel. 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... :-/ Thomas
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. -- PMM
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 C.
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... Thomas
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. Thanks, C.
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
On 12/8/21 14:19, Thomas Huth wrote: > 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) This is not what I meant ! sorry ! What I wanted to express is that now nothing is blocking us from fixing user space :) The patchset I sent does not break booting from U-Boot. It still very much works and we should keep it because it exercises more our models. Even if the SDRAM still needs a U-Boot hack. I believe that can be fixed in QEMU with some time. When done, I hope we can merge the U-Boot binary in QEMU. Regarding Linux loading in QEMU or U-Boot, the problem is the PPC board information which is out of sync in all components ... Today, the easiest is to boot directly from a Linux 405 hotfoot kernel under QEMU because we are a bit lucky. The proc frequency is set to some non zero value so the kernel boots. I pushed experiments a bit further and network seems to be OK also. My idea was to merge the first round of changes for direct linux boot. See how that goes (user space) and then merge a custom ppc405-hotfoot machine to work around the board information problem. Thanks, C.
diff --git a/hw/ppc/ppc405_boards.c b/hw/ppc/ppc405_boards.c index 972a7a4a3e5d..b4249f4626e6 100644 --- a/hw/ppc/ppc405_boards.c +++ b/hw/ppc/ppc405_boards.c @@ -45,7 +45,7 @@ #define BIOS_FILENAME "ppc405_rom.bin" #define BIOS_SIZE (2 * MiB) -#define KERNEL_LOAD_ADDR 0x00000000 +#define KERNEL_LOAD_ADDR 0x01000000 #define INITRD_LOAD_ADDR 0x01800000 #define USE_FLASH_BIOS
The default addresses to load the kernel, fdt, initrd of AMCC boards in U-Boot v2015.10 are : "kernel_addr_r=1000000\0" "fdt_addr_r=1800000\0" "ramdisk_addr_r=1900000\0" The taihu is one of these boards, the ref405ep is not but we don't have much information on it and both boards have a very similar address space layout. Change the kernel load address to match U-Boot expectations and fix loading. Signed-off-by: Cédric Le Goater <clg@kaod.org> --- hw/ppc/ppc405_boards.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)