From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from list by lists.gnu.org with archive (Exim 4.71) id 1VamKH-0000Wk-R2 for mharc-grub-devel@gnu.org; Mon, 28 Oct 2013 08:53:45 -0400 Received: from eggs.gnu.org ([2001:4830:134:3::10]:48747) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1VamK8-0000V3-GV for grub-devel@gnu.org; Mon, 28 Oct 2013 08:53:44 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1VamJz-0005zN-FD for grub-devel@gnu.org; Mon, 28 Oct 2013 08:53:36 -0400 Received: from mail-ee0-x236.google.com ([2a00:1450:4013:c00::236]:59584) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1VamJz-0005zH-1j for grub-devel@gnu.org; Mon, 28 Oct 2013 08:53:27 -0400 Received: by mail-ee0-f54.google.com with SMTP id t10so3168966eei.41 for ; Mon, 28 Oct 2013 05:53:26 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=message-id:date:from:user-agent:mime-version:to:subject:references :in-reply-to:content-type; bh=4wYUlqxwy7Hbm8/aOmTLmPO/vTAXou5kHK0tzARpXvg=; b=s4GdIzFUpv/j5F50VM08gMGJZ5rYX9rvoQwLonYPwj1pWrr2NeuJh9WYYBrpxujlyZ 5WZoDi1CnVbM/m6DNfYyhS6xjtRMjjfBq1/DnnsLc4RuDZ8iHETDz2NzQm5qhVnUPRJV rye5DtLC4rhIKVkWWSuZZm69ZngwEjHRMEmHablte6h6sjjOsilVK4wSlwB+pF8HFyiv 90uicopMhwuVvn9nI2mQtIOej4+316ckCfKvUlmBXgkozQtbRpVjVxZykwhVrxkkPkjX NnnLY3wun2I41atOVpJ0nIeMu+OwI8SCwgm1bvRxzGazc3pUprUbcbOQ+ltemucLeh3b XZsg== X-Received: by 10.14.37.4 with SMTP id x4mr22270544eea.16.1382964805846; Mon, 28 Oct 2013 05:53:25 -0700 (PDT) Received: from [192.168.1.16] (31-249.1-85.cust.bluewin.ch. [85.1.249.31]) by mx.google.com with ESMTPSA id z12sm56946803eev.6.2013.10.28.05.53.25 for (version=TLSv1 cipher=ECDHE-RSA-RC4-SHA bits=128/128); Mon, 28 Oct 2013 05:53:25 -0700 (PDT) Message-ID: <526E5E3E.3070908@gmail.com> Date: Mon, 28 Oct 2013 13:53:18 +0100 From: =?UTF-8?B?VmxhZGltaXIgJ8+GLWNvZGVyL3BoY29kZXInIFNlcmJpbmVua28=?= User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:17.0) Gecko/20131005 Icedove/17.0.9 MIME-Version: 1.0 To: grub-devel@gnu.org Subject: Re: [PATCH] grub-core/lib/fdt.c: Working device tree library References: <526E23C9.1070507@gmail.com> In-Reply-To: <526E23C9.1070507@gmail.com> X-Enigmail-Version: 1.5.1 Content-Type: multipart/signed; micalg=pgp-sha512; protocol="application/pgp-signature"; boundary="----enig2PSAMFUINPPEBPIOQXLRU" X-detected-operating-system: by eggs.gnu.org: Error: Malformed IPv6 address (bad octet value). X-Received-From: 2a00:1450:4013:c00::236 X-BeenThere: grub-devel@gnu.org X-Mailman-Version: 2.1.14 Precedence: list Reply-To: The development of GNU GRUB List-Id: The development of GNU GRUB List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 28 Oct 2013 12:53:45 -0000 This is an OpenPGP/MIME signed message (RFC 4880 and 3156) ------enig2PSAMFUINPPEBPIOQXLRU Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable On 28.10.2013 09:43, Francesco Lavra wrote: > Hi, > The current code in grub-core/lib/fdt.c doesn't work for me (and I > suspect it doesn't work for anybody else either): GRUB just hangs when > trying to load a Linux kernel with an associated device tree. In fact, > the current code is still the first version of fdt.c which I sent to > the list (warning that it wasn't completely tested), and afterwards my > attempts to push a fixed code didn't get much attention. I've looked at wrong patch probably. Committed patch, thanks. > I noticed that Leif's tree in Launchpad still uses the external libfdt,= > and this makes me think that the current version of fdt.c doesn't work > for him either. Now, with the port to 64-bit ARM in progress (which > supposedly will use our fdt files), I think it would good to get fdt.c > fixed as soon as possible, in order to avoid duplicated efforts. > So here goes another attempt at it, this time as a full blown patch. >=20 > Francesco >=20 > --- > ChangeLog | 4 ++ > grub-core/lib/fdt.c | 136 ++++++++++++++++++++++++++++++++-----------= -------- > 2 files changed, 89 insertions(+), 51 deletions(-) >=20 > diff --git a/ChangeLog b/ChangeLog > index bdd2c80..ad30352 100644 > --- a/ChangeLog > +++ b/ChangeLog > @@ -1,3 +1,7 @@ > +2013-10-28 Francesco Lavra > + > + * grub-core/lib/fdt.c: Fix miscellaneous bugs. > + > 2013-10-28 Vladimir Serbinenko > =20 > * grub-core/kern/emu/hostdisk.c (grub_util_check_file_presence): Use > diff --git a/grub-core/lib/fdt.c b/grub-core/lib/fdt.c > index 57528c5..9b77e1c 100644 > --- a/grub-core/lib/fdt.c > +++ b/grub-core/lib/fdt.c > @@ -43,6 +43,16 @@ > #define prop_entry_size(prop_len) \ > (3 * sizeof(grub_uint32_t) + ALIGN_UP(prop_len, sizeof(grub_uint32_t)= )) > =20 > +#define SKIP_NODE_NAME(name, token, end) \ > + name =3D (char *) ((token) + 1); \ > + while (name < (char *) end) \ > + { \ > + if (!*name++) \ > + break; \ > + } \ > + token =3D (grub_uint32_t *) ALIGN_UP((grub_addr_t) (name), sizeof(*t= oken)) > + > + > static grub_uint32_t *get_next_node (const void *fdt, char *node_name)= > { > grub_uint32_t *end =3D (void *) struct_end (fdt); > @@ -50,9 +60,9 @@ static grub_uint32_t *get_next_node (const void *fdt,= char *node_name) > =20 > if (node_name >=3D (char *) end) > return NULL; > - while (*node_name) > + while (*node_name++) > { > - if (++node_name >=3D (char *) end) > + if (node_name >=3D (char *) end) > return NULL; > } > token =3D (grub_uint32_t *) ALIGN_UP ((grub_addr_t) node_name, 4); > @@ -73,7 +83,8 @@ static grub_uint32_t *get_next_node (const void *fdt,= char *node_name) > case FDT_PROP: > /* Skip property token and following data (len, nameoff and pr= operty > value). */ > - token +=3D 3 + grub_be_to_cpu32 (*(token + 1)); > + token +=3D prop_entry_size(grub_be_to_cpu32(*(token + 1))) > + / sizeof(*token); > break; > case FDT_NOP: > token++; > @@ -116,12 +127,14 @@ static grub_uint32_t get_free_space (void *fdt) > =20 > static int add_subnode (void *fdt, int parentoffset, const char *name)= > { > - grub_uint32_t *begin =3D (void *) ((grub_addr_t) fdt > + grub_uint32_t *token =3D (void *) ((grub_addr_t) fdt > + grub_fdt_get_off_dt_struct(fdt) > + parentoffset); > grub_uint32_t *end =3D (void *) struct_end (fdt); > unsigned int entry_size =3D node_entry_size (name); > - grub_uint32_t *token =3D begin; > + char *node_name; > + > + SKIP_NODE_NAME(node_name, token, end); > =20 > /* Insert the new subnode just after the properties of the parent no= de (if > any).*/ > @@ -132,28 +145,28 @@ static int add_subnode (void *fdt, int parentoffs= et, const char *name) > switch (grub_be_to_cpu32(*token)) > { > case FDT_PROP: > - /* Skip len and nameoff. */ > - token +=3D 2; > + /* Skip len, nameoff and property value. */ > + token +=3D prop_entry_size(grub_be_to_cpu32(*(token + 1))) > + / sizeof(*token); > break; > case FDT_BEGIN_NODE: > case FDT_END_NODE: > goto insert; > case FDT_NOP: > + token++; > break; > default: > /* invalid token */ > return -1; > } > - token++; > } > insert: > - grub_memmove (token + entry_size, token, > + grub_memmove (token + entry_size / sizeof(*token), token, > (grub_addr_t) end - (grub_addr_t) token); > *token =3D grub_cpu_to_be32(FDT_BEGIN_NODE); > token[entry_size / sizeof(*token) - 2] =3D 0; /* padding bytes */ > grub_strcpy((char *) (token + 1), name); > - token +=3D entry_size / sizeof(*token) - 1; > - *token =3D grub_cpu_to_be32(FDT_END_NODE); > + token[entry_size / sizeof(*token) - 1] =3D grub_cpu_to_be32(FDT_END_= NODE); > return ((grub_addr_t) token - (grub_addr_t) fdt > - grub_fdt_get_off_dt_struct(fdt)); > } > @@ -175,29 +188,32 @@ static int rearrange_blocks (void *fdt, unsigned = int clearance) > =20 > if ((grub_fdt_get_off_mem_rsvmap (fdt) =3D=3D off_mem_rsvmap) > && (grub_fdt_get_off_dt_struct (fdt) =3D=3D off_dt_struct)) > - { > - /* No need to allocate memory for a temporary FDT, just move the s= trings > - block if needed. */ > - if (grub_fdt_get_off_dt_strings (fdt) !=3D off_dt_strings) > - grub_memmove(fdt_ptr + off_dt_strings, > - fdt_ptr + grub_fdt_get_off_dt_strings (fdt), > - grub_fdt_get_size_dt_strings (fdt)); > - return 0; > - } > + { > + /* No need to allocate memory for a temporary FDT, just move the= strings > + block if needed. */ > + if (grub_fdt_get_off_dt_strings (fdt) !=3D off_dt_strings) > + { > + grub_memmove(fdt_ptr + off_dt_strings, > + fdt_ptr + grub_fdt_get_off_dt_strings (fdt), > + grub_fdt_get_size_dt_strings (fdt)); > + grub_fdt_set_off_dt_strings (fdt, off_dt_strings); > + } > + return 0; > + } > tmp_fdt =3D grub_malloc (grub_fdt_get_totalsize (fdt)); > if (!tmp_fdt) > return -1; > grub_memcpy (tmp_fdt + off_mem_rsvmap, > - fdt_ptr + grub_fdt_get_off_mem_rsvmap (fdt), > - get_mem_rsvmap_size (fdt)); > + fdt_ptr + grub_fdt_get_off_mem_rsvmap (fdt), > + get_mem_rsvmap_size (fdt)); > grub_fdt_set_off_mem_rsvmap (fdt, off_mem_rsvmap); > grub_memcpy (tmp_fdt + off_dt_struct, > - fdt_ptr + grub_fdt_get_off_dt_struct (fdt), > - grub_fdt_get_size_dt_struct (fdt)); > + fdt_ptr + grub_fdt_get_off_dt_struct (fdt), > + grub_fdt_get_size_dt_struct (fdt)); > grub_fdt_set_off_dt_struct (fdt, off_dt_struct); > grub_memcpy (tmp_fdt + off_dt_strings, > - fdt_ptr + grub_fdt_get_off_dt_strings (fdt), > - grub_fdt_get_size_dt_strings (fdt)); > + fdt_ptr + grub_fdt_get_off_dt_strings (fdt), > + grub_fdt_get_size_dt_strings (fdt)); > grub_fdt_set_off_dt_strings (fdt, off_dt_strings); > =20 > /* Copy reordered blocks back to fdt. */ > @@ -214,9 +230,12 @@ static grub_uint32_t *find_prop (void *fdt, unsign= ed int nodeoffset, > grub_uint32_t *prop =3D (void *) ((grub_addr_t) fdt > + grub_fdt_get_off_dt_struct (fdt) > + nodeoffset); > + grub_uint32_t *end =3D (void *) struct_end(fdt); > grub_uint32_t nameoff; > + char *node_name; > =20 > - do > + SKIP_NODE_NAME(node_name, prop, end); > + while (prop < end - 2) > { > if (grub_be_to_cpu32(*prop) =3D=3D FDT_PROP) > { > @@ -224,15 +243,19 @@ static grub_uint32_t *find_prop (void *fdt, unsig= ned int nodeoffset, > if ((nameoff + grub_strlen (name) < grub_fdt_get_size_dt_strin= gs (fdt)) > && !grub_strcmp (name, (char *) fdt + > grub_fdt_get_off_dt_strings (fdt) + nameo= ff)) > - return prop; > - prop +=3D prop_entry_size(grub_be_to_cpu32(*prop + 1)) / sizeo= f (*prop); > + { > + if (prop + prop_entry_size(grub_be_to_cpu32(*(prop + 1))) > + / sizeof (*prop) >=3D end) > + return NULL; > + return prop; > + } > + prop +=3D prop_entry_size(grub_be_to_cpu32(*(prop + 1))) / siz= eof (*prop); > } > - else if (grub_be_to_cpu32(*prop) !=3D FDT_NOP) > + else if (grub_be_to_cpu32(*prop) =3D=3D FDT_NOP) > + prop++; > + else > return NULL; > - prop++; > - } while ((grub_addr_t) prop < ((grub_addr_t) fdt > - + grub_fdt_get_off_dt_struct (fdt) > - + grub_fdt_get_size_dt_struct (fdt)))= ; > + } > return NULL; > } > =20 > @@ -271,6 +294,9 @@ int grub_fdt_find_subnode (const void *fdt, unsigne= d int parentoffset, > token =3D (void *) ((grub_addr_t) fdt + grub_fdt_get_off_dt_struct(f= dt) > + parentoffset); > end =3D (void *) struct_end (fdt); > + if ((token >=3D end) || (grub_be_to_cpu32(*token) !=3D FDT_BEGIN_NOD= E)) > + return -1; > + SKIP_NODE_NAME(node_name, token, end); > while (token < end) > { > switch (grub_be_to_cpu32(*token)) > @@ -280,19 +306,19 @@ int grub_fdt_find_subnode (const void *fdt, unsig= ned int parentoffset, > if (node_name + grub_strlen (name) >=3D (char *) end) > return -1; > if (!grub_strcmp (node_name, name)) > - return (int) ((grub_addr_t) token > - + ALIGN_UP(grub_strlen (name) + 1, 4) > + return (int) ((grub_addr_t) token - (grub_addr_t) fdt > - grub_fdt_get_off_dt_struct (fdt)); > token =3D get_next_node (fdt, node_name); > if (!token) > return -1; > break; > - case FDT_END_NODE: > - return -1; > case FDT_PROP: > /* Skip property token and following data (len, nameoff and pr= operty > value). */ > - token +=3D 3 + grub_be_to_cpu32 (*(token + 1)); > + if (token >=3D end - 1) > + return -1; > + token +=3D prop_entry_size(grub_be_to_cpu32(*(token + 1))) > + / sizeof(*token); > break; > case FDT_NOP: > token++; > @@ -327,7 +353,10 @@ int grub_fdt_set_prop (void *fdt, unsigned int nod= eoffset, const char *name, > int prop_name_present =3D 0; > grub_uint32_t nameoff =3D 0; > =20 > - if ((nodeoffset >=3D grub_fdt_get_size_dt_struct (fdt)) || (nodeoffs= et & 0x3)) > + if ((nodeoffset >=3D grub_fdt_get_size_dt_struct (fdt)) || (nodeoffs= et & 0x3) > + || (grub_be_to_cpu32(*(grub_uint32_t *) ((grub_addr_t) fdt > + + grub_fdt_get_off_dt_struct (fdt) + nodeof= fset)) > + !=3D FDT_BEGIN_NODE)) > return -1; > prop =3D find_prop (fdt, nodeoffset, name); > if (prop) > @@ -339,7 +368,7 @@ int grub_fdt_set_prop (void *fdt, unsigned int node= offset, const char *name, > prop_name_present =3D 1; > for (i =3D 0; i < prop_len / sizeof(grub_uint32_t); i++) > *(prop + 3 + i) =3D grub_cpu_to_be32 (FDT_NOP); > - if (len > prop_len) > + if (len > ALIGN_UP(prop_len, sizeof(grub_uint32_t))) > { > /* Length of new property value is greater than the space al= located > for the current value: a new entry needs to be created, s= o save the > @@ -371,19 +400,24 @@ int grub_fdt_set_prop (void *fdt, unsigned int no= deoffset, const char *name, > + grub_strlen (name) + 1); > } > if (!prop) { > - prop =3D (void *) ((grub_addr_t) fdt + grub_fdt_get_off_dt_struct = (fdt) > - + nodeoffset); > - grub_memmove (prop + prop_entry_size(len), prop, > - grub_fdt_get_size_dt_struct (fdt) - nodeoffset); > + char *node_name =3D (char *) ((grub_addr_t) fdt > + + grub_fdt_get_off_dt_struct (fdt) + n= odeoffset > + + sizeof(grub_uint32_t)); > + > + prop =3D (void *) (node_name + ALIGN_UP(grub_strlen(node_name) + 1= , 4)); > + grub_memmove (prop + prop_entry_size(len) / sizeof(*prop), prop, > + struct_end(fdt) - (grub_addr_t) prop); > + grub_fdt_set_size_dt_struct (fdt, grub_fdt_get_size_dt_struct (fdt= ) > + + prop_entry_size(len)); > *prop =3D grub_cpu_to_be32 (FDT_PROP); > - *(prop + 1) =3D grub_cpu_to_be32 (len); > *(prop + 2) =3D grub_cpu_to_be32 (nameoff); > + } > + *(prop + 1) =3D grub_cpu_to_be32 (len); > =20 > - /* Insert padding bytes at the end of the value; if they are not n= eeded, > - they will be overwritten by the follozing memcpy. */ > - *(prop + prop_entry_size(len) / sizeof(grub_uint32_t) - 1) =3D 0; > + /* Insert padding bytes at the end of the value; if they are not nee= ded, they > + will be overwritten by the following memcpy. */ > + *(prop + prop_entry_size(len) / sizeof(grub_uint32_t) - 1) =3D 0; > =20 > - grub_memcpy (prop + 3, val, len); > - } > + grub_memcpy (prop + 3, val, len); > return 0; > } >=20 ------enig2PSAMFUINPPEBPIOQXLRU Content-Type: application/pgp-signature; name="signature.asc" Content-Description: OpenPGP digital signature Content-Disposition: attachment; filename="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4.15 (GNU/Linux) Comment: Using GnuPG with Icedove - http://www.enigmail.net/ iF4EAREKAAYFAlJuXkQACgkQNak7dOguQgmtUQD/ZtjU2NLK77teUgQg8mOjQQ+W zY0SkHdRCHpRI8N1kMABAJUFXBVnMWBZGX1aw1//rEqBwFu6ZDwmLoSTkKTOidxE =sNRi -----END PGP SIGNATURE----- ------enig2PSAMFUINPPEBPIOQXLRU--