[PATCH] RISC-V: Generate ELF priv attributes if priv instruction are explicited used.

[PATCH] RISC-V: Generate ELF priv attributes if priv instruction are explicited used.

[Nelson Chu]

We should generate the ELF priv attributes only if,

* The priv attributes are aleady set in the assembly file.
* The CSR are explicited used.
* The privileged instruction are explicited used.

There are seven privileged instruction, five of them are defined in the priv
spec v1.11, the remaining two are obselete and seems to be dropped in v1.11.

[v1.11]: mret, sret, uret, wfi and sfence.vma.
[obselete]: hret and sfence.vm.

[missing?]: hfence.bvma and hfence.gvma.
[debug]: dret.  We should record the debug spec versions once it is explicited
used in the future.

	gas/
	* config/tc-riscv.c (explicit_priv_attr): Rename explicit_csr to
	explicit_priv_attr.  It used to indicate CSR or priv instructions are
	explictly used
	(riscv_is_priv_insn): Return True if it is a privileged instruction.
	(riscv_ip): Call riscv_is_priv_insn to check whether the instruction
	is privileged or not.  If it is, then set explicit_priv_attr to TRUE.
	(riscv_write_out_attrs): Clarification of when to generate the elf
	priv spec attributes.

	* testsuite/gas/riscv/attribute-11.s: Add comments.
	* testsuite/gas/riscv/attribute-14.s: New testcase.  Use symbol
	`priv_insn_<n>` to control which priv instruction is expected to used.
	(<n> is a to b.)
	* testsuite/gas/riscv/attribute-14a.d: Likewise.
	* testsuite/gas/riscv/attribute-14b.d: Likewise.
	* testsuite/gas/riscv/attribute-14c.d: Likewise.
	* testsuite/gas/riscv/attribute-14d.d: Likewise.
	* testsuite/gas/riscv/attribute-14e.d: Likewise.
	* testsuite/gas/riscv/attribute-14f.d: Likewise.
	* testsuite/gas/riscv/attribute-14g.d: Likewise.
---
 gas/config/tc-riscv.c                   | 42 ++++++++++++++++++++++++++++-----
 gas/testsuite/gas/riscv/attribute-11.s  |  2 ++
 gas/testsuite/gas/riscv/attribute-14.s  | 25 ++++++++++++++++++++
 gas/testsuite/gas/riscv/attribute-14a.d |  8 +++++++
 gas/testsuite/gas/riscv/attribute-14b.d |  8 +++++++
 gas/testsuite/gas/riscv/attribute-14c.d |  8 +++++++
 gas/testsuite/gas/riscv/attribute-14d.d |  8 +++++++
 gas/testsuite/gas/riscv/attribute-14e.d |  8 +++++++
 gas/testsuite/gas/riscv/attribute-14f.d |  8 +++++++
 gas/testsuite/gas/riscv/attribute-14g.d |  8 +++++++
 10 files changed, 119 insertions(+), 6 deletions(-)
 create mode 100644 gas/testsuite/gas/riscv/attribute-14.s
 create mode 100644 gas/testsuite/gas/riscv/attribute-14a.d
 create mode 100644 gas/testsuite/gas/riscv/attribute-14b.d
 create mode 100644 gas/testsuite/gas/riscv/attribute-14c.d
 create mode 100644 gas/testsuite/gas/riscv/attribute-14d.d
 create mode 100644 gas/testsuite/gas/riscv/attribute-14e.d
 create mode 100644 gas/testsuite/gas/riscv/attribute-14f.d
 create mode 100644 gas/testsuite/gas/riscv/attribute-14g.d

diff --git a/gas/config/tc-riscv.c b/gas/config/tc-riscv.c
index b6c8c4e..d42c709 100644
--- a/gas/config/tc-riscv.c
+++ b/gas/config/tc-riscv.c
@@ -356,8 +356,8 @@ static bfd_boolean start_assemble = FALSE;
 /* Indicate ELF attributes are explictly set.  */
 static bfd_boolean explicit_attr = FALSE;
 
