Re: [PATCH] LoongArch: KVM: Add feature passing from user space

From: WANG Xuerui
Date: Thu Jun 06 2024 - 07:26:32 EST


Hi,

On 6/6/24 15:48, Bibo Mao wrote:
Currently features defined in cpucfg CPUCFG_KVM_FEATURE comes from
kvm kernel mode only. Some features are defined in user space VMM,

"come from kernel side only. But currently KVM is not aware of user-space VMM features which makes it hard to employ optimizations that are both (1) specific to the VM use case and (2) requiring cooperation from user space."

however KVM module does not know. Here interface is added to update
register CPUCFG_KVM_FEATURE from user space, only bit 24 - 31 is valid.

Feature KVM_LOONGARCH_VCPU_FEAT_VIRT_EXTIOI is added from user mdoe.
FEAT_VIRT_EXTIOI is virt EXTIOI extension which can route interrupt
to 256 VCPUs rather than 4 CPUs like real hw.

"A new feature bit ... is added which can be set from user space. FEAT_... indicates that the VM EXTIOI can route interrupts to 256 vCPUs, rather than 4 like with real HW."

(Am I right in paraphrasing the "EXTIOI" part?)


Signed-off-by: Bibo Mao <maobibo@xxxxxxxxxxx>
---
arch/loongarch/include/asm/kvm_host.h | 4 +++
arch/loongarch/include/asm/loongarch.h | 5 ++++
arch/loongarch/include/uapi/asm/kvm.h | 2 ++
arch/loongarch/kvm/exit.c | 1 +
arch/loongarch/kvm/vcpu.c | 36 +++++++++++++++++++++++---
5 files changed, 44 insertions(+), 4 deletions(-)

diff --git a/arch/loongarch/include/asm/kvm_host.h b/arch/loongarch/include/asm/kvm_host.h
index 88023ab59486..8fa50d757247 100644
--- a/arch/loongarch/include/asm/kvm_host.h
+++ b/arch/loongarch/include/asm/kvm_host.h
@@ -135,6 +135,9 @@ enum emulation_result {
#define KVM_LARCH_HWCSR_USABLE (0x1 << 4)
#define KVM_LARCH_LBT (0x1 << 5)
+#define KVM_LOONGARCH_USR_FEAT_MASK \
+ BIT(KVM_LOONGARCH_VCPU_FEAT_VIRT_EXTIOI)
+
struct kvm_vcpu_arch {
/*
* Switch pointer-to-function type to unsigned long
@@ -210,6 +213,7 @@ struct kvm_vcpu_arch {
u64 last_steal;
struct gfn_to_hva_cache cache;
} st;
+ unsigned int usr_features;
};
static inline unsigned long readl_sw_gcsr(struct loongarch_csrs *csr, int reg)
diff --git a/arch/loongarch/include/asm/loongarch.h b/arch/loongarch/include/asm/loongarch.h
index 7a4633ef284b..4d9837512c19 100644
--- a/arch/loongarch/include/asm/loongarch.h
+++ b/arch/loongarch/include/asm/loongarch.h
@@ -167,9 +167,14 @@
#define CPUCFG_KVM_SIG (CPUCFG_KVM_BASE + 0)
#define KVM_SIGNATURE "KVM\0"
+/*
+ * BIT 24 - 31 is features configurable by user space vmm
+ */
#define CPUCFG_KVM_FEATURE (CPUCFG_KVM_BASE + 4)
#define KVM_FEATURE_IPI BIT(1)
#define KVM_FEATURE_STEAL_TIME BIT(2)
+/* With VIRT_EXTIOI feature, interrupt can route to 256 VCPUs */
+#define KVM_FEATURE_VIRT_EXTIOI BIT(24)
#ifndef __ASSEMBLY__

What about assigning a new CPUCFG leaf ID for separating the two kinds of feature flags very cleanly?

@@ -896,7 +907,24 @@ static int kvm_loongarch_vcpu_get_attr(struct kvm_vcpu *vcpu,
static int kvm_loongarch_cpucfg_set_attr(struct kvm_vcpu *vcpu,
struct kvm_device_attr *attr)
{
- return -ENXIO;
+ u64 __user *user = (u64 __user *)attr->addr;
+ u64 val, valid_flags;
+
+ switch (attr->attr) {
+ case CPUCFG_KVM_FEATURE:
+ if (get_user(val, user))
+ return -EFAULT;
+
+ valid_flags = KVM_LOONGARCH_USR_FEAT_MASK;
+ if (val & ~valid_flags)
+ return -EINVAL;
+
+ vcpu->arch.usr_features |= val;

Isn't this usage of "|=" instead of "=" implying that the feature bits could not get disabled after being enabled earlier, for whatever reason?

+ return 0;
+
+ default:
+ return -ENXIO;
+ }
}
static int kvm_loongarch_vcpu_set_attr(struct kvm_vcpu *vcpu,

base-commit: 2df0193e62cf887f373995fb8a91068562784adc

--
WANG "xen0n" Xuerui

Linux/LoongArch mailing list: https://lore.kernel.org/loongarch/