2023-01-28 07:27:53

by Anup Patel

[permalink] [raw]
Subject: [PATCH v2 0/7] RISC-V KVM virtualize AIA CSRs

The RISC-V AIA specification is now frozen as-per the RISC-V international
process. The latest frozen specifcation can be found at:
https://github.com/riscv/riscv-aia/releases/download/1.0-RC1/riscv-interrupts-1.0-RC1.pdf

This series implements first phase of AIA virtualization which targets
virtualizing AIA CSRs. This also provides a foundation for the second
phase of AIA virtualization which will target in-kernel AIA irqchip
(including both IMSIC and APLIC).

The first two patches are shared with the "Linux RISC-V AIA Support"
series which adds AIA driver support.

To test this series, use AIA drivers from the "Linux RISC-V AIA Support"
series and use KVMTOOL from the riscv_aia_v1 branch at:
https://github.com/avpatel/kvmtool.git

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

Changes since v1:
- Addressed from Drew and Conor in PATCH1
- Use alphabetical ordering for SMAIA and SSAIA enum in PATCH2
- Use GENMASK() in PATCH3

Anup Patel (7):
RISC-V: Add AIA related CSR defines
RISC-V: Detect AIA CSRs from ISA string
RISC-V: KVM: Drop the _MASK suffix from hgatp.VMID mask defines
RISC-V: KVM: Initial skeletal support for AIA
RISC-V: KVM: Add ONE_REG interface for AIA CSRs
RISC-V: KVM: Virtualize per-HART AIA CSRs
RISC-V: KVM: Implement guest external interrupt line management

arch/riscv/include/asm/csr.h | 107 ++++-
arch/riscv/include/asm/hwcap.h | 8 +
arch/riscv/include/asm/kvm_aia.h | 137 +++++++
arch/riscv/include/asm/kvm_host.h | 14 +-
arch/riscv/include/uapi/asm/kvm.h | 22 +-
arch/riscv/kernel/cpu.c | 2 +
arch/riscv/kernel/cpufeature.c | 2 +
arch/riscv/kvm/Makefile | 1 +
arch/riscv/kvm/aia.c | 624 ++++++++++++++++++++++++++++++
arch/riscv/kvm/main.c | 14 +
arch/riscv/kvm/mmu.c | 3 +-
arch/riscv/kvm/vcpu.c | 185 +++++++--
arch/riscv/kvm/vcpu_insn.c | 4 +-
arch/riscv/kvm/vm.c | 4 +
arch/riscv/kvm/vmid.c | 4 +-
15 files changed, 1075 insertions(+), 56 deletions(-)
create mode 100644 arch/riscv/include/asm/kvm_aia.h
create mode 100644 arch/riscv/kvm/aia.c

--
2.34.1



2023-01-28 07:28:01

by Anup Patel

[permalink] [raw]
Subject: [PATCH v2 1/7] RISC-V: Add AIA related CSR defines

The RISC-V AIA specification improves handling per-HART local interrupts
in a backward compatible manner. This patch adds defines for new RISC-V
AIA CSRs.

Signed-off-by: Anup Patel <[email protected]>
Reviewed-by: Conor Dooley <[email protected]>
Reviewed-by: Andrew Jones <[email protected]>
---
arch/riscv/include/asm/csr.h | 95 +++++++++++++++++++++++++++++++++++-
1 file changed, 94 insertions(+), 1 deletion(-)

diff --git a/arch/riscv/include/asm/csr.h b/arch/riscv/include/asm/csr.h
index 0e571f6483d9..3c8d68152bce 100644
--- a/arch/riscv/include/asm/csr.h
+++ b/arch/riscv/include/asm/csr.h
@@ -7,7 +7,7 @@
#define _ASM_RISCV_CSR_H

#include <asm/asm.h>
-#include <linux/const.h>
+#include <linux/bits.h>

/* Status register flags */
#define SR_SIE _AC(0x00000002, UL) /* Supervisor Interrupt Enable */
@@ -73,7 +73,10 @@
#define IRQ_S_EXT 9
#define IRQ_VS_EXT 10
#define IRQ_M_EXT 11
+#define IRQ_S_GEXT 12
#define IRQ_PMU_OVF 13
+#define IRQ_LOCAL_MAX (IRQ_PMU_OVF + 1)
+#define IRQ_LOCAL_MASK GENMASK((IRQ_LOCAL_MAX - 1), 0)

/* Exception causes */
#define EXC_INST_MISALIGNED 0
@@ -156,6 +159,27 @@
(_AC(1, UL) << IRQ_S_TIMER) | \
(_AC(1, UL) << IRQ_S_EXT))

+/* AIA CSR bits */
+#define TOPI_IID_SHIFT 16
+#define TOPI_IID_MASK GENMASK(11, 0)
+#define TOPI_IPRIO_MASK GENMASK(7, 0)
+#define TOPI_IPRIO_BITS 8
+
+#define TOPEI_ID_SHIFT 16
+#define TOPEI_ID_MASK GENMASK(10, 0)
+#define TOPEI_PRIO_MASK GENMASK(10, 0)
+
+#define ISELECT_IPRIO0 0x30
+#define ISELECT_IPRIO15 0x3f
+#define ISELECT_MASK GENMASK(8, 0)
+
+#define HVICTL_VTI BIT(30)
+#define HVICTL_IID GENMASK(27, 16)
+#define HVICTL_IID_SHIFT 16
+#define HVICTL_DPR BIT(9)
+#define HVICTL_IPRIOM BIT(8)
+#define HVICTL_IPRIO GENMASK(7, 0)
+
/* xENVCFG flags */
#define ENVCFG_STCE (_AC(1, ULL) << 63)
#define ENVCFG_PBMTE (_AC(1, ULL) << 62)
@@ -250,6 +274,18 @@
#define CSR_STIMECMP 0x14D
#define CSR_STIMECMPH 0x15D

+/* Supervisor-Level Window to Indirectly Accessed Registers (AIA) */
+#define CSR_SISELECT 0x150
+#define CSR_SIREG 0x151
+
+/* Supervisor-Level Interrupts (AIA) */
+#define CSR_STOPEI 0x15c
+#define CSR_STOPI 0xdb0
+
+/* Supervisor-Level High-Half CSRs (AIA) */
+#define CSR_SIEH 0x114
+#define CSR_SIPH 0x154
+
#define CSR_VSSTATUS 0x200
#define CSR_VSIE 0x204
#define CSR_VSTVEC 0x205
@@ -279,8 +315,32 @@
#define CSR_HGATP 0x680
#define CSR_HGEIP 0xe12

+/* Virtual Interrupts and Interrupt Priorities (H-extension with AIA) */
+#define CSR_HVIEN 0x608
+#define CSR_HVICTL 0x609
+#define CSR_HVIPRIO1 0x646
+#define CSR_HVIPRIO2 0x647
+
+/* VS-Level Window to Indirectly Accessed Registers (H-extension with AIA) */
+#define CSR_VSISELECT 0x250
+#define CSR_VSIREG 0x251
+
+/* VS-Level Interrupts (H-extension with AIA) */
+#define CSR_VSTOPEI 0x25c
+#define CSR_VSTOPI 0xeb0
+
+/* Hypervisor and VS-Level High-Half CSRs (H-extension with AIA) */
+#define CSR_HIDELEGH 0x613
+#define CSR_HVIENH 0x618
+#define CSR_HVIPH 0x655
+#define CSR_HVIPRIO1H 0x656
+#define CSR_HVIPRIO2H 0x657
+#define CSR_VSIEH 0x214
+#define CSR_VSIPH 0x254
+
#define CSR_MSTATUS 0x300
#define CSR_MISA 0x301
+#define CSR_MIDELEG 0x303
#define CSR_MIE 0x304
#define CSR_MTVEC 0x305
#define CSR_MENVCFG 0x30a
@@ -297,6 +357,25 @@
#define CSR_MIMPID 0xf13
#define CSR_MHARTID 0xf14

+/* Machine-Level Window to Indirectly Accessed Registers (AIA) */
+#define CSR_MISELECT 0x350
+#define CSR_MIREG 0x351
+
+/* Machine-Level Interrupts (AIA) */
+#define CSR_MTOPEI 0x35c
+#define CSR_MTOPI 0xfb0
+
+/* Virtual Interrupts for Supervisor Level (AIA) */
+#define CSR_MVIEN 0x308
+#define CSR_MVIP 0x309
+
+/* Machine-Level High-Half CSRs (AIA) */
+#define CSR_MIDELEGH 0x313
+#define CSR_MIEH 0x314
+#define CSR_MVIENH 0x318
+#define CSR_MVIPH 0x319
+#define CSR_MIPH 0x354
+
#ifdef CONFIG_RISCV_M_MODE
# define CSR_STATUS CSR_MSTATUS
# define CSR_IE CSR_MIE
@@ -307,6 +386,13 @@
# define CSR_TVAL CSR_MTVAL
# define CSR_IP CSR_MIP

+# define CSR_IEH CSR_MIEH
+# define CSR_ISELECT CSR_MISELECT
+# define CSR_IREG CSR_MIREG
+# define CSR_IPH CSR_MIPH
+# define CSR_TOPEI CSR_MTOPEI
+# define CSR_TOPI CSR_MTOPI
+
# define SR_IE SR_MIE
# define SR_PIE SR_MPIE
# define SR_PP SR_MPP
@@ -324,6 +410,13 @@
# define CSR_TVAL CSR_STVAL
# define CSR_IP CSR_SIP

+# define CSR_IEH CSR_SIEH
+# define CSR_ISELECT CSR_SISELECT
+# define CSR_IREG CSR_SIREG
+# define CSR_IPH CSR_SIPH
+# define CSR_TOPEI CSR_STOPEI
+# define CSR_TOPI CSR_STOPI
+
# define SR_IE SR_SIE
# define SR_PIE SR_SPIE
# define SR_PP SR_SPP
--
2.34.1


2023-01-28 07:28:05

by Anup Patel

[permalink] [raw]
Subject: [PATCH v2 2/7] RISC-V: Detect AIA CSRs from ISA string

We have two extension names for AIA ISA support: Smaia (M-mode AIA CSRs)
and Ssaia (S-mode AIA CSRs).

We extend the ISA string parsing to detect Smaia and Ssaia extensions.

Signed-off-by: Anup Patel <[email protected]>
Reviewed-by: Andrew Jones <[email protected]>
---
arch/riscv/include/asm/hwcap.h | 2 ++
arch/riscv/kernel/cpu.c | 2 ++
arch/riscv/kernel/cpufeature.c | 2 ++
3 files changed, 6 insertions(+)

diff --git a/arch/riscv/include/asm/hwcap.h b/arch/riscv/include/asm/hwcap.h
index 86328e3acb02..341ef30a3718 100644
--- a/arch/riscv/include/asm/hwcap.h
+++ b/arch/riscv/include/asm/hwcap.h
@@ -59,6 +59,8 @@ enum riscv_isa_ext_id {
RISCV_ISA_EXT_ZIHINTPAUSE,
RISCV_ISA_EXT_SSTC,
RISCV_ISA_EXT_SVINVAL,
+ RISCV_ISA_EXT_SMAIA,
+ RISCV_ISA_EXT_SSAIA,
RISCV_ISA_EXT_ID_MAX
};
static_assert(RISCV_ISA_EXT_ID_MAX <= RISCV_ISA_EXT_MAX);
diff --git a/arch/riscv/kernel/cpu.c b/arch/riscv/kernel/cpu.c
index 1b9a5a66e55a..a215ec929160 100644
--- a/arch/riscv/kernel/cpu.c
+++ b/arch/riscv/kernel/cpu.c
@@ -162,6 +162,8 @@ arch_initcall(riscv_cpuinfo_init);
* extensions by an underscore.
*/
static struct riscv_isa_ext_data isa_ext_arr[] = {
+ __RISCV_ISA_EXT_DATA(smaia, RISCV_ISA_EXT_SMAIA),
+ __RISCV_ISA_EXT_DATA(ssaia, RISCV_ISA_EXT_SSAIA),
__RISCV_ISA_EXT_DATA(sscofpmf, RISCV_ISA_EXT_SSCOFPMF),
__RISCV_ISA_EXT_DATA(sstc, RISCV_ISA_EXT_SSTC),
__RISCV_ISA_EXT_DATA(svinval, RISCV_ISA_EXT_SVINVAL),
diff --git a/arch/riscv/kernel/cpufeature.c b/arch/riscv/kernel/cpufeature.c
index 93e45560af30..3c5b51f519d5 100644
--- a/arch/riscv/kernel/cpufeature.c
+++ b/arch/riscv/kernel/cpufeature.c
@@ -228,6 +228,8 @@ void __init riscv_fill_hwcap(void)
SET_ISA_EXT_MAP("zihintpause", RISCV_ISA_EXT_ZIHINTPAUSE);
SET_ISA_EXT_MAP("sstc", RISCV_ISA_EXT_SSTC);
SET_ISA_EXT_MAP("svinval", RISCV_ISA_EXT_SVINVAL);
+ SET_ISA_EXT_MAP("smaia", RISCV_ISA_EXT_SMAIA);
+ SET_ISA_EXT_MAP("ssaia", RISCV_ISA_EXT_SSAIA);
}
#undef SET_ISA_EXT_MAP
}
--
2.34.1


2023-01-28 07:28:15

by Anup Patel

[permalink] [raw]
Subject: [PATCH v2 3/7] RISC-V: KVM: Drop the _MASK suffix from hgatp.VMID mask defines

The hgatp.VMID mask defines are used before shifting when extracting
VMID value from hgatp CSR value so based on the convention followed
in the other parts of asm/csr.h, the hgatp.VMID mask defines should
not have a _MASK suffix.

While we are here, let's use GENMASK() for hgatp.VMID and hgatp.PPN.

Signed-off-by: Anup Patel <[email protected]>
Reviewed-by: Andrew Jones <[email protected]>
---
arch/riscv/include/asm/csr.h | 12 ++++++------
arch/riscv/kvm/mmu.c | 3 +--
arch/riscv/kvm/vmid.c | 4 ++--
3 files changed, 9 insertions(+), 10 deletions(-)

diff --git a/arch/riscv/include/asm/csr.h b/arch/riscv/include/asm/csr.h
index 3c8d68152bce..3176355cf4e9 100644
--- a/arch/riscv/include/asm/csr.h
+++ b/arch/riscv/include/asm/csr.h
@@ -131,25 +131,25 @@

#define HGATP32_MODE_SHIFT 31
#define HGATP32_VMID_SHIFT 22
-#define HGATP32_VMID_MASK _AC(0x1FC00000, UL)
-#define HGATP32_PPN _AC(0x003FFFFF, UL)
+#define HGATP32_VMID GENMASK(28, 22)
+#define HGATP32_PPN GENMASK(21, 0)

#define HGATP64_MODE_SHIFT 60
#define HGATP64_VMID_SHIFT 44
-#define HGATP64_VMID_MASK _AC(0x03FFF00000000000, UL)
-#define HGATP64_PPN _AC(0x00000FFFFFFFFFFF, UL)
+#define HGATP64_VMID GENMASK(57, 44)
+#define HGATP64_PPN GENMASK(43, 0)

#define HGATP_PAGE_SHIFT 12

#ifdef CONFIG_64BIT
#define HGATP_PPN HGATP64_PPN
#define HGATP_VMID_SHIFT HGATP64_VMID_SHIFT
-#define HGATP_VMID_MASK HGATP64_VMID_MASK
+#define HGATP_VMID HGATP64_VMID
#define HGATP_MODE_SHIFT HGATP64_MODE_SHIFT
#else
#define HGATP_PPN HGATP32_PPN
#define HGATP_VMID_SHIFT HGATP32_VMID_SHIFT
-#define HGATP_VMID_MASK HGATP32_VMID_MASK
+#define HGATP_VMID HGATP32_VMID
#define HGATP_MODE_SHIFT HGATP32_MODE_SHIFT
#endif

diff --git a/arch/riscv/kvm/mmu.c b/arch/riscv/kvm/mmu.c
index dbc4ca060174..829a7065ae01 100644
--- a/arch/riscv/kvm/mmu.c
+++ b/arch/riscv/kvm/mmu.c
@@ -748,8 +748,7 @@ void kvm_riscv_gstage_update_hgatp(struct kvm_vcpu *vcpu)
unsigned long hgatp = gstage_mode;
struct kvm_arch *k = &vcpu->kvm->arch;

- hgatp |= (READ_ONCE(k->vmid.vmid) << HGATP_VMID_SHIFT) &
- HGATP_VMID_MASK;
+ hgatp |= (READ_ONCE(k->vmid.vmid) << HGATP_VMID_SHIFT) & HGATP_VMID;
hgatp |= (k->pgd_phys >> PAGE_SHIFT) & HGATP_PPN;

csr_write(CSR_HGATP, hgatp);
diff --git a/arch/riscv/kvm/vmid.c b/arch/riscv/kvm/vmid.c
index 6cd93995fb65..6f4d4979a759 100644
--- a/arch/riscv/kvm/vmid.c
+++ b/arch/riscv/kvm/vmid.c
@@ -26,9 +26,9 @@ void kvm_riscv_gstage_vmid_detect(void)

/* Figure-out number of VMID bits in HW */
old = csr_read(CSR_HGATP);
- csr_write(CSR_HGATP, old | HGATP_VMID_MASK);
+ csr_write(CSR_HGATP, old | HGATP_VMID);
vmid_bits = csr_read(CSR_HGATP);
- vmid_bits = (vmid_bits & HGATP_VMID_MASK) >> HGATP_VMID_SHIFT;
+ vmid_bits = (vmid_bits & HGATP_VMID) >> HGATP_VMID_SHIFT;
vmid_bits = fls_long(vmid_bits);
csr_write(CSR_HGATP, old);

--
2.34.1


2023-01-28 07:28:28

by Anup Patel

[permalink] [raw]
Subject: [PATCH v2 4/7] RISC-V: KVM: Initial skeletal support for AIA

To incrementally implement AIA support, we first add minimal skeletal
support which only compiles and detects AIA hardware support at the
boot-time but does not provide any functionality.

Signed-off-by: Anup Patel <[email protected]>
---
arch/riscv/include/asm/hwcap.h | 6 ++
arch/riscv/include/asm/kvm_aia.h | 109 ++++++++++++++++++++++++++++++
arch/riscv/include/asm/kvm_host.h | 7 ++
arch/riscv/kvm/Makefile | 1 +
arch/riscv/kvm/aia.c | 66 ++++++++++++++++++
arch/riscv/kvm/main.c | 13 ++++
arch/riscv/kvm/vcpu.c | 40 ++++++++++-
arch/riscv/kvm/vcpu_insn.c | 4 +-
arch/riscv/kvm/vm.c | 4 ++
9 files changed, 246 insertions(+), 4 deletions(-)
create mode 100644 arch/riscv/include/asm/kvm_aia.h
create mode 100644 arch/riscv/kvm/aia.c

diff --git a/arch/riscv/include/asm/hwcap.h b/arch/riscv/include/asm/hwcap.h
index 341ef30a3718..a03d51df517b 100644
--- a/arch/riscv/include/asm/hwcap.h
+++ b/arch/riscv/include/asm/hwcap.h
@@ -65,6 +65,12 @@ enum riscv_isa_ext_id {
};
static_assert(RISCV_ISA_EXT_ID_MAX <= RISCV_ISA_EXT_MAX);

+#ifdef CONFIG_RISCV_M_MODE
+#define RISCV_ISA_EXT_SxAIA RISCV_ISA_EXT_SMAIA
+#else
+#define RISCV_ISA_EXT_SxAIA RISCV_ISA_EXT_SSAIA
+#endif
+
/*
* This enum represents the logical ID for each RISC-V ISA extension static
* keys. We can use static key to optimize code path if some ISA extensions
diff --git a/arch/riscv/include/asm/kvm_aia.h b/arch/riscv/include/asm/kvm_aia.h
new file mode 100644
index 000000000000..258a835d4c32
--- /dev/null
+++ b/arch/riscv/include/asm/kvm_aia.h
@@ -0,0 +1,109 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+/*
+ * Copyright (C) 2021 Western Digital Corporation or its affiliates.
+ * Copyright (C) 2022 Ventana Micro Systems Inc.
+ *
+ * Authors:
+ * Anup Patel <[email protected]>
+ */
+
+#ifndef __KVM_RISCV_AIA_H
+#define __KVM_RISCV_AIA_H
+
+#include <linux/jump_label.h>
+#include <linux/kvm_types.h>
+
+struct kvm_aia {
+ /* In-kernel irqchip created */
+ bool in_kernel;
+
+ /* In-kernel irqchip initialized */
+ bool initialized;
+};
+
+struct kvm_vcpu_aia {
+};
+
+#define kvm_riscv_aia_initialized(k) ((k)->arch.aia.initialized)
+
+#define irqchip_in_kernel(k) ((k)->arch.aia.in_kernel)
+
+DECLARE_STATIC_KEY_FALSE(kvm_riscv_aia_available);
+#define kvm_riscv_aia_available() \
+ static_branch_unlikely(&kvm_riscv_aia_available)
+
+static inline void kvm_riscv_vcpu_aia_flush_interrupts(struct kvm_vcpu *vcpu)
+{
+}
+
+static inline void kvm_riscv_vcpu_aia_sync_interrupts(struct kvm_vcpu *vcpu)
+{
+}
+
+static inline bool kvm_riscv_vcpu_aia_has_interrupts(struct kvm_vcpu *vcpu,
+ u64 mask)
+{
+ return false;
+}
+
+static inline void kvm_riscv_vcpu_aia_update_hvip(struct kvm_vcpu *vcpu)
+{
+}
+
+static inline void kvm_riscv_vcpu_aia_load(struct kvm_vcpu *vcpu, int cpu)
+{
+}
+
+static inline void kvm_riscv_vcpu_aia_put(struct kvm_vcpu *vcpu)
+{
+}
+
+static inline int kvm_riscv_vcpu_aia_get_csr(struct kvm_vcpu *vcpu,
+ unsigned long reg_num,
+ unsigned long *out_val)
+{
+ *out_val = 0;
+ return 0;
+}
+
+static inline int kvm_riscv_vcpu_aia_set_csr(struct kvm_vcpu *vcpu,
+ unsigned long reg_num,
+ unsigned long val)
+{
+ return 0;
+}
+
+#define KVM_RISCV_VCPU_AIA_CSR_FUNCS
+
+static inline int kvm_riscv_vcpu_aia_update(struct kvm_vcpu *vcpu)
+{
+ return 1;
+}
+
+static inline void kvm_riscv_vcpu_aia_reset(struct kvm_vcpu *vcpu)
+{
+}
+
+static inline int kvm_riscv_vcpu_aia_init(struct kvm_vcpu *vcpu)
+{
+ return 0;
+}
+
+static inline void kvm_riscv_vcpu_aia_deinit(struct kvm_vcpu *vcpu)
+{
+}
+
+static inline void kvm_riscv_aia_init_vm(struct kvm *kvm)
+{
+}
+
+static inline void kvm_riscv_aia_destroy_vm(struct kvm *kvm)
+{
+}
+
+void kvm_riscv_aia_enable(void);
+void kvm_riscv_aia_disable(void);
+int kvm_riscv_aia_init(void);
+void kvm_riscv_aia_exit(void);
+
+#endif
diff --git a/arch/riscv/include/asm/kvm_host.h b/arch/riscv/include/asm/kvm_host.h
index 93f43a3e7886..8776e7a465c7 100644
--- a/arch/riscv/include/asm/kvm_host.h
+++ b/arch/riscv/include/asm/kvm_host.h
@@ -14,6 +14,7 @@
#include <linux/kvm_types.h>
#include <linux/spinlock.h>
#include <asm/hwcap.h>
+#include <asm/kvm_aia.h>
#include <asm/kvm_vcpu_fp.h>
#include <asm/kvm_vcpu_insn.h>
#include <asm/kvm_vcpu_sbi.h>
@@ -93,6 +94,9 @@ struct kvm_arch {

/* Guest Timer */
struct kvm_guest_timer timer;
+
+ /* AIA Guest/VM context */
+ struct kvm_aia aia;
};

struct kvm_cpu_trap {
@@ -220,6 +224,9 @@ struct kvm_vcpu_arch {
/* SBI context */
struct kvm_vcpu_sbi_context sbi_context;

+ /* AIA VCPU context */
+ struct kvm_vcpu_aia aia_context;
+
/* Cache pages needed to program page tables with spinlock held */
struct kvm_mmu_memory_cache mmu_page_cache;

diff --git a/arch/riscv/kvm/Makefile b/arch/riscv/kvm/Makefile
index 019df9208bdd..adbc85a94364 100644
--- a/arch/riscv/kvm/Makefile
+++ b/arch/riscv/kvm/Makefile
@@ -25,3 +25,4 @@ kvm-y += vcpu_sbi_base.o
kvm-y += vcpu_sbi_replace.o
kvm-y += vcpu_sbi_hsm.o
kvm-y += vcpu_timer.o
+kvm-y += aia.o
diff --git a/arch/riscv/kvm/aia.c b/arch/riscv/kvm/aia.c
new file mode 100644
index 000000000000..7a633331cd3e
--- /dev/null
+++ b/arch/riscv/kvm/aia.c
@@ -0,0 +1,66 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (C) 2021 Western Digital Corporation or its affiliates.
+ * Copyright (C) 2022 Ventana Micro Systems Inc.
+ *
+ * Authors:
+ * Anup Patel <[email protected]>
+ */
+
+#include <linux/kvm_host.h>
+#include <asm/hwcap.h>
+
+DEFINE_STATIC_KEY_FALSE(kvm_riscv_aia_available);
+
+static void aia_set_hvictl(bool ext_irq_pending)
+{
+ unsigned long hvictl;
+
+ /*
+ * HVICTL.IID == 9 and HVICTL.IPRIO == 0 represents
+ * no interrupt in HVICTL.
+ */
+
+ hvictl = (IRQ_S_EXT << HVICTL_IID_SHIFT) & HVICTL_IID;
+ hvictl |= ext_irq_pending;
+ csr_write(CSR_HVICTL, hvictl);
+}
+
+void kvm_riscv_aia_enable(void)
+{
+ if (!kvm_riscv_aia_available())
+ return;
+
+ aia_set_hvictl(false);
+ csr_write(CSR_HVIPRIO1, 0x0);
+ csr_write(CSR_HVIPRIO2, 0x0);
+#ifdef CONFIG_32BIT
+ csr_write(CSR_HVIPH, 0x0);
+ csr_write(CSR_HIDELEGH, 0x0);
+ csr_write(CSR_HVIPRIO1H, 0x0);
+ csr_write(CSR_HVIPRIO2H, 0x0);
+#endif
+}
+
+void kvm_riscv_aia_disable(void)
+{
+ if (!kvm_riscv_aia_available())
+ return;
+
+ aia_set_hvictl(false);
+}
+
+int kvm_riscv_aia_init(void)
+{
+ if (!riscv_isa_extension_available(NULL, SxAIA))
+ return -ENODEV;
+
+ /* Enable KVM AIA support */
+ static_branch_enable(&kvm_riscv_aia_available);
+
+ return 0;
+}
+
+void kvm_riscv_aia_exit(void)
+{
+}
diff --git a/arch/riscv/kvm/main.c b/arch/riscv/kvm/main.c
index 58c5489d3031..d8ff44eb04ca 100644
--- a/arch/riscv/kvm/main.c
+++ b/arch/riscv/kvm/main.c
@@ -53,11 +53,15 @@ int kvm_arch_hardware_enable(void)

csr_write(CSR_HVIP, 0);

+ kvm_riscv_aia_enable();
+
return 0;
}

void kvm_arch_hardware_disable(void)
{
+ kvm_riscv_aia_disable();
+
/*
* After clearing the hideleg CSR, the host kernel will receive
* spurious interrupts if hvip CSR has pending interrupts and the
@@ -72,6 +76,7 @@ void kvm_arch_hardware_disable(void)

int kvm_arch_init(void *opaque)
{
+ int rc;
const char *str;

if (!riscv_isa_extension_available(NULL, h)) {
@@ -93,6 +98,10 @@ int kvm_arch_init(void *opaque)

kvm_riscv_gstage_vmid_detect();

+ rc = kvm_riscv_aia_init();
+ if (rc && rc != -ENODEV)
+ return rc;
+
kvm_info("hypervisor extension available\n");

switch (kvm_riscv_gstage_mode()) {
@@ -115,11 +124,15 @@ int kvm_arch_init(void *opaque)

kvm_info("VMID %ld bits available\n", kvm_riscv_gstage_vmid_bits());

+ if (kvm_riscv_aia_available())
+ kvm_info("AIA available\n");
+
return 0;
}

void kvm_arch_exit(void)
{
+ kvm_riscv_aia_exit();
}

static int __init riscv_kvm_init(void)
diff --git a/arch/riscv/kvm/vcpu.c b/arch/riscv/kvm/vcpu.c
index 2260adaf2de8..3cf50eadc8ce 100644
--- a/arch/riscv/kvm/vcpu.c
+++ b/arch/riscv/kvm/vcpu.c
@@ -135,6 +135,8 @@ static void kvm_riscv_reset_vcpu(struct kvm_vcpu *vcpu)

kvm_riscv_vcpu_timer_reset(vcpu);

+ kvm_riscv_vcpu_aia_reset(vcpu);
+
WRITE_ONCE(vcpu->arch.irqs_pending, 0);
WRITE_ONCE(vcpu->arch.irqs_pending_mask, 0);

@@ -155,6 +157,7 @@ int kvm_arch_vcpu_precreate(struct kvm *kvm, unsigned int id)

int kvm_arch_vcpu_create(struct kvm_vcpu *vcpu)
{
+ int rc;
struct kvm_cpu_context *cntx;
struct kvm_vcpu_csr *reset_csr = &vcpu->arch.guest_reset_csr;
unsigned long host_isa, i;
@@ -194,6 +197,11 @@ int kvm_arch_vcpu_create(struct kvm_vcpu *vcpu)
/* Setup VCPU timer */
kvm_riscv_vcpu_timer_init(vcpu);

+ /* Setup VCPU AIA */
+ rc = kvm_riscv_vcpu_aia_init(vcpu);
+ if (rc)
+ return rc;
+
/* Reset VCPU */
kvm_riscv_reset_vcpu(vcpu);

@@ -213,6 +221,9 @@ void kvm_arch_vcpu_postcreate(struct kvm_vcpu *vcpu)

void kvm_arch_vcpu_destroy(struct kvm_vcpu *vcpu)
{
+ /* Cleanup VCPU AIA context */
+ kvm_riscv_vcpu_aia_deinit(vcpu);
+
/* Cleanup VCPU timer */
kvm_riscv_vcpu_timer_deinit(vcpu);

@@ -730,6 +741,9 @@ void kvm_riscv_vcpu_flush_interrupts(struct kvm_vcpu *vcpu)
csr->hvip &= ~mask;
csr->hvip |= val;
}
+
+ /* Flush AIA high interrupts */
+ kvm_riscv_vcpu_aia_flush_interrupts(vcpu);
}

void kvm_riscv_vcpu_sync_interrupts(struct kvm_vcpu *vcpu)
@@ -755,6 +769,9 @@ void kvm_riscv_vcpu_sync_interrupts(struct kvm_vcpu *vcpu)
}
}

+ /* Sync-up AIA high interrupts */
+ kvm_riscv_vcpu_aia_sync_interrupts(vcpu);
+
/* Sync-up timer CSRs */
kvm_riscv_vcpu_timer_sync(vcpu);
}
@@ -791,10 +808,15 @@ int kvm_riscv_vcpu_unset_interrupt(struct kvm_vcpu *vcpu, unsigned int irq)

bool kvm_riscv_vcpu_has_interrupts(struct kvm_vcpu *vcpu, unsigned long mask)
{
- unsigned long ie = ((vcpu->arch.guest_csr.vsie & VSIP_VALID_MASK)
- << VSIP_TO_HVIP_SHIFT) & mask;
+ unsigned long ie;
+
+ ie = ((vcpu->arch.guest_csr.vsie & VSIP_VALID_MASK)
+ << VSIP_TO_HVIP_SHIFT) & mask;
+ if (READ_ONCE(vcpu->arch.irqs_pending) & ie)
+ return true;

- return (READ_ONCE(vcpu->arch.irqs_pending) & ie) ? true : false;
+ /* Check AIA high interrupts */
+ return kvm_riscv_vcpu_aia_has_interrupts(vcpu, mask);
}

void kvm_riscv_vcpu_power_off(struct kvm_vcpu *vcpu)
@@ -890,6 +912,8 @@ void kvm_arch_vcpu_load(struct kvm_vcpu *vcpu, int cpu)
kvm_riscv_vcpu_guest_fp_restore(&vcpu->arch.guest_context,
vcpu->arch.isa);

+ kvm_riscv_vcpu_aia_load(vcpu, cpu);
+
vcpu->cpu = cpu;
}

@@ -899,6 +923,8 @@ void kvm_arch_vcpu_put(struct kvm_vcpu *vcpu)

vcpu->cpu = -1;

+ kvm_riscv_vcpu_aia_put(vcpu);
+
kvm_riscv_vcpu_guest_fp_save(&vcpu->arch.guest_context,
vcpu->arch.isa);
kvm_riscv_vcpu_host_fp_restore(&vcpu->arch.host_context);
@@ -966,6 +992,7 @@ static void kvm_riscv_update_hvip(struct kvm_vcpu *vcpu)
struct kvm_vcpu_csr *csr = &vcpu->arch.guest_csr;

csr_write(CSR_HVIP, csr->hvip);
+ kvm_riscv_vcpu_aia_update_hvip(vcpu);
}

