[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-ppc] [v1][PATCH 1/1] PPC: e500: correct params->ram_size with
From: |
Chen, Tiejun |
Subject: |
Re: [Qemu-ppc] [v1][PATCH 1/1] PPC: e500: correct params->ram_size with ram_size |
Date: |
Tue, 30 Apr 2013 23:31:41 +0000 |
> -----Original Message-----
> From: Scott Wood [mailto:address@hidden
> Sent: Wednesday, May 01, 2013 7:09 AM
> To: Chen, Tiejun
> Cc: address@hidden; address@hidden; address@hidden
> 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:address@hidden
> > > Sent: Tuesday, April 30, 2013 3:19 AM
> > > To: Chen, Tiejun
> > > Cc: address@hidden; address@hidden; address@hidden
> > > 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 <address@hidden>
> > > > ---
> > > > 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
>