When compiling with toolchains that haven't yet been taught about
new instructions we need to encode them ourselves. This series
creates a new file where support for instruction definitions can
evolve. For starters the file is initiated with a macro for R-type
encodings. The series then applies the R-type encoding macro to all
instances of hard coded instruction definitions in KVM.
Not only should using instruction encoding macros improve readability
and maintainability of code, but we should also gain potential for
more optimized code after compilation as the compiler will have control
over the input and output registers used, which may provide more
opportunities for inlining.
I grepped for other places we may want to use these macros and the
only place I found was ALT_CMO_OP(), but I didn't dare touch it :-)
I do suggest we apply this to the Svinal support [1] as we won't
want to frustrate the compiler's inlining efforts with hard coded
register selection.
[1] https://lore.kernel.org/linux-riscv/[email protected]/
v2:
- Cleaned up some whitespace issues pointed out by Anup, myself
and checkpatch and dropped some {}
- Pushed the quoting of register tokens into the macros
- Picked up Anup's r-b's
Andrew Jones (4):
riscv: Add X register names to gpr-nums
riscv: Introduce support for defining instructions
riscv: KVM: Apply insn-def to hfence encodings
riscv: KVM: Apply insn-def to hlv encodings
arch/riscv/Kconfig | 3 +
arch/riscv/include/asm/gpr-num.h | 8 ++
arch/riscv/include/asm/insn-def.h | 113 ++++++++++++++++++++++++++
arch/riscv/kvm/tlb.c | 129 ++++--------------------------
arch/riscv/kvm/vcpu_exit.c | 29 ++-----
5 files changed, 146 insertions(+), 136 deletions(-)
create mode 100644 arch/riscv/include/asm/insn-def.h
--
2.37.2
When compiling with toolchains that haven't yet been taught about
new instructions we need to encode them ourselves. Create a new file
where support for instruction definitions will evolve. We initiate
the file with a macro called INSN_R(), which implements the R-type
instruction encoding. INSN_R() will use the assembler's .insn
directive when available, which should give the assembler a chance
to do some validation. When .insn is not available we fall back to
manual encoding.
Not only should using instruction encoding macros improve readability
and maintainability of code over the alternative of inserting
instructions directly (e.g. '.word 0xc0de'), but we should also gain
potential for more optimized code after compilation because the
compiler will have control over the input and output registers used.
Signed-off-by: Andrew Jones <[email protected]>
Reviewed-by: Anup Patel <[email protected]>
---
arch/riscv/Kconfig | 3 ++
arch/riscv/include/asm/insn-def.h | 86 +++++++++++++++++++++++++++++++
2 files changed, 89 insertions(+)
create mode 100644 arch/riscv/include/asm/insn-def.h
diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
index ed66c31e4655..f8f3b316b838 100644
--- a/arch/riscv/Kconfig
+++ b/arch/riscv/Kconfig
@@ -227,6 +227,9 @@ config RISCV_DMA_NONCOHERENT
select ARCH_HAS_SETUP_DMA_OPS
select DMA_DIRECT_REMAP
+config AS_HAS_INSN
+ def_bool $(as-instr,.insn r 51$(comma) 0$(comma) 0$(comma) t0$(comma) t0$(comma) zero)
+
source "arch/riscv/Kconfig.socs"
source "arch/riscv/Kconfig.erratas"
diff --git a/arch/riscv/include/asm/insn-def.h b/arch/riscv/include/asm/insn-def.h
new file mode 100644
index 000000000000..2dcd1d4781bf
--- /dev/null
+++ b/arch/riscv/include/asm/insn-def.h
@@ -0,0 +1,86 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+
+#ifndef __ASM_INSN_DEF_H
+#define __ASM_INSN_DEF_H
+
+#include <asm/asm.h>
+
+#define INSN_R_FUNC7_SHIFT 25
+#define INSN_R_RS2_SHIFT 20
+#define INSN_R_RS1_SHIFT 15
+#define INSN_R_FUNC3_SHIFT 12
+#define INSN_R_RD_SHIFT 7
+#define INSN_R_OPCODE_SHIFT 0
+
+#ifdef __ASSEMBLY__
+
+#ifdef CONFIG_AS_HAS_INSN
+
+ .macro insn_r, opcode, func3, func7, rd, rs1, rs2
+ .insn r \opcode, \func3, \func7, \rd, \rs1, \rs2
+ .endm
+
+#else
+
+#include <asm/gpr-num.h>
+
+ .macro insn_r, opcode, func3, func7, rd, rs1, rs2
+ .4byte ((\opcode << INSN_R_OPCODE_SHIFT) | \
+ (\func3 << INSN_R_FUNC3_SHIFT) | \
+ (\func7 << INSN_R_FUNC7_SHIFT) | \
+ (.L__gpr_num_\rd << INSN_R_RD_SHIFT) | \
+ (.L__gpr_num_\rs1 << INSN_R_RS1_SHIFT) | \
+ (.L__gpr_num_\rs2 << INSN_R_RS2_SHIFT))
+ .endm
+
+#endif
+
+#define INSN_R(...) insn_r __VA_ARGS__
+
+#else /* ! __ASSEMBLY__ */
+
+#ifdef CONFIG_AS_HAS_INSN
+
+#define INSN_R(opcode, func3, func7, rd, rs1, rs2) \
+ ".insn r " opcode ", " func3 ", " func7 ", " rd ", " rs1 ", " rs2 "\n"
+
+#else
+
+#include <linux/stringify.h>
+#include <asm/gpr-num.h>
+
+#define DEFINE_INSN_R \
+ __DEFINE_ASM_GPR_NUMS \
+" .macro insn_r, opcode, func3, func7, rd, rs1, rs2\n" \
+" .4byte ((\\opcode << " __stringify(INSN_R_OPCODE_SHIFT) ") |" \
+" (\\func3 << " __stringify(INSN_R_FUNC3_SHIFT) ") |" \
+" (\\func7 << " __stringify(INSN_R_FUNC7_SHIFT) ") |" \
+" (.L__gpr_num_\\rd << " __stringify(INSN_R_RD_SHIFT) ") |" \
+" (.L__gpr_num_\\rs1 << " __stringify(INSN_R_RS1_SHIFT) ") |" \
+" (.L__gpr_num_\\rs2 << " __stringify(INSN_R_RS2_SHIFT) "))\n" \
+" .endm\n"
+
+#define UNDEFINE_INSN_R \
+" .purgem insn_r\n"
+
+#define INSN_R(opcode, func3, func7, rd, rs1, rs2) \
+ DEFINE_INSN_R \
+ "insn_r " opcode ", " func3 ", " func7 ", " rd ", " rs1 ", " rs2 "\n" \
+ UNDEFINE_INSN_R
+
+#endif
+
+#endif /* ! __ASSEMBLY__ */
+
+#define OPCODE(v) __ASM_STR(v)
+#define FUNC3(v) __ASM_STR(v)
+#define FUNC7(v) __ASM_STR(v)
+#define RD(v) __ASM_STR(v)
+#define RS1(v) __ASM_STR(v)
+#define RS2(v) __ASM_STR(v)
+#define __REG(v) __ASM_STR(x ## v)
+#define __RD(v) __REG(v)
+#define __RS1(v) __REG(v)
+#define __RS2(v) __REG(v)
+
+#endif /* __ASM_INSN_DEF_H */
--
2.37.2
When encoding instructions it's sometimes necessary to set a
register field to a precise number. This is easiest to do using
the x<num> naming.
Signed-off-by: Andrew Jones <[email protected]>
Reviewed-by: Anup Patel <[email protected]>
---
arch/riscv/include/asm/gpr-num.h | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/arch/riscv/include/asm/gpr-num.h b/arch/riscv/include/asm/gpr-num.h
index dfee2829fc7c..efeb5edf8a3a 100644
--- a/arch/riscv/include/asm/gpr-num.h
+++ b/arch/riscv/include/asm/gpr-num.h
@@ -3,6 +3,11 @@
#define __ASM_GPR_NUM_H
#ifdef __ASSEMBLY__
+
+ .irp num,0,1,2,3,4,5,6,7,8,9,10,11,12,13,14,15,16,17,18,19,20,21,22,23,24,25,26,27,28,29,30,31
+ .equ .L__gpr_num_x\num, \num
+ .endr
+
.equ .L__gpr_num_zero, 0
.equ .L__gpr_num_ra, 1
.equ .L__gpr_num_sp, 2
@@ -39,6 +44,9 @@
#else /* __ASSEMBLY__ */
#define __DEFINE_ASM_GPR_NUMS \
+" .irp num,0,1,2,3,4,5,6,7,8,9,10,11,12,13,14,15,16,17,18,19,20,21,22,23,24,25,26,27,28,29,30,31\n" \
+" .equ .L__gpr_num_x\\num, \\num\n" \
+" .endr\n" \
" .equ .L__gpr_num_zero, 0\n" \
" .equ .L__gpr_num_ra, 1\n" \
" .equ .L__gpr_num_sp, 2\n" \
--
2.37.2
Introduce hfence instruction encodings and apply them to KVM's use.
With the self-documenting nature of the instruction encoding macros,
and a spec always within arm's reach, it's safe to remove the
comments, so we do that too.
Signed-off-by: Andrew Jones <[email protected]>
Reviewed-by: Anup Patel <[email protected]>
---
arch/riscv/include/asm/insn-def.h | 10 +++
arch/riscv/kvm/tlb.c | 129 ++++--------------------------
2 files changed, 27 insertions(+), 112 deletions(-)
diff --git a/arch/riscv/include/asm/insn-def.h b/arch/riscv/include/asm/insn-def.h
index 2dcd1d4781bf..86c1f602413b 100644
--- a/arch/riscv/include/asm/insn-def.h
+++ b/arch/riscv/include/asm/insn-def.h
@@ -83,4 +83,14 @@
#define __RS1(v) __REG(v)
#define __RS2(v) __REG(v)
+#define OPCODE_SYSTEM OPCODE(115)
+
+#define HFENCE_VVMA(vaddr, asid) \
+ INSN_R(OPCODE_SYSTEM, FUNC3(0), FUNC7(17), \
+ __RD(0), RS1(vaddr), RS2(asid))
+
+#define HFENCE_GVMA(gaddr, vmid) \
+ INSN_R(OPCODE_SYSTEM, FUNC3(0), FUNC7(49), \
+ __RD(0), RS1(gaddr), RS2(vmid))
+
#endif /* __ASM_INSN_DEF_H */
diff --git a/arch/riscv/kvm/tlb.c b/arch/riscv/kvm/tlb.c
index 1a76d0b1907d..1ce3394b3acf 100644
--- a/arch/riscv/kvm/tlb.c
+++ b/arch/riscv/kvm/tlb.c
@@ -12,22 +12,7 @@
#include <linux/kvm_host.h>
#include <asm/cacheflush.h>
#include <asm/csr.h>
-
-/*
- * 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
- */
+#include <asm/insn-def.h>
void kvm_riscv_local_hfence_gvma_vmid_gpa(unsigned long vmid,
gpa_t gpa, gpa_t gpsz,
@@ -40,32 +25,14 @@ void kvm_riscv_local_hfence_gvma_vmid_gpa(unsigned long vmid,
return;
}
- for (pos = gpa; pos < (gpa + gpsz); pos += BIT(order)) {
- /*
- * rs1 = a0 (GPA >> 2)
- * rs2 = a1 (VMID)
- * HFENCE.GVMA a0, a1
- * 0110001 01011 01010 000 00000 1110011
- */
- asm volatile ("srli a0, %0, 2\n"
- "add a1, %1, zero\n"
- ".word 0x62b50073\n"
- :: "r" (pos), "r" (vmid)
- : "a0", "a1", "memory");
- }
+ for (pos = gpa; pos < (gpa + gpsz); pos += BIT(order))
+ asm volatile (HFENCE_GVMA(%0, %1)
+ : : "r" (pos >> 2), "r" (vmid) : "memory");
}
void kvm_riscv_local_hfence_gvma_vmid_all(unsigned long vmid)
{
- /*
- * rs1 = zero
- * rs2 = a0 (VMID)
- * HFENCE.GVMA zero, a0
- * 0110001 01010 00000 000 00000 1110011
- */
- asm volatile ("add a0, %0, zero\n"
- ".word 0x62a00073\n"
- :: "r" (vmid) : "a0", "memory");
+ asm volatile(HFENCE_GVMA(zero, %0) : : "r" (vmid) : "memory");
}
void kvm_riscv_local_hfence_gvma_gpa(gpa_t gpa, gpa_t gpsz,
@@ -78,46 +45,16 @@ void kvm_riscv_local_hfence_gvma_gpa(gpa_t gpa, gpa_t gpsz,
return;
}
- for (pos = gpa; pos < (gpa + gpsz); pos += BIT(order)) {
- /*
- * rs1 = a0 (GPA >> 2)
- * rs2 = zero
- * HFENCE.GVMA a0
- * 0110001 00000 01010 000 00000 1110011
- */
- asm volatile ("srli a0, %0, 2\n"
- ".word 0x62050073\n"
- :: "r" (pos) : "a0", "memory");
- }
+ for (pos = gpa; pos < (gpa + gpsz); pos += BIT(order))
+ asm volatile(HFENCE_GVMA(%0, zero)
+ : : "r" (pos >> 2) : "memory");
}
void kvm_riscv_local_hfence_gvma_all(void)
{
- /*
- * rs1 = zero
- * rs2 = zero
- * HFENCE.GVMA
- * 0110001 00000 00000 000 00000 1110011
- */
- asm volatile (".word 0x62000073" ::: "memory");
+ asm volatile(HFENCE_GVMA(zero, zero) : : : "memory");
}
-/*
- * Instruction encoding of hfence.gvma is:
- * HFENCE.VVMA rs1, rs2
- * HFENCE.VVMA zero, rs2
- * HFENCE.VVMA rs1
- * HFENCE.VVMA
- *
- * rs1!=zero and rs2!=zero ==> HFENCE.VVMA rs1, rs2
- * rs1==zero and rs2!=zero ==> HFENCE.VVMA zero, rs2
- * rs1!=zero and rs2==zero ==> HFENCE.VVMA rs1
- * rs1==zero and rs2==zero ==> HFENCE.VVMA
- *
- * Instruction encoding of HFENCE.VVMA is:
- * 0010001 rs2(5) rs1(5) 000 00000 1110011
- */
-
void kvm_riscv_local_hfence_vvma_asid_gva(unsigned long vmid,
unsigned long asid,
unsigned long gva,
@@ -133,19 +70,9 @@ void kvm_riscv_local_hfence_vvma_asid_gva(unsigned long vmid,
hgatp = csr_swap(CSR_HGATP, vmid << HGATP_VMID_SHIFT);
- for (pos = gva; pos < (gva + gvsz); pos += BIT(order)) {
- /*
- * rs1 = a0 (GVA)
- * rs2 = a1 (ASID)
- * HFENCE.VVMA a0, a1
- * 0010001 01011 01010 000 00000 1110011
- */
- asm volatile ("add a0, %0, zero\n"
- "add a1, %1, zero\n"
- ".word 0x22b50073\n"
- :: "r" (pos), "r" (asid)
- : "a0", "a1", "memory");
- }
+ for (pos = gva; pos < (gva + gvsz); pos += BIT(order))
+ asm volatile(HFENCE_VVMA(%0, %1)
+ : : "r" (pos), "r" (asid) : "memory");
csr_write(CSR_HGATP, hgatp);
}
@@ -157,15 +84,7 @@ void kvm_riscv_local_hfence_vvma_asid_all(unsigned long vmid,
hgatp = csr_swap(CSR_HGATP, vmid << HGATP_VMID_SHIFT);
- /*
- * rs1 = zero
- * rs2 = a0 (ASID)
- * HFENCE.VVMA zero, a0
- * 0010001 01010 00000 000 00000 1110011
- */
- asm volatile ("add a0, %0, zero\n"
- ".word 0x22a00073\n"
- :: "r" (asid) : "a0", "memory");
+ asm volatile(HFENCE_VVMA(zero, %0) : : "r" (asid) : "memory");
csr_write(CSR_HGATP, hgatp);
}
@@ -183,17 +102,9 @@ void kvm_riscv_local_hfence_vvma_gva(unsigned long vmid,
hgatp = csr_swap(CSR_HGATP, vmid << HGATP_VMID_SHIFT);
- for (pos = gva; pos < (gva + gvsz); pos += BIT(order)) {
- /*
- * rs1 = a0 (GVA)
- * rs2 = zero
- * HFENCE.VVMA a0
- * 0010001 00000 01010 000 00000 1110011
- */
- asm volatile ("add a0, %0, zero\n"
- ".word 0x22050073\n"
- :: "r" (pos) : "a0", "memory");
- }
+ for (pos = gva; pos < (gva + gvsz); pos += BIT(order))
+ asm volatile(HFENCE_VVMA(%0, zero)
+ : : "r" (pos) : "memory");
csr_write(CSR_HGATP, hgatp);
}
@@ -204,13 +115,7 @@ void kvm_riscv_local_hfence_vvma_all(unsigned long vmid)
hgatp = csr_swap(CSR_HGATP, vmid << HGATP_VMID_SHIFT);
- /*
- * rs1 = zero
- * rs2 = zero
- * HFENCE.VVMA
- * 0010001 00000 00000 000 00000 1110011
- */
- asm volatile (".word 0x22000073" ::: "memory");
+ asm volatile(HFENCE_VVMA(zero, zero) : : : "memory");
csr_write(CSR_HGATP, hgatp);
}
--
2.37.2
Introduce hlv instruction encodings and apply them to KVM's use.
We're careful not to introduce hlv.d to 32-bit builds. Indeed,
we ensure the build fails if someone tries to use it.
Signed-off-by: Andrew Jones <[email protected]>
Reviewed-by: Anup Patel <[email protected]>
---
arch/riscv/include/asm/insn-def.h | 17 +++++++++++++++++
arch/riscv/kvm/vcpu_exit.c | 29 +++++------------------------
2 files changed, 22 insertions(+), 24 deletions(-)
diff --git a/arch/riscv/include/asm/insn-def.h b/arch/riscv/include/asm/insn-def.h
index 86c1f602413b..8fe9036efb68 100644
--- a/arch/riscv/include/asm/insn-def.h
+++ b/arch/riscv/include/asm/insn-def.h
@@ -93,4 +93,21 @@
INSN_R(OPCODE_SYSTEM, FUNC3(0), FUNC7(49), \
__RD(0), RS1(gaddr), RS2(vmid))
+#define HLVX_HU(dest, addr) \
+ INSN_R(OPCODE_SYSTEM, FUNC3(4), FUNC7(50), \
+ RD(dest), RS1(addr), __RS2(3))
+
+#define HLV_W(dest, addr) \
+ INSN_R(OPCODE_SYSTEM, FUNC3(4), FUNC7(52), \
+ RD(dest), RS1(addr), __RS2(0))
+
+#ifdef CONFIG_64BIT
+#define HLV_D(dest, addr) \
+ INSN_R(OPCODE_SYSTEM, FUNC3(4), FUNC7(54), \
+ RD(dest), RS1(addr), __RS2(0))
+#else
+#define HLV_D(dest, addr) \
+ __ASM_STR(.error "hlv.d requires 64-bit support")
+#endif
+
#endif /* __ASM_INSN_DEF_H */
diff --git a/arch/riscv/kvm/vcpu_exit.c b/arch/riscv/kvm/vcpu_exit.c
index d5c36386878a..da793f113a72 100644
--- a/arch/riscv/kvm/vcpu_exit.c
+++ b/arch/riscv/kvm/vcpu_exit.c
@@ -8,6 +8,7 @@
#include <linux/kvm_host.h>
#include <asm/csr.h>
+#include <asm/insn-def.h>
static int gstage_page_fault(struct kvm_vcpu *vcpu, struct kvm_run *run,
struct kvm_cpu_trap *trap)
@@ -82,22 +83,12 @@ unsigned long kvm_riscv_vcpu_unpriv_read(struct kvm_vcpu *vcpu,
".option push\n"
".option norvc\n"
"add %[ttmp], %[taddr], 0\n"
- /*
- * HLVX.HU %[val], (%[addr])
- * HLVX.HU t0, (t2)
- * 0110010 00011 00111 100 00101 1110011
- */
- ".word 0x6433c2f3\n"
+ HLVX_HU(%[val], %[addr])
"andi %[tmp], %[val], 3\n"
"addi %[tmp], %[tmp], -3\n"
"bne %[tmp], zero, 2f\n"
"addi %[addr], %[addr], 2\n"
- /*
- * HLVX.HU %[tmp], (%[addr])
- * HLVX.HU t1, (t2)
- * 0110010 00011 00111 100 00110 1110011
- */
- ".word 0x6433c373\n"
+ HLVX_HU(%[tmp], %[addr])
"sll %[tmp], %[tmp], 16\n"
"add %[val], %[val], %[tmp]\n"
"2:\n"
@@ -121,19 +112,9 @@ unsigned long kvm_riscv_vcpu_unpriv_read(struct kvm_vcpu *vcpu,
".option norvc\n"
"add %[ttmp], %[taddr], 0\n"
#ifdef CONFIG_64BIT
- /*
- * HLV.D %[val], (%[addr])
- * HLV.D t0, (t2)
- * 0110110 00000 00111 100 00101 1110011
- */
- ".word 0x6c03c2f3\n"
+ HLV_D(%[val], %[addr])
#else
- /*
- * HLV.W %[val], (%[addr])
- * HLV.W t0, (t2)
- * 0110100 00000 00111 100 00101 1110011
- */
- ".word 0x6803c2f3\n"
+ HLV_W(%[val], %[addr])
#endif
".option pop"
: [val] "=&r" (val),
--
2.37.2
On Wed, Aug 31, 2022 at 10:55 PM Andrew Jones <[email protected]> wrote:
>
> When compiling with toolchains that haven't yet been taught about
> new instructions we need to encode them ourselves. Create a new file
> where support for instruction definitions will evolve. We initiate
> the file with a macro called INSN_R(), which implements the R-type
> instruction encoding. INSN_R() will use the assembler's .insn
> directive when available, which should give the assembler a chance
> to do some validation. When .insn is not available we fall back to
> manual encoding.
>
> Not only should using instruction encoding macros improve readability
> and maintainability of code over the alternative of inserting
> instructions directly (e.g. '.word 0xc0de'), but we should also gain
> potential for more optimized code after compilation because the
> compiler will have control over the input and output registers used.
>
> Signed-off-by: Andrew Jones <[email protected]>
> Reviewed-by: Anup Patel <[email protected]>
I have queued this patch for Linux-6.1
Thanks,
Anup
> ---
> arch/riscv/Kconfig | 3 ++
> arch/riscv/include/asm/insn-def.h | 86 +++++++++++++++++++++++++++++++
> 2 files changed, 89 insertions(+)
> create mode 100644 arch/riscv/include/asm/insn-def.h
>
> diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
> index ed66c31e4655..f8f3b316b838 100644
> --- a/arch/riscv/Kconfig
> +++ b/arch/riscv/Kconfig
> @@ -227,6 +227,9 @@ config RISCV_DMA_NONCOHERENT
> select ARCH_HAS_SETUP_DMA_OPS
> select DMA_DIRECT_REMAP
>
> +config AS_HAS_INSN
> + def_bool $(as-instr,.insn r 51$(comma) 0$(comma) 0$(comma) t0$(comma) t0$(comma) zero)
> +
> source "arch/riscv/Kconfig.socs"
> source "arch/riscv/Kconfig.erratas"
>
> diff --git a/arch/riscv/include/asm/insn-def.h b/arch/riscv/include/asm/insn-def.h
> new file mode 100644
> index 000000000000..2dcd1d4781bf
> --- /dev/null
> +++ b/arch/riscv/include/asm/insn-def.h
> @@ -0,0 +1,86 @@
> +/* SPDX-License-Identifier: GPL-2.0-only */
> +
> +#ifndef __ASM_INSN_DEF_H
> +#define __ASM_INSN_DEF_H
> +
> +#include <asm/asm.h>
> +
> +#define INSN_R_FUNC7_SHIFT 25
> +#define INSN_R_RS2_SHIFT 20
> +#define INSN_R_RS1_SHIFT 15
> +#define INSN_R_FUNC3_SHIFT 12
> +#define INSN_R_RD_SHIFT 7
> +#define INSN_R_OPCODE_SHIFT 0
> +
> +#ifdef __ASSEMBLY__
> +
> +#ifdef CONFIG_AS_HAS_INSN
> +
> + .macro insn_r, opcode, func3, func7, rd, rs1, rs2
> + .insn r \opcode, \func3, \func7, \rd, \rs1, \rs2
> + .endm
> +
> +#else
> +
> +#include <asm/gpr-num.h>
> +
> + .macro insn_r, opcode, func3, func7, rd, rs1, rs2
> + .4byte ((\opcode << INSN_R_OPCODE_SHIFT) | \
> + (\func3 << INSN_R_FUNC3_SHIFT) | \
> + (\func7 << INSN_R_FUNC7_SHIFT) | \
> + (.L__gpr_num_\rd << INSN_R_RD_SHIFT) | \
> + (.L__gpr_num_\rs1 << INSN_R_RS1_SHIFT) | \
> + (.L__gpr_num_\rs2 << INSN_R_RS2_SHIFT))
> + .endm
> +
> +#endif
> +
> +#define INSN_R(...) insn_r __VA_ARGS__
> +
> +#else /* ! __ASSEMBLY__ */
> +
> +#ifdef CONFIG_AS_HAS_INSN
> +
> +#define INSN_R(opcode, func3, func7, rd, rs1, rs2) \
> + ".insn r " opcode ", " func3 ", " func7 ", " rd ", " rs1 ", " rs2 "\n"
> +
> +#else
> +
> +#include <linux/stringify.h>
> +#include <asm/gpr-num.h>
> +
> +#define DEFINE_INSN_R \
> + __DEFINE_ASM_GPR_NUMS \
> +" .macro insn_r, opcode, func3, func7, rd, rs1, rs2\n" \
> +" .4byte ((\\opcode << " __stringify(INSN_R_OPCODE_SHIFT) ") |" \
> +" (\\func3 << " __stringify(INSN_R_FUNC3_SHIFT) ") |" \
> +" (\\func7 << " __stringify(INSN_R_FUNC7_SHIFT) ") |" \
> +" (.L__gpr_num_\\rd << " __stringify(INSN_R_RD_SHIFT) ") |" \
> +" (.L__gpr_num_\\rs1 << " __stringify(INSN_R_RS1_SHIFT) ") |" \
> +" (.L__gpr_num_\\rs2 << " __stringify(INSN_R_RS2_SHIFT) "))\n" \
> +" .endm\n"
> +
> +#define UNDEFINE_INSN_R \
> +" .purgem insn_r\n"
> +
> +#define INSN_R(opcode, func3, func7, rd, rs1, rs2) \
> + DEFINE_INSN_R \
> + "insn_r " opcode ", " func3 ", " func7 ", " rd ", " rs1 ", " rs2 "\n" \
> + UNDEFINE_INSN_R
> +
> +#endif
> +
> +#endif /* ! __ASSEMBLY__ */
> +
> +#define OPCODE(v) __ASM_STR(v)
> +#define FUNC3(v) __ASM_STR(v)
> +#define FUNC7(v) __ASM_STR(v)
> +#define RD(v) __ASM_STR(v)
> +#define RS1(v) __ASM_STR(v)
> +#define RS2(v) __ASM_STR(v)
> +#define __REG(v) __ASM_STR(x ## v)
> +#define __RD(v) __REG(v)
> +#define __RS1(v) __REG(v)
> +#define __RS2(v) __REG(v)
> +
> +#endif /* __ASM_INSN_DEF_H */
> --
> 2.37.2
>
On Wed, Aug 31, 2022 at 10:55 PM Andrew Jones <[email protected]> wrote:
>
> Introduce hfence instruction encodings and apply them to KVM's use.
> With the self-documenting nature of the instruction encoding macros,
> and a spec always within arm's reach, it's safe to remove the
> comments, so we do that too.
>
> Signed-off-by: Andrew Jones <[email protected]>
> Reviewed-by: Anup Patel <[email protected]>
I have queued this patch for Linux-6.1
Thanks,
Anup
> ---
> arch/riscv/include/asm/insn-def.h | 10 +++
> arch/riscv/kvm/tlb.c | 129 ++++--------------------------
> 2 files changed, 27 insertions(+), 112 deletions(-)
>
> diff --git a/arch/riscv/include/asm/insn-def.h b/arch/riscv/include/asm/insn-def.h
> index 2dcd1d4781bf..86c1f602413b 100644
> --- a/arch/riscv/include/asm/insn-def.h
> +++ b/arch/riscv/include/asm/insn-def.h
> @@ -83,4 +83,14 @@
> #define __RS1(v) __REG(v)
> #define __RS2(v) __REG(v)
>
> +#define OPCODE_SYSTEM OPCODE(115)
> +
> +#define HFENCE_VVMA(vaddr, asid) \
> + INSN_R(OPCODE_SYSTEM, FUNC3(0), FUNC7(17), \
> + __RD(0), RS1(vaddr), RS2(asid))
> +
> +#define HFENCE_GVMA(gaddr, vmid) \
> + INSN_R(OPCODE_SYSTEM, FUNC3(0), FUNC7(49), \
> + __RD(0), RS1(gaddr), RS2(vmid))
> +
> #endif /* __ASM_INSN_DEF_H */
> diff --git a/arch/riscv/kvm/tlb.c b/arch/riscv/kvm/tlb.c
> index 1a76d0b1907d..1ce3394b3acf 100644
> --- a/arch/riscv/kvm/tlb.c
> +++ b/arch/riscv/kvm/tlb.c
> @@ -12,22 +12,7 @@
> #include <linux/kvm_host.h>
> #include <asm/cacheflush.h>
> #include <asm/csr.h>
> -
> -/*
> - * 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
> - */
> +#include <asm/insn-def.h>
>
> void kvm_riscv_local_hfence_gvma_vmid_gpa(unsigned long vmid,
> gpa_t gpa, gpa_t gpsz,
> @@ -40,32 +25,14 @@ void kvm_riscv_local_hfence_gvma_vmid_gpa(unsigned long vmid,
> return;
> }
>
> - for (pos = gpa; pos < (gpa + gpsz); pos += BIT(order)) {
> - /*
> - * rs1 = a0 (GPA >> 2)
> - * rs2 = a1 (VMID)
> - * HFENCE.GVMA a0, a1
> - * 0110001 01011 01010 000 00000 1110011
> - */
> - asm volatile ("srli a0, %0, 2\n"
> - "add a1, %1, zero\n"
> - ".word 0x62b50073\n"
> - :: "r" (pos), "r" (vmid)
> - : "a0", "a1", "memory");
> - }
> + for (pos = gpa; pos < (gpa + gpsz); pos += BIT(order))
> + asm volatile (HFENCE_GVMA(%0, %1)
> + : : "r" (pos >> 2), "r" (vmid) : "memory");
> }
>
> void kvm_riscv_local_hfence_gvma_vmid_all(unsigned long vmid)
> {
> - /*
> - * rs1 = zero
> - * rs2 = a0 (VMID)
> - * HFENCE.GVMA zero, a0
> - * 0110001 01010 00000 000 00000 1110011
> - */
> - asm volatile ("add a0, %0, zero\n"
> - ".word 0x62a00073\n"
> - :: "r" (vmid) : "a0", "memory");
> + asm volatile(HFENCE_GVMA(zero, %0) : : "r" (vmid) : "memory");
> }
>
> void kvm_riscv_local_hfence_gvma_gpa(gpa_t gpa, gpa_t gpsz,
> @@ -78,46 +45,16 @@ void kvm_riscv_local_hfence_gvma_gpa(gpa_t gpa, gpa_t gpsz,
> return;
> }
>
> - for (pos = gpa; pos < (gpa + gpsz); pos += BIT(order)) {
> - /*
> - * rs1 = a0 (GPA >> 2)
> - * rs2 = zero
> - * HFENCE.GVMA a0
> - * 0110001 00000 01010 000 00000 1110011
> - */
> - asm volatile ("srli a0, %0, 2\n"
> - ".word 0x62050073\n"
> - :: "r" (pos) : "a0", "memory");
> - }
> + for (pos = gpa; pos < (gpa + gpsz); pos += BIT(order))
> + asm volatile(HFENCE_GVMA(%0, zero)
> + : : "r" (pos >> 2) : "memory");
> }
>
> void kvm_riscv_local_hfence_gvma_all(void)
> {
> - /*
> - * rs1 = zero
> - * rs2 = zero
> - * HFENCE.GVMA
> - * 0110001 00000 00000 000 00000 1110011
> - */
> - asm volatile (".word 0x62000073" ::: "memory");
> + asm volatile(HFENCE_GVMA(zero, zero) : : : "memory");
> }
>
> -/*
> - * Instruction encoding of hfence.gvma is:
> - * HFENCE.VVMA rs1, rs2
> - * HFENCE.VVMA zero, rs2
> - * HFENCE.VVMA rs1
> - * HFENCE.VVMA
> - *
> - * rs1!=zero and rs2!=zero ==> HFENCE.VVMA rs1, rs2
> - * rs1==zero and rs2!=zero ==> HFENCE.VVMA zero, rs2
> - * rs1!=zero and rs2==zero ==> HFENCE.VVMA rs1
> - * rs1==zero and rs2==zero ==> HFENCE.VVMA
> - *
> - * Instruction encoding of HFENCE.VVMA is:
> - * 0010001 rs2(5) rs1(5) 000 00000 1110011
> - */
> -
> void kvm_riscv_local_hfence_vvma_asid_gva(unsigned long vmid,
> unsigned long asid,
> unsigned long gva,
> @@ -133,19 +70,9 @@ void kvm_riscv_local_hfence_vvma_asid_gva(unsigned long vmid,
>
> hgatp = csr_swap(CSR_HGATP, vmid << HGATP_VMID_SHIFT);
>
> - for (pos = gva; pos < (gva + gvsz); pos += BIT(order)) {
> - /*
> - * rs1 = a0 (GVA)
> - * rs2 = a1 (ASID)
> - * HFENCE.VVMA a0, a1
> - * 0010001 01011 01010 000 00000 1110011
> - */
> - asm volatile ("add a0, %0, zero\n"
> - "add a1, %1, zero\n"
> - ".word 0x22b50073\n"
> - :: "r" (pos), "r" (asid)
> - : "a0", "a1", "memory");
> - }
> + for (pos = gva; pos < (gva + gvsz); pos += BIT(order))
> + asm volatile(HFENCE_VVMA(%0, %1)
> + : : "r" (pos), "r" (asid) : "memory");
>
> csr_write(CSR_HGATP, hgatp);
> }
> @@ -157,15 +84,7 @@ void kvm_riscv_local_hfence_vvma_asid_all(unsigned long vmid,
>
> hgatp = csr_swap(CSR_HGATP, vmid << HGATP_VMID_SHIFT);
>
> - /*
> - * rs1 = zero
> - * rs2 = a0 (ASID)
> - * HFENCE.VVMA zero, a0
> - * 0010001 01010 00000 000 00000 1110011
> - */
> - asm volatile ("add a0, %0, zero\n"
> - ".word 0x22a00073\n"
> - :: "r" (asid) : "a0", "memory");
> + asm volatile(HFENCE_VVMA(zero, %0) : : "r" (asid) : "memory");
>
> csr_write(CSR_HGATP, hgatp);
> }
> @@ -183,17 +102,9 @@ void kvm_riscv_local_hfence_vvma_gva(unsigned long vmid,
>
> hgatp = csr_swap(CSR_HGATP, vmid << HGATP_VMID_SHIFT);
>
> - for (pos = gva; pos < (gva + gvsz); pos += BIT(order)) {
> - /*
> - * rs1 = a0 (GVA)
> - * rs2 = zero
> - * HFENCE.VVMA a0
> - * 0010001 00000 01010 000 00000 1110011
> - */
> - asm volatile ("add a0, %0, zero\n"
> - ".word 0x22050073\n"
> - :: "r" (pos) : "a0", "memory");
> - }
> + for (pos = gva; pos < (gva + gvsz); pos += BIT(order))
> + asm volatile(HFENCE_VVMA(%0, zero)
> + : : "r" (pos) : "memory");
>
> csr_write(CSR_HGATP, hgatp);
> }
> @@ -204,13 +115,7 @@ void kvm_riscv_local_hfence_vvma_all(unsigned long vmid)
>
> hgatp = csr_swap(CSR_HGATP, vmid << HGATP_VMID_SHIFT);
>
> - /*
> - * rs1 = zero
> - * rs2 = zero
> - * HFENCE.VVMA
> - * 0010001 00000 00000 000 00000 1110011
> - */
> - asm volatile (".word 0x22000073" ::: "memory");
> + asm volatile(HFENCE_VVMA(zero, zero) : : : "memory");
>
> csr_write(CSR_HGATP, hgatp);
> }
> --
> 2.37.2
>
On Wed, Aug 31, 2022 at 10:55 PM Andrew Jones <[email protected]> wrote:
>
> Introduce hlv instruction encodings and apply them to KVM's use.
> We're careful not to introduce hlv.d to 32-bit builds. Indeed,
> we ensure the build fails if someone tries to use it.
>
> Signed-off-by: Andrew Jones <[email protected]>
> Reviewed-by: Anup Patel <[email protected]>
I have queued this patch for Linux-6.1
Thanks,
Anup
> ---
> arch/riscv/include/asm/insn-def.h | 17 +++++++++++++++++
> arch/riscv/kvm/vcpu_exit.c | 29 +++++------------------------
> 2 files changed, 22 insertions(+), 24 deletions(-)
>
> diff --git a/arch/riscv/include/asm/insn-def.h b/arch/riscv/include/asm/insn-def.h
> index 86c1f602413b..8fe9036efb68 100644
> --- a/arch/riscv/include/asm/insn-def.h
> +++ b/arch/riscv/include/asm/insn-def.h
> @@ -93,4 +93,21 @@
> INSN_R(OPCODE_SYSTEM, FUNC3(0), FUNC7(49), \
> __RD(0), RS1(gaddr), RS2(vmid))
>
> +#define HLVX_HU(dest, addr) \
> + INSN_R(OPCODE_SYSTEM, FUNC3(4), FUNC7(50), \
> + RD(dest), RS1(addr), __RS2(3))
> +
> +#define HLV_W(dest, addr) \
> + INSN_R(OPCODE_SYSTEM, FUNC3(4), FUNC7(52), \
> + RD(dest), RS1(addr), __RS2(0))
> +
> +#ifdef CONFIG_64BIT
> +#define HLV_D(dest, addr) \
> + INSN_R(OPCODE_SYSTEM, FUNC3(4), FUNC7(54), \
> + RD(dest), RS1(addr), __RS2(0))
> +#else
> +#define HLV_D(dest, addr) \
> + __ASM_STR(.error "hlv.d requires 64-bit support")
> +#endif
> +
> #endif /* __ASM_INSN_DEF_H */
> diff --git a/arch/riscv/kvm/vcpu_exit.c b/arch/riscv/kvm/vcpu_exit.c
> index d5c36386878a..da793f113a72 100644
> --- a/arch/riscv/kvm/vcpu_exit.c
> +++ b/arch/riscv/kvm/vcpu_exit.c
> @@ -8,6 +8,7 @@
>
> #include <linux/kvm_host.h>
> #include <asm/csr.h>
> +#include <asm/insn-def.h>
>
> static int gstage_page_fault(struct kvm_vcpu *vcpu, struct kvm_run *run,
> struct kvm_cpu_trap *trap)
> @@ -82,22 +83,12 @@ unsigned long kvm_riscv_vcpu_unpriv_read(struct kvm_vcpu *vcpu,
> ".option push\n"
> ".option norvc\n"
> "add %[ttmp], %[taddr], 0\n"
> - /*
> - * HLVX.HU %[val], (%[addr])
> - * HLVX.HU t0, (t2)
> - * 0110010 00011 00111 100 00101 1110011
> - */
> - ".word 0x6433c2f3\n"
> + HLVX_HU(%[val], %[addr])
> "andi %[tmp], %[val], 3\n"
> "addi %[tmp], %[tmp], -3\n"
> "bne %[tmp], zero, 2f\n"
> "addi %[addr], %[addr], 2\n"
> - /*
> - * HLVX.HU %[tmp], (%[addr])
> - * HLVX.HU t1, (t2)
> - * 0110010 00011 00111 100 00110 1110011
> - */
> - ".word 0x6433c373\n"
> + HLVX_HU(%[tmp], %[addr])
> "sll %[tmp], %[tmp], 16\n"
> "add %[val], %[val], %[tmp]\n"
> "2:\n"
> @@ -121,19 +112,9 @@ unsigned long kvm_riscv_vcpu_unpriv_read(struct kvm_vcpu *vcpu,
> ".option norvc\n"
> "add %[ttmp], %[taddr], 0\n"
> #ifdef CONFIG_64BIT
> - /*
> - * HLV.D %[val], (%[addr])
> - * HLV.D t0, (t2)
> - * 0110110 00000 00111 100 00101 1110011
> - */
> - ".word 0x6c03c2f3\n"
> + HLV_D(%[val], %[addr])
> #else
> - /*
> - * HLV.W %[val], (%[addr])
> - * HLV.W t0, (t2)
> - * 0110100 00000 00111 100 00101 1110011
> - */
> - ".word 0x6803c2f3\n"
> + HLV_W(%[val], %[addr])
> #endif
> ".option pop"
> : [val] "=&r" (val),
> --
> 2.37.2
>
On Wed, Aug 31, 2022 at 10:55 PM Andrew Jones <[email protected]> wrote:
>
> When encoding instructions it's sometimes necessary to set a
> register field to a precise number. This is easiest to do using
> the x<num> naming.
>
> Signed-off-by: Andrew Jones <[email protected]>
> Reviewed-by: Anup Patel <[email protected]>
I have queued this patch for Linux-6.1
Thanks,
Anup
> ---
> arch/riscv/include/asm/gpr-num.h | 8 ++++++++
> 1 file changed, 8 insertions(+)
>
> diff --git a/arch/riscv/include/asm/gpr-num.h b/arch/riscv/include/asm/gpr-num.h
> index dfee2829fc7c..efeb5edf8a3a 100644
> --- a/arch/riscv/include/asm/gpr-num.h
> +++ b/arch/riscv/include/asm/gpr-num.h
> @@ -3,6 +3,11 @@
> #define __ASM_GPR_NUM_H
>
> #ifdef __ASSEMBLY__
> +
> + .irp num,0,1,2,3,4,5,6,7,8,9,10,11,12,13,14,15,16,17,18,19,20,21,22,23,24,25,26,27,28,29,30,31
> + .equ .L__gpr_num_x\num, \num
> + .endr
> +
> .equ .L__gpr_num_zero, 0
> .equ .L__gpr_num_ra, 1
> .equ .L__gpr_num_sp, 2
> @@ -39,6 +44,9 @@
> #else /* __ASSEMBLY__ */
>
> #define __DEFINE_ASM_GPR_NUMS \
> +" .irp num,0,1,2,3,4,5,6,7,8,9,10,11,12,13,14,15,16,17,18,19,20,21,22,23,24,25,26,27,28,29,30,31\n" \
> +" .equ .L__gpr_num_x\\num, \\num\n" \
> +" .endr\n" \
> " .equ .L__gpr_num_zero, 0\n" \
> " .equ .L__gpr_num_ra, 1\n" \
> " .equ .L__gpr_num_sp, 2\n" \
> --
> 2.37.2
>
Am Mittwoch, 31. August 2022, 19:24:58 CEST schrieb Andrew Jones:
> When compiling with toolchains that haven't yet been taught about
> new instructions we need to encode them ourselves. Create a new file
> where support for instruction definitions will evolve. We initiate
> the file with a macro called INSN_R(), which implements the R-type
> instruction encoding. INSN_R() will use the assembler's .insn
> directive when available, which should give the assembler a chance
> to do some validation. When .insn is not available we fall back to
> manual encoding.
>
> Not only should using instruction encoding macros improve readability
> and maintainability of code over the alternative of inserting
> instructions directly (e.g. '.word 0xc0de'), but we should also gain
> potential for more optimized code after compilation because the
> compiler will have control over the input and output registers used.
>
> Signed-off-by: Andrew Jones <[email protected]>
> Reviewed-by: Anup Patel <[email protected]>
> ---
> arch/riscv/Kconfig | 3 ++
> arch/riscv/include/asm/insn-def.h | 86 +++++++++++++++++++++++++++++++
> 2 files changed, 89 insertions(+)
> create mode 100644 arch/riscv/include/asm/insn-def.h
>
> diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
> index ed66c31e4655..f8f3b316b838 100644
> --- a/arch/riscv/Kconfig
> +++ b/arch/riscv/Kconfig
> @@ -227,6 +227,9 @@ config RISCV_DMA_NONCOHERENT
> select ARCH_HAS_SETUP_DMA_OPS
> select DMA_DIRECT_REMAP
>
> +config AS_HAS_INSN
> + def_bool $(as-instr,.insn r 51$(comma) 0$(comma) 0$(comma) t0$(comma) t0$(comma) zero)
> +
> source "arch/riscv/Kconfig.socs"
> source "arch/riscv/Kconfig.erratas"
>
> diff --git a/arch/riscv/include/asm/insn-def.h b/arch/riscv/include/asm/insn-def.h
> new file mode 100644
> index 000000000000..2dcd1d4781bf
> --- /dev/null
> +++ b/arch/riscv/include/asm/insn-def.h
> @@ -0,0 +1,86 @@
> +/* SPDX-License-Identifier: GPL-2.0-only */
> +
> +#ifndef __ASM_INSN_DEF_H
> +#define __ASM_INSN_DEF_H
> +
> +#include <asm/asm.h>
> +
> +#define INSN_R_FUNC7_SHIFT 25
> +#define INSN_R_RS2_SHIFT 20
> +#define INSN_R_RS1_SHIFT 15
> +#define INSN_R_FUNC3_SHIFT 12
> +#define INSN_R_RD_SHIFT 7
> +#define INSN_R_OPCODE_SHIFT 0
> +
> +#ifdef __ASSEMBLY__
> +
> +#ifdef CONFIG_AS_HAS_INSN
> +
> + .macro insn_r, opcode, func3, func7, rd, rs1, rs2
> + .insn r \opcode, \func3, \func7, \rd, \rs1, \rs2
> + .endm
> +
> +#else
> +
> +#include <asm/gpr-num.h>
> +
> + .macro insn_r, opcode, func3, func7, rd, rs1, rs2
> + .4byte ((\opcode << INSN_R_OPCODE_SHIFT) | \
> + (\func3 << INSN_R_FUNC3_SHIFT) | \
> + (\func7 << INSN_R_FUNC7_SHIFT) | \
> + (.L__gpr_num_\rd << INSN_R_RD_SHIFT) | \
> + (.L__gpr_num_\rs1 << INSN_R_RS1_SHIFT) | \
> + (.L__gpr_num_\rs2 << INSN_R_RS2_SHIFT))
> + .endm
> +
> +#endif
> +
> +#define INSN_R(...) insn_r __VA_ARGS__
> +
> +#else /* ! __ASSEMBLY__ */
> +
> +#ifdef CONFIG_AS_HAS_INSN
> +
> +#define INSN_R(opcode, func3, func7, rd, rs1, rs2) \
> + ".insn r " opcode ", " func3 ", " func7 ", " rd ", " rs1 ", " rs2 "\n"
> +
> +#else
> +
> +#include <linux/stringify.h>
> +#include <asm/gpr-num.h>
> +
> +#define DEFINE_INSN_R \
> + __DEFINE_ASM_GPR_NUMS \
> +" .macro insn_r, opcode, func3, func7, rd, rs1, rs2\n" \
> +" .4byte ((\\opcode << " __stringify(INSN_R_OPCODE_SHIFT) ") |" \
> +" (\\func3 << " __stringify(INSN_R_FUNC3_SHIFT) ") |" \
> +" (\\func7 << " __stringify(INSN_R_FUNC7_SHIFT) ") |" \
> +" (.L__gpr_num_\\rd << " __stringify(INSN_R_RD_SHIFT) ") |" \
> +" (.L__gpr_num_\\rs1 << " __stringify(INSN_R_RS1_SHIFT) ") |" \
> +" (.L__gpr_num_\\rs2 << " __stringify(INSN_R_RS2_SHIFT) "))\n" \
> +" .endm\n"
> +
> +#define UNDEFINE_INSN_R \
> +" .purgem insn_r\n"
> +
> +#define INSN_R(opcode, func3, func7, rd, rs1, rs2) \
> + DEFINE_INSN_R \
> + "insn_r " opcode ", " func3 ", " func7 ", " rd ", " rs1 ", " rs2 "\n" \
> + UNDEFINE_INSN_R
> +
> +#endif
> +
> +#endif /* ! __ASSEMBLY__ */
> +
> +#define OPCODE(v) __ASM_STR(v)
> +#define FUNC3(v) __ASM_STR(v)
> +#define FUNC7(v) __ASM_STR(v)
> +#define RD(v) __ASM_STR(v)
> +#define RS1(v) __ASM_STR(v)
> +#define RS2(v) __ASM_STR(v)
you might want some sort of prefix here
RISCV_RS1(v) ?
While trying to adapt this for the cmo stuff I ran into the issue
of bpf complaining that "IMM" is already defined there.
And names above are generic enough that these also
might conflict with other stuff.
> +#define __REG(v) __ASM_STR(x ## v)
> +#define __RD(v) __REG(v)
> +#define __RS1(v) __REG(v)
> +#define __RS2(v) __REG(v)
> +
> +#endif /* __ASM_INSN_DEF_H */
>
On Thu, Sep 8, 2022 at 9:19 PM Heiko Stübner <[email protected]> wrote:
>
> Am Mittwoch, 31. August 2022, 19:24:58 CEST schrieb Andrew Jones:
> > When compiling with toolchains that haven't yet been taught about
> > new instructions we need to encode them ourselves. Create a new file
> > where support for instruction definitions will evolve. We initiate
> > the file with a macro called INSN_R(), which implements the R-type
> > instruction encoding. INSN_R() will use the assembler's .insn
> > directive when available, which should give the assembler a chance
> > to do some validation. When .insn is not available we fall back to
> > manual encoding.
> >
> > Not only should using instruction encoding macros improve readability
> > and maintainability of code over the alternative of inserting
> > instructions directly (e.g. '.word 0xc0de'), but we should also gain
> > potential for more optimized code after compilation because the
> > compiler will have control over the input and output registers used.
> >
> > Signed-off-by: Andrew Jones <[email protected]>
> > Reviewed-by: Anup Patel <[email protected]>
> > ---
> > arch/riscv/Kconfig | 3 ++
> > arch/riscv/include/asm/insn-def.h | 86 +++++++++++++++++++++++++++++++
> > 2 files changed, 89 insertions(+)
> > create mode 100644 arch/riscv/include/asm/insn-def.h
> >
> > diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
> > index ed66c31e4655..f8f3b316b838 100644
> > --- a/arch/riscv/Kconfig
> > +++ b/arch/riscv/Kconfig
> > @@ -227,6 +227,9 @@ config RISCV_DMA_NONCOHERENT
> > select ARCH_HAS_SETUP_DMA_OPS
> > select DMA_DIRECT_REMAP
> >
> > +config AS_HAS_INSN
> > + def_bool $(as-instr,.insn r 51$(comma) 0$(comma) 0$(comma) t0$(comma) t0$(comma) zero)
> > +
> > source "arch/riscv/Kconfig.socs"
> > source "arch/riscv/Kconfig.erratas"
> >
> > diff --git a/arch/riscv/include/asm/insn-def.h b/arch/riscv/include/asm/insn-def.h
> > new file mode 100644
> > index 000000000000..2dcd1d4781bf
> > --- /dev/null
> > +++ b/arch/riscv/include/asm/insn-def.h
> > @@ -0,0 +1,86 @@
> > +/* SPDX-License-Identifier: GPL-2.0-only */
> > +
> > +#ifndef __ASM_INSN_DEF_H
> > +#define __ASM_INSN_DEF_H
> > +
> > +#include <asm/asm.h>
> > +
> > +#define INSN_R_FUNC7_SHIFT 25
> > +#define INSN_R_RS2_SHIFT 20
> > +#define INSN_R_RS1_SHIFT 15
> > +#define INSN_R_FUNC3_SHIFT 12
> > +#define INSN_R_RD_SHIFT 7
> > +#define INSN_R_OPCODE_SHIFT 0
> > +
> > +#ifdef __ASSEMBLY__
> > +
> > +#ifdef CONFIG_AS_HAS_INSN
> > +
> > + .macro insn_r, opcode, func3, func7, rd, rs1, rs2
> > + .insn r \opcode, \func3, \func7, \rd, \rs1, \rs2
> > + .endm
> > +
> > +#else
> > +
> > +#include <asm/gpr-num.h>
> > +
> > + .macro insn_r, opcode, func3, func7, rd, rs1, rs2
> > + .4byte ((\opcode << INSN_R_OPCODE_SHIFT) | \
> > + (\func3 << INSN_R_FUNC3_SHIFT) | \
> > + (\func7 << INSN_R_FUNC7_SHIFT) | \
> > + (.L__gpr_num_\rd << INSN_R_RD_SHIFT) | \
> > + (.L__gpr_num_\rs1 << INSN_R_RS1_SHIFT) | \
> > + (.L__gpr_num_\rs2 << INSN_R_RS2_SHIFT))
> > + .endm
> > +
> > +#endif
> > +
> > +#define INSN_R(...) insn_r __VA_ARGS__
> > +
> > +#else /* ! __ASSEMBLY__ */
> > +
> > +#ifdef CONFIG_AS_HAS_INSN
> > +
> > +#define INSN_R(opcode, func3, func7, rd, rs1, rs2) \
> > + ".insn r " opcode ", " func3 ", " func7 ", " rd ", " rs1 ", " rs2 "\n"
> > +
> > +#else
> > +
> > +#include <linux/stringify.h>
> > +#include <asm/gpr-num.h>
> > +
> > +#define DEFINE_INSN_R \
> > + __DEFINE_ASM_GPR_NUMS \
> > +" .macro insn_r, opcode, func3, func7, rd, rs1, rs2\n" \
> > +" .4byte ((\\opcode << " __stringify(INSN_R_OPCODE_SHIFT) ") |" \
> > +" (\\func3 << " __stringify(INSN_R_FUNC3_SHIFT) ") |" \
> > +" (\\func7 << " __stringify(INSN_R_FUNC7_SHIFT) ") |" \
> > +" (.L__gpr_num_\\rd << " __stringify(INSN_R_RD_SHIFT) ") |" \
> > +" (.L__gpr_num_\\rs1 << " __stringify(INSN_R_RS1_SHIFT) ") |" \
> > +" (.L__gpr_num_\\rs2 << " __stringify(INSN_R_RS2_SHIFT) "))\n" \
> > +" .endm\n"
> > +
> > +#define UNDEFINE_INSN_R \
> > +" .purgem insn_r\n"
> > +
> > +#define INSN_R(opcode, func3, func7, rd, rs1, rs2) \
> > + DEFINE_INSN_R \
> > + "insn_r " opcode ", " func3 ", " func7 ", " rd ", " rs1 ", " rs2 "\n" \
> > + UNDEFINE_INSN_R
> > +
> > +#endif
> > +
> > +#endif /* ! __ASSEMBLY__ */
> > +
> > +#define OPCODE(v) __ASM_STR(v)
> > +#define FUNC3(v) __ASM_STR(v)
> > +#define FUNC7(v) __ASM_STR(v)
> > +#define RD(v) __ASM_STR(v)
> > +#define RS1(v) __ASM_STR(v)
> > +#define RS2(v) __ASM_STR(v)
>
> you might want some sort of prefix here
> RISCV_RS1(v) ?
>
> While trying to adapt this for the cmo stuff I ran into the issue
> of bpf complaining that "IMM" is already defined there.
>
> And names above are generic enough that these also
> might conflict with other stuff.
I can update the KVM RISC-V queue but I suggest to
use a smaller prefix. Maybe RV_xyz() ?
Regards,
Anup
>
>
>
>
> > +#define __REG(v) __ASM_STR(x ## v)
> > +#define __RD(v) __REG(v)
> > +#define __RS1(v) __REG(v)
> > +#define __RS2(v) __REG(v)
> > +
> > +#endif /* __ASM_INSN_DEF_H */
> >
>
>
>
>
On Thu, Sep 08, 2022 at 05:49:44PM +0200, Heiko St?bner wrote:
...
> > +#define OPCODE(v) __ASM_STR(v)
> > +#define FUNC3(v) __ASM_STR(v)
> > +#define FUNC7(v) __ASM_STR(v)
> > +#define RD(v) __ASM_STR(v)
> > +#define RS1(v) __ASM_STR(v)
> > +#define RS2(v) __ASM_STR(v)
>
> you might want some sort of prefix here
> RISCV_RS1(v) ?
>
> While trying to adapt this for the cmo stuff I ran into the issue
> of bpf complaining that "IMM" is already defined there.
>
> And names above are generic enough that these also
> might conflict with other stuff.
>
Ah, thanks for the heads up. Indeed, if this gets included in another
header, which gets widely included, then we have a good chance of
bumping into something. It's a pity, but, as you suggest, we probably
need prefixes and __ isn't likely enough alone. I also see __REG is
used elsewhere.
Thanks,
drew
>
>
>
> > +#define __REG(v) __ASM_STR(x ## v)
> > +#define __RD(v) __REG(v)
> > +#define __RS1(v) __REG(v)
> > +#define __RS2(v) __REG(v)
> > +
> > +#endif /* __ASM_INSN_DEF_H */
> >
>
>
>
>
On Thu, Sep 08, 2022 at 09:33:15PM +0530, Anup Patel wrote:
> On Thu, Sep 8, 2022 at 9:19 PM Heiko St?bner <[email protected]> wrote:
> >
> > Am Mittwoch, 31. August 2022, 19:24:58 CEST schrieb Andrew Jones:
> > > When compiling with toolchains that haven't yet been taught about
> > > new instructions we need to encode them ourselves. Create a new file
> > > where support for instruction definitions will evolve. We initiate
> > > the file with a macro called INSN_R(), which implements the R-type
> > > instruction encoding. INSN_R() will use the assembler's .insn
> > > directive when available, which should give the assembler a chance
> > > to do some validation. When .insn is not available we fall back to
> > > manual encoding.
> > >
> > > Not only should using instruction encoding macros improve readability
> > > and maintainability of code over the alternative of inserting
> > > instructions directly (e.g. '.word 0xc0de'), but we should also gain
> > > potential for more optimized code after compilation because the
> > > compiler will have control over the input and output registers used.
> > >
> > > Signed-off-by: Andrew Jones <[email protected]>
> > > Reviewed-by: Anup Patel <[email protected]>
> > > ---
> > > arch/riscv/Kconfig | 3 ++
> > > arch/riscv/include/asm/insn-def.h | 86 +++++++++++++++++++++++++++++++
> > > 2 files changed, 89 insertions(+)
> > > create mode 100644 arch/riscv/include/asm/insn-def.h
> > >
> > > diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
> > > index ed66c31e4655..f8f3b316b838 100644
> > > --- a/arch/riscv/Kconfig
> > > +++ b/arch/riscv/Kconfig
> > > @@ -227,6 +227,9 @@ config RISCV_DMA_NONCOHERENT
> > > select ARCH_HAS_SETUP_DMA_OPS
> > > select DMA_DIRECT_REMAP
> > >
> > > +config AS_HAS_INSN
> > > + def_bool $(as-instr,.insn r 51$(comma) 0$(comma) 0$(comma) t0$(comma) t0$(comma) zero)
> > > +
> > > source "arch/riscv/Kconfig.socs"
> > > source "arch/riscv/Kconfig.erratas"
> > >
> > > diff --git a/arch/riscv/include/asm/insn-def.h b/arch/riscv/include/asm/insn-def.h
> > > new file mode 100644
> > > index 000000000000..2dcd1d4781bf
> > > --- /dev/null
> > > +++ b/arch/riscv/include/asm/insn-def.h
> > > @@ -0,0 +1,86 @@
> > > +/* SPDX-License-Identifier: GPL-2.0-only */
> > > +
> > > +#ifndef __ASM_INSN_DEF_H
> > > +#define __ASM_INSN_DEF_H
> > > +
> > > +#include <asm/asm.h>
> > > +
> > > +#define INSN_R_FUNC7_SHIFT 25
> > > +#define INSN_R_RS2_SHIFT 20
> > > +#define INSN_R_RS1_SHIFT 15
> > > +#define INSN_R_FUNC3_SHIFT 12
> > > +#define INSN_R_RD_SHIFT 7
> > > +#define INSN_R_OPCODE_SHIFT 0
> > > +
> > > +#ifdef __ASSEMBLY__
> > > +
> > > +#ifdef CONFIG_AS_HAS_INSN
> > > +
> > > + .macro insn_r, opcode, func3, func7, rd, rs1, rs2
> > > + .insn r \opcode, \func3, \func7, \rd, \rs1, \rs2
> > > + .endm
> > > +
> > > +#else
> > > +
> > > +#include <asm/gpr-num.h>
> > > +
> > > + .macro insn_r, opcode, func3, func7, rd, rs1, rs2
> > > + .4byte ((\opcode << INSN_R_OPCODE_SHIFT) | \
> > > + (\func3 << INSN_R_FUNC3_SHIFT) | \
> > > + (\func7 << INSN_R_FUNC7_SHIFT) | \
> > > + (.L__gpr_num_\rd << INSN_R_RD_SHIFT) | \
> > > + (.L__gpr_num_\rs1 << INSN_R_RS1_SHIFT) | \
> > > + (.L__gpr_num_\rs2 << INSN_R_RS2_SHIFT))
> > > + .endm
> > > +
> > > +#endif
> > > +
> > > +#define INSN_R(...) insn_r __VA_ARGS__
> > > +
> > > +#else /* ! __ASSEMBLY__ */
> > > +
> > > +#ifdef CONFIG_AS_HAS_INSN
> > > +
> > > +#define INSN_R(opcode, func3, func7, rd, rs1, rs2) \
> > > + ".insn r " opcode ", " func3 ", " func7 ", " rd ", " rs1 ", " rs2 "\n"
> > > +
> > > +#else
> > > +
> > > +#include <linux/stringify.h>
> > > +#include <asm/gpr-num.h>
> > > +
> > > +#define DEFINE_INSN_R \
> > > + __DEFINE_ASM_GPR_NUMS \
> > > +" .macro insn_r, opcode, func3, func7, rd, rs1, rs2\n" \
> > > +" .4byte ((\\opcode << " __stringify(INSN_R_OPCODE_SHIFT) ") |" \
> > > +" (\\func3 << " __stringify(INSN_R_FUNC3_SHIFT) ") |" \
> > > +" (\\func7 << " __stringify(INSN_R_FUNC7_SHIFT) ") |" \
> > > +" (.L__gpr_num_\\rd << " __stringify(INSN_R_RD_SHIFT) ") |" \
> > > +" (.L__gpr_num_\\rs1 << " __stringify(INSN_R_RS1_SHIFT) ") |" \
> > > +" (.L__gpr_num_\\rs2 << " __stringify(INSN_R_RS2_SHIFT) "))\n" \
> > > +" .endm\n"
> > > +
> > > +#define UNDEFINE_INSN_R \
> > > +" .purgem insn_r\n"
> > > +
> > > +#define INSN_R(opcode, func3, func7, rd, rs1, rs2) \
> > > + DEFINE_INSN_R \
> > > + "insn_r " opcode ", " func3 ", " func7 ", " rd ", " rs1 ", " rs2 "\n" \
> > > + UNDEFINE_INSN_R
> > > +
> > > +#endif
> > > +
> > > +#endif /* ! __ASSEMBLY__ */
> > > +
> > > +#define OPCODE(v) __ASM_STR(v)
> > > +#define FUNC3(v) __ASM_STR(v)
> > > +#define FUNC7(v) __ASM_STR(v)
> > > +#define RD(v) __ASM_STR(v)
> > > +#define RS1(v) __ASM_STR(v)
> > > +#define RS2(v) __ASM_STR(v)
> >
> > you might want some sort of prefix here
> > RISCV_RS1(v) ?
> >
> > While trying to adapt this for the cmo stuff I ran into the issue
> > of bpf complaining that "IMM" is already defined there.
> >
> > And names above are generic enough that these also
> > might conflict with other stuff.
>
> I can update the KVM RISC-V queue but I suggest to
> use a smaller prefix. Maybe RV_xyz() ?
Ack and thanks for doing the update.
drew
>
> Regards,
> Anup
>
> >
> >
> >
> >
> > > +#define __REG(v) __ASM_STR(x ## v)
> > > +#define __RD(v) __REG(v)
> > > +#define __RS1(v) __REG(v)
> > > +#define __RS2(v) __REG(v)
> > > +
> > > +#endif /* __ASM_INSN_DEF_H */
> > >
> >
> >
> >
> >
On Thu, Sep 8, 2022 at 9:20 PM Heiko Stübner <[email protected]> wrote:
>
> Am Mittwoch, 31. August 2022, 19:24:58 CEST schrieb Andrew Jones:
> > When compiling with toolchains that haven't yet been taught about
> > new instructions we need to encode them ourselves. Create a new file
> > where support for instruction definitions will evolve. We initiate
> > the file with a macro called INSN_R(), which implements the R-type
> > instruction encoding. INSN_R() will use the assembler's .insn
> > directive when available, which should give the assembler a chance
> > to do some validation. When .insn is not available we fall back to
> > manual encoding.
> >
> > Not only should using instruction encoding macros improve readability
> > and maintainability of code over the alternative of inserting
> > instructions directly (e.g. '.word 0xc0de'), but we should also gain
> > potential for more optimized code after compilation because the
> > compiler will have control over the input and output registers used.
> >
> > Signed-off-by: Andrew Jones <[email protected]>
> > Reviewed-by: Anup Patel <[email protected]>
> > ---
> > arch/riscv/Kconfig | 3 ++
> > arch/riscv/include/asm/insn-def.h | 86 +++++++++++++++++++++++++++++++
> > 2 files changed, 89 insertions(+)
> > create mode 100644 arch/riscv/include/asm/insn-def.h
> >
> > diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
> > index ed66c31e4655..f8f3b316b838 100644
> > --- a/arch/riscv/Kconfig
> > +++ b/arch/riscv/Kconfig
> > @@ -227,6 +227,9 @@ config RISCV_DMA_NONCOHERENT
> > select ARCH_HAS_SETUP_DMA_OPS
> > select DMA_DIRECT_REMAP
> >
> > +config AS_HAS_INSN
> > + def_bool $(as-instr,.insn r 51$(comma) 0$(comma) 0$(comma) t0$(comma) t0$(comma) zero)
> > +
> > source "arch/riscv/Kconfig.socs"
> > source "arch/riscv/Kconfig.erratas"
> >
> > diff --git a/arch/riscv/include/asm/insn-def.h b/arch/riscv/include/asm/insn-def.h
> > new file mode 100644
> > index 000000000000..2dcd1d4781bf
> > --- /dev/null
> > +++ b/arch/riscv/include/asm/insn-def.h
> > @@ -0,0 +1,86 @@
> > +/* SPDX-License-Identifier: GPL-2.0-only */
> > +
> > +#ifndef __ASM_INSN_DEF_H
> > +#define __ASM_INSN_DEF_H
> > +
> > +#include <asm/asm.h>
> > +
> > +#define INSN_R_FUNC7_SHIFT 25
> > +#define INSN_R_RS2_SHIFT 20
> > +#define INSN_R_RS1_SHIFT 15
> > +#define INSN_R_FUNC3_SHIFT 12
> > +#define INSN_R_RD_SHIFT 7
> > +#define INSN_R_OPCODE_SHIFT 0
> > +
> > +#ifdef __ASSEMBLY__
> > +
> > +#ifdef CONFIG_AS_HAS_INSN
> > +
> > + .macro insn_r, opcode, func3, func7, rd, rs1, rs2
> > + .insn r \opcode, \func3, \func7, \rd, \rs1, \rs2
> > + .endm
> > +
> > +#else
> > +
> > +#include <asm/gpr-num.h>
> > +
> > + .macro insn_r, opcode, func3, func7, rd, rs1, rs2
> > + .4byte ((\opcode << INSN_R_OPCODE_SHIFT) | \
> > + (\func3 << INSN_R_FUNC3_SHIFT) | \
> > + (\func7 << INSN_R_FUNC7_SHIFT) | \
> > + (.L__gpr_num_\rd << INSN_R_RD_SHIFT) | \
> > + (.L__gpr_num_\rs1 << INSN_R_RS1_SHIFT) | \
> > + (.L__gpr_num_\rs2 << INSN_R_RS2_SHIFT))
> > + .endm
> > +
> > +#endif
> > +
> > +#define INSN_R(...) insn_r __VA_ARGS__
> > +
> > +#else /* ! __ASSEMBLY__ */
> > +
> > +#ifdef CONFIG_AS_HAS_INSN
> > +
> > +#define INSN_R(opcode, func3, func7, rd, rs1, rs2) \
> > + ".insn r " opcode ", " func3 ", " func7 ", " rd ", " rs1 ", " rs2 "\n"
> > +
> > +#else
> > +
> > +#include <linux/stringify.h>
> > +#include <asm/gpr-num.h>
> > +
> > +#define DEFINE_INSN_R \
> > + __DEFINE_ASM_GPR_NUMS \
> > +" .macro insn_r, opcode, func3, func7, rd, rs1, rs2\n" \
> > +" .4byte ((\\opcode << " __stringify(INSN_R_OPCODE_SHIFT) ") |" \
> > +" (\\func3 << " __stringify(INSN_R_FUNC3_SHIFT) ") |" \
> > +" (\\func7 << " __stringify(INSN_R_FUNC7_SHIFT) ") |" \
> > +" (.L__gpr_num_\\rd << " __stringify(INSN_R_RD_SHIFT) ") |" \
> > +" (.L__gpr_num_\\rs1 << " __stringify(INSN_R_RS1_SHIFT) ") |" \
> > +" (.L__gpr_num_\\rs2 << " __stringify(INSN_R_RS2_SHIFT) "))\n" \
> > +" .endm\n"
> > +
> > +#define UNDEFINE_INSN_R \
> > +" .purgem insn_r\n"
> > +
> > +#define INSN_R(opcode, func3, func7, rd, rs1, rs2) \
> > + DEFINE_INSN_R \
> > + "insn_r " opcode ", " func3 ", " func7 ", " rd ", " rs1 ", " rs2 "\n" \
> > + UNDEFINE_INSN_R
> > +
> > +#endif
> > +
> > +#endif /* ! __ASSEMBLY__ */
> > +
> > +#define OPCODE(v) __ASM_STR(v)
> > +#define FUNC3(v) __ASM_STR(v)
> > +#define FUNC7(v) __ASM_STR(v)
> > +#define RD(v) __ASM_STR(v)
> > +#define RS1(v) __ASM_STR(v)
> > +#define RS2(v) __ASM_STR(v)
>
> you might want some sort of prefix here
> RISCV_RS1(v) ?
>
> While trying to adapt this for the cmo stuff I ran into the issue
> of bpf complaining that "IMM" is already defined there.
>
> And names above are generic enough that these also
> might conflict with other stuff.
I have updated the KVM RISC-V queue to use the "RV_" prefix
in macro names.
Regards,
Anup
>
>
>
>
> > +#define __REG(v) __ASM_STR(x ## v)
> > +#define __RD(v) __REG(v)
> > +#define __RS1(v) __REG(v)
> > +#define __RS2(v) __REG(v)
> > +
> > +#endif /* __ASM_INSN_DEF_H */
> >
>
>
>
>
Am Freitag, 9. September 2022, 13:23:47 CEST schrieb Anup Patel:
> On Thu, Sep 8, 2022 at 9:20 PM Heiko St?bner <[email protected]> wrote:
> >
> > Am Mittwoch, 31. August 2022, 19:24:58 CEST schrieb Andrew Jones:
> > > When compiling with toolchains that haven't yet been taught about
> > > new instructions we need to encode them ourselves. Create a new file
> > > where support for instruction definitions will evolve. We initiate
> > > the file with a macro called INSN_R(), which implements the R-type
> > > instruction encoding. INSN_R() will use the assembler's .insn
> > > directive when available, which should give the assembler a chance
> > > to do some validation. When .insn is not available we fall back to
> > > manual encoding.
> > >
> > > Not only should using instruction encoding macros improve readability
> > > and maintainability of code over the alternative of inserting
> > > instructions directly (e.g. '.word 0xc0de'), but we should also gain
> > > potential for more optimized code after compilation because the
> > > compiler will have control over the input and output registers used.
> > >
> > > Signed-off-by: Andrew Jones <[email protected]>
> > > Reviewed-by: Anup Patel <[email protected]>
> > > ---
> > > arch/riscv/Kconfig | 3 ++
> > > arch/riscv/include/asm/insn-def.h | 86 +++++++++++++++++++++++++++++++
> > > 2 files changed, 89 insertions(+)
> > > create mode 100644 arch/riscv/include/asm/insn-def.h
> > >
> > > diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
> > > index ed66c31e4655..f8f3b316b838 100644
> > > --- a/arch/riscv/Kconfig
> > > +++ b/arch/riscv/Kconfig
> > > @@ -227,6 +227,9 @@ config RISCV_DMA_NONCOHERENT
> > > select ARCH_HAS_SETUP_DMA_OPS
> > > select DMA_DIRECT_REMAP
> > >
> > > +config AS_HAS_INSN
> > > + def_bool $(as-instr,.insn r 51$(comma) 0$(comma) 0$(comma) t0$(comma) t0$(comma) zero)
> > > +
> > > source "arch/riscv/Kconfig.socs"
> > > source "arch/riscv/Kconfig.erratas"
> > >
> > > diff --git a/arch/riscv/include/asm/insn-def.h b/arch/riscv/include/asm/insn-def.h
> > > new file mode 100644
> > > index 000000000000..2dcd1d4781bf
> > > --- /dev/null
> > > +++ b/arch/riscv/include/asm/insn-def.h
> > > @@ -0,0 +1,86 @@
> > > +/* SPDX-License-Identifier: GPL-2.0-only */
> > > +
> > > +#ifndef __ASM_INSN_DEF_H
> > > +#define __ASM_INSN_DEF_H
> > > +
> > > +#include <asm/asm.h>
> > > +
> > > +#define INSN_R_FUNC7_SHIFT 25
> > > +#define INSN_R_RS2_SHIFT 20
> > > +#define INSN_R_RS1_SHIFT 15
> > > +#define INSN_R_FUNC3_SHIFT 12
> > > +#define INSN_R_RD_SHIFT 7
> > > +#define INSN_R_OPCODE_SHIFT 0
> > > +
> > > +#ifdef __ASSEMBLY__
> > > +
> > > +#ifdef CONFIG_AS_HAS_INSN
> > > +
> > > + .macro insn_r, opcode, func3, func7, rd, rs1, rs2
> > > + .insn r \opcode, \func3, \func7, \rd, \rs1, \rs2
> > > + .endm
> > > +
> > > +#else
> > > +
> > > +#include <asm/gpr-num.h>
> > > +
> > > + .macro insn_r, opcode, func3, func7, rd, rs1, rs2
> > > + .4byte ((\opcode << INSN_R_OPCODE_SHIFT) | \
> > > + (\func3 << INSN_R_FUNC3_SHIFT) | \
> > > + (\func7 << INSN_R_FUNC7_SHIFT) | \
> > > + (.L__gpr_num_\rd << INSN_R_RD_SHIFT) | \
> > > + (.L__gpr_num_\rs1 << INSN_R_RS1_SHIFT) | \
> > > + (.L__gpr_num_\rs2 << INSN_R_RS2_SHIFT))
> > > + .endm
> > > +
> > > +#endif
> > > +
> > > +#define INSN_R(...) insn_r __VA_ARGS__
> > > +
> > > +#else /* ! __ASSEMBLY__ */
> > > +
> > > +#ifdef CONFIG_AS_HAS_INSN
> > > +
> > > +#define INSN_R(opcode, func3, func7, rd, rs1, rs2) \
> > > + ".insn r " opcode ", " func3 ", " func7 ", " rd ", " rs1 ", " rs2 "\n"
> > > +
> > > +#else
> > > +
> > > +#include <linux/stringify.h>
> > > +#include <asm/gpr-num.h>
> > > +
> > > +#define DEFINE_INSN_R \
> > > + __DEFINE_ASM_GPR_NUMS \
> > > +" .macro insn_r, opcode, func3, func7, rd, rs1, rs2\n" \
> > > +" .4byte ((\\opcode << " __stringify(INSN_R_OPCODE_SHIFT) ") |" \
> > > +" (\\func3 << " __stringify(INSN_R_FUNC3_SHIFT) ") |" \
> > > +" (\\func7 << " __stringify(INSN_R_FUNC7_SHIFT) ") |" \
> > > +" (.L__gpr_num_\\rd << " __stringify(INSN_R_RD_SHIFT) ") |" \
> > > +" (.L__gpr_num_\\rs1 << " __stringify(INSN_R_RS1_SHIFT) ") |" \
> > > +" (.L__gpr_num_\\rs2 << " __stringify(INSN_R_RS2_SHIFT) "))\n" \
> > > +" .endm\n"
> > > +
> > > +#define UNDEFINE_INSN_R \
> > > +" .purgem insn_r\n"
> > > +
> > > +#define INSN_R(opcode, func3, func7, rd, rs1, rs2) \
> > > + DEFINE_INSN_R \
> > > + "insn_r " opcode ", " func3 ", " func7 ", " rd ", " rs1 ", " rs2 "\n" \
> > > + UNDEFINE_INSN_R
> > > +
> > > +#endif
> > > +
> > > +#endif /* ! __ASSEMBLY__ */
> > > +
> > > +#define OPCODE(v) __ASM_STR(v)
> > > +#define FUNC3(v) __ASM_STR(v)
> > > +#define FUNC7(v) __ASM_STR(v)
> > > +#define RD(v) __ASM_STR(v)
> > > +#define RS1(v) __ASM_STR(v)
> > > +#define RS2(v) __ASM_STR(v)
> >
> > you might want some sort of prefix here
> > RISCV_RS1(v) ?
> >
> > While trying to adapt this for the cmo stuff I ran into the issue
> > of bpf complaining that "IMM" is already defined there.
> >
> > And names above are generic enough that these also
> > might conflict with other stuff.
>
> I have updated the KVM RISC-V queue to use the "RV_" prefix
> in macro names.
that is great to hear. Thanks a lot for doing that.
Heiko
> > > +#define __REG(v) __ASM_STR(x ## v)
> > > +#define __RD(v) __REG(v)
> > > +#define __RS1(v) __REG(v)
> > > +#define __RS2(v) __REG(v)
> > > +
> > > +#endif /* __ASM_INSN_DEF_H */
> > >
> >
> >
> >
> >
>