From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:59819) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UXK8z-0000Mj-GE for qemu-devel@nongnu.org; Tue, 30 Apr 2013 19:39:35 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1UXK8w-0003lQ-Mm for qemu-devel@nongnu.org; Tue, 30 Apr 2013 19:39:33 -0400 Date: Tue, 30 Apr 2013 18:08:49 -0500 From: Scott Wood References: <1367145009-14512-1-git-send-email-tiejun.chen@windriver.com> <1367263124.32182.7@snotra> <82C960D7DF4A1F47B94FC1C67A29BEE3669305D1@ALA-MBB.corp.ad.wrs.com> In-Reply-To: <82C960D7DF4A1F47B94FC1C67A29BEE3669305D1@ALA-MBB.corp.ad.wrs.com> (from Tiejun.Chen@windriver.com on Tue Apr 30 18:03:54 2013) Message-ID: <1367363329.24133.12@snotra> MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii"; delsp=Yes; format=Flowed Content-Disposition: inline Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [Qemu-ppc] [v1][PATCH 1/1] PPC: e500: correct params->ram_size with ram_size List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: "Chen, Tiejun" Cc: "qemu-ppc@nongnu.org" , "agraf@suse.de" , "qemu-devel@nongnu.org" On 04/30/2013 06:03:54 PM, Chen, Tiejun wrote: > > -----Original Message----- > > From: Scott Wood [mailto:scottwood@freescale.com] > > Sent: Tuesday, April 30, 2013 3:19 AM > > To: Chen, Tiejun > > Cc: agraf@suse.de; qemu-ppc@nongnu.org; qemu-devel@nongnu.org > > Subject: Re: [Qemu-ppc] [v1][PATCH 1/1] PPC: e500: correct > > params->ram_size with ram_size > > > > On 04/28/2013 05:30:09 AM, Tiejun Chen wrote: > > > We should sync params->ram_size after we fixup memory size on a > > > alignment boundary. Otherwise Guest would exceed the actual memory > > > region. > > > > > > Signed-off-by: Tiejun Chen > > > --- > > > hw/ppc/e500.c | 2 ++ > > > 1 file changed, 2 insertions(+) > > > > > > diff --git a/hw/ppc/e500.c b/hw/ppc/e500.c index c1bdb6b..145da0e > > > 100644 > > > --- a/hw/ppc/e500.c > > > +++ b/hw/ppc/e500.c > > > @@ -523,6 +523,8 @@ void ppce500_init(PPCE500Params *params) > > > > > > /* Fixup Memory size on a alignment boundary */ > > > ram_size &=3D ~(RAM_SIZES_ALIGN - 1); > > > + /* Sync this for the system. */ > > > + params->ram_size =3D ram_size; > > > > Could you explain this further? When does params->ram_size > > ever get used after this point? >=20 > In that case we have to create a dtb without passing an extra dtb, we =20 > always use params->ram_size inside ppce500_load_device_tree(), >=20 > ppce500_load_device_tree() > { > ... > uint64_t mem_reg_property[] =3D { 0, cpu_to_be64(params->ram_size) }; OK, from reading the patch it looked like this was happening before you =20 modify params->ram_size, but it's a separate function that gets called =20 later. The comment doesn't make much sense to me -- it's not passing =20 any information back to "the system" (which I'd interpret as generic =20 QEMU code, if anything, which is why I thought you were trying to do =20 this for the benefit of the caller), just making sure later e500 =20 platform code does the right thing when generating the device tree. -Scott=