Re: [PULL 59/68] aspeed: Add an AST2600 eval board

[Philippe Mathieu-Daudé]

On 10/16/19 2:41 PM, Cédric Le Goater wrote:
> On 16/10/2019 14:20, Philippe Mathieu-Daudé wrote:
> > Hi Peter,
> >
> > On 10/15/19 7:03 PM, Peter Maydell wrote:
> > > On Mon, 14 Oct 2019 at 17:05, Peter Maydell <address@hidden> wrote:
> > > > From: Cédric Le Goater <address@hidden>
> > > >
> > > > Signed-off-by: Cédric Le Goater <address@hidden>
> > > > Reviewed-by: Joel Stanley <address@hidden>
> > > > Message-id: address@hidden
> > > > Signed-off-by: Peter Maydell <address@hidden>
> > > > ---
> > > >    include/hw/arm/aspeed.h |  1 +
> > > >    hw/arm/aspeed.c         | 23 +++++++++++++++++++++++
> > > >    2 files changed, 24 insertions(+)
> > >
> > > > @@ -455,6 +467,17 @@ static const AspeedBoardConfig aspeed_boards[] = {
> > > >            .num_cs    = 2,
> > > >            .i2c_init  = witherspoon_bmc_i2c_init,
> > > >            .ram       = 512 * MiB,
> > > > +    }, {
> > > > +        .name      = MACHINE_TYPE_NAME("ast2600-evb"),
> > > > +        .desc      = "Aspeed AST2600 EVB (Cortex A7)",
> > > > +        .soc_name  = "ast2600-a0",
> > > > +        .hw_strap1 = AST2600_EVB_HW_STRAP1,
> > > > +        .hw_strap2 = AST2600_EVB_HW_STRAP2,
> > > > +        .fmc_model = "w25q512jv",
> > > > +        .spi_model = "mx66u51235f",
> > > > +        .num_cs    = 1,
> > > > +        .i2c_init  = ast2600_evb_i2c_init,
> > > > +        .ram       = 2 * GiB,
> > >
> > > Hi. I just discovered that this makes 'make check' fail on
> > > 32-bit systems, because you can't default to 2GB of RAM
> > > for a board:
> > >
> > > (armhf)pmaydell@mustang-maydell:~/qemu$
> > > ./build/all-a32/arm-softmmu/qemu-system-arm -M ast2600-evb
> > > qemu-system-arm: at most 2047 MB RAM can be simulated
> > >
> > > It's also a pretty rudely large amount of RAM to allocate
> > > by default: it caused 'make check' to fail on my OSX
> > > box, which is 64-bits but doesn't have huge swathes
> > > of free RAM.
> >
> > It is unlikely you use this board on a 32-bit system...
> >
> > You usually prefer to have modeled hardware matching real-life,
> > what about making this board not available on 32-bit systems
> > (we will soon have more boards like this):
> >
> >    #if HOST_LONG_BITS > 32
> >        {
> >            .name      = MACHINE_TYPE_NAME("ast2600-evb"),
> >            .desc      = "Aspeed AST2600 EVB (Cortex A7)",
> >            ...
> >        },
> >    #endif /* HOST_LONG_BITS > 32 */
>
> I sent a patch to lower the default RAM size to 1G but you can always
> increase it on the command line.
>
> Making the machine available seems a better choice but that's fine with
> me if we prefer to restrict its use to 64bits hosts. As you wish.

I'd rather keep models consistent with real hardware and not cripple 
them to test them on unlikely setups.