[XenPPC] Re: [PATCH 3/6] bootwrapper: Add device tree ops for flattened device tree

["Mark A. Greer" <mgreer@xxxxxxxxxx>]

On Sun, Aug 06, 2006 at 07:38:02PM -0500, Hollis Blanchard wrote:
> On Wed, 2006-07-19 at 16:05 -0700, an unknown sender wrote:
> > --- /dev/null
> > +++ b/arch/powerpc/boot/fdt.c
> > @@ -0,0 +1,525 @@
> > +/*
> > + * Simple dtb (binary flattened device tree) search/manipulation routines.
> > + *
> > + * Author: Mark A. Greer <mgreer at mvista.com>
> > + *         - The code for strrchr() was copied from lib/string.c and is
> > + *         copyrighted by Linus Torvalds.
> > + *         - The smarts for fdt_finddevice() were copied with the author's
> > + *         permission from u-boot:common/ft_build.c which was written by
> > + *         Pantelis Antoniou <pantelis at embeddedalley.com>.
> 
> Hmm, so we'll have at least three copies of this code: uboot, kernel,
> and Xen. Would it make sense to put this stuff into a libdt.a?

Yes, it would.

> Technically, dtc has a "libdt" already, but it's absurdly incomplete (I
> don't even know why it's there), so we could just replace it.

Sounds like a plan!

> Xen needs all the finddevice and setprop functionality here, which looks
> like it's about 2/3rds of this code.

Yeah, pretty much.

> > +static void *dtb_start;
> > +static void *dtb_end;
> 
> I'd like to avoid the use of globals here. I know it's fine when you're
> running in early boot, but as I mentioned I'd like to copy this code
> elsewhere. Could these be moved into a structure that's passed as a
> function parameter?

Sure.

> > +static void
> > +fdt_modify_prop(u32 *dp, char *datap, u32 *old_prop_sizep, char *buf,
> > +           int buflen)
> > +{
> > +   u32 old_prop_data_len, new_prop_data_len;
> > +
> > +   old_prop_data_len = _ALIGN_UP(*old_prop_sizep, 4);
> > +   new_prop_data_len = _ALIGN_UP(buflen, 4);
> > +
> > +   /* Check if new prop data fits in old prop data area */
> > +   if (new_prop_data_len == old_prop_data_len) {
> > +           memcpy(datap, buf, buflen);
> > +           *old_prop_sizep = buflen;
> > +   }
> > +   else { /* Need to alloc new area to put larger or smaller fdt */
> > +           struct boot_param_header *old_bph, *new_bph;
> > +           u32 *old_tailp, *new_tailp, *new_datap;
> > +           u32 old_total_size, new_total_size, head_len, tail_len, diff;
> > +           void *new_dtb_start, *new_dtb_end;
> > +
> > +           old_bph = fdt_get_bph(dtb_start),
> > +           old_total_size = old_bph->totalsize;
> > +           head_len = (u32)datap - (u32)dtb_start;
> > +           tail_len = old_total_size - (head_len + old_prop_data_len);
> > +           old_tailp = (u32 *)((u32)dtb_end - tail_len);
> > +           new_total_size = head_len + new_prop_data_len + tail_len;
> > +
> > +           if (!(new_dtb_start = malloc(new_total_size))) {
> > +                   printf("Can't alloc space for new fdt\n\r");
> > +                   exit();
> > +           }
> > +
> > +           new_dtb_end = (void *)((u32)new_dtb_start + new_total_size);
> > +           new_datap = (u32 *)((u32)new_dtb_start + head_len);
> > +           new_tailp = (u32 *)((u32)new_dtb_end - tail_len);
> > +
> > +           memcpy(new_dtb_start, dtb_start, head_len);
> > +           memcpy(new_datap, buf, buflen);
> > +           memcpy(new_tailp, old_tailp, tail_len);
> > +           *(new_datap - 2) = buflen;
> > +
> > +           new_bph = fdt_get_bph(new_dtb_start),
> > +           new_bph->totalsize = new_total_size;
> > +
> > +           diff = new_prop_data_len - old_prop_data_len;
> > +
> > +           /* Adjust offsets of other sections, if necessary */
> > +           if (new_bph->off_dt_strings > new_bph->off_dt_struct)
> > +                   new_bph->off_dt_strings += diff;
> > +
> > +           if (new_bph->off_mem_rsvmap > new_bph->off_dt_struct)
> > +                   new_bph->off_mem_rsvmap += diff;
> > +
> > +           free(dtb_start, old_total_size);
> > +
> > +           dtb_start = new_dtb_start;
> > +           dtb_end = new_dtb_end;
> > +   }
> > +}
> 
> I didn't realize the boot wrapper had a full malloc() to work with.

It doesn't (really).  I made a standard interface to what may
someday be a real memory allocator.  If you look in dink.c,
you'll see I just have a hack malloc() and no free() at all.

Its one of those things that I was hoping someone would chip in
and help with.  IIRC, Matt McClintock was looking at that but I'm
not sure how far he got.

> I
> was actually planning to only allow overwriting properties with values
> of the same size, since for the most part I just need to modify some
> small fixed-size data. Do you need more? I guess if the code already
> works...

I know that we need to extend the size of a property because
/chosen/bootargs can be edited and potentially extended.

Mark

_______________________________________________
Xen-ppc-devel mailing list
Xen-ppc-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-ppc-devel