From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-sender-0.a4lg.com (mail-sender.a4lg.com [153.120.152.154]) by sourceware.org (Postfix) with ESMTPS id F07DF3858414 for ; Fri, 25 Feb 2022 10:51:06 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org F07DF3858414 Received: from [127.0.0.1] (localhost [127.0.0.1]) by mail-sender-0.a4lg.com (Postfix) with ESMTPSA id A721A300089; Fri, 25 Feb 2022 10:51:04 +0000 (UTC) Message-ID: <216cabba-5600-9e57-4c0b-b97dda951d7c@irq.a4lg.com> Date: Fri, 25 Feb 2022 19:51:03 +0900 Mime-Version: 1.0 Subject: Re: [PATCH 1/3] RISC-V: Add 'Smstateen' extension and its CSRs Content-Language: en-US To: Nelson Chu Cc: Binutils References: From: Tsukasa OI In-Reply-To: Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit X-Spam-Status: No, score=-6.2 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, SPF_HELO_NONE, SPF_PASS, TXREP, T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.4 X-Spam-Checker-Version: SpamAssassin 3.4.4 (2020-01-24) on server2.sourceware.org X-BeenThere: binutils@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Binutils mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 25 Feb 2022 10:51:08 -0000 On 2022/02/25 15:32, Nelson Chu wrote: > > The privileged spec doesn't define these CSRs, they are defined and > controlled by smstaeen extension. So I think the defined and aborted > versions of these DECLARE_CSR should be PRIV_SPEC_CLASS_NONE, just > like other unprivileged CSRs did. In the future, they should be > controlled by smstaeen extension versions, but for now we don't need > to care about this. > > Thanks > Nelson > >> /* Dropped CSRs. */ >> DECLARE_CSR(mbase, CSR_MBASE, CSR_CLASS_I, PRIV_SPEC_CLASS_1P9P1, PRIV_SPEC_CLASS_1P10) >> DECLARE_CSR(mbound, CSR_MBOUND, CSR_CLASS_I, PRIV_SPEC_CLASS_1P9P1, PRIV_SPEC_CLASS_1P10) >> -- >> 2.32.0 >> > 'Sscofpmf' patch (PATCH 2/3) is modified as per your advise. For the rest, I noticed that there are hypervisor-related CSRs. For consistency with CSR_CLASS_I CSRs, I think hypervisor-related CSRs should be masked with privileged architecture version 1.12. However, if need_check_version == true (gas/config/tc-riscv.c) for specific CSR class, there's no option to set PRIV_SPEC_CLASS_NONE because it would just mean "not supported in any version". Separating CSR classes (H/non-H) might be an option. However, I felt this is "too much". Option A: Set minimum privileged specification to 1.9.1 for non-H CSRs Option B: Disable version checking on certain conditions [Option A] How about this? - For 'Sscofpmf' (with no hypervisor-related CSRs), use PRIV_SPEC_CLASS_NONE, PRIV_SPEC_CLASS_NONE with need_check_version = false (as you recommended). - For 'Smstateen' and 'Sstc' (with some hypervisor-related CSRs), need_check_version = true and... - Use PRIV_SPEC_CLASS_1P12, PRIV_SPEC_CLASS_DRAFT for hypervisor-related CSRs (mask with both given extension and Privileged version 1.12) - Use PRIV_SPEC_CLASS_1P9P1, PRIV_SPEC_CLASS_DRAFT for other CSRs (much like unprivileged counter CSRs like `cycle') That means my 'Smstateen' patch (PATCH 1/3) will not be modified. On the other hand, 'Sstc' patch (PATCH 3/3) will be (relax minimum privileged version from 1.11 to 1.9.1 [minimum supported]). [Option B] Another option is to disable version checking if both define_version and abort_version are PRIV_SPEC_CLASS_NONE (or more simply, if define_version is PRIV_SPEC_CLASS_NONE). With current design, that would remove `need_check_version' variable too. TBH, I see both options look equally good but have their cons. I would like to hear everyone else's (including your) thoughts. Thanks, Tsukasa