arm64: dts: rockchip: rk3576: add cache hierarchy information to CPU nodes - #18
Conversation
alchark
left a comment
There was a problem hiding this comment.
Hi @munzzyy, this looks good to me but please reference the RK3576 datasheet, not RK3576S, as they are technically different products.
Cache sizes stated in the RK3576 datasheet are the same as those you used here, so it’s purely about wording in the commit description.
I’ll be able to test and confirm on actual hardware later today, and then it would be great if you submitted it directly to the upstream mailing list using b4.
…fset Commit 390ac56 ("ocfs2: add boundary check to ocfs2_check_dir_entry()") added an out-of-bounds guard using the caller-supplied 'offset' argument: if (offset > size - OCFS2_DIR_REC_LEN(1)) return 0; However, 'offset' and 'size' are not measured against the same base for all callers. In the block-based lookup path, ocfs2_find_entry_el() passes 'offset' as an absolute offset into the whole directory: i = ocfs2_search_dirblock(bh, dir, name, namelen, block << sb->s_blocksize_bits, bh->b_data, sb->s_blocksize, res_dir); while 'size' is a single block size (sb->s_blocksize). For any directory entry located in the second or later block, 'offset' is >= sb->s_blocksize, so the guard rejects every such entry even though it is perfectly valid and lies entirely within its block buffer. This makes mounting fail for filesystems whose system directory spans more than one block, e.g. a volume formatted with a small block size: mkfs.ocfs2 -b 512 -C 4096 -N 2 -T datafiles --fs-features=usrquota,grpquota ocfs2_check_dir_entry:314 ERROR: directory entry (#18: offset=512) too close to end or out-of-bounds ocfs2_init_local_system_inodes:496 ERROR: status=-22, sysfile=12, slot=0 ocfs2_mount_volume:1757 ERROR: status = -22 The dirent's position within the buffer being validated is ((char *)de - buf), which is what the rest of the function already uses (via next_offset) and what must be bounds-checked against 'size'. Compute that buffer-relative offset and use it for the guard. The subtraction is reordered to size - buf_offset < OCFS2_DIR_REC_LEN(1) to avoid an unsigned underflow when size is smaller than the minimal record length. Link: https://lore.kernel.org/20260710040512.3310736-1-joseph.qi@linux.alibaba.com Fixes: 390ac56 ("ocfs2: add boundary check to ocfs2_check_dir_entry()") Signed-off-by: Joseph Qi <joseph.qi@linux.alibaba.com> Reviewed-by: Dmitry Antipov <dmantipov@yandex.ru> Tested-by: Dmitry Antipov <dmantipov@yandex.ru> Cc: Mark Fasheh <mark@fasheh.com> Cc: Joel Becker <jlbec@evilplan.org> Cc: Junxiao Bi <junxiao.bi@oracle.com> Cc: Changwei Ge <gechangwei@live.cn> Cc: Jun Piao <piaojun@huawei.com> Cc: Heming Zhao <heming.zhao@suse.com> Cc: <stable@vger.kernel.org> Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
|
@munzzyy I've tested it on my Flipper One prototype board. The warning line is gone from dmesg, and /sys/devices/system/cpu/cpu*/cache/index* now contains meaningful cache information instead of just level, type and shared_cpu_list. Same for lscpu: So please feel free to include upon mainline submission: |
5fbc287 to
118825a
Compare
|
That lscpu output is great to see — thanks for putting it on real hardware so quickly. Datasheet reference fixed: the commit message now cites the RK3576 datasheet (same cache numbers, as you said, but the S is a different product — my mistake). Force-pushed with just the wording change; the diff itself is untouched. I'll prepare the mainline submission next and include your Tested-by there. Will follow up once it's on the list. |
…nodes The RK3576 CPU nodes carry no cache properties, so cache_setup_of_node() in the generic cacheinfo core fails with -ENOENT on the first CPU. That error propagates out of cache_shared_cpu_map_setup(), which discards the topology arm64 had already derived from CLIDR and prints "cacheinfo: Unable to detect cache hierarchy for CPU 0" on every boot. Add L1 i/d cache size, line-size and sets to all eight CPU nodes, plus per-cluster unified L2 nodes wired up through next-level-cache. Sizes come from the RK3576 datasheet (A72 cluster: 48KB/32KB L1 I/D, 1MB L2; A53 cluster: 32KB/32KB L1 I/D, 512KB L2). Line size and associativity are architecturally fixed per the Cortex-A53 and Cortex-A72 TRMs, and the *-sets values follow as size / (line-size * ways). Same shape as the rk3399 fix that landed upstream in rk3399-base.dtsi. Mainline rk3576.dtsi has the identical gap. Signed-off-by: Cole Munz <Munzzyy1@proton.me>
118825a to
c2fe199
Compare
alchark
left a comment
There was a problem hiding this comment.
Thanks a lot for your contribution! Eager to see the mainline submission next :)
Fixes #13.
Every boot logs
cacheinfo: Unable to detect cache hierarchy for CPU 0. The cause is in the generic cacheinfo core, not arm64:of_check_cache_nodes()finds no cache properties on the RK3576 CPU nodes, socache_setup_of_node()returns -ENOENT andcache_shared_cpu_map_setup()throws away the topology that CLIDR-based detection had already computed correctly.This adds L1 i/d size, line-size and sets to all eight CPU nodes, plus the two per-cluster unified L2 nodes with
next-level-cachephandles. It's the same shape as the rk3399 fix that went into mainlinerk3399-base.dtsi.Where the numbers come from:
*-setsvalues are derived as size / (line-size x ways). The A53 numbers come out identical to rk3399's, which is a decent cross-check.Verified by building dtc from the tree and compiling the full
rk3576-flipper-one-rev-f0b1c2.dtsinclude chain: no warnings, and decompiling the dtb shows all eightnext-level-cachephandles resolving to the right L2 node. I don't have a board, so this isn't boot-tested. On hardware the cacheinfo line should disappear from dmesg and/sys/devices/system/cpu/cpu0/cache/index*/sizeshould read 32K/32K/512K on the A53 side.Mainline
rk3576.dtsihas the identical gap, so this is probably worth sending upstream too. Happy to do that, or leave it to you since you're closer to the mainline RK3576 work.