On Fri, 2008-03-14 at 14:16 -0500, Hollis Blanchard wrote:
> On Fri, 2008-03-14 at 12:09 -0500, Jerone Young wrote:
> > # HG changeset patch
> > # User Jerone Young <[EMAIL PROTECTED]>
> > # Date 1205514174 18000
> > # Branch merge
> > # Node ID 3060b75a9597d4ab67c23871df41fc5e5476df2b
> > # Parent  63237bde74818a5dc3cdb1baee781dab101290ce
> > Add ability to specify ram on command line for bamboo board model
> > 
> > This patch adds the ability to now specify ram sizes on the command line.
> > Due to the nature of the code there are restictions on exactly how
> > much ram and the multiple that the size must match.
> > 
> > Signed-off-by: Jerone Young <[EMAIL PROTECTED]>
> > 
> > diff --git a/qemu/hw/ppc440_bamboo.c b/qemu/hw/ppc440_bamboo.c
> > --- a/qemu/hw/ppc440_bamboo.c
> > +++ b/qemu/hw/ppc440_bamboo.c
> > @@ -40,32 +40,50 @@ void bamboo_init(ram_addr_t ram_size, in
> >     target_ulong dt_base=0;
> >     void *fdt;
> >     int ret;
> > +   unsigned long ram_sticks[] = {0, 0}; /* Value will be in bytes */
> > +   ram_addr_t tmp_ram_size;
> > +   int ram_stick_sizes[] = {256, 128, 64, 32, 16, 8 }; /* in Mega bytes */
> > +   int i=0, k=0;
> > 
> >     uint32_t cpu_freq;
> >     uint32_t timebase_freq;
> > 
> >     printf("%s: START\n", __func__);
> > 
> > -   /* Setup Memory */
> > -   if (ram_size) {
> > -           printf("Ram size specified on command line is %i bytes\n",
> > -                                                           (int)ram_size);
> > -           printf("WARNING: RAM is hard coded to 144MB\n");
> > -   }
> > -   else {
> > -           printf("Using defualt ram size of %iMB\n",
> > -                                           ((int)ram_size/1024)/1024);
> > +   /* Setup Memory */
> > +   printf("Ram size passed is: %i MB\n",
> > +                           ((int)ram_size/1024)/1024);             
> > + 
> > +   tmp_ram_size = ram_size;
> > +
> > +   for (i=0; i < (sizeof(ram_sticks)/sizeof(unsigned long)); i++)
> > +   {
> > +           for (k=0; k < (sizeof(ram_stick_sizes)/sizeof(int)); k++)
> > +           {
> > +                   if ((((tmp_ram_size/1024)/1024)/ram_stick_sizes[k]) > 0)
> > +                   {
> > +                           ram_sticks[i] = ram_stick_sizes[k]*1024*1024;
> > +                           tmp_ram_size -= ram_stick_sizes[k]*1024*1024; 
> > +                           break;
> > +                   }
> > +           }
> > +   
> >     }
> 
> Please match the curly brace syntax used everywhere else.
> 
> Also, I don't think it makes any sense to multiply and divide by
> 1024*1024 everywhere; just use the proper units to begin with. Try N<<20
> to represent N MB.
> 
> "tmp_ram_size" is also unnecessary; just subtract from "ram_size" and
> see if it's non-zero at the end.
> 
> > -   /* Each bank can only have memory in configurations of
> > -    *   16MB, 32MB, 64MB, 128MB, or 256MB
> > -    */
> > -   ram_bases[0] = 0x0;
> > -   ram_sizes[0] = 0x08000000;
> > -   ram_bases[1] = 0x0;
> > -   ram_sizes[1] = 0x01000000;
> > +   if (tmp_ram_size)
> > +           printf("WARNING: %i left over memory is ram\n", 
> > +                   (tmp_ram_size/1024)/1024);
> > 
> > -   printf("Ram size of domain is %d bytes\n", (int)ram_size);
> > +   /* Each bank can only have memory in configurations of
> > +    *   4MB, 8MB, 16MB, 32MB, 64MB, 128MB, or 256MB
> > +    *   why? see sdram_bcr()
> > +    *   
> > +    *   Max of 512MB
> > +    */
> > +   ram_bases[0] = 0x0;
> > +   ram_sizes[0] = ram_sticks[0];
> > +   ram_bases[1] = 0x0;
> > +   ram_sizes[1] = ram_sticks[1];
> 
> Why keep a separate ram_sticks[] array? Just operate directly on
> ram_sizes[], and while you're at it stop hardcoding 2 entries: the SDRAM
> controller can emulate all 4.

Sure can just manipulate these directly. As for the ram_bases have been
this way since day 1. If it's works don't break it :-). I'll look into
the SDRAM controller doing 4 slots. 

> 
> Also, ram_bases[1] here is very wrong; that definitely needs to be
> fixed.

I need to see exactly what these ram_bases mean in the emulation code.
This could be right, as I believe this is how it's done in other qemu
code? I'll look and see.

> 


-------------------------------------------------------------------------
This SF.net email is sponsored by: Microsoft
Defy all challenges. Microsoft(R) Visual Studio 2008.
http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/
_______________________________________________
kvm-devel mailing list
kvm-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/kvm-devel

Reply via email to