From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from lists.xenproject.org (lists.xenproject.org [192.237.175.120]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id AE434C02199 for ; Thu, 6 Feb 2025 17:46:36 +0000 (UTC) Received: from list by lists.xenproject.org with outflank-mailman.883084.1293169 (Exim 4.92) (envelope-from ) id 1tg5xK-00089y-Uv; Thu, 06 Feb 2025 17:46:26 +0000 X-Outflank-Mailman: Message body and most headers restored to incoming version Received: by outflank-mailman (output) from mailman id 883084.1293169; Thu, 06 Feb 2025 17:46:26 +0000 Received: from localhost ([127.0.0.1] helo=lists.xenproject.org) by lists.xenproject.org with esmtp (Exim 4.92) (envelope-from ) id 1tg5xK-00089r-SG; Thu, 06 Feb 2025 17:46:26 +0000 Received: by outflank-mailman (input) for mailman id 883084; Thu, 06 Feb 2025 17:46:25 +0000 Received: from se1-gles-sth1-in.inumbo.com ([159.253.27.254] helo=se1-gles-sth1.inumbo.com) by lists.xenproject.org with esmtp (Exim 4.92) (envelope-from ) id 1tg5xJ-00089l-Bz for xen-devel@lists.xenproject.org; Thu, 06 Feb 2025 17:46:25 +0000 Received: from dfw.source.kernel.org (dfw.source.kernel.org [2604:1380:4641:c500::1]) by se1-gles-sth1.inumbo.com (Halon) with ESMTPS id 47781cc1-e4b2-11ef-a073-877d107080fb; Thu, 06 Feb 2025 18:46:23 +0100 (CET) Received: from smtp.kernel.org (transwarp.subspace.kernel.org [100.75.92.58]) by dfw.source.kernel.org (Postfix) with ESMTP id 442715C6948; Thu, 6 Feb 2025 17:45:42 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id 8D87EC4CEDF; Thu, 6 Feb 2025 17:46:19 +0000 (UTC) X-BeenThere: xen-devel@lists.xenproject.org List-Id: Xen developer discussion List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Errors-To: xen-devel-bounces@lists.xenproject.org Precedence: list Sender: "Xen-devel" X-Inumbo-ID: 47781cc1-e4b2-11ef-a073-877d107080fb DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1738863981; bh=An4y+1SPvmDBvolfIPC74k9QfOwmDLzu23ld0h/wz34=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=mZ/fqLgia+QglVdYjkzkTwQImEgFihjx6f2IMkfI0ZwWE1LEkdG9A+wJWgPd4Zbpr FW179H4NR3SuUzOm0gpt4FN6hAaLQ0zRyXYc/GzSPD6LxbJVhTChYTgux5SIoCdwjO 1vZ7R3DfsXdnDtCE5cRS8Hd3cAy/5o1Chbj1drkbl4qc/zkxAmP3BqAR4Wdx5ubUP4 5+PFplZ7pREg6jgKDBagVy1vkkBHDWnXnFsbFLyg2/fhN5Nykw0Dt8nj3QFogqW/sz U8eVORYWqPF+BFhq4q2WASZxRJjAX3mhhDD9dCgIyGI7GWf2oTByOOajIqgxmn7IKf g+n8KLX+UE/aQ== Date: Thu, 6 Feb 2025 17:46:17 +0000 From: Conor Dooley To: Oleksii Kurochko Cc: Jan Beulich , xen-devel@lists.xenproject.org, Alistair Francis , Bob Eshleman , Connor Davis , Andrew Cooper , Anthony PERARD , Michal Orzel , Julien Grall , Roger Pau =?iso-8859-1?Q?Monn=E9?= , Stefano Stabellini Subject: Re: [PATCH v3] xen/riscv: identify specific ISA supported by cpu Message-ID: <20250206-empower-density-26e2c6ec36d9@spud> References: <20250205-chariot-blandness-7e9a791f7f34@spud> <585eff97-af33-4bfd-be10-fdacbb9b9069@gmail.com> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="rR/Ry9TQKrZMd8z9" Content-Disposition: inline In-Reply-To: <585eff97-af33-4bfd-be10-fdacbb9b9069@gmail.com> --rR/Ry9TQKrZMd8z9 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Thu, Feb 06, 2025 at 02:05:31PM +0100, Oleksii Kurochko wrote: > On 2/5/25 8:07 PM, Conor Dooley wrote: > > On Thu, Jan 23, 2025 at 03:46:36PM +0100, Oleksii Kurochko wrote: > > > Supported ISA extensions are specified in the device tree within the = CPU > > > node, using two properties: `riscv,isa-extensions` and `riscv,isa`. > > >=20 > > > Currently, Xen does not support the `riscv,isa-extensions` property, = as > > > all available device tree source (DTS) files in the Linux kernel (v6.= 12-rc3) > > > and DTBs generated by QEMU use only the `riscv,isa` property. > > That's not true? The riscv,isa-extensions property went into linux in > > late 2023 (6.7 or so) and QEMU in v9.0.0 about a year ago, so all source > > files in linux and blobs generated by QEMU should have both. OpenSBI & > > U-Boot support both also. Might not affect your decision about what to > > support here - but the rationale provided for implementing the deprecat= ed > > (per the binding at least) property isn't accurate. >=20 > I confused something with Linux kernel sources. Double-check and riscv,is= a-extensions > is really supported. >=20 > Regarding QEMU, at the momemnt, Xen uses Debian bookworm and the followin= g version > is used: > QEMU emulator version 7.2.11 (Debian 1:7.2+dfsg-7+deb12u6) >=20 > I will update the commit message. >=20 > Do you ( Conor and Jan ) think that it makes sense to support deprecated = property? > Or it would be better start to use QEMU v9.0.0 and just drop support for = deprecated property? I'm not entirely sure how Xen re-assembles the dtb that it passes to the guess (cos you need to strip things you don't support yet, right?), but to keep things simpler as you're doing bringup I think you could continue on as you are. There's some guests that only support riscv,isa like the BSDs (IIRC), so you'd get yourself a better testing base by sticking to dealing with just that one property. If I were you though, I'd probably try to match interpretation with what guests are doing as that's less likely to cause problems. > > > Therefore, only `riscv,isa` parsing is implemented. > > >=20 > > > The `riscv,isa` property is parsed for each CPU, and the common exten= sions > > > are stored in the `host_riscv_isa` bitmap. > > > This bitmap is then used by `riscv_isa_extension_available()` to check > > > if a specific extension is supported. > > >=20 > > > The current implementation is based on Linux kernel v6.12-rc3 > > > implementation with the following changes: > > > - Drop unconditional setting of {RISCV_ISA_EXT_ZICSR, > > > RISCV_ISA_EXT_ZIFENCEI, RISCV_ISA_EXT_ZICNTR, RISCV_ISA_EXT_ZIHPM= } as they > > > are now part of the riscv,isa string. > > Hmm, not sure I follow your logic here. Linux unconditionally sets these > > extensions because the binding was written when these these were part of > > the base instruction set*. That they can be put into the "riscv,isa" > > string is actually the reason that the code setting them unconditionally > > in linux exists! Linux cannot tell the difference between an "old" dtb > > containing "rv64ima" meaning that Zicsr is present & a "new" one contai= ning > > "rv64ima" meaning that it is not! For backwards compatibility reasons, > > linux is then forced to set its internal flags for Zicsr et al when it = sees > > "i" in riscv,isa. If you were to use the "new" definition of "i", and u= se > > that to construct a dtb to pass to a linux guest, things would blow up. >=20 > My fault was that I didn't consider that someone will use "old" dtb and i= t was the reason > why "the binding was written when these these were part of the base instr= uction set*" was > interpreted as it isn't so any more and the mentioned extension should be= explicitly > mentioned in riscv,isa. >=20 > >=20 > > This is the whole reason that riscv,isa is marked deprecated in the bin= ding > > and replaced by riscv,isa-extensions - there are no concrete definitions > > for what extensions mean using "riscv,isa". > >=20 > > I suppose you're free to decide to interpret the property in Xen > > differently to linux - particularly because the hypervisor extension > > requirement means that you're only going to run on hardware produced af= ter > > the aforementioned extensions were split out of "i" - but if you are > > doing that, the rationale for a differing interpretation should be corr= ect > > IMO. >=20 > Agree, I will update the commit message with the wording to: > Drop unconditional setting of {...} because the hypervisor is going to = run on > hardware produced after the aforementioned extensions were split out of= "i". >=20 > >=20 > > Perhaps the wording of my comment in linux was not "strong" enough, and > > the "can be set" should be changed to "must be set"? >=20 > It would be better. Okay, I'll try to remember to get that changed. > > /* > > * These ones were as they were part of the base ISA when the > > * port & dt-bindings were upstreamed, and so can be set > > * unconditionally where `i` is in riscv,isa on DT systems. > > */ > > if (acpi_disabled) { > > set_bit(RISCV_ISA_EXT_ZICSR, source_isa); > > set_bit(RISCV_ISA_EXT_ZIFENCEI, source_isa); > > set_bit(RISCV_ISA_EXT_ZICNTR, source_isa); > > set_bit(RISCV_ISA_EXT_ZIHPM, source_isa); > > } > >=20 > >=20 > >=20 > > * IIRC only 2 of them were part of i, the other two were assumed to be = present > > by Linux etc and only became independent extensions later on. --rR/Ry9TQKrZMd8z9 Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iHUEABYIAB0WIQRh246EGq/8RLhDjO14tDGHoIJi0gUCZ6T1aQAKCRB4tDGHoIJi 0s3kAQC+XB8sqpJTgiUELVPwP9jOSXnUlSX0/ft/ldsTtRn5DQD9HLEBSHvyBiOU 2kHFfc3NWspJvEVU0ul56xvkZIgVYwQ= =wtpS -----END PGP SIGNATURE----- --rR/Ry9TQKrZMd8z9--