public inbox for binutils@sourceware.org
 help / color / mirror / Atom feed
* [PATCH] RISC-V: Support ZTSO extension
@ 2022-03-15  2:43 shihua
  2022-03-15 16:24 ` Christoph Muellner
  2022-09-15 19:49 ` Vineet Gupta
  0 siblings, 2 replies; 11+ messages in thread
From: shihua @ 2022-03-15  2:43 UTC (permalink / raw)
  To: binutils
  Cc: kito.cheng, jim.wilson.gcc, cmuellner, palmer, andrew,
	lazyparser, jiawei, nelson.chu, LiaoShihua

From: LiaoShihua <shihua@iscas.ac.cn>

   ZTSO is the extension of tatol store order model.
   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;
+
   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 },
   {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


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] RISC-V: Support ZTSO extension
  2022-03-15  2:43 [PATCH] RISC-V: Support ZTSO extension shihua
@ 2022-03-15 16:24 ` Christoph Muellner
  2022-03-15 18:41   ` Andrew Waterman
  2022-09-15 19:49 ` Vineet Gupta
  1 sibling, 1 reply; 11+ messages in thread
From: Christoph Muellner @ 2022-03-15 16:24 UTC (permalink / raw)
  To: Shihua Liao
  Cc: binutils, Kito Cheng, Jim Wilson, Palmer Dabbelt,
	Andrew Waterman, Wei Wu (吴伟),
	jiawei, nelson.chu

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.


> +
>    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 },
>    {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
>
>

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] RISC-V: Support ZTSO extension
  2022-03-15 16:24 ` Christoph Muellner
@ 2022-03-15 18:41   ` Andrew Waterman
  2022-03-15 18:52     ` Palmer Dabbelt
  0 siblings, 1 reply; 11+ messages in thread
From: Andrew Waterman @ 2022-03-15 18:41 UTC (permalink / raw)
  To: Christoph Muellner
  Cc: Shihua Liao, Binutils, Kito Cheng, Jim Wilson, Palmer Dabbelt,
	Wei Wu (吴伟),
	jiawei, Nelson Chu

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.)

>
>>
>> +
>>    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 },
>>    {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
>>

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] RISC-V: Support ZTSO extension
  2022-03-15 18:41   ` Andrew Waterman
@ 2022-03-15 18:52     ` Palmer Dabbelt
  2022-03-15 19:07       ` Andrew Waterman
  2022-03-15 20:22       ` Christoph Muellner
  0 siblings, 2 replies; 11+ messages in thread
From: Palmer Dabbelt @ 2022-03-15 18:52 UTC (permalink / raw)
  To: Andrew Waterman
  Cc: cmuellner, shihua, binutils, kito.cheng, Jim Wilson, lazyparser,
	jiawei, Nelson Chu

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.

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
>>>

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] RISC-V: Support ZTSO extension
  2022-03-15 18:52     ` Palmer Dabbelt
@ 2022-03-15 19:07       ` Andrew Waterman
  2022-03-15 20:22       ` Christoph Muellner
  1 sibling, 0 replies; 11+ messages in thread
From: Andrew Waterman @ 2022-03-15 19:07 UTC (permalink / raw)
  To: Palmer Dabbelt
  Cc: Christoph Muellner, Shihua Liao, Binutils, Kito Cheng,
	Jim Wilson, Wei Wu, jiawei, Nelson Chu

On Tue, Mar 15, 2022 at 11:52 AM 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

Cool.

> 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.
>
> 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.

Ztso has been frozen for several years and is defined in terms of the
RVWMO PPO formalisms; there isn't any disagreement about what it means
formally.

>
> 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
> >>>

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] RISC-V: Support ZTSO extension
  2022-03-15 18:52     ` Palmer Dabbelt
  2022-03-15 19:07       ` Andrew Waterman
@ 2022-03-15 20:22       ` Christoph Muellner
  2022-03-15 20:41         ` Palmer Dabbelt
  1 sibling, 1 reply; 11+ messages in thread
From: Christoph Muellner @ 2022-03-15 20:22 UTC (permalink / raw)
  To: Palmer Dabbelt
  Cc: Andrew Waterman, Shihua Liao, binutils, Kito Cheng, Jim Wilson,
	Wei Wu (吴伟),
	jiawei, Nelson Chu

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?

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
> >>>
>

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] RISC-V: Support ZTSO extension
  2022-03-15 20:22       ` Christoph Muellner
@ 2022-03-15 20:41         ` Palmer Dabbelt
  2022-03-15 21:42           ` Christoph Muellner
  0 siblings, 1 reply; 11+ messages in thread
From: Palmer Dabbelt @ 2022-03-15 20:41 UTC (permalink / raw)
  To: cmuellner
  Cc: Andrew Waterman, shihua, binutils, kito.cheng, Jim Wilson,
	lazyparser, jiawei, Nelson Chu

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,
> 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
>> >>>
>>

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] RISC-V: Support ZTSO extension
  2022-03-15 20:41         ` Palmer Dabbelt
@ 2022-03-15 21:42           ` Christoph Muellner
  0 siblings, 0 replies; 11+ messages in thread
