Attach CA35 memory to system_memory to ensure a valid FlatView.
Without this, dma_memory_write() used by ftgmac fail silently,
causing dhcp to break on ast2700fc, as flatview_write() returns
an error when system_memory is empty.

Signed-off-by: Steven Lee <steven_lee@aspeedtech.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 ff64605663..ccba5fc8a1 100644
--- a/hw/arm/aspeed_ast27x0-fc.c
+++ b/hw/arm/aspeed_ast27x0-fc.c
@@ -69,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);
 
     if (!memory_region_init_ram(&s->ca35_dram, OBJECT(&s->ca35), "ca35-dram",
                                 AST2700FC_BMC_RAM_SIZE, &error_abort)) {
-- 
2.43.0

Hello Steven,

On 5/14/25 11:03, Steven Lee wrote:
> Attach CA35 memory to system_memory to ensure a valid FlatView.
> Without this, dma_memory_write() used by ftgmac fail silently,
> causing dhcp to break on ast2700fc, as flatview_write() returns
> an error when system_memory is empty.

The change below fixes the network DMA transactions indeed but I think
this case can be addressed differently.

The transactions on address_space_memory in the ftgmac100 device model
should be replaced by transactions on a local address space which would
be initialized from a memory region passed to the model with a property.
This is very similar to what we do in the Aspeed SMC model. Since it is
more work, it can be addressed separately and later.

However, let's keep the change below for all other places which are
difficult to address, like rom_check_and_register_reset(). The commit
should be rephrased.

Thanks,

C.



> Signed-off-by: Steven Lee <steven_lee@aspeedtech.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 ff64605663..ccba5fc8a1 100644
> --- a/hw/arm/aspeed_ast27x0-fc.c
> +++ b/hw/arm/aspeed_ast27x0-fc.c
> @@ -69,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);
>   
>       if (!memory_region_init_ram(&s->ca35_dram, OBJECT(&s->ca35), "ca35-dram",
>                                   AST2700FC_BMC_RAM_SIZE, &error_abort)) {

Hi Cédric,

> -----Original Message-----
> From: Cédric Le Goater <clg@redhat.com>
> Sent: Wednesday, May 14, 2025 11:32 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 v2 3/5] hw/arm/aspeed_ast27x0-fc: Map ca35 memory
> into system memory
> 
> Hello Steven,
> 
> On 5/14/25 11:03, Steven Lee wrote:
> > Attach CA35 memory to system_memory to ensure a valid FlatView.
> > Without this, dma_memory_write() used by ftgmac fail silently, causing
> > dhcp to break on ast2700fc, as flatview_write() returns an error when
> > system_memory is empty.
> 
> The change below fixes the network DMA transactions indeed but I think this
> case can be addressed differently.
> 
> The transactions on address_space_memory in the ftgmac100 device model
> should be replaced by transactions on a local address space which would be
> initialized from a memory region passed to the model with a property.
> This is very similar to what we do in the Aspeed SMC model. Since it is more
> work, it can be addressed separately and later.
> 
> However, let's keep the change below for all other places which are difficult to
> address, like rom_check_and_register_reset(). The commit should be
> rephrased.
> 


Thanks for the suggestion, I will rewrite the commit message

Regards,
Steven

> 
> 
> > Signed-off-by: Steven Lee <steven_lee@aspeedtech.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 ff64605663..ccba5fc8a1 100644
> > --- a/hw/arm/aspeed_ast27x0-fc.c
> > +++ b/hw/arm/aspeed_ast27x0-fc.c
> > @@ -69,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);
> >
> >       if (!memory_region_init_ram(&s->ca35_dram, OBJECT(&s->ca35),
> "ca35-dram",
> >                                   AST2700FC_BMC_RAM_SIZE,
> > &error_abort)) {