Devicetree
 help / color / mirror / Atom feed
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

      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