Re: [PATCH 3/3] hw/riscv: opentitan: Expose the resetvec as a SoC property

[Wilfred Mallawa <wilfred.mallawa@wdc.com>]

On Wed, 2022-09-14 at 12:11 +0200, Alistair Francis via wrote:
> On the OpenTitan hardware the resetvec is fixed at the start of ROM.
> In
> QEMU we don't run the ROM code and instead just jump to the next
> stage.
> This means we need to be a little more flexible about what the
> resetvec
> is.
> 
> This patch allows us to set the resetvec from the command line with
> something like this:
>     -global
> driver=riscv.lowrisc.ibex.soc,property=resetvec,value=0x20000400
> 
> This way as the next stage changes we can update the resetvec.
> 
> Signed-off-by: Alistair Francis <alistair.francis@wdc.com>
> ---
>  include/hw/riscv/opentitan.h | 2 ++
>  hw/riscv/opentitan.c         | 8 +++++++-
>  2 files changed, 9 insertions(+), 1 deletion(-)
> 
> diff --git a/include/hw/riscv/opentitan.h
> b/include/hw/riscv/opentitan.h
> index 26d960f288..6665cd5794 100644
> --- a/include/hw/riscv/opentitan.h
> +++ b/include/hw/riscv/opentitan.h
> @@ -46,6 +46,8 @@ struct LowRISCIbexSoCState {
>      IbexTimerState timer;
>      IbexSPIHostState spi_host[OPENTITAN_NUM_SPI_HOSTS];
>  
> +    uint32_t resetvec;
> +
>      MemoryRegion flash_mem;
>      MemoryRegion rom;
>      MemoryRegion flash_alias;
> diff --git a/hw/riscv/opentitan.c b/hw/riscv/opentitan.c
> index 45c92c9bbc..be7ff1eea0 100644
> --- a/hw/riscv/opentitan.c
> +++ b/hw/riscv/opentitan.c
> @@ -142,7 +142,7 @@ static void lowrisc_ibex_soc_realize(DeviceState
> *dev_soc, Error **errp)
>                              &error_abort);
>      object_property_set_int(OBJECT(&s->cpus), "num-harts", ms-
> >smp.cpus,
>                              &error_abort);
> -    object_property_set_int(OBJECT(&s->cpus), "resetvec",
> 0x20000400,
> +    object_property_set_int(OBJECT(&s->cpus), "resetvec", s-
> >resetvec,
>                              &error_abort);
>      sysbus_realize(SYS_BUS_DEVICE(&s->cpus), &error_fatal);
>  
> @@ -297,10 +297,16 @@ static void
> lowrisc_ibex_soc_realize(DeviceState *dev_soc, Error **errp)
>          memmap[IBEX_DEV_PERI].base, memmap[IBEX_DEV_PERI].size);
>  }
>  
> +static Property lowrisc_ibex_soc_props[] = {
> +    DEFINE_PROP_UINT32("resetvec", LowRISCIbexSoCState, resetvec,
> 0x20000400),
> +    DEFINE_PROP_END_OF_LIST()
> +};
> +
>  static void lowrisc_ibex_soc_class_init(ObjectClass *oc, void *data)
>  {
>      DeviceClass *dc = DEVICE_CLASS(oc);
>  
> +    device_class_set_props(dc, lowrisc_ibex_soc_props);
>      dc->realize = lowrisc_ibex_soc_realize;
>      /* Reason: Uses serial_hds in realize function, thus can't be
> used twice */
>      dc->user_creatable = false;

Nice! I tested this on https://github.com/tock/tock/pull/3056 , with
the addition of `global
driver=riscv.lowrisc.ibex.soc,property=resetvec,value=0x20000450 `

Alot more convienient with this patch for when the entry point changes,
will look into parsing the manifest to dynamically set it! 

Reviewed by: Wilfred Mallawa <wilfred.mallawa@wdc.com>