[kvmtool PATCH v2 0/6] RISC-V AIA irqchip and Svnapot support

[kvmtool PATCH v2 0/6] RISC-V AIA irqchip and Svnapot support

[Anup Patel]

The latest KVM in Linux-6.5 has support for:
1) Svnapot ISA extension support
2) AIA in-kernel irqchip support

This series adds corresponding changes in KVMTOOL to use the above
mentioned features for Guest/VM.

These patches can also be found in the riscv_aia_v2 branch at:
https://github.com/avpatel/kvmtool.git

Changes since v1:
 - Rebased on commit 9cb1b46cb765972326a46bdba867d441a842af56
 - Updated PATCH1 to sync header with released Linux-6.5

Anup Patel (6):
  Sync-up header with Linux-6.5 for KVM RISC-V
  riscv: Add Svnapot extension support
  riscv: Make irqchip support pluggable
  riscv: Add IRQFD support for in-kernel AIA irqchip
  riscv: Use AIA in-kernel irqchip whenever KVM RISC-V supports
  riscv: Fix guest/init linkage for multilib toolchain

 Makefile                            |   3 +
 include/linux/kvm.h                 |   6 +-
 riscv/aia.c                         | 227 ++++++++++++++++++++++++++++
 riscv/fdt.c                         |  15 +-
 riscv/include/asm/kvm.h             |  81 ++++++++++
 riscv/include/kvm/fdt-arch.h        |   8 +-
 riscv/include/kvm/kvm-arch.h        |  38 ++++-
 riscv/include/kvm/kvm-config-arch.h |   3 +
 riscv/irq.c                         | 138 ++++++++++++++++-
 riscv/kvm.c                         |   2 +
 riscv/pci.c                         |  32 ++--
 riscv/plic.c                        |  61 ++++----
 12 files changed, 563 insertions(+), 51 deletions(-)
 create mode 100644 riscv/aia.c

-- 
2.34.1

[kvmtool PATCH v2 1/6] Sync-up header with Linux-6.5 for KVM RISC-V

[Anup Patel]

We sync-up Linux headers to get latest KVM RISC-V headers having
V, Svnapot, and AIA support.

Signed-off-by: Anup Patel <apatel@ventanamicro.com>
---
 include/linux/kvm.h     |  6 ++-
 riscv/include/asm/kvm.h | 81 +++++++++++++++++++++++++++++++++++++++++
 2 files changed, 86 insertions(+), 1 deletion(-)

diff --git a/include/linux/kvm.h b/include/linux/kvm.h
index 737318b..f089ab2 100644
--- a/include/linux/kvm.h
+++ b/include/linux/kvm.h
@@ -1190,6 +1190,8 @@ struct kvm_ppc_resize_hpt {
 #define KVM_CAP_DIRTY_LOG_RING_WITH_BITMAP 225
 #define KVM_CAP_PMU_EVENT_MASKED_EVENTS 226
 #define KVM_CAP_COUNTER_OFFSET 227
+#define KVM_CAP_ARM_EAGER_SPLIT_CHUNK_SIZE 228
+#define KVM_CAP_ARM_SUPPORTED_BLOCK_SIZES 229
 
 #ifdef KVM_CAP_IRQ_ROUTING
 
@@ -1442,6 +1444,8 @@ enum kvm_device_type {
 #define KVM_DEV_TYPE_XIVE		KVM_DEV_TYPE_XIVE
 	KVM_DEV_TYPE_ARM_PV_TIME,
 #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_MAX,
 };
 
@@ -1613,7 +1617,7 @@ struct kvm_s390_ucas_mapping {
 #define KVM_GET_DEBUGREGS         _IOR(KVMIO,  0xa1, struct kvm_debugregs)
 #define KVM_SET_DEBUGREGS         _IOW(KVMIO,  0xa2, struct kvm_debugregs)
 /*
- * vcpu version available with KVM_ENABLE_CAP
+ * vcpu version available with KVM_CAP_ENABLE_CAP
  * vm version available with KVM_CAP_ENABLE_CAP_VM
  */
 #define KVM_ENABLE_CAP            _IOW(KVMIO,  0xa3, struct kvm_enable_cap)
diff --git a/riscv/include/asm/kvm.h b/riscv/include/asm/kvm.h
index f92790c..930fdc4 100644
--- a/riscv/include/asm/kvm.h
+++ b/riscv/include/asm/kvm.h
@@ -15,6 +15,7 @@
 #include <asm/bitsperlong.h>
 #include <asm/ptrace.h>
 
+#define __KVM_HAVE_IRQ_LINE
 #define __KVM_HAVE_READONLY_MEM
 
 #define KVM_COALESCED_MMIO_PAGE_OFFSET 1
@@ -121,6 +122,8 @@ enum KVM_RISCV_ISA_EXT_ID {
 	KVM_RISCV_ISA_EXT_ZICBOZ,
 	KVM_RISCV_ISA_EXT_ZBB,
 	KVM_RISCV_ISA_EXT_SSAIA,
+	KVM_RISCV_ISA_EXT_V,
+	KVM_RISCV_ISA_EXT_SVNAPOT,
 	KVM_RISCV_ISA_EXT_MAX,
 };
 
@@ -203,6 +206,84 @@ enum KVM_RISCV_SBI_EXT_ID {
 #define KVM_REG_RISCV_SBI_MULTI_REG_LAST	\
 		KVM_REG_RISCV_SBI_MULTI_REG(KVM_RISCV_SBI_EXT_MAX - 1)
 
+/* V extension registers are mapped as type 9 */
+#define KVM_REG_RISCV_VECTOR		(0x09 << KVM_REG_RISCV_TYPE_SHIFT)
+#define KVM_REG_RISCV_VECTOR_CSR_REG(name)	\
+		(offsetof(struct __riscv_v_ext_state, name) / sizeof(unsigned long))
+#define KVM_REG_RISCV_VECTOR_REG(n)	\
+		((n) + sizeof(struct __riscv_v_ext_state) / sizeof(unsigned long))
+
+/* Device Control API: RISC-V AIA */
+#define KVM_DEV_RISCV_APLIC_ALIGN		0x1000
+#define KVM_DEV_RISCV_APLIC_SIZE		0x4000
+#define KVM_DEV_RISCV_APLIC_MAX_HARTS		0x4000
+#define KVM_DEV_RISCV_IMSIC_ALIGN		0x1000
+#define KVM_DEV_RISCV_IMSIC_SIZE		0x1000
+
+#define KVM_DEV_RISCV_AIA_GRP_CONFIG		0
+#define KVM_DEV_RISCV_AIA_CONFIG_MODE		0
+#define KVM_DEV_RISCV_AIA_CONFIG_IDS		1
+#define KVM_DEV_RISCV_AIA_CONFIG_SRCS		2
+#define KVM_DEV_RISCV_AIA_CONFIG_GROUP_BITS	3
+#define KVM_DEV_RISCV_AIA_CONFIG_GROUP_SHIFT	4
+#define KVM_DEV_RISCV_AIA_CONFIG_HART_BITS	5
+#define KVM_DEV_RISCV_AIA_CONFIG_GUEST_BITS	6
+
+/*
+ * Modes of RISC-V AIA device:
+ * 1) EMUL (aka Emulation): Trap-n-emulate IMSIC
+ * 2) HWACCEL (aka HW Acceleration): Virtualize IMSIC using IMSIC guest files
+ * 3) AUTO (aka Automatic): Virtualize IMSIC using IMSIC guest files whenever
+ *    available otherwise fallback to trap-n-emulation
+ */
+#define KVM_DEV_RISCV_AIA_MODE_EMUL		0
+#define KVM_DEV_RISCV_AIA_MODE_HWACCEL		1
+#define KVM_DEV_RISCV_AIA_MODE_AUTO		2
+
+#define KVM_DEV_RISCV_AIA_IDS_MIN		63
+#define KVM_DEV_RISCV_AIA_IDS_MAX		2048
+#define KVM_DEV_RISCV_AIA_SRCS_MAX		1024
+#define KVM_DEV_RISCV_AIA_GROUP_BITS_MAX	8
+#define KVM_DEV_RISCV_AIA_GROUP_SHIFT_MIN	24
+#define KVM_DEV_RISCV_AIA_GROUP_SHIFT_MAX	56
+#define KVM_DEV_RISCV_AIA_HART_BITS_MAX		16
+#define KVM_DEV_RISCV_AIA_GUEST_BITS_MAX	8
+
+#define KVM_DEV_RISCV_AIA_GRP_ADDR		1
+#define KVM_DEV_RISCV_AIA_ADDR_APLIC		0
+#define KVM_DEV_RISCV_AIA_ADDR_IMSIC(__vcpu)	(1 + (__vcpu))
+#define KVM_DEV_RISCV_AIA_ADDR_MAX		\
+		(1 + KVM_DEV_RISCV_APLIC_MAX_HARTS)
+
+#define KVM_DEV_RISCV_AIA_GRP_CTRL		2
+#define KVM_DEV_RISCV_AIA_CTRL_INIT		0
+
+/*
+ * The device attribute type contains the memory mapped offset of the
+ * APLIC register (range 0x0000-0x3FFF) and it must be 4-byte aligned.
+ */
+#define KVM_DEV_RISCV_AIA_GRP_APLIC		3
+
+/*
+ * The lower 12-bits of the device attribute type contains the iselect
+ * value of the IMSIC register (range 0x70-0xFF) whereas the higher order
+ * bits contains the VCPU id.
+ */
+#define KVM_DEV_RISCV_AIA_GRP_IMSIC		4
+#define KVM_DEV_RISCV_AIA_IMSIC_ISEL_BITS	12
+#define KVM_DEV_RISCV_AIA_IMSIC_ISEL_MASK	\
+		((1U << KVM_DEV_RISCV_AIA_IMSIC_ISEL_BITS) - 1)
+#define KVM_DEV_RISCV_AIA_IMSIC_MKATTR(__vcpu, __isel)	\
+		(((__vcpu) << KVM_DEV_RISCV_AIA_IMSIC_ISEL_BITS) | \
+		 ((__isel) & KVM_DEV_RISCV_AIA_IMSIC_ISEL_MASK))
+#define KVM_DEV_RISCV_AIA_IMSIC_GET_ISEL(__attr)	\
+		((__attr) & KVM_DEV_RISCV_AIA_IMSIC_ISEL_MASK)
+#define KVM_DEV_RISCV_AIA_IMSIC_GET_VCPU(__attr)	\
+		((__attr) >> KVM_DEV_RISCV_AIA_IMSIC_ISEL_BITS)
+
+/* One single KVM irqchip, ie. the AIA */
+#define KVM_NR_IRQCHIPS			1
+
 #endif
 
 #endif /* __LINUX_KVM_RISCV_H */
-- 
2.34.1

[kvmtool PATCH v2 2/6] riscv: Add Svnapot extension support

[Anup Patel]

When the Svnapot extension is available expose it to the guest via
device tree so that guest can use it.

Signed-off-by: Anup Patel <apatel@ventanamicro.com>
---
 riscv/fdt.c                         | 1 +
 riscv/include/kvm/kvm-config-arch.h | 3 +++
 2 files changed, 4 insertions(+)

diff --git a/riscv/fdt.c b/riscv/fdt.c
index df71ed4..2724c6e 100644
--- a/riscv/fdt.c
+++ b/riscv/fdt.c
@@ -19,6 +19,7 @@ struct isa_ext_info isa_info_arr[] = {
 	{"ssaia", KVM_RISCV_ISA_EXT_SSAIA},
 	{"sstc", KVM_RISCV_ISA_EXT_SSTC},
 	{"svinval", KVM_RISCV_ISA_EXT_SVINVAL},
+	{"svnapot", KVM_RISCV_ISA_EXT_SVNAPOT},
 	{"svpbmt", KVM_RISCV_ISA_EXT_SVPBMT},
 	{"zbb", KVM_RISCV_ISA_EXT_ZBB},
 	{"zicbom", KVM_RISCV_ISA_EXT_ZICBOM},
diff --git a/riscv/include/kvm/kvm-config-arch.h b/riscv/include/kvm/kvm-config-arch.h
index b0a7e25..863baea 100644
--- a/riscv/include/kvm/kvm-config-arch.h
+++ b/riscv/include/kvm/kvm-config-arch.h
@@ -34,6 +34,9 @@ struct kvm_config_arch {
 	OPT_BOOLEAN('\0', "disable-svinval",				\
 		    &(cfg)->ext_disabled[KVM_RISCV_ISA_EXT_SVINVAL],	\
 		    "Disable Svinval Extension"),			\
+	OPT_BOOLEAN('\0', "disable-svnapot",				\
+		    &(cfg)->ext_disabled[KVM_RISCV_ISA_EXT_SVNAPOT],	\
+		    "Disable Svnapot Extension"),			\
 	OPT_BOOLEAN('\0', "disable-svpbmt",				\
 		    &(cfg)->ext_disabled[KVM_RISCV_ISA_EXT_SVPBMT],	\
 		    "Disable Svpbmt Extension"),			\
-- 
2.34.1

Re: [kvmtool PATCH v2 2/6] riscv: Add Svnapot extension support

[Andrew Jones]

On Mon, Sep 18, 2023 at 06:27:26PM +0530, Anup Patel wrote:
> When the Svnapot extension is available expose it to the guest via
> device tree so that guest can use it.
> 
> Signed-off-by: Anup Patel <apatel@ventanamicro.com>
> ---
>  riscv/fdt.c                         | 1 +
>  riscv/include/kvm/kvm-config-arch.h | 3 +++
>  2 files changed, 4 insertions(+)
> 
> diff --git a/riscv/fdt.c b/riscv/fdt.c
> index df71ed4..2724c6e 100644
> --- a/riscv/fdt.c
> +++ b/riscv/fdt.c
> @@ -19,6 +19,7 @@ struct isa_ext_info isa_info_arr[] = {
>  	{"ssaia", KVM_RISCV_ISA_EXT_SSAIA},
>  	{"sstc", KVM_RISCV_ISA_EXT_SSTC},
>  	{"svinval", KVM_RISCV_ISA_EXT_SVINVAL},
> +	{"svnapot", KVM_RISCV_ISA_EXT_SVNAPOT},
>  	{"svpbmt", KVM_RISCV_ISA_EXT_SVPBMT},
>  	{"zbb", KVM_RISCV_ISA_EXT_ZBB},
>  	{"zicbom", KVM_RISCV_ISA_EXT_ZICBOM},
> diff --git a/riscv/include/kvm/kvm-config-arch.h b/riscv/include/kvm/kvm-config-arch.h
> index b0a7e25..863baea 100644
> --- a/riscv/include/kvm/kvm-config-arch.h
> +++ b/riscv/include/kvm/kvm-config-arch.h
> @@ -34,6 +34,9 @@ struct kvm_config_arch {
>  	OPT_BOOLEAN('\0', "disable-svinval",				\
>  		    &(cfg)->ext_disabled[KVM_RISCV_ISA_EXT_SVINVAL],	\
>  		    "Disable Svinval Extension"),			\
> +	OPT_BOOLEAN('\0', "disable-svnapot",				\
> +		    &(cfg)->ext_disabled[KVM_RISCV_ISA_EXT_SVNAPOT],	\
> +		    "Disable Svnapot Extension"),			\
>  	OPT_BOOLEAN('\0', "disable-svpbmt",				\
>  		    &(cfg)->ext_disabled[KVM_RISCV_ISA_EXT_SVPBMT],	\
>  		    "Disable Svpbmt Extension"),			\
> -- 
> 2.34.1
>


Reviewed-by: Andrew Jones <ajones@ventanamicro.com>

[kvmtool PATCH v2 3/6] riscv: Make irqchip support pluggable

[Anup Patel]

We will be having different types of irqchip:
1) PLIC emulated by user-space
2) AIA APLIC and IMSIC provided by in-kernel KVM module

To support above, we de-couple PLIC specific code from generic
RISC-V code (such as FDT generation) so that we can easily add
other types of irqchip.

Signed-off-by: Anup Patel <apatel@ventanamicro.com>
---
 riscv/fdt.c                  | 14 ++++++--
 riscv/include/kvm/kvm-arch.h | 25 ++++++++++++---
 riscv/irq.c                  | 62 ++++++++++++++++++++++++++++++++++--
 riscv/kvm.c                  |  2 ++
 riscv/pci.c                  | 32 +++++++++++++------
 riscv/plic.c                 | 61 +++++++++++++++++------------------
 6 files changed, 147 insertions(+), 49 deletions(-)

diff --git a/riscv/fdt.c b/riscv/fdt.c
index 2724c6e..9af71b5 100644
--- a/riscv/fdt.c
+++ b/riscv/fdt.c
@@ -209,19 +209,26 @@ static int setup_fdt(struct kvm *kvm)
 	/* CPUs */
 	generate_cpu_nodes(fdt, kvm);
 
+	/* IRQCHIP */
+	if (!riscv_irqchip_generate_fdt_node)
+		die("No way to generate IRQCHIP FDT node\n");
+	riscv_irqchip_generate_fdt_node(fdt, kvm);
+
 	/* Simple Bus */
 	_FDT(fdt_begin_node(fdt, "smb"));
 	_FDT(fdt_property_string(fdt, "compatible", "simple-bus"));
 	_FDT(fdt_property_cell(fdt, "#address-cells", 0x2));
 	_FDT(fdt_property_cell(fdt, "#size-cells", 0x2));
-	_FDT(fdt_property_cell(fdt, "interrupt-parent", PHANDLE_PLIC));
+	_FDT(fdt_property_cell(fdt, "interrupt-parent",
+			       riscv_irqchip_phandle));
 	_FDT(fdt_property(fdt, "ranges", NULL, 0));
 
 	/* Virtio MMIO devices */
 	dev_hdr = device__first_dev(DEVICE_BUS_MMIO);
 	while (dev_hdr) {
 		generate_mmio_fdt_nodes = dev_hdr->data;
-		generate_mmio_fdt_nodes(fdt, dev_hdr, plic__generate_irq_prop);
+		generate_mmio_fdt_nodes(fdt, dev_hdr,
+					riscv__generate_irq_prop);
 		dev_hdr = device__next_dev(dev_hdr);
 	}
 
@@ -229,7 +236,8 @@ static int setup_fdt(struct kvm *kvm)
 	dev_hdr = device__first_dev(DEVICE_BUS_IOPORT);
 	while (dev_hdr) {
 		generate_mmio_fdt_nodes = dev_hdr->data;
-		generate_mmio_fdt_nodes(fdt, dev_hdr, plic__generate_irq_prop);
+		generate_mmio_fdt_nodes(fdt, dev_hdr,
+					riscv__generate_irq_prop);
 		dev_hdr = device__next_dev(dev_hdr);
 	}
 
diff --git a/riscv/include/kvm/kvm-arch.h b/riscv/include/kvm/kvm-arch.h
index 660355b..cd37fc6 100644
--- a/riscv/include/kvm/kvm-arch.h
+++ b/riscv/include/kvm/kvm-arch.h
@@ -10,8 +10,8 @@
 
 #define RISCV_IOPORT		0x00000000ULL
 #define RISCV_IOPORT_SIZE	SZ_64K
-#define RISCV_PLIC		0x0c000000ULL
-#define RISCV_PLIC_SIZE		SZ_64M
+#define RISCV_IRQCHIP		0x08000000ULL
+#define RISCV_IRQCHIP_SIZE		SZ_128M
 #define RISCV_MMIO		0x10000000ULL
 #define RISCV_MMIO_SIZE		SZ_512M
 #define RISCV_PCI		0x30000000ULL
@@ -84,10 +84,27 @@ static inline bool riscv_addr_in_ioport_region(u64 phys_addr)
 
 enum irq_type;
 
-void plic__generate_irq_prop(void *fdt, u8 irq, enum irq_type irq_type);
+enum irqchip_type {
+	IRQCHIP_UNKNOWN = 0,
+	IRQCHIP_PLIC,
+	IRQCHIP_AIA
+};
+
+extern enum irqchip_type riscv_irqchip;
+extern bool riscv_irqchip_inkernel;
+extern void (*riscv_irqchip_trigger)(struct kvm *kvm, int irq,
+				     int level, bool edge);
+extern void (*riscv_irqchip_generate_fdt_node)(void *fdt, struct kvm *kvm);
+extern u32 riscv_irqchip_phandle;
+extern u32 riscv_irqchip_msi_phandle;
+extern bool riscv_irqchip_line_sensing;
 
-void plic__irq_trig(struct kvm *kvm, int irq, int level, bool edge);
+void plic__create(struct kvm *kvm);
 
 void pci__generate_fdt_nodes(void *fdt);
 
+void riscv__generate_irq_prop(void *fdt, u8 irq, enum irq_type irq_type);
+
+void riscv__irqchip_create(struct kvm *kvm);
+
 #endif /* KVM__KVM_ARCH_H */
diff --git a/riscv/irq.c b/riscv/irq.c
index 78a582d..b608a2f 100644
--- a/riscv/irq.c
+++ b/riscv/irq.c
@@ -1,13 +1,71 @@
 #include "kvm/kvm.h"
 #include "kvm/kvm-cpu.h"
 #include "kvm/irq.h"
+#include "kvm/fdt.h"
+#include "kvm/virtio.h"
+
+enum irqchip_type riscv_irqchip = IRQCHIP_UNKNOWN;
+bool riscv_irqchip_inkernel = false;
+void (*riscv_irqchip_trigger)(struct kvm *kvm, int irq, int level, bool edge)
+				= NULL;
+void (*riscv_irqchip_generate_fdt_node)(void *fdt, struct kvm *kvm) = NULL;
+u32 riscv_irqchip_phandle = PHANDLE_RESERVED;
+u32 riscv_irqchip_msi_phandle = PHANDLE_RESERVED;
+bool riscv_irqchip_line_sensing = false;
 
 void kvm__irq_line(struct kvm *kvm, int irq, int level)
 {
-	plic__irq_trig(kvm, irq, level, false);
+	struct kvm_irq_level irq_level;
+
+	if (riscv_irqchip_inkernel) {
+		irq_level.irq = irq;
+		irq_level.level = !!level;
+		if (ioctl(kvm->vm_fd, KVM_IRQ_LINE, &irq_level) < 0)
+			pr_warning("%s: Could not KVM_IRQ_LINE for irq %d\n",
+				   __func__, irq);
+	} else {
+		if (riscv_irqchip_trigger)
+			riscv_irqchip_trigger(kvm, irq, level, false);
+		else
+			pr_warning("%s: Can't change level for irq %d\n",
+				   __func__, irq);
+	}
 }
 
 void kvm__irq_trigger(struct kvm *kvm, int irq)
 {
-	plic__irq_trig(kvm, irq, 1, true);
+	if (riscv_irqchip_inkernel) {
+		kvm__irq_line(kvm, irq, VIRTIO_IRQ_HIGH);
+		kvm__irq_line(kvm, irq, VIRTIO_IRQ_LOW);
+	} else {
+		if (riscv_irqchip_trigger)
+			riscv_irqchip_trigger(kvm, irq, 1, true);
+		else
+			pr_warning("%s: Can't trigger irq %d\n",
+				   __func__, irq);
+	}
+}
+
+void riscv__generate_irq_prop(void *fdt, u8 irq, enum irq_type irq_type)
+{
+	u32 prop[2], size;
+
+	prop[0] = cpu_to_fdt32(irq);
+	size = sizeof(u32);
+	if (riscv_irqchip_line_sensing) {
+		prop[1] = cpu_to_fdt32(irq_type);
+		size += sizeof(u32);
+	}
+
+	_FDT(fdt_property(fdt, "interrupts", prop, size));
+}
+
+void riscv__irqchip_create(struct kvm *kvm)
+{
+	/* Try PLIC irqchip */
+	plic__create(kvm);
+
+	/* Fail if irqchip unknown */
+	if (riscv_irqchip == IRQCHIP_UNKNOWN)
+		die("No IRQCHIP found\n");
 }
diff --git a/riscv/kvm.c b/riscv/kvm.c
index 8daad94..1d49479 100644
--- a/riscv/kvm.c
+++ b/riscv/kvm.c
@@ -96,6 +96,8 @@ void kvm__arch_init(struct kvm *kvm)
 
 	madvise(kvm->arch.ram_alloc_start, kvm->arch.ram_alloc_size,
 		MADV_HUGEPAGE);
+
+	riscv__irqchip_create(kvm);
 }
 
 #define FDT_ALIGN	SZ_4M
diff --git a/riscv/pci.c b/riscv/pci.c
index 604fd20..61dee06 100644
--- a/riscv/pci.c
+++ b/riscv/pci.c
@@ -7,20 +7,21 @@
 
 /*
  * An entry in the interrupt-map table looks like:
- * <pci unit address> <pci interrupt pin> <plic phandle> <plic interrupt>
+ * <pci unit address> <pci interrupt pin> <irqchip phandle> <irqchip line>
  */
 
 struct of_interrupt_map_entry {
 	struct of_pci_irq_mask		pci_irq_mask;
-	u32				plic_phandle;
-	u32				plic_irq;
+	u32				irqchip_phandle;
+	u32				irqchip_line;
+	u32				irqchip_sense;
 } __attribute__((packed));
 
 void pci__generate_fdt_nodes(void *fdt)
 {
 	struct device_header *dev_hdr;
 	struct of_interrupt_map_entry irq_map[OF_PCI_IRQ_MAP_MAX];
-	unsigned nentries = 0;
+	unsigned nentries = 0, nsize;
 	/* Bus range */
 	u32 bus_range[] = { cpu_to_fdt32(0), cpu_to_fdt32(1), };
 	/* Configuration Space */
@@ -48,6 +49,11 @@ void pci__generate_fdt_nodes(void *fdt)
 		},
 	};
 
+	/* Find size of each interrupt map entery */
+	nsize = sizeof(struct of_interrupt_map_entry);
+	if (!riscv_irqchip_line_sensing)
+		nsize -= sizeof(u32);
+
 	/* Boilerplate PCI properties */
 	_FDT(fdt_begin_node(fdt, "pci"));
 	_FDT(fdt_property_string(fdt, "device_type", "pci"));
@@ -64,12 +70,13 @@ void pci__generate_fdt_nodes(void *fdt)
 	/* Generate the interrupt map ... */
 	dev_hdr = device__first_dev(DEVICE_BUS_PCI);
 	while (dev_hdr && nentries < ARRAY_SIZE(irq_map)) {
-		struct of_interrupt_map_entry *entry = &irq_map[nentries];
+		struct of_interrupt_map_entry *entry;
 		struct pci_device_header *pci_hdr = dev_hdr->data;
 		u8 dev_num = dev_hdr->dev_num;
 		u8 pin = pci_hdr->irq_pin;
 		u8 irq = pci_hdr->irq_line;
 
+		entry = ((void *)irq_map) + (nsize * nentries);
 		*entry = (struct of_interrupt_map_entry) {
 			.pci_irq_mask = {
 				.pci_addr = {
@@ -79,16 +86,18 @@ void pci__generate_fdt_nodes(void *fdt)
 				},
 				.pci_pin	= cpu_to_fdt32(pin),
 			},
-			.plic_phandle	= cpu_to_fdt32(PHANDLE_PLIC),
-			.plic_irq	= cpu_to_fdt32(irq),
+			.irqchip_phandle	= cpu_to_fdt32(riscv_irqchip_phandle),
+			.irqchip_line		= cpu_to_fdt32(irq),
 		};
 
+		if (riscv_irqchip_line_sensing)
+			entry->irqchip_sense = cpu_to_fdt32(IRQ_TYPE_LEVEL_HIGH);
+
 		nentries++;
 		dev_hdr = device__next_dev(dev_hdr);
 	}
 
-	_FDT(fdt_property(fdt, "interrupt-map", irq_map,
-			  sizeof(struct of_interrupt_map_entry) * nentries));
+	_FDT(fdt_property(fdt, "interrupt-map", irq_map, nsize * nentries));
 
 	/* ... and the corresponding mask. */
 	if (nentries) {
@@ -105,5 +114,10 @@ void pci__generate_fdt_nodes(void *fdt)
 				  sizeof(irq_mask)));
 	}
 
+	/* Set MSI parent if available */
+	if (riscv_irqchip_msi_phandle != PHANDLE_RESERVED)
+		_FDT(fdt_property_cell(fdt, "msi-parent",
+				       riscv_irqchip_msi_phandle));
+
 	_FDT(fdt_end_node(fdt));
 }
