[PATCH v2 0/6] Enable setexpr command to print cpu-list like bitmaps

[Lukas Funke]

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 at weidmueller.com wrote:
>>>>> From: Lukas Funke <lukas.funke at 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(-)
>>>>>
>>>>
>>>
>>
>