Received-SPF: pass (google.com: domain of linux-kernel+bounces-202934-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.199.223 as permitted sender) client-ip=147.75.199.223;
Date: Wed, 5 Jun 2024 17:54:58 +0100
From: Conor Dooley <conor@kernel.org>
To: Yong-Xuan Wang <yongxuan.wang@sifive.com>
Cc: linux-kernel@vger.kernel.org, linux-riscv@lists.infradead.org,
	kvm-riscv@lists.infradead.org, kvm@vger.kernel.org,
	apatel@ventanamicro.com, alex@ghiti.fr, ajones@ventanamicro.com,
	greentime.hu@sifive.com, vincent.chen@sifive.com,
	Rob Herring <robh@kernel.org>,
	Krzysztof Kozlowski <krzk+dt@kernel.org>,
	Paul Walmsley <paul.walmsley@sifive.com>,
	Palmer Dabbelt <palmer@dabbelt.com>,
	Albert Ou <aou@eecs.berkeley.edu>, devicetree@vger.kernel.org
Subject: Re: [PATCH v5 2/4] dt-bindings: riscv: Add Svade and Svadu Entries
Message-ID: <20240605-atrium-neuron-c2512b34d3da@spud>
References: <20240605121512.32083-1-yongxuan.wang@sifive.com>
 <20240605121512.32083-3-yongxuan.wang@sifive.com>
Precedence: bulk
MIME-Version: 1.0
Content-Type: multipart/signed; micalg=pgp-sha256;
	protocol="application/pgp-signature"; boundary="MuN5kP8SHGJqYdTt"
Content-Disposition: inline
In-Reply-To: <20240605121512.32083-3-yongxuan.wang@sifive.com>


--MuN5kP8SHGJqYdTt
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable

On Wed, Jun 05, 2024 at 08:15:08PM +0800, Yong-Xuan Wang wrote:
> Add entries for the Svade and Svadu extensions to the riscv,isa-extensions
> property.
>=20
> Signed-off-by: Yong-Xuan Wang <yongxuan.wang@sifive.com>
> ---
>  .../devicetree/bindings/riscv/extensions.yaml | 30 +++++++++++++++++++
>  1 file changed, 30 insertions(+)
>=20
> diff --git a/Documentation/devicetree/bindings/riscv/extensions.yaml b/Do=
cumentation/devicetree/bindings/riscv/extensions.yaml
> index 468c646247aa..1e30988826b9 100644
> --- a/Documentation/devicetree/bindings/riscv/extensions.yaml
> +++ b/Documentation/devicetree/bindings/riscv/extensions.yaml
> @@ -153,6 +153,36 @@ properties:
>              ratified at commit 3f9ed34 ("Add ability to manually trigger
>              workflow. (#2)") of riscv-time-compare.
> =20
> +        - const: svade
> +          description: |
> +            The standard Svade supervisor-level extension for raising pa=
ge-fault
> +            exceptions when PTE A/D bits need be set as ratified in the =
20240213
> +            version of the privileged ISA specification.
> +
> +            Both Svade and Svadu extensions control the hardware behavio=
r when
> +            the PTE A/D bits need to be set. The default behavior for th=
e four
> +            possible combinations of these extensions in the device tree=
 are:
> +            1. Neither svade nor svadu in DT: default to svade.

I think this needs to be expanded on, as to why nothing means svade.

> +            2. Only svade in DT: use svade.

That's a statement of the obvious, right?

> +            3. Only svadu in DT: use svadu.

This is not relevant for Svade.

> +            4. Both svade and svadu in DT: default to svade (Linux can s=
witch to
> +               svadu once the SBI FWFT extension is available).

"The privilege level to which this devicetree has been provided can switch =
to
Svadu if the SBI FWFT extension is available".

> +        - const: svadu
> +          description: |
> +            The standard Svadu supervisor-level extension for hardware u=
pdating
> +            of PTE A/D bits as ratified at commit c1abccf ("Merge pull r=
equest
> +            #25 from ved-rivos/ratified") of riscv-svadu.
> +
> +            Both Svade and Svadu extensions control the hardware behavio=
r when
> +            the PTE A/D bits need to be set. The default behavior for th=
e four
> +            possible combinations of these extensions in the device tree=
 are:

@Anup/Drew/Alex, are we missing some wording in here about it only being
valid to have Svadu in isolation if the provider of the devicetree has
actually turned on Svadu? The binding says "the default behaviour", but
it is not the "default" behaviour, the behaviour is a must AFAICT. If
you set Svadu in isolation, you /must/ have turned it on. If you set
Svadu and Svade, you must have Svadu turned off?

> +            1. Neither svade nor svadu in DT: default to svade.
> +            2. Only svade in DT: use svade.

These two are not relevant to Svadu, I'd leave them out.

> +            3. Only svadu in DT: use svadu.

Again, statement of the obvious?

> +            4. Both svade and svadu in DT: default to svade (Linux can s=
witch to
> +               svadu once the SBI FWFT extension is available).

Same here as in the Svade entry.

Thanks,
Conor.


--MuN5kP8SHGJqYdTt
Content-Type: application/pgp-signature; name="signature.asc"

-----BEGIN PGP SIGNATURE-----

iHUEABYIAB0WIQRh246EGq/8RLhDjO14tDGHoIJi0gUCZmCYYgAKCRB4tDGHoIJi
0tLVAQDaJxypeK+klc9eSIXgb1/wgPLmEylTnc8/4ceKF/GFnwEAq4ysX3JMpe/B
tD+9p+XFfXCPa2GD2rvlTIT6YJMYuwI=
=Q4pd
-----END PGP SIGNATURE-----

--MuN5kP8SHGJqYdTt--