/*
@@ -1040,6 +1067,13 @@ int kvm_arch_vcpu_ioctl_run(struct kvm_vcpu *vcpu)

local_irq_disable();

+ /* Update AIA HW state before entering guest */
+ ret = kvm_riscv_vcpu_aia_update(vcpu);
+ if (ret <= 0) {
+ local_irq_enable();
+ continue;
+ }
+
/*
* Ensure we set mode to IN_GUEST_MODE after we disable
* interrupts and before the final VCPU requests check.
diff --git a/arch/riscv/kvm/vcpu_insn.c b/arch/riscv/kvm/vcpu_insn.c
index 0bb52761a3f7..07e8c121922b 100644
--- a/arch/riscv/kvm/vcpu_insn.c
+++ b/arch/riscv/kvm/vcpu_insn.c
@@ -213,7 +213,9 @@ struct csr_func {
unsigned long wr_mask);
};

-static const struct csr_func csr_funcs[] = { };
+static const struct csr_func csr_funcs[] = {
+ KVM_RISCV_VCPU_AIA_CSR_FUNCS
+};

/**
* kvm_riscv_vcpu_csr_return -- Handle CSR read/write after user space
diff --git a/arch/riscv/kvm/vm.c b/arch/riscv/kvm/vm.c
index 65a964d7e70d..bc03d2ddcb51 100644
--- a/arch/riscv/kvm/vm.c
+++ b/arch/riscv/kvm/vm.c
@@ -41,6 +41,8 @@ int kvm_arch_init_vm(struct kvm *kvm, unsigned long type)
return r;
}

+ kvm_riscv_aia_init_vm(kvm);
+
kvm_riscv_guest_timer_init(kvm);

return 0;
@@ -49,6 +51,8 @@ int kvm_arch_init_vm(struct kvm *kvm, unsigned long type)
void kvm_arch_destroy_vm(struct kvm *kvm)
{
kvm_destroy_vcpus(kvm);
+
+ kvm_riscv_aia_destroy_vm(kvm);
}

int kvm_vm_ioctl_check_extension(struct kvm *kvm, long ext)
--
2.34.1


2023-01-28 07:28:49

by Anup Patel

[permalink] [raw]
Subject: [PATCH v2 5/7] RISC-V: KVM: Add ONE_REG interface for AIA CSRs

We extend the CSR ONE_REG interface to access both general CSRs and
AIA CSRs. To achieve this, we introduce "subtype" field in the ONE_REG
id which can be used for grouping registers within a particular "type"
of ONE_REG registers.

Signed-off-by: Anup Patel <[email protected]>
---
arch/riscv/include/uapi/asm/kvm.h | 15 ++++-
arch/riscv/kvm/vcpu.c | 96 ++++++++++++++++++++++++-------
2 files changed, 89 insertions(+), 22 deletions(-)

diff --git a/arch/riscv/include/uapi/asm/kvm.h b/arch/riscv/include/uapi/asm/kvm.h
index 71992ff1f9dd..d0704eff0121 100644
--- a/arch/riscv/include/uapi/asm/kvm.h
+++ b/arch/riscv/include/uapi/asm/kvm.h
@@ -64,7 +64,7 @@ struct kvm_riscv_core {
#define KVM_RISCV_MODE_S 1
#define KVM_RISCV_MODE_U 0

-/* CSR registers for KVM_GET_ONE_REG and KVM_SET_ONE_REG */
+/* General CSR registers for KVM_GET_ONE_REG and KVM_SET_ONE_REG */
struct kvm_riscv_csr {
unsigned long sstatus;
unsigned long sie;
@@ -78,6 +78,10 @@ struct kvm_riscv_csr {
unsigned long scounteren;
};

+/* AIA CSR registers for KVM_GET_ONE_REG and KVM_SET_ONE_REG */
+struct kvm_riscv_aia_csr {
+};
+
/* TIMER registers for KVM_GET_ONE_REG and KVM_SET_ONE_REG */
struct kvm_riscv_timer {
__u64 frequency;
@@ -105,6 +109,7 @@ enum KVM_RISCV_ISA_EXT_ID {
KVM_RISCV_ISA_EXT_SVINVAL,
KVM_RISCV_ISA_EXT_ZIHINTPAUSE,
KVM_RISCV_ISA_EXT_ZICBOM,
+ KVM_RISCV_ISA_EXT_SSAIA,
KVM_RISCV_ISA_EXT_MAX,
};

@@ -134,6 +139,8 @@ enum KVM_RISCV_SBI_EXT_ID {
/* If you need to interpret the index values, here is the key: */
#define KVM_REG_RISCV_TYPE_MASK 0x00000000FF000000
#define KVM_REG_RISCV_TYPE_SHIFT 24
+#define KVM_REG_RISCV_SUBTYPE_MASK 0x0000000000FF0000
+#define KVM_REG_RISCV_SUBTYPE_SHIFT 16

/* Config registers are mapped as type 1 */
#define KVM_REG_RISCV_CONFIG (0x01 << KVM_REG_RISCV_TYPE_SHIFT)
@@ -147,8 +154,12 @@ enum KVM_RISCV_SBI_EXT_ID {

/* Control and status registers are mapped as type 3 */
#define KVM_REG_RISCV_CSR (0x03 << KVM_REG_RISCV_TYPE_SHIFT)
+#define KVM_REG_RISCV_CSR_GENERAL 0x0
+#define KVM_REG_RISCV_CSR_AIA 0x1
#define KVM_REG_RISCV_CSR_REG(name) \
- (offsetof(struct kvm_riscv_csr, name) / sizeof(unsigned long))
+ (offsetof(struct kvm_riscv_csr, name) / sizeof(unsigned long))
+#define KVM_REG_RISCV_CSR_AIA_REG(name) \
+ (offsetof(struct kvm_riscv_aia_csr, name) / sizeof(unsigned long))

/* Timer registers are mapped as type 4 */
#define KVM_REG_RISCV_TIMER (0x04 << KVM_REG_RISCV_TYPE_SHIFT)
diff --git a/arch/riscv/kvm/vcpu.c b/arch/riscv/kvm/vcpu.c
index 3cf50eadc8ce..37933ea20274 100644
--- a/arch/riscv/kvm/vcpu.c
+++ b/arch/riscv/kvm/vcpu.c
@@ -58,6 +58,7 @@ static const unsigned long kvm_isa_ext_arr[] = {
[KVM_RISCV_ISA_EXT_I] = RISCV_ISA_EXT_i,
[KVM_RISCV_ISA_EXT_M] = RISCV_ISA_EXT_m,

+ KVM_ISA_EXT_ARR(SSAIA),
KVM_ISA_EXT_ARR(SSTC),
KVM_ISA_EXT_ARR(SVINVAL),
KVM_ISA_EXT_ARR(SVPBMT),
@@ -96,6 +97,7 @@ static bool kvm_riscv_vcpu_isa_disable_allowed(unsigned long ext)
case KVM_RISCV_ISA_EXT_C:
case KVM_RISCV_ISA_EXT_I:
case KVM_RISCV_ISA_EXT_M:
+ case KVM_RISCV_ISA_EXT_SSAIA:
case KVM_RISCV_ISA_EXT_SSTC:
case KVM_RISCV_ISA_EXT_SVINVAL:
case KVM_RISCV_ISA_EXT_ZIHINTPAUSE:
@@ -451,30 +453,79 @@ static int kvm_riscv_vcpu_set_reg_core(struct kvm_vcpu *vcpu,
return 0;
}

+static int kvm_riscv_vcpu_general_get_csr(struct kvm_vcpu *vcpu,
+ unsigned long reg_num,
+ unsigned long *out_val)
+{
+ struct kvm_vcpu_csr *csr = &vcpu->arch.guest_csr;
+
+ if (reg_num >= sizeof(struct kvm_riscv_csr) / sizeof(unsigned long))
+ return -EINVAL;
+
+ if (reg_num == KVM_REG_RISCV_CSR_REG(sip)) {
+ kvm_riscv_vcpu_flush_interrupts(vcpu);
+ *out_val = (csr->hvip >> VSIP_TO_HVIP_SHIFT) & VSIP_VALID_MASK;
+ } else
+ *out_val = ((unsigned long *)csr)[reg_num];
+
+ return 0;
+}
+
static int kvm_riscv_vcpu_get_reg_csr(struct kvm_vcpu *vcpu,
const struct kvm_one_reg *reg)
{
- struct kvm_vcpu_csr *csr = &vcpu->arch.guest_csr;
+ int rc;
unsigned long __user *uaddr =
(unsigned long __user *)(unsigned long)reg->addr;
unsigned long reg_num = reg->id & ~(KVM_REG_ARCH_MASK |
KVM_REG_SIZE_MASK |
KVM_REG_RISCV_CSR);
- unsigned long reg_val;
+ unsigned long reg_val, reg_subtype;

if (KVM_REG_SIZE(reg->id) != sizeof(unsigned long))
return -EINVAL;
+
+ reg_subtype = (reg_num & KVM_REG_RISCV_SUBTYPE_MASK)
+ >> KVM_REG_RISCV_SUBTYPE_SHIFT;
+ reg_num &= ~KVM_REG_RISCV_SUBTYPE_MASK;
+ switch (reg_subtype) {
+ case KVM_REG_RISCV_CSR_GENERAL:
+ rc = kvm_riscv_vcpu_general_get_csr(vcpu, reg_num, &reg_val);
+ break;
+ case KVM_REG_RISCV_CSR_AIA:
+ rc = kvm_riscv_vcpu_aia_get_csr(vcpu, reg_num, &reg_val);
+ break;
+ default:
+ rc = -EINVAL;
+ break;
+ }
+ if (rc)
+ return rc;
+
+ if (copy_to_user(uaddr, &reg_val, KVM_REG_SIZE(reg->id)))
+ return -EFAULT;
+
+ return 0;
+}
+
+static inline int kvm_riscv_vcpu_general_set_csr(struct kvm_vcpu *vcpu,
+ unsigned long reg_num,
+ unsigned long reg_val)
+{
+ struct kvm_vcpu_csr *csr = &vcpu->arch.guest_csr;
+
if (reg_num >= sizeof(struct kvm_riscv_csr) / sizeof(unsigned long))
return -EINVAL;

if (reg_num == KVM_REG_RISCV_CSR_REG(sip)) {
- kvm_riscv_vcpu_flush_interrupts(vcpu);
- reg_val = (csr->hvip >> VSIP_TO_HVIP_SHIFT) & VSIP_VALID_MASK;
- } else
- reg_val = ((unsigned long *)csr)[reg_num];
+ reg_val &= VSIP_VALID_MASK;
+ reg_val <<= VSIP_TO_HVIP_SHIFT;
+ }

- if (copy_to_user(uaddr, &reg_val, KVM_REG_SIZE(reg->id)))
- return -EFAULT;
+ ((unsigned long *)csr)[reg_num] = reg_val;
+
+ if (reg_num == KVM_REG_RISCV_CSR_REG(sip))
+ WRITE_ONCE(vcpu->arch.irqs_pending_mask, 0);

return 0;
}
@@ -482,31 +533,36 @@ static int kvm_riscv_vcpu_get_reg_csr(struct kvm_vcpu *vcpu,
static int kvm_riscv_vcpu_set_reg_csr(struct kvm_vcpu *vcpu,
const struct kvm_one_reg *reg)
{
- struct kvm_vcpu_csr *csr = &vcpu->arch.guest_csr;
+ int rc;
unsigned long __user *uaddr =
(unsigned long __user *)(unsigned long)reg->addr;
unsigned long reg_num = reg->id & ~(KVM_REG_ARCH_MASK |
KVM_REG_SIZE_MASK |
KVM_REG_RISCV_CSR);
- unsigned long reg_val;
+ unsigned long reg_val, reg_subtype;

if (KVM_REG_SIZE(reg->id) != sizeof(unsigned long))
return -EINVAL;
- if (reg_num >= sizeof(struct kvm_riscv_csr) / sizeof(unsigned long))
- return -EINVAL;

if (copy_from_user(&reg_val, uaddr, KVM_REG_SIZE(reg->id)))
return -EFAULT;

- if (reg_num == KVM_REG_RISCV_CSR_REG(sip)) {
- reg_val &= VSIP_VALID_MASK;
- reg_val <<= VSIP_TO_HVIP_SHIFT;
+ reg_subtype = (reg_num & KVM_REG_RISCV_SUBTYPE_MASK)
+ >> KVM_REG_RISCV_SUBTYPE_SHIFT;
+ reg_num &= ~KVM_REG_RISCV_SUBTYPE_MASK;
+ switch (reg_subtype) {
+ case KVM_REG_RISCV_CSR_GENERAL:
+ rc = kvm_riscv_vcpu_general_set_csr(vcpu, reg_num, reg_val);
+ break;
+ case KVM_REG_RISCV_CSR_AIA:
+ rc = kvm_riscv_vcpu_aia_set_csr(vcpu, reg_num, reg_val);
+ break;
+ default:
+ rc = -EINVAL;
+ break;
}
-
- ((unsigned long *)csr)[reg_num] = reg_val;
-
- if (reg_num == KVM_REG_RISCV_CSR_REG(sip))
- WRITE_ONCE(vcpu->arch.irqs_pending_mask, 0);
+ if (rc)
+ return rc;

return 0;
}
--
2.34.1


2023-01-28 07:28:52

by Anup Patel

[permalink] [raw]
Subject: [PATCH v2 6/7] RISC-V: KVM: Virtualize per-HART AIA CSRs

The AIA specification introduce per-HART AIA CSRs which primarily
support:
* 64 local interrupts on both RV64 and RV32
* priority for each of the 64 local interrupts
* interrupt filtering for local interrupts

This patch virtualize above mentioned AIA CSRs and also extend
ONE_REG interface to allow user-space save/restore Guest/VM
view of these CSRs.

Signed-off-by: Anup Patel <[email protected]>
---
arch/riscv/include/asm/kvm_aia.h | 88 +++++----
arch/riscv/include/asm/kvm_host.h | 7 +-
arch/riscv/include/uapi/asm/kvm.h | 7 +
arch/riscv/kvm/aia.c | 317 ++++++++++++++++++++++++++++++
arch/riscv/kvm/vcpu.c | 53 +++--
5 files changed, 415 insertions(+), 57 deletions(-)

diff --git a/arch/riscv/include/asm/kvm_aia.h b/arch/riscv/include/asm/kvm_aia.h
index 258a835d4c32..1de0717112e5 100644
--- a/arch/riscv/include/asm/kvm_aia.h
+++ b/arch/riscv/include/asm/kvm_aia.h
@@ -12,6 +12,7 @@

#include <linux/jump_label.h>
#include <linux/kvm_types.h>
+#include <asm/csr.h>

struct kvm_aia {
/* In-kernel irqchip created */
@@ -21,7 +22,22 @@ struct kvm_aia {
bool initialized;
};

+struct kvm_vcpu_aia_csr {
+ unsigned long vsiselect;
+ unsigned long hviprio1;
+ unsigned long hviprio2;
+ unsigned long vsieh;
+ unsigned long hviph;
+ unsigned long hviprio1h;
+ unsigned long hviprio2h;
+};
+
struct kvm_vcpu_aia {
+ /* CPU AIA CSR context of Guest VCPU */
+ struct kvm_vcpu_aia_csr guest_csr;
+
+ /* CPU AIA CSR context upon Guest VCPU reset */
+ struct kvm_vcpu_aia_csr guest_reset_csr;
};

#define kvm_riscv_aia_initialized(k) ((k)->arch.aia.initialized)
@@ -32,48 +48,50 @@ DECLARE_STATIC_KEY_FALSE(kvm_riscv_aia_available);
#define kvm_riscv_aia_available() \
static_branch_unlikely(&kvm_riscv_aia_available)

-static inline void kvm_riscv_vcpu_aia_flush_interrupts(struct kvm_vcpu *vcpu)
-{
-}
-
-static inline void kvm_riscv_vcpu_aia_sync_interrupts(struct kvm_vcpu *vcpu)
-{
-}
-
-static inline bool kvm_riscv_vcpu_aia_has_interrupts(struct kvm_vcpu *vcpu,
- u64 mask)
-{
- return false;
-}
-
-static inline void kvm_riscv_vcpu_aia_update_hvip(struct kvm_vcpu *vcpu)
-{
-}
-
-static inline void kvm_riscv_vcpu_aia_load(struct kvm_vcpu *vcpu, int cpu)
-{
-}
-
-static inline void kvm_riscv_vcpu_aia_put(struct kvm_vcpu *vcpu)
+#define KVM_RISCV_AIA_IMSIC_TOPEI (ISELECT_MASK + 1)
+static inline int kvm_riscv_vcpu_aia_imsic_rmw(struct kvm_vcpu *vcpu,
+ unsigned long isel,
+ unsigned long *val,
+ unsigned long new_val,
+ unsigned long wr_mask)
{
+ return 0;
}

-static inline int kvm_riscv_vcpu_aia_get_csr(struct kvm_vcpu *vcpu,
- unsigned long reg_num,
- unsigned long *out_val)
+#ifdef CONFIG_32BIT
+void kvm_riscv_vcpu_aia_flush_interrupts(struct kvm_vcpu *vcpu);
+void kvm_riscv_vcpu_aia_sync_interrupts(struct kvm_vcpu *vcpu);
+#else
+static inline void kvm_riscv_vcpu_aia_flush_interrupts(struct kvm_vcpu *vcpu)
{
- *out_val = 0;
- return 0;
}
-
-static inline int kvm_riscv_vcpu_aia_set_csr(struct kvm_vcpu *vcpu,
- unsigned long reg_num,
- unsigned long val)
+static inline void kvm_riscv_vcpu_aia_sync_interrupts(struct kvm_vcpu *vcpu)
{
- return 0;
}
-
-#define KVM_RISCV_VCPU_AIA_CSR_FUNCS
+#endif
+bool kvm_riscv_vcpu_aia_has_interrupts(struct kvm_vcpu *vcpu, u64 mask);
+
+void kvm_riscv_vcpu_aia_update_hvip(struct kvm_vcpu *vcpu);
+void kvm_riscv_vcpu_aia_load(struct kvm_vcpu *vcpu, int cpu);
+void kvm_riscv_vcpu_aia_put(struct kvm_vcpu *vcpu);
+int kvm_riscv_vcpu_aia_get_csr(struct kvm_vcpu *vcpu,
+ unsigned long reg_num,
+ unsigned long *out_val);
+int kvm_riscv_vcpu_aia_set_csr(struct kvm_vcpu *vcpu,
+ unsigned long reg_num,
+ unsigned long val);
+
+int kvm_riscv_vcpu_aia_rmw_topei(struct kvm_vcpu *vcpu,
+ unsigned int csr_num,
+ unsigned long *val,
+ unsigned long new_val,
+ unsigned long wr_mask);
+int kvm_riscv_vcpu_aia_rmw_ireg(struct kvm_vcpu *vcpu, unsigned int csr_num,
+ unsigned long *val, unsigned long new_val,
+ unsigned long wr_mask);
+#define KVM_RISCV_VCPU_AIA_CSR_FUNCS \
+{ .base = CSR_SIREG, .count = 1, .func = kvm_riscv_vcpu_aia_rmw_ireg }, \
+{ .base = CSR_STOPEI, .count = 1, .func = kvm_riscv_vcpu_aia_rmw_topei },

static inline int kvm_riscv_vcpu_aia_update(struct kvm_vcpu *vcpu)
{
diff --git a/arch/riscv/include/asm/kvm_host.h b/arch/riscv/include/asm/kvm_host.h
index 8776e7a465c7..607254177270 100644
--- a/arch/riscv/include/asm/kvm_host.h
+++ b/arch/riscv/include/asm/kvm_host.h
@@ -203,8 +203,9 @@ struct kvm_vcpu_arch {
* in irqs_pending. Our approach is modeled around multiple producer
* and single consumer problem where the consumer is the VCPU itself.
*/
- unsigned long irqs_pending;
- unsigned long irqs_pending_mask;
+#define KVM_RISCV_VCPU_NR_IRQS 64
+ DECLARE_BITMAP(irqs_pending, KVM_RISCV_VCPU_NR_IRQS);
+ DECLARE_BITMAP(irqs_pending_mask, KVM_RISCV_VCPU_NR_IRQS);

/* VCPU Timer */
struct kvm_vcpu_timer timer;
@@ -331,7 +332,7 @@ int kvm_riscv_vcpu_set_interrupt(struct kvm_vcpu *vcpu, unsigned int irq);
int kvm_riscv_vcpu_unset_interrupt(struct kvm_vcpu *vcpu, unsigned int irq);
void kvm_riscv_vcpu_flush_interrupts(struct kvm_vcpu *vcpu);
void kvm_riscv_vcpu_sync_interrupts(struct kvm_vcpu *vcpu);
-bool kvm_riscv_vcpu_has_interrupts(struct kvm_vcpu *vcpu, unsigned long mask);
+bool kvm_riscv_vcpu_has_interrupts(struct kvm_vcpu *vcpu, u64 mask);
void kvm_riscv_vcpu_power_off(struct kvm_vcpu *vcpu);
void kvm_riscv_vcpu_power_on(struct kvm_vcpu *vcpu);

diff --git a/arch/riscv/include/uapi/asm/kvm.h b/arch/riscv/include/uapi/asm/kvm.h
index d0704eff0121..8aae65424a43 100644
--- a/arch/riscv/include/uapi/asm/kvm.h
+++ b/arch/riscv/include/uapi/asm/kvm.h
@@ -80,6 +80,13 @@ struct kvm_riscv_csr {

/* AIA CSR registers for KVM_GET_ONE_REG and KVM_SET_ONE_REG */
struct kvm_riscv_aia_csr {
+ unsigned long siselect;
+ unsigned long siprio1;
+ unsigned long siprio2;
+ unsigned long sieh;
+ unsigned long siph;
+ unsigned long siprio1h;
+ unsigned long siprio2h;
};

/* TIMER registers for KVM_GET_ONE_REG and KVM_SET_ONE_REG */
diff --git a/arch/riscv/kvm/aia.c b/arch/riscv/kvm/aia.c
index 7a633331cd3e..3d68189e0bb1 100644
--- a/arch/riscv/kvm/aia.c
+++ b/arch/riscv/kvm/aia.c
@@ -26,6 +26,323 @@ static void aia_set_hvictl(bool ext_irq_pending)
csr_write(CSR_HVICTL, hvictl);
}

+#ifdef CONFIG_32BIT
+void kvm_riscv_vcpu_aia_flush_interrupts(struct kvm_vcpu *vcpu)
+{
+ struct kvm_vcpu_aia_csr *csr = &vcpu->arch.aia_context.guest_csr;
+ unsigned long mask, val;
+
+ if (!kvm_riscv_aia_available())
+ return;
+
+ if (READ_ONCE(vcpu->arch.irqs_pending_mask[1])) {
+ mask = xchg_acquire(&vcpu->arch.irqs_pending_mask[1], 0);
+ val = READ_ONCE(vcpu->arch.irqs_pending[1]) & mask;
+
+ csr->hviph &= ~mask;
+ csr->hviph |= val;
+ }
+}
+
+void kvm_riscv_vcpu_aia_sync_interrupts(struct kvm_vcpu *vcpu)
+{
+ struct kvm_vcpu_aia_csr *csr = &vcpu->arch.aia_context.guest_csr;
+
+ if (kvm_riscv_aia_available())
+ csr->vsieh = csr_read(CSR_VSIEH);
+}
+#endif
+
+bool kvm_riscv_vcpu_aia_has_interrupts(struct kvm_vcpu *vcpu, u64 mask)
+{
+ unsigned long seip;
+
+ if (!kvm_riscv_aia_available())
+ return false;
+
+#ifdef CONFIG_32BIT
+ if (READ_ONCE(vcpu->arch.irqs_pending[1]) &
+ (vcpu->arch.aia.guest_csr.vsieh & (unsigned long)(mask >> 32)))
+ return true;
+#endif
+
+ seip = vcpu->arch.guest_csr.vsie;
+ seip &= (unsigned long)mask;
+ seip &= BIT(IRQ_S_EXT);
+ if (!kvm_riscv_aia_initialized(vcpu->kvm) || !seip)
+ return false;
+
+ return false;
+}
+
+void kvm_riscv_vcpu_aia_update_hvip(struct kvm_vcpu *vcpu)
+{
+ struct kvm_vcpu_csr *csr = &vcpu->arch.guest_csr;
+
+ if (!kvm_riscv_aia_available())
+ return;
+
+#ifdef CONFIG_32BIT
+ csr_write(CSR_HVIPH, vcpu->arch.aia.guest_csr.hviph);
+#endif
+ aia_set_hvictl((csr->hvip & BIT(IRQ_VS_EXT)) ? true : false);
+}
+
+void kvm_riscv_vcpu_aia_load(struct kvm_vcpu *vcpu, int cpu)
+{
+ struct kvm_vcpu_aia_csr *csr = &vcpu->arch.aia_context.guest_csr;
+
+ if (!kvm_riscv_aia_available())
+ return;
+
+ csr_write(CSR_VSISELECT, csr->vsiselect);
+ csr_write(CSR_HVIPRIO1, csr->hviprio1);
+ csr_write(CSR_HVIPRIO2, csr->hviprio2);
+#ifdef CONFIG_32BIT
+ csr_write(CSR_VSIEH, csr->vsieh);
+ csr_write(CSR_HVIPH, csr->hviph);
+ csr_write(CSR_HVIPRIO1H, csr->hviprio1h);
+ csr_write(CSR_HVIPRIO2H, csr->hviprio2h);
+#endif
+}
+
+void kvm_riscv_vcpu_aia_put(struct kvm_vcpu *vcpu)
+{
+ struct kvm_vcpu_aia_csr *csr = &vcpu->arch.aia_context.guest_csr;
+
+ if (!kvm_riscv_aia_available())
+ return;
+
+ csr->vsiselect = csr_read(CSR_VSISELECT);
+ csr->hviprio1 = csr_read(CSR_HVIPRIO1);
+ csr->hviprio2 = csr_read(CSR_HVIPRIO2);
+#ifdef CONFIG_32BIT
+ csr->vsieh = csr_read(CSR_VSIEH);
+ csr->hviph = csr_read(CSR_HVIPH);
+ csr->hviprio1h = csr_read(CSR_HVIPRIO1H);
+ csr->hviprio2h = csr_read(CSR_HVIPRIO2H);
+#endif
+}
+
+int kvm_riscv_vcpu_aia_get_csr(struct kvm_vcpu *vcpu,
+ unsigned long reg_num,
+ unsigned long *out_val)
+{
+ struct kvm_vcpu_aia_csr *csr = &vcpu->arch.aia_context.guest_csr;
+
+ if (reg_num >= sizeof(struct kvm_riscv_aia_csr) / sizeof(unsigned long))
+ return -EINVAL;
+
+ *out_val = 0;
+ if (kvm_riscv_aia_available())
+ *out_val = ((unsigned long *)csr)[reg_num];
+
+ return 0;
+}
+
+int kvm_riscv_vcpu_aia_set_csr(struct kvm_vcpu *vcpu,
+ unsigned long reg_num,
+ unsigned long val)
+{
+ struct kvm_vcpu_aia_csr *csr = &vcpu->arch.aia_context.guest_csr;
+
+ if (reg_num >= sizeof(struct kvm_riscv_aia_csr) / sizeof(unsigned long))
+ return -EINVAL;
+
+ if (kvm_riscv_aia_available()) {
+ ((unsigned long *)csr)[reg_num] = val;
+
+#ifdef CONFIG_32BIT
+ if (reg_num == KVM_REG_RISCV_CSR_AIA_REG(siph))
+ WRITE_ONCE(vcpu->arch.irqs_pending_mask[1], 0);
+#endif
+ }
+
+ return 0;
+}
+
+int kvm_riscv_vcpu_aia_rmw_topei(struct kvm_vcpu *vcpu,
+ unsigned int csr_num,
+ unsigned long *val,
+ unsigned long new_val,
+ unsigned long wr_mask)
+{
+ /* If AIA not available then redirect trap */
+ if (!kvm_riscv_aia_available())
+ return KVM_INSN_ILLEGAL_TRAP;
+
+ /* If AIA not initialized then forward to user space */
+ if (!kvm_riscv_aia_initialized(vcpu->kvm))
+ return KVM_INSN_EXIT_TO_USER_SPACE;
+
+ return kvm_riscv_vcpu_aia_imsic_rmw(vcpu, KVM_RISCV_AIA_IMSIC_TOPEI,
+ val, new_val, wr_mask);
+}
+
+/*
+ * External IRQ priority always read-only zero. This means default
+ * priority order is always preferred for external IRQs unless
+ * HVICTL.IID == 9 and HVICTL.IPRIO != 0
+ */
+static int aia_irq2bitpos[] = {
+0, 8, -1, -1, 16, 24, -1, -1, /* 0 - 7 */
+32, -1, -1, -1, -1, 40, 48, 56, /* 8 - 15 */
+64, 72, 80, 88, 96, 104, 112, 120, /* 16 - 23 */
+-1, -1, -1, -1, -1, -1, -1, -1, /* 24 - 31 */
+-1, -1, -1, -1, -1, -1, -1, -1, /* 32 - 39 */
+-1, -1, -1, -1, -1, -1, -1, -1, /* 40 - 47 */
+-1, -1, -1, -1, -1, -1, -1, -1, /* 48 - 55 */
+-1, -1, -1, -1, -1, -1, -1, -1, /* 56 - 63 */
+};
+
+static u8 aia_get_iprio8(struct kvm_vcpu *vcpu, unsigned int irq)
+{
+ unsigned long hviprio;
+ int bitpos = aia_irq2bitpos[irq];
+
+ if (bitpos < 0)
+ return 0;
+
+ switch (bitpos / BITS_PER_LONG) {
+ case 0:
+ hviprio = csr_read(CSR_HVIPRIO1);
+ break;
+ case 1:
+#ifndef CONFIG_32BIT
+ hviprio = csr_read(CSR_HVIPRIO2);
+ break;
+#else
+ hviprio = csr_read(CSR_HVIPRIO1H);
+ break;
+ case 2:
+ hviprio = csr_read(CSR_HVIPRIO2);
+ break;
+ case 3:
+ hviprio = csr_read(CSR_HVIPRIO2H);
+ break;
+#endif
+ default:
+ return 0;
+ };
+
+ return (hviprio >> (bitpos % BITS_PER_LONG)) & TOPI_IPRIO_MASK;
+}
+
+static void aia_set_iprio8(struct kvm_vcpu *vcpu, unsigned int irq, u8 prio)
+{
+ unsigned long hviprio;
+ int bitpos = aia_irq2bitpos[irq];
+
+ if (bitpos < 0)
+ return;
+
+ switch (bitpos / BITS_PER_LONG) {
+ case 0:
+ hviprio = csr_read(CSR_HVIPRIO1);
+ break;
+ case 1:
+#ifndef CONFIG_32BIT
+ hviprio = csr_read(CSR_HVIPRIO2);
+ break;
+#else
+ hviprio = csr_read(CSR_HVIPRIO1H);
+ break;
+ case 2:
+ hviprio = csr_read(CSR_HVIPRIO2);
+ break;
+ case 3:
+ hviprio = csr_read(CSR_HVIPRIO2H);
+ break;
+#endif
+ default:
+ return;
+ };
+
+ hviprio &= ~((unsigned long)TOPI_IPRIO_MASK <<
+ (bitpos % BITS_PER_LONG));
+ hviprio |= (unsigned long)prio << (bitpos % BITS_PER_LONG);
+
+ switch (bitpos / BITS_PER_LONG) {
+ case 0:
+ csr_write(CSR_HVIPRIO1, hviprio);
+ break;
+ case 1:
+#ifndef CONFIG_32BIT
+ csr_write(CSR_HVIPRIO2, hviprio);
+ break;
+#else
+ csr_write(CSR_HVIPRIO1H, hviprio);
+ break;
+ case 2:
+ csr_write(CSR_HVIPRIO2, hviprio);
+ break;
+ case 3:
+ csr_write(CSR_HVIPRIO2H, hviprio);
+ break;
+#endif
+ default:
+ return;
+ };
+}
+
+static int aia_rmw_iprio(struct kvm_vcpu *vcpu, unsigned int isel,
+ unsigned long *val, unsigned long new_val,
+ unsigned long wr_mask)
+{
+ int i, firq, nirqs;
+ unsigned long old_val;
+
+#ifndef CONFIG_32BIT
+ if (isel & 0x1)
+ return KVM_INSN_ILLEGAL_TRAP;
+#endif
+
+ nirqs = 4 * (BITS_PER_LONG / 32);
+ firq = ((isel - ISELECT_IPRIO0) / (BITS_PER_LONG / 32)) * (nirqs);
+
+ old_val = 0;
+ for (i = 0; i < nirqs; i++)
+ old_val |= (unsigned long)aia_get_iprio8(vcpu, firq + i) <<
+ (TOPI_IPRIO_BITS * i);
+
+ if (val)
+ *val = old_val;
+
+ if (wr_mask) {
+ new_val = (old_val & ~wr_mask) | (new_val & wr_mask);
+ for (i = 0; i < nirqs; i++)
+ aia_set_iprio8(vcpu, firq + i,
+ (new_val >> (TOPI_IPRIO_BITS * i)) & TOPI_IPRIO_MASK);
+ }
+
+ return KVM_INSN_CONTINUE_NEXT_SEPC;
+}
+
+#define IMSIC_FIRST 0x70
+#define IMSIC_LAST 0xff
+int kvm_riscv_vcpu_aia_rmw_ireg(struct kvm_vcpu *vcpu, unsigned int csr_num,
+ unsigned long *val, unsigned long new_val,
+ unsigned long wr_mask)
+{
+ unsigned int isel;
+
+ /* If AIA not available then redirect trap */
+ if (!kvm_riscv_aia_available())
+ return KVM_INSN_ILLEGAL_TRAP;
+
+ /* First try to emulate in kernel space */
+ isel = csr_read(CSR_VSISELECT) & ISELECT_MASK;
+ if (isel >= ISELECT_IPRIO0 && isel <= ISELECT_IPRIO15)
+ return aia_rmw_iprio(vcpu, isel, val, new_val, wr_mask);
+ else if (isel >= IMSIC_FIRST && isel <= IMSIC_LAST &&
+ kvm_riscv_aia_initialized(vcpu->kvm))
+ return kvm_riscv_vcpu_aia_imsic_rmw(vcpu, isel, val, new_val,
+ wr_mask);
+
+ /* We can't handle it here so redirect to user space */
+ return KVM_INSN_EXIT_TO_USER_SPACE;
+}
+
void kvm_riscv_aia_enable(void)
{
if (!kvm_riscv_aia_available())
diff --git a/arch/riscv/kvm/vcpu.c b/arch/riscv/kvm/vcpu.c
index 37933ea20274..151b35b3b05f 100644
--- a/arch/riscv/kvm/vcpu.c
+++ b/arch/riscv/kvm/vcpu.c
@@ -139,8 +139,8 @@ static void kvm_riscv_reset_vcpu(struct kvm_vcpu *vcpu)

kvm_riscv_vcpu_aia_reset(vcpu);

- WRITE_ONCE(vcpu->arch.irqs_pending, 0);
- WRITE_ONCE(vcpu->arch.irqs_pending_mask, 0);
+ bitmap_zero(vcpu->arch.irqs_pending, KVM_RISCV_VCPU_NR_IRQS);
+ bitmap_zero(vcpu->arch.irqs_pending_mask, KVM_RISCV_VCPU_NR_IRQS);

vcpu->arch.hfence_head = 0;
vcpu->arch.hfence_tail = 0;
@@ -465,6 +465,7 @@ static int kvm_riscv_vcpu_general_get_csr(struct kvm_vcpu *vcpu,
if (reg_num == KVM_REG_RISCV_CSR_REG(sip)) {
kvm_riscv_vcpu_flush_interrupts(vcpu);
*out_val = (csr->hvip >> VSIP_TO_HVIP_SHIFT) & VSIP_VALID_MASK;
+ *out_val |= csr->hvip & ~IRQ_LOCAL_MASK;
} else
*out_val = ((unsigned long *)csr)[reg_num];

@@ -525,7 +526,7 @@ static inline int kvm_riscv_vcpu_general_set_csr(struct kvm_vcpu *vcpu,
((unsigned long *)csr)[reg_num] = reg_val;

if (reg_num == KVM_REG_RISCV_CSR_REG(sip))
- WRITE_ONCE(vcpu->arch.irqs_pending_mask, 0);
+ WRITE_ONCE(vcpu->arch.irqs_pending_mask[0], 0);

return 0;
}
@@ -790,9 +791,9 @@ void kvm_riscv_vcpu_flush_interrupts(struct kvm_vcpu *vcpu)
struct kvm_vcpu_csr *csr = &vcpu->arch.guest_csr;
unsigned long mask, val;

- if (READ_ONCE(vcpu->arch.irqs_pending_mask)) {
- mask = xchg_acquire(&vcpu->arch.irqs_pending_mask, 0);
- val = READ_ONCE(vcpu->arch.irqs_pending) & mask;
+ if (READ_ONCE(vcpu->arch.irqs_pending_mask[0])) {
+ mask = xchg_acquire(&vcpu->arch.irqs_pending_mask[0], 0);
+ val = READ_ONCE(vcpu->arch.irqs_pending[0]) & mask;

csr->hvip &= ~mask;
csr->hvip |= val;
@@ -816,12 +817,12 @@ void kvm_riscv_vcpu_sync_interrupts(struct kvm_vcpu *vcpu)
if ((csr->hvip ^ hvip) & (1UL << IRQ_VS_SOFT)) {
if (hvip & (1UL << IRQ_VS_SOFT)) {
if (!test_and_set_bit(IRQ_VS_SOFT,
- &v->irqs_pending_mask))
- set_bit(IRQ_VS_SOFT, &v->irqs_pending);
+ v->irqs_pending_mask))
+ set_bit(IRQ_VS_SOFT, v->irqs_pending);
} else {
if (!test_and_set_bit(IRQ_VS_SOFT,
- &v->irqs_pending_mask))
- clear_bit(IRQ_VS_SOFT, &v->irqs_pending);
+ v->irqs_pending_mask))
+ clear_bit(IRQ_VS_SOFT, v->irqs_pending);
}
}

@@ -834,14 +835,20 @@ void kvm_riscv_vcpu_sync_interrupts(struct kvm_vcpu *vcpu)

int kvm_riscv_vcpu_set_interrupt(struct kvm_vcpu *vcpu, unsigned int irq)
{
- if (irq != IRQ_VS_SOFT &&
+ /*
+ * We only allow VS-mode software, timer, and external
+ * interrupts when irq is one of the local interrupts
+ * defined by RISC-V privilege specification.
+ */
+ if (irq < IRQ_LOCAL_MAX &&
+ irq != IRQ_VS_SOFT &&
irq != IRQ_VS_TIMER &&
irq != IRQ_VS_EXT)
return -EINVAL;

- set_bit(irq, &vcpu->arch.irqs_pending);
+ set_bit(irq, vcpu->arch.irqs_pending);
smp_mb__before_atomic();
- set_bit(irq, &vcpu->arch.irqs_pending_mask);
+ set_bit(irq, vcpu->arch.irqs_pending_mask);

kvm_vcpu_kick(vcpu);

@@ -850,25 +857,33 @@ int kvm_riscv_vcpu_set_interrupt(struct kvm_vcpu *vcpu, unsigned int irq)

int kvm_riscv_vcpu_unset_interrupt(struct kvm_vcpu *vcpu, unsigned int irq)
{
- if (irq != IRQ_VS_SOFT &&
+ /*
+ * We only allow VS-mode software, timer, and external
+ * interrupts when irq is one of the local interrupts
+ * defined by RISC-V privilege specification.
+ */
+ if (irq < IRQ_LOCAL_MAX &&
+ irq != IRQ_VS_SOFT &&
irq != IRQ_VS_TIMER &&
irq != IRQ_VS_EXT)
return -EINVAL;

- clear_bit(irq, &vcpu->arch.irqs_pending);
+ clear_bit(irq, vcpu->arch.irqs_pending);
smp_mb__before_atomic();
- set_bit(irq, &vcpu->arch.irqs_pending_mask);
+ set_bit(irq, vcpu->arch.irqs_pending_mask);

return 0;
}

-bool kvm_riscv_vcpu_has_interrupts(struct kvm_vcpu *vcpu, unsigned long mask)
+bool kvm_riscv_vcpu_has_interrupts(struct kvm_vcpu *vcpu, u64 mask)
{
unsigned long ie;

ie = ((vcpu->arch.guest_csr.vsie & VSIP_VALID_MASK)
- << VSIP_TO_HVIP_SHIFT) & mask;
- if (READ_ONCE(vcpu->arch.irqs_pending) & ie)
+ << VSIP_TO_HVIP_SHIFT) & (unsigned long)mask;
+ ie |= vcpu->arch.guest_csr.vsie & ~IRQ_LOCAL_MASK &
+ (unsigned long)mask;
+ if (READ_ONCE(vcpu->arch.irqs_pending[0]) & ie)
return true;

/* Check AIA high interrupts */
--
2.34.1


2023-01-28 07:29:19

by Anup Patel

[permalink] [raw]
Subject: [PATCH v2 7/7] RISC-V: KVM: Implement guest external interrupt line management

The RISC-V host will have one guest external interrupt line for each
VS-level IMSICs associated with a HART. The guest external interrupt
lines are per-HART resources and hypervisor can use HGEIE, HGEIP, and
HIE CSRs to manage these guest external interrupt lines.

Signed-off-by: Anup Patel <[email protected]>
---
arch/riscv/include/asm/kvm_aia.h | 10 ++
arch/riscv/kvm/aia.c | 241 +++++++++++++++++++++++++++++++
arch/riscv/kvm/main.c | 3 +-
arch/riscv/kvm/vcpu.c | 2 +
4 files changed, 255 insertions(+), 1 deletion(-)

diff --git a/arch/riscv/include/asm/kvm_aia.h b/arch/riscv/include/asm/kvm_aia.h
index 1de0717112e5..0938e0cadf80 100644
--- a/arch/riscv/include/asm/kvm_aia.h
+++ b/arch/riscv/include/asm/kvm_aia.h
@@ -44,10 +44,15 @@ struct kvm_vcpu_aia {

#define irqchip_in_kernel(k) ((k)->arch.aia.in_kernel)

+extern unsigned int kvm_riscv_aia_nr_hgei;
DECLARE_STATIC_KEY_FALSE(kvm_riscv_aia_available);
#define kvm_riscv_aia_available() \
static_branch_unlikely(&kvm_riscv_aia_available)

+static inline void kvm_riscv_vcpu_aia_imsic_release(struct kvm_vcpu *vcpu)
+{
+}
+
#define KVM_RISCV_AIA_IMSIC_TOPEI (ISELECT_MASK + 1)
static inline int kvm_riscv_vcpu_aia_imsic_rmw(struct kvm_vcpu *vcpu,
unsigned long isel,
@@ -119,6 +124,11 @@ static inline void kvm_riscv_aia_destroy_vm(struct kvm *kvm)
{
}

+int kvm_riscv_aia_alloc_hgei(int cpu, struct kvm_vcpu *owner,
+ void __iomem **hgei_va, phys_addr_t *hgei_pa);
+void kvm_riscv_aia_free_hgei(int cpu, int hgei);
+void kvm_riscv_aia_wakeon_hgei(struct kvm_vcpu *owner, bool enable);
+
void kvm_riscv_aia_enable(void);
void kvm_riscv_aia_disable(void);
int kvm_riscv_aia_init(void);
diff --git a/arch/riscv/kvm/aia.c b/arch/riscv/kvm/aia.c
index 3d68189e0bb1..f375069fdaf4 100644
--- a/arch/riscv/kvm/aia.c
+++ b/arch/riscv/kvm/aia.c
@@ -7,11 +7,46 @@
* Anup Patel <[email protected]>
*/

+#include <linux/bitops.h>
+#include <linux/irq.h>
+#include <linux/irqdomain.h>
#include <linux/kvm_host.h>
+#include <linux/percpu.h>
+#include <linux/spinlock.h>
#include <asm/hwcap.h>

+struct aia_hgei_control {
+ raw_spinlock_t lock;
+ unsigned long free_bitmap;
+ struct kvm_vcpu *owners[BITS_PER_LONG];
+};
+static DEFINE_PER_CPU(struct aia_hgei_control, aia_hgei);
+static int hgei_parent_irq;
+
+unsigned int kvm_riscv_aia_nr_hgei;
DEFINE_STATIC_KEY_FALSE(kvm_riscv_aia_available);

+static int aia_find_hgei(struct kvm_vcpu *owner)
+{
+ int i, hgei;
+ unsigned long flags;
+ struct aia_hgei_control *hgctrl = this_cpu_ptr(&aia_hgei);
+
+ raw_spin_lock_irqsave(&hgctrl->lock, flags);
+
+ hgei = -1;
+ for (i = 1; i <= kvm_riscv_aia_nr_hgei; i++) {
+ if (hgctrl->owners[i] == owner) {
+ hgei = i;
+ break;
+ }
+ }
+
+ raw_spin_unlock_irqrestore(&hgctrl->lock, flags);
+
+ return hgei;
+}
+
static void aia_set_hvictl(bool ext_irq_pending)
{
unsigned long hvictl;
@@ -55,6 +90,7 @@ void kvm_riscv_vcpu_aia_sync_interrupts(struct kvm_vcpu *vcpu)

bool kvm_riscv_vcpu_aia_has_interrupts(struct kvm_vcpu *vcpu, u64 mask)
{
+ int hgei;
unsigned long seip;

if (!kvm_riscv_aia_available())
@@ -72,6 +108,10 @@ bool kvm_riscv_vcpu_aia_has_interrupts(struct kvm_vcpu *vcpu, u64 mask)
if (!kvm_riscv_aia_initialized(vcpu->kvm) || !seip)
return false;

+ hgei = aia_find_hgei(vcpu);
+ if (hgei > 0)
+ return (csr_read(CSR_HGEIP) & BIT(hgei)) ? true : false;
+
return false;
}

@@ -343,6 +383,144 @@ int kvm_riscv_vcpu_aia_rmw_ireg(struct kvm_vcpu *vcpu, unsigned int csr_num,
return KVM_INSN_EXIT_TO_USER_SPACE;
}

+int kvm_riscv_aia_alloc_hgei(int cpu, struct kvm_vcpu *owner,
+ void __iomem **hgei_va, phys_addr_t *hgei_pa)
+{
+ int ret = -ENOENT;
+ unsigned long flags;
+ struct aia_hgei_control *hgctrl = per_cpu_ptr(&aia_hgei, cpu);
+
+ if (!kvm_riscv_aia_available())
+ return -ENODEV;
+ if (!hgctrl)
+ return -ENODEV;
+
+ raw_spin_lock_irqsave(&hgctrl->lock, flags);
+
+ if (hgctrl->free_bitmap) {
+ ret = __ffs(hgctrl->free_bitmap);
+ hgctrl->free_bitmap &= ~BIT(ret);
+ hgctrl->owners[ret] = owner;
+ }
+
+ raw_spin_unlock_irqrestore(&hgctrl->lock, flags);
+
+ /* TODO: To be updated later by AIA in-kernel irqchip support */
+ if (hgei_va)
+ *hgei_va = NULL;
+ if (hgei_pa)
+ *hgei_pa = 0;
+
+ return ret;
+}
+
+void kvm_riscv_aia_free_hgei(int cpu, int hgei)
+{
+ unsigned long flags;
+ struct aia_hgei_control *hgctrl = per_cpu_ptr(&aia_hgei, cpu);
+
+ if (!kvm_riscv_aia_available() || !hgctrl)
+ return;
+
+ raw_spin_lock_irqsave(&hgctrl->lock, flags);
+
+ if (hgei > 0 && hgei <= kvm_riscv_aia_nr_hgei) {
+ if (!(hgctrl->free_bitmap & BIT(hgei))) {
+ hgctrl->free_bitmap |= BIT(hgei);
+ hgctrl->owners[hgei] = NULL;
+ }
+ }
+
+ raw_spin_unlock_irqrestore(&hgctrl->lock, flags);
+}
+
+void kvm_riscv_aia_wakeon_hgei(struct kvm_vcpu *owner, bool enable)
+{
+ int hgei;
+
+ if (!kvm_riscv_aia_available())
+ return;
+
+ hgei = aia_find_hgei(owner);
+ if (hgei > 0) {
+ if (enable)
+ csr_set(CSR_HGEIE, BIT(hgei));
+ else
+ csr_clear(CSR_HGEIE, BIT(hgei));
+ }
+}
+
+static irqreturn_t hgei_interrupt(int irq, void *dev_id)
+{
+ int i;
+ unsigned long hgei_mask, flags;
+ struct aia_hgei_control *hgctrl = this_cpu_ptr(&aia_hgei);
+
+ hgei_mask = csr_read(CSR_HGEIP) & csr_read(CSR_HGEIE);
+ csr_clear(CSR_HGEIE, hgei_mask);
+
+ raw_spin_lock_irqsave(&hgctrl->lock, flags);
+
+ for_each_set_bit(i, &hgei_mask, BITS_PER_LONG) {
+ if (hgctrl->owners[i])
+ kvm_vcpu_kick(hgctrl->owners[i]);
+ }
+
+ raw_spin_unlock_irqrestore(&hgctrl->lock, flags);
+
+ return IRQ_HANDLED;
+}
+
+static int aia_hgei_init(void)
+{
+ int cpu, rc;
+ struct irq_domain *domain;
+ struct aia_hgei_control *hgctrl;
+
+ /* Initialize per-CPU guest external interrupt line management */
+ for_each_possible_cpu(cpu) {
+ hgctrl = per_cpu_ptr(&aia_hgei, cpu);
+ raw_spin_lock_init(&hgctrl->lock);
+ if (kvm_riscv_aia_nr_hgei) {
+ hgctrl->free_bitmap =
+ BIT(kvm_riscv_aia_nr_hgei + 1) - 1;
+ hgctrl->free_bitmap &= ~BIT(0);
+ } else
+ hgctrl->free_bitmap = 0;
+ }
+
+ /* Find INTC irq domain */
+ domain = irq_find_matching_fwnode(riscv_get_intc_hwnode(),
+ DOMAIN_BUS_ANY);
+ if (!domain) {
+ kvm_err("unable to find INTC domain\n");
+ return -ENOENT;
+ }
+
+ /* Map per-CPU SGEI interrupt from INTC domain */
+ hgei_parent_irq = irq_create_mapping(domain, IRQ_S_GEXT);
+ if (!hgei_parent_irq) {
+ kvm_err("unable to map SGEI IRQ\n");
+ return -ENOMEM;
+ }
+
+ /* Request per-CPU SGEI interrupt */
+ rc = request_percpu_irq(hgei_parent_irq, hgei_interrupt,
+ "riscv-kvm", &aia_hgei);
+ if (rc) {
+ kvm_err("failed to request SGEI IRQ\n");
+ return rc;
+ }
+
+ return 0;
+}
+
+static void aia_hgei_exit(void)
+{
+ /* Free per-CPU SGEI interrupt */
+ free_percpu_irq(hgei_parent_irq, &aia_hgei);
+}
+
void kvm_riscv_aia_enable(void)
{
if (!kvm_riscv_aia_available())
@@ -357,21 +535,79 @@ void kvm_riscv_aia_enable(void)
csr_write(CSR_HVIPRIO1H, 0x0);
csr_write(CSR_HVIPRIO2H, 0x0);
#endif
+
+ /* Enable per-CPU SGEI interrupt */
+ enable_percpu_irq(hgei_parent_irq,
+ irq_get_trigger_type(hgei_parent_irq));
+ csr_set(CSR_HIE, BIT(IRQ_S_GEXT));
}

void kvm_riscv_aia_disable(void)
{
+ int i;
+ unsigned long flags;
+ struct kvm_vcpu *vcpu;
+ struct aia_hgei_control *hgctrl = this_cpu_ptr(&aia_hgei);
+
if (!kvm_riscv_aia_available())
return;

+ /* Disable per-CPU SGEI interrupt */
+ csr_clear(CSR_HIE, BIT(IRQ_S_GEXT));
+ disable_percpu_irq(hgei_parent_irq);
+
aia_set_hvictl(false);
+
+ raw_spin_lock_irqsave(&hgctrl->lock, flags);
+
+ for (i = 0; i <= kvm_riscv_aia_nr_hgei; i++) {
+ vcpu = hgctrl->owners[i];
+ if (!vcpu)
+ continue;
+
+ /*
+ * We release hgctrl->lock before notifying IMSIC
+ * so that we don't have lock ordering issues.
+ */
+ raw_spin_unlock_irqrestore(&hgctrl->lock, flags);
+
+ /* Notify IMSIC */
+ kvm_riscv_vcpu_aia_imsic_release(vcpu);
+
+ /*
+ * Wakeup VCPU if it was blocked so that it can
+ * run on other HARTs
+ */
+ if (csr_read(CSR_HGEIE) & BIT(i)) {
+ csr_clear(CSR_HGEIE, BIT(i));
+ kvm_vcpu_kick(vcpu);
+ }
+
+ raw_spin_lock_irqsave(&hgctrl->lock, flags);
+ }
+
+ raw_spin_unlock_irqrestore(&hgctrl->lock, flags);
}

int kvm_riscv_aia_init(void)
{
+ int rc;
+
if (!riscv_isa_extension_available(NULL, SxAIA))
return -ENODEV;

+ /* Figure-out number of bits in HGEIE */
+ csr_write(CSR_HGEIE, -1UL);
+ kvm_riscv_aia_nr_hgei = fls_long(csr_read(CSR_HGEIE));
+ csr_write(CSR_HGEIE, 0);
+ if (kvm_riscv_aia_nr_hgei)
+ kvm_riscv_aia_nr_hgei--;
+
+ /* Initialize guest external interrupt line management */
+ rc = aia_hgei_init();
+ if (rc)
+ return rc;
+
/* Enable KVM AIA support */
static_branch_enable(&kvm_riscv_aia_available);

@@ -380,4 +616,9 @@ int kvm_riscv_aia_init(void)

void kvm_riscv_aia_exit(void)
{
+ if (!kvm_riscv_aia_available())
+ return;
+
+ /* Cleanup the HGEI state */
+ aia_hgei_exit();
}
diff --git a/arch/riscv/kvm/main.c b/arch/riscv/kvm/main.c
index d8ff44eb04ca..5cf37dbe8a38 100644
--- a/arch/riscv/kvm/main.c
+++ b/arch/riscv/kvm/main.c
@@ -125,7 +125,8 @@ int kvm_arch_init(void *opaque)
kvm_info("VMID %ld bits available\n", kvm_riscv_gstage_vmid_bits());

if (kvm_riscv_aia_available())
- kvm_info("AIA available\n");
+ kvm_info("AIA available with %d guest external interrupts\n",
+ kvm_riscv_aia_nr_hgei);

return 0;
}
diff --git a/arch/riscv/kvm/vcpu.c b/arch/riscv/kvm/vcpu.c
index 151b35b3b05f..1daa1936b642 100644
--- a/arch/riscv/kvm/vcpu.c
+++ b/arch/riscv/kvm/vcpu.c
@@ -240,10 +240,12 @@ int kvm_cpu_has_pending_timer(struct kvm_vcpu *vcpu)

void kvm_arch_vcpu_blocking(struct kvm_vcpu *vcpu)
{
+ kvm_riscv_aia_wakeon_hgei(vcpu, true);
}

void kvm_arch_vcpu_unblocking(struct kvm_vcpu *vcpu)
{
+ kvm_riscv_aia_wakeon_hgei(vcpu, false);
}

int kvm_arch_vcpu_runnable(struct kvm_vcpu *vcpu)
--
2.34.1


2023-01-31 06:01:48

by Anup Patel

[permalink] [raw]
Subject: Re: [PATCH v2 0/7] RISC-V KVM virtualize AIA CSRs

Hi Palmer,

On Sat, Jan 28, 2023 at 12:57 PM Anup Patel <[email protected]> wrote:
>
> The RISC-V AIA specification is now frozen as-per the RISC-V international
> process. The latest frozen specifcation can be found at:
> https://github.com/riscv/riscv-aia/releases/download/1.0-RC1/riscv-interrupts-1.0-RC1.pdf
>
> This series implements first phase of AIA virtualization which targets
> virtualizing AIA CSRs. This also provides a foundation for the second
> phase of AIA virtualization which will target in-kernel AIA irqchip
> (including both IMSIC and APLIC).
>
> The first two patches are shared with the "Linux RISC-V AIA Support"
> series which adds AIA driver support.
>
> To test this series, use AIA drivers from the "Linux RISC-V AIA Support"
> series and use KVMTOOL from the riscv_aia_v1 branch at:
> https://github.com/avpatel/kvmtool.git
>
> These patches can also be found in the riscv_kvm_aia_csr_v2 branch at:
> https://github.com/avpatel/linux.git
>
> Changes since v1:
> - Addressed from Drew and Conor in PATCH1
> - Use alphabetical ordering for SMAIA and SSAIA enum in PATCH2
> - Use GENMASK() in PATCH3
>
> Anup Patel (7):
> RISC-V: Add AIA related CSR defines
> RISC-V: Detect AIA CSRs from ISA string
> RISC-V: KVM: Drop the _MASK suffix from hgatp.VMID mask defines
> RISC-V: KVM: Initial skeletal support for AIA
> RISC-V: KVM: Add ONE_REG interface for AIA CSRs
> RISC-V: KVM: Virtualize per-HART AIA CSRs
> RISC-V: KVM: Implement guest external interrupt line management

Can you please provide ACK for the first two patches so that I can
take it through the KVM RISC-V tree. ?

Regards,
Anup

>
> arch/riscv/include/asm/csr.h | 107 ++++-
> arch/riscv/include/asm/hwcap.h | 8 +
> arch/riscv/include/asm/kvm_aia.h | 137 +++++++
> arch/riscv/include/asm/kvm_host.h | 14 +-
> arch/riscv/include/uapi/asm/kvm.h | 22 +-
> arch/riscv/kernel/cpu.c | 2 +
> arch/riscv/kernel/cpufeature.c | 2 +
> arch/riscv/kvm/Makefile | 1 +
> arch/riscv/kvm/aia.c | 624 ++++++++++++++++++++++++++++++
> arch/riscv/kvm/main.c | 14 +
> arch/riscv/kvm/mmu.c | 3 +-
> arch/riscv/kvm/vcpu.c | 185 +++++++--
> arch/riscv/kvm/vcpu_insn.c | 4 +-
> arch/riscv/kvm/vm.c | 4 +
> arch/riscv/kvm/vmid.c | 4 +-
> 15 files changed, 1075 insertions(+), 56 deletions(-)
> create mode 100644 arch/riscv/include/asm/kvm_aia.h
> create mode 100644 arch/riscv/kvm/aia.c
>
> --
> 2.34.1
>

2023-01-31 09:22:41

by Atish Patra

[permalink] [raw]
Subject: Re: [PATCH v2 1/7] RISC-V: Add AIA related CSR defines

On Fri, Jan 27, 2023 at 11:27 PM Anup Patel <[email protected]> wrote:
>
> The RISC-V AIA specification improves handling per-HART local interrupts
> in a backward compatible manner. This patch adds defines for new RISC-V
> AIA CSRs.
>
> Signed-off-by: Anup Patel <[email protected]>
> Reviewed-by: Conor Dooley <[email protected]>
> Reviewed-by: Andrew Jones <[email protected]>
> ---
> arch/riscv/include/asm/csr.h | 95 +++++++++++++++++++++++++++++++++++-
> 1 file changed, 94 insertions(+), 1 deletion(-)
>
> diff --git a/arch/riscv/include/asm/csr.h b/arch/riscv/include/asm/csr.h
> index 0e571f6483d9..3c8d68152bce 100644
> --- a/arch/riscv/include/asm/csr.h
> +++ b/arch/riscv/include/asm/csr.h
> @@ -7,7 +7,7 @@
> #define _ASM_RISCV_CSR_H
>
> #include <asm/asm.h>
> -#include <linux/const.h>
> +#include <linux/bits.h>
>
> /* Status register flags */
> #define SR_SIE _AC(0x00000002, UL) /* Supervisor Interrupt Enable */
> @@ -73,7 +73,10 @@
> #define IRQ_S_EXT 9
> #define IRQ_VS_EXT 10
> #define IRQ_M_EXT 11
> +#define IRQ_S_GEXT 12
> #define IRQ_PMU_OVF 13
> +#define IRQ_LOCAL_MAX (IRQ_PMU_OVF + 1)
> +#define IRQ_LOCAL_MASK GENMASK((IRQ_LOCAL_MAX - 1), 0)
>
> /* Exception causes */
> #define EXC_INST_MISALIGNED 0
> @@ -156,6 +159,27 @@
> (_AC(1, UL) << IRQ_S_TIMER) | \
> (_AC(1, UL) << IRQ_S_EXT))
>
> +/* AIA CSR bits */
> +#define TOPI_IID_SHIFT 16
> +#define TOPI_IID_MASK GENMASK(11, 0)
> +#define TOPI_IPRIO_MASK GENMASK(7, 0)
> +#define TOPI_IPRIO_BITS 8
> +
> +#define TOPEI_ID_SHIFT 16
> +#define TOPEI_ID_MASK GENMASK(10, 0)
> +#define TOPEI_PRIO_MASK GENMASK(10, 0)
> +
> +#define ISELECT_IPRIO0 0x30
> +#define ISELECT_IPRIO15 0x3f
> +#define ISELECT_MASK GENMASK(8, 0)
> +
> +#define HVICTL_VTI BIT(30)
> +#define HVICTL_IID GENMASK(27, 16)
> +#define HVICTL_IID_SHIFT 16
> +#define HVICTL_DPR BIT(9)
> +#define HVICTL_IPRIOM BIT(8)
> +#define HVICTL_IPRIO GENMASK(7, 0)
> +
> /* xENVCFG flags */
> #define ENVCFG_STCE (_AC(1, ULL) << 63)
> #define ENVCFG_PBMTE (_AC(1, ULL) << 62)
> @@ -250,6 +274,18 @@
> #define CSR_STIMECMP 0x14D
> #define CSR_STIMECMPH 0x15D
>
> +/* Supervisor-Level Window to Indirectly Accessed Registers (AIA) */
> +#define CSR_SISELECT 0x150
> +#define CSR_SIREG 0x151
> +
> +/* Supervisor-Level Interrupts (AIA) */
> +#define CSR_STOPEI 0x15c
> +#define CSR_STOPI 0xdb0
> +
> +/* Supervisor-Level High-Half CSRs (AIA) */
> +#define CSR_SIEH 0x114
> +#define CSR_SIPH 0x154
> +
> #define CSR_VSSTATUS 0x200
> #define CSR_VSIE 0x204
> #define CSR_VSTVEC 0x205
> @@ -279,8 +315,32 @@
> #define CSR_HGATP 0x680
> #define CSR_HGEIP 0xe12
>
> +/* Virtual Interrupts and Interrupt Priorities (H-extension with AIA) */
> +#define CSR_HVIEN 0x608
> +#define CSR_HVICTL 0x609
> +#define CSR_HVIPRIO1 0x646
> +#define CSR_HVIPRIO2 0x647
> +
> +/* VS-Level Window to Indirectly Accessed Registers (H-extension with AIA) */
> +#define CSR_VSISELECT 0x250
> +#define CSR_VSIREG 0x251
> +
> +/* VS-Level Interrupts (H-extension with AIA) */
> +#define CSR_VSTOPEI 0x25c
> +#define CSR_VSTOPI 0xeb0
> +
> +/* Hypervisor and VS-Level High-Half CSRs (H-extension with AIA) */
> +#define CSR_HIDELEGH 0x613
> +#define CSR_HVIENH 0x618
> +#define CSR_HVIPH 0x655
> +#define CSR_HVIPRIO1H 0x656
> +#define CSR_HVIPRIO2H 0x657
> +#define CSR_VSIEH 0x214
> +#define CSR_VSIPH 0x254
> +
> #define CSR_MSTATUS 0x300
> #define CSR_MISA 0x301
> +#define CSR_MIDELEG 0x303
> #define CSR_MIE 0x304
> #define CSR_MTVEC 0x305
> #define CSR_MENVCFG 0x30a
> @@ -297,6 +357,25 @@
> #define CSR_MIMPID 0xf13
> #define CSR_MHARTID 0xf14
>
> +/* Machine-Level Window to Indirectly Accessed Registers (AIA) */
> +#define CSR_MISELECT 0x350
> +#define CSR_MIREG 0x351
> +
> +/* Machine-Level Interrupts (AIA) */
> +#define CSR_MTOPEI 0x35c
> +#define CSR_MTOPI 0xfb0
> +
> +/* Virtual Interrupts for Supervisor Level (AIA) */
> +#define CSR_MVIEN 0x308
> +#define CSR_MVIP 0x309
> +
> +/* Machine-Level High-Half CSRs (AIA) */
> +#define CSR_MIDELEGH 0x313
> +#define CSR_MIEH 0x314
> +#define CSR_MVIENH 0x318
> +#define CSR_MVIPH 0x319
> +#define CSR_MIPH 0x354
> +
> #ifdef CONFIG_RISCV_M_MODE
> # define CSR_STATUS CSR_MSTATUS
> # define CSR_IE CSR_MIE
> @@ -307,6 +386,13 @@
> # define CSR_TVAL CSR_MTVAL
> # define CSR_IP CSR_MIP
>
> +# define CSR_IEH CSR_MIEH
> +# define CSR_ISELECT CSR_MISELECT
> +# define CSR_IREG CSR_MIREG
> +# define CSR_IPH CSR_MIPH
> +# define CSR_TOPEI CSR_MTOPEI
> +# define CSR_TOPI CSR_MTOPI
> +
> # define SR_IE SR_MIE
> # define SR_PIE SR_MPIE
> # define SR_PP SR_MPP
> @@ -324,6 +410,13 @@
> # define CSR_TVAL CSR_STVAL
> # define CSR_IP CSR_SIP
>
> +# define CSR_IEH CSR_SIEH
> +# define CSR_ISELECT CSR_SISELECT
> +# define CSR_IREG CSR_SIREG
> +# define CSR_IPH CSR_SIPH
> +# define CSR_TOPEI CSR_STOPEI
> +# define CSR_TOPI CSR_STOPI
> +
> # define SR_IE SR_SIE
> # define SR_PIE SR_SPIE
> # define SR_PP SR_SPP
> --
> 2.34.1
>

LGTM.

Reviewed-by: Atish Patra <[email protected]>

--
Regards,
Atish

2023-01-31 09:26:15

by Atish Patra

[permalink] [raw]
Subject: Re: [PATCH v2 2/7] RISC-V: Detect AIA CSRs from ISA string

On Fri, Jan 27, 2023 at 11:27 PM Anup Patel <[email protected]> wrote:
>
> We have two extension names for AIA ISA support: Smaia (M-mode AIA CSRs)
> and Ssaia (S-mode AIA CSRs).
>
> We extend the ISA string parsing to detect Smaia and Ssaia extensions.
>
> Signed-off-by: Anup Patel <[email protected]>
> Reviewed-by: Andrew Jones <[email protected]>
> ---
> arch/riscv/include/asm/hwcap.h | 2 ++
> arch/riscv/kernel/cpu.c | 2 ++
> arch/riscv/kernel/cpufeature.c | 2 ++
> 3 files changed, 6 insertions(+)
>
> diff --git a/arch/riscv/include/asm/hwcap.h b/arch/riscv/include/asm/hwcap.h
> index 86328e3acb02..341ef30a3718 100644
> --- a/arch/riscv/include/asm/hwcap.h
> +++ b/arch/riscv/include/asm/hwcap.h
> @@ -59,6 +59,8 @@ enum riscv_isa_ext_id {
> RISCV_ISA_EXT_ZIHINTPAUSE,
> RISCV_ISA_EXT_SSTC,
> RISCV_ISA_EXT_SVINVAL,
> + RISCV_ISA_EXT_SMAIA,
> + RISCV_ISA_EXT_SSAIA,
> RISCV_ISA_EXT_ID_MAX
> };
> static_assert(RISCV_ISA_EXT_ID_MAX <= RISCV_ISA_EXT_MAX);
> diff --git a/arch/riscv/kernel/cpu.c b/arch/riscv/kernel/cpu.c
> index 1b9a5a66e55a..a215ec929160 100644
> --- a/arch/riscv/kernel/cpu.c
> +++ b/arch/riscv/kernel/cpu.c
> @@ -162,6 +162,8 @@ arch_initcall(riscv_cpuinfo_init);
> * extensions by an underscore.
> */
> static struct riscv_isa_ext_data isa_ext_arr[] = {
> + __RISCV_ISA_EXT_DATA(smaia, RISCV_ISA_EXT_SMAIA),
> + __RISCV_ISA_EXT_DATA(ssaia, RISCV_ISA_EXT_SSAIA),
> __RISCV_ISA_EXT_DATA(sscofpmf, RISCV_ISA_EXT_SSCOFPMF),
> __RISCV_ISA_EXT_DATA(sstc, RISCV_ISA_EXT_SSTC),
> __RISCV_ISA_EXT_DATA(svinval, RISCV_ISA_EXT_SVINVAL),
> diff --git a/arch/riscv/kernel/cpufeature.c b/arch/riscv/kernel/cpufeature.c
> index 93e45560af30..3c5b51f519d5 100644
> --- a/arch/riscv/kernel/cpufeature.c
> +++ b/arch/riscv/kernel/cpufeature.c
> @@ -228,6 +228,8 @@ void __init riscv_fill_hwcap(void)
> SET_ISA_EXT_MAP("zihintpause", RISCV_ISA_EXT_ZIHINTPAUSE);
> SET_ISA_EXT_MAP("sstc", RISCV_ISA_EXT_SSTC);
> SET_ISA_EXT_MAP("svinval", RISCV_ISA_EXT_SVINVAL);
> + SET_ISA_EXT_MAP("smaia", RISCV_ISA_EXT_SMAIA);
> + SET_ISA_EXT_MAP("ssaia", RISCV_ISA_EXT_SSAIA);
> }
> #undef SET_ISA_EXT_MAP
> }
> --
> 2.34.1
>


Reviewed-by: Atish Patra <[email protected]>
--
Regards,
Atish

2023-01-31 09:28:06

by Atish Patra

[permalink] [raw]
Subject: Re: [PATCH v2 3/7] RISC-V: KVM: Drop the _MASK suffix from hgatp.VMID mask defines

On Fri, Jan 27, 2023 at 11:28 PM Anup Patel <[email protected]> wrote:
>
> The hgatp.VMID mask defines are used before shifting when extracting
> VMID value from hgatp CSR value so based on the convention followed
> in the other parts of asm/csr.h, the hgatp.VMID mask defines should
> not have a _MASK suffix.
>
> While we are here, let's use GENMASK() for hgatp.VMID and hgatp.PPN.
>
> Signed-off-by: Anup Patel <[email protected]>
> Reviewed-by: Andrew Jones <[email protected]>
> ---
> arch/riscv/include/asm/csr.h | 12 ++++++------
> arch/riscv/kvm/mmu.c | 3 +--
> arch/riscv/kvm/vmid.c | 4 ++--
> 3 files changed, 9 insertions(+), 10 deletions(-)
>
> diff --git a/arch/riscv/include/asm/csr.h b/arch/riscv/include/asm/csr.h
> index 3c8d68152bce..3176355cf4e9 100644
> --- a/arch/riscv/include/asm/csr.h
> +++ b/arch/riscv/include/asm/csr.h
> @@ -131,25 +131,25 @@
>
> #define HGATP32_MODE_SHIFT 31
> #define HGATP32_VMID_SHIFT 22
> -#define HGATP32_VMID_MASK _AC(0x1FC00000, UL)
> -#define HGATP32_PPN _AC(0x003FFFFF, UL)
> +#define HGATP32_VMID GENMASK(28, 22)
> +#define HGATP32_PPN GENMASK(21, 0)
>
> #define HGATP64_MODE_SHIFT 60
> #define HGATP64_VMID_SHIFT 44
> -#define HGATP64_VMID_MASK _AC(0x03FFF00000000000, UL)
> -#define HGATP64_PPN _AC(0x00000FFFFFFFFFFF, UL)
> +#define HGATP64_VMID GENMASK(57, 44)
> +#define HGATP64_PPN GENMASK(43, 0)
>
> #define HGATP_PAGE_SHIFT 12
>
> #ifdef CONFIG_64BIT
> #define HGATP_PPN HGATP64_PPN
> #define HGATP_VMID_SHIFT HGATP64_VMID_SHIFT
> -#define HGATP_VMID_MASK HGATP64_VMID_MASK
> +#define HGATP_VMID HGATP64_VMID
> #define HGATP_MODE_SHIFT HGATP64_MODE_SHIFT
> #else
> #define HGATP_PPN HGATP32_PPN
> #define HGATP_VMID_SHIFT HGATP32_VMID_SHIFT
> -#define HGATP_VMID_MASK HGATP32_VMID_MASK
> +#define HGATP_VMID HGATP32_VMID
> #define HGATP_MODE_SHIFT HGATP32_MODE_SHIFT
> #endif
>
> diff --git a/arch/riscv/kvm/mmu.c b/arch/riscv/kvm/mmu.c
> index dbc4ca060174..829a7065ae01 100644
> --- a/arch/riscv/kvm/mmu.c
> +++ b/arch/riscv/kvm/mmu.c
> @@ -748,8 +748,7 @@ void kvm_riscv_gstage_update_hgatp(struct kvm_vcpu *vcpu)
> unsigned long hgatp = gstage_mode;
> struct kvm_arch *k = &vcpu->kvm->arch;
>
> - hgatp |= (READ_ONCE(k->vmid.vmid) << HGATP_VMID_SHIFT) &
> - HGATP_VMID_MASK;
> + hgatp |= (READ_ONCE(k->vmid.vmid) << HGATP_VMID_SHIFT) & HGATP_VMID;
> hgatp |= (k->pgd_phys >> PAGE_SHIFT) & HGATP_PPN;
>
> csr_write(CSR_HGATP, hgatp);
> diff --git a/arch/riscv/kvm/vmid.c b/arch/riscv/kvm/vmid.c
> index 6cd93995fb65..6f4d4979a759 100644
> --- a/arch/riscv/kvm/vmid.c
> +++ b/arch/riscv/kvm/vmid.c
> @@ -26,9 +26,9 @@ void kvm_riscv_gstage_vmid_detect(void)
>
> /* Figure-out number of VMID bits in HW */
> old = csr_read(CSR_HGATP);
> - csr_write(CSR_HGATP, old | HGATP_VMID_MASK);
> + csr_write(CSR_HGATP, old | HGATP_VMID);
> vmid_bits = csr_read(CSR_HGATP);
> - vmid_bits = (vmid_bits & HGATP_VMID_MASK) >> HGATP_VMID_SHIFT;
> + vmid_bits = (vmid_bits & HGATP_VMID) >> HGATP_VMID_SHIFT;
> vmid_bits = fls_long(vmid_bits);
> csr_write(CSR_HGATP, old);
>
> --
> 2.34.1
>

Reviewed-by: Atish Patra <[email protected]>

--
Regards,
Atish

2023-01-31 09:51:57

by Atish Patra

[permalink] [raw]
Subject: Re: [PATCH v2 4/7] RISC-V: KVM: Initial skeletal support for AIA

On Fri, Jan 27, 2023 at 11:28 PM Anup Patel <[email protected]> wrote:
>
> To incrementally implement AIA support, we first add minimal skeletal
> support which only compiles and detects AIA hardware support at the
> boot-time but does not provide any functionality.
>
> Signed-off-by: Anup Patel <[email protected]>
> ---
> arch/riscv/include/asm/hwcap.h | 6 ++
> arch/riscv/include/asm/kvm_aia.h | 109 ++++++++++++++++++++++++++++++
> arch/riscv/include/asm/kvm_host.h | 7 ++
> arch/riscv/kvm/Makefile | 1 +
> arch/riscv/kvm/aia.c | 66 ++++++++++++++++++
> arch/riscv/kvm/main.c | 13 ++++
> arch/riscv/kvm/vcpu.c | 40 ++++++++++-
> arch/riscv/kvm/vcpu_insn.c | 4 +-
> arch/riscv/kvm/vm.c | 4 ++
> 9 files changed, 246 insertions(+), 4 deletions(-)
> create mode 100644 arch/riscv/include/asm/kvm_aia.h
> create mode 100644 arch/riscv/kvm/aia.c
>
> diff --git a/arch/riscv/include/asm/hwcap.h b/arch/riscv/include/asm/hwcap.h
> index 341ef30a3718..a03d51df517b 100644
> --- a/arch/riscv/include/asm/hwcap.h
> +++ b/arch/riscv/include/asm/hwcap.h
> @@ -65,6 +65,12 @@ enum riscv_isa_ext_id {
> };
> static_assert(RISCV_ISA_EXT_ID_MAX <= RISCV_ISA_EXT_MAX);
>
> +#ifdef CONFIG_RISCV_M_MODE
> +#define RISCV_ISA_EXT_SxAIA RISCV_ISA_EXT_SMAIA
> +#else
> +#define RISCV_ISA_EXT_SxAIA RISCV_ISA_EXT_SSAIA
> +#endif
> +
> /*
> * This enum represents the logical ID for each RISC-V ISA extension static
> * keys. We can use static key to optimize code path if some ISA extensions
> diff --git a/arch/riscv/include/asm/kvm_aia.h b/arch/riscv/include/asm/kvm_aia.h
> new file mode 100644
> index 000000000000..258a835d4c32
> --- /dev/null
> +++ b/arch/riscv/include/asm/kvm_aia.h
> @@ -0,0 +1,109 @@
> +/* SPDX-License-Identifier: GPL-2.0-only */
> +/*
> + * Copyright (C) 2021 Western Digital Corporation or its affiliates.
> + * Copyright (C) 2022 Ventana Micro Systems Inc.
> + *
> + * Authors:
> + * Anup Patel <[email protected]>
> + */
> +
> +#ifndef __KVM_RISCV_AIA_H
> +#define __KVM_RISCV_AIA_H
> +
> +#include <linux/jump_label.h>
> +#include <linux/kvm_types.h>
> +
> +struct kvm_aia {
> + /* In-kernel irqchip created */
> + bool in_kernel;
> +
> + /* In-kernel irqchip initialized */
> + bool initialized;
> +};
> +
> +struct kvm_vcpu_aia {
> +};
> +
> +#define kvm_riscv_aia_initialized(k) ((k)->arch.aia.initialized)
> +
> +#define irqchip_in_kernel(k) ((k)->arch.aia.in_kernel)
> +
> +DECLARE_STATIC_KEY_FALSE(kvm_riscv_aia_available);
> +#define kvm_riscv_aia_available() \
> + static_branch_unlikely(&kvm_riscv_aia_available)
> +
> +static inline void kvm_riscv_vcpu_aia_flush_interrupts(struct kvm_vcpu *vcpu)
> +{
> +}
> +
> +static inline void kvm_riscv_vcpu_aia_sync_interrupts(struct kvm_vcpu *vcpu)
> +{
> +}
> +
> +static inline bool kvm_riscv_vcpu_aia_has_interrupts(struct kvm_vcpu *vcpu,
> + u64 mask)
> +{
> + return false;
> +}
> +
> +static inline void kvm_riscv_vcpu_aia_update_hvip(struct kvm_vcpu *vcpu)
> +{
> +}
> +
> +static inline void kvm_riscv_vcpu_aia_load(struct kvm_vcpu *vcpu, int cpu)
> +{
> +}
> +
> +static inline void kvm_riscv_vcpu_aia_put(struct kvm_vcpu *vcpu)
> +{
> +}
> +
> +static inline int kvm_riscv_vcpu_aia_get_csr(struct kvm_vcpu *vcpu,
> + unsigned long reg_num,
> + unsigned long *out_val)
> +{
> + *out_val = 0;
> + return 0;
> +}
> +
> +static inline int kvm_riscv_vcpu_aia_set_csr(struct kvm_vcpu *vcpu,
> + unsigned long reg_num,
> + unsigned long val)
> +{
> + return 0;
> +}
> +
> +#define KVM_RISCV_VCPU_AIA_CSR_FUNCS
> +
> +static inline int kvm_riscv_vcpu_aia_update(struct kvm_vcpu *vcpu)
> +{
> + return 1;
> +}
> +
> +static inline void kvm_riscv_vcpu_aia_reset(struct kvm_vcpu *vcpu)
> +{
> +}
> +
> +static inline int kvm_riscv_vcpu_aia_init(struct kvm_vcpu *vcpu)
> +{
> + return 0;
> +}
> +
> +static inline void kvm_riscv_vcpu_aia_deinit(struct kvm_vcpu *vcpu)
> +{
> +}
> +
> +static inline void kvm_riscv_aia_init_vm(struct kvm *kvm)
> +{
> +}
> +
> +static inline void kvm_riscv_aia_destroy_vm(struct kvm *kvm)
> +{
> +}
> +
> +void kvm_riscv_aia_enable(void);
> +void kvm_riscv_aia_disable(void);
> +int kvm_riscv_aia_init(void);
> +void kvm_riscv_aia_exit(void);
> +
> +#endif
> diff --git a/arch/riscv/include/asm/kvm_host.h b/arch/riscv/include/asm/kvm_host.h
> index 93f43a3e7886..8776e7a465c7 100644
> --- a/arch/riscv/include/asm/kvm_host.h
> +++ b/arch/riscv/include/asm/kvm_host.h
> @@ -14,6 +14,7 @@
> #include <linux/kvm_types.h>
> #include <linux/spinlock.h>
> #include <asm/hwcap.h>
> +#include <asm/kvm_aia.h>
> #include <asm/kvm_vcpu_fp.h>
> #include <asm/kvm_vcpu_insn.h>
> #include <asm/kvm_vcpu_sbi.h>
> @@ -93,6 +94,9 @@ struct kvm_arch {
>
> /* Guest Timer */
> struct kvm_guest_timer timer;
> +
> + /* AIA Guest/VM context */
> + struct kvm_aia aia;
> };
>
> struct kvm_cpu_trap {
> @@ -220,6 +224,9 @@ struct kvm_vcpu_arch {
> /* SBI context */
> struct kvm_vcpu_sbi_context sbi_context;
>
> + /* AIA VCPU context */
> + struct kvm_vcpu_aia aia_context;
> +
> /* Cache pages needed to program page tables with spinlock held */
> struct kvm_mmu_memory_cache mmu_page_cache;
>
> diff --git a/arch/riscv/kvm/Makefile b/arch/riscv/kvm/Makefile
> index 019df9208bdd..adbc85a94364 100644
> --- a/arch/riscv/kvm/Makefile
> +++ b/arch/riscv/kvm/Makefile
> @@ -25,3 +25,4 @@ kvm-y += vcpu_sbi_base.o
> kvm-y += vcpu_sbi_replace.o
> kvm-y += vcpu_sbi_hsm.o
> kvm-y += vcpu_timer.o
> +kvm-y += aia.o
> diff --git a/arch/riscv/kvm/aia.c b/arch/riscv/kvm/aia.c
> new file mode 100644
> index 000000000000..7a633331cd3e
> --- /dev/null
> +++ b/arch/riscv/kvm/aia.c
> @@ -0,0 +1,66 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (C) 2021 Western Digital Corporation or its affiliates.
> + * Copyright (C) 2022 Ventana Micro Systems Inc.
> + *
> + * Authors:
> + * Anup Patel <[email protected]>
> + */
> +
> +#include <linux/kvm_host.h>
> +#include <asm/hwcap.h>
> +
> +DEFINE_STATIC_KEY_FALSE(kvm_riscv_aia_available);
> +
> +static void aia_set_hvictl(bool ext_irq_pending)
> +{
> + unsigned long hvictl;
> +
> + /*
> + * HVICTL.IID == 9 and HVICTL.IPRIO == 0 represents
> + * no interrupt in HVICTL.
> + */
> +
> + hvictl = (IRQ_S_EXT << HVICTL_IID_SHIFT) & HVICTL_IID;
> + hvictl |= ext_irq_pending;
> + csr_write(CSR_HVICTL, hvictl);
> +}
> +
> +void kvm_riscv_aia_enable(void)
> +{
> + if (!kvm_riscv_aia_available())
> + return;
> +
> + aia_set_hvictl(false);
> + csr_write(CSR_HVIPRIO1, 0x0);
> + csr_write(CSR_HVIPRIO2, 0x0);
> +#ifdef CONFIG_32BIT
> + csr_write(CSR_HVIPH, 0x0);
> + csr_write(CSR_HIDELEGH, 0x0);
> + csr_write(CSR_HVIPRIO1H, 0x0);
> + csr_write(CSR_HVIPRIO2H, 0x0);
> +#endif
> +}
> +
> +void kvm_riscv_aia_disable(void)
> +{
> + if (!kvm_riscv_aia_available())
> + return;
> +
> + aia_set_hvictl(false);
> +}
> +
> +int kvm_riscv_aia_init(void)
> +{
> + if (!riscv_isa_extension_available(NULL, SxAIA))
> + return -ENODEV;
> +
> + /* Enable KVM AIA support */
> + static_branch_enable(&kvm_riscv_aia_available);
> +
> + return 0;
> +}
> +
> +void kvm_riscv_aia_exit(void)
> +{
> +}
> diff --git a/arch/riscv/kvm/main.c b/arch/riscv/kvm/main.c
> index 58c5489d3031..d8ff44eb04ca 100644
> --- a/arch/riscv/kvm/main.c
> +++ b/arch/riscv/kvm/main.c
> @@ -53,11 +53,15 @@ int kvm_arch_hardware_enable(void)
>
> csr_write(CSR_HVIP, 0);
>
> + kvm_riscv_aia_enable();
> +
> return 0;
> }
>
> void kvm_arch_hardware_disable(void)
> {
> + kvm_riscv_aia_disable();
> +
> /*
> * After clearing the hideleg CSR, the host kernel will receive
> * spurious interrupts if hvip CSR has pending interrupts and the
> @@ -72,6 +76,7 @@ void kvm_arch_hardware_disable(void)
>
> int kvm_arch_init(void *opaque)
> {
> + int rc;
> const char *str;
>
> if (!riscv_isa_extension_available(NULL, h)) {
> @@ -93,6 +98,10 @@ int kvm_arch_init(void *opaque)
>
> kvm_riscv_gstage_vmid_detect();
>
> + rc = kvm_riscv_aia_init();
> + if (rc && rc != -ENODEV)
> + return rc;
> +
> kvm_info("hypervisor extension available\n");
>
> switch (kvm_riscv_gstage_mode()) {
> @@ -115,11 +124,15 @@ int kvm_arch_init(void *opaque)
>
> kvm_info("VMID %ld bits available\n", kvm_riscv_gstage_vmid_bits());
>
> + if (kvm_riscv_aia_available())
> + kvm_info("AIA available\n");
> +
> return 0;
> }
>
> void kvm_arch_exit(void)
> {
> + kvm_riscv_aia_exit();
> }
>
> static int __init riscv_kvm_init(void)
> diff --git a/arch/riscv/kvm/vcpu.c b/arch/riscv/kvm/vcpu.c
> index 2260adaf2de8..3cf50eadc8ce 100644
> --- a/arch/riscv/kvm/vcpu.c
> +++ b/arch/riscv/kvm/vcpu.c
> @@ -135,6 +135,8 @@ static void kvm_riscv_reset_vcpu(struct kvm_vcpu *vcpu)
>
> kvm_riscv_vcpu_timer_reset(vcpu);
>
> + kvm_riscv_vcpu_aia_reset(vcpu);
> +
> WRITE_ONCE(vcpu->arch.irqs_pending, 0);
> WRITE_ONCE(vcpu->arch.irqs_pending_mask, 0);
>
> @@ -155,6 +157,7 @@ int kvm_arch_vcpu_precreate(struct kvm *kvm, unsigned int id)
>
> int kvm_arch_vcpu_create(struct kvm_vcpu *vcpu)
> {
> + int rc;
> struct kvm_cpu_context *cntx;
> struct kvm_vcpu_csr *reset_csr = &vcpu->arch.guest_reset_csr;
> unsigned long host_isa, i;
> @@ -194,6 +197,11 @@ int kvm_arch_vcpu_create(struct kvm_vcpu *vcpu)
> /* Setup VCPU timer */
> kvm_riscv_vcpu_timer_init(vcpu);
>
> + /* Setup VCPU AIA */
> + rc = kvm_riscv_vcpu_aia_init(vcpu);
> + if (rc)
> + return rc;
> +
> /* Reset VCPU */
> kvm_riscv_reset_vcpu(vcpu);
>
> @@ -213,6 +221,9 @@ void kvm_arch_vcpu_postcreate(struct kvm_vcpu *vcpu)
>
> void kvm_arch_vcpu_destroy(struct kvm_vcpu *vcpu)
> {
> + /* Cleanup VCPU AIA context */
> + kvm_riscv_vcpu_aia_deinit(vcpu);
> +
> /* Cleanup VCPU timer */
> kvm_riscv_vcpu_timer_deinit(vcpu);
>
> @@ -730,6 +741,9 @@ void kvm_riscv_vcpu_flush_interrupts(struct kvm_vcpu *vcpu)
> csr->hvip &= ~mask;
> csr->hvip |= val;
> }
> +
> + /* Flush AIA high interrupts */
> + kvm_riscv_vcpu_aia_flush_interrupts(vcpu);
> }
>
> void kvm_riscv_vcpu_sync_interrupts(struct kvm_vcpu *vcpu)
> @@ -755,6 +769,9 @@ void kvm_riscv_vcpu_sync_interrupts(struct kvm_vcpu *vcpu)
> }
> }
>
> + /* Sync-up AIA high interrupts */
> + kvm_riscv_vcpu_aia_sync_interrupts(vcpu);
> +
> /* Sync-up timer CSRs */
> kvm_riscv_vcpu_timer_sync(vcpu);
> }
> @@ -791,10 +808,15 @@ int kvm_riscv_vcpu_unset_interrupt(struct kvm_vcpu *vcpu, unsigned int irq)
>
> bool kvm_riscv_vcpu_has_interrupts(struct kvm_vcpu *vcpu, unsigned long mask)
> {
> - unsigned long ie = ((vcpu->arch.guest_csr.vsie & VSIP_VALID_MASK)
> - << VSIP_TO_HVIP_SHIFT) & mask;
> + unsigned long ie;
> +
> + ie = ((vcpu->arch.guest_csr.vsie & VSIP_VALID_MASK)
> + << VSIP_TO_HVIP_SHIFT) & mask;
> + if (READ_ONCE(vcpu->arch.irqs_pending) & ie)
> + return true;
>
> - return (READ_ONCE(vcpu->arch.irqs_pending) & ie) ? true : false;
> + /* Check AIA high interrupts */
> + return kvm_riscv_vcpu_aia_has_interrupts(vcpu, mask);
> }
>
> void kvm_riscv_vcpu_power_off(struct kvm_vcpu *vcpu)
> @@ -890,6 +912,8 @@ void kvm_arch_vcpu_load(struct kvm_vcpu *vcpu, int cpu)
> kvm_riscv_vcpu_guest_fp_restore(&vcpu->arch.guest_context,
> vcpu->arch.isa);
>
> + kvm_riscv_vcpu_aia_load(vcpu, cpu);
> +
> vcpu->cpu = cpu;
> }
>
> @@ -899,6 +923,8 @@ void kvm_arch_vcpu_put(struct kvm_vcpu *vcpu)
>
> vcpu->cpu = -1;
>
> + kvm_riscv_vcpu_aia_put(vcpu);
> +
> kvm_riscv_vcpu_guest_fp_save(&vcpu->arch.guest_context,
> vcpu->arch.isa);
> kvm_riscv_vcpu_host_fp_restore(&vcpu->arch.host_context);
> @@ -966,6 +992,7 @@ static void kvm_riscv_update_hvip(struct kvm_vcpu *vcpu)
> struct kvm_vcpu_csr *csr = &vcpu->arch.guest_csr;
>
> csr_write(CSR_HVIP, csr->hvip);
> + kvm_riscv_vcpu_aia_update_hvip(vcpu);
> }
>
> /*
> @@ -1040,6 +1067,13 @@ int kvm_arch_vcpu_ioctl_run(struct kvm_vcpu *vcpu)
>
> local_irq_disable();
>
> + /* Update AIA HW state before entering guest */
> + ret = kvm_riscv_vcpu_aia_update(vcpu);
> + if (ret <= 0) {
> + local_irq_enable();
> + continue;
> + }
> +
> /*
> * Ensure we set mode to IN_GUEST_MODE after we disable
> * interrupts and before the final VCPU requests check.
> diff --git a/arch/riscv/kvm/vcpu_insn.c b/arch/riscv/kvm/vcpu_insn.c
> index 0bb52761a3f7..07e8c121922b 100644
> --- a/arch/riscv/kvm/vcpu_insn.c
> +++ b/arch/riscv/kvm/vcpu_insn.c
> @@ -213,7 +213,9 @@ struct csr_func {
> unsigned long wr_mask);
> };
>
> -static const struct csr_func csr_funcs[] = { };
> +static const struct csr_func csr_funcs[] = {
> + KVM_RISCV_VCPU_AIA_CSR_FUNCS
> +};
>
> /**
> * kvm_riscv_vcpu_csr_return -- Handle CSR read/write after user space
> diff --git a/arch/riscv/kvm/vm.c b/arch/riscv/kvm/vm.c
> index 65a964d7e70d..bc03d2ddcb51 100644
> --- a/arch/riscv/kvm/vm.c
> +++ b/arch/riscv/kvm/vm.c
> @@ -41,6 +41,8 @@ int kvm_arch_init_vm(struct kvm *kvm, unsigned long type)
> return r;
> }
>
> + kvm_riscv_aia_init_vm(kvm);
> +
> kvm_riscv_guest_timer_init(kvm);
>
> return 0;
> @@ -49,6 +51,8 @@ int kvm_arch_init_vm(struct kvm *kvm, unsigned long type)
> void kvm_arch_destroy_vm(struct kvm *kvm)
> {
> kvm_destroy_vcpus(kvm);
> +
> + kvm_riscv_aia_destroy_vm(kvm);
> }
>
> int kvm_vm_ioctl_check_extension(struct kvm *kvm, long ext)
> --
> 2.34.1
>

LGTM.
Reviewed-by: Atish Patra <[email protected]>

--
Regards,
Atish

2023-02-03 00:24:55

by Palmer Dabbelt

[permalink] [raw]
Subject: Re: [PATCH v2 1/7] RISC-V: Add AIA related CSR defines

On Fri, 27 Jan 2023 23:27:31 PST (-0800), [email protected] wrote:
> The RISC-V AIA specification improves handling per-HART local interrupts
> in a backward compatible manner. This patch adds defines for new RISC-V
> AIA CSRs.
>
> Signed-off-by: Anup Patel <[email protected]>
> Reviewed-by: Conor Dooley <[email protected]>
> Reviewed-by: Andrew Jones <[email protected]>
> ---
> arch/riscv/include/asm/csr.h | 95 +++++++++++++++++++++++++++++++++++-
> 1 file changed, 94 insertions(+), 1 deletion(-)
>
> diff --git a/arch/riscv/include/asm/csr.h b/arch/riscv/include/asm/csr.h
> index 0e571f6483d9..3c8d68152bce 100644
> --- a/arch/riscv/include/asm/csr.h
> +++ b/arch/riscv/include/asm/csr.h
> @@ -7,7 +7,7 @@
> #define _ASM_RISCV_CSR_H
>
> #include <asm/asm.h>
> -#include <linux/const.h>
> +#include <linux/bits.h>
>
> /* Status register flags */
> #define SR_SIE _AC(0x00000002, UL) /* Supervisor Interrupt Enable */
> @@ -73,7 +73,10 @@
> #define IRQ_S_EXT 9
> #define IRQ_VS_EXT 10
> #define IRQ_M_EXT 11
> +#define IRQ_S_GEXT 12
> #define IRQ_PMU_OVF 13
> +#define IRQ_LOCAL_MAX (IRQ_PMU_OVF + 1)
> +#define IRQ_LOCAL_MASK GENMASK((IRQ_LOCAL_MAX - 1), 0)
>
> /* Exception causes */
> #define EXC_INST_MISALIGNED 0
> @@ -156,6 +159,27 @@
> (_AC(1, UL) << IRQ_S_TIMER) | \
> (_AC(1, UL) << IRQ_S_EXT))
>
> +/* AIA CSR bits */
> +#define TOPI_IID_SHIFT 16
> +#define TOPI_IID_MASK GENMASK(11, 0)
> +#define TOPI_IPRIO_MASK GENMASK(7, 0)
> +#define TOPI_IPRIO_BITS 8
> +
> +#define TOPEI_ID_SHIFT 16
> +#define TOPEI_ID_MASK GENMASK(10, 0)
> +#define TOPEI_PRIO_MASK GENMASK(10, 0)
> +
> +#define ISELECT_IPRIO0 0x30
> +#define ISELECT_IPRIO15 0x3f
> +#define ISELECT_MASK GENMASK(8, 0)
> +
> +#define HVICTL_VTI BIT(30)
> +#define HVICTL_IID GENMASK(27, 16)
> +#define HVICTL_IID_SHIFT 16
> +#define HVICTL_DPR BIT(9)
> +#define HVICTL_IPRIOM BIT(8)
> +#define HVICTL_IPRIO GENMASK(7, 0)
> +
> /* xENVCFG flags */
> #define ENVCFG_STCE (_AC(1, ULL) << 63)
> #define ENVCFG_PBMTE (_AC(1, ULL) << 62)
> @@ -250,6 +274,18 @@
> #define CSR_STIMECMP 0x14D
> #define CSR_STIMECMPH 0x15D
>
> +/* Supervisor-Level Window to Indirectly Accessed Registers (AIA) */
> +#define CSR_SISELECT 0x150
> +#define CSR_SIREG 0x151
> +
> +/* Supervisor-Level Interrupts (AIA) */
> +#define CSR_STOPEI 0x15c
> +#define CSR_STOPI 0xdb0
> +
> +/* Supervisor-Level High-Half CSRs (AIA) */
> +#define CSR_SIEH 0x114
> +#define CSR_SIPH 0x154
> +
> #define CSR_VSSTATUS 0x200
> #define CSR_VSIE 0x204
> #define CSR_VSTVEC 0x205
> @@ -279,8 +315,32 @@
> #define CSR_HGATP 0x680
> #define CSR_HGEIP 0xe12
>
> +/* Virtual Interrupts and Interrupt Priorities (H-extension with AIA) */
> +#define CSR_HVIEN 0x608
> +#define CSR_HVICTL 0x609
> +#define CSR_HVIPRIO1 0x646
> +#define CSR_HVIPRIO2 0x647
> +
> +/* VS-Level Window to Indirectly Accessed Registers (H-extension with AIA) */
> +#define CSR_VSISELECT 0x250
> +#define CSR_VSIREG 0x251
> +
> +/* VS-Level Interrupts (H-extension with AIA) */
> +#define CSR_VSTOPEI 0x25c
> +#define CSR_VSTOPI 0xeb0
> +
> +/* Hypervisor and VS-Level High-Half CSRs (H-extension with AIA) */
> +#define CSR_HIDELEGH 0x613
> +#define CSR_HVIENH 0x618
> +#define CSR_HVIPH 0x655
> +#define CSR_HVIPRIO1H 0x656
> +#define CSR_HVIPRIO2H 0x657
> +#define CSR_VSIEH 0x214
> +#define CSR_VSIPH 0x254
> +
> #define CSR_MSTATUS 0x300
> #define CSR_MISA 0x301
> +#define CSR_MIDELEG 0x303
> #define CSR_MIE 0x304
> #define CSR_MTVEC 0x305
> #define CSR_MENVCFG 0x30a
> @@ -297,6 +357,25 @@
> #define CSR_MIMPID 0xf13
> #define CSR_MHARTID 0xf14
>
> +/* Machine-Level Window to Indirectly Accessed Registers (AIA) */
> +#define CSR_MISELECT 0x350
> +#define CSR_MIREG 0x351
> +
> +/* Machine-Level Interrupts (AIA) */
> +#define CSR_MTOPEI 0x35c
> +#define CSR_MTOPI 0xfb0
> +
> +/* Virtual Interrupts for Supervisor Level (AIA) */
> +#define CSR_MVIEN 0x308
> +#define CSR_MVIP 0x309
> +
> +/* Machine-Level High-Half CSRs (AIA) */
> +#define CSR_MIDELEGH 0x313
> +#define CSR_MIEH 0x314
> +#define CSR_MVIENH 0x318
> +#define CSR_MVIPH 0x319
> +#define CSR_MIPH 0x354
> +
> #ifdef CONFIG_RISCV_M_MODE
> # define CSR_STATUS CSR_MSTATUS
> # define CSR_IE CSR_MIE
> @@ -307,6 +386,13 @@
> # define CSR_TVAL CSR_MTVAL
> # define CSR_IP CSR_MIP
>
> +# define CSR_IEH CSR_MIEH
> +# define CSR_ISELECT CSR_MISELECT
> +# define CSR_IREG CSR_MIREG
> +# define CSR_IPH CSR_MIPH
> +# define CSR_TOPEI CSR_MTOPEI
> +# define CSR_TOPI CSR_MTOPI
> +
> # define SR_IE SR_MIE
> # define SR_PIE SR_MPIE
> # define SR_PP SR_MPP
> @@ -324,6 +410,13 @@
> # define CSR_TVAL CSR_STVAL
> # define CSR_IP CSR_SIP
>
> +# define CSR_IEH CSR_SIEH
> +# define CSR_ISELECT CSR_SISELECT
> +# define CSR_IREG CSR_SIREG
> +# define CSR_IPH CSR_SIPH
> +# define CSR_TOPEI CSR_STOPEI
> +# define CSR_TOPI CSR_STOPI
> +
> # define SR_IE SR_SIE
> # define SR_PIE SR_SPIE
> # define SR_PP SR_SPP

Acked-by: Palmer Dabbelt <[email protected]>

2023-02-03 00:24:58

by Palmer Dabbelt

[permalink] [raw]
Subject: Re: [PATCH v2 2/7] RISC-V: Detect AIA CSRs from ISA string

On Fri, 27 Jan 2023 23:27:32 PST (-0800), [email protected] wrote:
> We have two extension names for AIA ISA support: Smaia (M-mode AIA CSRs)
> and Ssaia (S-mode AIA CSRs).

This has pretty much the same problem that we had with the other
AIA-related ISA string patches, where there's that ambiguity with the
non-ratified chapters. IIRC when this came up in GCC the rough idea was
to try and document that we're going to interpret the standard ISA
strings that way, but now that we're doing custom ISA extensions it
seems saner to just define on here that removes the ambiguity.

I just sent
<https://lore.kernel.org/r/[email protected]/>
which documents that.

> We extend the ISA string parsing to detect Smaia and Ssaia extensions.
>
> Signed-off-by: Anup Patel <[email protected]>
> Reviewed-by: Andrew Jones <[email protected]>
> ---
> arch/riscv/include/asm/hwcap.h | 2 ++
> arch/riscv/kernel/cpu.c | 2 ++
> arch/riscv/kernel/cpufeature.c | 2 ++
> 3 files changed, 6 insertions(+)
>
> diff --git a/arch/riscv/include/asm/hwcap.h b/arch/riscv/include/asm/hwcap.h
> index 86328e3acb02..341ef30a3718 100644
> --- a/arch/riscv/include/asm/hwcap.h
> +++ b/arch/riscv/include/asm/hwcap.h
> @@ -59,6 +59,8 @@ enum riscv_isa_ext_id {
> RISCV_ISA_EXT_ZIHINTPAUSE,
> RISCV_ISA_EXT_SSTC,
> RISCV_ISA_EXT_SVINVAL,
> + RISCV_ISA_EXT_SMAIA,
> + RISCV_ISA_EXT_SSAIA,
> RISCV_ISA_EXT_ID_MAX
> };
> static_assert(RISCV_ISA_EXT_ID_MAX <= RISCV_ISA_EXT_MAX);
> diff --git a/arch/riscv/kernel/cpu.c b/arch/riscv/kernel/cpu.c
> index 1b9a5a66e55a..a215ec929160 100644
> --- a/arch/riscv/kernel/cpu.c
> +++ b/arch/riscv/kernel/cpu.c
> @@ -162,6 +162,8 @@ arch_initcall(riscv_cpuinfo_init);
> * extensions by an underscore.
> */
> static struct riscv_isa_ext_data isa_ext_arr[] = {
> + __RISCV_ISA_EXT_DATA(smaia, RISCV_ISA_EXT_SMAIA),
> + __RISCV_ISA_EXT_DATA(ssaia, RISCV_ISA_EXT_SSAIA),

This will conflict with that ISA string refactoring I just merged. It
should be a pretty mechanical merge conflict, but if you want we can do
a shared tag with the first few patches and I can handle the merge
conflict locally.

> __RISCV_ISA_EXT_DATA(sscofpmf, RISCV_ISA_EXT_SSCOFPMF),
> __RISCV_ISA_EXT_DATA(sstc, RISCV_ISA_EXT_SSTC),
> __RISCV_ISA_EXT_DATA(svinval, RISCV_ISA_EXT_SVINVAL),
> diff --git a/arch/riscv/kernel/cpufeature.c b/arch/riscv/kernel/cpufeature.c
> index 93e45560af30..3c5b51f519d5 100644
> --- a/arch/riscv/kernel/cpufeature.c
> +++ b/arch/riscv/kernel/cpufeature.c
> @@ -228,6 +228,8 @@ void __init riscv_fill_hwcap(void)
> SET_ISA_EXT_MAP("zihintpause", RISCV_ISA_EXT_ZIHINTPAUSE);
> SET_ISA_EXT_MAP("sstc", RISCV_ISA_EXT_SSTC);
> SET_ISA_EXT_MAP("svinval", RISCV_ISA_EXT_SVINVAL);
> + SET_ISA_EXT_MAP("smaia", RISCV_ISA_EXT_SMAIA);
> + SET_ISA_EXT_MAP("ssaia", RISCV_ISA_EXT_SSAIA);
> }
> #undef SET_ISA_EXT_MAP
> }

2023-02-03 12:01:19

by Anup Patel

[permalink] [raw]
Subject: Re: [PATCH v2 2/7] RISC-V: Detect AIA CSRs from ISA string

On Fri, Feb 3, 2023 at 5:54 AM Palmer Dabbelt <[email protected]> wrote:
>
> On Fri, 27 Jan 2023 23:27:32 PST (-0800), [email protected] wrote:
> > We have two extension names for AIA ISA support: Smaia (M-mode AIA CSRs)
> > and Ssaia (S-mode AIA CSRs).
>
> This has pretty much the same problem that we had with the other
> AIA-related ISA string patches, where there's that ambiguity with the
> non-ratified chapters. IIRC when this came up in GCC the rough idea was
> to try and document that we're going to interpret the standard ISA
> strings that way, but now that we're doing custom ISA extensions it
> seems saner to just define on here that removes the ambiguity.
>
> I just sent
> <https://lore.kernel.org/r/[email protected]/>
> which documents that.

I am not sure why you say that these are custom extensions.

Multiple folks have clarified that both Smaia and Ssaia are frozen
ISA extensions as-per RVI process. The individual chapters which
are in the draft state have nothing to do with Smaia and Ssaia CSRs.

Please refer:
https://github.com/riscv/riscv-aia/pull/36
https://lists.riscv.org/g/tech-aia/message/336
https://lists.riscv.org/g/tech-aia/message/337

>
> > We extend the ISA string parsing to detect Smaia and Ssaia extensions.
> >
> > Signed-off-by: Anup Patel <[email protected]>
> > Reviewed-by: Andrew Jones <[email protected]>
> > ---
> > arch/riscv/include/asm/hwcap.h | 2 ++
> > arch/riscv/kernel/cpu.c | 2 ++
> > arch/riscv/kernel/cpufeature.c | 2 ++
> > 3 files changed, 6 insertions(+)
> >
> > diff --git a/arch/riscv/include/asm/hwcap.h b/arch/riscv/include/asm/hwcap.h
> > index 86328e3acb02..341ef30a3718 100644
> > --- a/arch/riscv/include/asm/hwcap.h
> > +++ b/arch/riscv/include/asm/hwcap.h
> > @@ -59,6 +59,8 @@ enum riscv_isa_ext_id {
> > RISCV_ISA_EXT_ZIHINTPAUSE,
> > RISCV_ISA_EXT_SSTC,
> > RISCV_ISA_EXT_SVINVAL,
> > + RISCV_ISA_EXT_SMAIA,
> > + RISCV_ISA_EXT_SSAIA,
> > RISCV_ISA_EXT_ID_MAX
> > };
> > static_assert(RISCV_ISA_EXT_ID_MAX <= RISCV_ISA_EXT_MAX);
> > diff --git a/arch/riscv/kernel/cpu.c b/arch/riscv/kernel/cpu.c
> > index 1b9a5a66e55a..a215ec929160 100644
> > --- a/arch/riscv/kernel/cpu.c
> > +++ b/arch/riscv/kernel/cpu.c
> > @@ -162,6 +162,8 @@ arch_initcall(riscv_cpuinfo_init);
> > * extensions by an underscore.
> > */
> > static struct riscv_isa_ext_data isa_ext_arr[] = {
> > + __RISCV_ISA_EXT_DATA(smaia, RISCV_ISA_EXT_SMAIA),
> > + __RISCV_ISA_EXT_DATA(ssaia, RISCV_ISA_EXT_SSAIA),
>
> This will conflict with that ISA string refactoring I just merged. It
> should be a pretty mechanical merge conflict, but if you want we can do
> a shared tag with the first few patches and I can handle the merge
> conflict locally.

I am planning to send this series as a second PR for Linux-6.3 after your
PR (which includes ISA string refactoring) is merged. Is that okay with you?

With that said, it would request you to ACK this patch as well.

>
> > __RISCV_ISA_EXT_DATA(sscofpmf, RISCV_ISA_EXT_SSCOFPMF),
> > __RISCV_ISA_EXT_DATA(sstc, RISCV_ISA_EXT_SSTC),
> > __RISCV_ISA_EXT_DATA(svinval, RISCV_ISA_EXT_SVINVAL),
> > diff --git a/arch/riscv/kernel/cpufeature.c b/arch/riscv/kernel/cpufeature.c
> > index 93e45560af30..3c5b51f519d5 100644
> > --- a/arch/riscv/kernel/cpufeature.c
> > +++ b/arch/riscv/kernel/cpufeature.c
> > @@ -228,6 +228,8 @@ void __init riscv_fill_hwcap(void)
> > SET_ISA_EXT_MAP("zihintpause", RISCV_ISA_EXT_ZIHINTPAUSE);
> > SET_ISA_EXT_MAP("sstc", RISCV_ISA_EXT_SSTC);
> > SET_ISA_EXT_MAP("svinval", RISCV_ISA_EXT_SVINVAL);
> > + SET_ISA_EXT_MAP("smaia", RISCV_ISA_EXT_SMAIA);
> > + SET_ISA_EXT_MAP("ssaia", RISCV_ISA_EXT_SSAIA);
> > }
> > #undef SET_ISA_EXT_MAP
> > }

Thanks,
Anup

2023-02-07 18:05:26

by Conor Dooley

[permalink] [raw]
Subject: Re: [PATCH v2 2/7] RISC-V: Detect AIA CSRs from ISA string

Hey Anup, Palmer,

On Fri, Feb 03, 2023 at 05:31:01PM +0530, Anup Patel wrote:
> On Fri, Feb 3, 2023 at 5:54 AM Palmer Dabbelt <[email protected]> wrote:
> >
> > On Fri, 27 Jan 2023 23:27:32 PST (-0800), [email protected] wrote:
> > > We have two extension names for AIA ISA support: Smaia (M-mode AIA CSRs)
> > > and Ssaia (S-mode AIA CSRs).
> >
> > This has pretty much the same problem that we had with the other
> > AIA-related ISA string patches, where there's that ambiguity with the
> > non-ratified chapters. IIRC when this came up in GCC the rough idea was
> > to try and document that we're going to interpret the standard ISA
> > strings that way, but now that we're doing custom ISA extensions it
> > seems saner to just define on here that removes the ambiguity.
> >
> > I just sent
> > <https://lore.kernel.org/r/[email protected]/>
> > which documents that.
>
> I am not sure why you say that these are custom extensions.
>
> Multiple folks have clarified that both Smaia and Ssaia are frozen
> ISA extensions as-per RVI process. The individual chapters which
> are in the draft state have nothing to do with Smaia and Ssaia CSRs.
>
> Please refer:
> https://github.com/riscv/riscv-aia/pull/36
> https://lists.riscv.org/g/tech-aia/message/336
> https://lists.riscv.org/g/tech-aia/message/337

All of these links seem to discuss the draft chapters somehow being
incompatible with the non-draft ones. I would very expect that that,
as pointed out in several places there, that the draft chapters
finalisation would not lead to meaningful (and incompatible!) changes
being made to the non-draft chapters.

Maybe yourself and Palmer are looking at this from different
perspectives? Looking at his patch from Friday:
https://lore.kernel.org/linux-riscv/[email protected]/
He specifically mentioned this aspect, as opposed to the aspect that
your links refer to.

Surely a duo-plic, if that ever comes to be, could be detected from
compatible strings in DT or w/e - but how do you intend differentiating
between an implementation of S*aia that contains the IOMMU support in
Chapter 9 in a finalised form, versus an implementation that may make
"different decisions" when it comes to that chapter of the spec?
I thought that would be handled by extension versions, but I am told
that those are not a thing any more.
If that's not true, and there'll be a version number that we can pull in
from a DT and parse which will distinguish between the two, then please
correct my misunderstanding here!

Thanks,
Conor.

> > > We extend the ISA string parsing to detect Smaia and Ssaia extensions.
> > >
> > > Signed-off-by: Anup Patel <[email protected]>
> > > Reviewed-by: Andrew Jones <[email protected]>
> > > ---
> > > arch/riscv/include/asm/hwcap.h | 2 ++
> > > arch/riscv/kernel/cpu.c | 2 ++
> > > arch/riscv/kernel/cpufeature.c | 2 ++
> > > 3 files changed, 6 insertions(+)
> > >
> > > diff --git a/arch/riscv/include/asm/hwcap.h b/arch/riscv/include/asm/hwcap.h
> > > index 86328e3acb02..341ef30a3718 100644
> > > --- a/arch/riscv/include/asm/hwcap.h
> > > +++ b/arch/riscv/include/asm/hwcap.h
> > > @@ -59,6 +59,8 @@ enum riscv_isa_ext_id {
> > > RISCV_ISA_EXT_ZIHINTPAUSE,
> > > RISCV_ISA_EXT_SSTC,
> > > RISCV_ISA_EXT_SVINVAL,
> > > + RISCV_ISA_EXT_SMAIA,
> > > + RISCV_ISA_EXT_SSAIA,
> > > RISCV_ISA_EXT_ID_MAX
> > > };
> > > static_assert(RISCV_ISA_EXT_ID_MAX <= RISCV_ISA_EXT_MAX);
> > > diff --git a/arch/riscv/kernel/cpu.c b/arch/riscv/kernel/cpu.c
> > > index 1b9a5a66e55a..a215ec929160 100644
> > > --- a/arch/riscv/kernel/cpu.c
> > > +++ b/arch/riscv/kernel/cpu.c
> > > @@ -162,6 +162,8 @@ arch_initcall(riscv_cpuinfo_init);
> > > * extensions by an underscore.
> > > */
> > > static struct riscv_isa_ext_data isa_ext_arr[] = {
> > > + __RISCV_ISA_EXT_DATA(smaia, RISCV_ISA_EXT_SMAIA),
> > > + __RISCV_ISA_EXT_DATA(ssaia, RISCV_ISA_EXT_SSAIA),
> >
> > This will conflict with that ISA string refactoring I just merged. It
> > should be a pretty mechanical merge conflict, but if you want we can do
> > a shared tag with the first few patches and I can handle the merge
> > conflict locally.
>
> I am planning to send this series as a second PR for Linux-6.3 after your
> PR (which includes ISA string refactoring) is merged. Is that okay with you?
>
> With that said, it would request you to ACK this patch as well.
>
> >
> > > __RISCV_ISA_EXT_DATA(sscofpmf, RISCV_ISA_EXT_SSCOFPMF),
> > > __RISCV_ISA_EXT_DATA(sstc, RISCV_ISA_EXT_SSTC),
> > > __RISCV_ISA_EXT_DATA(svinval, RISCV_ISA_EXT_SVINVAL),
> > > diff --git a/arch/riscv/kernel/cpufeature.c b/arch/riscv/kernel/cpufeature.c
> > > index 93e45560af30..3c5b51f519d5 100644
> > > --- a/arch/riscv/kernel/cpufeature.c
> > > +++ b/arch/riscv/kernel/cpufeature.c
> > > @@ -228,6 +228,8 @@ void __init riscv_fill_hwcap(void)
> > > SET_ISA_EXT_MAP("zihintpause", RISCV_ISA_EXT_ZIHINTPAUSE);
> > > SET_ISA_EXT_MAP("sstc", RISCV_ISA_EXT_SSTC);
> > > SET_ISA_EXT_MAP("svinval", RISCV_ISA_EXT_SVINVAL);
> > > + SET_ISA_EXT_MAP("smaia", RISCV_ISA_EXT_SMAIA);
> > > + SET_ISA_EXT_MAP("ssaia", RISCV_ISA_EXT_SSAIA);
> > > }
> > > #undef SET_ISA_EXT_MAP
> > > }
>
> Thanks,
> Anup


Attachments:
(No filename) (5.35 kB)
signature.asc (228.00 B)
Download all attachments

2023-02-07 18:09:18

by Conor Dooley

[permalink] [raw]
Subject: Re: [PATCH v2 2/7] RISC-V: Detect AIA CSRs from ISA string

On Tue, Feb 07, 2023 at 06:05:11PM +0000, Conor Dooley wrote:
> Hey Anup, Palmer,
>
> On Fri, Feb 03, 2023 at 05:31:01PM +0530, Anup Patel wrote:
> > On Fri, Feb 3, 2023 at 5:54 AM Palmer Dabbelt <[email protected]> wrote:
> > >
> > > On Fri, 27 Jan 2023 23:27:32 PST (-0800), [email protected] wrote:
> > > > We have two extension names for AIA ISA support: Smaia (M-mode AIA CSRs)
> > > > and Ssaia (S-mode AIA CSRs).
> > >
> > > This has pretty much the same problem that we had with the other
> > > AIA-related ISA string patches, where there's that ambiguity with the
> > > non-ratified chapters. IIRC when this came up in GCC the rough idea was
> > > to try and document that we're going to interpret the standard ISA
> > > strings that way, but now that we're doing custom ISA extensions it
> > > seems saner to just define on here that removes the ambiguity.
> > >
> > > I just sent
> > > <https://lore.kernel.org/r/[email protected]/>
> > > which documents that.
> >
> > I am not sure why you say that these are custom extensions.
> >
> > Multiple folks have clarified that both Smaia and Ssaia are frozen
> > ISA extensions as-per RVI process. The individual chapters which
> > are in the draft state have nothing to do with Smaia and Ssaia CSRs.
> >
> > Please refer:
> > https://github.com/riscv/riscv-aia/pull/36
> > https://lists.riscv.org/g/tech-aia/message/336
> > https://lists.riscv.org/g/tech-aia/message/337
>
> All of these links seem to discuss the draft chapters somehow being
> incompatible with the non-draft ones. I would very expect that that,
> as pointed out in several places there, that the draft chapters
> finalisation would not lead to meaningful (and incompatible!) changes
> being made to the non-draft chapters.
>
> Maybe yourself and Palmer are looking at this from different
> perspectives? Looking at his patch from Friday:
> https://lore.kernel.org/linux-riscv/[email protected]/
> He specifically mentioned this aspect, as opposed to the aspect that
> your links refer to.

Meh, bad re-ordering of my paragraphs. By "this aspect" I meant what is
in the below paragraph! Apologies for any confusion there :)

> Surely a duo-plic, if that ever comes to be, could be detected from
> compatible strings in DT or w/e - but how do you intend differentiating
> between an implementation of S*aia that contains the IOMMU support in
> Chapter 9 in a finalised form, versus an implementation that may make
> "different decisions" when it comes to that chapter of the spec?
> I thought that would be handled by extension versions, but I am told
> that those are not a thing any more.
> If that's not true, and there'll be a version number that we can pull in
> from a DT and parse which will distinguish between the two, then please
> correct my misunderstanding here!
>
> Thanks,
> Conor.
>
> > > > We extend the ISA string parsing to detect Smaia and Ssaia extensions.
> > > >
> > > > Signed-off-by: Anup Patel <[email protected]>
> > > > Reviewed-by: Andrew Jones <[email protected]>
> > > > ---
> > > > arch/riscv/include/asm/hwcap.h | 2 ++
> > > > arch/riscv/kernel/cpu.c | 2 ++
> > > > arch/riscv/kernel/cpufeature.c | 2 ++
> > > > 3 files changed, 6 insertions(+)
> > > >
> > > > diff --git a/arch/riscv/include/asm/hwcap.h b/arch/riscv/include/asm/hwcap.h
> > > > index 86328e3acb02..341ef30a3718 100644
> > > > --- a/arch/riscv/include/asm/hwcap.h
> > > > +++ b/arch/riscv/include/asm/hwcap.h
> > > > @@ -59,6 +59,8 @@ enum riscv_isa_ext_id {
> > > > RISCV_ISA_EXT_ZIHINTPAUSE,
> > > > RISCV_ISA_EXT_SSTC,
> > > > RISCV_ISA_EXT_SVINVAL,
> > > > + RISCV_ISA_EXT_SMAIA,
> > > > + RISCV_ISA_EXT_SSAIA,
> > > > RISCV_ISA_EXT_ID_MAX
> > > > };
> > > > static_assert(RISCV_ISA_EXT_ID_MAX <= RISCV_ISA_EXT_MAX);
> > > > diff --git a/arch/riscv/kernel/cpu.c b/arch/riscv/kernel/cpu.c
> > > > index 1b9a5a66e55a..a215ec929160 100644
> > > > --- a/arch/riscv/kernel/cpu.c
> > > > +++ b/arch/riscv/kernel/cpu.c
> > > > @@ -162,6 +162,8 @@ arch_initcall(riscv_cpuinfo_init);
> > > > * extensions by an underscore.
> > > > */
> > > > static struct riscv_isa_ext_data isa_ext_arr[] = {
> > > > + __RISCV_ISA_EXT_DATA(smaia, RISCV_ISA_EXT_SMAIA),
> > > > + __RISCV_ISA_EXT_DATA(ssaia, RISCV_ISA_EXT_SSAIA),
> > >
> > > This will conflict with that ISA string refactoring I just merged. It
> > > should be a pretty mechanical merge conflict, but if you want we can do
> > > a shared tag with the first few patches and I can handle the merge
> > > conflict locally.
> >
> > I am planning to send this series as a second PR for Linux-6.3 after your
> > PR (which includes ISA string refactoring) is merged. Is that okay with you?
> >
> > With that said, it would request you to ACK this patch as well.
> >
> > >
> > > > __RISCV_ISA_EXT_DATA(sscofpmf, RISCV_ISA_EXT_SSCOFPMF),
> > > > __RISCV_ISA_EXT_DATA(sstc, RISCV_ISA_EXT_SSTC),
> > > > __RISCV_ISA_EXT_DATA(svinval, RISCV_ISA_EXT_SVINVAL),
> > > > diff --git a/arch/riscv/kernel/cpufeature.c b/arch/riscv/kernel/cpufeature.c
> > > > index 93e45560af30..3c5b51f519d5 100644
> > > > --- a/arch/riscv/kernel/cpufeature.c
> > > > +++ b/arch/riscv/kernel/cpufeature.c
> > > > @@ -228,6 +228,8 @@ void __init riscv_fill_hwcap(void)
> > > > SET_ISA_EXT_MAP("zihintpause", RISCV_ISA_EXT_ZIHINTPAUSE);
> > > > SET_ISA_EXT_MAP("sstc", RISCV_ISA_EXT_SSTC);
> > > > SET_ISA_EXT_MAP("svinval", RISCV_ISA_EXT_SVINVAL);
> > > > + SET_ISA_EXT_MAP("smaia", RISCV_ISA_EXT_SMAIA);
> > > > + SET_ISA_EXT_MAP("ssaia", RISCV_ISA_EXT_SSAIA);
> > > > }
> > > > #undef SET_ISA_EXT_MAP
> > > > }
> >
> > Thanks,
> > Anup



Attachments:
(No filename) (5.78 kB)
signature.asc (228.00 B)
Download all attachments

2023-02-07 18:15:39

by Atish Patra

[permalink] [raw]
Subject: Re: [PATCH v2 2/7] RISC-V: Detect AIA CSRs from ISA string

On Tue, Feb 7, 2023 at 10:05 AM Conor Dooley <[email protected]> wrote:
>
> Hey Anup, Palmer,
>
> On Fri, Feb 03, 2023 at 05:31:01PM +0530, Anup Patel wrote:
> > On Fri, Feb 3, 2023 at 5:54 AM Palmer Dabbelt <[email protected]> wrote:
> > >
> > > On Fri, 27 Jan 2023 23:27:32 PST (-0800), [email protected] wrote:
> > > > We have two extension names for AIA ISA support: Smaia (M-mode AIA CSRs)
> > > > and Ssaia (S-mode AIA CSRs).
> > >
> > > This has pretty much the same problem that we had with the other
> > > AIA-related ISA string patches, where there's that ambiguity with the
> > > non-ratified chapters. IIRC when this came up in GCC the rough idea was
> > > to try and document that we're going to interpret the standard ISA
> > > strings that way, but now that we're doing custom ISA extensions it
> > > seems saner to just define on here that removes the ambiguity.
> > >
> > > I just sent
> > > <https://lore.kernel.org/r/[email protected]/>
> > > which documents that.
> >
> > I am not sure why you say that these are custom extensions.
> >
> > Multiple folks have clarified that both Smaia and Ssaia are frozen
> > ISA extensions as-per RVI process. The individual chapters which
> > are in the draft state have nothing to do with Smaia and Ssaia CSRs.
> >
> > Please refer:
> > https://github.com/riscv/riscv-aia/pull/36
> > https://lists.riscv.org/g/tech-aia/message/336
> > https://lists.riscv.org/g/tech-aia/message/337
>
> All of these links seem to discuss the draft chapters somehow being
> incompatible with the non-draft ones. I would very expect that that,
> as pointed out in several places there, that the draft chapters
> finalisation would not lead to meaningful (and incompatible!) changes
> being made to the non-draft chapters.
>

Here is the status of all RVI specs. It states that the Smaia, Ssaia
extensions are frozen (i.e. public review complete).
https://wiki.riscv.org/display/HOME/Specification+Status

I have added stephano/Jeff to confirm the same.

AFAIK, IOMMU spec is close to the public review phase and should be
frozen in this or next quarter.
IIRC, this chapter in AIA will be frozen along with IOMMU spec.

Anup: Please correct me if that's not correct.

> Maybe yourself and Palmer are looking at this from different
> perspectives? Looking at his patch from Friday:
> https://lore.kernel.org/linux-riscv/[email protected]/
> He specifically mentioned this aspect, as opposed to the aspect that
> your links refer to.
>
> Surely a duo-plic, if that ever comes to be, could be detected from
> compatible strings in DT or w/e - but how do you intend differentiating
> between an implementation of S*aia that contains the IOMMU support in
> Chapter 9 in a finalised form, versus an implementation that may make
> "different decisions" when it comes to that chapter of the spec?

We will most likely have an extension specific to iommu spec as well.

> I thought that would be handled by extension versions, but I am told
> that those are not a thing any more.
> If that's not true, and there'll be a version number that we can pull in
> from a DT and parse which will distinguish between the two, then please
> correct my misunderstanding here!
>
> Thanks,
> Conor.
>
> > > > We extend the ISA string parsing to detect Smaia and Ssaia extensions.
> > > >
> > > > Signed-off-by: Anup Patel <[email protected]>
> > > > Reviewed-by: Andrew Jones <[email protected]>
> > > > ---
> > > > arch/riscv/include/asm/hwcap.h | 2 ++
> > > > arch/riscv/kernel/cpu.c | 2 ++
> > > > arch/riscv/kernel/cpufeature.c | 2 ++
> > > > 3 files changed, 6 insertions(+)
> > > >
> > > > diff --git a/arch/riscv/include/asm/hwcap.h b/arch/riscv/include/asm/hwcap.h
> > > > index 86328e3acb02..341ef30a3718 100644
> > > > --- a/arch/riscv/include/asm/hwcap.h
> > > > +++ b/arch/riscv/include/asm/hwcap.h
> > > > @@ -59,6 +59,8 @@ enum riscv_isa_ext_id {
> > > > RISCV_ISA_EXT_ZIHINTPAUSE,
> > > > RISCV_ISA_EXT_SSTC,
> > > > RISCV_ISA_EXT_SVINVAL,
> > > > + RISCV_ISA_EXT_SMAIA,
> > > > + RISCV_ISA_EXT_SSAIA,
> > > > RISCV_ISA_EXT_ID_MAX
> > > > };
> > > > static_assert(RISCV_ISA_EXT_ID_MAX <= RISCV_ISA_EXT_MAX);
> > > > diff --git a/arch/riscv/kernel/cpu.c b/arch/riscv/kernel/cpu.c
> > > > index 1b9a5a66e55a..a215ec929160 100644
> > > > --- a/arch/riscv/kernel/cpu.c
> > > > +++ b/arch/riscv/kernel/cpu.c
> > > > @@ -162,6 +162,8 @@ arch_initcall(riscv_cpuinfo_init);
> > > > * extensions by an underscore.
> > > > */
> > > > static struct riscv_isa_ext_data isa_ext_arr[] = {
> > > > + __RISCV_ISA_EXT_DATA(smaia, RISCV_ISA_EXT_SMAIA),
> > > > + __RISCV_ISA_EXT_DATA(ssaia, RISCV_ISA_EXT_SSAIA),
> > >
> > > This will conflict with that ISA string refactoring I just merged. It
> > > should be a pretty mechanical merge conflict, but if you want we can do
> > > a shared tag with the first few patches and I can handle the merge
> > > conflict locally.
> >
> > I am planning to send this series as a second PR for Linux-6.3 after your
> > PR (which includes ISA string refactoring) is merged. Is that okay with you?
> >
> > With that said, it would request you to ACK this patch as well.
> >
> > >
> > > > __RISCV_ISA_EXT_DATA(sscofpmf, RISCV_ISA_EXT_SSCOFPMF),
> > > > __RISCV_ISA_EXT_DATA(sstc, RISCV_ISA_EXT_SSTC),
> > > > __RISCV_ISA_EXT_DATA(svinval, RISCV_ISA_EXT_SVINVAL),
> > > > diff --git a/arch/riscv/kernel/cpufeature.c b/arch/riscv/kernel/cpufeature.c
> > > > index 93e45560af30..3c5b51f519d5 100644
> > > > --- a/arch/riscv/kernel/cpufeature.c
> > > > +++ b/arch/riscv/kernel/cpufeature.c
> > > > @@ -228,6 +228,8 @@ void __init riscv_fill_hwcap(void)
> > > > SET_ISA_EXT_MAP("zihintpause", RISCV_ISA_EXT_ZIHINTPAUSE);
> > > > SET_ISA_EXT_MAP("sstc", RISCV_ISA_EXT_SSTC);
> > > > SET_ISA_EXT_MAP("svinval", RISCV_ISA_EXT_SVINVAL);
> > > > + SET_ISA_EXT_MAP("smaia", RISCV_ISA_EXT_SMAIA);
> > > > + SET_ISA_EXT_MAP("ssaia", RISCV_ISA_EXT_SSAIA);
> > > > }
> > > > #undef SET_ISA_EXT_MAP
> > > > }
> >
> > Thanks,
> > Anup



--
Regards,
Atish

2023-02-07 20:40:09

by Conor Dooley

[permalink] [raw]
Subject: Re: [PATCH v2 2/7] RISC-V: Detect AIA CSRs from ISA string

On Tue, Feb 07, 2023 at 10:15:22AM -0800, Atish Patra wrote:
> On Tue, Feb 7, 2023 at 10:05 AM Conor Dooley <[email protected]> wrote:
> > On Fri, Feb 03, 2023 at 05:31:01PM +0530, Anup Patel wrote:
> > > On Fri, Feb 3, 2023 at 5:54 AM Palmer Dabbelt <[email protected]> wrote:
> > > >
> > > > On Fri, 27 Jan 2023 23:27:32 PST (-0800), [email protected] wrote:
> > > > > We have two extension names for AIA ISA support: Smaia (M-mode AIA CSRs)
> > > > > and Ssaia (S-mode AIA CSRs).
> > > >
> > > > This has pretty much the same problem that we had with the other
> > > > AIA-related ISA string patches, where there's that ambiguity with the
> > > > non-ratified chapters. IIRC when this came up in GCC the rough idea was
> > > > to try and document that we're going to interpret the standard ISA
> > > > strings that way, but now that we're doing custom ISA extensions it
> > > > seems saner to just define on here that removes the ambiguity.
> > > >
> > > > I just sent
> > > > <https://lore.kernel.org/r/[email protected]/>
> > > > which documents that.
> > >
> > > I am not sure why you say that these are custom extensions.
> > >
> > > Multiple folks have clarified that both Smaia and Ssaia are frozen
> > > ISA extensions as-per RVI process. The individual chapters which
> > > are in the draft state have nothing to do with Smaia and Ssaia CSRs.
> > >
> > > Please refer:
> > > https://github.com/riscv/riscv-aia/pull/36
> > > https://lists.riscv.org/g/tech-aia/message/336
> > > https://lists.riscv.org/g/tech-aia/message/337
> >
> > All of these links seem to discuss the draft chapters somehow being
> > incompatible with the non-draft ones. I would very expect that that,
> > as pointed out in several places there, that the draft chapters
> > finalisation would not lead to meaningful (and incompatible!) changes
> > being made to the non-draft chapters.
> >
>
> Here is the status of all RVI specs. It states that the Smaia, Ssaia
> extensions are frozen (i.e. public review complete).
> https://wiki.riscv.org/display/HOME/Specification+Status
>
> I have added stephano/Jeff to confirm the same.
>
> AFAIK, IOMMU spec is close to the public review phase and should be
> frozen in this or next quarter.
> IIRC, this chapter in AIA will be frozen along with IOMMU spec.
>
> Anup: Please correct me if that's not correct.
>
> > Maybe yourself and Palmer are looking at this from different
> > perspectives? Looking at his patch from Friday:
> > https://lore.kernel.org/linux-riscv/[email protected]/
> > He specifically mentioned this aspect, as opposed to the aspect that
> > your links refer to.
> >
> > Surely a duo-plic, if that ever comes to be, could be detected from
> > compatible strings in DT or w/e - but how do you intend differentiating
> > between an implementation of S*aia that contains the IOMMU support in
> > Chapter 9 in a finalised form, versus an implementation that may make
> > "different decisions" when it comes to that chapter of the spec?
>
> We will most likely have an extension specific to iommu spec as well.

Right, but unless I am misunderstanding you, that is an extension for the
IOMMU spec, not for Chapter 9 of the AIA spec?

I would say that it is likely that if you have AIA and IOMMU that you'd
want to be implementing Chapter 9, but that would not appear sufficient to
draw a conclusion from.

Maybe the RVI lads that you've added (or Anup for that matter!) can
clarify if there is a requirement that if you do AIA and IOMMU that you
must do Chapter 9.
If not, my prior question about a differentiation mechanism still applies
I think!

> > I thought that would be handled by extension versions, but I am told
> > that those are not a thing any more.
> > If that's not true, and there'll be a version number that we can pull in
> > from a DT and parse which will distinguish between the two, then please
> > correct my misunderstanding here!


Attachments:
(No filename) (3.87 kB)
signature.asc (228.00 B)
Download all attachments

2023-02-08 00:04:54

by Atish Patra

[permalink] [raw]
Subject: Re: [PATCH v2 5/7] RISC-V: KVM: Add ONE_REG interface for AIA CSRs

On Fri, Jan 27, 2023 at 11:28 PM Anup Patel <[email protected]> wrote:
>
> We extend the CSR ONE_REG interface to access both general CSRs and
> AIA CSRs. To achieve this, we introduce "subtype" field in the ONE_REG
> id which can be used for grouping registers within a particular "type"
> of ONE_REG registers.
>
> Signed-off-by: Anup Patel <[email protected]>
> ---
> arch/riscv/include/uapi/asm/kvm.h | 15 ++++-
> arch/riscv/kvm/vcpu.c | 96 ++++++++++++++++++++++++-------
> 2 files changed, 89 insertions(+), 22 deletions(-)
>
> diff --git a/arch/riscv/include/uapi/asm/kvm.h b/arch/riscv/include/uapi/asm/kvm.h
> index 71992ff1f9dd..d0704eff0121 100644
> --- a/arch/riscv/include/uapi/asm/kvm.h
> +++ b/arch/riscv/include/uapi/asm/kvm.h
> @@ -64,7 +64,7 @@ struct kvm_riscv_core {
> #define KVM_RISCV_MODE_S 1
> #define KVM_RISCV_MODE_U 0
>
> -/* CSR registers for KVM_GET_ONE_REG and KVM_SET_ONE_REG */
> +/* General CSR registers for KVM_GET_ONE_REG and KVM_SET_ONE_REG */
> struct kvm_riscv_csr {
> unsigned long sstatus;
> unsigned long sie;
> @@ -78,6 +78,10 @@ struct kvm_riscv_csr {
> unsigned long scounteren;
> };
>
> +/* AIA CSR registers for KVM_GET_ONE_REG and KVM_SET_ONE_REG */
> +struct kvm_riscv_aia_csr {
> +};
> +
> /* TIMER registers for KVM_GET_ONE_REG and KVM_SET_ONE_REG */
> struct kvm_riscv_timer {
> __u64 frequency;
> @@ -105,6 +109,7 @@ enum KVM_RISCV_ISA_EXT_ID {
> KVM_RISCV_ISA_EXT_SVINVAL,
> KVM_RISCV_ISA_EXT_ZIHINTPAUSE,
> KVM_RISCV_ISA_EXT_ZICBOM,
> + KVM_RISCV_ISA_EXT_SSAIA,
> KVM_RISCV_ISA_EXT_MAX,
> };
>
> @@ -134,6 +139,8 @@ enum KVM_RISCV_SBI_EXT_ID {
> /* If you need to interpret the index values, here is the key: */
> #define KVM_REG_RISCV_TYPE_MASK 0x00000000FF000000
> #define KVM_REG_RISCV_TYPE_SHIFT 24
> +#define KVM_REG_RISCV_SUBTYPE_MASK 0x0000000000FF0000
> +#define KVM_REG_RISCV_SUBTYPE_SHIFT 16
>
> /* Config registers are mapped as type 1 */
> #define KVM_REG_RISCV_CONFIG (0x01 << KVM_REG_RISCV_TYPE_SHIFT)
> @@ -147,8 +154,12 @@ enum KVM_RISCV_SBI_EXT_ID {
>
> /* Control and status registers are mapped as type 3 */
> #define KVM_REG_RISCV_CSR (0x03 << KVM_REG_RISCV_TYPE_SHIFT)
> +#define KVM_REG_RISCV_CSR_GENERAL 0x0
> +#define KVM_REG_RISCV_CSR_AIA 0x1
> #define KVM_REG_RISCV_CSR_REG(name) \
> - (offsetof(struct kvm_riscv_csr, name) / sizeof(unsigned long))
> + (offsetof(struct kvm_riscv_csr, name) / sizeof(unsigned long))
> +#define KVM_REG_RISCV_CSR_AIA_REG(name) \
> + (offsetof(struct kvm_riscv_aia_csr, name) / sizeof(unsigned long))
>
> /* Timer registers are mapped as type 4 */
> #define KVM_REG_RISCV_TIMER (0x04 << KVM_REG_RISCV_TYPE_SHIFT)
> diff --git a/arch/riscv/kvm/vcpu.c b/arch/riscv/kvm/vcpu.c
> index 3cf50eadc8ce..37933ea20274 100644
> --- a/arch/riscv/kvm/vcpu.c
> +++ b/arch/riscv/kvm/vcpu.c
> @@ -58,6 +58,7 @@ static const unsigned long kvm_isa_ext_arr[] = {
> [KVM_RISCV_ISA_EXT_I] = RISCV_ISA_EXT_i,
> [KVM_RISCV_ISA_EXT_M] = RISCV_ISA_EXT_m,
>
> + KVM_ISA_EXT_ARR(SSAIA),
> KVM_ISA_EXT_ARR(SSTC),
> KVM_ISA_EXT_ARR(SVINVAL),
> KVM_ISA_EXT_ARR(SVPBMT),
> @@ -96,6 +97,7 @@ static bool kvm_riscv_vcpu_isa_disable_allowed(unsigned long ext)
> case KVM_RISCV_ISA_EXT_C:
> case KVM_RISCV_ISA_EXT_I:
> case KVM_RISCV_ISA_EXT_M:
> + case KVM_RISCV_ISA_EXT_SSAIA:
> case KVM_RISCV_ISA_EXT_SSTC:
> case KVM_RISCV_ISA_EXT_SVINVAL:
> case KVM_RISCV_ISA_EXT_ZIHINTPAUSE:
> @@ -451,30 +453,79 @@ static int kvm_riscv_vcpu_set_reg_core(struct kvm_vcpu *vcpu,
> return 0;
> }
>
> +static int kvm_riscv_vcpu_general_get_csr(struct kvm_vcpu *vcpu,
> + unsigned long reg_num,
> + unsigned long *out_val)
> +{
> + struct kvm_vcpu_csr *csr = &vcpu->arch.guest_csr;
> +
> + if (reg_num >= sizeof(struct kvm_riscv_csr) / sizeof(unsigned long))
> + return -EINVAL;
> +
> + if (reg_num == KVM_REG_RISCV_CSR_REG(sip)) {
> + kvm_riscv_vcpu_flush_interrupts(vcpu);
> + *out_val = (csr->hvip >> VSIP_TO_HVIP_SHIFT) & VSIP_VALID_MASK;
> + } else
> + *out_val = ((unsigned long *)csr)[reg_num];
> +
> + return 0;
> +}
> +
> static int kvm_riscv_vcpu_get_reg_csr(struct kvm_vcpu *vcpu,
> const struct kvm_one_reg *reg)
> {
> - struct kvm_vcpu_csr *csr = &vcpu->arch.guest_csr;
> + int rc;
> unsigned long __user *uaddr =
> (unsigned long __user *)(unsigned long)reg->addr;
> unsigned long reg_num = reg->id & ~(KVM_REG_ARCH_MASK |
> KVM_REG_SIZE_MASK |
> KVM_REG_RISCV_CSR);
> - unsigned long reg_val;
> + unsigned long reg_val, reg_subtype;
>
> if (KVM_REG_SIZE(reg->id) != sizeof(unsigned long))
> return -EINVAL;
> +
> + reg_subtype = (reg_num & KVM_REG_RISCV_SUBTYPE_MASK)
> + >> KVM_REG_RISCV_SUBTYPE_SHIFT;
> + reg_num &= ~KVM_REG_RISCV_SUBTYPE_MASK;
> + switch (reg_subtype) {
> + case KVM_REG_RISCV_CSR_GENERAL:
> + rc = kvm_riscv_vcpu_general_get_csr(vcpu, reg_num, &reg_val);
> + break;
> + case KVM_REG_RISCV_CSR_AIA:
> + rc = kvm_riscv_vcpu_aia_get_csr(vcpu, reg_num, &reg_val);
> + break;
> + default:
> + rc = -EINVAL;
> + break;
> + }
> + if (rc)
> + return rc;
> +
> + if (copy_to_user(uaddr, &reg_val, KVM_REG_SIZE(reg->id)))
> + return -EFAULT;
> +
> + return 0;
> +}
> +
> +static inline int kvm_riscv_vcpu_general_set_csr(struct kvm_vcpu *vcpu,
> + unsigned long reg_num,
> + unsigned long reg_val)
> +{
> + struct kvm_vcpu_csr *csr = &vcpu->arch.guest_csr;
> +
> if (reg_num >= sizeof(struct kvm_riscv_csr) / sizeof(unsigned long))
> return -EINVAL;
>
> if (reg_num == KVM_REG_RISCV_CSR_REG(sip)) {
> - kvm_riscv_vcpu_flush_interrupts(vcpu);
> - reg_val = (csr->hvip >> VSIP_TO_HVIP_SHIFT) & VSIP_VALID_MASK;
> - } else
> - reg_val = ((unsigned long *)csr)[reg_num];
> + reg_val &= VSIP_VALID_MASK;
> + reg_val <<= VSIP_TO_HVIP_SHIFT;
> + }
>
> - if (copy_to_user(uaddr, &reg_val, KVM_REG_SIZE(reg->id)))
> - return -EFAULT;
> + ((unsigned long *)csr)[reg_num] = reg_val;
> +
> + if (reg_num == KVM_REG_RISCV_CSR_REG(sip))
> + WRITE_ONCE(vcpu->arch.irqs_pending_mask, 0);
>
> return 0;
> }
> @@ -482,31 +533,36 @@ static int kvm_riscv_vcpu_get_reg_csr(struct kvm_vcpu *vcpu,
> static int kvm_riscv_vcpu_set_reg_csr(struct kvm_vcpu *vcpu,
> const struct kvm_one_reg *reg)
> {
> - struct kvm_vcpu_csr *csr = &vcpu->arch.guest_csr;
> + int rc;
> unsigned long __user *uaddr =
> (unsigned long __user *)(unsigned long)reg->addr;
> unsigned long reg_num = reg->id & ~(KVM_REG_ARCH_MASK |
> KVM_REG_SIZE_MASK |
> KVM_REG_RISCV_CSR);
> - unsigned long reg_val;
> + unsigned long reg_val, reg_subtype;
>
> if (KVM_REG_SIZE(reg->id) != sizeof(unsigned long))
> return -EINVAL;
> - if (reg_num >= sizeof(struct kvm_riscv_csr) / sizeof(unsigned long))
> - return -EINVAL;
>
> if (copy_from_user(&reg_val, uaddr, KVM_REG_SIZE(reg->id)))
> return -EFAULT;
>
> - if (reg_num == KVM_REG_RISCV_CSR_REG(sip)) {
> - reg_val &= VSIP_VALID_MASK;
> - reg_val <<= VSIP_TO_HVIP_SHIFT;
> + reg_subtype = (reg_num & KVM_REG_RISCV_SUBTYPE_MASK)
> + >> KVM_REG_RISCV_SUBTYPE_SHIFT;
> + reg_num &= ~KVM_REG_RISCV_SUBTYPE_MASK;
> + switch (reg_subtype) {
> + case KVM_REG_RISCV_CSR_GENERAL:
> + rc = kvm_riscv_vcpu_general_set_csr(vcpu, reg_num, reg_val);
> + break;
> + case KVM_REG_RISCV_CSR_AIA:
> + rc = kvm_riscv_vcpu_aia_set_csr(vcpu, reg_num, reg_val);
> + break;
> + default:
> + rc = -EINVAL;
> + break;
> }
> -
> - ((unsigned long *)csr)[reg_num] = reg_val;
> -
> - if (reg_num == KVM_REG_RISCV_CSR_REG(sip))
> - WRITE_ONCE(vcpu->arch.irqs_pending_mask, 0);
> + if (rc)
> + return rc;
>
> return 0;
> }
> --
> 2.34.1
>

This can be split into two patches. The first patch can just modify
the existing implementation
to parse subtypes for general CSRs. AIA support can be added in the next one.


--
Regards,
Atish

2023-02-08 00:15:42

by Atish Patra

[permalink] [raw]
Subject: Re: [PATCH v2 7/7] RISC-V: KVM: Implement guest external interrupt line management

On Fri, Jan 27, 2023 at 11:28 PM Anup Patel <[email protected]> wrote:
>
> The RISC-V host will have one guest external interrupt line for each
> VS-level IMSICs associated with a HART. The guest external interrupt
> lines are per-HART resources and hypervisor can use HGEIE, HGEIP, and
> HIE CSRs to manage these guest external interrupt lines.
>
> Signed-off-by: Anup Patel <[email protected]>
> ---
> arch/riscv/include/asm/kvm_aia.h | 10 ++
> arch/riscv/kvm/aia.c | 241 +++++++++++++++++++++++++++++++
> arch/riscv/kvm/main.c | 3 +-
> arch/riscv/kvm/vcpu.c | 2 +
> 4 files changed, 255 insertions(+), 1 deletion(-)
>
> diff --git a/arch/riscv/include/asm/kvm_aia.h b/arch/riscv/include/asm/kvm_aia.h
> index 1de0717112e5..0938e0cadf80 100644
> --- a/arch/riscv/include/asm/kvm_aia.h
> +++ b/arch/riscv/include/asm/kvm_aia.h
> @@ -44,10 +44,15 @@ struct kvm_vcpu_aia {
>
> #define irqchip_in_kernel(k) ((k)->arch.aia.in_kernel)
>
> +extern unsigned int kvm_riscv_aia_nr_hgei;
> DECLARE_STATIC_KEY_FALSE(kvm_riscv_aia_available);
> #define kvm_riscv_aia_available() \
> static_branch_unlikely(&kvm_riscv_aia_available)
>
> +static inline void kvm_riscv_vcpu_aia_imsic_release(struct kvm_vcpu *vcpu)
> +{
> +}
> +
> #define KVM_RISCV_AIA_IMSIC_TOPEI (ISELECT_MASK + 1)
> static inline int kvm_riscv_vcpu_aia_imsic_rmw(struct kvm_vcpu *vcpu,
> unsigned long isel,
> @@ -119,6 +124,11 @@ static inline void kvm_riscv_aia_destroy_vm(struct kvm *kvm)
> {
> }
>
> +int kvm_riscv_aia_alloc_hgei(int cpu, struct kvm_vcpu *owner,
> + void __iomem **hgei_va, phys_addr_t *hgei_pa);
> +void kvm_riscv_aia_free_hgei(int cpu, int hgei);
> +void kvm_riscv_aia_wakeon_hgei(struct kvm_vcpu *owner, bool enable);
> +
> void kvm_riscv_aia_enable(void);
> void kvm_riscv_aia_disable(void);
> int kvm_riscv_aia_init(void);
> diff --git a/arch/riscv/kvm/aia.c b/arch/riscv/kvm/aia.c
> index 3d68189e0bb1..f375069fdaf4 100644
> --- a/arch/riscv/kvm/aia.c
> +++ b/arch/riscv/kvm/aia.c
> @@ -7,11 +7,46 @@
> * Anup Patel <[email protected]>
> */
>
> +#include <linux/bitops.h>
> +#include <linux/irq.h>
> +#include <linux/irqdomain.h>
> #include <linux/kvm_host.h>
> +#include <linux/percpu.h>
> +#include <linux/spinlock.h>
> #include <asm/hwcap.h>
>
> +struct aia_hgei_control {
> + raw_spinlock_t lock;
> + unsigned long free_bitmap;
> + struct kvm_vcpu *owners[BITS_PER_LONG];
> +};
> +static DEFINE_PER_CPU(struct aia_hgei_control, aia_hgei);
> +static int hgei_parent_irq;
> +
> +unsigned int kvm_riscv_aia_nr_hgei;
> DEFINE_STATIC_KEY_FALSE(kvm_riscv_aia_available);
>
> +static int aia_find_hgei(struct kvm_vcpu *owner)
> +{
> + int i, hgei;
> + unsigned long flags;
> + struct aia_hgei_control *hgctrl = this_cpu_ptr(&aia_hgei);
> +
> + raw_spin_lock_irqsave(&hgctrl->lock, flags);
> +
> + hgei = -1;
> + for (i = 1; i <= kvm_riscv_aia_nr_hgei; i++) {
> + if (hgctrl->owners[i] == owner) {
> + hgei = i;
> + break;
> + }
> + }
> +
> + raw_spin_unlock_irqrestore(&hgctrl->lock, flags);
> +
> + return hgei;
> +}
> +
> static void aia_set_hvictl(bool ext_irq_pending)
> {
> unsigned long hvictl;
> @@ -55,6 +90,7 @@ void kvm_riscv_vcpu_aia_sync_interrupts(struct kvm_vcpu *vcpu)
>
> bool kvm_riscv_vcpu_aia_has_interrupts(struct kvm_vcpu *vcpu, u64 mask)
> {
> + int hgei;
> unsigned long seip;
>
> if (!kvm_riscv_aia_available())
> @@ -72,6 +108,10 @@ bool kvm_riscv_vcpu_aia_has_interrupts(struct kvm_vcpu *vcpu, u64 mask)
> if (!kvm_riscv_aia_initialized(vcpu->kvm) || !seip)
> return false;
>
> + hgei = aia_find_hgei(vcpu);
> + if (hgei > 0)
> + return (csr_read(CSR_HGEIP) & BIT(hgei)) ? true : false;
> +
> return false;
> }
>
> @@ -343,6 +383,144 @@ int kvm_riscv_vcpu_aia_rmw_ireg(struct kvm_vcpu *vcpu, unsigned int csr_num,
> return KVM_INSN_EXIT_TO_USER_SPACE;
> }
>
> +int kvm_riscv_aia_alloc_hgei(int cpu, struct kvm_vcpu *owner,
> + void __iomem **hgei_va, phys_addr_t *hgei_pa)
> +{
> + int ret = -ENOENT;
> + unsigned long flags;
> + struct aia_hgei_control *hgctrl = per_cpu_ptr(&aia_hgei, cpu);
> +
> + if (!kvm_riscv_aia_available())
> + return -ENODEV;
> + if (!hgctrl)
> + return -ENODEV;
> +
> + raw_spin_lock_irqsave(&hgctrl->lock, flags);
> +
> + if (hgctrl->free_bitmap) {
> + ret = __ffs(hgctrl->free_bitmap);
> + hgctrl->free_bitmap &= ~BIT(ret);
> + hgctrl->owners[ret] = owner;
> + }
> +
> + raw_spin_unlock_irqrestore(&hgctrl->lock, flags);
> +
> + /* TODO: To be updated later by AIA in-kernel irqchip support */
> + if (hgei_va)
> + *hgei_va = NULL;
> + if (hgei_pa)
> + *hgei_pa = 0;
> +
> + return ret;
> +}
> +
> +void kvm_riscv_aia_free_hgei(int cpu, int hgei)
> +{
> + unsigned long flags;
> + struct aia_hgei_control *hgctrl = per_cpu_ptr(&aia_hgei, cpu);
> +
> + if (!kvm_riscv_aia_available() || !hgctrl)
> + return;
> +
> + raw_spin_lock_irqsave(&hgctrl->lock, flags);
> +
> + if (hgei > 0 && hgei <= kvm_riscv_aia_nr_hgei) {
> + if (!(hgctrl->free_bitmap & BIT(hgei))) {
> + hgctrl->free_bitmap |= BIT(hgei);
> + hgctrl->owners[hgei] = NULL;
> + }
> + }
> +
> + raw_spin_unlock_irqrestore(&hgctrl->lock, flags);
> +}
> +
> +void kvm_riscv_aia_wakeon_hgei(struct kvm_vcpu *owner, bool enable)
> +{
> + int hgei;
> +
> + if (!kvm_riscv_aia_available())
> + return;
> +
> + hgei = aia_find_hgei(owner);
> + if (hgei > 0) {
> + if (enable)
> + csr_set(CSR_HGEIE, BIT(hgei));
> + else
> + csr_clear(CSR_HGEIE, BIT(hgei));
> + }
> +}
> +
> +static irqreturn_t hgei_interrupt(int irq, void *dev_id)
> +{
> + int i;
> + unsigned long hgei_mask, flags;
> + struct aia_hgei_control *hgctrl = this_cpu_ptr(&aia_hgei);
> +
> + hgei_mask = csr_read(CSR_HGEIP) & csr_read(CSR_HGEIE);
> + csr_clear(CSR_HGEIE, hgei_mask);
> +
> + raw_spin_lock_irqsave(&hgctrl->lock, flags);
> +
> + for_each_set_bit(i, &hgei_mask, BITS_PER_LONG) {
> + if (hgctrl->owners[i])
> + kvm_vcpu_kick(hgctrl->owners[i]);
> + }
> +
> + raw_spin_unlock_irqrestore(&hgctrl->lock, flags);
> +
> + return IRQ_HANDLED;
> +}
> +
> +static int aia_hgei_init(void)
> +{
> + int cpu, rc;
> + struct irq_domain *domain;
> + struct aia_hgei_control *hgctrl;
> +
> + /* Initialize per-CPU guest external interrupt line management */
> + for_each_possible_cpu(cpu) {
> + hgctrl = per_cpu_ptr(&aia_hgei, cpu);
> + raw_spin_lock_init(&hgctrl->lock);
> + if (kvm_riscv_aia_nr_hgei) {
> + hgctrl->free_bitmap =
> + BIT(kvm_riscv_aia_nr_hgei + 1) - 1;
> + hgctrl->free_bitmap &= ~BIT(0);
> + } else
> + hgctrl->free_bitmap = 0;
> + }
> +
> + /* Find INTC irq domain */
> + domain = irq_find_matching_fwnode(riscv_get_intc_hwnode(),
> + DOMAIN_BUS_ANY);
> + if (!domain) {
> + kvm_err("unable to find INTC domain\n");
> + return -ENOENT;
> + }
> +
> + /* Map per-CPU SGEI interrupt from INTC domain */
> + hgei_parent_irq = irq_create_mapping(domain, IRQ_S_GEXT);
> + if (!hgei_parent_irq) {
> + kvm_err("unable to map SGEI IRQ\n");
> + return -ENOMEM;
> + }
> +
> + /* Request per-CPU SGEI interrupt */
> + rc = request_percpu_irq(hgei_parent_irq, hgei_interrupt,
> + "riscv-kvm", &aia_hgei);
> + if (rc) {
> + kvm_err("failed to request SGEI IRQ\n");
> + return rc;
> + }
> +
> + return 0;
> +}
> +
> +static void aia_hgei_exit(void)
> +{
> + /* Free per-CPU SGEI interrupt */
> + free_percpu_irq(hgei_parent_irq, &aia_hgei);
> +}
> +
> void kvm_riscv_aia_enable(void)
> {
> if (!kvm_riscv_aia_available())
> @@ -357,21 +535,79 @@ void kvm_riscv_aia_enable(void)
> csr_write(CSR_HVIPRIO1H, 0x0);
> csr_write(CSR_HVIPRIO2H, 0x0);
> #endif
> +
> + /* Enable per-CPU SGEI interrupt */
> + enable_percpu_irq(hgei_parent_irq,
> + irq_get_trigger_type(hgei_parent_irq));
> + csr_set(CSR_HIE, BIT(IRQ_S_GEXT));
> }
>
> void kvm_riscv_aia_disable(void)
> {
> + int i;
> + unsigned long flags;
> + struct kvm_vcpu *vcpu;
> + struct aia_hgei_control *hgctrl = this_cpu_ptr(&aia_hgei);
> +
> if (!kvm_riscv_aia_available())
> return;
>
> + /* Disable per-CPU SGEI interrupt */
> + csr_clear(CSR_HIE, BIT(IRQ_S_GEXT));
> + disable_percpu_irq(hgei_parent_irq);
> +
> aia_set_hvictl(false);
> +
> + raw_spin_lock_irqsave(&hgctrl->lock, flags);
> +
> + for (i = 0; i <= kvm_riscv_aia_nr_hgei; i++) {
> + vcpu = hgctrl->owners[i];
> + if (!vcpu)
> + continue;
> +
> + /*
> + * We release hgctrl->lock before notifying IMSIC
> + * so that we don't have lock ordering issues.
> + */
> + raw_spin_unlock_irqrestore(&hgctrl->lock, flags);
> +
> + /* Notify IMSIC */
> + kvm_riscv_vcpu_aia_imsic_release(vcpu);
> +
> + /*
> + * Wakeup VCPU if it was blocked so that it can
> + * run on other HARTs
> + */
> + if (csr_read(CSR_HGEIE) & BIT(i)) {
> + csr_clear(CSR_HGEIE, BIT(i));
> + kvm_vcpu_kick(vcpu);
> + }
> +
> + raw_spin_lock_irqsave(&hgctrl->lock, flags);
> + }
> +

We shouldn't access vcpu related data here as
kvm_arch_destroy_vm/kvm_destroy_vcpus
is called before hardware_disable_all is invoked.

> + raw_spin_unlock_irqrestore(&hgctrl->lock, flags);
> }
>
> int kvm_riscv_aia_init(void)
> {
> + int rc;
> +
> if (!riscv_isa_extension_available(NULL, SxAIA))
> return -ENODEV;
>
> + /* Figure-out number of bits in HGEIE */
> + csr_write(CSR_HGEIE, -1UL);
> + kvm_riscv_aia_nr_hgei = fls_long(csr_read(CSR_HGEIE));
> + csr_write(CSR_HGEIE, 0);
> + if (kvm_riscv_aia_nr_hgei)
> + kvm_riscv_aia_nr_hgei--;
> +
> + /* Initialize guest external interrupt line management */
> + rc = aia_hgei_init();
> + if (rc)
> + return rc;
> +
> /* Enable KVM AIA support */
> static_branch_enable(&kvm_riscv_aia_available);
>
> @@ -380,4 +616,9 @@ int kvm_riscv_aia_init(void)
>
> void kvm_riscv_aia_exit(void)
> {
> + if (!kvm_riscv_aia_available())
> + return;
> +
> + /* Cleanup the HGEI state */
> + aia_hgei_exit();
> }
> diff --git a/arch/riscv/kvm/main.c b/arch/riscv/kvm/main.c
> index d8ff44eb04ca..5cf37dbe8a38 100644
> --- a/arch/riscv/kvm/main.c
> +++ b/arch/riscv/kvm/main.c
> @@ -125,7 +125,8 @@ int kvm_arch_init(void *opaque)
> kvm_info("VMID %ld bits available\n", kvm_riscv_gstage_vmid_bits());
>
> if (kvm_riscv_aia_available())
> - kvm_info("AIA available\n");
> + kvm_info("AIA available with %d guest external interrupts\n",
> + kvm_riscv_aia_nr_hgei);
>
> return 0;
> }
> diff --git a/arch/riscv/kvm/vcpu.c b/arch/riscv/kvm/vcpu.c
> index 151b35b3b05f..1daa1936b642 100644
> --- a/arch/riscv/kvm/vcpu.c
> +++ b/arch/riscv/kvm/vcpu.c
> @@ -240,10 +240,12 @@ int kvm_cpu_has_pending_timer(struct kvm_vcpu *vcpu)
>
> void kvm_arch_vcpu_blocking(struct kvm_vcpu *vcpu)
> {
> + kvm_riscv_aia_wakeon_hgei(vcpu, true);
> }
>
> void kvm_arch_vcpu_unblocking(struct kvm_vcpu *vcpu)
> {
> + kvm_riscv_aia_wakeon_hgei(vcpu, false);
> }
>
> int kvm_arch_vcpu_runnable(struct kvm_vcpu *vcpu)
> --
> 2.34.1
>


--
Regards,
Atish

2023-02-08 03:06:41

by Anup Patel

[permalink] [raw]
Subject: Re: [PATCH v2 2/7] RISC-V: Detect AIA CSRs from ISA string

On Tue, Feb 7, 2023 at 11:45 PM Atish Patra <[email protected]> wrote:
>
> On Tue, Feb 7, 2023 at 10:05 AM Conor Dooley <[email protected]> wrote:
> >
> > Hey Anup, Palmer,
> >
> > On Fri, Feb 03, 2023 at 05:31:01PM +0530, Anup Patel wrote:
> > > On Fri, Feb 3, 2023 at 5:54 AM Palmer Dabbelt <[email protected]> wrote:
> > > >
> > > > On Fri, 27 Jan 2023 23:27:32 PST (-0800), [email protected] wrote:
> > > > > We have two extension names for AIA ISA support: Smaia (M-mode AIA CSRs)
> > > > > and Ssaia (S-mode AIA CSRs).
> > > >
> > > > This has pretty much the same problem that we had with the other
> > > > AIA-related ISA string patches, where there's that ambiguity with the
> > > > non-ratified chapters. IIRC when this came up in GCC the rough idea was
> > > > to try and document that we're going to interpret the standard ISA
> > > > strings that way, but now that we're doing custom ISA extensions it
> > > > seems saner to just define on here that removes the ambiguity.
> > > >
> > > > I just sent
> > > > <https://lore.kernel.org/r/[email protected]/>
> > > > which documents that.
> > >
> > > I am not sure why you say that these are custom extensions.
> > >
> > > Multiple folks have clarified that both Smaia and Ssaia are frozen
> > > ISA extensions as-per RVI process. The individual chapters which
> > > are in the draft state have nothing to do with Smaia and Ssaia CSRs.
> > >
> > > Please refer:
> > > https://github.com/riscv/riscv-aia/pull/36
> > > https://lists.riscv.org/g/tech-aia/message/336
> > > https://lists.riscv.org/g/tech-aia/message/337
> >
> > All of these links seem to discuss the draft chapters somehow being
> > incompatible with the non-draft ones. I would very expect that that,
> > as pointed out in several places there, that the draft chapters
> > finalisation would not lead to meaningful (and incompatible!) changes
> > being made to the non-draft chapters.
> >
>
> Here is the status of all RVI specs. It states that the Smaia, Ssaia
> extensions are frozen (i.e. public review complete).
> https://wiki.riscv.org/display/HOME/Specification+Status
>
> I have added stephano/Jeff to confirm the same.
>
> AFAIK, IOMMU spec is close to the public review phase and should be
> frozen in this or next quarter.
> IIRC, this chapter in AIA will be frozen along with IOMMU spec.
>
> Anup: Please correct me if that's not correct.

Yes, that's correct.

>
> > Maybe yourself and Palmer are looking at this from different
> > perspectives? Looking at his patch from Friday:
> > https://lore.kernel.org/linux-riscv/[email protected]/
> > He specifically mentioned this aspect, as opposed to the aspect that
> > your links refer to.
> >
> > Surely a duo-plic, if that ever comes to be, could be detected from
> > compatible strings in DT or w/e - but how do you intend differentiating
> > between an implementation of S*aia that contains the IOMMU support in
> > Chapter 9 in a finalised form, versus an implementation that may make
> > "different decisions" when it comes to that chapter of the spec?
>
> We will most likely have an extension specific to iommu spec as well.
>
> > I thought that would be handled by extension versions, but I am told
> > that those are not a thing any more.
> > If that's not true, and there'll be a version number that we can pull in
> > from a DT and parse which will distinguish between the two, then please
> > correct my misunderstanding here!
> >
> > Thanks,
> > Conor.
> >
> > > > > We extend the ISA string parsing to detect Smaia and Ssaia extensions.
> > > > >
> > > > > Signed-off-by: Anup Patel <[email protected]>
> > > > > Reviewed-by: Andrew Jones <[email protected]>
> > > > > ---
> > > > > arch/riscv/include/asm/hwcap.h | 2 ++
> > > > > arch/riscv/kernel/cpu.c | 2 ++
> > > > > arch/riscv/kernel/cpufeature.c | 2 ++
> > > > > 3 files changed, 6 insertions(+)
> > > > >
> > > > > diff --git a/arch/riscv/include/asm/hwcap.h b/arch/riscv/include/asm/hwcap.h
> > > > > index 86328e3acb02..341ef30a3718 100644
> > > > > --- a/arch/riscv/include/asm/hwcap.h
> > > > > +++ b/arch/riscv/include/asm/hwcap.h
> > > > > @@ -59,6 +59,8 @@ enum riscv_isa_ext_id {
> > > > > RISCV_ISA_EXT_ZIHINTPAUSE,
> > > > > RISCV_ISA_EXT_SSTC,
> > > > > RISCV_ISA_EXT_SVINVAL,
> > > > > + RISCV_ISA_EXT_SMAIA,
> > > > > + RISCV_ISA_EXT_SSAIA,
> > > > > RISCV_ISA_EXT_ID_MAX
> > > > > };
> > > > > static_assert(RISCV_ISA_EXT_ID_MAX <= RISCV_ISA_EXT_MAX);
> > > > > diff --git a/arch/riscv/kernel/cpu.c b/arch/riscv/kernel/cpu.c
> > > > > index 1b9a5a66e55a..a215ec929160 100644
> > > > > --- a/arch/riscv/kernel/cpu.c
> > > > > +++ b/arch/riscv/kernel/cpu.c
> > > > > @@ -162,6 +162,8 @@ arch_initcall(riscv_cpuinfo_init);
> > > > > * extensions by an underscore.
> > > > > */
> > > > > static struct riscv_isa_ext_data isa_ext_arr[] = {
> > > > > + __RISCV_ISA_EXT_DATA(smaia, RISCV_ISA_EXT_SMAIA),
> > > > > + __RISCV_ISA_EXT_DATA(ssaia, RISCV_ISA_EXT_SSAIA),
> > > >
> > > > This will conflict with that ISA string refactoring I just merged. It
> > > > should be a pretty mechanical merge conflict, but if you want we can do
> > > > a shared tag with the first few patches and I can handle the merge
> > > > conflict locally.
> > >
> > > I am planning to send this series as a second PR for Linux-6.3 after your
> > > PR (which includes ISA string refactoring) is merged. Is that okay with you?
> > >
> > > With that said, it would request you to ACK this patch as well.
> > >
> > > >
> > > > > __RISCV_ISA_EXT_DATA(sscofpmf, RISCV_ISA_EXT_SSCOFPMF),
> > > > > __RISCV_ISA_EXT_DATA(sstc, RISCV_ISA_EXT_SSTC),
> > > > > __RISCV_ISA_EXT_DATA(svinval, RISCV_ISA_EXT_SVINVAL),
> > > > > diff --git a/arch/riscv/kernel/cpufeature.c b/arch/riscv/kernel/cpufeature.c
> > > > > index 93e45560af30..3c5b51f519d5 100644
> > > > > --- a/arch/riscv/kernel/cpufeature.c
> > > > > +++ b/arch/riscv/kernel/cpufeature.c
> > > > > @@ -228,6 +228,8 @@ void __init riscv_fill_hwcap(void)
> > > > > SET_ISA_EXT_MAP("zihintpause", RISCV_ISA_EXT_ZIHINTPAUSE);
> > > > > SET_ISA_EXT_MAP("sstc", RISCV_ISA_EXT_SSTC);
> > > > > SET_ISA_EXT_MAP("svinval", RISCV_ISA_EXT_SVINVAL);
> > > > > + SET_ISA_EXT_MAP("smaia", RISCV_ISA_EXT_SMAIA);
> > > > > + SET_ISA_EXT_MAP("ssaia", RISCV_ISA_EXT_SSAIA);
> > > > > }
> > > > > #undef SET_ISA_EXT_MAP
> > > > > }
> > >
> > > Thanks,
> > > Anup
>
>
>
> --
> Regards,
> Atish

Regards,
Anup

2023-02-08 03:54:47

by Anup Patel

[permalink] [raw]
Subject: Re: [PATCH v2 2/7] RISC-V: Detect AIA CSRs from ISA string

On Wed, Feb 8, 2023 at 2:09 AM Conor Dooley <[email protected]> wrote:
>
> On Tue, Feb 07, 2023 at 10:15:22AM -0800, Atish Patra wrote:
> > On Tue, Feb 7, 2023 at 10:05 AM Conor Dooley <[email protected]> wrote:
> > > On Fri, Feb 03, 2023 at 05:31:01PM +0530, Anup Patel wrote:
> > > > On Fri, Feb 3, 2023 at 5:54 AM Palmer Dabbelt <[email protected]> wrote:
> > > > >
> > > > > On Fri, 27 Jan 2023 23:27:32 PST (-0800), [email protected] wrote:
> > > > > > We have two extension names for AIA ISA support: Smaia (M-mode AIA CSRs)
> > > > > > and Ssaia (S-mode AIA CSRs).
> > > > >
> > > > > This has pretty much the same problem that we had with the other
> > > > > AIA-related ISA string patches, where there's that ambiguity with the
> > > > > non-ratified chapters. IIRC when this came up in GCC the rough idea was
> > > > > to try and document that we're going to interpret the standard ISA
> > > > > strings that way, but now that we're doing custom ISA extensions it
> > > > > seems saner to just define on here that removes the ambiguity.
> > > > >
> > > > > I just sent
> > > > > <https://lore.kernel.org/r/[email protected]/>
> > > > > which documents that.
> > > >
> > > > I am not sure why you say that these are custom extensions.
> > > >
> > > > Multiple folks have clarified that both Smaia and Ssaia are frozen
> > > > ISA extensions as-per RVI process. The individual chapters which
> > > > are in the draft state have nothing to do with Smaia and Ssaia CSRs.
> > > >
> > > > Please refer:
> > > > https://github.com/riscv/riscv-aia/pull/36
> > > > https://lists.riscv.org/g/tech-aia/message/336
> > > > https://lists.riscv.org/g/tech-aia/message/337
> > >
> > > All of these links seem to discuss the draft chapters somehow being
> > > incompatible with the non-draft ones. I would very expect that that,
> > > as pointed out in several places there, that the draft chapters
> > > finalisation would not lead to meaningful (and incompatible!) changes
> > > being made to the non-draft chapters.
> > >
> >
> > Here is the status of all RVI specs. It states that the Smaia, Ssaia
> > extensions are frozen (i.e. public review complete).
> > https://wiki.riscv.org/display/HOME/Specification+Status
> >
> > I have added stephano/Jeff to confirm the same.
> >
> > AFAIK, IOMMU spec is close to the public review phase and should be
> > frozen in this or next quarter.
> > IIRC, this chapter in AIA will be frozen along with IOMMU spec.
> >
> > Anup: Please correct me if that's not correct.
> >
> > > Maybe yourself and Palmer are looking at this from different
> > > perspectives? Looking at his patch from Friday:
> > > https://lore.kernel.org/linux-riscv/[email protected]/
> > > He specifically mentioned this aspect, as opposed to the aspect that
> > > your links refer to.
> > >
> > > Surely a duo-plic, if that ever comes to be, could be detected from
> > > compatible strings in DT or w/e - but how do you intend differentiating
> > > between an implementation of S*aia that contains the IOMMU support in
> > > Chapter 9 in a finalised form, versus an implementation that may make
> > > "different decisions" when it comes to that chapter of the spec?
> >
> > We will most likely have an extension specific to iommu spec as well.
>
> Right, but unless I am misunderstanding you, that is an extension for the
> IOMMU spec, not for Chapter 9 of the AIA spec?
>
> I would say that it is likely that if you have AIA and IOMMU that you'd
> want to be implementing Chapter 9, but that would not appear sufficient to
> draw a conclusion from.
>
> Maybe the RVI lads that you've added (or Anup for that matter!) can
> clarify if there is a requirement that if you do AIA and IOMMU that you
> must do Chapter 9.
> If not, my prior question about a differentiation mechanism still applies
> I think!

For the benefit of everyone, the AIA spec mainly defines three
modular components:
1) Extended Local Interrupt CSRs (Smaia and Ssaia extensions)
(ISA extension covered by: Chapter 2, Chapter 6, and Chapter 7)
2) Incoming MSI Controller (IMSIC)
(ISA and Non-ISA extension covered by: Chapter 3 and Chapter 8)
3) Advanced PLIC (APLIC)
(Non-ISA extension covered by: Chapter 4)

Apart from above, we have Chapter 5 ("Duo-PLIC") and Chapter 9
("IOMMU Support for MSIs to Virtual Machines") which are in draft
state.

Currently, there are no RISC-V members who have expressed
interest in implementing Chapter 5 ("Duo-PLIC") so this chapter
will stay in draft state for a foreseeable future.

The Chapter 9 ("IOMMU Support for MSIs to Virtual Machines")
defines an optional feature of IOMMU which can be implemented
by a standard IOMMU (such as RISC-V IOMMU) or a vendor specific
IOMMU. A RISC-V platform can certainly support device pass-through
using IMSIC guest files and an IOMMU which does not implement
Chapter 9. Unfortunately, there is a limit on the maximum number
of per-HART IMSIC guest files which can further limit the number
of pass-through devices. The Chapter 9 allows RISC-V platforms
to support large number of pass-through devices by defining "MRIF
- memory resident interrupt files" for an IOMMU. Further, the MRIFs
defined by Chapter 9 are simply interrupt files located in main memory
and have nothing to do with AIA local interrupt CSRs (Smaia and Ssaia).

The presence of S*aia in ISA string only implies that AIA extended
local interrupt CSRs are implemented by the underlying RISC-V
implementation.

I confirm that it is certainly not mandatory for a RISC-V platform to
implement Chapter 9 of the AIA specification if the RISC-V platform
already implements AIA and IOMMU.

>
> > > I thought that would be handled by extension versions, but I am told
> > > that those are not a thing any more.
> > > If that's not true, and there'll be a version number that we can pull in
> > > from a DT and parse which will distinguish between the two, then please
> > > correct my misunderstanding here!

Regards,
Anup

2023-02-08 12:57:45

by Conor Dooley

[permalink] [raw]
Subject: Re: [PATCH v2 2/7] RISC-V: Detect AIA CSRs from ISA string

Hey Anup!

On Wed, Feb 08, 2023 at 09:24:28AM +0530, Anup Patel wrote:
> On Wed, Feb 8, 2023 at 2:09 AM Conor Dooley <[email protected]> wrote:
> > On Tue, Feb 07, 2023 at 10:15:22AM -0800, Atish Patra wrote:
> > > On Tue, Feb 7, 2023 at 10:05 AM Conor Dooley <[email protected]> wrote:
> > > > On Fri, Feb 03, 2023 at 05:31:01PM +0530, Anup Patel wrote:
> > > > > On Fri, Feb 3, 2023 at 5:54 AM Palmer Dabbelt <[email protected]> wrote:
> > > > > >
> > > > > > On Fri, 27 Jan 2023 23:27:32 PST (-0800), [email protected] wrote:
> > > > > > > We have two extension names for AIA ISA support: Smaia (M-mode AIA CSRs)
> > > > > > > and Ssaia (S-mode AIA CSRs).
> > > > > >
> > > > > > This has pretty much the same problem that we had with the other
> > > > > > AIA-related ISA string patches, where there's that ambiguity with the
> > > > > > non-ratified chapters. IIRC when this came up in GCC the rough idea was
> > > > > > to try and document that we're going to interpret the standard ISA
> > > > > > strings that way, but now that we're doing custom ISA extensions it
> > > > > > seems saner to just define on here that removes the ambiguity.
> > > > > >
> > > > > > I just sent
> > > > > > <https://lore.kernel.org/r/[email protected]/>
> > > > > > which documents that.
> > > > >
> > > > > I am not sure why you say that these are custom extensions.
> > > > >
> > > > > Multiple folks have clarified that both Smaia and Ssaia are frozen
> > > > > ISA extensions as-per RVI process. The individual chapters which
> > > > > are in the draft state have nothing to do with Smaia and Ssaia CSRs.
> > > > >
> > > > > Please refer:
> > > > > https://github.com/riscv/riscv-aia/pull/36
> > > > > https://lists.riscv.org/g/tech-aia/message/336
> > > > > https://lists.riscv.org/g/tech-aia/message/337
> > > >
> > > > All of these links seem to discuss the draft chapters somehow being
> > > > incompatible with the non-draft ones. I would very expect that that,
> > > > as pointed out in several places there, that the draft chapters
> > > > finalisation would not lead to meaningful (and incompatible!) changes
> > > > being made to the non-draft chapters.
> > > >
> > >
> > > Here is the status of all RVI specs. It states that the Smaia, Ssaia
> > > extensions are frozen (i.e. public review complete).
> > > https://wiki.riscv.org/display/HOME/Specification+Status
> > >
> > > I have added stephano/Jeff to confirm the same.
> > >
> > > AFAIK, IOMMU spec is close to the public review phase and should be
> > > frozen in this or next quarter.
> > > IIRC, this chapter in AIA will be frozen along with IOMMU spec.
> > >
> > > Anup: Please correct me if that's not correct.
> > >
> > > > Maybe yourself and Palmer are looking at this from different
> > > > perspectives? Looking at his patch from Friday:
> > > > https://lore.kernel.org/linux-riscv/[email protected]/
> > > > He specifically mentioned this aspect, as opposed to the aspect that
> > > > your links refer to.
> > > >
> > > > Surely a duo-plic, if that ever comes to be, could be detected from
> > > > compatible strings in DT or w/e - but how do you intend differentiating
> > > > between an implementation of S*aia that contains the IOMMU support in
> > > > Chapter 9 in a finalised form, versus an implementation that may make
> > > > "different decisions" when it comes to that chapter of the spec?
> > >
> > > We will most likely have an extension specific to iommu spec as well.
> >
> > Right, but unless I am misunderstanding you, that is an extension for the
> > IOMMU spec, not for Chapter 9 of the AIA spec?
> >
> > I would say that it is likely that if you have AIA and IOMMU that you'd
> > want to be implementing Chapter 9, but that would not appear sufficient to
> > draw a conclusion from.
> >
> > Maybe the RVI lads that you've added (or Anup for that matter!) can
> > clarify if there is a requirement that if you do AIA and IOMMU that you
> > must do Chapter 9.
> > If not, my prior question about a differentiation mechanism still applies
> > I think!
>
> For the benefit of everyone, the AIA spec mainly defines three
> modular components:
> 1) Extended Local Interrupt CSRs (Smaia and Ssaia extensions)
> (ISA extension covered by: Chapter 2, Chapter 6, and Chapter 7)
> 2) Incoming MSI Controller (IMSIC)
> (ISA and Non-ISA extension covered by: Chapter 3 and Chapter 8)
> 3) Advanced PLIC (APLIC)
> (Non-ISA extension covered by: Chapter 4)
>
> Apart from above, we have Chapter 5 ("Duo-PLIC") and Chapter 9
> ("IOMMU Support for MSIs to Virtual Machines") which are in draft
> state.
>
> Currently, there are no RISC-V members who have expressed
> interest in implementing Chapter 5 ("Duo-PLIC") so this chapter
> will stay in draft state for a foreseeable future.

Thanks for the clarifications :)

> The Chapter 9 ("IOMMU Support for MSIs to Virtual Machines")
> defines an optional feature of IOMMU which can be implemented
> by a standard IOMMU (such as RISC-V IOMMU) or a vendor specific
> IOMMU. A RISC-V platform can certainly support device pass-through
> using IMSIC guest files and an IOMMU which does not implement
> Chapter 9. Unfortunately, there is a limit on the maximum number
> of per-HART IMSIC guest files which can further limit the number
> of pass-through devices. The Chapter 9 allows RISC-V platforms
> to support large number of pass-through devices by defining "MRIF
> - memory resident interrupt files" for an IOMMU. Further, the MRIFs
> defined by Chapter 9 are simply interrupt files located in main memory
> and have nothing to do with AIA local interrupt CSRs (Smaia and Ssaia).
>
> The presence of S*aia in ISA string only implies that AIA extended
> local interrupt CSRs are implemented by the underlying RISC-V
> implementation.

Would you mind linking to where this is documented & explaining in your
commit message why it is okay operate on the basis of s*aia in the ISA
string only mandates the presence of the CSRs and nothing more.

I think when I was reading it last night, I saw some commentary in this
vein in Section 1.6 of the rc2 spec. Although IIRC it noted changes in
interrupt behaviour there too, so I'm not sure if that section is what you
are referring to here.

Perhaps this is all just a good argument for providing more information
in commit messages ;)

> I confirm that it is certainly not mandatory for a RISC-V platform to
> implement Chapter 9 of the AIA specification if the RISC-V platform
> already implements AIA and IOMMU.

Cool, thanks.
By what mechanism, since you say that "s*aia" in the ISA string only
implies presence of the CSRs [sic], are we meant to discover the
presence of Chapter 9?
A property of the IOMMU node seems the most logical I suppose, or
perhaps inferred from the presence/config of other properties of said
node?

> > > > I thought that would be handled by extension versions, but I am told
> > > > that those are not a thing any more.
> > > > If that's not true, and there'll be a version number that we can pull in
> > > > from a DT and parse which will distinguish between the two, then please
> > > > correct my misunderstanding here!

Thanks again Anup,
Conor.


Attachments:
(No filename) (7.05 kB)
signature.asc (228.00 B)
Download all attachments

2023-02-08 14:57:41

by Anup Patel

[permalink] [raw]
Subject: Re: [PATCH v2 2/7] RISC-V: Detect AIA CSRs from ISA string

On Wed, Feb 8, 2023 at 6:27 PM Conor Dooley <[email protected]> wrote:
>
> Hey Anup!
>
> On Wed, Feb 08, 2023 at 09:24:28AM +0530, Anup Patel wrote:
> > On Wed, Feb 8, 2023 at 2:09 AM Conor Dooley <[email protected]> wrote:
> > > On Tue, Feb 07, 2023 at 10:15:22AM -0800, Atish Patra wrote:
> > > > On Tue, Feb 7, 2023 at 10:05 AM Conor Dooley <[email protected]> wrote:
> > > > > On Fri, Feb 03, 2023 at 05:31:01PM +0530, Anup Patel wrote:
> > > > > > On Fri, Feb 3, 2023 at 5:54 AM Palmer Dabbelt <[email protected]> wrote:
> > > > > > >
> > > > > > > On Fri, 27 Jan 2023 23:27:32 PST (-0800), [email protected] wrote:
> > > > > > > > We have two extension names for AIA ISA support: Smaia (M-mode AIA CSRs)
> > > > > > > > and Ssaia (S-mode AIA CSRs).
> > > > > > >
> > > > > > > This has pretty much the same problem that we had with the other
> > > > > > > AIA-related ISA string patches, where there's that ambiguity with the
> > > > > > > non-ratified chapters. IIRC when this came up in GCC the rough idea was
> > > > > > > to try and document that we're going to interpret the standard ISA
> > > > > > > strings that way, but now that we're doing custom ISA extensions it
> > > > > > > seems saner to just define on here that removes the ambiguity.
> > > > > > >
> > > > > > > I just sent
> > > > > > > <https://lore.kernel.org/r/[email protected]/>
> > > > > > > which documents that.
> > > > > >
> > > > > > I am not sure why you say that these are custom extensions.
> > > > > >
> > > > > > Multiple folks have clarified that both Smaia and Ssaia are frozen
> > > > > > ISA extensions as-per RVI process. The individual chapters which
> > > > > > are in the draft state have nothing to do with Smaia and Ssaia CSRs.
> > > > > >
> > > > > > Please refer:
> > > > > > https://github.com/riscv/riscv-aia/pull/36
> > > > > > https://lists.riscv.org/g/tech-aia/message/336
> > > > > > https://lists.riscv.org/g/tech-aia/message/337
> > > > >
> > > > > All of these links seem to discuss the draft chapters somehow being
> > > > > incompatible with the non-draft ones. I would very expect that that,
> > > > > as pointed out in several places there, that the draft chapters
> > > > > finalisation would not lead to meaningful (and incompatible!) changes
> > > > > being made to the non-draft chapters.
> > > > >
> > > >
> > > > Here is the status of all RVI specs. It states that the Smaia, Ssaia
> > > > extensions are frozen (i.e. public review complete).
> > > > https://wiki.riscv.org/display/HOME/Specification+Status
> > > >
> > > > I have added stephano/Jeff to confirm the same.
> > > >
> > > > AFAIK, IOMMU spec is close to the public review phase and should be
> > > > frozen in this or next quarter.
> > > > IIRC, this chapter in AIA will be frozen along with IOMMU spec.
> > > >
> > > > Anup: Please correct me if that's not correct.
> > > >
> > > > > Maybe yourself and Palmer are looking at this from different
> > > > > perspectives? Looking at his patch from Friday:
> > > > > https://lore.kernel.org/linux-riscv/[email protected]/
> > > > > He specifically mentioned this aspect, as opposed to the aspect that
> > > > > your links refer to.
> > > > >
> > > > > Surely a duo-plic, if that ever comes to be, could be detected from
> > > > > compatible strings in DT or w/e - but how do you intend differentiating
> > > > > between an implementation of S*aia that contains the IOMMU support in
> > > > > Chapter 9 in a finalised form, versus an implementation that may make
> > > > > "different decisions" when it comes to that chapter of the spec?
> > > >
> > > > We will most likely have an extension specific to iommu spec as well.
> > >
> > > Right, but unless I am misunderstanding you, that is an extension for the
> > > IOMMU spec, not for Chapter 9 of the AIA spec?
> > >
> > > I would say that it is likely that if you have AIA and IOMMU that you'd
> > > want to be implementing Chapter 9, but that would not appear sufficient to
> > > draw a conclusion from.
> > >
> > > Maybe the RVI lads that you've added (or Anup for that matter!) can
> > > clarify if there is a requirement that if you do AIA and IOMMU that you
> > > must do Chapter 9.
> > > If not, my prior question about a differentiation mechanism still applies
> > > I think!
> >
> > For the benefit of everyone, the AIA spec mainly defines three
> > modular components:
> > 1) Extended Local Interrupt CSRs (Smaia and Ssaia extensions)
> > (ISA extension covered by: Chapter 2, Chapter 6, and Chapter 7)
> > 2) Incoming MSI Controller (IMSIC)
> > (ISA and Non-ISA extension covered by: Chapter 3 and Chapter 8)
> > 3) Advanced PLIC (APLIC)
> > (Non-ISA extension covered by: Chapter 4)
> >
> > Apart from above, we have Chapter 5 ("Duo-PLIC") and Chapter 9
> > ("IOMMU Support for MSIs to Virtual Machines") which are in draft
> > state.
> >
> > Currently, there are no RISC-V members who have expressed
> > interest in implementing Chapter 5 ("Duo-PLIC") so this chapter
> > will stay in draft state for a foreseeable future.
>
> Thanks for the clarifications :)
>
> > The Chapter 9 ("IOMMU Support for MSIs to Virtual Machines")
> > defines an optional feature of IOMMU which can be implemented
> > by a standard IOMMU (such as RISC-V IOMMU) or a vendor specific
> > IOMMU. A RISC-V platform can certainly support device pass-through
> > using IMSIC guest files and an IOMMU which does not implement
> > Chapter 9. Unfortunately, there is a limit on the maximum number
> > of per-HART IMSIC guest files which can further limit the number
> > of pass-through devices. The Chapter 9 allows RISC-V platforms
> > to support large number of pass-through devices by defining "MRIF
> > - memory resident interrupt files" for an IOMMU. Further, the MRIFs
> > defined by Chapter 9 are simply interrupt files located in main memory
> > and have nothing to do with AIA local interrupt CSRs (Smaia and Ssaia).
> >
> > The presence of S*aia in ISA string only implies that AIA extended
> > local interrupt CSRs are implemented by the underlying RISC-V
> > implementation.
>
> Would you mind linking to where this is documented & explaining in your
> commit message why it is okay operate on the basis of s*aia in the ISA
> string only mandates the presence of the CSRs and nothing more.
>
> I think when I was reading it last night, I saw some commentary in this
> vein in Section 1.6 of the rc2 spec. Although IIRC it noted changes in
> interrupt behaviour there too, so I'm not sure if that section is what you
> are referring to here.
>
> Perhaps this is all just a good argument for providing more information
> in commit messages ;)

