Re: [PATCH 02/11] LoongArch: KVM: Add IPI device support

From: maobibo
Date: Fri Jul 12 2024 - 03:22:13 EST


Hi xianglai,

On 2024/7/5 上午10:38, Xianglai Li wrote:
Added device model for IPI interrupt controller,
implemented basic create destroy interface,
and registered device model to kvm device table.

Signed-off-by: Tianrui Zhao <zhaotianrui@xxxxxxxxxxx>
Signed-off-by: Xianglai Li <lixianglai@xxxxxxxxxxx>
---
Cc: Bibo Mao <maobibo@xxxxxxxxxxx>
Cc: Huacai Chen <chenhuacai@xxxxxxxxxx>
Cc: kvm@xxxxxxxxxxxxxxx
Cc: loongarch@xxxxxxxxxxxxxxx
Cc: Min Zhou <zhoumin@xxxxxxxxxxx>
Cc: Paolo Bonzini <pbonzini@xxxxxxxxxx>
Cc: Tianrui Zhao <zhaotianrui@xxxxxxxxxxx>
Cc: WANG Xuerui <kernel@xxxxxxxxxx>
Cc: Xianglai li <lixianglai@xxxxxxxxxxx>

arch/loongarch/include/asm/kvm_host.h | 4 +
arch/loongarch/include/asm/kvm_ipi.h | 36 ++++++
arch/loongarch/kvm/Makefile | 1 +
arch/loongarch/kvm/intc/ipi.c | 155 ++++++++++++++++++++++++++
arch/loongarch/kvm/main.c | 7 +-
arch/loongarch/kvm/vcpu.c | 3 +
include/uapi/linux/kvm.h | 4 +
7 files changed, 209 insertions(+), 1 deletion(-)
create mode 100644 arch/loongarch/include/asm/kvm_ipi.h
create mode 100644 arch/loongarch/kvm/intc/ipi.c

diff --git a/arch/loongarch/include/asm/kvm_host.h b/arch/loongarch/include/asm/kvm_host.h
index c87b6ea0ec47..4f6ccc688c1b 100644
--- a/arch/loongarch/include/asm/kvm_host.h
+++ b/arch/loongarch/include/asm/kvm_host.h
@@ -19,6 +19,7 @@
#include <asm/inst.h>
#include <asm/kvm_mmu.h>
#include <asm/loongarch.h>
+#include <asm/kvm_ipi.h>
/* Loongarch KVM register ids */
#define KVM_GET_IOC_CSR_IDX(id) ((id & KVM_CSR_IDX_MASK) >> LOONGARCH_REG_SHIFT)
@@ -107,6 +108,7 @@ struct kvm_arch {
s64 time_offset;
struct kvm_context __percpu *vmcs;
+ struct loongarch_ipi *ipi;
};
#define CSR_MAX_NUMS 0x800
@@ -199,6 +201,8 @@ struct kvm_vcpu_arch {
int last_sched_cpu;
/* mp state */
struct kvm_mp_state mp_state;
+ /* ipi state */
+ struct ipi_state ipi_state;
/* cpucfg */
u32 cpucfg[KVM_MAX_CPUCFG_REGS];
};
diff --git a/arch/loongarch/include/asm/kvm_ipi.h b/arch/loongarch/include/asm/kvm_ipi.h
new file mode 100644
index 000000000000..875a93008802
--- /dev/null
+++ b/arch/loongarch/include/asm/kvm_ipi.h
@@ -0,0 +1,36 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Copyright (C) 2024 Loongson Technology Corporation Limited
+ */
+
+#ifndef __LS3A_KVM_IPI_H
+#define __LS3A_KVM_IPI_H
__ASM_KVM_IPI_H rather than __LS3A_KVM_IPI_H, it should be capital letter from string "asm/kvm_ipi.h"