diff --git a/riscv/plic.c b/riscv/plic.c
index 6242286..ab7c574 100644
--- a/riscv/plic.c
+++ b/riscv/plic.c
@@ -118,7 +118,6 @@ struct plic_context {
 struct plic_state {
 	bool ready;
 	struct kvm *kvm;
-	struct device_header dev_hdr;
 
 	/* Static Configuration */
 	u32 num_irq;
@@ -204,7 +203,7 @@ static u32 __plic_context_irq_claim(struct plic_state *s,
 	return best_irq;
 }
 
-void plic__irq_trig(struct kvm *kvm, int irq, int level, bool edge)
+static void plic__irq_trig(struct kvm *kvm, int irq, int level, bool edge)
 {
 	bool irq_marked = false;
 	u8 i, irq_prio, irq_word;
@@ -425,7 +424,7 @@ static void plic__mmio_callback(struct kvm_cpu *vcpu,
 		die("plic: invalid len=%d", len);
 
 	addr &= ~0x3;
-	addr -= RISCV_PLIC;
+	addr -= RISCV_IRQCHIP;
 
 	if (is_write) {
 		if (PRIORITY_BASE <= addr && addr < ENABLE_BASE) {
@@ -464,34 +463,23 @@ static void plic__mmio_callback(struct kvm_cpu *vcpu,
 	}
 }
 
-void plic__generate_irq_prop(void *fdt, u8 irq, enum irq_type irq_type)
-{
-	u32 irq_prop[] = {
-		cpu_to_fdt32(irq)
-	};
-
-	_FDT(fdt_property(fdt, "interrupts", irq_prop, sizeof(irq_prop)));
-}
-
-static void plic__generate_fdt_node(void *fdt,
-				    struct device_header *dev_hdr,
-				    void (*generate_irq_prop)(void *fdt,
-							      u8 irq,
-							      enum irq_type))
+static void plic__generate_fdt_node(void *fdt, struct kvm *kvm)
 {
 	u32 i;
+	char name[64];
 	u32 reg_cells[4], *irq_cells;
 
 	reg_cells[0] = 0;
-	reg_cells[1] = cpu_to_fdt32(RISCV_PLIC);
+	reg_cells[1] = cpu_to_fdt32(RISCV_IRQCHIP);
 	reg_cells[2] = 0;
-	reg_cells[3] = cpu_to_fdt32(RISCV_PLIC_SIZE);
+	reg_cells[3] = cpu_to_fdt32(RISCV_IRQCHIP_SIZE);
 
 	irq_cells = calloc(plic.num_context * 2, sizeof(u32));
 	if (!irq_cells)
 		die("Failed to alloc irq_cells");
 
-	_FDT(fdt_begin_node(fdt, "interrupt-controller@0c000000"));
+	sprintf(name, "interrupt-controller@%08x", (u32)RISCV_IRQCHIP);
+	_FDT(fdt_begin_node(fdt, name));
 	_FDT(fdt_property_string(fdt, "compatible", "riscv,plic0"));
 	_FDT(fdt_property(fdt, "reg", reg_cells, sizeof(reg_cells)));
 	_FDT(fdt_property_cell(fdt, "#interrupt-cells", 1));
@@ -518,12 +506,10 @@ static int plic__init(struct kvm *kvm)
 	int ret;
 	struct plic_context *c;
 
-	plic.kvm = kvm;
-	plic.dev_hdr = (struct device_header) {
-		.bus_type	= DEVICE_BUS_MMIO,
-		.data		= plic__generate_fdt_node,
-	};
+	if (riscv_irqchip != IRQCHIP_PLIC)
+		return 0;
 
+	plic.kvm = kvm;
 	plic.num_irq = MAX_DEVICES;
 	plic.num_irq_word = plic.num_irq / 32;
 	if ((plic.num_irq_word * 32) < plic.num_irq)
@@ -544,15 +530,11 @@ static int plic__init(struct kvm *kvm)
 
 	mutex_init(&plic.irq_lock);
 
-	ret = kvm__register_mmio(kvm, RISCV_PLIC, RISCV_PLIC_SIZE,
+	ret = kvm__register_mmio(kvm, RISCV_IRQCHIP, RISCV_IRQCHIP_SIZE,
 				 false, plic__mmio_callback, &plic);
 	if (ret)
 		return ret;
 
-	ret = device__register(&plic.dev_hdr);
-	if (ret)
-		return ret;
-
 	plic.ready = true;
 
 	return 0;
@@ -562,10 +544,27 @@ dev_init(plic__init);
 
 static int plic__exit(struct kvm *kvm)
 {
+	if (riscv_irqchip != IRQCHIP_PLIC)
+		return 0;
+
 	plic.ready = false;
-	kvm__deregister_mmio(kvm, RISCV_PLIC);
+	kvm__deregister_mmio(kvm, RISCV_IRQCHIP);
 	free(plic.contexts);
 
 	return 0;
 }
 dev_exit(plic__exit);
+
+void plic__create(struct kvm *kvm)
+{
+	if (riscv_irqchip != IRQCHIP_UNKNOWN)
+		return;
+
+	riscv_irqchip = IRQCHIP_PLIC;
+	riscv_irqchip_inkernel = false;
+	riscv_irqchip_trigger = plic__irq_trig;
+	riscv_irqchip_generate_fdt_node = plic__generate_fdt_node;
+	riscv_irqchip_phandle = PHANDLE_PLIC;
+	riscv_irqchip_msi_phandle = PHANDLE_RESERVED;
+	riscv_irqchip_line_sensing = false;
+}
-- 
2.34.1

Re: [kvmtool PATCH v2 3/6] riscv: Make irqchip support pluggable

[Andrew Jones]

On Mon, Sep 18, 2023 at 06:27:27PM +0530, Anup Patel wrote:
> We will be having different types of irqchip:
> 1) PLIC emulated by user-space
> 2) AIA APLIC and IMSIC provided by in-kernel KVM module
> 
> To support above, we de-couple PLIC specific code from generic
> RISC-V code (such as FDT generation) so that we can easily add
> other types of irqchip.
> 
> Signed-off-by: Anup Patel <apatel@ventanamicro.com>
> ---
>  riscv/fdt.c                  | 14 ++++++--
>  riscv/include/kvm/kvm-arch.h | 25 ++++++++++++---
>  riscv/irq.c                  | 62 ++++++++++++++++++++++++++++++++++--
>  riscv/kvm.c                  |  2 ++
>  riscv/pci.c                  | 32 +++++++++++++------
>  riscv/plic.c                 | 61 +++++++++++++++++------------------
>  6 files changed, 147 insertions(+), 49 deletions(-)
> 
> diff --git a/riscv/fdt.c b/riscv/fdt.c
> index 2724c6e..9af71b5 100644
> --- a/riscv/fdt.c
> +++ b/riscv/fdt.c
> @@ -209,19 +209,26 @@ static int setup_fdt(struct kvm *kvm)
>  	/* CPUs */
>  	generate_cpu_nodes(fdt, kvm);
>  
> +	/* IRQCHIP */
> +	if (!riscv_irqchip_generate_fdt_node)
> +		die("No way to generate IRQCHIP FDT node\n");
> +	riscv_irqchip_generate_fdt_node(fdt, kvm);
> +
>  	/* Simple Bus */
>  	_FDT(fdt_begin_node(fdt, "smb"));
>  	_FDT(fdt_property_string(fdt, "compatible", "simple-bus"));
>  	_FDT(fdt_property_cell(fdt, "#address-cells", 0x2));
>  	_FDT(fdt_property_cell(fdt, "#size-cells", 0x2));
> -	_FDT(fdt_property_cell(fdt, "interrupt-parent", PHANDLE_PLIC));
> +	_FDT(fdt_property_cell(fdt, "interrupt-parent",
> +			       riscv_irqchip_phandle));
>  	_FDT(fdt_property(fdt, "ranges", NULL, 0));
>  
>  	/* Virtio MMIO devices */
>  	dev_hdr = device__first_dev(DEVICE_BUS_MMIO);
>  	while (dev_hdr) {
>  		generate_mmio_fdt_nodes = dev_hdr->data;
> -		generate_mmio_fdt_nodes(fdt, dev_hdr, plic__generate_irq_prop);
> +		generate_mmio_fdt_nodes(fdt, dev_hdr,
> +					riscv__generate_irq_prop);
>  		dev_hdr = device__next_dev(dev_hdr);
>  	}
>  
> @@ -229,7 +236,8 @@ static int setup_fdt(struct kvm *kvm)
>  	dev_hdr = device__first_dev(DEVICE_BUS_IOPORT);
>  	while (dev_hdr) {
>  		generate_mmio_fdt_nodes = dev_hdr->data;
> -		generate_mmio_fdt_nodes(fdt, dev_hdr, plic__generate_irq_prop);
> +		generate_mmio_fdt_nodes(fdt, dev_hdr,
> +					riscv__generate_irq_prop);
>  		dev_hdr = device__next_dev(dev_hdr);
>  	}

nit: I'd let the above lines run long, but I know you prefer wrapping
     at 80.

>  
> diff --git a/riscv/include/kvm/kvm-arch.h b/riscv/include/kvm/kvm-arch.h
> index 660355b..cd37fc6 100644
> --- a/riscv/include/kvm/kvm-arch.h
> +++ b/riscv/include/kvm/kvm-arch.h
> @@ -10,8 +10,8 @@
>  
>  #define RISCV_IOPORT		0x00000000ULL
>  #define RISCV_IOPORT_SIZE	SZ_64K
> -#define RISCV_PLIC		0x0c000000ULL
> -#define RISCV_PLIC_SIZE		SZ_64M
> +#define RISCV_IRQCHIP		0x08000000ULL
> +#define RISCV_IRQCHIP_SIZE		SZ_128M

I checked the applied patch and the SZ_128M is over indented.

>  #define RISCV_MMIO		0x10000000ULL
>  #define RISCV_MMIO_SIZE		SZ_512M
>  #define RISCV_PCI		0x30000000ULL
> @@ -84,10 +84,27 @@ static inline bool riscv_addr_in_ioport_region(u64 phys_addr)
>  
>  enum irq_type;
>  
> -void plic__generate_irq_prop(void *fdt, u8 irq, enum irq_type irq_type);
> +enum irqchip_type {
> +	IRQCHIP_UNKNOWN = 0,
> +	IRQCHIP_PLIC,
> +	IRQCHIP_AIA
> +};
> +
> +extern enum irqchip_type riscv_irqchip;
> +extern bool riscv_irqchip_inkernel;
> +extern void (*riscv_irqchip_trigger)(struct kvm *kvm, int irq,
> +				     int level, bool edge);
> +extern void (*riscv_irqchip_generate_fdt_node)(void *fdt, struct kvm *kvm);
> +extern u32 riscv_irqchip_phandle;
> +extern u32 riscv_irqchip_msi_phandle;
> +extern bool riscv_irqchip_line_sensing;
>  
> -void plic__irq_trig(struct kvm *kvm, int irq, int level, bool edge);
> +void plic__create(struct kvm *kvm);
>  
>  void pci__generate_fdt_nodes(void *fdt);
>  
> +void riscv__generate_irq_prop(void *fdt, u8 irq, enum irq_type irq_type);
> +
> +void riscv__irqchip_create(struct kvm *kvm);
> +
>  #endif /* KVM__KVM_ARCH_H */
> diff --git a/riscv/irq.c b/riscv/irq.c
> index 78a582d..b608a2f 100644
> --- a/riscv/irq.c
> +++ b/riscv/irq.c
> @@ -1,13 +1,71 @@
>  #include "kvm/kvm.h"
>  #include "kvm/kvm-cpu.h"
>  #include "kvm/irq.h"
> +#include "kvm/fdt.h"
> +#include "kvm/virtio.h"
> +
> +enum irqchip_type riscv_irqchip = IRQCHIP_UNKNOWN;
> +bool riscv_irqchip_inkernel = false;
> +void (*riscv_irqchip_trigger)(struct kvm *kvm, int irq, int level, bool edge)
> +				= NULL;
> +void (*riscv_irqchip_generate_fdt_node)(void *fdt, struct kvm *kvm) = NULL;
> +u32 riscv_irqchip_phandle = PHANDLE_RESERVED;
> +u32 riscv_irqchip_msi_phandle = PHANDLE_RESERVED;
> +bool riscv_irqchip_line_sensing = false;

nit: no need to init BSS symbols with zeros.

>  
>  void kvm__irq_line(struct kvm *kvm, int irq, int level)
>  {
> -	plic__irq_trig(kvm, irq, level, false);
> +	struct kvm_irq_level irq_level;
> +
> +	if (riscv_irqchip_inkernel) {
> +		irq_level.irq = irq;
> +		irq_level.level = !!level;
> +		if (ioctl(kvm->vm_fd, KVM_IRQ_LINE, &irq_level) < 0)
> +			pr_warning("%s: Could not KVM_IRQ_LINE for irq %d\n",
> +				   __func__, irq);
> +	} else {
> +		if (riscv_irqchip_trigger)
> +			riscv_irqchip_trigger(kvm, irq, level, false);
> +		else
> +			pr_warning("%s: Can't change level for irq %d\n",
> +				   __func__, irq);
> +	}
>  }
>  
>  void kvm__irq_trigger(struct kvm *kvm, int irq)
>  {
> -	plic__irq_trig(kvm, irq, 1, true);
> +	if (riscv_irqchip_inkernel) {
> +		kvm__irq_line(kvm, irq, VIRTIO_IRQ_HIGH);
> +		kvm__irq_line(kvm, irq, VIRTIO_IRQ_LOW);
> +	} else {
> +		if (riscv_irqchip_trigger)
> +			riscv_irqchip_trigger(kvm, irq, 1, true);
> +		else
> +			pr_warning("%s: Can't trigger irq %d\n",
> +				   __func__, irq);
> +	}
> +}
> +
> +void riscv__generate_irq_prop(void *fdt, u8 irq, enum irq_type irq_type)
> +{
> +	u32 prop[2], size;
> +
> +	prop[0] = cpu_to_fdt32(irq);
> +	size = sizeof(u32);
> +	if (riscv_irqchip_line_sensing) {
> +		prop[1] = cpu_to_fdt32(irq_type);
> +		size += sizeof(u32);
> +	}
> +
> +	_FDT(fdt_property(fdt, "interrupts", prop, size));
> +}
> +
> +void riscv__irqchip_create(struct kvm *kvm)
> +{
> +	/* Try PLIC irqchip */
> +	plic__create(kvm);
> +
> +	/* Fail if irqchip unknown */
> +	if (riscv_irqchip == IRQCHIP_UNKNOWN)
> +		die("No IRQCHIP found\n");
>  }
> diff --git a/riscv/kvm.c b/riscv/kvm.c
> index 8daad94..1d49479 100644
> --- a/riscv/kvm.c
> +++ b/riscv/kvm.c
> @@ -96,6 +96,8 @@ void kvm__arch_init(struct kvm *kvm)
>  
>  	madvise(kvm->arch.ram_alloc_start, kvm->arch.ram_alloc_size,
>  		MADV_HUGEPAGE);
> +
> +	riscv__irqchip_create(kvm);
>  }
>  
>  #define FDT_ALIGN	SZ_4M
> diff --git a/riscv/pci.c b/riscv/pci.c
> index 604fd20..61dee06 100644
> --- a/riscv/pci.c
> +++ b/riscv/pci.c
> @@ -7,20 +7,21 @@
>  
>  /*
>   * An entry in the interrupt-map table looks like:
> - * <pci unit address> <pci interrupt pin> <plic phandle> <plic interrupt>
> + * <pci unit address> <pci interrupt pin> <irqchip phandle> <irqchip line>
>   */
>  
>  struct of_interrupt_map_entry {
>  	struct of_pci_irq_mask		pci_irq_mask;
> -	u32				plic_phandle;
> -	u32				plic_irq;
> +	u32				irqchip_phandle;
> +	u32				irqchip_line;
> +	u32				irqchip_sense;
>  } __attribute__((packed));
>  
>  void pci__generate_fdt_nodes(void *fdt)
>  {
>  	struct device_header *dev_hdr;
>  	struct of_interrupt_map_entry irq_map[OF_PCI_IRQ_MAP_MAX];
> -	unsigned nentries = 0;
> +	unsigned nentries = 0, nsize;
>  	/* Bus range */
>  	u32 bus_range[] = { cpu_to_fdt32(0), cpu_to_fdt32(1), };
>  	/* Configuration Space */
> @@ -48,6 +49,11 @@ void pci__generate_fdt_nodes(void *fdt)
>  		},
>  	};
>  
> +	/* Find size of each interrupt map entery */
> +	nsize = sizeof(struct of_interrupt_map_entry);
> +	if (!riscv_irqchip_line_sensing)
> +		nsize -= sizeof(u32);
> +
>  	/* Boilerplate PCI properties */
>  	_FDT(fdt_begin_node(fdt, "pci"));
>  	_FDT(fdt_property_string(fdt, "device_type", "pci"));
> @@ -64,12 +70,13 @@ void pci__generate_fdt_nodes(void *fdt)
>  	/* Generate the interrupt map ... */
>  	dev_hdr = device__first_dev(DEVICE_BUS_PCI);
>  	while (dev_hdr && nentries < ARRAY_SIZE(irq_map)) {
> -		struct of_interrupt_map_entry *entry = &irq_map[nentries];
> +		struct of_interrupt_map_entry *entry;
>  		struct pci_device_header *pci_hdr = dev_hdr->data;
>  		u8 dev_num = dev_hdr->dev_num;
>  		u8 pin = pci_hdr->irq_pin;
>  		u8 irq = pci_hdr->irq_line;
>  
> +		entry = ((void *)irq_map) + (nsize * nentries);

nit: The outer () of both terms are superfluous.

>  		*entry = (struct of_interrupt_map_entry) {
>  			.pci_irq_mask = {
>  				.pci_addr = {
> @@ -79,16 +86,18 @@ void pci__generate_fdt_nodes(void *fdt)
>  				},
>  				.pci_pin	= cpu_to_fdt32(pin),
>  			},
> -			.plic_phandle	= cpu_to_fdt32(PHANDLE_PLIC),
> -			.plic_irq	= cpu_to_fdt32(irq),
> +			.irqchip_phandle	= cpu_to_fdt32(riscv_irqchip_phandle),
> +			.irqchip_line		= cpu_to_fdt32(irq),
>  		};
>  
> +		if (riscv_irqchip_line_sensing)
> +			entry->irqchip_sense = cpu_to_fdt32(IRQ_TYPE_LEVEL_HIGH);
> +
>  		nentries++;
>  		dev_hdr = device__next_dev(dev_hdr);
>  	}
>  
> -	_FDT(fdt_property(fdt, "interrupt-map", irq_map,
> -			  sizeof(struct of_interrupt_map_entry) * nentries));
> +	_FDT(fdt_property(fdt, "interrupt-map", irq_map, nsize * nentries));
>  
>  	/* ... and the corresponding mask. */
>  	if (nentries) {
> @@ -105,5 +114,10 @@ void pci__generate_fdt_nodes(void *fdt)
>  				  sizeof(irq_mask)));
>  	}
>  
> +	/* Set MSI parent if available */
> +	if (riscv_irqchip_msi_phandle != PHANDLE_RESERVED)
> +		_FDT(fdt_property_cell(fdt, "msi-parent",
> +				       riscv_irqchip_msi_phandle));
> +
>  	_FDT(fdt_end_node(fdt));
>  }
> diff --git a/riscv/plic.c b/riscv/plic.c
> index 6242286..ab7c574 100644
> --- a/riscv/plic.c
> +++ b/riscv/plic.c
> @@ -118,7 +118,6 @@ struct plic_context {
>  struct plic_state {
>  	bool ready;
>  	struct kvm *kvm;
> -	struct device_header dev_hdr;
>  
>  	/* Static Configuration */
>  	u32 num_irq;
> @@ -204,7 +203,7 @@ static u32 __plic_context_irq_claim(struct plic_state *s,
>  	return best_irq;
>  }
>  
> -void plic__irq_trig(struct kvm *kvm, int irq, int level, bool edge)
> +static void plic__irq_trig(struct kvm *kvm, int irq, int level, bool edge)
>  {
>  	bool irq_marked = false;
>  	u8 i, irq_prio, irq_word;
> @@ -425,7 +424,7 @@ static void plic__mmio_callback(struct kvm_cpu *vcpu,
>  		die("plic: invalid len=%d", len);
>  
>  	addr &= ~0x3;
> -	addr -= RISCV_PLIC;
> +	addr -= RISCV_IRQCHIP;
>  
>  	if (is_write) {
>  		if (PRIORITY_BASE <= addr && addr < ENABLE_BASE) {
> @@ -464,34 +463,23 @@ static void plic__mmio_callback(struct kvm_cpu *vcpu,
>  	}
>  }
>  
> -void plic__generate_irq_prop(void *fdt, u8 irq, enum irq_type irq_type)
> -{
> -	u32 irq_prop[] = {
> -		cpu_to_fdt32(irq)
> -	};
> -
> -	_FDT(fdt_property(fdt, "interrupts", irq_prop, sizeof(irq_prop)));
> -}
> -
> -static void plic__generate_fdt_node(void *fdt,
> -				    struct device_header *dev_hdr,
> -				    void (*generate_irq_prop)(void *fdt,
> -							      u8 irq,
> -							      enum irq_type))
> +static void plic__generate_fdt_node(void *fdt, struct kvm *kvm)
>  {
>  	u32 i;
> +	char name[64];
>  	u32 reg_cells[4], *irq_cells;
>  
>  	reg_cells[0] = 0;
> -	reg_cells[1] = cpu_to_fdt32(RISCV_PLIC);
> +	reg_cells[1] = cpu_to_fdt32(RISCV_IRQCHIP);
>  	reg_cells[2] = 0;
> -	reg_cells[3] = cpu_to_fdt32(RISCV_PLIC_SIZE);
> +	reg_cells[3] = cpu_to_fdt32(RISCV_IRQCHIP_SIZE);
>  
>  	irq_cells = calloc(plic.num_context * 2, sizeof(u32));
>  	if (!irq_cells)
>  		die("Failed to alloc irq_cells");
>  
> -	_FDT(fdt_begin_node(fdt, "interrupt-controller@0c000000"));
> +	sprintf(name, "interrupt-controller@%08x", (u32)RISCV_IRQCHIP);
> +	_FDT(fdt_begin_node(fdt, name));
>  	_FDT(fdt_property_string(fdt, "compatible", "riscv,plic0"));
>  	_FDT(fdt_property(fdt, "reg", reg_cells, sizeof(reg_cells)));
>  	_FDT(fdt_property_cell(fdt, "#interrupt-cells", 1));
> @@ -518,12 +506,10 @@ static int plic__init(struct kvm *kvm)
>  	int ret;
>  	struct plic_context *c;
>  
> -	plic.kvm = kvm;
> -	plic.dev_hdr = (struct device_header) {
> -		.bus_type	= DEVICE_BUS_MMIO,
> -		.data		= plic__generate_fdt_node,
> -	};
> +	if (riscv_irqchip != IRQCHIP_PLIC)
> +		return 0;
>  
> +	plic.kvm = kvm;
>  	plic.num_irq = MAX_DEVICES;
>  	plic.num_irq_word = plic.num_irq / 32;
>  	if ((plic.num_irq_word * 32) < plic.num_irq)
> @@ -544,15 +530,11 @@ static int plic__init(struct kvm *kvm)
>  
>  	mutex_init(&plic.irq_lock);
>  
> -	ret = kvm__register_mmio(kvm, RISCV_PLIC, RISCV_PLIC_SIZE,
> +	ret = kvm__register_mmio(kvm, RISCV_IRQCHIP, RISCV_IRQCHIP_SIZE,
>  				 false, plic__mmio_callback, &plic);
>  	if (ret)
>  		return ret;
>  
> -	ret = device__register(&plic.dev_hdr);
> -	if (ret)
> -		return ret;
> -

Dropping this device__register() made me scratch my head a bit. I think
it's not necessary to enumerate the irqchip in the device list and its
fdt node is now generated by riscv_irqchip_generate_fdt_node(), but it'd
be a lot easier for me if the commit message explained why this is OK.

>  	plic.ready = true;
>  
>  	return 0;
> @@ -562,10 +544,27 @@ dev_init(plic__init);
>  
>  static int plic__exit(struct kvm *kvm)
>  {
> +	if (riscv_irqchip != IRQCHIP_PLIC)
> +		return 0;
> +
>  	plic.ready = false;
> -	kvm__deregister_mmio(kvm, RISCV_PLIC);
> +	kvm__deregister_mmio(kvm, RISCV_IRQCHIP);
>  	free(plic.contexts);
>  
>  	return 0;
>  }
>  dev_exit(plic__exit);
> +
> +void plic__create(struct kvm *kvm)
> +{
> +	if (riscv_irqchip != IRQCHIP_UNKNOWN)
> +		return;
> +
> +	riscv_irqchip = IRQCHIP_PLIC;
> +	riscv_irqchip_inkernel = false;
> +	riscv_irqchip_trigger = plic__irq_trig;
> +	riscv_irqchip_generate_fdt_node = plic__generate_fdt_node;
> +	riscv_irqchip_phandle = PHANDLE_PLIC;
> +	riscv_irqchip_msi_phandle = PHANDLE_RESERVED;
> +	riscv_irqchip_line_sensing = false;
> +}
> -- 
> 2.34.1
>

Besides the nits, the whitespace issue, and request for another sentence
in the commit message,

Reviewed-by: Andrew Jones <ajones@ventanamicro.com>

Thanks,
drew

Re: [kvmtool PATCH v2 3/6] riscv: Make irqchip support pluggable

[Anup Patel]

On Wed, Oct 25, 2023 at 6:40 PM Andrew Jones <ajones@ventanamicro.com> wrote:
>
> On Mon, Sep 18, 2023 at 06:27:27PM +0530, Anup Patel wrote:
> > We will be having different types of irqchip:
> > 1) PLIC emulated by user-space
> > 2) AIA APLIC and IMSIC provided by in-kernel KVM module
> >
> > To support above, we de-couple PLIC specific code from generic
> > RISC-V code (such as FDT generation) so that we can easily add
> > other types of irqchip.
> >
> > Signed-off-by: Anup Patel <apatel@ventanamicro.com>
> > ---
> >  riscv/fdt.c                  | 14 ++++++--
> >  riscv/include/kvm/kvm-arch.h | 25 ++++++++++++---
> >  riscv/irq.c                  | 62 ++++++++++++++++++++++++++++++++++--
> >  riscv/kvm.c                  |  2 ++
> >  riscv/pci.c                  | 32 +++++++++++++------
> >  riscv/plic.c                 | 61 +++++++++++++++++------------------
> >  6 files changed, 147 insertions(+), 49 deletions(-)
> >
> > diff --git a/riscv/fdt.c b/riscv/fdt.c
> > index 2724c6e..9af71b5 100644
> > --- a/riscv/fdt.c
> > +++ b/riscv/fdt.c
> > @@ -209,19 +209,26 @@ static int setup_fdt(struct kvm *kvm)
> >       /* CPUs */
> >       generate_cpu_nodes(fdt, kvm);
> >
> > +     /* IRQCHIP */
> > +     if (!riscv_irqchip_generate_fdt_node)
> > +             die("No way to generate IRQCHIP FDT node\n");
> > +     riscv_irqchip_generate_fdt_node(fdt, kvm);
> > +
> >       /* Simple Bus */
> >       _FDT(fdt_begin_node(fdt, "smb"));
> >       _FDT(fdt_property_string(fdt, "compatible", "simple-bus"));
> >       _FDT(fdt_property_cell(fdt, "#address-cells", 0x2));
> >       _FDT(fdt_property_cell(fdt, "#size-cells", 0x2));
> > -     _FDT(fdt_property_cell(fdt, "interrupt-parent", PHANDLE_PLIC));
> > +     _FDT(fdt_property_cell(fdt, "interrupt-parent",
> > +                            riscv_irqchip_phandle));
> >       _FDT(fdt_property(fdt, "ranges", NULL, 0));
> >
> >       /* Virtio MMIO devices */
> >       dev_hdr = device__first_dev(DEVICE_BUS_MMIO);
> >       while (dev_hdr) {
> >               generate_mmio_fdt_nodes = dev_hdr->data;
> > -             generate_mmio_fdt_nodes(fdt, dev_hdr, plic__generate_irq_prop);
> > +             generate_mmio_fdt_nodes(fdt, dev_hdr,
> > +                                     riscv__generate_irq_prop);
> >               dev_hdr = device__next_dev(dev_hdr);
> >       }
> >
> > @@ -229,7 +236,8 @@ static int setup_fdt(struct kvm *kvm)
> >       dev_hdr = device__first_dev(DEVICE_BUS_IOPORT);
> >       while (dev_hdr) {
> >               generate_mmio_fdt_nodes = dev_hdr->data;
> > -             generate_mmio_fdt_nodes(fdt, dev_hdr, plic__generate_irq_prop);
> > +             generate_mmio_fdt_nodes(fdt, dev_hdr,
> > +                                     riscv__generate_irq_prop);
> >               dev_hdr = device__next_dev(dev_hdr);
> >       }
>
> nit: I'd let the above lines run long, but I know you prefer wrapping
>      at 80.
>
> >
> > diff --git a/riscv/include/kvm/kvm-arch.h b/riscv/include/kvm/kvm-arch.h
> > index 660355b..cd37fc6 100644
> > --- a/riscv/include/kvm/kvm-arch.h
> > +++ b/riscv/include/kvm/kvm-arch.h
> > @@ -10,8 +10,8 @@
> >
> >  #define RISCV_IOPORT         0x00000000ULL
> >  #define RISCV_IOPORT_SIZE    SZ_64K
> > -#define RISCV_PLIC           0x0c000000ULL
> > -#define RISCV_PLIC_SIZE              SZ_64M
> > +#define RISCV_IRQCHIP                0x08000000ULL
> > +#define RISCV_IRQCHIP_SIZE           SZ_128M
>
> I checked the applied patch and the SZ_128M is over indented.
>
> >  #define RISCV_MMIO           0x10000000ULL
> >  #define RISCV_MMIO_SIZE              SZ_512M
> >  #define RISCV_PCI            0x30000000ULL
> > @@ -84,10 +84,27 @@ static inline bool riscv_addr_in_ioport_region(u64 phys_addr)
> >
> >  enum irq_type;
> >
> > -void plic__generate_irq_prop(void *fdt, u8 irq, enum irq_type irq_type);
> > +enum irqchip_type {
> > +     IRQCHIP_UNKNOWN = 0,
> > +     IRQCHIP_PLIC,
> > +     IRQCHIP_AIA
> > +};
> > +
> > +extern enum irqchip_type riscv_irqchip;
> > +extern bool riscv_irqchip_inkernel;
> > +extern void (*riscv_irqchip_trigger)(struct kvm *kvm, int irq,
> > +                                  int level, bool edge);
> > +extern void (*riscv_irqchip_generate_fdt_node)(void *fdt, struct kvm *kvm);
> > +extern u32 riscv_irqchip_phandle;
> > +extern u32 riscv_irqchip_msi_phandle;
> > +extern bool riscv_irqchip_line_sensing;
> >
> > -void plic__irq_trig(struct kvm *kvm, int irq, int level, bool edge);
> > +void plic__create(struct kvm *kvm);
> >
> >  void pci__generate_fdt_nodes(void *fdt);
> >
> > +void riscv__generate_irq_prop(void *fdt, u8 irq, enum irq_type irq_type);
> > +
> > +void riscv__irqchip_create(struct kvm *kvm);
> > +
> >  #endif /* KVM__KVM_ARCH_H */
> > diff --git a/riscv/irq.c b/riscv/irq.c
> > index 78a582d..b608a2f 100644
> > --- a/riscv/irq.c
> > +++ b/riscv/irq.c
> > @@ -1,13 +1,71 @@
> >  #include "kvm/kvm.h"
> >  #include "kvm/kvm-cpu.h"
> >  #include "kvm/irq.h"
> > +#include "kvm/fdt.h"
> > +#include "kvm/virtio.h"
> > +
> > +enum irqchip_type riscv_irqchip = IRQCHIP_UNKNOWN;
> > +bool riscv_irqchip_inkernel = false;
> > +void (*riscv_irqchip_trigger)(struct kvm *kvm, int irq, int level, bool edge)
> > +                             = NULL;
> > +void (*riscv_irqchip_generate_fdt_node)(void *fdt, struct kvm *kvm) = NULL;
> > +u32 riscv_irqchip_phandle = PHANDLE_RESERVED;
> > +u32 riscv_irqchip_msi_phandle = PHANDLE_RESERVED;
> > +bool riscv_irqchip_line_sensing = false;
>
> nit: no need to init BSS symbols with zeros.

Okay, I will update.

>
> >
> >  void kvm__irq_line(struct kvm *kvm, int irq, int level)
> >  {
> > -     plic__irq_trig(kvm, irq, level, false);
> > +     struct kvm_irq_level irq_level;
> > +
> > +     if (riscv_irqchip_inkernel) {
> > +             irq_level.irq = irq;
> > +             irq_level.level = !!level;
> > +             if (ioctl(kvm->vm_fd, KVM_IRQ_LINE, &irq_level) < 0)
> > +                     pr_warning("%s: Could not KVM_IRQ_LINE for irq %d\n",
> > +                                __func__, irq);
> > +     } else {
> > +             if (riscv_irqchip_trigger)
> > +                     riscv_irqchip_trigger(kvm, irq, level, false);
> > +             else
> > +                     pr_warning("%s: Can't change level for irq %d\n",
> > +                                __func__, irq);
> > +     }
> >  }
> >
> >  void kvm__irq_trigger(struct kvm *kvm, int irq)
> >  {
> > -     plic__irq_trig(kvm, irq, 1, true);
> > +     if (riscv_irqchip_inkernel) {
> > +             kvm__irq_line(kvm, irq, VIRTIO_IRQ_HIGH);
> > +             kvm__irq_line(kvm, irq, VIRTIO_IRQ_LOW);
> > +     } else {
> > +             if (riscv_irqchip_trigger)
> > +                     riscv_irqchip_trigger(kvm, irq, 1, true);
> > +             else
> > +                     pr_warning("%s: Can't trigger irq %d\n",
> > +                                __func__, irq);
> > +     }
> > +}
> > +
> > +void riscv__generate_irq_prop(void *fdt, u8 irq, enum irq_type irq_type)
> > +{
> > +     u32 prop[2], size;
> > +
> > +     prop[0] = cpu_to_fdt32(irq);
> > +     size = sizeof(u32);
> > +     if (riscv_irqchip_line_sensing) {
> > +             prop[1] = cpu_to_fdt32(irq_type);
> > +             size += sizeof(u32);
> > +     }
> > +
> > +     _FDT(fdt_property(fdt, "interrupts", prop, size));
> > +}
> > +
> > +void riscv__irqchip_create(struct kvm *kvm)
> > +{
> > +     /* Try PLIC irqchip */
> > +     plic__create(kvm);
> > +
> > +     /* Fail if irqchip unknown */
> > +     if (riscv_irqchip == IRQCHIP_UNKNOWN)
> > +             die("No IRQCHIP found\n");
> >  }
> > diff --git a/riscv/kvm.c b/riscv/kvm.c
> > index 8daad94..1d49479 100644
> > --- a/riscv/kvm.c
> > +++ b/riscv/kvm.c
> > @@ -96,6 +96,8 @@ void kvm__arch_init(struct kvm *kvm)
> >
> >       madvise(kvm->arch.ram_alloc_start, kvm->arch.ram_alloc_size,
> >               MADV_HUGEPAGE);
> > +
> > +     riscv__irqchip_create(kvm);
> >  }
> >
> >  #define FDT_ALIGN    SZ_4M
> > diff --git a/riscv/pci.c b/riscv/pci.c
> > index 604fd20..61dee06 100644
> > --- a/riscv/pci.c
> > +++ b/riscv/pci.c
> > @@ -7,20 +7,21 @@
> >
> >  /*
> >   * An entry in the interrupt-map table looks like:
> > - * <pci unit address> <pci interrupt pin> <plic phandle> <plic interrupt>
> > + * <pci unit address> <pci interrupt pin> <irqchip phandle> <irqchip line>
> >   */
> >
> >  struct of_interrupt_map_entry {
> >       struct of_pci_irq_mask          pci_irq_mask;
> > -     u32                             plic_phandle;
> > -     u32                             plic_irq;
> > +     u32                             irqchip_phandle;
> > +     u32                             irqchip_line;
> > +     u32                             irqchip_sense;
> >  } __attribute__((packed));
> >
> >  void pci__generate_fdt_nodes(void *fdt)
> >  {
> >       struct device_header *dev_hdr;
> >       struct of_interrupt_map_entry irq_map[OF_PCI_IRQ_MAP_MAX];
> > -     unsigned nentries = 0;
> > +     unsigned nentries = 0, nsize;
> >       /* Bus range */
> >       u32 bus_range[] = { cpu_to_fdt32(0), cpu_to_fdt32(1), };
> >       /* Configuration Space */
> > @@ -48,6 +49,11 @@ void pci__generate_fdt_nodes(void *fdt)
> >               },
> >       };
> >
> > +     /* Find size of each interrupt map entery */
> > +     nsize = sizeof(struct of_interrupt_map_entry);
> > +     if (!riscv_irqchip_line_sensing)
> > +             nsize -= sizeof(u32);
> > +
> >       /* Boilerplate PCI properties */
> >       _FDT(fdt_begin_node(fdt, "pci"));
> >       _FDT(fdt_property_string(fdt, "device_type", "pci"));
> > @@ -64,12 +70,13 @@ void pci__generate_fdt_nodes(void *fdt)
> >       /* Generate the interrupt map ... */
> >       dev_hdr = device__first_dev(DEVICE_BUS_PCI);
> >       while (dev_hdr && nentries < ARRAY_SIZE(irq_map)) {
> > -             struct of_interrupt_map_entry *entry = &irq_map[nentries];
> > +             struct of_interrupt_map_entry *entry;
> >               struct pci_device_header *pci_hdr = dev_hdr->data;
> >               u8 dev_num = dev_hdr->dev_num;
> >               u8 pin = pci_hdr->irq_pin;
> >               u8 irq = pci_hdr->irq_line;
> >
> > +             entry = ((void *)irq_map) + (nsize * nentries);
>
> nit: The outer () of both terms are superfluous.

Okay, I will update.

>
> >               *entry = (struct of_interrupt_map_entry) {
> >                       .pci_irq_mask = {
> >                               .pci_addr = {
> > @@ -79,16 +86,18 @@ void pci__generate_fdt_nodes(void *fdt)
> >                               },
> >                               .pci_pin        = cpu_to_fdt32(pin),
> >                       },
> > -                     .plic_phandle   = cpu_to_fdt32(PHANDLE_PLIC),
> > -                     .plic_irq       = cpu_to_fdt32(irq),
> > +                     .irqchip_phandle        = cpu_to_fdt32(riscv_irqchip_phandle),
> > +                     .irqchip_line           = cpu_to_fdt32(irq),
> >               };
> >
> > +             if (riscv_irqchip_line_sensing)
> > +                     entry->irqchip_sense = cpu_to_fdt32(IRQ_TYPE_LEVEL_HIGH);
> > +
> >               nentries++;
> >               dev_hdr = device__next_dev(dev_hdr);
> >       }
> >
> > -     _FDT(fdt_property(fdt, "interrupt-map", irq_map,
> > -                       sizeof(struct of_interrupt_map_entry) * nentries));
> > +     _FDT(fdt_property(fdt, "interrupt-map", irq_map, nsize * nentries));
> >
> >       /* ... and the corresponding mask. */
> >       if (nentries) {
> > @@ -105,5 +114,10 @@ void pci__generate_fdt_nodes(void *fdt)
> >                                 sizeof(irq_mask)));
> >       }
> >
> > +     /* Set MSI parent if available */
> > +     if (riscv_irqchip_msi_phandle != PHANDLE_RESERVED)
> > +             _FDT(fdt_property_cell(fdt, "msi-parent",
> > +                                    riscv_irqchip_msi_phandle));
> > +
> >       _FDT(fdt_end_node(fdt));
> >  }
> > diff --git a/riscv/plic.c b/riscv/plic.c
> > index 6242286..ab7c574 100644
> > --- a/riscv/plic.c
> > +++ b/riscv/plic.c
> > @@ -118,7 +118,6 @@ struct plic_context {
> >  struct plic_state {
> >       bool ready;
> >       struct kvm *kvm;
> > -     struct device_header dev_hdr;
> >
> >       /* Static Configuration */
> >       u32 num_irq;
> > @@ -204,7 +203,7 @@ static u32 __plic_context_irq_claim(struct plic_state *s,
> >       return best_irq;
> >  }
> >
> > -void plic__irq_trig(struct kvm *kvm, int irq, int level, bool edge)
> > +static void plic__irq_trig(struct kvm *kvm, int irq, int level, bool edge)
> >  {
> >       bool irq_marked = false;
> >       u8 i, irq_prio, irq_word;
> > @@ -425,7 +424,7 @@ static void plic__mmio_callback(struct kvm_cpu *vcpu,
> >               die("plic: invalid len=%d", len);
> >
> >       addr &= ~0x3;
> > -     addr -= RISCV_PLIC;
> > +     addr -= RISCV_IRQCHIP;
> >
> >       if (is_write) {
> >               if (PRIORITY_BASE <= addr && addr < ENABLE_BASE) {
> > @@ -464,34 +463,23 @@ static void plic__mmio_callback(struct kvm_cpu *vcpu,
> >       }
> >  }
> >
> > -void plic__generate_irq_prop(void *fdt, u8 irq, enum irq_type irq_type)
> > -{
> > -     u32 irq_prop[] = {
> > -             cpu_to_fdt32(irq)
> > -     };
> > -
> > -     _FDT(fdt_property(fdt, "interrupts", irq_prop, sizeof(irq_prop)));
> > -}
> > -
> > -static void plic__generate_fdt_node(void *fdt,
> > -                                 struct device_header *dev_hdr,
> > -                                 void (*generate_irq_prop)(void *fdt,
> > -                                                           u8 irq,
> > -                                                           enum irq_type))
> > +static void plic__generate_fdt_node(void *fdt, struct kvm *kvm)
> >  {
> >       u32 i;
> > +     char name[64];
> >       u32 reg_cells[4], *irq_cells;
> >
> >       reg_cells[0] = 0;
> > -     reg_cells[1] = cpu_to_fdt32(RISCV_PLIC);
> > +     reg_cells[1] = cpu_to_fdt32(RISCV_IRQCHIP);
> >       reg_cells[2] = 0;
> > -     reg_cells[3] = cpu_to_fdt32(RISCV_PLIC_SIZE);
> > +     reg_cells[3] = cpu_to_fdt32(RISCV_IRQCHIP_SIZE);
> >
> >       irq_cells = calloc(plic.num_context * 2, sizeof(u32));
> >       if (!irq_cells)
> >               die("Failed to alloc irq_cells");
> >
> > -     _FDT(fdt_begin_node(fdt, "interrupt-controller@0c000000"));
> > +     sprintf(name, "interrupt-controller@%08x", (u32)RISCV_IRQCHIP);
> > +     _FDT(fdt_begin_node(fdt, name));
> >       _FDT(fdt_property_string(fdt, "compatible", "riscv,plic0"));
> >       _FDT(fdt_property(fdt, "reg", reg_cells, sizeof(reg_cells)));
> >       _FDT(fdt_property_cell(fdt, "#interrupt-cells", 1));
> > @@ -518,12 +506,10 @@ static int plic__init(struct kvm *kvm)
> >       int ret;
> >       struct plic_context *c;
> >
> > -     plic.kvm = kvm;
> > -     plic.dev_hdr = (struct device_header) {
> > -             .bus_type       = DEVICE_BUS_MMIO,
> > -             .data           = plic__generate_fdt_node,
> > -     };
> > +     if (riscv_irqchip != IRQCHIP_PLIC)
> > +             return 0;
> >
> > +     plic.kvm = kvm;
> >       plic.num_irq = MAX_DEVICES;
> >       plic.num_irq_word = plic.num_irq / 32;
> >       if ((plic.num_irq_word * 32) < plic.num_irq)
> > @@ -544,15 +530,11 @@ static int plic__init(struct kvm *kvm)
> >
> >       mutex_init(&plic.irq_lock);
> >
> > -     ret = kvm__register_mmio(kvm, RISCV_PLIC, RISCV_PLIC_SIZE,
> > +     ret = kvm__register_mmio(kvm, RISCV_IRQCHIP, RISCV_IRQCHIP_SIZE,
> >                                false, plic__mmio_callback, &plic);
> >       if (ret)
> >               return ret;
> >
> > -     ret = device__register(&plic.dev_hdr);
> > -     if (ret)
> > -             return ret;
> > -
>
> Dropping this device__register() made me scratch my head a bit. I think
> it's not necessary to enumerate the irqchip in the device list and its
> fdt node is now generated by riscv_irqchip_generate_fdt_node(), but it'd
> be a lot easier for me if the commit message explained why this is OK.

Okay, I will update the commit description.

>
> >       plic.ready = true;
> >
> >       return 0;
> > @@ -562,10 +544,27 @@ dev_init(plic__init);
> >
> >  static int plic__exit(struct kvm *kvm)
> >  {
> > +     if (riscv_irqchip != IRQCHIP_PLIC)
> > +             return 0;
> > +
> >       plic.ready = false;
> > -     kvm__deregister_mmio(kvm, RISCV_PLIC);
> > +     kvm__deregister_mmio(kvm, RISCV_IRQCHIP);
> >       free(plic.contexts);
> >
> >       return 0;
> >  }
> >  dev_exit(plic__exit);
> > +
> > +void plic__create(struct kvm *kvm)
> > +{
> > +     if (riscv_irqchip != IRQCHIP_UNKNOWN)
> > +             return;
> > +
> > +     riscv_irqchip = IRQCHIP_PLIC;
> > +     riscv_irqchip_inkernel = false;
> > +     riscv_irqchip_trigger = plic__irq_trig;
> > +     riscv_irqchip_generate_fdt_node = plic__generate_fdt_node;
> > +     riscv_irqchip_phandle = PHANDLE_PLIC;
> > +     riscv_irqchip_msi_phandle = PHANDLE_RESERVED;
> > +     riscv_irqchip_line_sensing = false;
> > +}
> > --
> > 2.34.1
> >
>
> Besides the nits, the whitespace issue, and request for another sentence
> in the commit message,
>
> Reviewed-by: Andrew Jones <ajones@ventanamicro.com>
>
> Thanks,
> drew

Regards,
Anup

[kvmtool PATCH v2 4/6] riscv: Add IRQFD support for in-kernel AIA irqchip

[Anup Patel]

To use irqfd with in-kernel AIA irqchip, we add custom
irq__add_irqfd and irq__del_irqfd functions. This allows
us to defer actual KVM_IRQFD ioctl() until AIA irqchip
is initialized by KVMTOOL.

Signed-off-by: Anup Patel <apatel@ventanamicro.com>
---
 riscv/include/kvm/kvm-arch.h | 11 ++++++
 riscv/irq.c                  | 73 ++++++++++++++++++++++++++++++++++++
 2 files changed, 84 insertions(+)

diff --git a/riscv/include/kvm/kvm-arch.h b/riscv/include/kvm/kvm-arch.h
index cd37fc6..1a8af6a 100644
--- a/riscv/include/kvm/kvm-arch.h
+++ b/riscv/include/kvm/kvm-arch.h
@@ -98,11 +98,22 @@ extern void (*riscv_irqchip_generate_fdt_node)(void *fdt, struct kvm *kvm);
 extern u32 riscv_irqchip_phandle;
 extern u32 riscv_irqchip_msi_phandle;
 extern bool riscv_irqchip_line_sensing;
+extern bool riscv_irqchip_irqfd_ready;
 
 void plic__create(struct kvm *kvm);
 
 void pci__generate_fdt_nodes(void *fdt);
 
+int riscv__add_irqfd(struct kvm *kvm, unsigned int gsi, int trigger_fd,
+		     int resample_fd);
+
+void riscv__del_irqfd(struct kvm *kvm, unsigned int gsi, int trigger_fd);
+
+#define irq__add_irqfd riscv__add_irqfd
+#define irq__del_irqfd riscv__del_irqfd
+
+int riscv__setup_irqfd_lines(struct kvm *kvm);
+
 void riscv__generate_irq_prop(void *fdt, u8 irq, enum irq_type irq_type);
 
 void riscv__irqchip_create(struct kvm *kvm);
diff --git a/riscv/irq.c b/riscv/irq.c
index b608a2f..e6c0939 100644
--- a/riscv/irq.c
+++ b/riscv/irq.c
@@ -12,6 +12,7 @@ void (*riscv_irqchip_generate_fdt_node)(void *fdt, struct kvm *kvm) = NULL;
 u32 riscv_irqchip_phandle = PHANDLE_RESERVED;
 u32 riscv_irqchip_msi_phandle = PHANDLE_RESERVED;
 bool riscv_irqchip_line_sensing = false;
+bool riscv_irqchip_irqfd_ready = false;
 
 void kvm__irq_line(struct kvm *kvm, int irq, int level)
 {
@@ -46,6 +47,78 @@ void kvm__irq_trigger(struct kvm *kvm, int irq)
 	}
 }
 
+struct riscv_irqfd_line {
+	unsigned int		gsi;
+	int			trigger_fd;
+	int			resample_fd;
+	struct list_head	list;
+};
+
+static LIST_HEAD(irqfd_lines);
+
+int riscv__add_irqfd(struct kvm *kvm, unsigned int gsi, int trigger_fd,
+		     int resample_fd)
+{
+	struct riscv_irqfd_line *line;
+
+	if (riscv_irqchip_irqfd_ready)
+		return irq__common_add_irqfd(kvm, gsi, trigger_fd,
+					     resample_fd);
+
+	/* Postpone the routing setup until we have a distributor */
+	line = malloc(sizeof(*line));
+	if (!line)
+		return -ENOMEM;
+
+	*line = (struct riscv_irqfd_line) {
+		.gsi		= gsi,
+		.trigger_fd	= trigger_fd,
+		.resample_fd	= resample_fd,
+	};
+	list_add(&line->list, &irqfd_lines);
+
+	return 0;
+}
+
+void riscv__del_irqfd(struct kvm *kvm, unsigned int gsi, int trigger_fd)
+{
+	struct riscv_irqfd_line *line;
+
+	if (riscv_irqchip_irqfd_ready) {
+		irq__common_del_irqfd(kvm, gsi, trigger_fd);
+		return;
+	}
+
+	list_for_each_entry(line, &irqfd_lines, list) {
+		if (line->gsi != gsi)
+			continue;
+
+		list_del(&line->list);
+		free(line);
+		break;
+	}
+}
+
+int riscv__setup_irqfd_lines(struct kvm *kvm)
+{
+	int ret;
+	struct riscv_irqfd_line *line, *tmp;
+
+	list_for_each_entry_safe(line, tmp, &irqfd_lines, list) {
+		ret = irq__common_add_irqfd(kvm, line->gsi, line->trigger_fd,
+					    line->resample_fd);
+		if (ret < 0) {
+			pr_err("Failed to register IRQFD");
+			return ret;
+		}
+
+		list_del(&line->list);
+		free(line);
+	}
+
+	return 0;
+}
+
 void riscv__generate_irq_prop(void *fdt, u8 irq, enum irq_type irq_type)
 {
 	u32 prop[2], size;
-- 
2.34.1

Re: [kvmtool PATCH v2 4/6] riscv: Add IRQFD support for in-kernel AIA irqchip

[Andrew Jones]

On Mon, Sep 18, 2023 at 06:27:28PM +0530, Anup Patel wrote:
> To use irqfd with in-kernel AIA irqchip, we add custom
> irq__add_irqfd and irq__del_irqfd functions. This allows
> us to defer actual KVM_IRQFD ioctl() until AIA irqchip
> is initialized by KVMTOOL.
> 
> Signed-off-by: Anup Patel <apatel@ventanamicro.com>
> ---
>  riscv/include/kvm/kvm-arch.h | 11 ++++++
>  riscv/irq.c                  | 73 ++++++++++++++++++++++++++++++++++++
>  2 files changed, 84 insertions(+)
> 
> diff --git a/riscv/include/kvm/kvm-arch.h b/riscv/include/kvm/kvm-arch.h
> index cd37fc6..1a8af6a 100644
> --- a/riscv/include/kvm/kvm-arch.h
> +++ b/riscv/include/kvm/kvm-arch.h
> @@ -98,11 +98,22 @@ extern void (*riscv_irqchip_generate_fdt_node)(void *fdt, struct kvm *kvm);
>  extern u32 riscv_irqchip_phandle;
>  extern u32 riscv_irqchip_msi_phandle;
>  extern bool riscv_irqchip_line_sensing;
> +extern bool riscv_irqchip_irqfd_ready;
>  
>  void plic__create(struct kvm *kvm);
>  
>  void pci__generate_fdt_nodes(void *fdt);
>  
> +int riscv__add_irqfd(struct kvm *kvm, unsigned int gsi, int trigger_fd,
> +		     int resample_fd);
> +
> +void riscv__del_irqfd(struct kvm *kvm, unsigned int gsi, int trigger_fd);
> +
> +#define irq__add_irqfd riscv__add_irqfd
> +#define irq__del_irqfd riscv__del_irqfd
> +
> +int riscv__setup_irqfd_lines(struct kvm *kvm);
> +
>  void riscv__generate_irq_prop(void *fdt, u8 irq, enum irq_type irq_type);
>  
>  void riscv__irqchip_create(struct kvm *kvm);
> diff --git a/riscv/irq.c b/riscv/irq.c
> index b608a2f..e6c0939 100644
> --- a/riscv/irq.c
> +++ b/riscv/irq.c
> @@ -12,6 +12,7 @@ void (*riscv_irqchip_generate_fdt_node)(void *fdt, struct kvm *kvm) = NULL;
>  u32 riscv_irqchip_phandle = PHANDLE_RESERVED;
>  u32 riscv_irqchip_msi_phandle = PHANDLE_RESERVED;
>  bool riscv_irqchip_line_sensing = false;
> +bool riscv_irqchip_irqfd_ready = false;
>  
>  void kvm__irq_line(struct kvm *kvm, int irq, int level)
>  {
> @@ -46,6 +47,78 @@ void kvm__irq_trigger(struct kvm *kvm, int irq)
>  	}
>  }
>  
> +struct riscv_irqfd_line {
> +	unsigned int		gsi;
> +	int			trigger_fd;
> +	int			resample_fd;
> +	struct list_head	list;
> +};
> +
> +static LIST_HEAD(irqfd_lines);
> +
> +int riscv__add_irqfd(struct kvm *kvm, unsigned int gsi, int trigger_fd,
> +		     int resample_fd)
> +{
> +	struct riscv_irqfd_line *line;
> +
> +	if (riscv_irqchip_irqfd_ready)
> +		return irq__common_add_irqfd(kvm, gsi, trigger_fd,
> +					     resample_fd);
> +
> +	/* Postpone the routing setup until we have a distributor */

This comment comes from the Arm code. We probably want to replace
distributor with "until the AIA is initialized" or something.

> +	line = malloc(sizeof(*line));
> +	if (!line)
> +		return -ENOMEM;
> +
> +	*line = (struct riscv_irqfd_line) {
> +		.gsi		= gsi,
> +		.trigger_fd	= trigger_fd,
> +		.resample_fd	= resample_fd,
> +	};
> +	list_add(&line->list, &irqfd_lines);
> +
> +	return 0;
> +}
> +
> +void riscv__del_irqfd(struct kvm *kvm, unsigned int gsi, int trigger_fd)
> +{
> +	struct riscv_irqfd_line *line;
> +
> +	if (riscv_irqchip_irqfd_ready) {
> +		irq__common_del_irqfd(kvm, gsi, trigger_fd);
> +		return;
> +	}
> +
> +	list_for_each_entry(line, &irqfd_lines, list) {
> +		if (line->gsi != gsi)
> +			continue;
> +
> +		list_del(&line->list);
> +		free(line);
> +		break;
> +	}
> +}
> +
> +int riscv__setup_irqfd_lines(struct kvm *kvm)
> +{
> +	int ret;
> +	struct riscv_irqfd_line *line, *tmp;
> +
> +	list_for_each_entry_safe(line, tmp, &irqfd_lines, list) {
> +		ret = irq__common_add_irqfd(kvm, line->gsi, line->trigger_fd,
> +					    line->resample_fd);
> +		if (ret < 0) {
> +			pr_err("Failed to register IRQFD");
> +			return ret;
> +		}
> +
> +		list_del(&line->list);
> +		free(line);
> +	}
> +
> +	return 0;
> +}
> +
>  void riscv__generate_irq_prop(void *fdt, u8 irq, enum irq_type irq_type)
>  {
>  	u32 prop[2], size;
> -- 
> 2.34.1
>

Other than the comment,

Reviewed-by: Andrew Jones <ajones@ventanamicro.com>

Re: [kvmtool PATCH v2 4/6] riscv: Add IRQFD support for in-kernel AIA irqchip

[Anup Patel]

On Wed, Oct 25, 2023 at 6:47 PM Andrew Jones <ajones@ventanamicro.com> wrote:
>
> On Mon, Sep 18, 2023 at 06:27:28PM +0530, Anup Patel wrote:
> > To use irqfd with in-kernel AIA irqchip, we add custom
> > irq__add_irqfd and irq__del_irqfd functions. This allows
> > us to defer actual KVM_IRQFD ioctl() until AIA irqchip
> > is initialized by KVMTOOL.
> >
> > Signed-off-by: Anup Patel <apatel@ventanamicro.com>
> > ---
> >  riscv/include/kvm/kvm-arch.h | 11 ++++++
> >  riscv/irq.c                  | 73 ++++++++++++++++++++++++++++++++++++
> >  2 files changed, 84 insertions(+)
> >
> > diff --git a/riscv/include/kvm/kvm-arch.h b/riscv/include/kvm/kvm-arch.h
> > index cd37fc6..1a8af6a 100644
> > --- a/riscv/include/kvm/kvm-arch.h
> > +++ b/riscv/include/kvm/kvm-arch.h
> > @@ -98,11 +98,22 @@ extern void (*riscv_irqchip_generate_fdt_node)(void *fdt, struct kvm *kvm);
> >  extern u32 riscv_irqchip_phandle;
> >  extern u32 riscv_irqchip_msi_phandle;
> >  extern bool riscv_irqchip_line_sensing;
> > +extern bool riscv_irqchip_irqfd_ready;
> >
> >  void plic__create(struct kvm *kvm);
> >
> >  void pci__generate_fdt_nodes(void *fdt);
> >
> > +int riscv__add_irqfd(struct kvm *kvm, unsigned int gsi, int trigger_fd,
> > +                  int resample_fd);
> > +
> > +void riscv__del_irqfd(struct kvm *kvm, unsigned int gsi, int trigger_fd);
> > +
> > +#define irq__add_irqfd riscv__add_irqfd
> > +#define irq__del_irqfd riscv__del_irqfd
> > +
> > +int riscv__setup_irqfd_lines(struct kvm *kvm);
> > +
> >  void riscv__generate_irq_prop(void *fdt, u8 irq, enum irq_type irq_type);
> >
> >  void riscv__irqchip_create(struct kvm *kvm);
> > diff --git a/riscv/irq.c b/riscv/irq.c
> > index b608a2f..e6c0939 100644
> > --- a/riscv/irq.c
> > +++ b/riscv/irq.c
> > @@ -12,6 +12,7 @@ void (*riscv_irqchip_generate_fdt_node)(void *fdt, struct kvm *kvm) = NULL;
> >  u32 riscv_irqchip_phandle = PHANDLE_RESERVED;
> >  u32 riscv_irqchip_msi_phandle = PHANDLE_RESERVED;
> >  bool riscv_irqchip_line_sensing = false;
> > +bool riscv_irqchip_irqfd_ready = false;
> >
> >  void kvm__irq_line(struct kvm *kvm, int irq, int level)
> >  {
> > @@ -46,6 +47,78 @@ void kvm__irq_trigger(struct kvm *kvm, int irq)
> >       }
> >  }
> >
> > +struct riscv_irqfd_line {
> > +     unsigned int            gsi;
> > +     int                     trigger_fd;
> > +     int                     resample_fd;
> > +     struct list_head        list;
> > +};
> > +
> > +static LIST_HEAD(irqfd_lines);
> > +
> > +int riscv__add_irqfd(struct kvm *kvm, unsigned int gsi, int trigger_fd,
> > +                  int resample_fd)
> > +{
> > +     struct riscv_irqfd_line *line;
> > +
> > +     if (riscv_irqchip_irqfd_ready)
> > +             return irq__common_add_irqfd(kvm, gsi, trigger_fd,
> > +                                          resample_fd);
> > +
> > +     /* Postpone the routing setup until we have a distributor */
>
> This comment comes from the Arm code. We probably want to replace
> distributor with "until the AIA is initialized" or something.

Okay, I will update.

>
> > +     line = malloc(sizeof(*line));
> > +     if (!line)
> > +             return -ENOMEM;
> > +
> > +     *line = (struct riscv_irqfd_line) {
> > +             .gsi            = gsi,
> > +             .trigger_fd     = trigger_fd,
> > +             .resample_fd    = resample_fd,
> > +     };
> > +     list_add(&line->list, &irqfd_lines);
> > +
> > +     return 0;
> > +}
> > +
> > +void riscv__del_irqfd(struct kvm *kvm, unsigned int gsi, int trigger_fd)
> > +{
> > +     struct riscv_irqfd_line *line;
> > +
> > +     if (riscv_irqchip_irqfd_ready) {
> > +             irq__common_del_irqfd(kvm, gsi, trigger_fd);
> > +             return;
> > +     }
> > +
> > +     list_for_each_entry(line, &irqfd_lines, list) {
> > +             if (line->gsi != gsi)
> > +                     continue;
> > +
> > +             list_del(&line->list);
> > +             free(line);
> > +             break;
> > +     }
> > +}
> > +
> > +int riscv__setup_irqfd_lines(struct kvm *kvm)
> > +{
> > +     int ret;
> > +     struct riscv_irqfd_line *line, *tmp;
> > +
> > +     list_for_each_entry_safe(line, tmp, &irqfd_lines, list) {
> > +             ret = irq__common_add_irqfd(kvm, line->gsi, line->trigger_fd,
> > +                                         line->resample_fd);
> > +             if (ret < 0) {
> > +                     pr_err("Failed to register IRQFD");
> > +                     return ret;
> > +             }
> > +
> > +             list_del(&line->list);
> > +             free(line);
> > +     }
> > +
> > +     return 0;
> > +}
> > +
> >  void riscv__generate_irq_prop(void *fdt, u8 irq, enum irq_type irq_type)
> >  {
> >       u32 prop[2], size;
> > --
> > 2.34.1
> >
>
> Other than the comment,
>
> Reviewed-by: Andrew Jones <ajones@ventanamicro.com>

Regards,
Anup

[kvmtool PATCH v2 5/6] riscv: Use AIA in-kernel irqchip whenever KVM RISC-V supports

[Anup Patel]

The KVM RISC-V kernel module supports AIA in-kernel irqchip when
underlying host has AIA support. We detect and use AIA in-kernel
irqchip whenever possible otherwise we fallback to PLIC emulated
in user-space.

Signed-off-by: Anup Patel <apatel@ventanamicro.com>
---
 Makefile                     |   1 +
 riscv/aia.c                  | 227 +++++++++++++++++++++++++++++++++++
 riscv/include/kvm/fdt-arch.h |   8 +-
 riscv/include/kvm/kvm-arch.h |   2 +
 riscv/irq.c                  |   3 +
 5 files changed, 240 insertions(+), 1 deletion(-)
 create mode 100644 riscv/aia.c

diff --git a/Makefile b/Makefile
index e711670..acd5ffd 100644
--- a/Makefile
+++ b/Makefile
@@ -220,6 +220,7 @@ ifeq ($(ARCH),riscv)
 	OBJS		+= riscv/kvm-cpu.o
 	OBJS		+= riscv/pci.o
 	OBJS		+= riscv/plic.o
+	OBJS		+= riscv/aia.o
 	ifeq ($(RISCV_XLEN),32)
 		CFLAGS	+= -mabi=ilp32d -march=rv32gc
 	endif
diff --git a/riscv/aia.c b/riscv/aia.c
new file mode 100644
index 0000000..8c85b3f
--- /dev/null
+++ b/riscv/aia.c
@@ -0,0 +1,227 @@
+#include "kvm/devices.h"
+#include "kvm/fdt.h"
+#include "kvm/ioeventfd.h"
+#include "kvm/ioport.h"
+#include "kvm/kvm.h"
+#include "kvm/kvm-cpu.h"
+#include "kvm/irq.h"
+#include "kvm/util.h"
+
+static int aia_fd = -1;
+
+static u32 aia_mode = KVM_DEV_RISCV_AIA_MODE_EMUL;
+static struct kvm_device_attr aia_mode_attr = {
+	.group	= KVM_DEV_RISCV_AIA_GRP_CONFIG,
+	.attr	= KVM_DEV_RISCV_AIA_CONFIG_MODE,
+};
+
+static u32 aia_nr_ids = 0;
+static struct kvm_device_attr aia_nr_ids_attr = {
+	.group	= KVM_DEV_RISCV_AIA_GRP_CONFIG,
+	.attr	= KVM_DEV_RISCV_AIA_CONFIG_IDS,
+};
+
+static u32 aia_nr_sources = 0;
+static struct kvm_device_attr aia_nr_sources_attr = {
+	.group	= KVM_DEV_RISCV_AIA_GRP_CONFIG,
+	.attr	= KVM_DEV_RISCV_AIA_CONFIG_SRCS,
+};
+
+static u32 aia_hart_bits = 0;
+static struct kvm_device_attr aia_hart_bits_attr = {
+	.group	= KVM_DEV_RISCV_AIA_GRP_CONFIG,
+	.attr	= KVM_DEV_RISCV_AIA_CONFIG_HART_BITS,
+};
+
+static u32 aia_nr_harts = 0;
+
+#define IRQCHIP_AIA_NR			0
+
+#define AIA_IMSIC_BASE			RISCV_IRQCHIP
+#define AIA_IMSIC_ADDR(__hart)		\
+	(AIA_IMSIC_BASE + (__hart) * KVM_DEV_RISCV_IMSIC_SIZE)
+#define AIA_IMSIC_SIZE			\
+	(aia_nr_harts * KVM_DEV_RISCV_IMSIC_SIZE)
+#define AIA_APLIC_ADDR(__nr_harts)	\
+	(AIA_IMSIC_BASE + (__nr_harts) * KVM_DEV_RISCV_IMSIC_SIZE)
+
+static void aia__generate_fdt_node(void *fdt, struct kvm *kvm)
+{
+	u32 i;
+	char name[64];
+	u32 reg_cells[4], *irq_cells;
+
+	irq_cells = calloc(aia_nr_harts * 2, sizeof(u32));
+	if (!irq_cells)
+		die("Failed to alloc irq_cells");
+
+	sprintf(name, "imsics@%08x", (u32)AIA_IMSIC_BASE);
+	_FDT(fdt_begin_node(fdt, name));
+	_FDT(fdt_property_string(fdt, "compatible", "riscv,imsics"));
+	reg_cells[0] = 0;
+	reg_cells[1] = cpu_to_fdt32(AIA_IMSIC_BASE);
+	reg_cells[2] = 0;
+	reg_cells[3] = cpu_to_fdt32(AIA_IMSIC_SIZE);
+	_FDT(fdt_property(fdt, "reg", reg_cells, sizeof(reg_cells)));
+	_FDT(fdt_property_cell(fdt, "#interrupt-cells", 0));
+	_FDT(fdt_property(fdt, "interrupt-controller", NULL, 0));
+	_FDT(fdt_property(fdt, "msi-controller", NULL, 0));
+	_FDT(fdt_property_cell(fdt, "riscv,num-ids", aia_nr_ids));
+	_FDT(fdt_property_cell(fdt, "phandle", PHANDLE_AIA_IMSIC));
+	for (i = 0; i < aia_nr_harts; i++) {
+		irq_cells[2*i + 0] = cpu_to_fdt32(PHANDLE_CPU_INTC_BASE + i);
+		irq_cells[2*i + 1] = cpu_to_fdt32(9);
+	}
+	_FDT(fdt_property(fdt, "interrupts-extended", irq_cells,
+			  sizeof(u32) * aia_nr_harts * 2));
+	_FDT(fdt_end_node(fdt));
+
+	free(irq_cells);
+
+	/* Skip APLIC node if we have no interrupt sources */
+	if (!aia_nr_sources)
+		return;
+
+	sprintf(name, "aplic@%08x", (u32)AIA_APLIC_ADDR(aia_nr_harts));
+	_FDT(fdt_begin_node(fdt, name));
+	_FDT(fdt_property_string(fdt, "compatible", "riscv,aplic"));
+	reg_cells[0] = 0;
+	reg_cells[1] = cpu_to_fdt32(AIA_APLIC_ADDR(aia_nr_harts));
+	reg_cells[2] = 0;
+	reg_cells[3] = cpu_to_fdt32(KVM_DEV_RISCV_APLIC_SIZE);
+	_FDT(fdt_property(fdt, "reg", reg_cells, sizeof(reg_cells)));
+	_FDT(fdt_property_cell(fdt, "#interrupt-cells", 2));
+	_FDT(fdt_property(fdt, "interrupt-controller", NULL, 0));
+	_FDT(fdt_property_cell(fdt, "riscv,num-sources", aia_nr_sources));
+	_FDT(fdt_property_cell(fdt, "phandle", PHANDLE_AIA_APLIC));
+	_FDT(fdt_property_cell(fdt, "msi-parent", PHANDLE_AIA_IMSIC));
+	_FDT(fdt_end_node(fdt));
+}
+
+static int aia__irq_routing_init(struct kvm *kvm)
+{
+	int r;
+	int irqlines = aia_nr_sources + 1;
+
+	/* Skip this if we have no interrupt sources */
+	if (!aia_nr_sources)
+		return 0;
+
+	/*
+	 * This describes the default routing that the kernel uses without
+	 * any routing explicitly set up via KVM_SET_GSI_ROUTING. So we
+	 * don't need to commit these setting right now. The first actual
+	 * user (MSI routing) will engage these mappings then.
+	 */
+	for (next_gsi = 0; next_gsi < irqlines; next_gsi++) {
+		r = irq__allocate_routing_entry();
+		if (r)
+			return r;
+
+		irq_routing->entries[irq_routing->nr++] =
+			(struct kvm_irq_routing_entry) {
+				.gsi = next_gsi,
+				.type = KVM_IRQ_ROUTING_IRQCHIP,
+				.u.irqchip.irqchip = IRQCHIP_AIA_NR,
+				.u.irqchip.pin = next_gsi,
+		};
+	}
+
+	return 0;
+}
+
+static int aia__init(struct kvm *kvm)
+{
+	int i, ret;
+	u64 aia_addr = 0;
+	struct kvm_device_attr aia_addr_attr = {
+		.group	= KVM_DEV_RISCV_AIA_GRP_ADDR,
+		.addr	= (u64)(unsigned long)&aia_addr,
+	};
+	struct kvm_device_attr aia_init_attr = {
+		.group	= KVM_DEV_RISCV_AIA_GRP_CTRL,
+		.attr	= KVM_DEV_RISCV_AIA_CTRL_INIT,
+	};
+
+	/* Setup global device attribute variables */
+	aia_mode_attr.addr = (u64)(unsigned long)&aia_mode;
+	aia_nr_ids_attr.addr = (u64)(unsigned long)&aia_nr_ids;
+	aia_nr_sources_attr.addr = (u64)(unsigned long)&aia_nr_sources;
+	aia_hart_bits_attr.addr = (u64)(unsigned long)&aia_hart_bits;
+
+	/* Do nothing if AIA device not created */
+	if (aia_fd < 0)
+		return 0;
+
+	/* Set/Get AIA device config parameters */
+	ret = ioctl(aia_fd, KVM_GET_DEVICE_ATTR, &aia_mode_attr);
+	if (ret)
+		return ret;
+	ret = ioctl(aia_fd, KVM_GET_DEVICE_ATTR, &aia_nr_ids_attr);
+	if (ret)
+		return ret;
+	aia_nr_sources = irq__get_nr_allocated_lines();
+	ret = ioctl(aia_fd, KVM_SET_DEVICE_ATTR, &aia_nr_sources_attr);
+	if (ret)
+		return ret;
+	aia_hart_bits = fls_long(kvm->nrcpus);
+	ret = ioctl(aia_fd, KVM_SET_DEVICE_ATTR, &aia_hart_bits_attr);
+	if (ret)
+		return ret;
+
+	/* Save number of HARTs for FDT generation */
+	aia_nr_harts = kvm->nrcpus;
+
+	/* Set AIA device addresses */
+	aia_addr = AIA_APLIC_ADDR(aia_nr_harts);
+	aia_addr_attr.attr = KVM_DEV_RISCV_AIA_ADDR_APLIC;
+	ret = ioctl(aia_fd, KVM_SET_DEVICE_ATTR, &aia_addr_attr);
+	if (ret)
+		return ret;
+	for (i = 0; i < kvm->nrcpus; i++) {
+		aia_addr = AIA_IMSIC_ADDR(i);
+		aia_addr_attr.attr = KVM_DEV_RISCV_AIA_ADDR_IMSIC(i);
+		ret = ioctl(aia_fd, KVM_SET_DEVICE_ATTR, &aia_addr_attr);
+		if (ret)
+			return ret;
+	}
+
+	/* Setup default IRQ routing */
+	aia__irq_routing_init(kvm);
+
+	/* Initialize the AIA device */
+	ret = ioctl(aia_fd, KVM_SET_DEVICE_ATTR, &aia_init_attr);
+	if (ret)
+		return ret;
+
+	/* Mark IRQFD as ready */
+	riscv_irqchip_irqfd_ready = true;
+
+	return 0;
+}
+late_init(aia__init);
+
+void aia__create(struct kvm *kvm)
+{
+	int err;
+	struct kvm_create_device aia_device = {
+		.type = KVM_DEV_TYPE_RISCV_AIA,
+		.flags = 0,
+	};
+
+	if (kvm->cfg.arch.ext_disabled[KVM_RISCV_ISA_EXT_SSAIA])
+		return;
+
+	err = ioctl(kvm->vm_fd, KVM_CREATE_DEVICE, &aia_device);
+	if (err)
+		return;
+	aia_fd = aia_device.fd;
+
+	riscv_irqchip = IRQCHIP_AIA;
+	riscv_irqchip_inkernel = true;
+	riscv_irqchip_trigger = NULL;
+	riscv_irqchip_generate_fdt_node = aia__generate_fdt_node;
+	riscv_irqchip_phandle = PHANDLE_AIA_APLIC;
+	riscv_irqchip_msi_phandle = PHANDLE_AIA_IMSIC;
+	riscv_irqchip_line_sensing = true;
+}
diff --git a/riscv/include/kvm/fdt-arch.h b/riscv/include/kvm/fdt-arch.h
index f7548e8..d88b832 100644
--- a/riscv/include/kvm/fdt-arch.h
+++ b/riscv/include/kvm/fdt-arch.h
@@ -1,7 +1,13 @@
 #ifndef KVM__KVM_FDT_H
 #define KVM__KVM_FDT_H
 
-enum phandles {PHANDLE_RESERVED = 0, PHANDLE_PLIC, PHANDLES_MAX};
+enum phandles {
+	PHANDLE_RESERVED = 0,
+	PHANDLE_PLIC,
+	PHANDLE_AIA_APLIC,
+	PHANDLE_AIA_IMSIC,
+	PHANDLES_MAX
+};
 
 #define PHANDLE_CPU_INTC_BASE	PHANDLES_MAX
 
diff --git a/riscv/include/kvm/kvm-arch.h b/riscv/include/kvm/kvm-arch.h
index 1a8af6a..9f2159f 100644
--- a/riscv/include/kvm/kvm-arch.h
+++ b/riscv/include/kvm/kvm-arch.h
@@ -100,6 +100,8 @@ extern u32 riscv_irqchip_msi_phandle;
 extern bool riscv_irqchip_line_sensing;
 extern bool riscv_irqchip_irqfd_ready;
 
+void aia__create(struct kvm *kvm);
+
 void plic__create(struct kvm *kvm);
 
 void pci__generate_fdt_nodes(void *fdt);
diff --git a/riscv/irq.c b/riscv/irq.c
index e6c0939..be3e7ac 100644
--- a/riscv/irq.c
+++ b/riscv/irq.c
@@ -135,6 +135,9 @@ void riscv__generate_irq_prop(void *fdt, u8 irq, enum irq_type irq_type)
 
 void riscv__irqchip_create(struct kvm *kvm)
 {
+	/* Try AIA in-kernel irqchip. */
+	aia__create(kvm);
+
 	/* Try PLIC irqchip */
 	plic__create(kvm);
 
-- 
2.34.1

Re: [kvmtool PATCH v2 5/6] riscv: Use AIA in-kernel irqchip whenever KVM RISC-V supports

[Andrew Jones]

On Mon, Sep 18, 2023 at 06:27:29PM +0530, Anup Patel wrote:
> The KVM RISC-V kernel module supports AIA in-kernel irqchip when
> underlying host has AIA support. We detect and use AIA in-kernel
> irqchip whenever possible otherwise we fallback to PLIC emulated
> in user-space.
> 
> Signed-off-by: Anup Patel <apatel@ventanamicro.com>
> ---
>  Makefile                     |   1 +
>  riscv/aia.c                  | 227 +++++++++++++++++++++++++++++++++++
>  riscv/include/kvm/fdt-arch.h |   8 +-
>  riscv/include/kvm/kvm-arch.h |   2 +
>  riscv/irq.c                  |   3 +
>  5 files changed, 240 insertions(+), 1 deletion(-)
>  create mode 100644 riscv/aia.c
> 
> diff --git a/Makefile b/Makefile
> index e711670..acd5ffd 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -220,6 +220,7 @@ ifeq ($(ARCH),riscv)
>  	OBJS		+= riscv/kvm-cpu.o
>  	OBJS		+= riscv/pci.o
>  	OBJS		+= riscv/plic.o
> +	OBJS		+= riscv/aia.o
>  	ifeq ($(RISCV_XLEN),32)
>  		CFLAGS	+= -mabi=ilp32d -march=rv32gc
>  	endif
> diff --git a/riscv/aia.c b/riscv/aia.c
> new file mode 100644
> index 0000000..8c85b3f
> --- /dev/null
> +++ b/riscv/aia.c
> @@ -0,0 +1,227 @@
> +#include "kvm/devices.h"
> +#include "kvm/fdt.h"
> +#include "kvm/ioeventfd.h"
> +#include "kvm/ioport.h"
> +#include "kvm/kvm.h"
> +#include "kvm/kvm-cpu.h"
> +#include "kvm/irq.h"
> +#include "kvm/util.h"
> +
> +static int aia_fd = -1;
> +
> +static u32 aia_mode = KVM_DEV_RISCV_AIA_MODE_EMUL;
> +static struct kvm_device_attr aia_mode_attr = {
> +	.group	= KVM_DEV_RISCV_AIA_GRP_CONFIG,
> +	.attr	= KVM_DEV_RISCV_AIA_CONFIG_MODE,
> +};
> +
> +static u32 aia_nr_ids = 0;
> +static struct kvm_device_attr aia_nr_ids_attr = {
> +	.group	= KVM_DEV_RISCV_AIA_GRP_CONFIG,
> +	.attr	= KVM_DEV_RISCV_AIA_CONFIG_IDS,
> +};
> +
> +static u32 aia_nr_sources = 0;
> +static struct kvm_device_attr aia_nr_sources_attr = {
> +	.group	= KVM_DEV_RISCV_AIA_GRP_CONFIG,
> +	.attr	= KVM_DEV_RISCV_AIA_CONFIG_SRCS,
> +};
> +
> +static u32 aia_hart_bits = 0;
> +static struct kvm_device_attr aia_hart_bits_attr = {
> +	.group	= KVM_DEV_RISCV_AIA_GRP_CONFIG,
> +	.attr	= KVM_DEV_RISCV_AIA_CONFIG_HART_BITS,
> +};
> +
> +static u32 aia_nr_harts = 0;
> +
> +#define IRQCHIP_AIA_NR			0
> +
> +#define AIA_IMSIC_BASE			RISCV_IRQCHIP
> +#define AIA_IMSIC_ADDR(__hart)		\
> +	(AIA_IMSIC_BASE + (__hart) * KVM_DEV_RISCV_IMSIC_SIZE)
> +#define AIA_IMSIC_SIZE			\
> +	(aia_nr_harts * KVM_DEV_RISCV_IMSIC_SIZE)
> +#define AIA_APLIC_ADDR(__nr_harts)	\
> +	(AIA_IMSIC_BASE + (__nr_harts) * KVM_DEV_RISCV_IMSIC_SIZE)

AIA_APLIC_ADDR() probably doesn't need to take nr_harts since it's
always called with aia_nr_harts. So it could just be defined as

 #define AIA_APLIC_ADDR (AIA_IMSIC_BASE + AIA_IMSIC_SIZE)

> +
> +static void aia__generate_fdt_node(void *fdt, struct kvm *kvm)
> +{
> +	u32 i;
> +	char name[64];
> +	u32 reg_cells[4], *irq_cells;
> +
> +	irq_cells = calloc(aia_nr_harts * 2, sizeof(u32));
> +	if (!irq_cells)
> +		die("Failed to alloc irq_cells");
> +
> +	sprintf(name, "imsics@%08x", (u32)AIA_IMSIC_BASE);
> +	_FDT(fdt_begin_node(fdt, name));
> +	_FDT(fdt_property_string(fdt, "compatible", "riscv,imsics"));
> +	reg_cells[0] = 0;
> +	reg_cells[1] = cpu_to_fdt32(AIA_IMSIC_BASE);
> +	reg_cells[2] = 0;
> +	reg_cells[3] = cpu_to_fdt32(AIA_IMSIC_SIZE);
> +	_FDT(fdt_property(fdt, "reg", reg_cells, sizeof(reg_cells)));
> +	_FDT(fdt_property_cell(fdt, "#interrupt-cells", 0));
> +	_FDT(fdt_property(fdt, "interrupt-controller", NULL, 0));
> +	_FDT(fdt_property(fdt, "msi-controller", NULL, 0));
> +	_FDT(fdt_property_cell(fdt, "riscv,num-ids", aia_nr_ids));
> +	_FDT(fdt_property_cell(fdt, "phandle", PHANDLE_AIA_IMSIC));
> +	for (i = 0; i < aia_nr_harts; i++) {
> +		irq_cells[2*i + 0] = cpu_to_fdt32(PHANDLE_CPU_INTC_BASE + i);
> +		irq_cells[2*i + 1] = cpu_to_fdt32(9);
> +	}
> +	_FDT(fdt_property(fdt, "interrupts-extended", irq_cells,
> +			  sizeof(u32) * aia_nr_harts * 2));
> +	_FDT(fdt_end_node(fdt));
> +
> +	free(irq_cells);
> +
> +	/* Skip APLIC node if we have no interrupt sources */
> +	if (!aia_nr_sources)
> +		return;
> +
> +	sprintf(name, "aplic@%08x", (u32)AIA_APLIC_ADDR(aia_nr_harts));
> +	_FDT(fdt_begin_node(fdt, name));
> +	_FDT(fdt_property_string(fdt, "compatible", "riscv,aplic"));
> +	reg_cells[0] = 0;
> +	reg_cells[1] = cpu_to_fdt32(AIA_APLIC_ADDR(aia_nr_harts));
> +	reg_cells[2] = 0;
> +	reg_cells[3] = cpu_to_fdt32(KVM_DEV_RISCV_APLIC_SIZE);
> +	_FDT(fdt_property(fdt, "reg", reg_cells, sizeof(reg_cells)));
> +	_FDT(fdt_property_cell(fdt, "#interrupt-cells", 2));
> +	_FDT(fdt_property(fdt, "interrupt-controller", NULL, 0));
> +	_FDT(fdt_property_cell(fdt, "riscv,num-sources", aia_nr_sources));
> +	_FDT(fdt_property_cell(fdt, "phandle", PHANDLE_AIA_APLIC));
> +	_FDT(fdt_property_cell(fdt, "msi-parent", PHANDLE_AIA_IMSIC));
> +	_FDT(fdt_end_node(fdt));
> +}
> +
> +static int aia__irq_routing_init(struct kvm *kvm)
> +{
> +	int r;
> +	int irqlines = aia_nr_sources + 1;
> +
> +	/* Skip this if we have no interrupt sources */
> +	if (!aia_nr_sources)
> +		return 0;
> +
> +	/*
> +	 * This describes the default routing that the kernel uses without
> +	 * any routing explicitly set up via KVM_SET_GSI_ROUTING. So we
> +	 * don't need to commit these setting right now. The first actual
> +	 * user (MSI routing) will engage these mappings then.
> +	 */
> +	for (next_gsi = 0; next_gsi < irqlines; next_gsi++) {
> +		r = irq__allocate_routing_entry();
> +		if (r)
> +			return r;
> +
> +		irq_routing->entries[irq_routing->nr++] =
> +			(struct kvm_irq_routing_entry) {
> +				.gsi = next_gsi,
> +				.type = KVM_IRQ_ROUTING_IRQCHIP,
> +				.u.irqchip.irqchip = IRQCHIP_AIA_NR,
> +				.u.irqchip.pin = next_gsi,
> +		};
> +	}
> +
> +	return 0;
> +}
> +
> +static int aia__init(struct kvm *kvm)
> +{
> +	int i, ret;
> +	u64 aia_addr = 0;
> +	struct kvm_device_attr aia_addr_attr = {
> +		.group	= KVM_DEV_RISCV_AIA_GRP_ADDR,
> +		.addr	= (u64)(unsigned long)&aia_addr,
> +	};
> +	struct kvm_device_attr aia_init_attr = {
> +		.group	= KVM_DEV_RISCV_AIA_GRP_CTRL,
> +		.attr	= KVM_DEV_RISCV_AIA_CTRL_INIT,
> +	};
> +
> +	/* Setup global device attribute variables */
> +	aia_mode_attr.addr = (u64)(unsigned long)&aia_mode;
> +	aia_nr_ids_attr.addr = (u64)(unsigned long)&aia_nr_ids;
> +	aia_nr_sources_attr.addr = (u64)(unsigned long)&aia_nr_sources;
> +	aia_hart_bits_attr.addr = (u64)(unsigned long)&aia_hart_bits;
> +
> +	/* Do nothing if AIA device not created */
> +	if (aia_fd < 0)
> +		return 0;
> +
> +	/* Set/Get AIA device config parameters */
> +	ret = ioctl(aia_fd, KVM_GET_DEVICE_ATTR, &aia_mode_attr);
> +	if (ret)
> +		return ret;
> +	ret = ioctl(aia_fd, KVM_GET_DEVICE_ATTR, &aia_nr_ids_attr);
> +	if (ret)
> +		return ret;
> +	aia_nr_sources = irq__get_nr_allocated_lines();
> +	ret = ioctl(aia_fd, KVM_SET_DEVICE_ATTR, &aia_nr_sources_attr);
> +	if (ret)
> +		return ret;
> +	aia_hart_bits = fls_long(kvm->nrcpus);
> +	ret = ioctl(aia_fd, KVM_SET_DEVICE_ATTR, &aia_hart_bits_attr);
> +	if (ret)
> +		return ret;
> +
> +	/* Save number of HARTs for FDT generation */
> +	aia_nr_harts = kvm->nrcpus;
> +
> +	/* Set AIA device addresses */
> +	aia_addr = AIA_APLIC_ADDR(aia_nr_harts);
> +	aia_addr_attr.attr = KVM_DEV_RISCV_AIA_ADDR_APLIC;
> +	ret = ioctl(aia_fd, KVM_SET_DEVICE_ATTR, &aia_addr_attr);
> +	if (ret)
> +		return ret;
> +	for (i = 0; i < kvm->nrcpus; i++) {
> +		aia_addr = AIA_IMSIC_ADDR(i);
> +		aia_addr_attr.attr = KVM_DEV_RISCV_AIA_ADDR_IMSIC(i);
> +		ret = ioctl(aia_fd, KVM_SET_DEVICE_ATTR, &aia_addr_attr);
> +		if (ret)
> +			return ret;
> +	}
> +
> +	/* Setup default IRQ routing */
> +	aia__irq_routing_init(kvm);
> +
> +	/* Initialize the AIA device */
> +	ret = ioctl(aia_fd, KVM_SET_DEVICE_ATTR, &aia_init_attr);
> +	if (ret)
> +		return ret;
> +
> +	/* Mark IRQFD as ready */
> +	riscv_irqchip_irqfd_ready = true;
> +
> +	return 0;
> +}
> +late_init(aia__init);
> +
> +void aia__create(struct kvm *kvm)
> +{
> +	int err;
> +	struct kvm_create_device aia_device = {
> +		.type = KVM_DEV_TYPE_RISCV_AIA,
> +		.flags = 0,
> +	};
> +
> +	if (kvm->cfg.arch.ext_disabled[KVM_RISCV_ISA_EXT_SSAIA])
> +		return;
> +
> +	err = ioctl(kvm->vm_fd, KVM_CREATE_DEVICE, &aia_device);
> +	if (err)
> +		return;
> +	aia_fd = aia_device.fd;
> +
> +	riscv_irqchip = IRQCHIP_AIA;
> +	riscv_irqchip_inkernel = true;
> +	riscv_irqchip_trigger = NULL;
> +	riscv_irqchip_generate_fdt_node = aia__generate_fdt_node;
> +	riscv_irqchip_phandle = PHANDLE_AIA_APLIC;
> +	riscv_irqchip_msi_phandle = PHANDLE_AIA_IMSIC;
> +	riscv_irqchip_line_sensing = true;
> +}
> diff --git a/riscv/include/kvm/fdt-arch.h b/riscv/include/kvm/fdt-arch.h
> index f7548e8..d88b832 100644
> --- a/riscv/include/kvm/fdt-arch.h
> +++ b/riscv/include/kvm/fdt-arch.h
> @@ -1,7 +1,13 @@
>  #ifndef KVM__KVM_FDT_H
>  #define KVM__KVM_FDT_H
>  
> -enum phandles {PHANDLE_RESERVED = 0, PHANDLE_PLIC, PHANDLES_MAX};
> +enum phandles {
> +	PHANDLE_RESERVED = 0,
> +	PHANDLE_PLIC,
> +	PHANDLE_AIA_APLIC,
> +	PHANDLE_AIA_IMSIC,
> +	PHANDLES_MAX
> +};
>  
>  #define PHANDLE_CPU_INTC_BASE	PHANDLES_MAX
>  
> diff --git a/riscv/include/kvm/kvm-arch.h b/riscv/include/kvm/kvm-arch.h
> index 1a8af6a..9f2159f 100644
> --- a/riscv/include/kvm/kvm-arch.h
> +++ b/riscv/include/kvm/kvm-arch.h
> @@ -100,6 +100,8 @@ extern u32 riscv_irqchip_msi_phandle;
>  extern bool riscv_irqchip_line_sensing;
>  extern bool riscv_irqchip_irqfd_ready;
>  
> +void aia__create(struct kvm *kvm);
> +

nit: I'd remove the blank line between *__create functions.

>  void plic__create(struct kvm *kvm);
>  
>  void pci__generate_fdt_nodes(void *fdt);
> diff --git a/riscv/irq.c b/riscv/irq.c
> index e6c0939..be3e7ac 100644
> --- a/riscv/irq.c
> +++ b/riscv/irq.c
> @@ -135,6 +135,9 @@ void riscv__generate_irq_prop(void *fdt, u8 irq, enum irq_type irq_type)
>  
>  void riscv__irqchip_create(struct kvm *kvm)
>  {
> +	/* Try AIA in-kernel irqchip. */
> +	aia__create(kvm);
> +
>  	/* Try PLIC irqchip */
>  	plic__create(kvm);

I realize plic__create() just returns if aia__create() successful set up
the irqchip, but structuring this like

 ret = aia__create(kvm);
 if (!ret)
    ret = plic__create(kvm);
 if (!ret)
    die(...);

Might be a little easier to for future readers of the code to grok on
first read.


Either way,

Reviewed-by: Andrew Jones <ajones@ventanamicro.com>

Thanks,
drew

Re: [kvmtool PATCH v2 5/6] riscv: Use AIA in-kernel irqchip whenever KVM RISC-V supports

[Anup Patel]

On Wed, Oct 25, 2023 at 7:09 PM Andrew Jones <ajones@ventanamicro.com> wrote:
>
> On Mon, Sep 18, 2023 at 06:27:29PM +0530, Anup Patel wrote:
> > The KVM RISC-V kernel module supports AIA in-kernel irqchip when
> > underlying host has AIA support. We detect and use AIA in-kernel
> > irqchip whenever possible otherwise we fallback to PLIC emulated
> > in user-space.
> >
> > Signed-off-by: Anup Patel <apatel@ventanamicro.com>
> > ---
> >  Makefile                     |   1 +
> >  riscv/aia.c                  | 227 +++++++++++++++++++++++++++++++++++
> >  riscv/include/kvm/fdt-arch.h |   8 +-
> >  riscv/include/kvm/kvm-arch.h |   2 +
> >  riscv/irq.c                  |   3 +
> >  5 files changed, 240 insertions(+), 1 deletion(-)
> >  create mode 100644 riscv/aia.c
> >
> > diff --git a/Makefile b/Makefile
> > index e711670..acd5ffd 100644
> > --- a/Makefile
> > +++ b/Makefile
> > @@ -220,6 +220,7 @@ ifeq ($(ARCH),riscv)
> >       OBJS            += riscv/kvm-cpu.o
> >       OBJS            += riscv/pci.o
> >       OBJS            += riscv/plic.o
> > +     OBJS            += riscv/aia.o
> >       ifeq ($(RISCV_XLEN),32)
> >               CFLAGS  += -mabi=ilp32d -march=rv32gc
> >       endif
> > diff --git a/riscv/aia.c b/riscv/aia.c
> > new file mode 100644
> > index 0000000..8c85b3f
> > --- /dev/null
> > +++ b/riscv/aia.c
> > @@ -0,0 +1,227 @@
> > +#include "kvm/devices.h"
> > +#include "kvm/fdt.h"
> > +#include "kvm/ioeventfd.h"
> > +#include "kvm/ioport.h"
> > +#include "kvm/kvm.h"
> > +#include "kvm/kvm-cpu.h"
> > +#include "kvm/irq.h"
> > +#include "kvm/util.h"
> > +
> > +static int aia_fd = -1;
> > +
> > +static u32 aia_mode = KVM_DEV_RISCV_AIA_MODE_EMUL;
> > +static struct kvm_device_attr aia_mode_attr = {
> > +     .group  = KVM_DEV_RISCV_AIA_GRP_CONFIG,
> > +     .attr   = KVM_DEV_RISCV_AIA_CONFIG_MODE,
> > +};
> > +
> > +static u32 aia_nr_ids = 0;
> > +static struct kvm_device_attr aia_nr_ids_attr = {
> > +     .group  = KVM_DEV_RISCV_AIA_GRP_CONFIG,
> > +     .attr   = KVM_DEV_RISCV_AIA_CONFIG_IDS,
> > +};
> > +
> > +static u32 aia_nr_sources = 0;
> > +static struct kvm_device_attr aia_nr_sources_attr = {
> > +     .group  = KVM_DEV_RISCV_AIA_GRP_CONFIG,
> > +     .attr   = KVM_DEV_RISCV_AIA_CONFIG_SRCS,
> > +};
> > +
> > +static u32 aia_hart_bits = 0;
> > +static struct kvm_device_attr aia_hart_bits_attr = {
> > +     .group  = KVM_DEV_RISCV_AIA_GRP_CONFIG,
> > +     .attr   = KVM_DEV_RISCV_AIA_CONFIG_HART_BITS,
> > +};
> > +
> > +static u32 aia_nr_harts = 0;
> > +
> > +#define IRQCHIP_AIA_NR                       0
> > +
> > +#define AIA_IMSIC_BASE                       RISCV_IRQCHIP
> > +#define AIA_IMSIC_ADDR(__hart)               \
> > +     (AIA_IMSIC_BASE + (__hart) * KVM_DEV_RISCV_IMSIC_SIZE)
> > +#define AIA_IMSIC_SIZE                       \
> > +     (aia_nr_harts * KVM_DEV_RISCV_IMSIC_SIZE)
> > +#define AIA_APLIC_ADDR(__nr_harts)   \
> > +     (AIA_IMSIC_BASE + (__nr_harts) * KVM_DEV_RISCV_IMSIC_SIZE)
>
> AIA_APLIC_ADDR() probably doesn't need to take nr_harts since it's
> always called with aia_nr_harts. So it could just be defined as
>
>  #define AIA_APLIC_ADDR (AIA_IMSIC_BASE + AIA_IMSIC_SIZE)

Okay, I will update.

>
> > +
> > +static void aia__generate_fdt_node(void *fdt, struct kvm *kvm)
> > +{
> > +     u32 i;
> > +     char name[64];
> > +     u32 reg_cells[4], *irq_cells;
> > +
> > +     irq_cells = calloc(aia_nr_harts * 2, sizeof(u32));
> > +     if (!irq_cells)
> > +             die("Failed to alloc irq_cells");
> > +
> > +     sprintf(name, "imsics@%08x", (u32)AIA_IMSIC_BASE);
> > +     _FDT(fdt_begin_node(fdt, name));
> > +     _FDT(fdt_property_string(fdt, "compatible", "riscv,imsics"));
> > +     reg_cells[0] = 0;
> > +     reg_cells[1] = cpu_to_fdt32(AIA_IMSIC_BASE);
> > +     reg_cells[2] = 0;
> > +     reg_cells[3] = cpu_to_fdt32(AIA_IMSIC_SIZE);
> > +     _FDT(fdt_property(fdt, "reg", reg_cells, sizeof(reg_cells)));
> > +     _FDT(fdt_property_cell(fdt, "#interrupt-cells", 0));
> > +     _FDT(fdt_property(fdt, "interrupt-controller", NULL, 0));
> > +     _FDT(fdt_property(fdt, "msi-controller", NULL, 0));
> > +     _FDT(fdt_property_cell(fdt, "riscv,num-ids", aia_nr_ids));
> > +     _FDT(fdt_property_cell(fdt, "phandle", PHANDLE_AIA_IMSIC));
> > +     for (i = 0; i < aia_nr_harts; i++) {
> > +             irq_cells[2*i + 0] = cpu_to_fdt32(PHANDLE_CPU_INTC_BASE + i);
> > +             irq_cells[2*i + 1] = cpu_to_fdt32(9);
> > +     }
> > +     _FDT(fdt_property(fdt, "interrupts-extended", irq_cells,
> > +                       sizeof(u32) * aia_nr_harts * 2));
> > +     _FDT(fdt_end_node(fdt));
> > +
> > +     free(irq_cells);
> > +
> > +     /* Skip APLIC node if we have no interrupt sources */
> > +     if (!aia_nr_sources)
> > +             return;
> > +
> > +     sprintf(name, "aplic@%08x", (u32)AIA_APLIC_ADDR(aia_nr_harts));
> > +     _FDT(fdt_begin_node(fdt, name));
> > +     _FDT(fdt_property_string(fdt, "compatible", "riscv,aplic"));
> > +     reg_cells[0] = 0;
> > +     reg_cells[1] = cpu_to_fdt32(AIA_APLIC_ADDR(aia_nr_harts));
> > +     reg_cells[2] = 0;
> > +     reg_cells[3] = cpu_to_fdt32(KVM_DEV_RISCV_APLIC_SIZE);
> > +     _FDT(fdt_property(fdt, "reg", reg_cells, sizeof(reg_cells)));
> > +     _FDT(fdt_property_cell(fdt, "#interrupt-cells", 2));
> > +     _FDT(fdt_property(fdt, "interrupt-controller", NULL, 0));
> > +     _FDT(fdt_property_cell(fdt, "riscv,num-sources", aia_nr_sources));
> > +     _FDT(fdt_property_cell(fdt, "phandle", PHANDLE_AIA_APLIC));
> > +     _FDT(fdt_property_cell(fdt, "msi-parent", PHANDLE_AIA_IMSIC));
> > +     _FDT(fdt_end_node(fdt));
> > +}
> > +
> > +static int aia__irq_routing_init(struct kvm *kvm)
> > +{
> > +     int r;
> > +     int irqlines = aia_nr_sources + 1;
> > +
> > +     /* Skip this if we have no interrupt sources */
> > +     if (!aia_nr_sources)
> > +             return 0;
> > +
> > +     /*
> > +      * This describes the default routing that the kernel uses without
> > +      * any routing explicitly set up via KVM_SET_GSI_ROUTING. So we
> > +      * don't need to commit these setting right now. The first actual
> > +      * user (MSI routing) will engage these mappings then.
> > +      */
> > +     for (next_gsi = 0; next_gsi < irqlines; next_gsi++) {
> > +             r = irq__allocate_routing_entry();
> > +             if (r)
> > +                     return r;
> > +
> > +             irq_routing->entries[irq_routing->nr++] =
> > +                     (struct kvm_irq_routing_entry) {
> > +                             .gsi = next_gsi,
> > +                             .type = KVM_IRQ_ROUTING_IRQCHIP,
> > +                             .u.irqchip.irqchip = IRQCHIP_AIA_NR,
> > +                             .u.irqchip.pin = next_gsi,
> > +             };
> > +     }
> > +
> > +     return 0;
> > +}
> > +
> > +static int aia__init(struct kvm *kvm)
> > +{
> > +     int i, ret;
> > +     u64 aia_addr = 0;
> > +     struct kvm_device_attr aia_addr_attr = {
> > +             .group  = KVM_DEV_RISCV_AIA_GRP_ADDR,
> > +             .addr   = (u64)(unsigned long)&aia_addr,
> > +     };
> > +     struct kvm_device_attr aia_init_attr = {
> > +             .group  = KVM_DEV_RISCV_AIA_GRP_CTRL,
> > +             .attr   = KVM_DEV_RISCV_AIA_CTRL_INIT,
> > +     };
> > +
> > +     /* Setup global device attribute variables */
> > +     aia_mode_attr.addr = (u64)(unsigned long)&aia_mode;
> > +     aia_nr_ids_attr.addr = (u64)(unsigned long)&aia_nr_ids;
> > +     aia_nr_sources_attr.addr = (u64)(unsigned long)&aia_nr_sources;
> > +     aia_hart_bits_attr.addr = (u64)(unsigned long)&aia_hart_bits;
> > +
> > +     /* Do nothing if AIA device not created */
> > +     if (aia_fd < 0)
> > +             return 0;
> > +
> > +     /* Set/Get AIA device config parameters */
> > +     ret = ioctl(aia_fd, KVM_GET_DEVICE_ATTR, &aia_mode_attr);
> > +     if (ret)
> > +             return ret;
> > +     ret = ioctl(aia_fd, KVM_GET_DEVICE_ATTR, &aia_nr_ids_attr);
> > +     if (ret)
> > +             return ret;
> > +     aia_nr_sources = irq__get_nr_allocated_lines();
> > +     ret = ioctl(aia_fd, KVM_SET_DEVICE_ATTR, &aia_nr_sources_attr);
> > +     if (ret)
> > +             return ret;
> > +     aia_hart_bits = fls_long(kvm->nrcpus);
> > +     ret = ioctl(aia_fd, KVM_SET_DEVICE_ATTR, &aia_hart_bits_attr);
> > +     if (ret)
> > +             return ret;
> > +
> > +     /* Save number of HARTs for FDT generation */
> > +     aia_nr_harts = kvm->nrcpus;
> > +
> > +     /* Set AIA device addresses */
> > +     aia_addr = AIA_APLIC_ADDR(aia_nr_harts);
> > +     aia_addr_attr.attr = KVM_DEV_RISCV_AIA_ADDR_APLIC;
> > +     ret = ioctl(aia_fd, KVM_SET_DEVICE_ATTR, &aia_addr_attr);
> > +     if (ret)
> > +             return ret;
> > +     for (i = 0; i < kvm->nrcpus; i++) {
> > +             aia_addr = AIA_IMSIC_ADDR(i);
> > +             aia_addr_attr.attr = KVM_DEV_RISCV_AIA_ADDR_IMSIC(i);
> > +             ret = ioctl(aia_fd, KVM_SET_DEVICE_ATTR, &aia_addr_attr);
> > +             if (ret)
> > +                     return ret;
> > +     }
> > +
> > +     /* Setup default IRQ routing */
> > +     aia__irq_routing_init(kvm);
> > +
> > +     /* Initialize the AIA device */
> > +     ret = ioctl(aia_fd, KVM_SET_DEVICE_ATTR, &aia_init_attr);
> > +     if (ret)
> > +             return ret;
> > +
> > +     /* Mark IRQFD as ready */
> > +     riscv_irqchip_irqfd_ready = true;
> > +
> > +     return 0;
> > +}
> > +late_init(aia__init);
> > +
> > +void aia__create(struct kvm *kvm)
> > +{
> > +     int err;
> > +     struct kvm_create_device aia_device = {
> > +             .type = KVM_DEV_TYPE_RISCV_AIA,
> > +             .flags = 0,
> > +     };
> > +
> > +     if (kvm->cfg.arch.ext_disabled[KVM_RISCV_ISA_EXT_SSAIA])
> > +             return;
> > +
> > +     err = ioctl(kvm->vm_fd, KVM_CREATE_DEVICE, &aia_device);
> > +     if (err)
> > +             return;
> > +     aia_fd = aia_device.fd;
> > +
> > +     riscv_irqchip = IRQCHIP_AIA;
> > +     riscv_irqchip_inkernel = true;
> > +     riscv_irqchip_trigger = NULL;
> > +     riscv_irqchip_generate_fdt_node = aia__generate_fdt_node;
> > +     riscv_irqchip_phandle = PHANDLE_AIA_APLIC;
> > +     riscv_irqchip_msi_phandle = PHANDLE_AIA_IMSIC;
> > +     riscv_irqchip_line_sensing = true;
> > +}
> > diff --git a/riscv/include/kvm/fdt-arch.h b/riscv/include/kvm/fdt-arch.h
> > index f7548e8..d88b832 100644
> > --- a/riscv/include/kvm/fdt-arch.h
> > +++ b/riscv/include/kvm/fdt-arch.h
> > @@ -1,7 +1,13 @@
> >  #ifndef KVM__KVM_FDT_H
> >  #define KVM__KVM_FDT_H
> >
> > -enum phandles {PHANDLE_RESERVED = 0, PHANDLE_PLIC, PHANDLES_MAX};
> > +enum phandles {
> > +     PHANDLE_RESERVED = 0,
> > +     PHANDLE_PLIC,
> > +     PHANDLE_AIA_APLIC,
> > +     PHANDLE_AIA_IMSIC,
> > +     PHANDLES_MAX
> > +};
> >
> >  #define PHANDLE_CPU_INTC_BASE        PHANDLES_MAX
> >
> > diff --git a/riscv/include/kvm/kvm-arch.h b/riscv/include/kvm/kvm-arch.h
> > index 1a8af6a..9f2159f 100644
> > --- a/riscv/include/kvm/kvm-arch.h
> > +++ b/riscv/include/kvm/kvm-arch.h
> > @@ -100,6 +100,8 @@ extern u32 riscv_irqchip_msi_phandle;
> >  extern bool riscv_irqchip_line_sensing;
> >  extern bool riscv_irqchip_irqfd_ready;
> >
> > +void aia__create(struct kvm *kvm);
> > +
>
> nit: I'd remove the blank line between *__create functions.

Okay, I will update.

>
> >  void plic__create(struct kvm *kvm);
> >
> >  void pci__generate_fdt_nodes(void *fdt);
> > diff --git a/riscv/irq.c b/riscv/irq.c
> > index e6c0939..be3e7ac 100644
> > --- a/riscv/irq.c
> > +++ b/riscv/irq.c
> > @@ -135,6 +135,9 @@ void riscv__generate_irq_prop(void *fdt, u8 irq, enum irq_type irq_type)
> >
> >  void riscv__irqchip_create(struct kvm *kvm)
> >  {
> > +     /* Try AIA in-kernel irqchip. */
> > +     aia__create(kvm);
> > +
> >       /* Try PLIC irqchip */
> >       plic__create(kvm);
>
> I realize plic__create() just returns if aia__create() successful set up
> the irqchip, but structuring this like
>
>  ret = aia__create(kvm);
>  if (!ret)
>     ret = plic__create(kvm);
>  if (!ret)
>     die(...);
>
> Might be a little easier to for future readers of the code to grok on
> first read.

Okay, I will update it so that it is obvious to readers that __create()
functions are tried one after another until one suceeds.

>
>
> Either way,
>
> Reviewed-by: Andrew Jones <ajones@ventanamicro.com>
>
> Thanks,
> drew

Regards,
Anup

[kvmtool PATCH v2 6/6] riscv: Fix guest/init linkage for multilib toolchain

[Anup Patel]

For RISC-V multilib toolchains, we must specify -mabi and -march
options when linking guest/init.

Fixes: 2e99678314c2 ("riscv: Initial skeletal support")
Signed-off-by: Anup Patel <apatel@ventanamicro.com>
---
 Makefile | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/Makefile b/Makefile
index acd5ffd..d84dc8e 100644
--- a/Makefile
+++ b/Makefile
@@ -223,9 +223,11 @@ ifeq ($(ARCH),riscv)
 	OBJS		+= riscv/aia.o
 	ifeq ($(RISCV_XLEN),32)
 		CFLAGS	+= -mabi=ilp32d -march=rv32gc
+		GUEST_INIT_FLAGS += -mabi=ilp32d -march=rv32gc
 	endif
 	ifeq ($(RISCV_XLEN),64)
 		CFLAGS	+= -mabi=lp64d -march=rv64gc
+		GUEST_INIT_FLAGS += -mabi=lp64d -march=rv64gc
 	endif
 
 	ARCH_WANT_LIBFDT := y
-- 
2.34.1

Re: [kvmtool PATCH v2 6/6] riscv: Fix guest/init linkage for multilib toolchain

[Andrew Jones]

On Mon, Sep 18, 2023 at 06:27:30PM +0530, Anup Patel wrote:
> For RISC-V multilib toolchains, we must specify -mabi and -march
> options when linking guest/init.
> 
> Fixes: 2e99678314c2 ("riscv: Initial skeletal support")
> Signed-off-by: Anup Patel <apatel@ventanamicro.com>
> ---
>  Makefile | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/Makefile b/Makefile
> index acd5ffd..d84dc8e 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -223,9 +223,11 @@ ifeq ($(ARCH),riscv)
>  	OBJS		+= riscv/aia.o
>  	ifeq ($(RISCV_XLEN),32)
>  		CFLAGS	+= -mabi=ilp32d -march=rv32gc
> +		GUEST_INIT_FLAGS += -mabi=ilp32d -march=rv32gc
>  	endif
>  	ifeq ($(RISCV_XLEN),64)
>  		CFLAGS	+= -mabi=lp64d -march=rv64gc
> +		GUEST_INIT_FLAGS += -mabi=lp64d -march=rv64gc
>  	endif
>  
>  	ARCH_WANT_LIBFDT := y
> -- 
> 2.34.1
>

Looks good and like something that could be posted independently of this
series.

Reviewed-by: Andrew Jones <ajones@ventanamicro.com>

Thanks,
drew

Re: [kvmtool PATCH v2 0/6] RISC-V AIA irqchip and Svnapot support

[Anup Patel]

Hi Will,

On Mon, Sep 18, 2023 at 6:27 PM Anup Patel <apatel@ventanamicro.com> wrote:
>
> The latest KVM in Linux-6.5 has support for:
> 1) Svnapot ISA extension support
> 2) AIA in-kernel irqchip support
>
> This series adds corresponding changes in KVMTOOL to use the above
> mentioned features for Guest/VM.
>
> These patches can also be found in the riscv_aia_v2 branch at:
> https://github.com/avpatel/kvmtool.git
>
> Changes since v1:
>  - Rebased on commit 9cb1b46cb765972326a46bdba867d441a842af56
>  - Updated PATCH1 to sync header with released Linux-6.5
>
> Anup Patel (6):
>   Sync-up header with Linux-6.5 for KVM RISC-V
>   riscv: Add Svnapot extension support
>   riscv: Make irqchip support pluggable
>   riscv: Add IRQFD support for in-kernel AIA irqchip
>   riscv: Use AIA in-kernel irqchip whenever KVM RISC-V supports
>   riscv: Fix guest/init linkage for multilib toolchain

Friendly ping ?

>
>  Makefile                            |   3 +
>  include/linux/kvm.h                 |   6 +-
>  riscv/aia.c                         | 227 ++++++++++++++++++++++++++++
>  riscv/fdt.c                         |  15 +-
>  riscv/include/asm/kvm.h             |  81 ++++++++++
>  riscv/include/kvm/fdt-arch.h        |   8 +-
>  riscv/include/kvm/kvm-arch.h        |  38 ++++-
>  riscv/include/kvm/kvm-config-arch.h |   3 +
>  riscv/irq.c                         | 138 ++++++++++++++++-
>  riscv/kvm.c                         |   2 +
>  riscv/pci.c                         |  32 ++--
>  riscv/plic.c                        |  61 ++++----
>  12 files changed, 563 insertions(+), 51 deletions(-)
>  create mode 100644 riscv/aia.c
>
> --
> 2.34.1
>

Regards,
Anup

Re: [kvmtool PATCH v2 0/6] RISC-V AIA irqchip and Svnapot support

[Will Deacon]

On Thu, Oct 12, 2023 at 09:50:29AM +0530, Anup Patel wrote:
> On Mon, Sep 18, 2023 at 6:27 PM Anup Patel <apatel@ventanamicro.com> wrote:
> >
> > The latest KVM in Linux-6.5 has support for:
> > 1) Svnapot ISA extension support
> > 2) AIA in-kernel irqchip support
> >
> > This series adds corresponding changes in KVMTOOL to use the above
> > mentioned features for Guest/VM.
> >
> > These patches can also be found in the riscv_aia_v2 branch at:
> > https://github.com/avpatel/kvmtool.git
> >
> > Changes since v1:
> >  - Rebased on commit 9cb1b46cb765972326a46bdba867d441a842af56
> >  - Updated PATCH1 to sync header with released Linux-6.5
> >
> > Anup Patel (6):
> >   Sync-up header with Linux-6.5 for KVM RISC-V
> >   riscv: Add Svnapot extension support
> >   riscv: Make irqchip support pluggable
> >   riscv: Add IRQFD support for in-kernel AIA irqchip
> >   riscv: Use AIA in-kernel irqchip whenever KVM RISC-V supports
> >   riscv: Fix guest/init linkage for multilib toolchain
> 
> Friendly ping ?

There are a bunch of open review comments from Drew that need to be
addressed in a subsequent version.

Will

Re: [kvmtool PATCH v2 0/6] RISC-V AIA irqchip and Svnapot support

[Anup Patel]

On Tue, Nov 7, 2023 at 4:41 PM Will Deacon <will@kernel.org> wrote:
>
> On Thu, Oct 12, 2023 at 09:50:29AM +0530, Anup Patel wrote:
> > On Mon, Sep 18, 2023 at 6:27 PM Anup Patel <apatel@ventanamicro.com> wrote:
> > >
> > > The latest KVM in Linux-6.5 has support for:
> > > 1) Svnapot ISA extension support
> > > 2) AIA in-kernel irqchip support
> > >
> > > This series adds corresponding changes in KVMTOOL to use the above
> > > mentioned features for Guest/VM.
> > >
> > > These patches can also be found in the riscv_aia_v2 branch at:
> > > https://github.com/avpatel/kvmtool.git
> > >
> > > Changes since v1:
> > >  - Rebased on commit 9cb1b46cb765972326a46bdba867d441a842af56
> > >  - Updated PATCH1 to sync header with released Linux-6.5
> > >
> > > Anup Patel (6):
> > >   Sync-up header with Linux-6.5 for KVM RISC-V
> > >   riscv: Add Svnapot extension support
> > >   riscv: Make irqchip support pluggable
> > >   riscv: Add IRQFD support for in-kernel AIA irqchip
> > >   riscv: Use AIA in-kernel irqchip whenever KVM RISC-V supports
> > >   riscv: Fix guest/init linkage for multilib toolchain
> >
> > Friendly ping ?
>
> There are a bunch of open review comments from Drew that need to be
> addressed in a subsequent version.

I have sent-out v3 with Drew's comments addressed.

Thanks,
Anup