Sure, I am anyway going to send v3 after rebase so I will cite the
Section 1.6 of AIA spec in the commit description.

>
> > I confirm that it is certainly not mandatory for a RISC-V platform to
> > implement Chapter 9 of the AIA specification if the RISC-V platform
> > already implements AIA and IOMMU.
>
> Cool, thanks.
> By what mechanism, since you say that "s*aia" in the ISA string only
> implies presence of the CSRs [sic], are we meant to discover the
> presence of Chapter 9?
> A property of the IOMMU node seems the most logical I suppose, or
> perhaps inferred from the presence/config of other properties of said
> node?

The discovery of AIA Chapter 9 features is IOMMU specific.

The upcoming RISC-V IOMMU spec defines "capabilities" MMIO register
which has read-only bits MSI_MRIF and MSI_FLAT for discovering features
defined in AIA Chapter 9. Due to this dependency, the AIA Chapter 9 is
going to be ratified along with RISC-V IOMMU ratification.

>
> > > > > I thought that would be handled by extension versions, but I am told
> > > > > that those are not a thing any more.
> > > > > If that's not true, and there'll be a version number that we can pull in
> > > > > from a DT and parse which will distinguish between the two, then please
> > > > > correct my misunderstanding here!
>
> Thanks again Anup,
> Conor.
>

