Re: [PATCH 5/9] hw/arm/aspeed_ast10x0: Map the secure SRAM

[Peter Delevoryas]

On Thu, Dec 29, 2022 at 04:23:21PM +0100, Philippe Mathieu-Daudé wrote:
> Some SRAM appears to be used by the Secure Boot unit and
> crypto accelerators. Name it 'secure sram'.
> 
> Note, the SRAM base address was already present but unused
> (the 'SBC' index is used for the MMIO peripheral).
> 
> Interestingly using CFLAGS=-Winitializer-overrides reports:
> 
>   ../hw/arm/aspeed_ast10x0.c:32:30: warning: initializer overrides prior 
> initialization of this subobject [-Winitializer-overrides]
>     [ASPEED_DEV_SBC]       = 0x7E6F2000,
>                              ^~~~~~~~~~
>   ../hw/arm/aspeed_ast10x0.c:24:30: note: previous initialization is here
>     [ASPEED_DEV_SBC]       = 0x79000000,
>                              ^~~~~~~~~~

Ohhhh! Oh no, yeah I think Zephyr has this warning enabled by default, right? I
guess it's not enabled in QEMU? Hmmmm.

Reviewed-by: Peter Delevoryas <peter@pjd.dev>

Also maybe include this tag?

Fixes: 356b230ed1 ("aspeed/soc : Add AST1030 support")

> This fixes with Zephyr:
> 
>   uart:~$ rsa test
>   rsa test vector[0]:
>   [00:00:26.156,000] <err> os: ***** BUS FAULT *****
>   [00:00:26.157,000] <err> os:   Precise data bus error
>   [00:00:26.157,000] <err> os:   BFAR Address: 0x79000000
>   [00:00:26.158,000] <err> os: r0/a1:  0x79000000  r1/a2:  0x00000000  r2/a3: 
>  0x00001800
>   [00:00:26.158,000] <err> os: r3/a4:  0x79001800 r12/ip:  0x00000800 r14/lr: 
>  0x0001098d
>   [00:00:26.158,000] <err> os:  xpsr:  0x81000000
>   [00:00:26.158,000] <err> os: Faulting instruction address (r15/pc): 
> 0x0001e1bc
>   [00:00:26.158,000] <err> os: >>> ZEPHYR FATAL ERROR 0: CPU exception on CPU > 0
>   [00:00:26.158,000] <err> os: Current thread: 0x38248 (shell_uart)
>   [00:00:26.165,000] <err> os: Halting system
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>  hw/arm/aspeed_ast10x0.c     | 11 ++++++++++-
>  include/hw/arm/aspeed_soc.h |  3 +++
>  2 files changed, 13 insertions(+), 1 deletion(-)
> 
> diff --git a/hw/arm/aspeed_ast10x0.c b/hw/arm/aspeed_ast10x0.c
> index 53ea6d471f..21a2e62345 100644
> --- a/hw/arm/aspeed_ast10x0.c
> +++ b/hw/arm/aspeed_ast10x0.c
> @@ -22,7 +22,7 @@
>  
>  static const hwaddr aspeed_soc_ast1030_memmap[] = {
>      [ASPEED_DEV_SRAM]      = 0x00000000,
> -    [ASPEED_DEV_SBC]       = 0x79000000,
> +    [ASPEED_DEV_SECSRAM]   = 0x79000000,
>      [ASPEED_DEV_IOMEM]     = 0x7E600000,
>      [ASPEED_DEV_PWM]       = 0x7E610000,
>      [ASPEED_DEV_FMC]       = 0x7E620000,
> @@ -222,6 +222,14 @@ static void aspeed_soc_ast1030_realize(DeviceState 
> *dev_soc, Error **errp)
>      memory_region_add_subregion(s->memory,
>                                  sc->memmap[ASPEED_DEV_SRAM],
>                                  &s->sram);
> +    memory_region_init_ram(&s->secsram, OBJECT(s), "sec.sram",
> +                           sc->secsram_size, &err);
> +    if (err != NULL) {
> +        error_propagate(errp, err);
> +        return;
> +    }
> +    memory_region_add_subregion(s->memory, sc->memmap[ASPEED_DEV_SECSRAM],
> +                                &s->secsram);
>  
>      /* SCU */
>      if (!sysbus_realize(SYS_BUS_DEVICE(&s->scu), errp)) {
> @@ -401,6 +409,7 @@ static void aspeed_soc_ast1030_class_init(ObjectClass 
> *klass, void *data)
>      sc->cpu_type = ARM_CPU_TYPE_NAME("cortex-m4");
>      sc->silicon_rev = AST1030_A1_SILICON_REV;
>      sc->sram_size = 768 * KiB;
> +    sc->secsram_size = 9 * KiB;
>      sc->spis_num = 2;
>      sc->ehcis_num = 0;
>      sc->wdts_num = 4;
> diff --git a/include/hw/arm/aspeed_soc.h b/include/hw/arm/aspeed_soc.h
> index 9a5e3c0bac..bd1e03e78a 100644
> --- a/include/hw/arm/aspeed_soc.h
> +++ b/include/hw/arm/aspeed_soc.h
> @@ -71,6 +71,7 @@ struct AspeedSoCState {
>      AspeedSMCState spi[ASPEED_SPIS_NUM];
>      EHCISysBusState ehci[ASPEED_EHCIS_NUM];
>      AspeedSBCState sbc;
> +    MemoryRegion secsram;
>      UnimplementedDeviceState sbc_unimplemented;
>      AspeedSDMCState sdmc;
>      AspeedWDTState wdt[ASPEED_WDTS_NUM];
> @@ -105,6 +106,7 @@ struct AspeedSoCClass {
>      const char *cpu_type;
>      uint32_t silicon_rev;
>      uint64_t sram_size;
> +    uint64_t secsram_size;
>      int spis_num;
>      int ehcis_num;
>      int wdts_num;
> @@ -143,6 +145,7 @@ enum {
>      ASPEED_DEV_SCU,
>      ASPEED_DEV_ADC,
>      ASPEED_DEV_SBC,
> +    ASPEED_DEV_SECSRAM,
>      ASPEED_DEV_EMMC_BC,
>      ASPEED_DEV_VIDEO,
>      ASPEED_DEV_SRAM,
> -- 
> 2.38.1
>