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 85EC3385E00A for ; Tue, 28 Jun 2022 05:33:38 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 85EC3385E00A Received: from [127.0.0.1] (localhost [127.0.0.1]) by mail-sender-0.a4lg.com (Postfix) with ESMTPSA id F17A7300089; Tue, 28 Jun 2022 05:33:34 +0000 (UTC) Message-ID: Date: Tue, 28 Jun 2022 14:33:32 +0900 Mime-Version: 1.0 Subject: Re: [PATCH v3 0/4] RISC-V: Add CSRs for several supervisor extensions Content-Language: en-US To: Nelson Chu Cc: Kito Cheng , Palmer Dabbelt , 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.5 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.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) 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: Tue, 28 Jun 2022 05:33:40 -0000 Thanks for your opinion! On 2022/06/28 10:40, Nelson Chu wrote: > Thanks, passed toolchain regressions, so all committed. > > On Fri, Jun 24, 2022 at 10:52 AM Tsukasa OI > wrote: >> >> v1: >> >> v2: >> >> CURRENT (GitHub): >> >> >> This patchset adds CSRs for following extensions: >> >> - Smstateen >> - Sscofpmf >> - Sstc >> >> Because 'H' extension and related CSR class is now implemented, we can >> now safely and consistently implement hypervisor-related CSR feature >> gate masking. >> >> [CHANGES: v2 -> v3] >> - Follow up to now implemented 'H' extension support >> - Add (and refactor) complex CSR feature gate handling (on H and RV32) >> - Make new CSRs independent to privileged architecture >> (as initially suggested by Nelson Chu) >> >> Thanks, >> Tsukasa >> >> >> >> >> Tsukasa OI (4): >> RISC-V: Add new CSR feature gate handling (RV32,H) > > I think this one is a workaround since we may have more complicate > macros like INSN_CLASS in the future (or, and, ...). Maybe we should > have the function similar to riscv_multi_subset_supports_ext for CSRs. > Or maybe we could combine the riscv_multi_subset_supports and > riscv_multi_subset_supports_ext functions into one, just return NULL > if everything went well, otherwise return the error msg directly, and > so does the CSR_CLASS. Anyway, the fix is fine for now, since we will > get wrong error message without it. We could have a better fix in the > future patches. Yes, definitely a workaround. Making something like INSN_CLASS_* handling is LGTM but I have some other thoughts. Unlike INSN_CLASS_*, CSR_CLASS_* will have simpler and comon "additional" requirements like RV32 and H. Then, how about splitting base CSR class and additional requirements? Current DECLARE_CSR design: DECLARE_CSR(mstateen0, CSR_MSTATEEN0, CSR_CLASS_SMSTATEEN,... DECLARE_CSR(hstateen0, CSR_HSTATEEN0, CSR_CLASS_SMSTATEEN_AND_H,... DECLARE_CSR(mstateen0h, CSR_MSTATEEN0H, CSR_CLASS_SMSTATEEN_32,... DECLARE_CSR(hstateen0h, CSR_HSTATEEN0H, CSR_CLASS_SMSTATEEN_AND_H_32,... My idea: DECLARE_CSR(mstateen0, CSR_MSTATEEN0, CSR_CLASS_SMSTATEEN, 0,... DECLARE_CSR(hstateen0, CSR_HSTATEEN0, CSR_CLASS_SMSTATEEN, CSR_REQ_H,... DECLARE_CSR(mstateen0h, CSR_MSTATEEN0H, CSR_CLASS_SMSTATEEN, CSR_REQ_RV32,... DECLARE_CSR(hstateen0h, CSR_HSTATEEN0H, CSR_CLASS_SMSTATEEN, CSR_REQ_H | CSR_REQ_RV32,... Of course, this can be used together with your idea. I would like to hear everyone's thoughts. Thanks, Tsukasa > > Thanks > Nelson > >> RISC-V: Add 'Smstateen' extension and its CSRs >> RISC-V: Add 'Sscofpmf' extension with its CSRs >> RISC-V: Add 'Sstc' extension and its CSRs >> >> bfd/elfxx-riscv.c | 3 + >> gas/config/tc-riscv.c | 47 ++++- >> gas/testsuite/gas/riscv/csr-dw-regnums.d | 54 +++++ >> gas/testsuite/gas/riscv/csr-dw-regnums.s | 57 ++++++ >> gas/testsuite/gas/riscv/csr-version-1p10.d | 108 ++++++++++ >> gas/testsuite/gas/riscv/csr-version-1p10.l | 207 ++++++++++++++++++++ >> gas/testsuite/gas/riscv/csr-version-1p11.d | 108 ++++++++++ >> gas/testsuite/gas/riscv/csr-version-1p11.l | 207 ++++++++++++++++++++ >> gas/testsuite/gas/riscv/csr-version-1p12.d | 108 ++++++++++ >> gas/testsuite/gas/riscv/csr-version-1p12.l | 207 ++++++++++++++++++++ >> gas/testsuite/gas/riscv/csr-version-1p9p1.d | 108 ++++++++++ >> gas/testsuite/gas/riscv/csr-version-1p9p1.l | 207 ++++++++++++++++++++ >> gas/testsuite/gas/riscv/csr.s | 60 ++++++ >> include/opcode/riscv-opc.h | 114 +++++++++++ >> 14 files changed, 1591 insertions(+), 4 deletions(-) >> >> >> base-commit: 54603ee2aeaf248220f0f440c322ff02e98cd403 >> -- >> 2.34.1 >> >