From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-lj1-x233.google.com (mail-lj1-x233.google.com [IPv6:2a00:1450:4864:20::233]) by sourceware.org (Postfix) with ESMTPS id DF63A3858D20 for ; Tue, 15 Mar 2022 21:42:23 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org DF63A3858D20 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-lj1-x233.google.com with SMTP id q10so704660ljc.7 for ; Tue, 15 Mar 2022 14:42:23 -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=LtLJQFiCBEcRguyKWmbAhhnJIpSdACZef0H40qn59nI=; b=FVAarrXB1phDQfEdZvwstdNCGwSYF8N1FYVMRlZW1Tada0XT5uU5mnHlHSY/NvHTRt ET8lBbjiwcR8g/+xbg6/SuQYYVaUPDcXNDlM6s2Br31SmBjowLaKDKqAqC/k1f5fjWY9 LpS6XMd3+poKAQU4wHaC8/ZYfgEeovEJ4MZ+W4e5iCdhZ348DEIzeLvMsWiUG+nvUw+V oVj9PTdJi/Z2mZCiXgoCS6kp1rcdsvxlNSnlucLMaPt29qDlLiZ5QjeWDcv/hnRQHk5F n7uMy8iJ/97TOiyCfnXehKKUVNAnx81mE/L6Ad32cwHaaeyFJZfn3lIGH/h0ZRWV4VTe H22g== 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=LtLJQFiCBEcRguyKWmbAhhnJIpSdACZef0H40qn59nI=; b=OgAVsmSiql51RYb/kfyeP0weRVOLYWri1vT/93qt40khZh4Jmm7QAAcd02poT6/C2h FVMt7/zq8VHzDSJvXtu9DbN32k5tvSy2PVCOchPvHIwlJ3muTZSIRSN33D9GxJYzBkl9 KP/ef0i1ps+2HiqQjmoRK3x8/KfRygy99tkf0O6MU2TizJ3zihpOJYv9Pf9qtLJcyVWW 9w24+6PSuhMO4MQzayciCVx2xbqT8z3Omt3TQOCpqWCUbcUiNz4T57znC+TwbxV8s9eP S51ccyDJ3kx37Guw9QQ3OdLzcwMMPA/99FZTr6xCN23Ki4bhZ5CwrCVZh8Z3RdTy6e+C S+jQ== X-Gm-Message-State: AOAM530oKwOCqFwH1QSRv4GAr6H01IbTIQ3ycRhHDt4HDuqcmMHJFn4H OouQyHEKJuTk3ndUr/o1WC1mSJMW3sWiFSDuo7He2A== X-Google-Smtp-Source: ABdhPJzKxWMDJFoSrO7fWYC/fxJb12ccpDZ4fT7lnGaT9OVHpVWoB+Jg1Bj+muRPKsQ5oEEfB7PVaKYvly2b5XPaaLE= X-Received: by 2002:a2e:3914:0:b0:247:f8fa:5070 with SMTP id g20-20020a2e3914000000b00247f8fa5070mr18407010lja.191.1647380542216; Tue, 15 Mar 2022 14:42:22 -0700 (PDT) MIME-Version: 1.0 References: In-Reply-To: From: Christoph Muellner Date: Tue, 15 Mar 2022 22:42:11 +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=-9.7 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 21:42:27 -0000 On Tue, Mar 15, 2022 at 9:41 PM Palmer Dabbelt wrote: > On Tue, 15 Mar 2022 13:22:04 PDT (-0700), cmuellner@ventanamicro.com > wrote: > > 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? > > Something like this should do it > > diff --git a/arch/riscv/include/asm/elf.h b/arch/riscv/include/asm/elf.h > index f53c40026c7a..5d55021a11ff 100644 > --- a/arch/riscv/include/asm/elf.h > +++ b/arch/riscv/include/asm/elf.h > @@ -28,8 +28,11 @@ > > /* > * This is used to ensure we don't load something for the wrong > architecture. > + * We currently have no TSO implementations and don't probe for it, so > just go > + * ahead and disallow binaries with the TSO flag set. > */ > -#define elf_check_arch(x) ((x)->e_machine == EM_RISCV) > +#define EF_RISCV_TSO (1 << 3) > +#define elf_check_arch(x) ((x)->e_machine == EM_RISCV && > !((x)->e_flags & EF_RISCV_TSO)) > > #define CORE_DUMP_USE_REGSET > #define ELF_EXEC_PAGESIZE (PAGE_SIZE) > > but I haven't event built it (much less tested it). It's probably worth > breaking that out into a function and providing a more useful error > message too, not sure if there's a smarter way to fit that all together. > I'm pretty buried right now so if you want to pick it up that's always > appreciated, otherwise it'll go in the pile -- not sure it's all that > important, as we don't have any TSO userspace, but it wouldn't hurt. > Thanks for the draft, Palmer! I'll ask RVI dev partners to pick this up from here. 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 > >> >>> > >> >