[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <718b5b5d-2bb1-5d59-409e-54f54516a6b7@loongson.cn>
Date: Mon, 24 Nov 2025 15:48:09 +0800
From: Bibo Mao <maobibo@...ngson.cn>
To: Huacai Chen <chenhuacai@...nel.org>
Cc: Paolo Bonzini <pbonzini@...hat.com>, WANG Xuerui <kernel@...0n.name>,
Juergen Gross <jgross@...e.com>, Ajay Kaher <ajay.kaher@...adcom.com>,
Alexey Makhalov <alexey.makhalov@...adcom.com>,
Broadcom internal kernel review list
<bcm-kernel-feedback-list@...adcom.com>, kvm@...r.kernel.org,
loongarch@...ts.linux.dev, linux-kernel@...r.kernel.org,
virtualization@...ts.linux.dev, x86@...nel.org
Subject: Re: [PATCH v2 2/3] LoongArch: Add paravirt support with
vcpu_is_preempted() in guest side
On 2025/11/24 下午3:13, Huacai Chen wrote:
> On Mon, Nov 24, 2025 at 3:03 PM Bibo Mao <maobibo@...ngson.cn> wrote:
>>
>>
>>
>> On 2025/11/24 下午2:33, Huacai Chen wrote:
>>> Hi, Bibo,
>>>
>>> On Mon, Nov 24, 2025 at 11:54 AM Bibo Mao <maobibo@...ngson.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@...ngson.cn>
>>>> ---
>>>> arch/loongarch/include/asm/qspinlock.h | 5 +++++
>>>> arch/loongarch/kernel/paravirt.c | 16 ++++++++++++++++
>>>> 2 files changed, 21 insertions(+)
>>>>
>>>> diff --git a/arch/loongarch/include/asm/qspinlock.h b/arch/loongarch/include/asm/qspinlock.h
>>>> index e76d3aa1e1eb..9a5b7ba1f4cb 100644
>>>> --- a/arch/loongarch/include/asm/qspinlock.h
>>>> +++ b/arch/loongarch/include/asm/qspinlock.h
>>>> @@ -34,6 +34,11 @@ static inline bool virt_spin_lock(struct qspinlock *lock)
>>>> return true;
>>>> }
>>>>
>>>> +#ifdef CONFIG_SMP
>>>> +#define vcpu_is_preempted vcpu_is_preempted
>>>> +bool vcpu_is_preempted(int cpu);
>>> In V1 there is a build error because you reference mp_ops, so in V2
>>> you needn't put it in CONFIG_SMP.
>> The compile failure problem is that vcpu_is_preempted() is redefined in
>> both arch/loongarch/kernel/paravirt.c and include/linux/sched.h
> But other archs don't define vcpu_is_preempted() under CONFIG_SMP, and
so what is advantage to implement this function if CONFIG_SMP is disabled?
> you can consider to inline the whole vcpu_is_preempted() here.
Defining the function vcpu_is_preempted() as inlined is not so easy for
me, it beyond my ability now :(
With static key method, the static key need be exported, all modules
need apply the jump label, that is dangerous and I doubt whether it is
deserved.
Regards
Bibo Mao
>
>>
>> The problem is that <asm/spinlock.h> is not included by sched.h, if
>> CONFIG_SMP is disabled. Here is part of file include/linux/spinlock.h
>> #ifdef CONFIG_SMP
>> # include <asm/spinlock.h>
>> #else
>> # include <linux/spinlock_up.h>
>> #endif
>>
>>> On the other hand, even if you really build a UP guest kernel, when
>>> multiple guests run together, you probably need vcpu_is_preemtped.
>> It is not relative with multiple VMs. When vcpu_is_preempted() is
>> called, it is to detect whether dest CPU is preempted or not, the cpu
>> from smp_processor_id() should not be preempted. So in generic
>> vcpu_is_preempted() works on multiple vCPUs.
> OK, I'm wrong here.
>
>
> Huacai
>
>>
>> Regards
>> Bibo Mao
>>>
>>>
>>> Huacai
>>>
>>>> +#endif
>>>> +
>>>> #endif /* CONFIG_PARAVIRT */
>>>>
>>>> #include <asm-generic/qspinlock.h>
>>>> diff --git a/arch/loongarch/kernel/paravirt.c b/arch/loongarch/kernel/paravirt.c
>>>> index b1b51f920b23..d4163679adc4 100644
>>>> --- a/arch/loongarch/kernel/paravirt.c
>>>> +++ b/arch/loongarch/kernel/paravirt.c
>>>> @@ -246,6 +246,7 @@ static void pv_disable_steal_time(void)
>>>> }
>>>>
>>>> #ifdef CONFIG_SMP
>>>> +DEFINE_STATIC_KEY_FALSE(virt_preempt_key);
>>>> static int pv_time_cpu_online(unsigned int cpu)
>>>> {
>>>> unsigned long flags;
>>>> @@ -267,6 +268,18 @@ static int pv_time_cpu_down_prepare(unsigned int cpu)
>>>>
>>>> return 0;
>>>> }
>>>> +
>>>> +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);
>>>> +}
>>>> +EXPORT_SYMBOL(vcpu_is_preempted);
>>>> #endif
>>>>
>>>> static void pv_cpu_reboot(void *unused)
>>>> @@ -308,6 +321,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))
>>>> + static_branch_enable(&virt_preempt_key);
>>>> #endif
>>>>
>>>> static_call_update(pv_steal_clock, paravt_steal_clock);
>>>> --
>>>> 2.39.3
>>>>
>>
>>
Powered by blists - more mailing lists