+
+#include <kvm/iodev.h>
+
+#define LARCH_INT_IPI 12
+
+struct loongarch_ipi {
+ spinlock_t lock;
+ struct kvm *kvm;
+ struct kvm_io_device device;
+ struct kvm_io_device mail_dev;
+};
+
+struct ipi_state {
+ spinlock_t lock;
+ uint32_t status;
+ uint32_t en;
+ uint32_t set;
+ uint32_t clear;
+ uint64_t buf[4];
+};
+
+#define SMP_MAILBOX 0x1000
+#define KVM_IOCSR_IPI_ADDR_SIZE 0x48
+
+#define MAIL_SEND_ADDR (SMP_MAILBOX + IOCSR_MAIL_SEND)
+#define KVM_IOCSR_MAIL_ADDR_SIZE 0x118
+
+int kvm_loongarch_register_ipi_device(void);
+#endif
diff --git a/arch/loongarch/kvm/Makefile b/arch/loongarch/kvm/Makefile
index b2f4cbe01ae8..36c3009fe89c 100644
--- a/arch/loongarch/kvm/Makefile
+++ b/arch/loongarch/kvm/Makefile
@@ -18,5 +18,6 @@ kvm-y += timer.o
kvm-y += tlb.o
kvm-y += vcpu.o
kvm-y += vm.o
+kvm-y += intc/ipi.o
CFLAGS_exit.o += $(call cc-option,-Wno-override-init,)
diff --git a/arch/loongarch/kvm/intc/ipi.c b/arch/loongarch/kvm/intc/ipi.c
new file mode 100644
index 000000000000..a9dc0aaec502
--- /dev/null
+++ b/arch/loongarch/kvm/intc/ipi.c
@@ -0,0 +1,155 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (C) 2024 Loongson Technology Corporation Limited
+ */
+
+#include <linux/kvm_host.h>
+#include <asm/kvm_ipi.h>
+#include <asm/kvm_vcpu.h>
+
+static int kvm_loongarch_ipi_write(struct kvm_vcpu *vcpu,
+ struct kvm_io_device *dev,
+ gpa_t addr, int len, const void *val)
+{
+ return 0;
+}
+
+static int kvm_loongarch_ipi_read(struct kvm_vcpu *vcpu,
+ struct kvm_io_device *dev,
+ gpa_t addr, int len, void *val)
+{
+ return 0;
+}
+
+static int kvm_loongarch_mail_write(struct kvm_vcpu *vcpu,
+ struct kvm_io_device *dev,
+ gpa_t addr, int len, const void *val)
+{
+ return 0;
+}
+
+static const struct kvm_io_device_ops kvm_loongarch_ipi_ops = {
+ .read = kvm_loongarch_ipi_read,
+ .write = kvm_loongarch_ipi_write,
+};
+
+static const struct kvm_io_device_ops kvm_loongarch_mail_ops = {
+ .write = kvm_loongarch_mail_write,
+};
+
+static int kvm_loongarch_ipi_get_attr(struct kvm_device *dev,
+ struct kvm_device_attr *attr)
+{
+ return 0;
+}
+
+static int kvm_loongarch_ipi_set_attr(struct kvm_device *dev,
+ struct kvm_device_attr *attr)
+{
+ return 0;
+}
+
+static void kvm_loongarch_ipi_destroy(struct kvm_device *dev)
+{
+ struct kvm *kvm;
+ struct loongarch_ipi *ipi;
+ struct kvm_io_device *device;
+
+ if (!dev)
+ return;
+
+ kvm = dev->kvm;
+ if (!kvm)
+ return;
+
+ ipi = kvm->arch.ipi;
+ if (!ipi)
+ return;
+
+ device = &ipi->device;
+ kvm_io_bus_unregister_dev(kvm, KVM_IOCSR_BUS, device);
+
+ device = &ipi->mail_dev;
+ kvm_io_bus_unregister_dev(kvm, KVM_IOCSR_BUS, device);
Variable device seems be not much useful, how about something like this?
kvm_io_bus_unregister_dev(kvm, KVM_IOCSR_BUS, &ipi->device);
kvm_io_bus_unregister_dev(kvm, KVM_IOCSR_BUS, &ipi->mail_dev);
+
Empty line can be removed.
+ kfree(ipi);
+}
+
+static int kvm_loongarch_ipi_create(struct kvm_device *dev, u32 type)
+{
+ struct kvm *kvm;
+ struct loongarch_ipi *s;
+ unsigned long addr;
+ struct kvm_io_device *device;
+ int ret;
+
+ kvm_info("begin create loongarch ipi in kvm ...\n");
It is a noise for kvm_info(), every time VM is created there will too much printing log.
+ if (!dev) {
+ kvm_err("%s: kvm_device ptr is invalid!\n", __func__);
+ return -EINVAL;
+ }
+
+ kvm = dev->kvm;
+ if (kvm->arch.ipi) {
+ kvm_err("%s: loongarch ipi has been created!\n", __func__);
+ return -EINVAL;
+ }
+
+ s = kzalloc(sizeof(struct loongarch_ipi), GFP_KERNEL);
+ if (!s)
+ return -ENOMEM;
+ spin_lock_init(&s->lock);
+ s->kvm = kvm;
+
+ /*
+ * Initialize IOCSR device
+ */
+ device = &s->device;
+ kvm_iodevice_init(device, &kvm_loongarch_ipi_ops);
+ addr = SMP_MAILBOX;
+ mutex_lock(&kvm->slots_lock);
+ ret = kvm_io_bus_register_dev(kvm, KVM_IOCSR_BUS, addr,
+ KVM_IOCSR_IPI_ADDR_SIZE, device);
+ mutex_unlock(&kvm->slots_lock);
+ if (ret < 0) {
+ kvm_err("%s: initialize IOCSR dev failed, ret = %d\n", __func__, ret);
+ goto err;
+ }
+
+ device = &s->mail_dev;
+ kvm_iodevice_init(device, &kvm_loongarch_mail_ops);
+ addr = MAIL_SEND_ADDR;
+ mutex_lock(&kvm->slots_lock);
+ ret = kvm_io_bus_register_dev(kvm, KVM_IOCSR_BUS, addr,
+ KVM_IOCSR_MAIL_ADDR_SIZE, device);
+ mutex_unlock(&kvm->slots_lock);
+ if (ret < 0) {
+ device = &s->device;
+ kvm_io_bus_unregister_dev(kvm, KVM_IOCSR_BUS, device);
Should it be kvm_io_bus_unregister_dev(kvm, KVM_IOCSR_BUS, &s->device)?
+ kvm_err("%s: initialize mail box dev failed, ret = %d\n", __func__, ret);
+ goto err;
+ }
+
+ kvm->arch.ipi = s;
+ kvm_info("create loongarch ipi in kvm done!\n");
kvm_info should be removed.
+
Empty line can be removed.
+ return 0;
+
+err:
+ kfree(s);
+ return -EFAULT;
+}
+
+static struct kvm_device_ops kvm_loongarch_ipi_dev_ops = {
+ .name = "kvm-loongarch-ipi",
+ .create = kvm_loongarch_ipi_create,
+ .destroy = kvm_loongarch_ipi_destroy,
+ .set_attr = kvm_loongarch_ipi_set_attr,
+ .get_attr = kvm_loongarch_ipi_get_attr,
+};
+
+int kvm_loongarch_register_ipi_device(void)
+{
+ return kvm_register_device_ops(&kvm_loongarch_ipi_dev_ops,
+ KVM_DEV_TYPE_LA_IPI);
+}
diff --git a/arch/loongarch/kvm/main.c b/arch/loongarch/kvm/main.c
index 86a2f2d0cb27..36efc7b38f83 100644
--- a/arch/loongarch/kvm/main.c
+++ b/arch/loongarch/kvm/main.c
@@ -312,7 +312,7 @@ void kvm_arch_hardware_disable(void)
static int kvm_loongarch_env_init(void)
{
- int cpu, order;
+ int cpu, order, ret;
void *addr;
struct kvm_context *context;
@@ -367,6 +367,11 @@ static int kvm_loongarch_env_init(void)
kvm_init_gcsr_flag();
+ /* Register loongarch ipi interrupt controller interface. */
+ ret = kvm_loongarch_register_ipi_device();
+ if (ret)
+ return ret;
+
how about "return ret;" directly without the if judgement sentence?

Regards
Bibo Mao
return 0;
}
diff --git a/arch/loongarch/kvm/vcpu.c b/arch/loongarch/kvm/vcpu.c
index 9e8030d45129..1d369b92a84d 100644
--- a/arch/loongarch/kvm/vcpu.c
+++ b/arch/loongarch/kvm/vcpu.c
@@ -1013,6 +1013,9 @@ int kvm_arch_vcpu_create(struct kvm_vcpu *vcpu)
/* Init */
vcpu->arch.last_sched_cpu = -1;
+ /* Init ipi_state lock */
+ spin_lock_init(&vcpu->arch.ipi_state.lock);
+
/*
* Initialize guest register state to valid architectural reset state.
*/
diff --git a/include/uapi/linux/kvm.h b/include/uapi/linux/kvm.h
index d03842abae57..341fc9d5f3ec 100644
--- a/include/uapi/linux/kvm.h
+++ b/include/uapi/linux/kvm.h
@@ -1142,7 +1142,11 @@ enum kvm_device_type {
#define KVM_DEV_TYPE_ARM_PV_TIME KVM_DEV_TYPE_ARM_PV_TIME
KVM_DEV_TYPE_RISCV_AIA,
#define KVM_DEV_TYPE_RISCV_AIA KVM_DEV_TYPE_RISCV_AIA
+ KVM_DEV_TYPE_LA_IPI,
+#define KVM_DEV_TYPE_LA_IPI KVM_DEV_TYPE_LA_IPI
+
KVM_DEV_TYPE_MAX,
+
};
struct kvm_vfio_spapr_tce {