2023-01-12 15:11:23

by Anup Patel

[permalink] [raw]
Subject: [PATCH 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/kvmtoo.git

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

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 | 101 ++++-
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, 1072 insertions(+), 53 deletions(-)
create mode 100644 arch/riscv/include/asm/kvm_aia.h
create mode 100644 arch/riscv/kvm/aia.c

--
2.34.1


2023-01-12 15:20:42

by Anup Patel

[permalink] [raw]
Subject: [PATCH 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]>
---
arch/riscv/include/asm/hwcap.h | 8 ++++++++
arch/riscv/kernel/cpu.c | 2 ++
arch/riscv/kernel/cpufeature.c | 2 ++
3 files changed, 12 insertions(+)

diff --git a/arch/riscv/include/asm/hwcap.h b/arch/riscv/include/asm/hwcap.h
index 86328e3acb02..c649e85ed7bb 100644
--- a/arch/riscv/include/asm/hwcap.h
+++ b/arch/riscv/include/asm/hwcap.h
@@ -59,10 +59,18 @@ enum riscv_isa_ext_id {
RISCV_ISA_EXT_ZIHINTPAUSE,
RISCV_ISA_EXT_SSTC,
RISCV_ISA_EXT_SVINVAL,
+ RISCV_ISA_EXT_SSAIA,
+ RISCV_ISA_EXT_SMAIA,
RISCV_ISA_EXT_ID_MAX
};
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/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-12 16:03:17

by Anup Patel

[permalink] [raw]
Subject: [PATCH 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/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 ++
8 files changed, 240 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/kvm_aia.h b/arch/riscv/include/asm/kvm_aia.h
new file mode 100644
index 000000000000..52b5e8aefb30
--- /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..b71f0e639b39 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;
+
/* 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..e7fa4e014d08
--- /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 inline 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) ? 1 : 0;
+ 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-12 16:13:55

by Anup Patel

[permalink] [raw]
Subject: [PATCH 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-26 16:02:50

by Andrew Jones

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

On Thu, Jan 12, 2023 at 07:32:59PM +0530, Anup Patel 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]>
> ---
> arch/riscv/include/asm/hwcap.h | 8 ++++++++
> arch/riscv/kernel/cpu.c | 2 ++
> arch/riscv/kernel/cpufeature.c | 2 ++
> 3 files changed, 12 insertions(+)
>
> diff --git a/arch/riscv/include/asm/hwcap.h b/arch/riscv/include/asm/hwcap.h
> index 86328e3acb02..c649e85ed7bb 100644
> --- a/arch/riscv/include/asm/hwcap.h
> +++ b/arch/riscv/include/asm/hwcap.h
> @@ -59,10 +59,18 @@ enum riscv_isa_ext_id {
> RISCV_ISA_EXT_ZIHINTPAUSE,
> RISCV_ISA_EXT_SSTC,
> RISCV_ISA_EXT_SVINVAL,
> + RISCV_ISA_EXT_SSAIA,
> + RISCV_ISA_EXT_SMAIA,

These will change a couple different ways due other other patches in
flight, but let's put the pair in alphabetical order now so they get
moved together that way.

> RISCV_ISA_EXT_ID_MAX
> };
> 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 isn't used in this patch, so should probably be introduced in a later
patch when it is.

> +
> /*
> * 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/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
>

Otherwise,

Reviewed-by: Andrew Jones <[email protected]>

Thanks,
drew

2023-01-26 16:41:45

by Andrew Jones

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

On Thu, Jan 12, 2023 at 07:33:01PM +0530, Anup Patel 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/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 ++
> 8 files changed, 240 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/kvm_aia.h b/arch/riscv/include/asm/kvm_aia.h
> new file mode 100644
> index 000000000000..52b5e8aefb30
> --- /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))

No need for the '!!' operator in the above two defines since in both
cases the type it's operating on is bool.

> +
> +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..b71f0e639b39 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;

Maybe name this one vcpu_aia?

> +
> /* 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

Shouldn't the compilation of aia.c depend on the same config(s) as the
rest of the kernel's AIA support?

> diff --git a/arch/riscv/kvm/aia.c b/arch/riscv/kvm/aia.c
> new file mode 100644
> index 000000000000..e7fa4e014d08
> --- /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 inline void aia_set_hvictl(bool ext_irq_pending)

I'd drop the 'inline' from functions in .c files and let the compiler
fully decide.

> +{
> + 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) ? 1 : 0;

ext_irq_pending is bool, so this can just be '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
>

Thanks,
drew

2023-01-26 17:19:46

by Andrew Jones

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

On Thu, Jan 12, 2023 at 07:33:02PM +0530, Anup Patel 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

We could just define a new AIA_CSR type, rather than introduce CSR
subtypes. While grouping all CSRs together under the CSR type also
makes sense, having to teach all userspaces about subtypes may not
be worth the organizational benefits.

>
> /* 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
>

Thanks,
drew

2023-01-27 12:40:15

by Anup Patel

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

On Thu, Jan 26, 2023 at 9:32 PM Andrew Jones <[email protected]> wrote:
>
> On Thu, Jan 12, 2023 at 07:32:59PM +0530, Anup Patel 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]>
> > ---
> > arch/riscv/include/asm/hwcap.h | 8 ++++++++
> > arch/riscv/kernel/cpu.c | 2 ++
> > arch/riscv/kernel/cpufeature.c | 2 ++
> > 3 files changed, 12 insertions(+)
> >
> > diff --git a/arch/riscv/include/asm/hwcap.h b/arch/riscv/include/asm/hwcap.h
> > index 86328e3acb02..c649e85ed7bb 100644
> > --- a/arch/riscv/include/asm/hwcap.h
> > +++ b/arch/riscv/include/asm/hwcap.h
> > @@ -59,10 +59,18 @@ enum riscv_isa_ext_id {
> > RISCV_ISA_EXT_ZIHINTPAUSE,
> > RISCV_ISA_EXT_SSTC,
> > RISCV_ISA_EXT_SVINVAL,
> > + RISCV_ISA_EXT_SSAIA,
> > + RISCV_ISA_EXT_SMAIA,
>
> These will change a couple different ways due other other patches in
> flight, but let's put the pair in alphabetical order now so they get
> moved together that way.

Okay, I will update.

>
> > RISCV_ISA_EXT_ID_MAX
> > };
> > 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 isn't used in this patch, so should probably be introduced in a later
> patch when it is.

Okay, I will move this to the patch where it is used.

>
> > +
> > /*
> > * 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/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
> >
>
> Otherwise,
>
> Reviewed-by: Andrew Jones <[email protected]>
>

Thanks,
Anup

2023-01-27 14:31:19

by Anup Patel

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

On Thu, Jan 26, 2023 at 10:11 PM Andrew Jones <[email protected]> wrote:
>
> On Thu, Jan 12, 2023 at 07:33:01PM +0530, Anup Patel 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/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 ++
> > 8 files changed, 240 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/kvm_aia.h b/arch/riscv/include/asm/kvm_aia.h
> > new file mode 100644
> > index 000000000000..52b5e8aefb30
> > --- /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))
>
> No need for the '!!' operator in the above two defines since in both
> cases the type it's operating on is bool.

Okay, I will update.

>
> > +
> > +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..b71f0e639b39 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;
>
> Maybe name this one vcpu_aia?

To be consistent with a few other variables in struct kvm_vcpu_arch, I will
add "_context" suffix to aia variable name.

>
> > +
> > /* 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
>
> Shouldn't the compilation of aia.c depend on the same config(s) as the
> rest of the kernel's AIA support?

The AIA CSRs are just like other ISA extensions which we detect
from the host ISA string. A kconfig option is not required for
AIA CSRs since users can always disable AIA CSRs from
their ISA strings in DT.

>
> > diff --git a/arch/riscv/kvm/aia.c b/arch/riscv/kvm/aia.c
> > new file mode 100644
> > index 000000000000..e7fa4e014d08
> > --- /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 inline void aia_set_hvictl(bool ext_irq_pending)
>
> I'd drop the 'inline' from functions in .c files and let the compiler
> fully decide.

Okay, I will update.

>
> > +{
> > + 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) ? 1 : 0;
>
> ext_irq_pending is bool, so this can just be 'hvictl |= ext_irq_pending'.

Okay, I will update.

>
> > + 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
> >
>
> Thanks,
> drew

Thanks,
Anup

2023-01-27 15:55:41

by Anup Patel

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

On Thu, Jan 26, 2023 at 10:49 PM Andrew Jones <[email protected]> wrote:
>
> On Thu, Jan 12, 2023 at 07:33:02PM +0530, Anup Patel 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
>
> We could just define a new AIA_CSR type, rather than introduce CSR
> subtypes. While grouping all CSRs together under the CSR type also
> makes sense, having to teach all userspaces about subtypes may not
> be worth the organizational benefits.

My main concern is that we have chosen a 8-bit wide "type" field
in ONE_REG id which can't be changed now and it will be consumed
very fast if we start adding separate "type" for each ISA extension.
This "type" field is shared with SBI extensions and we will be also
having separate nested CSR state for various ISA extensions.

Since KVM RISC-V is young, I think it is better to introduce a
"subtype" field now so that we have ample space for the future.

Also, both ONE_REG "type" and "subtype" are arch specific fields
of ONE_REG id so all KVM user space changes will be contained
in RISC-V specific code.

>
> >
> > /* 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
> >
>
> Thanks,
> drew