Re: [Qemu-arm] [Qemu-devel] [PATCH for-2.9 24/30] aspeed: use first SPI flash as a boot ROM

[Marcin Krzemiński]

2016-12-05 10:36 GMT+01:00 Cédric Le Goater <address@hidden>:

Hello Marcin,

On 12/04/2016 06:00 PM, mar.krzeminski wrote:
> Hi Cedric,
>
> it looks like good idea for now to handle boot from flash.
> As I understand you are trying to omit bootrom code in Qemu model?

I suppose you mean handling a romd memory region under the m25p80
object ?

It could be that I mess up everything because my understanding how the real HW

work and boot is wrong. Please correct my assumptions:

1. You are setting boot source to spi-nor (jumper, resistor whatever)

2. There is a small bootrom in SoC (not u-boot) that set eg. reset vector, configure SMC

and start execude code from flash.

3. Memory mapped flash content is at address 0x1c000000.

 

> This could lead you to some hacks in device models (eg SMC).

I haven't had to, yet.

> W dniu 29.11.2016 o 16:44, Cédric Le Goater pisze:
>> Fill a ROM region with the flash content to support U-Boot. This is a
>> little hacky but until we can boot from a MMIO region, it seems
>> difficult to do anything else.
>>
>> Signed-off-by: Cédric Le Goater <address@hidden>
>> Reviewed-by: Joel Stanley <address@hidden>
>> Reviewed-by: Andrew Jeffery <address@hidden>
>> ---
>>  hw/arm/aspeed.c | 41 +++++++++++++++++++++++++++++++++++++++++
>>  1 file changed, 41 insertions(+)
>>
>> diff --git a/hw/arm/aspeed.c b/hw/arm/aspeed.c
>> index 40c13838fb2d..a92c2f1c362b 100644
>> --- a/hw/arm/aspeed.c
>> +++ b/hw/arm/aspeed.c
>> @@ -20,6 +20,8 @@
>>  #include "qemu/log.h"
>>  #include "sysemu/block-backend.h"
>>  #include "sysemu/blockdev.h"
>> +#include "hw/loader.h"
>> +#include "qemu/error-report.h"
>>
>>  static struct arm_boot_info aspeed_board_binfo = {
>>      .board_id = -1, /* device-tree-only board */
>> @@ -104,6 +106,28 @@ static const AspeedBoardConfig aspeed_boards[] = {
>>      },
>>  };
>>
>> +#define FIRMWARE_ADDR 0x0
>> +
>> +static void write_boot_rom(DriveInfo *dinfo, hwaddr addr, size_t rom_size,
>> +                           Error **errp)
>> +{
>> +    BlockBackend *blk = blk_by_legacy_dinfo(dinfo);
>> +    uint8_t *storage;
>> +
>> +    if (rom_size > blk_getlength(blk)) {
>> +        rom_size = blk_getlength(blk);
>> +    }
> I was not able to attach smaller file as m25p80 storage.

yes that's most probably because m25p80_realize() does :

       if (blk_pread(s->blk, 0, s->storage, s->size) != s->size) {
            error_setg(errp, "failed to read the initial flash content");
            return;
        }

my bad. May be, we could relax a bit the test and allow smaller block
backends ?

 

I think not, if you have fs on flash (eg. UBI) and smaller image fs will not mount,

or in worse case you can issue writes outside the file. 

>> +
>> +    storage = g_new0(uint8_t, rom_size);
>> +    if (blk_pread(blk, 0, storage, rom_size) < 0) {
>> +        error_setg(errp, "failed to read the initial flash content");
>> +        return;
>> +    }
>> +
>> +    rom_add_blob_fixed("aspeed.boot_rom", storage, rom_size, addr);
>> +    g_free(storage);
>> +}
>> +
>>  static void aspeed_board_init_flashes(AspeedSMCState *s, const char *flashtype,
>>                                        Error **errp)
>>  {
>> @@ -135,6 +159,7 @@ static void aspeed_board_init(MachineState *machine,
>>  {
>>      AspeedBoardState *bmc;
>>      AspeedSoCClass *sc;
>> +    DriveInfo *drive0 = drive_get(IF_MTD, 0, 0);
>>
>>      bmc = g_new0(AspeedBoardState, 1);
>>      object_initialize(&bmc->soc, (sizeof(bmc->soc)), cfg->soc_name);
>> @@ -168,6 +193,22 @@ static void aspeed_board_init(MachineState *machine,
>>      aspeed_board_init_flashes(&bmc->soc.fmc, cfg->fmc_model, &error_abort);
>>      aspeed_board_init_flashes(&bmc->soc.spi[0], cfg->spi_model, &error_abort);
>>
>> +    /* Install first FMC flash content as a boot rom. */
>> +    if (drive0) {
>> +        AspeedSMCFlash *fl = &bmc->soc.fmc.flashes[0];
>> +        MemoryRegion *boot_rom = g_new(MemoryRegion, 1);
>> +
>> +        /*
>> +         * create a ROM region using the default mapping window size of
>> +         * the flash module.
>> +         */
>> +        memory_region_init_rom(boot_rom, OBJECT(bmc), "aspeed.boot_rom",
>> +                               fl->size, &error_abort);
>> +        memory_region_add_subregion(get_system_memory(), FIRMWARE_ADDR,
>> +                                    boot_rom);
>> +        write_boot_rom(drive0, FIRMWARE_ADDR, fl->size, &error_abort);
>
> Is it possible that fl->size will be bigger than segment size?
>
> I think max_size here should be segment size in smc.

I am not sure what you mean by "segment" ?

fl->size holds the default mapping window size on the AHB bus of the
flash chip CS0. The size can be changed by the guest using the segment
address registers but for CS0 this should be "autoconfigured" by HW.

This is exactly what i meant, I was thinking that fl->size is the size of the whole flash,

not the size of memory mapped part.

Here, we are just using the default from the specs but we could go a
little further in the model and setup the mapping window size of CS0
using the block backend size, and set up the segment registers accordingly.
There are some checks to be done. It might be a little complex.

I do not think there is a reason for doing that yet :)

Thanks,

Marcin

Thanks,

C.

> Thanks,
> Marcin
>> +    }
>> +
>>      aspeed_board_binfo.kernel_filename = machine->kernel_filename;
>>      aspeed_board_binfo.initrd_filename = machine->initrd_filename;
>>      aspeed_board_binfo.kernel_cmdline = machine->kernel_cmdline;
>