| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <1e64a738-5197-8bd2-5977-9d95bdf61a2d@loongson.cn>
Date: Tue, 30 Apr 2024 17:21:53 +0800
From: maobibo <maobibo@...ngson.cn>
To: Huacai Chen <chenhuacai@...nel.org>
Cc: Tianrui Zhao <zhaotianrui@...ngson.cn>, Juergen Gross <jgross@...e.com>,
kvm@...r.kernel.org, loongarch@...ts.linux.dev,
linux-kernel@...r.kernel.org, x86@...nel.org, virtualization@...ts.linux.dev
Subject: Re: [PATCH v2 2/2] LoongArch: Add steal time support in guest side
On 2024/4/30 下午4:23, Huacai Chen wrote:
> Hi, Bibo,
>
> On Tue, Apr 30, 2024 at 9:45 AM Bibo Mao <maobibo@...ngson.cn> wrote:
>>
>> Percpu struct kvm_steal_time is added here, its size is 64 bytes and
>> also defined as 64 bytes, so that the whole structure is in one physical
>> page.
>>
>> When vcpu is onlined, function pv_enable_steal_time() is called. This
>> function will pass guest physical address of struct kvm_steal_time and
>> tells hypervisor to enable steal time. When vcpu is offline, physical
>> address is set as 0 and tells hypervisor to disable steal time.
>>
>> Here is output of vmstat on guest when there is workload on both host
>> and guest. It includes steal time stat information.
>>
>> procs -----------memory---------- -----io---- -system-- ------cpu-----
>> r b swpd free inact active bi bo in cs us sy id wa st
>> 15 1 0 7583616 184112 72208 20 0 162 52 31 6 43 0 20
>> 17 0 0 7583616 184704 72192 0 0 6318 6885 5 60 8 5 22
>> 16 0 0 7583616 185392 72144 0 0 1766 1081 0 49 0 1 50
>> 16 0 0 7583616 184816 72304 0 0 6300 6166 4 62 12 2 20
>> 18 0 0 7583632 184480 72240 0 0 2814 1754 2 58 4 1 35
>>
>> Signed-off-by: Bibo Mao <maobibo@...ngson.cn>
>> ---
>> arch/loongarch/Kconfig | 11 +++
>> arch/loongarch/include/asm/paravirt.h | 5 +
>> arch/loongarch/kernel/paravirt.c | 131 ++++++++++++++++++++++++++
>> arch/loongarch/kernel/time.c | 2 +
>> 4 files changed, 149 insertions(+)
>>
>> diff --git a/arch/loongarch/Kconfig b/arch/loongarch/Kconfig
>> index 0a1540a8853e..f3a03c33a052 100644
>> --- a/arch/loongarch/Kconfig
>> +++ b/arch/loongarch/Kconfig
>> @@ -592,6 +592,17 @@ config PARAVIRT
>> over full virtualization. However, when run without a hypervisor
>> the kernel is theoretically slower and slightly larger.
>>
>> +config PARAVIRT_TIME_ACCOUNTING
>> + bool "Paravirtual steal time accounting"
>> + select PARAVIRT
>> + help
>> + Select this option to enable fine granularity task steal time
>> + accounting. Time spent executing other tasks in parallel with
>> + the current vCPU is discounted from the vCPU power. To account for
>> + that, there can be a small performance impact.
>> +
>> + If in doubt, say N here.
>> +
> Can we use a hidden selection manner, which means:
>
> config PARAVIRT_TIME_ACCOUNTING
> def_bool PARAVIRT
>
> Because I think we needn't give too many choices to users (and bring
> much more effort to test).
>
> PowerPC even hide all the PARAVIRT config options...
> see arch/powerpc/platforms/pseries/Kconfig
I do not know neither :( It is just used at beginning, maybe there is
negative effect or potential issue, I just think that it can be selected
by user for easily debugging. After it is matured, hidden selection
manner can be used.
Regards
Bibo Mao
>
> Huacai
>
>> config ARCH_SUPPORTS_KEXEC
>> def_bool y
>>
>> diff --git a/arch/loongarch/include/asm/paravirt.h b/arch/loongarch/include/asm/paravirt.h
>> index 58f7b7b89f2c..fe27fb5e82b8 100644
>> --- a/arch/loongarch/include/asm/paravirt.h
>> +++ b/arch/loongarch/include/asm/paravirt.h
>> @@ -17,11 +17,16 @@ static inline u64 paravirt_steal_clock(int cpu)
>> }
>>
>> int pv_ipi_init(void);
>> +int __init pv_time_init(void);
>> #else
>> static inline int pv_ipi_init(void)
>> {
>> return 0;
>> }
>>
>> +static inline int pv_time_init(void)
>> +{
>> + return 0;
>> +}
>> #endif // CONFIG_PARAVIRT
>> #endif
>> diff --git a/arch/loongarch/kernel/paravirt.c b/arch/loongarch/kernel/paravirt.c
>> index 9044ed62045c..3f83afc7e2d2 100644
>> --- a/arch/loongarch/kernel/paravirt.c
>> +++ b/arch/loongarch/kernel/paravirt.c
>> @@ -5,10 +5,13 @@
>> #include <linux/jump_label.h>
>> #include <linux/kvm_para.h>
>> #include <asm/paravirt.h>
>> +#include <linux/reboot.h>
>> #include <linux/static_call.h>
>>
>> struct static_key paravirt_steal_enabled;
>> struct static_key paravirt_steal_rq_enabled;
>> +static DEFINE_PER_CPU(struct kvm_steal_time, steal_time) __aligned(64);
>> +static int has_steal_clock;
>>
>> static u64 native_steal_clock(int cpu)
>> {
>> @@ -17,6 +20,57 @@ static u64 native_steal_clock(int cpu)
>>
>> DEFINE_STATIC_CALL(pv_steal_clock, native_steal_clock);
>>
>> +static bool steal_acc = true;
>> +static int __init parse_no_stealacc(char *arg)
>> +{
>> + steal_acc = false;
>> + return 0;
>> +}
>> +early_param("no-steal-acc", parse_no_stealacc);
>> +
>> +static u64 para_steal_clock(int cpu)
>> +{
>> + u64 steal;
>> + struct kvm_steal_time *src;
>> + int version;
>> +
>> + src = &per_cpu(steal_time, cpu);
>> + do {
>> +
>> + version = src->version;
>> + /* Make sure that the version is read before the steal */
>> + virt_rmb();
>> + steal = src->steal;
>> + /* Make sure that the steal is read before the next version */
>> + virt_rmb();
>> +
>> + } while ((version & 1) || (version != src->version));
>> + return steal;
>> +}
>> +
>> +static int pv_enable_steal_time(void)
>> +{
>> + int cpu = smp_processor_id();
>> + struct kvm_steal_time *st;
>> + unsigned long addr;
>> +
>> + if (!has_steal_clock)
>> + return -EPERM;
>> +
>> + st = &per_cpu(steal_time, cpu);
>> + addr = per_cpu_ptr_to_phys(st);
>> +
>> + /* The whole structure kvm_steal_time should be one page */
>> + if (PFN_DOWN(addr) != PFN_DOWN(addr + sizeof(*st))) {
>> + pr_warn("Illegal PV steal time addr %lx\n", addr);
>> + return -EFAULT;
>> + }
>> +
>> + addr |= KVM_STEAL_PHYS_VALID;
>> + kvm_hypercall2(KVM_HCALL_FUNC_NOTIFY, KVM_FEATURE_STEAL_TIME, addr);
>> + return 0;
>> +}
>> +
>> #ifdef CONFIG_SMP
>> static void pv_send_ipi_single(int cpu, unsigned int action)
>> {
>> @@ -110,6 +164,32 @@ static void pv_init_ipi(void)
>> if (r < 0)
>> panic("SWI0 IRQ request failed\n");
>> }
>> +
>> +static void pv_disable_steal_time(void)
>> +{
>> + if (has_steal_clock)
>> + kvm_hypercall2(KVM_HCALL_FUNC_NOTIFY, KVM_FEATURE_STEAL_TIME, 0);
>> +}
>> +
>> +static int pv_time_cpu_online(unsigned int cpu)
>> +{
>> + unsigned long flags;
>> +
>> + local_irq_save(flags);
>> + pv_enable_steal_time();
>> + local_irq_restore(flags);
>> + return 0;
>> +}
>> +
>> +static int pv_time_cpu_down_prepare(unsigned int cpu)
>> +{
>> + unsigned long flags;
>> +
>> + local_irq_save(flags);
>> + pv_disable_steal_time();
>> + local_irq_restore(flags);
>> + return 0;
>> +}
>> #endif
>>
>> static bool kvm_para_available(void)
>> @@ -149,3 +229,54 @@ int __init pv_ipi_init(void)
>>
>> return 1;
>> }
>> +
>> +static void pv_cpu_reboot(void *unused)
>> +{
>> + pv_disable_steal_time();
>> +}
>> +
>> +static int pv_reboot_notify(struct notifier_block *nb, unsigned long code,
>> + void *unused)
>> +{
>> + on_each_cpu(pv_cpu_reboot, NULL, 1);
>> + return NOTIFY_DONE;
>> +}
>> +
>> +static struct notifier_block pv_reboot_nb = {
>> + .notifier_call = pv_reboot_notify,
>> +};
>> +
>> +int __init pv_time_init(void)
>> +{
>> + int feature;
>> +
>> + if (!cpu_has_hypervisor)
>> + return 0;
>> + if (!kvm_para_available())
>> + return 0;
>> +
>> + feature = read_cpucfg(CPUCFG_KVM_FEATURE);
>> + if (!(feature & KVM_FEATURE_STEAL_TIME))
>> + return 0;
>> +
>> + has_steal_clock = 1;
>> + if (pv_enable_steal_time()) {
>> + has_steal_clock = 0;
>> + return 0;
>> + }
>> +
>> + register_reboot_notifier(&pv_reboot_nb);
>> + static_call_update(pv_steal_clock, para_steal_clock);
>> + static_key_slow_inc(¶virt_steal_enabled);
>> + if (steal_acc)
>> + static_key_slow_inc(¶virt_steal_rq_enabled);
>> +
>> +#ifdef CONFIG_SMP
>> + if (cpuhp_setup_state_nocalls(CPUHP_AP_ONLINE_DYN,
>> + "loongarch/pvi_time:online",
>> + pv_time_cpu_online, pv_time_cpu_down_prepare) < 0)
>> + pr_err("Failed to install cpu hotplug callbacks\n");
>> +#endif
>> + pr_info("Using stolen time PV\n");
>> + return 0;
>> +}
>> diff --git a/arch/loongarch/kernel/time.c b/arch/loongarch/kernel/time.c
>> index fd5354f9be7c..46d7d40c87e3 100644
>> --- a/arch/loongarch/kernel/time.c
>> +++ b/arch/loongarch/kernel/time.c
>> @@ -15,6 +15,7 @@
>>
>> #include <asm/cpu-features.h>
>> #include <asm/loongarch.h>
>> +#include <asm/paravirt.h>
>> #include <asm/time.h>
>>
>> u64 cpu_clock_freq;
>> @@ -214,4 +215,5 @@ void __init time_init(void)
>>
>> constant_clockevent_init();
>> constant_clocksource_init();
>> + pv_time_init();
>> }
>> --
>> 2.39.3
>>
>>
Powered by blists - more mailing lists