diff mbox series

[v0,2/2] aspeed: fix hardcode boot address 0

Message ID 20240205091415.935686-3-jamin_lin@aspeedtech.com (mailing list archive)
State New, archived
Headers show
Series uart base and hardcode boot address 0 | expand

Commit Message

Jamin Lin Feb. 5, 2024, 9:14 a.m. UTC 
In the previous design of QEMU model for ASPEED SOCs, it set the boot
address at 0 which was the hardcode setting for ast10x0, ast2600,
ast2500 and ast2400.

According to the design of ast2700, it has bootmcu which is used for
executing SPL and initialize DRAM, then, CPUs(cortex-a35)
execute u-boot, kernel and rofs. QEMU will only support CPU(coretax-a35)
parts and the boot address is "0x400000000" for ast2700.
Therefore, fixed hardcode boot address 0.

Signed-off-by: Troy Lee <troy_lee@aspeedtech.com>
Signed-off-by: Jamin Lin <jamin_lin@aspeedtech.com>
---
 hw/arm/aspeed.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

Comments

Philippe Mathieu-Daudé Feb. 5, 2024, 1:20 p.m. UTC | #1 
Hi Jamin,

On 5/2/24 10:14, Jamin Lin via wrote:
> In the previous design of QEMU model for ASPEED SOCs, it set the boot
> address at 0 which was the hardcode setting for ast10x0, ast2600,
> ast2500 and ast2400.
> 
> According to the design of ast2700, it has bootmcu which is used for
> executing SPL and initialize DRAM,

Out of curiosity, what architecture is this MCU?

> then, CPUs(cortex-a35)
> execute u-boot, kernel and rofs. QEMU will only support CPU(coretax-a35)
> parts and the boot address is "0x400000000" for ast2700.

OK, but I don't get how you get from here ...

> Therefore, fixed hardcode boot address 0.

... to here.

