From: sashiko-bot@kernel.org
To: "Yu-Chien Peter Lin" <peter.lin@sifive.com>
Cc: conor+dt@kernel.org, robh@kernel.org, devicetree@vger.kernel.org
Subject: Re: [RFC PATCH 3/3] dt-bindings: sifive: Add WorldGuard Checker
Date: Fri, 19 Jun 2026 10:59:56 +0000 [thread overview]
Message-ID: <20260619105957.444AB1F000E9@smtp.kernel.org> (raw)
In-Reply-To: <20260619105834.1277302-4-peter.lin@sifive.com>
Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:
- [High] The `#access-controller-cells` specifier inappropriately encodes 64-bit absolute physical addresses and sizes, bypassing Devicetree's native address translation.
- [Low] The schema examples reference an undefined label `&aplic_m`, causing a `dt_binding_check` compilation failure.
- [Low] The `sifive,trustedwid` property is not explicitly disallowed in `riscv/cpus.yaml`, allowing it to bypass schema validation if erroneously placed in a `cpu@*` node.
- [Low] The examples in `sifive,wgchecker2.yaml` omit the required `interrupt-controller` child node in their `cpu@0` definitions, leading to a schema validation error.
--
commit 26cbbd0f7310c04b5d6fe920375466bc89dd2479
Author: Yu-Chien Peter Lin <peter.lin@sifive.com>
dt-bindings: sifive: Add WorldGuard Checker
This commit adds a Device Tree binding for the SiFive wgChecker2, a
hardware firewall enforcing World ID (WID) based access control. It enables
spatial partitioning of memory and memory-mapped devices by providing checker
slots to program per-WID permissions.
Link: https://github.com/riscvarchive/security/blob/main/papers/worldguard%20proposal.pdf
> diff --git a/Documentation/devicetree/bindings/riscv/worlds.yaml b/Documentation/devicetree/bindings/riscv/worlds.yaml
> index cc8b3747591eb..c39a06c2dd8d3 100644
> --- a/Documentation/devicetree/bindings/riscv/worlds.yaml
> +++ b/Documentation/devicetree/bindings/riscv/worlds.yaml
> @@ -34,6 +34,14 @@ properties:
> minimum: 2
> maximum: 64
>
> + sifive,trustedwid:
> + $ref: /schemas/types.yaml#/definitions/uint32
[Severity: Low]
Because riscv/cpus.yaml includes worlds.yaml via $ref, does this
allow the property to be erroneously placed on individual cpu@* nodes
without failing validation?
To enforce correct usage, should sifive,trustedwid: false be explicitly
added to riscv/cpus.yaml, similar to riscv,nworlds: false?
[ ... ]
> diff --git a/Documentation/devicetree/bindings/sifive/sifive,wgchecker2.yaml b/Documentation/devicetree/bindings/sifive/sifive,wgchecker2.yaml
> new file mode 100644
> index 0000000000000..043c748385ed4
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/sifive/sifive,wgchecker2.yaml
[ ... ]
> + '#access-controller-cells':
> + const: 7
> + description: |
> + Specifier for one access-control rule, encoded as seven u32 cells:
> + <addr-hi addr-lo size-hi size-lo perm-hi perm-lo config>
[Severity: High]
Does encoding absolute physical addresses and sizes directly into the
access-controllers array bypass Devicetree's native address translation?
If the protected device is behind a bus with a translation mapping, or if
a bootloader dynamically patches reg sizes, the hardcoded addresses in the
firewall configuration might be incorrect since they are opaque to DT
address translation APIs.
[ ... ]
> + cpus {
> + #address-cells = <1>;
> + #size-cells = <0>;
> + timebase-frequency = <1000000>;
> + riscv,nworlds = <4>;
> + sifive,trustedwid = <3>;
> +
> + cpu@0 {
> + device_type = "cpu";
> + reg = <0>;
> + compatible = "riscv";
> + riscv,isa = "rv64imac";
> + };
[Severity: Low]
Will this omission cause a dt_binding_check failure?
When validated against riscv/cpus.yaml, the schema strictly mandates an
interrupt-controller child node for cpu@0 nodes with compatible = "riscv".
[ ... ]
> + wgchecker0: wgchecker@1c2000 {
> + compatible = "sifive,wgchecker2";
> + reg = <0x0 0x001c2000 0x0 0x1000>;
> + #access-controller-cells = <7>;
> + interrupts = <80 IRQ_TYPE_LEVEL_HIGH>;
> + interrupt-parent = <&aplic_m>;
[Severity: Low]
Is the aplic_m label defined anywhere in these examples?
It appears missing, which might cause make dt_binding_check to fail with a
"Reference to non-existent node or label" error.
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260619105834.1277302-1-peter.lin@sifive.com?part=3
prev parent reply other threads:[~2026-06-19 10:59 UTC|newest]
Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-06-19 10:58 [RFC PATCH 0/3] dt-bindings: riscv: Add RISC-V Worlds and SiFive WorldGuard DT bindings Yu-Chien Peter Lin
2026-06-19 10:58 ` [RFC PATCH 1/3] dt-bindings: riscv: Add Worlds ISA extensions Yu-Chien Peter Lin
2026-06-19 10:57 ` sashiko-bot
2026-06-19 10:58 ` [RFC PATCH 2/3] dt-bindings: riscv: Add Worlds per-hart properties Yu-Chien Peter Lin
2026-06-19 10:59 ` sashiko-bot
2026-06-19 10:58 ` [RFC PATCH 3/3] dt-bindings: sifive: Add WorldGuard Checker Yu-Chien Peter Lin
2026-06-19 10:59 ` sashiko-bot [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20260619105957.444AB1F000E9@smtp.kernel.org \
--to=sashiko-bot@kernel.org \
--cc=conor+dt@kernel.org \
--cc=devicetree@vger.kernel.org \
--cc=peter.lin@sifive.com \
--cc=robh@kernel.org \
--cc=sashiko-reviews@lists.linux.dev \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox