Re: [PATCH v5 09/79] arm/aspeed: actually check RAM size

[Richard Henderson]

On 2/17/20 9:33 AM, Igor Mammedov wrote:
> It's supposed that SOC will check if "-m" provided
> RAM size is valid by setting "ram-size" property and
> then board would read back valid (possibly corrected
> value) to map RAM MemoryReging with valid size.
> It isn't doing so, since check is called only
> indirectly from
>   aspeed_sdmc_reset()->asc->compute_conf()
> or much later when guest writes to configuration
> register.
> 
> So depending on "-m" value QEMU end-ups with a warning
> and an invalid MemoryRegion size allocated and mapped.
> (examples:
>  -M ast2500-evb -m 1M
>     0000000080000000-000000017ffffffe (prio 0, i/o): aspeed-ram-container
>       0000000080000000-00000000800fffff (prio 0, ram): ram
>       0000000080100000-00000000bfffffff (prio 0, i/o): max_ram
>  -M ast2500-evb -m 3G
>     0000000080000000-000000017ffffffe (prio 0, i/o): aspeed-ram-container
>       0000000080000000-000000013fffffff (prio 0, ram): ram
>       [DETECTED OVERFLOW!] 0000000140000000-00000000bfffffff (prio 0, i/o): 
> max_ram
> )
> On top of that sdmc falls back and reports to guest
> "default" size, it thinks machine should have.
> 
> This patch makes ram-size check actually work and
> changes behavior from a warning later on during
> machine reset to error_fatal at the moment SOC.ram-size
> is set so user will have to fix RAM size on CLI
> to start machine.
> 
> It also gets out of the way mutable ram-size logic,
> so we could consolidate RAM allocation logic around
> pre-allocated hostmem backend (supplied by user or
> auto created by generic machine code depending on
> supplied -m/mem-path/mem-prealloc options.
> 
> Signed-off-by: Igor Mammedov <address@hidden>
> Reviewed-by: Cédric Le Goater <address@hidden>
> ---
> v3:
>   * replace
>      [PATCH v2 07/86] arm:aspeed: convert valid RAM sizes to data
>      [PATCH v2 08/86] arm:aspeed: actually check RAM size
>     with a simplified variant that adds ram_size check to sdmc.ram-size
>     property
>   * use g_assert_not_reached() in default branch
>     (Cédric Le Goater <address@hidden>)
> ---

Reviewed-by: Richard Henderson <address@hidden>


r~