[PATCH v4 00/10] Support vbootrom for AST2700

[PATCH v4 00/10] Support vbootrom for AST2700

[Jamin Lin via]

v1:
  Add initial support for AST27x0
  The purpose of vbootrom here is to simulate the work of BootMCU SPL (riscv)
  in AST2700, because QEMU doesn't support heterogenous architecture yet.

  ast27x0_bootrom.bin is a simplified, free (Apache 2.0) boot ROM for
  ASPEED AST27x0 BMC SOC. It currently implements the bare minimum to
  load, parse, initialize and run boot images stored in SPI flash, but may grow
  more features over time as needed. The source code is available at:
  https://github.com/google/vbootrom

v2:
  Add "Introduced ASPEED_DEV_VBOOTROM in the device enumeration" patch to fix
  build failed.

v3:
  1. Supports both vbootrom and device loader boot methods, with vbootrom used as
  the default.
  2. Fix review and QTEST test failed issues. 

v4: 
  Adjust the patch order.

Jamin Lin (10):
  hw/arm/aspeed_ast27x0: Rename variable sram_name to name in ast2700
    realize
  hw/arm/aspeed_ast27x0 Introduce vbootrom memory region
  hw/arm/aspeed: Add vbootrom support on AST2700 EVB machines
  hw/arm/aspeed: Reuse rom_size variable for vbootrom setup
  pc-bios: Add AST27x0 vBootrom
  hw/arm/aspeed: Add support for loading vbootrom image via "-bios"
  tests/functional/aspeed: Move I2C test into shared helper for AST2700
    reuse
  tests/functional/aspeed: Update test ASPEED SDK v09.06
  tests/functional/aspeed: Add to test vbootrom for AST2700
  docs/system/arm/aspeed: Support vbootrom for AST2700

 MAINTAINERS                             |   1 +
 docs/system/arm/aspeed.rst              |  29 +++++++++++-
 include/hw/arm/aspeed.h                 |   2 +
 include/hw/arm/aspeed_soc.h             |   3 ++
 hw/arm/aspeed.c                         |  41 +++++++++++++++-
 hw/arm/aspeed_ast27x0.c                 |  20 ++++++--
 pc-bios/README                          |   6 +++
 pc-bios/ast27x0_bootrom.bin             | Bin 0 -> 15464 bytes
 pc-bios/meson.build                     |   1 +
 tests/functional/test_aarch64_aspeed.py |  59 +++++++++++++++---------
 10 files changed, 135 insertions(+), 27 deletions(-)
 create mode 100644 pc-bios/ast27x0_bootrom.bin

-- 
2.43.0

[PATCH v4 01/10] hw/arm/aspeed_ast27x0: Rename variable sram_name to name in ast2700 realize

[Jamin Lin via]

The variable "sram_name" was only used for naming the SRAM memory region.
Rename it to "name" for consistency with similar code and avoid unnecessary
new local variable declarations.

Signed-off-by: Jamin Lin <jamin_lin@aspeedtech.com>
Reviewed-by: Cédric Le Goater <clg@redhat.com>
Tested-by: Nabih Estefan <nabihestefan@google.com>
---
 hw/arm/aspeed_ast27x0.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/hw/arm/aspeed_ast27x0.c b/hw/arm/aspeed_ast27x0.c
index dce7255a2c..b05ed75ff4 100644
--- a/hw/arm/aspeed_ast27x0.c
+++ b/hw/arm/aspeed_ast27x0.c
@@ -577,7 +577,7 @@ static void aspeed_soc_ast2700_realize(DeviceState *dev, Error **errp)
     AspeedSoCClass *sc = ASPEED_SOC_GET_CLASS(s);
     AspeedINTCClass *ic = ASPEED_INTC_GET_CLASS(&a->intc[0]);
     AspeedINTCClass *icio = ASPEED_INTC_GET_CLASS(&a->intc[1]);
-    g_autofree char *sram_name = NULL;
+    g_autofree char *name = NULL;
     qemu_irq irq;
 
     /* Default boot region (SPI memory or ROMs) */
@@ -649,9 +649,9 @@ static void aspeed_soc_ast2700_realize(DeviceState *dev, Error **errp)
     }
 
     /* SRAM */