> Signed-off-by: Troy Lee <troy_lee@aspeedtech.com>
> Signed-off-by: Jamin Lin <jamin_lin@aspeedtech.com>
> ---
>   hw/arm/aspeed.c | 4 +++-
>   1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/hw/arm/aspeed.c b/hw/arm/aspeed.c
> index 218b81298e..82a92e8142 100644
> --- a/hw/arm/aspeed.c
> +++ b/hw/arm/aspeed.c
> @@ -289,12 +289,14 @@ static void aspeed_install_boot_rom(AspeedMachineState *bmc, BlockBackend *blk,
>                                       uint64_t rom_size)
>   {
>       AspeedSoCState *soc = bmc->soc;
> +    AspeedSoCClass *sc = ASPEED_SOC_GET_CLASS(soc);
>   
>       memory_region_init_rom(&bmc->boot_rom, NULL, "aspeed.boot_rom", rom_size,
>                              &error_abort);
>       memory_region_add_subregion_overlap(&soc->spi_boot_container, 0,
>                                           &bmc->boot_rom, 1);
> -    write_boot_rom(blk, ASPEED_SOC_SPI_BOOT_ADDR, rom_size, &error_abort);
> +    write_boot_rom(blk, sc->memmap[ASPEED_DEV_SPI_BOOT],
> +                   rom_size, &error_abort);

Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>

>   }
>   
>   void aspeed_board_init_flashes(AspeedSMCState *s, const char *flashtype,
Cédric Le Goater Feb. 5, 2024, 1:34 p.m. UTC | #2 
On 2/5/24 10:14, Jamin Lin wrote:
> In the previous design of QEMU model for ASPEED SOCs, it set the boot
> address at 0 which was the hardcode setting for ast10x0, ast2600,
> ast2500 and ast2400.
> 
> According to the design of ast2700, it has bootmcu which is used for
> executing SPL and initialize DRAM, then, CPUs(cortex-a35)
> execute u-boot, kernel and rofs. QEMU will only support CPU(coretax-a35)
> parts and the boot address is "0x400000000" for ast2700.

On the previous SoC, the ASPEED_DEV_SPI_BOOT region is an alias, at 0x0,
to the FMC CE0 region, mapped at 0x20000000.

Is 0x400000000 (or 0x40000000 ?) the address for FMC CE0 region on the
ast2700 ? or an alias ?

What is the cortex-a35 reset address ?

It would help to also introduce a basic skeleton of the ast2700 SoC.

Anyhow, this change makes sense. Could you please respin and also
remove ASPEED_SOC_SPI_BOOT_ADDR. ?

Thanks,

C.

> Therefore, fixed hardcode boot address 0.
> 
> Signed-off-by: Troy Lee <troy_lee@aspeedtech.com>
> Signed-off-by: Jamin Lin <jamin_lin@aspeedtech.com>
> ---
>   hw/arm/aspeed.c | 4 +++-
>   1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/hw/arm/aspeed.c b/hw/arm/aspeed.c
> index 218b81298e..82a92e8142 100644
> --- a/hw/arm/aspeed.c
> +++ b/hw/arm/aspeed.c
> @@ -289,12 +289,14 @@ static void aspeed_install_boot_rom(AspeedMachineState *bmc, BlockBackend *blk,
>                                       uint64_t rom_size)
>   {
>       AspeedSoCState *soc = bmc->soc;
> +    AspeedSoCClass *sc = ASPEED_SOC_GET_CLASS(soc);
>   
>       memory_region_init_rom(&bmc->boot_rom, NULL, "aspeed.boot_rom", rom_size,
>                              &error_abort);
>       memory_region_add_subregion_overlap(&soc->spi_boot_container, 0,
>                                           &bmc->boot_rom, 1);
> -    write_boot_rom(blk, ASPEED_SOC_SPI_BOOT_ADDR, rom_size, &error_abort);
> +    write_boot_rom(blk, sc->memmap[ASPEED_DEV_SPI_BOOT],
> +                   rom_size, &error_abort);
>   }
>   
>   void aspeed_board_init_flashes(AspeedSMCState *s, const char *flashtype,
Jamin Lin Feb. 6, 2024, 1:48 a.m. UTC | #3 
> -----Original Message-----
> From: Philippe Mathieu-Daudé <philmd@linaro.org>
> Sent: Monday, February 5, 2024 9:20 PM
> To: Jamin Lin <jamin_lin@aspeedtech.com>; Cédric Le Goater <clg@kaod.org>;
> Peter Maydell <peter.maydell@linaro.org>; Andrew Jeffery
> <andrew@codeconstruct.com.au>; Joel Stanley <joel@jms.id.au>; open
> list:ASPEED BMCs <qemu-arm@nongnu.org>; open list:All patches CC here
> <qemu-devel@nongnu.org>
> Cc: Troy Lee <troy_lee@aspeedtech.com>
> Subject: Re: [PATCH v0 2/2] aspeed: fix hardcode boot address 0
> 
> Hi Jamin,
> 
> On 5/2/24 10:14, Jamin Lin via wrote:
> > In the previous design of QEMU model for ASPEED SOCs, it set the boot
> > address at 0 which was the hardcode setting for ast10x0, ast2600,
> > ast2500 and ast2400.
> >
> > According to the design of ast2700, it has bootmcu which is used for
> > executing SPL and initialize DRAM,
> 
> Out of curiosity, what architecture is this MCU?
MCU is riscv-ibex and its architecture is riscv-32.

> 
> > then, CPUs(cortex-a35)
> > execute u-boot, kernel and rofs. QEMU will only support
> > CPU(coretax-a35) parts and the boot address is "0x400000000" for ast2700.
> 
> OK, but I don't get how you get from here ...
> 
Our design make MCU execute SPL and copy u-boot image from SPI to DRAM at address 0x400000000 at SPL boot stage.
However, QEMU will only support to emulate CPU sides (coretex-a35) for ast2700, that was why we want to change the boot address at 0x400000000
And use the following start command by QEMU.

./qemu-system-aarch64 -M ast2750-evb -nographic -m 8G \
 -device loader,addr=0x400000000,file=${IMGDIR}/u-boot-nodtb.bin,force-raw=on \
 -device loader,addr=$((0x400000000 + ${UBOOT_SIZE})),file=${IMGDIR}/u-boot.dtb,force-raw=on \
 ---
 ---

By the way, I will send a new patch series to support ast2700 in two weeks and 
We set memory map for ast2700 as following.

static const hwaddr aspeed_soc_ast2700_memmap[] = {
    [ASPEED_DEV_SPI_BOOT]  =  0x400000000,
    [ASPEED_DEV_SRAM]      =  0x10000000,

Jamin
> > Therefore, fixed hardcode boot address 0.
> 
> ... to here.
> 
> > Signed-off-by: Troy Lee <troy_lee@aspeedtech.com>
> > Signed-off-by: Jamin Lin <jamin_lin@aspeedtech.com>
> > ---
> >   hw/arm/aspeed.c | 4 +++-
> >   1 file changed, 3 insertions(+), 1 deletion(-)
> >
> > diff --git a/hw/arm/aspeed.c b/hw/arm/aspeed.c index
> > 218b81298e..82a92e8142 100644
> > --- a/hw/arm/aspeed.c
> > +++ b/hw/arm/aspeed.c
> > @@ -289,12 +289,14 @@ static void
> aspeed_install_boot_rom(AspeedMachineState *bmc, BlockBackend *blk,
> >                                       uint64_t rom_size)
> >   {
> >       AspeedSoCState *soc = bmc->soc;
> > +    AspeedSoCClass *sc = ASPEED_SOC_GET_CLASS(soc);
> >
> >       memory_region_init_rom(&bmc->boot_rom, NULL,
> "aspeed.boot_rom", rom_size,
> >                              &error_abort);
> >       memory_region_add_subregion_overlap(&soc->spi_boot_container,
> 0,
> >                                           &bmc->boot_rom, 1);
> > -    write_boot_rom(blk, ASPEED_SOC_SPI_BOOT_ADDR, rom_size,
> &error_abort);
> > +    write_boot_rom(blk, sc->memmap[ASPEED_DEV_SPI_BOOT],
> > +                   rom_size, &error_abort);
> 
> Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> 
> >   }
> >
> >   void aspeed_board_init_flashes(AspeedSMCState *s, const char
> > *flashtype,
Jamin Lin Feb. 6, 2024, 2:15 a.m. UTC | #4 
> -----Original Message-----
> From: Cédric Le Goater <clg@kaod.org>
> Sent: Monday, February 5, 2024 9:34 PM
> To: Jamin Lin <jamin_lin@aspeedtech.com>; Peter Maydell
> <peter.maydell@linaro.org>; Andrew Jeffery <andrew@codeconstruct.com.au>;
> Joel Stanley <joel@jms.id.au>; open list:ASPEED BMCs
> <qemu-arm@nongnu.org>; open list:All patches CC here
> <qemu-devel@nongnu.org>
> Cc: Troy Lee <troy_lee@aspeedtech.com>
> Subject: Re: [PATCH v0 2/2] aspeed: fix hardcode boot address 0
> 
> On 2/5/24 10:14, Jamin Lin wrote:
> > In the previous design of QEMU model for ASPEED SOCs, it set the boot
> > address at 0 which was the hardcode setting for ast10x0, ast2600,
> > ast2500 and ast2400.
> >
> > According to the design of ast2700, it has bootmcu which is used for
> > executing SPL and initialize DRAM, then, CPUs(cortex-a35) execute
> > u-boot, kernel and rofs. QEMU will only support CPU(coretax-a35) parts
> > and the boot address is "0x400000000" for ast2700.
> 
> On the previous SoC, the ASPEED_DEV_SPI_BOOT region is an alias, at 0x0, to
> the FMC CE0 region, mapped at 0x20000000.
> 
> Is 0x400000000 (or 0x40000000 ?) the address for FMC CE0 region on the
> ast2700 ? or an alias ?
> 
It is "0x4 00000000"(64bits address). CPU is armv8 cortex-a35 which is 64 bits CPU. 
The dram base address is "0x4 00000000".
The SPL base address is "0x1 00000000".
FMC_CS0 region mapped at "0x1 00000000" address.

> What is the cortex-a35 reset address ?
> 
> It would help to also introduce a basic skeleton of the ast2700 SoC.
> 
AST2700
Primary Service Processor:
Embedded quad-core ARM Cortex A35 64-bit RISC CPU
Maximum running frequency: 1.6GHZ
Support: MMU, FPU, NEON, trust-zone, GIC-500 controller and so on.

BootMCU:
Ibex-riscv 32bits riscv.

Boot flow
ROM Code -> BootMCU(SPL) -> CPU Cortex A35(U-boot-> kernel -> rofs)

> Anyhow, this change makes sense. Could you please respin and also remove
> ASPEED_SOC_SPI_BOOT_ADDR. ?
> 
Okay, will remove it.

> Thanks,
> 
> C.
> 
> > Therefore, fixed hardcode boot address 0.
> >
> > Signed-off-by: Troy Lee <troy_lee@aspeedtech.com>
> > Signed-off-by: Jamin Lin <jamin_lin@aspeedtech.com>
> > ---
> >   hw/arm/aspeed.c | 4 +++-
> >   1 file changed, 3 insertions(+), 1 deletion(-)
> >
> > diff --git a/hw/arm/aspeed.c b/hw/arm/aspeed.c index
> > 218b81298e..82a92e8142 100644
> > --- a/hw/arm/aspeed.c
> > +++ b/hw/arm/aspeed.c
> > @@ -289,12 +289,14 @@ static void
> aspeed_install_boot_rom(AspeedMachineState *bmc, BlockBackend *blk,
> >                                       uint64_t rom_size)
> >   {
> >       AspeedSoCState *soc = bmc->soc;
> > +    AspeedSoCClass *sc = ASPEED_SOC_GET_CLASS(soc);
> >
> >       memory_region_init_rom(&bmc->boot_rom, NULL,
> "aspeed.boot_rom", rom_size,
> >                              &error_abort);
> >       memory_region_add_subregion_overlap(&soc->spi_boot_container,
> 0,
> >                                           &bmc->boot_rom, 1);
> > -    write_boot_rom(blk, ASPEED_SOC_SPI_BOOT_ADDR, rom_size,
> &error_abort);
> > +    write_boot_rom(blk, sc->memmap[ASPEED_DEV_SPI_BOOT],
> > +                   rom_size, &error_abort);
> >   }
> >
> >   void aspeed_board_init_flashes(AspeedSMCState *s, const char
> > *flashtype,
Cédric Le Goater Feb. 6, 2024, 4:54 p.m. UTC | #5 
On 2/6/24 02:48, Jamin Lin wrote:
>> -----Original Message-----
>> From: Philippe Mathieu-Daudé <philmd@linaro.org>
>> Sent: Monday, February 5, 2024 9:20 PM
>> To: Jamin Lin <jamin_lin@aspeedtech.com>; Cédric Le Goater <clg@kaod.org>;
>> Peter Maydell <peter.maydell@linaro.org>; Andrew Jeffery
>> <andrew@codeconstruct.com.au>; Joel Stanley <joel@jms.id.au>; open
>> list:ASPEED BMCs <qemu-arm@nongnu.org>; open list:All patches CC here
>> <qemu-devel@nongnu.org>
>> Cc: Troy Lee <troy_lee@aspeedtech.com>
>> Subject: Re: [PATCH v0 2/2] aspeed: fix hardcode boot address 0
>>
>> Hi Jamin,
>>
>> On 5/2/24 10:14, Jamin Lin via wrote:
>>> In the previous design of QEMU model for ASPEED SOCs, it set the boot
>>> address at 0 which was the hardcode setting for ast10x0, ast2600,
>>> ast2500 and ast2400.
>>>
>>> According to the design of ast2700, it has bootmcu which is used for
>>> executing SPL and initialize DRAM,
>>
>> Out of curiosity, what architecture is this MCU?
> MCU is riscv-ibex and its architecture is riscv-32.
> 
>>
>>> then, CPUs(cortex-a35)
>>> execute u-boot, kernel and rofs. QEMU will only support
>>> CPU(coretax-a35) parts and the boot address is "0x400000000" for ast2700.
>>
>> OK, but I don't get how you get from here ...
>>
> Our design make MCU execute SPL and copy u-boot image from SPI to DRAM at address 0x400000000 at SPL boot stage.
> However, QEMU will only support to emulate CPU sides (coretex-a35) for ast2700, 

The fby35 is an example of a machine with two ARM SoCs : ast1030-a1
and ast2600-a3. There is work in progress for heterogeneous QEMU
machines and It might be possible to model RISC-V and ARM one day.

> that was why we want to change the boot address at 0x400000000
> And use the following start command by QEMU.
> 
> ./qemu-system-aarch64 -M ast2750-evb -nographic -m 8G \
>   -device loader,addr=0x400000000,file=${IMGDIR}/u-boot-nodtb.bin,force-raw=on \
>   -device loader,addr=$((0x400000000 + ${UBOOT_SIZE})),file=${IMGDIR}/u-boot.dtb,force-raw=on \
>   ---
>   ---
ok. Makes sense.
  
> By the way, I will send a new patch series to support ast2700 in two weeks and
> We set memory map for ast2700 as following.
> 
> static const hwaddr aspeed_soc_ast2700_memmap[] = {
>      [ASPEED_DEV_SPI_BOOT]  =  0x400000000,
>      [ASPEED_DEV_SRAM]      =  0x10000000,

Excellent !

Thanks,

C.
diff mbox series

Patch

diff --git a/hw/arm/aspeed.c b/hw/arm/aspeed.c
index 218b81298e..82a92e8142 100644
--- a/hw/arm/aspeed.c
+++ b/hw/arm/aspeed.c
@@ -289,12 +289,14 @@  static void aspeed_install_boot_rom(AspeedMachineState *bmc, BlockBackend *blk,
                                     uint64_t rom_size)
 {
     AspeedSoCState *soc = bmc->soc;
+    AspeedSoCClass *sc = ASPEED_SOC_GET_CLASS(soc);
 
     memory_region_init_rom(&bmc->boot_rom, NULL, "aspeed.boot_rom", rom_size,
                            &error_abort);
     memory_region_add_subregion_overlap(&soc->spi_boot_container, 0,
                                         &bmc->boot_rom, 1);
-    write_boot_rom(blk, ASPEED_SOC_SPI_BOOT_ADDR, rom_size, &error_abort);
+    write_boot_rom(blk, sc->memmap[ASPEED_DEV_SPI_BOOT],
+                   rom_size, &error_abort);
 }
 
 void aspeed_board_init_flashes(AspeedSMCState *s, const char *flashtype,