Received-SPF: pass (google.com: domain of linux-kernel+bounces-124392-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45e3:2400::1 as permitted sender) client-ip=2604:1380:45e3:2400::1;
Date: Fri, 29 Mar 2024 10:31:10 +0000
From: Conor Dooley <conor.dooley@microchip.com>
To: Max Hsu <max.hsu@sifive.com>
CC: Conor Dooley <conor@kernel.org>, Rob Herring <robh@kernel.org>, Krzysztof
 Kozlowski <krzysztof.kozlowski+dt@linaro.org>, Paul Walmsley
	<paul.walmsley@sifive.com>, Palmer Dabbelt <palmer@dabbelt.com>, Albert Ou
	<aou@eecs.berkeley.edu>, "Rafael J. Wysocki" <rafael@kernel.org>, Pavel
 Machek <pavel@ucw.cz>, Anup Patel <anup@brainfault.org>, Atish Patra
	<atishp@atishpatra.org>, Paolo Bonzini <pbonzini@redhat.com>, Shuah Khan
	<shuah@kernel.org>, Palmer Dabbelt <palmer@sifive.com>,
	<linux-riscv@lists.infradead.org>, <devicetree@vger.kernel.org>,
	<linux-kernel@vger.kernel.org>, <linux-pm@vger.kernel.org>,
	<kvm@vger.kernel.org>, <kvm-riscv@lists.infradead.org>,
	<linux-kselftest@vger.kernel.org>
Subject: Re: [PATCH RFC 02/11] dt-bindings: riscv: Add Sdtrig optional CSRs
 existence on DT
Message-ID: <20240329-affidavit-anatomist-1118a12c3e60@wendy>
References: <20240329-dev-maxh-lin-452-6-9-v1-0-1534f93b94a7@sifive.com>
 <20240329-dev-maxh-lin-452-6-9-v1-2-1534f93b94a7@sifive.com>
Precedence: bulk
MIME-Version: 1.0
Content-Type: multipart/signed; micalg=pgp-sha256;
	protocol="application/pgp-signature"; boundary="qvN9g2Kofm3vp+Ez"
Content-Disposition: inline
In-Reply-To: <20240329-dev-maxh-lin-452-6-9-v1-2-1534f93b94a7@sifive.com>

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

On Fri, Mar 29, 2024 at 05:26:18PM +0800, Max Hsu wrote:
> The mcontext/hcontext/scontext CSRs are optional in the Sdtrig extension,
> to prevent RW operations to the missing CSRs, which will cause
> illegal instructions.
>=20
> As a solution, we have proposed the dt format for these CSRs.

As I mentioned in your other patch, I amn't sure what the actual value
is in being told about "sdtrig" itself if so many of the CSRs are
optional. I think we should define pseudo extensions that represent
usable subsets that are allowed by riscv,isa-extensions, such as
those you describe here: sdtrig + mcontext, sdtrig + scontext and
sdtrig + hcontext. Probably also for strig + mscontext. What
additional value does having a debug child node give us that makes
it worth having over something like the above?

Thanks,
Conor.

>=20
> Signed-off-by: Max Hsu <max.hsu@sifive.com>
> ---
>  Documentation/devicetree/bindings/riscv/cpus.yaml | 18 ++++++++++++++++++
>  1 file changed, 18 insertions(+)
>=20
> diff --git a/Documentation/devicetree/bindings/riscv/cpus.yaml b/Document=
ation/devicetree/bindings/riscv/cpus.yaml
> index d87dd50f1a4b..c713a48c5025 100644
> --- a/Documentation/devicetree/bindings/riscv/cpus.yaml
> +++ b/Documentation/devicetree/bindings/riscv/cpus.yaml
> @@ -137,6 +137,24 @@ properties:
>        DMIPS/MHz, relative to highest capacity-dmips-mhz
>        in the system.
> =20
> +  debug:
> +    type: object
> +    properties:
> +      compatible:
> +        const: riscv,debug-v1.0.0
> +      trigger-module:
> +        type: object
> +        description: |
> +          An indication set of optional CSR existence from
> +          riscv-debug-spec Sdtrig extension
> +        properties:
> +          mcontext-present:
> +            type: boolean
> +          hcontext-present:
> +            type: boolean
> +          scontext-present:
> +            type: boolean
> +
>  anyOf:
>    - required:
>        - riscv,isa
>=20
> --=20
> 2.43.2
>=20

--qvN9g2Kofm3vp+Ez
Content-Type: application/pgp-signature; name="signature.asc"

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

iHUEABYIAB0WIQRh246EGq/8RLhDjO14tDGHoIJi0gUCZgaYbgAKCRB4tDGHoIJi
0lR6APsFCJD2xF6mivD5+737cpFwlpgCAnMllMxOmWsck2HUJgD9EN3UFh3/VEM/
G9adYYLQT0pjKI5lJOtelWT/0qhIsg4=
=evUa
-----END PGP SIGNATURE-----

--qvN9g2Kofm3vp+Ez--