Re: [PATCH v4 3/6] cmd: fru: fix a sandbox segfault issue

[Michal Simek]

On 8/25/22 18:42, Jae Hyun Yoo wrote:

This command doesn't work with sandbox because direct memory access
causes a segfault error. Fix it up using map_sysmem().

Signed-off-by: Jae Hyun Yoo <quic_jaeh...@quicinc.com>
Reviewed-by: Simon Glass <s...@chromium.org>
---
Changes from v3:
  * None.

Changes from v2:
  * Added a 'Reviewed-by' tag. (Simon)

Changes from v1:
  * Newly added in v2.

  board/xilinx/common/board.c |  2 +-
  cmd/fru.c                   | 19 ++++++++++++++++---
  include/fru.h               |  4 ++--
  lib/fru_ops.c               | 17 ++++++++---------
  4 files changed, 27 insertions(+), 15 deletions(-)

diff --git a/board/xilinx/common/board.c b/board/xilinx/common/board.c
index e58b11d7f757..3292083c5250 100644
--- a/board/xilinx/common/board.c
+++ b/board/xilinx/common/board.c
@@ -241,7 +241,7 @@ static int xilinx_read_eeprom_fru(struct udevice *dev, char 
*name,
                goto end;
        }

- fru_capture((unsigned long)fru_content);

+       fru_capture(fru_content);
        if (gd->flags & GD_FLG_RELOC || (_DEBUG && 
CONFIG_IS_ENABLED(DTB_RESELECT))) {
                printf("Xilinx I2C FRU format at %s:\n", name);
                ret = fru_display(0);
diff --git a/cmd/fru.c b/cmd/fru.c
index 2ec5012af5ac..b2cadbec9780 100644
--- a/cmd/fru.c
+++ b/cmd/fru.c
@@ -9,12 +9,15 @@
  #include <fdtdec.h>
  #include <fru.h>
  #include <malloc.h>
+#include <mapmem.h>

static int do_fru_capture(struct cmd_tbl *cmdtp, int flag, int argc,

                          char *const argv[])
  {
        unsigned long addr;
+       const void *buf;
        char *endp;
+       int ret;

if (argc < cmdtp->maxargs)

                return CMD_RET_USAGE;
@@ -23,7 +26,11 @@ static int do_fru_capture(struct cmd_tbl *cmdtp, int flag, 
int argc,
        if (*argv[1] == 0 || *endp != 0)
                return -1;

- return fru_capture(addr);

+       buf = map_sysmem(addr, 0);
+       ret = fru_capture(buf);
+       unmap_sysmem(buf);
+
+       return ret;
  }

static int do_fru_display(struct cmd_tbl *cmdtp, int flag, int argc,

@@ -37,13 +44,19 @@ static int do_fru_generate(struct cmd_tbl *cmdtp, int flag, 
int argc,
                           char *const argv[])
  {
        unsigned long addr;
+       const void *buf;
+       int ret;

if (argc < cmdtp->maxargs)

                return CMD_RET_USAGE;

- addr = hextoul(argv[2], NULL);

+       addr = hextoul(argv[3], NULL);

What's the reason for changing arguments here? Wasn't origin version correct?

Thanks,
Michal