On Tue, Sep 24, 2019 at 1:51 AM Alistair Francis <alistai...@gmail.com> wrote:
>
> On Sat, Sep 21, 2019 at 7:19 PM Bin Meng <bmeng...@gmail.com> wrote:
> >
> > On Sat, Sep 21, 2019 at 6:12 AM Alistair Francis <alistai...@gmail.com>
> > wrote:
> > >
> > > On Thu, Sep 19, 2019 at 10:15 PM Bin Meng <bmeng...@gmail.com> wrote:
> > > >
> > > > On Fri, Sep 20, 2019 at 6:32 AM Alistair Francis
> > > > <alistair.fran...@wdc.com> wrote:
> > > > >
> > > > > Add a property that when set to true QEMU will jump from the ROM code
> > > > > to
> > > > > the start of flash memory instead of DRAM which is the default
> > > > > behaviour.
> > > > >
> > > > > Signed-off-by: Alistair Francis <alistair.fran...@wdc.com>
> > > > > ---
> > > > > hw/riscv/sifive_u.c | 27 +++++++++++++++++++++++++++
> > > > > include/hw/riscv/sifive_u.h | 2 ++
> > > > > 2 files changed, 29 insertions(+)
> > > > >
> > > > > diff --git a/hw/riscv/sifive_u.c b/hw/riscv/sifive_u.c
> > > > > index c3949fb316..b7cd3631cd 100644
> > > > > --- a/hw/riscv/sifive_u.c
> > > > > +++ b/hw/riscv/sifive_u.c
> > > > > @@ -373,6 +373,10 @@ static void riscv_sifive_u_init(MachineState
> > > > > *machine)
> > > > > /* dtb: */
> > > > > };
> > > > >
> > > > > + if (s->start_in_flash) {
> > > > > + reset_vec[6] = memmap[SIFIVE_U_FLASH0].base; /* start:
> > > > > .dword FLASH0_BASE */
> > > > > + }
> > > > > +
> > > > > /* copy in the reset vector in little_endian byte order */
> > > > > for (i = 0; i < sizeof(reset_vec) >> 2; i++) {
> > > > > reset_vec[i] = cpu_to_le32(reset_vec[i]);
> > > > > @@ -544,8 +548,31 @@ static void
> > > > > riscv_sifive_u_soc_realize(DeviceState *dev, Error **errp)
> > > > > memmap[SIFIVE_U_GEM_MGMT].base,
> > > > > memmap[SIFIVE_U_GEM_MGMT].size);
> > > > > }
> > > > >
> > > > > +static bool virt_get_start_in_flash(Object *obj, Error **errp)
> > > > > +{
> > > > > + SiFiveUState *s = RISCV_U_MACHINE(obj);
> > > > > +
> > > > > + return s->start_in_flash;
> > > > > +}
> > > > > +
> > > > > +static void virt_set_start_in_flash(Object *obj, bool value, Error
> > > > > **errp)
> > > > > +{
> > > > > + SiFiveUState *s = RISCV_U_MACHINE(obj);
> > > > > +
> > > > > + s->start_in_flash = value;
> > > > > +}
> > > > > +
> > > > > static void riscv_sifive_u_machine_instance_init(Object *obj)
> > > > > {
> > > > > + SiFiveUState *s = RISCV_U_MACHINE(obj);
> > > > > +
> > > > > + s->start_in_flash = false;
> > > > > + object_property_add_bool(obj, "start-in-flash",
> > > > > virt_get_start_in_flash,
> > > > > + virt_set_start_in_flash, NULL);
> > > > > + object_property_set_description(obj, "start-in-flash",
> > > > > + "Set on to tell QEMU's ROM to
> > > > > jump to " \
> > > > > + "flash. Otherwise QEMU will jump
> > > > > to DRAM",
> > > > > + NULL);
> > > > >
> > > > > }
> > > > >
> > > > > diff --git a/include/hw/riscv/sifive_u.h b/include/hw/riscv/sifive_u.h
> > > > > index a921079fbe..2656b43c58 100644
> > > > > --- a/include/hw/riscv/sifive_u.h
> > > > > +++ b/include/hw/riscv/sifive_u.h
> > > > > @@ -57,6 +57,8 @@ typedef struct SiFiveUState {
> > > > >
> > > > > void *fdt;
> > > > > int fdt_size;
> > > > > +
> > > > > + bool start_in_flash;
> > > > > } SiFiveUState;
> > > > >
> > > > > enum {
> > > >
> > > > This patch chose a different way from the one used in patch "[v1,6/6]
> > > > riscv/virt: Jump to pflash if specified":
> > > >
> > > > - this patch uses reset_vec[6] while patch [6/6] defines a variable
> > > > start_addr
> > > > - this patch adds a "start-in-flash" property to the machine, while
> > > > patch [6/6] tests against drive IF_PFLASH
> > >
> > > Yes, we do it differently for the sifive_u board as the sifive_u board
> > > doesn't use pflash so there is no way to know if the user has loaded
> > > anything into the SPI memory.
> > >
> >
> > OK.
> >
> > > >
> > > > We should be consistent and I would prefer to use the patch [6/6] way.
> > > > On Unleashed an SPI flash is mounted so we cannot add a PFlash to
> > > > sifive_u machine like what was done on virt machine, so we should test
> > > > IF_MTD instead. Thoughts?
> > >
> > > How would we test that?
> > >
> > > Right now I am loading the binary in SPI with the -device loader
> > > option. The machine can't really know what is/isn't loaded there.
> > >
> > > It's not ideal, but I don't see a nicer way.
> >
> > I think we need write a SiFive SPI model to support this in a clean
> > way. Ideally we should simulate the hardware boot workflow as
> > documented in the FU540 manual chapter 6 "Boot Process".
>
> I really didn't want to do this. For me it's low priority and there
> are enough other things to work on rather then adding SiFive device
> models. Maybe someone who works at SiFive would be able to do this?
>
> My hope with this series is that we could unblock firmware developers
> (oreboot and coreboot) while the SPI model is written.
>
OK, so let's set the expectation that this is a temporary flash
solution and will be changed in the future when the SiFive SPI model
is ready.
Regards,
Bin
