[PATCH v1 0/3] Updates to the OpenTitan machine

[PATCH v1 0/3] Updates to the OpenTitan machine

[Alistair Francis]

Alistair Francis (3):
  char: ibex_uart: Update the register layout
  hw/riscv: opentitan: Add the unimplement rv_core_ibex_peri
  hw/riscv: opentitan: Add the flash alias

 include/hw/riscv/opentitan.h |  3 +++
 hw/char/ibex_uart.c          | 19 ++++++++++---------
 hw/riscv/opentitan.c         |  9 +++++++++
 3 files changed, 22 insertions(+), 9 deletions(-)

-- 
2.31.1

[PATCH v1 1/3] char: ibex_uart: Update the register layout

[Alistair Francis]

Update the register layout to match the latest OpenTitan bitstream.

Signed-off-by: Alistair Francis <alistair.francis@wdc.com>
---
 hw/char/ibex_uart.c | 19 ++++++++++---------
 1 file changed, 10 insertions(+), 9 deletions(-)

diff --git a/hw/char/ibex_uart.c b/hw/char/ibex_uart.c
index fe4b6c3c9e..6b0c9330bf 100644
--- a/hw/char/ibex_uart.c
+++ b/hw/char/ibex_uart.c
@@ -42,7 +42,8 @@ REG32(INTR_STATE, 0x00)
     FIELD(INTR_STATE, RX_OVERFLOW, 3, 1)
 REG32(INTR_ENABLE, 0x04)
 REG32(INTR_TEST, 0x08)
-REG32(CTRL, 0x0C)
+REG32(ALERT_TEST, 0x0C)
+REG32(CTRL, 0x10)
     FIELD(CTRL, TX_ENABLE, 0, 1)
     FIELD(CTRL, RX_ENABLE, 1, 1)
     FIELD(CTRL, NF, 2, 1)
@@ -52,25 +53,25 @@ REG32(CTRL, 0x0C)
     FIELD(CTRL, PARITY_ODD, 7, 1)
     FIELD(CTRL, RXBLVL, 8, 2)
     FIELD(CTRL, NCO, 16, 16)
-REG32(STATUS, 0x10)
+REG32(STATUS, 0x14)
     FIELD(STATUS, TXFULL, 0, 1)
     FIELD(STATUS, RXFULL, 1, 1)
     FIELD(STATUS, TXEMPTY, 2, 1)
     FIELD(STATUS, RXIDLE, 4, 1)
     FIELD(STATUS, RXEMPTY, 5, 1)
-REG32(RDATA, 0x14)
-REG32(WDATA, 0x18)
-REG32(FIFO_CTRL, 0x1c)
+REG32(RDATA, 0x18)
+REG32(WDATA, 0x1C)
+REG32(FIFO_CTRL, 0x20)
     FIELD(FIFO_CTRL, RXRST, 0, 1)
     FIELD(FIFO_CTRL, TXRST, 1, 1)
     FIELD(FIFO_CTRL, RXILVL, 2, 3)
     FIELD(FIFO_CTRL, TXILVL, 5, 2)
-REG32(FIFO_STATUS, 0x20)
+REG32(FIFO_STATUS, 0x24)
     FIELD(FIFO_STATUS, TXLVL, 0, 5)
     FIELD(FIFO_STATUS, RXLVL, 16, 5)
