[Toybox] [PATCH] Support non memory mapped access

Karthik Ramasubramanian kramasub at google.com
Fri Oct 25 13:19:56 PDT 2024


On Thu, Oct 24, 2024 at 7:45 AM enh <enh at google.com> wrote:

>
>
> On Wed, Oct 23, 2024 at 7:02 PM Karthikeyan Ramasubramanian <
> kramasub at google.com> wrote:
>
>> Add --no-mmap flag to indicate seek and read/write access. This allows
>> accessing devices that do not support mapping into memory - eg.
>> /dev/nvram, /dev/msr0 etc.
>>
>> Also currently only WIDTH bytes are mapped into memory even when more
>> data is accessed. Fix this by mapping WIDTH * number of data.
>>
>> Test: ./post_update.sh && m toybox. Push devmem test into DUT and access
>> /dev/mem through memory mapped access, /dev/nvram & /dev/msr* through
>> non memory-mapped access.
>>
>> ---
>>  android/device/generated/flags.h   |  8 +++--
>>  android/device/generated/help.h    |  2 +-
>>  android/device/generated/newtoys.h |  2 +-
>>  android/linux/generated/flags.h    |  8 +++--
>>  android/linux/generated/help.h     |  2 +-
>>  android/linux/generated/newtoys.h  |  2 +-
>>  android/mac/generated/flags.h      |  8 +++--
>>  android/mac/generated/help.h       |  2 +-
>>  android/mac/generated/newtoys.h    |  2 +-
>>
>
> (these files only exist in AOSP ... sorry, i should have said to apply
> your devmem.c change to a fresh `git clone` of upstream!)
>
>
>>  toys/other/devmem.c                | 49 +++++++++++++++++++-----------
>>  10 files changed, 53 insertions(+), 32 deletions(-)
>>
>> diff --git a/android/device/generated/flags.h
>> b/android/device/generated/flags.h
>> index 9c056073..aa13b2c6 100644
>> --- a/android/device/generated/flags.h
>> +++ b/android/device/generated/flags.h
>> @@ -641,13 +641,14 @@
>>  #undef FOR_demo_utf8towc
>>  #endif
>>
>> -// devmem <1(no-sync)f: <1(no-sync)f:
>> +// devmem <1(no-sync)(no-mmap)f: <1(no-sync)(no-mmap)f:
>>  #undef OPTSTR_devmem
>> -#define OPTSTR_devmem "<1(no-sync)f:"
>> +#define OPTSTR_devmem "<1(no-sync)(no-mmap)f:"
>>  #ifdef CLEANUP_devmem
>>  #undef CLEANUP_devmem
>>  #undef FOR_devmem
>>  #undef FLAG_f
>> +#undef FLAG_no_mmap
>>  #undef FLAG_no_sync
>>  #endif
>>
>> @@ -4547,7 +4548,8 @@
>>  #define TT this.devmem
>>  #endif
>>  #define FLAG_f (1LL<<0)
>> -#define FLAG_no_sync (1LL<<1)
>> +#define FLAG_no_mmap (1LL<<1)
>> +#define FLAG_no_sync (1LL<<2)
>>  #endif
>>
>>  #ifdef FOR_df
>> diff --git a/android/device/generated/help.h
>> b/android/device/generated/help.h
>> index 92dd3687..87f92453 100644
>> --- a/android/device/generated/help.h
>> +++ b/android/device/generated/help.h
>> @@ -330,7 +330,7 @@
>>
>>  #define HELP_dos2unix "usage: dos2unix [FILE...]\n\nConvert newline
>> format from dos \"\\r\\n\" to unix \"\\n\".\nIf no files listed copy from
>> stdin, \"-\" is a synonym for stdin."
>>
>> -#define HELP_devmem "usage: devmem [-f FILE] ADDR [WIDTH
>> [DATA...]]\n\nRead/write physical addresses. WIDTH is 1, 2, 4, or 8 bytes
>> (default 4).\nPrefix ADDR with 0x for hexadecimal, output is in same base
>> as address.\n\n-f FILE            File to operate on (default
>> /dev/mem)\n--no-sync        Don't open the file with O_SYNC (for cached
>> access)"
>> +#define HELP_devmem "usage: devmem [-f FILE] ADDR [WIDTH
>> [DATA...]]\n\nRead/write physical addresses. WIDTH is 1, 2, 4, or 8 bytes
>> (default 4).\nPrefix ADDR with 0x for hexadecimal, output is in same base
>> as address.\n\n-f FILE            File to operate on (default
>> /dev/mem)\n--no-sync        Don't open the file with O_SYNC (for cached
>> access)\n--no-mmap  Don't mmap the file"
>>
>>  #define HELP_count "usage: count [-l]\n\n-l    Long output (total bytes,
>> human readable, transfer rate, elapsed time)\n\nCopy stdin to stdout,
>> displaying simple progress indicator to stderr."
>>
>> diff --git a/android/device/generated/newtoys.h
>> b/android/device/generated/newtoys.h
>> index c41eb83f..e99f4f1c 100644
>> --- a/android/device/generated/newtoys.h
>> +++ b/android/device/generated/newtoys.h
>> @@ -60,7 +60,7 @@ USE_DEMO_MANY_OPTIONS(NEWTOY(demo_many_options,
>> "ZYXWVUTSRQPONMLKJIHGFEDCBAzyxwv
>>  USE_DEMO_NUMBER(NEWTOY(demo_number, "D#=3<3M#<0hcdbs", TOYFLAG_BIN))
>>  USE_DEMO_SCANKEY(NEWTOY(demo_scankey, 0, TOYFLAG_BIN))
>>  USE_DEMO_UTF8TOWC(NEWTOY(demo_utf8towc, 0, TOYFLAG_USR|TOYFLAG_BIN))
>> -USE_DEVMEM(NEWTOY(devmem, "<1(no-sync)f:", TOYFLAG_USR|TOYFLAG_SBIN))
>> +USE_DEVMEM(NEWTOY(devmem, "<1(no-sync)(no-mmap)f:",
>> TOYFLAG_USR|TOYFLAG_SBIN))
>>  USE_DF(NEWTOY(df, "HPkhit*a[-HPh]", TOYFLAG_BIN))
>>  USE_DHCP(NEWTOY(dhcp,
>> "V:H:F:x*r:O*A#<0=20T#<0=3t#<0=3s:p:i:SBRCaovqnbf",
>> TOYFLAG_SBIN|TOYFLAG_ROOTONLY))
>>  USE_DHCP6(NEWTOY(dhcp6, "r:A#<0T#<0t#<0s:p:i:SRvqnbf",
>> TOYFLAG_SBIN|TOYFLAG_ROOTONLY))
>> diff --git a/android/linux/generated/flags.h
>> b/android/linux/generated/flags.h
>> index e73e4bf8..27fcd60d 100644
>> --- a/android/linux/generated/flags.h
>> +++ b/android/linux/generated/flags.h
>> @@ -641,13 +641,14 @@
>>  #undef FOR_demo_utf8towc
>>  #endif
>>
>> -// devmem   <1(no-sync)f:
>> +// devmem   <1(no-sync)(no-mmap)f:
>>  #undef OPTSTR_devmem
>> -#define OPTSTR_devmem "<1(no-sync)f:"
>> +#define OPTSTR_devmem "<1(no-sync)(no-mmap)f:"
>>  #ifdef CLEANUP_devmem
>>  #undef CLEANUP_devmem
>>  #undef FOR_devmem
>>  #undef FLAG_f
>> +#undef FLAG_no_mmap
>>  #undef FLAG_no_sync
>>  #endif
>>
>> @@ -4547,7 +4548,8 @@
>>  #define TT this.devmem
>>  #endif
>>  #define FLAG_f (FORCED_FLAG<<0)
>> -#define FLAG_no_sync (FORCED_FLAG<<1)
>> +#define FLAG_no_mmap (FORCED_FLAG<<1)
>> +#define FLAG_no_sync (FORCED_FLAG<<2)
>>  #endif
>>
>>  #ifdef FOR_df
>> diff --git a/android/linux/generated/help.h
>> b/android/linux/generated/help.h
>> index f25a0785..eccc159a 100644
>> --- a/android/linux/generated/help.h
>> +++ b/android/linux/generated/help.h
>> @@ -332,7 +332,7 @@
>>
>>  #define HELP_dos2unix "usage: dos2unix [FILE...]\n\nConvert newline
>> format from dos \"\\r\\n\" to unix \"\\n\".\nIf no files listed copy from
>> stdin, \"-\" is a synonym for stdin."
>>
>> -#define HELP_devmem "usage: devmem [-f FILE] ADDR [WIDTH
>> [DATA...]]\n\nRead/write physical addresses. WIDTH is 1, 2, 4, or 8 bytes
>> (default 4).\nPrefix ADDR with 0x for hexadecimal, output is in same base
>> as address.\n\n-f FILE            File to operate on (default
>> /dev/mem)\n--no-sync        Don't open the file with O_SYNC (for cached
>> access)"
>> +#define HELP_devmem "usage: devmem [-f FILE] ADDR [WIDTH
>> [DATA...]]\n\nRead/write physical addresses. WIDTH is 1, 2, 4, or 8 bytes
>> (default 4).\nPrefix ADDR with 0x for hexadecimal, output is in same base
>> as address.\n\n-f FILE            File to operate on (default
>> /dev/mem)\n--no-sync        Don't open the file with O_SYNC (for cached
>> access)\n--no-mmap  Don't mmap the file"
>>
>>  #define HELP_count "usage: count [-l]\n\n-l    Long output (total bytes,
>> human readable, transfer rate, elapsed time)\n\nCopy stdin to stdout,
>> displaying simple progress indicator to stderr."
>>
>> diff --git a/android/linux/generated/newtoys.h
>> b/android/linux/generated/newtoys.h
>> index c41eb83f..e99f4f1c 100644
>> --- a/android/linux/generated/newtoys.h
>> +++ b/android/linux/generated/newtoys.h
>> @@ -60,7 +60,7 @@ USE_DEMO_MANY_OPTIONS(NEWTOY(demo_many_options,
>> "ZYXWVUTSRQPONMLKJIHGFEDCBAzyxwv
>>  USE_DEMO_NUMBER(NEWTOY(demo_number, "D#=3<3M#<0hcdbs", TOYFLAG_BIN))
>>  USE_DEMO_SCANKEY(NEWTOY(demo_scankey, 0, TOYFLAG_BIN))
>>  USE_DEMO_UTF8TOWC(NEWTOY(demo_utf8towc, 0, TOYFLAG_USR|TOYFLAG_BIN))
>> -USE_DEVMEM(NEWTOY(devmem, "<1(no-sync)f:", TOYFLAG_USR|TOYFLAG_SBIN))
>> +USE_DEVMEM(NEWTOY(devmem, "<1(no-sync)(no-mmap)f:",
>> TOYFLAG_USR|TOYFLAG_SBIN))
>>  USE_DF(NEWTOY(df, "HPkhit*a[-HPh]", TOYFLAG_BIN))
>>  USE_DHCP(NEWTOY(dhcp,
>> "V:H:F:x*r:O*A#<0=20T#<0=3t#<0=3s:p:i:SBRCaovqnbf",
>> TOYFLAG_SBIN|TOYFLAG_ROOTONLY))
>>  USE_DHCP6(NEWTOY(dhcp6, "r:A#<0T#<0t#<0s:p:i:SRvqnbf",
>> TOYFLAG_SBIN|TOYFLAG_ROOTONLY))
>> diff --git a/android/mac/generated/flags.h b/android/mac/generated/flags.h
>> index 504dd782..59fd25af 100644
>> --- a/android/mac/generated/flags.h
>> +++ b/android/mac/generated/flags.h
>> @@ -641,13 +641,14 @@
>>  #undef FOR_demo_utf8towc
>>  #endif
>>
>> -// devmem   <1(no-sync)f:
>> +// devmem   <1(no-sync)(no-mmap)f:
>>  #undef OPTSTR_devmem
>> -#define OPTSTR_devmem "<1(no-sync)f:"
>> +#define OPTSTR_devmem "<1(no-sync)(no-mmap)f:"
>>  #ifdef CLEANUP_devmem
>>  #undef CLEANUP_devmem
>>  #undef FOR_devmem
>>  #undef FLAG_f
>> +#undef FLAG_no_mmap
>>  #undef FLAG_no_sync
>>  #endif
>>
>> @@ -4547,7 +4548,8 @@
>>  #define TT this.devmem
>>  #endif
>>  #define FLAG_f (FORCED_FLAG<<0)
>> -#define FLAG_no_sync (FORCED_FLAG<<1)
>> +#define FLAG_no_mmap (FORCED_FLAG<<1)
>> +#define FLAG_no_sync (FORCED_FLAG<<2)
>>  #endif
>>
>>  #ifdef FOR_df
>> diff --git a/android/mac/generated/help.h b/android/mac/generated/help.h
>> index f25a0785..eccc159a 100644
>> --- a/android/mac/generated/help.h
>> +++ b/android/mac/generated/help.h
>> @@ -332,7 +332,7 @@
>>
>>  #define HELP_dos2unix "usage: dos2unix [FILE...]\n\nConvert newline
>> format from dos \"\\r\\n\" to unix \"\\n\".\nIf no files listed copy from
>> stdin, \"-\" is a synonym for stdin."
>>
>> -#define HELP_devmem "usage: devmem [-f FILE] ADDR [WIDTH
>> [DATA...]]\n\nRead/write physical addresses. WIDTH is 1, 2, 4, or 8 bytes
>> (default 4).\nPrefix ADDR with 0x for hexadecimal, output is in same base
>> as address.\n\n-f FILE            File to operate on (default
>> /dev/mem)\n--no-sync        Don't open the file with O_SYNC (for cached
>> access)"
>> +#define HELP_devmem "usage: devmem [-f FILE] ADDR [WIDTH
>> [DATA...]]\n\nRead/write physical addresses. WIDTH is 1, 2, 4, or 8 bytes
>> (default 4).\nPrefix ADDR with 0x for hexadecimal, output is in same base
>> as address.\n\n-f FILE            File to operate on (default
>> /dev/mem)\n--no-sync        Don't open the file with O_SYNC (for cached
>> access)\n--no-mmap  Don't mmap the file"
>>
>>  #define HELP_count "usage: count [-l]\n\n-l    Long output (total bytes,
>> human readable, transfer rate, elapsed time)\n\nCopy stdin to stdout,
>> displaying simple progress indicator to stderr."
>>
>> diff --git a/android/mac/generated/newtoys.h
>> b/android/mac/generated/newtoys.h
>> index c41eb83f..e99f4f1c 100644
>> --- a/android/mac/generated/newtoys.h
>> +++ b/android/mac/generated/newtoys.h
>> @@ -60,7 +60,7 @@ USE_DEMO_MANY_OPTIONS(NEWTOY(demo_many_options,
>> "ZYXWVUTSRQPONMLKJIHGFEDCBAzyxwv
>>  USE_DEMO_NUMBER(NEWTOY(demo_number, "D#=3<3M#<0hcdbs", TOYFLAG_BIN))
>>  USE_DEMO_SCANKEY(NEWTOY(demo_scankey, 0, TOYFLAG_BIN))
>>  USE_DEMO_UTF8TOWC(NEWTOY(demo_utf8towc, 0, TOYFLAG_USR|TOYFLAG_BIN))
>> -USE_DEVMEM(NEWTOY(devmem, "<1(no-sync)f:", TOYFLAG_USR|TOYFLAG_SBIN))
>> +USE_DEVMEM(NEWTOY(devmem, "<1(no-sync)(no-mmap)f:",
>> TOYFLAG_USR|TOYFLAG_SBIN))
>>  USE_DF(NEWTOY(df, "HPkhit*a[-HPh]", TOYFLAG_BIN))
>>  USE_DHCP(NEWTOY(dhcp,
>> "V:H:F:x*r:O*A#<0=20T#<0=3t#<0=3s:p:i:SBRCaovqnbf",
>> TOYFLAG_SBIN|TOYFLAG_ROOTONLY))
>>  USE_DHCP6(NEWTOY(dhcp6, "r:A#<0T#<0t#<0s:p:i:SRvqnbf",
>> TOYFLAG_SBIN|TOYFLAG_ROOTONLY))
>> diff --git a/toys/other/devmem.c b/toys/other/devmem.c
>> index 9f9a9e03..f37e8ef3 100644
>> --- a/toys/other/devmem.c
>> +++ b/toys/other/devmem.c
>> @@ -2,7 +2,7 @@
>>   *
>>   * Copyright 2019 The Android Open Source Project
>>
>> -USE_DEVMEM(NEWTOY(devmem, "<1(no-sync)f:", TOYFLAG_USR|TOYFLAG_SBIN))
>> +USE_DEVMEM(NEWTOY(devmem, "<1(no-sync)(no-mmap)f:",
>> TOYFLAG_USR|TOYFLAG_SBIN))
>>
>>  config DEVMEM
>>    bool "devmem"
>> @@ -15,6 +15,7 @@ config DEVMEM
>>
>>      -f FILE            File to operate on (default /dev/mem)
>>      --no-sync  Don't open the file with O_SYNC (for cached access)
>> +    --no-mmap  Don't mmap the file
>>  */
>>
>>  #define FOR_devmem
>> @@ -62,32 +63,46 @@ void devmem_main(void)
>>      flags = writing ? O_RDWR : O_RDONLY;
>>      if (!FLAG(no_sync)) flags |= O_SYNC;
>>      fd = xopen(TT.f ?: "/dev/mem", flags);
>> -    map_off = addr & ~(page_size - 1ULL);
>> -    map_len = (addr+bytes-map_off);
>> -    map = xmmap(0, map_len, writing ? PROT_WRITE : PROT_READ,
>> MAP_SHARED, fd,
>> -        map_off);
>> -    p = map + (addr & (page_size - 1));
>> -    close(fd);
>> +    if (FLAG(no_mmap)) xlseek(fd, addr, SEEK_SET);
>> +    else {
>> +      map_off = addr & ~(page_size - 1ULL);
>> +      map_len = addr + (writing ? (toys.optc - 2) * bytes : bytes) -
>> map_off;
>> +      map = xmmap(0, map_len, writing ? PROT_WRITE : PROT_READ,
>> MAP_SHARED, fd,
>> +          map_off);
>> +      p = map + (addr & (page_size - 1));
>> +      close(fd);
>> +    }
>>    } else p = (void *)addr;
>>
>>    // Not using peek()/poke() because registers care about size of
>> read/write.
>>    if (writing) {
>>      for (int i = 2; i < toys.optc; i++) {
>>        data = xatolu(toys.optargs[i], bytes);
>> -      if (bytes==1) *(char *)p = data;
>> -      else if (bytes==2) *(unsigned short *)p = data;
>> -      else if (bytes==4) *(unsigned int *)p = data;
>> -      else if (sizeof(long)==8 && bytes==8) *(unsigned long *)p = data;
>> -      p += bytes;
>> +      if (FLAG(no_mmap)) {
>> +        xwrite(fd, &data, bytes);
>> +        xlseek(fd, bytes, SEEK_CUR);
>>
>
> is this clearer if you remove the xlseek at the top, and move this
> xlseek() before the xwrite()? i know that would mean incrementing addr, but
> at least the reasoning is localized (and you don't have (a) "aren't these
> the wrong way round?" and (b) "do we actually need a SEEK_CUR? why didn't
> the write advance the file offset?").
>
xlseek at the top helps to maintain a consistent code structure. Also have
removed the incorrect xlseek after xwrite. That usage is incorrect on my
part. I will add test cases to verify in the upcoming patch revision.

Thanks and Regards,
Karthikeyan.

>
>
>> +      } else {
>> +        if (bytes==1) *(char *)p = data;
>> +        else if (bytes==2) *(unsigned short *)p = data;
>> +        else if (bytes==4) *(unsigned int *)p = data;
>> +        else if (sizeof(long)==8 && bytes==8) *(unsigned long *)p = data;
>> +        p += bytes;
>> +      }
>>      }
>>    } else {
>> -    if (bytes==1) data = *(char *)p;
>> -    else if (bytes==2) data = *(unsigned short *)p;
>> -    else if (bytes==4) data = *(unsigned int *)p;
>> -    else if (sizeof(long)==8 && bytes==8) data = *(unsigned long *)p;
>> +    if (FLAG(no_mmap)) xread(fd, &data, bytes);
>> +    else {
>> +      if (bytes==1) data = *(char *)p;
>> +      else if (bytes==2) data = *(unsigned short *)p;
>> +      else if (bytes==4) data = *(unsigned int *)p;
>> +      else if (sizeof(long)==8 && bytes==8) data = *(unsigned long *)p;
>> +    }
>>      printf((!strchr(*toys.optargs, 'x')) ? "%0*ld\n" : "0x%0*lx\n",
>>        bytes*2, data);
>>    }
>>
>> -  if (CFG_TOYBOX_FORK) munmap(map, map_len);
>> +  if (CFG_TOYBOX_FORK) {
>> +    if (FLAG(no_mmap)) close(fd);
>> +    else munmap(map, map_len);
>> +  }
>>  }
>> --
>> 2.47.0.105.g07ac214952-goog
>>
>>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.landley.net/pipermail/toybox-landley.net/attachments/20241025/0d0fa1c1/attachment-0001.htm>


More information about the Toybox mailing list