[PATCH v3 00/11] Enable setexpr command to print cpu-list like bitmaps

[PATCH v3 00/11] Enable setexpr command to print cpu-list like bitmaps

[lukas.funke-oss]

From: Lukas Funke <lukas.funke@weidmueller.com>


This series enables the 'setexpr' command to print "cpu list"-like
bitmaps based on the printk format specifier [1].

One use-case is to pass cpu list [2] based kernel parameter like
'isolcpu', 'nohz_full', irq affinity or RCU related CPU parameter to
the kernel via a separate firmware variable without exposing the
'bootargs' variable to directly.

Example:

=> env set value 0xdeadbeef
=> setexpr a fmt isolcpus=%32pbl $value
=> echo $a
isolcpus=0-3,5-7,9-13,15-16,18-19,21,23,25-28,30-31

[1] https://www.kernel.org/doc/Documentation/printk-formats.txt
[2] https://www.kernel.org/doc/html/latest/admin-guide/kernel-parameters.html


Changes in v3:
- Use generic find_next_zero_bit() from arch/arm/include/asm/bitops.h
- Redirect sandbox ffz() implementation to generic __ffs() impl
- Remove '%bp' from documentation
- Give an example output in the documentation
- Remove bitmap_string() conversion function since the same function
  can be achieved using other format specifier
- Dereference pointer argument (i.e. *value) in the
  'setexpr name fmt <format> value' case. This is currently only
  supported in the 'setexptr <name> [*]<value>' and
  'setexptr <name> [*]<value> <op> [*]<value2>' case

Changes in v2:
- Add bitmap format specifier to documentation

Lukas Funke (11):
  sandbox: add generic find_next_zero_bit implementation
  linux: bitmap.h: add 'for_each_set_bitrange' iteration macro
  test: cmd: setexpr: Add tests for bitmap string format
  doc: printf() codes: Add bitmap format specifier
  cmd: printf: Correctly handle field width
  lib: Add hextobarray() function
  lib: vsprintf: enable '%pbl' format specifier
  setexpr: rename 'get_arg()' to 'setexpr_get_arg()'
  setexpr: Promote 'setexpr_get_arg()' to a public function
  setexptr: Extend setexpr_get_arg() to handle pointer to memory
  cmd: printf: forward '%p' format string specifier

 arch/sandbox/include/asm/bitops.h | 60 +++++++++++++++++++------------
 cmd/printf.c                      | 53 ++++++++++++++++++++++++++-
 cmd/setexpr.c                     | 48 +++++++++++++++----------
 doc/develop/printf.rst            |  4 +++
 include/command.h                 | 27 ++++++++++++++
 include/linux/bitmap.h            |  7 ++++
 include/vsprintf.h                |  7 ++++
 lib/strto.c                       | 35 ++++++++++++++++++
 lib/vsprintf.c                    | 42 ++++++++++++++++++++++
 test/cmd/setexpr.c                | 22 ++++++++++++
 10 files changed, 263 insertions(+), 42 deletions(-)

-- 
2.30.2

[PATCH v3 01/11] sandbox: add generic find_next_zero_bit implementation

[lukas.funke-oss]

From: Lukas Funke <lukas.funke@weidmueller.com>

Add generic 'find_next_zero_bit()' implementation in order to enable the
use of the 'for_each_set_bitrange' macro. The implementation is currently
missing for the sandbox-arch and using the function results in a linker
error. The implementation is copied from the 'arm' implementation.

Signed-off-by: Lukas Funke <lukas.funke@weidmueller.com>
Reviewed-by: Simon Glass <sjg@chromium.org>
---

Changes in v3:
- Use generic find_next_zero_bit() from arch/arm/include/asm/bitops.h
- Redirect sandbox ffz() implementation to generic __ffs() impl

 arch/sandbox/include/asm/bitops.h | 60 +++++++++++++++++++------------
 1 file changed, 38 insertions(+), 22 deletions(-)

diff --git a/arch/sandbox/include/asm/bitops.h b/arch/sandbox/include/asm/bitops.h
index f27d5e98c5..6950916962 100644
--- a/arch/sandbox/include/asm/bitops.h
+++ b/arch/sandbox/include/asm/bitops.h
@@ -104,9 +104,6 @@ static inline int __test_and_change_bit(int nr, void *addr)
 	return (old & mask) != 0;
 }
 
-extern int find_first_zero_bit(void *addr, unsigned size);
-extern int find_next_zero_bit(void *addr, int size, int offset);
-
 /*
  * This routine doesn't need to be atomic.
  */
