Hi Cédric,
> -----Original Message-----
> From: Cédric Le Goater <c...@kaod.org>
> Sent: Monday, May 12, 2025 4:09 PM
> To: Steven Lee <steven_...@aspeedtech.com>; Peter Maydell
> <peter.mayd...@linaro.org>; Troy Lee <leet...@gmail.com>; Jamin Lin
> <jamin_...@aspeedtech.com>; Andrew Jeffery
> <and...@codeconstruct.com.au>; Joel Stanley <j...@jms.id.au>; open
> list:ASPEED BMCs <qemu-...@nongnu.org>; open list:All patches CC here
> <qemu-devel@nongnu.org>
> Cc: Troy Lee <troy_...@aspeedtech.com>; long...@lenovo.com; Yunlin Tang
> <yunlin.t...@aspeedtech.com>
> Subject: Re: [PATCH v1 1/3] hw/arm/aspeed_ast2700-fc: Fix null pointer
> dereference in ca35 init
>
> On 5/7/25 12:10, 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().
> >
> > The root cause is that CA35 memory region is not mapped to system
> > memory. In addition, unconfigured NICs (due to missing peers) lead to
> > a cascade of warnings and possibly misbehavior.
> >
> > Fix by:
> > - Reduce ca35 ram size to 1GiB to match the ast2700a1-evb.
> > - Map ca35_memory into system memory
> > - Add nic configuration in ast2700fc's ca35 init function.
> >
> > Signed-off-by: Steven Lee <steven_...@aspeedtech.com>
> > Change-Id: Id9c0e6f16861c64a11f6299afb6ef02eb4086041
>
> As said earlier, please try to remove these tags.
>
Will remove it.
> > ---
> > hw/arm/aspeed_ast27x0-fc.c | 16 ++++++++++++++--
> > 1 file changed, 14 insertions(+), 2 deletions(-)
> >
> > diff --git a/hw/arm/aspeed_ast27x0-fc.c b/hw/arm/aspeed_ast27x0-fc.c
> > index 125a3ade40..ccba5fc8a1 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)
>
> why ?
>
I noticed that the ast2700-a1 class sets the RAM size to 1 GiB.
To prevent ca35-dram from exceeding the bounds of the ram-container, I reduced
the RAM size accordingly.
0000000400000000-000000043fffffff (prio 0, i/o): ram-container
0000000400000000-000000043fffffff (prio 0, ram): ca35-dram
> > #define AST2700FC_CM4_DRAM_SIZE (32 * MiB)
> >
> > #define AST2700FC_HW_STRAP1 0x000000C0 @@ -59,6 +59,7 @@ struct
> > Ast2700FCState {
> > static void ast2700fc_ca35_init(MachineState *machine)
> > {
> > Ast2700FCState *s = AST2700A1FC(machine);
> > + AspeedMachineClass *amc =
> ASPEED_MACHINE_GET_CLASS(machine);
> > AspeedSoCState *soc;
> > AspeedSoCClass *sc;
> >
> > @@ -68,6 +69,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);
>
> I think this belongs to another patch. Please also modify the fby35 machine
> which suffers from the same problem regarding the global system memory
> usage.
>
I will split this into a separate patch, and prepare another patch to update
the fby35 machine as well.
Thanks,
Steven
>
> > if (!memory_region_init_ram(&s->ca35_dram, OBJECT(&s->ca35),
> "ca35-dram",
> > AST2700FC_BMC_RAM_SIZE,
> > &error_abort)) { @@ -86,6 +88,14 @@ static void
> ast2700fc_ca35_init(MachineState *machine)
> > AST2700FC_BMC_RAM_SIZE,
> &error_abort)) {
> > return;
> > }
> > +
> > + for (int i = 0; i < sc->macs_num; i++) {
> > + if ((amc->macs_mask & (1 << i)) &&
> > + !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;
> > @@ -171,6 +181,7 @@ static void ast2700fc_init(MachineState *machine)
> > static void ast2700fc_class_init(ObjectClass *oc, const void *data)
> > {
> > MachineClass *mc = MACHINE_CLASS(oc);
> > + AspeedMachineClass *amc = ASPEED_MACHINE_CLASS(oc);
> >
> > mc->alias = "ast2700fc";
> > mc->desc = "ast2700 full core support"; @@ -178,12 +189,13 @@
> > static void ast2700fc_class_init(ObjectClass *oc, const void *data)
> > mc->no_floppy = 1;
> > mc->no_cdrom = 1;
> > mc->min_cpus = mc->max_cpus = mc->default_cpus = 6;
> > + amc->macs_mask = ASPEED_MAC0_ON | ASPEED_MAC1_ON |
> > + ASPEED_MAC2_ON;
> > }
> >
> > static const TypeInfo ast2700fc_types[] = {
> > {
> > .name = MACHINE_TYPE_NAME("ast2700fc"),
> > - .parent = TYPE_MACHINE,
> > + .parent = TYPE_ASPEED_MACHINE,
> > .class_init = ast2700fc_class_init,
> > .instance_size = sizeof(Ast2700FCState),
> > },
