[PATCH v4 0/6] hw/arm/aspeed_ast2700-fc: Fix null pointer dereference

[PATCH v4 0/6] hw/arm/aspeed_ast2700-fc: Fix null pointer dereference

[Steven Lee via]

Clang's sanitizer reports a runtime error when booting with
'-net nic -net user', due to a null pointer being passed
to memory_region_find(), which subsequently triggers a crash in
flatview_lookup().

Root cause:
- Missing NIC configuration in the CA35 initialization.

Fix:
- Reduce ca35 ram size from 2GiB to 1GiB to align with ast2700a1-evb,
  where the ram-container is defined as 1GiB in its class.
- Add nic configuration in ast2700fc's ca35 init function.


v2:
- Split the CA35 memory mapping into a separate patch.
- Added a new patch to fix BMC memory mapping in the fby35 machine,
  which had a similar issue (unmapped system_memory).
- Removed Change-Id tag from commit messages

v3:
- Fixed issue with incorrectly inheriting TYPE_ASPEED_MACHINE.
- Rewrote the commit message for "Map BMC memory into system memory".
- Added more details to the commit message for "Fix unimplemented region
  overlap with VBootROM".

v4:
- Split the first patch "Fix null pointer dereference" into two patches.
- Remove the patch "hw/arm/aspeed_ast27x0: Fix unimplemented
  region overlap with vbootrom".

Steven Lee (6):
  hw/arm/aspeed_ast2700-fc: Fix null pointer dereference in ca35 init
  hw/arm/aspeed_ast2700-fc: Reduce ca35 ram size to align with ast2700a1
  hw/arm/aspeed_ast27x0: Fix unimplemented region overlap with vbootrom
  hw/arm/aspeed_ast27x0-fc: Map ca35 memory into system memory
  hw/arm/fby35: Map BMC memory into system memory
  docs: Remove ast2700fc from Aspeed family boards

 docs/system/arm/aspeed.rst |  2 +-
 hw/arm/aspeed_ast27x0-fc.c | 10 +++++++++-
 hw/arm/aspeed_ast27x0.c    |  4 ++--
 hw/arm/fby35.c             |  1 +
 4 files changed, 13 insertions(+), 4 deletions(-)

-- 
2.43.0

[PATCH v4 1/6] hw/arm/aspeed_ast2700-fc: Fix null pointer dereference in ca35 init

[Steven Lee via]

Clang's sanitizer reports a runtime error when booting with
'-net nic -net user', due to a null pointer being passed
to memory_region_find(), which subsequently triggers a crash in
flatview_lookup().

Root cause:
- Missing NIC configuration in the CA35 initialization.

Fix:
- Add nic configuration in ast2700fc's ca35 init function.

Signed-off-by: Steven Lee <steven_lee@aspeedtech.com>
---
 hw/arm/aspeed_ast27x0-fc.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/hw/arm/aspeed_ast27x0-fc.c b/hw/arm/aspeed_ast27x0-fc.c
index 125a3ade40..7bf4f2a52d 100644
--- a/hw/arm/aspeed_ast27x0-fc.c
+++ b/hw/arm/aspeed_ast27x0-fc.c
@@ -86,6 +86,13 @@ static void ast2700fc_ca35_init(MachineState *machine)
                                  AST2700FC_BMC_RAM_SIZE, &error_abort)) {
         return;
     }
