From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-lf1-x132.google.com (mail-lf1-x132.google.com [IPv6:2a00:1450:4864:20::132]) by sourceware.org (Postfix) with ESMTPS id CF15C3858C3A for ; Tue, 15 Mar 2022 20:22:16 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org CF15C3858C3A Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=ventanamicro.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=ventanamicro.com Received: by mail-lf1-x132.google.com with SMTP id g17so444281lfh.2 for ; Tue, 15 Mar 2022 13:22:16 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=ventanamicro.com; s=google; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=3eNtJKcC0o89P9RlCr+dYX0HYkIBIuDAtb5EPekHldE=; b=ORDD/GT267NhLYjul5NHzTMD0Po2fOuaR/ENnwwY3RPyV2jFTEhvsMWLZ4yuvoo3xp ZKQAABIdsgn1bj7FyXYDtRuuoZl7CBIfXQ43jGIbEAchxhiazB0XORjvHOo/y1lUsI4U Ilr4wGiMvocVT9v3wIS2w3Y3yknbI0MMGEveSB91/qQwDeMk/8vg2CcIXOSMLUYacHsj OOaD7ABSE4Hi34eosKk02Gc7NZ61d91mCMbsLW1MBNkNU93y+48muN/+dnaQ6Vc/sJzH g2Sq4wZRIsSbbaejlMoX4nu+ywn3SqXNwuRFYo3g+BPA2EDD8oXcVCjTdnx9J63Um17H OerQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=3eNtJKcC0o89P9RlCr+dYX0HYkIBIuDAtb5EPekHldE=; b=a5LU3GhYtmH2li3hk9iF6dSI4XSF9nhG+lDNday2jK8hjL6z5Jt+l0kBob856Wm4QW coew/Azp9P3EOVs1UVHiXYDOX+eqr7GFmjaWBlR2upT0uw3/KHfst5AiSLYqL/Cxf73n C/eIsxQgX5UMOjYIo82lZgmn252JRp3rElV3E4bX0a2GXqkirKP4kvoYtGq+j1iIQfXF pwk2a533S6Fad/+MxddHTzziuaga0VCHgcRYM/eyKG+zfSjOpMUDEjf7O1vtNNz9EYh3 wfmpQSRPbEiyIwSET2O0GsI8mQWSiDxFpcRGGpqJr7X4wHlJWqVaY3nka9T/j2lo40gb Cpeg== X-Gm-Message-State: AOAM530wvPQfOxJMgFcshjyl8KoYGMQuquw/VGz/ekRQq341s8SjmYQC I9JjXlyXcEqWOFO1jip/EbEEST/v6gg4r1tUx22HtA== X-Google-Smtp-Source: ABdhPJx1bjRr3z+uq4s/+jE7C5DdLS4eASXW0py/C8t2fRbuibLa6SrPCHoPPv0LidfA5hfVMLPelO0phJxRIyoHbzU= X-Received: by 2002:a05:6512:36d3:b0:448:b502:884f with SMTP id e19-20020a05651236d300b00448b502884fmr739435lfs.225.1647375735285; Tue, 15 Mar 2022 13:22:15 -0700 (PDT) MIME-Version: 1.0 References: In-Reply-To: From: Christoph Muellner Date: Tue, 15 Mar 2022 21:22:04 +0100 Message-ID: Subject: Re: [PATCH] RISC-V: Support ZTSO extension To: Palmer Dabbelt Cc: Andrew Waterman , Shihua Liao , binutils@sourceware.org, Kito Cheng , Jim Wilson , =?UTF-8?B?V2VpIFd1ICjlkLTkvJ8p?= , jiawei , Nelson Chu X-Spam-Status: No, score=-10.1 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, HTML_MESSAGE, RCVD_IN_DNSWL_NONE, 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 Content-Type: text/plain; charset="UTF-8" X-Content-Filtered-By: Mailman/MimeDel 2.1.29 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, 15 Mar 2022 20:22:20 -0000 On Tue, Mar 15, 2022 at 7:52 PM Palmer Dabbelt wrote: > On Tue, 15 Mar 2022 11:41:14 PDT (-0700), Andrew Waterman wrote: > > On Tue, Mar 15, 2022 at 9:24 AM Christoph Muellner > > wrote: > >> > >> > >> > >> On Tue, Mar 15, 2022 at 3:44 AM wrote: > >>> > >>> From: LiaoShihua > >>> > >>> ZTSO is the extension of tatol store order model. > >> > >> > >> typo: tatol -> total > >> > >>> > >>> This extension adds no new instructions to the ISA, and you can use > it with arch "ztso". > >>> If you use it, TSO flag will be generate in the ELF header. > >>> > >>> bfd\ChangeLog: > >>> > >>> * elfnn-riscv.c (_bfd_riscv_elf_merge_private_bfd_data):Add > support for ztso extension. > >>> * elfxx-riscv.c (riscv_multi_subset_supports):Ditto. > >>> > >>> binutils\ChangeLog: > >>> > >>> * readelf.c (get_machine_flags):Ditto. > >>> > >>> gas\ChangeLog: > >>> > >>> * config/tc-riscv.c (struct riscv_set_options):Ditto. > >>> (riscv_set_tso):Ditto. > >>> (riscv_set_arch):Ditto. > >>> > >>> include\ChangeLog: > >>> > >>> * elf/riscv.h (EF_RISCV_TSO):Ditto. > >>> * opcode/riscv.h (enum riscv_insn_class):Ditto. > >>> > >>> --- > >>> bfd/elfnn-riscv.c | 3 +++ > >>> bfd/elfxx-riscv.c | 3 +++ > >>> binutils/readelf.c | 3 +++ > >>> gas/config/tc-riscv.c | 17 +++++++++++++++++ > >>> include/elf/riscv.h | 3 +++ > >>> include/opcode/riscv.h | 1 + > >>> 6 files changed, 30 insertions(+) > >>> > >>> diff --git a/bfd/elfnn-riscv.c b/bfd/elfnn-riscv.c > >>> index 8f9f0d8a86a..25e8082b957 100644 > >>> --- a/bfd/elfnn-riscv.c > >>> +++ b/bfd/elfnn-riscv.c > >>> @@ -3886,6 +3886,9 @@ _bfd_riscv_elf_merge_private_bfd_data (bfd > *ibfd, struct bfd_link_info *info) > >>> /* Allow linking RVC and non-RVC, and keep the RVC flag. */ > >>> elf_elfheader (obfd)->e_flags |= new_flags & EF_RISCV_RVC; > >>> > >>> + /* Allow linking ZTSO and non-ZTSO, and keep the TSO flag. */ > >>> + elf_elfheader (obfd)->e_flags |= new_flags & EF_RISCV_TSO; > >> > >> > >> Is this flag evaluated anywhere? > >> I.e. how do we protect users from executing TSO binaries on RVWMO > systems? > >> In the case of e.g. compressed instructions, they will run into a > SIG_ILL, but here they will observe unpredictable behavior if we don't do > anything. > > > > The intent had always been that the ELF loaders (OS kernel, dynamic > > linker, etc.) would enforce this property. The work hasn't been done > > for the Linux kernel or glibc, AFAIK, presumably because Ztso hasn't > > been a priority. The EF_RISCV_TSO bit has long been reserved for this > > purpose, though, and so nothing is holding up that work. (The Linux > > kernel will need to be informed about the presence of Ztso via DT, and > > the dynamic linker will need to be informed via HWCAP. Until all of > > that plumbing exists, these loaders should conservatively reject Ztso > > binaries.) > > IIUC we're half way there: glibc is already rejecting dynamic libraries > with the TSO bit set, but Linux appears to ignore the flags entirely. I > think that means static binaries with the TSO bit would end up loaded, > but IMO that's just a Linux bug and it shouldn't be that hard to fix. > Do you think you will find some time to address this in the kernel or should somebody else work this out? Thanks, Christoph > > My bigger worry here is allowing TSO to be flipped on before we actually > know what the memory model is. I'm not sure what the right way to go is > here: we've got a "it's TSO" extension ratified, but there's no formal > description of the memory model. I know we're relying on poorly defined > semantics for other stuff, but IMO memory model land is just too > complicated for that sort of thing. > > If there's actual hardware implementations of this around then it's a > different story, but without that I'd be somewhat reluctant to start > generating TSO binaries as then we'll be stuck remaining compatible with > whatever's allowed by Ztso v0.1. > > > > >> > >>> > >>> + > >>> return true; > >>> > >>> fail: > >>> diff --git a/bfd/elfxx-riscv.c b/bfd/elfxx-riscv.c > >>> index 2915b74dd0f..a041c89a623 100644 > >>> --- a/bfd/elfxx-riscv.c > >>> +++ b/bfd/elfxx-riscv.c > >>> @@ -1215,6 +1215,7 @@ static struct riscv_supported_ext > riscv_supported_std_z_ext[] = > >>> {"zvl16384b", ISA_SPEC_CLASS_DRAFT, 1, 0, > 0 }, > >>> {"zvl32768b", ISA_SPEC_CLASS_DRAFT, 1, 0, > 0 }, > >>> {"zvl65536b", ISA_SPEC_CLASS_DRAFT, 1, 0, > 0 }, > >>> + {"ztso", ISA_SPEC_CLASS_DRAFT, 0, 1, 0 }, > > Ztso v0.1 is in 20191213, so at least we don't have to call it a draft. > > >>> {NULL, 0, 0, 0, 0} > >>> }; > >>> > >>> @@ -2393,6 +2394,8 @@ riscv_multi_subset_supports > (riscv_parse_subset_t *rps, > >>> || riscv_subset_supports (rps, "zve32f")); > >>> case INSN_CLASS_SVINVAL: > >>> return riscv_subset_supports (rps, "svinval"); > >>> + case INSN_CLASS_ZTSO: > >>> + return riscv_subset_supports (rps, "ztso"); > >>> default: > >>> rps->error_handler > >>> (_("internal: unreachable INSN_CLASS_*")); > >>> diff --git a/binutils/readelf.c b/binutils/readelf.c > >>> index 16efe1dfd2d..ba4d6f9db4f 100644 > >>> --- a/binutils/readelf.c > >>> +++ b/binutils/readelf.c > >>> @@ -3975,6 +3975,9 @@ get_machine_flags (Filedata * filedata, unsigned > e_flags, unsigned e_machine) > >>> > >>> if (e_flags & EF_RISCV_RVE) > >>> strcat (buf, ", RVE"); > >>> + > >>> + if (e_flags & EF_RISCV_TSO) > >>> + strcat (buf, ", TSO"); > >>> > >>> switch (e_flags & EF_RISCV_FLOAT_ABI) > >>> { > >>> diff --git a/gas/config/tc-riscv.c b/gas/config/tc-riscv.c > >>> index 9cc0abfda88..ed33cfa919a 100644 > >>> --- a/gas/config/tc-riscv.c > >>> +++ b/gas/config/tc-riscv.c > >>> @@ -222,6 +222,7 @@ struct riscv_set_options > >>> int relax; /* Emit relocs the linker is allowed to relax. */ > >>> int arch_attr; /* Emit architecture and privileged elf attributes. > */ > >>> int csr_check; /* Enable the CSR checking. */ > >>> + int tso; /* Use TSO model. */ > >>> }; > >>> > >>> static struct riscv_set_options riscv_opts = > >>> @@ -231,6 +232,7 @@ static struct riscv_set_options riscv_opts = > >>> 1, /* relax */ > >>> DEFAULT_RISCV_ATTR, /* arch_attr */ > >>> 0, /* csr_check */ > >>> + 0, /* tso */ > >>> }; > >>> > >>> /* Enable or disable the rvc flags for riscv_opts. Turn on the rvc > flag > >>> @@ -245,6 +247,18 @@ riscv_set_rvc (bool rvc_value) > >>> riscv_opts.rvc = rvc_value; > >>> } > >>> > >>> +/* Enable or disable the tso flags for riscv_opts. Turn on the tso > flag > >>> + for elf_flags once we have enabled ztso extension. */ > >>> + > >>> +static void > >>> +riscv_set_tso (bool tso_value) > >>> +{ > >>> + if (tso_value) > >>> + elf_flags |= EF_RISCV_TSO; > >>> + > >>> + riscv_opts.tso = tso_value; > >>> +} > >>> + > >>> /* This linked list records all enabled extensions, which are parsed > from > >>> the architecture string. The architecture string can be set by the > >>> -march option, the elf architecture attributes, and the --with-arch > >>> @@ -295,6 +309,9 @@ riscv_set_arch (const char *s) > >>> riscv_set_rvc (false); > >>> if (riscv_subset_supports (&riscv_rps_as, "c")) > >>> riscv_set_rvc (true); > >>> + > >>> + if (riscv_subset_supports (&riscv_rps_as, "ztso")) > >>> + riscv_set_tso (true); > >>> } > >>> > >>> /* Indicate -mabi option is explictly set. */ > >>> diff --git a/include/elf/riscv.h b/include/elf/riscv.h > >>> index d0acf6886d8..eed3ec5f82e 100644 > >>> --- a/include/elf/riscv.h > >>> +++ b/include/elf/riscv.h > >>> @@ -114,6 +114,9 @@ END_RELOC_NUMBERS (R_RISCV_max) > >>> /* File uses the 32E base integer instruction. */ > >>> #define EF_RISCV_RVE 0x0008 > >>> > >>> +/* File uses the TSO model. */ > >>> +#define EF_RISCV_TSO 0x0010 > >>> + > >>> /* The name of the global pointer symbol. */ > >>> #define RISCV_GP_SYMBOL "__global_pointer$" > >>> > >>> diff --git a/include/opcode/riscv.h b/include/opcode/riscv.h > >>> index 048ab0a5d68..ed81df271c1 100644 > >>> --- a/include/opcode/riscv.h > >>> +++ b/include/opcode/riscv.h > >>> @@ -388,6 +388,7 @@ enum riscv_insn_class > >>> INSN_CLASS_V, > >>> INSN_CLASS_ZVEF, > >>> INSN_CLASS_SVINVAL, > >>> + INSN_CLASS_ZTSO, > >>> }; > >>> > >>> /* This structure holds information for a particular instruction. */ > >>> -- > >>> 2.31.1.windows.1 > >>> >