[PATCH v2 0/6] Enable setexpr command to print cpu-list like bitmaps
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:
setexpr isolcpu_bootarg=%32pbl $myCPUisolation && env set bootargs "$isolcpu_bootarg" && bootm
[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 v2: - Add bitmap format specifier to documentation
Lukas Funke (6): sandbox: add generic find_next_zero_bit implementation linux: bitmap.h: add 'for_each_set_bitrange' iteration macro test: cmd: setexptr: Add tests for bitmap string format doc: printf() codes: Add bitmap format specifier lib: vsprintf: enable '%*pb[l]' format specifier cmd: printf: forward '%p' format string specifier
arch/sandbox/include/asm/bitops.h | 16 ++++++- cmd/printf.c | 29 ++++++++++++ doc/develop/printf.rst | 6 +++ include/linux/bitmap.h | 7 +++ lib/vsprintf.c | 75 +++++++++++++++++++++++++++++++ test/cmd/setexpr.c | 9 ++++ 6 files changed, 140 insertions(+), 2 deletions(-)
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.
There are more efficient implementations in the architecture specific implementations. However, for the sandbox the implementation should be simple and portable.
Signed-off-by: Lukas Funke lukas.funke@weidmueller.com ---
(no changes since v1)
arch/sandbox/include/asm/bitops.h | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-)
diff --git a/arch/sandbox/include/asm/bitops.h b/arch/sandbox/include/asm/bitops.h index f27d5e98c5..453ff005d2 100644 --- a/arch/sandbox/include/asm/bitops.h +++ b/arch/sandbox/include/asm/bitops.h @@ -104,8 +104,20 @@ 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); +#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 >> 5); + + while ((~(*p) & 0x1 << offset) == 0x0ll && (offset < size)) { + offset++; + p = ((unsigned long *)addr) + (offset >> 5); + } + + return offset; +}
/* * This routine doesn't need to be atomic.
Hi,
On Tue, 12 Dec 2023 at 01:53, lukas.funke-oss@weidmueller.com wrote:
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.
There are more efficient implementations in the architecture specific implementations. However, for the sandbox the implementation should be simple and portable.
Signed-off-by: Lukas Funke lukas.funke@weidmueller.com
(no changes since v1)
arch/sandbox/include/asm/bitops.h | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-)
Reviewed-by: Simon Glass sjg@chromium.org
If you wrote this yourself, this is fine. If it came from Linux, etc. then please add mention of that.
Regards, Simon
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 ---
(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,
From: Lukas Funke lukas.funke@weidmueller.com
Add test to test the bitmap format specifier
Signed-off-by: Lukas Funke lukas.funke@weidmueller.com ---
(no changes since v1)
test/cmd/setexpr.c | 9 +++++++++ 1 file changed, 9 insertions(+)
diff --git a/test/cmd/setexpr.c b/test/cmd/setexpr.c index 312593e1e3..4e1c9e983b 100644 --- a/test/cmd/setexpr.c +++ b/test/cmd/setexpr.c @@ -465,6 +465,15 @@ 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=%32pbl 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")); + /* Test bitmask short string*/ + ut_assertok(run_command("setexpr fred fmt %32pb 0x55555555", 0)); + ut_asserteq_str("55555555", env_get("fred"));
unmap_sysmem(buf);
On Tue, 12 Dec 2023 at 01:53, lukas.funke-oss@weidmueller.com wrote:
From: Lukas Funke lukas.funke@weidmueller.com
Add test to test the bitmap format specifier
Signed-off-by: Lukas Funke lukas.funke@weidmueller.com
(no changes since v1)
test/cmd/setexpr.c | 9 +++++++++ 1 file changed, 9 insertions(+)
Reviewed-by: Simon Glass sjg@chromium.org
From: Lukas Funke lukas.funke@weidmueller.com
Add '%*pb[l]' 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 ---
(no changes since v1)
doc/develop/printf.rst | 6 ++++++ 1 file changed, 6 insertions(+)
diff --git a/doc/develop/printf.rst b/doc/develop/printf.rst index 99d05061b1..c3537b1796 100644 --- a/doc/develop/printf.rst +++ b/doc/develop/printf.rst @@ -165,6 +165,12 @@ Pointers * phys_size_t * resource_size_t
+%*pb, %*pbl + prints bitmap and its derivatives such as cpumask and nodemask. + '%*pb' outputs the bitmap with field width as the number of bits + and '%*pbl' outputs the bitmap as range list with field width as + the number of bits. + %pD prints a UEFI device path
On 12.12.23 09:52, lukas.funke-oss@weidmueller.com wrote:
From: Lukas Funke lukas.funke@weidmueller.com
Add '%*pb[l]' 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
(no changes since v1)
doc/develop/printf.rst | 6 ++++++ 1 file changed, 6 insertions(+)
diff --git a/doc/develop/printf.rst b/doc/develop/printf.rst index 99d05061b1..c3537b1796 100644 --- a/doc/develop/printf.rst +++ b/doc/develop/printf.rst @@ -165,6 +165,12 @@ Pointers
- phys_size_t
- resource_size_t
+%*pb, %*pbl
prints bitmap and its derivatives such as cpumask and nodemask.'%*pb' outputs the bitmap with field width as the number of bitsand '%*pbl' outputs the bitmap as range list with field width asthe number of bits.
This looks like '*' has to added as character. I would leave it away as you can always have %<width>.<precision><format specifier>. '*' is only needed if you want to path the width as extra parameter.
With the given description I would not have expected an output like '2,5-6' for the number 0x64. Please, add separate entries for %pb and %pbl and provide examples.
=> setexpr a fmt '%320pb' 100; echo $a 0000555a,00000000,00000000,00000000,00000000,00000080,00007fff,f0094f48,00000000,00000064 => setexpr a fmt '%3200pb' 100; echo $a `%3200pb': invalid format 0000555a,00000000,00000000,00000000,00000000,00000080,00007fff,f0094f48,00000000,00000064
Nothing in the documentation makes me assume that width 320 should work and 3200 should not.
Best regards
Heinrich
- %pD prints a UEFI device path
From: Lukas Funke lukas.funke@weidmueller.com
The commit enables vsprintf() to handle the '%*pb[l]' 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 ---
(no changes since v1)
lib/vsprintf.c | 75 ++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 75 insertions(+)
diff --git a/lib/vsprintf.c b/lib/vsprintf.c index e14c6ca9f9..abbd80ea9c 100644 --- a/lib/vsprintf.c +++ b/lib/vsprintf.c @@ -25,6 +25,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') @@ -390,6 +391,71 @@ static char *ip4_addr_string(char *buf, char *end, u8 *addr, int field_width, flags & ~SPECIAL); }
+static char *bitmap_string(char *buf, char *end, const unsigned long *bitmap, + int field_width, int precision, int flags) +{ + const int CHUNKSIZE = 32; + int nr_bits = max_t(int, field_width, 0); + int i, chunksz; + int first = 1; + + chunksz = nr_bits & (CHUNKSIZE - 1); + if (chunksz == 0) + chunksz = CHUNKSIZE; + + i = ALIGN(nr_bits, CHUNKSIZE) - CHUNKSIZE; + for (; i >= 0; i -= CHUNKSIZE) { + u32 chunkmask, val; + int word, bit; + + chunkmask = ((1ULL << chunksz) - 1); + word = i / BITS_PER_LONG; + bit = i % BITS_PER_LONG; + val = (bitmap[word] >> bit) & chunkmask; + + if (!first) { + if (buf < end) + *buf = ','; + buf++; + } + first = 0; + + field_width = DIV_ROUND_UP(chunksz, 4); + buf = number(buf, end, val, 16, field_width, precision, + (SMALL | ZEROPAD)); + + chunksz = CHUNKSIZE; + } + return buf; +} + +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. @@ -503,6 +569,15 @@ static char *pointer(const char *fmt, char *buf, char *end, void *ptr, precision, flags); flags &= ~SPECIAL; break; + case 'b': + switch (fmt[1]) { + case 'l': + return bitmap_list_string(buf, end, ptr, field_width, + precision, flags); + default: + return bitmap_string(buf, end, ptr, field_width, + precision, flags); + } #ifdef CONFIG_LIB_UUID case 'U': return uuid_string(buf, end, ptr, field_width, precision,
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 ---
(no changes since v1)
cmd/printf.c | 29 +++++++++++++++++++++++++++++ 1 file changed, 29 insertions(+)
diff --git a/cmd/printf.c b/cmd/printf.c index 0c6887e0d6..a90c923871 100644 --- a/cmd/printf.c +++ b/cmd/printf.c @@ -90,6 +90,7 @@ #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 +477,16 @@ 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, const char *argument) +{ + u64 value = simple_strtoull(argument, NULL, 0); + + printf_str(inf, format, &value); + + return inf->error; +} + /* Print the text in FORMAT, using ARGV for arguments to any '%' directives. * Return advanced ARGV. */ @@ -536,6 +547,24 @@ 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, *argv++)) { + printf("`%s': invalid format\n", direc_start); + /* causes main() to exit with error */ + return saved_argv - 1; + } + f--; + break; + }
/* Remove "lLhz" size modifiers, repeatedly. * bash does not like "%lld", but coreutils
On 12.12.23 09:52, lukas.funke-oss@weidmueller.com wrote:
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
(no changes since v1)
cmd/printf.c | 29 +++++++++++++++++++++++++++++ 1 file changed, 29 insertions(+)
diff --git a/cmd/printf.c b/cmd/printf.c index 0c6887e0d6..a90c923871 100644 --- a/cmd/printf.c +++ b/cmd/printf.c @@ -90,6 +90,7 @@ #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 +477,16 @@ 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, const char *argument)+{
- u64 value = simple_strtoull(argument, NULL, 0);
- printf_str(inf, format, &value);
- return inf->error;
+}
- /* Print the text in FORMAT, using ARGV for arguments to any '%' directives.
*/
- Return advanced ARGV.
@@ -536,6 +547,24 @@ 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, *argv++)) {printf("`%s': invalid format\n", direc_start);
There are different error codes. PRINT_TRUNCATED_ERROR does not indicate an invalid format.
Best regards
Heinrich
/* causes main() to exit with error */return saved_argv - 1;}f--;break;} /* Remove "lLhz" size modifiers, repeatedly. * bash does not like "%lld", but coreutils
On 12.12.23 09:52, lukas.funke-oss@weidmueller.com wrote:
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:
setexpr isolcpu_bootarg=%32pbl $myCPUisolation
I applied your patches on top of origin/next but your example simply does not work:
=> setenv myCPUisolation 123456789abcdef0 => setexpr isolcpu_bootarg=%32pbl $myCPUisolation ## Error: illegal character '='in variable name "isolcpu_bootarg=%32pbl"
This produces some output:
=> setexpr a fmt '%32pbl' 64; echo $a 6
But the result is completely unexpected. Number arguments in U-Boot are always hexadecimal. So the output should be
2,5-6
Best regards
Heinrich
&& env set bootargs "$isolcpu_bootarg" && bootm
[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 v2:
- Add bitmap format specifier to documentation
Lukas Funke (6): sandbox: add generic find_next_zero_bit implementation linux: bitmap.h: add 'for_each_set_bitrange' iteration macro test: cmd: setexptr: Add tests for bitmap string format doc: printf() codes: Add bitmap format specifier lib: vsprintf: enable '%*pb[l]' format specifier cmd: printf: forward '%p' format string specifier
arch/sandbox/include/asm/bitops.h | 16 ++++++- cmd/printf.c | 29 ++++++++++++ doc/develop/printf.rst | 6 +++ include/linux/bitmap.h | 7 +++ lib/vsprintf.c | 75 +++++++++++++++++++++++++++++++ test/cmd/setexpr.c | 9 ++++ 6 files changed, 140 insertions(+), 2 deletions(-)
On 12.12.23 11:13, Heinrich Schuchardt wrote:
On 12.12.23 09:52, lukas.funke-oss@weidmueller.com wrote:
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:
setexpr isolcpu_bootarg=%32pbl $myCPUisolation
I applied your patches on top of origin/next but your example simply does not work:
=> setenv myCPUisolation 123456789abcdef0 => setexpr isolcpu_bootarg=%32pbl $myCPUisolation ## Error: illegal character '='in variable name "isolcpu_bootarg=%32pbl"
This produces some output:
=> setexpr a fmt '%32pbl' 64; echo $a 6
But the result is completely unexpected. Number arguments in U-Boot are always hexadecimal. So the output should be
2,5-6
doc/usage/cmd/setexpr.rst
has an example showing this.
=> setexpr foo fmt 0x%08x 63 => echo $foo 0x00000063 =>
You provide a use case for %pbl but not an reason why you want to support %pb. If you wanted to use the output in a device-tree manipulation the %pb output would have to be big-endian formatted.
Best regards
Heinrcih
&& env set bootargs "$isolcpu_bootarg" && bootm
[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 v2:
- Add bitmap format specifier to documentation
Lukas Funke (6): sandbox: add generic find_next_zero_bit implementation linux: bitmap.h: add 'for_each_set_bitrange' iteration macro test: cmd: setexptr: Add tests for bitmap string format doc: printf() codes: Add bitmap format specifier lib: vsprintf: enable '%*pb[l]' format specifier cmd: printf: forward '%p' format string specifier
arch/sandbox/include/asm/bitops.h | 16 ++++++- cmd/printf.c | 29 ++++++++++++ doc/develop/printf.rst | 6 +++ include/linux/bitmap.h | 7 +++ lib/vsprintf.c | 75 +++++++++++++++++++++++++++++++ test/cmd/setexpr.c | 9 ++++ 6 files changed, 140 insertions(+), 2 deletions(-)
On 12.12.2023 12:40, Heinrich Schuchardt wrote:
On 12.12.23 11:13, Heinrich Schuchardt wrote:
On 12.12.23 09:52, lukas.funke-oss@weidmueller.com wrote:
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:
setexpr isolcpu_bootarg=%32pbl $myCPUisolation
I applied your patches on top of origin/next but your example simply does not work:
=> setenv myCPUisolation 123456789abcdef0 => setexpr isolcpu_bootarg=%32pbl $myCPUisolation ## Error: illegal character '='in variable name "isolcpu_bootarg=%32pbl"
This produces some output:
=> setexpr a fmt '%32pbl' 64; echo $a 6
But the result is completely unexpected. Number arguments in U-Boot are always hexadecimal. So the output should be
2,5-6
doc/usage/cmd/setexpr.rst
has an example showing this.
=> setexpr foo fmt 0x%08x 63 => echo $foo 0x00000063 =>
You provide a use case for %pbl but not an reason why you want to support %pb. If you wanted to use the output in a device-tree manipulation the %pb output would have to be big-endian formatted.
I added it to be consistent with the kernel printk. Currently there is no use case for it. Shall we remove the %pb or leave it in for consistency?
Best regards Lukas
Best regards
Heinrcih
&& env set bootargs "$isolcpu_bootarg" && bootm
[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 v2:
- Add bitmap format specifier to documentation
Lukas Funke (6): sandbox: add generic find_next_zero_bit implementation linux: bitmap.h: add 'for_each_set_bitrange' iteration macro test: cmd: setexptr: Add tests for bitmap string format doc: printf() codes: Add bitmap format specifier lib: vsprintf: enable '%*pb[l]' format specifier cmd: printf: forward '%p' format string specifier
arch/sandbox/include/asm/bitops.h | 16 ++++++- cmd/printf.c | 29 ++++++++++++ doc/develop/printf.rst | 6 +++ include/linux/bitmap.h | 7 +++ lib/vsprintf.c | 75 +++++++++++++++++++++++++++++++ test/cmd/setexpr.c | 9 ++++ 6 files changed, 140 insertions(+), 2 deletions(-)
On 12.12.23 12:56, Lukas Funke wrote:
On 12.12.2023 12:40, Heinrich Schuchardt wrote:
On 12.12.23 11:13, Heinrich Schuchardt wrote:
On 12.12.23 09:52, lukas.funke-oss@weidmueller.com wrote:
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:
setexpr isolcpu_bootarg=%32pbl $myCPUisolation
I applied your patches on top of origin/next but your example simply does not work:
=> setenv myCPUisolation 123456789abcdef0 => setexpr isolcpu_bootarg=%32pbl $myCPUisolation ## Error: illegal character '='in variable name "isolcpu_bootarg=%32pbl"
This produces some output:
=> setexpr a fmt '%32pbl' 64; echo $a 6
But the result is completely unexpected. Number arguments in U-Boot are always hexadecimal. So the output should be
2,5-6
doc/usage/cmd/setexpr.rst
has an example showing this.
=> setexpr foo fmt 0x%08x 63 => echo $foo 0x00000063 =>
You provide a use case for %pbl but not an reason why you want to support %pb. If you wanted to use the output in a device-tree manipulation the %pb output would have to be big-endian formatted.
I added it to be consistent with the kernel printk. Currently there is no use case for it. Shall we remove the %pb or leave it in for consistency?
As long as we don't know if usage requires big-endian or little-endian I would keep it out.
Looking at the %pbl use case: What sources of the value to convert do you envision? When the argument is a pointer, e.g.
setexpr a fmt '%128pbl' *$fdtcontroladdr
you seem to expect that the 32bit tuples are stored in low endian order.
Please, test the output of a 64bit number on a big-endian system. You must not assume a unsigned long long to be stored as 32bit tuples in low endian order here.
The output does not match what is in memory. This needs to be fixed:
=> setexpr a fmt '%128pbl' *0x8bae060; echo $a 64,66,69-72,74,76,79,82,85-87,89,91-92,94,96-98,108,110,112,114,117,119,121,123,125,127 => setexpr a fmt '%128pb' *0x8bae060; echo $a feedf00d,deadbeef,00000000,00000000 => mm.l 8bae060 08bae060: edfe0dd0 ? 08bae064: 92ac0000 ? 08bae068: 78000000 ? 08bae06c: 208a0000 ?
Here is another wrong output:
=> setexpr a fmt '%128pb' 0x92acd00dfeed; echo $a feedf00d,deadbeef,000092ac,d00dfeed
Expected output is
00000000,00000000,000092ac,d00dfeed
A buffer overrun occurs. You are reading memory after the u64 value that you create internally. To do it correctly you need to copy the u64 value into a 128 bit buffer that has been zeroed out.
Best regards
Heinrich
Best regards Lukas
Best regards
Heinrcih
&& env set bootargs "$isolcpu_bootarg" && bootm
[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 v2:
- Add bitmap format specifier to documentation
Lukas Funke (6): sandbox: add generic find_next_zero_bit implementation linux: bitmap.h: add 'for_each_set_bitrange' iteration macro test: cmd: setexptr: Add tests for bitmap string format doc: printf() codes: Add bitmap format specifier lib: vsprintf: enable '%*pb[l]' format specifier cmd: printf: forward '%p' format string specifier
arch/sandbox/include/asm/bitops.h | 16 ++++++- cmd/printf.c | 29 ++++++++++++ doc/develop/printf.rst | 6 +++ include/linux/bitmap.h | 7 +++ lib/vsprintf.c | 75 +++++++++++++++++++++++++++++++ test/cmd/setexpr.c | 9 ++++ 6 files changed, 140 insertions(+), 2 deletions(-)
On 12.12.2023 14:09, Heinrich Schuchardt wrote:
On 12.12.23 12:56, Lukas Funke wrote:
On 12.12.2023 12:40, Heinrich Schuchardt wrote:
On 12.12.23 11:13, Heinrich Schuchardt wrote:
On 12.12.23 09:52, lukas.funke-oss@weidmueller.com wrote:
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:
setexpr isolcpu_bootarg=%32pbl $myCPUisolation
I applied your patches on top of origin/next but your example simply does not work:
=> setenv myCPUisolation 123456789abcdef0 => setexpr isolcpu_bootarg=%32pbl $myCPUisolation ## Error: illegal character '='in variable name "isolcpu_bootarg=%32pbl"
This produces some output:
=> setexpr a fmt '%32pbl' 64; echo $a 6
But the result is completely unexpected. Number arguments in U-Boot are always hexadecimal. So the output should be
2,5-6
doc/usage/cmd/setexpr.rst
has an example showing this.
=> setexpr foo fmt 0x%08x 63 => echo $foo 0x00000063 =>
You provide a use case for %pbl but not an reason why you want to support %pb. If you wanted to use the output in a device-tree manipulation the %pb output would have to be big-endian formatted.
I added it to be consistent with the kernel printk. Currently there is no use case for it. Shall we remove the %pb or leave it in for consistency?
As long as we don't know if usage requires big-endian or little-endian I would keep it out.
Ok, will remove '%bp'.
Looking at the %pbl use case: What sources of the value to convert do you envision? When the argument is a pointer, e.g.
setexpr a fmt '%128pbl' *$fdtcontroladdr
The dereference of an address in 'setexpr name fmt <format> <value> ...' doesn't work. This is only handled in 'setexptr name <value>. I'll try to add this to the 'fmt' case.
Best regards Lukas
you seem to expect that the 32bit tuples are stored in low endian order.
Please, test the output of a 64bit number on a big-endian system. You must not assume a unsigned long long to be stored as 32bit tuples in low endian order here.
The output does not match what is in memory. This needs to be fixed:
=> setexpr a fmt '%128pbl' *0x8bae060; echo $a 64,66,69-72,74,76,79,82,85-87,89,91-92,94,96-98,108,110,112,114,117,119,121,123,125,127 => setexpr a fmt '%128pb' *0x8bae060; echo $a feedf00d,deadbeef,00000000,00000000 => mm.l 8bae060 08bae060: edfe0dd0 ? 08bae064: 92ac0000 ? 08bae068: 78000000 ? 08bae06c: 208a0000 ?
Here is another wrong output:
=> setexpr a fmt '%128pb' 0x92acd00dfeed; echo $a feedf00d,deadbeef,000092ac,d00dfeed
Expected output is
00000000,00000000,000092ac,d00dfeed
A buffer overrun occurs. You are reading memory after the u64 value that you create internally. To do it correctly you need to copy the u64 value into a 128 bit buffer that has been zeroed out.
Best regards
Heinrich
Best regards Lukas
Best regards
Heinrcih
&& env set bootargs "$isolcpu_bootarg" && bootm
[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 v2:
- Add bitmap format specifier to documentation
Lukas Funke (6): sandbox: add generic find_next_zero_bit implementation linux: bitmap.h: add 'for_each_set_bitrange' iteration macro test: cmd: setexptr: Add tests for bitmap string format doc: printf() codes: Add bitmap format specifier lib: vsprintf: enable '%*pb[l]' format specifier cmd: printf: forward '%p' format string specifier
arch/sandbox/include/asm/bitops.h | 16 ++++++- cmd/printf.c | 29 ++++++++++++ doc/develop/printf.rst | 6 +++ include/linux/bitmap.h | 7 +++ lib/vsprintf.c | 75 +++++++++++++++++++++++++++++++ test/cmd/setexpr.c | 9 ++++ 6 files changed, 140 insertions(+), 2 deletions(-)
Hi Heinrich,
On 12.12.2023 11:13, Heinrich Schuchardt wrote:
On 12.12.23 09:52, lukas.funke-oss@weidmueller.com wrote:
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:
setexpr isolcpu_bootarg=%32pbl $myCPUisolation
I applied your patches on top of origin/next but your example simply does not work:
=> setenv myCPUisolation 123456789abcdef0 => setexpr isolcpu_bootarg=%32pbl $myCPUisolation ## Error: illegal character '='in variable name "isolcpu_bootarg=%32pbl"
Thanks for testing.
I messed up the example in the cover letter. In your case it actually should be
=> setenv myCPUisolation 0x123456789abcdef0 => setexpr bootargs fmt isolcpu_bootarg=%64pbl $myCPUisolation => echo $bootargs isolcpu_bootarg=4-7,9-12,14-15,18-21,23,25,27-28,31,35-37,41,44,46-47,50,52,57-59
...which is wrong btw (the upper half bitmask is crooked).
This produces some output:
=> setexpr a fmt '%32pbl' 64; echo $a 6
But the result is completely unexpected. Number arguments in U-Boot are always hexadecimal. So the output should be
Thanks for pointing it out! The internals use autodetect from strtoll(...) which handles decimal and hexadecimal. All other uses of strtoll use 16 as base. I'll address this issue in the next version.
2,5-6
Best regards
Heinrich
&& env set bootargs "$isolcpu_bootarg" && bootm
[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 v2:
- Add bitmap format specifier to documentation
Lukas Funke (6): sandbox: add generic find_next_zero_bit implementation linux: bitmap.h: add 'for_each_set_bitrange' iteration macro test: cmd: setexptr: Add tests for bitmap string format doc: printf() codes: Add bitmap format specifier lib: vsprintf: enable '%*pb[l]' format specifier cmd: printf: forward '%p' format string specifier
arch/sandbox/include/asm/bitops.h | 16 ++++++- cmd/printf.c | 29 ++++++++++++ doc/develop/printf.rst | 6 +++ include/linux/bitmap.h | 7 +++ lib/vsprintf.c | 75 +++++++++++++++++++++++++++++++ test/cmd/setexpr.c | 9 ++++ 6 files changed, 140 insertions(+), 2 deletions(-)
participants (4)
-
Heinrich Schuchardt -
Lukas Funke -
lukas.funke-oss＠weidmueller.com
-
Simon Glass