qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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


From: Cédric Le Goater
Subject: Re: [Qemu-devel] [PATCH for-2.9 24/30] aspeed: use first SPI flash as a boot ROM
Date: Mon, 5 Dec 2016 15:53:15 +0100
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.4.0

On 12/05/2016 10:57 AM, Marcin Krzemiński wrote:
> 
> 2016-12-05 10:36 GMT+01:00 Cédric Le Goater <address@hidden 
> <mailto: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.

No. The memory flash content CS0 is mapped at 0x0 and starts 
with U-Boot directly, there is no preliminary loader.

U-Boot is not merged in mainline yet. We are "slowly" building a
tree for upstream : 
 
        https://github.com/openbmc/u-boot/
        https://github.com/legoater/u-boot/


>     > 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 
> <mailto:address@hidden>>
>     >> Reviewed-by: Joel Stanley <address@hidden <mailto:address@hidden>>
>     >> Reviewed-by: Andrew Jeffery <address@hidden <mailto: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 :)

ok. I need to think about the API between the different objects, SCU and SMC,
to configure the hw strapping.

Thanks,

C. 

> 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;
>     >
> 
> 




reply via email to

[Prev in Thread] Current Thread [Next in Thread]