Regards,
Anup

2023-02-09 23:31:40

by Conor Dooley

[permalink] [raw]
Subject: Re: [PATCH v2 2/7] RISC-V: Detect AIA CSRs from ISA string

Hey all,

Just circling back to this one, since the reply from Palmer was to
another thread with a much smaller CC list.

On Wed, Feb 08, 2023 at 08:27:23PM +0530, Anup Patel wrote:
> On Wed, Feb 8, 2023 at 6:27 PM Conor Dooley <[email protected]> wrote:
> > On Wed, Feb 08, 2023 at 09:24:28AM +0530, Anup Patel wrote:

> > > The presence of S*aia in ISA string only implies that AIA extended
> > > local interrupt CSRs are implemented by the underlying RISC-V
> > > implementation.
> >
> > Would you mind linking to where this is documented & explaining in your
> > commit message why it is okay operate on the basis of s*aia in the ISA
> > string only mandates the presence of the CSRs and nothing more.
> >
> > I think when I was reading it last night, I saw some commentary in this
> > vein in Section 1.6 of the rc2 spec. Although IIRC it noted changes in
> > interrupt behaviour there too, so I'm not sure if that section is what you
> > are referring to here.
> >
> > Perhaps this is all just a good argument for providing more information
> > in commit messages ;)
>
> Sure, I am anyway going to send v3 after rebase so I will cite the
> Section 1.6 of AIA spec in the commit description.

