From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:57068) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bhHqe-0000NI-Qh for qemu-devel@nongnu.org; Tue, 06 Sep 2016 10:59:45 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1bhHqW-00051D-E1 for qemu-devel@nongnu.org; Tue, 06 Sep 2016 10:59:35 -0400 Received: from mail-ua0-x229.google.com ([2607:f8b0:400c:c08::229]:34291) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bhHqW-00050i-8d for qemu-devel@nongnu.org; Tue, 06 Sep 2016 10:59:32 -0400 Received: by mail-ua0-x229.google.com with SMTP id u68so32035490uau.1 for ; Tue, 06 Sep 2016 07:59:29 -0700 (PDT) MIME-Version: 1.0 In-Reply-To: <1473167287-8326-2-git-send-email-peter.maydell@linaro.org> References: <1473167287-8326-1-git-send-email-peter.maydell@linaro.org> <1473167287-8326-2-git-send-email-peter.maydell@linaro.org> From: Peter Maydell Date: Tue, 6 Sep 2016 15:59:08 +0100 Message-ID: Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [PULL 01/14] ast2400: add a memory controller device model List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: QEMU Developers Cc: =?UTF-8?Q?C=C3=A9dric_Le_Goater?= On 6 September 2016 at 14:07, Peter Maydell wrot= e: > From: C=C3=A9dric Le Goater > > 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