diff mbox

[U-Boot,5/6] pxe: Ensure all memory access is to mapped memory

Message ID 1424385674-19920-6-git-send-email-sjoerd.simons@collabora.co.uk
State Superseded
Delegated to: Simon Glass
Headers show

Commit Message

Sjoerd Simons Feb. 19, 2015, 10:41 p.m. UTC
Properly map memory through map_sysmem so that pxe can be used from the
sandbox.

Signed-off-by: Sjoerd Simons <sjoerd.simons@collabora.co.uk>
---
 common/cmd_pxe.c | 80 ++++++++++++++++++++++++++++++++------------------------
 1 file changed, 46 insertions(+), 34 deletions(-)

Comments

Simon Glass Feb. 20, 2015, 7:23 p.m. UTC | #1
Hi Sjoerd,

On 19 February 2015 at 15:41, Sjoerd Simons
<sjoerd.simons@collabora.co.uk> wrote:
> Properly map memory through map_sysmem so that pxe can be used from the
> sandbox.
>
> Signed-off-by: Sjoerd Simons <sjoerd.simons@collabora.co.uk>

Please run your patches through patman as you seem to have style
violations. Also can you add some notes about how you have tested this
on real hardware?

> ---
>  common/cmd_pxe.c | 80 ++++++++++++++++++++++++++++++++------------------------
>  1 file changed, 46 insertions(+), 34 deletions(-)
>
> diff --git a/common/cmd_pxe.c b/common/cmd_pxe.c
> index 7e32c95..ec81e70 100644
> --- a/common/cmd_pxe.c
> +++ b/common/cmd_pxe.c
> @@ -13,6 +13,7 @@
>  #include <errno.h>
>  #include <linux/list.h>
>  #include <fs.h>
> +#include <asm/io.h>
>
>  #include "menu.h"
>  #include "cli.h"
> @@ -188,11 +189,11 @@ static int do_get_any(cmd_tbl_t *cmdtp, const char *file_path, char *file_addr)
>   *
>   * Returns 1 for success, or < 0 on error.
>   */
> -static int get_relfile(cmd_tbl_t *cmdtp, const char *file_path, void *file_addr)
> +static int get_relfile(cmd_tbl_t *cmdtp, const char *file_path, unsigned long file_addr)
>  {
>         size_t path_len;
>         char relfile[MAX_TFTP_PATH_LEN+1];
> -       char addr_buf[10];
> +       char addr_buf[18];
>         int err;
>
>         err = get_bootfile_path(file_path, relfile, sizeof(relfile));
> @@ -215,7 +216,7 @@ static int get_relfile(cmd_tbl_t *cmdtp, const char *file_path, void *file_addr)
>
>         printf("Retrieving file: %s\n", relfile);
>
> -       sprintf(addr_buf, "%p", file_addr);
> +       sprintf(addr_buf, "%lx", file_addr);
>
>         return do_getfile(cmdtp, relfile, addr_buf);
>  }
> @@ -227,11 +228,12 @@ static int get_relfile(cmd_tbl_t *cmdtp, const char *file_path, void *file_addr)
>   *
>   * Returns 1 on success, or < 0 for error.
>   */
> -static int get_pxe_file(cmd_tbl_t *cmdtp, const char *file_path, void *file_addr)
> +static int get_pxe_file(cmd_tbl_t *cmdtp, const char *file_path, unsigned long file_addr)
>  {
>         unsigned long config_file_size;
>         char *tftp_filesize;
>         int err;
> +       void *buf;

Can we make this char * please to remove the cast below?

>
>         err = get_relfile(cmdtp, file_path, file_addr);
>
> @@ -250,7 +252,9 @@ static int get_pxe_file(cmd_tbl_t *cmdtp, const char *file_path, void *file_addr
>         if (strict_strtoul(tftp_filesize, 16, &config_file_size) < 0)
>                 return -EINVAL;
>
> -       *(char *)(file_addr + config_file_size) = '\0';
> +       buf = map_sysmem (file_addr + config_file_size, 1);
> +       * (char *)buf = '\0';
> +       unmap_sysmem (buf);
>
>         return 1;
>  }
> @@ -266,7 +270,7 @@ static int get_pxe_file(cmd_tbl_t *cmdtp, const char *file_path, void *file_addr
>   *
>   * Returns 1 on success or < 0 on error.
>   */
> -static int get_pxelinux_path(cmd_tbl_t *cmdtp, const char *file, void *pxefile_addr_r)
> +static int get_pxelinux_path(cmd_tbl_t *cmdtp, const char *file, unsigned long pxefile_addr_r)
>  {
>         size_t base_len = strlen(PXELINUX_DIR);
>         char path[MAX_TFTP_PATH_LEN+1];
> @@ -287,7 +291,7 @@ static int get_pxelinux_path(cmd_tbl_t *cmdtp, const char *file, void *pxefile_a
>   *
>   * Returns 1 on success or < 0 on error.
>   */
> -static int pxe_uuid_path(cmd_tbl_t *cmdtp, void *pxefile_addr_r)
> +static int pxe_uuid_path(cmd_tbl_t *cmdtp, unsigned long pxefile_addr_r)
>  {
>         char *uuid_str;
>
> @@ -305,7 +309,7 @@ static int pxe_uuid_path(cmd_tbl_t *cmdtp, void *pxefile_addr_r)
>   *
>   * Returns 1 on success or < 0 on error.
>   */
> -static int pxe_mac_path(cmd_tbl_t *cmdtp, void *pxefile_addr_r)
> +static int pxe_mac_path(cmd_tbl_t *cmdtp, unsigned long pxefile_addr_r)
>  {
>         char mac_str[21];
>         int err;
> @@ -325,7 +329,7 @@ static int pxe_mac_path(cmd_tbl_t *cmdtp, void *pxefile_addr_r)
>   *
>   * Returns 1 on success or < 0 on error.
>   */
> -static int pxe_ipaddr_paths(cmd_tbl_t *cmdtp, void *pxefile_addr_r)
> +static int pxe_ipaddr_paths(cmd_tbl_t *cmdtp, unsigned long pxefile_addr_r)
>  {
>         char ip_addr[9];
>         int mask_pos, err;
> @@ -384,9 +388,9 @@ do_pxe_get(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
>          * Keep trying paths until we successfully get a file we're looking
>          * for.
>          */
> -       if (pxe_uuid_path(cmdtp, (void *)pxefile_addr_r) > 0 ||
> -           pxe_mac_path(cmdtp, (void *)pxefile_addr_r) > 0 ||
> -           pxe_ipaddr_paths(cmdtp, (void *)pxefile_addr_r) > 0) {
> +       if (pxe_uuid_path(cmdtp, pxefile_addr_r) > 0 ||
> +           pxe_mac_path(cmdtp, pxefile_addr_r) > 0 ||
> +           pxe_ipaddr_paths(cmdtp, pxefile_addr_r) > 0) {
>                 printf("Config file found\n");
>
>                 return 0;
> @@ -394,7 +398,7 @@ do_pxe_get(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
>
>         while (pxe_default_paths[i]) {
>                 if (get_pxelinux_path(cmdtp, pxe_default_paths[i],
> -                                     (void *)pxefile_addr_r) > 0) {
> +                                     pxefile_addr_r) > 0) {
>                         printf("Config file found\n");
>                         return 0;
>                 }
> @@ -427,7 +431,7 @@ static int get_relfile_envaddr(cmd_tbl_t *cmdtp, const char *file_path, const ch
>         if (strict_strtoul(envaddr, 16, &file_addr) < 0)
>                 return -EINVAL;
>
> -       return get_relfile(cmdtp, file_path, (void *)file_addr);
> +       return get_relfile(cmdtp, file_path, file_addr);
>  }
>
>  /*
> @@ -1054,7 +1058,7 @@ static int parse_integer(char **c, int *dst)
>         return 1;
>  }
>
> -static int parse_pxefile_top(cmd_tbl_t *cmdtp, char *p, struct pxe_menu *cfg, int nest_level);
> +static int parse_pxefile_top(cmd_tbl_t *cmdtp, char *p, unsigned long b, struct pxe_menu *cfg, int nest_level);
>
>  /*
>   * Parse an include statement, and retrieve and parse the file it mentions.
> @@ -1064,12 +1068,13 @@ static int parse_pxefile_top(cmd_tbl_t *cmdtp, char *p, struct pxe_menu *cfg, in
>   * include, nest_level has already been incremented and doesn't need to be
>   * incremented here.
>   */
> -static int handle_include(cmd_tbl_t *cmdtp, char **c, char *base,
> +static int handle_include(cmd_tbl_t *cmdtp, char **c, unsigned long base,
>                                 struct pxe_menu *cfg, int nest_level)
>  {
>         char *include_path;
>         char *s = *c;
>         int err;
> +       char *buf;
>
>         err = parse_sliteral(c, &include_path);
>
> @@ -1086,20 +1091,22 @@ static int handle_include(cmd_tbl_t *cmdtp, char **c, char *base,
>                 return err;
>         }
>
> -       return parse_pxefile_top(cmdtp, base, cfg, nest_level);
> +       buf = map_sysmem (base, 0);
> +       return parse_pxefile_top(cmdtp, buf, base, cfg, nest_level);

You should call unmap_sysmem() before the return.

>  }
>
>  /*
>   * Parse lines that begin with 'menu'.
>   *
> - * b and nest are provided to handle the 'menu include' case.
> - *
> - * b should be the address where the file currently being parsed is stored.
> + * base and nest are provided to handle the 'menu include' case.
>   *
> - * nest_level should be 1 when parsing the top level pxe file, 2 when parsing
> - * a file it includes, 3 when parsing a file included by that file, and so on.
> + * base should point to a location where it's safe to store the file, and
> + * nest_level should indicate how many nested includes have occurred. For this
> + * include, nest_level has already been incremented and doesn't need to be
> + * incremented here.
>   */
> -static int parse_menu(cmd_tbl_t *cmdtp, char **c, struct pxe_menu *cfg, char *b, int nest_level)
> +static int parse_menu(cmd_tbl_t *cmdtp, char **c, struct pxe_menu *cfg,
> +                               unsigned long base, int nest_level)
>  {
>         struct token t;
>         char *s = *c;
> @@ -1114,7 +1121,7 @@ static int parse_menu(cmd_tbl_t *cmdtp, char **c, struct pxe_menu *cfg, char *b,
>                 break;
>
>         case T_INCLUDE:
> -               err = handle_include(cmdtp, c, b + strlen(b) + 1, cfg,
> +               err = handle_include(cmdtp, c, base, cfg,
>                                                 nest_level + 1);
>                 break;
>
> @@ -1281,13 +1288,14 @@ static int parse_label(char **c, struct pxe_menu *cfg)
>   *
>   * Returns 1 on success, < 0 on error.
>   */
> -static int parse_pxefile_top(cmd_tbl_t *cmdtp, char *p, struct pxe_menu *cfg, int nest_level)
> +static int parse_pxefile_top(cmd_tbl_t *cmdtp, char *p, unsigned long b,
> +                               struct pxe_menu *cfg, int nest_level)

s/b/base/ or something more meaningful (fix above/below also)

>  {
>         struct token t;
> -       char *s, *b, *label_name;
> +       char *s, *label_name, *base;
>         int err;
>
> -       b = p;
> +       base = p;

This worries me - assigning a pointer to a ulong.

>
>         if (nest_level > MAX_NEST_LEVEL) {
>                 printf("Maximum nesting (%d) exceeded\n", MAX_NEST_LEVEL);
> @@ -1303,7 +1311,9 @@ static int parse_pxefile_top(cmd_tbl_t *cmdtp, char *p, struct pxe_menu *cfg, in
>                 switch (t.type) {
>                 case T_MENU:
>                         cfg->prompt = 1;
> -                       err = parse_menu(cmdtp, &p, cfg, b, nest_level);
> +                       err = parse_menu(cmdtp, &p, cfg,
> +                                               b + ALIGN(strlen(base), 4),
> +                                               nest_level);
>                         break;
>
>                 case T_TIMEOUT:
> @@ -1328,7 +1338,7 @@ static int parse_pxefile_top(cmd_tbl_t *cmdtp, char *p, struct pxe_menu *cfg, in
>                         break;
>
>                 case T_INCLUDE:
> -                       err = handle_include(cmdtp, &p, b + ALIGN(strlen(b), 4), cfg,
> +                       err = handle_include(cmdtp, &p, b + ALIGN(strlen(base), 4), cfg,
>                                                         nest_level + 1);
>                         break;
>
> @@ -1385,9 +1395,10 @@ static void destroy_pxe_menu(struct pxe_menu *cfg)
>   * files it includes). The resulting pxe_menu struct can be free()'d by using
>   * the destroy_pxe_menu() function.
>   */
> -static struct pxe_menu *parse_pxefile(cmd_tbl_t *cmdtp, char *menucfg)
> +static struct pxe_menu *parse_pxefile(cmd_tbl_t *cmdtp, unsigned long menucfg)
>  {
>         struct pxe_menu *cfg;
> +       char *buf;
>
>         cfg = malloc(sizeof(struct pxe_menu));
>
> @@ -1398,7 +1409,8 @@ static struct pxe_menu *parse_pxefile(cmd_tbl_t *cmdtp, char *menucfg)
>
>         INIT_LIST_HEAD(&cfg->labels);
>
> -       if (parse_pxefile_top(cmdtp, menucfg, cfg, 1) < 0) {
> +       buf = map_sysmem (menucfg, 0);
> +       if (parse_pxefile_top(cmdtp, buf, menucfg, cfg, 1) < 0) {
>                 destroy_pxe_menu(cfg);
>                 return NULL;

Need unmap_sysmem() after.

>         }
> @@ -1556,7 +1568,7 @@ do_pxe_boot(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
>                 return 1;
>         }
>
> -       cfg = parse_pxefile(cmdtp, (char *)(pxefile_addr_r));
> +       cfg = parse_pxefile(cmdtp, pxefile_addr_r);
>
>         if (cfg == NULL) {
>                 printf("Error parsing config file\n");
> @@ -1663,12 +1675,12 @@ static int do_sysboot(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
>                 return 1;
>         }
>
> -       if (get_pxe_file(cmdtp, filename, (void *)pxefile_addr_r) < 0) {
> +       if (get_pxe_file(cmdtp, filename, pxefile_addr_r) < 0) {
>                 printf("Error reading config file\n");
>                 return 1;
>         }
>
> -       cfg = parse_pxefile(cmdtp, (char *)(pxefile_addr_r));
> +       cfg = parse_pxefile(cmdtp, pxefile_addr_r);
>
>         if (cfg == NULL) {
>                 printf("Error parsing config file\n");
> --
> 2.1.4
>

Regards,
Simon
Sjoerd Simons Feb. 28, 2015, 2:12 p.m. UTC | #2
On Fri, 2015-02-20 at 12:23 -0700, Simon Glass wrote:
> Hi Sjoerd,
> 
> On 19 February 2015 at 15:41, Sjoerd Simons
> <sjoerd.simons@collabora.co.uk> wrote:
> > Properly map memory through map_sysmem so that pxe can be used from the
> > sandbox.
> >
> > Signed-off-by: Sjoerd Simons <sjoerd.simons@collabora.co.uk>
> 
> Please run your patches through patman as you seem to have style
> violations. Also can you add some notes about how you have tested this
> on real hardware?

Will do for the next round together with addressing your comments. One
confused me tough, see below.

> > ---
> >  common/cmd_pxe.c | 80 ++++++++++++++++++++++++++++++++------------------------
> >  1 file changed, 46 insertions(+), 34 deletions(-)

> > @@ -1281,13 +1288,14 @@ static int parse_label(char **c, struct pxe_menu *cfg)
> >   *
> >   * Returns 1 on success, < 0 on error.
> >   */
> > -static int parse_pxefile_top(cmd_tbl_t *cmdtp, char *p, struct pxe_menu *cfg, int nest_level)
> > +static int parse_pxefile_top(cmd_tbl_t *cmdtp, char *p, unsigned long b,
> > +                               struct pxe_menu *cfg, int nest_level)
> 
> s/b/base/ or something more meaningful (fix above/below also)
> >  {
> >         struct token t;
> > -       char *s, *b, *label_name;
> > +       char *s, *label_name, *base;
> >         int err;
> >
> > -       b = p;
> > +       base = p;
> 
> This worries me - assigning a pointer to a ulong.


base is a pointer here though. So this comment confuses me.
Simon Glass Feb. 28, 2015, 2:16 p.m. UTC | #3
Hi Sjoerd,

On 28 February 2015 at 07:12, Sjoerd Simons
<sjoerd.simons@collabora.co.uk> wrote:
> On Fri, 2015-02-20 at 12:23 -0700, Simon Glass wrote:
>> Hi Sjoerd,
>>
>> On 19 February 2015 at 15:41, Sjoerd Simons
>> <sjoerd.simons@collabora.co.uk> wrote:
>> > Properly map memory through map_sysmem so that pxe can be used from the
>> > sandbox.
>> >
>> > Signed-off-by: Sjoerd Simons <sjoerd.simons@collabora.co.uk>
>>
>> Please run your patches through patman as you seem to have style
>> violations. Also can you add some notes about how you have tested this
>> on real hardware?
>
> Will do for the next round together with addressing your comments. One
> confused me tough, see below.
>
>> > ---
>> >  common/cmd_pxe.c | 80 ++++++++++++++++++++++++++++++++------------------------
>> >  1 file changed, 46 insertions(+), 34 deletions(-)
>
>> > @@ -1281,13 +1288,14 @@ static int parse_label(char **c, struct pxe_menu *cfg)
>> >   *
>> >   * Returns 1 on success, < 0 on error.
>> >   */
>> > -static int parse_pxefile_top(cmd_tbl_t *cmdtp, char *p, struct pxe_menu *cfg, int nest_level)
>> > +static int parse_pxefile_top(cmd_tbl_t *cmdtp, char *p, unsigned long b,
>> > +                               struct pxe_menu *cfg, int nest_level)
>>
>> s/b/base/ or something more meaningful (fix above/below also)
>> >  {
>> >         struct token t;
>> > -       char *s, *b, *label_name;
>> > +       char *s, *label_name, *base;
>> >         int err;
>> >
>> > -       b = p;
>> > +       base = p;
>>
>> This worries me - assigning a pointer to a ulong.
>
>
> base is a pointer here though. So this comment confuses me.

Actually it confused me. I don't think it's good to use base as a
pointer or a ulong in the same file. Maybe use base for the ulong and
base_ptr for the pointer. Or base_addr and base. But code is harder to
read if different functions in the file have different conventions.

Regards,
Simon
diff mbox

Patch

diff --git a/common/cmd_pxe.c b/common/cmd_pxe.c
index 7e32c95..ec81e70 100644
--- a/common/cmd_pxe.c
+++ b/common/cmd_pxe.c
@@ -13,6 +13,7 @@ 
 #include <errno.h>
 #include <linux/list.h>
 #include <fs.h>
+#include <asm/io.h>
 
 #include "menu.h"
 #include "cli.h"
@@ -188,11 +189,11 @@  static int do_get_any(cmd_tbl_t *cmdtp, const char *file_path, char *file_addr)
  *
  * Returns 1 for success, or < 0 on error.
  */
-static int get_relfile(cmd_tbl_t *cmdtp, const char *file_path, void *file_addr)
+static int get_relfile(cmd_tbl_t *cmdtp, const char *file_path, unsigned long file_addr)
 {
 	size_t path_len;
 	char relfile[MAX_TFTP_PATH_LEN+1];
-	char addr_buf[10];
+	char addr_buf[18];
 	int err;
 
 	err = get_bootfile_path(file_path, relfile, sizeof(relfile));
@@ -215,7 +216,7 @@  static int get_relfile(cmd_tbl_t *cmdtp, const char *file_path, void *file_addr)
 
 	printf("Retrieving file: %s\n", relfile);
 
-	sprintf(addr_buf, "%p", file_addr);
+	sprintf(addr_buf, "%lx", file_addr);
 
 	return do_getfile(cmdtp, relfile, addr_buf);
 }
@@ -227,11 +228,12 @@  static int get_relfile(cmd_tbl_t *cmdtp, const char *file_path, void *file_addr)
  *
  * Returns 1 on success, or < 0 for error.
  */
-static int get_pxe_file(cmd_tbl_t *cmdtp, const char *file_path, void *file_addr)
+static int get_pxe_file(cmd_tbl_t *cmdtp, const char *file_path, unsigned long file_addr)
 {
 	unsigned long config_file_size;
 	char *tftp_filesize;
 	int err;
+	void *buf;
 
 	err = get_relfile(cmdtp, file_path, file_addr);
 
@@ -250,7 +252,9 @@  static int get_pxe_file(cmd_tbl_t *cmdtp, const char *file_path, void *file_addr
 	if (strict_strtoul(tftp_filesize, 16, &config_file_size) < 0)
 		return -EINVAL;
 
-	*(char *)(file_addr + config_file_size) = '\0';
+	buf = map_sysmem (file_addr + config_file_size, 1);
+	* (char *)buf = '\0';
+	unmap_sysmem (buf);
 
 	return 1;
 }
@@ -266,7 +270,7 @@  static int get_pxe_file(cmd_tbl_t *cmdtp, const char *file_path, void *file_addr
  *
  * Returns 1 on success or < 0 on error.
  */
-static int get_pxelinux_path(cmd_tbl_t *cmdtp, const char *file, void *pxefile_addr_r)
+static int get_pxelinux_path(cmd_tbl_t *cmdtp, const char *file, unsigned long pxefile_addr_r)
 {
 	size_t base_len = strlen(PXELINUX_DIR);
 	char path[MAX_TFTP_PATH_LEN+1];
@@ -287,7 +291,7 @@  static int get_pxelinux_path(cmd_tbl_t *cmdtp, const char *file, void *pxefile_a
  *
  * Returns 1 on success or < 0 on error.
  */
-static int pxe_uuid_path(cmd_tbl_t *cmdtp, void *pxefile_addr_r)
+static int pxe_uuid_path(cmd_tbl_t *cmdtp, unsigned long pxefile_addr_r)
 {
 	char *uuid_str;
 
@@ -305,7 +309,7 @@  static int pxe_uuid_path(cmd_tbl_t *cmdtp, void *pxefile_addr_r)
  *
  * Returns 1 on success or < 0 on error.
  */
-static int pxe_mac_path(cmd_tbl_t *cmdtp, void *pxefile_addr_r)
+static int pxe_mac_path(cmd_tbl_t *cmdtp, unsigned long pxefile_addr_r)
 {
 	char mac_str[21];
 	int err;
@@ -325,7 +329,7 @@  static int pxe_mac_path(cmd_tbl_t *cmdtp, void *pxefile_addr_r)
  *
  * Returns 1 on success or < 0 on error.
  */
-static int pxe_ipaddr_paths(cmd_tbl_t *cmdtp, void *pxefile_addr_r)
+static int pxe_ipaddr_paths(cmd_tbl_t *cmdtp, unsigned long pxefile_addr_r)
 {
 	char ip_addr[9];
 	int mask_pos, err;
@@ -384,9 +388,9 @@  do_pxe_get(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
 	 * Keep trying paths until we successfully get a file we're looking
 	 * for.
 	 */
-	if (pxe_uuid_path(cmdtp, (void *)pxefile_addr_r) > 0 ||
-	    pxe_mac_path(cmdtp, (void *)pxefile_addr_r) > 0 ||
-	    pxe_ipaddr_paths(cmdtp, (void *)pxefile_addr_r) > 0) {
+	if (pxe_uuid_path(cmdtp, pxefile_addr_r) > 0 ||
+	    pxe_mac_path(cmdtp, pxefile_addr_r) > 0 ||
+	    pxe_ipaddr_paths(cmdtp, pxefile_addr_r) > 0) {
 		printf("Config file found\n");
 
 		return 0;
@@ -394,7 +398,7 @@  do_pxe_get(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
 
 	while (pxe_default_paths[i]) {
 		if (get_pxelinux_path(cmdtp, pxe_default_paths[i],
-				      (void *)pxefile_addr_r) > 0) {
+				      pxefile_addr_r) > 0) {
 			printf("Config file found\n");
 			return 0;
 		}
@@ -427,7 +431,7 @@  static int get_relfile_envaddr(cmd_tbl_t *cmdtp, const char *file_path, const ch
 	if (strict_strtoul(envaddr, 16, &file_addr) < 0)
 		return -EINVAL;
 
-	return get_relfile(cmdtp, file_path, (void *)file_addr);
+	return get_relfile(cmdtp, file_path, file_addr);
 }
 
 /*
@@ -1054,7 +1058,7 @@  static int parse_integer(char **c, int *dst)
 	return 1;
 }
 
-static int parse_pxefile_top(cmd_tbl_t *cmdtp, char *p, struct pxe_menu *cfg, int nest_level);
+static int parse_pxefile_top(cmd_tbl_t *cmdtp, char *p, unsigned long b, struct pxe_menu *cfg, int nest_level);
 
 /*
  * Parse an include statement, and retrieve and parse the file it mentions.
@@ -1064,12 +1068,13 @@  static int parse_pxefile_top(cmd_tbl_t *cmdtp, char *p, struct pxe_menu *cfg, in
  * include, nest_level has already been incremented and doesn't need to be
  * incremented here.
  */
-static int handle_include(cmd_tbl_t *cmdtp, char **c, char *base,
+static int handle_include(cmd_tbl_t *cmdtp, char **c, unsigned long base,
 				struct pxe_menu *cfg, int nest_level)
 {
 	char *include_path;
 	char *s = *c;
 	int err;
+	char *buf;
 
 	err = parse_sliteral(c, &include_path);
 
@@ -1086,20 +1091,22 @@  static int handle_include(cmd_tbl_t *cmdtp, char **c, char *base,
 		return err;
 	}
 
-	return parse_pxefile_top(cmdtp, base, cfg, nest_level);
+	buf = map_sysmem (base, 0);
+	return parse_pxefile_top(cmdtp, buf, base, cfg, nest_level);
 }
 
 /*
  * Parse lines that begin with 'menu'.
  *
- * b and nest are provided to handle the 'menu include' case.
- *
- * b should be the address where the file currently being parsed is stored.
+ * base and nest are provided to handle the 'menu include' case.
  *
- * nest_level should be 1 when parsing the top level pxe file, 2 when parsing
- * a file it includes, 3 when parsing a file included by that file, and so on.
+ * base should point to a location where it's safe to store the file, and
+ * nest_level should indicate how many nested includes have occurred. For this
+ * include, nest_level has already been incremented and doesn't need to be
+ * incremented here.
  */
-static int parse_menu(cmd_tbl_t *cmdtp, char **c, struct pxe_menu *cfg, char *b, int nest_level)
+static int parse_menu(cmd_tbl_t *cmdtp, char **c, struct pxe_menu *cfg,
+				unsigned long base, int nest_level)
 {
 	struct token t;
 	char *s = *c;
@@ -1114,7 +1121,7 @@  static int parse_menu(cmd_tbl_t *cmdtp, char **c, struct pxe_menu *cfg, char *b,
 		break;
 
 	case T_INCLUDE:
-		err = handle_include(cmdtp, c, b + strlen(b) + 1, cfg,
+		err = handle_include(cmdtp, c, base, cfg,
 						nest_level + 1);
 		break;
 
@@ -1281,13 +1288,14 @@  static int parse_label(char **c, struct pxe_menu *cfg)
  *
  * Returns 1 on success, < 0 on error.
  */
-static int parse_pxefile_top(cmd_tbl_t *cmdtp, char *p, struct pxe_menu *cfg, int nest_level)
+static int parse_pxefile_top(cmd_tbl_t *cmdtp, char *p, unsigned long b,
+				struct pxe_menu *cfg, int nest_level)
 {
 	struct token t;
-	char *s, *b, *label_name;
+	char *s, *label_name, *base;
 	int err;
 
-	b = p;
+	base = p;
 
 	if (nest_level > MAX_NEST_LEVEL) {
 		printf("Maximum nesting (%d) exceeded\n", MAX_NEST_LEVEL);
@@ -1303,7 +1311,9 @@  static int parse_pxefile_top(cmd_tbl_t *cmdtp, char *p, struct pxe_menu *cfg, in
 		switch (t.type) {
 		case T_MENU:
 			cfg->prompt = 1;
-			err = parse_menu(cmdtp, &p, cfg, b, nest_level);
+			err = parse_menu(cmdtp, &p, cfg,
+						b + ALIGN(strlen(base), 4),
+						nest_level);
 			break;
 
 		case T_TIMEOUT:
@@ -1328,7 +1338,7 @@  static int parse_pxefile_top(cmd_tbl_t *cmdtp, char *p, struct pxe_menu *cfg, in
 			break;
 
 		case T_INCLUDE:
-			err = handle_include(cmdtp, &p, b + ALIGN(strlen(b), 4), cfg,
+			err = handle_include(cmdtp, &p, b + ALIGN(strlen(base), 4), cfg,
 							nest_level + 1);
 			break;
 
@@ -1385,9 +1395,10 @@  static void destroy_pxe_menu(struct pxe_menu *cfg)
  * files it includes). The resulting pxe_menu struct can be free()'d by using
  * the destroy_pxe_menu() function.
  */
-static struct pxe_menu *parse_pxefile(cmd_tbl_t *cmdtp, char *menucfg)
+static struct pxe_menu *parse_pxefile(cmd_tbl_t *cmdtp, unsigned long menucfg)
 {
 	struct pxe_menu *cfg;
+	char *buf;
 
 	cfg = malloc(sizeof(struct pxe_menu));
 
@@ -1398,7 +1409,8 @@  static struct pxe_menu *parse_pxefile(cmd_tbl_t *cmdtp, char *menucfg)
 
 	INIT_LIST_HEAD(&cfg->labels);
 
-	if (parse_pxefile_top(cmdtp, menucfg, cfg, 1) < 0) {
+	buf = map_sysmem (menucfg, 0);
+	if (parse_pxefile_top(cmdtp, buf, menucfg, cfg, 1) < 0) {
 		destroy_pxe_menu(cfg);
 		return NULL;
 	}
@@ -1556,7 +1568,7 @@  do_pxe_boot(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
 		return 1;
 	}
 
-	cfg = parse_pxefile(cmdtp, (char *)(pxefile_addr_r));
+	cfg = parse_pxefile(cmdtp, pxefile_addr_r);
 
 	if (cfg == NULL) {
 		printf("Error parsing config file\n");
@@ -1663,12 +1675,12 @@  static int do_sysboot(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
 		return 1;
 	}
 
-	if (get_pxe_file(cmdtp, filename, (void *)pxefile_addr_r) < 0) {
+	if (get_pxe_file(cmdtp, filename, pxefile_addr_r) < 0) {
 		printf("Error reading config file\n");
 		return 1;
 	}
 
-	cfg = parse_pxefile(cmdtp, (char *)(pxefile_addr_r));
+	cfg = parse_pxefile(cmdtp, pxefile_addr_r);
 
 	if (cfg == NULL) {
 		printf("Error parsing config file\n");