We had a nice conversation about this on during the weekly patchwork
sync call :)
The end result of that one was "inconclusive" and the outcome appears to
be that we will wait until the entire spec is frozen before doing
anything here.
Palmer left a comment in response to another thread to that effect:
https://lore.kernel.org/linux-riscv/mhng-474f7ecd-65b8-4cfa-8b75-e51b896cc58e@palmer-ri-x1c9/

Cheers,
Conor.


Attachments:
(No filename) (1.58 kB)
signature.asc (228.00 B)
Download all attachments

2023-02-15 15:41:55

by Christoph Müllner

[permalink] [raw]
Subject: Re: [PATCH v2 2/7] RISC-V: Detect AIA CSRs from ISA string

On Fri, Feb 3, 2023 at 1:25 AM Palmer Dabbelt <[email protected]> wrote:
>
> On Fri, 27 Jan 2023 23:27:32 PST (-0800), [email protected] wrote:
> > We have two extension names for AIA ISA support: Smaia (M-mode AIA CSRs)
> > and Ssaia (S-mode AIA CSRs).
>
> This has pretty much the same problem that we had with the other
> AIA-related ISA string patches, where there's that ambiguity with the
> non-ratified chapters. IIRC when this came up in GCC the rough idea was
> to try and document that we're going to interpret the standard ISA
> strings that way, but now that we're doing custom ISA extensions it
> seems saner to just define on here that removes the ambiguity.