+
+    for (int i = 0; i < sc->macs_num; i++) {
+        if (!qemu_configure_nic_device(DEVICE(&soc->ftgmac100[i]),
+                                       true, NULL)) {
+            break;
+        }
+    }
     if (!object_property_set_int(OBJECT(&s->ca35), "hw-strap1",
                                  AST2700FC_HW_STRAP1, &error_abort)) {
         return;
-- 
2.43.0

Re: [PATCH v4 1/6] hw/arm/aspeed_ast2700-fc: Fix null pointer dereference in ca35 init

[Cédric Le Goater]

On 5/22/25 11:16, Steven Lee wrote:
> Clang's sanitizer reports a runtime error when booting with
> '-net nic -net user', due to a null pointer being passed
> to memory_region_find(), which subsequently triggers a crash in
> flatview_lookup().

I don't see such errors.

> Root cause:
> - Missing NIC configuration in the CA35 initialization.
> 
> Fix:
> - Add nic configuration in ast2700fc's ca35 init function.

However it would be nice to have network support.

Could you please rephrase the commit log ?


Thanks,

C.



> 
> Signed-off-by: Steven Lee <steven_lee@aspeedtech.com>
> ---
>   hw/arm/aspeed_ast27x0-fc.c | 7 +++++++
>   1 file changed, 7 insertions(+)
> 
> diff --git a/hw/arm/aspeed_ast27x0-fc.c b/hw/arm/aspeed_ast27x0-fc.c
> index 125a3ade40..7bf4f2a52d 100644
> --- a/hw/arm/aspeed_ast27x0-fc.c
> +++ b/hw/arm/aspeed_ast27x0-fc.c
> @@ -86,6 +86,13 @@ static void ast2700fc_ca35_init(MachineState *machine)
>                                    AST2700FC_BMC_RAM_SIZE, &error_abort)) {
>           return;
>       }
> +
> +    for (int i = 0; i < sc->macs_num; i++) {
> +        if (!qemu_configure_nic_device(DEVICE(&soc->ftgmac100[i]),
> +                                       true, NULL)) {
> +            break;
> +        }
> +    }
>       if (!object_property_set_int(OBJECT(&s->ca35), "hw-strap1",
>                                    AST2700FC_HW_STRAP1, &error_abort)) {
>           return;

RE: [PATCH v4 1/6] hw/arm/aspeed_ast2700-fc: Fix null pointer dereference in ca35 init

[Steven Lee]

Hi Cédric,

> -----Original Message-----
> From: Cédric Le Goater <clg@kaod.org>
> Sent: Friday, May 23, 2025 4:09 PM
> To: Steven Lee <steven_lee@aspeedtech.com>; Peter Maydell
> <peter.maydell@linaro.org>; Troy Lee <leetroy@gmail.com>; Jamin Lin
> <jamin_lin@aspeedtech.com>; Andrew Jeffery
> <andrew@codeconstruct.com.au>; Joel Stanley <joel@jms.id.au>; open
> list:ASPEED BMCs <qemu-arm@nongnu.org>; open list:All patches CC here
> <qemu-devel@nongnu.org>
> Cc: Troy Lee <troy_lee@aspeedtech.com>; longzl2@lenovo.com; Yunlin Tang
> <yunlin.tang@aspeedtech.com>
> Subject: Re: [PATCH v4 1/6] hw/arm/aspeed_ast2700-fc: Fix null pointer
> dereference in ca35 init
> 
> On 5/22/25 11:16, Steven Lee wrote:
> > Clang's sanitizer reports a runtime error when booting with '-net nic
> > -net user', due to a null pointer being passed to
> > memory_region_find(), which subsequently triggers a crash in
> > flatview_lookup().
> 
> I don't see such errors.
> 
> > Root cause:
> > - Missing NIC configuration in the CA35 initialization.
> >
> > Fix:
> > - Add nic configuration in ast2700fc's ca35 init function.
> 
> However it would be nice to have network support.
> 
> Could you please rephrase the commit log ?
> 

Will rewrite commit log to "Add network support for ast2700fc"

Regards,
Steven

> >
> > Signed-off-by: Steven Lee <steven_lee@aspeedtech.com>
> > ---
> >   hw/arm/aspeed_ast27x0-fc.c | 7 +++++++
> >   1 file changed, 7 insertions(+)
> >
> > diff --git a/hw/arm/aspeed_ast27x0-fc.c b/hw/arm/aspeed_ast27x0-fc.c
> > index 125a3ade40..7bf4f2a52d 100644
> > --- a/hw/arm/aspeed_ast27x0-fc.c
> > +++ b/hw/arm/aspeed_ast27x0-fc.c
> > @@ -86,6 +86,13 @@ static void ast2700fc_ca35_init(MachineState
> *machine)
> >                                    AST2700FC_BMC_RAM_SIZE,
> &error_abort)) {
> >           return;
> >       }
> > +
> > +    for (int i = 0; i < sc->macs_num; i++) {
> > +        if (!qemu_configure_nic_device(DEVICE(&soc->ftgmac100[i]),
> > +                                       true, NULL)) {
> > +            break;
> > +        }
> > +    }
> >       if (!object_property_set_int(OBJECT(&s->ca35), "hw-strap1",
> >                                    AST2700FC_HW_STRAP1,
> &error_abort)) {
> >           return;

[PATCH v4 2/6] hw/arm/aspeed_ast2700-fc: Reduce ca35 ram size to align with ast2700a1

[Steven Lee via]

Reduce ca35 ram size from 2GiB to 1GiB to align with ast2700a1-evb,
where the ram-container is defined as 1GiB in its class.

Signed-off-by: Steven Lee <steven_lee@aspeedtech.com>
---
 hw/arm/aspeed_ast27x0-fc.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/arm/aspeed_ast27x0-fc.c b/hw/arm/aspeed_ast27x0-fc.c
index 7bf4f2a52d..f8cb632bca 100644
--- a/hw/arm/aspeed_ast27x0-fc.c
+++ b/hw/arm/aspeed_ast27x0-fc.c
@@ -48,7 +48,7 @@ struct Ast2700FCState {
     bool mmio_exec;
 };
 
-#define AST2700FC_BMC_RAM_SIZE (2 * GiB)
+#define AST2700FC_BMC_RAM_SIZE (1 * GiB)
 #define AST2700FC_CM4_DRAM_SIZE (32 * MiB)
 
 #define AST2700FC_HW_STRAP1 0x000000C0
-- 
2.43.0

Re: [PATCH v4 2/6] hw/arm/aspeed_ast2700-fc: Reduce ca35 ram size to align with ast2700a1

[Cédric Le Goater]

On 5/22/25 11:16, Steven Lee wrote:
> Reduce ca35 ram size from 2GiB to 1GiB to align with ast2700a1-evb,
> where the ram-container is defined as 1GiB in its class.
> 
> Signed-off-by: Steven Lee <steven_lee@aspeedtech.com>


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

Thanks,

C.


> ---
>   hw/arm/aspeed_ast27x0-fc.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/hw/arm/aspeed_ast27x0-fc.c b/hw/arm/aspeed_ast27x0-fc.c
> index 7bf4f2a52d..f8cb632bca 100644
> --- a/hw/arm/aspeed_ast27x0-fc.c
> +++ b/hw/arm/aspeed_ast27x0-fc.c
> @@ -48,7 +48,7 @@ struct Ast2700FCState {
>       bool mmio_exec;
>   };
>   
> -#define AST2700FC_BMC_RAM_SIZE (2 * GiB)
> +#define AST2700FC_BMC_RAM_SIZE (1 * GiB)
>   #define AST2700FC_CM4_DRAM_SIZE (32 * MiB)
>   
>   #define AST2700FC_HW_STRAP1 0x000000C0

[PATCH v4 3/6] hw/arm/aspeed_ast27x0: Fix unimplemented region overlap with vbootrom

[Steven Lee via]

The unimplemented memory region overlaps with the VBootROM address
range, causing incorrect memory layout.

This patch adjusts the size and start address of the unimplemented
region to avoid collision. The IO memory region (ASPEED_DEV_IOMEM) is
now moved to 0x20000 to reserve space for VBootROM at 0x0.

Although the memory range 0x20000 - 0x10000000 is undefined in the
datasheet and should not be required, further testing shows OP-TEE
or U-Boot may access 0x400000 during early boot.
Removing the unimplemented region causes firmware hangs.

To prevent unexpected accesses, retain the region as a safeguard.

Signed-off-by: Steven Lee <steven_lee@aspeedtech.com>
---
 hw/arm/aspeed_ast27x0.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/hw/arm/aspeed_ast27x0.c b/hw/arm/aspeed_ast27x0.c
index 1974a25766..bb61c30cf4 100644
--- a/hw/arm/aspeed_ast27x0.c
+++ b/hw/arm/aspeed_ast27x0.c
@@ -23,14 +23,14 @@
 #include "qobject/qlist.h"
 #include "qemu/log.h"
 
-#define AST2700_SOC_IO_SIZE          0x01000000
+#define AST2700_SOC_IO_SIZE          0x00FE0000
 #define AST2700_SOC_IOMEM_SIZE       0x01000000
 #define AST2700_SOC_DPMCU_SIZE       0x00040000
 #define AST2700_SOC_LTPI_SIZE        0x01000000
 
 static const hwaddr aspeed_soc_ast2700_memmap[] = {
-    [ASPEED_DEV_IOMEM]     =  0x00000000,
     [ASPEED_DEV_VBOOTROM]  =  0x00000000,
+    [ASPEED_DEV_IOMEM]     =  0x00020000,
     [ASPEED_DEV_SRAM]      =  0x10000000,
     [ASPEED_DEV_DPMCU]     =  0x11000000,
     [ASPEED_DEV_IOMEM0]    =  0x12000000,
-- 
2.43.0

Re: [PATCH v4 3/6] hw/arm/aspeed_ast27x0: Fix unimplemented region overlap with vbootrom

[Cédric Le Goater]

On 5/22/25 11:16, Steven Lee wrote:
> The unimplemented memory region overlaps with the VBootROM address
> range, causing incorrect memory layout.
> 
> This patch adjusts the size and start address of the unimplemented
> region to avoid collision. The IO memory region (ASPEED_DEV_IOMEM) is
> now moved to 0x20000 to reserve space for VBootROM at 0x0.
> 
> Although the memory range 0x20000 - 0x10000000 is undefined in the
> datasheet and should not be required, further testing shows OP-TEE
> or U-Boot may access 0x400000 during early boot.
> Removing the unimplemented region causes firmware hangs.
> > To prevent unexpected accesses, retain the region as a safeguard.
> 
> Signed-off-by: Steven Lee <steven_lee@aspeedtech.com>



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

Thanks,

C.




> ---
>   hw/arm/aspeed_ast27x0.c | 4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/hw/arm/aspeed_ast27x0.c b/hw/arm/aspeed_ast27x0.c
> index 1974a25766..bb61c30cf4 100644
> --- a/hw/arm/aspeed_ast27x0.c
> +++ b/hw/arm/aspeed_ast27x0.c
> @@ -23,14 +23,14 @@
>   #include "qobject/qlist.h"
>   #include "qemu/log.h"
>   
> -#define AST2700_SOC_IO_SIZE          0x01000000
> +#define AST2700_SOC_IO_SIZE          0x00FE0000
>   #define AST2700_SOC_IOMEM_SIZE       0x01000000
>   #define AST2700_SOC_DPMCU_SIZE       0x00040000
>   #define AST2700_SOC_LTPI_SIZE        0x01000000
>   
>   static const hwaddr aspeed_soc_ast2700_memmap[] = {
> -    [ASPEED_DEV_IOMEM]     =  0x00000000,
>       [ASPEED_DEV_VBOOTROM]  =  0x00000000,
> +    [ASPEED_DEV_IOMEM]     =  0x00020000,
>       [ASPEED_DEV_SRAM]      =  0x10000000,
>       [ASPEED_DEV_DPMCU]     =  0x11000000,
>       [ASPEED_DEV_IOMEM0]    =  0x12000000,

[PATCH v4 4/6] hw/arm/aspeed_ast27x0-fc: Map ca35 memory into system memory

[Steven Lee via]

Map the CA35 memory region as a subregion of system_memory to ensure
a valid FlatView. This prevents failures in APIs that rely on the
global memory view, such as rom_check_and_register_reset().

Signed-off-by: Steven Lee <steven_lee@aspeedtech.com>
Reviewed-by: Cédric Le Goater <clg@redhat.com>
---
 hw/arm/aspeed_ast27x0-fc.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/hw/arm/aspeed_ast27x0-fc.c b/hw/arm/aspeed_ast27x0-fc.c
index f8cb632bca..7087be4288 100644
--- a/hw/arm/aspeed_ast27x0-fc.c
+++ b/hw/arm/aspeed_ast27x0-fc.c
@@ -68,6 +68,7 @@ static void ast2700fc_ca35_init(MachineState *machine)
 
     memory_region_init(&s->ca35_memory, OBJECT(&s->ca35), "ca35-memory",
                        UINT64_MAX);
+    memory_region_add_subregion(get_system_memory(), 0, &s->ca35_memory);
 
     if (!memory_region_init_ram(&s->ca35_dram, OBJECT(&s->ca35), "ca35-dram",
                                 AST2700FC_BMC_RAM_SIZE, &error_abort)) {
-- 
2.43.0

[PATCH v4 5/6] hw/arm/fby35: Map BMC memory into system memory

[Steven Lee via]

Add the BMC memory region as a subregion of system_memory so that
modules relying on system memory can operate correctly.

Signed-off-by: Steven Lee <steven_lee@aspeedtech.com>
Reviewed-by: Cédric Le Goater <clg@redhat.com>
---
 hw/arm/fby35.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/hw/arm/fby35.c b/hw/arm/fby35.c
index e123fa69e1..c14fc2efe9 100644
--- a/hw/arm/fby35.c
+++ b/hw/arm/fby35.c
@@ -77,6 +77,7 @@ static void fby35_bmc_init(Fby35State *s)
 
     memory_region_init(&s->bmc_memory, OBJECT(&s->bmc), "bmc-memory",
                        UINT64_MAX);
+    memory_region_add_subregion(get_system_memory(), 0, &s->bmc_memory);
     memory_region_init_ram(&s->bmc_dram, OBJECT(&s->bmc), "bmc-dram",
                            FBY35_BMC_RAM_SIZE, &error_abort);
 
-- 
2.43.0

[PATCH v4 6/6] docs: Remove ast2700fc from Aspeed family boards

[Steven Lee via]

The ast2700fc machine is now covered in the dedicated ast2700-evb
section. Listing it in the general Aspeed board family list is
redundant.

Signed-off-by: Steven Lee <steven_lee@aspeedtech.com>
Reviewed-by: Cédric Le Goater <clg@redhat.com>
---
 docs/system/arm/aspeed.rst | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/docs/system/arm/aspeed.rst b/docs/system/arm/aspeed.rst
index 58a8020eec..43d27d83cb 100644
--- a/docs/system/arm/aspeed.rst
+++ b/docs/system/arm/aspeed.rst
@@ -1,4 +1,4 @@
-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``)
+Aspeed family boards (``ast2500-evb``, ``ast2600-evb``, ``ast2700-evb``, ``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``)
 =================================================================================================================================================================================================================================================================================================================================================================================================================================
 
 The QEMU Aspeed machines model BMCs of various OpenPOWER systems and
-- 
2.43.0