diff mbox series

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

Message ID 20240628093711.11716-3-yongxuan.wang@sifive.com
State New
Headers show
Series Add Svade and Svadu Extensions Support | expand

Commit Message

Yong-Xuan Wang June 28, 2024, 9:37 a.m. UTC
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(+)

Comments

Conor Dooley June 28, 2024, 4:19 p.m. UTC | #1
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
>
Jessica Clarke June 29, 2024, 1:09 p.m. UTC | #2
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
Conor Dooley June 30, 2024, 2:09 p.m. UTC | #3
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
?
Yong-Xuan Wang July 11, 2024, 10:34 a.m. UTC | #4
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
diff mbox series

Patch

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