From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-pj1-x102a.google.com (mail-pj1-x102a.google.com [IPv6:2607:f8b0:4864:20::102a]) by sourceware.org (Postfix) with ESMTPS id 6D9993858403 for ; Tue, 15 Mar 2022 20:41:46 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 6D9993858403 Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=dabbelt.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=dabbelt.com Received: by mail-pj1-x102a.google.com with SMTP id mj15-20020a17090b368f00b001c637aa358eso2966340pjb.0 for ; Tue, 15 Mar 2022 13:41:46 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=dabbelt-com.20210112.gappssmtp.com; s=20210112; h=date:subject:in-reply-to:cc:from:to:message-id:mime-version :content-transfer-encoding; bh=lkrcr8PotKKnYSrC81gIO6MTD+UhQWPKvE/tapDnIEw=; b=m5D2cQLhBpzX5CTXEQ8M6SuBtxRsov/6FKHV8E9rYsQl1FTj2k2V+AB6ya5KcKsH8W ncUxhUe4Q2MicfuXK/lfgLB39K6V4CCy19jEtMOjnAbt++dJpAdoKa/O7h9jRQ2etwJj 3i/7WDoRLdskoh68UjNOtAgAhspB6YOSbSCvctYqI3C/nsjQ7IdOnphYRAxutITpVzYD fhbalXvfG1Ef6gz7CbsTjEZXlnS7rS87pmsdyMw0Bz2rHUKal3Y5Nw7wrjJZYrd9mLVG VWFvEm3mXPfanOJ4wzAP0yyxIc2k1pk+lra5a4sFhcEXnKF1C2FVG2PIziL0yLaedJcD zihQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:date:subject:in-reply-to:cc:from:to:message-id :mime-version:content-transfer-encoding; bh=lkrcr8PotKKnYSrC81gIO6MTD+UhQWPKvE/tapDnIEw=; b=sktpvOuYKADdrrr3A1wZeZzLx4kRb2YolQYuzSmU2I2MXnVtGM7OH7xG/SGTlRztlu FUJmWZcP+oYUnox7YpZAAHnjAlLUjdkkrlzbIXkgC1O4hbaVRsJZxIS7AdicuzLFg32d H9K9rV4/seaJVwJVvLDHUUxTavWSc28CMUsAiicKyvT6Sc3NPEHF7I+ewb2JwBG/H6jq JAElAzqFivTaq+t4juhUYhlQuuoczisxgm7D/un0nRrkPkh75/y+6KXEeEpsXZ+t1jhm mb5pwss+BeDFUryNrwdjgoBBJG6iKpE+qz4vEM+4TyhhfbXbsXAN07HWhlbIl2lzZaos wU0g== X-Gm-Message-State: AOAM530yqB5la/JSTki9l6pkiS8P/3WZN24jauCq0ngi/rcnaPX9toxD 5H12ZMtrEqM8nRwH/T15dV9bbg== X-Google-Smtp-Source: ABdhPJyIyOtGcBzI4IL+fjFK5qZJAw+GjuNgZoV+BMcW5c2JCYg2P/LaWMwOke22O6m4wwIMCsKyxw== X-Received: by 2002:a17:902:e944:b0:14e:dc4f:f099 with SMTP id b4-20020a170902e94400b0014edc4ff099mr29992131pll.161.1647376905212; Tue, 15 Mar 2022 13:41:45 -0700 (PDT) Received: from localhost ([12.3.194.138]) by smtp.gmail.com with ESMTPSA id w8-20020a63a748000000b0038117e18f02sm119777pgo.29.2022.03.15.13.41.44 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 15 Mar 2022 13:41:44 -0700 (PDT) Date: Tue, 15 Mar 2022 13:41:44 -0700 (PDT) X-Google-Original-Date: Tue, 15 Mar 2022 13:40:56 PDT (-0700) Subject: Re: [PATCH] RISC-V: Support ZTSO extension In-Reply-To: CC: Andrew Waterman , shihua@iscas.ac.cn, binutils@sourceware.org, kito.cheng@sifive.com, Jim Wilson , lazyparser@gmail.com, jiawei@iscas.ac.cn, Nelson Chu From: Palmer Dabbelt To: cmuellner@ventanamicro.com Message-ID: Mime-Version: 1.0 (MHng) Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 8bit X-Spam-Status: No, score=-11.0 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, GIT_PATCH_0, 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 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:41:49 -0000 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, > 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 >> >>> >>