Re: [RFC PATCH 3/3] dt-bindings: sifive: Add WorldGuard Checker

[Conor Dooley <conor@kernel.org>]

[-- Attachment #1: Type: text/plain, Size: 8912 bytes --]

On Fri, Jun 19, 2026 at 06:58:34PM +0800, Yu-Chien Peter Lin wrote:
> Add DT binding for SiFive wgChecker2, a hardware firewall enforcing
> WID-based access control in RISC-V Worlds. Provides checker slots to
> program per-WID permissions for downstream resources, with optional
> sub-range partitioning.
> 
> Link: https://github.com/riscvarchive/security/blob/main/papers/worldguard%20proposal.pdf
> Signed-off-by: Yu-Chien Peter Lin <peter.lin@sifive.com>
> Reviewed-by: Zong Li <zong.li@sifive.com>
> Reviewed-by: Jim Shu <jim.shu@sifive.com>
> ---
>  .../devicetree/bindings/riscv/worlds.yaml     |   9 +
>  .../bindings/sifive/sifive,wgchecker2.yaml    | 237 ++++++++++++++++++
>  2 files changed, 246 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/sifive/sifive,wgchecker2.yaml
> 
> diff --git a/Documentation/devicetree/bindings/riscv/worlds.yaml b/Documentation/devicetree/bindings/riscv/worlds.yaml
> index cc8b3747591e..c39a06c2dd8d 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:

What's sifive specific about this? Wouldn't other vendors also have
trusted worlds?

> +    $ref: /schemas/types.yaml#/definitions/uint32
> +    maximum: 31
> +    description: |
> +      The World ID (WID) designated as the trusted WID for this platform.
> +      Transactions tagged with this WID are authorized to access and configure
> +      WorldGuard blocks, including wgCheckers and wgMarkers.
> +
>  additionalProperties: true
>  
>  examples:
> @@ -44,6 +52,7 @@ examples:
>          #size-cells = <0>;
>          timebase-frequency = <1000000>;
>          riscv,nworlds = <4>;
> +        sifive,trustedwid = <3>;
>  
>          cpu@0 {
>              device_type = "cpu";
> diff --git a/Documentation/devicetree/bindings/sifive/sifive,wgchecker2.yaml b/Documentation/devicetree/bindings/sifive/sifive,wgchecker2.yaml
> new file mode 100644
> index 000000000000..043c748385ed
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/sifive/sifive,wgchecker2.yaml
> @@ -0,0 +1,237 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +# Copyright (C) 2026 SiFive, Inc.
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/sifive/sifive,wgchecker2.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: SiFive WorldGuard Checker
> +
> +maintainers:
> +  - Yu-Chien Peter Lin <peter.lin@sifive.com>
> +
> +description: |
> +  The RISC-V Worlds ISA extension defines World IDs (WIDs) as architectural
> +  identifiers that tag each system transaction with its originating context.
> +  System integrators assign WIDs to execution contexts such as privilege modes,
> +  trusted execution environments, or other isolation boundaries.
> +
> +  The SiFive WorldGuard Checker is a hardware firewall positioned in the
> +  system interconnect fabric. It inspects every transaction, evaluating the
> +  WID against access control policies encoded in checker slots for each
> +  protected resource. Transactions from unauthorized WIDs are blocked and
> +  reported as bus errors, interrupts, or both.
> +
> +  This enables spatial partitioning of memory regions and memory-mapped devices
> +  across execution contexts. Different address ranges can enforce distinct
> +  policies, allowing isolated workloads to coexist with hardware-enforced
> +  protection.
> +
> +  The wgChecker acts as an access-controller provider as defined in the
> +  access-controllers framework. Protected devices are consumers that declare
> +  their access policy via the access-controllers property. The hardware
> +  supports up to 32 World IDs.
> +
> +  The World ID authorized to configure WorldGuard blocks is specified by the
> +  sifive,trustedwid property in the /cpus node.
> +
> +allOf:
> +  - $ref: /schemas/access-controllers/access-controllers.yaml#
> +
> +properties:
> +  compatible:
> +    const: sifive,wgchecker2

Missing device specific compatibles.

> +
> +  reg:
> +    maxItems: 1
> +    description:
> +      Base address and size of the wgChecker memory-mapped I/O registers.
> +
> +  interrupts:
> +    maxItems: 1
> +    description:
> +      Interrupt line asserted when a WID access violation is detected and
> +      interrupt reporting is enabled in the slot configuration (IR or IW
> +      bits set).
> +
> +  '#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>
> +
> +      where:
> +        - addr-hi, addr-lo: 64-bit base address of the protected region.
> +        - size-hi, size-lo: 64-bit size of the protected region in bytes.

These two cells effectively just duplicate the reg property.

> +        - perm-hi: Permission bitmap for WIDs 16..31. Two bits per WID:
> +                     bit 2*(WID-16)   = Read  permission
> +                     bit 2*(WID-16)+1 = Write permission
> +                   Set bits grant access. Use 0x0 for systems with
> +                   riscv,nworlds <= 16.
> +        - perm-lo: Permission bitmap for WIDs 0..15. Two bits per WID:
> +                     bit 2*WID   = Read  permission
> +                     bit 2*WID+1 = Write permission
> +                   Set bits grant access.

And these two look like a layering violation to me. Why does the
consumer contain its own configuration information? If firmware provides
this to s-mode, it is either useless (because firmware has already done
the configuration) or it makes the access control pointless because
s-mode is expected to program its own access.
With that in mind, the first 4 cells can probably just be transmuted to
a single cell with platform-specific unique identifiers.
Surely the ecall involved with actually requesting access needs
something like that anyway?

The only value I can see in this is if some worlds that a bit of
software is running on can access a peripheral (or part thereof) and
others can't? Though platforms like that might benefit more from being
reworked to have homogeneous access! I've got no idea how a Linux driver
etc would handle the only some CPUs being permitted to access a register
region.

> +        - config:  Slot configuration bits:
> +                     Bit 0 (ER): Report read  violations as bus errors
> +                     Bit 1 (EW): Report write violations as bus errors
> +                     Bit 2 (IR): Report read  violations via interrupt
> +                     Bit 3 (IW): Report write violations via interrupt
> +                     Bit 4 (L):  Lock bit - prevents further modification
> +                   Bits 5..31 are reserved and must be zero.

For the next revision of this, I really would like to see the access
controller driver.

> +
> +      Multiple entries may be listed to apply different policies to
> +      different address ranges, including sub-ranges within a single
> +      physical resource.
> +
> +required:
> +  - compatible
> +  - reg
> +  - '#access-controller-cells'
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    #include <dt-bindings/interrupt-controller/irq.h>
> +
> +    // Example 1: Single device protection
> +    // WID 0 and WID 3 have RW access to UART; errors and IRQs reported.
> +
> +    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";
> +        };
> +    };
> +
> +    soc {
> +        #address-cells = <2>;
> +        #size-cells = <2>;
> +
> +        uart: uart@1c1000 {
> +            compatible = "ns16550a";
> +            reg = <0x0 0x001c1000 0x0 0x1000>;
> +            reg-names = "control";
> +            interrupts = <10 IRQ_TYPE_LEVEL_HIGH>;
> +            // WID 0,3 RW; report errors+IRQs
> +            access-controllers = <&wgchecker0
> +                                  0x0 0x001c1000 0x0 0x00001000
> +                                  0x0 0x000000c3 0x0f>;
> +        };
> +
> +        wgchecker0: wgchecker@1c2000 {

I think this should be access-controller@

> +            compatible = "sifive,wgchecker2";
> +            reg = <0x0 0x001c2000 0x0 0x1000>;
> +            #access-controller-cells = <7>;
> +            interrupts = <80 IRQ_TYPE_LEVEL_HIGH>;
> +            interrupt-parent = <&aplic_m>;
> +        };
> +    };

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]