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

[Jae Hyun Yoo]

Hello Michal,

On 9/21/2022 6:07 AM, Michal Simek wrote:
> 
> 
> 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_jaehyoo at quicinc.com>
>> Reviewed-by: Simon Glass <sjg at 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?

The 'board_gen' command is replaced with 'generate' command by this
series and the 'generate' command has one more preceding argument
to check '-b' and '-p' for board and product respectively. So actually
this change is part of the 4th patch of this series. I'll move it to
the patch in the next version.

Thanks for your pointing it out.

Jae