Re: [PATCH v19 05/30] LoongArch: KVM: Add vcpu related header files
From: WANG Xuerui
Date: Sat Aug 26 2023 - 07:30:23 EST
Hi,
On 8/17/23 20:59, Tianrui Zhao wrote:
Add LoongArch vcpu related header files, including vcpu csr
information, irq number defines, and some vcpu interfaces.
Reviewed-by: Bibo Mao <maobibo@xxxxxxxxxxx>
Signed-off-by: Tianrui Zhao <zhaotianrui@xxxxxxxxxxx>
---
arch/loongarch/include/asm/insn-def.h | 55 ++++++
arch/loongarch/include/asm/kvm_csr.h | 252 +++++++++++++++++++++++++
arch/loongarch/include/asm/kvm_vcpu.h | 95 ++++++++++
arch/loongarch/include/asm/loongarch.h | 20 +-
arch/loongarch/kvm/trace.h | 168 +++++++++++++++++
5 files changed, 585 insertions(+), 5 deletions(-)
create mode 100644 arch/loongarch/include/asm/insn-def.h
create mode 100644 arch/loongarch/include/asm/kvm_csr.h
create mode 100644 arch/loongarch/include/asm/kvm_vcpu.h
create mode 100644 arch/loongarch/kvm/trace.h
diff --git a/arch/loongarch/include/asm/insn-def.h b/arch/loongarch/include/asm/insn-def.h
new file mode 100644
index 000000000000..e285ee108fb0
--- /dev/null
+++ b/arch/loongarch/include/asm/insn-def.h
@@ -0,0 +1,55 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+
+#ifndef __ASM_INSN_DEF_H
+#define __ASM_INSN_DEF_H
+
+#include <linux/stringify.h>
+#include <asm/gpr-num.h>
+#include <asm/asm.h>
+
+#define INSN_STR(x) __stringify(x)
+#define CSR_RD_SHIFT 0
+#define CSR_RJ_SHIFT 5
+#define CSR_SIMM14_SHIFT 10
+#define CSR_OPCODE_SHIFT 24
+
+#define DEFINE_INSN_CSR \
+ __DEFINE_ASM_GPR_NUMS \
+" .macro insn_csr, opcode, rj, rd, simm14\n" \
+" .4byte ((\\opcode << " INSN_STR(CSR_OPCODE_SHIFT) ") |" \
+" (.L__gpr_num_\\rj << " INSN_STR(CSR_RJ_SHIFT) ") |" \
+" (.L__gpr_num_\\rd << " INSN_STR(CSR_RD_SHIFT) ") |" \
+" (\\simm14 << " INSN_STR(CSR_SIMM14_SHIFT) "))\n" \
+" .endm\n"
Okay so the previous request (just removing this file and related
machinery, with detailed analysis [1] as to why this is doable across
the whole LoongArch ecosystem, and agreed by at least one downstream
commercial distro [2]) still isn't being honored...
Maybe give your arguments as to what edge case the previous analysis
wasn't covering, or *at the very least* re-use the existing "parse_r"
helper and don't re-invent it, should it happen that such usage is
actually deemed acceptable in the kvm subsystem as previously pointed
out by Ruoyao [3].
[1]:
https://lore.kernel.org/loongarch/81270b55-37c4-d566-8cd7-acc90b490c10@xxxxxxxxxx/
[2]:
https://lore.kernel.org/loongarch/367239C38ED9D19C+9ef16061-9057-482c-bd8c-0b9ede71cfa7@xxxxxxxxxxxxx/
[3]:
https://lore.kernel.org/loongarch/6a5ed2266138cc61cbe27577424bb53cda72378d.camel@xxxxxxxxxxx/
+
+#define UNDEFINE_INSN_CSR \
+" .purgem insn_csr\n"
+
+#define __INSN_CSR(opcode, rj, rd, simm14) \
+ DEFINE_INSN_CSR \
+ "insn_csr " opcode ", " rj ", " rd ", " simm14 "\n" \
+ UNDEFINE_INSN_CSR
+
+
+#define INSN_CSR(opcode, rj, rd, simm14) \
+ __INSN_CSR(LARCH_##opcode, LARCH_##rj, LARCH_##rd, \
+ LARCH_##simm14)
+
+#define __ASM_STR(x) #x
+#define LARCH_OPCODE(v) __ASM_STR(v)
+#define LARCH_SIMM14(v) __ASM_STR(v)
+#define __LARCH_REG(v) __ASM_STR(v)
+#define LARCH___RD(v) __LARCH_REG(v)
+#define LARCH___RJ(v) __LARCH_REG(v)
+#define LARCH_OPCODE_GCSR LARCH_OPCODE(5)
+
+#define GCSR_read(csr, rd) \
+ INSN_CSR(OPCODE_GCSR, __RJ(zero), __RD(rd), SIMM14(csr))
+
+#define GCSR_write(csr, rd) \
+ INSN_CSR(OPCODE_GCSR, __RJ($r1), __RD(rd), SIMM14(csr))
+
+#define GCSR_xchg(csr, rj, rd) \
+ INSN_CSR(OPCODE_GCSR, __RJ(rj), __RD(rd), SIMM14(csr))
+
+#endif /* __ASM_INSN_DEF_H */
diff --git a/arch/loongarch/include/asm/kvm_csr.h b/arch/loongarch/include/asm/kvm_csr.h
new file mode 100644
index 000000000000..34483bbaec15
--- /dev/null
+++ b/arch/loongarch/include/asm/kvm_csr.h
@@ -0,0 +1,252 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Copyright (C) 2020-2023 Loongson Technology Corporation Limited
+ */
+
+#ifndef __ASM_LOONGARCH_KVM_CSR_H__
+#define __ASM_LOONGARCH_KVM_CSR_H__
+#include <asm/loongarch.h>
+#include <asm/kvm_vcpu.h>
+#include <linux/uaccess.h>
+#include <linux/kvm_host.h>
+
+#ifdef CONFIG_AS_HAS_LVZ_EXTENSION
+/* binutils support virtualization instructions */
It may also be LLVM IAS, so I'd suggest removing this comment as the
config symbol name is pretty self-documenting.
But this Kconfig flag isn't being defined until Patch 28, which feels a
bit reversed. I'd suggest splitting the definition for this symbol out
for a small patch before this one, or merging that definition into this
patch.
+#define gcsr_read(csr) \
+({ \
+ register unsigned long __v; \
+ __asm__ __volatile__( \
+ " gcsrrd %[val], %[reg]\n\t" \
+ : [val] "=r" (__v) \
+ : [reg] "i" (csr) \
+ : "memory"); \
+ __v; \
+})
+
+#define gcsr_write(v, csr) \
+({ \
+ register unsigned long __v = v; \
+ __asm__ __volatile__ ( \
+ " gcsrwr %[val], %[reg]\n\t" \
+ : [val] "+r" (__v) \
+ : [reg] "i" (csr) \
+ : "memory"); \
+})
+
+#define gcsr_xchg(v, m, csr) \
+({ \
+ register unsigned long __v = v; \
+ __asm__ __volatile__( \
+ " gcsrxchg %[val], %[mask], %[reg]\n\t" \
+ : [val] "+r" (__v) \
+ : [mask] "r" (m), [reg] "i" (csr) \
+ : "memory"); \
+ __v; \
+})
+#else
+/* binutils do not support virtualization instructions */
Similarly this comment could be removed. Or better, instead hint at the
symbol being checked so readers don't lose context:
#else /* CONFIG_AS_HAS_LVZ_EXTENSION */
+#define gcsr_read(csr) \
+({ \
+ register unsigned long __v; \
+ __asm__ __volatile__ (GCSR_read(csr, %0) \
+ : "=r" (__v) : \
+ : "memory"); \
+ __v; \
+})
+
+#define gcsr_write(val, csr) \
+({ \
+ register unsigned long __v = val; \
+ __asm__ __volatile__ (GCSR_write(csr, %0) \
+ : "+r" (__v) : \
+ : "memory"); \
+})
+
+#define gcsr_xchg(val, mask, csr) \
+({ \
+ register unsigned long __v = val; \
+ __asm__ __volatile__ (GCSR_xchg(csr, %1, %0) \
+ : "+r" (__v) \
+ : "r" (mask) \
+ : "memory"); \
+ __v; \
+})
+#endif
+
[snip]
--
WANG "xen0n" Xuerui
Linux/LoongArch mailing list: https://lore.kernel.org/loongarch/