> -----Original Message----- > From: Scott Wood [mailto:scottw...@freescale.com] > Sent: Wednesday, May 01, 2013 7:09 AM > To: Chen, Tiejun > Cc: ag...@suse.de; qemu-...@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/30/2013 06:03:54 PM, Chen, Tiejun wrote: > > > -----Original Message----- > > > From: Scott Wood [mailto:scottw...@freescale.com] > > > Sent: Tuesday, April 30, 2013 3:19 AM > > > To: Chen, Tiejun > > > Cc: ag...@suse.de; qemu-...@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 <tiejun.c...@windriver.com> > > > > --- > > > > 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 &= ~(RAM_SIZES_ALIGN - 1); > > > > + /* Sync this for the system. */ > > > > + params->ram_size = ram_size; > > > > > > Could you explain this further? When does > params->ram_size ever get > > > used after this point? > > > > In that case we have to create a dtb without passing an > extra dtb, we > > always use params->ram_size inside ppce500_load_device_tree(), > > > > ppce500_load_device_tree() > > { > > ... > > uint64_t mem_reg_property[] = { 0, cpu_to_be64(params->ram_size) }; > > OK, from reading the patch it looked like this was happening > before you modify params->ram_size, but it's a separate
Yes. > function that gets called later. The comment doesn't make Are you saying I should replace cpu_to_be64(params->ram_size) with cpu_to_be64(ram_size) directly in ppce500_load_device_tree()? But as I understand we should sync this global argument after we fixup something associated to that since params->ram_size may impact on something else in the future. Tiejun > much sense to me -- it's not passing any information back to > "the system" (which I'd interpret as generic QEMU code, if > anything, which is why I thought you were trying to do this > for the benefit of the caller), just making sure later e500 > platform code does the right thing when generating the device tree. > > -Scott >
