public inbox for binutils@sourceware.org
 help / color / mirror / Atom feed
* [RFC 0/4] riscv/binutils support Hypervisor Extension
@ 2021-12-16 17:33 Vineet Gupta
  2021-12-16 17:33 ` [RFC 1/4] RISC-V: Hypervisor ext: Treat as "Standard" extension Vineet Gupta
                   ` (3 more replies)
  0 siblings, 4 replies; 17+ messages in thread
From: Vineet Gupta @ 2021-12-16 17:33 UTC (permalink / raw)
  To: Binutils, Nelson Chu; +Cc: Kito Cheng, jim.wilson.gcc, palmer, Vineet Gupta

Hi,

This patchset adds support for Hypervisor Extension ratified recently
[1].

This is an RFC as I seek some guidance. The core changes to add new
instructions and CSRs are strightfwd however some of the peripheral
changes require some consensus:

1. single letter 'h' prefix in arch string (vs. existing multi-letter)
   This started because I couldn't coax the existing gcc driver to
   take a multi-letter h prefix. And while that was the initial motivation,
   following other discussion [2] it seems to make sense anyways.

2. I don't know how to generate the testsuite output files (.d) for the
   new files I added - does it start with copy/paste or does tooling
   exist to generate those.

Please review/comment !

Thx,
-Vineet

[1] https://wiki.riscv.org/display/TECH/Recently+Ratified+Extensions
[2] https://github.com/riscv/riscv-isa-manual/issues/781#issuecomment-983233088

Vineet Gupta (4):
  RISC-V: Hypervisor ext: Treat as "Standard" extension
  RISC-V: Hypervisor ext: CSR and Instructions
  RISC-V: Hypervisor ext: tests for new isns/csr and cleanup old csrs
  RISC-V: fix a comment for adding CSR entry and annotate switch-break

 bfd/elfxx-riscv.c                             |   5 +
 gas/config/tc-riscv.c                         |  15 ++-
 gas/testsuite/gas/riscv/csr-dw-regnums.d      |  10 --
 gas/testsuite/gas/riscv/csr-dw-regnums.s      |  10 --
 gas/testsuite/gas/riscv/h-ext-32.s            |  61 +++++++++
 gas/testsuite/gas/riscv/h-ext-64.s            |  68 ++++++++++
 .../gas/riscv/march-fail-single-prefix-h.d    |   3 -
 .../gas/riscv/march-fail-single-prefix.l      |   2 +-
 .../gas/riscv/priv-reg-fail-read-only-01.s    |  10 --
 .../gas/riscv/priv-reg-fail-version-1p10.l    |  10 --
 .../gas/riscv/priv-reg-fail-version-1p11.l    |  10 --
 .../gas/riscv/priv-reg-version-1p10.d         |  10 --
 .../gas/riscv/priv-reg-version-1p11.d         |  10 --
 .../gas/riscv/priv-reg-version-1p9p1.d        |  10 --
 gas/testsuite/gas/riscv/priv-reg.s            |  10 --
 include/opcode/riscv-opc.h                    | 123 ++++++++++++++----
 include/opcode/riscv.h                        |   1 +
 opcodes/riscv-opc.c                           |  23 +++-
 18 files changed, 264 insertions(+), 127 deletions(-)
 create mode 100644 gas/testsuite/gas/riscv/h-ext-32.s
 create mode 100644 gas/testsuite/gas/riscv/h-ext-64.s
 delete mode 100644 gas/testsuite/gas/riscv/march-fail-single-prefix-h.d

-- 
2.30.2


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

* [RFC 1/4] RISC-V: Hypervisor ext: Treat as "Standard" extension
  2021-12-16 17:33 [RFC 0/4] riscv/binutils support Hypervisor Extension Vineet Gupta
@ 2021-12-16 17:33 ` Vineet Gupta
  2021-12-17  4:10   ` Palmer Dabbelt
  2021-12-16 17:33 ` [RFC 2/4] RISC-V: Hypervisor ext: CSR and Instructions Vineet Gupta
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 17+ messages in thread
From: Vineet Gupta @ 2021-12-16 17:33 UTC (permalink / raw)
  To: Binutils, Nelson Chu; +Cc: Kito Cheng, jim.wilson.gcc, palmer, Vineet Gupta

gas currently fails for single-letter 'h' prefix in arch string.

| riscv64-unknown-elf-as -march=rv64gh empty.s
| Assembler messages:
| Error: rv64gh: unknown prefixed ISA extension `h'

It implements the "multi-letter" prefix for H-ext as recommended in
ISA manual subsection "ISA Extension Naming Conventions". However given
the discussions at [1] there seems to be no pressing need to keep version
for the current H-ext since
 - it will remain at 1.0
 - any future Hypervisor extensions wull called "Sh*" or some such
 - H was the original standard extension with its own MISA bit

This patch thus relaxes the constraint and allows single letter prefix
'h'

Note: To keep the binutils dejagnu failures same, I've fixed the
testsuite files within the same patch. I can break it up if reviewers
feel as such.

[1] https://github.com/riscv/riscv-isa-manual/issues/781#issuecomment-983233088

bfd/
	* elfxx-riscv.c (riscv_supported_std_ext): Add "h" entry.
	(riscv_supported_std_h_ext): Add "h" entry.
	(riscv_get_default_ext_version): Handle PRIV_SPEC_CLASS_DRAFT.
	(riscv_multi_subset_supports): Handle INSN_CLASS_H.

gas/
	* testsuite/gas/riscv/march-fail-single-prefix-h.d: Deleted as
	single-letter prefix 'h' is now valid.
	* testsuite/gas/riscv/march-fail-single-prefix-l: Removed 'h'.

include/
	* opcode/riscv.h (enum riscv_insn_class): Add INSN_CLASS_H.

Signed-off-by: Vineet Gupta <vineetg@rivosinc.com>
---
 bfd/elfxx-riscv.c                                    | 5 +++++
 gas/testsuite/gas/riscv/march-fail-single-prefix-h.d | 3 ---
 gas/testsuite/gas/riscv/march-fail-single-prefix.l   | 2 +-
 include/opcode/riscv.h                               | 1 +
 4 files changed, 7 insertions(+), 4 deletions(-)
 delete mode 100644 gas/testsuite/gas/riscv/march-fail-single-prefix-h.d

diff --git a/bfd/elfxx-riscv.c b/bfd/elfxx-riscv.c
index 3bd41ff2b551..7d5ae5a4657d 100644
--- a/bfd/elfxx-riscv.c
+++ b/bfd/elfxx-riscv.c
@@ -1173,6 +1173,7 @@ static struct riscv_supported_ext riscv_supported_std_ext[] =
   {"t",		ISA_SPEC_CLASS_NONE, RISCV_UNKNOWN_VERSION, RISCV_UNKNOWN_VERSION, 0 },
   {"p",		ISA_SPEC_CLASS_NONE, RISCV_UNKNOWN_VERSION, RISCV_UNKNOWN_VERSION, 0 },
   {"v",		ISA_SPEC_CLASS_DRAFT,		1, 0, 0 },
+  {"h",		PRIV_SPEC_CLASS_DRAFT,		1, 0, 0 },
   {"n",		ISA_SPEC_CLASS_NONE, RISCV_UNKNOWN_VERSION, RISCV_UNKNOWN_VERSION, 0 },
   {NULL, 0, 0, 0, 0}
 };
@@ -1232,6 +1233,7 @@ static struct riscv_supported_ext riscv_supported_std_s_ext[] =
 
 static struct riscv_supported_ext riscv_supported_std_h_ext[] =
 {
+  {"h",                 PRIV_SPEC_CLASS_DRAFT,          1, 0, 0 },
   {NULL, 0, 0, 0, 0}
 };
 