To avoid the impression that I did not work on that, here is the v2
from November,
that attempts to document this:
https://gcc.gnu.org/pipermail/gcc-patches/2022-November/607326.html

My proposed text was:
+Note, that AIA support (@samp{Smaia} and @samp{Ssaia}) is based on an AIA
+specification, which is frozen but contains draft chapters ("Duo-PLIC" and
+"IOMMU Support").

Btw, I did not get any feedback on that patch.

I also tried to address this on spec level (the PR has been linked) and raised
that to tech-aia (the conversation has been linked).

Another thing that I want to highlight, since it was discussed a lot recently
(e.g. just a few minutes ago in tech-chairs).
There is a chance of a last-minute spec change of AIA:
https://github.com/riscv/riscv-aia/pull/37

BR
Christoph



>
>
> I just sent
> <https://lore.kernel.org/r/[email protected]/>
> which documents that.
>
> > We extend the ISA string parsing to detect Smaia and Ssaia extensions.
> >
> > Signed-off-by: Anup Patel <[email protected]>
> > Reviewed-by: Andrew Jones <[email protected]>
> > ---
> > arch/riscv/include/asm/hwcap.h | 2 ++
> > arch/riscv/kernel/cpu.c | 2 ++
> > arch/riscv/kernel/cpufeature.c | 2 ++
> > 3 files changed, 6 insertions(+)
> >
> > diff --git a/arch/riscv/include/asm/hwcap.h b/arch/riscv/include/asm/hwcap.h
> > index 86328e3acb02..341ef30a3718 100644
> > --- a/arch/riscv/include/asm/hwcap.h
> > +++ b/arch/riscv/include/asm/hwcap.h
> > @@ -59,6 +59,8 @@ enum riscv_isa_ext_id {
> > RISCV_ISA_EXT_ZIHINTPAUSE,
> > RISCV_ISA_EXT_SSTC,
> > RISCV_ISA_EXT_SVINVAL,
> > + RISCV_ISA_EXT_SMAIA,
> > + RISCV_ISA_EXT_SSAIA,
> > RISCV_ISA_EXT_ID_MAX
> > };
> > static_assert(RISCV_ISA_EXT_ID_MAX <= RISCV_ISA_EXT_MAX);
> > diff --git a/arch/riscv/kernel/cpu.c b/arch/riscv/kernel/cpu.c
> > index 1b9a5a66e55a..a215ec929160 100644
> > --- a/arch/riscv/kernel/cpu.c
> > +++ b/arch/riscv/kernel/cpu.c
> > @@ -162,6 +162,8 @@ arch_initcall(riscv_cpuinfo_init);
> > * extensions by an underscore.
> > */
> > static struct riscv_isa_ext_data isa_ext_arr[] = {
> > + __RISCV_ISA_EXT_DATA(smaia, RISCV_ISA_EXT_SMAIA),
> > + __RISCV_ISA_EXT_DATA(ssaia, RISCV_ISA_EXT_SSAIA),
>
> This will conflict with that ISA string refactoring I just merged. It
> should be a pretty mechanical merge conflict, but if you want we can do
> a shared tag with the first few patches and I can handle the merge
> conflict locally.
>
> > __RISCV_ISA_EXT_DATA(sscofpmf, RISCV_ISA_EXT_SSCOFPMF),
> > __RISCV_ISA_EXT_DATA(sstc, RISCV_ISA_EXT_SSTC),
> > __RISCV_ISA_EXT_DATA(svinval, RISCV_ISA_EXT_SVINVAL),
> > diff --git a/arch/riscv/kernel/cpufeature.c b/arch/riscv/kernel/cpufeature.c
> > index 93e45560af30..3c5b51f519d5 100644
> > --- a/arch/riscv/kernel/cpufeature.c
> > +++ b/arch/riscv/kernel/cpufeature.c
> > @@ -228,6 +228,8 @@ void __init riscv_fill_hwcap(void)
> > SET_ISA_EXT_MAP("zihintpause", RISCV_ISA_EXT_ZIHINTPAUSE);
> > SET_ISA_EXT_MAP("sstc", RISCV_ISA_EXT_SSTC);
> > SET_ISA_EXT_MAP("svinval", RISCV_ISA_EXT_SVINVAL);
> > + SET_ISA_EXT_MAP("smaia", RISCV_ISA_EXT_SMAIA);
> > + SET_ISA_EXT_MAP("ssaia", RISCV_ISA_EXT_SSAIA);
> > }
> > #undef SET_ISA_EXT_MAP
> > }
>
> _______________________________________________
> linux-riscv mailing list
> [email protected]
> http://lists.infradead.org/mailman/listinfo/linux-riscv