@@ -119,27 +116,46 @@ static inline int test_bit(int nr, const void *addr)
  * ffz = Find First Zero in word. Undefined if no zero exists,
  * so code should check against ~0UL first..
  */
-static inline unsigned long ffz(unsigned long word)
-{
-	int k;
-
-	word = ~word;
-	k = 31;
-	if (word & 0x0000ffff) {
-		k -= 16; word <<= 16;
-	}
-	if (word & 0x00ff0000) {
-		k -= 8;  word <<= 8;
-	}
-	if (word & 0x0f000000) {
-		k -= 4;  word <<= 4;
+#define ffz(x)  __ffs(~(x))
+
+#define find_first_zero_bit(addr, size) \
+	find_next_zero_bit((addr), (size), 0)
+
+static inline int find_next_zero_bit(const unsigned long *addr, int size,
+				     int offset) {
+	unsigned long *p = ((unsigned long *)addr) + (offset / BITS_PER_LONG);
+	unsigned long result = offset & ~(BITS_PER_LONG - 1);
+	unsigned long tmp;
+
+	if (offset >= size)
+		return size;
+	size -= result;
+	offset &= (BITS_PER_LONG - 1);
+	if (offset) {
+		tmp = *(p++);
+		tmp |= ~0UL >> (BITS_PER_LONG - offset);
+		if (size < BITS_PER_LONG)
+			goto found_first;
+		if (~tmp)
+			goto found_middle;
+		size -= BITS_PER_LONG;
+		result += BITS_PER_LONG;
 	}
-	if (word & 0x30000000) {
-		k -= 2;  word <<= 2;
+	while (size & ~(BITS_PER_LONG - 1)) {
+		tmp = *(p++);
+		if (~tmp)
+			goto found_middle;
+		result += BITS_PER_LONG;
+		size -= BITS_PER_LONG;
 	}
-	if (word & 0x40000000)
-		k -= 1;
-	return k;
+	if (!size)
+		return result;
+	tmp = *p;
+
+found_first:
+	tmp |= ~0UL << size;
+found_middle:
+	return result + ffz(tmp);
 }
 
 /*
-- 
2.30.2

[PATCH v3 02/11] linux: bitmap.h: add 'for_each_set_bitrange' iteration macro

[lukas.funke-oss]

From: Lukas Funke <lukas.funke@weidmueller.com>

Add 'for_each_set_bitrange' (from Linux kernel) in order to iterate
over each set bitrange of a bitmap. This becomes handy if one wants
to generate a cpu list i.e. for isolcpu or nohz_full.

Signed-off-by: Lukas Funke <lukas.funke@weidmueller.com>
Reviewed-by: Simon Glass <sjg@chromium.org>
---

(no changes since v1)

 include/linux/bitmap.h | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/include/linux/bitmap.h b/include/linux/bitmap.h
index 0a8503af9f..9714533078 100644
--- a/include/linux/bitmap.h
+++ b/include/linux/bitmap.h
@@ -159,6 +159,13 @@ static inline unsigned long find_first_bit(const unsigned long *addr, unsigned l
 	     (bit) < (size);					\
 	     (bit) = find_next_bit((addr), (size), (bit) + 1))
 
+#define for_each_set_bitrange(b, e, addr, size)			\
+	for ((b) = 0;						\
+	     (b) = find_next_bit((addr), (size), b),		\
+	     (e) = find_next_zero_bit((addr), (size), (b) + 1),	\
+	     (b) < (size);					\
+	     (b) = (e) + 1)
+
 static inline unsigned long
 bitmap_find_next_zero_area(unsigned long *map,
 			   unsigned long size,
-- 
2.30.2

[PATCH v3 03/11] test: cmd: setexpr: Add tests for bitmap string format

[lukas.funke-oss]

From: Lukas Funke <lukas.funke@weidmueller.com>

Add tests to test the bitmap format specifier. Test different bit
widths and access to memory by pointer.

Signed-off-by: Lukas Funke <lukas.funke@weidmueller.com>
---

(no changes since v1)

 test/cmd/setexpr.c | 22 ++++++++++++++++++++++
 1 file changed, 22 insertions(+)

diff --git a/test/cmd/setexpr.c b/test/cmd/setexpr.c
index 312593e1e3..c536c9e963 100644
--- a/test/cmd/setexpr.c
+++ b/test/cmd/setexpr.c
@@ -465,6 +465,28 @@ static int setexpr_test_fmt(struct unit_test_state *uts)
 	ut_asserteq(1, run_command("setexpr fred fmt hello% bf", 0));
 	/* Error exceeding maximum string length */
 	ut_asserteq(1, run_command("setexpr fred fmt \"%0128d\" 456", 0));
+	/* Test bitmask long string */
+	ut_assertok(run_command("setexpr fred fmt isolcpu=%64pbl 0x1F1", 0));
+	ut_asserteq_str("isolcpu=0,4-8", env_get("fred"));
+	/* Test bitmask long string (more complicated) */
+	ut_assertok(run_command("setexpr fred fmt nohz_full=%32pbl 0x55555555", 0));
+	ut_asserteq_str("nohz_full=0,2,4,6,8,10,12,14,16,18,20,22,24,26,28,30", env_get("fred"));
+	ut_assertok(run_command("setexpr fred fmt %64pbl 0xdeadbeef", 0));
+	ut_asserteq_str("0-3,5-7,9-13,15-16,18-19,21,23,25-28,30-31", env_get("fred"));
+	/* Test bitmask on 64...256 */
+	ut_assertok(run_command("setexpr fred fmt %64pbl 0xf0f0f0f0f0f0f0f0", 0));
+	ut_asserteq_str("4-7,12-15,20-23,28-31,36-39,44-47,52-55,60-63", env_get("fred"));
+	ut_assertok(run_command("setexpr fred fmt %128pbl 0xf0f0f0f0f0f0f0f0f0f0f0f0f0f0f0f0", 0));
+	ut_asserteq_str("4-7,12-15,20-23,28-31,36-39,44-47,52-55,60-63,68-71,76-79,84-87,92-95,100-103,108-111,116-119,124-127", env_get("fred"));
+	/* clear lower bitmask, otherwise output gets truncated */
+	ut_assertok(run_command("setexpr fred fmt %256pbl 0xf0f0f0f0f0f0f0f0f0f0f0f0f0f0f0f000000000000000000000000000000000", 0));
+	ut_asserteq_str("132-135,140-143,148-151,156-159,164-167,172-175,180-183,188-191,196-199,204-207,212-215,220-223,228-231,236-239,244-247,252-255", env_get("fred"));
+	/* Test memory access */
+	memset(buf, 0, BUF_SIZE);
+	ut_assertok(run_command("env set myaddr 0x0;"
+							"mw.l $myaddr 0xdeadbeef 1;"
+							"setexpr fred fmt %64pbl *$myaddr", 0));
+	ut_asserteq_str("0-3,5-7,9-13,15-16,18-19,21,23,25-28,30-31", env_get("fred"));
 
 	unmap_sysmem(buf);
 
-- 
2.30.2

[PATCH v3 04/11] doc: printf() codes: Add bitmap format specifier

[lukas.funke-oss]

From: Lukas Funke <lukas.funke@weidmueller.com>

Add '%pbl' printf format specifier as descriped in [1].

[1] https://www.kernel.org/doc/Documentation/printk-formats.txt

Signed-off-by: Lukas Funke <lukas.funke@weidmueller.com>
---

Changes in v3:
- Remove '%bp' from documentation
- Give an example output in the documentation

 doc/develop/printf.rst | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/doc/develop/printf.rst b/doc/develop/printf.rst
index 99d05061b1..8220c7c12b 100644
--- a/doc/develop/printf.rst
+++ b/doc/develop/printf.rst
@@ -165,6 +165,10 @@ Pointers
 	* phys_size_t
 	* resource_size_t
 
+%pbl
+        '%pbl' outputs a bitmap as range list with field width as
+        the number of bits. e.g. '0,8-11,13-16,18-19,22-25,27,29,31'
+
 %pD
         prints a UEFI device path
 
-- 
2.30.2

[PATCH v3 05/11] cmd: printf: Correctly handle field width

[lukas.funke-oss]

From: Lukas Funke <lukas.funke@weidmueller.com>

Correctly parse the field width from the format specifier. Before this
commit the field_width was simply ignored.

Signed-off-by: Lukas Funke <lukas.funke@weidmueller.com>
---

(no changes since v1)

 cmd/printf.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/cmd/printf.c b/cmd/printf.c
index 0c6887e0d6..f56543b79e 100644
--- a/cmd/printf.c
+++ b/cmd/printf.c
@@ -517,7 +517,7 @@ static char **print_formatted(struct print_inf *inf, char *f, char **argv, int *
 					field_width = get_width_prec(*argv++);
 			} else {
 				while (isdigit(*f)) {
-					++f;
+					field_width = field_width * 10 + *(f++) - '0';
 					++direc_length;
 				}
 			}
-- 
2.30.2

[PATCH v3 06/11] lib: Add hextobarray() function

[lukas.funke-oss]

From: Lukas Funke <lukas.funke@weidmueller.com>

Add a 'hextobarray()' function which converts a hex string to it's
memory representation. This can be used to represent large integer
numbers or bitmasks which do not fit in a regular unsigned long value.

Signed-off-by: Lukas Funke <lukas.funke@weidmueller.com>
---

(no changes since v1)

 include/vsprintf.h |  7 +++++++
 lib/strto.c        | 35 +++++++++++++++++++++++++++++++++++
 2 files changed, 42 insertions(+)

diff --git a/include/vsprintf.h b/include/vsprintf.h
index ed8a060ee1..82c8bf029e 100644
--- a/include/vsprintf.h
+++ b/include/vsprintf.h
@@ -368,4 +368,11 @@ int vsscanf(const char *inp, char const *fmt0, va_list ap);
  */
 int sscanf(const char *buf, const char *fmt, ...);
 
+/**
+ * hextobarray - Convert a hex-string to a byte array
+ * @cp:		hex string to convert
+ * Return: a pointer to a byte array, -ENOMEM on error
+ */
+uchar *hextobarray(const char *cp);
+
 #endif
diff --git a/lib/strto.c b/lib/strto.c
index 5157332d6c..eb507e4ab8 100644
--- a/lib/strto.c
+++ b/lib/strto.c
@@ -13,6 +13,7 @@
 #include <malloc.h>
 #include <vsprintf.h>
 #include <linux/ctype.h>
+#include <linux/err.h>
 
 /* from lib/kstrtox.c */
 static const char *_parse_integer_fixup_radix(const char *s, uint *basep)
@@ -73,6 +74,40 @@ ulong simple_strtoul(const char *cp, char **endp, uint base)
 	return result;
 }
 
+uchar *hextobarray(const char *cp)
+{
+	int i, len;
+	__maybe_unused unsigned int base;
+	__maybe_unused const char *endptr;
+	unsigned char *array;
+
+	len = strlen(cp);
+	array = (unsigned char *)malloc(len);
+	if (!array)
+		return ERR_PTR(-ENOMEM);
+
+	memset(array, 0, len);
+
+#if __BYTE_ORDER == __LITTLE_ENDIAN
+	endptr = (cp + len - 1);
+	for (i = 0; i < len && endptr > cp; i++) {
+		array[i] |= decode_digit(*(endptr));
+		endptr--;
+		array[i] |= decode_digit(*endptr) << 4;
+		endptr--;
+	}
+#else
+	cp = _parse_integer_fixup_radix(cp, &base);
+	for (i = 0; i < len && *cp; i++) {
+		array[i] |= decode_digit(*cp) << 4;
+		cp++;
+		array[i] |= decode_digit(*cp);
+		cp++;
+	}
+#endif
+	return array;
+}
+
 ulong hextoul(const char *cp, char **endp)
 {
 	return simple_strtoul(cp, endp, 16);
-- 
2.30.2

[PATCH v3 07/11] lib: vsprintf: enable '%pbl' format specifier

[lukas.funke-oss]

From: Lukas Funke <lukas.funke@weidmueller.com>

The commit enables vsprintf() to handle the '%pbl' format specifier
in order to print bitmaps and its derivatives such as cpumask and
nodemask [1]. This can be used to derive kernel boot parameters from
bitmaks such as 'isolcpu' or 'nohz_full' [2].

[1] https://www.kernel.org/doc/Documentation/printk-formats.txt
[2] https://www.kernel.org/doc/html/latest/admin-guide/kernel-parameters.html

Signed-off-by: Lukas Funke <lukas.funke@weidmueller.com>
---

Changes in v3:
- Remove bitmap_string() conversion function since the same function
  can be achieved using other format specifier

 lib/vsprintf.c | 42 ++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 42 insertions(+)

diff --git a/lib/vsprintf.c b/lib/vsprintf.c
index 27ea9c907a..e1779d75f8 100644
--- a/lib/vsprintf.c
+++ b/lib/vsprintf.c
@@ -24,6 +24,7 @@
 #include <linux/err.h>
 #include <linux/types.h>
 #include <linux/string.h>
+#include <linux/bitmap.h>
 
 /* we use this so that we can do without the ctype library */
 #define is_digit(c)	((c) >= '0' && (c) <= '9')
@@ -389,6 +390,33 @@ static char *ip4_addr_string(char *buf, char *end, u8 *addr, int field_width,
 		      flags & ~SPECIAL);
 }
 
+static char *bitmap_list_string(char *buf, char *end, unsigned long *addr,
+				int field_width, int precision, int flags)
+{
+	int nr_bits = max_t(int, field_width, 0);
+	int first = 1;
+	int rbot, rtop;
+
+	for_each_set_bitrange(rbot, rtop, addr, nr_bits) {
+		if (!first) {
+			if (buf < end)
+				*buf = ',';
+			buf++;
+		}
+		first = 0;
+
+		buf = number(buf, end, rbot, 10, 0, -1, 0);
+		if (rtop == rbot + 1)
+			continue;
+
+		if (buf < end)
+			*buf = '-';
+		buf = number(++buf, end, rtop - 1, 10, 0, -1, 0);
+	}
+
+	return buf;
+}
+
 #ifdef CONFIG_LIB_UUID
 /*
  * This works (roughly) the same way as Linux's.
@@ -502,6 +530,20 @@ static char *pointer(const char *fmt, char *buf, char *end, void *ptr,
 					       precision, flags);
 		flags &= ~SPECIAL;
 		break;
+	case 'b':
+		switch (fmt[1]) {
+		case 'l':
+			/* if the field width is not a multiple of the underlying
+			 * datatype (ulong), we get incorrect results from the bit twiddle
+			 * macros. Thus, round up to a multiple of field width of ulong
+			 */
+			field_width = field_width % BITS_PER_LONG ?
+				ALIGN(field_width, BITS_PER_LONG) : field_width;
+			return bitmap_list_string(buf, end, ptr, field_width,
+							precision, flags);
+		default:
+			return ERR_PTR(-EINVAL);
+		}
 #ifdef CONFIG_LIB_UUID
 	case 'U':
 		return uuid_string(buf, end, ptr, field_width, precision,
-- 
2.30.2

Re: [PATCH v3 07/11] lib: vsprintf: enable '%pbl' format specifier

[Tom Rini]

[-- Attachment #1: Type: text/plain, Size: 965 bytes --]

On Wed, Jan 10, 2024 at 10:10:33AM +0100, lukas.funke-oss@weidmueller.com wrote:

> From: Lukas Funke <lukas.funke@weidmueller.com>
> 
> The commit enables vsprintf() to handle the '%pbl' format specifier
> in order to print bitmaps and its derivatives such as cpumask and
> nodemask [1]. This can be used to derive kernel boot parameters from
> bitmaks such as 'isolcpu' or 'nohz_full' [2].
> 
> [1] https://www.kernel.org/doc/Documentation/printk-formats.txt
> [2] https://www.kernel.org/doc/html/latest/admin-guide/kernel-parameters.html
> 
> Signed-off-by: Lukas Funke <lukas.funke@weidmueller.com>

This breaks building ARC platforms, and possibly others.  Note that
tools/buildman/buildman is happy to fetch toolchains for you, and in
this case it's also just the current kernel.org cross-toolchain. Please
see https://docs.u-boot.org/en/latest/develop/ci_testing.html about how
to trigger a CI run prior to sending v4, thanks.

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]

Re: [PATCH v3 07/11] lib: vsprintf: enable '%pbl' format specifier

[Lukas Funke]

Hi Tom,

On 18.01.2024 21:22, Tom Rini wrote:
> On Wed, Jan 10, 2024 at 10:10:33AM +0100, lukas.funke-oss@weidmueller.com wrote:
> 
>> From: Lukas Funke <lukas.funke@weidmueller.com>
>>
>> The commit enables vsprintf() to handle the '%pbl' format specifier
>> in order to print bitmaps and its derivatives such as cpumask and
>> nodemask [1]. This can be used to derive kernel boot parameters from
>> bitmaks such as 'isolcpu' or 'nohz_full' [2].
>>
>> [1] https://www.kernel.org/doc/Documentation/printk-formats.txt
>> [2] https://www.kernel.org/doc/html/latest/admin-guide/kernel-parameters.html
>>
>> Signed-off-by: Lukas Funke <lukas.funke@weidmueller.com>
> 
> This breaks building ARC platforms, and possibly others.  Note that
> tools/buildman/buildman is happy to fetch toolchains for you, and in
> this case it's also just the current kernel.org cross-toolchain. Please
> see https://docs.u-boot.org/en/latest/develop/ci_testing.html about how
> to trigger a CI run prior to sending v4, thanks.
> 

Thanks for the input. I ran the pipeline on github and it failed for 
riscv, m86k, x86_64. The reason is the missing 'find_next_zero_bit()' 
implementation (actually it's faulty on x86_64). What is a good way to 
cope with this problem? My suggestions would be:

- Add a generic 'find_next_zero_bit()'. This would be the
'Linux way' and requires refactoring of the bitops header.
- Add ifdef-macro/kconfig to only enable the format specifier only for 
the supported platforms. This would be the 'quick way'.

What would be your or the maintainers preferred solution?

Best regards
Lukas

Re: [PATCH v3 07/11] lib: vsprintf: enable '%pbl' format specifier

[Tom Rini]

[-- Attachment #1: Type: text/plain, Size: 1599 bytes --]

On Tue, Jan 23, 2024 at 03:48:10PM +0100, Lukas Funke wrote:
> Hi Tom,
> 
> On 18.01.2024 21:22, Tom Rini wrote:
> > On Wed, Jan 10, 2024 at 10:10:33AM +0100, lukas.funke-oss@weidmueller.com wrote:
> > 
> > > From: Lukas Funke <lukas.funke@weidmueller.com>
> > > 
> > > The commit enables vsprintf() to handle the '%pbl' format specifier
> > > in order to print bitmaps and its derivatives such as cpumask and
> > > nodemask [1]. This can be used to derive kernel boot parameters from
> > > bitmaks such as 'isolcpu' or 'nohz_full' [2].
> > > 
> > > [1] https://www.kernel.org/doc/Documentation/printk-formats.txt
> > > [2] https://www.kernel.org/doc/html/latest/admin-guide/kernel-parameters.html
> > > 
> > > Signed-off-by: Lukas Funke <lukas.funke@weidmueller.com>
> > 
> > This breaks building ARC platforms, and possibly others.  Note that
> > tools/buildman/buildman is happy to fetch toolchains for you, and in
> > this case it's also just the current kernel.org cross-toolchain. Please
> > see https://docs.u-boot.org/en/latest/develop/ci_testing.html about how
> > to trigger a CI run prior to sending v4, thanks.
> > 
> 
> Thanks for the input. I ran the pipeline on github and it failed for riscv,
> m86k, x86_64. The reason is the missing 'find_next_zero_bit()'
> implementation (actually it's faulty on x86_64). What is a good way to cope
> with this problem? My suggestions would be:
> 
> - Add a generic 'find_next_zero_bit()'. This would be the
> 'Linux way' and requires refactoring of the bitops header.

Yes, this way please, thanks.

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]

[PATCH v3 08/11] setexpr: rename 'get_arg()' to 'setexpr_get_arg()'

[lukas.funke-oss]

From: Lukas Funke <lukas.funke@weidmueller.com>

Prefix the get_arg() function with 'setexpr_' in order to prepare
the removal of the static specifier. The prefix shall denote the origin
of the function.

Signed-off-by: Lukas Funke <lukas.funke@weidmueller.com>
---

(no changes since v1)

 cmd/setexpr.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/cmd/setexpr.c b/cmd/setexpr.c
index 233471f6cb..bc57d41448 100644
--- a/cmd/setexpr.c
+++ b/cmd/setexpr.c
@@ -34,7 +34,7 @@ struct expr_arg {
 	};
 };
 
-static int get_arg(char *s, int w, struct expr_arg *argp)
+static int setexpr_get_arg(char *s, int w, struct expr_arg *argp)
 {
 	struct expr_arg arg;
 
@@ -388,7 +388,7 @@ static int do_setexpr(struct cmd_tbl *cmdtp, int flag, int argc,
 
 	w = cmd_get_data_size(argv[0], 4);
 
-	if (get_arg(argv[2], w, &aval))
+	if (setexpr_get_arg(argv[2], w, &aval))
 		return CMD_RET_FAILURE;
 
 	/* format string assignment: "setexpr name fmt %d value" */
@@ -441,7 +441,7 @@ static int do_setexpr(struct cmd_tbl *cmdtp, int flag, int argc,
 	if (strlen(argv[3]) != 1)
 		return CMD_RET_USAGE;
 
-	if (get_arg(argv[4], w, &bval)) {
+	if (setexpr_get_arg(argv[4], w, &bval)) {
 		if (w == CMD_DATA_SIZE_STR)
 			free(aval.sval);
 		return CMD_RET_FAILURE;
-- 
2.30.2

[PATCH v3 09/11] setexpr: Promote 'setexpr_get_arg()' to a public function

[lukas.funke-oss]

From: Lukas Funke <lukas.funke@weidmueller.com>

Promote 'setexpr_get_arg()' to a public function in order to use it
from the setexpr command and in the printf-internals.

Signed-off-by: Lukas Funke <lukas.funke@weidmueller.com>
---

(no changes since v1)

 cmd/setexpr.c     | 15 +--------------
 include/command.h | 27 +++++++++++++++++++++++++++
 2 files changed, 28 insertions(+), 14 deletions(-)

diff --git a/cmd/setexpr.c b/cmd/setexpr.c
index bc57d41448..9caa68d20d 100644
--- a/cmd/setexpr.c
+++ b/cmd/setexpr.c
@@ -21,20 +21,7 @@
 
 #define MAX_STR_LEN 128
 
-/**
- * struct expr_arg: Holds an argument to an expression
- *
- * @ival: Integer value (if width is not CMD_DATA_SIZE_STR)
- * @sval: String value (if width is CMD_DATA_SIZE_STR)
- */
-struct expr_arg {
-	union {
-		ulong ival;
-		char *sval;
-	};
-};
-
-static int setexpr_get_arg(char *s, int w, struct expr_arg *argp)
+int setexpr_get_arg(char *s, int w, struct expr_arg *argp)
 {
 	struct expr_arg arg;
 
diff --git a/include/command.h b/include/command.h
index 4cec634545..d0aa98b1f6 100644
--- a/include/command.h
+++ b/include/command.h
@@ -248,6 +248,33 @@ int do_env_set_efi(struct cmd_tbl *cmdtp, int flag, int argc,
 int setexpr_regex_sub(char *data, uint data_size, char *nbuf, uint nbuf_size,
 		      const char *r, const char *s, bool global);
 
+/**
+ * struct expr_arg: Holds an argument to an expression
+ *
+ * @ival: Integer value (if width is not CMD_DATA_SIZE_STR)
+ * @sval: String value (if width is CMD_DATA_SIZE_STR)
+ * @bmap: Bitmap value (if width is > u64)
+ */
+struct expr_arg {
+	union {
+		ulong ival;
+		char *sval;
+		uchar *bmap;
+	};
+};
+
+/**
+ * setexpr_get_arg() - Converts a string argument to it's value. If argument
+ * starts with a '*' treat it as a pointer and dereference.
+ *
+ * @s: Argument string
+ * @w: Byte width of argument
+ * @argp: Pointer where the value should be stored
+ *
+ * Return: 0 on success, -EINVAL on failure
+ */
+int setexpr_get_arg(char *s, int w, struct expr_arg *argp);
+
 /*
  * Error codes that commands return to cmd_process(). We use the standard 0
  * and 1 for success and failure, but add one more case - failure with a
-- 
2.30.2

[PATCH v3 10/11] setexptr: Extend setexpr_get_arg() to handle pointer to memory

[lukas.funke-oss]

From: Lukas Funke <lukas.funke@weidmueller.com>

Extend setexpr_get_arg() function in order to handle bitmaps with
length greater than 8 byte. If the bitmap is provided as hex string
the string is parsed into a bitmap.

Signed-off-by: Lukas Funke <lukas.funke@weidmueller.com>
---

(no changes since v1)

 cmd/setexpr.c | 29 ++++++++++++++++++++++++++---
 1 file changed, 26 insertions(+), 3 deletions(-)

diff --git a/cmd/setexpr.c b/cmd/setexpr.c
index 9caa68d20d..fed457bb7e 100644
--- a/cmd/setexpr.c
+++ b/cmd/setexpr.c
@@ -17,6 +17,7 @@
 #include <malloc.h>
 #include <mapmem.h>
 #include <linux/sizes.h>
+#include <linux/err.h>
 #include "printf.h"
 
 #define MAX_STR_LEN 128
@@ -24,6 +25,8 @@
 int setexpr_get_arg(char *s, int w, struct expr_arg *argp)
 {
 	struct expr_arg arg;
+	uchar *bmap;
+	ulong val;
 
 	/*
 	 * If the parameter starts with a '*' then assume it is a pointer to
@@ -32,7 +35,6 @@ int setexpr_get_arg(char *s, int w, struct expr_arg *argp)
 	if (s[0] == '*') {
 		ulong *p;
 		ulong addr;
-		ulong val;
 		int len;
 		char *str;
 
@@ -71,17 +73,38 @@ int setexpr_get_arg(char *s, int w, struct expr_arg *argp)
 			unmap_sysmem(p);
 			arg.ival = val;
 			break;
-		default:
+#if BITS_PER_LONG == 64
+		case 8:
 			p = map_sysmem(addr, sizeof(ulong));
 			val = *p;
 			unmap_sysmem(p);
 			arg.ival = val;
 			break;
+#endif
+		default:
+			p = map_sysmem(addr, w);
+			bmap = malloc(w);
+			if (!bmap) {
+				printf("Out of memory\n");
+				return -ENOMEM;
+			}
+			memcpy(bmap, p, w);
+			arg.bmap = bmap;
+			unmap_sysmem(p);
 		}
 	} else {
 		if (w == CMD_DATA_SIZE_STR)
 			return -EINVAL;
-		arg.ival = hextoul(s, NULL);
+		if (w > sizeof(ulong)) {
+			bmap = hextobarray(s);
+			if (IS_ERR(bmap)) {
+				printf("Out of memory\n");
+				return -ENOMEM;
+			}
+			arg.bmap = bmap;
+		} else {
+			arg.ival = hextoul(s, NULL);
+		}
 	}
 	*argp = arg;
 
-- 
2.30.2

[PATCH v3 11/11] cmd: printf: forward '%p' format string specifier

[lukas.funke-oss]

From: Lukas Funke <lukas.funke@weidmueller.com>

Forward '%p' format specifier to the underlying format logic in order
to print pointers, especially bitmaps.

Signed-off-by: Lukas Funke <lukas.funke@weidmueller.com>
---

Changes in v3:
- Dereference pointer argument (i.e. *value) in the
  'setexpr name fmt <format> value' case. This is currently only
  supported in the 'setexptr <name> [*]<value>' and
  'setexptr <name> [*]<value> <op> [*]<value2>' case

 cmd/printf.c | 51 +++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 51 insertions(+)

diff --git a/cmd/printf.c b/cmd/printf.c
index f56543b79e..2e54faf339 100644
--- a/cmd/printf.c
+++ b/cmd/printf.c
@@ -85,11 +85,13 @@
  */
 
 #include <common.h>
+#include <command.h>
 #include <ctype.h>
 #include <errno.h>
 #include <stddef.h>
 #include <stdio.h>
 #include <stdlib.h>
+#include <linux/bitmap.h>
 
 #define WANT_HEX_ESCAPES 0
 #define PRINT_CONVERSION_ERROR 1
@@ -476,6 +478,38 @@ static int get_width_prec(const char *str)
 	return (int)v;
 }
 
+static int print_pointer(struct print_inf *inf, char *format,
+			 unsigned int fmt_length, int field_width,
+			 int precision, const char *argument)
+{
+	struct expr_arg aval;
+
+	if (setexpr_get_arg(skip_whitespace(argument), field_width >> 3, &aval))
+		return CMD_RET_FAILURE;
+
+	if (field_width > BITS_PER_LONG) {
+		printf_str(inf, format, aval.bmap);
+		free(aval.bmap);
+	} else {
+		printf_str(inf, format, &aval.ival);
+	}
+
+	switch (inf->error) {
+	case 0:
+		return 0;
+	case PRINT_SIZE_ERROR:
+		printf("printf: size error\n"); break;
+	case PRINT_CONVERSION_ERROR:
+		printf("printf: conversion error\n"); break;
+	case PRINT_TRUNCATED_ERROR:
+		printf("printf: output truncated\n"); break;
+	default:
+		printf("printf: unknown error\n");
+	}
+
+	return -1;
+}
+
 /* Print the text in FORMAT, using ARGV for arguments to any '%' directives.
  * Return advanced ARGV.
  */
@@ -536,6 +570,23 @@ static char **print_formatted(struct print_inf *inf, char *f, char **argv, int *
 					}
 				}
 			}
+			if (*f == 'p') {
+				static const char ptr_format_chars[] = "bl";
+				++f;
+				++direc_length;
+				char *p = strchr(ptr_format_chars, *f);
+				/* consume whole format token */
+				while (*f != '\0' && *(p++) == *f) {
+					++f;
+					++direc_length;
+				}
+				if (print_pointer(inf, direc_start, direc_length,
+						  field_width, precision, *argv++)) {
+					return saved_argv - 1;
+				}
+				f--;
+				break;
+			}
 
 			/* Remove "lLhz" size modifiers, repeatedly.
 			 * bash does not like "%lld", but coreutils
-- 
2.30.2