On 6 September 2016 at 14:07, Peter Maydell <peter.mayd...@linaro.org> wrote:
> From: Cédric Le Goater <c...@kaod.org>
>
> The uboot in the previous release of the SDK was using a hardcoded
> value for memory size. This is not true anymore, the value is now
> retrieved from the memory controller.
>
> Below is a model for this device, only supporting unlock and
> configuration. Without it, we endup running a guest with 64MB, which
> is a bit low nowdays. It uses a 'silicon-rev' property and ram_size to
> build a default value. Some bits should be linked to SCU strapping
> registers but it seems a bit complex to add for the current need.
>
> The model is ready for the AST2500 SOC.
> +static int ast2400_rambits(void)
> +{
> + switch (ram_size >> 20) {
> + case 64:
> + return ASPEED_SDMC_DRAM_64MB;
> + case 128:
> + return ASPEED_SDMC_DRAM_128MB;
> + case 256:
> + return ASPEED_SDMC_DRAM_256MB;
> + case 512:
> + return ASPEED_SDMC_DRAM_512MB;
> + default:
> + qemu_log_mask(LOG_GUEST_ERROR, "%s: Invalid RAM size: 0x%"
> + HWADDR_PRIx "\n", __func__, ram_size);
> + break;
This doesn't compile on 32-bit systems, because ram_size
is not a hwaddr, it's a ram_addr_t, and it needs a different
format specifier.
I'll fix this up locally and resend the pullreq.
However, I notice looking at this code more closely that it's
using the global ram_size to determine the behaviour of the
device. That seems a bit dubious to me, we don't have other
devices that look at global config settings like that.
Can you look at doing a followup patch where the board level
code tells the memory controller how much RAM it has directly,
please?
(Also it seems wrong to call this a GUEST_ERROR, because the
thing setting the ram_size is the user running QEMU (possibly
in conjunction with any restrictions imposed by the board
model code), not the guest code.)
thanks
-- PMM
