Home Original page

[RFC 2/4] RISC-V: Hypervisor ext: CSR and Instructions

Nelson Chu nelson.chu@sifive.com
Fri Dec 17 17:30:30 GMT 2021 
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}
> >  };
More information about the Binutils mailing list