2023-02-21 07:13:32

by Christoph Müllner

[permalink] [raw]
Subject: Re: [PATCH v2 2/7] RISC-V: Detect AIA CSRs from ISA string

Hi all,

The RISC-V Architectural Review Committee has discussed the concerns
regarding the non-ratified chapters in the AIA specification.

Here is the relevant quote from the meeting minutes:
"""
Although the Advanced Interrupt Architecture (AIA) has already passed
Architecture Review (with a minor edit still pending), the committee
has some suggestions about its final steps to ratification, to avoid
the AIA document having a mixture of ratified and non-ratified content:

- The AIA document's remaining draft chapter on the Duo-PLIC, which is
not currently on a path to ratification, can be removed to a separate
document.

- Ratification of the full AIA (without Duo-PLIC) can be postponed to
coincide with ratification of the IOMMU specification, given that
the latter is now expected in a reasonable time, and the AIA's last
chapter concerning IOMMUs is already scheduled to go through public
review and be ratified only together with the IOMMU specification.
"""

The full meeting minutes can be found here:
https://lists.riscv.org/g/tech-chairs/message/1381

BR
Christoph

On Wed, Feb 15, 2023 at 4:41 PM Christoph Müllner
<[email protected]> wrote:
>
> On Fri, Feb 3, 2023 at 1:25 AM Palmer Dabbelt <[email protected]> wrote:
> >
> > On Fri, 27 Jan 2023 23:27:32 PST (-0800), [email protected] wrote:
> > > We have two extension names for AIA ISA support: Smaia (M-mode AIA CSRs)
> > > and Ssaia (S-mode AIA CSRs).
> >
> > This has pretty much the same problem that we had with the other
> > AIA-related ISA string patches, where there's that ambiguity with the
> > non-ratified chapters. IIRC when this came up in GCC the rough idea was
> > to try and document that we're going to interpret the standard ISA
> > strings that way, but now that we're doing custom ISA extensions it
> > seems saner to just define on here that removes the ambiguity.
>
>
> To avoid the impression that I did not work on that, here is the v2
> from November,
> that attempts to document this:
> https://gcc.gnu.org/pipermail/gcc-patches/2022-November/607326.html
>
> My proposed text was:
> +Note, that AIA support (@samp{Smaia} and @samp{Ssaia}) is based on an AIA
> +specification, which is frozen but contains draft chapters ("Duo-PLIC" and
> +"IOMMU Support").
>
> Btw, I did not get any feedback on that patch.
>
> I also tried to address this on spec level (the PR has been linked) and raised
> that to tech-aia (the conversation has been linked).
>
> Another thing that I want to highlight, since it was discussed a lot recently
> (e.g. just a few minutes ago in tech-chairs).
> There is a chance of a last-minute spec change of AIA:
> https://github.com/riscv/riscv-aia/pull/37
>
> BR
> Christoph
>
>
>
> >
> >
> > I just sent
> > <https://lore.kernel.org/r/[email protected]/>
> > which documents that.
> >
> > > We extend the ISA string parsing to detect Smaia and Ssaia extensions.
> > >
> > > Signed-off-by: Anup Patel <[email protected]>
> > > Reviewed-by: Andrew Jones <[email protected]>
> > > ---
> > > arch/riscv/include/asm/hwcap.h | 2 ++
> > > arch/riscv/kernel/cpu.c | 2 ++
> > > arch/riscv/kernel/cpufeature.c | 2 ++
> > > 3 files changed, 6 insertions(+)
> > >
> > > diff --git a/arch/riscv/include/asm/hwcap.h b/arch/riscv/include/asm/hwcap.h
> > > index 86328e3acb02..341ef30a3718 100644
> > > --- a/arch/riscv/include/asm/hwcap.h
> > > +++ b/arch/riscv/include/asm/hwcap.h
> > > @@ -59,6 +59,8 @@ enum riscv_isa_ext_id {
> > > RISCV_ISA_EXT_ZIHINTPAUSE,
> > > RISCV_ISA_EXT_SSTC,
> > > RISCV_ISA_EXT_SVINVAL,
> > > + RISCV_ISA_EXT_SMAIA,
> > > + RISCV_ISA_EXT_SSAIA,
> > > RISCV_ISA_EXT_ID_MAX
> > > };
> > > static_assert(RISCV_ISA_EXT_ID_MAX <= RISCV_ISA_EXT_MAX);
> > > diff --git a/arch/riscv/kernel/cpu.c b/arch/riscv/kernel/cpu.c
> > > index 1b9a5a66e55a..a215ec929160 100644
> > > --- a/arch/riscv/kernel/cpu.c
> > > +++ b/arch/riscv/kernel/cpu.c
> > > @@ -162,6 +162,8 @@ arch_initcall(riscv_cpuinfo_init);
> > > * extensions by an underscore.
> > > */
> > > static struct riscv_isa_ext_data isa_ext_arr[] = {
> > > + __RISCV_ISA_EXT_DATA(smaia, RISCV_ISA_EXT_SMAIA),
> > > + __RISCV_ISA_EXT_DATA(ssaia, RISCV_ISA_EXT_SSAIA),
> >
> > This will conflict with that ISA string refactoring I just merged. It
> > should be a pretty mechanical merge conflict, but if you want we can do
> > a shared tag with the first few patches and I can handle the merge
> > conflict locally.
> >
> > > __RISCV_ISA_EXT_DATA(sscofpmf, RISCV_ISA_EXT_SSCOFPMF),
> > > __RISCV_ISA_EXT_DATA(sstc, RISCV_ISA_EXT_SSTC),
> > > __RISCV_ISA_EXT_DATA(svinval, RISCV_ISA_EXT_SVINVAL),
> > > diff --git a/arch/riscv/kernel/cpufeature.c b/arch/riscv/kernel/cpufeature.c
> > > index 93e45560af30..3c5b51f519d5 100644
> > > --- a/arch/riscv/kernel/cpufeature.c
> > > +++ b/arch/riscv/kernel/cpufeature.c
> > > @@ -228,6 +228,8 @@ void __init riscv_fill_hwcap(void)
> > > SET_ISA_EXT_MAP("zihintpause", RISCV_ISA_EXT_ZIHINTPAUSE);
> > > SET_ISA_EXT_MAP("sstc", RISCV_ISA_EXT_SSTC);
> > > SET_ISA_EXT_MAP("svinval", RISCV_ISA_EXT_SVINVAL);
> > > + SET_ISA_EXT_MAP("smaia", RISCV_ISA_EXT_SMAIA);
> > > + SET_ISA_EXT_MAP("ssaia", RISCV_ISA_EXT_SSAIA);
> > > }
> > > #undef SET_ISA_EXT_MAP
> > > }
> >
> > _______________________________________________
> > linux-riscv mailing list
> > [email protected]
> > http://lists.infradead.org/mailman/listinfo/linux-riscv

2023-02-21 10:41:18

by Conor Dooley

[permalink] [raw]
Subject: Re: [PATCH v2 2/7] RISC-V: Detect AIA CSRs from ISA string

Hey Christoph,

On Tue, Feb 21, 2023 at 08:12:58AM +0100, Christoph M?llner wrote:
> Hi all,
>
> The RISC-V Architectural Review Committee has discussed the concerns
> regarding the non-ratified chapters in the AIA specification.

Thanks for the update!

> Here is the relevant quote from the meeting minutes:
> """
> Although the Advanced Interrupt Architecture (AIA) has already passed
> Architecture Review (with a minor edit still pending), the committee
> has some suggestions about its final steps to ratification, to avoid
> the AIA document having a mixture of ratified and non-ratified content:

> - The AIA document's remaining draft chapter on the Duo-PLIC, which is
> not currently on a path to ratification, can be removed to a separate
> document.

That sounds promising...

> - Ratification of the full AIA (without Duo-PLIC) can be postponed to
> coincide with ratification of the IOMMU specification, given that
> the latter is now expected in a reasonable time, and the AIA's last
> chapter concerning IOMMUs is already scheduled to go through public
> review and be ratified only together with the IOMMU specification.
> """

...and so does this. AIA stuff's acceptability only depending on the
IOMMU spec's freeze (and thus Chapter 9's) seems like a vast improvement
on the status quo to me!

> The full meeting minutes can be found here:
> https://lists.riscv.org/g/tech-chairs/message/1381

This link is non functional unfortunately :/

Cheers,
Conor.


Attachments:
(No filename) (1.46 kB)
signature.asc (228.00 B)
Download all attachments

2023-02-21 10:51:56

by Jessica Clarke

[permalink] [raw]
Subject: Re: [PATCH v2 2/7] RISC-V: Detect AIA CSRs from ISA string

On 21 Feb 2023, at 10:40, Conor Dooley <[email protected]> wrote:
>
> Hey Christoph,
>
> On Tue, Feb 21, 2023 at 08:12:58AM +0100, Christoph Müllner wrote:
>> Hi all,
>>
>> The RISC-V Architectural Review Committee has discussed the concerns
>> regarding the non-ratified chapters in the AIA specification.
>
> Thanks for the update!
>
>> Here is the relevant quote from the meeting minutes:
>> """
>> Although the Advanced Interrupt Architecture (AIA) has already passed
>> Architecture Review (with a minor edit still pending), the committee
>> has some suggestions about its final steps to ratification, to avoid
>> the AIA document having a mixture of ratified and non-ratified content:
>
>> - The AIA document's remaining draft chapter on the Duo-PLIC, which is
>> not currently on a path to ratification, can be removed to a separate
>> document.
>
> That sounds promising...
>
>> - Ratification of the full AIA (without Duo-PLIC) can be postponed to
>> coincide with ratification of the IOMMU specification, given that
>> the latter is now expected in a reasonable time, and the AIA's last
>> chapter concerning IOMMUs is already scheduled to go through public
>> review and be ratified only together with the IOMMU specification.
>> """
>
> ...and so does this. AIA stuff's acceptability only depending on the
> IOMMU spec's freeze (and thus Chapter 9's) seems like a vast improvement
> on the status quo to me!
>
>> The full meeting minutes can be found here:
>> https://lists.riscv.org/g/tech-chairs/message/1381
>
> This link is non functional unfortunately :/

tech-chairs is private, for (co-)chairs only... not sure why it went
there rather than tech-privileged.

Jess

> Cheers,
> Conor.
>
> _______________________________________________
> linux-riscv mailing list
> [email protected]
> http://lists.infradead.org/mailman/listinfo/linux-riscv


2023-02-21 11:00:09

by Conor Dooley

[permalink] [raw]
Subject: Re: [PATCH v2 2/7] RISC-V: Detect AIA CSRs from ISA string

On Tue, Feb 21, 2023 at 10:51:13AM +0000, Jessica Clarke wrote:
> On 21 Feb 2023, at 10:40, Conor Dooley <[email protected]> wrote:
> > On Tue, Feb 21, 2023 at 08:12:58AM +0100, Christoph M?llner wrote:
> >> The full meeting minutes can be found here:
> >> https://lists.riscv.org/g/tech-chairs/message/1381
> >
> > This link is non functional unfortunately :/
>
> tech-chairs is private, for (co-)chairs only... not sure why it went
> there rather than tech-privileged.

Yah, that's what I was getting at.. This is a conversation on a public
ML, so it'd be annoying enough for some readers if it was gated around
RVI membership, but gating on membership of the inner circle makes it
kinda useless!



Attachments:
(No filename) (713.00 B)
signature.asc (228.00 B)
Download all attachments

2023-02-21 11:04:03

by Christoph Müllner

[permalink] [raw]
Subject: Re: [PATCH v2 2/7] RISC-V: Detect AIA CSRs from ISA string

On Tue, Feb 21, 2023 at 12:00 PM Conor Dooley
<[email protected]> wrote:
>
> On Tue, Feb 21, 2023 at 10:51:13AM +0000, Jessica Clarke wrote:
> > On 21 Feb 2023, at 10:40, Conor Dooley <[email protected]> wrote:
> > > On Tue, Feb 21, 2023 at 08:12:58AM +0100, Christoph Müllner wrote:
> > >> The full meeting minutes can be found here:
> > >> https://lists.riscv.org/g/tech-chairs/message/1381
> > >
> > > This link is non functional unfortunately :/
> >
> > tech-chairs is private, for (co-)chairs only... not sure why it went
> > there rather than tech-privileged.
>
> Yah, that's what I was getting at.. This is a conversation on a public
> ML, so it'd be annoying enough for some readers if it was gated around
> RVI membership, but gating on membership of the inner circle makes it
> kinda useless!

The mail was forwarded there as well (and to tech-unprivileged):
https://lists.riscv.org/g/tech-privileged/message/1294
https://lists.riscv.org/g/tech-unprivileged/message/430

BR
Christoph

2023-02-21 11:22:49

by Conor Dooley

[permalink] [raw]
Subject: Re: [PATCH v2 2/7] RISC-V: Detect AIA CSRs from ISA string