-    sram_name = g_strdup_printf("aspeed.sram.%d", CPU(&a->cpu[0])->cpu_index);
-    if (!memory_region_init_ram(&s->sram, OBJECT(s), sram_name, sc->sram_size,
-                                 errp)) {
+    name = g_strdup_printf("aspeed.sram.%d", CPU(&a->cpu[0])->cpu_index);
+    if (!memory_region_init_ram(&s->sram, OBJECT(s), name, sc->sram_size,
+                                errp)) {
         return;
     }
     memory_region_add_subregion(s->memory,
-- 
2.43.0

[PATCH v4 02/10] hw/arm/aspeed_ast27x0 Introduce vbootrom memory region

[Jamin Lin via]

Introduce a new vbootrom memory region. The region is mapped at address
"0x00000000" and has a size of 128KB, identical to the SRAM region size.
This memory region is intended for loading a vbootrom image file as part of the
boot process.

The vbootrom registered in the SoC's address space using the ASPEED_DEV_VBOOTROM
index.

Introduced a "vbootrom_size" attribute in "AspeedSoCClass" to define virtual
boot ROM size.

Signed-off-by: Jamin Lin <jamin_lin@aspeedtech.com>
Reviewed-by: Nabih Estefan <nabihestefan@google.com>
Tested-by: Nabih Estefan <nabihestefan@google.com>
---
 include/hw/arm/aspeed_soc.h |  3 +++
 hw/arm/aspeed_ast27x0.c     | 12 ++++++++++++
 2 files changed, 15 insertions(+)

diff --git a/include/hw/arm/aspeed_soc.h b/include/hw/arm/aspeed_soc.h
index f069d17d16..9af8cfbc3e 100644
--- a/include/hw/arm/aspeed_soc.h
+++ b/include/hw/arm/aspeed_soc.h
@@ -59,6 +59,7 @@ struct AspeedSoCState {
     MemoryRegion sram;
     MemoryRegion spi_boot_container;
     MemoryRegion spi_boot;
+    MemoryRegion vbootrom;
     AddressSpace dram_as;
     AspeedRtcState rtc;
     AspeedTimerCtrlState timerctrl;
@@ -152,6 +153,7 @@ struct AspeedSoCClass {
     const char * const *valid_cpu_types;
     uint32_t silicon_rev;
     uint64_t sram_size;
+    uint64_t vbootrom_size;
     uint64_t secsram_size;
     int spis_num;
     int ehcis_num;
@@ -169,6 +171,7 @@ struct AspeedSoCClass {
 const char *aspeed_soc_cpu_type(AspeedSoCClass *sc);
 
 enum {
+    ASPEED_DEV_VBOOTROM,
     ASPEED_DEV_SPI_BOOT,
     ASPEED_DEV_IOMEM,
     ASPEED_DEV_UART0,
diff --git a/hw/arm/aspeed_ast27x0.c b/hw/arm/aspeed_ast27x0.c
index b05ed75ff4..7eece8e286 100644
--- a/hw/arm/aspeed_ast27x0.c
+++ b/hw/arm/aspeed_ast27x0.c
@@ -24,6 +24,7 @@
 #include "qemu/log.h"
 
 static const hwaddr aspeed_soc_ast2700_memmap[] = {
+    [ASPEED_DEV_VBOOTROM]  =  0x00000000,
     [ASPEED_DEV_SRAM]      =  0x10000000,
     [ASPEED_DEV_HACE]      =  0x12070000,
     [ASPEED_DEV_EMMC]      =  0x12090000,
@@ -657,6 +658,15 @@ static void aspeed_soc_ast2700_realize(DeviceState *dev, Error **errp)
     memory_region_add_subregion(s->memory,
                                 sc->memmap[ASPEED_DEV_SRAM], &s->sram);
 
+    /* VBOOTROM */
+    name = g_strdup_printf("aspeed.vbootrom.%d", CPU(&a->cpu[0])->cpu_index);
+    if (!memory_region_init_ram(&s->vbootrom, OBJECT(s), name,
+                                sc->vbootrom_size, errp)) {
+        return;
+    }
+    memory_region_add_subregion(s->memory,
+                                sc->memmap[ASPEED_DEV_VBOOTROM], &s->vbootrom);
+
     /* SCU */
     if (!sysbus_realize(SYS_BUS_DEVICE(&s->scu), errp)) {
         return;
@@ -898,6 +908,7 @@ static void aspeed_soc_ast2700a0_class_init(ObjectClass *oc, void *data)
 
     sc->valid_cpu_types = valid_cpu_types;
     sc->silicon_rev  = AST2700_A0_SILICON_REV;
+    sc->vbootrom_size = 0x20000;
     sc->sram_size    = 0x20000;
     sc->spis_num     = 3;
     sc->wdts_num     = 8;
@@ -925,6 +936,7 @@ static void aspeed_soc_ast2700a1_class_init(ObjectClass *oc, void *data)
 
     sc->valid_cpu_types = valid_cpu_types;
     sc->silicon_rev  = AST2700_A1_SILICON_REV;
+    sc->vbootrom_size = 0x20000;
     sc->sram_size    = 0x20000;
     sc->spis_num     = 3;
     sc->wdts_num     = 8;
-- 
2.43.0

[PATCH v4 03/10] hw/arm/aspeed: Add vbootrom support on AST2700 EVB machines

[Jamin Lin via]

Introduce a new "vbootrom" field in the AspeedMachineClass to indicate whether
a machine supports the virtual boot ROM region.

Set this field to true by default for the AST2700-A0 and AST2700-A1 EVB
machines.

Signed-off-by: Jamin Lin <jamin_lin@aspeedtech.com>
Reviewed-by: Nabih Estefan <nabihestefan@google.com>
Tested-by: Nabih Estefan <nabihestefan@google.com>
---
 include/hw/arm/aspeed.h | 1 +
 hw/arm/aspeed.c         | 2 ++
 2 files changed, 3 insertions(+)

diff --git a/include/hw/arm/aspeed.h b/include/hw/arm/aspeed.h
index 9cae45a1c9..973277bea6 100644
--- a/include/hw/arm/aspeed.h
+++ b/include/hw/arm/aspeed.h
@@ -40,6 +40,7 @@ struct AspeedMachineClass {
     void (*i2c_init)(AspeedMachineState *bmc);
     uint32_t uart_default;
     bool sdhci_wp_inverted;
+    bool vbootrom;
 };
 
 
diff --git a/hw/arm/aspeed.c b/hw/arm/aspeed.c
index 82f42582fa..e852bbc4cb 100644
--- a/hw/arm/aspeed.c
+++ b/hw/arm/aspeed.c
@@ -1689,6 +1689,7 @@ static void aspeed_machine_ast2700a0_evb_class_init(ObjectClass *oc, void *data)
     amc->macs_mask = ASPEED_MAC0_ON | ASPEED_MAC1_ON | ASPEED_MAC2_ON;
     amc->uart_default = ASPEED_DEV_UART12;
     amc->i2c_init  = ast2700_evb_i2c_init;
+    amc->vbootrom = true;
     mc->auto_create_sdcard = true;
     mc->default_ram_size = 1 * GiB;
     aspeed_machine_class_init_cpus_defaults(mc);
@@ -1709,6 +1710,7 @@ static void aspeed_machine_ast2700a1_evb_class_init(ObjectClass *oc, void *data)
     amc->macs_mask = ASPEED_MAC0_ON | ASPEED_MAC1_ON | ASPEED_MAC2_ON;
     amc->uart_default = ASPEED_DEV_UART12;
     amc->i2c_init  = ast2700_evb_i2c_init;
+    amc->vbootrom = true;
     mc->auto_create_sdcard = true;
     mc->default_ram_size = 1 * GiB;
     aspeed_machine_class_init_cpus_defaults(mc);
-- 
2.43.0

[PATCH v4 04/10] hw/arm/aspeed: Reuse rom_size variable for vbootrom setup

[Jamin Lin via]

Move the declaration of "rom_size" to an outer scope in aspeed_machine_init()
so it can be reused for setting up the vbootrom region as well.

This avoids introducing a redundant local variable and ensures consistent
ROM sizing logic when both SPI boot and vbootrom are used.

Signed-off-by: Jamin Lin <jamin_lin@aspeedtech.com>
Reviewed-by: Cédric Le Goater <clg@redhat.com>
Tested-by: Nabih Estefan <nabihestefan@google.com>
---
 hw/arm/aspeed.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/hw/arm/aspeed.c b/hw/arm/aspeed.c
index e852bbc4cb..b70a120e62 100644
--- a/hw/arm/aspeed.c
+++ b/hw/arm/aspeed.c
@@ -381,6 +381,7 @@ static void aspeed_machine_init(MachineState *machine)
     AspeedSoCClass *sc;
     int i;
     DriveInfo *emmc0 = NULL;
+    uint64_t rom_size;
     bool boot_emmc;
 
     bmc->soc = ASPEED_SOC(object_new(amc->soc_name));
@@ -475,7 +476,7 @@ static void aspeed_machine_init(MachineState *machine)
         BlockBackend *fmc0 = dev ? m25p80_get_blk(dev) : NULL;
 
         if (fmc0 && !boot_emmc) {
-            uint64_t rom_size = memory_region_size(&bmc->soc->spi_boot);
+            rom_size = memory_region_size(&bmc->soc->spi_boot);
             aspeed_install_boot_rom(bmc, fmc0, rom_size);
         } else if (emmc0) {
             aspeed_install_boot_rom(bmc, blk_by_legacy_dinfo(emmc0), 64 * KiB);
-- 
2.43.0

[PATCH v4 05/10] pc-bios: Add AST27x0 vBootrom

[Jamin Lin via]

The boot ROM is a minimal implementation designed to load an AST27x0 boot image.
Its source code is available at:
https://github.com/google/vbootrom

Signed-off-by: Jamin Lin <jamin_lin@aspeedtech.com>
Reviewed-by: Nabih Estefan <nabihestefan@google.com>
Tested-by: Nabih Estefan <nabihestefan@google.com>
---
 MAINTAINERS                 |   1 +
 pc-bios/README              |   6 ++++++
 pc-bios/ast27x0_bootrom.bin | Bin 0 -> 15464 bytes
 pc-bios/meson.build         |   1 +
 4 files changed, 8 insertions(+)
 create mode 100644 pc-bios/ast27x0_bootrom.bin

diff --git a/MAINTAINERS b/MAINTAINERS
index d54b5578f8..70ab0d0afa 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1163,6 +1163,7 @@ F: docs/system/arm/fby35.rst
 F: tests/*/*aspeed*
 F: tests/*/*ast2700*
 F: hw/arm/fby35.c
+F: pc-bios/ast27x0_bootrom.bin
 
 NRF51
 M: Joel Stanley <joel@jms.id.au>
diff --git a/pc-bios/README b/pc-bios/README
index f0f13e15f2..d009c37895 100644
--- a/pc-bios/README
+++ b/pc-bios/README
@@ -89,6 +89,12 @@
   more features over time as needed. The source code is available at:
   https://github.com/google/vbootrom
 
+- ast27x0_bootrom.bin is a simplified, free (Apache 2.0) boot ROM for
+  ASPEED AST27x0 BMC SOC. It currently implements the bare minimum to
+  load, parse, initialize and run boot images stored in SPI flash, but may grow
+  more features over time as needed. The source code is available at:
+  https://github.com/google/vbootrom
+
 - hppa-firmware.img (32-bit) and hppa-firmware64.img (64-bit) are firmware
   files for the HP-PARISC (hppa) architecture.
   They are built form the SeaBIOS-hppa sources, which is a fork of SeaBIOS
diff --git a/pc-bios/ast27x0_bootrom.bin b/pc-bios/ast27x0_bootrom.bin
new file mode 100644
index 0000000000000000000000000000000000000000..e50f56eed9deeea65cbc1dd37897d59b1ff64ddd
GIT binary patch
literal 15464
zcmdUWeRNdix$m?0o`jG@3gios)$AnDOGgk80*TUQXA&tE+d@pO%a4@_Bs3T#gb9@b
zJ(*DNEw}0FVwwp-1QMt{$4ou<dTMA%k3B@{u^+hZK}2iqhm!=YJ+Y@MAc=}|f4_I{
zJs%Nm*ZuDfPWImK`#jJ4d_T|o?$IZbI|bhie5c}@iLc<pYZkty1kbL()xxzDS3j;x
zaSh_S1lJ*47vUPkbs?_A{r~CHZW=GdVzB}N|3{_lg(8pjy-H06bdTIwCDz0Sk>hoM
zc^@#n6~+s%;Om#d7vIM7QBCgjN#9wYCC^p(i#u->3Hx-p(-N8J{jBjqFYsI(0nV#x
z9hsi2pCFXSkG0ai23OkiP0qEV#CYM#t|$9-@OQ<K(1qKz{!4f3{g(`hykltIe@~O}
zkS@n_M9u`Zc|$P3neahxX!L@FMvUCZu#xYL8hMeDDS*@DI953$k9nQBoXD3o2~+k9
zw^xF$um8%fryo6uHHVD47cXFqH^B2~g*DhdU0ORtYQgECWDMPP5##Nqv_6F~EyMb>
ziLpw2&7eg7f;oHboQPV_ygqa@)_D!%wmd)S7yq};`VSm?+m&gl44tK1ZTi~K<qbyv
zM6;*=y+B%j>~k{VQ9R0<GI!NP6wlETJ@g*NG{{nP@%oTjBfK<JjIpSZ6Io$M*wm$T
zeY!O`jAtMCQZRi>L)QX^I_lW6EJWBJLodUIc>Tusas{)uG&CJB^#6d*3bpIGWA#^J
zkQ;jY&NApNCsMDkeXay{PY2vA=uXY;TN=7Q7VDuNqsFWVb$NM1738Rf9JfHPzl!UX
z>lS+?3>$hC?)lPCbcIZGi^D^-Ec6O=HEiU0X?Mb}ft`m}SQGESCWcO)^Spf`%Q*K*
zht`|G;d20dj00{6zN_fJUmAiRL|%q}o>R6F`h{(qrsbh?7?0-Yp;6#Om&ycj^fSwW
zlLz0)iG<T7d>L~=k%5tKS%Y;Vt&b^RfKJX6E<f^T(9M&a)B4YgJL9w$uL%7Nw23nc
zALRIK(6)ZKBJ>kyJi5#JQK2DYm$K@AlxO619^Z4|pbL8)VEs3jg|HTA1Yyg92z~hr
zkbe&5f~U(uKfv5&%w5)J{jl#stk$)w<=8dq>J)Z82w0_$#W#n(<>+b%z7n^?>hcin
z8NJ!ofjn9sidM+PCD6F@gV?|@?5glHR))R>TK^q1TpiI)Bd~*Oql5HCO_q%=wLYXz
z3;LXJ^cjV23Zc&hn$$*bwkD{ft)T7F{U?V%*B@w)_dbaATm-)5fjw}I>l?PL2QGEm
ztIOCX*eD9UhIQ$Tg06~}1LdIrc;lM0a>55`n-U}EG-=Q->ws_DOXZFT^?#chqyDdj
z{&ORL0X}uV3ga(Q2Nl)?>EFw7O-i|b4C`OS+86cQ&hVa|gCnqAoX->GA=H%cFmRPV
zgEuK3I)&%DxyB2cWV~5awrWD=$jp+p5?YU(GZI_0+!PsPUUFf~vh)cp+ZrsEYj4!E
z1=nDW4w#V{!I?#(XBK*7u*f4fYM2|I9<0$aWDvQohBWLv^l$xc{>jCf9iLq_ft)^&
zgLT`z(h6PH_G!TyQz~BM9CYtxe7|XEkt@4SsdyH>@I{Wf&k5QCaoWtIQNswSyj`Lp
z@1!F~=*ZQ<nme@MS>`+XTr@p6GrCI#W9{I#GoydBYij?i4`=rOsZJ*Rx>YXFE%OR&
zp!i^Zxw7locwQ+COKWAm)cVt{^1>l$^&3(fNS6?NvxfMEk87<*J<dGt;rnXdgZUB%
zMy|w`6+Vl(G^yQ-dD865!F8Y3+BXgQ@C0lA1^I>akR)gPFWfI?uFSO8se--~uS2p<
zgB~m9O6&ahW3@gpt|KoKurXohVm$-jQ9N_~E4UU$wN?$eHj0=T-enCw?U7|2Y2z!j
zl@cm?^BdnAMsCtjlVcmS@(Pi+tPE?XHWf7~Jy`P{@G%X15RdprwN$(U;Ax1_%jxo5
znMXp=`*xK1q0g|cWs$E(v3{{BGe)3)=C=hOfDhzb87k_03v+Ft&-my4hWBEnivEHW
zz6M{$TnT85f<`-N-1csQ#@R`8lj)n;*7u6uuYNw*<S+9#%`1x<X%Whp=8@+-v#g!j
zG1Q>lR+mQ))}%q+4v{r=qBY;o`U@D!^1zVU8O^uKf)lZ2tHCdI>Or1-@Y$82QTUzc
z!Lwr+i^6{!n~KW*ihEaI^#=5nBu{|^2Y+IW|F|zLSW^mGQKR5A^=D+fc<?gt*9Y?-
z_6^C5;-=Cv=Aa_#)G#7-bLFkqp}WifV0HD~_dpr-JrDS?bXn2?8Ge8@ll8nAzTxVb
zzJPTi>=RIPW)uMLA>1?G;rj#33&kr}O&n8o&6+S*rX#o7@oeJyfdk82T++0@482=v
zVN-D#W75FY2Oi%Aj2rv#Kf9;Ok_pCWyXa$n=+kW4P-a1&e%<JV-%{=xBo5VET*$H}
zl4IR}UrE^p(4}9H&m#D|E><~h69lb2-~o1Q73n=lJ_NAiNwCx%WH^gnv%DQRGJVfM
z^7+kJtd#JiiT)%~jQRDTWs02UIQ9KrlDr1wor`a~c$%5FNMi$N_{Fovr9*k^#2AaB
z)}%qkx%l=<nu2cx{CUt<^S~F(72zwQDg06RBW0w%*$a~|_6x7U7hW~=hy|OlU#<tA
z;J5TVe1YrHCmumROdjezRylRmp|fvEj|{-R{#fidVLOPY`kC|yb4mx`X!Bl?^7AU+
zKJ_SRokVZy(&IS$aCfcCxVLS4Qsv%1InSdC|78Dp@XLJvL(m@vE!d}%aeZYMX?qlH
z^6AoW;a>&*Ymo6UV2MMyUV$F{(9==q=qcrAC1s>RTI7**v?kj(E;;(z19@PV=aBEt
z8chvl^YE<`&lqUdJ_vc+VG9c};+Hg~bL!glX+yNr^iR;BZJ(om)2EL?e_V^U^f>F$
zhKx1(I(c>N#e2?GV!g>Y$#Fp+R&~*d3(O(@b0U*{-}M3X-2=Mloy4P!8gec8mF@rJ
zxt_TTYjaKF#C;yQ4l%l{P_#Ht0zHrycb`EV*B-<pU)UirhI;BUthY7PGxqP_|3h!h
z_qCsHLtUkx_7sZITn}AY=pV2j=|L~h+7G!czl6}M_7ql2>#dkKwcyNb)Qzg*U`^Er
zKiEb*^Z>QiK4A~u6GRUeeW370#Bg&|f-}v1!P&_+w)Egk>Q_LAd=CM}jc=|w6|@R7
z5dXYd*h6`+Ur56@*rD=4&<LKTjRoVR<H{=Fo3j2JcyQNa?xy|<A@^{G)Vi>)UXDTs
zN1=m%p?uIk`W+?zKjRs(>Qy?JV(S3)PU&Dip4%bYXgYdD=q`$J`n?4m_u`(q?X}FY
zUxNQW)Ktro5OQ)4^|8mac4lC_9dcQwgkFb!zn3)kE6h=6*FwLn&xqw_@?q|?c2>bB
zsN?^EarnUXr%*F0p!;v)YAu#fa1#8_(HB@j3GH;?eRw83>C58Y57~r%<JyEarcGRZ
zF>fh90?!*k5AtFkrd|K)ir(28WPBCzoF@A9s57;G=;2<#oCNRerv-k@KEuCOLSMl+
zZTe@77X!YSu?v~jVZ09Tk3yCr)a<#|+)m`-PS=iZ9J~&FyF86R4)UaHGWaL(_h;<w
z_aOeC2QBPn2N(~PkVC`#r)J0u&EHp(iG3~Y97GJ%T_de~p>NIXtKrxX?=Ab@uS=~L
zeHqt}8kydcZhi_n^9t{14fE8nk?t+Vx=~|F<QdVkp5Xe}`(XWY6ZPbjOkFc<O!ad9
zG0a(4Vgt`)%Zz8*J~<Qn;X&qH(%?M#|32~Gj|~{$pK-JSJl9M5^~H;=olWnd_Ct@K
zLeJy@T;WP-&469o=Sb_X&=>q%j5Y1>bwBPuwOD3wzhLXj?%Rh8W$-BWW`63<>Bp(t
z5$LYYW4&DodyE7mgq}mi|5LC>w9p!)oc}L$x=1G*XC7Cf^B2dbV&Lfw*tJq0U;bZM
zdkpK17}FwA;~MOLvb=jp1Np#$O$yN)M|nk_AWiH62Z9>TS3EKUd1o*Ue)crrqo6qk
z9}QlKEsugmvaH$Ae+O{T53K6|PRIMP<y`X}$jIwy=%>9%mQ;d%uO@Fb&*;62eP0%I
z#&eovJy2KV|1{T>@fE|suK;i8UG7R=6EUE2o{RfeNA|aYZ_;-_W5k#VnLiOZ0b0x<
z7UGJ!FGMf&7sLx?9;O~)t7O_5XtS@eY5y#~hgya>3ZE)DjlS?X_G-D3+i9PJ>5;Et
zj^{MYwK<4Sdyj;ArS@e%kJsZ7>`}3=246XmIoLy^ewX_1`&wD>gV-|e0jL+|*JJwl
z3VZ!T&*1bq-g|*ZS%Sz5U%`DLYIYOWMGp238y?V3kK|$;I#Tg=(up+-a?v+={hRd-
zIhOTJ<;u--Zf`_>WZh>@M!%!-)q^;5tUpuKX^Ga^Yt0)Iv-e<fe!==l_2kL89(itX
z0Xn~wgL91wJR{_}OpcfP`rlC=_;(ccxljRH!9OdpZo3YDMeYgwC^-944)^-@J~;WT
zun^~ieTZG?e<u4d$sT%H<zU1+uaoUb8(u`5kdIQ>Zw};wUYFP6j47zgpt7UA)`#rq
zFP6~bfba8}odKL}oI`%{<E-yhj1iYT;pFwyT1n@oV^?B5v{^KLd_~`l5;_Fj9LdkB
zlbq&$)WT}XS;Mm?x2A?M@52~#Yd!YG`>`+WtC7%mG0qx5-|O2Wp>N~)6&<w^IoXi>
zHSEXN?|X3e6TYQ5gUP`eOt9uhSQ~rqfi}o0I5Q*9ALX-^=u0Y9UxIb92F_5{{1|%T
zS}j}~{FGq47uP={PZmR$&_lr<*y|#Ca_W(1bL(Lj_7@|L|Lwt=+>6uSb-<t(zpBnI
zpr1=VZ$1p(*$03R_)K1ewGQze=|k=k^#9CnkY~d5SQG8I_v0SESi@MPj8}Fg;?eK$
zLLON2XtQ(`H+KK-){!#M8G<aY0p6}*>G2vSSf>&5=$i+gLEgOZ5puXP2x@*2;Fm*B
z@IfU*6J!cv|Le;1RmLx#pNGw!&~rLz=fF$}<p7U0>X(QQ+Hx3s<L|z&dc=Vo^sQmo
zljB_+`xu)tmf9TKFjff5y}IqQKTU~MH^;qW7+#(eVejqb+4#-inf&`<iwj19cOsyL
zUc#RH-3?kOcDD>h@3sat$??|1SD14H$9nH_d@O!;#IxxuyPk>XN0pONk4A9L=jH_V
zjy(G{(Sr=vS%a_WTJy8;w-KC$Fqi%svRc?HU&eV#0nUy%PQTDHf;HoqLykQj)3sLg
z0Px)$?}#xQ^SO~5p=0vT^TQ9vDsvQiQ71oKg3O-<EuOtIC-Q96&O^B=xdA!Bq2-+G
zIs2Cj(3PrZ%HFB_m=hnw3MTJsY<^N{T>Kas|MZbG29+(}yIv-C(cheRH$20?WOyR9
z=eUs;xolh)p-m?s@4HWEq5pu+shcj;?k}gwahwyL%||bg$kq9g$Dlvva={s}WU$AW
z9r*#qSZjF}JA`|SHfym4q5tM#WwYyGvuh*w(k`&sINk@@K4I%I{=VXRo^kvl?iaQ%
z<VSw$@bxnEoMXr!_E`m8@a5m&`UHG9Vat>K@g5uf-7Vln^>*L^XBY$Q$1R-GGM14W
z&YpB?q#fJ#9={Dbwe4`S|24>|`gPEv-?+3I99qGTq?H~?bLb<#f4rZeHXz1e7tQ<q
z_gG`|m9Np~l)u^Mt||WZ(j=O%K7sdhj?Z!b!n`<yyy!>%@Hl+G4jn`n1qWrZgt(7`
z9cqy)2bXgmeJ}SCMX3MuNuJXs=gT7Cuy?Ta@~=sG0`*++ProuEm#>qF%kZTU+!uo`
zd4NAf9^gKLzOqyc&BnOvE2X%PB2R;t<J+0TzGMyd!Jf2@o3}4tN1yojxK9j0rvE@p
z&=#v9Lm2t7H`97M3jY15#kwO8_e_4Dg#HVB`)`!c8*%;<y`9~YIPsojhswXx4%nwU
zI^la#dE7VuGivYb_Bec>#Mq+_F2cT1*aki`fcF-I73f=N1BEyJGk6Eaekd0)loNqJ
zGq=Uzh9Qg62l^5b3;t`rEB(b4<Pg+1=&Oc#$sNC1--Gp4J!KAHuapNIH@;Yh-#~0K
z_D$w(k1Xi}Z^?N(*&kf{Jq12bqUShX9GuOb-l;7$?1Q>fYKx<D<)d0;82$<$Mr}oJ
zz6kwZhAe@+wa+7GQx-cfChE1IzWPCreS_fK13hY7?~G6CnDSB2n#jJQ)t?C|8yLFx
z9-N<==&Q-|dGwvoGipIDa(qr?QL>(${egya_o6rJ$~w@aYuI~MJc%9@`UhXOPhM5e
zi9Q!SuG6>ZUOOHnvIYJGU-JebFZ89}xrJc^CkNVfQTch|9k?IwIgX*O7GnMFr%F2o
z??qVm$+rc1sI711Pwzem8%9p)>=_Fp{|b2+`zH2ge7=BOg7=kSuHCNZjqO1#d<XCz
z<n$M@?>MRFk43Q;9^yM1tmDVN^_ZT2+FmC=@+oJX9JNlacY~h4hI<)B8!<?_7WRh!
z2ln7QCHLI(%C5xwEjz|E;AoNNcgbH4@&jvyTSM*lSvY%jY<hLvdp-vD8*$v^*iQ0?
z%?W>?-?4Y0KhPH#H?BW0mUu?$`nsyGc6^<+<168lDYaVppW5FjIhTA4uZdc9A}Nnz
zKa#XZu=_1T^;`Tc!XH78u}S~I`;6Ajh?U#p{$uZn;x$yg^Z#W$Uq&xzp##{vBIax6
z;Cn5;@6wO(PAfW!cY=8TALDx?_@)2Y9Xp@pc^uwlp%?Q`g`ae^TV3zNcG%}XhZFv?
zUie7|{N&ISS?9yvzfkn%G3=)`xwg6JOz-S+L(3|}eLefRrF^f<Jzid<9R4!WZsA?7
zMEr<z^jY3Bu-n}-C#&5kX#P*cSn$-!&hjZTV>iA&^ewHB`{1u{E^F8Gn=2tR`66xa
zIL;KNV88gA-r0u?Swq=+f4#EoNzfa?yEAvqpTK{qw_jnr9q)ng!lSl*Ce9?@iPheY
zUVwdD0D4ULUUF^Iy|zs>FYB{y6N&6{Y~hD3mLXSSZ6}Tsc1W&Q<EVM;Rk;6(eMtr@
zkY~)kFYn}81kN~?d>eW~Em%T+mQL?dd6adqP;#3CGdpHiNM7?(fV~5CV+c8r<KIRc
zk5E?C^A<$D0XgZ<+;35y-{b1~CvtlYbL411Lcf6TcVHd%<z`=SHqTjPdI!!h?HEkd
zjYJGGr?PgTe)ga?tk7X|w@xu$c((82|8K0zG50w^@56hGci3Ya9<SYtYTnDx@kEJC
zlsNk3T6piN@&xPCct8tPLC!(sMA~#Ibe~+Gl>e8o1}63uqtK6i_F|ti2zmoyUB!Na
zdOZrg+Ix<I$d4ybo3W3ueJ3}<HUFFAKit>a`b^{(yS`{%#KpRbY*{iNdvlfRK!^NA
z;UA;{yW`zNZY0;Ch4JO~9>P7#NaPfEZJrSnf<N@qYVSfie+3UufEIgSo0pWD<M=Ay
z_v;b25Bmgo$wlo%&G&|(H^$L@keM}hhU)A2TMM2`6-2%USU=u5tOuOhM*-#^G1iAO
z4o&h-<D6<;K-bpfVZX_q%EUZvO&i!fTB3F(^EZl`tM(r_Z|Q?v(Ess^=znY-*uN)8
z?prQZ1PAGJd6L)3n!>tu40<@K=bUEk`!0B5KM*)&c5<!>^6;JQJK*UYeEUD+`bosH
z-J=?jQN$ws?H|C0Nj?x~DK%LAt|8HDk`L}_el{NKx%8#rAYN+E*sdq|!ux|-@HmsU
zarxWm@V5i}>BwP)M&20v)Iupx{>I;;P>$Jv!`^fmdA|TZWq(Tk6S14H??uC->I3qS
zm-_r7_=l~Iqu&~cLYG101>~Vg=Z^_{B<qwq`zCd!&em;za(Q0?o;1ifTp@$?r;0ie
zBV+#)u*=X-41qVkAK#C@=p4?~x#oF1qmNj}o+qJqJ4QyIaQ7rSVoCP-`aTmvU!u-a
z*z-;9@rE(RJ<qMkGt5uXg5KFYqnI3bPCsFvBf{4Uz;jgP3OmLbE9FiO5sWAHuDKD`
ze)hJ_kSX{<tQ2-oz1QU0zH<AP<}ED`Z27EgZmDmOyXOV&HRlCxGPiDP+1j|R?d#^2
zmNs)^%l0h|nTg3wTfSDmc~gVAxpB)^+nUVz^BT<cUvFy+EHE2)Y;D}o*4SV!Gh4Qq
zWuMLzv(Ch)PJQfa^54nhYK{o*jSb-bwNdLp(jLj&ZSFSBMfiuYPpiN8IDZvP%7|(1
zUPNGi_;<HLt6OA4g1|kCNO=*#Y(z!gdBD(B{?#Ro?dF5x3s*9B4{0hautz}NgI|{r
z$#Zy-O(prS0;yYIDv}CDVL8-sF?Zt+COC)xcuyS4g$rjg6@-leP5@#5wPyjbyKc8V
zqHy_#$h<-lg`ghnTTbA=-E}rfX67A@+X9<fw%oMjhQ&85&dii~0a-;iF>Tjab!)A;
z>Hhk!Hk$Qq=HeamHt#U!2R7|&yonDxY<GcoEK)E5`C`k5@RZY;o3@zS>bHQ5O=Dr4
zR$Jp%MQig8kO8_w3I5v9=sN87^(`%Jw%az<2QX#Q$<6wPhHZ_3K#}9o$zu-5OlhcZ
zt53n?Kng}9WOXRTu~hgF@+Q-b3DYuLHf{_wI$B6tbJLcuCZO6{9La9D;RaJ0TDCL-
zQt^LpA_S=oDAu77Xx;=BKqxMlDfMj|Wn+UQZnCHg%#HP%H?ObX&}?Fr?F*?C6WphA
z9jBJ84znIYf|g4lQ*PO|t!3Lyrth|;OU(JT+qMT7?5j3yyZ?duZH)^nKb>H9f!S2Q
zxeY?zkSUp;-+upA$_oZQ8rAg_c+=t~H!fXPx_m`hdHwng4UHRR^QJA^cP!Z21co*@
z$|9tn`aokq;GBW4-@m?PbD*wHHniNowZ3iBdH|$+raMpC&KmLT_@s7Sb|H?2L}uaZ
zIU9?e;e6%7%4yc&j3ZO3yVApH-Jbo%S2F+jhZkOa{@gUq;dlB{jW2$uiC-nhVkS10
z3TK~oc;=BA)m_uWpIFsBZT~er*$1*7$~-vr;fzC50-*4_-~aaI-@KGUhx~-`-GL3#
zTwM9#7!K-WFUGhIKjiUAd{1BudwY95^RVZL;p@`F+803G<<95c;>Sez5w}PbIBN|D
zXGl9d>qvpGD?gl9)t$RPXI0P412et^^i;-j0gm^ha`CsZ*aF<UAM#d>@eCYFy8QBb
zC&p%B>`nqwe~P!NF8wxgW~+_3LBOpBoJ*5q{9(BtWAllN562$GSR2Mn4&cMFCovYB
z1a}5wVT`R+P_~}VW9$sZs6+X}wCRWSBih2X;A5Kj7W%SonI?bu<3jSNW<*SY%|fxR
z{#`7#j5w7GefHevY#a=k_^t#@EnsSKuVhsCUsQzY7UU7<HS!=}RuTpuj<sRzF^svk
z<k((}{T^e<Iy{82H!!xCh{-y<#Ze)KiTgC*E&@*R>WsaFv0OA!$vS)kWAibV4H$k*
z{(&Eu7#rYyTn8IcWS9$>#{onC;m5?c3}fdo#`^?bHT6@u5JOz|9>AOd%t{Rp)vkVZ
z_#=U;97EXj@LN}j{vj6o4nPz1>f-cF3_k{#)qeyZq*vwAyUQ_TWfo!10OlV2aA-B|
z6&>Ywx5VWk40<OC{tcK}9+5`?GX(eqoqJN~tOSgO!%St{xP5L-f$;<8PQawb=(;!z
zZHwQY$v(jBaMxOVIQ>YPugeoQR(0$9HBwAY64dPp;G6?a6LI3YSUXv@%0JEncKUFt
z-N!MOfw4GW%6AidF&^jO2M`uu;(Vzb=wlA7>h=)!1vkg2b#4RvUchINCw%CeevIu-
z8YA5{jBUl(RsvCP33;o!_0Ol!rG6d({NsS1PrIkW`>3D$Ch3Rkz66{{a5&)lI_bQD
zu|pV(%agM1e(m;2bWGrw>F`V7ti?U_?B)SLGI>;Hk#03$vPWaF6NI@6fBs2y=`TBg
zvky4qE{^|j_K_@KS7vysuRCM^l&YTe18Kg8JO>SOm0Bj0{eiz48;{#u{Ddsk-TJL@
z(@@{%0ow%FY8Te66Q2WVCl|9wFe(Yp{~>Mn-wAI4z8diR03VlS>)~lfuJJwWsp`rO
zXLV=ppIX(EabQZ-L+J<88nEQ=QpHiZVlQY^<A5P9kMc+Qg01ID`$oYI0JZ_J$|ltJ
z)nKa@3T24`b}wM#IXXp8`?WQR(2}`;pAP4J4DfBZPuQ}m%Tt@s74vKv;Kl(r<if2z
zoOxubuPY-w#n+v_Kdq|Aa|bYflN`6?XDjf6*g*b0{FDK{5Acrw{(!rV-G_8Tw+O>8
zV~KbSBKbP^fTdsl9=L+T=)Ep(LUt85tn$hHDfw~OSqq#yC*!P1$Uh%*Wq^AynRX&K
z+|up&N}LtyssXT3z~cYwvUQq(^|6`q^{B2OMOWnI5b#dCJ~_Xt{ss<iryjyR4Y+6j
z1{`(y2KfNoR@^6StMcqE-8u!lx-PlK{7l3J4i-~$eJU^2fLjZ=`G8{&qt*y&ho>L;
zgs*E__!?h#_WrD@p3DPNs~*ZYIOXB=Luu^OKGX%0*8`x_hQow2xL0&O>|X~AvYgtc
zi3#v?z|FvqRY^zTs(AmrT^le&-xvqX9Kftifl=}>=-j*>?;R=k0yMg1!1<+%bG4jR
z^^mFzzn#PveSb6Xhk*Yb7hm}!Vk>NP>or(4u6~=xLx7)y1ED?_ev;o*_2~B|g_w%>
zGr-vkoL~x$)3-C<C9Of<lx)-rzz@0bO3zh{V<(Ic7^{JnEDrTo20TAr_MLF?Ch^8L
wcXDiP2F`ilq{cqv=K@Y1W^9E4HywwIZ7%IZ+@e0##%18N``L#a|8bxH3u>WpivR!s

literal 0
HcmV?d00001

diff --git a/pc-bios/meson.build b/pc-bios/meson.build
index 34d6616c32..83998f6071 100644
--- a/pc-bios/meson.build
+++ b/pc-bios/meson.build
@@ -84,6 +84,7 @@ blobs = [
   'npcm8xx_bootrom.bin',
   'vof.bin',
   'vof-nvram.bin',
+  'ast27x0_bootrom.bin',
 ]
 
 dtc = find_program('dtc', required: false)
-- 
2.43.0

[PATCH v4 06/10] hw/arm/aspeed: Add support for loading vbootrom image via "-bios"

[Jamin Lin via]

Introduce "aspeed_load_vbootrom()" to support loading a virtual boot ROM image
into the vbootrom memory region, using the "-bios" command-line option.

Signed-off-by: Jamin Lin <jamin_lin@aspeedtech.com>
Reviewed-by: Nabih Estefan <nabihestefan@google.com>
Tested-by: Nabih Estefan <nabihestefan@google.com>
---
 include/hw/arm/aspeed.h |  1 +
 hw/arm/aspeed.c         | 36 ++++++++++++++++++++++++++++++++++++
 2 files changed, 37 insertions(+)

diff --git a/include/hw/arm/aspeed.h b/include/hw/arm/aspeed.h
index 973277bea6..2b8332a7d7 100644
--- a/include/hw/arm/aspeed.h
+++ b/include/hw/arm/aspeed.h
@@ -41,6 +41,7 @@ struct AspeedMachineClass {
     uint32_t uart_default;
     bool sdhci_wp_inverted;
     bool vbootrom;
+    const char *vbootrom_name;
 };
 
 
diff --git a/hw/arm/aspeed.c b/hw/arm/aspeed.c
index b70a120e62..7f11371f02 100644
--- a/hw/arm/aspeed.c
+++ b/hw/arm/aspeed.c
@@ -27,6 +27,7 @@
 #include "system/reset.h"
 #include "hw/loader.h"
 #include "qemu/error-report.h"
+#include "qemu/datadir.h"
 #include "qemu/units.h"
 #include "hw/qdev-clock.h"
 #include "system/system.h"
@@ -305,6 +306,34 @@ static void aspeed_install_boot_rom(AspeedMachineState *bmc, BlockBackend *blk,
                    rom_size, &error_abort);
 }
 
+/*
+ * This function locates the vbootrom image file specified via the command line
+ * using the -bios option. It loads the specified image into the vbootrom
+ * memory region and handles errors if the file cannot be found or loaded.
+ */
+static void aspeed_load_vbootrom(MachineState *machine, uint64_t rom_size,
+                                 Error **errp)
+{
+    AspeedMachineState *bmc = ASPEED_MACHINE(machine);
+    AspeedMachineClass *amc = ASPEED_MACHINE_GET_CLASS(machine);
+    const char *bios_name = machine->firmware ?: amc->vbootrom_name;
+    g_autofree char *filename = NULL;
+    AspeedSoCState *soc = bmc->soc;
+    int ret;
+
+    filename = qemu_find_file(QEMU_FILE_TYPE_BIOS, bios_name);
+    if (!filename) {
+        error_setg(errp, "Could not find vbootrom image '%s'", bios_name);
+        return;
+    }
+
+    ret = load_image_mr(filename, &soc->vbootrom);
+    if (ret < 0) {
+        error_setg(errp, "Failed to load vbootrom image '%s'", bios_name);
+        return;
+    }
+}
+
 void aspeed_board_init_flashes(AspeedSMCState *s, const char *flashtype,
                                       unsigned int count, int unit0)
 {
@@ -483,6 +512,11 @@ static void aspeed_machine_init(MachineState *machine)
         }
     }
 
+    if (amc->vbootrom) {
+        rom_size = memory_region_size(&bmc->soc->vbootrom);
+        aspeed_load_vbootrom(machine, rom_size, &error_abort);
+    }
+
     arm_load_kernel(ARM_CPU(first_cpu), machine, &aspeed_board_binfo);
 }
 
@@ -1691,6 +1725,7 @@ static void aspeed_machine_ast2700a0_evb_class_init(ObjectClass *oc, void *data)
     amc->uart_default = ASPEED_DEV_UART12;
     amc->i2c_init  = ast2700_evb_i2c_init;
     amc->vbootrom = true;
+    amc->vbootrom_name = "ast27x0_bootrom.bin";
     mc->auto_create_sdcard = true;
     mc->default_ram_size = 1 * GiB;
     aspeed_machine_class_init_cpus_defaults(mc);
@@ -1712,6 +1747,7 @@ static void aspeed_machine_ast2700a1_evb_class_init(ObjectClass *oc, void *data)
     amc->uart_default = ASPEED_DEV_UART12;
     amc->i2c_init  = ast2700_evb_i2c_init;
     amc->vbootrom = true;
+    amc->vbootrom_name = "ast27x0_bootrom.bin";
     mc->auto_create_sdcard = true;
     mc->default_ram_size = 1 * GiB;
     aspeed_machine_class_init_cpus_defaults(mc);
-- 
2.43.0

[PATCH v4 07/10] tests/functional/aspeed: Move I2C test into shared helper for AST2700 reuse

[Jamin Lin via]

Move the I2C test case into a common helper function (do_ast2700_i2c_test) so it
can be reused across multiple AST2700-based test cases. This reduces duplication
and improves maintainability.

Signed-off-by: Jamin Lin <jamin_lin@aspeedtech.com>
---
 tests/functional/test_aarch64_aspeed.py | 28 +++++++++++++------------
 1 file changed, 15 insertions(+), 13 deletions(-)

diff --git a/tests/functional/test_aarch64_aspeed.py b/tests/functional/test_aarch64_aspeed.py
index c25c966278..441f7f3919 100755
--- a/tests/functional/test_aarch64_aspeed.py
+++ b/tests/functional/test_aarch64_aspeed.py
@@ -18,6 +18,8 @@ class AST2x00MachineSDK(QemuSystemTest):
     def do_test_aarch64_aspeed_sdk_start(self, image):
         self.require_netdev('user')
         self.vm.set_console()
+        self.vm.add_args('-device',
+                         'tmp105,bus=aspeed.i2c.bus.1,address=0x4d,id=tmp-test')
         self.vm.add_args('-drive', 'file=' + image + ',if=mtd,format=raw',
                          '-net', 'nic', '-net', 'user', '-snapshot')
 
@@ -35,6 +37,17 @@ def do_test_aarch64_aspeed_sdk_start(self, image):
             'https://github.com/AspeedTech-BMC/openbmc/releases/download/v09.05/ast2700-default-obmc.tar.gz',
             'c1f4496aec06743c812a6e9a1a18d032f34d62f3ddb6956e924fef62aa2046a5')
 
+    def do_ast2700_i2c_test(self):
+        exec_command_and_wait_for_pattern(self,
+            'echo lm75 0x4d > /sys/class/i2c-dev/i2c-1/device/new_device ',
+            'i2c i2c-1: new_device: Instantiated device lm75 at 0x4d');
+        exec_command_and_wait_for_pattern(self,
+            'cat /sys/bus/i2c/devices/1-004d/hwmon/hwmon*/temp1_input', '0')
+        self.vm.cmd('qom-set', path='/machine/peripheral/tmp-test',
+                    property='temperature', value=18000)
+        exec_command_and_wait_for_pattern(self,
+            'cat /sys/bus/i2c/devices/1-004d/hwmon/hwmon*/temp1_input', '18000')
+
     def start_ast2700_test(self, name):
         num_cpu = 4
         uboot_size = os.path.getsize(self.scratch_file(name,
@@ -73,8 +86,6 @@ def start_ast2700_test(self, name):
                              f'loader,addr=0x430000000,cpu-num={i}')
 
         self.vm.add_args('-smp', str(num_cpu))
-        self.vm.add_args('-device',
-                         'tmp105,bus=aspeed.i2c.bus.1,address=0x4d,id=tmp-test')
         self.do_test_aarch64_aspeed_sdk_start(
             self.scratch_file(name, 'image-bmc'))
 
@@ -83,28 +94,19 @@ def start_ast2700_test(self, name):
         exec_command_and_wait_for_pattern(self, 'root', 'Password:')
         exec_command_and_wait_for_pattern(self, '0penBmc', f'root@{name}:~#')
 
-        exec_command_and_wait_for_pattern(self,
-            'echo lm75 0x4d > /sys/class/i2c-dev/i2c-1/device/new_device ',
-            'i2c i2c-1: new_device: Instantiated device lm75 at 0x4d');
-        exec_command_and_wait_for_pattern(self,
-            'cat /sys/bus/i2c/devices/1-004d/hwmon/hwmon*/temp1_input', '0')
-        self.vm.cmd('qom-set', path='/machine/peripheral/tmp-test',
-                    property='temperature', value=18000)
-        exec_command_and_wait_for_pattern(self,
-            'cat /sys/bus/i2c/devices/1-004d/hwmon/hwmon*/temp1_input', '18000')
-
     def test_aarch64_ast2700_evb_sdk_v09_05(self):
         self.set_machine('ast2700-evb')
 
         self.archive_extract(self.ASSET_SDK_V905_AST2700)
         self.start_ast2700_test('ast2700-a0-default')
+        self.do_ast2700_i2c_test()
 
     def test_aarch64_ast2700a1_evb_sdk_v09_05(self):
         self.set_machine('ast2700a1-evb')
 
         self.archive_extract(self.ASSET_SDK_V905_AST2700A1)
         self.start_ast2700_test('ast2700-default')
-
+        self.do_ast2700_i2c_test()
 
 if __name__ == '__main__':
     QemuSystemTest.main()
-- 
2.43.0

[PATCH v4 08/10] tests/functional/aspeed: Update test ASPEED SDK v09.06

[Jamin Lin via]

Signed-off-by: Jamin Lin <jamin_lin@aspeedtech.com>
---
 tests/functional/test_aarch64_aspeed.py | 20 ++++++++++----------
 1 file changed, 10 insertions(+), 10 deletions(-)

diff --git a/tests/functional/test_aarch64_aspeed.py b/tests/functional/test_aarch64_aspeed.py
index 441f7f3919..337d701917 100755
--- a/tests/functional/test_aarch64_aspeed.py
+++ b/tests/functional/test_aarch64_aspeed.py
@@ -29,13 +29,13 @@ def do_test_aarch64_aspeed_sdk_start(self, image):
         wait_for_console_pattern(self, '## Loading kernel from FIT Image')
         wait_for_console_pattern(self, 'Starting kernel ...')
 
-    ASSET_SDK_V905_AST2700 = Asset(
-            'https://github.com/AspeedTech-BMC/openbmc/releases/download/v09.05/ast2700-a0-default-obmc.tar.gz',
-            'cfbbd1cce72f2a3b73b9080c41eecdadebb7077fba4f7806d72ac99f3e84b74a')
+    ASSET_SDK_V906_AST2700 = Asset(
+            'https://github.com/AspeedTech-BMC/openbmc/releases/download/v09.06/ast2700-a0-default-obmc.tar.gz',
+            '7247b6f19dbfb700686f8d9f723ac23f3eb229226c0589cb9b06b80d1b61f3cb')
 
-    ASSET_SDK_V905_AST2700A1 = Asset(
-            'https://github.com/AspeedTech-BMC/openbmc/releases/download/v09.05/ast2700-default-obmc.tar.gz',
-            'c1f4496aec06743c812a6e9a1a18d032f34d62f3ddb6956e924fef62aa2046a5')
+    ASSET_SDK_V906_AST2700A1 = Asset(
+            'https://github.com/AspeedTech-BMC/openbmc/releases/download/v09.06/ast2700-default-obmc.tar.gz',
+            'f1d53e0be8a404ecce3e105f72bc50fa4e090ad13160ffa91b10a6e0233a9dc6')
 
     def do_ast2700_i2c_test(self):
         exec_command_and_wait_for_pattern(self,
@@ -94,17 +94,17 @@ def start_ast2700_test(self, name):
         exec_command_and_wait_for_pattern(self, 'root', 'Password:')
         exec_command_and_wait_for_pattern(self, '0penBmc', f'root@{name}:~#')
 
-    def test_aarch64_ast2700_evb_sdk_v09_05(self):
+    def test_aarch64_ast2700_evb_sdk_v09_06(self):
         self.set_machine('ast2700-evb')
 
-        self.archive_extract(self.ASSET_SDK_V905_AST2700)
+        self.archive_extract(self.ASSET_SDK_V906_AST2700)
         self.start_ast2700_test('ast2700-a0-default')
         self.do_ast2700_i2c_test()
 
-    def test_aarch64_ast2700a1_evb_sdk_v09_05(self):
+    def test_aarch64_ast2700a1_evb_sdk_v09_06(self):
         self.set_machine('ast2700a1-evb')
 
-        self.archive_extract(self.ASSET_SDK_V905_AST2700A1)
+        self.archive_extract(self.ASSET_SDK_V906_AST2700A1)
         self.start_ast2700_test('ast2700-default')
         self.do_ast2700_i2c_test()
 
-- 
2.43.0

[PATCH v4 09/10] tests/functional/aspeed: Add to test vbootrom for AST2700

[Jamin Lin via]

Add the AST2700 functional test to boot using the vbootrom image
instead of manually loading boot components with -device loader.
The boot ROM binary is now passed via the
-bios option, using the image located in pc-bios/ast27x0_bootrom.bin.

Signed-off-by: Jamin Lin <jamin_lin@aspeedtech.com>
---
 tests/functional/test_aarch64_aspeed.py | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

diff --git a/tests/functional/test_aarch64_aspeed.py b/tests/functional/test_aarch64_aspeed.py
index 337d701917..85789c1b1d 100755
--- a/tests/functional/test_aarch64_aspeed.py
+++ b/tests/functional/test_aarch64_aspeed.py
@@ -94,6 +94,14 @@ def start_ast2700_test(self, name):
         exec_command_and_wait_for_pattern(self, 'root', 'Password:')
         exec_command_and_wait_for_pattern(self, '0penBmc', f'root@{name}:~#')
 
+    def start_ast2700_test_vbootrom(self, name):
+        self.vm.add_args('-bios', 'ast27x0_bootrom.bin')
+        self.do_test_aarch64_aspeed_sdk_start(
+                self.scratch_file(name, 'image-bmc'))
+        wait_for_console_pattern(self, f'{name} login:')
+        exec_command_and_wait_for_pattern(self, 'root', 'Password:')
+        exec_command_and_wait_for_pattern(self, '0penBmc', f'root@{name}:~#')
+
     def test_aarch64_ast2700_evb_sdk_v09_06(self):
         self.set_machine('ast2700-evb')
 
@@ -108,5 +116,12 @@ def test_aarch64_ast2700a1_evb_sdk_v09_06(self):
         self.start_ast2700_test('ast2700-default')
         self.do_ast2700_i2c_test()
 
+    def test_aarch64_ast2700a1_evb_sdk_vboottom_v09_06(self):
+        self.set_machine('ast2700a1-evb')
+
+        self.archive_extract(self.ASSET_SDK_V906_AST2700A1)
+        self.start_ast2700_test_vbootrom('ast2700-default')
+        self.do_ast2700_i2c_test()
+
 if __name__ == '__main__':
     QemuSystemTest.main()
-- 
2.43.0

[PATCH v4 10/10] docs/system/arm/aspeed: Support vbootrom for AST2700

[Jamin Lin via]

Using the vbootrom image support and the boot ROM binary is
now passed via the -bios option, using the image located in
pc-bios/ast27x0_bootrom.bin.

Signed-off-by: Jamin Lin <jamin_lin@aspeedtech.com>
Reviewed-by: Nabih Estefan <nabihestefan@google.com>
---
 docs/system/arm/aspeed.rst | 29 ++++++++++++++++++++++++++++-
 1 file changed, 28 insertions(+), 1 deletion(-)

diff --git a/docs/system/arm/aspeed.rst b/docs/system/arm/aspeed.rst
index 97fd6a0e7f..c87a2cf796 100644
--- a/docs/system/arm/aspeed.rst
+++ b/docs/system/arm/aspeed.rst
@@ -250,7 +250,14 @@ under Linux), use :
 Booting the ast2700-evb machine
 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
 
-Boot the AST2700 machine from the flash image, use an MTD drive :
+Boot the AST2700 machine from the flash image.
+
+There are two supported methods for booting the AST2700 machine with a flash image:
+
+Manual boot using ``-device loader``:
+
+It causes all 4 CPU cores to start execution from address ``0x430000000``, which
+corresponds to the BL31 image load address.
 
 .. code-block:: bash
 
@@ -270,6 +277,26 @@ Boot the AST2700 machine from the flash image, use an MTD drive :
        -drive file=${IMGDIR}/image-bmc,format=raw,if=mtd \
        -nographic
 
+Boot using a virtual boot ROM (``-bios``):
+
+If users do not specify the ``-bios option``, QEMU will attempt to load the
+default vbootrom image ``ast27x0_bootrom.bin`` from either the current working
+directory or the ``pc-bios`` directory within the QEMU source tree.
+
+.. code-block:: bash
+
+  $ qemu-system-aarch64 -M ast2700-evb \
+      -drive file=image-bmc,format=raw,if=mtd \
+      -nographic
+
+The ``-bios`` option allows users to specify a custom path for the vbootrom
+image to be loaded during boot. This will load the vbootrom image from the
+specified path in the ${HOME} directory.
+
+.. code-block:: bash
+
+  -bios ${HOME}/ast27x0_bootrom.bin
+
 Aspeed minibmc family boards (``ast1030-evb``)
 ==================================================================
 
-- 
2.43.0

Re: [PATCH v4 07/10] tests/functional/aspeed: Move I2C test into shared helper for AST2700 reuse

[Nabih Estefan]

On Wed, Apr 16, 2025 at 8:12 PM Jamin Lin <jamin_lin@aspeedtech.com> wrote:
>
> Move the I2C test case into a common helper function (do_ast2700_i2c_test) so it
> can be reused across multiple AST2700-based test cases. This reduces duplication
> and improves maintainability.
>
> Signed-off-by: Jamin Lin <jamin_lin@aspeedtech.com>
Reviewed-by: Nabih Estefan <nabihestefan@google.com>
> ---
>  tests/functional/test_aarch64_aspeed.py | 28 +++++++++++++------------
>  1 file changed, 15 insertions(+), 13 deletions(-)
>
> diff --git a/tests/functional/test_aarch64_aspeed.py b/tests/functional/test_aarch64_aspeed.py
> index c25c966278..441f7f3919 100755
> --- a/tests/functional/test_aarch64_aspeed.py
> +++ b/tests/functional/test_aarch64_aspeed.py
> @@ -18,6 +18,8 @@ class AST2x00MachineSDK(QemuSystemTest):
>      def do_test_aarch64_aspeed_sdk_start(self, image):
>          self.require_netdev('user')
>          self.vm.set_console()
> +        self.vm.add_args('-device',
> +                         'tmp105,bus=aspeed.i2c.bus.1,address=0x4d,id=tmp-test')
>          self.vm.add_args('-drive', 'file=' + image + ',if=mtd,format=raw',
>                           '-net', 'nic', '-net', 'user', '-snapshot')
>
> @@ -35,6 +37,17 @@ def do_test_aarch64_aspeed_sdk_start(self, image):
>              'https://github.com/AspeedTech-BMC/openbmc/releases/download/v09.05/ast2700-default-obmc.tar.gz',
>              'c1f4496aec06743c812a6e9a1a18d032f34d62f3ddb6956e924fef62aa2046a5')
>
> +    def do_ast2700_i2c_test(self):
> +        exec_command_and_wait_for_pattern(self,
> +            'echo lm75 0x4d > /sys/class/i2c-dev/i2c-1/device/new_device ',
> +            'i2c i2c-1: new_device: Instantiated device lm75 at 0x4d');
> +        exec_command_and_wait_for_pattern(self,
> +            'cat /sys/bus/i2c/devices/1-004d/hwmon/hwmon*/temp1_input', '0')
> +        self.vm.cmd('qom-set', path='/machine/peripheral/tmp-test',
> +                    property='temperature', value=18000)
> +        exec_command_and_wait_for_pattern(self,
> +            'cat /sys/bus/i2c/devices/1-004d/hwmon/hwmon*/temp1_input', '18000')
> +
>      def start_ast2700_test(self, name):
>          num_cpu = 4
>          uboot_size = os.path.getsize(self.scratch_file(name,
> @@ -73,8 +86,6 @@ def start_ast2700_test(self, name):
>                               f'loader,addr=0x430000000,cpu-num={i}')
>
>          self.vm.add_args('-smp', str(num_cpu))
> -        self.vm.add_args('-device',
> -                         'tmp105,bus=aspeed.i2c.bus.1,address=0x4d,id=tmp-test')
>          self.do_test_aarch64_aspeed_sdk_start(
>              self.scratch_file(name, 'image-bmc'))
>
> @@ -83,28 +94,19 @@ def start_ast2700_test(self, name):
>          exec_command_and_wait_for_pattern(self, 'root', 'Password:')
>          exec_command_and_wait_for_pattern(self, '0penBmc', f'root@{name}:~#')
>
> -        exec_command_and_wait_for_pattern(self,
> -            'echo lm75 0x4d > /sys/class/i2c-dev/i2c-1/device/new_device ',
> -            'i2c i2c-1: new_device: Instantiated device lm75 at 0x4d');
> -        exec_command_and_wait_for_pattern(self,
> -            'cat /sys/bus/i2c/devices/1-004d/hwmon/hwmon*/temp1_input', '0')
> -        self.vm.cmd('qom-set', path='/machine/peripheral/tmp-test',
> -                    property='temperature', value=18000)
> -        exec_command_and_wait_for_pattern(self,
> -            'cat /sys/bus/i2c/devices/1-004d/hwmon/hwmon*/temp1_input', '18000')
> -
>      def test_aarch64_ast2700_evb_sdk_v09_05(self):
>          self.set_machine('ast2700-evb')
>
>          self.archive_extract(self.ASSET_SDK_V905_AST2700)
>          self.start_ast2700_test('ast2700-a0-default')
> +        self.do_ast2700_i2c_test()
>
>      def test_aarch64_ast2700a1_evb_sdk_v09_05(self):
>          self.set_machine('ast2700a1-evb')
>
>          self.archive_extract(self.ASSET_SDK_V905_AST2700A1)
>          self.start_ast2700_test('ast2700-default')
> -
> +        self.do_ast2700_i2c_test()
>
>  if __name__ == '__main__':
>      QemuSystemTest.main()
> --
> 2.43.0
>

Re: [PATCH v4 09/10] tests/functional/aspeed: Add to test vbootrom for AST2700

[Nabih Estefan]

On Wed, Apr 16, 2025 at 8:12 PM Jamin Lin <jamin_lin@aspeedtech.com> wrote:
>
> Add the AST2700 functional test to boot using the vbootrom image
> instead of manually loading boot components with -device loader.
> The boot ROM binary is now passed via the
> -bios option, using the image located in pc-bios/ast27x0_bootrom.bin.
>
> Signed-off-by: Jamin Lin <jamin_lin@aspeedtech.com>
Reviewed-by: Nabih Estefan <nabihestefan@google.com>
> ---
>  tests/functional/test_aarch64_aspeed.py | 15 +++++++++++++++
>  1 file changed, 15 insertions(+)
>
> diff --git a/tests/functional/test_aarch64_aspeed.py b/tests/functional/test_aarch64_aspeed.py
> index 337d701917..85789c1b1d 100755
> --- a/tests/functional/test_aarch64_aspeed.py
> +++ b/tests/functional/test_aarch64_aspeed.py
> @@ -94,6 +94,14 @@ def start_ast2700_test(self, name):
>          exec_command_and_wait_for_pattern(self, 'root', 'Password:')
>          exec_command_and_wait_for_pattern(self, '0penBmc', f'root@{name}:~#')
>
> +    def start_ast2700_test_vbootrom(self, name):
> +        self.vm.add_args('-bios', 'ast27x0_bootrom.bin')
> +        self.do_test_aarch64_aspeed_sdk_start(
> +                self.scratch_file(name, 'image-bmc'))
> +        wait_for_console_pattern(self, f'{name} login:')
> +        exec_command_and_wait_for_pattern(self, 'root', 'Password:')
> +        exec_command_and_wait_for_pattern(self, '0penBmc', f'root@{name}:~#')
> +
>      def test_aarch64_ast2700_evb_sdk_v09_06(self):
>          self.set_machine('ast2700-evb')
>
> @@ -108,5 +116,12 @@ def test_aarch64_ast2700a1_evb_sdk_v09_06(self):
>          self.start_ast2700_test('ast2700-default')
>          self.do_ast2700_i2c_test()
>
> +    def test_aarch64_ast2700a1_evb_sdk_vboottom_v09_06(self):
> +        self.set_machine('ast2700a1-evb')
> +
> +        self.archive_extract(self.ASSET_SDK_V906_AST2700A1)
> +        self.start_ast2700_test_vbootrom('ast2700-default')
> +        self.do_ast2700_i2c_test()
> +
>  if __name__ == '__main__':
>      QemuSystemTest.main()
> --
> 2.43.0
>

Re: [PATCH v4 02/10] hw/arm/aspeed_ast27x0 Introduce vbootrom memory region

[Cédric Le Goater]

On 4/17/25 05:11, Jamin Lin wrote:
> Introduce a new vbootrom memory region. The region is mapped at address
> "0x00000000" and has a size of 128KB, identical to the SRAM region size.
> This memory region is intended for loading a vbootrom image file as part of the
> boot process.
> 
> The vbootrom registered in the SoC's address space using the ASPEED_DEV_VBOOTROM
> index.
> 
> Introduced a "vbootrom_size" attribute in "AspeedSoCClass" to define virtual
> boot ROM size.

Could you please explain why we need a class attribute to size the
vbootrom region ? The rest looks good.


Thanks,

C.



> 
> Signed-off-by: Jamin Lin <jamin_lin@aspeedtech.com>
> Reviewed-by: Nabih Estefan <nabihestefan@google.com>
> Tested-by: Nabih Estefan <nabihestefan@google.com>
> ---
>   include/hw/arm/aspeed_soc.h |  3 +++
>   hw/arm/aspeed_ast27x0.c     | 12 ++++++++++++
>   2 files changed, 15 insertions(+)
> 
> diff --git a/include/hw/arm/aspeed_soc.h b/include/hw/arm/aspeed_soc.h
> index f069d17d16..9af8cfbc3e 100644
> --- a/include/hw/arm/aspeed_soc.h
> +++ b/include/hw/arm/aspeed_soc.h
> @@ -59,6 +59,7 @@ struct AspeedSoCState {
>       MemoryRegion sram;
>       MemoryRegion spi_boot_container;
>       MemoryRegion spi_boot;
> +    MemoryRegion vbootrom;
>       AddressSpace dram_as;
>       AspeedRtcState rtc;
>       AspeedTimerCtrlState timerctrl;
> @@ -152,6 +153,7 @@ struct AspeedSoCClass {
>       const char * const *valid_cpu_types;
>       uint32_t silicon_rev;
>       uint64_t sram_size;
> +    uint64_t vbootrom_size;
>       uint64_t secsram_size;
>       int spis_num;
>       int ehcis_num;
> @@ -169,6 +171,7 @@ struct AspeedSoCClass {
>   const char *aspeed_soc_cpu_type(AspeedSoCClass *sc);
>   
>   enum {
> +    ASPEED_DEV_VBOOTROM,
>       ASPEED_DEV_SPI_BOOT,
>       ASPEED_DEV_IOMEM,
>       ASPEED_DEV_UART0,
> diff --git a/hw/arm/aspeed_ast27x0.c b/hw/arm/aspeed_ast27x0.c
> index b05ed75ff4..7eece8e286 100644
> --- a/hw/arm/aspeed_ast27x0.c
> +++ b/hw/arm/aspeed_ast27x0.c
> @@ -24,6 +24,7 @@
>   #include "qemu/log.h"
>   
>   static const hwaddr aspeed_soc_ast2700_memmap[] = {
> +    [ASPEED_DEV_VBOOTROM]  =  0x00000000,
>       [ASPEED_DEV_SRAM]      =  0x10000000,
>       [ASPEED_DEV_HACE]      =  0x12070000,
>       [ASPEED_DEV_EMMC]      =  0x12090000,
> @@ -657,6 +658,15 @@ static void aspeed_soc_ast2700_realize(DeviceState *dev, Error **errp)
>       memory_region_add_subregion(s->memory,
>                                   sc->memmap[ASPEED_DEV_SRAM], &s->sram);
>   
> +    /* VBOOTROM */
> +    name = g_strdup_printf("aspeed.vbootrom.%d", CPU(&a->cpu[0])->cpu_index);
> +    if (!memory_region_init_ram(&s->vbootrom, OBJECT(s), name,
> +                                sc->vbootrom_size, errp)) {
> +        return;
> +    }
> +    memory_region_add_subregion(s->memory,
> +                                sc->memmap[ASPEED_DEV_VBOOTROM], &s->vbootrom);
> +
>       /* SCU */
>       if (!sysbus_realize(SYS_BUS_DEVICE(&s->scu), errp)) {
>           return;
> @@ -898,6 +908,7 @@ static void aspeed_soc_ast2700a0_class_init(ObjectClass *oc, void *data)
>   
>       sc->valid_cpu_types = valid_cpu_types;
>       sc->silicon_rev  = AST2700_A0_SILICON_REV;
> +    sc->vbootrom_size = 0x20000;
>       sc->sram_size    = 0x20000;
>       sc->spis_num     = 3;
>       sc->wdts_num     = 8;
> @@ -925,6 +936,7 @@ static void aspeed_soc_ast2700a1_class_init(ObjectClass *oc, void *data)
>   
>       sc->valid_cpu_types = valid_cpu_types;
>       sc->silicon_rev  = AST2700_A1_SILICON_REV;
> +    sc->vbootrom_size = 0x20000;
>       sc->sram_size    = 0x20000;
>       sc->spis_num     = 3;
>       sc->wdts_num     = 8;

Re: [PATCH v4 06/10] hw/arm/aspeed: Add support for loading vbootrom image via "-bios"

[Cédric Le Goater]

On 4/17/25 05:12, Jamin Lin wrote:
> Introduce "aspeed_load_vbootrom()" to support loading a virtual boot ROM image
> into the vbootrom memory region, using the "-bios" command-line option.
> 
> Signed-off-by: Jamin Lin <jamin_lin@aspeedtech.com>
> Reviewed-by: Nabih Estefan <nabihestefan@google.com>
> Tested-by: Nabih Estefan <nabihestefan@google.com>
> ---
>   include/hw/arm/aspeed.h |  1 +
>   hw/arm/aspeed.c         | 36 ++++++++++++++++++++++++++++++++++++
>   2 files changed, 37 insertions(+)
> 
> diff --git a/include/hw/arm/aspeed.h b/include/hw/arm/aspeed.h
> index 973277bea6..2b8332a7d7 100644
> --- a/include/hw/arm/aspeed.h
> +++ b/include/hw/arm/aspeed.h
> @@ -41,6 +41,7 @@ struct AspeedMachineClass {
>       uint32_t uart_default;
>       bool sdhci_wp_inverted;
>       bool vbootrom;
> +    const char *vbootrom_name;

I don't think adding a class attribute for the default firmware
image file is that useful. A define should be enough :

     #define VBOOTROM_FILE_NAME  "ast27x0_bootrom.bin"

and use VBOOTROM_FILE_NAME when defining the bios_name variable.

Unless you already plan to have different default firmware files
for the ast27* machines ?


>   };
>   
>   
> diff --git a/hw/arm/aspeed.c b/hw/arm/aspeed.c
> index b70a120e62..7f11371f02 100644
> --- a/hw/arm/aspeed.c
> +++ b/hw/arm/aspeed.c
> @@ -27,6 +27,7 @@
>   #include "system/reset.h"
>   #include "hw/loader.h"
>   #include "qemu/error-report.h"
> +#include "qemu/datadir.h"
>   #include "qemu/units.h"
>   #include "hw/qdev-clock.h"
>   #include "system/system.h"
> @@ -305,6 +306,34 @@ static void aspeed_install_boot_rom(AspeedMachineState *bmc, BlockBackend *blk,
>                      rom_size, &error_abort);
>   }
>   
> +/*
> + * This function locates the vbootrom image file specified via the command line
> + * using the -bios option. It loads the specified image into the vbootrom
> + * memory region and handles errors if the file cannot be found or loaded.
> + */
> +static void aspeed_load_vbootrom(MachineState *machine, uint64_t rom_size,
> +                                 Error **errp)
> +{
> +    AspeedMachineState *bmc = ASPEED_MACHINE(machine);
> +    AspeedMachineClass *amc = ASPEED_MACHINE_GET_CLASS(machine);
> +    const char *bios_name = machine->firmware ?: amc->vbootrom_name;
> +    g_autofree char *filename = NULL;
> +    AspeedSoCState *soc = bmc->soc;
> +    int ret;
> +
> +    filename = qemu_find_file(QEMU_FILE_TYPE_BIOS, bios_name);
> +    if (!filename) {
> +        error_setg(errp, "Could not find vbootrom image '%s'", bios_name);
> +        return;
> +    }
> +
> +    ret = load_image_mr(filename, &soc->vbootrom);
> +    if (ret < 0) {
> +        error_setg(errp, "Failed to load vbootrom image '%s'", bios_name);
> +        return;
> +    }
> +}
> +
>   void aspeed_board_init_flashes(AspeedSMCState *s, const char *flashtype,
>                                         unsigned int count, int unit0)
>   {
> @@ -483,6 +512,11 @@ static void aspeed_machine_init(MachineState *machine)
>           }
>       }
>   
> +    if (amc->vbootrom) {

what about the "aspeed.boot_rom" region ? Is it ok to have both ?


Thanks,

C.



> +        rom_size = memory_region_size(&bmc->soc->vbootrom);
> +        aspeed_load_vbootrom(machine, rom_size, &error_abort);
> +    }
> +
>       arm_load_kernel(ARM_CPU(first_cpu), machine, &aspeed_board_binfo);
>   }
>   
> @@ -1691,6 +1725,7 @@ static void aspeed_machine_ast2700a0_evb_class_init(ObjectClass *oc, void *data)
>       amc->uart_default = ASPEED_DEV_UART12;
>       amc->i2c_init  = ast2700_evb_i2c_init;
>       amc->vbootrom = true;
> +    amc->vbootrom_name = "ast27x0_bootrom.bin";
>       mc->auto_create_sdcard = true;
>       mc->default_ram_size = 1 * GiB;
>       aspeed_machine_class_init_cpus_defaults(mc);
> @@ -1712,6 +1747,7 @@ static void aspeed_machine_ast2700a1_evb_class_init(ObjectClass *oc, void *data)
>       amc->uart_default = ASPEED_DEV_UART12;
>       amc->i2c_init  = ast2700_evb_i2c_init;
>       amc->vbootrom = true;
> +    amc->vbootrom_name = "ast27x0_bootrom.bin";
>       mc->auto_create_sdcard = true;
>       mc->default_ram_size = 1 * GiB;
>       aspeed_machine_class_init_cpus_defaults(mc);

Re: [PATCH v4 02/10] hw/arm/aspeed_ast27x0 Introduce vbootrom memory region

[Cédric Le Goater]

On 4/17/25 05:11, Jamin Lin wrote:
> Introduce a new vbootrom memory region. The region is mapped at address
> "0x00000000" and has a size of 128KB, identical to the SRAM region size.
> This memory region is intended for loading a vbootrom image file as part of the
> boot process.
> 
> The vbootrom registered in the SoC's address space using the ASPEED_DEV_VBOOTROM
> index.
> 
> Introduced a "vbootrom_size" attribute in "AspeedSoCClass" to define virtual
> boot ROM size.
> 
> Signed-off-by: Jamin Lin <jamin_lin@aspeedtech.com>
> Reviewed-by: Nabih Estefan <nabihestefan@google.com>
> Tested-by: Nabih Estefan <nabihestefan@google.com>
> ---
>   include/hw/arm/aspeed_soc.h |  3 +++
>   hw/arm/aspeed_ast27x0.c     | 12 ++++++++++++
>   2 files changed, 15 insertions(+)
> 
> diff --git a/include/hw/arm/aspeed_soc.h b/include/hw/arm/aspeed_soc.h
> index f069d17d16..9af8cfbc3e 100644
> --- a/include/hw/arm/aspeed_soc.h
> +++ b/include/hw/arm/aspeed_soc.h
> @@ -59,6 +59,7 @@ struct AspeedSoCState {
>       MemoryRegion sram;
>       MemoryRegion spi_boot_container;
>       MemoryRegion spi_boot;
> +    MemoryRegion vbootrom;
>       AddressSpace dram_as;
>       AspeedRtcState rtc;
>       AspeedTimerCtrlState timerctrl;
> @@ -152,6 +153,7 @@ struct AspeedSoCClass {
>       const char * const *valid_cpu_types;
>       uint32_t silicon_rev;
>       uint64_t sram_size;
> +    uint64_t vbootrom_size;
>       uint64_t secsram_size;
>       int spis_num;
>       int ehcis_num;
> @@ -169,6 +171,7 @@ struct AspeedSoCClass {
>   const char *aspeed_soc_cpu_type(AspeedSoCClass *sc);
>   
>   enum {
> +    ASPEED_DEV_VBOOTROM,
>       ASPEED_DEV_SPI_BOOT,
>       ASPEED_DEV_IOMEM,
>       ASPEED_DEV_UART0,
> diff --git a/hw/arm/aspeed_ast27x0.c b/hw/arm/aspeed_ast27x0.c
> index b05ed75ff4..7eece8e286 100644
> --- a/hw/arm/aspeed_ast27x0.c
> +++ b/hw/arm/aspeed_ast27x0.c
> @@ -24,6 +24,7 @@
>   #include "qemu/log.h"
>   
>   static const hwaddr aspeed_soc_ast2700_memmap[] = {
> +    [ASPEED_DEV_VBOOTROM]  =  0x00000000,
>       [ASPEED_DEV_SRAM]      =  0x10000000,
>       [ASPEED_DEV_HACE]      =  0x12070000,
>       [ASPEED_DEV_EMMC]      =  0x12090000,
> @@ -657,6 +658,15 @@ static void aspeed_soc_ast2700_realize(DeviceState *dev, Error **errp)
>       memory_region_add_subregion(s->memory,
>                                   sc->memmap[ASPEED_DEV_SRAM], &s->sram);
>   
> +    /* VBOOTROM */
> +    name = g_strdup_printf("aspeed.vbootrom.%d", CPU(&a->cpu[0])->cpu_index);

can't we simply name the region "aspeed.vbootrom" or "aspeed.vboot_rom" ?


Thanks,

C.



> +    if (!memory_region_init_ram(&s->vbootrom, OBJECT(s), name,> +                                sc->vbootrom_size, errp)) {
> +        return;
> +    }
> +    memory_region_add_subregion(s->memory,
> +                                sc->memmap[ASPEED_DEV_VBOOTROM], &s->vbootrom);
> +
>       /* SCU */
>       if (!sysbus_realize(SYS_BUS_DEVICE(&s->scu), errp)) {
>           return;
> @@ -898,6 +908,7 @@ static void aspeed_soc_ast2700a0_class_init(ObjectClass *oc, void *data)
>   
>       sc->valid_cpu_types = valid_cpu_types;
>       sc->silicon_rev  = AST2700_A0_SILICON_REV;
> +    sc->vbootrom_size = 0x20000;
>       sc->sram_size    = 0x20000;
>       sc->spis_num     = 3;
>       sc->wdts_num     = 8;
> @@ -925,6 +936,7 @@ static void aspeed_soc_ast2700a1_class_init(ObjectClass *oc, void *data)
>   
>       sc->valid_cpu_types = valid_cpu_types;
>       sc->silicon_rev  = AST2700_A1_SILICON_REV;
> +    sc->vbootrom_size = 0x20000;
>       sc->sram_size    = 0x20000;
>       sc->spis_num     = 3;
>       sc->wdts_num     = 8;

Re: [PATCH v4 07/10] tests/functional/aspeed: Move I2C test into shared helper for AST2700 reuse

[Cédric Le Goater]

On 4/17/25 05:12, Jamin Lin wrote:
> Move the I2C test case into a common helper function (do_ast2700_i2c_test) so it
> can be reused across multiple AST2700-based test cases. This reduces duplication
> and improves maintainability.
> 
> Signed-off-by: Jamin Lin <jamin_lin@aspeedtech.com>


Reviewed-by: Cédric Le Goater <clg@redhat.com>

Thanks,

C.


> ---
>   tests/functional/test_aarch64_aspeed.py | 28 +++++++++++++------------
>   1 file changed, 15 insertions(+), 13 deletions(-)
> 
> diff --git a/tests/functional/test_aarch64_aspeed.py b/tests/functional/test_aarch64_aspeed.py
> index c25c966278..441f7f3919 100755
> --- a/tests/functional/test_aarch64_aspeed.py
> +++ b/tests/functional/test_aarch64_aspeed.py
> @@ -18,6 +18,8 @@ class AST2x00MachineSDK(QemuSystemTest):
>       def do_test_aarch64_aspeed_sdk_start(self, image):
>           self.require_netdev('user')
>           self.vm.set_console()
> +        self.vm.add_args('-device',
> +                         'tmp105,bus=aspeed.i2c.bus.1,address=0x4d,id=tmp-test')
>           self.vm.add_args('-drive', 'file=' + image + ',if=mtd,format=raw',
>                            '-net', 'nic', '-net', 'user', '-snapshot')
>   
> @@ -35,6 +37,17 @@ def do_test_aarch64_aspeed_sdk_start(self, image):
>               'https://github.com/AspeedTech-BMC/openbmc/releases/download/v09.05/ast2700-default-obmc.tar.gz',
>               'c1f4496aec06743c812a6e9a1a18d032f34d62f3ddb6956e924fef62aa2046a5')
>   
> +    def do_ast2700_i2c_test(self):
> +        exec_command_and_wait_for_pattern(self,
> +            'echo lm75 0x4d > /sys/class/i2c-dev/i2c-1/device/new_device ',
> +            'i2c i2c-1: new_device: Instantiated device lm75 at 0x4d');
> +        exec_command_and_wait_for_pattern(self,
> +            'cat /sys/bus/i2c/devices/1-004d/hwmon/hwmon*/temp1_input', '0')
> +        self.vm.cmd('qom-set', path='/machine/peripheral/tmp-test',
> +                    property='temperature', value=18000)
> +        exec_command_and_wait_for_pattern(self,
> +            'cat /sys/bus/i2c/devices/1-004d/hwmon/hwmon*/temp1_input', '18000')
> +
>       def start_ast2700_test(self, name):
>           num_cpu = 4
>           uboot_size = os.path.getsize(self.scratch_file(name,
> @@ -73,8 +86,6 @@ def start_ast2700_test(self, name):
>                                f'loader,addr=0x430000000,cpu-num={i}')
>   
>           self.vm.add_args('-smp', str(num_cpu))
> -        self.vm.add_args('-device',
> -                         'tmp105,bus=aspeed.i2c.bus.1,address=0x4d,id=tmp-test')
>           self.do_test_aarch64_aspeed_sdk_start(
>               self.scratch_file(name, 'image-bmc'))
>   
> @@ -83,28 +94,19 @@ def start_ast2700_test(self, name):
>           exec_command_and_wait_for_pattern(self, 'root', 'Password:')
>           exec_command_and_wait_for_pattern(self, '0penBmc', f'root@{name}:~#')
>   
> -        exec_command_and_wait_for_pattern(self,
> -            'echo lm75 0x4d > /sys/class/i2c-dev/i2c-1/device/new_device ',
> -            'i2c i2c-1: new_device: Instantiated device lm75 at 0x4d');
> -        exec_command_and_wait_for_pattern(self,
> -            'cat /sys/bus/i2c/devices/1-004d/hwmon/hwmon*/temp1_input', '0')
> -        self.vm.cmd('qom-set', path='/machine/peripheral/tmp-test',
> -                    property='temperature', value=18000)
> -        exec_command_and_wait_for_pattern(self,
> -            'cat /sys/bus/i2c/devices/1-004d/hwmon/hwmon*/temp1_input', '18000')
> -
>       def test_aarch64_ast2700_evb_sdk_v09_05(self):
>           self.set_machine('ast2700-evb')
>   
>           self.archive_extract(self.ASSET_SDK_V905_AST2700)
>           self.start_ast2700_test('ast2700-a0-default')
> +        self.do_ast2700_i2c_test()
>   
>       def test_aarch64_ast2700a1_evb_sdk_v09_05(self):
>           self.set_machine('ast2700a1-evb')
>   
>           self.archive_extract(self.ASSET_SDK_V905_AST2700A1)
>           self.start_ast2700_test('ast2700-default')
> -
> +        self.do_ast2700_i2c_test()
>   
>   if __name__ == '__main__':
>       QemuSystemTest.main()

Re: [PATCH v4 08/10] tests/functional/aspeed: Update test ASPEED SDK v09.06

[Cédric Le Goater]

On 4/17/25 05:12, Jamin Lin wrote:
> Signed-off-by: Jamin Lin <jamin_lin@aspeedtech.com>


Reviewed-by: Cédric Le Goater <clg@redhat.com>

Thanks,

C.


> ---
>   tests/functional/test_aarch64_aspeed.py | 20 ++++++++++----------
>   1 file changed, 10 insertions(+), 10 deletions(-)
> 
> diff --git a/tests/functional/test_aarch64_aspeed.py b/tests/functional/test_aarch64_aspeed.py
> index 441f7f3919..337d701917 100755
> --- a/tests/functional/test_aarch64_aspeed.py
> +++ b/tests/functional/test_aarch64_aspeed.py
> @@ -29,13 +29,13 @@ def do_test_aarch64_aspeed_sdk_start(self, image):
>           wait_for_console_pattern(self, '## Loading kernel from FIT Image')
>           wait_for_console_pattern(self, 'Starting kernel ...')
>   
> -    ASSET_SDK_V905_AST2700 = Asset(
> -            'https://github.com/AspeedTech-BMC/openbmc/releases/download/v09.05/ast2700-a0-default-obmc.tar.gz',
> -            'cfbbd1cce72f2a3b73b9080c41eecdadebb7077fba4f7806d72ac99f3e84b74a')
> +    ASSET_SDK_V906_AST2700 = Asset(
> +            'https://github.com/AspeedTech-BMC/openbmc/releases/download/v09.06/ast2700-a0-default-obmc.tar.gz',
> +            '7247b6f19dbfb700686f8d9f723ac23f3eb229226c0589cb9b06b80d1b61f3cb')
>   
> -    ASSET_SDK_V905_AST2700A1 = Asset(
> -            'https://github.com/AspeedTech-BMC/openbmc/releases/download/v09.05/ast2700-default-obmc.tar.gz',
> -            'c1f4496aec06743c812a6e9a1a18d032f34d62f3ddb6956e924fef62aa2046a5')
> +    ASSET_SDK_V906_AST2700A1 = Asset(
> +            'https://github.com/AspeedTech-BMC/openbmc/releases/download/v09.06/ast2700-default-obmc.tar.gz',
> +            'f1d53e0be8a404ecce3e105f72bc50fa4e090ad13160ffa91b10a6e0233a9dc6')
>   
>       def do_ast2700_i2c_test(self):
>           exec_command_and_wait_for_pattern(self,
> @@ -94,17 +94,17 @@ def start_ast2700_test(self, name):
>           exec_command_and_wait_for_pattern(self, 'root', 'Password:')
>           exec_command_and_wait_for_pattern(self, '0penBmc', f'root@{name}:~#')
>   
> -    def test_aarch64_ast2700_evb_sdk_v09_05(self):
> +    def test_aarch64_ast2700_evb_sdk_v09_06(self):
>           self.set_machine('ast2700-evb')
>   
> -        self.archive_extract(self.ASSET_SDK_V905_AST2700)
> +        self.archive_extract(self.ASSET_SDK_V906_AST2700)
>           self.start_ast2700_test('ast2700-a0-default')
>           self.do_ast2700_i2c_test()
>   
> -    def test_aarch64_ast2700a1_evb_sdk_v09_05(self):
> +    def test_aarch64_ast2700a1_evb_sdk_v09_06(self):
>           self.set_machine('ast2700a1-evb')
>   
> -        self.archive_extract(self.ASSET_SDK_V905_AST2700A1)
> +        self.archive_extract(self.ASSET_SDK_V906_AST2700A1)
>           self.start_ast2700_test('ast2700-default')
>           self.do_ast2700_i2c_test()
>   

Re: [PATCH v4 09/10] tests/functional/aspeed: Add to test vbootrom for AST2700

[Cédric Le Goater]

On 4/17/25 05:12, Jamin Lin wrote:
> Add the AST2700 functional test to boot using the vbootrom image
> instead of manually loading boot components with -device loader.
> The boot ROM binary is now passed via the
> -bios option, using the image located in pc-bios/ast27x0_bootrom.bin.
> 
> Signed-off-by: Jamin Lin <jamin_lin@aspeedtech.com>
> ---
>   tests/functional/test_aarch64_aspeed.py | 15 +++++++++++++++
>   1 file changed, 15 insertions(+)
> 
> diff --git a/tests/functional/test_aarch64_aspeed.py b/tests/functional/test_aarch64_aspeed.py
> index 337d701917..85789c1b1d 100755
> --- a/tests/functional/test_aarch64_aspeed.py
> +++ b/tests/functional/test_aarch64_aspeed.py
> @@ -94,6 +94,14 @@ def start_ast2700_test(self, name):
>           exec_command_and_wait_for_pattern(self, 'root', 'Password:')
>           exec_command_and_wait_for_pattern(self, '0penBmc', f'root@{name}:~#')
>   
> +    def start_ast2700_test_vbootrom(self, name):
> +        self.vm.add_args('-bios', 'ast27x0_bootrom.bin')
> +        self.do_test_aarch64_aspeed_sdk_start(
> +                self.scratch_file(name, 'image-bmc'))
> +        wait_for_console_pattern(self, f'{name} login:')
> +        exec_command_and_wait_for_pattern(self, 'root', 'Password:')
> +        exec_command_and_wait_for_pattern(self, '0penBmc', f'root@{name}:~#')
> +
>       def test_aarch64_ast2700_evb_sdk_v09_06(self):
>           self.set_machine('ast2700-evb')
>   
> @@ -108,5 +116,12 @@ def test_aarch64_ast2700a1_evb_sdk_v09_06(self):
>           self.start_ast2700_test('ast2700-default')
>           self.do_ast2700_i2c_test()
>   
> +    def test_aarch64_ast2700a1_evb_sdk_vboottom_v09_06(self):

vboottom -> vbootrom

> +        self.set_machine('ast2700a1-evb')
> +
> +        self.archive_extract(self.ASSET_SDK_V906_AST2700A1)
> +        self.start_ast2700_test_vbootrom('ast2700-default')
> +        self.do_ast2700_i2c_test()
> +
>   if __name__ == '__main__':
>       QemuSystemTest.main()

I think we should add some patterns to catch the vbootrom output. See
below.


Thanks,

C.



  _    ______  ____  ____  __________  ____  __  ___      ___   ______________  ______  ______
| |  / / __ )/ __ \/ __ \/_  __/ __ \/ __ \/  |/  /     /   | / ___/_  __/__ \/__  / |/ / __ \
| | / / __  / / / / / / / / / / /_/ / / / / /|_/ /_____/ /| | \__ \ / /  __/ /  / /|   / / / /
| |/ / /_/ / /_/ / /_/ / / / / _, _/ /_/ / /  / /_____/ ___ |___/ // /  / __/  / //   / /_/ /
|___/_____/\____/\____/ /_/ /_/ |_|\____/_/  /_/     /_/  |_/____//_/  /____/ /_//_/|_\____/

Version:1.0.0

Found valid FIT image at 0x100100000 (size: 0x15908b bytes)
[uboot] load address: 0x80000000
[uboot] load end address: 0x800b39a0
[uboot] data: 735648 bytes @ offset 0x11c
[uboot] loading 735648 bytes to 0x400000000 ... done
[fdt] load: property not found
[fdt] no load addr, fallback to u-boot end: 0x800b39a0
[fdt] data: 25752 bytes @ offset 0xb3b5c
[fdt] loading 25752 bytes to 0x4000b39a0 ... done
[tee] load address: 0xb0080000
[tee] data: 433304 bytes @ offset 0xba070
[tee] loading 433304 bytes to 0x430080000 ... done
[atf] load address: 0xb0000000
[atf] data: 28777 bytes @ offset 0x123da8
[atf] loading 28777 bytes to 0x430000000 ... done
[sspfw] load address: 0xac000000
[sspfw] data: 67260 bytes @ offset 0x12aecc
[sspfw] loading 67260 bytes to 0x42c000000 ... done
[tspfw] load address: 0xae000000
[tspfw] data: 67392 bytes @ offset 0x13b630
[tspfw] loading 67392 bytes to 0x42e000000 ... done
[ibexfw] load address: 0x14ba8000
[ibexfw] data: 52460 bytes @ offset 0x14be18
[ibexfw] loading 52460 bytes to 0x14ba8000 ... done

Jumping to BL31 (Trusted Firmware-A) at 0x430000000

RE: [PATCH v4 02/10] hw/arm/aspeed_ast27x0 Introduce vbootrom memory region

[Jamin Lin]

Hi Cedric, 

> Cc: Troy Lee <troy_lee@aspeedtech.com>; nabihestefan@google.com
> Subject: Re: [PATCH v4 02/10] hw/arm/aspeed_ast27x0 Introduce vbootrom
> memory region
> 
> On 4/17/25 05:11, Jamin Lin wrote:
> > Introduce a new vbootrom memory region. The region is mapped at
> > address "0x00000000" and has a size of 128KB, identical to the SRAM region
> size.
> > This memory region is intended for loading a vbootrom image file as
> > part of the boot process.
> >
> > The vbootrom registered in the SoC's address space using the
> > ASPEED_DEV_VBOOTROM index.
> >
> > Introduced a "vbootrom_size" attribute in "AspeedSoCClass" to define
> > virtual boot ROM size.
> >
> > Signed-off-by: Jamin Lin <jamin_lin@aspeedtech.com>
> > Reviewed-by: Nabih Estefan <nabihestefan@google.com>
> > Tested-by: Nabih Estefan <nabihestefan@google.com>
> > ---
> >   include/hw/arm/aspeed_soc.h |  3 +++
> >   hw/arm/aspeed_ast27x0.c     | 12 ++++++++++++
> >   2 files changed, 15 insertions(+)
> >
> > diff --git a/include/hw/arm/aspeed_soc.h b/include/hw/arm/aspeed_soc.h
> > index f069d17d16..9af8cfbc3e 100644
> > --- a/include/hw/arm/aspeed_soc.h
> > +++ b/include/hw/arm/aspeed_soc.h
> > @@ -59,6 +59,7 @@ struct AspeedSoCState {
> >       MemoryRegion sram;
> >       MemoryRegion spi_boot_container;
> >       MemoryRegion spi_boot;
> > +    MemoryRegion vbootrom;
> >       AddressSpace dram_as;
> >       AspeedRtcState rtc;
> >       AspeedTimerCtrlState timerctrl;
> > @@ -152,6 +153,7 @@ struct AspeedSoCClass {
> >       const char * const *valid_cpu_types;
> >       uint32_t silicon_rev;
> >       uint64_t sram_size;
> > +    uint64_t vbootrom_size;
> >       uint64_t secsram_size;
> >       int spis_num;
> >       int ehcis_num;
> > @@ -169,6 +171,7 @@ struct AspeedSoCClass {
> >   const char *aspeed_soc_cpu_type(AspeedSoCClass *sc);
> >
> >   enum {
> > +    ASPEED_DEV_VBOOTROM,
> >       ASPEED_DEV_SPI_BOOT,
> >       ASPEED_DEV_IOMEM,
> >       ASPEED_DEV_UART0,
> > diff --git a/hw/arm/aspeed_ast27x0.c b/hw/arm/aspeed_ast27x0.c index
> > b05ed75ff4..7eece8e286 100644
> > --- a/hw/arm/aspeed_ast27x0.c
> > +++ b/hw/arm/aspeed_ast27x0.c
> > @@ -24,6 +24,7 @@
> >   #include "qemu/log.h"
> >
> >   static const hwaddr aspeed_soc_ast2700_memmap[] = {
> > +    [ASPEED_DEV_VBOOTROM]  =  0x00000000,
> >       [ASPEED_DEV_SRAM]      =  0x10000000,
> >       [ASPEED_DEV_HACE]      =  0x12070000,
> >       [ASPEED_DEV_EMMC]      =  0x12090000,
> > @@ -657,6 +658,15 @@ static void
> aspeed_soc_ast2700_realize(DeviceState *dev, Error **errp)
> >       memory_region_add_subregion(s->memory,
> >
> sc->memmap[ASPEED_DEV_SRAM],
> > &s->sram);
> >
> > +    /* VBOOTROM */
> > +    name = g_strdup_printf("aspeed.vbootrom.%d",
> > + CPU(&a->cpu[0])->cpu_index);
> 
> can't we simply name the region "aspeed.vbootrom" or "aspeed.vboot_rom" ?
> 
Thanks for review.
Will do
Jamin
> 
> Thanks,
> 
> C.
> 
> 
> 
> > +    if (!memory_region_init_ram(&s->vbootrom, OBJECT(s), name,> +
> sc->vbootrom_size, errp)) {
> > +        return;
> > +    }
> > +    memory_region_add_subregion(s->memory,
> > +
> sc->memmap[ASPEED_DEV_VBOOTROM],
> > + &s->vbootrom);
> > +
> >       /* SCU */
> >       if (!sysbus_realize(SYS_BUS_DEVICE(&s->scu), errp)) {
> >           return;
> > @@ -898,6 +908,7 @@ static void
> > aspeed_soc_ast2700a0_class_init(ObjectClass *oc, void *data)
> >
> >       sc->valid_cpu_types = valid_cpu_types;
> >       sc->silicon_rev  = AST2700_A0_SILICON_REV;
> > +    sc->vbootrom_size = 0x20000;
> >       sc->sram_size    = 0x20000;
> >       sc->spis_num     = 3;
> >       sc->wdts_num     = 8;
> > @@ -925,6 +936,7 @@ static void
> > aspeed_soc_ast2700a1_class_init(ObjectClass *oc, void *data)
> >
> >       sc->valid_cpu_types = valid_cpu_types;
> >       sc->silicon_rev  = AST2700_A1_SILICON_REV;
> > +    sc->vbootrom_size = 0x20000;
> >       sc->sram_size    = 0x20000;
> >       sc->spis_num     = 3;
> >       sc->wdts_num     = 8;

RE: [PATCH v4 02/10] hw/arm/aspeed_ast27x0 Introduce vbootrom memory region

[Jamin Lin]

Hi Cedric,

> Subject: Re: [PATCH v4 02/10] hw/arm/aspeed_ast27x0 Introduce vbootrom
> memory region
> 
> On 4/17/25 05:11, Jamin Lin wrote:
> > Introduce a new vbootrom memory region. The region is mapped at
> > address "0x00000000" and has a size of 128KB, identical to the SRAM region
> size.
> > This memory region is intended for loading a vbootrom image file as
> > part of the boot process.
> >
> > The vbootrom registered in the SoC's address space using the
> > ASPEED_DEV_VBOOTROM index.
> >
> > Introduced a "vbootrom_size" attribute in "AspeedSoCClass" to define
> > virtual boot ROM size.
> 
> Could you please explain why we need a class attribute to size the vbootrom
> region ? The rest looks good.
> 

I've reviewed the SRAM design and used it as a reference to create a new class attribute for setting the size of the vbootrom memory region.
Currently, I don't plan to support different vbootrom images for the AST27x0. My understanding is that a single vbootrom image should be sufficient to support all AST27x0 variants.
If you agree, I will remove this class attribute and instead hardcode the vbootrom size to 128KB.

Thanks-Jamin
> 
> Thanks,
> 
> C.
> 
> 
> 
> >
> > Signed-off-by: Jamin Lin <jamin_lin@aspeedtech.com>
> > Reviewed-by: Nabih Estefan <nabihestefan@google.com>
> > Tested-by: Nabih Estefan <nabihestefan@google.com>
> > ---
> >   include/hw/arm/aspeed_soc.h |  3 +++
> >   hw/arm/aspeed_ast27x0.c     | 12 ++++++++++++
> >   2 files changed, 15 insertions(+)
> >
> > diff --git a/include/hw/arm/aspeed_soc.h b/include/hw/arm/aspeed_soc.h
> > index f069d17d16..9af8cfbc3e 100644
> > --- a/include/hw/arm/aspeed_soc.h
> > +++ b/include/hw/arm/aspeed_soc.h
> > @@ -59,6 +59,7 @@ struct AspeedSoCState {
> >       MemoryRegion sram;
> >       MemoryRegion spi_boot_container;
> >       MemoryRegion spi_boot;
> > +    MemoryRegion vbootrom;
> >       AddressSpace dram_as;
> >       AspeedRtcState rtc;
> >       AspeedTimerCtrlState timerctrl;
> > @@ -152,6 +153,7 @@ struct AspeedSoCClass {
> >       const char * const *valid_cpu_types;
> >       uint32_t silicon_rev;
> >       uint64_t sram_size;
> > +    uint64_t vbootrom_size;
> >       uint64_t secsram_size;
> >       int spis_num;
> >       int ehcis_num;
> > @@ -169,6 +171,7 @@ struct AspeedSoCClass {
> >   const char *aspeed_soc_cpu_type(AspeedSoCClass *sc);
> >
> >   enum {
> > +    ASPEED_DEV_VBOOTROM,
> >       ASPEED_DEV_SPI_BOOT,
> >       ASPEED_DEV_IOMEM,
> >       ASPEED_DEV_UART0,
> > diff --git a/hw/arm/aspeed_ast27x0.c b/hw/arm/aspeed_ast27x0.c index
> > b05ed75ff4..7eece8e286 100644
> > --- a/hw/arm/aspeed_ast27x0.c
> > +++ b/hw/arm/aspeed_ast27x0.c
> > @@ -24,6 +24,7 @@
> >   #include "qemu/log.h"
> >
> >   static const hwaddr aspeed_soc_ast2700_memmap[] = {
> > +    [ASPEED_DEV_VBOOTROM]  =  0x00000000,
> >       [ASPEED_DEV_SRAM]      =  0x10000000,
> >       [ASPEED_DEV_HACE]      =  0x12070000,
> >       [ASPEED_DEV_EMMC]      =  0x12090000,
> > @@ -657,6 +658,15 @@ static void
> aspeed_soc_ast2700_realize(DeviceState *dev, Error **errp)
> >       memory_region_add_subregion(s->memory,
> >
> sc->memmap[ASPEED_DEV_SRAM],
> > &s->sram);
> >
> > +    /* VBOOTROM */
> > +    name = g_strdup_printf("aspeed.vbootrom.%d",
> CPU(&a->cpu[0])->cpu_index);
> > +    if (!memory_region_init_ram(&s->vbootrom, OBJECT(s), name,
> > +                                sc->vbootrom_size, errp)) {
> > +        return;
> > +    }
> > +    memory_region_add_subregion(s->memory,
> > +
> sc->memmap[ASPEED_DEV_VBOOTROM],
> > + &s->vbootrom);
> > +
> >       /* SCU */
> >       if (!sysbus_realize(SYS_BUS_DEVICE(&s->scu), errp)) {
> >           return;
> > @@ -898,6 +908,7 @@ static void
> > aspeed_soc_ast2700a0_class_init(ObjectClass *oc, void *data)
> >
> >       sc->valid_cpu_types = valid_cpu_types;
> >       sc->silicon_rev  = AST2700_A0_SILICON_REV;
> > +    sc->vbootrom_size = 0x20000;
> >       sc->sram_size    = 0x20000;
> >       sc->spis_num     = 3;
> >       sc->wdts_num     = 8;
> > @@ -925,6 +936,7 @@ static void
> > aspeed_soc_ast2700a1_class_init(ObjectClass *oc, void *data)
> >
> >       sc->valid_cpu_types = valid_cpu_types;
> >       sc->silicon_rev  = AST2700_A1_SILICON_REV;
> > +    sc->vbootrom_size = 0x20000;
> >       sc->sram_size    = 0x20000;
> >       sc->spis_num     = 3;
> >       sc->wdts_num     = 8;

RE: [PATCH v4 06/10] hw/arm/aspeed: Add support for loading vbootrom image via "-bios"

[Jamin Lin]

Hi Cedric, 

> Cc: Troy Lee <troy_lee@aspeedtech.com>; nabihestefan@google.com
> Subject: Re: [PATCH v4 06/10] hw/arm/aspeed: Add support for loading
> vbootrom image via "-bios"
> 
> On 4/17/25 05:12, Jamin Lin wrote:
> > Introduce "aspeed_load_vbootrom()" to support loading a virtual boot
> > ROM image into the vbootrom memory region, using the "-bios"
> command-line option.
> >
> > Signed-off-by: Jamin Lin <jamin_lin@aspeedtech.com>
> > Reviewed-by: Nabih Estefan <nabihestefan@google.com>
> > Tested-by: Nabih Estefan <nabihestefan@google.com>
> > ---
> >   include/hw/arm/aspeed.h |  1 +
> >   hw/arm/aspeed.c         | 36
> ++++++++++++++++++++++++++++++++++++
> >   2 files changed, 37 insertions(+)
> >
> > diff --git a/include/hw/arm/aspeed.h b/include/hw/arm/aspeed.h index
> > 973277bea6..2b8332a7d7 100644
> > --- a/include/hw/arm/aspeed.h
> > +++ b/include/hw/arm/aspeed.h
> > @@ -41,6 +41,7 @@ struct AspeedMachineClass {
> >       uint32_t uart_default;
> >       bool sdhci_wp_inverted;
> >       bool vbootrom;
> > +    const char *vbootrom_name;
> 
> I don't think adding a class attribute for the default firmware image file is that
> useful. A define should be enough :
> 
>      #define VBOOTROM_FILE_NAME  "ast27x0_bootrom.bin"
> 
> and use VBOOTROM_FILE_NAME when defining the bios_name variable.
> 
> Unless you already plan to have different default firmware files for the ast27*
> machines ?
> 

If we plan to support the vbootrom feature for multiple ASPEED SoCs such as the AST28x0 series, it seems that the current default define
for the image name cannot be reused for both AST27x0 and future ASPEED SoCs. This is because the vbootrom file names differ between the two series.
That’s the reason I introduced a machine class attribute — to allow flexibility in specifying the correct vbootrom name for each SoC family.
I will use the DEFINE for now in the v5 patch. When we plan to support vbootrom for new SoCs in the future, we can revisit and discuss the appropriate default naming convention at that time.

> 
> >   };
> >
> >
> > diff --git a/hw/arm/aspeed.c b/hw/arm/aspeed.c index
> > b70a120e62..7f11371f02 100644
> > --- a/hw/arm/aspeed.c
> > +++ b/hw/arm/aspeed.c
> > @@ -27,6 +27,7 @@
> >   #include "system/reset.h"
> >   #include "hw/loader.h"
> >   #include "qemu/error-report.h"
> > +#include "qemu/datadir.h"
> >   #include "qemu/units.h"
> >   #include "hw/qdev-clock.h"
> >   #include "system/system.h"
> > @@ -305,6 +306,34 @@ static void
> aspeed_install_boot_rom(AspeedMachineState *bmc, BlockBackend *blk,
> >                      rom_size, &error_abort);
> >   }
> >
> > +/*
> > + * This function locates the vbootrom image file specified via the
> > +command line
> > + * using the -bios option. It loads the specified image into the
> > +vbootrom
> > + * memory region and handles errors if the file cannot be found or loaded.
> > + */
> > +static void aspeed_load_vbootrom(MachineState *machine, uint64_t
> rom_size,
> > +                                 Error **errp) {
> > +    AspeedMachineState *bmc = ASPEED_MACHINE(machine);
> > +    AspeedMachineClass *amc =
> ASPEED_MACHINE_GET_CLASS(machine);
> > +    const char *bios_name = machine->firmware ?:
> amc->vbootrom_name;
> > +    g_autofree char *filename = NULL;
> > +    AspeedSoCState *soc = bmc->soc;
> > +    int ret;
> > +
> > +    filename = qemu_find_file(QEMU_FILE_TYPE_BIOS, bios_name);
> > +    if (!filename) {
> > +        error_setg(errp, "Could not find vbootrom image '%s'",
> bios_name);
> > +        return;
> > +    }
> > +
> > +    ret = load_image_mr(filename, &soc->vbootrom);
> > +    if (ret < 0) {
> > +        error_setg(errp, "Failed to load vbootrom image '%s'",
> bios_name);
> > +        return;
> > +    }
> > +}
> > +
> >   void aspeed_board_init_flashes(AspeedSMCState *s, const char
> *flashtype,
> >                                         unsigned int count, int
> unit0)
> >   {
> > @@ -483,6 +512,11 @@ static void aspeed_machine_init(MachineState
> *machine)
> >           }
> >       }
> >
> > +    if (amc->vbootrom) {
> 
> what about the "aspeed.boot_rom" region ? Is it ok to have both ?
> 
Based on the design of aspeed_install_boot_rom, users can place their ROM code at the top of the image-bmc, and this function will install image-bmc which included the user's ROM IMAGE at the ASPEED_DEV_SPI_BOOT address.
For AST2600, users typically set the boot address to 0x0 and boot directly from there.

For AST2700, we introduced a vbootrom to simulate the ROM code and the BOOTMCU SPL (RISC-V).
We use aspeed_install_boot_rom to load the image-bmc at the FMC CS0 memory-mapped I/O address, so we set ASPEED_DEV_SPI_BOOT to 0x100000000.
We load the vbootrom image into the vbootrom memory region at address 0x0 and start execution from there.
The guest OS first enters the vbootrom. From there, it can easily access the flash data (image-bmc) at 0x100000000, since vbootrom itself doesn’t require SPI/flash/emmc host drivers.
To support future eMMC booting, we also plan to install the eMMC image at the ASPEED_DEV_SPI_BOOT address.
https://github.com/qemu/qemu/blob/master/hw/arm/aspeed.c#L477

It is fully supported to have both options. If users want to include their own ROM code within image-bmc, they can set the program counter (PC) to 0x100000000, just like how a manual loader set it to 0x43000000 (e.g., to jump directly to BL31).
This allows users to bypass the vbootrom if desired.
However, I believe this use case will be rare, as the vbootrom design should be able to satisfy most, if not all, user requirements.

Thanks-Jamin

> 
> Thanks,
> 
> C.
> 
> 
> 
> > +        rom_size = memory_region_size(&bmc->soc->vbootrom);
> > +        aspeed_load_vbootrom(machine, rom_size, &error_abort);
> > +    }
> > +
> >       arm_load_kernel(ARM_CPU(first_cpu), machine,
> &aspeed_board_binfo);
> >   }
> >
> > @@ -1691,6 +1725,7 @@ static void
> aspeed_machine_ast2700a0_evb_class_init(ObjectClass *oc, void *data)
> >       amc->uart_default = ASPEED_DEV_UART12;
> >       amc->i2c_init  = ast2700_evb_i2c_init;
> >       amc->vbootrom = true;
> > +    amc->vbootrom_name = "ast27x0_bootrom.bin";
> >       mc->auto_create_sdcard = true;
> >       mc->default_ram_size = 1 * GiB;
> >       aspeed_machine_class_init_cpus_defaults(mc);
> > @@ -1712,6 +1747,7 @@ static void
> aspeed_machine_ast2700a1_evb_class_init(ObjectClass *oc, void *data)
> >       amc->uart_default = ASPEED_DEV_UART12;
> >       amc->i2c_init  = ast2700_evb_i2c_init;
> >       amc->vbootrom = true;
> > +    amc->vbootrom_name = "ast27x0_bootrom.bin";
> >       mc->auto_create_sdcard = true;
> >       mc->default_ram_size = 1 * GiB;
> >       aspeed_machine_class_init_cpus_defaults(mc);

Re: [PATCH v4 02/10] hw/arm/aspeed_ast27x0 Introduce vbootrom memory region

[Cédric Le Goater]

On 4/22/25 03:59, Jamin Lin wrote:
> Hi Cedric,
> 
>> Subject: Re: [PATCH v4 02/10] hw/arm/aspeed_ast27x0 Introduce vbootrom
>> memory region
>>
>> On 4/17/25 05:11, Jamin Lin wrote:
>>> Introduce a new vbootrom memory region. The region is mapped at
>>> address "0x00000000" and has a size of 128KB, identical to the SRAM region
>> size.
>>> This memory region is intended for loading a vbootrom image file as
>>> part of the boot process.
>>>
>>> The vbootrom registered in the SoC's address space using the
>>> ASPEED_DEV_VBOOTROM index.
>>>
>>> Introduced a "vbootrom_size" attribute in "AspeedSoCClass" to define
>>> virtual boot ROM size.
>>
>> Could you please explain why we need a class attribute to size the vbootrom
>> region ? The rest looks good.
>>
> 
> I've reviewed the SRAM design and used it as a reference to create a new class attribute for setting the size of the vbootrom memory region.
> Currently, I don't plan to support different vbootrom images for the AST27x0. My understanding is that a single vbootrom image should be sufficient to support all AST27x0 variants.
> If you agree, I will remove this class attribute and instead hardcode the vbootrom size to 128KB.

Looks good to me.

Thanks,

C.

RE: [PATCH v4 09/10] tests/functional/aspeed: Add to test vbootrom for AST2700

[Jamin Lin]

Hi Cedric,

> Subject: Re: [PATCH v4 09/10] tests/functional/aspeed: Add to test vbootrom
> for AST2700
> 
> On 4/17/25 05:12, Jamin Lin wrote:
> > Add the AST2700 functional test to boot using the vbootrom image
> > instead of manually loading boot components with -device loader.
> > The boot ROM binary is now passed via the -bios option, using the
> > image located in pc-bios/ast27x0_bootrom.bin.
> >
> > Signed-off-by: Jamin Lin <jamin_lin@aspeedtech.com>
> > ---
> >   tests/functional/test_aarch64_aspeed.py | 15 +++++++++++++++
> >   1 file changed, 15 insertions(+)
> >
> > diff --git a/tests/functional/test_aarch64_aspeed.py
> > b/tests/functional/test_aarch64_aspeed.py
> > index 337d701917..85789c1b1d 100755
> > --- a/tests/functional/test_aarch64_aspeed.py
> > +++ b/tests/functional/test_aarch64_aspeed.py
> > @@ -94,6 +94,14 @@ def start_ast2700_test(self, name):
> >           exec_command_and_wait_for_pattern(self, 'root', 'Password:')
> >           exec_command_and_wait_for_pattern(self, '0penBmc',
> > f'root@{name}:~#')
> >
> > +    def start_ast2700_test_vbootrom(self, name):
> > +        self.vm.add_args('-bios', 'ast27x0_bootrom.bin')
> > +        self.do_test_aarch64_aspeed_sdk_start(
> > +                self.scratch_file(name, 'image-bmc'))
> > +        wait_for_console_pattern(self, f'{name} login:')
> > +        exec_command_and_wait_for_pattern(self, 'root', 'Password:')
> > +        exec_command_and_wait_for_pattern(self, '0penBmc',
> > + f'root@{name}:~#')
> > +
> >       def test_aarch64_ast2700_evb_sdk_v09_06(self):
> >           self.set_machine('ast2700-evb')
> >
> > @@ -108,5 +116,12 @@ def test_aarch64_ast2700a1_evb_sdk_v09_06(self):
> >           self.start_ast2700_test('ast2700-default')
> >           self.do_ast2700_i2c_test()
> >
> > +    def test_aarch64_ast2700a1_evb_sdk_vboottom_v09_06(self):
> 
> vboottom -> vbootrom
> 
> > +        self.set_machine('ast2700a1-evb')
> > +
> > +        self.archive_extract(self.ASSET_SDK_V906_AST2700A1)
> > +        self.start_ast2700_test_vbootrom('ast2700-default')
> > +        self.do_ast2700_i2c_test()
> > +
> >   if __name__ == '__main__':
> >       QemuSystemTest.main()
> 
> I think we should add some patterns to catch the vbootrom output. See below.
> 
Thanks for your suggestion and review.
Will add
Thanks-Jamin
> 
> Thanks,
> 
> C.
> 
> 
> 
>   _    ______  ____  ____  __________  ____  __  ___      ___
> ______________  ______  ______
> | |  / / __ )/ __ \/ __ \/_  __/ __ \/ __ \/  |/  /     /   | / ___/_  __/__
> \/__  / |/ / __ \
> | | / / __  / / / / / / / / / / /_/ / / / / /|_/ /_____/ /| | \__ \ / /  __/ /  / /|
> / / / /
> | |/ / /_/ / /_/ / /_/ / / / / _, _/ /_/ / /  / /_____/ ___ |___/ // /  / __/  / //
> / /_/ /
> |___/_____/\____/\____/ /_/ /_/ |_|\____/_/  /_/     /_/  |_/____//_/
> /____/ /_//_/|_\____/
> 
> Version:1.0.0
> 
> Found valid FIT image at 0x100100000 (size: 0x15908b bytes) [uboot] load
> address: 0x80000000 [uboot] load end address: 0x800b39a0 [uboot] data:
> 735648 bytes @ offset 0x11c [uboot] loading 735648 bytes to 0x400000000 ...
> done [fdt] load: property not found [fdt] no load addr, fallback to u-boot end:
> 0x800b39a0 [fdt] data: 25752 bytes @ offset 0xb3b5c [fdt] loading 25752 bytes
> to 0x4000b39a0 ... done [tee] load address: 0xb0080000 [tee] data: 433304
> bytes @ offset 0xba070 [tee] loading 433304 bytes to 0x430080000 ... done
> [atf] load address: 0xb0000000 [atf] data: 28777 bytes @ offset 0x123da8 [atf]
> loading 28777 bytes to 0x430000000 ... done [sspfw] load address: 0xac000000
> [sspfw] data: 67260 bytes @ offset 0x12aecc [sspfw] loading 67260 bytes to
> 0x42c000000 ... done [tspfw] load address: 0xae000000 [tspfw] data: 67392
> bytes @ offset 0x13b630 [tspfw] loading 67392 bytes to 0x42e000000 ... done
> [ibexfw] load address: 0x14ba8000 [ibexfw] data: 52460 bytes @ offset
> 0x14be18 [ibexfw] loading 52460 bytes to 0x14ba8000 ... done
> 
> Jumping to BL31 (Trusted Firmware-A) at 0x430000000
> 

Re: [PATCH v4 08/10] tests/functional/aspeed: Update test ASPEED SDK v09.06

[Cédric Le Goater]

Jamin,

On 4/17/25 05:12, Jamin Lin wrote:
> Signed-off-by: Jamin Lin <jamin_lin@aspeedtech.com>
> ---
>   tests/functional/test_aarch64_aspeed.py | 20 ++++++++++----------
>   1 file changed, 10 insertions(+), 10 deletions(-)
> 
> diff --git a/tests/functional/test_aarch64_aspeed.py b/tests/functional/test_aarch64_aspeed.py
> index 441f7f3919..337d701917 100755
> --- a/tests/functional/test_aarch64_aspeed.py
> +++ b/tests/functional/test_aarch64_aspeed.py
> @@ -29,13 +29,13 @@ def do_test_aarch64_aspeed_sdk_start(self, image):
>           wait_for_console_pattern(self, '## Loading kernel from FIT Image')
>           wait_for_console_pattern(self, 'Starting kernel ...')
>   
> -    ASSET_SDK_V905_AST2700 = Asset(
> -            'https://github.com/AspeedTech-BMC/openbmc/releases/download/v09.05/ast2700-a0-default-obmc.tar.gz',
> -            'cfbbd1cce72f2a3b73b9080c41eecdadebb7077fba4f7806d72ac99f3e84b74a')
> +    ASSET_SDK_V906_AST2700 = Asset(
> +            'https://github.com/AspeedTech-BMC/openbmc/releases/download/v09.06/ast2700-a0-default-obmc.tar.gz',
> +            '7247b6f19dbfb700686f8d9f723ac23f3eb229226c0589cb9b06b80d1b61f3cb')
>   
> -    ASSET_SDK_V905_AST2700A1 = Asset(
> -            'https://github.com/AspeedTech-BMC/openbmc/releases/download/v09.05/ast2700-default-obmc.tar.gz',
> -            'c1f4496aec06743c812a6e9a1a18d032f34d62f3ddb6956e924fef62aa2046a5')
> +    ASSET_SDK_V906_AST2700A1 = Asset(
> +            'https://github.com/AspeedTech-BMC/openbmc/releases/download/v09.06/ast2700-default-obmc.tar.gz',
> +            'f1d53e0be8a404ecce3e105f72bc50fa4e090ad13160ffa91b10a6e0233a9dc6')
>   
>       def do_ast2700_i2c_test(self):
>           exec_command_and_wait_for_pattern(self,
> @@ -94,17 +94,17 @@ def start_ast2700_test(self, name):
>           exec_command_and_wait_for_pattern(self, 'root', 'Password:')
>           exec_command_and_wait_for_pattern(self, '0penBmc', f'root@{name}:~#')
>   
> -    def test_aarch64_ast2700_evb_sdk_v09_05(self):
> +    def test_aarch64_ast2700_evb_sdk_v09_06(self):
>           self.set_machine('ast2700-evb')
>   
> -        self.archive_extract(self.ASSET_SDK_V905_AST2700)
> +        self.archive_extract(self.ASSET_SDK_V906_AST2700)
>           self.start_ast2700_test('ast2700-a0-default')
>           self.do_ast2700_i2c_test()
>   
> -    def test_aarch64_ast2700a1_evb_sdk_v09_05(self):
> +    def test_aarch64_ast2700a1_evb_sdk_v09_06(self):
>           self.set_machine('ast2700a1-evb')
>   
> -        self.archive_extract(self.ASSET_SDK_V905_AST2700A1)
> +        self.archive_extract(self.ASSET_SDK_V906_AST2700A1)
>           self.start_ast2700_test('ast2700-default')
>           self.do_ast2700_i2c_test()
>   

While at it, could you please update the SDK version for the tests
in the other aspeed files too ?

   tests/functional/test_aarch64_ast2700fc.py
   tests/functional/test_arm_aspeed_ast2500.py
   tests/functional/test_arm_aspeed_ast2600.py

Thanks,

C.

RE: [PATCH v4 08/10] tests/functional/aspeed: Update test ASPEED SDK v09.06

[Jamin Lin]

Hi Cedric,

> Cc: Troy Lee <troy_lee@aspeedtech.com>; nabihestefan@google.com
> Subject: Re: [PATCH v4 08/10] tests/functional/aspeed: Update test ASPEED
> SDK v09.06
> 
> Jamin,
> 
> On 4/17/25 05:12, Jamin Lin wrote:
> > Signed-off-by: Jamin Lin <jamin_lin@aspeedtech.com>
> > ---
> >   tests/functional/test_aarch64_aspeed.py | 20 ++++++++++----------
> >   1 file changed, 10 insertions(+), 10 deletions(-)
> >
> > diff --git a/tests/functional/test_aarch64_aspeed.py
> > b/tests/functional/test_aarch64_aspeed.py
> > index 441f7f3919..337d701917 100755
> > --- a/tests/functional/test_aarch64_aspeed.py
> > +++ b/tests/functional/test_aarch64_aspeed.py
> > @@ -29,13 +29,13 @@ def do_test_aarch64_aspeed_sdk_start(self, image):
> >           wait_for_console_pattern(self, '## Loading kernel from FIT
> Image')
> >           wait_for_console_pattern(self, 'Starting kernel ...')
> >
> > -    ASSET_SDK_V905_AST2700 = Asset(
> > -
> 'https://github.com/AspeedTech-BMC/openbmc/releases/download/v09.05/ast
> 2700-a0-default-obmc.tar.gz',
> > -
> 'cfbbd1cce72f2a3b73b9080c41eecdadebb7077fba4f7806d72ac99f3e84b74a')
> > +    ASSET_SDK_V906_AST2700 = Asset(
> > +
> 'https://github.com/AspeedTech-BMC/openbmc/releases/download/v09.06/ast
> 2700-a0-default-obmc.tar.gz',
> > +
> > +
> '7247b6f19dbfb700686f8d9f723ac23f3eb229226c0589cb9b06b80d1b61f3cb')
> >
> > -    ASSET_SDK_V905_AST2700A1 = Asset(
> > -
> 'https://github.com/AspeedTech-BMC/openbmc/releases/download/v09.05/ast
> 2700-default-obmc.tar.gz',
> > -
> 'c1f4496aec06743c812a6e9a1a18d032f34d62f3ddb6956e924fef62aa2046a5')
> > +    ASSET_SDK_V906_AST2700A1 = Asset(
> > +
> 'https://github.com/AspeedTech-BMC/openbmc/releases/download/v09.06/ast
> 2700-default-obmc.tar.gz',
> > +
> > +
> 'f1d53e0be8a404ecce3e105f72bc50fa4e090ad13160ffa91b10a6e0233a9dc6')
> >
> >       def do_ast2700_i2c_test(self):
> >           exec_command_and_wait_for_pattern(self,
> > @@ -94,17 +94,17 @@ def start_ast2700_test(self, name):
> >           exec_command_and_wait_for_pattern(self, 'root', 'Password:')
> >           exec_command_and_wait_for_pattern(self, '0penBmc',
> > f'root@{name}:~#')
> >
> > -    def test_aarch64_ast2700_evb_sdk_v09_05(self):
> > +    def test_aarch64_ast2700_evb_sdk_v09_06(self):
> >           self.set_machine('ast2700-evb')
> >
> > -        self.archive_extract(self.ASSET_SDK_V905_AST2700)
> > +        self.archive_extract(self.ASSET_SDK_V906_AST2700)
> >           self.start_ast2700_test('ast2700-a0-default')
> >           self.do_ast2700_i2c_test()
> >
> > -    def test_aarch64_ast2700a1_evb_sdk_v09_05(self):
> > +    def test_aarch64_ast2700a1_evb_sdk_v09_06(self):
> >           self.set_machine('ast2700a1-evb')
> >
> > -        self.archive_extract(self.ASSET_SDK_V905_AST2700A1)
> > +        self.archive_extract(self.ASSET_SDK_V906_AST2700A1)
> >           self.start_ast2700_test('ast2700-default')
> >           self.do_ast2700_i2c_test()
> >
> 
> While at it, could you please update the SDK version for the tests in the other
> aspeed files too ?
> 
Got it.
Will update them.
Thanks-Jamin

>    tests/functional/test_aarch64_ast2700fc.py
>    tests/functional/test_arm_aspeed_ast2500.py
>    tests/functional/test_arm_aspeed_ast2600.py
> 
> Thanks,
> 
> C.
> 

Re: [PATCH v4 10/10] docs/system/arm/aspeed: Support vbootrom for AST2700

[Cédric Le Goater]

Jamin,

On 4/17/25 05:12, Jamin Lin wrote:
> Using the vbootrom image support and the boot ROM binary is
> now passed via the -bios option, using the image located in
> pc-bios/ast27x0_bootrom.bin.
> 
> Signed-off-by: Jamin Lin <jamin_lin@aspeedtech.com>
> Reviewed-by: Nabih Estefan <nabihestefan@google.com>
> ---
>   docs/system/arm/aspeed.rst | 29 ++++++++++++++++++++++++++++-
>   1 file changed, 28 insertions(+), 1 deletion(-)
> 
> diff --git a/docs/system/arm/aspeed.rst b/docs/system/arm/aspeed.rst
> index 97fd6a0e7f..c87a2cf796 100644
> --- a/docs/system/arm/aspeed.rst
> +++ b/docs/system/arm/aspeed.rst
> @@ -250,7 +250,14 @@ under Linux), use :
>   Booting the ast2700-evb machine
>   ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^


Currently, we have 2 sections for the Aspeed boards :

1. Aspeed family boards (ast2500-evb, ast2600-evb, ast2700-evb, ast2700fc, bletchley-bmc, fuji-bmc, fby35-bmc, fp5280g2-bmc, g220a-bmc, palmetto-bmc, qcom-dc-scm-v1-bmc, qcom-firework-bmc, quanta-q71l-bmc, rainier-bmc, romulus-bmc, sonorapass-bmc, supermicrox11-bmc, supermicrox11spi-bmc, tiogapass-bmc, witherspoon-bmc, yosemitev2-bmc)
2. Aspeed minibmc family boards (ast1030-evb)

The first is quite big and relatively consistent for the AST2400, AST2500
and AST2600 SoCs. Since the AST2700 SoC boards boot differently, I think
it is time to introduce a new section for the ast2700 machines.


Thanks,

C.



> -Boot the AST2700 machine from the flash image, use an MTD drive :
> +Boot the AST2700 machine from the flash image.
> +
> +There are two supported methods for booting the AST2700 machine with a flash image:
> +
> +Manual boot using ``-device loader``:
> +
> +It causes all 4 CPU cores to start execution from address ``0x430000000``, which
> +corresponds to the BL31 image load address.
>   
>   .. code-block:: bash
>   
> @@ -270,6 +277,26 @@ Boot the AST2700 machine from the flash image, use an MTD drive :
>          -drive file=${IMGDIR}/image-bmc,format=raw,if=mtd \
>          -nographic
>   
> +Boot using a virtual boot ROM (``-bios``):
> +
> +If users do not specify the ``-bios option``, QEMU will attempt to load the
> +default vbootrom image ``ast27x0_bootrom.bin`` from either the current working
> +directory or the ``pc-bios`` directory within the QEMU source tree.
> +
> +.. code-block:: bash
> +
> +  $ qemu-system-aarch64 -M ast2700-evb \
> +      -drive file=image-bmc,format=raw,if=mtd \
> +      -nographic
> +
> +The ``-bios`` option allows users to specify a custom path for the vbootrom
> +image to be loaded during boot. This will load the vbootrom image from the
> +specified path in the ${HOME} directory.
> +
> +.. code-block:: bash
> +
> +  -bios ${HOME}/ast27x0_bootrom.bin
> +
>   Aspeed minibmc family boards (``ast1030-evb``)
>   ==================================================================
>   

RE: [PATCH v4 10/10] docs/system/arm/aspeed: Support vbootrom for AST2700

[Jamin Lin]

Hi Cedric,

> Cc: Troy Lee <troy_lee@aspeedtech.com>; nabihestefan@google.com
> Subject: Re: [PATCH v4 10/10] docs/system/arm/aspeed: Support vbootrom for
> AST2700
> 
> Jamin,
> 
> On 4/17/25 05:12, Jamin Lin wrote:
> > Using the vbootrom image support and the boot ROM binary is now passed
> > via the -bios option, using the image located in
> > pc-bios/ast27x0_bootrom.bin.
> >
> > Signed-off-by: Jamin Lin <jamin_lin@aspeedtech.com>
> > Reviewed-by: Nabih Estefan <nabihestefan@google.com>
> > ---
> >   docs/system/arm/aspeed.rst | 29 ++++++++++++++++++++++++++++-
> >   1 file changed, 28 insertions(+), 1 deletion(-)
> >
> > diff --git a/docs/system/arm/aspeed.rst b/docs/system/arm/aspeed.rst
> > index 97fd6a0e7f..c87a2cf796 100644
> > --- a/docs/system/arm/aspeed.rst
> > +++ b/docs/system/arm/aspeed.rst
> > @@ -250,7 +250,14 @@ under Linux), use :
> >   Booting the ast2700-evb machine
> >   ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> 
> 
> Currently, we have 2 sections for the Aspeed boards :
> 
> 1. Aspeed family boards (ast2500-evb, ast2600-evb, ast2700-evb, ast2700fc,
> bletchley-bmc, fuji-bmc, fby35-bmc, fp5280g2-bmc, g220a-bmc,
> palmetto-bmc, qcom-dc-scm-v1-bmc, qcom-firework-bmc, quanta-q71l-bmc,
> rainier-bmc, romulus-bmc, sonorapass-bmc, supermicrox11-bmc,
> supermicrox11spi-bmc, tiogapass-bmc, witherspoon-bmc, yosemitev2-bmc) 2.
> Aspeed minibmc family boards (ast1030-evb)
> 
> The first is quite big and relatively consistent for the AST2400, AST2500 and
> AST2600 SoCs. Since the AST2700 SoC boards boot differently, I think it is time
> to introduce a new section for the ast2700 machines.
> 
Thanks for review.
Will create a new section for the AST2700.
Jamin
> 
> Thanks,
> 
> C.
> 
> 
> 
> > -Boot the AST2700 machine from the flash image, use an MTD drive :
> > +Boot the AST2700 machine from the flash image.
> > +
> > +There are two supported methods for booting the AST2700 machine with a
> flash image:
> > +
> > +Manual boot using ``-device loader``:
> > +
> > +It causes all 4 CPU cores to start execution from address
> > +``0x430000000``, which corresponds to the BL31 image load address.
> >
> >   .. code-block:: bash
> >
> > @@ -270,6 +277,26 @@ Boot the AST2700 machine from the flash image,
> use an MTD drive :
> >          -drive file=${IMGDIR}/image-bmc,format=raw,if=mtd \
> >          -nographic
> >
> > +Boot using a virtual boot ROM (``-bios``):
> > +
> > +If users do not specify the ``-bios option``, QEMU will attempt to
> > +load the default vbootrom image ``ast27x0_bootrom.bin`` from either
> > +the current working directory or the ``pc-bios`` directory within the QEMU
> source tree.
> > +
> > +.. code-block:: bash
> > +
> > +  $ qemu-system-aarch64 -M ast2700-evb \
> > +      -drive file=image-bmc,format=raw,if=mtd \
> > +      -nographic
> > +
> > +The ``-bios`` option allows users to specify a custom path for the
> > +vbootrom image to be loaded during boot. This will load the vbootrom
> > +image from the specified path in the ${HOME} directory.
> > +
> > +.. code-block:: bash
> > +
> > +  -bios ${HOME}/ast27x0_bootrom.bin
> > +
> >   Aspeed minibmc family boards (``ast1030-evb``)
> >
> ================================================================
> ==
> >

Re: [PATCH v4 06/10] hw/arm/aspeed: Add support for loading vbootrom image via "-bios"

[Cédric Le Goater]

Hello Jamin,

> Based on the design of aspeed_install_boot_rom, users can place
> their ROM code at the top of the image-bmc, and this function will
> install image-bmc which included the user's ROM IMAGE at the
> ASPEED_DEV_SPI_BOOT address.  For AST2600, users typically set the
> boot address to 0x0 and boot directly from there.
>
> For AST2700, we introduced a vbootrom to simulate the ROM code and
> the BOOTMCU SPL (RISC-V).

Side question, is anyone working on the BOOTMCU SPL (RISC-V) models ?
heterogeneous machines should be supported one day.

> We use aspeed_install_boot_rom to load the image-bmc at the FMC CS0
> memory-mapped I/O address, so we set ASPEED_DEV_SPI_BOOT to
> 0x100000000.
>
> We load the vbootrom image into the vbootrom memory region at
> address 0x0 and start execution from there.
>
> The guest OS first enters the vbootrom. From there, it can easily
> access the flash data (image-bmc) at 0x100000000, since vbootrom
> itself doesn’t require SPI/flash/emmc host drivers.
>
> To support future eMMC booting, we also plan to install the eMMC
> image at the ASPEED_DEV_SPI_BOOT address.
> https://github.com/qemu/qemu/blob/master/hw/arm/aspeed.c#L477

ok.

> It is fully supported to have both options. If users want to include
> their own ROM code within image-bmc, they can set the program
> counter (PC) to 0x100000000, just like how a manual loader set it to
> 0x43000000 (e.g., to jump directly to BL31).  This allows users to
> bypass the vbootrom if desired.

ok.

> However, I believe this use case will be rare, as the vbootrom
> design should be able to satisfy most, if not all, user
> requirements.

We need to be careful about what we offer the user in terms of boot
method. It's difficult to maintain on the long term. Let's recap.

For the AST2[456]00 machines, we have :

   1. kernel boot
   2. flash device boot with or without "execute-in-place" machine
      option

and this could work for AST2700 machines with some loader magic.
It would be good to decide to or not to support it. If not supported,
let's inform the user asap.

For the AST2600 ast2600evb and rainier machines only, we have :

   3. emmc device boot

and we plan to extend it for AST2700 machines.

For the AST2700 machines, we have :

   4. manual loader boot (this could work for the other SoCs, although
      we have never tried)
   5. firmware boot

We need to define a priority between these methods too (the list
above is more or less ordered, apart from 4.) and handle conflicts.

All of which is to say that the piece of code below will require
some care:

     if (!bmc->mmio_exec) {
         DeviceState *dev = ssi_get_cs(bmc->soc->fmc.spi, 0);
         BlockBackend *fmc0 = dev ? m25p80_get_blk(dev) : NULL;

         if (fmc0 && !boot_emmc) {
             rom_size = memory_region_size(&bmc->soc->spi_boot);
             aspeed_install_boot_rom(bmc, fmc0, rom_size);
         } else if (emmc0) {
             aspeed_install_boot_rom(bmc, blk_by_legacy_dinfo(emmc0), 64 * KiB);
         }
     }

     if (amc->vbootrom) {
         rom_size = memory_region_size(&bmc->soc->vbootrom);
         aspeed_load_vbootrom(machine, rom_size, &error_abort);
     }

Thanks,

C.

RE: [PATCH v4 06/10] hw/arm/aspeed: Add support for loading vbootrom image via "-bios"

[Jamin Lin]

Hi Cedric,

> Cc: Troy Lee <troy_lee@aspeedtech.com>; nabihestefan@google.com
> Subject: Re: [PATCH v4 06/10] hw/arm/aspeed: Add support for loading
> vbootrom image via "-bios"
> 
> Hello Jamin,
> 
> > Based on the design of aspeed_install_boot_rom, users can place their
> > ROM code at the top of the image-bmc, and this function will install
> > image-bmc which included the user's ROM IMAGE at the
> > ASPEED_DEV_SPI_BOOT address.  For AST2600, users typically set the
> > boot address to 0x0 and boot directly from there.
> >
> > For AST2700, we introduced a vbootrom to simulate the ROM code and the
> > BOOTMCU SPL (RISC-V).
> 
> Side question, is anyone working on the BOOTMCU SPL (RISC-V) models ?
> heterogeneous machines should be supported one day.
> 

Troy developed an initial implementation, but testing has not yet been performed due to uncertainty around
"how to share DRAM memory and controllers registers" between the RISC-V and the Cortex-A35 cores.
Furthermore, RISC-V interrupt support is currently not implemented.

> > We use aspeed_install_boot_rom to load the image-bmc at the FMC CS0
> > memory-mapped I/O address, so we set ASPEED_DEV_SPI_BOOT to
> > 0x100000000.
> >
> > We load the vbootrom image into the vbootrom memory region at address
> > 0x0 and start execution from there.
> >
> > The guest OS first enters the vbootrom. From there, it can easily
> > access the flash data (image-bmc) at 0x100000000, since vbootrom
> > itself doesn’t require SPI/flash/emmc host drivers.
> >
> > To support future eMMC booting, we also plan to install the eMMC image
> > at the ASPEED_DEV_SPI_BOOT address.
> > https://github.com/qemu/qemu/blob/master/hw/arm/aspeed.c#L477
> 
> ok.
> 
> > It is fully supported to have both options. If users want to include
> > their own ROM code within image-bmc, they can set the program counter
> > (PC) to 0x100000000, just like how a manual loader set it to
> > 0x43000000 (e.g., to jump directly to BL31).  This allows users to
> > bypass the vbootrom if desired.
> 
> ok.
> 
> > However, I believe this use case will be rare, as the vbootrom design
> > should be able to satisfy most, if not all, user requirements.
> 
> We need to be careful about what we offer the user in terms of boot method.
> It's difficult to maintain on the long term. Let's recap.
> 

For the AST2700, I am currently considering support for the following boot methods only:

1. eMMC device boot
2. UFS device boot (Note: QEMU currently only supports PCI-based UFS and does not support platform-based UFS. This remains a long-term goal)
3. Firmware boot using vbootrom
4. Firmware boot using a manual loader

The boot flow for the AST2700’s Cortex-A35 should follow this sequence:
BL31 (Trusted Firmware-A) → BL32 (OP-TEE OS) → BL33 (U-Boot)

I don't think we should support direct kernel boot , as I don't know how to start execution directly from the kernel.

By the way, AST2700 support to boot from either EMMC or UFS.
I am considering to read OTP configs and strap instead of command line.
Ex: we used bootmmc=true to change hardstrap. 
It just my briefly plane and future goal.

Thanks-Jamin
> For the AST2[456]00 machines, we have :
> 
>    1. kernel boot
>    2. flash device boot with or without "execute-in-place" machine
>       option
> 
> and this could work for AST2700 machines with some loader magic.
> It would be good to decide to or not to support it. If not supported, let's inform
> the user asap.
> 
> For the AST2600 ast2600evb and rainier machines only, we have :
> 
>    3. emmc device boot
> 
> and we plan to extend it for AST2700 machines.
> 
> For the AST2700 machines, we have :
> 
>    4. manual loader boot (this could work for the other SoCs, although
>       we have never tried)
>    5. firmware boot
> 
> We need to define a priority between these methods too (the list above is
> more or less ordered, apart from 4.) and handle conflicts.
> 
> All of which is to say that the piece of code below will require some care:
> 
>      if (!bmc->mmio_exec) {
>          DeviceState *dev = ssi_get_cs(bmc->soc->fmc.spi, 0);
>          BlockBackend *fmc0 = dev ? m25p80_get_blk(dev) : NULL;
> 
>          if (fmc0 && !boot_emmc) {
>              rom_size = memory_region_size(&bmc->soc->spi_boot);
>              aspeed_install_boot_rom(bmc, fmc0, rom_size);
>          } else if (emmc0) {
>              aspeed_install_boot_rom(bmc, blk_by_legacy_dinfo(emmc0),
> 64 * KiB);
>          }
>      }
> 
>      if (amc->vbootrom) {
>          rom_size = memory_region_size(&bmc->soc->vbootrom);
>          aspeed_load_vbootrom(machine, rom_size, &error_abort);
>      }
> 
> Thanks,
> 
> C.
> 

Re: [PATCH v4 06/10] hw/arm/aspeed: Add support for loading vbootrom image via "-bios"

[Cédric Le Goater]

Hello Jamin,

+ Phil.

On 4/23/25 09:02, Jamin Lin wrote:
> Hi Cedric,
> 
>> Cc: Troy Lee <troy_lee@aspeedtech.com>; nabihestefan@google.com
>> Subject: Re: [PATCH v4 06/10] hw/arm/aspeed: Add support for loading
>> vbootrom image via "-bios"
>>
>> Hello Jamin,
>>
>>> Based on the design of aspeed_install_boot_rom, users can place their
>>> ROM code at the top of the image-bmc, and this function will install
>>> image-bmc which included the user's ROM IMAGE at the
>>> ASPEED_DEV_SPI_BOOT address.  For AST2600, users typically set the
>>> boot address to 0x0 and boot directly from there.
>>>
>>> For AST2700, we introduced a vbootrom to simulate the ROM code and the
>>> BOOTMCU SPL (RISC-V).
>>
>> Side question, is anyone working on the BOOTMCU SPL (RISC-V) models ?
>> heterogeneous machines should be supported one day.
>>
> 
> Troy developed an initial implementation, but testing has not yet been performed due to uncertainty around
> "how to share DRAM memory and controllers registers" between the RISC-V and the Cortex-A35 cores.
> Furthermore, RISC-V interrupt support is currently not implemented.
Could you explain a bit more the issues you are facing ? Single QEMU
binary is expected to become a reality in the near future and the
ast2700 models could benefit from it.


Thanks,

C.

RE: [PATCH v4 06/10] hw/arm/aspeed: Add support for loading vbootrom image via "-bios"

[Jamin Lin]

Hi Cedric, 

> Subject: Re: [PATCH v4 06/10] hw/arm/aspeed: Add support for loading
> vbootrom image via "-bios"
> 
> Hello Jamin,
> 
> + Phil.
> 
> On 4/23/25 09:02, Jamin Lin wrote:
> > Hi Cedric,
> >
> >> Cc: Troy Lee <troy_lee@aspeedtech.com>; nabihestefan@google.com
> >> Subject: Re: [PATCH v4 06/10] hw/arm/aspeed: Add support for loading
> >> vbootrom image via "-bios"
> >>
> >> Hello Jamin,
> >>
> >>> Based on the design of aspeed_install_boot_rom, users can place
> >>> their ROM code at the top of the image-bmc, and this function will
> >>> install image-bmc which included the user's ROM IMAGE at the
> >>> ASPEED_DEV_SPI_BOOT address.  For AST2600, users typically set the
> >>> boot address to 0x0 and boot directly from there.
> >>>
> >>> For AST2700, we introduced a vbootrom to simulate the ROM code and
> >>> the BOOTMCU SPL (RISC-V).
> >>
> >> Side question, is anyone working on the BOOTMCU SPL (RISC-V) models ?
> >> heterogeneous machines should be supported one day.
> >>
> >
> > Troy developed an initial implementation, but testing has not yet been
> > performed due to uncertainty around "how to share DRAM memory and
> controllers registers" between the RISC-V and the Cortex-A35 cores.
> > Furthermore, RISC-V interrupt support is currently not implemented.
> Could you explain a bit more the issues you are facing ? Single QEMU binary is
> expected to become a reality in the near future and the
> ast2700 models could benefit from it.
> 
There is a BootMCU which is a 32-bit RISC-V CPU, and its DRAM start address is 0x80000000 (32-bit address space).
The primary CPU is a Cortex-A35, and its DRAM start address is 0x400000000 (64-bit address space).

If the BootMCU writes 0x12345678 to address 0x80000000, the Cortex-A35 should be able to read 0x12345678 from address 0x400000000.
Similarly, if the Cortex-A35 writes 0x00ABCDEF to address 0x400000000, the BootMCU should be able to read 0x00ABCDEF from address 0x80000000.

Thanks-Jamin

> 
> Thanks,
> 
> C.
> 
> 

Re: [PATCH v4 06/10] hw/arm/aspeed: Add support for loading vbootrom image via "-bios"

[Philippe Mathieu-Daudé]

On 28/4/25 09:54, Jamin Lin wrote:
> Hi Cedric,
> 
>> Subject: Re: [PATCH v4 06/10] hw/arm/aspeed: Add support for loading
>> vbootrom image via "-bios"
>>
>> Hello Jamin,
>>
>> + Phil.
>>
>> On 4/23/25 09:02, Jamin Lin wrote:
>>> Hi Cedric,
>>>
>>>> Cc: Troy Lee <troy_lee@aspeedtech.com>; nabihestefan@google.com
>>>> Subject: Re: [PATCH v4 06/10] hw/arm/aspeed: Add support for loading
>>>> vbootrom image via "-bios"
>>>>
>>>> Hello Jamin,
>>>>
>>>>> Based on the design of aspeed_install_boot_rom, users can place
>>>>> their ROM code at the top of the image-bmc, and this function will
>>>>> install image-bmc which included the user's ROM IMAGE at the
>>>>> ASPEED_DEV_SPI_BOOT address.  For AST2600, users typically set the
>>>>> boot address to 0x0 and boot directly from there.
>>>>>
>>>>> For AST2700, we introduced a vbootrom to simulate the ROM code and
>>>>> the BOOTMCU SPL (RISC-V).
>>>>
>>>> Side question, is anyone working on the BOOTMCU SPL (RISC-V) models ?
>>>> heterogeneous machines should be supported one day.
>>>>
>>>
>>> Troy developed an initial implementation, but testing has not yet been
>>> performed due to uncertainty around "how to share DRAM memory and
>> controllers registers" between the RISC-V and the Cortex-A35 cores.
>>> Furthermore, RISC-V interrupt support is currently not implemented.
>> Could you explain a bit more the issues you are facing ? Single QEMU binary is
>> expected to become a reality in the near future and the
>> ast2700 models could benefit from it.
>>
> There is a BootMCU which is a 32-bit RISC-V CPU, and its DRAM start address is 0x80000000 (32-bit address space).
> The primary CPU is a Cortex-A35, and its DRAM start address is 0x400000000 (64-bit address space).
> 
> If the BootMCU writes 0x12345678 to address 0x80000000, the Cortex-A35 should be able to read 0x12345678 from address 0x400000000.
> Similarly, if the Cortex-A35 writes 0x00ABCDEF to address 0x400000000, the BootMCU should be able to read 0x00ABCDEF from address 0x80000000.

This shouldn't be a problem, the raspi machines have something similar.

I remember having an issue when displaying the address spaces on the
monitor, using your example, if you start mapping the dram on the rv
core, then the AS has some internal offset, and when you map it on the
arm cluster, the offset persists and you'd see it mapped at
0x3_8000_0000 while being at 0x4_0000_0000 (it is a bug). However the
accesses from the arm cores really hit 0x4_0000_0000 base: it is just
a display problem IIRC.

Maybe the thread around this issue was this one:
https://lore.kernel.org/qemu-devel/20210421144846.GI4440@xz-x1/

RE: [PATCH v4 06/10] hw/arm/aspeed: Add support for loading vbootrom image via "-bios"

[Jamin Lin]

Hi Cedric, Philippe

> Subject: Re: [PATCH v4 06/10] hw/arm/aspeed: Add support for loading
> vbootrom image via "-bios"
> 
> On 28/4/25 09:54, Jamin Lin wrote:
> > Hi Cedric,
> >
> >> Subject: Re: [PATCH v4 06/10] hw/arm/aspeed: Add support for loading
> >> vbootrom image via "-bios"
> >>
> >> Hello Jamin,
> >>
> >> + Phil.
> >>
> >> On 4/23/25 09:02, Jamin Lin wrote:
> >>> Hi Cedric,
> >>>
> >>>> Cc: Troy Lee <troy_lee@aspeedtech.com>; nabihestefan@google.com
> >>>> Subject: Re: [PATCH v4 06/10] hw/arm/aspeed: Add support for
> >>>> loading vbootrom image via "-bios"
> >>>>
> >>>> Hello Jamin,
> >>>>
> >>>>> Based on the design of aspeed_install_boot_rom, users can place
> >>>>> their ROM code at the top of the image-bmc, and this function will
> >>>>> install image-bmc which included the user's ROM IMAGE at the
> >>>>> ASPEED_DEV_SPI_BOOT address.  For AST2600, users typically set the
> >>>>> boot address to 0x0 and boot directly from there.
> >>>>>
> >>>>> For AST2700, we introduced a vbootrom to simulate the ROM code and
> >>>>> the BOOTMCU SPL (RISC-V).
> >>>>
> >>>> Side question, is anyone working on the BOOTMCU SPL (RISC-V) models ?
> >>>> heterogeneous machines should be supported one day.
> >>>>
> >>>
> >>> Troy developed an initial implementation, but testing has not yet
> >>> been performed due to uncertainty around "how to share DRAM memory
> >>> and
> >> controllers registers" between the RISC-V and the Cortex-A35 cores.
> >>> Furthermore, RISC-V interrupt support is currently not implemented.
> >> Could you explain a bit more the issues you are facing ? Single QEMU
> >> binary is expected to become a reality in the near future and the
> >> ast2700 models could benefit from it.
> >>
> > There is a BootMCU which is a 32-bit RISC-V CPU, and its DRAM start
> address is 0x80000000 (32-bit address space).
> > The primary CPU is a Cortex-A35, and its DRAM start address is
> 0x400000000 (64-bit address space).
> >
> > If the BootMCU writes 0x12345678 to address 0x80000000, the Cortex-A35
> should be able to read 0x12345678 from address 0x400000000.
> > Similarly, if the Cortex-A35 writes 0x00ABCDEF to address 0x400000000, the
> BootMCU should be able to read 0x00ABCDEF from address 0x80000000.
> 
> This shouldn't be a problem, the raspi machines have something similar.
> 
> I remember having an issue when displaying the address spaces on the monitor,
> using your example, if you start mapping the dram on the rv core, then the AS
> has some internal offset, and when you map it on the arm cluster, the offset
> persists and you'd see it mapped at
> 0x3_8000_0000 while being at 0x4_0000_0000 (it is a bug). However the
> accesses from the arm cores really hit 0x4_0000_0000 base: it is just a display
> problem IIRC.
> 
> Maybe the thread around this issue was this one:
> https://lore.kernel.org/qemu-devel/20210421144846.GI4440@xz-x1/

Thanks for your reply.
After QEMU adds support for a "single binary and this issue is resolved", I will study the single binary design and plan to enable the AST2700 to boot starting from the "BOOTMCU (RISC-V 32-bit)".
This is our long-term goal.

Jamin