-REG32(OVRD, 0x24)
-REG32(VAL, 0x28)
-REG32(TIMEOUT_CTRL, 0x2c)
+REG32(OVRD, 0x28)
+REG32(VAL, 0x2C)
+REG32(TIMEOUT_CTRL, 0x30)
 
 static void ibex_uart_update_irqs(IbexUartState *s)
 {
-- 
2.31.1

Re: [PATCH v1 1/3] char: ibex_uart: Update the register layout

[Bin Meng]

On Fri, Jul 2, 2021 at 1:19 PM Alistair Francis
<alistair.francis@wdc.com> wrote:
>
> Update the register layout to match the latest OpenTitan bitstream.
>
> Signed-off-by: Alistair Francis <alistair.francis@wdc.com>
> ---
>  hw/char/ibex_uart.c | 19 ++++++++++---------
>  1 file changed, 10 insertions(+), 9 deletions(-)
>

Reviewed-by: Bin Meng <bmeng.cn@gmail.com>

[PATCH v1 2/3] hw/riscv: opentitan: Add the unimplement rv_core_ibex_peri

[Alistair Francis]

Signed-off-by: Alistair Francis <alistair.francis@wdc.com>
---
 include/hw/riscv/opentitan.h | 1 +
 hw/riscv/opentitan.c         | 3 +++
 2 files changed, 4 insertions(+)

diff --git a/include/hw/riscv/opentitan.h b/include/hw/riscv/opentitan.h
index 86cceef698..a488f5e8ec 100644
--- a/include/hw/riscv/opentitan.h
+++ b/include/hw/riscv/opentitan.h
@@ -81,6 +81,7 @@ enum {
     IBEX_DEV_ALERT_HANDLER,
     IBEX_DEV_NMI_GEN,
     IBEX_DEV_OTBN,
+    IBEX_DEV_PERI,
 };
 
 enum {
diff --git a/hw/riscv/opentitan.c b/hw/riscv/opentitan.c
index c5a7e3bacb..933c211b11 100644
--- a/hw/riscv/opentitan.c
+++ b/hw/riscv/opentitan.c
@@ -58,6 +58,7 @@ static const MemMapEntry ibex_memmap[] = {
     [IBEX_DEV_ALERT_HANDLER] =  {  0x411b0000,  0x1000  },
     [IBEX_DEV_NMI_GEN] =        {  0x411c0000,  0x1000  },
     [IBEX_DEV_OTBN] =           {  0x411d0000,  0x10000 },
+    [IBEX_DEV_PERI] =           {  0x411f0000,  0x10000 },
 };
 
 static void opentitan_board_init(MachineState *machine)
@@ -217,6 +218,8 @@ static void lowrisc_ibex_soc_realize(DeviceState *dev_soc, Error **errp)
         memmap[IBEX_DEV_NMI_GEN].base, memmap[IBEX_DEV_NMI_GEN].size);
     create_unimplemented_device("riscv.lowrisc.ibex.otbn",
         memmap[IBEX_DEV_OTBN].base, memmap[IBEX_DEV_OTBN].size);
+    create_unimplemented_device("riscv.lowrisc.ibex.peri",
+        memmap[IBEX_DEV_PERI].base, memmap[IBEX_DEV_PERI].size);
 }
 
 static void lowrisc_ibex_soc_class_init(ObjectClass *oc, void *data)
-- 
2.31.1

Re: [PATCH v1 2/3] hw/riscv: opentitan: Add the unimplement rv_core_ibex_peri

[Bin Meng]

On Fri, Jul 2, 2021 at 1:20 PM Alistair Francis
<alistair.francis@wdc.com> wrote:
>
> Signed-off-by: Alistair Francis <alistair.francis@wdc.com>
> ---
>  include/hw/riscv/opentitan.h | 1 +
>  hw/riscv/opentitan.c         | 3 +++
>  2 files changed, 4 insertions(+)
>

Reviewed-by: Bin Meng <bmeng.cn@gmail.com>

[PATCH v1 3/3] hw/riscv: opentitan: Add the flash alias

[Alistair Francis]

Signed-off-by: Alistair Francis <alistair.francis@wdc.com>
---
 include/hw/riscv/opentitan.h | 2 ++
 hw/riscv/opentitan.c         | 6 ++++++
 2 files changed, 8 insertions(+)

