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/