@@ -1531,6 +1533,7 @@ riscv_get_default_ext_version (enum riscv_spec_class *default_isa_spec,
     {
       if (strcmp (table[i].name, name) == 0
 	  && (table[i].isa_spec_class == ISA_SPEC_CLASS_DRAFT
+	      || table[i].isa_spec_class == PRIV_SPEC_CLASS_DRAFT
 	      || table[i].isa_spec_class == *default_isa_spec))
 	{
 	  *major_version = table[i].major_version;
@@ -2412,6 +2415,8 @@ riscv_multi_subset_supports (riscv_parse_subset_t *rps,
 	      || riscv_subset_supports (rps, "zve64d")
 	      || riscv_subset_supports (rps, "zve64f")
 	      || riscv_subset_supports (rps, "zve32f"));
+    case INSN_CLASS_H:
+      return riscv_subset_supports (rps, "h");
     default:
       rps->error_handler
         (_("internal: unreachable INSN_CLASS_*"));
diff --git a/gas/testsuite/gas/riscv/march-fail-single-prefix-h.d b/gas/testsuite/gas/riscv/march-fail-single-prefix-h.d
deleted file mode 100644
index eb101bd71353..000000000000
--- a/gas/testsuite/gas/riscv/march-fail-single-prefix-h.d
+++ /dev/null
@@ -1,3 +0,0 @@
-#as: -march=rv32ih
-#source: empty.s
-#error_output: march-fail-single-prefix.l
diff --git a/gas/testsuite/gas/riscv/march-fail-single-prefix.l b/gas/testsuite/gas/riscv/march-fail-single-prefix.l
index 13942ed66642..e4418de69acb 100644
--- a/gas/testsuite/gas/riscv/march-fail-single-prefix.l
+++ b/gas/testsuite/gas/riscv/march-fail-single-prefix.l
@@ -1,2 +1,2 @@
 .*Assembler messages:
-.*Error: .*unknown prefixed ISA extension `(s|h|z|x|zxm)'
+.*Error: .*unknown prefixed ISA extension `(s|z|x|zxm)'
diff --git a/include/opcode/riscv.h b/include/opcode/riscv.h
index 14889972abce..2783bdecaae5 100644
--- a/include/opcode/riscv.h
+++ b/include/opcode/riscv.h
@@ -387,6 +387,7 @@ enum riscv_insn_class
   INSN_CLASS_ZKND_OR_ZKNE,
   INSN_CLASS_V,
   INSN_CLASS_ZVEF,
+  INSN_CLASS_H,
 };
 
 /* This structure holds information for a particular instruction.  */
-- 
2.30.2


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

* [RFC 2/4] RISC-V: Hypervisor ext: CSR and Instructions
  2021-12-16 17:33 [RFC 0/4] riscv/binutils support Hypervisor Extension Vineet Gupta
  2021-12-16 17:33 ` [RFC 1/4] RISC-V: Hypervisor ext: Treat as "Standard" extension Vineet Gupta
@ 2021-12-16 17:33 ` Vineet Gupta
  2021-12-17  4:10   ` Palmer Dabbelt
  2021-12-16 17:33 ` [RFC 3/4] RISC-V: Hypervisor ext: tests for new isns/csr and cleanup old csrs Vineet Gupta
  2021-12-16 17:33 ` [RFC 4/4] RISC-V: fix a comment for adding CSR entry and annotate switch-break Vineet Gupta
  3 siblings, 1 reply; 17+ messages in thread
From: Vineet Gupta @ 2021-12-16 17:33 UTC (permalink / raw)
  To: Binutils, Nelson Chu; +Cc: Kito Cheng, jim.wilson.gcc, palmer, Vineet Gupta

This implements Hypervisor Extension 1.0 [1]

 - Hypervisor Memory-Management Instructions
   HFENCE.VVMA, HFENCE.GVMA,
   HINVAL.GVMA, HINVAL.VVMA

 - Hypervisor Virtual Machine Load and Store Instructions
   HLV.B, HLV.BU,          HSV.B,
   HLV.H, HLV.HU, HLVX.HU, HSB.H,
   HLV.W, HLV.WU, HLVX.WU, HSV.W,
   HLV.D,                  HSV.D

 - Hypervisor CSRs (some new, some address changed)
   hstatus, hedeleg, hideleg, hie, hcounteren, hgeie, htval, hip, hvip,
   htinst, hgeip, henvcfg, henvcfgh, hgatp, hcontext, htimedelta, htimedeltah,
   vsstatus, vsie, vstvec, vsscratch, vsepc, vscause, vstval, vsip, vsatp,

This removed a couple of stale instructions: HRET, DRET.

[1] https://github.com/riscv/riscv-isa-manual/releases/tag/draft-20211213-ea38395

gas/
	* config/tc-riscv.c (enum riscv_csr_class): Added CSR_CLASS_H.
	(riscv_csr_address): check CSR_CLASS_H.

include/
	* opcode/riscv-opc.h: Removed {MATCH,MASK}_{D,H}RET.
	Added {MATCH,MASK}_{HLV*,HSV*,HFENCE*,HINVAL*}.
	Added/updated CSR_H*, CSR_V*.
	DECLARE_INSN(hlv*), DECLARE_INSN(hsv*).
	DECLARE_INSN(hfence*), DECLARE_INSN(hinval*).
	DECLARE_CSR(h*), DECLARE_CSR(v*).

opcodes/
	* riscv-opc.c (riscv_opcodes): Removed {d,h}ret.
	Added hlv*, hsv*, hfence*, hinval*.

Signed-off-by: Vineet Gupta <vineetg@rivosinc.com>
---
 gas/config/tc-riscv.c      |   5 ++
 include/opcode/riscv-opc.h | 123 +++++++++++++++++++++++++++++--------
 opcodes/riscv-opc.c        |  23 ++++++-
 3 files changed, 123 insertions(+), 28 deletions(-)

diff --git a/gas/config/tc-riscv.c b/gas/config/tc-riscv.c
index e8061217e7cd..e28818b4fd15 100644
--- a/gas/config/tc-riscv.c
+++ b/gas/config/tc-riscv.c
@@ -65,6 +65,7 @@ enum riscv_csr_class
   CSR_CLASS_F,		/* f-ext only */
   CSR_CLASS_ZKR,	/* zkr only */
   CSR_CLASS_V,		/* rvv only */
+  CSR_CLASS_H,		/* hypervisor extension only */
   CSR_CLASS_DEBUG	/* debug CSR */
 };
 
@@ -905,6 +906,10 @@ riscv_csr_address (const char *csr_name,
       result = riscv_subset_supports (&riscv_rps_as, "v");
       need_check_version = false;
       break;
+    case CSR_CLASS_H:
+      result = riscv_subset_supports (&riscv_rps_as, "h");
+      need_check_version = false;
+      break;
     case CSR_CLASS_DEBUG:
       need_check_version = false;
       break;
diff --git a/include/opcode/riscv-opc.h b/include/opcode/riscv-opc.h
index 41c8ef163c42..542f875d051d 100644
--- a/include/opcode/riscv-opc.h
+++ b/include/opcode/riscv-opc.h
@@ -243,12 +243,8 @@
 #define MASK_URET  0xffffffff
 #define MATCH_SRET 0x10200073
 #define MASK_SRET  0xffffffff
-#define MATCH_HRET 0x20200073
-#define MASK_HRET  0xffffffff
 #define MATCH_MRET 0x30200073
 #define MASK_MRET  0xffffffff
-#define MATCH_DRET 0x7b200073
-#define MASK_DRET  0xffffffff
 #define MATCH_SFENCE_VM 0x10400073
 #define MASK_SFENCE_VM  0xfff07fff
 #define MATCH_SFENCE_VMA 0x12000073
@@ -1987,6 +1983,32 @@
 #define MASK_VDOTUVV  0xfc00707f
 #define MATCH_VFDOTVV  0xe4001057
 #define MASK_VFDOTVV  0xfc00707f
+
+#define MASK_HLV      0xfff0707f
+#define MATCH_HLVB    0x60004073
+#define MATCH_HLVBU   0x60104073
+#define MATCH_HLVH    0x64004073
+#define MATCH_HLVHU   0x64104073
+#define MATCH_HLVXHU  0x64304073
+#define MATCH_HLVW    0x68004073
+#define MATCH_HLVXWU  0x68304073
+#define MATCH_HLVWU   0x68104073
+#define MATCH_HLVD    0x6c004073
+
+#define MASK_HSV      0xfe007fff
+#define MATCH_HSVB    0x62004073
+#define MATCH_HSVH    0x66004073
+#define MATCH_HSVW    0x6a004073
+#define MATCH_HSVD    0x6e004073
+
+#define MASK_HFENCE       0xfe007fff
+#define MATCH_HFENCE_VVMA 0x22000073
+#define MATCH_HFENCE_GVMA 0x62000073
+
+#define MASK_HINVAL       0xfe007fff
+#define MATCH_HINVAL_VVMA 0x26000073
+#define MATCH_HINVAL_GVMA 0x66000073
+
 /* Privileged CSR addresses.  */
 #define CSR_USTATUS 0x0
 #define CSR_UIE 0x4
@@ -2200,16 +2222,32 @@
 #define CSR_MHPMEVENT29 0x33d
 #define CSR_MHPMEVENT30 0x33e
 #define CSR_MHPMEVENT31 0x33f
-#define CSR_HSTATUS 0x200
-#define CSR_HEDELEG 0x202
-#define CSR_HIDELEG 0x203
-#define CSR_HIE 0x204
-#define CSR_HTVEC 0x205
-#define CSR_HSCRATCH 0x240
-#define CSR_HEPC 0x241
-#define CSR_HCAUSE 0x242
-#define CSR_HBADADDR 0x243
-#define CSR_HIP 0x244
+#define CSR_HSTATUS     0x600
+#define CSR_HEDELEG     0x602
+#define CSR_HIDELEG     0x603
+#define CSR_HIE         0x604
+#define CSR_HCOUNTEREN  0x606
+#define CSR_HGEIE       0x607
+#define CSR_HTVAL       0x643
+#define CSR_HIP         0x644
+#define CSR_HVIP        0x645
+#define CSR_HTINST      0x64a
+#define CSR_HGEIP       0xe12
+#define CSR_HENVCFG     0x60a
+#define CSR_HENVCFGH    0x61a
+#define CSR_HGATP       0x680
+#define CSR_HCONTEXT    0x6a8
+#define CSR_HTIMEDELTA  0x605
+#define CSR_HTIMEDELTAH 0x615
+#define CSR_VSSTATUS    0x200
+#define CSR_VSIE        0x204
+#define CSR_VSTVEC      0x205
+#define CSR_VSSCRATCH   0x240
+#define CSR_VSEPC       0x241
+#define CSR_VSCAUSE     0x242
+#define CSR_VSTVAL      0x243
+#define CSR_VSIP        0x244
+#define CSR_VSATP       0x280
 #define CSR_MBASE 0x380
 #define CSR_MBOUND 0x381
 #define CSR_MIBASE 0x382
@@ -2354,9 +2392,7 @@ DECLARE_INSN(ecall, MATCH_ECALL, MASK_ECALL)
 DECLARE_INSN(ebreak, MATCH_EBREAK, MASK_EBREAK)
 DECLARE_INSN(uret, MATCH_URET, MASK_URET)
 DECLARE_INSN(sret, MATCH_SRET, MASK_SRET)
-DECLARE_INSN(hret, MATCH_HRET, MASK_HRET)
 DECLARE_INSN(mret, MATCH_MRET, MASK_MRET)
-DECLARE_INSN(dret, MATCH_DRET, MASK_DRET)
 DECLARE_INSN(sfence_vm, MATCH_SFENCE_VM, MASK_SFENCE_VM)
 DECLARE_INSN(sfence_vma, MATCH_SFENCE_VMA, MASK_SFENCE_VMA)
 DECLARE_INSN(wfi, MATCH_WFI, MASK_WFI)
@@ -2549,6 +2585,24 @@ DECLARE_INSN(c_sd, MATCH_C_SD, MASK_C_SD)
 DECLARE_INSN(c_addiw, MATCH_C_ADDIW, MASK_C_ADDIW)
 DECLARE_INSN(c_ldsp, MATCH_C_LDSP, MASK_C_LDSP)
 DECLARE_INSN(c_sdsp, MATCH_C_SDSP, MASK_C_SDSP)
+/* Hypervisor instructions.  */
+DECLARE_INSN(hlv_b,   MATCH_HLVB,   MASK_HLV)
+DECLARE_INSN(hlv_h,   MATCH_HLVH,   MASK_HLV)
+DECLARE_INSN(hlv_w,   MATCH_HLVW,   MASK_HLV)
+DECLARE_INSN(hlv_d,   MATCH_HLVD,   MASK_HLV)
+DECLARE_INSN(hlv_bu,  MATCH_HLVBU,  MASK_HLV)
+DECLARE_INSN(hlv_hu,  MATCH_HLVHU,  MASK_HLV)
+DECLARE_INSN(hlv_wu,  MATCH_HLVWU,  MASK_HLV)
+DECLARE_INSN(hlvx_hu, MATCH_HLVXHU, MASK_HLV)
+DECLARE_INSN(hlvx_wu, MATCH_HLVXWU, MASK_HLV)
+DECLARE_INSN(hsv_b,   MATCH_HSVB,   MASK_HSV)
+DECLARE_INSN(hsv_h,   MATCH_HSVH,   MASK_HSV)
+DECLARE_INSN(hsv_w,   MATCH_HSVW,   MASK_HSV)
+DECLARE_INSN(hsv_d,   MATCH_HSVD,   MASK_HSV)
+DECLARE_INSN(hfence_vvma, MATCH_HFENCE_VVMA, MASK_HFENCE)
+DECLARE_INSN(hfence_gvma, MATCH_HFENCE_GVMA, MASK_HFENCE)
+DECLARE_INSN(hinval_vvma, MATCH_HINVAL_VVMA, MASK_HINVAL)
+DECLARE_INSN(hinval_gvma, MATCH_HINVAL_GVMA, MASK_HINVAL)
 #endif /* DECLARE_INSN */
 #ifdef DECLARE_CSR
 /* Privileged CSRs.  */
@@ -2764,17 +2818,34 @@ DECLARE_CSR(mhpmevent28, CSR_MHPMEVENT28, CSR_CLASS_I, PRIV_SPEC_CLASS_1P9P1, PR
 DECLARE_CSR(mhpmevent29, CSR_MHPMEVENT29, CSR_CLASS_I, PRIV_SPEC_CLASS_1P9P1, PRIV_SPEC_CLASS_DRAFT)
 DECLARE_CSR(mhpmevent30, CSR_MHPMEVENT30, CSR_CLASS_I, PRIV_SPEC_CLASS_1P9P1, PRIV_SPEC_CLASS_DRAFT)
 DECLARE_CSR(mhpmevent31, CSR_MHPMEVENT31, CSR_CLASS_I, PRIV_SPEC_CLASS_1P9P1, PRIV_SPEC_CLASS_DRAFT)
+
+DECLARE_CSR(hstatus,     CSR_HSTATUS,     CSR_CLASS_H, PRIV_SPEC_CLASS_1P9P1, PRIV_SPEC_CLASS_DRAFT)
+DECLARE_CSR(hedeleg,     CSR_HEDELEG,     CSR_CLASS_H, PRIV_SPEC_CLASS_1P9P1, PRIV_SPEC_CLASS_DRAFT)
+DECLARE_CSR(hideleg,     CSR_HIDELEG,     CSR_CLASS_H, PRIV_SPEC_CLASS_1P9P1, PRIV_SPEC_CLASS_DRAFT)
+DECLARE_CSR(hie,         CSR_HIE,         CSR_CLASS_H, PRIV_SPEC_CLASS_1P9P1, PRIV_SPEC_CLASS_DRAFT)
+DECLARE_CSR(hcounteren,  CSR_HCOUNTEREN,  CSR_CLASS_H, PRIV_SPEC_CLASS_1P9P1, PRIV_SPEC_CLASS_DRAFT)
+DECLARE_CSR(hgeie,       CSR_HGEIE,       CSR_CLASS_H, PRIV_SPEC_CLASS_1P9P1, PRIV_SPEC_CLASS_DRAFT)
+DECLARE_CSR(htval,       CSR_HTVAL,       CSR_CLASS_H, PRIV_SPEC_CLASS_1P9P1, PRIV_SPEC_CLASS_DRAFT)
+DECLARE_CSR(hip,         CSR_HIP,         CSR_CLASS_H, PRIV_SPEC_CLASS_1P9P1, PRIV_SPEC_CLASS_DRAFT)
+DECLARE_CSR(hvip,        CSR_HVIP,        CSR_CLASS_H, PRIV_SPEC_CLASS_1P9P1, PRIV_SPEC_CLASS_DRAFT)
+DECLARE_CSR(htinst,      CSR_HTINST,      CSR_CLASS_H, PRIV_SPEC_CLASS_1P9P1, PRIV_SPEC_CLASS_DRAFT)
+DECLARE_CSR(hgeip,       CSR_HGEIP,       CSR_CLASS_H, PRIV_SPEC_CLASS_1P9P1, PRIV_SPEC_CLASS_DRAFT)
+DECLARE_CSR(henvcfg,     CSR_HENVCFG,     CSR_CLASS_H, PRIV_SPEC_CLASS_1P9P1, PRIV_SPEC_CLASS_DRAFT)
+DECLARE_CSR(henvcfgh,    CSR_HENVCFGH,    CSR_CLASS_H, PRIV_SPEC_CLASS_1P9P1, PRIV_SPEC_CLASS_DRAFT)
+DECLARE_CSR(hgatp,       CSR_HGATP,       CSR_CLASS_H, PRIV_SPEC_CLASS_1P9P1, PRIV_SPEC_CLASS_DRAFT)
+DECLARE_CSR(hcontext,    CSR_HCONTEXT,    CSR_CLASS_H, PRIV_SPEC_CLASS_1P9P1, PRIV_SPEC_CLASS_DRAFT)
+DECLARE_CSR(htimedelta,  CSR_HTIMEDELTA,  CSR_CLASS_H, PRIV_SPEC_CLASS_1P9P1, PRIV_SPEC_CLASS_DRAFT)
+DECLARE_CSR(htimedeltah, CSR_HTIMEDELTAH, CSR_CLASS_H, PRIV_SPEC_CLASS_1P9P1, PRIV_SPEC_CLASS_DRAFT)
+DECLARE_CSR(vsstatus,    CSR_VSSTATUS,    CSR_CLASS_H, PRIV_SPEC_CLASS_1P9P1, PRIV_SPEC_CLASS_DRAFT)
+DECLARE_CSR(vsie,        CSR_VSIE,        CSR_CLASS_H, PRIV_SPEC_CLASS_1P9P1, PRIV_SPEC_CLASS_DRAFT)
+DECLARE_CSR(vstvec,      CSR_VSTVEC,      CSR_CLASS_H, PRIV_SPEC_CLASS_1P9P1, PRIV_SPEC_CLASS_DRAFT)
+DECLARE_CSR(vsscratch,   CSR_VSSCRATCH,   CSR_CLASS_H, PRIV_SPEC_CLASS_1P9P1, PRIV_SPEC_CLASS_DRAFT)
+DECLARE_CSR(vsepc,       CSR_VSEPC,       CSR_CLASS_H, PRIV_SPEC_CLASS_1P9P1, PRIV_SPEC_CLASS_DRAFT)
+DECLARE_CSR(vscause,     CSR_VSCAUSE,     CSR_CLASS_H, PRIV_SPEC_CLASS_1P9P1, PRIV_SPEC_CLASS_DRAFT)
+DECLARE_CSR(vstval,      CSR_VSTVAL,      CSR_CLASS_H, PRIV_SPEC_CLASS_1P9P1, PRIV_SPEC_CLASS_DRAFT)
+DECLARE_CSR(vsip,        CSR_VSIP,        CSR_CLASS_H, PRIV_SPEC_CLASS_1P9P1, PRIV_SPEC_CLASS_DRAFT)
+DECLARE_CSR(vsatp,       CSR_VSATP,       CSR_CLASS_H, PRIV_SPEC_CLASS_1P9P1, PRIV_SPEC_CLASS_DRAFT)
 /* Dropped CSRs.  */
-DECLARE_CSR(hstatus, CSR_HSTATUS, CSR_CLASS_I, PRIV_SPEC_CLASS_1P9P1, PRIV_SPEC_CLASS_1P10)
-DECLARE_CSR(hedeleg, CSR_HEDELEG, CSR_CLASS_I, PRIV_SPEC_CLASS_1P9P1, PRIV_SPEC_CLASS_1P10)
-DECLARE_CSR(hideleg, CSR_HIDELEG, CSR_CLASS_I, PRIV_SPEC_CLASS_1P9P1, PRIV_SPEC_CLASS_1P10)
-DECLARE_CSR(hie, CSR_HIE, CSR_CLASS_I, PRIV_SPEC_CLASS_1P9P1, PRIV_SPEC_CLASS_1P10)
-DECLARE_CSR(htvec, CSR_HTVEC, CSR_CLASS_I, PRIV_SPEC_CLASS_1P9P1, PRIV_SPEC_CLASS_1P10)
-DECLARE_CSR(hscratch, CSR_HSCRATCH, CSR_CLASS_I, PRIV_SPEC_CLASS_1P9P1, PRIV_SPEC_CLASS_1P10)
-DECLARE_CSR(hepc, CSR_HEPC, CSR_CLASS_I, PRIV_SPEC_CLASS_1P9P1, PRIV_SPEC_CLASS_1P10)
-DECLARE_CSR(hcause, CSR_HCAUSE, CSR_CLASS_I, PRIV_SPEC_CLASS_1P9P1, PRIV_SPEC_CLASS_1P10)
-DECLARE_CSR(hbadaddr, CSR_HBADADDR, CSR_CLASS_I, PRIV_SPEC_CLASS_1P9P1, PRIV_SPEC_CLASS_1P10)
-DECLARE_CSR(hip, CSR_HIP, CSR_CLASS_I, PRIV_SPEC_CLASS_1P9P1, PRIV_SPEC_CLASS_1P10)
 DECLARE_CSR(mbase, CSR_MBASE, CSR_CLASS_I, PRIV_SPEC_CLASS_1P9P1, PRIV_SPEC_CLASS_1P10)
 DECLARE_CSR(mbound, CSR_MBOUND, CSR_CLASS_I, PRIV_SPEC_CLASS_1P9P1, PRIV_SPEC_CLASS_1P10)
 DECLARE_CSR(mibase, CSR_MIBASE, CSR_CLASS_I, PRIV_SPEC_CLASS_1P9P1, PRIV_SPEC_CLASS_1P10)
diff --git a/opcodes/riscv-opc.c b/opcodes/riscv-opc.c
index 40037db83c05..794eee2d413f 100644
--- a/opcodes/riscv-opc.c
+++ b/opcodes/riscv-opc.c
@@ -839,9 +839,7 @@ const struct riscv_opcode riscv_opcodes[] =
 {"csrrc",      0, INSN_CLASS_ZICSR,"d,E,Z",    MATCH_CSRRCI, MASK_CSRRCI, match_opcode, INSN_ALIAS },
 {"uret",       0, INSN_CLASS_I,    "",         MATCH_URET, MASK_URET, match_opcode, 0 },
 {"sret",       0, INSN_CLASS_I,    "",         MATCH_SRET, MASK_SRET, match_opcode, 0 },
-{"hret",       0, INSN_CLASS_I,    "",         MATCH_HRET, MASK_HRET, match_opcode, 0 },
 {"mret",       0, INSN_CLASS_I,    "",         MATCH_MRET, MASK_MRET, match_opcode, 0 },
-{"dret",       0, INSN_CLASS_I,    "",         MATCH_DRET, MASK_DRET, match_opcode, 0 },
 {"sfence.vm",  0, INSN_CLASS_I,    "",         MATCH_SFENCE_VM, MASK_SFENCE_VM | MASK_RS1, match_opcode, 0 },
 {"sfence.vm",  0, INSN_CLASS_I,    "s",        MATCH_SFENCE_VM, MASK_SFENCE_VM, match_opcode, 0 },
 {"sfence.vma", 0, INSN_CLASS_I,    "",         MATCH_SFENCE_VMA, MASK_SFENCE_VMA|MASK_RS1|MASK_RS2, match_opcode, INSN_ALIAS },
@@ -1725,6 +1723,27 @@ const struct riscv_opcode riscv_opcodes[] =
 {"vmv4r.v",    0, INSN_CLASS_V, "Vd,Vt", MATCH_VMV4RV, MASK_VMV4RV, match_opcode, 0},
 {"vmv8r.v",    0, INSN_CLASS_V, "Vd,Vt", MATCH_VMV8RV, MASK_VMV8RV, match_opcode, 0},
 
+/* Hypervisor instructions.  */
+{"hlv.b",       0, INSN_CLASS_H, "d,(s)", MATCH_HLVB,   MASK_HLV, match_opcode, INSN_DREF|INSN_1_BYTE },
+{"hlv.bu",      0, INSN_CLASS_H, "d,(s)", MATCH_HLVBU,  MASK_HLV, match_opcode, INSN_DREF|INSN_1_BYTE },
+{"hlv.h",       0, INSN_CLASS_H, "d,(s)", MATCH_HLVH,   MASK_HLV, match_opcode, INSN_DREF|INSN_2_BYTE },
+{"hlv.hu",      0, INSN_CLASS_H, "d,(s)", MATCH_HLVHU,  MASK_HLV, match_opcode, INSN_DREF|INSN_2_BYTE },
+{"hlvx.hu",     0, INSN_CLASS_H, "d,(s)", MATCH_HLVXHU, MASK_HLV, match_opcode, INSN_DREF|INSN_2_BYTE },
+{"hlv.w",       0, INSN_CLASS_H, "d,(s)", MATCH_HLVW,   MASK_HLV, match_opcode, INSN_DREF|INSN_4_BYTE },
+{"hlv.wu",     64, INSN_CLASS_H, "d,(s)", MATCH_HLVWU,  MASK_HLV, match_opcode, INSN_DREF|INSN_4_BYTE },
+{"hlvx.wu",     0, INSN_CLASS_H, "d,(s)", MATCH_HLVXWU, MASK_HLV, match_opcode, INSN_DREF|INSN_4_BYTE },
+{"hlv.d",      64, INSN_CLASS_H, "d,(s)", MATCH_HLVD,   MASK_HLV, match_opcode, INSN_DREF|INSN_8_BYTE },
+
+{"hsv.b",       0, INSN_CLASS_H, "t,(s)", MATCH_HSVB,   MASK_HSV, match_opcode, INSN_DREF|INSN_1_BYTE },
+{"hsv.h",       0, INSN_CLASS_H, "t,(s)", MATCH_HSVH,   MASK_HSV, match_opcode, INSN_DREF|INSN_2_BYTE },
+{"hsv.w",       0, INSN_CLASS_H, "t,(s)", MATCH_HSVW,   MASK_HSV, match_opcode, INSN_DREF|INSN_4_BYTE },
+{"hsv.d",      64, INSN_CLASS_H, "t,(s)", MATCH_HSVD,   MASK_HSV, match_opcode, INSN_DREF|INSN_8_BYTE },
+
+{"hfence.vvma", 0, INSN_CLASS_H, "t,s", MATCH_HFENCE_VVMA, MASK_HFENCE, match_opcode, 0 },
+{"hfence.gvma", 0, INSN_CLASS_H, "t,s", MATCH_HFENCE_GVMA, MASK_HFENCE, match_opcode, 0 },
+{"hinval.vvma", 0, INSN_CLASS_H, "t,s", MATCH_HINVAL_VVMA, MASK_HINVAL, match_opcode, 0 },
+{"hinval.gvma", 0, INSN_CLASS_H, "t,s", MATCH_HINVAL_GVMA, MASK_HINVAL, match_opcode, 0 },
+
 /* Terminate the list.  */
 {0, 0, INSN_CLASS_NONE, 0, 0, 0, 0, 0}
 };
-- 
2.30.2


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

* [RFC 3/4] RISC-V: Hypervisor ext: tests for new isns/csr and cleanup old csrs
  2021-12-16 17:33 [RFC 0/4] riscv/binutils support Hypervisor Extension Vineet Gupta
  2021-12-16 17:33 ` [RFC 1/4] RISC-V: Hypervisor ext: Treat as "Standard" extension Vineet Gupta
  2021-12-16 17:33 ` [RFC 2/4] RISC-V: Hypervisor ext: CSR and Instructions Vineet Gupta
@ 2021-12-16 17:33 ` Vineet Gupta
  2021-12-17  4:10   ` Palmer Dabbelt
  2021-12-21  8:17   ` Jan Beulich
  2021-12-16 17:33 ` [RFC 4/4] RISC-V: fix a comment for adding CSR entry and annotate switch-break Vineet Gupta
  3 siblings, 2 replies; 17+ messages in thread
From: Vineet Gupta @ 2021-12-16 17:33 UTC (permalink / raw)
  To: Binutils, Nelson Chu; +Cc: Kito Cheng, jim.wilson.gcc, palmer, Vineet Gupta

Since the prior versions were not fully supported (and or functional
even from ISA pov), we just remove any old baggage - this avoids the
mess in keeping csr with same names but different addresses etc.

Signed-off-by: Vineet Gupta <vineetg@rivosinc.com>
---
 gas/testsuite/gas/riscv/csr-dw-regnums.d      | 10 ---
 gas/testsuite/gas/riscv/csr-dw-regnums.s      | 10 ---
 gas/testsuite/gas/riscv/h-ext-32.s            | 61 +++++++++++++++++
 gas/testsuite/gas/riscv/h-ext-64.s            | 68 +++++++++++++++++++
 .../gas/riscv/priv-reg-fail-read-only-01.s    | 10 ---
 .../gas/riscv/priv-reg-fail-version-1p10.l    | 10 ---
 .../gas/riscv/priv-reg-fail-version-1p11.l    | 10 ---
 .../gas/riscv/priv-reg-version-1p10.d         | 10 ---
 .../gas/riscv/priv-reg-version-1p11.d         | 10 ---
 .../gas/riscv/priv-reg-version-1p9p1.d        | 10 ---
 gas/testsuite/gas/riscv/priv-reg.s            | 10 ---
 11 files changed, 129 insertions(+), 90 deletions(-)
 create mode 100644 gas/testsuite/gas/riscv/h-ext-32.s
 create mode 100644 gas/testsuite/gas/riscv/h-ext-64.s

diff --git a/gas/testsuite/gas/riscv/csr-dw-regnums.d b/gas/testsuite/gas/riscv/csr-dw-regnums.d
index de17ad81fbde..ea0a445c39ce 100644
--- a/gas/testsuite/gas/riscv/csr-dw-regnums.d
+++ b/gas/testsuite/gas/riscv/csr-dw-regnums.d
@@ -229,16 +229,6 @@ Contents of the .* section:
   DW_CFA_offset_extended_sf: r4925 \(mhpmevent29\) at cfa\+3316
   DW_CFA_offset_extended_sf: r4926 \(mhpmevent30\) at cfa\+3320
   DW_CFA_offset_extended_sf: r4927 \(mhpmevent31\) at cfa\+3324
-  DW_CFA_offset_extended_sf: r4608 \(hstatus\) at cfa\+2048
-  DW_CFA_offset_extended_sf: r4610 \(hedeleg\) at cfa\+2056
-  DW_CFA_offset_extended_sf: r4611 \(hideleg\) at cfa\+2060
-  DW_CFA_offset_extended_sf: r4612 \(hie\) at cfa\+2064
-  DW_CFA_offset_extended_sf: r4613 \(htvec\) at cfa\+2068
-  DW_CFA_offset_extended_sf: r4672 \(hscratch\) at cfa\+2304
-  DW_CFA_offset_extended_sf: r4673 \(hepc\) at cfa\+2308
-  DW_CFA_offset_extended_sf: r4674 \(hcause\) at cfa\+2312
-  DW_CFA_offset_extended_sf: r4675 \(hbadaddr\) at cfa\+2316
-  DW_CFA_offset_extended_sf: r4676 \(hip\) at cfa\+2320
   DW_CFA_offset_extended_sf: r4992 \(mbase\) at cfa\+3584
   DW_CFA_offset_extended_sf: r4993 \(mbound\) at cfa\+3588
   DW_CFA_offset_extended_sf: r4994 \(mibase\) at cfa\+3592
diff --git a/gas/testsuite/gas/riscv/csr-dw-regnums.s b/gas/testsuite/gas/riscv/csr-dw-regnums.s
index ecc801469763..549475d650e8 100644
--- a/gas/testsuite/gas/riscv/csr-dw-regnums.s
+++ b/gas/testsuite/gas/riscv/csr-dw-regnums.s
@@ -220,16 +220,6 @@ _start:
 	.cfi_offset mhpmevent30, 3320
 	.cfi_offset mhpmevent31, 3324
 	# dropped
-	.cfi_offset hstatus, 2048
-	.cfi_offset hedeleg, 2056
-	.cfi_offset hideleg, 2060
-	.cfi_offset hie, 2064
-	.cfi_offset htvec, 2068
-	.cfi_offset hscratch, 2304
-	.cfi_offset hepc, 2308
-	.cfi_offset hcause, 2312
-	.cfi_offset hbadaddr, 2316
-	.cfi_offset hip, 2320
 	.cfi_offset mbase, 3584
 	.cfi_offset mbound, 3588
 	.cfi_offset mibase, 3592
diff --git a/gas/testsuite/gas/riscv/h-ext-32.s b/gas/testsuite/gas/riscv/h-ext-32.s
new file mode 100644
index 000000000000..2818d7d17381
--- /dev/null
+++ b/gas/testsuite/gas/riscv/h-ext-32.s
@@ -0,0 +1,61 @@
+target:
+	hlv.b   a0, (a1)
+	hlv.bu  a0, (a1)
+	hlv.h   a1, (a2)
+	hlv.hu  a1, (a1)
+	hlvx.hu a1, (a2)
+	hlv.w   a2, (a2)
+	hlvx.wu a2, (a3)
+
+	hsv.b   a0, (a1)
+	hsv.h   a0, (a1)
+	hsv.w   a0, (a1)
+
+	csrr    a0, hstatus
+	csrw    hstatus, a1
+	csrr    a0, hedeleg
+	csrw    hedeleg, a1
+	csrr    a0, hideleg
+	csrw    hideleg, a1
+	csrr    a0, hie
+	csrw    hie, a1
+	csrr    a0, hcounteren
+	csrw    hcounteren, a1
+	csrr    a0, hgeie
+	csrw    hgeie, a1
+	csrr    a0, htval
+	csrw    htval, a1
+	csrr    a0, hip
+	csrw    hip, a1
+	csrr    a0, hvip
+	csrw    hvip, a1
+	csrr    a0, htinst
+	csrw    htinst, a1
+	csrr    a0, hgeip
+	csrw    hgeip, a1
+	csrr    a0, henvcfg
+	csrw    henvcfg, a1
+	csrr    a0, hgatp
+	csrw    hgatp, a1
+	csrr    a0, hcontext
+	csrw    hcontext, a1
+	csrr    a0, htimedelta
+	csrw    htimedelta, a1
+	csrr    a0, vsstatus
+	csrw    vsstatus, a1
+	csrr    a0, vsie
+	csrw    vsie, a1
+	csrr    a0, vstvec
+	csrw    vstvec, a1
+	csrr    a0, vsscratch
+	csrw    vsscratch, a1
+	csrr    a0, vsepc
+	csrw    vsepc, a1
+	csrr    a0, vscause
+	csrw    vscause, a1
+	csrr    a0, vstval
+	csrw    vstval, a1
+	csrr    a0, vsip
+	csrw    vsip, a1
+	csrr    a0, vsatp
+	csrw    vsatp, a1
diff --git a/gas/testsuite/gas/riscv/h-ext-64.s b/gas/testsuite/gas/riscv/h-ext-64.s
new file mode 100644
index 000000000000..25455fd4fdca
--- /dev/null
+++ b/gas/testsuite/gas/riscv/h-ext-64.s
@@ -0,0 +1,68 @@
+target:
+	hlv.b   a0, (a1)
+	hlv.bu  a0, (a1)
+	hlv.h   a1, (a2)
+	hlv.hu  a1, (a1)
+	hlvx.hu a1, (a2)
+	hlv.w   a2, (a2)
+	hlv.wu  a2, (a3)
+	hlvx.wu a2, (a3)
+	hlv.d   a3, (a4)
+
+	hsv.b   a0, (a1)
+	hsv.h   a0, (a1)
+	hsv.w   a0, (a1)
+	hsv.d   a0, (a1)
+
+	csrr    a0, hstatus
+	csrw    hstatus, a1
+	csrr    a0, hedeleg
+	csrw    hedeleg, a1
+	csrr    a0, hideleg
+	csrw    hideleg, a1
+	csrr    a0, hie
+	csrw    hie, a1
+	csrr    a0, hcounteren
+	csrw    hcounteren, a1
+	csrr    a0, hgeie
+	csrw    hgeie, a1
+	csrr    a0, htval
+	csrw    htval, a1
+	csrr    a0, hip
+	csrw    hip, a1
+	csrr    a0, hvip
+	csrw    hvip, a1
+	csrr    a0, htinst
+	csrw    htinst, a1
+	csrr    a0, hgeip
+	csrw    hgeip, a1
+	csrr    a0, henvcfg
+	csrw    henvcfg, a1
+	csrr    a0, henvcfgh
+	csrw    henvcfgh, a1
+	csrr    a0, hgatp
+	csrw    hgatp, a1
+	csrr    a0, hcontext
+	csrw    hcontext, a1
+	csrr    a0, htimedelta
+	csrw    htimedelta, a1
+	csrr    a0, htimedeltah
+	csrw    htimedeltah, a1
+	csrr    a0, vsstatus
+	csrw    vsstatus, a1
+	csrr    a0, vsie
+	csrw    vsie, a1
+	csrr    a0, vstvec
+	csrw    vstvec, a1
+	csrr    a0, vsscratch
+	csrw    vsscratch, a1
+	csrr    a0, vsepc
+	csrw    vsepc, a1
+	csrr    a0, vscause
+	csrw    vscause, a1
+	csrr    a0, vstval
+	csrw    vstval, a1
+	csrr    a0, vsip
+	csrw    vsip, a1
+	csrr    a0, vsatp
+	csrw    vsatp, a1
diff --git a/gas/testsuite/gas/riscv/priv-reg-fail-read-only-01.s b/gas/testsuite/gas/riscv/priv-reg-fail-read-only-01.s
index af0fc4e14a42..aff34e9a3470 100644
--- a/gas/testsuite/gas/riscv/priv-reg-fail-read-only-01.s
+++ b/gas/testsuite/gas/riscv/priv-reg-fail-read-only-01.s
@@ -249,16 +249,6 @@
 	csr mucounteren         # 0x320 in 1.9.1, dropped in 1.10, but the value is mcountinhibit since 1.11
 	csr dscratch            # 0x7b2 in 1.10,  but the value is dscratch0 since 1.11
 
-	csr hstatus             # 0x200, dropped in 1.10
-	csr hedeleg             # 0x202, dropped in 1.10
-	csr hideleg             # 0x203, dropped in 1.10
-	csr hie                 # 0x204, dropped in 1.10
-	csr htvec               # 0x205, dropped in 1.10
-	csr hscratch            # 0x240, dropped in 1.10
-	csr hepc                # 0x241, dropped in 1.10
-	csr hcause              # 0x242, dropped in 1.10
-	csr hbadaddr            # 0x243, dropped in 1.10
-	csr hip                 # 0x244, dropped in 1.10
 	csr mbase               # 0x380, dropped in 1.10
 	csr mbound              # 0x381, dropped in 1.10
 	csr mibase              # 0x382, dropped in 1.10
diff --git a/gas/testsuite/gas/riscv/priv-reg-fail-version-1p10.l b/gas/testsuite/gas/riscv/priv-reg-fail-version-1p10.l
index fbba6e525670..5f7a8d60d7dd 100644
--- a/gas/testsuite/gas/riscv/priv-reg-fail-version-1p10.l
+++ b/gas/testsuite/gas/riscv/priv-reg-fail-version-1p10.l
@@ -5,16 +5,6 @@
 .*Warning: invalid CSR `sptbr' for the privileged spec `1.10'
 .*Warning: invalid CSR `mbadaddr' for the privileged spec `1.10'
 .*Warning: invalid CSR `mucounteren' for the privileged spec `1.10'
-.*Warning: invalid CSR `hstatus' for the privileged spec `1.10'
-.*Warning: invalid CSR `hedeleg' for the privileged spec `1.10'
-.*Warning: invalid CSR `hideleg' for the privileged spec `1.10'
-.*Warning: invalid CSR `hie' for the privileged spec `1.10'
-.*Warning: invalid CSR `htvec' for the privileged spec `1.10'
-.*Warning: invalid CSR `hscratch' for the privileged spec `1.10'
-.*Warning: invalid CSR `hepc' for the privileged spec `1.10'
-.*Warning: invalid CSR `hcause' for the privileged spec `1.10'
-.*Warning: invalid CSR `hbadaddr' for the privileged spec `1.10'
-.*Warning: invalid CSR `hip' for the privileged spec `1.10'
 .*Warning: invalid CSR `mbase' for the privileged spec `1.10'
 .*Warning: invalid CSR `mbound' for the privileged spec `1.10'
 .*Warning: invalid CSR `mibase' for the privileged spec `1.10'
diff --git a/gas/testsuite/gas/riscv/priv-reg-fail-version-1p11.l b/gas/testsuite/gas/riscv/priv-reg-fail-version-1p11.l
index 68354ce25ce9..888cbb211d7f 100644
--- a/gas/testsuite/gas/riscv/priv-reg-fail-version-1p11.l
+++ b/gas/testsuite/gas/riscv/priv-reg-fail-version-1p11.l
@@ -4,16 +4,6 @@
 .*Warning: invalid CSR `sptbr' for the privileged spec `1.11'
 .*Warning: invalid CSR `mbadaddr' for the privileged spec `1.11'
 .*Warning: invalid CSR `mucounteren' for the privileged spec `1.11'
-.*Warning: invalid CSR `hstatus' for the privileged spec `1.11'
-.*Warning: invalid CSR `hedeleg' for the privileged spec `1.11'
-.*Warning: invalid CSR `hideleg' for the privileged spec `1.11'
-.*Warning: invalid CSR `hie' for the privileged spec `1.11'
-.*Warning: invalid CSR `htvec' for the privileged spec `1.11'
-.*Warning: invalid CSR `hscratch' for the privileged spec `1.11'
-.*Warning: invalid CSR `hepc' for the privileged spec `1.11'
-.*Warning: invalid CSR `hcause' for the privileged spec `1.11'
-.*Warning: invalid CSR `hbadaddr' for the privileged spec `1.11'
-.*Warning: invalid CSR `hip' for the privileged spec `1.11'
 .*Warning: invalid CSR `mbase' for the privileged spec `1.11'
 .*Warning: invalid CSR `mbound' for the privileged spec `1.11'
 .*Warning: invalid CSR `mibase' for the privileged spec `1.11'
diff --git a/gas/testsuite/gas/riscv/priv-reg-version-1p10.d b/gas/testsuite/gas/riscv/priv-reg-version-1p10.d
index ee4f405adaae..44e9af63a31f 100644
--- a/gas/testsuite/gas/riscv/priv-reg-version-1p10.d
+++ b/gas/testsuite/gas/riscv/priv-reg-version-1p10.d
@@ -225,16 +225,6 @@ Disassembly of section .text:
 [ 	]+[0-9a-f]+:[  	]+18002573[    	]+csrr[        	]+a0,satp
 [ 	]+[0-9a-f]+:[  	]+34302573[    	]+csrr[        	]+a0,mtval
 [ 	]+[0-9a-f]+:[  	]+32002573[    	]+csrr[        	]+a0,0x320
-[ 	]+[0-9a-f]+:[  	]+20002573[    	]+csrr[        	]+a0,0x200
-[ 	]+[0-9a-f]+:[  	]+20202573[    	]+csrr[        	]+a0,0x202
-[ 	]+[0-9a-f]+:[  	]+20302573[    	]+csrr[        	]+a0,0x203
-[ 	]+[0-9a-f]+:[  	]+20402573[    	]+csrr[        	]+a0,0x204
-[ 	]+[0-9a-f]+:[  	]+20502573[    	]+csrr[        	]+a0,0x205
-[ 	]+[0-9a-f]+:[  	]+24002573[    	]+csrr[        	]+a0,0x240
-[ 	]+[0-9a-f]+:[  	]+24102573[    	]+csrr[        	]+a0,0x241
-[ 	]+[0-9a-f]+:[  	]+24202573[    	]+csrr[        	]+a0,0x242
-[ 	]+[0-9a-f]+:[  	]+24302573[    	]+csrr[        	]+a0,0x243
-[ 	]+[0-9a-f]+:[  	]+24402573[    	]+csrr[        	]+a0,0x244
 [ 	]+[0-9a-f]+:[  	]+38002573[    	]+csrr[        	]+a0,0x380
 [ 	]+[0-9a-f]+:[  	]+38102573[    	]+csrr[        	]+a0,0x381
 [ 	]+[0-9a-f]+:[  	]+38202573[    	]+csrr[        	]+a0,0x382
diff --git a/gas/testsuite/gas/riscv/priv-reg-version-1p11.d b/gas/testsuite/gas/riscv/priv-reg-version-1p11.d
index 185e84dbd70f..c456cb945689 100644
--- a/gas/testsuite/gas/riscv/priv-reg-version-1p11.d
+++ b/gas/testsuite/gas/riscv/priv-reg-version-1p11.d
@@ -225,16 +225,6 @@ Disassembly of section .text:
 [     	]+[0-9a-f]+:[  	]+18002573[    	]+csrr[        	]+a0,satp
 [     	]+[0-9a-f]+:[  	]+34302573[    	]+csrr[        	]+a0,mtval
 [     	]+[0-9a-f]+:[  	]+32002573[    	]+csrr[        	]+a0,mcountinhibit
-[     	]+[0-9a-f]+:[  	]+20002573[    	]+csrr[        	]+a0,0x200
-[     	]+[0-9a-f]+:[  	]+20202573[    	]+csrr[        	]+a0,0x202
-[     	]+[0-9a-f]+:[  	]+20302573[    	]+csrr[        	]+a0,0x203
-[     	]+[0-9a-f]+:[  	]+20402573[    	]+csrr[        	]+a0,0x204
-[     	]+[0-9a-f]+:[  	]+20502573[    	]+csrr[        	]+a0,0x205
-[     	]+[0-9a-f]+:[  	]+24002573[    	]+csrr[        	]+a0,0x240
-[     	]+[0-9a-f]+:[  	]+24102573[    	]+csrr[        	]+a0,0x241
-[     	]+[0-9a-f]+:[  	]+24202573[    	]+csrr[        	]+a0,0x242
-[     	]+[0-9a-f]+:[  	]+24302573[    	]+csrr[        	]+a0,0x243
-[     	]+[0-9a-f]+:[  	]+24402573[    	]+csrr[        	]+a0,0x244
 [     	]+[0-9a-f]+:[  	]+38002573[    	]+csrr[        	]+a0,0x380
 [     	]+[0-9a-f]+:[  	]+38102573[    	]+csrr[        	]+a0,0x381
 [     	]+[0-9a-f]+:[  	]+38202573[    	]+csrr[        	]+a0,0x382
diff --git a/gas/testsuite/gas/riscv/priv-reg-version-1p9p1.d b/gas/testsuite/gas/riscv/priv-reg-version-1p9p1.d
index 0e0ba7797014..432aecc6691c 100644
--- a/gas/testsuite/gas/riscv/priv-reg-version-1p9p1.d
+++ b/gas/testsuite/gas/riscv/priv-reg-version-1p9p1.d
@@ -225,16 +225,6 @@ Disassembly of section .text:
 [     	]+[0-9a-f]+:[  	]+18002573[    	]+csrr[        	]+a0,sptbr
 [     	]+[0-9a-f]+:[  	]+34302573[    	]+csrr[        	]+a0,mbadaddr
 [     	]+[0-9a-f]+:[  	]+32002573[    	]+csrr[        	]+a0,mucounteren
-[     	]+[0-9a-f]+:[  	]+20002573[    	]+csrr[        	]+a0,hstatus
-[     	]+[0-9a-f]+:[  	]+20202573[    	]+csrr[        	]+a0,hedeleg
-[     	]+[0-9a-f]+:[  	]+20302573[    	]+csrr[        	]+a0,hideleg
-[     	]+[0-9a-f]+:[  	]+20402573[    	]+csrr[        	]+a0,hie
-[     	]+[0-9a-f]+:[  	]+20502573[    	]+csrr[        	]+a0,htvec
-[     	]+[0-9a-f]+:[  	]+24002573[    	]+csrr[        	]+a0,hscratch
-[     	]+[0-9a-f]+:[  	]+24102573[    	]+csrr[        	]+a0,hepc
-[     	]+[0-9a-f]+:[  	]+24202573[    	]+csrr[        	]+a0,hcause
-[     	]+[0-9a-f]+:[  	]+24302573[    	]+csrr[        	]+a0,hbadaddr
-[     	]+[0-9a-f]+:[  	]+24402573[    	]+csrr[        	]+a0,hip
 [     	]+[0-9a-f]+:[  	]+38002573[    	]+csrr[        	]+a0,mbase
 [     	]+[0-9a-f]+:[  	]+38102573[    	]+csrr[        	]+a0,mbound
 [     	]+[0-9a-f]+:[  	]+38202573[    	]+csrr[        	]+a0,mibase
diff --git a/gas/testsuite/gas/riscv/priv-reg.s b/gas/testsuite/gas/riscv/priv-reg.s
index 5cf3ebc1b3e0..8c507fec4bb9 100644
--- a/gas/testsuite/gas/riscv/priv-reg.s
+++ b/gas/testsuite/gas/riscv/priv-reg.s
@@ -234,16 +234,6 @@
 	csr mbadaddr		# 0x343 in 1.9.1, but the value is mtval since 1.10
 	csr mucounteren		# 0x320 in 1.9.1, dropped in 1.10, but the value is mcountinhibit since 1.11
 
-	csr hstatus		# 0x200, dropped in 1.10
-	csr hedeleg		# 0x202, dropped in 1.10
-	csr hideleg		# 0x203, dropped in 1.10
-	csr hie			# 0x204, dropped in 1.10
-	csr htvec		# 0x205, dropped in 1.10
-	csr hscratch		# 0x240, dropped in 1.10
-	csr hepc		# 0x241, dropped in 1.10
-	csr hcause		# 0x242, dropped in 1.10
-	csr hbadaddr		# 0x243, dropped in 1.10
-	csr hip			# 0x244, dropped in 1.10
 	csr mbase		# 0x380, dropped in 1.10
 	csr mbound		# 0x381, dropped in 1.10
 	csr mibase		# 0x382, dropped in 1.10
-- 
2.30.2


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

* [RFC 4/4] RISC-V: fix a comment for adding CSR entry and annotate switch-break
  2021-12-16 17:33 [RFC 0/4] riscv/binutils support Hypervisor Extension Vineet Gupta
                   ` (2 preceding siblings ...)
  2021-12-16 17:33 ` [RFC 3/4] RISC-V: Hypervisor ext: tests for new isns/csr and cleanup old csrs Vineet Gupta
@ 2021-12-16 17:33 ` Vineet Gupta
  2021-12-17  4:10   ` Palmer Dabbelt
  3 siblings, 1 reply; 17+ messages in thread
From: Vineet Gupta @ 2021-12-16 17:33 UTC (permalink / raw)
  To: Binutils, Nelson Chu; +Cc: Kito Cheng, jim.wilson.gcc, palmer, Vineet Gupta

Nothing functional ...

Signed-off-by: Vineet Gupta <vineetg@rivosinc.com>
---
 gas/config/tc-riscv.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/gas/config/tc-riscv.c b/gas/config/tc-riscv.c
index e28818b4fd15..082dc59aa9f9 100644
--- a/gas/config/tc-riscv.c
+++ b/gas/config/tc-riscv.c
@@ -83,7 +83,7 @@ struct riscv_csr_extra
   enum riscv_spec_class define_version;
 
   /* Record the CSR is aborted/invalid from which versions.  If it isn't
-     aborted in the current version, then it should be CSR_CLASS_VDRAFT.  */
+     aborted in the current version, then it should be PRIV_SPEC_CLASS_DRAFT.  */
   enum riscv_spec_class abort_version;
 
   /* The CSR may have more than one setting.  */
@@ -1109,7 +1109,7 @@ validate_riscv_insn (const struct riscv_opcode *opc, int length)
 	    default:
 	      goto unknown_validate_operand;
 	    }
-	  break;
+	  break;  /* end RVC */
 	case 'V': /* RVV */
 	  switch (*++oparg)
 	    {
@@ -1133,7 +1133,7 @@ validate_riscv_insn (const struct riscv_opcode *opc, int length)
 	    default:
 	      goto unknown_validate_operand;
 	    }
-	  break;
+	  break; /* end RVV */
 	case ',': break;
 	case '(': break;
 	case ')': break;
@@ -2610,7 +2610,7 @@ riscv_ip (char *str, struct riscv_cl_insn *ip, expressionS *imm_expr,
 		default:
 		  goto unknown_riscv_ip_operand;
 		}
-	      break;
+	      break; /* end RVC */
 
 	    case 'V': /* RVV */
 	      switch (*++oparg)
@@ -2776,7 +2776,7 @@ riscv_ip (char *str, struct riscv_cl_insn *ip, expressionS *imm_expr,
 		default:
 		  goto unknown_riscv_ip_operand;
 		}
-	      break;
+	      break; /* end RVV */
 
 	    case ',':
 	      ++argnum;
-- 
2.30.2


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

* Re: [RFC 1/4] RISC-V: Hypervisor ext: Treat as "Standard" extension
  2021-12-16 17:33 ` [RFC 1/4] RISC-V: Hypervisor ext: Treat as "Standard" extension Vineet Gupta
@ 2021-12-17  4:10   ` Palmer Dabbelt
  2021-12-17 16:10     ` Nelson Chu
  0 siblings, 1 reply; 17+ messages in thread
From: Palmer Dabbelt @ 2021-12-17  4:10 UTC (permalink / raw)
  To: Vineet Gupta; +Cc: binutils, Nelson Chu, kito.cheng, Jim Wilson, Vineet Gupta

On Thu, 16 Dec 2021 09:33:25 PST (-0800), Vineet Gupta wrote:
> gas currently fails for single-letter 'h' prefix in arch string.
>
> | riscv64-unknown-elf-as -march=rv64gh empty.s
> | Assembler messages:
> | Error: rv64gh: unknown prefixed ISA extension `h'
>
> It implements the "multi-letter" prefix for H-ext as recommended in
> ISA manual subsection "ISA Extension Naming Conventions". However given
> the discussions at [1] there seems to be no pressing need to keep version
> for the current H-ext since
>  - it will remain at 1.0
>  - any future Hypervisor extensions wull called "Sh*" or some such
>  - H was the original standard extension with its own MISA bit
>
> This patch thus relaxes the constraint and allows single letter prefix
> 'h'
>
> Note: To keep the binutils dejagnu failures same, I've fixed the
> testsuite files within the same patch. I can break it up if reviewers
> feel as such.
>
> [1] https://github.com/riscv/riscv-isa-manual/issues/781#issuecomment-983233088
>
> bfd/
> 	* elfxx-riscv.c (riscv_supported_std_ext): Add "h" entry.
> 	(riscv_supported_std_h_ext): Add "h" entry.
> 	(riscv_get_default_ext_version): Handle PRIV_SPEC_CLASS_DRAFT.
> 	(riscv_multi_subset_supports): Handle INSN_CLASS_H.
>
> gas/
> 	* testsuite/gas/riscv/march-fail-single-prefix-h.d: Deleted as
> 	single-letter prefix 'h' is now valid.
> 	* testsuite/gas/riscv/march-fail-single-prefix-l: Removed 'h'.
>
> include/
> 	* opcode/riscv.h (enum riscv_insn_class): Add INSN_CLASS_H.
>
> Signed-off-by: Vineet Gupta <vineetg@rivosinc.com>
> ---
>  bfd/elfxx-riscv.c                                    | 5 +++++
>  gas/testsuite/gas/riscv/march-fail-single-prefix-h.d | 3 ---
>  gas/testsuite/gas/riscv/march-fail-single-prefix.l   | 2 +-
>  include/opcode/riscv.h                               | 1 +
>  4 files changed, 7 insertions(+), 4 deletions(-)
>  delete mode 100644 gas/testsuite/gas/riscv/march-fail-single-prefix-h.d
>
> diff --git a/bfd/elfxx-riscv.c b/bfd/elfxx-riscv.c
> index 3bd41ff2b551..7d5ae5a4657d 100644
> --- a/bfd/elfxx-riscv.c
> +++ b/bfd/elfxx-riscv.c
> @@ -1173,6 +1173,7 @@ static struct riscv_supported_ext riscv_supported_std_ext[] =
>    {"t",		ISA_SPEC_CLASS_NONE, RISCV_UNKNOWN_VERSION, RISCV_UNKNOWN_VERSION, 0 },
>    {"p",		ISA_SPEC_CLASS_NONE, RISCV_UNKNOWN_VERSION, RISCV_UNKNOWN_VERSION, 0 },
>    {"v",		ISA_SPEC_CLASS_DRAFT,		1, 0, 0 },
> +  {"h",		PRIV_SPEC_CLASS_DRAFT,		1, 0, 0 },

According to 
<https://wiki.riscv.org/display/TECH/Recently+Ratified+Extensions>, this 
was ratified in November so we should be able to update this to 
something like PRIV_SPEC_CLASS_1P12 (the V stuff was also ratified, but 
that's a different issue).

>    {"n",		ISA_SPEC_CLASS_NONE, RISCV_UNKNOWN_VERSION, RISCV_UNKNOWN_VERSION, 0 },
>    {NULL, 0, 0, 0, 0}
>  };
> @@ -1232,6 +1233,7 @@ static struct riscv_supported_ext riscv_supported_std_s_ext[] =
>
>  static struct riscv_supported_ext riscv_supported_std_h_ext[] =
>  {
> +  {"h",                 PRIV_SPEC_CLASS_DRAFT,          1, 0, 0 },
>    {NULL, 0, 0, 0, 0}
>  };
>
> @@ -1531,6 +1533,7 @@ riscv_get_default_ext_version (enum riscv_spec_class *default_isa_spec,
>      {
>        if (strcmp (table[i].name, name) == 0
>  	  && (table[i].isa_spec_class == ISA_SPEC_CLASS_DRAFT
> +	      || table[i].isa_spec_class == PRIV_SPEC_CLASS_DRAFT
>  	      || table[i].isa_spec_class == *default_isa_spec))
>  	{
>  	  *major_version = table[i].major_version;
> @@ -2412,6 +2415,8 @@ riscv_multi_subset_supports (riscv_parse_subset_t *rps,
>  	      || riscv_subset_supports (rps, "zve64d")
>  	      || riscv_subset_supports (rps, "zve64f")
>  	      || riscv_subset_supports (rps, "zve32f"));
> +    case INSN_CLASS_H:
> +      return riscv_subset_supports (rps, "h");
>      default:
>        rps->error_handler
>          (_("internal: unreachable INSN_CLASS_*"));
> diff --git a/gas/testsuite/gas/riscv/march-fail-single-prefix-h.d b/gas/testsuite/gas/riscv/march-fail-single-prefix-h.d
> deleted file mode 100644
> index eb101bd71353..000000000000
> --- a/gas/testsuite/gas/riscv/march-fail-single-prefix-h.d
> +++ /dev/null
> @@ -1,3 +0,0 @@
> -#as: -march=rv32ih
> -#source: empty.s
> -#error_output: march-fail-single-prefix.l
> diff --git a/gas/testsuite/gas/riscv/march-fail-single-prefix.l b/gas/testsuite/gas/riscv/march-fail-single-prefix.l
> index 13942ed66642..e4418de69acb 100644
> --- a/gas/testsuite/gas/riscv/march-fail-single-prefix.l
> +++ b/gas/testsuite/gas/riscv/march-fail-single-prefix.l
> @@ -1,2 +1,2 @@
>  .*Assembler messages:
> -.*Error: .*unknown prefixed ISA extension `(s|h|z|x|zxm)'
> +.*Error: .*unknown prefixed ISA extension `(s|z|x|zxm)'
> diff --git a/include/opcode/riscv.h b/include/opcode/riscv.h
> index 14889972abce..2783bdecaae5 100644
> --- a/include/opcode/riscv.h
> +++ b/include/opcode/riscv.h
> @@ -387,6 +387,7 @@ enum riscv_insn_class
>    INSN_CLASS_ZKND_OR_ZKNE,
>    INSN_CLASS_V,
>    INSN_CLASS_ZVEF,
> +  INSN_CLASS_H,
>  };
>
>  /* This structure holds information for a particular instruction.  */

Reviewed-by: Palmer Dabbelt <palmer@rivosinc.com>

We can always deal with the draft/ratified stuff later, as there's a 
bunch of them.

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

* Re: [RFC 3/4] RISC-V: Hypervisor ext: tests for new isns/csr and cleanup old csrs
  2021-12-16 17:33 ` [RFC 3/4] RISC-V: Hypervisor ext: tests for new isns/csr and cleanup old csrs Vineet Gupta
@ 2021-12-17  4:10   ` Palmer Dabbelt
  2021-12-21  8:17   ` Jan Beulich
  1 sibling, 0 replies; 17+ messages in thread
From: Palmer Dabbelt @ 2021-12-17  4:10 UTC (permalink / raw)
  To: Vineet Gupta; +Cc: binutils, Nelson Chu, kito.cheng, Jim Wilson, Vineet Gupta

On Thu, 16 Dec 2021 09:33:27 PST (-0800), Vineet Gupta wrote:
> Since the prior versions were not fully supported (and or functional
> even from ISA pov), we just remove any old baggage - this avoids the
> mess in keeping csr with same names but different addresses etc.

IMO it's generally a bad idea to drop stuff that we used to support when 
a new version of the spec came out, but given that these old bits of H 
never really did anything I think it's safe to just get rid of them.

Reviewed-by: Palmer Dabbelt <palmer@rivosinc.com>

Though we could probably get rid of some churn by just going straight to 
the priv-1.12 class for the H stuff, so might be better to do that 
sooner rather than later.

> Signed-off-by: Vineet Gupta <vineetg@rivosinc.com>
> ---
>  gas/testsuite/gas/riscv/csr-dw-regnums.d      | 10 ---
>  gas/testsuite/gas/riscv/csr-dw-regnums.s      | 10 ---
>  gas/testsuite/gas/riscv/h-ext-32.s            | 61 +++++++++++++++++
>  gas/testsuite/gas/riscv/h-ext-64.s            | 68 +++++++++++++++++++
>  .../gas/riscv/priv-reg-fail-read-only-01.s    | 10 ---
>  .../gas/riscv/priv-reg-fail-version-1p10.l    | 10 ---
>  .../gas/riscv/priv-reg-fail-version-1p11.l    | 10 ---
>  .../gas/riscv/priv-reg-version-1p10.d         | 10 ---
>  .../gas/riscv/priv-reg-version-1p11.d         | 10 ---
>  .../gas/riscv/priv-reg-version-1p9p1.d        | 10 ---
>  gas/testsuite/gas/riscv/priv-reg.s            | 10 ---
>  11 files changed, 129 insertions(+), 90 deletions(-)
>  create mode 100644 gas/testsuite/gas/riscv/h-ext-32.s
>  create mode 100644 gas/testsuite/gas/riscv/h-ext-64.s
>
> diff --git a/gas/testsuite/gas/riscv/csr-dw-regnums.d b/gas/testsuite/gas/riscv/csr-dw-regnums.d
> index de17ad81fbde..ea0a445c39ce 100644
> --- a/gas/testsuite/gas/riscv/csr-dw-regnums.d
> +++ b/gas/testsuite/gas/riscv/csr-dw-regnums.d
> @@ -229,16 +229,6 @@ Contents of the .* section:
>    DW_CFA_offset_extended_sf: r4925 \(mhpmevent29\) at cfa\+3316
>    DW_CFA_offset_extended_sf: r4926 \(mhpmevent30\) at cfa\+3320
>    DW_CFA_offset_extended_sf: r4927 \(mhpmevent31\) at cfa\+3324
> -  DW_CFA_offset_extended_sf: r4608 \(hstatus\) at cfa\+2048
> -  DW_CFA_offset_extended_sf: r4610 \(hedeleg\) at cfa\+2056
> -  DW_CFA_offset_extended_sf: r4611 \(hideleg\) at cfa\+2060
> -  DW_CFA_offset_extended_sf: r4612 \(hie\) at cfa\+2064
> -  DW_CFA_offset_extended_sf: r4613 \(htvec\) at cfa\+2068
> -  DW_CFA_offset_extended_sf: r4672 \(hscratch\) at cfa\+2304
> -  DW_CFA_offset_extended_sf: r4673 \(hepc\) at cfa\+2308
> -  DW_CFA_offset_extended_sf: r4674 \(hcause\) at cfa\+2312
> -  DW_CFA_offset_extended_sf: r4675 \(hbadaddr\) at cfa\+2316
> -  DW_CFA_offset_extended_sf: r4676 \(hip\) at cfa\+2320
>    DW_CFA_offset_extended_sf: r4992 \(mbase\) at cfa\+3584
>    DW_CFA_offset_extended_sf: r4993 \(mbound\) at cfa\+3588
>    DW_CFA_offset_extended_sf: r4994 \(mibase\) at cfa\+3592
> diff --git a/gas/testsuite/gas/riscv/csr-dw-regnums.s b/gas/testsuite/gas/riscv/csr-dw-regnums.s
> index ecc801469763..549475d650e8 100644
> --- a/gas/testsuite/gas/riscv/csr-dw-regnums.s
> +++ b/gas/testsuite/gas/riscv/csr-dw-regnums.s
> @@ -220,16 +220,6 @@ _start:
>  	.cfi_offset mhpmevent30, 3320
>  	.cfi_offset mhpmevent31, 3324
>  	# dropped
> -	.cfi_offset hstatus, 2048
> -	.cfi_offset hedeleg, 2056
> -	.cfi_offset hideleg, 2060
> -	.cfi_offset hie, 2064
> -	.cfi_offset htvec, 2068
> -	.cfi_offset hscratch, 2304
> -	.cfi_offset hepc, 2308
> -	.cfi_offset hcause, 2312
> -	.cfi_offset hbadaddr, 2316
> -	.cfi_offset hip, 2320
>  	.cfi_offset mbase, 3584
>  	.cfi_offset mbound, 3588
>  	.cfi_offset mibase, 3592
> diff --git a/gas/testsuite/gas/riscv/h-ext-32.s b/gas/testsuite/gas/riscv/h-ext-32.s
> new file mode 100644
> index 000000000000..2818d7d17381
> --- /dev/null
> +++ b/gas/testsuite/gas/riscv/h-ext-32.s
> @@ -0,0 +1,61 @@
> +target:
> +	hlv.b   a0, (a1)
> +	hlv.bu  a0, (a1)
> +	hlv.h   a1, (a2)
> +	hlv.hu  a1, (a1)
> +	hlvx.hu a1, (a2)
> +	hlv.w   a2, (a2)
> +	hlvx.wu a2, (a3)
> +
> +	hsv.b   a0, (a1)
> +	hsv.h   a0, (a1)
> +	hsv.w   a0, (a1)
> +
> +	csrr    a0, hstatus
> +	csrw    hstatus, a1
> +	csrr    a0, hedeleg
> +	csrw    hedeleg, a1
> +	csrr    a0, hideleg
> +	csrw    hideleg, a1
> +	csrr    a0, hie
> +	csrw    hie, a1
> +	csrr    a0, hcounteren
> +	csrw    hcounteren, a1
> +	csrr    a0, hgeie
> +	csrw    hgeie, a1
> +	csrr    a0, htval
> +	csrw    htval, a1
> +	csrr    a0, hip
> +	csrw    hip, a1
> +	csrr    a0, hvip
> +	csrw    hvip, a1
> +	csrr    a0, htinst
> +	csrw    htinst, a1
> +	csrr    a0, hgeip
> +	csrw    hgeip, a1
> +	csrr    a0, henvcfg
> +	csrw    henvcfg, a1
> +	csrr    a0, hgatp
> +	csrw    hgatp, a1
> +	csrr    a0, hcontext
> +	csrw    hcontext, a1
> +	csrr    a0, htimedelta
> +	csrw    htimedelta, a1
> +	csrr    a0, vsstatus
> +	csrw    vsstatus, a1
> +	csrr    a0, vsie
> +	csrw    vsie, a1
> +	csrr    a0, vstvec
> +	csrw    vstvec, a1
> +	csrr    a0, vsscratch
> +	csrw    vsscratch, a1
> +	csrr    a0, vsepc
> +	csrw    vsepc, a1
> +	csrr    a0, vscause
> +	csrw    vscause, a1
> +	csrr    a0, vstval
> +	csrw    vstval, a1
> +	csrr    a0, vsip
> +	csrw    vsip, a1
> +	csrr    a0, vsatp
> +	csrw    vsatp, a1
> diff --git a/gas/testsuite/gas/riscv/h-ext-64.s b/gas/testsuite/gas/riscv/h-ext-64.s
> new file mode 100644
> index 000000000000..25455fd4fdca
> --- /dev/null
> +++ b/gas/testsuite/gas/riscv/h-ext-64.s
> @@ -0,0 +1,68 @@
> +target:
> +	hlv.b   a0, (a1)
> +	hlv.bu  a0, (a1)
> +	hlv.h   a1, (a2)
> +	hlv.hu  a1, (a1)
> +	hlvx.hu a1, (a2)
> +	hlv.w   a2, (a2)
> +	hlv.wu  a2, (a3)
> +	hlvx.wu a2, (a3)
> +	hlv.d   a3, (a4)
> +
> +	hsv.b   a0, (a1)
> +	hsv.h   a0, (a1)
> +	hsv.w   a0, (a1)
> +	hsv.d   a0, (a1)
> +
> +	csrr    a0, hstatus
> +	csrw    hstatus, a1
> +	csrr    a0, hedeleg
> +	csrw    hedeleg, a1
> +	csrr    a0, hideleg
> +	csrw    hideleg, a1
> +	csrr    a0, hie
> +	csrw    hie, a1
> +	csrr    a0, hcounteren
> +	csrw    hcounteren, a1
> +	csrr    a0, hgeie
> +	csrw    hgeie, a1
> +	csrr    a0, htval
> +	csrw    htval, a1
> +	csrr    a0, hip
> +	csrw    hip, a1
> +	csrr    a0, hvip
> +	csrw    hvip, a1
> +	csrr    a0, htinst
> +	csrw    htinst, a1
> +	csrr    a0, hgeip
> +	csrw    hgeip, a1
> +	csrr    a0, henvcfg
> +	csrw    henvcfg, a1
> +	csrr    a0, henvcfgh
> +	csrw    henvcfgh, a1
> +	csrr    a0, hgatp
> +	csrw    hgatp, a1
> +	csrr    a0, hcontext
> +	csrw    hcontext, a1
> +	csrr    a0, htimedelta
> +	csrw    htimedelta, a1
> +	csrr    a0, htimedeltah
> +	csrw    htimedeltah, a1
> +	csrr    a0, vsstatus
> +	csrw    vsstatus, a1
> +	csrr    a0, vsie
> +	csrw    vsie, a1
> +	csrr    a0, vstvec
> +	csrw    vstvec, a1
> +	csrr    a0, vsscratch
> +	csrw    vsscratch, a1
> +	csrr    a0, vsepc
> +	csrw    vsepc, a1
> +	csrr    a0, vscause
> +	csrw    vscause, a1
> +	csrr    a0, vstval
> +	csrw    vstval, a1
> +	csrr    a0, vsip
> +	csrw    vsip, a1
> +	csrr    a0, vsatp
> +	csrw    vsatp, a1
> diff --git a/gas/testsuite/gas/riscv/priv-reg-fail-read-only-01.s b/gas/testsuite/gas/riscv/priv-reg-fail-read-only-01.s
> index af0fc4e14a42..aff34e9a3470 100644
> --- a/gas/testsuite/gas/riscv/priv-reg-fail-read-only-01.s
> +++ b/gas/testsuite/gas/riscv/priv-reg-fail-read-only-01.s
> @@ -249,16 +249,6 @@
>  	csr mucounteren         # 0x320 in 1.9.1, dropped in 1.10, but the value is mcountinhibit since 1.11
>  	csr dscratch            # 0x7b2 in 1.10,  but the value is dscratch0 since 1.11
>
> -	csr hstatus             # 0x200, dropped in 1.10
> -	csr hedeleg             # 0x202, dropped in 1.10
> -	csr hideleg             # 0x203, dropped in 1.10
> -	csr hie                 # 0x204, dropped in 1.10
> -	csr htvec               # 0x205, dropped in 1.10
> -	csr hscratch            # 0x240, dropped in 1.10
> -	csr hepc                # 0x241, dropped in 1.10
> -	csr hcause              # 0x242, dropped in 1.10
> -	csr hbadaddr            # 0x243, dropped in 1.10
> -	csr hip                 # 0x244, dropped in 1.10
>  	csr mbase               # 0x380, dropped in 1.10
>  	csr mbound              # 0x381, dropped in 1.10
>  	csr mibase              # 0x382, dropped in 1.10
> diff --git a/gas/testsuite/gas/riscv/priv-reg-fail-version-1p10.l b/gas/testsuite/gas/riscv/priv-reg-fail-version-1p10.l
> index fbba6e525670..5f7a8d60d7dd 100644
> --- a/gas/testsuite/gas/riscv/priv-reg-fail-version-1p10.l
> +++ b/gas/testsuite/gas/riscv/priv-reg-fail-version-1p10.l
> @@ -5,16 +5,6 @@
>  .*Warning: invalid CSR `sptbr' for the privileged spec `1.10'
>  .*Warning: invalid CSR `mbadaddr' for the privileged spec `1.10'
>  .*Warning: invalid CSR `mucounteren' for the privileged spec `1.10'
> -.*Warning: invalid CSR `hstatus' for the privileged spec `1.10'
> -.*Warning: invalid CSR `hedeleg' for the privileged spec `1.10'
> -.*Warning: invalid CSR `hideleg' for the privileged spec `1.10'
> -.*Warning: invalid CSR `hie' for the privileged spec `1.10'
> -.*Warning: invalid CSR `htvec' for the privileged spec `1.10'
> -.*Warning: invalid CSR `hscratch' for the privileged spec `1.10'
> -.*Warning: invalid CSR `hepc' for the privileged spec `1.10'
> -.*Warning: invalid CSR `hcause' for the privileged spec `1.10'
> -.*Warning: invalid CSR `hbadaddr' for the privileged spec `1.10'
> -.*Warning: invalid CSR `hip' for the privileged spec `1.10'
>  .*Warning: invalid CSR `mbase' for the privileged spec `1.10'
>  .*Warning: invalid CSR `mbound' for the privileged spec `1.10'
>  .*Warning: invalid CSR `mibase' for the privileged spec `1.10'
> diff --git a/gas/testsuite/gas/riscv/priv-reg-fail-version-1p11.l b/gas/testsuite/gas/riscv/priv-reg-fail-version-1p11.l
> index 68354ce25ce9..888cbb211d7f 100644
> --- a/gas/testsuite/gas/riscv/priv-reg-fail-version-1p11.l
> +++ b/gas/testsuite/gas/riscv/priv-reg-fail-version-1p11.l
> @@ -4,16 +4,6 @@
>  .*Warning: invalid CSR `sptbr' for the privileged spec `1.11'
>  .*Warning: invalid CSR `mbadaddr' for the privileged spec `1.11'
>  .*Warning: invalid CSR `mucounteren' for the privileged spec `1.11'
> -.*Warning: invalid CSR `hstatus' for the privileged spec `1.11'
> -.*Warning: invalid CSR `hedeleg' for the privileged spec `1.11'
> -.*Warning: invalid CSR `hideleg' for the privileged spec `1.11'
> -.*Warning: invalid CSR `hie' for the privileged spec `1.11'
> -.*Warning: invalid CSR `htvec' for the privileged spec `1.11'
> -.*Warning: invalid CSR `hscratch' for the privileged spec `1.11'
> -.*Warning: invalid CSR `hepc' for the privileged spec `1.11'
> -.*Warning: invalid CSR `hcause' for the privileged spec `1.11'
> -.*Warning: invalid CSR `hbadaddr' for the privileged spec `1.11'
> -.*Warning: invalid CSR `hip' for the privileged spec `1.11'
>  .*Warning: invalid CSR `mbase' for the privileged spec `1.11'
>  .*Warning: invalid CSR `mbound' for the privileged spec `1.11'
>  .*Warning: invalid CSR `mibase' for the privileged spec `1.11'
> diff --git a/gas/testsuite/gas/riscv/priv-reg-version-1p10.d b/gas/testsuite/gas/riscv/priv-reg-version-1p10.d
> index ee4f405adaae..44e9af63a31f 100644
> --- a/gas/testsuite/gas/riscv/priv-reg-version-1p10.d
> +++ b/gas/testsuite/gas/riscv/priv-reg-version-1p10.d
> @@ -225,16 +225,6 @@ Disassembly of section .text:
>  [ 	]+[0-9a-f]+:[  	]+18002573[    	]+csrr[        	]+a0,satp
>  [ 	]+[0-9a-f]+:[  	]+34302573[    	]+csrr[        	]+a0,mtval
>  [ 	]+[0-9a-f]+:[  	]+32002573[    	]+csrr[        	]+a0,0x320
> -[ 	]+[0-9a-f]+:[  	]+20002573[    	]+csrr[        	]+a0,0x200
> -[ 	]+[0-9a-f]+:[  	]+20202573[    	]+csrr[        	]+a0,0x202
> -[ 	]+[0-9a-f]+:[  	]+20302573[    	]+csrr[        	]+a0,0x203
> -[ 	]+[0-9a-f]+:[  	]+20402573[    	]+csrr[        	]+a0,0x204
> -[ 	]+[0-9a-f]+:[  	]+20502573[    	]+csrr[        	]+a0,0x205
> -[ 	]+[0-9a-f]+:[  	]+24002573[    	]+csrr[        	]+a0,0x240
> -[ 	]+[0-9a-f]+:[  	]+24102573[    	]+csrr[        	]+a0,0x241
> -[ 	]+[0-9a-f]+:[  	]+24202573[    	]+csrr[        	]+a0,0x242
> -[ 	]+[0-9a-f]+:[  	]+24302573[    	]+csrr[        	]+a0,0x243
> -[ 	]+[0-9a-f]+:[  	]+24402573[    	]+csrr[        	]+a0,0x244
>  [ 	]+[0-9a-f]+:[  	]+38002573[    	]+csrr[        	]+a0,0x380
>  [ 	]+[0-9a-f]+:[  	]+38102573[    	]+csrr[        	]+a0,0x381
>  [ 	]+[0-9a-f]+:[  	]+38202573[    	]+csrr[        	]+a0,0x382
> diff --git a/gas/testsuite/gas/riscv/priv-reg-version-1p11.d b/gas/testsuite/gas/riscv/priv-reg-version-1p11.d
> index 185e84dbd70f..c456cb945689 100644
> --- a/gas/testsuite/gas/riscv/priv-reg-version-1p11.d
> +++ b/gas/testsuite/gas/riscv/priv-reg-version-1p11.d
> @@ -225,16 +225,6 @@ Disassembly of section .text:
>  [     	]+[0-9a-f]+:[  	]+18002573[    	]+csrr[        	]+a0,satp
>  [     	]+[0-9a-f]+:[  	]+34302573[    	]+csrr[        	]+a0,mtval
>  [     	]+[0-9a-f]+:[  	]+32002573[    	]+csrr[        	]+a0,mcountinhibit
> -[     	]+[0-9a-f]+:[  	]+20002573[    	]+csrr[        	]+a0,0x200
> -[     	]+[0-9a-f]+:[  	]+20202573[    	]+csrr[        	]+a0,0x202
> -[     	]+[0-9a-f]+:[  	]+20302573[    	]+csrr[        	]+a0,0x203
> -[     	]+[0-9a-f]+:[  	]+20402573[    	]+csrr[        	]+a0,0x204
> -[     	]+[0-9a-f]+:[  	]+20502573[    	]+csrr[        	]+a0,0x205
> -[     	]+[0-9a-f]+:[  	]+24002573[    	]+csrr[        	]+a0,0x240
> -[     	]+[0-9a-f]+:[  	]+24102573[    	]+csrr[        	]+a0,0x241
> -[     	]+[0-9a-f]+:[  	]+24202573[    	]+csrr[        	]+a0,0x242
> -[     	]+[0-9a-f]+:[  	]+24302573[    	]+csrr[        	]+a0,0x243
> -[     	]+[0-9a-f]+:[  	]+24402573[    	]+csrr[        	]+a0,0x244
>  [     	]+[0-9a-f]+:[  	]+38002573[    	]+csrr[        	]+a0,0x380
>  [     	]+[0-9a-f]+:[  	]+38102573[    	]+csrr[        	]+a0,0x381
>  [     	]+[0-9a-f]+:[  	]+38202573[    	]+csrr[        	]+a0,0x382
> diff --git a/gas/testsuite/gas/riscv/priv-reg-version-1p9p1.d b/gas/testsuite/gas/riscv/priv-reg-version-1p9p1.d
> index 0e0ba7797014..432aecc6691c 100644
> --- a/gas/testsuite/gas/riscv/priv-reg-version-1p9p1.d
> +++ b/gas/testsuite/gas/riscv/priv-reg-version-1p9p1.d
> @@ -225,16 +225,6 @@ Disassembly of section .text:
>  [     	]+[0-9a-f]+:[  	]+18002573[    	]+csrr[        	]+a0,sptbr
>  [     	]+[0-9a-f]+:[  	]+34302573[    	]+csrr[        	]+a0,mbadaddr
>  [     	]+[0-9a-f]+:[  	]+32002573[    	]+csrr[        	]+a0,mucounteren
> -[     	]+[0-9a-f]+:[  	]+20002573[    	]+csrr[        	]+a0,hstatus
> -[     	]+[0-9a-f]+:[  	]+20202573[    	]+csrr[        	]+a0,hedeleg
> -[     	]+[0-9a-f]+:[  	]+20302573[    	]+csrr[        	]+a0,hideleg
> -[     	]+[0-9a-f]+:[  	]+20402573[    	]+csrr[        	]+a0,hie
> -[     	]+[0-9a-f]+:[  	]+20502573[    	]+csrr[        	]+a0,htvec
> -[     	]+[0-9a-f]+:[  	]+24002573[    	]+csrr[        	]+a0,hscratch
> -[     	]+[0-9a-f]+:[  	]+24102573[    	]+csrr[        	]+a0,hepc
> -[     	]+[0-9a-f]+:[  	]+24202573[    	]+csrr[        	]+a0,hcause
> -[     	]+[0-9a-f]+:[  	]+24302573[    	]+csrr[        	]+a0,hbadaddr
> -[     	]+[0-9a-f]+:[  	]+24402573[    	]+csrr[        	]+a0,hip
>  [     	]+[0-9a-f]+:[  	]+38002573[    	]+csrr[        	]+a0,mbase
>  [     	]+[0-9a-f]+:[  	]+38102573[    	]+csrr[        	]+a0,mbound
>  [     	]+[0-9a-f]+:[  	]+38202573[    	]+csrr[        	]+a0,mibase
> diff --git a/gas/testsuite/gas/riscv/priv-reg.s b/gas/testsuite/gas/riscv/priv-reg.s
> index 5cf3ebc1b3e0..8c507fec4bb9 100644
> --- a/gas/testsuite/gas/riscv/priv-reg.s
> +++ b/gas/testsuite/gas/riscv/priv-reg.s
> @@ -234,16 +234,6 @@
>  	csr mbadaddr		# 0x343 in 1.9.1, but the value is mtval since 1.10
>  	csr mucounteren		# 0x320 in 1.9.1, dropped in 1.10, but the value is mcountinhibit since 1.11
>
> -	csr hstatus		# 0x200, dropped in 1.10
> -	csr hedeleg		# 0x202, dropped in 1.10
> -	csr hideleg		# 0x203, dropped in 1.10
> -	csr hie			# 0x204, dropped in 1.10
> -	csr htvec		# 0x205, dropped in 1.10
> -	csr hscratch		# 0x240, dropped in 1.10
> -	csr hepc		# 0x241, dropped in 1.10
> -	csr hcause		# 0x242, dropped in 1.10
> -	csr hbadaddr		# 0x243, dropped in 1.10
> -	csr hip			# 0x244, dropped in 1.10
>  	csr mbase		# 0x380, dropped in 1.10
>  	csr mbound		# 0x381, dropped in 1.10
>  	csr mibase		# 0x382, dropped in 1.10

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

* Re: [RFC 2/4] RISC-V: Hypervisor ext: CSR and Instructions
  2021-12-16 17:33 ` [RFC 2/4] RISC-V: Hypervisor ext: CSR and Instructions Vineet Gupta
@ 2021-12-17  4:10   ` Palmer Dabbelt
  2021-12-17 17:30     ` Nelson Chu
  0 siblings, 1 reply; 17+ messages in thread
From: Palmer Dabbelt @ 2021-12-17  4:10 UTC (permalink / raw)
  To: Vineet Gupta; +Cc: binutils, Nelson Chu, kito.cheng, Jim Wilson, Vineet Gupta

On Thu, 16 Dec 2021 09:33:26 PST (-0800), Vineet Gupta wrote:
> This implements Hypervisor Extension 1.0 [1]
>
>  - Hypervisor Memory-Management Instructions
>    HFENCE.VVMA, HFENCE.GVMA,
>    HINVAL.GVMA, HINVAL.VVMA
>
>  - Hypervisor Virtual Machine Load and Store Instructions
>    HLV.B, HLV.BU,          HSV.B,
>    HLV.H, HLV.HU, HLVX.HU, HSB.H,
>    HLV.W, HLV.WU, HLVX.WU, HSV.W,
>    HLV.D,                  HSV.D
>
>  - Hypervisor CSRs (some new, some address changed)
>    hstatus, hedeleg, hideleg, hie, hcounteren, hgeie, htval, hip, hvip,
>    htinst, hgeip, henvcfg, henvcfgh, hgatp, hcontext, htimedelta, htimedeltah,
>    vsstatus, vsie, vstvec, vsscratch, vsepc, vscause, vstval, vsip, vsatp,
>
> This removed a couple of stale instructions: HRET, DRET.
>
> [1] https://github.com/riscv/riscv-isa-manual/releases/tag/draft-20211213-ea38395
>
> gas/
> 	* config/tc-riscv.c (enum riscv_csr_class): Added CSR_CLASS_H.
> 	(riscv_csr_address): check CSR_CLASS_H.
>
> include/
> 	* opcode/riscv-opc.h: Removed {MATCH,MASK}_{D,H}RET.
> 	Added {MATCH,MASK}_{HLV*,HSV*,HFENCE*,HINVAL*}.
> 	Added/updated CSR_H*, CSR_V*.
> 	DECLARE_INSN(hlv*), DECLARE_INSN(hsv*).
> 	DECLARE_INSN(hfence*), DECLARE_INSN(hinval*).
> 	DECLARE_CSR(h*), DECLARE_CSR(v*).
>
> opcodes/
> 	* riscv-opc.c (riscv_opcodes): Removed {d,h}ret.
> 	Added hlv*, hsv*, hfence*, hinval*.
>
> Signed-off-by: Vineet Gupta <vineetg@rivosinc.com>
> ---
>  gas/config/tc-riscv.c      |   5 ++
>  include/opcode/riscv-opc.h | 123 +++++++++++++++++++++++++++++--------
>  opcodes/riscv-opc.c        |  23 ++++++-
>  3 files changed, 123 insertions(+), 28 deletions(-)
>
> diff --git a/gas/config/tc-riscv.c b/gas/config/tc-riscv.c
> index e8061217e7cd..e28818b4fd15 100644
> --- a/gas/config/tc-riscv.c
> +++ b/gas/config/tc-riscv.c
> @@ -65,6 +65,7 @@ enum riscv_csr_class
>    CSR_CLASS_F,		/* f-ext only */
>    CSR_CLASS_ZKR,	/* zkr only */
>    CSR_CLASS_V,		/* rvv only */
> +  CSR_CLASS_H,		/* hypervisor extension only */
>    CSR_CLASS_DEBUG	/* debug CSR */
>  };
>
> @@ -905,6 +906,10 @@ riscv_csr_address (const char *csr_name,
>        result = riscv_subset_supports (&riscv_rps_as, "v");
>        need_check_version = false;
>        break;
> +    case CSR_CLASS_H:
> +      result = riscv_subset_supports (&riscv_rps_as, "h");
> +      need_check_version = false;
> +      break;
>      case CSR_CLASS_DEBUG:
>        need_check_version = false;
>        break;
> diff --git a/include/opcode/riscv-opc.h b/include/opcode/riscv-opc.h
> index 41c8ef163c42..542f875d051d 100644
> --- a/include/opcode/riscv-opc.h
> +++ b/include/opcode/riscv-opc.h
> @@ -243,12 +243,8 @@
>  #define MASK_URET  0xffffffff
>  #define MATCH_SRET 0x10200073
>  #define MASK_SRET  0xffffffff
> -#define MATCH_HRET 0x20200073
> -#define MASK_HRET  0xffffffff
>  #define MATCH_MRET 0x30200073
>  #define MASK_MRET  0xffffffff
> -#define MATCH_DRET 0x7b200073
> -#define MASK_DRET  0xffffffff
>  #define MATCH_SFENCE_VM 0x10400073
>  #define MASK_SFENCE_VM  0xfff07fff
>  #define MATCH_SFENCE_VMA 0x12000073
> @@ -1987,6 +1983,32 @@
>  #define MASK_VDOTUVV  0xfc00707f
>  #define MATCH_VFDOTVV  0xe4001057
>  #define MASK_VFDOTVV  0xfc00707f
> +
> +#define MASK_HLV      0xfff0707f
> +#define MATCH_HLVB    0x60004073
> +#define MATCH_HLVBU   0x60104073
> +#define MATCH_HLVH    0x64004073
> +#define MATCH_HLVHU   0x64104073
> +#define MATCH_HLVXHU  0x64304073
> +#define MATCH_HLVW    0x68004073
> +#define MATCH_HLVXWU  0x68304073
> +#define MATCH_HLVWU   0x68104073
> +#define MATCH_HLVD    0x6c004073
> +
> +#define MASK_HSV      0xfe007fff
> +#define MATCH_HSVB    0x62004073
> +#define MATCH_HSVH    0x66004073
> +#define MATCH_HSVW    0x6a004073
> +#define MATCH_HSVD    0x6e004073
> +
> +#define MASK_HFENCE       0xfe007fff
> +#define MATCH_HFENCE_VVMA 0x22000073
> +#define MATCH_HFENCE_GVMA 0x62000073
> +
> +#define MASK_HINVAL       0xfe007fff
> +#define MATCH_HINVAL_VVMA 0x26000073
> +#define MATCH_HINVAL_GVMA 0x66000073
> +
>  /* Privileged CSR addresses.  */
>  #define CSR_USTATUS 0x0
>  #define CSR_UIE 0x4
> @@ -2200,16 +2222,32 @@
>  #define CSR_MHPMEVENT29 0x33d
>  #define CSR_MHPMEVENT30 0x33e
>  #define CSR_MHPMEVENT31 0x33f
> -#define CSR_HSTATUS 0x200
> -#define CSR_HEDELEG 0x202
> -#define CSR_HIDELEG 0x203
> -#define CSR_HIE 0x204
> -#define CSR_HTVEC 0x205
> -#define CSR_HSCRATCH 0x240
> -#define CSR_HEPC 0x241
> -#define CSR_HCAUSE 0x242
> -#define CSR_HBADADDR 0x243
> -#define CSR_HIP 0x244
> +#define CSR_HSTATUS     0x600
> +#define CSR_HEDELEG     0x602
> +#define CSR_HIDELEG     0x603
> +#define CSR_HIE         0x604
> +#define CSR_HCOUNTEREN  0x606
> +#define CSR_HGEIE       0x607
> +#define CSR_HTVAL       0x643
> +#define CSR_HIP         0x644
> +#define CSR_HVIP        0x645
> +#define CSR_HTINST      0x64a
> +#define CSR_HGEIP       0xe12
> +#define CSR_HENVCFG     0x60a
> +#define CSR_HENVCFGH    0x61a
> +#define CSR_HGATP       0x680
> +#define CSR_HCONTEXT    0x6a8
> +#define CSR_HTIMEDELTA  0x605
> +#define CSR_HTIMEDELTAH 0x615
> +#define CSR_VSSTATUS    0x200
> +#define CSR_VSIE        0x204
> +#define CSR_VSTVEC      0x205
> +#define CSR_VSSCRATCH   0x240
> +#define CSR_VSEPC       0x241
> +#define CSR_VSCAUSE     0x242
> +#define CSR_VSTVAL      0x243
> +#define CSR_VSIP        0x244
> +#define CSR_VSATP       0x280
>  #define CSR_MBASE 0x380
>  #define CSR_MBOUND 0x381
>  #define CSR_MIBASE 0x382
> @@ -2354,9 +2392,7 @@ DECLARE_INSN(ecall, MATCH_ECALL, MASK_ECALL)
>  DECLARE_INSN(ebreak, MATCH_EBREAK, MASK_EBREAK)
>  DECLARE_INSN(uret, MATCH_URET, MASK_URET)
>  DECLARE_INSN(sret, MATCH_SRET, MASK_SRET)
> -DECLARE_INSN(hret, MATCH_HRET, MASK_HRET)
>  DECLARE_INSN(mret, MATCH_MRET, MASK_MRET)
> -DECLARE_INSN(dret, MATCH_DRET, MASK_DRET)
>  DECLARE_INSN(sfence_vm, MATCH_SFENCE_VM, MASK_SFENCE_VM)
>  DECLARE_INSN(sfence_vma, MATCH_SFENCE_VMA, MASK_SFENCE_VMA)
>  DECLARE_INSN(wfi, MATCH_WFI, MASK_WFI)
> @@ -2549,6 +2585,24 @@ DECLARE_INSN(c_sd, MATCH_C_SD, MASK_C_SD)
>  DECLARE_INSN(c_addiw, MATCH_C_ADDIW, MASK_C_ADDIW)
>  DECLARE_INSN(c_ldsp, MATCH_C_LDSP, MASK_C_LDSP)
>  DECLARE_INSN(c_sdsp, MATCH_C_SDSP, MASK_C_SDSP)
> +/* Hypervisor instructions.  */
> +DECLARE_INSN(hlv_b,   MATCH_HLVB,   MASK_HLV)
> +DECLARE_INSN(hlv_h,   MATCH_HLVH,   MASK_HLV)
> +DECLARE_INSN(hlv_w,   MATCH_HLVW,   MASK_HLV)
> +DECLARE_INSN(hlv_d,   MATCH_HLVD,   MASK_HLV)
> +DECLARE_INSN(hlv_bu,  MATCH_HLVBU,  MASK_HLV)
> +DECLARE_INSN(hlv_hu,  MATCH_HLVHU,  MASK_HLV)
> +DECLARE_INSN(hlv_wu,  MATCH_HLVWU,  MASK_HLV)
> +DECLARE_INSN(hlvx_hu, MATCH_HLVXHU, MASK_HLV)
> +DECLARE_INSN(hlvx_wu, MATCH_HLVXWU, MASK_HLV)
> +DECLARE_INSN(hsv_b,   MATCH_HSVB,   MASK_HSV)
> +DECLARE_INSN(hsv_h,   MATCH_HSVH,   MASK_HSV)
> +DECLARE_INSN(hsv_w,   MATCH_HSVW,   MASK_HSV)
> +DECLARE_INSN(hsv_d,   MATCH_HSVD,   MASK_HSV)
> +DECLARE_INSN(hfence_vvma, MATCH_HFENCE_VVMA, MASK_HFENCE)
> +DECLARE_INSN(hfence_gvma, MATCH_HFENCE_GVMA, MASK_HFENCE)
> +DECLARE_INSN(hinval_vvma, MATCH_HINVAL_VVMA, MASK_HINVAL)
> +DECLARE_INSN(hinval_gvma, MATCH_HINVAL_GVMA, MASK_HINVAL)
>  #endif /* DECLARE_INSN */
>  #ifdef DECLARE_CSR
>  /* Privileged CSRs.  */
> @@ -2764,17 +2818,34 @@ DECLARE_CSR(mhpmevent28, CSR_MHPMEVENT28, CSR_CLASS_I, PRIV_SPEC_CLASS_1P9P1, PR
>  DECLARE_CSR(mhpmevent29, CSR_MHPMEVENT29, CSR_CLASS_I, PRIV_SPEC_CLASS_1P9P1, PRIV_SPEC_CLASS_DRAFT)
>  DECLARE_CSR(mhpmevent30, CSR_MHPMEVENT30, CSR_CLASS_I, PRIV_SPEC_CLASS_1P9P1, PRIV_SPEC_CLASS_DRAFT)
>  DECLARE_CSR(mhpmevent31, CSR_MHPMEVENT31, CSR_CLASS_I, PRIV_SPEC_CLASS_1P9P1, PRIV_SPEC_CLASS_DRAFT)
> +
> +DECLARE_CSR(hstatus,     CSR_HSTATUS,     CSR_CLASS_H, PRIV_SPEC_CLASS_1P9P1, PRIV_SPEC_CLASS_DRAFT)
> +DECLARE_CSR(hedeleg,     CSR_HEDELEG,     CSR_CLASS_H, PRIV_SPEC_CLASS_1P9P1, PRIV_SPEC_CLASS_DRAFT)
> +DECLARE_CSR(hideleg,     CSR_HIDELEG,     CSR_CLASS_H, PRIV_SPEC_CLASS_1P9P1, PRIV_SPEC_CLASS_DRAFT)
> +DECLARE_CSR(hie,         CSR_HIE,         CSR_CLASS_H, PRIV_SPEC_CLASS_1P9P1, PRIV_SPEC_CLASS_DRAFT)
> +DECLARE_CSR(hcounteren,  CSR_HCOUNTEREN,  CSR_CLASS_H, PRIV_SPEC_CLASS_1P9P1, PRIV_SPEC_CLASS_DRAFT)
> +DECLARE_CSR(hgeie,       CSR_HGEIE,       CSR_CLASS_H, PRIV_SPEC_CLASS_1P9P1, PRIV_SPEC_CLASS_DRAFT)
> +DECLARE_CSR(htval,       CSR_HTVAL,       CSR_CLASS_H, PRIV_SPEC_CLASS_1P9P1, PRIV_SPEC_CLASS_DRAFT)
> +DECLARE_CSR(hip,         CSR_HIP,         CSR_CLASS_H, PRIV_SPEC_CLASS_1P9P1, PRIV_SPEC_CLASS_DRAFT)
> +DECLARE_CSR(hvip,        CSR_HVIP,        CSR_CLASS_H, PRIV_SPEC_CLASS_1P9P1, PRIV_SPEC_CLASS_DRAFT)
> +DECLARE_CSR(htinst,      CSR_HTINST,      CSR_CLASS_H, PRIV_SPEC_CLASS_1P9P1, PRIV_SPEC_CLASS_DRAFT)
> +DECLARE_CSR(hgeip,       CSR_HGEIP,       CSR_CLASS_H, PRIV_SPEC_CLASS_1P9P1, PRIV_SPEC_CLASS_DRAFT)
> +DECLARE_CSR(henvcfg,     CSR_HENVCFG,     CSR_CLASS_H, PRIV_SPEC_CLASS_1P9P1, PRIV_SPEC_CLASS_DRAFT)
> +DECLARE_CSR(henvcfgh,    CSR_HENVCFGH,    CSR_CLASS_H, PRIV_SPEC_CLASS_1P9P1, PRIV_SPEC_CLASS_DRAFT)
> +DECLARE_CSR(hgatp,       CSR_HGATP,       CSR_CLASS_H, PRIV_SPEC_CLASS_1P9P1, PRIV_SPEC_CLASS_DRAFT)
> +DECLARE_CSR(hcontext,    CSR_HCONTEXT,    CSR_CLASS_H, PRIV_SPEC_CLASS_1P9P1, PRIV_SPEC_CLASS_DRAFT)
> +DECLARE_CSR(htimedelta,  CSR_HTIMEDELTA,  CSR_CLASS_H, PRIV_SPEC_CLASS_1P9P1, PRIV_SPEC_CLASS_DRAFT)
> +DECLARE_CSR(htimedeltah, CSR_HTIMEDELTAH, CSR_CLASS_H, PRIV_SPEC_CLASS_1P9P1, PRIV_SPEC_CLASS_DRAFT)
> +DECLARE_CSR(vsstatus,    CSR_VSSTATUS,    CSR_CLASS_H, PRIV_SPEC_CLASS_1P9P1, PRIV_SPEC_CLASS_DRAFT)
> +DECLARE_CSR(vsie,        CSR_VSIE,        CSR_CLASS_H, PRIV_SPEC_CLASS_1P9P1, PRIV_SPEC_CLASS_DRAFT)
> +DECLARE_CSR(vstvec,      CSR_VSTVEC,      CSR_CLASS_H, PRIV_SPEC_CLASS_1P9P1, PRIV_SPEC_CLASS_DRAFT)
> +DECLARE_CSR(vsscratch,   CSR_VSSCRATCH,   CSR_CLASS_H, PRIV_SPEC_CLASS_1P9P1, PRIV_SPEC_CLASS_DRAFT)
> +DECLARE_CSR(vsepc,       CSR_VSEPC,       CSR_CLASS_H, PRIV_SPEC_CLASS_1P9P1, PRIV_SPEC_CLASS_DRAFT)
> +DECLARE_CSR(vscause,     CSR_VSCAUSE,     CSR_CLASS_H, PRIV_SPEC_CLASS_1P9P1, PRIV_SPEC_CLASS_DRAFT)
> +DECLARE_CSR(vstval,      CSR_VSTVAL,      CSR_CLASS_H, PRIV_SPEC_CLASS_1P9P1, PRIV_SPEC_CLASS_DRAFT)
> +DECLARE_CSR(vsip,        CSR_VSIP,        CSR_CLASS_H, PRIV_SPEC_CLASS_1P9P1, PRIV_SPEC_CLASS_DRAFT)
> +DECLARE_CSR(vsatp,       CSR_VSATP,       CSR_CLASS_H, PRIV_SPEC_CLASS_1P9P1, PRIV_SPEC_CLASS_DRAFT)
>  /* Dropped CSRs.  */
> -DECLARE_CSR(hstatus, CSR_HSTATUS, CSR_CLASS_I, PRIV_SPEC_CLASS_1P9P1, PRIV_SPEC_CLASS_1P10)
> -DECLARE_CSR(hedeleg, CSR_HEDELEG, CSR_CLASS_I, PRIV_SPEC_CLASS_1P9P1, PRIV_SPEC_CLASS_1P10)
> -DECLARE_CSR(hideleg, CSR_HIDELEG, CSR_CLASS_I, PRIV_SPEC_CLASS_1P9P1, PRIV_SPEC_CLASS_1P10)
> -DECLARE_CSR(hie, CSR_HIE, CSR_CLASS_I, PRIV_SPEC_CLASS_1P9P1, PRIV_SPEC_CLASS_1P10)
> -DECLARE_CSR(htvec, CSR_HTVEC, CSR_CLASS_I, PRIV_SPEC_CLASS_1P9P1, PRIV_SPEC_CLASS_1P10)
> -DECLARE_CSR(hscratch, CSR_HSCRATCH, CSR_CLASS_I, PRIV_SPEC_CLASS_1P9P1, PRIV_SPEC_CLASS_1P10)
> -DECLARE_CSR(hepc, CSR_HEPC, CSR_CLASS_I, PRIV_SPEC_CLASS_1P9P1, PRIV_SPEC_CLASS_1P10)
> -DECLARE_CSR(hcause, CSR_HCAUSE, CSR_CLASS_I, PRIV_SPEC_CLASS_1P9P1, PRIV_SPEC_CLASS_1P10)
> -DECLARE_CSR(hbadaddr, CSR_HBADADDR, CSR_CLASS_I, PRIV_SPEC_CLASS_1P9P1, PRIV_SPEC_CLASS_1P10)
> -DECLARE_CSR(hip, CSR_HIP, CSR_CLASS_I, PRIV_SPEC_CLASS_1P9P1, PRIV_SPEC_CLASS_1P10)
>  DECLARE_CSR(mbase, CSR_MBASE, CSR_CLASS_I, PRIV_SPEC_CLASS_1P9P1, PRIV_SPEC_CLASS_1P10)
>  DECLARE_CSR(mbound, CSR_MBOUND, CSR_CLASS_I, PRIV_SPEC_CLASS_1P9P1, PRIV_SPEC_CLASS_1P10)
>  DECLARE_CSR(mibase, CSR_MIBASE, CSR_CLASS_I, PRIV_SPEC_CLASS_1P9P1, PRIV_SPEC_CLASS_1P10)
> diff --git a/opcodes/riscv-opc.c b/opcodes/riscv-opc.c
> index 40037db83c05..794eee2d413f 100644
> --- a/opcodes/riscv-opc.c
> +++ b/opcodes/riscv-opc.c
> @@ -839,9 +839,7 @@ const struct riscv_opcode riscv_opcodes[] =
>  {"csrrc",      0, INSN_CLASS_ZICSR,"d,E,Z",    MATCH_CSRRCI, MASK_CSRRCI, match_opcode, INSN_ALIAS },
>  {"uret",       0, INSN_CLASS_I,    "",         MATCH_URET, MASK_URET, match_opcode, 0 },
>  {"sret",       0, INSN_CLASS_I,    "",         MATCH_SRET, MASK_SRET, match_opcode, 0 },
> -{"hret",       0, INSN_CLASS_I,    "",         MATCH_HRET, MASK_HRET, match_opcode, 0 },
>  {"mret",       0, INSN_CLASS_I,    "",         MATCH_MRET, MASK_MRET, match_opcode, 0 },
> -{"dret",       0, INSN_CLASS_I,    "",         MATCH_DRET, MASK_DRET, match_opcode, 0 },

I don't know where the debug stuff ended up going, but dret was in some 
earlier specs and was used.  I guess we should split it out into a D 
spec of some sort.

>  {"sfence.vm",  0, INSN_CLASS_I,    "",         MATCH_SFENCE_VM, MASK_SFENCE_VM | MASK_RS1, match_opcode, 0 },
>  {"sfence.vm",  0, INSN_CLASS_I,    "s",        MATCH_SFENCE_VM, MASK_SFENCE_VM, match_opcode, 0 },
>  {"sfence.vma", 0, INSN_CLASS_I,    "",         MATCH_SFENCE_VMA, MASK_SFENCE_VMA|MASK_RS1|MASK_RS2, match_opcode, INSN_ALIAS },
> @@ -1725,6 +1723,27 @@ const struct riscv_opcode riscv_opcodes[] =
>  {"vmv4r.v",    0, INSN_CLASS_V, "Vd,Vt", MATCH_VMV4RV, MASK_VMV4RV, match_opcode, 0},
>  {"vmv8r.v",    0, INSN_CLASS_V, "Vd,Vt", MATCH_VMV8RV, MASK_VMV8RV, match_opcode, 0},
>
> +/* Hypervisor instructions.  */
> +{"hlv.b",       0, INSN_CLASS_H, "d,(s)", MATCH_HLVB,   MASK_HLV, match_opcode, INSN_DREF|INSN_1_BYTE },
> +{"hlv.bu",      0, INSN_CLASS_H, "d,(s)", MATCH_HLVBU,  MASK_HLV, match_opcode, INSN_DREF|INSN_1_BYTE },
> +{"hlv.h",       0, INSN_CLASS_H, "d,(s)", MATCH_HLVH,   MASK_HLV, match_opcode, INSN_DREF|INSN_2_BYTE },
> +{"hlv.hu",      0, INSN_CLASS_H, "d,(s)", MATCH_HLVHU,  MASK_HLV, match_opcode, INSN_DREF|INSN_2_BYTE },
> +{"hlvx.hu",     0, INSN_CLASS_H, "d,(s)", MATCH_HLVXHU, MASK_HLV, match_opcode, INSN_DREF|INSN_2_BYTE },
> +{"hlv.w",       0, INSN_CLASS_H, "d,(s)", MATCH_HLVW,   MASK_HLV, match_opcode, INSN_DREF|INSN_4_BYTE },
> +{"hlv.wu",     64, INSN_CLASS_H, "d,(s)", MATCH_HLVWU,  MASK_HLV, match_opcode, INSN_DREF|INSN_4_BYTE },
> +{"hlvx.wu",     0, INSN_CLASS_H, "d,(s)", MATCH_HLVXWU, MASK_HLV, match_opcode, INSN_DREF|INSN_4_BYTE },
> +{"hlv.d",      64, INSN_CLASS_H, "d,(s)", MATCH_HLVD,   MASK_HLV, match_opcode, INSN_DREF|INSN_8_BYTE },
> +
> +{"hsv.b",       0, INSN_CLASS_H, "t,(s)", MATCH_HSVB,   MASK_HSV, match_opcode, INSN_DREF|INSN_1_BYTE },
> +{"hsv.h",       0, INSN_CLASS_H, "t,(s)", MATCH_HSVH,   MASK_HSV, match_opcode, INSN_DREF|INSN_2_BYTE },
> +{"hsv.w",       0, INSN_CLASS_H, "t,(s)", MATCH_HSVW,   MASK_HSV, match_opcode, INSN_DREF|INSN_4_BYTE },
> +{"hsv.d",      64, INSN_CLASS_H, "t,(s)", MATCH_HSVD,   MASK_HSV, match_opcode, INSN_DREF|INSN_8_BYTE },
> +
> +{"hfence.vvma", 0, INSN_CLASS_H, "t,s", MATCH_HFENCE_VVMA, MASK_HFENCE, match_opcode, 0 },
> +{"hfence.gvma", 0, INSN_CLASS_H, "t,s", MATCH_HFENCE_GVMA, MASK_HFENCE, match_opcode, 0 },
> +{"hinval.vvma", 0, INSN_CLASS_H, "t,s", MATCH_HINVAL_VVMA, MASK_HINVAL, match_opcode, 0 },
> +{"hinval.gvma", 0, INSN_CLASS_H, "t,s", MATCH_HINVAL_GVMA, MASK_HINVAL, match_opcode, 0 },
> +
>  /* Terminate the list.  */
>  {0, 0, INSN_CLASS_NONE, 0, 0, 0, 0, 0}
>  };

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

* Re: [RFC 4/4] RISC-V: fix a comment for adding CSR entry and annotate switch-break
  2021-12-16 17:33 ` [RFC 4/4] RISC-V: fix a comment for adding CSR entry and annotate switch-break Vineet Gupta
@ 2021-12-17  4:10   ` Palmer Dabbelt
  0 siblings, 0 replies; 17+ messages in thread
From: Palmer Dabbelt @ 2021-12-17  4:10 UTC (permalink / raw)
  To: Vineet Gupta; +Cc: binutils, Nelson Chu, kito.cheng, Jim Wilson, Vineet Gupta

On Thu, 16 Dec 2021 09:33:28 PST (-0800), Vineet Gupta wrote:
> Nothing functional ...
>
> Signed-off-by: Vineet Gupta <vineetg@rivosinc.com>
> ---
>  gas/config/tc-riscv.c | 10 +++++-----
>  1 file changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/gas/config/tc-riscv.c b/gas/config/tc-riscv.c
> index e28818b4fd15..082dc59aa9f9 100644
> --- a/gas/config/tc-riscv.c
> +++ b/gas/config/tc-riscv.c
> @@ -83,7 +83,7 @@ struct riscv_csr_extra
>    enum riscv_spec_class define_version;
>
>    /* Record the CSR is aborted/invalid from which versions.  If it isn't
> -     aborted in the current version, then it should be CSR_CLASS_VDRAFT.  */
> +     aborted in the current version, then it should be PRIV_SPEC_CLASS_DRAFT.  */
>    enum riscv_spec_class abort_version;
>
>    /* The CSR may have more than one setting.  */
> @@ -1109,7 +1109,7 @@ validate_riscv_insn (const struct riscv_opcode *opc, int length)
>  	    default:
>  	      goto unknown_validate_operand;
>  	    }
> -	  break;
> +	  break;  /* end RVC */
>  	case 'V': /* RVV */
>  	  switch (*++oparg)
>  	    {
> @@ -1133,7 +1133,7 @@ validate_riscv_insn (const struct riscv_opcode *opc, int length)
>  	    default:
>  	      goto unknown_validate_operand;
>  	    }
> -	  break;
> +	  break; /* end RVV */
>  	case ',': break;
>  	case '(': break;
>  	case ')': break;
> @@ -2610,7 +2610,7 @@ riscv_ip (char *str, struct riscv_cl_insn *ip, expressionS *imm_expr,
>  		default:
>  		  goto unknown_riscv_ip_operand;
>  		}
> -	      break;
> +	      break; /* end RVC */
>
>  	    case 'V': /* RVV */
>  	      switch (*++oparg)
> @@ -2776,7 +2776,7 @@ riscv_ip (char *str, struct riscv_cl_insn *ip, expressionS *imm_expr,
>  		default:
>  		  goto unknown_riscv_ip_operand;
>  		}
> -	      break;
> +	      break; /* end RVV */
>
>  	    case ',':
>  	      ++argnum;

Reviewed-by: Palmer Dabbelt <palmer@rivosinc.com>

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

* Re: [RFC 1/4] RISC-V: Hypervisor ext: Treat as "Standard" extension
  2021-12-17  4:10   ` Palmer Dabbelt
@ 2021-12-17 16:10     ` Nelson Chu
  2021-12-17 22:40       ` Vineet Gupta
  2021-12-17 22:50       ` Palmer Dabbelt
  0 siblings, 2 replies; 17+ messages in thread
From: Nelson Chu @ 2021-12-17 16:10 UTC (permalink / raw)
  To: Palmer Dabbelt; +Cc: Vineet Gupta, Binutils, Kito Cheng, Jim Wilson

On Fri, Dec 17, 2021 at 12:10 PM Palmer Dabbelt <palmer@dabbelt.com> wrote:
>
> On Thu, 16 Dec 2021 09:33:25 PST (-0800), Vineet Gupta wrote:
> > gas currently fails for single-letter 'h' prefix in arch string.
> >
> > | riscv64-unknown-elf-as -march=rv64gh empty.s
> > | Assembler messages:
> > | Error: rv64gh: unknown prefixed ISA extension `h'
> >
> > It implements the "multi-letter" prefix for H-ext as recommended in
> > ISA manual subsection "ISA Extension Naming Conventions". However given
> > the discussions at [1] there seems to be no pressing need to keep version
> > for the current H-ext since
> >  - it will remain at 1.0
> >  - any future Hypervisor extensions wull called "Sh*" or some such
> >  - H was the original standard extension with its own MISA bit
> >
> > This patch thus relaxes the constraint and allows single letter prefix
> > 'h'
> >
> > Note: To keep the binutils dejagnu failures same, I've fixed the
> > testsuite files within the same patch. I can break it up if reviewers
> > feel as such.
> >
> > [1] https://github.com/riscv/riscv-isa-manual/issues/781#issuecomment-983233088
> >
> > bfd/
> >       * elfxx-riscv.c (riscv_supported_std_ext): Add "h" entry.
> >       (riscv_supported_std_h_ext): Add "h" entry.
> >       (riscv_get_default_ext_version): Handle PRIV_SPEC_CLASS_DRAFT.
> >       (riscv_multi_subset_supports): Handle INSN_CLASS_H.
> >
> > gas/
> >       * testsuite/gas/riscv/march-fail-single-prefix-h.d: Deleted as
> >       single-letter prefix 'h' is now valid.
> >       * testsuite/gas/riscv/march-fail-single-prefix-l: Removed 'h'.
> >
> > include/
> >       * opcode/riscv.h (enum riscv_insn_class): Add INSN_CLASS_H.
> >
> > Signed-off-by: Vineet Gupta <vineetg@rivosinc.com>
> > ---
> >  bfd/elfxx-riscv.c                                    | 5 +++++
> >  gas/testsuite/gas/riscv/march-fail-single-prefix-h.d | 3 ---
> >  gas/testsuite/gas/riscv/march-fail-single-prefix.l   | 2 +-
> >  include/opcode/riscv.h                               | 1 +
> >  4 files changed, 7 insertions(+), 4 deletions(-)
> >  delete mode 100644 gas/testsuite/gas/riscv/march-fail-single-prefix-h.d
> >
> > diff --git a/bfd/elfxx-riscv.c b/bfd/elfxx-riscv.c
> > index 3bd41ff2b551..7d5ae5a4657d 100644
> > --- a/bfd/elfxx-riscv.c
> > +++ b/bfd/elfxx-riscv.c
> > @@ -1173,6 +1173,7 @@ static struct riscv_supported_ext riscv_supported_std_ext[] =
> >    {"t",              ISA_SPEC_CLASS_NONE, RISCV_UNKNOWN_VERSION, RISCV_UNKNOWN_VERSION, 0 },
> >    {"p",              ISA_SPEC_CLASS_NONE, RISCV_UNKNOWN_VERSION, RISCV_UNKNOWN_VERSION, 0 },
> >    {"v",              ISA_SPEC_CLASS_DRAFT,           1, 0, 0 },
> > +  {"h",              PRIV_SPEC_CLASS_DRAFT,          1, 0, 0 },
>
> According to
> <https://wiki.riscv.org/display/TECH/Recently+Ratified+Extensions>, this
> was ratified in November so we should be able to update this to
> something like PRIV_SPEC_CLASS_1P12 (the V stuff was also ratified, but
> that's a different issue).

I can understand that why Vineet are doing these changes, but I had a
quick talk with Kito this morning, and I think we also have three
issues for now,

1. The current draft ISA spec doesn't have any related changes, so if
we change the `h' as a single standard extension, then this will
conflict with the spec.

2. Even if we decide to change the `h' as a single extension rather
than the multiple prefixed keyword, then in what order should we place
h.  I expect the table 27.1 in the ISA spec will also mention the
order of the single h.

3. I never considered that an extension version may be controlled by
the privileged spec before, so in the riscv_get_default_ext_version,

> >        if (strcmp (table[i].name, name) == 0
> >         && (table[i].isa_spec_class == ISA_SPEC_CLASS_DRAFT
> > +           || table[i].isa_spec_class == PRIV_SPEC_CLASS_DRAFT
> >             || table[i].isa_spec_class == *default_isa_spec))

We will need to rewrite the related code, since the isa_spec_class may
be one of the PRIV_SPEC_CLASS_XXX in the future, so we will never
match it to the default_isa_spec...

> >    {"n",              ISA_SPEC_CLASS_NONE, RISCV_UNKNOWN_VERSION, RISCV_UNKNOWN_VERSION, 0 },
> >    {NULL, 0, 0, 0, 0}
> >  };
> > @@ -1232,6 +1233,7 @@ static struct riscv_supported_ext riscv_supported_std_s_ext[] =
> >
> >  static struct riscv_supported_ext riscv_supported_std_h_ext[] =
> >  {
> > +  {"h",                 PRIV_SPEC_CLASS_DRAFT,          1, 0, 0 },
> >    {NULL, 0, 0, 0, 0}
> >  };
> >
> > @@ -1531,6 +1533,7 @@ riscv_get_default_ext_version (enum riscv_spec_class *default_isa_spec,
> >      {
> >        if (strcmp (table[i].name, name) == 0
> >         && (table[i].isa_spec_class == ISA_SPEC_CLASS_DRAFT
> > +           || table[i].isa_spec_class == PRIV_SPEC_CLASS_DRAFT
> >             || table[i].isa_spec_class == *default_isa_spec))
> >       {
> >         *major_version = table[i].major_version;
> > @@ -2412,6 +2415,8 @@ riscv_multi_subset_supports (riscv_parse_subset_t *rps,
> >             || riscv_subset_supports (rps, "zve64d")
> >             || riscv_subset_supports (rps, "zve64f")
> >             || riscv_subset_supports (rps, "zve32f"));
> > +    case INSN_CLASS_H:
> > +      return riscv_subset_supports (rps, "h");
> >      default:
> >        rps->error_handler
> >          (_("internal: unreachable INSN_CLASS_*"));
> > diff --git a/gas/testsuite/gas/riscv/march-fail-single-prefix-h.d b/gas/testsuite/gas/riscv/march-fail-single-prefix-h.d
> > deleted file mode 100644
> > index eb101bd71353..000000000000
> > --- a/gas/testsuite/gas/riscv/march-fail-single-prefix-h.d
> > +++ /dev/null
> > @@ -1,3 +0,0 @@
> > -#as: -march=rv32ih
> > -#source: empty.s
> > -#error_output: march-fail-single-prefix.l
> > diff --git a/gas/testsuite/gas/riscv/march-fail-single-prefix.l b/gas/testsuite/gas/riscv/march-fail-single-prefix.l
> > index 13942ed66642..e4418de69acb 100644
> > --- a/gas/testsuite/gas/riscv/march-fail-single-prefix.l
> > +++ b/gas/testsuite/gas/riscv/march-fail-single-prefix.l
> > @@ -1,2 +1,2 @@
> >  .*Assembler messages:
> > -.*Error: .*unknown prefixed ISA extension `(s|h|z|x|zxm)'
> > +.*Error: .*unknown prefixed ISA extension `(s|z|x|zxm)'
> > diff --git a/include/opcode/riscv.h b/include/opcode/riscv.h
> > index 14889972abce..2783bdecaae5 100644
> > --- a/include/opcode/riscv.h
> > +++ b/include/opcode/riscv.h
> > @@ -387,6 +387,7 @@ enum riscv_insn_class
> >    INSN_CLASS_ZKND_OR_ZKNE,
> >    INSN_CLASS_V,
> >    INSN_CLASS_ZVEF,
> > +  INSN_CLASS_H,
> >  };
> >
> >  /* This structure holds information for a particular instruction.  */
>
> Reviewed-by: Palmer Dabbelt <palmer@rivosinc.com>
>
> We can always deal with the draft/ratified stuff later, as there's a
> bunch of them.

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

* Re: [RFC 2/4] RISC-V: Hypervisor ext: CSR and Instructions
  2021-12-17  4:10   ` Palmer Dabbelt
@ 2021-12-17 17:30     ` Nelson Chu
  2021-12-18  1:44       ` Vineet Gupta
  0 siblings, 1 reply; 17+ messages in thread
From: Nelson Chu @ 2021-12-17 17:30 UTC (permalink / raw)
  To: Palmer Dabbelt; +Cc: Vineet Gupta, Binutils, Kito Cheng, Jim Wilson

On Fri, Dec 17, 2021 at 12:10 PM Palmer Dabbelt <palmer@dabbelt.com> wrote:
>
> On Thu, 16 Dec 2021 09:33:26 PST (-0800), Vineet Gupta wrote:
> > This implements Hypervisor Extension 1.0 [1]
> >
> >  - Hypervisor Memory-Management Instructions
> >    HFENCE.VVMA, HFENCE.GVMA,
> >    HINVAL.GVMA, HINVAL.VVMA
> >
> >  - Hypervisor Virtual Machine Load and Store Instructions
> >    HLV.B, HLV.BU,          HSV.B,
> >    HLV.H, HLV.HU, HLVX.HU, HSB.H,
> >    HLV.W, HLV.WU, HLVX.WU, HSV.W,
> >    HLV.D,                  HSV.D
> >
> >  - Hypervisor CSRs (some new, some address changed)
> >    hstatus, hedeleg, hideleg, hie, hcounteren, hgeie, htval, hip, hvip,
> >    htinst, hgeip, henvcfg, henvcfgh, hgatp, hcontext, htimedelta, htimedeltah,
> >    vsstatus, vsie, vstvec, vsscratch, vsepc, vscause, vstval, vsip, vsatp,
> >
> > This removed a couple of stale instructions: HRET, DRET.
> >
> > [1] https://github.com/riscv/riscv-isa-manual/releases/tag/draft-20211213-ea38395
> >
> > gas/
> >       * config/tc-riscv.c (enum riscv_csr_class): Added CSR_CLASS_H.
> >       (riscv_csr_address): check CSR_CLASS_H.
> >
> > include/
> >       * opcode/riscv-opc.h: Removed {MATCH,MASK}_{D,H}RET.
> >       Added {MATCH,MASK}_{HLV*,HSV*,HFENCE*,HINVAL*}.
> >       Added/updated CSR_H*, CSR_V*.
> >       DECLARE_INSN(hlv*), DECLARE_INSN(hsv*).
> >       DECLARE_INSN(hfence*), DECLARE_INSN(hinval*).
> >       DECLARE_CSR(h*), DECLARE_CSR(v*).
> >
> > opcodes/
> >       * riscv-opc.c (riscv_opcodes): Removed {d,h}ret.
> >       Added hlv*, hsv*, hfence*, hinval*.
> >
> > Signed-off-by: Vineet Gupta <vineetg@rivosinc.com>
> > ---
> >  gas/config/tc-riscv.c      |   5 ++
> >  include/opcode/riscv-opc.h | 123 +++++++++++++++++++++++++++++--------
> >  opcodes/riscv-opc.c        |  23 ++++++-
> >  3 files changed, 123 insertions(+), 28 deletions(-)
> >
> > diff --git a/gas/config/tc-riscv.c b/gas/config/tc-riscv.c
> > index e8061217e7cd..e28818b4fd15 100644
> > --- a/gas/config/tc-riscv.c
> > +++ b/gas/config/tc-riscv.c
> > @@ -65,6 +65,7 @@ enum riscv_csr_class
> >    CSR_CLASS_F,               /* f-ext only */
> >    CSR_CLASS_ZKR,     /* zkr only */
> >    CSR_CLASS_V,               /* rvv only */
> > +  CSR_CLASS_H,               /* hypervisor extension only */
> >    CSR_CLASS_DEBUG    /* debug CSR */
> >  };
> >
> > @@ -905,6 +906,10 @@ riscv_csr_address (const char *csr_name,
> >        result = riscv_subset_supports (&riscv_rps_as, "v");
> >        need_check_version = false;
> >        break;
> > +    case CSR_CLASS_H:
> > +      result = riscv_subset_supports (&riscv_rps_as, "h");
> > +      need_check_version = false;
> > +      break;
> >      case CSR_CLASS_DEBUG:
> >        need_check_version = false;
> >        break;
> > diff --git a/include/opcode/riscv-opc.h b/include/opcode/riscv-opc.h
> > index 41c8ef163c42..542f875d051d 100644
> > --- a/include/opcode/riscv-opc.h
> > +++ b/include/opcode/riscv-opc.h
> > @@ -243,12 +243,8 @@
> >  #define MASK_URET  0xffffffff
> >  #define MATCH_SRET 0x10200073
> >  #define MASK_SRET  0xffffffff
> > -#define MATCH_HRET 0x20200073
> > -#define MASK_HRET  0xffffffff
> >  #define MATCH_MRET 0x30200073
> >  #define MASK_MRET  0xffffffff
> > -#define MATCH_DRET 0x7b200073
> > -#define MASK_DRET  0xffffffff
> >  #define MATCH_SFENCE_VM 0x10400073
> >  #define MASK_SFENCE_VM  0xfff07fff
> >  #define MATCH_SFENCE_VMA 0x12000073
> > @@ -1987,6 +1983,32 @@
> >  #define MASK_VDOTUVV  0xfc00707f
> >  #define MATCH_VFDOTVV  0xe4001057
> >  #define MASK_VFDOTVV  0xfc00707f
> > +
> > +#define MASK_HLV      0xfff0707f
> > +#define MATCH_HLVB    0x60004073
> > +#define MATCH_HLVBU   0x60104073
> > +#define MATCH_HLVH    0x64004073
> > +#define MATCH_HLVHU   0x64104073
> > +#define MATCH_HLVXHU  0x64304073
> > +#define MATCH_HLVW    0x68004073
> > +#define MATCH_HLVXWU  0x68304073
> > +#define MATCH_HLVWU   0x68104073
> > +#define MATCH_HLVD    0x6c004073
> > +
> > +#define MASK_HSV      0xfe007fff
> > +#define MATCH_HSVB    0x62004073
> > +#define MATCH_HSVH    0x66004073
> > +#define MATCH_HSVW    0x6a004073
> > +#define MATCH_HSVD    0x6e004073
> > +
> > +#define MASK_HFENCE       0xfe007fff
> > +#define MATCH_HFENCE_VVMA 0x22000073
> > +#define MATCH_HFENCE_GVMA 0x62000073
> > +
> > +#define MASK_HINVAL       0xfe007fff
> > +#define MATCH_HINVAL_VVMA 0x26000073
> > +#define MATCH_HINVAL_GVMA 0x66000073

We already have HINVAL in the mainline for now.

> >  /* Privileged CSR addresses.  */
> >  #define CSR_USTATUS 0x0
> >  #define CSR_UIE 0x4
> > @@ -2200,16 +2222,32 @@
> >  #define CSR_MHPMEVENT29 0x33d
> >  #define CSR_MHPMEVENT30 0x33e
> >  #define CSR_MHPMEVENT31 0x33f
> > -#define CSR_HSTATUS 0x200
> > -#define CSR_HEDELEG 0x202
> > -#define CSR_HIDELEG 0x203
> > -#define CSR_HIE 0x204
> > -#define CSR_HTVEC 0x205
> > -#define CSR_HSCRATCH 0x240
> > -#define CSR_HEPC 0x241
> > -#define CSR_HCAUSE 0x242
> > -#define CSR_HBADADDR 0x243
> > -#define CSR_HIP 0x244
> > +#define CSR_HSTATUS     0x600
> > +#define CSR_HEDELEG     0x602
> > +#define CSR_HIDELEG     0x603
> > +#define CSR_HIE         0x604
> > +#define CSR_HCOUNTEREN  0x606
> > +#define CSR_HGEIE       0x607
> > +#define CSR_HTVAL       0x643
> > +#define CSR_HIP         0x644
> > +#define CSR_HVIP        0x645
> > +#define CSR_HTINST      0x64a
> > +#define CSR_HGEIP       0xe12
> > +#define CSR_HENVCFG     0x60a
> > +#define CSR_HENVCFGH    0x61a
> > +#define CSR_HGATP       0x680
> > +#define CSR_HCONTEXT    0x6a8

The hcontext is one of the hypervisor csr, but it is also a debug csr.
I have sent a patch to define this csr before, and it is controlled by
the debug spec,
https://sourceware.org/pipermail/binutils/2021-August/117568.html

Maybe we should only choose one spec or one extension to control the
hcontext csr, it should be worth discussing.

> > +#define CSR_HTIMEDELTA  0x605
> > +#define CSR_HTIMEDELTAH 0x615
> > +#define CSR_VSSTATUS    0x200
> > +#define CSR_VSIE        0x204
> > +#define CSR_VSTVEC      0x205
> > +#define CSR_VSSCRATCH   0x240
> > +#define CSR_VSEPC       0x241
> > +#define CSR_VSCAUSE     0x242
> > +#define CSR_VSTVAL      0x243
> > +#define CSR_VSIP        0x244
> > +#define CSR_VSATP       0x280
> >  #define CSR_MBASE 0x380
> >  #define CSR_MBOUND 0x381
> >  #define CSR_MIBASE 0x382
> > @@ -2354,9 +2392,7 @@ DECLARE_INSN(ecall, MATCH_ECALL, MASK_ECALL)
> >  DECLARE_INSN(ebreak, MATCH_EBREAK, MASK_EBREAK)
> >  DECLARE_INSN(uret, MATCH_URET, MASK_URET)
> >  DECLARE_INSN(sret, MATCH_SRET, MASK_SRET)
> > -DECLARE_INSN(hret, MATCH_HRET, MASK_HRET)
> >  DECLARE_INSN(mret, MATCH_MRET, MASK_MRET)
> > -DECLARE_INSN(dret, MATCH_DRET, MASK_DRET)
> >  DECLARE_INSN(sfence_vm, MATCH_SFENCE_VM, MASK_SFENCE_VM)
> >  DECLARE_INSN(sfence_vma, MATCH_SFENCE_VMA, MASK_SFENCE_VMA)
> >  DECLARE_INSN(wfi, MATCH_WFI, MASK_WFI)
> > @@ -2549,6 +2585,24 @@ DECLARE_INSN(c_sd, MATCH_C_SD, MASK_C_SD)
> >  DECLARE_INSN(c_addiw, MATCH_C_ADDIW, MASK_C_ADDIW)
> >  DECLARE_INSN(c_ldsp, MATCH_C_LDSP, MASK_C_LDSP)
> >  DECLARE_INSN(c_sdsp, MATCH_C_SDSP, MASK_C_SDSP)
> > +/* Hypervisor instructions.  */
> > +DECLARE_INSN(hlv_b,   MATCH_HLVB,   MASK_HLV)
> > +DECLARE_INSN(hlv_h,   MATCH_HLVH,   MASK_HLV)
> > +DECLARE_INSN(hlv_w,   MATCH_HLVW,   MASK_HLV)
> > +DECLARE_INSN(hlv_d,   MATCH_HLVD,   MASK_HLV)
> > +DECLARE_INSN(hlv_bu,  MATCH_HLVBU,  MASK_HLV)
> > +DECLARE_INSN(hlv_hu,  MATCH_HLVHU,  MASK_HLV)
> > +DECLARE_INSN(hlv_wu,  MATCH_HLVWU,  MASK_HLV)
> > +DECLARE_INSN(hlvx_hu, MATCH_HLVXHU, MASK_HLV)
> > +DECLARE_INSN(hlvx_wu, MATCH_HLVXWU, MASK_HLV)
> > +DECLARE_INSN(hsv_b,   MATCH_HSVB,   MASK_HSV)
> > +DECLARE_INSN(hsv_h,   MATCH_HSVH,   MASK_HSV)
> > +DECLARE_INSN(hsv_w,   MATCH_HSVW,   MASK_HSV)
> > +DECLARE_INSN(hsv_d,   MATCH_HSVD,   MASK_HSV)
> > +DECLARE_INSN(hfence_vvma, MATCH_HFENCE_VVMA, MASK_HFENCE)
> > +DECLARE_INSN(hfence_gvma, MATCH_HFENCE_GVMA, MASK_HFENCE)
> > +DECLARE_INSN(hinval_vvma, MATCH_HINVAL_VVMA, MASK_HINVAL)
> > +DECLARE_INSN(hinval_gvma, MATCH_HINVAL_GVMA, MASK_HINVAL)
> >  #endif /* DECLARE_INSN */
> >  #ifdef DECLARE_CSR
> >  /* Privileged CSRs.  */
> > @@ -2764,17 +2818,34 @@ DECLARE_CSR(mhpmevent28, CSR_MHPMEVENT28, CSR_CLASS_I, PRIV_SPEC_CLASS_1P9P1, PR
> >  DECLARE_CSR(mhpmevent29, CSR_MHPMEVENT29, CSR_CLASS_I, PRIV_SPEC_CLASS_1P9P1, PRIV_SPEC_CLASS_DRAFT)
> >  DECLARE_CSR(mhpmevent30, CSR_MHPMEVENT30, CSR_CLASS_I, PRIV_SPEC_CLASS_1P9P1, PRIV_SPEC_CLASS_DRAFT)
> >  DECLARE_CSR(mhpmevent31, CSR_MHPMEVENT31, CSR_CLASS_I, PRIV_SPEC_CLASS_1P9P1, PRIV_SPEC_CLASS_DRAFT)
> > +
> > +DECLARE_CSR(hstatus,     CSR_HSTATUS,     CSR_CLASS_H, PRIV_SPEC_CLASS_1P9P1, PRIV_SPEC_CLASS_DRAFT)

The fourth and fifth fields means which spec starts to define the csr,
and which spec starts to drop it.  Therefore, I think this should be,

DECLARE_CSR(hstatus, CSR_HSTATUS, CSR_CLASS_H, PRIV_SPEC_CLASS_1P12,
PRIV_SPEC_CLASS_DRAFT)

Which means the csr hstatus is defined to 0x600 since the privileged
spec 1.12, and until now (until draft).  Besides, we haven't dropped
the privileged 1.9.1, so we also need an extra definition,

DECLARE_CSR_ALIAS(hstatus, CSR_VSSTATUS, CSR_CLASS_H,
PRIV_SPEC_CLASS_1P9P1, PRIV_SPEC_CLASS_1P10)

Which means the csr hstatus is defined to 0x200 since the privileged
spec 1.9.1, but dropped since the spec 1.10.

> > +DECLARE_CSR(hedeleg,     CSR_HEDELEG,     CSR_CLASS_H, PRIV_SPEC_CLASS_1P9P1, PRIV_SPEC_CLASS_DRAFT)
> > +DECLARE_CSR(hideleg,     CSR_HIDELEG,     CSR_CLASS_H, PRIV_SPEC_CLASS_1P9P1, PRIV_SPEC_CLASS_DRAFT)
> > +DECLARE_CSR(hie,         CSR_HIE,         CSR_CLASS_H, PRIV_SPEC_CLASS_1P9P1, PRIV_SPEC_CLASS_DRAFT)
> > +DECLARE_CSR(hcounteren,  CSR_HCOUNTEREN,  CSR_CLASS_H, PRIV_SPEC_CLASS_1P9P1, PRIV_SPEC_CLASS_DRAFT)
> > +DECLARE_CSR(hgeie,       CSR_HGEIE,       CSR_CLASS_H, PRIV_SPEC_CLASS_1P9P1, PRIV_SPEC_CLASS_DRAFT)
> > +DECLARE_CSR(htval,       CSR_HTVAL,       CSR_CLASS_H, PRIV_SPEC_CLASS_1P9P1, PRIV_SPEC_CLASS_DRAFT)
> > +DECLARE_CSR(hip,         CSR_HIP,         CSR_CLASS_H, PRIV_SPEC_CLASS_1P9P1, PRIV_SPEC_CLASS_DRAFT)
> > +DECLARE_CSR(hvip,        CSR_HVIP,        CSR_CLASS_H, PRIV_SPEC_CLASS_1P9P1, PRIV_SPEC_CLASS_DRAFT)
> > +DECLARE_CSR(htinst,      CSR_HTINST,      CSR_CLASS_H, PRIV_SPEC_CLASS_1P9P1, PRIV_SPEC_CLASS_DRAFT)
> > +DECLARE_CSR(hgeip,       CSR_HGEIP,       CSR_CLASS_H, PRIV_SPEC_CLASS_1P9P1, PRIV_SPEC_CLASS_DRAFT)
> > +DECLARE_CSR(henvcfg,     CSR_HENVCFG,     CSR_CLASS_H, PRIV_SPEC_CLASS_1P9P1, PRIV_SPEC_CLASS_DRAFT)
> > +DECLARE_CSR(henvcfgh,    CSR_HENVCFGH,    CSR_CLASS_H, PRIV_SPEC_CLASS_1P9P1, PRIV_SPEC_CLASS_DRAFT)
> > +DECLARE_CSR(hgatp,       CSR_HGATP,       CSR_CLASS_H, PRIV_SPEC_CLASS_1P9P1, PRIV_SPEC_CLASS_DRAFT)
> > +DECLARE_CSR(hcontext,    CSR_HCONTEXT,    CSR_CLASS_H, PRIV_SPEC_CLASS_1P9P1, PRIV_SPEC_CLASS_DRAFT)
> > +DECLARE_CSR(htimedelta,  CSR_HTIMEDELTA,  CSR_CLASS_H, PRIV_SPEC_CLASS_1P9P1, PRIV_SPEC_CLASS_DRAFT)
> > +DECLARE_CSR(htimedeltah, CSR_HTIMEDELTAH, CSR_CLASS_H, PRIV_SPEC_CLASS_1P9P1, PRIV_SPEC_CLASS_DRAFT)
> > +DECLARE_CSR(vsstatus,    CSR_VSSTATUS,    CSR_CLASS_H, PRIV_SPEC_CLASS_1P9P1, PRIV_SPEC_CLASS_DRAFT)
> > +DECLARE_CSR(vsie,        CSR_VSIE,        CSR_CLASS_H, PRIV_SPEC_CLASS_1P9P1, PRIV_SPEC_CLASS_DRAFT)
> > +DECLARE_CSR(vstvec,      CSR_VSTVEC,      CSR_CLASS_H, PRIV_SPEC_CLASS_1P9P1, PRIV_SPEC_CLASS_DRAFT)
> > +DECLARE_CSR(vsscratch,   CSR_VSSCRATCH,   CSR_CLASS_H, PRIV_SPEC_CLASS_1P9P1, PRIV_SPEC_CLASS_DRAFT)
> > +DECLARE_CSR(vsepc,       CSR_VSEPC,       CSR_CLASS_H, PRIV_SPEC_CLASS_1P9P1, PRIV_SPEC_CLASS_DRAFT)
> > +DECLARE_CSR(vscause,     CSR_VSCAUSE,     CSR_CLASS_H, PRIV_SPEC_CLASS_1P9P1, PRIV_SPEC_CLASS_DRAFT)
> > +DECLARE_CSR(vstval,      CSR_VSTVAL,      CSR_CLASS_H, PRIV_SPEC_CLASS_1P9P1, PRIV_SPEC_CLASS_DRAFT)
> > +DECLARE_CSR(vsip,        CSR_VSIP,        CSR_CLASS_H, PRIV_SPEC_CLASS_1P9P1, PRIV_SPEC_CLASS_DRAFT)
> > +DECLARE_CSR(vsatp,       CSR_VSATP,       CSR_CLASS_H, PRIV_SPEC_CLASS_1P9P1, PRIV_SPEC_CLASS_DRAFT)
> >  /* Dropped CSRs.  */
> > -DECLARE_CSR(hstatus, CSR_HSTATUS, CSR_CLASS_I, PRIV_SPEC_CLASS_1P9P1, PRIV_SPEC_CLASS_1P10)
> > -DECLARE_CSR(hedeleg, CSR_HEDELEG, CSR_CLASS_I, PRIV_SPEC_CLASS_1P9P1, PRIV_SPEC_CLASS_1P10)
> > -DECLARE_CSR(hideleg, CSR_HIDELEG, CSR_CLASS_I, PRIV_SPEC_CLASS_1P9P1, PRIV_SPEC_CLASS_1P10)
> > -DECLARE_CSR(hie, CSR_HIE, CSR_CLASS_I, PRIV_SPEC_CLASS_1P9P1, PRIV_SPEC_CLASS_1P10)
> > -DECLARE_CSR(htvec, CSR_HTVEC, CSR_CLASS_I, PRIV_SPEC_CLASS_1P9P1, PRIV_SPEC_CLASS_1P10)
> > -DECLARE_CSR(hscratch, CSR_HSCRATCH, CSR_CLASS_I, PRIV_SPEC_CLASS_1P9P1, PRIV_SPEC_CLASS_1P10)
> > -DECLARE_CSR(hepc, CSR_HEPC, CSR_CLASS_I, PRIV_SPEC_CLASS_1P9P1, PRIV_SPEC_CLASS_1P10)
> > -DECLARE_CSR(hcause, CSR_HCAUSE, CSR_CLASS_I, PRIV_SPEC_CLASS_1P9P1, PRIV_SPEC_CLASS_1P10)
> > -DECLARE_CSR(hbadaddr, CSR_HBADADDR, CSR_CLASS_I, PRIV_SPEC_CLASS_1P9P1, PRIV_SPEC_CLASS_1P10)
> > -DECLARE_CSR(hip, CSR_HIP, CSR_CLASS_I, PRIV_SPEC_CLASS_1P9P1, PRIV_SPEC_CLASS_1P10)
> >  DECLARE_CSR(mbase, CSR_MBASE, CSR_CLASS_I, PRIV_SPEC_CLASS_1P9P1, PRIV_SPEC_CLASS_1P10)
> >  DECLARE_CSR(mbound, CSR_MBOUND, CSR_CLASS_I, PRIV_SPEC_CLASS_1P9P1, PRIV_SPEC_CLASS_1P10)
> >  DECLARE_CSR(mibase, CSR_MIBASE, CSR_CLASS_I, PRIV_SPEC_CLASS_1P9P1, PRIV_SPEC_CLASS_1P10)
> > diff --git a/opcodes/riscv-opc.c b/opcodes/riscv-opc.c
> > index 40037db83c05..794eee2d413f 100644
> > --- a/opcodes/riscv-opc.c
> > +++ b/opcodes/riscv-opc.c
> > @@ -839,9 +839,7 @@ const struct riscv_opcode riscv_opcodes[] =
> >  {"csrrc",      0, INSN_CLASS_ZICSR,"d,E,Z",    MATCH_CSRRCI, MASK_CSRRCI, match_opcode, INSN_ALIAS },
> >  {"uret",       0, INSN_CLASS_I,    "",         MATCH_URET, MASK_URET, match_opcode, 0 },
> >  {"sret",       0, INSN_CLASS_I,    "",         MATCH_SRET, MASK_SRET, match_opcode, 0 },
> > -{"hret",       0, INSN_CLASS_I,    "",         MATCH_HRET, MASK_HRET, match_opcode, 0 },
> >  {"mret",       0, INSN_CLASS_I,    "",         MATCH_MRET, MASK_MRET, match_opcode, 0 },
> > -{"dret",       0, INSN_CLASS_I,    "",         MATCH_DRET, MASK_DRET, match_opcode, 0 },
>
> I don't know where the debug stuff ended up going, but dret was in some
> earlier specs and was used.  I guess we should split it out into a D
> spec of some sort.

They are defined in the debug spec, including the debug instructions and csrs,
https://github.com/riscv/riscv-debug-spec/blob/master/riscv-debug-stable.pdf

However, we never consider that the instructions may be controlled not
only by the ISA spec, and I don't know if there is any extension name
defined for the debug spec or not...  Anyway, yes, eventually these
debug instructions should be splitted to something like
INSN_CLASS_DEBUG?  I don't know, we never consider that in fact...
But I would suggest that we just keep it for now.

> >  {"sfence.vm",  0, INSN_CLASS_I,    "",         MATCH_SFENCE_VM, MASK_SFENCE_VM | MASK_RS1, match_opcode, 0 },
> >  {"sfence.vm",  0, INSN_CLASS_I,    "s",        MATCH_SFENCE_VM, MASK_SFENCE_VM, match_opcode, 0 },
> >  {"sfence.vma", 0, INSN_CLASS_I,    "",         MATCH_SFENCE_VMA, MASK_SFENCE_VMA|MASK_RS1|MASK_RS2, match_opcode, INSN_ALIAS },
> > @@ -1725,6 +1723,27 @@ const struct riscv_opcode riscv_opcodes[] =
> >  {"vmv4r.v",    0, INSN_CLASS_V, "Vd,Vt", MATCH_VMV4RV, MASK_VMV4RV, match_opcode, 0},
> >  {"vmv8r.v",    0, INSN_CLASS_V, "Vd,Vt", MATCH_VMV8RV, MASK_VMV8RV, match_opcode, 0},
> >
> > +/* Hypervisor instructions.  */
> > +{"hlv.b",       0, INSN_CLASS_H, "d,(s)", MATCH_HLVB,   MASK_HLV, match_opcode, INSN_DREF|INSN_1_BYTE },

Consider that users may write the assembly like this,
hlv.b   a0, 0(a1)

I would suggest adding the '0' operand into the string like "d,0(s)".
We also have similar support for atomic instructions (A-ext), so
keeping the compatibility should be good to users.

> > +{"hlv.bu",      0, INSN_CLASS_H, "d,(s)", MATCH_HLVBU,  MASK_HLV, match_opcode, INSN_DREF|INSN_1_BYTE },
> > +{"hlv.h",       0, INSN_CLASS_H, "d,(s)", MATCH_HLVH,   MASK_HLV, match_opcode, INSN_DREF|INSN_2_BYTE },
> > +{"hlv.hu",      0, INSN_CLASS_H, "d,(s)", MATCH_HLVHU,  MASK_HLV, match_opcode, INSN_DREF|INSN_2_BYTE },
> > +{"hlvx.hu",     0, INSN_CLASS_H, "d,(s)", MATCH_HLVXHU, MASK_HLV, match_opcode, INSN_DREF|INSN_2_BYTE },
> > +{"hlv.w",       0, INSN_CLASS_H, "d,(s)", MATCH_HLVW,   MASK_HLV, match_opcode, INSN_DREF|INSN_4_BYTE },
> > +{"hlv.wu",     64, INSN_CLASS_H, "d,(s)", MATCH_HLVWU,  MASK_HLV, match_opcode, INSN_DREF|INSN_4_BYTE },
> > +{"hlvx.wu",     0, INSN_CLASS_H, "d,(s)", MATCH_HLVXWU, MASK_HLV, match_opcode, INSN_DREF|INSN_4_BYTE },
> > +{"hlv.d",      64, INSN_CLASS_H, "d,(s)", MATCH_HLVD,   MASK_HLV, match_opcode, INSN_DREF|INSN_8_BYTE },
> > +
> > +{"hsv.b",       0, INSN_CLASS_H, "t,(s)", MATCH_HSVB,   MASK_HSV, match_opcode, INSN_DREF|INSN_1_BYTE },
> > +{"hsv.h",       0, INSN_CLASS_H, "t,(s)", MATCH_HSVH,   MASK_HSV, match_opcode, INSN_DREF|INSN_2_BYTE },
> > +{"hsv.w",       0, INSN_CLASS_H, "t,(s)", MATCH_HSVW,   MASK_HSV, match_opcode, INSN_DREF|INSN_4_BYTE },
> > +{"hsv.d",      64, INSN_CLASS_H, "t,(s)", MATCH_HSVD,   MASK_HSV, match_opcode, INSN_DREF|INSN_8_BYTE },
> > +
> > +{"hfence.vvma", 0, INSN_CLASS_H, "t,s", MATCH_HFENCE_VVMA, MASK_HFENCE, match_opcode, 0 },
> > +{"hfence.gvma", 0, INSN_CLASS_H, "t,s", MATCH_HFENCE_GVMA, MASK_HFENCE, match_opcode, 0 },

I cannot find the assembly syntax in the ISA spec, maybe they are
defined somewhere just I don't know.  However, I find this patch,
https://patchwork.kernel.org/project/linux-riscv/patch/20210115121846.114528-10-anup.patel@wdc.com/

And according to the link, it shows the syntax as follows,

/*
* Instruction encoding of hfence.gvma is:
* HFENCE.GVMA rs1, rs2
* HFENCE.GVMA zero, rs2
* HFENCE.GVMA rs1
* HFENCE.GVMA
*
* rs1!=zero and rs2!=zero ==> HFENCE.GVMA rs1, rs2
* rs1==zero and rs2!=zero ==> HFENCE.GVMA zero, rs2
* rs1!=zero and rs2==zero ==> HFENCE.GVMA rs1
* rs1==zero and rs2==zero ==> HFENCE.GVMA
*
* Instruction encoding of HFENCE.GVMA is:
* 0110001 rs2(5) rs1(5) 000 00000 1110011
*/

The syntax is compatible with sfence.vma, so this is also what I
expected.  Maybe we should change the entries to,

{"hfence.gvma", 0, INSN_CLASS_I,  "",     MATCH_HFENCE_GVMA,
MASK_HFENCE_GVMA|MASK_RS1|MASK_RS2, match_opcode, INSN_ALIAS },
{"hfence.gvma", 0, INSN_CLASS_I,  "s",    MATCH_HFENCE_GVMA,
MASK_HFENCE_GVMA|MASK_RS2, match_opcode, INSN_ALIAS },
{"hfence.gvma", 0, INSN_CLASS_I,  "s,t",  MATCH_HFENCE_GVMA,
MASK_HFENCE_GVMA, match_opcode, 0 },

And so does the hfence.vvma.

> > +{"hinval.vvma", 0, INSN_CLASS_H, "t,s", MATCH_HINVAL_VVMA, MASK_HINVAL, match_opcode, 0 },
> > +{"hinval.gvma", 0, INSN_CLASS_H, "t,s", MATCH_HINVAL_GVMA, MASK_HINVAL, match_opcode, 0 },

I have cherry-picked the Svinval extension from the integration branch
back to here, so I think hinval instruction may be changed to
something like INSN_CLASS_SVINVAL_AND_H?  But we still have to clarify
and make sure that the single h won't conflict with the ISA spec
first.


> > +
> >  /* Terminate the list.  */
> >  {0, 0, INSN_CLASS_NONE, 0, 0, 0, 0, 0}
> >  };

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

* Re: [RFC 1/4] RISC-V: Hypervisor ext: Treat as "Standard" extension
  2021-12-17 16:10     ` Nelson Chu
@ 2021-12-17 22:40       ` Vineet Gupta
  2021-12-17 22:50       ` Palmer Dabbelt
  1 sibling, 0 replies; 17+ messages in thread
From: Vineet Gupta @ 2021-12-17 22:40 UTC (permalink / raw)
  To: Nelson Chu, Palmer Dabbelt; +Cc: Binutils, Kito Cheng, Jim Wilson



On 12/17/21 8:10 AM, Nelson Chu wrote:
> On Fri, Dec 17, 2021 at 12:10 PM Palmer Dabbelt <palmer@dabbelt.com> wrote:
>> On Thu, 16 Dec 2021 09:33:25 PST (-0800), Vineet Gupta wrote:
>>> diff --git a/bfd/elfxx-riscv.c b/bfd/elfxx-riscv.c
>>> index 3bd41ff2b551..7d5ae5a4657d 100644
>>> --- a/bfd/elfxx-riscv.c
>>> +++ b/bfd/elfxx-riscv.c
>>> @@ -1173,6 +1173,7 @@ static struct riscv_supported_ext riscv_supported_std_ext[] =
>>>     {"t",              ISA_SPEC_CLASS_NONE, RISCV_UNKNOWN_VERSION, RISCV_UNKNOWN_VERSION, 0 },
>>>     {"p",              ISA_SPEC_CLASS_NONE, RISCV_UNKNOWN_VERSION, RISCV_UNKNOWN_VERSION, 0 },
>>>     {"v",              ISA_SPEC_CLASS_DRAFT,           1, 0, 0 },
>>> +  {"h",              PRIV_SPEC_CLASS_DRAFT,          1, 0, 0 },
>> According to
>> <https://wiki.riscv.org/display/TECH/Recently+Ratified+Extensions>, this
>> was ratified in November so we should be able to update this to
>> something like PRIV_SPEC_CLASS_1P12 (the V stuff was also ratified, but
>> that's a different issue).
> I can understand that why Vineet are doing these changes, but I had a
> quick talk with Kito this morning, and I think we also have three
> issues for now,
>
> 1. The current draft ISA spec doesn't have any related changes, so if
> we change the `h' as a single standard extension, then this will
> conflict with the spec.

Sure it does conflict, but this has implications on downstream projects 
(linux kernel, opensbi,...) as they need to add the toggles to their 
build systems. And when the spec changes we need to update the tools and 
then change the toggles downstream. This is all tricky to coordinate. 
However I do understand spec correctness argument so we'll keep 
thestatus quo and stick to multi-letter naming for "h".

So given h extension has been supported so far how is one to use it - I 
can't make gas or gcc grok any of the following
-march=rv64{h,_h,_h1p10,...}

riscv64-unknown-elf-gcc: error: '-march=rv64g_h1p10': name of hypervisor 
extension must be more than 1 letter

What am I missing ?

> 2. Even if we decide to change the `h' as a single extension rather
> than the multiple prefixed keyword, then in what order should we place
> h.  I expect the table 27.1 in the ISA spec will also mention the
> order of the single h.
>
> 3. I never considered that an extension version may be controlled by
> the privileged spec before, so in the riscv_get_default_ext_version,
>
>>>         if (strcmp (table[i].name, name) == 0
>>>          && (table[i].isa_spec_class == ISA_SPEC_CLASS_DRAFT
>>> +           || table[i].isa_spec_class == PRIV_SPEC_CLASS_DRAFT
>>>              || table[i].isa_spec_class == *default_isa_spec))
> We will need to rewrite the related code, since the isa_spec_class may
> be one of the PRIV_SPEC_CLASS_XXX in the future, so we will never
> match it to the default_isa_spec...

OK lets leave 'h' to multi-letter for now.

Thx,
-Vineet

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

* Re: [RFC 1/4] RISC-V: Hypervisor ext: Treat as "Standard" extension
  2021-12-17 16:10     ` Nelson Chu
  2021-12-17 22:40       ` Vineet Gupta
@ 2021-12-17 22:50       ` Palmer Dabbelt
  2021-12-17 23:14         ` Vineet Gupta
  1 sibling, 1 reply; 17+ messages in thread
From: Palmer Dabbelt @ 2021-12-17 22:50 UTC (permalink / raw)
  To: Nelson Chu, Paul Walmsley; +Cc: Vineet Gupta, binutils, kito.cheng, Jim Wilson

On Fri, 17 Dec 2021 08:10:42 PST (-0800), Nelson Chu wrote:
> On Fri, Dec 17, 2021 at 12:10 PM Palmer Dabbelt <palmer@dabbelt.com> wrote:
>>
>> On Thu, 16 Dec 2021 09:33:25 PST (-0800), Vineet Gupta wrote:
>> > gas currently fails for single-letter 'h' prefix in arch string.
>> >
>> > | riscv64-unknown-elf-as -march=rv64gh empty.s
>> > | Assembler messages:
>> > | Error: rv64gh: unknown prefixed ISA extension `h'
>> >
>> > It implements the "multi-letter" prefix for H-ext as recommended in
>> > ISA manual subsection "ISA Extension Naming Conventions". However given
>> > the discussions at [1] there seems to be no pressing need to keep version
>> > for the current H-ext since
>> >  - it will remain at 1.0
>> >  - any future Hypervisor extensions wull called "Sh*" or some such
>> >  - H was the original standard extension with its own MISA bit
>> >
>> > This patch thus relaxes the constraint and allows single letter prefix
>> > 'h'
>> >
>> > Note: To keep the binutils dejagnu failures same, I've fixed the
>> > testsuite files within the same patch. I can break it up if reviewers
>> > feel as such.
>> >
>> > [1] https://github.com/riscv/riscv-isa-manual/issues/781#issuecomment-983233088
>> >
>> > bfd/
>> >       * elfxx-riscv.c (riscv_supported_std_ext): Add "h" entry.
>> >       (riscv_supported_std_h_ext): Add "h" entry.
>> >       (riscv_get_default_ext_version): Handle PRIV_SPEC_CLASS_DRAFT.
>> >       (riscv_multi_subset_supports): Handle INSN_CLASS_H.
>> >
>> > gas/
>> >       * testsuite/gas/riscv/march-fail-single-prefix-h.d: Deleted as
>> >       single-letter prefix 'h' is now valid.
>> >       * testsuite/gas/riscv/march-fail-single-prefix-l: Removed 'h'.
>> >
>> > include/
>> >       * opcode/riscv.h (enum riscv_insn_class): Add INSN_CLASS_H.
>> >
>> > Signed-off-by: Vineet Gupta <vineetg@rivosinc.com>
>> > ---
>> >  bfd/elfxx-riscv.c                                    | 5 +++++
>> >  gas/testsuite/gas/riscv/march-fail-single-prefix-h.d | 3 ---
>> >  gas/testsuite/gas/riscv/march-fail-single-prefix.l   | 2 +-
>> >  include/opcode/riscv.h                               | 1 +
>> >  4 files changed, 7 insertions(+), 4 deletions(-)
>> >  delete mode 100644 gas/testsuite/gas/riscv/march-fail-single-prefix-h.d
>> >
>> > diff --git a/bfd/elfxx-riscv.c b/bfd/elfxx-riscv.c
>> > index 3bd41ff2b551..7d5ae5a4657d 100644
>> > --- a/bfd/elfxx-riscv.c
>> > +++ b/bfd/elfxx-riscv.c
>> > @@ -1173,6 +1173,7 @@ static struct riscv_supported_ext riscv_supported_std_ext[] =
>> >    {"t",              ISA_SPEC_CLASS_NONE, RISCV_UNKNOWN_VERSION, RISCV_UNKNOWN_VERSION, 0 },
>> >    {"p",              ISA_SPEC_CLASS_NONE, RISCV_UNKNOWN_VERSION, RISCV_UNKNOWN_VERSION, 0 },
>> >    {"v",              ISA_SPEC_CLASS_DRAFT,           1, 0, 0 },
>> > +  {"h",              PRIV_SPEC_CLASS_DRAFT,          1, 0, 0 },
>>
>> According to
>> <https://wiki.riscv.org/display/TECH/Recently+Ratified+Extensions>, this
>> was ratified in November so we should be able to update this to
>> something like PRIV_SPEC_CLASS_1P12 (the V stuff was also ratified, but
>> that's a different issue).
>
> I can understand that why Vineet are doing these changes, but I had a
> quick talk with Kito this morning, and I think we also have three
> issues for now,
>
> 1. The current draft ISA spec doesn't have any related changes, so if
> we change the `h' as a single standard extension, then this will
> conflict with the spec.

Can you be specific about which specification you're talking about?   By 
my count there's more than a dozen now and it gets kind of tricky to 
figure out which is which.

I'm assuming you're talking about the user ISA spec, the last ratified 
version I can find is 
<https://github.com/riscv/riscv-isa-manual/releases/download/Ratified-IMAFDQC/riscv-spec-20191213.pdf>, 
which definately lists H as a prefix.

> 2. Even if we decide to change the `h' as a single extension rather
> than the multiple prefixed keyword, then in what order should we place
> h.  I expect the table 27.1 in the ISA spec will also mention the
> order of the single h.

I guess I'd just assumed this was called "H", given that's what we've 
all been calling it for a long time, but after actually reading the 
ratified specs I don't see that anywhere.

The Hypervisor extension itself is ratified, via priv-1.12, but I don't 
see anything in a ratified spec that describes what it's called in ISA 
strings.  It's called "Hypervisor ISA, Version 1.0" in the preface and 
"Hypervisor Extension, Version 1.0.0-rc" in the chapter that defines it 
(both in the ratified priv-1.12).  Both of those claim they're frozen, 
in contrast to the ratified user spec which calls out extensions as 
ratified -- not sure if that's relevant, though, as I'm generally pretty 
lost WRT the state of the specifications/extensions/versions right now.

The rules in the ratified user spec make it sound like this would either 
be called "S*" or "H*" (depending on whether this is a supervisor-mode 
extension or an ISA), but I can't find anything pointing to where the 
second letter should be.  Also no idea what to do with the version, 
assuming that RC suffix is canonical.

Maybe I'm missing something?

> 3. I never considered that an extension version may be controlled by
> the privileged spec before, so in the riscv_get_default_ext_version,
>
>> >        if (strcmp (table[i].name, name) == 0
>> >         && (table[i].isa_spec_class == ISA_SPEC_CLASS_DRAFT
>> > +           || table[i].isa_spec_class == PRIV_SPEC_CLASS_DRAFT
>> >             || table[i].isa_spec_class == *default_isa_spec))
>
> We will need to rewrite the related code, since the isa_spec_class may
> be one of the PRIV_SPEC_CLASS_XXX in the future, so we will never
> match it to the default_isa_spec...

More that that: it now sounds like the hypervisor extension (and/or ISA?) 
encoding for ISA strings changes based on the user specification in 
play, despite it being defined in the priv specification.

Certainly sounds like we should hold off until we get something concrete 
on the naming scheme, as making something up on our own is going to be a 
mess in the long run.

>> >    {"n",              ISA_SPEC_CLASS_NONE, RISCV_UNKNOWN_VERSION, RISCV_UNKNOWN_VERSION, 0 },
>> >    {NULL, 0, 0, 0, 0}
>> >  };
>> > @@ -1232,6 +1233,7 @@ static struct riscv_supported_ext riscv_supported_std_s_ext[] =
>> >
>> >  static struct riscv_supported_ext riscv_supported_std_h_ext[] =
>> >  {
>> > +  {"h",                 PRIV_SPEC_CLASS_DRAFT,          1, 0, 0 },
>> >    {NULL, 0, 0, 0, 0}
>> >  };
>> >
>> > @@ -1531,6 +1533,7 @@ riscv_get_default_ext_version (enum riscv_spec_class *default_isa_spec,
>> >      {
>> >        if (strcmp (table[i].name, name) == 0
>> >         && (table[i].isa_spec_class == ISA_SPEC_CLASS_DRAFT
>> > +           || table[i].isa_spec_class == PRIV_SPEC_CLASS_DRAFT
>> >             || table[i].isa_spec_class == *default_isa_spec))
>> >       {
>> >         *major_version = table[i].major_version;
>> > @@ -2412,6 +2415,8 @@ riscv_multi_subset_supports (riscv_parse_subset_t *rps,
>> >             || riscv_subset_supports (rps, "zve64d")
>> >             || riscv_subset_supports (rps, "zve64f")
>> >             || riscv_subset_supports (rps, "zve32f"));
>> > +    case INSN_CLASS_H:
>> > +      return riscv_subset_supports (rps, "h");
>> >      default:
>> >        rps->error_handler
>> >          (_("internal: unreachable INSN_CLASS_*"));
>> > diff --git a/gas/testsuite/gas/riscv/march-fail-single-prefix-h.d b/gas/testsuite/gas/riscv/march-fail-single-prefix-h.d
>> > deleted file mode 100644
>> > index eb101bd71353..000000000000
>> > --- a/gas/testsuite/gas/riscv/march-fail-single-prefix-h.d
>> > +++ /dev/null
>> > @@ -1,3 +0,0 @@
>> > -#as: -march=rv32ih
>> > -#source: empty.s
>> > -#error_output: march-fail-single-prefix.l
>> > diff --git a/gas/testsuite/gas/riscv/march-fail-single-prefix.l b/gas/testsuite/gas/riscv/march-fail-single-prefix.l
>> > index 13942ed66642..e4418de69acb 100644
>> > --- a/gas/testsuite/gas/riscv/march-fail-single-prefix.l
>> > +++ b/gas/testsuite/gas/riscv/march-fail-single-prefix.l
>> > @@ -1,2 +1,2 @@
>> >  .*Assembler messages:
>> > -.*Error: .*unknown prefixed ISA extension `(s|h|z|x|zxm)'
>> > +.*Error: .*unknown prefixed ISA extension `(s|z|x|zxm)'
>> > diff --git a/include/opcode/riscv.h b/include/opcode/riscv.h
>> > index 14889972abce..2783bdecaae5 100644
>> > --- a/include/opcode/riscv.h
>> > +++ b/include/opcode/riscv.h
>> > @@ -387,6 +387,7 @@ enum riscv_insn_class
>> >    INSN_CLASS_ZKND_OR_ZKNE,
>> >    INSN_CLASS_V,
>> >    INSN_CLASS_ZVEF,
>> > +  INSN_CLASS_H,
>> >  };
>> >
>> >  /* This structure holds information for a particular instruction.  */
>>
>> Reviewed-by: Palmer Dabbelt <palmer@rivosinc.com>
>>
>> We can always deal with the draft/ratified stuff later, as there's a
>> bunch of them.

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

* Re: [RFC 1/4] RISC-V: Hypervisor ext: Treat as "Standard" extension
  2021-12-17 22:50       ` Palmer Dabbelt
@ 2021-12-17 23:14         ` Vineet Gupta
  0 siblings, 0 replies; 17+ messages in thread
From: Vineet Gupta @ 2021-12-17 23:14 UTC (permalink / raw)
  To: Palmer Dabbelt, Nelson Chu, Paul Walmsley
  Cc: binutils, kito.cheng, Jim Wilson, Andrew Waterman



On 12/17/21 2:50 PM, Palmer Dabbelt wrote:
>>>
>>> According to
>>> <https://wiki.riscv.org/display/TECH/Recently+Ratified+Extensions>, 
>>> this
>>> was ratified in November so we should be able to update this to
>>> something like PRIV_SPEC_CLASS_1P12 (the V stuff was also ratified, but
>>> that's a different issue).
>>
>> I can understand that why Vineet are doing these changes, but I had a
>> quick talk with Kito this morning, and I think we also have three
>> issues for now,
>>
>> 1. The current draft ISA spec doesn't have any related changes, so if
>> we change the `h' as a single standard extension, then this will
>> conflict with the spec.
>
> Can you be specific about which specification you're talking about?   
> By my count there's more than a dozen now and it gets kind of tricky 
> to figure out which is which.
>
> I'm assuming you're talking about the user ISA spec, the last ratified 
> version I can find is 
> <https://github.com/riscv/riscv-isa-manual/releases/download/Ratified-IMAFDQC/riscv-spec-20191213.pdf>, 
> which definately lists H as a prefix.

I think Nelson is referring to section 27.8 of above document which 
specifies "H" to be named as "S" which in turn suggests a scheme of 'S' 
prefix + alphabetical name + optional ver number.
>
>> 2. Even if we decide to change the `h' as a single extension rather
>> than the multiple prefixed keyword, then in what order should we place
>> h.  I expect the table 27.1 in the ISA spec will also mention the
>> order of the single h.
>
> I guess I'd just assumed this was called "H", given that's what we've 
> all been calling it for a long time, but after actually reading the 
> ratified specs I don't see that anywhere.
>
> The Hypervisor extension itself is ratified, via priv-1.12, but I 
> don't see anything in a ratified spec that describes what it's called 
> in ISA strings.  It's called "Hypervisor ISA, Version 1.0" in the 
> preface and "Hypervisor Extension, Version 1.0.0-rc" in the chapter 
> that defines it (both in the ratified priv-1.12).  Both of those claim 
> they're frozen, in contrast to the ratified user spec which calls out 
> extensions as ratified -- not sure if that's relevant, though, as I'm 
> generally pretty lost WRT the state of the 
> specifications/extensions/versions right now.
>
> The rules in the ratified user spec make it sound like this would 
> either be called "S*" or "H*" (depending on whether this is a 
> supervisor-mode extension or an ISA), but I can't find anything 
> pointing to where the second letter should be.  Also no idea what to 
> do with the version, assuming that RC suffix is canonical.
>
> Maybe I'm missing something?

Yeah it seems section 27.8 is the contention point.

>
>> 3. I never considered that an extension version may be controlled by
>> the privileged spec before, so in the riscv_get_default_ext_version,
>>
>>> >        if (strcmp (table[i].name, name) == 0
>>> >         && (table[i].isa_spec_class == ISA_SPEC_CLASS_DRAFT
>>> > +           || table[i].isa_spec_class == PRIV_SPEC_CLASS_DRAFT
>>> >             || table[i].isa_spec_class == *default_isa_spec))
>>
>> We will need to rewrite the related code, since the isa_spec_class may
>> be one of the PRIV_SPEC_CLASS_XXX in the future, so we will never
>> match it to the default_isa_spec...
>
> More that that: it now sounds like the hypervisor extension (and/or 
> ISA?) encoding for ISA strings changes based on the user specification 
> in play, despite it being defined in the priv specification.
>
> Certainly sounds like we should hold off until we get something 
> concrete on the naming scheme, as making something up on our own is 
> going to be a mess in the long run.

The naming scheme is already a mess - I'd suggest we get the H-code 
support in (adhering to existing naming scheme) and clean up the naming 
/ arch string / toggles when that gets fixed. No point in holding the 
development based on hypervisor for that - and having to hand code 
opcodes and csrs in the relevant projects.


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

* Re: [RFC 2/4] RISC-V: Hypervisor ext: CSR and Instructions
  2021-12-17 17:30     ` Nelson Chu
@ 2021-12-18  1:44       ` Vineet Gupta
  0 siblings, 0 replies; 17+ messages in thread
From: Vineet Gupta @ 2021-12-18  1:44 UTC (permalink / raw)
  To: Nelson Chu, Palmer Dabbelt; +Cc: Binutils, Kito Cheng, Jim Wilson



On 12/17/21 9:30 AM, Nelson Chu wrote:
> On Fri, Dec 17, 2021 at 12:10 PM Palmer Dabbelt <palmer@dabbelt.com> wrote:
>> On Thu, 16 Dec 2021 09:33:26 PST (-0800), Vineet Gupta wrote:
>>> +#define MASK_HFENCE       0xfe007fff
>>> +#define MATCH_HFENCE_VVMA 0x22000073
>>> +#define MATCH_HFENCE_GVMA 0x62000073
>>> +
>>> +#define MASK_HINVAL       0xfe007fff
>>> +#define MATCH_HINVAL_VVMA 0x26000073
>>> +#define MATCH_HINVAL_GVMA 0x66000073
> We already have HINVAL in the mainline for now.

Ok I'll rebase for next version.

>>> +#define CSR_HCONTEXT    0x6a8
> The hcontext is one of the hypervisor csr, but it is also a debug csr.
> I have sent a patch to define this csr before, and it is controlled by
> the debug spec,
> https://sourceware.org/pipermail/binutils/2021-August/117568.html
>
> Maybe we should only choose one spec or one extension to control the
> hcontext csr, it should be worth discussing.

Indeed we need to sort out how to handle features that span multiple 
extensions (hypervisor + svinval). I'll send an email to tech-toolchain 
to solicit t

>>> +DECLARE_CSR(hstatus,     CSR_HSTATUS,     CSR_CLASS_H, PRIV_SPEC_CLASS_1P9P1, PRIV_SPEC_CLASS_DRAFT)
> The fourth and fifth fields means which spec starts to define the csr,
> and which spec starts to drop it.  Therefore, I think this should be,
>
> DECLARE_CSR(hstatus, CSR_HSTATUS, CSR_CLASS_H, PRIV_SPEC_CLASS_1P12,
> PRIV_SPEC_CLASS_DRAFT)
>
> Which means the csr hstatus is defined to 0x600 since the privileged
> spec 1.12, and until now (until draft).


[1] 
https://github.com/riscv/riscv-isa-manual/releases/download/draft-20211216-5651528/riscv-privileged.pdf

> Besides, we haven't dropped
> the privileged 1.9.1, so we also need an extra definition,
>
> DECLARE_CSR_ALIAS(hstatus, CSR_VSSTATUS, CSR_CLASS_H,
> PRIV_SPEC_CLASS_1P9P1, PRIV_SPEC_CLASS_1P10)
>
> Which means the csr hstatus is defined to 0x200 since the privileged
> spec 1.9.1, but dropped since the spec 1.10.

I like this approach for keeping backward compatible items and it is 
essential to do this to not break users. However in this specific case I 
wonder if we can clean the house a bit. Is there a way to tell the 
assembler to allow H-extension from 1.9.1 ? If not then it is 
effectively a dead code - until we fix it - in which case we just say 
that we only implement the latest 1.12 priv spec's H-ext 1.0.
What do you think ?

>
>>> +++ b/opcodes/riscv-opc.c
>>> @@ -839,9 +839,7 @@ const struct riscv_opcode riscv_opcodes[] =
>>>   {"csrrc",      0, INSN_CLASS_ZICSR,"d,E,Z",    MATCH_CSRRCI, MASK_CSRRCI, match_opcode, INSN_ALIAS },
>>>   {"uret",       0, INSN_CLASS_I,    "",         MATCH_URET, MASK_URET, match_opcode, 0 },
>>>   {"sret",       0, INSN_CLASS_I,    "",         MATCH_SRET, MASK_SRET, match_opcode, 0 },
>>> -{"hret",       0, INSN_CLASS_I,    "",         MATCH_HRET, MASK_HRET, match_opcode, 0 },
>>>   {"mret",       0, INSN_CLASS_I,    "",         MATCH_MRET, MASK_MRET, match_opcode, 0 },
>>> -{"dret",       0, INSN_CLASS_I,    "",         MATCH_DRET, MASK_DRET, match_opcode, 0 },
>> I don't know where the debug stuff ended up going, but dret was in some
>> earlier specs and was used.  I guess we should split it out into a D
>> spec of some sort.
> They are defined in the debug spec, including the debug instructions and csrs,
> https://github.com/riscv/riscv-debug-spec/blob/master/riscv-debug-stable.pdf
>
> However, we never consider that the instructions may be controlled not
> only by the ISA spec, and I don't know if there is any extension name
> defined for the debug spec or not...  Anyway, yes, eventually these
> debug instructions should be splitted to something like
> INSN_CLASS_DEBUG?  I don't know, we never consider that in fact...
> But I would suggest that we just keep it for now.

Make sense

>>> +/* Hypervisor instructions.  */
>>> +{"hlv.b",       0, INSN_CLASS_H, "d,(s)", MATCH_HLVB,   MASK_HLV, match_opcode, INSN_DREF|INSN_1_BYTE },
> Consider that users may write the assembly like this,
> hlv.b   a0, 0(a1)
>
> I would suggest adding the '0' operand into the string like "d,0(s)".
> We also have similar support for atomic instructions (A-ext), so
> keeping the compatibility should be good to users.

Make sense, I'll keep that.

>>> +{"hfence.vvma", 0, INSN_CLASS_H, "t,s", MATCH_HFENCE_VVMA, MASK_HFENCE, match_opcode, 0 },
>>> +{"hfence.gvma", 0, INSN_CLASS_H, "t,s", MATCH_HFENCE_GVMA, MASK_HFENCE, match_opcode, 0 },
> I cannot find the assembly syntax in the ISA spec, maybe they are
> defined somewhere just I don't know.  However, I find this patch,
> https://patchwork.kernel.org/project/linux-riscv/patch/20210115121846.114528-10-anup.patel@wdc.com/
>
> And according to the link, it shows the syntax as follows,
>
> /*
> * Instruction encoding of hfence.gvma is:
> * HFENCE.GVMA rs1, rs2
> * HFENCE.GVMA zero, rs2
> * HFENCE.GVMA rs1
> * HFENCE.GVMA
> *
> * rs1!=zero and rs2!=zero ==> HFENCE.GVMA rs1, rs2
> * rs1==zero and rs2!=zero ==> HFENCE.GVMA zero, rs2
> * rs1!=zero and rs2==zero ==> HFENCE.GVMA rs1
> * rs1==zero and rs2==zero ==> HFENCE.GVMA
> *
> * Instruction encoding of HFENCE.GVMA is:
> * 0110001 rs2(5) rs1(5) 000 00000 1110011
> */
>
> The syntax is compatible with sfence.vma, so this is also what I
> expected.  Maybe we should change the entries to,
>
> {"hfence.gvma", 0, INSN_CLASS_I,  "",     MATCH_HFENCE_GVMA,
> MASK_HFENCE_GVMA|MASK_RS1|MASK_RS2, match_opcode, INSN_ALIAS },
> {"hfence.gvma", 0, INSN_CLASS_I,  "s",    MATCH_HFENCE_GVMA,
> MASK_HFENCE_GVMA|MASK_RS2, match_opcode, INSN_ALIAS },
> {"hfence.gvma", 0, INSN_CLASS_I,  "s,t",  MATCH_HFENCE_GVMA,
> MASK_HFENCE_GVMA, match_opcode, 0 },
>
> And so does the hfence.vvma.

OK will fix.
  I just looked at the table os all encodings and missed the fact that 
some cases could omit an operand.

>>> +{"hinval.vvma", 0, INSN_CLASS_H, "t,s", MATCH_HINVAL_VVMA, MASK_HINVAL, match_opcode, 0 },
>>> +{"hinval.gvma", 0, INSN_CLASS_H, "t,s", MATCH_HINVAL_GVMA, MASK_HINVAL, match_opcode, 0 },
> I have cherry-picked the Svinval extension from the integration branch
> back to here, so I think hinval instruction may be changed to
> something like INSN_CLASS_SVINVAL_AND_H?

And this is after there's consensus on using both extensions for things 
will span multiple extensions.

And I think it would be a mess to define INSN_CLASS_SVINVAL_AND_H - who 
knows in future we have features crossing 3 extensions ?
IMO we should use indiv one and OR them - 
(INSN_CLASS_SVINVAL|INSN_CLASS__H)  -except that these would no longer 
be an enum but a bitmask

>   But we still have to clarify
> and make sure that the single h won't conflict with the ISA spec
> first.

Yeah for now we don't change it.

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

* Re: [RFC 3/4] RISC-V: Hypervisor ext: tests for new isns/csr and cleanup old csrs
  2021-12-16 17:33 ` [RFC 3/4] RISC-V: Hypervisor ext: tests for new isns/csr and cleanup old csrs Vineet Gupta
  2021-12-17  4:10   ` Palmer Dabbelt
@ 2021-12-21  8:17   ` Jan Beulich
  2021-12-21 15:28     ` Vineet Gupta
  1 sibling, 1 reply; 17+ messages in thread
From: Jan Beulich @ 2021-12-21  8:17 UTC (permalink / raw)
  To: Vineet Gupta; +Cc: Kito Cheng, Binutils, Nelson Chu

On 16.12.2021 18:33, Vineet Gupta wrote:
> Since the prior versions were not fully supported (and or functional
> even from ISA pov), we just remove any old baggage - this avoids the
> mess in keeping csr with same names but different addresses etc.
> 
> Signed-off-by: Vineet Gupta <vineetg@rivosinc.com>
> ---
>  gas/testsuite/gas/riscv/csr-dw-regnums.d      | 10 ---
>  gas/testsuite/gas/riscv/csr-dw-regnums.s      | 10 ---
>  gas/testsuite/gas/riscv/h-ext-32.s            | 61 +++++++++++++++++
>  gas/testsuite/gas/riscv/h-ext-64.s            | 68 +++++++++++++++++++

To be useful, don't these two files need to be accompanied by respective
.d ones?

Jan

>  .../gas/riscv/priv-reg-fail-read-only-01.s    | 10 ---
>  .../gas/riscv/priv-reg-fail-version-1p10.l    | 10 ---
>  .../gas/riscv/priv-reg-fail-version-1p11.l    | 10 ---
>  .../gas/riscv/priv-reg-version-1p10.d         | 10 ---
>  .../gas/riscv/priv-reg-version-1p11.d         | 10 ---
>  .../gas/riscv/priv-reg-version-1p9p1.d        | 10 ---
>  gas/testsuite/gas/riscv/priv-reg.s            | 10 ---
>  11 files changed, 129 insertions(+), 90 deletions(-)
>  create mode 100644 gas/testsuite/gas/riscv/h-ext-32.s
>  create mode 100644 gas/testsuite/gas/riscv/h-ext-64.s


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

* Re: [RFC 3/4] RISC-V: Hypervisor ext: tests for new isns/csr and cleanup old csrs
  2021-12-21  8:17   ` Jan Beulich
@ 2021-12-21 15:28     ` Vineet Gupta
  0 siblings, 0 replies; 17+ messages in thread
From: Vineet Gupta @ 2021-12-21 15:28 UTC (permalink / raw)
  To: Jan Beulich; +Cc: Kito Cheng, Binutils, Nelson Chu



On 12/21/21 12:17 AM, Jan Beulich wrote:
>>   gas/testsuite/gas/riscv/h-ext-32.s            | 61 +++++++++++++++++
>>   gas/testsuite/gas/riscv/h-ext-64.s            | 68 +++++++++++++++++++
> To be useful, don't these two files need to be accompanied by respective
> .d ones?

Right I wasn't sure how to generate them - if tooling existed - since 
they have regex stuff.
v1 posted yesterday now has them.

Thx,
-Vineet

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

end of thread, other threads:[~2021-12-21 15:28 UTC | newest]

Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-12-16 17:33 [RFC 0/4] riscv/binutils support Hypervisor Extension Vineet Gupta
2021-12-16 17:33 ` [RFC 1/4] RISC-V: Hypervisor ext: Treat as "Standard" extension Vineet Gupta
2021-12-17  4:10   ` Palmer Dabbelt
2021-12-17 16:10     ` Nelson Chu
2021-12-17 22:40       ` Vineet Gupta
2021-12-17 22:50       ` Palmer Dabbelt
2021-12-17 23:14         ` Vineet Gupta
2021-12-16 17:33 ` [RFC 2/4] RISC-V: Hypervisor ext: CSR and Instructions Vineet Gupta
2021-12-17  4:10   ` Palmer Dabbelt
2021-12-17 17:30     ` Nelson Chu
2021-12-18  1:44       ` Vineet Gupta
2021-12-16 17:33 ` [RFC 3/4] RISC-V: Hypervisor ext: tests for new isns/csr and cleanup old csrs Vineet Gupta
2021-12-17  4:10   ` Palmer Dabbelt
2021-12-21  8:17   ` Jan Beulich
2021-12-21 15:28     ` Vineet Gupta
2021-12-16 17:33 ` [RFC 4/4] RISC-V: fix a comment for adding CSR entry and annotate switch-break Vineet Gupta
2021-12-17  4:10   ` 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).