Re: [RFC PATCH v3 1/3] dt-bindings: riscv: Add Worlds ISA extensions

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [Medium] Placeholder 'TBD' commit hashes are used for the new RISC-V Worlds extensions instead of actual ratified commit hashes.
- [Medium] Enforcing an S-mode extension (`sswid`) to strictly depend on an M-mode extension (`smwiddeleg`) breaks device tree validation for S-mode guest VMs.
- [Medium] The schema fails to enforce the architectural dependency that `smlwid` requires the base `smwid` extension.
--

--- Patch [1]: [RFC PATCH v3 1/3] dt-bindings: riscv: Add Worlds ISA extensions ---
commit 0933aea6342a2dfd4ed22a9a32e7d69364ef3cfc
Author: Yu-Chien Peter Lin <[email protected]>

dt-bindings: riscv: Add Worlds ISA extensions

This commit adds device tree bindings for the RISC-V Worlds ISA extensions
(Smwid, Smlwidlist, Smlwid, Smwiddeleg, Sswid). It also adds extension
dependency rules to the schema.

> diff --git a/Documentation/devicetree/bindings/riscv/extensions.yaml b/Documentation/devicetree/bindings/riscv/extensions.yaml
> index 5ffc40d599c02..0408d11165275 100644
> --- a/Documentation/devicetree/bindings/riscv/extensions.yaml
> +++ b/Documentation/devicetree/bindings/riscv/extensions.yaml
> @@ -181,6 +181,18 @@ properties:
>              changes to interrupts as frozen at commit ccbddab ("Merge pull
>              request #42 from riscv/jhauser-2023-RC4") of riscv-aia.
>  
> +        - const: smlwid
> +          description: |
> +            The standard Smlwid extension for M-mode control of lower
> +            privilege World ID via the mlwid CSR as ratified at commit
> +            TBD ("TBD") of riscv-worlds.

[Severity: Medium]
Does this schema use literal "TBD" placeholders instead of the Git commit
hash of the ratified RISC-V specification? This prevents exact traceability
of the specification version used to define the schema, which is typically
required for RISC-V extensions.

[ ... ]

> @@ -924,6 +953,30 @@ properties:
>          then:
>            contains:
>              const: b
> +      # Smlwidlist depends on Smwid and Smlwid
> +      - if:
> +          contains:
> +            const: smlwidlist
> +        then:
> +          allOf:
> +            - contains:
> +                const: smwid
> +            - contains:
> +                const: smlwid

[Severity: Medium]
Does this schema miss enforcing the architectural dependency that smlwid
requires the base smwid extension? A device tree could declare smlwid
without smwid and silently pass validation, even though that would be an
architecturally invalid hardware configuration.

> +      # Smwiddeleg depends on Smlwid
> +      - if:
> +          contains:
> +            const: smwiddeleg
> +        then:
> +          contains:
> +            const: smlwid
> +      # Sswid depends on Smwiddeleg
> +      - if:
> +          contains:
> +            const: sswid
> +        then:
> +          contains:
> +            const: smwiddeleg

[Severity: Medium]
Will this strict dependency break device tree validation for S-mode guest
VMs?

In virtualized RISC-V environments, a hypervisor provides a device tree to
an S-mode guest VM that intentionally strips out M-mode extensions, since
M-mode is not accessible to the guest. Valid guest device trees containing
sswid might unconditionally fail dt-validate because the M-mode smwiddeleg
extension is stripped.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=1