-/* Indicate CSR are explictly used.  */
-static bfd_boolean explicit_csr = FALSE;
+/* Indicate CSR or priv instructions are explictly used.  */
+static bfd_boolean explicit_priv_attr = FALSE;
 
 /* Macros for encoding relaxation state for RVC branches and far jumps.  */
 #define RELAX_BRANCH_ENCODE(uncond, rvc, length)	\
@@ -1766,6 +1766,29 @@ riscv_csr_read_only_check (insn_t insn)
   return TRUE;
 }
 
+/* Return True if it is a privileged instruction.  Otherwise, return FALSE.
+
+   We don't have Hypervisor instructions for now, but they are defined
+   in the v1.11 priv spec.  We propably need to add hfence.bvma and
+   hfence.gvma, or update the priv spec.
+
+   dret is defined in the debug spec, so it should be checked in
+   the future, too.  */
+
+static bfd_boolean
+riscv_is_priv_insn (insn_t insn)
+{
+  return (((insn ^ MATCH_URET) & MASK_URET) == 0
+	  || ((insn ^ MATCH_SRET) & MASK_SRET) == 0
+	  || ((insn ^ MATCH_MRET) & MASK_MRET) == 0
+	  || ((insn ^ MATCH_SFENCE_VMA) & MASK_SFENCE_VMA) == 0
+	  || ((insn ^ MATCH_WFI) & MASK_WFI) == 0
+  /* These are dropped in the v1.11 priv specs, but we still check
+     them here to keep the compatible.  */
+	  || ((insn ^ MATCH_HRET) & MASK_HRET) == 0
+	  || ((insn ^ MATCH_SFENCE_VM) & MASK_SFENCE_VM) == 0);
+}
+
 /* This routine assembles an instruction into its binary format.  As a
    side effect, it sets the global variable imm_reloc to the type of
    relocation to do if one of the operands is an address expression.  */
@@ -1833,6 +1856,9 @@ riscv_ip (char *str, struct riscv_cl_insn *ip, expressionS *imm_expr,
 		      && !riscv_opts.rvc)
 		    break;
 