On Tue, Feb 21, 2023 at 12:03:45PM +0100, Christoph M?llner wrote:
> On Tue, Feb 21, 2023 at 12:00 PM Conor Dooley
> <[email protected]> wrote:
> >
> > On Tue, Feb 21, 2023 at 10:51:13AM +0000, Jessica Clarke wrote:
> > > On 21 Feb 2023, at 10:40, Conor Dooley <[email protected]> wrote:
> > > > On Tue, Feb 21, 2023 at 08:12:58AM +0100, Christoph M?llner wrote:
> > > >> The full meeting minutes can be found here:
> > > >> https://lists.riscv.org/g/tech-chairs/message/1381
> > > >
> > > > This link is non functional unfortunately :/
> > >
> > > tech-chairs is private, for (co-)chairs only... not sure why it went
> > > there rather than tech-privileged.
> >
> > Yah, that's what I was getting at.. This is a conversation on a public
> > ML, so it'd be annoying enough for some readers if it was gated around
> > RVI membership, but gating on membership of the inner circle makes it
> > kinda useless!
>
> The mail was forwarded there as well (and to tech-unprivileged):
> https://lists.riscv.org/g/tech-privileged/message/1294
> https://lists.riscv.org/g/tech-unprivileged/message/430

Great, those I can actually read. Thanks for sharing with the unwashed!

Conor.


Attachments:
(No filename) (1.17 kB)
signature.asc (228.00 B)
Download all attachments

2023-03-31 12:58:59

by Anup Patel

[permalink] [raw]
Subject: Re: [PATCH v2 2/7] RISC-V: Detect AIA CSRs from ISA string

Hi Palmer

On Fri, Feb 3, 2023 at 5:54 AM Palmer Dabbelt <[email protected]> wrote:
>
> On Fri, 27 Jan 2023 23:27:32 PST (-0800), [email protected] wrote:
> > We have two extension names for AIA ISA support: Smaia (M-mode AIA CSRs)
> > and Ssaia (S-mode AIA CSRs).
>
> This has pretty much the same problem that we had with the other
> AIA-related ISA string patches, where there's that ambiguity with the
> non-ratified chapters. IIRC when this came up in GCC the rough idea was
> to try and document that we're going to interpret the standard ISA
> strings that way, but now that we're doing custom ISA extensions it
> seems saner to just define on here that removes the ambiguity.
>
> I just sent
> <https://lore.kernel.org/r/[email protected]/>
> which documents that.

The IOMMU and AIA chapter8 are frozen and in public review.
Refer, https://lists.riscv.org/g/tech-aia/message/346

This means the entire AIA specification is now frozen (i.e. no
chapters in draft state).

I will rebase this series and send-out v3. It would be great if you
can ACK the PATCH2 of this series.

Thanks,
Anup

>
> > We extend the ISA string parsing to detect Smaia and Ssaia extensions.
> >
> > Signed-off-by: Anup Patel <[email protected]>
> > Reviewed-by: Andrew Jones <[email protected]>
> > ---
> > arch/riscv/include/asm/hwcap.h | 2 ++
> > arch/riscv/kernel/cpu.c | 2 ++
> > arch/riscv/kernel/cpufeature.c | 2 ++
> > 3 files changed, 6 insertions(+)
> >
> > diff --git a/arch/riscv/include/asm/hwcap.h b/arch/riscv/include/asm/hwcap.h
> > index 86328e3acb02..341ef30a3718 100644
> > --- a/arch/riscv/include/asm/hwcap.h
> > +++ b/arch/riscv/include/asm/hwcap.h
> > @@ -59,6 +59,8 @@ enum riscv_isa_ext_id {
> > RISCV_ISA_EXT_ZIHINTPAUSE,
> > RISCV_ISA_EXT_SSTC,
> > RISCV_ISA_EXT_SVINVAL,
> > + RISCV_ISA_EXT_SMAIA,
> > + RISCV_ISA_EXT_SSAIA,
> > RISCV_ISA_EXT_ID_MAX
> > };
> > static_assert(RISCV_ISA_EXT_ID_MAX <= RISCV_ISA_EXT_MAX);
> > diff --git a/arch/riscv/kernel/cpu.c b/arch/riscv/kernel/cpu.c
> > index 1b9a5a66e55a..a215ec929160 100644
> > --- a/arch/riscv/kernel/cpu.c
> > +++ b/arch/riscv/kernel/cpu.c
> > @@ -162,6 +162,8 @@ arch_initcall(riscv_cpuinfo_init);
> > * extensions by an underscore.
> > */
> > static struct riscv_isa_ext_data isa_ext_arr[] = {
> > + __RISCV_ISA_EXT_DATA(smaia, RISCV_ISA_EXT_SMAIA),
> > + __RISCV_ISA_EXT_DATA(ssaia, RISCV_ISA_EXT_SSAIA),
>
> This will conflict with that ISA string refactoring I just merged. It
> should be a pretty mechanical merge conflict, but if you want we can do
> a shared tag with the first few patches and I can handle the merge
> conflict locally.
>
> > __RISCV_ISA_EXT_DATA(sscofpmf, RISCV_ISA_EXT_SSCOFPMF),
> > __RISCV_ISA_EXT_DATA(sstc, RISCV_ISA_EXT_SSTC),
> > __RISCV_ISA_EXT_DATA(svinval, RISCV_ISA_EXT_SVINVAL),
> > diff --git a/arch/riscv/kernel/cpufeature.c b/arch/riscv/kernel/cpufeature.c
> > index 93e45560af30..3c5b51f519d5 100644
> > --- a/arch/riscv/kernel/cpufeature.c
> > +++ b/arch/riscv/kernel/cpufeature.c
> > @@ -228,6 +228,8 @@ void __init riscv_fill_hwcap(void)
> > SET_ISA_EXT_MAP("zihintpause", RISCV_ISA_EXT_ZIHINTPAUSE);
> > SET_ISA_EXT_MAP("sstc", RISCV_ISA_EXT_SSTC);
> > SET_ISA_EXT_MAP("svinval", RISCV_ISA_EXT_SVINVAL);
> > + SET_ISA_EXT_MAP("smaia", RISCV_ISA_EXT_SMAIA);
> > + SET_ISA_EXT_MAP("ssaia", RISCV_ISA_EXT_SSAIA);
> > }
> > #undef SET_ISA_EXT_MAP
> > }

2023-03-31 17:31:17

by Anup Patel

[permalink] [raw]
Subject: Re: [PATCH v2 7/7] RISC-V: KVM: Implement guest external interrupt line management

On Wed, Feb 8, 2023 at 5:45 AM Atish Patra <[email protected]> wrote:
>
> On Fri, Jan 27, 2023 at 11:28 PM Anup Patel <[email protected]> wrote:
> >
> > The RISC-V host will have one guest external interrupt line for each
> > VS-level IMSICs associated with a HART. The guest external interrupt
> > lines are per-HART resources and hypervisor can use HGEIE, HGEIP, and
> > HIE CSRs to manage these guest external interrupt lines.
> >
> > Signed-off-by: Anup Patel <[email protected]>
> > ---
> > arch/riscv/include/asm/kvm_aia.h | 10 ++
> > arch/riscv/kvm/aia.c | 241 +++++++++++++++++++++++++++++++
> > arch/riscv/kvm/main.c | 3 +-
> > arch/riscv/kvm/vcpu.c | 2 +
> > 4 files changed, 255 insertions(+), 1 deletion(-)
> >
> > diff --git a/arch/riscv/include/asm/kvm_aia.h b/arch/riscv/include/asm/kvm_aia.h
> > index 1de0717112e5..0938e0cadf80 100644
> > --- a/arch/riscv/include/asm/kvm_aia.h
> > +++ b/arch/riscv/include/asm/kvm_aia.h
> > @@ -44,10 +44,15 @@ struct kvm_vcpu_aia {
> >
> > #define irqchip_in_kernel(k) ((k)->arch.aia.in_kernel)
> >
> > +extern unsigned int kvm_riscv_aia_nr_hgei;
> > DECLARE_STATIC_KEY_FALSE(kvm_riscv_aia_available);
> > #define kvm_riscv_aia_available() \
> > static_branch_unlikely(&kvm_riscv_aia_available)
> >
> > +static inline void kvm_riscv_vcpu_aia_imsic_release(struct kvm_vcpu *vcpu)
> > +{
> > +}
> > +
> > #define KVM_RISCV_AIA_IMSIC_TOPEI (ISELECT_MASK + 1)
> > static inline int kvm_riscv_vcpu_aia_imsic_rmw(struct kvm_vcpu *vcpu,
> > unsigned long isel,
> > @@ -119,6 +124,11 @@ static inline void kvm_riscv_aia_destroy_vm(struct kvm *kvm)
> > {
> > }
> >
> > +int kvm_riscv_aia_alloc_hgei(int cpu, struct kvm_vcpu *owner,
> > + void __iomem **hgei_va, phys_addr_t *hgei_pa);
> > +void kvm_riscv_aia_free_hgei(int cpu, int hgei);
> > +void kvm_riscv_aia_wakeon_hgei(struct kvm_vcpu *owner, bool enable);
> > +
> > void kvm_riscv_aia_enable(void);
> > void kvm_riscv_aia_disable(void);
> > int kvm_riscv_aia_init(void);
> > diff --git a/arch/riscv/kvm/aia.c b/arch/riscv/kvm/aia.c
> > index 3d68189e0bb1..f375069fdaf4 100644
> > --- a/arch/riscv/kvm/aia.c
> > +++ b/arch/riscv/kvm/aia.c
> > @@ -7,11 +7,46 @@
> > * Anup Patel <[email protected]>
> > */
> >
> > +#include <linux/bitops.h>
> > +#include <linux/irq.h>
> > +#include <linux/irqdomain.h>
> > #include <linux/kvm_host.h>
> > +#include <linux/percpu.h>
> > +#include <linux/spinlock.h>
> > #include <asm/hwcap.h>
> >
> > +struct aia_hgei_control {
> > + raw_spinlock_t lock;
> > + unsigned long free_bitmap;
> > + struct kvm_vcpu *owners[BITS_PER_LONG];
> > +};
> > +static DEFINE_PER_CPU(struct aia_hgei_control, aia_hgei);
> > +static int hgei_parent_irq;
> > +
> > +unsigned int kvm_riscv_aia_nr_hgei;
> > DEFINE_STATIC_KEY_FALSE(kvm_riscv_aia_available);
> >
> > +static int aia_find_hgei(struct kvm_vcpu *owner)
> > +{
> > + int i, hgei;
> > + unsigned long flags;
> > + struct aia_hgei_control *hgctrl = this_cpu_ptr(&aia_hgei);
> > +
> > + raw_spin_lock_irqsave(&hgctrl->lock, flags);
> > +
> > + hgei = -1;
> > + for (i = 1; i <= kvm_riscv_aia_nr_hgei; i++) {
> > + if (hgctrl->owners[i] == owner) {
> > + hgei = i;
> > + break;
> > + }
> > + }
> > +
> > + raw_spin_unlock_irqrestore(&hgctrl->lock, flags);
> > +
> > + return hgei;
> > +}
> > +
> > static void aia_set_hvictl(bool ext_irq_pending)
> > {
> > unsigned long hvictl;
> > @@ -55,6 +90,7 @@ void kvm_riscv_vcpu_aia_sync_interrupts(struct kvm_vcpu *vcpu)
> >
> > bool kvm_riscv_vcpu_aia_has_interrupts(struct kvm_vcpu *vcpu, u64 mask)
> > {
> > + int hgei;
> > unsigned long seip;
> >
> > if (!kvm_riscv_aia_available())
> > @@ -72,6 +108,10 @@ bool kvm_riscv_vcpu_aia_has_interrupts(struct kvm_vcpu *vcpu, u64 mask)
> > if (!kvm_riscv_aia_initialized(vcpu->kvm) || !seip)
> > return false;
> >
> > + hgei = aia_find_hgei(vcpu);
> > + if (hgei > 0)
> > + return (csr_read(CSR_HGEIP) & BIT(hgei)) ? true : false;
> > +
> > return false;
> > }
> >
> > @@ -343,6 +383,144 @@ int kvm_riscv_vcpu_aia_rmw_ireg(struct kvm_vcpu *vcpu, unsigned int csr_num,
> > return KVM_INSN_EXIT_TO_USER_SPACE;
> > }
> >
> > +int kvm_riscv_aia_alloc_hgei(int cpu, struct kvm_vcpu *owner,
> > + void __iomem **hgei_va, phys_addr_t *hgei_pa)
> > +{
> > + int ret = -ENOENT;
> > + unsigned long flags;
> > + struct aia_hgei_control *hgctrl = per_cpu_ptr(&aia_hgei, cpu);
> > +
> > + if (!kvm_riscv_aia_available())
> > + return -ENODEV;
> > + if (!hgctrl)
> > + return -ENODEV;
> > +
> > + raw_spin_lock_irqsave(&hgctrl->lock, flags);
> > +
> > + if (hgctrl->free_bitmap) {
> > + ret = __ffs(hgctrl->free_bitmap);
> > + hgctrl->free_bitmap &= ~BIT(ret);
> > + hgctrl->owners[ret] = owner;
> > + }
> > +
> > + raw_spin_unlock_irqrestore(&hgctrl->lock, flags);
> > +
> > + /* TODO: To be updated later by AIA in-kernel irqchip support */
> > + if (hgei_va)
> > + *hgei_va = NULL;
> > + if (hgei_pa)
> > + *hgei_pa = 0;
> > +
> > + return ret;
> > +}
> > +
> > +void kvm_riscv_aia_free_hgei(int cpu, int hgei)
> > +{
> > + unsigned long flags;
> > + struct aia_hgei_control *hgctrl = per_cpu_ptr(&aia_hgei, cpu);
> > +
> > + if (!kvm_riscv_aia_available() || !hgctrl)
> > + return;
> > +
> > + raw_spin_lock_irqsave(&hgctrl->lock, flags);
> > +
> > + if (hgei > 0 && hgei <= kvm_riscv_aia_nr_hgei) {
> > + if (!(hgctrl->free_bitmap & BIT(hgei))) {
> > + hgctrl->free_bitmap |= BIT(hgei);
> > + hgctrl->owners[hgei] = NULL;
> > + }
> > + }
> > +
> > + raw_spin_unlock_irqrestore(&hgctrl->lock, flags);
> > +}
> > +
> > +void kvm_riscv_aia_wakeon_hgei(struct kvm_vcpu *owner, bool enable)
> > +{
> > + int hgei;
> > +
> > + if (!kvm_riscv_aia_available())
> > + return;
> > +
> > + hgei = aia_find_hgei(owner);
> > + if (hgei > 0) {
> > + if (enable)
> > + csr_set(CSR_HGEIE, BIT(hgei));
> > + else
> > + csr_clear(CSR_HGEIE, BIT(hgei));
> > + }
> > +}
> > +
> > +static irqreturn_t hgei_interrupt(int irq, void *dev_id)
> > +{
> > + int i;
> > + unsigned long hgei_mask, flags;
> > + struct aia_hgei_control *hgctrl = this_cpu_ptr(&aia_hgei);
> > +
> > + hgei_mask = csr_read(CSR_HGEIP) & csr_read(CSR_HGEIE);
> > + csr_clear(CSR_HGEIE, hgei_mask);
> > +
> > + raw_spin_lock_irqsave(&hgctrl->lock, flags);
> > +
> > + for_each_set_bit(i, &hgei_mask, BITS_PER_LONG) {
> > + if (hgctrl->owners[i])
> > + kvm_vcpu_kick(hgctrl->owners[i]);
> > + }
> > +
> > + raw_spin_unlock_irqrestore(&hgctrl->lock, flags);
> > +
> > + return IRQ_HANDLED;
> > +}
> > +
> > +static int aia_hgei_init(void)
> > +{
> > + int cpu, rc;
> > + struct irq_domain *domain;
> > + struct aia_hgei_control *hgctrl;
> > +
> > + /* Initialize per-CPU guest external interrupt line management */
> > + for_each_possible_cpu(cpu) {
> > + hgctrl = per_cpu_ptr(&aia_hgei, cpu);
> > + raw_spin_lock_init(&hgctrl->lock);
> > + if (kvm_riscv_aia_nr_hgei) {
> > + hgctrl->free_bitmap =
> > + BIT(kvm_riscv_aia_nr_hgei + 1) - 1;
> > + hgctrl->free_bitmap &= ~BIT(0);
> > + } else
> > + hgctrl->free_bitmap = 0;
> > + }
> > +
> > + /* Find INTC irq domain */
> > + domain = irq_find_matching_fwnode(riscv_get_intc_hwnode(),
> > + DOMAIN_BUS_ANY);
> > + if (!domain) {
> > + kvm_err("unable to find INTC domain\n");
> > + return -ENOENT;
> > + }
> > +
> > + /* Map per-CPU SGEI interrupt from INTC domain */
> > + hgei_parent_irq = irq_create_mapping(domain, IRQ_S_GEXT);
> > + if (!hgei_parent_irq) {
> > + kvm_err("unable to map SGEI IRQ\n");
> > + return -ENOMEM;
> > + }
> > +
> > + /* Request per-CPU SGEI interrupt */
> > + rc = request_percpu_irq(hgei_parent_irq, hgei_interrupt,
> > + "riscv-kvm", &aia_hgei);
> > + if (rc) {
> > + kvm_err("failed to request SGEI IRQ\n");
> > + return rc;
> > + }
> > +
> > + return 0;
> > +}
> > +
> > +static void aia_hgei_exit(void)
> > +{
> > + /* Free per-CPU SGEI interrupt */
> > + free_percpu_irq(hgei_parent_irq, &aia_hgei);
> > +}
> > +
> > void kvm_riscv_aia_enable(void)
> > {
> > if (!kvm_riscv_aia_available())
> > @@ -357,21 +535,79 @@ void kvm_riscv_aia_enable(void)
> > csr_write(CSR_HVIPRIO1H, 0x0);
> > csr_write(CSR_HVIPRIO2H, 0x0);
> > #endif
> > +
> > + /* Enable per-CPU SGEI interrupt */
> > + enable_percpu_irq(hgei_parent_irq,
> > + irq_get_trigger_type(hgei_parent_irq));
> > + csr_set(CSR_HIE, BIT(IRQ_S_GEXT));
> > }
> >
> > void kvm_riscv_aia_disable(void)
> > {
> > + int i;
> > + unsigned long flags;
> > + struct kvm_vcpu *vcpu;
> > + struct aia_hgei_control *hgctrl = this_cpu_ptr(&aia_hgei);
> > +
> > if (!kvm_riscv_aia_available())
> > return;
> >
> > + /* Disable per-CPU SGEI interrupt */
> > + csr_clear(CSR_HIE, BIT(IRQ_S_GEXT));
> > + disable_percpu_irq(hgei_parent_irq);
> > +
> > aia_set_hvictl(false);
> > +
> > + raw_spin_lock_irqsave(&hgctrl->lock, flags);
> > +
> > + for (i = 0; i <= kvm_riscv_aia_nr_hgei; i++) {
> > + vcpu = hgctrl->owners[i];
> > + if (!vcpu)
> > + continue;
> > +
> > + /*
> > + * We release hgctrl->lock before notifying IMSIC
> > + * so that we don't have lock ordering issues.
> > + */
> > + raw_spin_unlock_irqrestore(&hgctrl->lock, flags);
> > +
> > + /* Notify IMSIC */
> > + kvm_riscv_vcpu_aia_imsic_release(vcpu);
> > +
> > + /*
> > + * Wakeup VCPU if it was blocked so that it can
> > + * run on other HARTs
> > + */
> > + if (csr_read(CSR_HGEIE) & BIT(i)) {
> > + csr_clear(CSR_HGEIE, BIT(i));
> > + kvm_vcpu_kick(vcpu);
> > + }
> > +
> > + raw_spin_lock_irqsave(&hgctrl->lock, flags);
> > + }
> > +
>
> We shouldn't access vcpu related data here as
> kvm_arch_destroy_vm/kvm_destroy_vcpus
> is called before hardware_disable_all is invoked.

This is for CPU hotplug where the VM/VCPU are not
destroyed but the host CPU is going down so we need
to cleanup sleeping VCPUs.

Regards,
Anup

>
> > + raw_spin_unlock_irqrestore(&hgctrl->lock, flags);
> > }
> >
> > int kvm_riscv_aia_init(void)
> > {
> > + int rc;
> > +
> > if (!riscv_isa_extension_available(NULL, SxAIA))
> > return -ENODEV;
> >
> > + /* Figure-out number of bits in HGEIE */
> > + csr_write(CSR_HGEIE, -1UL);
> > + kvm_riscv_aia_nr_hgei = fls_long(csr_read(CSR_HGEIE));
> > + csr_write(CSR_HGEIE, 0);
> > + if (kvm_riscv_aia_nr_hgei)
> > + kvm_riscv_aia_nr_hgei--;
> > +
> > + /* Initialize guest external interrupt line management */
> > + rc = aia_hgei_init();
> > + if (rc)
> > + return rc;
> > +
> > /* Enable KVM AIA support */
> > static_branch_enable(&kvm_riscv_aia_available);
> >
> > @@ -380,4 +616,9 @@ int kvm_riscv_aia_init(void)
> >
> > void kvm_riscv_aia_exit(void)
> > {
> > + if (!kvm_riscv_aia_available())
> > + return;
> > +
> > + /* Cleanup the HGEI state */
> > + aia_hgei_exit();
> > }
> > diff --git a/arch/riscv/kvm/main.c b/arch/riscv/kvm/main.c
> > index d8ff44eb04ca..5cf37dbe8a38 100644
> > --- a/arch/riscv/kvm/main.c
> > +++ b/arch/riscv/kvm/main.c
> > @@ -125,7 +125,8 @@ int kvm_arch_init(void *opaque)
> > kvm_info("VMID %ld bits available\n", kvm_riscv_gstage_vmid_bits());
> >
> > if (kvm_riscv_aia_available())
> > - kvm_info("AIA available\n");
> > + kvm_info("AIA available with %d guest external interrupts\n",
> > + kvm_riscv_aia_nr_hgei);
> >
> > return 0;
> > }
> > diff --git a/arch/riscv/kvm/vcpu.c b/arch/riscv/kvm/vcpu.c
> > index 151b35b3b05f..1daa1936b642 100644
> > --- a/arch/riscv/kvm/vcpu.c
> > +++ b/arch/riscv/kvm/vcpu.c
> > @@ -240,10 +240,12 @@ int kvm_cpu_has_pending_timer(struct kvm_vcpu *vcpu)
> >
> > void kvm_arch_vcpu_blocking(struct kvm_vcpu *vcpu)
> > {
> > + kvm_riscv_aia_wakeon_hgei(vcpu, true);
> > }
> >
> > void kvm_arch_vcpu_unblocking(struct kvm_vcpu *vcpu)
> > {
> > + kvm_riscv_aia_wakeon_hgei(vcpu, false);
> > }
> >
> > int kvm_arch_vcpu_runnable(struct kvm_vcpu *vcpu)
> > --
> > 2.34.1
> >
>
>
> --
> Regards,
> Atish

2023-03-31 17:33:48

by Anup Patel

[permalink] [raw]
Subject: Re: [PATCH v2 5/7] RISC-V: KVM: Add ONE_REG interface for AIA CSRs

On Wed, Feb 8, 2023 at 5:34 AM Atish Patra <[email protected]> wrote:
>
> On Fri, Jan 27, 2023 at 11:28 PM Anup Patel <[email protected]> wrote:
> >
> > We extend the CSR ONE_REG interface to access both general CSRs and
> > AIA CSRs. To achieve this, we introduce "subtype" field in the ONE_REG
> > id which can be used for grouping registers within a particular "type"
> > of ONE_REG registers.
> >
> > Signed-off-by: Anup Patel <[email protected]>
> > ---
> > arch/riscv/include/uapi/asm/kvm.h | 15 ++++-
> > arch/riscv/kvm/vcpu.c | 96 ++++++++++++++++++++++++-------
> > 2 files changed, 89 insertions(+), 22 deletions(-)
> >
> > diff --git a/arch/riscv/include/uapi/asm/kvm.h b/arch/riscv/include/uapi/asm/kvm.h
> > index 71992ff1f9dd..d0704eff0121 100644
> > --- a/arch/riscv/include/uapi/asm/kvm.h
> > +++ b/arch/riscv/include/uapi/asm/kvm.h
> > @@ -64,7 +64,7 @@ struct kvm_riscv_core {
> > #define KVM_RISCV_MODE_S 1
> > #define KVM_RISCV_MODE_U 0
> >
> > -/* CSR registers for KVM_GET_ONE_REG and KVM_SET_ONE_REG */
> > +/* General CSR registers for KVM_GET_ONE_REG and KVM_SET_ONE_REG */
> > struct kvm_riscv_csr {
> > unsigned long sstatus;
> > unsigned long sie;
> > @@ -78,6 +78,10 @@ struct kvm_riscv_csr {
> > unsigned long scounteren;
> > };
> >
> > +/* AIA CSR registers for KVM_GET_ONE_REG and KVM_SET_ONE_REG */
> > +struct kvm_riscv_aia_csr {
> > +};
> > +
> > /* TIMER registers for KVM_GET_ONE_REG and KVM_SET_ONE_REG */
> > struct kvm_riscv_timer {
> > __u64 frequency;
> > @@ -105,6 +109,7 @@ enum KVM_RISCV_ISA_EXT_ID {
> > KVM_RISCV_ISA_EXT_SVINVAL,
> > KVM_RISCV_ISA_EXT_ZIHINTPAUSE,
> > KVM_RISCV_ISA_EXT_ZICBOM,
> > + KVM_RISCV_ISA_EXT_SSAIA,
> > KVM_RISCV_ISA_EXT_MAX,
> > };
> >
> > @@ -134,6 +139,8 @@ enum KVM_RISCV_SBI_EXT_ID {
> > /* If you need to interpret the index values, here is the key: */
> > #define KVM_REG_RISCV_TYPE_MASK 0x00000000FF000000
> > #define KVM_REG_RISCV_TYPE_SHIFT 24
> > +#define KVM_REG_RISCV_SUBTYPE_MASK 0x0000000000FF0000
> > +#define KVM_REG_RISCV_SUBTYPE_SHIFT 16
> >
> > /* Config registers are mapped as type 1 */
> > #define KVM_REG_RISCV_CONFIG (0x01 << KVM_REG_RISCV_TYPE_SHIFT)
> > @@ -147,8 +154,12 @@ enum KVM_RISCV_SBI_EXT_ID {
> >
> > /* Control and status registers are mapped as type 3 */
> > #define KVM_REG_RISCV_CSR (0x03 << KVM_REG_RISCV_TYPE_SHIFT)
> > +#define KVM_REG_RISCV_CSR_GENERAL 0x0
> > +#define KVM_REG_RISCV_CSR_AIA 0x1
> > #define KVM_REG_RISCV_CSR_REG(name) \
> > - (offsetof(struct kvm_riscv_csr, name) / sizeof(unsigned long))
> > + (offsetof(struct kvm_riscv_csr, name) / sizeof(unsigned long))
> > +#define KVM_REG_RISCV_CSR_AIA_REG(name) \
> > + (offsetof(struct kvm_riscv_aia_csr, name) / sizeof(unsigned long))
> >
> > /* Timer registers are mapped as type 4 */
> > #define KVM_REG_RISCV_TIMER (0x04 << KVM_REG_RISCV_TYPE_SHIFT)
> > diff --git a/arch/riscv/kvm/vcpu.c b/arch/riscv/kvm/vcpu.c
> > index 3cf50eadc8ce..37933ea20274 100644
> > --- a/arch/riscv/kvm/vcpu.c
> > +++ b/arch/riscv/kvm/vcpu.c
> > @@ -58,6 +58,7 @@ static const unsigned long kvm_isa_ext_arr[] = {
> > [KVM_RISCV_ISA_EXT_I] = RISCV_ISA_EXT_i,
> > [KVM_RISCV_ISA_EXT_M] = RISCV_ISA_EXT_m,
> >
> > + KVM_ISA_EXT_ARR(SSAIA),
> > KVM_ISA_EXT_ARR(SSTC),
> > KVM_ISA_EXT_ARR(SVINVAL),
> > KVM_ISA_EXT_ARR(SVPBMT),
> > @@ -96,6 +97,7 @@ static bool kvm_riscv_vcpu_isa_disable_allowed(unsigned long ext)
> > case KVM_RISCV_ISA_EXT_C:
> > case KVM_RISCV_ISA_EXT_I:
> > case KVM_RISCV_ISA_EXT_M:
> > + case KVM_RISCV_ISA_EXT_SSAIA:
> > case KVM_RISCV_ISA_EXT_SSTC:
> > case KVM_RISCV_ISA_EXT_SVINVAL:
> > case KVM_RISCV_ISA_EXT_ZIHINTPAUSE:
> > @@ -451,30 +453,79 @@ static int kvm_riscv_vcpu_set_reg_core(struct kvm_vcpu *vcpu,
> > return 0;
> > }
> >
> > +static int kvm_riscv_vcpu_general_get_csr(struct kvm_vcpu *vcpu,
> > + unsigned long reg_num,
> > + unsigned long *out_val)
> > +{
> > + struct kvm_vcpu_csr *csr = &vcpu->arch.guest_csr;
> > +
> > + if (reg_num >= sizeof(struct kvm_riscv_csr) / sizeof(unsigned long))
> > + return -EINVAL;
> > +
> > + if (reg_num == KVM_REG_RISCV_CSR_REG(sip)) {
> > + kvm_riscv_vcpu_flush_interrupts(vcpu);
> > + *out_val = (csr->hvip >> VSIP_TO_HVIP_SHIFT) & VSIP_VALID_MASK;
> > + } else
> > + *out_val = ((unsigned long *)csr)[reg_num];
> > +
> > + return 0;
> > +}
> > +
> > static int kvm_riscv_vcpu_get_reg_csr(struct kvm_vcpu *vcpu,
> > const struct kvm_one_reg *reg)
> > {
> > - struct kvm_vcpu_csr *csr = &vcpu->arch.guest_csr;
> > + int rc;
> > unsigned long __user *uaddr =
> > (unsigned long __user *)(unsigned long)reg->addr;
> > unsigned long reg_num = reg->id & ~(KVM_REG_ARCH_MASK |
> > KVM_REG_SIZE_MASK |
> > KVM_REG_RISCV_CSR);
> > - unsigned long reg_val;
> > + unsigned long reg_val, reg_subtype;
> >
> > if (KVM_REG_SIZE(reg->id) != sizeof(unsigned long))
> > return -EINVAL;
> > +
> > + reg_subtype = (reg_num & KVM_REG_RISCV_SUBTYPE_MASK)
> > + >> KVM_REG_RISCV_SUBTYPE_SHIFT;
> > + reg_num &= ~KVM_REG_RISCV_SUBTYPE_MASK;
> > + switch (reg_subtype) {
> > + case KVM_REG_RISCV_CSR_GENERAL:
> > + rc = kvm_riscv_vcpu_general_get_csr(vcpu, reg_num, &reg_val);
> > + break;
> > + case KVM_REG_RISCV_CSR_AIA:
> > + rc = kvm_riscv_vcpu_aia_get_csr(vcpu, reg_num, &reg_val);
> > + break;
> > + default:
> > + rc = -EINVAL;
> > + break;
> > + }
> > + if (rc)
> > + return rc;
> > +
> > + if (copy_to_user(uaddr, &reg_val, KVM_REG_SIZE(reg->id)))
> > + return -EFAULT;
> > +
> > + return 0;
> > +}
> > +
> > +static inline int kvm_riscv_vcpu_general_set_csr(struct kvm_vcpu *vcpu,
> > + unsigned long reg_num,
> > + unsigned long reg_val)
> > +{
> > + struct kvm_vcpu_csr *csr = &vcpu->arch.guest_csr;
> > +
> > if (reg_num >= sizeof(struct kvm_riscv_csr) / sizeof(unsigned long))
> > return -EINVAL;
> >
> > if (reg_num == KVM_REG_RISCV_CSR_REG(sip)) {
> > - kvm_riscv_vcpu_flush_interrupts(vcpu);
> > - reg_val = (csr->hvip >> VSIP_TO_HVIP_SHIFT) & VSIP_VALID_MASK;
> > - } else
> > - reg_val = ((unsigned long *)csr)[reg_num];
> > + reg_val &= VSIP_VALID_MASK;
> > + reg_val <<= VSIP_TO_HVIP_SHIFT;
> > + }
> >
> > - if (copy_to_user(uaddr, &reg_val, KVM_REG_SIZE(reg->id)))
> > - return -EFAULT;
> > + ((unsigned long *)csr)[reg_num] = reg_val;
> > +
> > + if (reg_num == KVM_REG_RISCV_CSR_REG(sip))
> > + WRITE_ONCE(vcpu->arch.irqs_pending_mask, 0);
> >
> > return 0;
> > }
> > @@ -482,31 +533,36 @@ static int kvm_riscv_vcpu_get_reg_csr(struct kvm_vcpu *vcpu,
> > static int kvm_riscv_vcpu_set_reg_csr(struct kvm_vcpu *vcpu,
> > const struct kvm_one_reg *reg)
> > {
> > - struct kvm_vcpu_csr *csr = &vcpu->arch.guest_csr;
> > + int rc;
> > unsigned long __user *uaddr =
> > (unsigned long __user *)(unsigned long)reg->addr;
> > unsigned long reg_num = reg->id & ~(KVM_REG_ARCH_MASK |
> > KVM_REG_SIZE_MASK |
> > KVM_REG_RISCV_CSR);
> > - unsigned long reg_val;
> > + unsigned long reg_val, reg_subtype;
> >
> > if (KVM_REG_SIZE(reg->id) != sizeof(unsigned long))
> > return -EINVAL;
> > - if (reg_num >= sizeof(struct kvm_riscv_csr) / sizeof(unsigned long))
> > - return -EINVAL;
> >
> > if (copy_from_user(&reg_val, uaddr, KVM_REG_SIZE(reg->id)))
> > return -EFAULT;
> >
> > - if (reg_num == KVM_REG_RISCV_CSR_REG(sip)) {
> > - reg_val &= VSIP_VALID_MASK;
> > - reg_val <<= VSIP_TO_HVIP_SHIFT;
> > + reg_subtype = (reg_num & KVM_REG_RISCV_SUBTYPE_MASK)
> > + >> KVM_REG_RISCV_SUBTYPE_SHIFT;
> > + reg_num &= ~KVM_REG_RISCV_SUBTYPE_MASK;
> > + switch (reg_subtype) {
> > + case KVM_REG_RISCV_CSR_GENERAL:
> > + rc = kvm_riscv_vcpu_general_set_csr(vcpu, reg_num, reg_val);
> > + break;
> > + case KVM_REG_RISCV_CSR_AIA:
> > + rc = kvm_riscv_vcpu_aia_set_csr(vcpu, reg_num, reg_val);
> > + break;
> > + default:
> > + rc = -EINVAL;
> > + break;
> > }
> > -
> > - ((unsigned long *)csr)[reg_num] = reg_val;
> > -
> > - if (reg_num == KVM_REG_RISCV_CSR_REG(sip))
> > - WRITE_ONCE(vcpu->arch.irqs_pending_mask, 0);
> > + if (rc)
> > + return rc;
> >
> > return 0;
> > }
> > --
> > 2.34.1
> >
>
> This can be split into two patches. The first patch can just modify
> the existing implementation
> to parse subtypes for general CSRs. AIA support can be added in the next one.

Okay, I will split this into two patches.

Regards,
Anup