[PATCH v6 2/4] dt-bindings: riscv: Add Svade and Svadu Entries

[PATCH v6 2/4] dt-bindings: riscv: Add Svade and Svadu Entries

[Yong-Xuan Wang]

Add entries for the Svade and Svadu extensions to the riscv,isa-extensions
property.

Signed-off-by: Yong-Xuan Wang <yongxuan.wang@sifive.com>
---
 .../devicetree/bindings/riscv/extensions.yaml | 28 +++++++++++++++++++
 1 file changed, 28 insertions(+)

diff --git a/Documentation/devicetree/bindings/riscv/extensions.yaml b/Documentation/devicetree/bindings/riscv/extensions.yaml
index 468c646247aa..c3d053ce7783 100644
--- a/Documentation/devicetree/bindings/riscv/extensions.yaml
+++ b/Documentation/devicetree/bindings/riscv/extensions.yaml
@@ -153,6 +153,34 @@ properties:
             ratified at commit 3f9ed34 ("Add ability to manually trigger
             workflow. (#2)") of riscv-time-compare.
 
+        - const: svade
+          description: |
+            The standard Svade supervisor-level extension for SW-managed PTE A/D
+            bit updates as ratified in the 20240213 version of the privileged
+            ISA specification.
+
+            Both Svade and Svadu extensions control the hardware behavior when
+            the PTE A/D bits need to be set. The default behavior for the four
+            possible combinations of these extensions in the device tree are:
+            1) Neither Svade nor Svadu present in DT => It is technically
+               unknown whether the platform uses Svade or Svadu. Supervisor may
+               assume Svade to be present and enabled or it can discover based
+               on mvendorid, marchid, and mimpid.
+            2) Only Svade present in DT => Supervisor must assume Svade to be
+               always enabled. (Obvious)
+            3) Only Svadu present in DT => Supervisor must assume Svadu to be
+               always enabled. (Obvious)
+            4) Both Svade and Svadu present in DT => Supervisor must assume
+               Svadu turned-off at boot time. To use Svadu, supervisor must
+               explicitly enable it using the SBI FWFT extension.
+
+        - const: svadu
+          description: |
+            The standard Svadu supervisor-level extension for hardware updating
+            of PTE A/D bits as ratified at commit c1abccf ("Merge pull request
+            #25 from ved-rivos/ratified") of riscv-svadu. Please refer to Svade
+            dt-binding description for more details.
+
         - const: svinval
           description:
             The standard Svinval supervisor-level extension for fine-grained
-- 
2.17.1

Re: [PATCH v6 2/4] dt-bindings: riscv: Add Svade and Svadu Entries

[Conor Dooley]

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

On Fri, Jun 28, 2024 at 05:37:06PM +0800, Yong-Xuan Wang wrote:
> Add entries for the Svade and Svadu extensions to the riscv,isa-extensions
> property.
> 
> Signed-off-by: Yong-Xuan Wang <yongxuan.wang@sifive.com>
> ---
>  .../devicetree/bindings/riscv/extensions.yaml | 28 +++++++++++++++++++
>  1 file changed, 28 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/riscv/extensions.yaml b/Documentation/devicetree/bindings/riscv/extensions.yaml
> index 468c646247aa..c3d053ce7783 100644
> --- a/Documentation/devicetree/bindings/riscv/extensions.yaml
> +++ b/Documentation/devicetree/bindings/riscv/extensions.yaml
> @@ -153,6 +153,34 @@ properties:
>              ratified at commit 3f9ed34 ("Add ability to manually trigger
>              workflow. (#2)") of riscv-time-compare.
>  
> +        - const: svade
> +          description: |
> +            The standard Svade supervisor-level extension for SW-managed PTE A/D
> +            bit updates as ratified in the 20240213 version of the privileged
> +            ISA specification.
> +
> +            Both Svade and Svadu extensions control the hardware behavior when
> +            the PTE A/D bits need to be set. The default behavior for the four
> +            possible combinations of these extensions in the device tree are:
> +            1) Neither Svade nor Svadu present in DT =>

>                 It is technically
> +               unknown whether the platform uses Svade or Svadu. Supervisor may
> +               assume Svade to be present and enabled or it can discover based
> +               on mvendorid, marchid, and mimpid.

I would just write "for backwards compatibility, if neither Svade nor
Svadu appear in the devicetree the supervisor may assume Svade to be
present and enabled". If there are systems that this behaviour causes
problems for, we can deal with them iff they appear. I don't think
looking at m*id would be sufficient here anyway, since the firmware can
have an impact. I'd just drop that part entirely.

> +            2) Only Svade present in DT => Supervisor must assume Svade to be
> +               always enabled. (Obvious)

nit: I'd drop the "(Obvious)" comments from here.

Cheers,
Conor.

> +            3) Only Svadu present in DT => Supervisor must assume Svadu to be
> +               always enabled. (Obvious)
> +            4) Both Svade and Svadu present in DT => Supervisor must assume
> +               Svadu turned-off at boot time. To use Svadu, supervisor must
> +               explicitly enable it using the SBI FWFT extension.
> +
> +        - const: svadu
> +          description: |
> +            The standard Svadu supervisor-level extension for hardware updating
> +            of PTE A/D bits as ratified at commit c1abccf ("Merge pull request
> +            #25 from ved-rivos/ratified") of riscv-svadu. Please refer to Svade
> +            dt-binding description for more details.
> +
>          - const: svinval
>            description:
>              The standard Svinval supervisor-level extension for fine-grained
> -- 
> 2.17.1
> 

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

Re: [PATCH v6 2/4] dt-bindings: riscv: Add Svade and Svadu Entries

[Jessica Clarke]

On 28 Jun 2024, at 17:19, Conor Dooley <conor@kernel.org> wrote:
> 
> On Fri, Jun 28, 2024 at 05:37:06PM +0800, Yong-Xuan Wang wrote:
>> Add entries for the Svade and Svadu extensions to the riscv,isa-extensions
>> property.
>> 
>> Signed-off-by: Yong-Xuan Wang <yongxuan.wang@sifive.com>
>> ---
>> .../devicetree/bindings/riscv/extensions.yaml | 28 +++++++++++++++++++
>> 1 file changed, 28 insertions(+)
>> 
>> diff --git a/Documentation/devicetree/bindings/riscv/extensions.yaml b/Documentation/devicetree/bindings/riscv/extensions.yaml
>> index 468c646247aa..c3d053ce7783 100644
>> --- a/Documentation/devicetree/bindings/riscv/extensions.yaml
>> +++ b/Documentation/devicetree/bindings/riscv/extensions.yaml
>> @@ -153,6 +153,34 @@ properties:
>>             ratified at commit 3f9ed34 ("Add ability to manually trigger
>>             workflow. (#2)") of riscv-time-compare.
>> 
>> +        - const: svade
>> +          description: |
>> +            The standard Svade supervisor-level extension for SW-managed PTE A/D
>> +            bit updates as ratified in the 20240213 version of the privileged
>> +            ISA specification.
>> +
>> +            Both Svade and Svadu extensions control the hardware behavior when
>> +            the PTE A/D bits need to be set. The default behavior for the four
>> +            possible combinations of these extensions in the device tree are:
>> +            1) Neither Svade nor Svadu present in DT =>
> 
>>                It is technically
>> +               unknown whether the platform uses Svade or Svadu. Supervisor may
>> +               assume Svade to be present and enabled or it can discover based
>> +               on mvendorid, marchid, and mimpid.
> 
> I would just write "for backwards compatibility, if neither Svade nor
> Svadu appear in the devicetree the supervisor may assume Svade to be
> present and enabled". If there are systems that this behaviour causes
> problems for, we can deal with them iff they appear. I don't think
> looking at m*id would be sufficient here anyway, since the firmware can
> have an impact. I'd just drop that part entirely.

Older QEMU falls into that category, as do Bluespec’s soft-cores (which
ours are derived from at Cambridge). I feel that, in reality, one
should be prepared to handle both trapping and atomic updates if
writing an OS that aims to support case 1.

Jess

Re: [PATCH v6 2/4] dt-bindings: riscv: Add Svade and Svadu Entries

[Conor Dooley]

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

On Sat, Jun 29, 2024 at 02:09:34PM +0100, Jessica Clarke wrote:
> On 28 Jun 2024, at 17:19, Conor Dooley <conor@kernel.org> wrote:
> > 
> > On Fri, Jun 28, 2024 at 05:37:06PM +0800, Yong-Xuan Wang wrote:
> >> Add entries for the Svade and Svadu extensions to the riscv,isa-extensions
> >> property.
> >> 
> >> Signed-off-by: Yong-Xuan Wang <yongxuan.wang@sifive.com>
> >> ---
> >> .../devicetree/bindings/riscv/extensions.yaml | 28 +++++++++++++++++++
> >> 1 file changed, 28 insertions(+)
> >> 
> >> diff --git a/Documentation/devicetree/bindings/riscv/extensions.yaml b/Documentation/devicetree/bindings/riscv/extensions.yaml
> >> index 468c646247aa..c3d053ce7783 100644
> >> --- a/Documentation/devicetree/bindings/riscv/extensions.yaml
> >> +++ b/Documentation/devicetree/bindings/riscv/extensions.yaml
> >> @@ -153,6 +153,34 @@ properties:
> >>             ratified at commit 3f9ed34 ("Add ability to manually trigger
> >>             workflow. (#2)") of riscv-time-compare.
> >> 
> >> +        - const: svade
> >> +          description: |
> >> +            The standard Svade supervisor-level extension for SW-managed PTE A/D
> >> +            bit updates as ratified in the 20240213 version of the privileged
> >> +            ISA specification.
> >> +
> >> +            Both Svade and Svadu extensions control the hardware behavior when
> >> +            the PTE A/D bits need to be set. The default behavior for the four
> >> +            possible combinations of these extensions in the device tree are:
> >> +            1) Neither Svade nor Svadu present in DT =>
> > 
> >>                It is technically
> >> +               unknown whether the platform uses Svade or Svadu. Supervisor may
> >> +               assume Svade to be present and enabled or it can discover based
> >> +               on mvendorid, marchid, and mimpid.
> > 
> > I would just write "for backwards compatibility, if neither Svade nor
> > Svadu appear in the devicetree the supervisor may assume Svade to be
> > present and enabled". If there are systems that this behaviour causes
> > problems for, we can deal with them iff they appear. I don't think
> > looking at m*id would be sufficient here anyway, since the firmware can
> > have an impact. I'd just drop that part entirely.
> 
> Older QEMU falls into that category, as do Bluespec’s soft-cores (which
> ours are derived from at Cambridge). I feel that, in reality, one
> should be prepared to handle both trapping and atomic updates if
> writing an OS that aims to support case 1.

I guess that is actually what we should put in then, to use an
approximation of your wording, something like
	Neither Svade nor Svadu present in DT => Supervisor software should be
	prepared to handle either hardware updating of the PTE A/D bits or page
	faults when they need updated
?

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

Re: [PATCH v6 2/4] dt-bindings: riscv: Add Svade and Svadu Entries

[Yong-Xuan Wang]

Hi Conor and Jessica,

On Sun, Jun 30, 2024 at 10:09 PM Conor Dooley <conor@kernel.org> wrote:
>
> On Sat, Jun 29, 2024 at 02:09:34PM +0100, Jessica Clarke wrote:
> > On 28 Jun 2024, at 17:19, Conor Dooley <conor@kernel.org> wrote:
> > >
> > > On Fri, Jun 28, 2024 at 05:37:06PM +0800, Yong-Xuan Wang wrote:
> > >> Add entries for the Svade and Svadu extensions to the riscv,isa-extensions
> > >> property.
> > >>
> > >> Signed-off-by: Yong-Xuan Wang <yongxuan.wang@sifive.com>
> > >> ---
> > >> .../devicetree/bindings/riscv/extensions.yaml | 28 +++++++++++++++++++
> > >> 1 file changed, 28 insertions(+)
> > >>
> > >> diff --git a/Documentation/devicetree/bindings/riscv/extensions.yaml b/Documentation/devicetree/bindings/riscv/extensions.yaml
> > >> index 468c646247aa..c3d053ce7783 100644
> > >> --- a/Documentation/devicetree/bindings/riscv/extensions.yaml
> > >> +++ b/Documentation/devicetree/bindings/riscv/extensions.yaml
> > >> @@ -153,6 +153,34 @@ properties:
> > >>             ratified at commit 3f9ed34 ("Add ability to manually trigger
> > >>             workflow. (#2)") of riscv-time-compare.
> > >>
> > >> +        - const: svade
> > >> +          description: |
> > >> +            The standard Svade supervisor-level extension for SW-managed PTE A/D
> > >> +            bit updates as ratified in the 20240213 version of the privileged
> > >> +            ISA specification.
> > >> +
> > >> +            Both Svade and Svadu extensions control the hardware behavior when
> > >> +            the PTE A/D bits need to be set. The default behavior for the four
> > >> +            possible combinations of these extensions in the device tree are:
> > >> +            1) Neither Svade nor Svadu present in DT =>
> > >
> > >>                It is technically
> > >> +               unknown whether the platform uses Svade or Svadu. Supervisor may
> > >> +               assume Svade to be present and enabled or it can discover based
> > >> +               on mvendorid, marchid, and mimpid.
> > >
> > > I would just write "for backwards compatibility, if neither Svade nor
> > > Svadu appear in the devicetree the supervisor may assume Svade to be
> > > present and enabled". If there are systems that this behaviour causes
> > > problems for, we can deal with them iff they appear. I don't think
> > > looking at m*id would be sufficient here anyway, since the firmware can
> > > have an impact. I'd just drop that part entirely.
> >
> > Older QEMU falls into that category, as do Bluespec’s soft-cores (which
> > ours are derived from at Cambridge). I feel that, in reality, one
> > should be prepared to handle both trapping and atomic updates if
> > writing an OS that aims to support case 1.
>
> I guess that is actually what we should put in then, to use an
> approximation of your wording, something like
>         Neither Svade nor Svadu present in DT => Supervisor software should be
>         prepared to handle either hardware updating of the PTE A/D bits or page
>         faults when they need updated
> ?

Thank you! I will update in the next version.

Regards,
Yong-Xuan