Re: [PATCH v19 05/30] LoongArch: KVM: Add vcpu related header files

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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/




[Index of Archives]     [KVM ARM]     [KVM ia64]     [KVM ppc]     [Virtualization Tools]     [Spice Development]     [Libvirt]     [Libvirt Users]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite Questions]     [Linux Kernel]     [Linux SCSI]     [XFree86]

  Powered by Linux