linux-kernel - Re: [PATCH v2 3/4] arm64/sysreg: Sort sysreg by encoding

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <4cfcb9d8-3187-4ed7-8951-d54d781b4bd9@linaro.org>
Date: Mon, 17 Mar 2025 11:11:00 +0000
From: James Clark <james.clark@...aro.org>
To: Will Deacon <will@...nel.org>, Marc Zyngier <maz@...nel.org>,
 Mark Brown <broonie@...nel.org>
Cc: linux-arm-kernel@...ts.infradead.org, robh@...nel.org,
 Catalin Marinas <catalin.marinas@....com>,
 Mark Rutland <mark.rutland@....com>, Oliver Upton <oliver.upton@...ux.dev>,
 Anshuman Khandual <anshuman.khandual@....com>,
 James Morse <james.morse@....com>, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2 3/4] arm64/sysreg: Sort sysreg by encoding



On 13/03/2025 9:58 pm, Will Deacon wrote:
> On Wed, Jan 15, 2025 at 04:25:57PM +0000, James Clark wrote:
>> It's mostly been sorted by sysreg encoding, but not 100%. Sort it so
>> new entries can be added without wondering where to put them.
>>
>> The following python script was used to sort, keeping the top level
>> SysregFields and comments next to their current Sysreg entries by
>> splitting on "EndSysreg":
>>
>>    # cat arch/arm64/tools/sysreg | python3 sort.py > sorted-sysreg
>>    import sys, re
>>    def key(block):
>>            reg = r"\w+\s+(\d+)\s+(\d+)\s+(\d+)\s+(\d+)\s+(\d+)"
>>            match = re.search(reg, block)
>>            sort_val = ''.join(f"{int(n):02d}" for n in match.groups())
>>            return (sort_val, block)
>>    sysreg = sys.stdin.read().split("\nEndSysreg\n")[:-1]
>>    sysreg = sorted(sysreg, key=key)
>>    print("\nEndSysreg\n".join(sysreg) + "\nEndSysreg")
>>
>> Tested by diffing sorted outputs:
>>
>>    $ diff <(sort arch/arm64/include/generated/asm/sysreg-defs.h)  \
>>           <(sort before-sysreg-defs.h) -s
>>
>>    Files /dev/fd/63 and /dev/fd/62 are identical
>>
>> Signed-off-by: James Clark <james.clark@...aro.org>
>> ---
>>   arch/arm64/tools/sysreg | 1006 +++++++++++++++++++--------------------
>>   1 file changed, 503 insertions(+), 503 deletions(-)
> 
> This looks like unnecessary pain for backporting...
> 
> What do we gain from sorting this?
> 
> Will

It's from the discussion here [1]. But yeah backporting wasn't mentioned 
as a possible issue, it's something to think about.

The summary is:

  * I added one out of order because it wasn't obvious from the unsorted
    file that they were supposed to be in order
  * To avoid more of the same review comments and save time I wanted to
    put a "keep this file sorted" comment
  * But the comment wasn't ok because the file wasn't already sorted, so
    we ended up with sorting it

To be fair adding a "keep it sorted" comment is a bit awkward because 
there are sometimes multiple places you can insert something if it's not 
already fully sorted.

[1]: 
https://lore.kernel.org/all/996c7843-7f51-49a0-9122-e688e37f9902@sirena.org.uk/