+		  if (riscv_is_priv_insn (ip->insn_opcode))
+		    explicit_priv_attr = TRUE;
+
 		  /* Check if we write a read-only CSR by the CSR
 		     instruction.  */
 		  if (insn_with_csr
@@ -2197,7 +2223,7 @@ riscv_ip (char *str, struct riscv_cl_insn *ip, expressionS *imm_expr,
 
 	    case 'E':		/* Control register.  */
 	      insn_with_csr = TRUE;
-	      explicit_csr = TRUE;
+	      explicit_priv_attr = TRUE;
 	      if (reg_lookup (&s, RCLASS_CSR, &regno))
 		INSERT_OPERAND (CSR, *ip, regno);
 	      else
@@ -3591,9 +3617,13 @@ riscv_write_out_attrs (void)
       && !riscv_set_default_priv_spec (NULL))
     return;
 
-  /* If we already have set elf priv attributes, then generate them.
-     Otherwise, don't generate them when no CSR are used.  */
-  if (!explicit_csr)
+  /* If we already have set elf priv attributes, then no need to do anything,
+     assembler will generate them according to what you set.  Otherwise, don't
+     generate or update them when no CSR and priv instructions are used.
+     Generate the priv attributes according to default_priv_spec, which can be
+     set by -mpriv-spec and --with-priv-spec, and be updated by the original
+     priv attribute sets.  */
+  if (!explicit_priv_attr)
     return;
 
   /* Re-write priv attributes by default_priv_spec.  */
diff --git a/gas/testsuite/gas/riscv/attribute-11.s b/gas/testsuite/gas/riscv/attribute-11.s
index 81099b2..81367da 100644
--- a/gas/testsuite/gas/riscv/attribute-11.s
+++ b/gas/testsuite/gas/riscv/attribute-11.s
@@ -1 +1,3 @@
+	# The csr is explicit used, so we need to generate the priv spec
+	# attributes even if the attributes are not set.
 	csrr a0, 0x0
diff --git a/gas/testsuite/gas/riscv/attribute-14.s b/gas/testsuite/gas/riscv/attribute-14.s
new file mode 100644
index 0000000..0bd59da
--- /dev/null
+++ b/gas/testsuite/gas/riscv/attribute-14.s
@@ -0,0 +1,25 @@
+	# The priv instruction is explicit used, so we need to generate
+	# the priv spec attributes even if the attributes are not set.
+.ifdef priv_insn_a
+	mret
+.endif
+.ifdef priv_insn_b
+	sret
+.endif
+.ifdef priv_insn_c
+	uret
+.endif
+.ifdef priv_insn_d
+	wfi
+.endif
+.ifdef priv_insn_e
+	sfence.vma
+.endif
+
+	# These are obselete priv instruction.
+.ifdef priv_insn_f
+	hret
+.endif
+.ifdef priv_insn_g
+	sfence.vm
+.endif
diff --git a/gas/testsuite/gas/riscv/attribute-14a.d b/gas/testsuite/gas/riscv/attribute-14a.d
new file mode 100644
index 0000000..7b66f0f
--- /dev/null
+++ b/gas/testsuite/gas/riscv/attribute-14a.d
@@ -0,0 +1,8 @@
+#as: -march-attr --defsym priv_insn_a=1
+#readelf: -A
+#source: attribute-14.s
+Attribute Section: riscv
+File Attributes
+  Tag_RISCV_arch: [a-zA-Z0-9_\"].*
+  Tag_RISCV_priv_spec: [0-9_\"].*
+#...
diff --git a/gas/testsuite/gas/riscv/attribute-14b.d b/gas/testsuite/gas/riscv/attribute-14b.d
new file mode 100644
index 0000000..e044f4f
--- /dev/null
+++ b/gas/testsuite/gas/riscv/attribute-14b.d
@@ -0,0 +1,8 @@
+#as: -march-attr --defsym priv_insn_b=1
+#readelf: -A
+#source: attribute-14.s
+Attribute Section: riscv
+File Attributes
+  Tag_RISCV_arch: [a-zA-Z0-9_\"].*
+  Tag_RISCV_priv_spec: [0-9_\"].*
+#...
diff --git a/gas/testsuite/gas/riscv/attribute-14c.d b/gas/testsuite/gas/riscv/attribute-14c.d
new file mode 100644
index 0000000..6311a9d
--- /dev/null
+++ b/gas/testsuite/gas/riscv/attribute-14c.d
@@ -0,0 +1,8 @@
+#as: -march-attr --defsym priv_insn_c=1
+#readelf: -A
+#source: attribute-14.s
+Attribute Section: riscv
+File Attributes
+  Tag_RISCV_arch: [a-zA-Z0-9_\"].*
+  Tag_RISCV_priv_spec: [0-9_\"].*
+#...
diff --git a/gas/testsuite/gas/riscv/attribute-14d.d b/gas/testsuite/gas/riscv/attribute-14d.d
new file mode 100644
index 0000000..f0f2d63
--- /dev/null
+++ b/gas/testsuite/gas/riscv/attribute-14d.d
@@ -0,0 +1,8 @@
+#as: -march-attr --defsym priv_insn_d=1
+#readelf: -A
+#source: attribute-14.s
+Attribute Section: riscv
+File Attributes
+  Tag_RISCV_arch: [a-zA-Z0-9_\"].*
+  Tag_RISCV_priv_spec: [0-9_\"].*
+#...
diff --git a/gas/testsuite/gas/riscv/attribute-14e.d b/gas/testsuite/gas/riscv/attribute-14e.d
new file mode 100644
index 0000000..47fdc2e
--- /dev/null
+++ b/gas/testsuite/gas/riscv/attribute-14e.d
@@ -0,0 +1,8 @@
+#as: -march-attr --defsym priv_insn_e=1
+#readelf: -A
+#source: attribute-14.s
+Attribute Section: riscv
+File Attributes
+  Tag_RISCV_arch: [a-zA-Z0-9_\"].*
+  Tag_RISCV_priv_spec: [0-9_\"].*
+#...
diff --git a/gas/testsuite/gas/riscv/attribute-14f.d b/gas/testsuite/gas/riscv/attribute-14f.d
new file mode 100644
index 0000000..0bfd5dc
--- /dev/null
+++ b/gas/testsuite/gas/riscv/attribute-14f.d
@@ -0,0 +1,8 @@
+#as: -march-attr --defsym priv_insn_f=1
+#readelf: -A
+#source: attribute-14.s
+Attribute Section: riscv
+File Attributes
+  Tag_RISCV_arch: [a-zA-Z0-9_\"].*
+  Tag_RISCV_priv_spec: [0-9_\"].*
+#...
diff --git a/gas/testsuite/gas/riscv/attribute-14g.d b/gas/testsuite/gas/riscv/attribute-14g.d
new file mode 100644
index 0000000..ea595ef
--- /dev/null
+++ b/gas/testsuite/gas/riscv/attribute-14g.d
@@ -0,0 +1,8 @@
+#as: -march-attr --defsym priv_insn_g=1
+#readelf: -A
+#source: attribute-14.s
+Attribute Section: riscv
+File Attributes
+  Tag_RISCV_arch: [a-zA-Z0-9_\"].*
+  Tag_RISCV_priv_spec: [0-9_\"].*
+#...
-- 
2.7.4

Re: [PATCH] RISC-V: Generate ELF priv attributes if priv instruction are explicited used.

[Jim Wilson]

On Tue, Jun 16, 2020 at 7:38 PM Nelson Chu <nelson.chu@sifive.com> wrote:
> [v1.11]: mret, sret, uret, wfi and sfence.vma.

FYI uret is N extension only, though we never tried to track that.
And since this is more of a priv extension than a user extension, it
isn't clear if we want to ask people to specify that.  We don't ask
people to specify whether supervisor mode exists before using sret for
instance.

> [obselete]: hret and sfence.vm.

These were dropped in priv spec 1.10.  The comments in the patch say
1.11.  I'd suggest updating the comments.

FYI if you check out riscv-priv-1.10 from riscv-isa-manual you can get
copies of old versions of the priv spec manuals in the release subdir.
Just remember to make a copy of the release dir, or rename it, or you
will lose it when checking out master again.

> [missing?]: hfence.bvma and hfence.gvma.

These are in the new hypervisor spec, part of the priv spec 1.12
draft.  Looks like a bug in the priv spec 1.11 that it has instruction
encodings for them.  Looking at the history, I see that the draft
hypervisor was included for a while and then dropped before
ratification, except they apparently forgot to drop the hypervisor
instruction encoding table.  And I see that the encoding of one
changed in v1.12, so we definitely don't want to implement the v1.11
versions of these.  I would suggest not having any mention of these
instructions in the patch.

> [debug]: dret.  We should record the debug spec versions once it is explicited
> used in the future.

Sounds reasonable.

I notice you set priv spec to 1.11 when hret or sfence.vm is used.
But to get that right we would have to have min/max priv spec versions
for priv insns which we don't have, so this is OK for now.

The patch is OK with the minor comment fixes I suggested above.

Jim

Re: [PATCH] RISC-V: Generate ELF priv attributes if priv instruction are explicited used.

[Andreas Schwab]

On Jun 20 2020, Jim Wilson wrote:

> FYI if you check out riscv-priv-1.10 from riscv-isa-manual you can get
> copies of old versions of the priv spec manuals in the release subdir.
> Just remember to make a copy of the release dir, or rename it, or you
> will lose it when checking out master again.

Or check it out in a new worktree.

Andreas.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 7578 EB47 D4E5 4D69 2510  2552 DF73 E780 A9DA AEC1
"And now for something completely different."