[PATCH] RISC-V: Support ZTSO extension
Christoph Muellner
cmuellner@ventanamicro.com
Tue Mar 15 21:42:11 GMT 2022
On Tue, Mar 15, 2022 at 9:41 PM Palmer Dabbelt <palmer@dabbelt.com> 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 <palmer@dabbelt.com>
> 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
> >> > <cmuellner@ventanamicro.com> wrote:
> >> >>
> >> >>
> >> >>
> >> >> On Tue, Mar 15, 2022 at 3:44 AM <shihua@iscas.ac.cn> wrote:
> >> >>>
> >> >>> From: LiaoShihua <shihua@iscas.ac.cn>
> >> >>>
> >> >>> 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
> >> >>>
> >>
>
More information about the Binutils
mailing list