* [PATCH 0/3] LoongArch: KVM: Add paravirt preempt hint support
@ 2025-11-18 8:06 Bibo Mao
2025-11-18 8:06 ` [PATCH 1/3] LoongArch: KVM: Add preempt hint feature in hypervisor side Bibo Mao
` (2 more replies)
0 siblings, 3 replies; 18+ messages in thread
From: Bibo Mao @ 2025-11-18 8:06 UTC (permalink / raw)
To: Paolo Bonzini, Huacai Chen; +Cc: kvm, loongarch, linux-kernel
vCPU preempt hint is useful with sched and lock on some platforms, here
new feature KVM_FEATURE_PREEMPT_HINT is added and VMM can selectively
enable it.
Test case kcbench is used to compile Linux kernel code, the test result
shows that it is useful on 3D6000 Dual-way machine with 64 cores and 128
hyperthreads, however no improvemet on 3C5000 Dual-way machine with 32
cores. With perf top command when running test case, the main difference
between over-commited VM and host is osq_lock(), it can avoid
unnecessary busy-loop waiting and enter sleep state quickly if lock-hold
vCPU is preempted.
Here is test result with kcbench, time unit is second to compile kernel
with defconfig, performance is better with smaller value.
3D6000 Dual-way 64 Core 128 Threads
One VM with 128 vCPUs, no overcommit, NUMA
Orginal With-patch Improvement
VM 91.72 92.4 < -1%
Host 89.7 89.75 < -0.1%
Two VMs overcommit with 128 vCPUs, UMA
Orginal With-patch Improvement
VM1 306.9 197.5 36%
VM2 303.7 197.8 35%
Host 89.7 89.75 < -0.1%
Two VMs overcommit with 128 vCPUs, NUMA
Orginal With-patch Improvement
VM1 317.1 159 50%
VM2 317.5 158 50%
Host 89.7 89.75 < -0.1%
3C5000 Dual-way 32 Core
One VM with 32 vCPUs, NUMA
Orginal With-patch Improvement
VM 208 207 < 0.5%
Host 184 185 < -0.5%
Two VMs overcommit with 32 vCPUs, UMA
Orginal With-patch Improvement
VM1 439 444 -1%
VM2 437 438 < -0.2%
Host 184 185 < -0.5%
Two VMs overcommit with 32 vCPUs, NUMA
Orginal With-patch Improvement
VM1 422 425 < -1%
VM2 418 415 < -1%
Host 184 185 < -0.5%
Bibo Mao (3):
LoongArch: KVM: Add preempt hint feature in hypervisor side
LoongArch: Add paravirt support with vcpu_is_preempted()
LoongArch: Add paravirt preempt hint print prompt
arch/loongarch/include/asm/kvm_host.h | 2 +
arch/loongarch/include/asm/kvm_para.h | 5 +-
arch/loongarch/include/asm/smp.h | 1 +
arch/loongarch/include/asm/spinlock.h | 5 ++
arch/loongarch/include/uapi/asm/kvm.h | 1 +
arch/loongarch/include/uapi/asm/kvm_para.h | 1 +
arch/loongarch/kernel/paravirt.c | 24 +++++++++-
arch/loongarch/kernel/smp.c | 6 +++
arch/loongarch/kvm/vcpu.c | 54 +++++++++++++++++++++-
arch/loongarch/kvm/vm.c | 5 +-
10 files changed, 100 insertions(+), 4 deletions(-)
base-commit: 6a23ae0a96a600d1d12557add110e0bb6e32730c
--
2.39.3
^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH 1/3] LoongArch: KVM: Add preempt hint feature in hypervisor side
2025-11-18 8:06 [PATCH 0/3] LoongArch: KVM: Add paravirt preempt hint support Bibo Mao
@ 2025-11-18 8:06 ` Bibo Mao
2025-11-18 12:46 ` Huacai Chen
2025-11-18 8:06 ` [PATCH 2/3] LoongArch: Add paravirt support with vcpu_is_preempted() Bibo Mao
2025-11-18 8:06 ` [PATCH 3/3] LoongArch: Add paravirt preempt hint print prompt Bibo Mao
2 siblings, 1 reply; 18+ messages in thread
From: Bibo Mao @ 2025-11-18 8:06 UTC (permalink / raw)
To: Paolo Bonzini, Huacai Chen, Tianrui Zhao, WANG Xuerui
Cc: kvm, loongarch, linux-kernel
Feature KVM_FEATURE_PREEMPT_HINT is added to show whether vCPU is
preempted or not. It is to help guest OS scheduling or lock checking
etc. Here add KVM_FEATURE_PREEMPT_HINT feature and use one byte as
preempted flag in steal time structure.
Signed-off-by: Bibo Mao <maobibo@loongson.cn>
---
arch/loongarch/include/asm/kvm_host.h | 2 +
arch/loongarch/include/asm/kvm_para.h | 5 +-
arch/loongarch/include/uapi/asm/kvm.h | 1 +
arch/loongarch/include/uapi/asm/kvm_para.h | 1 +
arch/loongarch/kvm/vcpu.c | 54 +++++++++++++++++++++-
arch/loongarch/kvm/vm.c | 5 +-
6 files changed, 65 insertions(+), 3 deletions(-)
diff --git a/arch/loongarch/include/asm/kvm_host.h b/arch/loongarch/include/asm/kvm_host.h
index 0cecbd038bb3..04c6dd171877 100644
--- a/arch/loongarch/include/asm/kvm_host.h
+++ b/arch/loongarch/include/asm/kvm_host.h
@@ -163,6 +163,7 @@ enum emulation_result {
#define LOONGARCH_PV_FEAT_UPDATED BIT_ULL(63)
#define LOONGARCH_PV_FEAT_MASK (BIT(KVM_FEATURE_IPI) | \
BIT(KVM_FEATURE_STEAL_TIME) | \
+ BIT(KVM_FEATURE_PREEMPT_HINT) |\
BIT(KVM_FEATURE_USER_HCALL) | \
BIT(KVM_FEATURE_VIRT_EXTIOI))
@@ -250,6 +251,7 @@ struct kvm_vcpu_arch {
u64 guest_addr;
u64 last_steal;
struct gfn_to_hva_cache cache;
+ u8 preempted;
} st;
};
diff --git a/arch/loongarch/include/asm/kvm_para.h b/arch/loongarch/include/asm/kvm_para.h
index 3e4b397f423f..d8592a7f5922 100644
--- a/arch/loongarch/include/asm/kvm_para.h
+++ b/arch/loongarch/include/asm/kvm_para.h
@@ -37,8 +37,11 @@ struct kvm_steal_time {
__u64 steal;
__u32 version;
__u32 flags;
- __u32 pad[12];
+ __u8 preempted;
+ __u8 u8_pad[3];
+ __u32 pad[11];
};
+#define KVM_VCPU_PREEMPTED (1 << 0)
/*
* Hypercall interface for KVM hypervisor
diff --git a/arch/loongarch/include/uapi/asm/kvm.h b/arch/loongarch/include/uapi/asm/kvm.h
index 57ba1a563bb1..bca7154aa651 100644
--- a/arch/loongarch/include/uapi/asm/kvm.h
+++ b/arch/loongarch/include/uapi/asm/kvm.h
@@ -104,6 +104,7 @@ struct kvm_fpu {
#define KVM_LOONGARCH_VM_FEAT_PV_IPI 6
#define KVM_LOONGARCH_VM_FEAT_PV_STEALTIME 7
#define KVM_LOONGARCH_VM_FEAT_PTW 8
+#define KVM_LOONGARCH_VM_FEAT_PV_PREEMPT_HINT 10
/* Device Control API on vcpu fd */
#define KVM_LOONGARCH_VCPU_CPUCFG 0
diff --git a/arch/loongarch/include/uapi/asm/kvm_para.h b/arch/loongarch/include/uapi/asm/kvm_para.h
index 76d802ef01ce..fe4107869ce6 100644
--- a/arch/loongarch/include/uapi/asm/kvm_para.h
+++ b/arch/loongarch/include/uapi/asm/kvm_para.h
@@ -15,6 +15,7 @@
#define CPUCFG_KVM_FEATURE (CPUCFG_KVM_BASE + 4)
#define KVM_FEATURE_IPI 1
#define KVM_FEATURE_STEAL_TIME 2
+#define KVM_FEATURE_PREEMPT_HINT 3
/* BIT 24 - 31 are features configurable by user space vmm */
#define KVM_FEATURE_VIRT_EXTIOI 24
#define KVM_FEATURE_USER_HCALL 25
diff --git a/arch/loongarch/kvm/vcpu.c b/arch/loongarch/kvm/vcpu.c
index 1245a6b35896..33a94b191b5d 100644
--- a/arch/loongarch/kvm/vcpu.c
+++ b/arch/loongarch/kvm/vcpu.c
@@ -180,6 +180,11 @@ static void kvm_update_stolen_time(struct kvm_vcpu *vcpu)
}
st = (struct kvm_steal_time __user *)ghc->hva;
+ if (kvm_guest_has_pv_feature(vcpu, KVM_FEATURE_PREEMPT_HINT)) {
+ unsafe_put_user(0, &st->preempted, out);
+ vcpu->arch.st.preempted = 0;
+ }
+
unsafe_get_user(version, &st->version, out);
if (version & 1)
version += 1; /* first time write, random junk */
@@ -1757,11 +1762,58 @@ static int _kvm_vcpu_put(struct kvm_vcpu *vcpu, int cpu)
return 0;
}
+static void _kvm_set_vcpu_preempted(struct kvm_vcpu *vcpu)
+{
+ struct gfn_to_hva_cache *ghc;
+ struct kvm_steal_time __user *st;
+ struct kvm_memslots *slots;
+ static const u8 preempted = KVM_VCPU_PREEMPTED;
+ gpa_t gpa;
+
+ gpa = vcpu->arch.st.guest_addr;
+ if (!(gpa & KVM_STEAL_PHYS_VALID))
+ return;
+
+ /* vCPU may be preempted for many times */
+ if (vcpu->arch.st.preempted)
+ return;
+
+ /* This happens on process exit */
+ if (unlikely(current->mm != vcpu->kvm->mm))
+ return;
+
+ gpa &= KVM_STEAL_PHYS_MASK;
+ ghc = &vcpu->arch.st.cache;
+ slots = kvm_memslots(vcpu->kvm);
+ if (slots->generation != ghc->generation || gpa != ghc->gpa) {
+ if (kvm_gfn_to_hva_cache_init(vcpu->kvm, ghc, gpa, sizeof(*st))) {
+ ghc->gpa = INVALID_GPA;
+ return;
+ }
+ }
+
+ st = (struct kvm_steal_time __user *)ghc->hva;
+ unsafe_put_user(preempted, &st->preempted, out);
+ vcpu->arch.st.preempted = KVM_VCPU_PREEMPTED;
+out:
+ mark_page_dirty_in_slot(vcpu->kvm, ghc->memslot, gpa_to_gfn(ghc->gpa));
+}
+
void kvm_arch_vcpu_put(struct kvm_vcpu *vcpu)
{
- int cpu;
+ int cpu, idx;
unsigned long flags;
+ if (vcpu->preempted && kvm_guest_has_pv_feature(vcpu, KVM_FEATURE_PREEMPT_HINT)) {
+ /*
+ * Take the srcu lock as memslots will be accessed to check the gfn
+ * cache generation against the memslots generation.
+ */
+ idx = srcu_read_lock(&vcpu->kvm->srcu);
+ _kvm_set_vcpu_preempted(vcpu);
+ srcu_read_unlock(&vcpu->kvm->srcu, idx);
+ }
+
local_irq_save(flags);
cpu = smp_processor_id();
vcpu->arch.last_sched_cpu = cpu;
diff --git a/arch/loongarch/kvm/vm.c b/arch/loongarch/kvm/vm.c
index a49b1c1a3dd1..b8879110a0a1 100644
--- a/arch/loongarch/kvm/vm.c
+++ b/arch/loongarch/kvm/vm.c
@@ -45,8 +45,10 @@ int kvm_arch_init_vm(struct kvm *kvm, unsigned long type)
/* Enable all PV features by default */
kvm->arch.pv_features = BIT(KVM_FEATURE_IPI);
- if (kvm_pvtime_supported())
+ if (kvm_pvtime_supported()) {
kvm->arch.pv_features |= BIT(KVM_FEATURE_STEAL_TIME);
+ kvm->arch.pv_features |= BIT(KVM_FEATURE_PREEMPT_HINT);
+ }
/*
* cpu_vabits means user address space only (a half of total).
@@ -143,6 +145,7 @@ static int kvm_vm_feature_has_attr(struct kvm *kvm, struct kvm_device_attr *attr
case KVM_LOONGARCH_VM_FEAT_PV_IPI:
return 0;
case KVM_LOONGARCH_VM_FEAT_PV_STEALTIME:
+ case KVM_LOONGARCH_VM_FEAT_PV_PREEMPT_HINT:
if (kvm_pvtime_supported())
return 0;
return -ENXIO;
--
2.39.3
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH 2/3] LoongArch: Add paravirt support with vcpu_is_preempted()
2025-11-18 8:06 [PATCH 0/3] LoongArch: KVM: Add paravirt preempt hint support Bibo Mao
2025-11-18 8:06 ` [PATCH 1/3] LoongArch: KVM: Add preempt hint feature in hypervisor side Bibo Mao
@ 2025-11-18 8:06 ` Bibo Mao
2025-11-18 12:48 ` Huacai Chen
2025-11-20 2:51 ` kernel test robot
2025-11-18 8:06 ` [PATCH 3/3] LoongArch: Add paravirt preempt hint print prompt Bibo Mao
2 siblings, 2 replies; 18+ messages in thread
From: Bibo Mao @ 2025-11-18 8:06 UTC (permalink / raw)
To: Paolo Bonzini, Huacai Chen, WANG Xuerui, Peter Zijlstra,
Ingo Molnar, Will Deacon, Boqun Feng, Waiman Long, Juergen Gross,
Ajay Kaher, Alexey Makhalov, Broadcom internal kernel review list
Cc: kvm, loongarch, linux-kernel, virtualization, x86
Function vcpu_is_preempted() is used to check whether vCPU is preempted
or not. Here add implementation with vcpu_is_preempted() when option
CONFIG_PARAVIRT is enabled.
Signed-off-by: Bibo Mao <maobibo@loongson.cn>
---
arch/loongarch/include/asm/smp.h | 1 +
arch/loongarch/include/asm/spinlock.h | 5 +++++
arch/loongarch/kernel/paravirt.c | 16 ++++++++++++++++
arch/loongarch/kernel/smp.c | 6 ++++++
4 files changed, 28 insertions(+)
diff --git a/arch/loongarch/include/asm/smp.h b/arch/loongarch/include/asm/smp.h
index 3a47f52959a8..5b37f7bf2060 100644
--- a/arch/loongarch/include/asm/smp.h
+++ b/arch/loongarch/include/asm/smp.h
@@ -18,6 +18,7 @@ struct smp_ops {
void (*init_ipi)(void);
void (*send_ipi_single)(int cpu, unsigned int action);
void (*send_ipi_mask)(const struct cpumask *mask, unsigned int action);
+ bool (*vcpu_is_preempted)(int cpu);
};
extern struct smp_ops mp_ops;
diff --git a/arch/loongarch/include/asm/spinlock.h b/arch/loongarch/include/asm/spinlock.h
index 7cb3476999be..c001cef893aa 100644
--- a/arch/loongarch/include/asm/spinlock.h
+++ b/arch/loongarch/include/asm/spinlock.h
@@ -5,6 +5,11 @@
#ifndef _ASM_SPINLOCK_H
#define _ASM_SPINLOCK_H
+#ifdef CONFIG_PARAVIRT
+#define vcpu_is_preempted vcpu_is_preempted
+bool vcpu_is_preempted(int cpu);
+#endif
+
#include <asm/processor.h>
#include <asm/qspinlock.h>
#include <asm/qrwlock.h>
diff --git a/arch/loongarch/kernel/paravirt.c b/arch/loongarch/kernel/paravirt.c
index b1b51f920b23..b99404b6b13f 100644
--- a/arch/loongarch/kernel/paravirt.c
+++ b/arch/loongarch/kernel/paravirt.c
@@ -52,6 +52,13 @@ static u64 paravt_steal_clock(int cpu)
#ifdef CONFIG_SMP
static struct smp_ops native_ops;
+static bool pv_vcpu_is_preempted(int cpu)
+{
+ struct kvm_steal_time *src = &per_cpu(steal_time, cpu);
+
+ return !!(src->preempted & KVM_VCPU_PREEMPTED);
+}
+
static void pv_send_ipi_single(int cpu, unsigned int action)
{
int min, old;
@@ -308,6 +315,9 @@ int __init pv_time_init(void)
pr_err("Failed to install cpu hotplug callbacks\n");
return r;
}
+
+ if (kvm_para_has_feature(KVM_FEATURE_PREEMPT_HINT))
+ mp_ops.vcpu_is_preempted = pv_vcpu_is_preempted;
#endif
static_call_update(pv_steal_clock, paravt_steal_clock);
@@ -332,3 +342,9 @@ int __init pv_spinlock_init(void)
return 0;
}
+
+bool notrace vcpu_is_preempted(int cpu)
+{
+ return mp_ops.vcpu_is_preempted(cpu);
+}
+EXPORT_SYMBOL(vcpu_is_preempted);
diff --git a/arch/loongarch/kernel/smp.c b/arch/loongarch/kernel/smp.c
index 46036d98da75..f04192fedf8d 100644
--- a/arch/loongarch/kernel/smp.c
+++ b/arch/loongarch/kernel/smp.c
@@ -307,10 +307,16 @@ static void loongson_init_ipi(void)
panic("IPI IRQ request failed\n");
}
+static bool loongson_vcpu_is_preempted(int cpu)
+{
+ return false;
+}
+
struct smp_ops mp_ops = {
.init_ipi = loongson_init_ipi,
.send_ipi_single = loongson_send_ipi_single,
.send_ipi_mask = loongson_send_ipi_mask,
+ .vcpu_is_preempted = loongson_vcpu_is_preempted,
};
static void __init fdt_smp_setup(void)
--
2.39.3
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH 3/3] LoongArch: Add paravirt preempt hint print prompt
2025-11-18 8:06 [PATCH 0/3] LoongArch: KVM: Add paravirt preempt hint support Bibo Mao
2025-11-18 8:06 ` [PATCH 1/3] LoongArch: KVM: Add preempt hint feature in hypervisor side Bibo Mao
2025-11-18 8:06 ` [PATCH 2/3] LoongArch: Add paravirt support with vcpu_is_preempted() Bibo Mao
@ 2025-11-18 8:06 ` Bibo Mao
2 siblings, 0 replies; 18+ messages in thread
From: Bibo Mao @ 2025-11-18 8:06 UTC (permalink / raw)
To: Paolo Bonzini, Huacai Chen, Juergen Gross, Ajay Kaher,
Alexey Makhalov, Broadcom internal kernel review list,
WANG Xuerui
Cc: kvm, loongarch, linux-kernel, virtualization, x86
Add paravirt preempt hint print prompt together with steal timer
information, so that it is easy to check whether paravirt preempt hint
feature is enabled or not.
Signed-off-by: Bibo Mao <maobibo@loongson.cn>
---
arch/loongarch/kernel/paravirt.c | 10 ++++++++--
1 file changed, 8 insertions(+), 2 deletions(-)
diff --git a/arch/loongarch/kernel/paravirt.c b/arch/loongarch/kernel/paravirt.c
index b99404b6b13f..b7ea511c288b 100644
--- a/arch/loongarch/kernel/paravirt.c
+++ b/arch/loongarch/kernel/paravirt.c
@@ -294,6 +294,7 @@ static struct notifier_block pv_reboot_nb = {
int __init pv_time_init(void)
{
int r;
+ bool pv_preempted = false;
if (!kvm_para_has_feature(KVM_FEATURE_STEAL_TIME))
return 0;
@@ -316,8 +317,10 @@ int __init pv_time_init(void)
return r;
}
- if (kvm_para_has_feature(KVM_FEATURE_PREEMPT_HINT))
+ if (kvm_para_has_feature(KVM_FEATURE_PREEMPT_HINT)) {
mp_ops.vcpu_is_preempted = pv_vcpu_is_preempted;
+ pv_preempted = true;
+ }
#endif
static_call_update(pv_steal_clock, paravt_steal_clock);
@@ -328,7 +331,10 @@ int __init pv_time_init(void)
static_key_slow_inc(¶virt_steal_rq_enabled);
#endif
- pr_info("Using paravirt steal-time\n");
+ if (pv_preempted)
+ pr_info("Using paravirt steal-time with preempt hint enabled\n");
+ else
+ pr_info("Using paravirt steal-time with preempt hint disabled\n");
return 0;
}
--
2.39.3
^ permalink raw reply related [flat|nested] 18+ messages in thread
* Re: [PATCH 1/3] LoongArch: KVM: Add preempt hint feature in hypervisor side
2025-11-18 8:06 ` [PATCH 1/3] LoongArch: KVM: Add preempt hint feature in hypervisor side Bibo Mao
@ 2025-11-18 12:46 ` Huacai Chen
2025-11-19 1:20 ` Bibo Mao
0 siblings, 1 reply; 18+ messages in thread
From: Huacai Chen @ 2025-11-18 12:46 UTC (permalink / raw)
To: Bibo Mao
Cc: Paolo Bonzini, Tianrui Zhao, WANG Xuerui, kvm, loongarch,
linux-kernel
Hi, Bibo,
On Tue, Nov 18, 2025 at 4:07 PM Bibo Mao <maobibo@loongson.cn> wrote:
>
> Feature KVM_FEATURE_PREEMPT_HINT is added to show whether vCPU is
> preempted or not. It is to help guest OS scheduling or lock checking
> etc. Here add KVM_FEATURE_PREEMPT_HINT feature and use one byte as
> preempted flag in steal time structure.
>
> Signed-off-by: Bibo Mao <maobibo@loongson.cn>
> ---
> arch/loongarch/include/asm/kvm_host.h | 2 +
> arch/loongarch/include/asm/kvm_para.h | 5 +-
> arch/loongarch/include/uapi/asm/kvm.h | 1 +
> arch/loongarch/include/uapi/asm/kvm_para.h | 1 +
> arch/loongarch/kvm/vcpu.c | 54 +++++++++++++++++++++-
> arch/loongarch/kvm/vm.c | 5 +-
> 6 files changed, 65 insertions(+), 3 deletions(-)
>
> diff --git a/arch/loongarch/include/asm/kvm_host.h b/arch/loongarch/include/asm/kvm_host.h
> index 0cecbd038bb3..04c6dd171877 100644
> --- a/arch/loongarch/include/asm/kvm_host.h
> +++ b/arch/loongarch/include/asm/kvm_host.h
> @@ -163,6 +163,7 @@ enum emulation_result {
> #define LOONGARCH_PV_FEAT_UPDATED BIT_ULL(63)
> #define LOONGARCH_PV_FEAT_MASK (BIT(KVM_FEATURE_IPI) | \
> BIT(KVM_FEATURE_STEAL_TIME) | \
> + BIT(KVM_FEATURE_PREEMPT_HINT) |\
> BIT(KVM_FEATURE_USER_HCALL) | \
> BIT(KVM_FEATURE_VIRT_EXTIOI))
>
> @@ -250,6 +251,7 @@ struct kvm_vcpu_arch {
> u64 guest_addr;
> u64 last_steal;
> struct gfn_to_hva_cache cache;
> + u8 preempted;
> } st;
> };
>
> diff --git a/arch/loongarch/include/asm/kvm_para.h b/arch/loongarch/include/asm/kvm_para.h
> index 3e4b397f423f..d8592a7f5922 100644
> --- a/arch/loongarch/include/asm/kvm_para.h
> +++ b/arch/loongarch/include/asm/kvm_para.h
> @@ -37,8 +37,11 @@ struct kvm_steal_time {
> __u64 steal;
> __u32 version;
> __u32 flags;
> - __u32 pad[12];
> + __u8 preempted;
> + __u8 u8_pad[3];
> + __u32 pad[11];
Maybe a single __u8 pad[47] is enough?
> };
> +#define KVM_VCPU_PREEMPTED (1 << 0)
>
> /*
> * Hypercall interface for KVM hypervisor
> diff --git a/arch/loongarch/include/uapi/asm/kvm.h b/arch/loongarch/include/uapi/asm/kvm.h
> index 57ba1a563bb1..bca7154aa651 100644
> --- a/arch/loongarch/include/uapi/asm/kvm.h
> +++ b/arch/loongarch/include/uapi/asm/kvm.h
> @@ -104,6 +104,7 @@ struct kvm_fpu {
> #define KVM_LOONGARCH_VM_FEAT_PV_IPI 6
> #define KVM_LOONGARCH_VM_FEAT_PV_STEALTIME 7
> #define KVM_LOONGARCH_VM_FEAT_PTW 8
> +#define KVM_LOONGARCH_VM_FEAT_PV_PREEMPT_HINT 10
From the name it is a "hint", from include/linux/kvm_para.h we know
features and hints are different. If preempt is really a feature,
rename it?
>
> /* Device Control API on vcpu fd */
> #define KVM_LOONGARCH_VCPU_CPUCFG 0
> diff --git a/arch/loongarch/include/uapi/asm/kvm_para.h b/arch/loongarch/include/uapi/asm/kvm_para.h
> index 76d802ef01ce..fe4107869ce6 100644
> --- a/arch/loongarch/include/uapi/asm/kvm_para.h
> +++ b/arch/loongarch/include/uapi/asm/kvm_para.h
> @@ -15,6 +15,7 @@
> #define CPUCFG_KVM_FEATURE (CPUCFG_KVM_BASE + 4)
> #define KVM_FEATURE_IPI 1
> #define KVM_FEATURE_STEAL_TIME 2
> +#define KVM_FEATURE_PREEMPT_HINT 3
> /* BIT 24 - 31 are features configurable by user space vmm */
> #define KVM_FEATURE_VIRT_EXTIOI 24
> #define KVM_FEATURE_USER_HCALL 25
> diff --git a/arch/loongarch/kvm/vcpu.c b/arch/loongarch/kvm/vcpu.c
> index 1245a6b35896..33a94b191b5d 100644
> --- a/arch/loongarch/kvm/vcpu.c
> +++ b/arch/loongarch/kvm/vcpu.c
> @@ -180,6 +180,11 @@ static void kvm_update_stolen_time(struct kvm_vcpu *vcpu)
> }
>
> st = (struct kvm_steal_time __user *)ghc->hva;
> + if (kvm_guest_has_pv_feature(vcpu, KVM_FEATURE_PREEMPT_HINT)) {
> + unsafe_put_user(0, &st->preempted, out);
> + vcpu->arch.st.preempted = 0;
> + }
> +
> unsafe_get_user(version, &st->version, out);
> if (version & 1)
> version += 1; /* first time write, random junk */
> @@ -1757,11 +1762,58 @@ static int _kvm_vcpu_put(struct kvm_vcpu *vcpu, int cpu)
> return 0;
> }
>
> +static void _kvm_set_vcpu_preempted(struct kvm_vcpu *vcpu)
Just using kvm_set_vcpu_preempted() is enough, no "_".
> +{
> + struct gfn_to_hva_cache *ghc;
> + struct kvm_steal_time __user *st;
> + struct kvm_memslots *slots;
> + static const u8 preempted = KVM_VCPU_PREEMPTED;
I'm not sure whether "static" is right, it's not reentrant.
Huacai
> + gpa_t gpa;
> +
> + gpa = vcpu->arch.st.guest_addr;
> + if (!(gpa & KVM_STEAL_PHYS_VALID))
> + return;
> +
> + /* vCPU may be preempted for many times */
> + if (vcpu->arch.st.preempted)
> + return;
> +
> + /* This happens on process exit */
> + if (unlikely(current->mm != vcpu->kvm->mm))
> + return;
> +
> + gpa &= KVM_STEAL_PHYS_MASK;
> + ghc = &vcpu->arch.st.cache;
> + slots = kvm_memslots(vcpu->kvm);
> + if (slots->generation != ghc->generation || gpa != ghc->gpa) {
> + if (kvm_gfn_to_hva_cache_init(vcpu->kvm, ghc, gpa, sizeof(*st))) {
> + ghc->gpa = INVALID_GPA;
> + return;
> + }
> + }
> +
> + st = (struct kvm_steal_time __user *)ghc->hva;
> + unsafe_put_user(preempted, &st->preempted, out);
> + vcpu->arch.st.preempted = KVM_VCPU_PREEMPTED;
> +out:
> + mark_page_dirty_in_slot(vcpu->kvm, ghc->memslot, gpa_to_gfn(ghc->gpa));
> +}
> +
> void kvm_arch_vcpu_put(struct kvm_vcpu *vcpu)
> {
> - int cpu;
> + int cpu, idx;
> unsigned long flags;
>
> + if (vcpu->preempted && kvm_guest_has_pv_feature(vcpu, KVM_FEATURE_PREEMPT_HINT)) {
> + /*
> + * Take the srcu lock as memslots will be accessed to check the gfn
> + * cache generation against the memslots generation.
> + */
> + idx = srcu_read_lock(&vcpu->kvm->srcu);
> + _kvm_set_vcpu_preempted(vcpu);
> + srcu_read_unlock(&vcpu->kvm->srcu, idx);
> + }
> +
> local_irq_save(flags);
> cpu = smp_processor_id();
> vcpu->arch.last_sched_cpu = cpu;
> diff --git a/arch/loongarch/kvm/vm.c b/arch/loongarch/kvm/vm.c
> index a49b1c1a3dd1..b8879110a0a1 100644
> --- a/arch/loongarch/kvm/vm.c
> +++ b/arch/loongarch/kvm/vm.c
> @@ -45,8 +45,10 @@ int kvm_arch_init_vm(struct kvm *kvm, unsigned long type)
>
> /* Enable all PV features by default */
> kvm->arch.pv_features = BIT(KVM_FEATURE_IPI);
> - if (kvm_pvtime_supported())
> + if (kvm_pvtime_supported()) {
> kvm->arch.pv_features |= BIT(KVM_FEATURE_STEAL_TIME);
> + kvm->arch.pv_features |= BIT(KVM_FEATURE_PREEMPT_HINT);
> + }
>
> /*
> * cpu_vabits means user address space only (a half of total).
> @@ -143,6 +145,7 @@ static int kvm_vm_feature_has_attr(struct kvm *kvm, struct kvm_device_attr *attr
> case KVM_LOONGARCH_VM_FEAT_PV_IPI:
> return 0;
> case KVM_LOONGARCH_VM_FEAT_PV_STEALTIME:
> + case KVM_LOONGARCH_VM_FEAT_PV_PREEMPT_HINT:
> if (kvm_pvtime_supported())
> return 0;
> return -ENXIO;
> --
> 2.39.3
>
>
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 2/3] LoongArch: Add paravirt support with vcpu_is_preempted()
2025-11-18 8:06 ` [PATCH 2/3] LoongArch: Add paravirt support with vcpu_is_preempted() Bibo Mao
@ 2025-11-18 12:48 ` Huacai Chen
2025-11-19 1:59 ` Bibo Mao
2025-11-19 2:50 ` Bibo Mao
2025-11-20 2:51 ` kernel test robot
1 sibling, 2 replies; 18+ messages in thread
From: Huacai Chen @ 2025-11-18 12:48 UTC (permalink / raw)
To: Bibo Mao
Cc: Paolo Bonzini, WANG Xuerui, Peter Zijlstra, Ingo Molnar,
Will Deacon, Boqun Feng, Waiman Long, Juergen Gross, Ajay Kaher,
Alexey Makhalov, Broadcom internal kernel review list, kvm,
loongarch, linux-kernel, virtualization, x86
Hi, Bibo,
On Tue, Nov 18, 2025 at 4:07 PM Bibo Mao <maobibo@loongson.cn> wrote:
>
> Function vcpu_is_preempted() is used to check whether vCPU is preempted
> or not. Here add implementation with vcpu_is_preempted() when option
> CONFIG_PARAVIRT is enabled.
>
> Signed-off-by: Bibo Mao <maobibo@loongson.cn>
> ---
> arch/loongarch/include/asm/smp.h | 1 +
> arch/loongarch/include/asm/spinlock.h | 5 +++++
> arch/loongarch/kernel/paravirt.c | 16 ++++++++++++++++
> arch/loongarch/kernel/smp.c | 6 ++++++
> 4 files changed, 28 insertions(+)
>
> diff --git a/arch/loongarch/include/asm/smp.h b/arch/loongarch/include/asm/smp.h
> index 3a47f52959a8..5b37f7bf2060 100644
> --- a/arch/loongarch/include/asm/smp.h
> +++ b/arch/loongarch/include/asm/smp.h
> @@ -18,6 +18,7 @@ struct smp_ops {
> void (*init_ipi)(void);
> void (*send_ipi_single)(int cpu, unsigned int action);
> void (*send_ipi_mask)(const struct cpumask *mask, unsigned int action);
> + bool (*vcpu_is_preempted)(int cpu);
> };
> extern struct smp_ops mp_ops;
>
> diff --git a/arch/loongarch/include/asm/spinlock.h b/arch/loongarch/include/asm/spinlock.h
> index 7cb3476999be..c001cef893aa 100644
> --- a/arch/loongarch/include/asm/spinlock.h
> +++ b/arch/loongarch/include/asm/spinlock.h
> @@ -5,6 +5,11 @@
> #ifndef _ASM_SPINLOCK_H
> #define _ASM_SPINLOCK_H
>
> +#ifdef CONFIG_PARAVIRT
> +#define vcpu_is_preempted vcpu_is_preempted
> +bool vcpu_is_preempted(int cpu);
> +#endif
Maybe paravirt.h is a better place?
> +
> #include <asm/processor.h>
> #include <asm/qspinlock.h>
> #include <asm/qrwlock.h>
> diff --git a/arch/loongarch/kernel/paravirt.c b/arch/loongarch/kernel/paravirt.c
> index b1b51f920b23..b99404b6b13f 100644
> --- a/arch/loongarch/kernel/paravirt.c
> +++ b/arch/loongarch/kernel/paravirt.c
> @@ -52,6 +52,13 @@ static u64 paravt_steal_clock(int cpu)
> #ifdef CONFIG_SMP
> static struct smp_ops native_ops;
>
> +static bool pv_vcpu_is_preempted(int cpu)
> +{
> + struct kvm_steal_time *src = &per_cpu(steal_time, cpu);
> +
> + return !!(src->preempted & KVM_VCPU_PREEMPTED);
> +}
> +
> static void pv_send_ipi_single(int cpu, unsigned int action)
> {
> int min, old;
> @@ -308,6 +315,9 @@ int __init pv_time_init(void)
> pr_err("Failed to install cpu hotplug callbacks\n");
> return r;
> }
> +
> + if (kvm_para_has_feature(KVM_FEATURE_PREEMPT_HINT))
> + mp_ops.vcpu_is_preempted = pv_vcpu_is_preempted;
> #endif
>
> static_call_update(pv_steal_clock, paravt_steal_clock);
> @@ -332,3 +342,9 @@ int __init pv_spinlock_init(void)
>
> return 0;
> }
> +
> +bool notrace vcpu_is_preempted(int cpu)
> +{
> + return mp_ops.vcpu_is_preempted(cpu);
> +}
We can simplify the whole patch like this, then we don't need to touch
smp.c, and we can merge Patch-2/3.
+bool notrace vcpu_is_preempted(int cpu)
+{
+ if (!kvm_para_has_feature(KVM_FEATURE_PREEMPT_HINT))
+ return false;
+ else {
+ struct kvm_steal_time *src = &per_cpu(steal_time, cpu);
+ return !!(src->preempted & KVM_VCPU_PREEMPTED);
+ }
+}
Huacai
> +EXPORT_SYMBOL(vcpu_is_preempted);
> diff --git a/arch/loongarch/kernel/smp.c b/arch/loongarch/kernel/smp.c
> index 46036d98da75..f04192fedf8d 100644
> --- a/arch/loongarch/kernel/smp.c
> +++ b/arch/loongarch/kernel/smp.c
> @@ -307,10 +307,16 @@ static void loongson_init_ipi(void)
> panic("IPI IRQ request failed\n");
> }
>
> +static bool loongson_vcpu_is_preempted(int cpu)
> +{
> + return false;
> +}
> +
> struct smp_ops mp_ops = {
> .init_ipi = loongson_init_ipi,
> .send_ipi_single = loongson_send_ipi_single,
> .send_ipi_mask = loongson_send_ipi_mask,
> + .vcpu_is_preempted = loongson_vcpu_is_preempted,
> };
>
> static void __init fdt_smp_setup(void)
> --
> 2.39.3
>
>
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 1/3] LoongArch: KVM: Add preempt hint feature in hypervisor side
2025-11-18 12:46 ` Huacai Chen
@ 2025-11-19 1:20 ` Bibo Mao
2025-11-19 2:45 ` Huacai Chen
0 siblings, 1 reply; 18+ messages in thread
From: Bibo Mao @ 2025-11-19 1:20 UTC (permalink / raw)
To: Huacai Chen
Cc: Paolo Bonzini, Tianrui Zhao, WANG Xuerui, kvm, loongarch,
linux-kernel
On 2025/11/18 下午8:46, Huacai Chen wrote:
> Hi, Bibo,
>
> On Tue, Nov 18, 2025 at 4:07 PM Bibo Mao <maobibo@loongson.cn> wrote:
>>
>> Feature KVM_FEATURE_PREEMPT_HINT is added to show whether vCPU is
>> preempted or not. It is to help guest OS scheduling or lock checking
>> etc. Here add KVM_FEATURE_PREEMPT_HINT feature and use one byte as
>> preempted flag in steal time structure.
>>
>> Signed-off-by: Bibo Mao <maobibo@loongson.cn>
>> ---
>> arch/loongarch/include/asm/kvm_host.h | 2 +
>> arch/loongarch/include/asm/kvm_para.h | 5 +-
>> arch/loongarch/include/uapi/asm/kvm.h | 1 +
>> arch/loongarch/include/uapi/asm/kvm_para.h | 1 +
>> arch/loongarch/kvm/vcpu.c | 54 +++++++++++++++++++++-
>> arch/loongarch/kvm/vm.c | 5 +-
>> 6 files changed, 65 insertions(+), 3 deletions(-)
>>
>> diff --git a/arch/loongarch/include/asm/kvm_host.h b/arch/loongarch/include/asm/kvm_host.h
>> index 0cecbd038bb3..04c6dd171877 100644
>> --- a/arch/loongarch/include/asm/kvm_host.h
>> +++ b/arch/loongarch/include/asm/kvm_host.h
>> @@ -163,6 +163,7 @@ enum emulation_result {
>> #define LOONGARCH_PV_FEAT_UPDATED BIT_ULL(63)
>> #define LOONGARCH_PV_FEAT_MASK (BIT(KVM_FEATURE_IPI) | \
>> BIT(KVM_FEATURE_STEAL_TIME) | \
>> + BIT(KVM_FEATURE_PREEMPT_HINT) |\
>> BIT(KVM_FEATURE_USER_HCALL) | \
>> BIT(KVM_FEATURE_VIRT_EXTIOI))
>>
>> @@ -250,6 +251,7 @@ struct kvm_vcpu_arch {
>> u64 guest_addr;
>> u64 last_steal;
>> struct gfn_to_hva_cache cache;
>> + u8 preempted;
>> } st;
>> };
>>
>> diff --git a/arch/loongarch/include/asm/kvm_para.h b/arch/loongarch/include/asm/kvm_para.h
>> index 3e4b397f423f..d8592a7f5922 100644
>> --- a/arch/loongarch/include/asm/kvm_para.h
>> +++ b/arch/loongarch/include/asm/kvm_para.h
>> @@ -37,8 +37,11 @@ struct kvm_steal_time {
>> __u64 steal;
>> __u32 version;
>> __u32 flags;
>> - __u32 pad[12];
>> + __u8 preempted;
>> + __u8 u8_pad[3];
>> + __u32 pad[11];
> Maybe a single __u8 pad[47] is enough?
yes, pad[47] seems better unless there is definitely __u32 type
requirement in future.
Will do in next version.
>
>> };
>> +#define KVM_VCPU_PREEMPTED (1 << 0)
>>
>> /*
>> * Hypercall interface for KVM hypervisor
>> diff --git a/arch/loongarch/include/uapi/asm/kvm.h b/arch/loongarch/include/uapi/asm/kvm.h
>> index 57ba1a563bb1..bca7154aa651 100644
>> --- a/arch/loongarch/include/uapi/asm/kvm.h
>> +++ b/arch/loongarch/include/uapi/asm/kvm.h
>> @@ -104,6 +104,7 @@ struct kvm_fpu {
>> #define KVM_LOONGARCH_VM_FEAT_PV_IPI 6
>> #define KVM_LOONGARCH_VM_FEAT_PV_STEALTIME 7
>> #define KVM_LOONGARCH_VM_FEAT_PTW 8
>> +#define KVM_LOONGARCH_VM_FEAT_PV_PREEMPT_HINT 10
> From the name it is a "hint", from include/linux/kvm_para.h we know
> features and hints are different. If preempt is really a feature,
> rename it?
It is a feature. yes, in generic hint is suggestion for VM and VM can
selectively do or not.
Will rename it with KVM_LOONGARCH_VM_FEAT_PV_PREEMPT.
>
>>
>> /* Device Control API on vcpu fd */
>> #define KVM_LOONGARCH_VCPU_CPUCFG 0
>> diff --git a/arch/loongarch/include/uapi/asm/kvm_para.h b/arch/loongarch/include/uapi/asm/kvm_para.h
>> index 76d802ef01ce..fe4107869ce6 100644
>> --- a/arch/loongarch/include/uapi/asm/kvm_para.h
>> +++ b/arch/loongarch/include/uapi/asm/kvm_para.h
>> @@ -15,6 +15,7 @@
>> #define CPUCFG_KVM_FEATURE (CPUCFG_KVM_BASE + 4)
>> #define KVM_FEATURE_IPI 1
>> #define KVM_FEATURE_STEAL_TIME 2
>> +#define KVM_FEATURE_PREEMPT_HINT 3
>> /* BIT 24 - 31 are features configurable by user space vmm */
>> #define KVM_FEATURE_VIRT_EXTIOI 24
>> #define KVM_FEATURE_USER_HCALL 25
>> diff --git a/arch/loongarch/kvm/vcpu.c b/arch/loongarch/kvm/vcpu.c
>> index 1245a6b35896..33a94b191b5d 100644
>> --- a/arch/loongarch/kvm/vcpu.c
>> +++ b/arch/loongarch/kvm/vcpu.c
>> @@ -180,6 +180,11 @@ static void kvm_update_stolen_time(struct kvm_vcpu *vcpu)
>> }
>>
>> st = (struct kvm_steal_time __user *)ghc->hva;
>> + if (kvm_guest_has_pv_feature(vcpu, KVM_FEATURE_PREEMPT_HINT)) {
>> + unsafe_put_user(0, &st->preempted, out);
>> + vcpu->arch.st.preempted = 0;
>> + }
>> +
>> unsafe_get_user(version, &st->version, out);
>> if (version & 1)
>> version += 1; /* first time write, random junk */
>> @@ -1757,11 +1762,58 @@ static int _kvm_vcpu_put(struct kvm_vcpu *vcpu, int cpu)
>> return 0;
>> }
>>
>> +static void _kvm_set_vcpu_preempted(struct kvm_vcpu *vcpu)
> Just using kvm_set_vcpu_preempted() is enough, no "_".
>
>> +{
>> + struct gfn_to_hva_cache *ghc;
>> + struct kvm_steal_time __user *st;
>> + struct kvm_memslots *slots;
>> + static const u8 preempted = KVM_VCPU_PREEMPTED;
> I'm not sure whether "static" is right, it's not reentrant.
I think static is better here, it saves one cycle with assignment here.
Regards
Bibo Mao
>
>
> Huacai
>
>> + gpa_t gpa;
>> +
>> + gpa = vcpu->arch.st.guest_addr;
>> + if (!(gpa & KVM_STEAL_PHYS_VALID))
>> + return;
>> +
>> + /* vCPU may be preempted for many times */
>> + if (vcpu->arch.st.preempted)
>> + return;
>> +
>> + /* This happens on process exit */
>> + if (unlikely(current->mm != vcpu->kvm->mm))
>> + return;
>> +
>> + gpa &= KVM_STEAL_PHYS_MASK;
>> + ghc = &vcpu->arch.st.cache;
>> + slots = kvm_memslots(vcpu->kvm);
>> + if (slots->generation != ghc->generation || gpa != ghc->gpa) {
>> + if (kvm_gfn_to_hva_cache_init(vcpu->kvm, ghc, gpa, sizeof(*st))) {
>> + ghc->gpa = INVALID_GPA;
>> + return;
>> + }
>> + }
>> +
>> + st = (struct kvm_steal_time __user *)ghc->hva;
>> + unsafe_put_user(preempted, &st->preempted, out);
>> + vcpu->arch.st.preempted = KVM_VCPU_PREEMPTED;
>> +out:
>> + mark_page_dirty_in_slot(vcpu->kvm, ghc->memslot, gpa_to_gfn(ghc->gpa));
>> +}
>> +
>> void kvm_arch_vcpu_put(struct kvm_vcpu *vcpu)
>> {
>> - int cpu;
>> + int cpu, idx;
>> unsigned long flags;
>>
>> + if (vcpu->preempted && kvm_guest_has_pv_feature(vcpu, KVM_FEATURE_PREEMPT_HINT)) {
>> + /*
>> + * Take the srcu lock as memslots will be accessed to check the gfn
>> + * cache generation against the memslots generation.
>> + */
>> + idx = srcu_read_lock(&vcpu->kvm->srcu);
>> + _kvm_set_vcpu_preempted(vcpu);
>> + srcu_read_unlock(&vcpu->kvm->srcu, idx);
>> + }
>> +
>> local_irq_save(flags);
>> cpu = smp_processor_id();
>> vcpu->arch.last_sched_cpu = cpu;
>> diff --git a/arch/loongarch/kvm/vm.c b/arch/loongarch/kvm/vm.c
>> index a49b1c1a3dd1..b8879110a0a1 100644
>> --- a/arch/loongarch/kvm/vm.c
>> +++ b/arch/loongarch/kvm/vm.c
>> @@ -45,8 +45,10 @@ int kvm_arch_init_vm(struct kvm *kvm, unsigned long type)
>>
>> /* Enable all PV features by default */
>> kvm->arch.pv_features = BIT(KVM_FEATURE_IPI);
>> - if (kvm_pvtime_supported())
>> + if (kvm_pvtime_supported()) {
>> kvm->arch.pv_features |= BIT(KVM_FEATURE_STEAL_TIME);
>> + kvm->arch.pv_features |= BIT(KVM_FEATURE_PREEMPT_HINT);
>> + }
>>
>> /*
>> * cpu_vabits means user address space only (a half of total).
>> @@ -143,6 +145,7 @@ static int kvm_vm_feature_has_attr(struct kvm *kvm, struct kvm_device_attr *attr
>> case KVM_LOONGARCH_VM_FEAT_PV_IPI:
>> return 0;
>> case KVM_LOONGARCH_VM_FEAT_PV_STEALTIME:
>> + case KVM_LOONGARCH_VM_FEAT_PV_PREEMPT_HINT:
>> if (kvm_pvtime_supported())
>> return 0;
>> return -ENXIO;
>> --
>> 2.39.3
>>
>>
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 2/3] LoongArch: Add paravirt support with vcpu_is_preempted()
2025-11-18 12:48 ` Huacai Chen
@ 2025-11-19 1:59 ` Bibo Mao
2025-11-19 2:58 ` Huacai Chen
2025-11-19 6:09 ` Bibo Mao
2025-11-19 2:50 ` Bibo Mao
1 sibling, 2 replies; 18+ messages in thread
From: Bibo Mao @ 2025-11-19 1:59 UTC (permalink / raw)
To: Huacai Chen
Cc: Paolo Bonzini, WANG Xuerui, Peter Zijlstra, Ingo Molnar,
Will Deacon, Boqun Feng, Waiman Long, Juergen Gross, Ajay Kaher,
Alexey Makhalov, Broadcom internal kernel review list, kvm,
loongarch, linux-kernel, virtualization, x86
On 2025/11/18 下午8:48, Huacai Chen wrote:
> Hi, Bibo,
>
> On Tue, Nov 18, 2025 at 4:07 PM Bibo Mao <maobibo@loongson.cn> wrote:
>>
>> Function vcpu_is_preempted() is used to check whether vCPU is preempted
>> or not. Here add implementation with vcpu_is_preempted() when option
>> CONFIG_PARAVIRT is enabled.
>>
>> Signed-off-by: Bibo Mao <maobibo@loongson.cn>
>> ---
>> arch/loongarch/include/asm/smp.h | 1 +
>> arch/loongarch/include/asm/spinlock.h | 5 +++++
>> arch/loongarch/kernel/paravirt.c | 16 ++++++++++++++++
>> arch/loongarch/kernel/smp.c | 6 ++++++
>> 4 files changed, 28 insertions(+)
>>
>> diff --git a/arch/loongarch/include/asm/smp.h b/arch/loongarch/include/asm/smp.h
>> index 3a47f52959a8..5b37f7bf2060 100644
>> --- a/arch/loongarch/include/asm/smp.h
>> +++ b/arch/loongarch/include/asm/smp.h
>> @@ -18,6 +18,7 @@ struct smp_ops {
>> void (*init_ipi)(void);
>> void (*send_ipi_single)(int cpu, unsigned int action);
>> void (*send_ipi_mask)(const struct cpumask *mask, unsigned int action);
>> + bool (*vcpu_is_preempted)(int cpu);
>> };
>> extern struct smp_ops mp_ops;
>>
>> diff --git a/arch/loongarch/include/asm/spinlock.h b/arch/loongarch/include/asm/spinlock.h
>> index 7cb3476999be..c001cef893aa 100644
>> --- a/arch/loongarch/include/asm/spinlock.h
>> +++ b/arch/loongarch/include/asm/spinlock.h
>> @@ -5,6 +5,11 @@
>> #ifndef _ASM_SPINLOCK_H
>> #define _ASM_SPINLOCK_H
>>
>> +#ifdef CONFIG_PARAVIRT
>> +#define vcpu_is_preempted vcpu_is_preempted
>> +bool vcpu_is_preempted(int cpu);
>> +#endif
> Maybe paravirt.h is a better place?
It is actually a little strange to add macro CONFIG_PARAVIRT in file
asm/spinlock.h
vcpu_is_preempted is originally defined in header file
include/linux/sched.h like this
#ifndef vcpu_is_preempted
static inline bool vcpu_is_preempted(int cpu)
{
return false;
}
#endif
that requires that header file is included before sched.h, file
asm/spinlock.h can meet this requirement, however header file paravirt.h
maybe it is not included before sched.h in generic.
Here vcpu_is_preempted definition is added before the following including.
#include <asm/processor.h>
#include <asm/qspinlock.h>
#include <asm/qrwlock.h>
Maybe it is better to be added after the above header files including
sentences, but need further investigation.
>
>> +
>> #include <asm/processor.h>
>> #include <asm/qspinlock.h>
>> #include <asm/qrwlock.h>
>> diff --git a/arch/loongarch/kernel/paravirt.c b/arch/loongarch/kernel/paravirt.c
>> index b1b51f920b23..b99404b6b13f 100644
>> --- a/arch/loongarch/kernel/paravirt.c
>> +++ b/arch/loongarch/kernel/paravirt.c
>> @@ -52,6 +52,13 @@ static u64 paravt_steal_clock(int cpu)
>> #ifdef CONFIG_SMP
>> static struct smp_ops native_ops;
>>
>> +static bool pv_vcpu_is_preempted(int cpu)
>> +{
>> + struct kvm_steal_time *src = &per_cpu(steal_time, cpu);
>> +
>> + return !!(src->preempted & KVM_VCPU_PREEMPTED);
>> +}
>> +
>> static void pv_send_ipi_single(int cpu, unsigned int action)
>> {
>> int min, old;
>> @@ -308,6 +315,9 @@ int __init pv_time_init(void)
>> pr_err("Failed to install cpu hotplug callbacks\n");
>> return r;
>> }
>> +
>> + if (kvm_para_has_feature(KVM_FEATURE_PREEMPT_HINT))
>> + mp_ops.vcpu_is_preempted = pv_vcpu_is_preempted;
>> #endif
>>
>> static_call_update(pv_steal_clock, paravt_steal_clock);
>> @@ -332,3 +342,9 @@ int __init pv_spinlock_init(void)
>>
>> return 0;
>> }
>> +
>> +bool notrace vcpu_is_preempted(int cpu)
>> +{
>> + return mp_ops.vcpu_is_preempted(cpu);
>> +}
>
> We can simplify the whole patch like this, then we don't need to touch
> smp.c, and we can merge Patch-2/3.
>
> +bool notrace vcpu_is_preempted(int cpu)
> +{
> + if (!kvm_para_has_feature(KVM_FEATURE_PREEMPT_HINT))
> + return false;
> + else {
> + struct kvm_steal_time *src = &per_cpu(steal_time, cpu);
> + return !!(src->preempted & KVM_VCPU_PREEMPTED);
> + }
> +}
1. there is assembly output about relative vcpu_is_preempted
<loongson_vcpu_is_preempted>:
move $r4,$r0
jirl $r0,$r1,0
<pv_vcpu_is_preempted>:
pcalau12i $r13,8759(0x2237)
slli.d $r4,$r4,0x3
addi.d $r13,$r13,-1000(0xc18)
ldx.d $r13,$r13,$r4
pcalau12i $r12,5462(0x1556)
addi.d $r12,$r12,384(0x180)
add.d $r12,$r13,$r12
ld.bu $r4,$r12,16(0x10)
andi $r4,$r4,0x1
jirl $r0,$r1,0
<vcpu_is_preempted>:
pcalau12i $r12,8775(0x2247)
ld.d $r12,$r12,-472(0xe28)
jirl $r0,$r12,0
andi $r0,$r0,0x0
<vcpu_is_preempted_new>:
pcalau12i $r12,8151(0x1fd7)
ld.d $r12,$r12,-1008(0xc10)
bstrpick.d $r12,$r12,0x1a,0x1a
beqz $r12,188(0xbc) # 900000000024ec60
pcalau12i $r12,11802(0x2e1a)
addi.d $r12,$r12,-1400(0xa88)
ldptr.w $r14,$r12,36(0x24)
beqz $r14,108(0x6c) # 900000000024ec20
addi.w $r13,$r0,1(0x1)
bne $r14,$r13,164(0xa4) # 900000000024ec60
ldptr.w $r13,$r12,40(0x28)
bnez $r13,24(0x18) # 900000000024ebdc
lu12i.w $r14,262144(0x40000)
ori $r14,$r14,0x4
cpucfg $r14,$r14
slli.w $r13,$r14,0x0
st.w $r14,$r12,40(0x28)
bstrpick.d $r13,$r13,0x3,0x3
beqz $r13,128(0x80) # 900000000024ec60
pcalau12i $r13,8759(0x2237)
slli.d $r4,$r4,0x3
addi.d $r13,$r13,-1000(0xc18)
ldx.d $r13,$r13,$r4
pcalau12i $r12,5462(0x1556)
addi.d $r12,$r12,384(0x180)
add.d $r12,$r13,$r12
ld.bu $r4,$r12,16(0x10)
andi $r4,$r4,0x1
jirl $r0,$r1,0
andi $r0,$r0,0x0
andi $r0,$r0,0x0
andi $r0,$r0,0x0
andi $r0,$r0,0x0
andi $r0,$r0,0x0
lu12i.w $r13,262144(0x40000)
cpucfg $r13,$r13
lu12i.w $r15,1237(0x4d5)
ori $r15,$r15,0x64b
slli.w $r13,$r13,0x0
bne $r13,$r15,-124(0x3ff84) # 900000000024ebb8
addi.w $r13,$r0,1(0x1)
st.w $r13,$r12,36(0x24)
b -128(0xfffff80) # 900000000024ebc0
andi $r0,$r0,0x0
andi $r0,$r0,0x0
andi $r0,$r0,0x0
andi $r0,$r0,0x0
andi $r0,$r0,0x0
andi $r0,$r0,0x0
andi $r0,$r0,0x0
move $r4,$r0
jirl $r0,$r1,0
With vcpu_is_preempted(), there is one memory load and one jirl jump,
with vcpu_is_preempted_new(), there is two memory load and two beq
compare instructions.
2. In some scenery such nr_cpus == 1, loongson_vcpu_is_preempted() is
better than pv_vcpu_is_preempted() even if the preempt feature is enabled.
Regards
Bibo Mao
> Huacai
>
>> +EXPORT_SYMBOL(vcpu_is_preempted);
>> diff --git a/arch/loongarch/kernel/smp.c b/arch/loongarch/kernel/smp.c
>> index 46036d98da75..f04192fedf8d 100644
>> --- a/arch/loongarch/kernel/smp.c
>> +++ b/arch/loongarch/kernel/smp.c
>> @@ -307,10 +307,16 @@ static void loongson_init_ipi(void)
>> panic("IPI IRQ request failed\n");
>> }
>>
>> +static bool loongson_vcpu_is_preempted(int cpu)
>> +{
>> + return false;
>> +}
>> +
>> struct smp_ops mp_ops = {
>> .init_ipi = loongson_init_ipi,
>> .send_ipi_single = loongson_send_ipi_single,
>> .send_ipi_mask = loongson_send_ipi_mask,
>> + .vcpu_is_preempted = loongson_vcpu_is_preempted,
>> };
>>
>> static void __init fdt_smp_setup(void)
>> --
>> 2.39.3
>>
>>
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 1/3] LoongArch: KVM: Add preempt hint feature in hypervisor side
2025-11-19 1:20 ` Bibo Mao
@ 2025-11-19 2:45 ` Huacai Chen
2025-11-19 2:55 ` Bibo Mao
0 siblings, 1 reply; 18+ messages in thread
From: Huacai Chen @ 2025-11-19 2:45 UTC (permalink / raw)
To: Bibo Mao
Cc: Paolo Bonzini, Tianrui Zhao, WANG Xuerui, kvm, loongarch,
linux-kernel
On Wed, Nov 19, 2025 at 9:23 AM Bibo Mao <maobibo@loongson.cn> wrote:
>
>
>
> On 2025/11/18 下午8:46, Huacai Chen wrote:
> > Hi, Bibo,
> >
> > On Tue, Nov 18, 2025 at 4:07 PM Bibo Mao <maobibo@loongson.cn> wrote:
> >>
> >> Feature KVM_FEATURE_PREEMPT_HINT is added to show whether vCPU is
> >> preempted or not. It is to help guest OS scheduling or lock checking
> >> etc. Here add KVM_FEATURE_PREEMPT_HINT feature and use one byte as
> >> preempted flag in steal time structure.
> >>
> >> Signed-off-by: Bibo Mao <maobibo@loongson.cn>
> >> ---
> >> arch/loongarch/include/asm/kvm_host.h | 2 +
> >> arch/loongarch/include/asm/kvm_para.h | 5 +-
> >> arch/loongarch/include/uapi/asm/kvm.h | 1 +
> >> arch/loongarch/include/uapi/asm/kvm_para.h | 1 +
> >> arch/loongarch/kvm/vcpu.c | 54 +++++++++++++++++++++-
> >> arch/loongarch/kvm/vm.c | 5 +-
> >> 6 files changed, 65 insertions(+), 3 deletions(-)
> >>
> >> diff --git a/arch/loongarch/include/asm/kvm_host.h b/arch/loongarch/include/asm/kvm_host.h
> >> index 0cecbd038bb3..04c6dd171877 100644
> >> --- a/arch/loongarch/include/asm/kvm_host.h
> >> +++ b/arch/loongarch/include/asm/kvm_host.h
> >> @@ -163,6 +163,7 @@ enum emulation_result {
> >> #define LOONGARCH_PV_FEAT_UPDATED BIT_ULL(63)
> >> #define LOONGARCH_PV_FEAT_MASK (BIT(KVM_FEATURE_IPI) | \
> >> BIT(KVM_FEATURE_STEAL_TIME) | \
> >> + BIT(KVM_FEATURE_PREEMPT_HINT) |\
> >> BIT(KVM_FEATURE_USER_HCALL) | \
> >> BIT(KVM_FEATURE_VIRT_EXTIOI))
> >>
> >> @@ -250,6 +251,7 @@ struct kvm_vcpu_arch {
> >> u64 guest_addr;
> >> u64 last_steal;
> >> struct gfn_to_hva_cache cache;
> >> + u8 preempted;
> >> } st;
> >> };
> >>
> >> diff --git a/arch/loongarch/include/asm/kvm_para.h b/arch/loongarch/include/asm/kvm_para.h
> >> index 3e4b397f423f..d8592a7f5922 100644
> >> --- a/arch/loongarch/include/asm/kvm_para.h
> >> +++ b/arch/loongarch/include/asm/kvm_para.h
> >> @@ -37,8 +37,11 @@ struct kvm_steal_time {
> >> __u64 steal;
> >> __u32 version;
> >> __u32 flags;
> >> - __u32 pad[12];
> >> + __u8 preempted;
> >> + __u8 u8_pad[3];
> >> + __u32 pad[11];
> > Maybe a single __u8 pad[47] is enough?
> yes, pad[47] seems better unless there is definitely __u32 type
> requirement in future.
>
> Will do in next version.
> >
> >> };
> >> +#define KVM_VCPU_PREEMPTED (1 << 0)
> >>
> >> /*
> >> * Hypercall interface for KVM hypervisor
> >> diff --git a/arch/loongarch/include/uapi/asm/kvm.h b/arch/loongarch/include/uapi/asm/kvm.h
> >> index 57ba1a563bb1..bca7154aa651 100644
> >> --- a/arch/loongarch/include/uapi/asm/kvm.h
> >> +++ b/arch/loongarch/include/uapi/asm/kvm.h
> >> @@ -104,6 +104,7 @@ struct kvm_fpu {
> >> #define KVM_LOONGARCH_VM_FEAT_PV_IPI 6
> >> #define KVM_LOONGARCH_VM_FEAT_PV_STEALTIME 7
> >> #define KVM_LOONGARCH_VM_FEAT_PTW 8
> >> +#define KVM_LOONGARCH_VM_FEAT_PV_PREEMPT_HINT 10
> > From the name it is a "hint", from include/linux/kvm_para.h we know
> > features and hints are different. If preempt is really a feature,
> > rename it?
> It is a feature. yes, in generic hint is suggestion for VM and VM can
> selectively do or not.
>
> Will rename it with KVM_LOONGARCH_VM_FEAT_PV_PREEMPT.
> >
> >>
> >> /* Device Control API on vcpu fd */
> >> #define KVM_LOONGARCH_VCPU_CPUCFG 0
> >> diff --git a/arch/loongarch/include/uapi/asm/kvm_para.h b/arch/loongarch/include/uapi/asm/kvm_para.h
> >> index 76d802ef01ce..fe4107869ce6 100644
> >> --- a/arch/loongarch/include/uapi/asm/kvm_para.h
> >> +++ b/arch/loongarch/include/uapi/asm/kvm_para.h
> >> @@ -15,6 +15,7 @@
> >> #define CPUCFG_KVM_FEATURE (CPUCFG_KVM_BASE + 4)
> >> #define KVM_FEATURE_IPI 1
> >> #define KVM_FEATURE_STEAL_TIME 2
> >> +#define KVM_FEATURE_PREEMPT_HINT 3
> >> /* BIT 24 - 31 are features configurable by user space vmm */
> >> #define KVM_FEATURE_VIRT_EXTIOI 24
> >> #define KVM_FEATURE_USER_HCALL 25
> >> diff --git a/arch/loongarch/kvm/vcpu.c b/arch/loongarch/kvm/vcpu.c
> >> index 1245a6b35896..33a94b191b5d 100644
> >> --- a/arch/loongarch/kvm/vcpu.c
> >> +++ b/arch/loongarch/kvm/vcpu.c
> >> @@ -180,6 +180,11 @@ static void kvm_update_stolen_time(struct kvm_vcpu *vcpu)
> >> }
> >>
> >> st = (struct kvm_steal_time __user *)ghc->hva;
> >> + if (kvm_guest_has_pv_feature(vcpu, KVM_FEATURE_PREEMPT_HINT)) {
> >> + unsafe_put_user(0, &st->preempted, out);
> >> + vcpu->arch.st.preempted = 0;
> >> + }
> >> +
> >> unsafe_get_user(version, &st->version, out);
> >> if (version & 1)
> >> version += 1; /* first time write, random junk */
> >> @@ -1757,11 +1762,58 @@ static int _kvm_vcpu_put(struct kvm_vcpu *vcpu, int cpu)
> >> return 0;
> >> }
> >>
> >> +static void _kvm_set_vcpu_preempted(struct kvm_vcpu *vcpu)
> > Just using kvm_set_vcpu_preempted() is enough, no "_".
> >
> >> +{
> >> + struct gfn_to_hva_cache *ghc;
> >> + struct kvm_steal_time __user *st;
> >> + struct kvm_memslots *slots;
> >> + static const u8 preempted = KVM_VCPU_PREEMPTED;
> > I'm not sure whether "static" is right, it's not reentrant.
> I think static is better here, it saves one cycle with assignment here.
I know, but I want to know whether the logic is correct.
vcpu->arch.st.preempted is per-cpu, but the local variable "preempted"
can be used across multiple VCPU? I'm not sure.
Huacai
>
> Regards
> Bibo Mao
> >
> >
> > Huacai
> >
> >> + gpa_t gpa;
> >> +
> >> + gpa = vcpu->arch.st.guest_addr;
> >> + if (!(gpa & KVM_STEAL_PHYS_VALID))
> >> + return;
> >> +
> >> + /* vCPU may be preempted for many times */
> >> + if (vcpu->arch.st.preempted)
> >> + return;
> >> +
> >> + /* This happens on process exit */
> >> + if (unlikely(current->mm != vcpu->kvm->mm))
> >> + return;
> >> +
> >> + gpa &= KVM_STEAL_PHYS_MASK;
> >> + ghc = &vcpu->arch.st.cache;
> >> + slots = kvm_memslots(vcpu->kvm);
> >> + if (slots->generation != ghc->generation || gpa != ghc->gpa) {
> >> + if (kvm_gfn_to_hva_cache_init(vcpu->kvm, ghc, gpa, sizeof(*st))) {
> >> + ghc->gpa = INVALID_GPA;
> >> + return;
> >> + }
> >> + }
> >> +
> >> + st = (struct kvm_steal_time __user *)ghc->hva;
> >> + unsafe_put_user(preempted, &st->preempted, out);
> >> + vcpu->arch.st.preempted = KVM_VCPU_PREEMPTED;
> >> +out:
> >> + mark_page_dirty_in_slot(vcpu->kvm, ghc->memslot, gpa_to_gfn(ghc->gpa));
> >> +}
> >> +
> >> void kvm_arch_vcpu_put(struct kvm_vcpu *vcpu)
> >> {
> >> - int cpu;
> >> + int cpu, idx;
> >> unsigned long flags;
> >>
> >> + if (vcpu->preempted && kvm_guest_has_pv_feature(vcpu, KVM_FEATURE_PREEMPT_HINT)) {
> >> + /*
> >> + * Take the srcu lock as memslots will be accessed to check the gfn
> >> + * cache generation against the memslots generation.
> >> + */
> >> + idx = srcu_read_lock(&vcpu->kvm->srcu);
> >> + _kvm_set_vcpu_preempted(vcpu);
> >> + srcu_read_unlock(&vcpu->kvm->srcu, idx);
> >> + }
> >> +
> >> local_irq_save(flags);
> >> cpu = smp_processor_id();
> >> vcpu->arch.last_sched_cpu = cpu;
> >> diff --git a/arch/loongarch/kvm/vm.c b/arch/loongarch/kvm/vm.c
> >> index a49b1c1a3dd1..b8879110a0a1 100644
> >> --- a/arch/loongarch/kvm/vm.c
> >> +++ b/arch/loongarch/kvm/vm.c
> >> @@ -45,8 +45,10 @@ int kvm_arch_init_vm(struct kvm *kvm, unsigned long type)
> >>
> >> /* Enable all PV features by default */
> >> kvm->arch.pv_features = BIT(KVM_FEATURE_IPI);
> >> - if (kvm_pvtime_supported())
> >> + if (kvm_pvtime_supported()) {
> >> kvm->arch.pv_features |= BIT(KVM_FEATURE_STEAL_TIME);
> >> + kvm->arch.pv_features |= BIT(KVM_FEATURE_PREEMPT_HINT);
> >> + }
> >>
> >> /*
> >> * cpu_vabits means user address space only (a half of total).
> >> @@ -143,6 +145,7 @@ static int kvm_vm_feature_has_attr(struct kvm *kvm, struct kvm_device_attr *attr
> >> case KVM_LOONGARCH_VM_FEAT_PV_IPI:
> >> return 0;
> >> case KVM_LOONGARCH_VM_FEAT_PV_STEALTIME:
> >> + case KVM_LOONGARCH_VM_FEAT_PV_PREEMPT_HINT:
> >> if (kvm_pvtime_supported())
> >> return 0;
> >> return -ENXIO;
> >> --
> >> 2.39.3
> >>
> >>
>
>
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 2/3] LoongArch: Add paravirt support with vcpu_is_preempted()
2025-11-18 12:48 ` Huacai Chen
2025-11-19 1:59 ` Bibo Mao
@ 2025-11-19 2:50 ` Bibo Mao
2025-11-19 7:36 ` Huacai Chen
1 sibling, 1 reply; 18+ messages in thread
From: Bibo Mao @ 2025-11-19 2:50 UTC (permalink / raw)
To: Huacai Chen
Cc: Paolo Bonzini, WANG Xuerui, Peter Zijlstra, Ingo Molnar,
Will Deacon, Boqun Feng, Waiman Long, Juergen Gross, Ajay Kaher,
Alexey Makhalov, Broadcom internal kernel review list, kvm,
loongarch, linux-kernel, virtualization, x86
On 2025/11/18 下午8:48, Huacai Chen wrote:
> Hi, Bibo,
>
> On Tue, Nov 18, 2025 at 4:07 PM Bibo Mao <maobibo@loongson.cn> wrote:
>>
>> Function vcpu_is_preempted() is used to check whether vCPU is preempted
>> or not. Here add implementation with vcpu_is_preempted() when option
>> CONFIG_PARAVIRT is enabled.
>>
>> Signed-off-by: Bibo Mao <maobibo@loongson.cn>
>> ---
>> arch/loongarch/include/asm/smp.h | 1 +
>> arch/loongarch/include/asm/spinlock.h | 5 +++++
>> arch/loongarch/kernel/paravirt.c | 16 ++++++++++++++++
>> arch/loongarch/kernel/smp.c | 6 ++++++
>> 4 files changed, 28 insertions(+)
>>
>> diff --git a/arch/loongarch/include/asm/smp.h b/arch/loongarch/include/asm/smp.h
>> index 3a47f52959a8..5b37f7bf2060 100644
>> --- a/arch/loongarch/include/asm/smp.h
>> +++ b/arch/loongarch/include/asm/smp.h
>> @@ -18,6 +18,7 @@ struct smp_ops {
>> void (*init_ipi)(void);
>> void (*send_ipi_single)(int cpu, unsigned int action);
>> void (*send_ipi_mask)(const struct cpumask *mask, unsigned int action);
>> + bool (*vcpu_is_preempted)(int cpu);
>> };
>> extern struct smp_ops mp_ops;
>>
>> diff --git a/arch/loongarch/include/asm/spinlock.h b/arch/loongarch/include/asm/spinlock.h
>> index 7cb3476999be..c001cef893aa 100644
>> --- a/arch/loongarch/include/asm/spinlock.h
>> +++ b/arch/loongarch/include/asm/spinlock.h
>> @@ -5,6 +5,11 @@
>> #ifndef _ASM_SPINLOCK_H
>> #define _ASM_SPINLOCK_H
>>
>> +#ifdef CONFIG_PARAVIRT
>> +#define vcpu_is_preempted vcpu_is_preempted
>> +bool vcpu_is_preempted(int cpu);
>> +#endif
> Maybe paravirt.h is a better place?
how about put it in asm/qspinlock.h since it is included by header file
asm/spinlock.h already?
>
>> +
>> #include <asm/processor.h>
>> #include <asm/qspinlock.h>
>> #include <asm/qrwlock.h>
>> diff --git a/arch/loongarch/kernel/paravirt.c b/arch/loongarch/kernel/paravirt.c
>> index b1b51f920b23..b99404b6b13f 100644
>> --- a/arch/loongarch/kernel/paravirt.c
>> +++ b/arch/loongarch/kernel/paravirt.c
>> @@ -52,6 +52,13 @@ static u64 paravt_steal_clock(int cpu)
>> #ifdef CONFIG_SMP
>> static struct smp_ops native_ops;
>>
>> +static bool pv_vcpu_is_preempted(int cpu)
>> +{
>> + struct kvm_steal_time *src = &per_cpu(steal_time, cpu);
>> +
>> + return !!(src->preempted & KVM_VCPU_PREEMPTED);
>> +}
>> +
>> static void pv_send_ipi_single(int cpu, unsigned int action)
>> {
>> int min, old;
>> @@ -308,6 +315,9 @@ int __init pv_time_init(void)
>> pr_err("Failed to install cpu hotplug callbacks\n");
>> return r;
>> }
>> +
>> + if (kvm_para_has_feature(KVM_FEATURE_PREEMPT_HINT))
>> + mp_ops.vcpu_is_preempted = pv_vcpu_is_preempted;
>> #endif
>>
>> static_call_update(pv_steal_clock, paravt_steal_clock);
>> @@ -332,3 +342,9 @@ int __init pv_spinlock_init(void)
>>
>> return 0;
>> }
>> +
>> +bool notrace vcpu_is_preempted(int cpu)
>> +{
>> + return mp_ops.vcpu_is_preempted(cpu);
>> +}
>
> We can simplify the whole patch like this, then we don't need to touch
> smp.c, and we can merge Patch-2/3.
>
> +bool notrace vcpu_is_preempted(int cpu)
> +{
> + if (!kvm_para_has_feature(KVM_FEATURE_PREEMPT_HINT))
> + return false;
> + else {
> + struct kvm_steal_time *src = &per_cpu(steal_time, cpu);
> + return !!(src->preempted & KVM_VCPU_PREEMPTED);
> + }
> +}
> Huacai
>
>> +EXPORT_SYMBOL(vcpu_is_preempted);
>> diff --git a/arch/loongarch/kernel/smp.c b/arch/loongarch/kernel/smp.c
>> index 46036d98da75..f04192fedf8d 100644
>> --- a/arch/loongarch/kernel/smp.c
>> +++ b/arch/loongarch/kernel/smp.c
>> @@ -307,10 +307,16 @@ static void loongson_init_ipi(void)
>> panic("IPI IRQ request failed\n");
>> }
>>
>> +static bool loongson_vcpu_is_preempted(int cpu)
>> +{
>> + return false;
>> +}
>> +
>> struct smp_ops mp_ops = {
>> .init_ipi = loongson_init_ipi,
>> .send_ipi_single = loongson_send_ipi_single,
>> .send_ipi_mask = loongson_send_ipi_mask,
>> + .vcpu_is_preempted = loongson_vcpu_is_preempted,
>> };
>>
>> static void __init fdt_smp_setup(void)
>> --
>> 2.39.3
>>
>>
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 1/3] LoongArch: KVM: Add preempt hint feature in hypervisor side
2025-11-19 2:45 ` Huacai Chen
@ 2025-11-19 2:55 ` Bibo Mao
2025-11-19 3:01 ` Huacai Chen
0 siblings, 1 reply; 18+ messages in thread
From: Bibo Mao @ 2025-11-19 2:55 UTC (permalink / raw)
To: Huacai Chen
Cc: Paolo Bonzini, Tianrui Zhao, WANG Xuerui, kvm, loongarch,
linux-kernel
On 2025/11/19 上午10:45, Huacai Chen wrote:
> On Wed, Nov 19, 2025 at 9:23 AM Bibo Mao <maobibo@loongson.cn> wrote:
>>
>>
>>
>> On 2025/11/18 下午8:46, Huacai Chen wrote:
>>> Hi, Bibo,
>>>
>>> On Tue, Nov 18, 2025 at 4:07 PM Bibo Mao <maobibo@loongson.cn> wrote:
>>>>
>>>> Feature KVM_FEATURE_PREEMPT_HINT is added to show whether vCPU is
>>>> preempted or not. It is to help guest OS scheduling or lock checking
>>>> etc. Here add KVM_FEATURE_PREEMPT_HINT feature and use one byte as
>>>> preempted flag in steal time structure.
>>>>
>>>> Signed-off-by: Bibo Mao <maobibo@loongson.cn>
>>>> ---
>>>> arch/loongarch/include/asm/kvm_host.h | 2 +
>>>> arch/loongarch/include/asm/kvm_para.h | 5 +-
>>>> arch/loongarch/include/uapi/asm/kvm.h | 1 +
>>>> arch/loongarch/include/uapi/asm/kvm_para.h | 1 +
>>>> arch/loongarch/kvm/vcpu.c | 54 +++++++++++++++++++++-
>>>> arch/loongarch/kvm/vm.c | 5 +-
>>>> 6 files changed, 65 insertions(+), 3 deletions(-)
>>>>
>>>> diff --git a/arch/loongarch/include/asm/kvm_host.h b/arch/loongarch/include/asm/kvm_host.h
>>>> index 0cecbd038bb3..04c6dd171877 100644
>>>> --- a/arch/loongarch/include/asm/kvm_host.h
>>>> +++ b/arch/loongarch/include/asm/kvm_host.h
>>>> @@ -163,6 +163,7 @@ enum emulation_result {
>>>> #define LOONGARCH_PV_FEAT_UPDATED BIT_ULL(63)
>>>> #define LOONGARCH_PV_FEAT_MASK (BIT(KVM_FEATURE_IPI) | \
>>>> BIT(KVM_FEATURE_STEAL_TIME) | \
>>>> + BIT(KVM_FEATURE_PREEMPT_HINT) |\
>>>> BIT(KVM_FEATURE_USER_HCALL) | \
>>>> BIT(KVM_FEATURE_VIRT_EXTIOI))
>>>>
>>>> @@ -250,6 +251,7 @@ struct kvm_vcpu_arch {
>>>> u64 guest_addr;
>>>> u64 last_steal;
>>>> struct gfn_to_hva_cache cache;
>>>> + u8 preempted;
>>>> } st;
>>>> };
>>>>
>>>> diff --git a/arch/loongarch/include/asm/kvm_para.h b/arch/loongarch/include/asm/kvm_para.h
>>>> index 3e4b397f423f..d8592a7f5922 100644
>>>> --- a/arch/loongarch/include/asm/kvm_para.h
>>>> +++ b/arch/loongarch/include/asm/kvm_para.h
>>>> @@ -37,8 +37,11 @@ struct kvm_steal_time {
>>>> __u64 steal;
>>>> __u32 version;
>>>> __u32 flags;
>>>> - __u32 pad[12];
>>>> + __u8 preempted;
>>>> + __u8 u8_pad[3];
>>>> + __u32 pad[11];
>>> Maybe a single __u8 pad[47] is enough?
>> yes, pad[47] seems better unless there is definitely __u32 type
>> requirement in future.
>>
>> Will do in next version.
>>>
>>>> };
>>>> +#define KVM_VCPU_PREEMPTED (1 << 0)
>>>>
>>>> /*
>>>> * Hypercall interface for KVM hypervisor
>>>> diff --git a/arch/loongarch/include/uapi/asm/kvm.h b/arch/loongarch/include/uapi/asm/kvm.h
>>>> index 57ba1a563bb1..bca7154aa651 100644
>>>> --- a/arch/loongarch/include/uapi/asm/kvm.h
>>>> +++ b/arch/loongarch/include/uapi/asm/kvm.h
>>>> @@ -104,6 +104,7 @@ struct kvm_fpu {
>>>> #define KVM_LOONGARCH_VM_FEAT_PV_IPI 6
>>>> #define KVM_LOONGARCH_VM_FEAT_PV_STEALTIME 7
>>>> #define KVM_LOONGARCH_VM_FEAT_PTW 8
>>>> +#define KVM_LOONGARCH_VM_FEAT_PV_PREEMPT_HINT 10
>>> From the name it is a "hint", from include/linux/kvm_para.h we know
>>> features and hints are different. If preempt is really a feature,
>>> rename it?
>> It is a feature. yes, in generic hint is suggestion for VM and VM can
>> selectively do or not.
>>
>> Will rename it with KVM_LOONGARCH_VM_FEAT_PV_PREEMPT.
>>>
>>>>
>>>> /* Device Control API on vcpu fd */
>>>> #define KVM_LOONGARCH_VCPU_CPUCFG 0
>>>> diff --git a/arch/loongarch/include/uapi/asm/kvm_para.h b/arch/loongarch/include/uapi/asm/kvm_para.h
>>>> index 76d802ef01ce..fe4107869ce6 100644
>>>> --- a/arch/loongarch/include/uapi/asm/kvm_para.h
>>>> +++ b/arch/loongarch/include/uapi/asm/kvm_para.h
>>>> @@ -15,6 +15,7 @@
>>>> #define CPUCFG_KVM_FEATURE (CPUCFG_KVM_BASE + 4)
>>>> #define KVM_FEATURE_IPI 1
>>>> #define KVM_FEATURE_STEAL_TIME 2
>>>> +#define KVM_FEATURE_PREEMPT_HINT 3
>>>> /* BIT 24 - 31 are features configurable by user space vmm */
>>>> #define KVM_FEATURE_VIRT_EXTIOI 24
>>>> #define KVM_FEATURE_USER_HCALL 25
>>>> diff --git a/arch/loongarch/kvm/vcpu.c b/arch/loongarch/kvm/vcpu.c
>>>> index 1245a6b35896..33a94b191b5d 100644
>>>> --- a/arch/loongarch/kvm/vcpu.c
>>>> +++ b/arch/loongarch/kvm/vcpu.c
>>>> @@ -180,6 +180,11 @@ static void kvm_update_stolen_time(struct kvm_vcpu *vcpu)
>>>> }
>>>>
>>>> st = (struct kvm_steal_time __user *)ghc->hva;
>>>> + if (kvm_guest_has_pv_feature(vcpu, KVM_FEATURE_PREEMPT_HINT)) {
>>>> + unsafe_put_user(0, &st->preempted, out);
>>>> + vcpu->arch.st.preempted = 0;
>>>> + }
>>>> +
>>>> unsafe_get_user(version, &st->version, out);
>>>> if (version & 1)
>>>> version += 1; /* first time write, random junk */
>>>> @@ -1757,11 +1762,58 @@ static int _kvm_vcpu_put(struct kvm_vcpu *vcpu, int cpu)
>>>> return 0;
>>>> }
>>>>
>>>> +static void _kvm_set_vcpu_preempted(struct kvm_vcpu *vcpu)
>>> Just using kvm_set_vcpu_preempted() is enough, no "_".
>>>
>>>> +{
>>>> + struct gfn_to_hva_cache *ghc;
>>>> + struct kvm_steal_time __user *st;
>>>> + struct kvm_memslots *slots;
>>>> + static const u8 preempted = KVM_VCPU_PREEMPTED;
>>> I'm not sure whether "static" is right, it's not reentrant.
>> I think static is better here, it saves one cycle with assignment here.
> I know, but I want to know whether the logic is correct.
> vcpu->arch.st.preempted is per-cpu, but the local variable "preempted"
> can be used across multiple VCPU? I'm not sure.
It is read-only, of course can be used by multiple vCPUs. or remove it
directly?
@@ -1767,7 +1767,6 @@ static void _kvm_set_vcpu_preempted(struct
kvm_vcpu *vcpu)
struct gfn_to_hva_cache *ghc;
struct kvm_steal_time __user *st;
struct kvm_memslots *slots;
- static const u8 preempted = KVM_VCPU_PREEMPTED;
gpa_t gpa;
gpa = vcpu->arch.st.guest_addr;
@@ -1793,7 +1792,7 @@ static void _kvm_set_vcpu_preempted(struct
kvm_vcpu *vcpu)
}
st = (struct kvm_steal_time __user *)ghc->hva;
- unsafe_put_user(preempted, &st->preempted, out);
+ unsafe_put_user(KVM_VCPU_PREEMPTED, &st->preempted, out);
vcpu->arch.st.preempted = KVM_VCPU_PREEMPTED;
>
> Huacai
>
>>
>> Regards
>> Bibo Mao
>>>
>>>
>>> Huacai
>>>
>>>> + gpa_t gpa;
>>>> +
>>>> + gpa = vcpu->arch.st.guest_addr;
>>>> + if (!(gpa & KVM_STEAL_PHYS_VALID))
>>>> + return;
>>>> +
>>>> + /* vCPU may be preempted for many times */
>>>> + if (vcpu->arch.st.preempted)
>>>> + return;
>>>> +
>>>> + /* This happens on process exit */
>>>> + if (unlikely(current->mm != vcpu->kvm->mm))
>>>> + return;
>>>> +
>>>> + gpa &= KVM_STEAL_PHYS_MASK;
>>>> + ghc = &vcpu->arch.st.cache;
>>>> + slots = kvm_memslots(vcpu->kvm);
>>>> + if (slots->generation != ghc->generation || gpa != ghc->gpa) {
>>>> + if (kvm_gfn_to_hva_cache_init(vcpu->kvm, ghc, gpa, sizeof(*st))) {
>>>> + ghc->gpa = INVALID_GPA;
>>>> + return;
>>>> + }
>>>> + }
>>>> +
>>>> + st = (struct kvm_steal_time __user *)ghc->hva;
>>>> + unsafe_put_user(preempted, &st->preempted, out);
>>>> + vcpu->arch.st.preempted = KVM_VCPU_PREEMPTED;
>>>> +out:
>>>> + mark_page_dirty_in_slot(vcpu->kvm, ghc->memslot, gpa_to_gfn(ghc->gpa));
>>>> +}
>>>> +
>>>> void kvm_arch_vcpu_put(struct kvm_vcpu *vcpu)
>>>> {
>>>> - int cpu;
>>>> + int cpu, idx;
>>>> unsigned long flags;
>>>>
>>>> + if (vcpu->preempted && kvm_guest_has_pv_feature(vcpu, KVM_FEATURE_PREEMPT_HINT)) {
>>>> + /*
>>>> + * Take the srcu lock as memslots will be accessed to check the gfn
>>>> + * cache generation against the memslots generation.
>>>> + */
>>>> + idx = srcu_read_lock(&vcpu->kvm->srcu);
>>>> + _kvm_set_vcpu_preempted(vcpu);
>>>> + srcu_read_unlock(&vcpu->kvm->srcu, idx);
>>>> + }
>>>> +
>>>> local_irq_save(flags);
>>>> cpu = smp_processor_id();
>>>> vcpu->arch.last_sched_cpu = cpu;
>>>> diff --git a/arch/loongarch/kvm/vm.c b/arch/loongarch/kvm/vm.c
>>>> index a49b1c1a3dd1..b8879110a0a1 100644
>>>> --- a/arch/loongarch/kvm/vm.c
>>>> +++ b/arch/loongarch/kvm/vm.c
>>>> @@ -45,8 +45,10 @@ int kvm_arch_init_vm(struct kvm *kvm, unsigned long type)
>>>>
>>>> /* Enable all PV features by default */
>>>> kvm->arch.pv_features = BIT(KVM_FEATURE_IPI);
>>>> - if (kvm_pvtime_supported())
>>>> + if (kvm_pvtime_supported()) {
>>>> kvm->arch.pv_features |= BIT(KVM_FEATURE_STEAL_TIME);
>>>> + kvm->arch.pv_features |= BIT(KVM_FEATURE_PREEMPT_HINT);
>>>> + }
>>>>
>>>> /*
>>>> * cpu_vabits means user address space only (a half of total).
>>>> @@ -143,6 +145,7 @@ static int kvm_vm_feature_has_attr(struct kvm *kvm, struct kvm_device_attr *attr
>>>> case KVM_LOONGARCH_VM_FEAT_PV_IPI:
>>>> return 0;
>>>> case KVM_LOONGARCH_VM_FEAT_PV_STEALTIME:
>>>> + case KVM_LOONGARCH_VM_FEAT_PV_PREEMPT_HINT:
>>>> if (kvm_pvtime_supported())
>>>> return 0;
>>>> return -ENXIO;
>>>> --
>>>> 2.39.3
>>>>
>>>>
>>
>>
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 2/3] LoongArch: Add paravirt support with vcpu_is_preempted()
2025-11-19 1:59 ` Bibo Mao
@ 2025-11-19 2:58 ` Huacai Chen
2025-11-19 3:08 ` Bibo Mao
2025-11-19 6:09 ` Bibo Mao
1 sibling, 1 reply; 18+ messages in thread
From: Huacai Chen @ 2025-11-19 2:58 UTC (permalink / raw)
To: Bibo Mao
Cc: Paolo Bonzini, WANG Xuerui, Peter Zijlstra, Ingo Molnar,
Will Deacon, Boqun Feng, Waiman Long, Juergen Gross, Ajay Kaher,
Alexey Makhalov, Broadcom internal kernel review list, kvm,
loongarch, linux-kernel, virtualization, x86
On Wed, Nov 19, 2025 at 10:01 AM Bibo Mao <maobibo@loongson.cn> wrote:
>
>
>
> On 2025/11/18 下午8:48, Huacai Chen wrote:
> > Hi, Bibo,
> >
> > On Tue, Nov 18, 2025 at 4:07 PM Bibo Mao <maobibo@loongson.cn> wrote:
> >>
> >> Function vcpu_is_preempted() is used to check whether vCPU is preempted
> >> or not. Here add implementation with vcpu_is_preempted() when option
> >> CONFIG_PARAVIRT is enabled.
> >>
> >> Signed-off-by: Bibo Mao <maobibo@loongson.cn>
> >> ---
> >> arch/loongarch/include/asm/smp.h | 1 +
> >> arch/loongarch/include/asm/spinlock.h | 5 +++++
> >> arch/loongarch/kernel/paravirt.c | 16 ++++++++++++++++
> >> arch/loongarch/kernel/smp.c | 6 ++++++
> >> 4 files changed, 28 insertions(+)
> >>
> >> diff --git a/arch/loongarch/include/asm/smp.h b/arch/loongarch/include/asm/smp.h
> >> index 3a47f52959a8..5b37f7bf2060 100644
> >> --- a/arch/loongarch/include/asm/smp.h
> >> +++ b/arch/loongarch/include/asm/smp.h
> >> @@ -18,6 +18,7 @@ struct smp_ops {
> >> void (*init_ipi)(void);
> >> void (*send_ipi_single)(int cpu, unsigned int action);
> >> void (*send_ipi_mask)(const struct cpumask *mask, unsigned int action);
> >> + bool (*vcpu_is_preempted)(int cpu);
> >> };
> >> extern struct smp_ops mp_ops;
> >>
> >> diff --git a/arch/loongarch/include/asm/spinlock.h b/arch/loongarch/include/asm/spinlock.h
> >> index 7cb3476999be..c001cef893aa 100644
> >> --- a/arch/loongarch/include/asm/spinlock.h
> >> +++ b/arch/loongarch/include/asm/spinlock.h
> >> @@ -5,6 +5,11 @@
> >> #ifndef _ASM_SPINLOCK_H
> >> #define _ASM_SPINLOCK_H
> >>
> >> +#ifdef CONFIG_PARAVIRT
> >> +#define vcpu_is_preempted vcpu_is_preempted
> >> +bool vcpu_is_preempted(int cpu);
> >> +#endif
> > Maybe paravirt.h is a better place?
>
> It is actually a little strange to add macro CONFIG_PARAVIRT in file
> asm/spinlock.h
>
> vcpu_is_preempted is originally defined in header file
> include/linux/sched.h like this
> #ifndef vcpu_is_preempted
> static inline bool vcpu_is_preempted(int cpu)
> {
> return false;
> }
> #endif
>
> that requires that header file is included before sched.h, file
> asm/spinlock.h can meet this requirement, however header file paravirt.h
> maybe it is not included before sched.h in generic.
>
> Here vcpu_is_preempted definition is added before the following including.
> #include <asm/processor.h>
> #include <asm/qspinlock.h>
> #include <asm/qrwlock.h>
> Maybe it is better to be added after the above header files including
> sentences, but need further investigation.
powerpc put it in paravirt.h, so I think it is possible.
> >
> >> +
> >> #include <asm/processor.h>
> >> #include <asm/qspinlock.h>
> >> #include <asm/qrwlock.h>
> >> diff --git a/arch/loongarch/kernel/paravirt.c b/arch/loongarch/kernel/paravirt.c
> >> index b1b51f920b23..b99404b6b13f 100644
> >> --- a/arch/loongarch/kernel/paravirt.c
> >> +++ b/arch/loongarch/kernel/paravirt.c
> >> @@ -52,6 +52,13 @@ static u64 paravt_steal_clock(int cpu)
> >> #ifdef CONFIG_SMP
> >> static struct smp_ops native_ops;
> >>
> >> +static bool pv_vcpu_is_preempted(int cpu)
> >> +{
> >> + struct kvm_steal_time *src = &per_cpu(steal_time, cpu);
> >> +
> >> + return !!(src->preempted & KVM_VCPU_PREEMPTED);
> >> +}
> >> +
> >> static void pv_send_ipi_single(int cpu, unsigned int action)
> >> {
> >> int min, old;
> >> @@ -308,6 +315,9 @@ int __init pv_time_init(void)
> >> pr_err("Failed to install cpu hotplug callbacks\n");
> >> return r;
> >> }
> >> +
> >> + if (kvm_para_has_feature(KVM_FEATURE_PREEMPT_HINT))
> >> + mp_ops.vcpu_is_preempted = pv_vcpu_is_preempted;
> >> #endif
> >>
> >> static_call_update(pv_steal_clock, paravt_steal_clock);
> >> @@ -332,3 +342,9 @@ int __init pv_spinlock_init(void)
> >>
> >> return 0;
> >> }
> >> +
> >> +bool notrace vcpu_is_preempted(int cpu)
> >> +{
> >> + return mp_ops.vcpu_is_preempted(cpu);
> >> +}
> >
> > We can simplify the whole patch like this, then we don't need to touch
> > smp.c, and we can merge Patch-2/3.
> >
> > +bool notrace vcpu_is_preempted(int cpu)
> > +{
> > + if (!kvm_para_has_feature(KVM_FEATURE_PREEMPT_HINT))
> > + return false;
> > + else {
> > + struct kvm_steal_time *src = &per_cpu(steal_time, cpu);
> > + return !!(src->preempted & KVM_VCPU_PREEMPTED);
> > + }
> > +}
> 1. there is assembly output about relative vcpu_is_preempted
> <loongson_vcpu_is_preempted>:
> move $r4,$r0
> jirl $r0,$r1,0
>
> <pv_vcpu_is_preempted>:
> pcalau12i $r13,8759(0x2237)
> slli.d $r4,$r4,0x3
> addi.d $r13,$r13,-1000(0xc18)
> ldx.d $r13,$r13,$r4
> pcalau12i $r12,5462(0x1556)
> addi.d $r12,$r12,384(0x180)
> add.d $r12,$r13,$r12
> ld.bu $r4,$r12,16(0x10)
> andi $r4,$r4,0x1
> jirl $r0,$r1,0
>
> <vcpu_is_preempted>:
> pcalau12i $r12,8775(0x2247)
> ld.d $r12,$r12,-472(0xe28)
> jirl $r0,$r12,0
> andi $r0,$r0,0x0
>
> <vcpu_is_preempted_new>:
> pcalau12i $r12,8151(0x1fd7)
> ld.d $r12,$r12,-1008(0xc10)
> bstrpick.d $r12,$r12,0x1a,0x1a
> beqz $r12,188(0xbc) # 900000000024ec60
> pcalau12i $r12,11802(0x2e1a)
> addi.d $r12,$r12,-1400(0xa88)
> ldptr.w $r14,$r12,36(0x24)
> beqz $r14,108(0x6c) # 900000000024ec20
> addi.w $r13,$r0,1(0x1)
> bne $r14,$r13,164(0xa4) # 900000000024ec60
> ldptr.w $r13,$r12,40(0x28)
> bnez $r13,24(0x18) # 900000000024ebdc
> lu12i.w $r14,262144(0x40000)
> ori $r14,$r14,0x4
> cpucfg $r14,$r14
> slli.w $r13,$r14,0x0
> st.w $r14,$r12,40(0x28)
> bstrpick.d $r13,$r13,0x3,0x3
> beqz $r13,128(0x80) # 900000000024ec60
> pcalau12i $r13,8759(0x2237)
> slli.d $r4,$r4,0x3
> addi.d $r13,$r13,-1000(0xc18)
> ldx.d $r13,$r13,$r4
> pcalau12i $r12,5462(0x1556)
> addi.d $r12,$r12,384(0x180)
> add.d $r12,$r13,$r12
> ld.bu $r4,$r12,16(0x10)
> andi $r4,$r4,0x1
> jirl $r0,$r1,0
> andi $r0,$r0,0x0
> andi $r0,$r0,0x0
> andi $r0,$r0,0x0
> andi $r0,$r0,0x0
> andi $r0,$r0,0x0
> lu12i.w $r13,262144(0x40000)
> cpucfg $r13,$r13
> lu12i.w $r15,1237(0x4d5)
> ori $r15,$r15,0x64b
> slli.w $r13,$r13,0x0
> bne $r13,$r15,-124(0x3ff84) # 900000000024ebb8
> addi.w $r13,$r0,1(0x1)
> st.w $r13,$r12,36(0x24)
> b -128(0xfffff80) # 900000000024ebc0
> andi $r0,$r0,0x0
> andi $r0,$r0,0x0
> andi $r0,$r0,0x0
> andi $r0,$r0,0x0
> andi $r0,$r0,0x0
> andi $r0,$r0,0x0
> andi $r0,$r0,0x0
> move $r4,$r0
> jirl $r0,$r1,0
>
> With vcpu_is_preempted(), there is one memory load and one jirl jump,
> with vcpu_is_preempted_new(), there is two memory load and two beq
> compare instructions.
Is vcpu_is_preempted() performance critical (we need performance data
here)? It seems the powerpc version is also complex.
>
> 2. In some scenery such nr_cpus == 1, loongson_vcpu_is_preempted() is
> better than pv_vcpu_is_preempted() even if the preempt feature is enabled.
In your original patch, "mp_ops.vcpu_is_preempted =
pv_vcpu_is_preempted" if the preempt feature is enabled. Why is
loongson_vcpu_is_preempted() called when nr_cpus=1?
Huacai
>
> Regards
> Bibo Mao
> > Huacai
> >
> >> +EXPORT_SYMBOL(vcpu_is_preempted);
> >> diff --git a/arch/loongarch/kernel/smp.c b/arch/loongarch/kernel/smp.c
> >> index 46036d98da75..f04192fedf8d 100644
> >> --- a/arch/loongarch/kernel/smp.c
> >> +++ b/arch/loongarch/kernel/smp.c
> >> @@ -307,10 +307,16 @@ static void loongson_init_ipi(void)
> >> panic("IPI IRQ request failed\n");
> >> }
> >>
> >> +static bool loongson_vcpu_is_preempted(int cpu)
> >> +{
> >> + return false;
> >> +}
> >> +
> >> struct smp_ops mp_ops = {
> >> .init_ipi = loongson_init_ipi,
> >> .send_ipi_single = loongson_send_ipi_single,
> >> .send_ipi_mask = loongson_send_ipi_mask,
> >> + .vcpu_is_preempted = loongson_vcpu_is_preempted,
> >> };
> >>
> >> static void __init fdt_smp_setup(void)
> >> --
> >> 2.39.3
> >>
> >>
>
>
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 1/3] LoongArch: KVM: Add preempt hint feature in hypervisor side
2025-11-19 2:55 ` Bibo Mao
@ 2025-11-19 3:01 ` Huacai Chen
0 siblings, 0 replies; 18+ messages in thread
From: Huacai Chen @ 2025-11-19 3:01 UTC (permalink / raw)
To: Bibo Mao
Cc: Paolo Bonzini, Tianrui Zhao, WANG Xuerui, kvm, loongarch,
linux-kernel
On Wed, Nov 19, 2025 at 10:58 AM Bibo Mao <maobibo@loongson.cn> wrote:
>
>
>
> On 2025/11/19 上午10:45, Huacai Chen wrote:
> > On Wed, Nov 19, 2025 at 9:23 AM Bibo Mao <maobibo@loongson.cn> wrote:
> >>
> >>
> >>
> >> On 2025/11/18 下午8:46, Huacai Chen wrote:
> >>> Hi, Bibo,
> >>>
> >>> On Tue, Nov 18, 2025 at 4:07 PM Bibo Mao <maobibo@loongson.cn> wrote:
> >>>>
> >>>> Feature KVM_FEATURE_PREEMPT_HINT is added to show whether vCPU is
> >>>> preempted or not. It is to help guest OS scheduling or lock checking
> >>>> etc. Here add KVM_FEATURE_PREEMPT_HINT feature and use one byte as
> >>>> preempted flag in steal time structure.
> >>>>
> >>>> Signed-off-by: Bibo Mao <maobibo@loongson.cn>
> >>>> ---
> >>>> arch/loongarch/include/asm/kvm_host.h | 2 +
> >>>> arch/loongarch/include/asm/kvm_para.h | 5 +-
> >>>> arch/loongarch/include/uapi/asm/kvm.h | 1 +
> >>>> arch/loongarch/include/uapi/asm/kvm_para.h | 1 +
> >>>> arch/loongarch/kvm/vcpu.c | 54 +++++++++++++++++++++-
> >>>> arch/loongarch/kvm/vm.c | 5 +-
> >>>> 6 files changed, 65 insertions(+), 3 deletions(-)
> >>>>
> >>>> diff --git a/arch/loongarch/include/asm/kvm_host.h b/arch/loongarch/include/asm/kvm_host.h
> >>>> index 0cecbd038bb3..04c6dd171877 100644
> >>>> --- a/arch/loongarch/include/asm/kvm_host.h
> >>>> +++ b/arch/loongarch/include/asm/kvm_host.h
> >>>> @@ -163,6 +163,7 @@ enum emulation_result {
> >>>> #define LOONGARCH_PV_FEAT_UPDATED BIT_ULL(63)
> >>>> #define LOONGARCH_PV_FEAT_MASK (BIT(KVM_FEATURE_IPI) | \
> >>>> BIT(KVM_FEATURE_STEAL_TIME) | \
> >>>> + BIT(KVM_FEATURE_PREEMPT_HINT) |\
> >>>> BIT(KVM_FEATURE_USER_HCALL) | \
> >>>> BIT(KVM_FEATURE_VIRT_EXTIOI))
> >>>>
> >>>> @@ -250,6 +251,7 @@ struct kvm_vcpu_arch {
> >>>> u64 guest_addr;
> >>>> u64 last_steal;
> >>>> struct gfn_to_hva_cache cache;
> >>>> + u8 preempted;
> >>>> } st;
> >>>> };
> >>>>
> >>>> diff --git a/arch/loongarch/include/asm/kvm_para.h b/arch/loongarch/include/asm/kvm_para.h
> >>>> index 3e4b397f423f..d8592a7f5922 100644
> >>>> --- a/arch/loongarch/include/asm/kvm_para.h
> >>>> +++ b/arch/loongarch/include/asm/kvm_para.h
> >>>> @@ -37,8 +37,11 @@ struct kvm_steal_time {
> >>>> __u64 steal;
> >>>> __u32 version;
> >>>> __u32 flags;
> >>>> - __u32 pad[12];
> >>>> + __u8 preempted;
> >>>> + __u8 u8_pad[3];
> >>>> + __u32 pad[11];
> >>> Maybe a single __u8 pad[47] is enough?
> >> yes, pad[47] seems better unless there is definitely __u32 type
> >> requirement in future.
> >>
> >> Will do in next version.
> >>>
> >>>> };
> >>>> +#define KVM_VCPU_PREEMPTED (1 << 0)
> >>>>
> >>>> /*
> >>>> * Hypercall interface for KVM hypervisor
> >>>> diff --git a/arch/loongarch/include/uapi/asm/kvm.h b/arch/loongarch/include/uapi/asm/kvm.h
> >>>> index 57ba1a563bb1..bca7154aa651 100644
> >>>> --- a/arch/loongarch/include/uapi/asm/kvm.h
> >>>> +++ b/arch/loongarch/include/uapi/asm/kvm.h
> >>>> @@ -104,6 +104,7 @@ struct kvm_fpu {
> >>>> #define KVM_LOONGARCH_VM_FEAT_PV_IPI 6
> >>>> #define KVM_LOONGARCH_VM_FEAT_PV_STEALTIME 7
> >>>> #define KVM_LOONGARCH_VM_FEAT_PTW 8
> >>>> +#define KVM_LOONGARCH_VM_FEAT_PV_PREEMPT_HINT 10
> >>> From the name it is a "hint", from include/linux/kvm_para.h we know
> >>> features and hints are different. If preempt is really a feature,
> >>> rename it?
> >> It is a feature. yes, in generic hint is suggestion for VM and VM can
> >> selectively do or not.
> >>
> >> Will rename it with KVM_LOONGARCH_VM_FEAT_PV_PREEMPT.
> >>>
> >>>>
> >>>> /* Device Control API on vcpu fd */
> >>>> #define KVM_LOONGARCH_VCPU_CPUCFG 0
> >>>> diff --git a/arch/loongarch/include/uapi/asm/kvm_para.h b/arch/loongarch/include/uapi/asm/kvm_para.h
> >>>> index 76d802ef01ce..fe4107869ce6 100644
> >>>> --- a/arch/loongarch/include/uapi/asm/kvm_para.h
> >>>> +++ b/arch/loongarch/include/uapi/asm/kvm_para.h
> >>>> @@ -15,6 +15,7 @@
> >>>> #define CPUCFG_KVM_FEATURE (CPUCFG_KVM_BASE + 4)
> >>>> #define KVM_FEATURE_IPI 1
> >>>> #define KVM_FEATURE_STEAL_TIME 2
> >>>> +#define KVM_FEATURE_PREEMPT_HINT 3
> >>>> /* BIT 24 - 31 are features configurable by user space vmm */
> >>>> #define KVM_FEATURE_VIRT_EXTIOI 24
> >>>> #define KVM_FEATURE_USER_HCALL 25
> >>>> diff --git a/arch/loongarch/kvm/vcpu.c b/arch/loongarch/kvm/vcpu.c
> >>>> index 1245a6b35896..33a94b191b5d 100644
> >>>> --- a/arch/loongarch/kvm/vcpu.c
> >>>> +++ b/arch/loongarch/kvm/vcpu.c
> >>>> @@ -180,6 +180,11 @@ static void kvm_update_stolen_time(struct kvm_vcpu *vcpu)
> >>>> }
> >>>>
> >>>> st = (struct kvm_steal_time __user *)ghc->hva;
> >>>> + if (kvm_guest_has_pv_feature(vcpu, KVM_FEATURE_PREEMPT_HINT)) {
> >>>> + unsafe_put_user(0, &st->preempted, out);
> >>>> + vcpu->arch.st.preempted = 0;
> >>>> + }
> >>>> +
> >>>> unsafe_get_user(version, &st->version, out);
> >>>> if (version & 1)
> >>>> version += 1; /* first time write, random junk */
> >>>> @@ -1757,11 +1762,58 @@ static int _kvm_vcpu_put(struct kvm_vcpu *vcpu, int cpu)
> >>>> return 0;
> >>>> }
> >>>>
> >>>> +static void _kvm_set_vcpu_preempted(struct kvm_vcpu *vcpu)
> >>> Just using kvm_set_vcpu_preempted() is enough, no "_".
> >>>
> >>>> +{
> >>>> + struct gfn_to_hva_cache *ghc;
> >>>> + struct kvm_steal_time __user *st;
> >>>> + struct kvm_memslots *slots;
> >>>> + static const u8 preempted = KVM_VCPU_PREEMPTED;
> >>> I'm not sure whether "static" is right, it's not reentrant.
> >> I think static is better here, it saves one cycle with assignment here.
> > I know, but I want to know whether the logic is correct.
> > vcpu->arch.st.preempted is per-cpu, but the local variable "preempted"
> > can be used across multiple VCPU? I'm not sure.
> It is read-only, of course can be used by multiple vCPUs. or remove it
> directly?
Good, remove it directly.
Huacai
>
> @@ -1767,7 +1767,6 @@ static void _kvm_set_vcpu_preempted(struct
> kvm_vcpu *vcpu)
> struct gfn_to_hva_cache *ghc;
> struct kvm_steal_time __user *st;
> struct kvm_memslots *slots;
> - static const u8 preempted = KVM_VCPU_PREEMPTED;
> gpa_t gpa;
>
> gpa = vcpu->arch.st.guest_addr;
> @@ -1793,7 +1792,7 @@ static void _kvm_set_vcpu_preempted(struct
> kvm_vcpu *vcpu)
> }
>
> st = (struct kvm_steal_time __user *)ghc->hva;
> - unsafe_put_user(preempted, &st->preempted, out);
> + unsafe_put_user(KVM_VCPU_PREEMPTED, &st->preempted, out);
> vcpu->arch.st.preempted = KVM_VCPU_PREEMPTED;
>
> >
> > Huacai
> >
> >>
> >> Regards
> >> Bibo Mao
> >>>
> >>>
> >>> Huacai
> >>>
> >>>> + gpa_t gpa;
> >>>> +
> >>>> + gpa = vcpu->arch.st.guest_addr;
> >>>> + if (!(gpa & KVM_STEAL_PHYS_VALID))
> >>>> + return;
> >>>> +
> >>>> + /* vCPU may be preempted for many times */
> >>>> + if (vcpu->arch.st.preempted)
> >>>> + return;
> >>>> +
> >>>> + /* This happens on process exit */
> >>>> + if (unlikely(current->mm != vcpu->kvm->mm))
> >>>> + return;
> >>>> +
> >>>> + gpa &= KVM_STEAL_PHYS_MASK;
> >>>> + ghc = &vcpu->arch.st.cache;
> >>>> + slots = kvm_memslots(vcpu->kvm);
> >>>> + if (slots->generation != ghc->generation || gpa != ghc->gpa) {
> >>>> + if (kvm_gfn_to_hva_cache_init(vcpu->kvm, ghc, gpa, sizeof(*st))) {
> >>>> + ghc->gpa = INVALID_GPA;
> >>>> + return;
> >>>> + }
> >>>> + }
> >>>> +
> >>>> + st = (struct kvm_steal_time __user *)ghc->hva;
> >>>> + unsafe_put_user(preempted, &st->preempted, out);
> >>>> + vcpu->arch.st.preempted = KVM_VCPU_PREEMPTED;
> >>>> +out:
> >>>> + mark_page_dirty_in_slot(vcpu->kvm, ghc->memslot, gpa_to_gfn(ghc->gpa));
> >>>> +}
> >>>> +
> >>>> void kvm_arch_vcpu_put(struct kvm_vcpu *vcpu)
> >>>> {
> >>>> - int cpu;
> >>>> + int cpu, idx;
> >>>> unsigned long flags;
> >>>>
> >>>> + if (vcpu->preempted && kvm_guest_has_pv_feature(vcpu, KVM_FEATURE_PREEMPT_HINT)) {
> >>>> + /*
> >>>> + * Take the srcu lock as memslots will be accessed to check the gfn
> >>>> + * cache generation against the memslots generation.
> >>>> + */
> >>>> + idx = srcu_read_lock(&vcpu->kvm->srcu);
> >>>> + _kvm_set_vcpu_preempted(vcpu);
> >>>> + srcu_read_unlock(&vcpu->kvm->srcu, idx);
> >>>> + }
> >>>> +
> >>>> local_irq_save(flags);
> >>>> cpu = smp_processor_id();
> >>>> vcpu->arch.last_sched_cpu = cpu;
> >>>> diff --git a/arch/loongarch/kvm/vm.c b/arch/loongarch/kvm/vm.c
> >>>> index a49b1c1a3dd1..b8879110a0a1 100644
> >>>> --- a/arch/loongarch/kvm/vm.c
> >>>> +++ b/arch/loongarch/kvm/vm.c
> >>>> @@ -45,8 +45,10 @@ int kvm_arch_init_vm(struct kvm *kvm, unsigned long type)
> >>>>
> >>>> /* Enable all PV features by default */
> >>>> kvm->arch.pv_features = BIT(KVM_FEATURE_IPI);
> >>>> - if (kvm_pvtime_supported())
> >>>> + if (kvm_pvtime_supported()) {
> >>>> kvm->arch.pv_features |= BIT(KVM_FEATURE_STEAL_TIME);
> >>>> + kvm->arch.pv_features |= BIT(KVM_FEATURE_PREEMPT_HINT);
> >>>> + }
> >>>>
> >>>> /*
> >>>> * cpu_vabits means user address space only (a half of total).
> >>>> @@ -143,6 +145,7 @@ static int kvm_vm_feature_has_attr(struct kvm *kvm, struct kvm_device_attr *attr
> >>>> case KVM_LOONGARCH_VM_FEAT_PV_IPI:
> >>>> return 0;
> >>>> case KVM_LOONGARCH_VM_FEAT_PV_STEALTIME:
> >>>> + case KVM_LOONGARCH_VM_FEAT_PV_PREEMPT_HINT:
> >>>> if (kvm_pvtime_supported())
> >>>> return 0;
> >>>> return -ENXIO;
> >>>> --
> >>>> 2.39.3
> >>>>
> >>>>
> >>
> >>
>
>
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 2/3] LoongArch: Add paravirt support with vcpu_is_preempted()
2025-11-19 2:58 ` Huacai Chen
@ 2025-11-19 3:08 ` Bibo Mao
0 siblings, 0 replies; 18+ messages in thread
From: Bibo Mao @ 2025-11-19 3:08 UTC (permalink / raw)
To: Huacai Chen
Cc: Paolo Bonzini, WANG Xuerui, Peter Zijlstra, Ingo Molnar,
Will Deacon, Boqun Feng, Waiman Long, Juergen Gross, Ajay Kaher,
Alexey Makhalov, Broadcom internal kernel review list, kvm,
loongarch, linux-kernel, virtualization, x86
On 2025/11/19 上午10:58, Huacai Chen wrote:
> On Wed, Nov 19, 2025 at 10:01 AM Bibo Mao <maobibo@loongson.cn> wrote:
>>
>>
>>
>> On 2025/11/18 下午8:48, Huacai Chen wrote:
>>> Hi, Bibo,
>>>
>>> On Tue, Nov 18, 2025 at 4:07 PM Bibo Mao <maobibo@loongson.cn> wrote:
>>>>
>>>> Function vcpu_is_preempted() is used to check whether vCPU is preempted
>>>> or not. Here add implementation with vcpu_is_preempted() when option
>>>> CONFIG_PARAVIRT is enabled.
>>>>
>>>> Signed-off-by: Bibo Mao <maobibo@loongson.cn>
>>>> ---
>>>> arch/loongarch/include/asm/smp.h | 1 +
>>>> arch/loongarch/include/asm/spinlock.h | 5 +++++
>>>> arch/loongarch/kernel/paravirt.c | 16 ++++++++++++++++
>>>> arch/loongarch/kernel/smp.c | 6 ++++++
>>>> 4 files changed, 28 insertions(+)
>>>>
>>>> diff --git a/arch/loongarch/include/asm/smp.h b/arch/loongarch/include/asm/smp.h
>>>> index 3a47f52959a8..5b37f7bf2060 100644
>>>> --- a/arch/loongarch/include/asm/smp.h
>>>> +++ b/arch/loongarch/include/asm/smp.h
>>>> @@ -18,6 +18,7 @@ struct smp_ops {
>>>> void (*init_ipi)(void);
>>>> void (*send_ipi_single)(int cpu, unsigned int action);
>>>> void (*send_ipi_mask)(const struct cpumask *mask, unsigned int action);
>>>> + bool (*vcpu_is_preempted)(int cpu);
>>>> };
>>>> extern struct smp_ops mp_ops;
>>>>
>>>> diff --git a/arch/loongarch/include/asm/spinlock.h b/arch/loongarch/include/asm/spinlock.h
>>>> index 7cb3476999be..c001cef893aa 100644
>>>> --- a/arch/loongarch/include/asm/spinlock.h
>>>> +++ b/arch/loongarch/include/asm/spinlock.h
>>>> @@ -5,6 +5,11 @@
>>>> #ifndef _ASM_SPINLOCK_H
>>>> #define _ASM_SPINLOCK_H
>>>>
>>>> +#ifdef CONFIG_PARAVIRT
>>>> +#define vcpu_is_preempted vcpu_is_preempted
>>>> +bool vcpu_is_preempted(int cpu);
>>>> +#endif
>>> Maybe paravirt.h is a better place?
>>
>> It is actually a little strange to add macro CONFIG_PARAVIRT in file
>> asm/spinlock.h
>>
>> vcpu_is_preempted is originally defined in header file
>> include/linux/sched.h like this
>> #ifndef vcpu_is_preempted
>> static inline bool vcpu_is_preempted(int cpu)
>> {
>> return false;
>> }
>> #endif
>>
>> that requires that header file is included before sched.h, file
>> asm/spinlock.h can meet this requirement, however header file paravirt.h
>> maybe it is not included before sched.h in generic.
>>
>> Here vcpu_is_preempted definition is added before the following including.
>> #include <asm/processor.h>
>> #include <asm/qspinlock.h>
>> #include <asm/qrwlock.h>
>> Maybe it is better to be added after the above header files including
>> sentences, but need further investigation.
> powerpc put it in paravirt.h, so I think it is possible.
paravirt.h is included by header file asm/qspinlock.h on powerpc,
however it is not so on loongarch :)
# grep paravirt.h arch/powerpc/* -r
arch/powerpc/include/asm/paravirt_api_clock.h:#include <asm/paravirt.h>
arch/powerpc/include/asm/qspinlock.h:#include <asm/paravirt.h>
arch/powerpc/include/asm/simple_spinlock.h:#include <asm/paravirt.h>
$ grep paravirt.h arch/loongarch/* -r
arch/loongarch/include/asm/paravirt_api_clock.h:#include <asm/paravirt.h>
>
>>>
>>>> +
>>>> #include <asm/processor.h>
>>>> #include <asm/qspinlock.h>
>>>> #include <asm/qrwlock.h>
>>>> diff --git a/arch/loongarch/kernel/paravirt.c b/arch/loongarch/kernel/paravirt.c
>>>> index b1b51f920b23..b99404b6b13f 100644
>>>> --- a/arch/loongarch/kernel/paravirt.c
>>>> +++ b/arch/loongarch/kernel/paravirt.c
>>>> @@ -52,6 +52,13 @@ static u64 paravt_steal_clock(int cpu)
>>>> #ifdef CONFIG_SMP
>>>> static struct smp_ops native_ops;
>>>>
>>>> +static bool pv_vcpu_is_preempted(int cpu)
>>>> +{
>>>> + struct kvm_steal_time *src = &per_cpu(steal_time, cpu);
>>>> +
>>>> + return !!(src->preempted & KVM_VCPU_PREEMPTED);
>>>> +}
>>>> +
>>>> static void pv_send_ipi_single(int cpu, unsigned int action)
>>>> {
>>>> int min, old;
>>>> @@ -308,6 +315,9 @@ int __init pv_time_init(void)
>>>> pr_err("Failed to install cpu hotplug callbacks\n");
>>>> return r;
>>>> }
>>>> +
>>>> + if (kvm_para_has_feature(KVM_FEATURE_PREEMPT_HINT))
>>>> + mp_ops.vcpu_is_preempted = pv_vcpu_is_preempted;
>>>> #endif
>>>>
>>>> static_call_update(pv_steal_clock, paravt_steal_clock);
>>>> @@ -332,3 +342,9 @@ int __init pv_spinlock_init(void)
>>>>
>>>> return 0;
>>>> }
>>>> +
>>>> +bool notrace vcpu_is_preempted(int cpu)
>>>> +{
>>>> + return mp_ops.vcpu_is_preempted(cpu);
>>>> +}
>>>
>>> We can simplify the whole patch like this, then we don't need to touch
>>> smp.c, and we can merge Patch-2/3.
>>>
>>> +bool notrace vcpu_is_preempted(int cpu)
>>> +{
>>> + if (!kvm_para_has_feature(KVM_FEATURE_PREEMPT_HINT))
>>> + return false;
>>> + else {
>>> + struct kvm_steal_time *src = &per_cpu(steal_time, cpu);
>>> + return !!(src->preempted & KVM_VCPU_PREEMPTED);
>>> + }
>>> +}
>> 1. there is assembly output about relative vcpu_is_preempted
>> <loongson_vcpu_is_preempted>:
>> move $r4,$r0
>> jirl $r0,$r1,0
>>
>> <pv_vcpu_is_preempted>:
>> pcalau12i $r13,8759(0x2237)
>> slli.d $r4,$r4,0x3
>> addi.d $r13,$r13,-1000(0xc18)
>> ldx.d $r13,$r13,$r4
>> pcalau12i $r12,5462(0x1556)
>> addi.d $r12,$r12,384(0x180)
>> add.d $r12,$r13,$r12
>> ld.bu $r4,$r12,16(0x10)
>> andi $r4,$r4,0x1
>> jirl $r0,$r1,0
>>
>> <vcpu_is_preempted>:
>> pcalau12i $r12,8775(0x2247)
>> ld.d $r12,$r12,-472(0xe28)
>> jirl $r0,$r12,0
>> andi $r0,$r0,0x0
>>
>> <vcpu_is_preempted_new>:
>> pcalau12i $r12,8151(0x1fd7)
>> ld.d $r12,$r12,-1008(0xc10)
>> bstrpick.d $r12,$r12,0x1a,0x1a
>> beqz $r12,188(0xbc) # 900000000024ec60
>> pcalau12i $r12,11802(0x2e1a)
>> addi.d $r12,$r12,-1400(0xa88)
>> ldptr.w $r14,$r12,36(0x24)
>> beqz $r14,108(0x6c) # 900000000024ec20
>> addi.w $r13,$r0,1(0x1)
>> bne $r14,$r13,164(0xa4) # 900000000024ec60
>> ldptr.w $r13,$r12,40(0x28)
>> bnez $r13,24(0x18) # 900000000024ebdc
>> lu12i.w $r14,262144(0x40000)
>> ori $r14,$r14,0x4
>> cpucfg $r14,$r14
>> slli.w $r13,$r14,0x0
>> st.w $r14,$r12,40(0x28)
>> bstrpick.d $r13,$r13,0x3,0x3
>> beqz $r13,128(0x80) # 900000000024ec60
>> pcalau12i $r13,8759(0x2237)
>> slli.d $r4,$r4,0x3
>> addi.d $r13,$r13,-1000(0xc18)
>> ldx.d $r13,$r13,$r4
>> pcalau12i $r12,5462(0x1556)
>> addi.d $r12,$r12,384(0x180)
>> add.d $r12,$r13,$r12
>> ld.bu $r4,$r12,16(0x10)
>> andi $r4,$r4,0x1
>> jirl $r0,$r1,0
>> andi $r0,$r0,0x0
>> andi $r0,$r0,0x0
>> andi $r0,$r0,0x0
>> andi $r0,$r0,0x0
>> andi $r0,$r0,0x0
>> lu12i.w $r13,262144(0x40000)
>> cpucfg $r13,$r13
>> lu12i.w $r15,1237(0x4d5)
>> ori $r15,$r15,0x64b
>> slli.w $r13,$r13,0x0
>> bne $r13,$r15,-124(0x3ff84) # 900000000024ebb8
>> addi.w $r13,$r0,1(0x1)
>> st.w $r13,$r12,36(0x24)
>> b -128(0xfffff80) # 900000000024ebc0
>> andi $r0,$r0,0x0
>> andi $r0,$r0,0x0
>> andi $r0,$r0,0x0
>> andi $r0,$r0,0x0
>> andi $r0,$r0,0x0
>> andi $r0,$r0,0x0
>> andi $r0,$r0,0x0
>> move $r4,$r0
>> jirl $r0,$r1,0
>>
>> With vcpu_is_preempted(), there is one memory load and one jirl jump,
>> with vcpu_is_preempted_new(), there is two memory load and two beq
>> compare instructions.
> Is vcpu_is_preempted() performance critical (we need performance data
> here)? It seems the powerpc version is also complex.
>
>>
>> 2. In some scenery such nr_cpus == 1, loongson_vcpu_is_preempted() is
>> better than pv_vcpu_is_preempted() even if the preempt feature is enabled.
> In your original patch, "mp_ops.vcpu_is_preempted =
> pv_vcpu_is_preempted" if the preempt feature is enabled. Why is
> loongson_vcpu_is_preempted() called when nr_cpus=1?
>
> Huacai
>
>>
>> Regards
>> Bibo Mao
>>> Huacai
>>>
>>>> +EXPORT_SYMBOL(vcpu_is_preempted);
>>>> diff --git a/arch/loongarch/kernel/smp.c b/arch/loongarch/kernel/smp.c
>>>> index 46036d98da75..f04192fedf8d 100644
>>>> --- a/arch/loongarch/kernel/smp.c
>>>> +++ b/arch/loongarch/kernel/smp.c
>>>> @@ -307,10 +307,16 @@ static void loongson_init_ipi(void)
>>>> panic("IPI IRQ request failed\n");
>>>> }
>>>>
>>>> +static bool loongson_vcpu_is_preempted(int cpu)
>>>> +{
>>>> + return false;
>>>> +}
>>>> +
>>>> struct smp_ops mp_ops = {
>>>> .init_ipi = loongson_init_ipi,
>>>> .send_ipi_single = loongson_send_ipi_single,
>>>> .send_ipi_mask = loongson_send_ipi_mask,
>>>> + .vcpu_is_preempted = loongson_vcpu_is_preempted,
>>>> };
>>>>
>>>> static void __init fdt_smp_setup(void)
>>>> --
>>>> 2.39.3
>>>>
>>>>
>>
>>
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 2/3] LoongArch: Add paravirt support with vcpu_is_preempted()
2025-11-19 1:59 ` Bibo Mao
2025-11-19 2:58 ` Huacai Chen
@ 2025-11-19 6:09 ` Bibo Mao
2025-11-19 7:41 ` Huacai Chen
1 sibling, 1 reply; 18+ messages in thread
From: Bibo Mao @ 2025-11-19 6:09 UTC (permalink / raw)
To: Huacai Chen
Cc: Paolo Bonzini, WANG Xuerui, Peter Zijlstra, Ingo Molnar,
Will Deacon, Boqun Feng, Waiman Long, Juergen Gross, Ajay Kaher,
Alexey Makhalov, Broadcom internal kernel review list, kvm,
loongarch, linux-kernel, virtualization, x86
On 2025/11/19 上午9:59, Bibo Mao wrote:
>
>
> On 2025/11/18 下午8:48, Huacai Chen wrote:
>> Hi, Bibo,
>>
>> On Tue, Nov 18, 2025 at 4:07 PM Bibo Mao <maobibo@loongson.cn> wrote:
>>>
>>> Function vcpu_is_preempted() is used to check whether vCPU is preempted
>>> or not. Here add implementation with vcpu_is_preempted() when option
>>> CONFIG_PARAVIRT is enabled.
>>>
>>> Signed-off-by: Bibo Mao <maobibo@loongson.cn>
>>> ---
>>> arch/loongarch/include/asm/smp.h | 1 +
>>> arch/loongarch/include/asm/spinlock.h | 5 +++++
>>> arch/loongarch/kernel/paravirt.c | 16 ++++++++++++++++
>>> arch/loongarch/kernel/smp.c | 6 ++++++
>>> 4 files changed, 28 insertions(+)
>>>
>>> diff --git a/arch/loongarch/include/asm/smp.h
>>> b/arch/loongarch/include/asm/smp.h
>>> index 3a47f52959a8..5b37f7bf2060 100644
>>> --- a/arch/loongarch/include/asm/smp.h
>>> +++ b/arch/loongarch/include/asm/smp.h
>>> @@ -18,6 +18,7 @@ struct smp_ops {
>>> void (*init_ipi)(void);
>>> void (*send_ipi_single)(int cpu, unsigned int action);
>>> void (*send_ipi_mask)(const struct cpumask *mask, unsigned
>>> int action);
>>> + bool (*vcpu_is_preempted)(int cpu);
>>> };
>>> extern struct smp_ops mp_ops;
>>>
>>> diff --git a/arch/loongarch/include/asm/spinlock.h
>>> b/arch/loongarch/include/asm/spinlock.h
>>> index 7cb3476999be..c001cef893aa 100644
>>> --- a/arch/loongarch/include/asm/spinlock.h
>>> +++ b/arch/loongarch/include/asm/spinlock.h
>>> @@ -5,6 +5,11 @@
>>> #ifndef _ASM_SPINLOCK_H
>>> #define _ASM_SPINLOCK_H
>>>
>>> +#ifdef CONFIG_PARAVIRT
>>> +#define vcpu_is_preempted vcpu_is_preempted
>>> +bool vcpu_is_preempted(int cpu);
>>> +#endif
>> Maybe paravirt.h is a better place?
>
> It is actually a little strange to add macro CONFIG_PARAVIRT in file
> asm/spinlock.h
>
> vcpu_is_preempted is originally defined in header file
> include/linux/sched.h like this
> #ifndef vcpu_is_preempted
> static inline bool vcpu_is_preempted(int cpu)
> {
> return false;
> }
> #endif
>
> that requires that header file is included before sched.h, file
> asm/spinlock.h can meet this requirement, however header file paravirt.h
> maybe it is not included before sched.h in generic.
>
> Here vcpu_is_preempted definition is added before the following including.
> #include <asm/processor.h>
> #include <asm/qspinlock.h>
> #include <asm/qrwlock.h>
> Maybe it is better to be added after the above header files including
> sentences, but need further investigation.
>>
>>> +
>>> #include <asm/processor.h>
>>> #include <asm/qspinlock.h>
>>> #include <asm/qrwlock.h>
>>> diff --git a/arch/loongarch/kernel/paravirt.c
>>> b/arch/loongarch/kernel/paravirt.c
>>> index b1b51f920b23..b99404b6b13f 100644
>>> --- a/arch/loongarch/kernel/paravirt.c
>>> +++ b/arch/loongarch/kernel/paravirt.c
>>> @@ -52,6 +52,13 @@ static u64 paravt_steal_clock(int cpu)
>>> #ifdef CONFIG_SMP
>>> static struct smp_ops native_ops;
>>>
>>> +static bool pv_vcpu_is_preempted(int cpu)
>>> +{
>>> + struct kvm_steal_time *src = &per_cpu(steal_time, cpu);
>>> +
>>> + return !!(src->preempted & KVM_VCPU_PREEMPTED);
>>> +}
>>> +
>>> static void pv_send_ipi_single(int cpu, unsigned int action)
>>> {
>>> int min, old;
>>> @@ -308,6 +315,9 @@ int __init pv_time_init(void)
>>> pr_err("Failed to install cpu hotplug callbacks\n");
>>> return r;
>>> }
>>> +
>>> + if (kvm_para_has_feature(KVM_FEATURE_PREEMPT_HINT))
>>> + mp_ops.vcpu_is_preempted = pv_vcpu_is_preempted;
>>> #endif
>>>
>>> static_call_update(pv_steal_clock, paravt_steal_clock);
>>> @@ -332,3 +342,9 @@ int __init pv_spinlock_init(void)
>>>
>>> return 0;
>>> }
>>> +
>>> +bool notrace vcpu_is_preempted(int cpu)
>>> +{
>>> + return mp_ops.vcpu_is_preempted(cpu);
>>> +}
>>
>> We can simplify the whole patch like this, then we don't need to touch
>> smp.c, and we can merge Patch-2/3.
>>
>> +bool notrace vcpu_is_preempted(int cpu)
>> +{
>> + if (!kvm_para_has_feature(KVM_FEATURE_PREEMPT_HINT))
>> + return false;
>> + else {
>> + struct kvm_steal_time *src = &per_cpu(steal_time, cpu);
>> + return !!(src->preempted & KVM_VCPU_PREEMPTED);
>> + }
>> +}
> 1. there is assembly output about relative vcpu_is_preempted
> <loongson_vcpu_is_preempted>:
> move $r4,$r0
> jirl $r0,$r1,0
>
> <pv_vcpu_is_preempted>:
> pcalau12i $r13,8759(0x2237)
> slli.d $r4,$r4,0x3
> addi.d $r13,$r13,-1000(0xc18)
> ldx.d $r13,$r13,$r4
> pcalau12i $r12,5462(0x1556)
> addi.d $r12,$r12,384(0x180)
> add.d $r12,$r13,$r12
> ld.bu $r4,$r12,16(0x10)
> andi $r4,$r4,0x1
> jirl $r0,$r1,0
>
> <vcpu_is_preempted>:
> pcalau12i $r12,8775(0x2247)
> ld.d $r12,$r12,-472(0xe28)
> jirl $r0,$r12,0
> andi $r0,$r0,0x0
>
> <vcpu_is_preempted_new>:
> pcalau12i $r12,8151(0x1fd7)
> ld.d $r12,$r12,-1008(0xc10)
> bstrpick.d $r12,$r12,0x1a,0x1a
> beqz $r12,188(0xbc) # 900000000024ec60
> pcalau12i $r12,11802(0x2e1a)
> addi.d $r12,$r12,-1400(0xa88)
> ldptr.w $r14,$r12,36(0x24)
> beqz $r14,108(0x6c) # 900000000024ec20
> addi.w $r13,$r0,1(0x1)
> bne $r14,$r13,164(0xa4) # 900000000024ec60
> ldptr.w $r13,$r12,40(0x28)
> bnez $r13,24(0x18) # 900000000024ebdc
> lu12i.w $r14,262144(0x40000)
> ori $r14,$r14,0x4
> cpucfg $r14,$r14
> slli.w $r13,$r14,0x0
> st.w $r14,$r12,40(0x28)
> bstrpick.d $r13,$r13,0x3,0x3
> beqz $r13,128(0x80) # 900000000024ec60
> pcalau12i $r13,8759(0x2237)
> slli.d $r4,$r4,0x3
> addi.d $r13,$r13,-1000(0xc18)
> ldx.d $r13,$r13,$r4
> pcalau12i $r12,5462(0x1556)
> addi.d $r12,$r12,384(0x180)
> add.d $r12,$r13,$r12
> ld.bu $r4,$r12,16(0x10)
> andi $r4,$r4,0x1
> jirl $r0,$r1,0
> andi $r0,$r0,0x0
> andi $r0,$r0,0x0
> andi $r0,$r0,0x0
> andi $r0,$r0,0x0
> andi $r0,$r0,0x0
> lu12i.w $r13,262144(0x40000)
> cpucfg $r13,$r13
> lu12i.w $r15,1237(0x4d5)
> ori $r15,$r15,0x64b
> slli.w $r13,$r13,0x0
> bne $r13,$r15,-124(0x3ff84) # 900000000024ebb8
> addi.w $r13,$r0,1(0x1)
> st.w $r13,$r12,36(0x24)
> b -128(0xfffff80) # 900000000024ebc0
> andi $r0,$r0,0x0
> andi $r0,$r0,0x0
> andi $r0,$r0,0x0
> andi $r0,$r0,0x0
> andi $r0,$r0,0x0
> andi $r0,$r0,0x0
> andi $r0,$r0,0x0
> move $r4,$r0
> jirl $r0,$r1,0
>
> With vcpu_is_preempted(), there is one memory load and one jirl jump,
> with vcpu_is_preempted_new(), there is two memory load and two beq
> compare instructions.
>
> 2. In some scenery such nr_cpus == 1, loongson_vcpu_is_preempted() is
> better than pv_vcpu_is_preempted() even if the preempt feature is enabled.
how about use static key and keep file smp.c untouched?
bool notrace vcpu_is_preempted(int cpu)
{
struct kvm_steal_time *src;
if (!static_branch_unlikely(&virt_preempt_key))
return false;
src = &per_cpu(steal_time, cpu);
return !!(src->preempted & KVM_VCPU_PREEMPTED);
}
it reduces one memory load, here is assembly output:
<vcpu_is_preempted>:
andi $r0,$r0,0x0
move $r4,$r0
jirl $r0,$r1,0
andi $r0,$r0,0x0
pcalau12i $r13,8759(0x2237)
slli.d $r4,$r4,0x3
addi.d $r13,$r13,-1000(0xc18)
ldx.d $r13,$r13,$r4
pcalau12i $r12,5462(0x1556)
addi.d $r12,$r12,384(0x180)
add.d $r12,$r13,$r12
ld.bu $r4,$r12,16(0x10)
andi $r4,$r4,0x1
jirl $r0,$r1,0
Regards
Bibo Mao
>
> Regards
> Bibo Mao
>> Huacai
>>
>>> +EXPORT_SYMBOL(vcpu_is_preempted);
>>> diff --git a/arch/loongarch/kernel/smp.c b/arch/loongarch/kernel/smp.c
>>> index 46036d98da75..f04192fedf8d 100644
>>> --- a/arch/loongarch/kernel/smp.c
>>> +++ b/arch/loongarch/kernel/smp.c
>>> @@ -307,10 +307,16 @@ static void loongson_init_ipi(void)
>>> panic("IPI IRQ request failed\n");
>>> }
>>>
>>> +static bool loongson_vcpu_is_preempted(int cpu)
>>> +{
>>> + return false;
>>> +}
>>> +
>>> struct smp_ops mp_ops = {
>>> .init_ipi = loongson_init_ipi,
>>> .send_ipi_single = loongson_send_ipi_single,
>>> .send_ipi_mask = loongson_send_ipi_mask,
>>> + .vcpu_is_preempted = loongson_vcpu_is_preempted,
>>> };
>>>
>>> static void __init fdt_smp_setup(void)
>>> --
>>> 2.39.3
>>>
>>>
>
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 2/3] LoongArch: Add paravirt support with vcpu_is_preempted()
2025-11-19 2:50 ` Bibo Mao
@ 2025-11-19 7:36 ` Huacai Chen
0 siblings, 0 replies; 18+ messages in thread
From: Huacai Chen @ 2025-11-19 7:36 UTC (permalink / raw)
To: Bibo Mao
Cc: Paolo Bonzini, WANG Xuerui, Peter Zijlstra, Ingo Molnar,
Will Deacon, Boqun Feng, Waiman Long, Juergen Gross, Ajay Kaher,
Alexey Makhalov, Broadcom internal kernel review list, kvm,
loongarch, linux-kernel, virtualization, x86
On Wed, Nov 19, 2025 at 10:53 AM Bibo Mao <maobibo@loongson.cn> wrote:
>
>
>
> On 2025/11/18 下午8:48, Huacai Chen wrote:
> > Hi, Bibo,
> >
> > On Tue, Nov 18, 2025 at 4:07 PM Bibo Mao <maobibo@loongson.cn> wrote:
> >>
> >> Function vcpu_is_preempted() is used to check whether vCPU is preempted
> >> or not. Here add implementation with vcpu_is_preempted() when option
> >> CONFIG_PARAVIRT is enabled.
> >>
> >> Signed-off-by: Bibo Mao <maobibo@loongson.cn>
> >> ---
> >> arch/loongarch/include/asm/smp.h | 1 +
> >> arch/loongarch/include/asm/spinlock.h | 5 +++++
> >> arch/loongarch/kernel/paravirt.c | 16 ++++++++++++++++
> >> arch/loongarch/kernel/smp.c | 6 ++++++
> >> 4 files changed, 28 insertions(+)
> >>
> >> diff --git a/arch/loongarch/include/asm/smp.h b/arch/loongarch/include/asm/smp.h
> >> index 3a47f52959a8..5b37f7bf2060 100644
> >> --- a/arch/loongarch/include/asm/smp.h
> >> +++ b/arch/loongarch/include/asm/smp.h
> >> @@ -18,6 +18,7 @@ struct smp_ops {
> >> void (*init_ipi)(void);
> >> void (*send_ipi_single)(int cpu, unsigned int action);
> >> void (*send_ipi_mask)(const struct cpumask *mask, unsigned int action);
> >> + bool (*vcpu_is_preempted)(int cpu);
> >> };
> >> extern struct smp_ops mp_ops;
> >>
> >> diff --git a/arch/loongarch/include/asm/spinlock.h b/arch/loongarch/include/asm/spinlock.h
> >> index 7cb3476999be..c001cef893aa 100644
> >> --- a/arch/loongarch/include/asm/spinlock.h
> >> +++ b/arch/loongarch/include/asm/spinlock.h
> >> @@ -5,6 +5,11 @@
> >> #ifndef _ASM_SPINLOCK_H
> >> #define _ASM_SPINLOCK_H
> >>
> >> +#ifdef CONFIG_PARAVIRT
> >> +#define vcpu_is_preempted vcpu_is_preempted
> >> +bool vcpu_is_preempted(int cpu);
> >> +#endif
> > Maybe paravirt.h is a better place?
> how about put it in asm/qspinlock.h since it is included by header file
> asm/spinlock.h already?
qspinlock.h is better than spinlock.h
Huacai
>
> >
> >> +
> >> #include <asm/processor.h>
> >> #include <asm/qspinlock.h>
> >> #include <asm/qrwlock.h>
> >> diff --git a/arch/loongarch/kernel/paravirt.c b/arch/loongarch/kernel/paravirt.c
> >> index b1b51f920b23..b99404b6b13f 100644
> >> --- a/arch/loongarch/kernel/paravirt.c
> >> +++ b/arch/loongarch/kernel/paravirt.c
> >> @@ -52,6 +52,13 @@ static u64 paravt_steal_clock(int cpu)
> >> #ifdef CONFIG_SMP
> >> static struct smp_ops native_ops;
> >>
> >> +static bool pv_vcpu_is_preempted(int cpu)
> >> +{
> >> + struct kvm_steal_time *src = &per_cpu(steal_time, cpu);
> >> +
> >> + return !!(src->preempted & KVM_VCPU_PREEMPTED);
> >> +}
> >> +
> >> static void pv_send_ipi_single(int cpu, unsigned int action)
> >> {
> >> int min, old;
> >> @@ -308,6 +315,9 @@ int __init pv_time_init(void)
> >> pr_err("Failed to install cpu hotplug callbacks\n");
> >> return r;
> >> }
> >> +
> >> + if (kvm_para_has_feature(KVM_FEATURE_PREEMPT_HINT))
> >> + mp_ops.vcpu_is_preempted = pv_vcpu_is_preempted;
> >> #endif
> >>
> >> static_call_update(pv_steal_clock, paravt_steal_clock);
> >> @@ -332,3 +342,9 @@ int __init pv_spinlock_init(void)
> >>
> >> return 0;
> >> }
> >> +
> >> +bool notrace vcpu_is_preempted(int cpu)
> >> +{
> >> + return mp_ops.vcpu_is_preempted(cpu);
> >> +}
> >
> > We can simplify the whole patch like this, then we don't need to touch
> > smp.c, and we can merge Patch-2/3.
> >
> > +bool notrace vcpu_is_preempted(int cpu)
> > +{
> > + if (!kvm_para_has_feature(KVM_FEATURE_PREEMPT_HINT))
> > + return false;
> > + else {
> > + struct kvm_steal_time *src = &per_cpu(steal_time, cpu);
> > + return !!(src->preempted & KVM_VCPU_PREEMPTED);
> > + }
> > +}
> > Huacai
> >
> >> +EXPORT_SYMBOL(vcpu_is_preempted);
> >> diff --git a/arch/loongarch/kernel/smp.c b/arch/loongarch/kernel/smp.c
> >> index 46036d98da75..f04192fedf8d 100644
> >> --- a/arch/loongarch/kernel/smp.c
> >> +++ b/arch/loongarch/kernel/smp.c
> >> @@ -307,10 +307,16 @@ static void loongson_init_ipi(void)
> >> panic("IPI IRQ request failed\n");
> >> }
> >>
> >> +static bool loongson_vcpu_is_preempted(int cpu)
> >> +{
> >> + return false;
> >> +}
> >> +
> >> struct smp_ops mp_ops = {
> >> .init_ipi = loongson_init_ipi,
> >> .send_ipi_single = loongson_send_ipi_single,
> >> .send_ipi_mask = loongson_send_ipi_mask,
> >> + .vcpu_is_preempted = loongson_vcpu_is_preempted,
> >> };
> >>
> >> static void __init fdt_smp_setup(void)
> >> --
> >> 2.39.3
> >>
> >>
>
>
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 2/3] LoongArch: Add paravirt support with vcpu_is_preempted()
2025-11-19 6:09 ` Bibo Mao
@ 2025-11-19 7:41 ` Huacai Chen
0 siblings, 0 replies; 18+ messages in thread
From: Huacai Chen @ 2025-11-19 7:41 UTC (permalink / raw)
To: Bibo Mao
Cc: Paolo Bonzini, WANG Xuerui, Peter Zijlstra, Ingo Molnar,
Will Deacon, Boqun Feng, Waiman Long, Juergen Gross, Ajay Kaher,
Alexey Makhalov, Broadcom internal kernel review list, kvm,
loongarch, linux-kernel, virtualization, x86
On Wed, Nov 19, 2025 at 2:12 PM Bibo Mao <maobibo@loongson.cn> wrote:
>
>
>
> On 2025/11/19 上午9:59, Bibo Mao wrote:
> >
> >
> > On 2025/11/18 下午8:48, Huacai Chen wrote:
> >> Hi, Bibo,
> >>
> >> On Tue, Nov 18, 2025 at 4:07 PM Bibo Mao <maobibo@loongson.cn> wrote:
> >>>
> >>> Function vcpu_is_preempted() is used to check whether vCPU is preempted
> >>> or not. Here add implementation with vcpu_is_preempted() when option
> >>> CONFIG_PARAVIRT is enabled.
> >>>
> >>> Signed-off-by: Bibo Mao <maobibo@loongson.cn>
> >>> ---
> >>> arch/loongarch/include/asm/smp.h | 1 +
> >>> arch/loongarch/include/asm/spinlock.h | 5 +++++
> >>> arch/loongarch/kernel/paravirt.c | 16 ++++++++++++++++
> >>> arch/loongarch/kernel/smp.c | 6 ++++++
> >>> 4 files changed, 28 insertions(+)
> >>>
> >>> diff --git a/arch/loongarch/include/asm/smp.h
> >>> b/arch/loongarch/include/asm/smp.h
> >>> index 3a47f52959a8..5b37f7bf2060 100644
> >>> --- a/arch/loongarch/include/asm/smp.h
> >>> +++ b/arch/loongarch/include/asm/smp.h
> >>> @@ -18,6 +18,7 @@ struct smp_ops {
> >>> void (*init_ipi)(void);
> >>> void (*send_ipi_single)(int cpu, unsigned int action);
> >>> void (*send_ipi_mask)(const struct cpumask *mask, unsigned
> >>> int action);
> >>> + bool (*vcpu_is_preempted)(int cpu);
> >>> };
> >>> extern struct smp_ops mp_ops;
> >>>
> >>> diff --git a/arch/loongarch/include/asm/spinlock.h
> >>> b/arch/loongarch/include/asm/spinlock.h
> >>> index 7cb3476999be..c001cef893aa 100644
> >>> --- a/arch/loongarch/include/asm/spinlock.h
> >>> +++ b/arch/loongarch/include/asm/spinlock.h
> >>> @@ -5,6 +5,11 @@
> >>> #ifndef _ASM_SPINLOCK_H
> >>> #define _ASM_SPINLOCK_H
> >>>
> >>> +#ifdef CONFIG_PARAVIRT
> >>> +#define vcpu_is_preempted vcpu_is_preempted
> >>> +bool vcpu_is_preempted(int cpu);
> >>> +#endif
> >> Maybe paravirt.h is a better place?
> >
> > It is actually a little strange to add macro CONFIG_PARAVIRT in file
> > asm/spinlock.h
> >
> > vcpu_is_preempted is originally defined in header file
> > include/linux/sched.h like this
> > #ifndef vcpu_is_preempted
> > static inline bool vcpu_is_preempted(int cpu)
> > {
> > return false;
> > }
> > #endif
> >
> > that requires that header file is included before sched.h, file
> > asm/spinlock.h can meet this requirement, however header file paravirt.h
> > maybe it is not included before sched.h in generic.
> >
> > Here vcpu_is_preempted definition is added before the following including.
> > #include <asm/processor.h>
> > #include <asm/qspinlock.h>
> > #include <asm/qrwlock.h>
> > Maybe it is better to be added after the above header files including
> > sentences, but need further investigation.
> >>
> >>> +
> >>> #include <asm/processor.h>
> >>> #include <asm/qspinlock.h>
> >>> #include <asm/qrwlock.h>
> >>> diff --git a/arch/loongarch/kernel/paravirt.c
> >>> b/arch/loongarch/kernel/paravirt.c
> >>> index b1b51f920b23..b99404b6b13f 100644
> >>> --- a/arch/loongarch/kernel/paravirt.c
> >>> +++ b/arch/loongarch/kernel/paravirt.c
> >>> @@ -52,6 +52,13 @@ static u64 paravt_steal_clock(int cpu)
> >>> #ifdef CONFIG_SMP
> >>> static struct smp_ops native_ops;
> >>>
> >>> +static bool pv_vcpu_is_preempted(int cpu)
> >>> +{
> >>> + struct kvm_steal_time *src = &per_cpu(steal_time, cpu);
> >>> +
> >>> + return !!(src->preempted & KVM_VCPU_PREEMPTED);
> >>> +}
> >>> +
> >>> static void pv_send_ipi_single(int cpu, unsigned int action)
> >>> {
> >>> int min, old;
> >>> @@ -308,6 +315,9 @@ int __init pv_time_init(void)
> >>> pr_err("Failed to install cpu hotplug callbacks\n");
> >>> return r;
> >>> }
> >>> +
> >>> + if (kvm_para_has_feature(KVM_FEATURE_PREEMPT_HINT))
> >>> + mp_ops.vcpu_is_preempted = pv_vcpu_is_preempted;
> >>> #endif
> >>>
> >>> static_call_update(pv_steal_clock, paravt_steal_clock);
> >>> @@ -332,3 +342,9 @@ int __init pv_spinlock_init(void)
> >>>
> >>> return 0;
> >>> }
> >>> +
> >>> +bool notrace vcpu_is_preempted(int cpu)
> >>> +{
> >>> + return mp_ops.vcpu_is_preempted(cpu);
> >>> +}
> >>
> >> We can simplify the whole patch like this, then we don't need to touch
> >> smp.c, and we can merge Patch-2/3.
> >>
> >> +bool notrace vcpu_is_preempted(int cpu)
> >> +{
> >> + if (!kvm_para_has_feature(KVM_FEATURE_PREEMPT_HINT))
> >> + return false;
> >> + else {
> >> + struct kvm_steal_time *src = &per_cpu(steal_time, cpu);
> >> + return !!(src->preempted & KVM_VCPU_PREEMPTED);
> >> + }
> >> +}
> > 1. there is assembly output about relative vcpu_is_preempted
> > <loongson_vcpu_is_preempted>:
> > move $r4,$r0
> > jirl $r0,$r1,0
> >
> > <pv_vcpu_is_preempted>:
> > pcalau12i $r13,8759(0x2237)
> > slli.d $r4,$r4,0x3
> > addi.d $r13,$r13,-1000(0xc18)
> > ldx.d $r13,$r13,$r4
> > pcalau12i $r12,5462(0x1556)
> > addi.d $r12,$r12,384(0x180)
> > add.d $r12,$r13,$r12
> > ld.bu $r4,$r12,16(0x10)
> > andi $r4,$r4,0x1
> > jirl $r0,$r1,0
> >
> > <vcpu_is_preempted>:
> > pcalau12i $r12,8775(0x2247)
> > ld.d $r12,$r12,-472(0xe28)
> > jirl $r0,$r12,0
> > andi $r0,$r0,0x0
> >
> > <vcpu_is_preempted_new>:
> > pcalau12i $r12,8151(0x1fd7)
> > ld.d $r12,$r12,-1008(0xc10)
> > bstrpick.d $r12,$r12,0x1a,0x1a
> > beqz $r12,188(0xbc) # 900000000024ec60
> > pcalau12i $r12,11802(0x2e1a)
> > addi.d $r12,$r12,-1400(0xa88)
> > ldptr.w $r14,$r12,36(0x24)
> > beqz $r14,108(0x6c) # 900000000024ec20
> > addi.w $r13,$r0,1(0x1)
> > bne $r14,$r13,164(0xa4) # 900000000024ec60
> > ldptr.w $r13,$r12,40(0x28)
> > bnez $r13,24(0x18) # 900000000024ebdc
> > lu12i.w $r14,262144(0x40000)
> > ori $r14,$r14,0x4
> > cpucfg $r14,$r14
> > slli.w $r13,$r14,0x0
> > st.w $r14,$r12,40(0x28)
> > bstrpick.d $r13,$r13,0x3,0x3
> > beqz $r13,128(0x80) # 900000000024ec60
> > pcalau12i $r13,8759(0x2237)
> > slli.d $r4,$r4,0x3
> > addi.d $r13,$r13,-1000(0xc18)
> > ldx.d $r13,$r13,$r4
> > pcalau12i $r12,5462(0x1556)
> > addi.d $r12,$r12,384(0x180)
> > add.d $r12,$r13,$r12
> > ld.bu $r4,$r12,16(0x10)
> > andi $r4,$r4,0x1
> > jirl $r0,$r1,0
> > andi $r0,$r0,0x0
> > andi $r0,$r0,0x0
> > andi $r0,$r0,0x0
> > andi $r0,$r0,0x0
> > andi $r0,$r0,0x0
> > lu12i.w $r13,262144(0x40000)
> > cpucfg $r13,$r13
> > lu12i.w $r15,1237(0x4d5)
> > ori $r15,$r15,0x64b
> > slli.w $r13,$r13,0x0
> > bne $r13,$r15,-124(0x3ff84) # 900000000024ebb8
> > addi.w $r13,$r0,1(0x1)
> > st.w $r13,$r12,36(0x24)
> > b -128(0xfffff80) # 900000000024ebc0
> > andi $r0,$r0,0x0
> > andi $r0,$r0,0x0
> > andi $r0,$r0,0x0
> > andi $r0,$r0,0x0
> > andi $r0,$r0,0x0
> > andi $r0,$r0,0x0
> > andi $r0,$r0,0x0
> > move $r4,$r0
> > jirl $r0,$r1,0
> >
> > With vcpu_is_preempted(), there is one memory load and one jirl jump,
> > with vcpu_is_preempted_new(), there is two memory load and two beq
> > compare instructions.
> >
> > 2. In some scenery such nr_cpus == 1, loongson_vcpu_is_preempted() is
> > better than pv_vcpu_is_preempted() even if the preempt feature is enabled.
> how about use static key and keep file smp.c untouched?
OK, it's better.
Huacai
> bool notrace vcpu_is_preempted(int cpu)
> {
> struct kvm_steal_time *src;
>
> if (!static_branch_unlikely(&virt_preempt_key))
> return false;
>
> src = &per_cpu(steal_time, cpu);
> return !!(src->preempted & KVM_VCPU_PREEMPTED);
> }
>
> it reduces one memory load, here is assembly output:
> <vcpu_is_preempted>:
> andi $r0,$r0,0x0
> move $r4,$r0
> jirl $r0,$r1,0
> andi $r0,$r0,0x0
> pcalau12i $r13,8759(0x2237)
> slli.d $r4,$r4,0x3
> addi.d $r13,$r13,-1000(0xc18)
> ldx.d $r13,$r13,$r4
> pcalau12i $r12,5462(0x1556)
> addi.d $r12,$r12,384(0x180)
> add.d $r12,$r13,$r12
> ld.bu $r4,$r12,16(0x10)
> andi $r4,$r4,0x1
> jirl $r0,$r1,0
>
> Regards
> Bibo Mao
>
> >
> > Regards
> > Bibo Mao
> >> Huacai
> >>
> >>> +EXPORT_SYMBOL(vcpu_is_preempted);
> >>> diff --git a/arch/loongarch/kernel/smp.c b/arch/loongarch/kernel/smp.c
> >>> index 46036d98da75..f04192fedf8d 100644
> >>> --- a/arch/loongarch/kernel/smp.c
> >>> +++ b/arch/loongarch/kernel/smp.c
> >>> @@ -307,10 +307,16 @@ static void loongson_init_ipi(void)
> >>> panic("IPI IRQ request failed\n");
> >>> }
> >>>
> >>> +static bool loongson_vcpu_is_preempted(int cpu)
> >>> +{
> >>> + return false;
> >>> +}
> >>> +
> >>> struct smp_ops mp_ops = {
> >>> .init_ipi = loongson_init_ipi,
> >>> .send_ipi_single = loongson_send_ipi_single,
> >>> .send_ipi_mask = loongson_send_ipi_mask,
> >>> + .vcpu_is_preempted = loongson_vcpu_is_preempted,
> >>> };
> >>>
> >>> static void __init fdt_smp_setup(void)
> >>> --
> >>> 2.39.3
> >>>
> >>>
> >
>
>
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 2/3] LoongArch: Add paravirt support with vcpu_is_preempted()
2025-11-18 8:06 ` [PATCH 2/3] LoongArch: Add paravirt support with vcpu_is_preempted() Bibo Mao
2025-11-18 12:48 ` Huacai Chen
@ 2025-11-20 2:51 ` kernel test robot
1 sibling, 0 replies; 18+ messages in thread
From: kernel test robot @ 2025-11-20 2:51 UTC (permalink / raw)
To: Bibo Mao, Paolo Bonzini, Huacai Chen, WANG Xuerui, Peter Zijlstra,
Ingo Molnar, Will Deacon, Boqun Feng, Waiman Long, Juergen Gross,
Ajay Kaher, Alexey Makhalov, Broadcom internal kernel review list
Cc: oe-kbuild-all, kvm, loongarch, linux-kernel, virtualization, x86
Hi Bibo,
kernel test robot noticed the following build errors:
[auto build test ERROR on 6a23ae0a96a600d1d12557add110e0bb6e32730c]
url: https://github.com/intel-lab-lkp/linux/commits/Bibo-Mao/LoongArch-KVM-Add-preempt-hint-feature-in-hypervisor-side/20251118-161212
base: 6a23ae0a96a600d1d12557add110e0bb6e32730c
patch link: https://lore.kernel.org/r/20251118080656.2012805-3-maobibo%40loongson.cn
patch subject: [PATCH 2/3] LoongArch: Add paravirt support with vcpu_is_preempted()
config: loongarch-randconfig-r052-20251120 (https://download.01.org/0day-ci/archive/20251120/202511201009.WLpYNMAM-lkp@intel.com/config)
compiler: clang version 18.1.8 (https://github.com/llvm/llvm-project 3b5b5c1ec4a3095ab096dd780e84d7ab81f3d7ff)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20251120/202511201009.WLpYNMAM-lkp@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202511201009.WLpYNMAM-lkp@intel.com/
All errors (new ones prefixed by >>):
>> arch/loongarch/kernel/paravirt.c:346:14: error: redefinition of 'vcpu_is_preempted'
346 | bool notrace vcpu_is_preempted(int cpu)
| ^
include/linux/sched.h:2263:20: note: previous definition is here
2263 | static inline bool vcpu_is_preempted(int cpu)
| ^
>> arch/loongarch/kernel/paravirt.c:348:9: error: use of undeclared identifier 'mp_ops'
348 | return mp_ops.vcpu_is_preempted(cpu);
| ^
2 errors generated.
vim +/vcpu_is_preempted +346 arch/loongarch/kernel/paravirt.c
345
> 346 bool notrace vcpu_is_preempted(int cpu)
347 {
> 348 return mp_ops.vcpu_is_preempted(cpu);
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
^ permalink raw reply [flat|nested] 18+ messages in thread
end of thread, other threads:[~2025-11-20 2:52 UTC | newest]
Thread overview: 18+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-11-18 8:06 [PATCH 0/3] LoongArch: KVM: Add paravirt preempt hint support Bibo Mao
2025-11-18 8:06 ` [PATCH 1/3] LoongArch: KVM: Add preempt hint feature in hypervisor side Bibo Mao
2025-11-18 12:46 ` Huacai Chen
2025-11-19 1:20 ` Bibo Mao
2025-11-19 2:45 ` Huacai Chen
2025-11-19 2:55 ` Bibo Mao
2025-11-19 3:01 ` Huacai Chen
2025-11-18 8:06 ` [PATCH 2/3] LoongArch: Add paravirt support with vcpu_is_preempted() Bibo Mao
2025-11-18 12:48 ` Huacai Chen
2025-11-19 1:59 ` Bibo Mao
2025-11-19 2:58 ` Huacai Chen
2025-11-19 3:08 ` Bibo Mao
2025-11-19 6:09 ` Bibo Mao
2025-11-19 7:41 ` Huacai Chen
2025-11-19 2:50 ` Bibo Mao
2025-11-19 7:36 ` Huacai Chen
2025-11-20 2:51 ` kernel test robot
2025-11-18 8:06 ` [PATCH 3/3] LoongArch: Add paravirt preempt hint print prompt Bibo Mao
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox