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(-)