Re: [Qemu-devel] [Qemu-ppc] [v1][PATCH 1/1] PPC: e500: correct params->ram_size with ram_size

[Chen, Tiejun]

> -----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) };
...
qemu_devtree_setprop(fdt, "/memory", "reg", mem_reg_property, 
sizeof(mem_reg_property));

So occasionally if we don't pass a aligned memory size with "-m" like "-m 500", 
then Guest still think it owns 500M but we register memory with ram_size 
actually,

ppce500_init()
{
...
/* Fixup Memory size on a alignment boundary */
ram_size &= ~(RAM_SIZES_ALIGN - 1); ==> 448M

/* Register Memory */
memory_region_init_ram(ram, "mpc8544ds.ram", ram_size);

Guest would trigger TLB error when Guest allocate to access somewhere between 
448M and 500M, its always failed since KVM hasn't that memory slot at all.

So we need to sync params->ram_size with ram_size.

Tiejun