From: Christoph Muellner @ 2022-03-15 21:42 UTC (permalink / raw)
  To: Palmer Dabbelt
  Cc: Andrew Waterman, Shihua Liao, binutils, Kito Cheng, Jim Wilson,
	Wei Wu (吴伟),
	jiawei, Nelson Chu

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
> >> >>>
> >>
>

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] RISC-V: Support ZTSO extension
  2022-03-15  2:43 [PATCH] RISC-V: Support ZTSO extension shihua
  2022-03-15 16:24 ` Christoph Muellner
@ 2022-09-15 19:49 ` Vineet Gupta
  2022-09-15 20:05   ` Christoph Muellner
  1 sibling, 1 reply; 11+ messages in thread
From: Vineet Gupta @ 2022-09-15 19:49 UTC (permalink / raw)
  To: shihua, binutils; +Cc: cmuellner, jiawei, kito.cheng

On 3/14/22 19:43, shihua@iscas.ac.cn wrote:
> From: LiaoShihua <shihua@iscas.ac.cn>
> 
>     ZTSO is the extension of tatol store order model.
>     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.

Ping ! This got lost because kernel bits were missing.
But this was a pre-req for gcc patch not being merged and I'm sure that 
circularly gates a kernel change :-)

Can we please break the dependency chain and merge this first.

Thx,
-Vineet

> 
> ---
>   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;
> +
>     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 },
>     {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.  */


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] RISC-V: Support ZTSO extension
  2022-09-15 19:49 ` Vineet Gupta
@ 2022-09-15 20:05   ` Christoph Muellner
  2022-09-16 11:36     ` Palmer Dabbelt
  0 siblings, 1 reply; 11+ messages in thread
From: Christoph Muellner @ 2022-09-15 20:05 UTC (permalink / raw)
  To: Vineet Gupta; +Cc: binutils, jiawei, kito.cheng, 廖仕华

[-- Attachment #1: Type: text/plain, Size: 6515 bytes --]

Hi Vineet,

Ztso is currently in public review.
I would expect the kernel patch to be part of the ratification plan.
I'll follow up with Jeff to see where we are.

Thanks,
Christoph


On Thu, Sep 15, 2022 at 9:49 PM Vineet Gupta <vineetg@rivosinc.com> wrote:

> On 3/14/22 19:43, shihua@iscas.ac.cn wrote:
> > From: LiaoShihua <shihua@iscas.ac.cn>
> >
> >     ZTSO is the extension of tatol store order model.
> >     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.
>
> Ping ! This got lost because kernel bits were missing.
> But this was a pre-req for gcc patch not being merged and I'm sure that
> circularly gates a kernel change :-)
>
> Can we please break the dependency chain and merge this first.
>
> Thx,
> -Vineet
>
> >
> > ---
> >   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;
> > +
> >     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 },
> >     {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.  */
>
>

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] RISC-V: Support ZTSO extension
  2022-09-15 20:05   ` Christoph Muellner
@ 2022-09-16 11:36     ` Palmer Dabbelt
  0 siblings, 0 replies; 11+ messages in thread
From: Palmer Dabbelt @ 2022-09-16 11:36 UTC (permalink / raw)
  To: cmuellner; +Cc: Vineet Gupta, kito.cheng, binutils, shihua, jiawei

On Thu, 15 Sep 2022 13:05:47 PDT (-0700), cmuellner@ventanamicro.com wrote:
> Hi Vineet,
>
> Ztso is currently in public review.

It's frozen, though, which is usually the bar for taking code (modulo 
some recent discussions, but I don't think anyone was talking about 
making the bar higher for the RISC-V specs).  IMO it's fine to take 
this, my real worry was about the spec as opposed to the kernel patch -- 
my guess is we'll find little bugs all over the place where the TSO/WMO 
compatibility rules aren't properly handled by software, but we won't 
know until we start setting the bit.

> I would expect the kernel patch to be part of the ratification plan.
> I'll follow up with Jeff to see where we are.
>
> Thanks,
> Christoph
>
>
> On Thu, Sep 15, 2022 at 9:49 PM Vineet Gupta <vineetg@rivosinc.com> wrote:
>
>> On 3/14/22 19:43, shihua@iscas.ac.cn wrote:
>> > From: LiaoShihua <shihua@iscas.ac.cn>
>> >
>> >     ZTSO is the extension of tatol store order model.
>> >     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.
>>
>> Ping ! This got lost because kernel bits were missing.
>> But this was a pre-req for gcc patch not being merged and I'm sure that
>> circularly gates a kernel change :-)
>>
>> Can we please break the dependency chain and merge this first.
>>
>> Thx,
>> -Vineet
>>
>> >
>> > ---
>> >   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;
>> > +
>> >     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 },
>> >     {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.  */
>>
>>

^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2022-09-16 11:36 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-03-15  2:43 [PATCH] RISC-V: Support ZTSO extension shihua
2022-03-15 16:24 ` Christoph Muellner
2022-03-15 18:41   ` Andrew Waterman
2022-03-15 18:52     ` Palmer Dabbelt
2022-03-15 19:07       ` Andrew Waterman
2022-03-15 20:22       ` Christoph Muellner
2022-03-15 20:41         ` Palmer Dabbelt
2022-03-15 21:42           ` Christoph Muellner
2022-09-15 19:49 ` Vineet Gupta
2022-09-15 20:05   ` Christoph Muellner
2022-09-16 11:36     ` Palmer Dabbelt

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).