diff --git a/include/hw/riscv/opentitan.h b/include/hw/riscv/opentitan.h
index a488f5e8ec..9f93bebdac 100644
--- a/include/hw/riscv/opentitan.h
+++ b/include/hw/riscv/opentitan.h
@@ -40,6 +40,7 @@ struct LowRISCIbexSoCState {
 
     MemoryRegion flash_mem;
     MemoryRegion rom;
+    MemoryRegion flash_alias;
 };
 
 typedef struct OpenTitanState {
@@ -54,6 +55,7 @@ enum {
     IBEX_DEV_ROM,
     IBEX_DEV_RAM,
     IBEX_DEV_FLASH,
+    IBEX_DEV_FLASH_VIRTUAL,
     IBEX_DEV_UART,
     IBEX_DEV_GPIO,
     IBEX_DEV_SPI,
diff --git a/hw/riscv/opentitan.c b/hw/riscv/opentitan.c
index 933c211b11..36a41c8b5b 100644
--- a/hw/riscv/opentitan.c
+++ b/hw/riscv/opentitan.c
@@ -59,6 +59,7 @@ static const MemMapEntry ibex_memmap[] = {
     [IBEX_DEV_NMI_GEN] =        {  0x411c0000,  0x1000  },
     [IBEX_DEV_OTBN] =           {  0x411d0000,  0x10000 },
     [IBEX_DEV_PERI] =           {  0x411f0000,  0x10000 },
+    [IBEX_DEV_FLASH_VIRTUAL] =  {  0x80000000,  0x80000 },
 };
 
 static void opentitan_board_init(MachineState *machine)
@@ -134,8 +135,13 @@ static void lowrisc_ibex_soc_realize(DeviceState *dev_soc, Error **errp)
     /* Flash memory */
     memory_region_init_rom(&s->flash_mem, OBJECT(dev_soc), "riscv.lowrisc.ibex.flash",
                            memmap[IBEX_DEV_FLASH].size, &error_fatal);
+    memory_region_init_alias(&s->flash_alias, OBJECT(dev_soc),
+                             "riscv.lowrisc.ibex.flash_virtual", &s->flash_mem, 0,
+                             memmap[IBEX_DEV_FLASH_VIRTUAL].size);
     memory_region_add_subregion(sys_mem, memmap[IBEX_DEV_FLASH].base,
                                 &s->flash_mem);
+    memory_region_add_subregion(sys_mem, memmap[IBEX_DEV_FLASH_VIRTUAL].base,
+                                &s->flash_alias);
 
     /* PLIC */
     if (!sysbus_realize(SYS_BUS_DEVICE(&s->plic), errp)) {
-- 
2.31.1

Re: [PATCH v1 3/3] hw/riscv: opentitan: Add the flash alias

[Bin Meng]

On Fri, Jul 2, 2021 at 1:20 PM Alistair Francis
<alistair.francis@wdc.com> wrote:

Could you add some commit message to explain this alias?

>
> Signed-off-by: Alistair Francis <alistair.francis@wdc.com>
> ---
>  include/hw/riscv/opentitan.h | 2 ++
>  hw/riscv/opentitan.c         | 6 ++++++
>  2 files changed, 8 insertions(+)
>
> diff --git a/include/hw/riscv/opentitan.h b/include/hw/riscv/opentitan.h
> index a488f5e8ec..9f93bebdac 100644
> --- a/include/hw/riscv/opentitan.h
> +++ b/include/hw/riscv/opentitan.h
> @@ -40,6 +40,7 @@ struct LowRISCIbexSoCState {
>
>      MemoryRegion flash_mem;
>      MemoryRegion rom;
> +    MemoryRegion flash_alias;
>  };
>
>  typedef struct OpenTitanState {
> @@ -54,6 +55,7 @@ enum {
>      IBEX_DEV_ROM,
>      IBEX_DEV_RAM,
>      IBEX_DEV_FLASH,
> +    IBEX_DEV_FLASH_VIRTUAL,

Is this virtual address? But it is still physical?

>      IBEX_DEV_UART,
>      IBEX_DEV_GPIO,
>      IBEX_DEV_SPI,
> diff --git a/hw/riscv/opentitan.c b/hw/riscv/opentitan.c
> index 933c211b11..36a41c8b5b 100644
> --- a/hw/riscv/opentitan.c
> +++ b/hw/riscv/opentitan.c
> @@ -59,6 +59,7 @@ static const MemMapEntry ibex_memmap[] = {
>      [IBEX_DEV_NMI_GEN] =        {  0x411c0000,  0x1000  },
>      [IBEX_DEV_OTBN] =           {  0x411d0000,  0x10000 },
>      [IBEX_DEV_PERI] =           {  0x411f0000,  0x10000 },
> +    [IBEX_DEV_FLASH_VIRTUAL] =  {  0x80000000,  0x80000 },
>  };
>
>  static void opentitan_board_init(MachineState *machine)
> @@ -134,8 +135,13 @@ static void lowrisc_ibex_soc_realize(DeviceState *dev_soc, Error **errp)
>      /* Flash memory */
>      memory_region_init_rom(&s->flash_mem, OBJECT(dev_soc), "riscv.lowrisc.ibex.flash",
>                             memmap[IBEX_DEV_FLASH].size, &error_fatal);
> +    memory_region_init_alias(&s->flash_alias, OBJECT(dev_soc),
> +                             "riscv.lowrisc.ibex.flash_virtual", &s->flash_mem, 0,
> +                             memmap[IBEX_DEV_FLASH_VIRTUAL].size);
>      memory_region_add_subregion(sys_mem, memmap[IBEX_DEV_FLASH].base,
>                                  &s->flash_mem);
> +    memory_region_add_subregion(sys_mem, memmap[IBEX_DEV_FLASH_VIRTUAL].base,
> +                                &s->flash_alias);
>
>      /* PLIC */
>      if (!sysbus_realize(SYS_BUS_DEVICE(&s->plic), errp)) {
> --

Regards,
Bin

Re: [PATCH v1 3/3] hw/riscv: opentitan: Add the flash alias

[Alistair Francis]

On Mon, Jul 5, 2021 at 4:16 PM Bin Meng <bmeng.cn@gmail.com> wrote:
>
> On Fri, Jul 2, 2021 at 1:20 PM Alistair Francis
> <alistair.francis@wdc.com> wrote:
>
> Could you add some commit message to explain this alias?

Yep, I'll add something.

>
> >
> > Signed-off-by: Alistair Francis <alistair.francis@wdc.com>
> > ---
> >  include/hw/riscv/opentitan.h | 2 ++
> >  hw/riscv/opentitan.c         | 6 ++++++
> >  2 files changed, 8 insertions(+)
> >
> > diff --git a/include/hw/riscv/opentitan.h b/include/hw/riscv/opentitan.h
> > index a488f5e8ec..9f93bebdac 100644
> > --- a/include/hw/riscv/opentitan.h
> > +++ b/include/hw/riscv/opentitan.h
> > @@ -40,6 +40,7 @@ struct LowRISCIbexSoCState {
> >
> >      MemoryRegion flash_mem;
> >      MemoryRegion rom;
> > +    MemoryRegion flash_alias;
> >  };
> >
> >  typedef struct OpenTitanState {
> > @@ -54,6 +55,7 @@ enum {
> >      IBEX_DEV_ROM,
> >      IBEX_DEV_RAM,
> >      IBEX_DEV_FLASH,
> > +    IBEX_DEV_FLASH_VIRTUAL,
>
> Is this virtual address? But it is still physical?

It's a physical address (OpenTitan has no MMU so all addresses are physical).

There is an alias region to access the flash region, so the region can
be accessed either by it's "real" address or this "virtual" address
range. It's similar to some other MCUs, like the STM range.

The virtual is just the name that they call it.

Alistair

>
> >      IBEX_DEV_UART,
> >      IBEX_DEV_GPIO,
> >      IBEX_DEV_SPI,
> > diff --git a/hw/riscv/opentitan.c b/hw/riscv/opentitan.c
> > index 933c211b11..36a41c8b5b 100644
> > --- a/hw/riscv/opentitan.c
> > +++ b/hw/riscv/opentitan.c
> > @@ -59,6 +59,7 @@ static const MemMapEntry ibex_memmap[] = {
> >      [IBEX_DEV_NMI_GEN] =        {  0x411c0000,  0x1000  },
> >      [IBEX_DEV_OTBN] =           {  0x411d0000,  0x10000 },
> >      [IBEX_DEV_PERI] =           {  0x411f0000,  0x10000 },
> > +    [IBEX_DEV_FLASH_VIRTUAL] =  {  0x80000000,  0x80000 },
> >  };
> >
> >  static void opentitan_board_init(MachineState *machine)
> > @@ -134,8 +135,13 @@ static void lowrisc_ibex_soc_realize(DeviceState *dev_soc, Error **errp)
> >      /* Flash memory */
> >      memory_region_init_rom(&s->flash_mem, OBJECT(dev_soc), "riscv.lowrisc.ibex.flash",
> >                             memmap[IBEX_DEV_FLASH].size, &error_fatal);
> > +    memory_region_init_alias(&s->flash_alias, OBJECT(dev_soc),
> > +                             "riscv.lowrisc.ibex.flash_virtual", &s->flash_mem, 0,
> > +                             memmap[IBEX_DEV_FLASH_VIRTUAL].size);
> >      memory_region_add_subregion(sys_mem, memmap[IBEX_DEV_FLASH].base,
> >                                  &s->flash_mem);
> > +    memory_region_add_subregion(sys_mem, memmap[IBEX_DEV_FLASH_VIRTUAL].base,
> > +                                &s->flash_alias);
> >
> >      /* PLIC */
> >      if (!sysbus_realize(SYS_BUS_DEVICE(&s->plic), errp)) {
> > --
>
> Regards,
> Bin