[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <4240d67f-1e5d-f865-c16e-32b64aea8099@loongson.cn>
Date: Wed, 3 Jan 2024 16:00:41 +0800
From: maobibo <maobibo@...ngson.cn>
To: Jürgen Groß <jgross@...e.com>,
Huacai Chen <chenhuacai@...nel.org>, Tianrui Zhao <zhaotianrui@...ngson.cn>
Cc: loongarch@...ts.linux.dev, linux-kernel@...r.kernel.org,
virtualization@...ts.linux.dev, kvm@...r.kernel.org
Subject: Re: [PATCH 4/5] LoongArch: Add paravirt interface for guest kernel
On 2024/1/3 下午3:40, Jürgen Groß wrote:
> On 03.01.24 08:16, Bibo Mao wrote:
>> The patch add paravirt interface for guest kernel, it checks whether
>> system runs on VM mode. If it is, it will detect hypervisor type. And
>> returns true it is KVM hypervisor, else return false. Currently only
>> KVM hypervisor is supported, so there is only hypervisor detection
>> for KVM type.
>
> I guess you are talking of pv_guest_init() here? Or do you mean
> kvm_para_available()?
yes, it is pv_guest_init. It will be better if all hypervisor detection
is called in function pv_guest_init. Currently there is only kvm
hypervisor, kvm_para_available is hard-coded in pv_guest_init here.
I can split file paravirt.c into paravirt.c and kvm.c, paravirt.c is
used for hypervisor detection, and move code relative with pv_ipi into kvm.c
Regards
Bibo Mao
>
>>
>> Signed-off-by: Bibo Mao <maobibo@...ngson.cn>
>> ---
>> arch/loongarch/Kconfig | 8 ++++
>> arch/loongarch/include/asm/kvm_para.h | 7 ++++
>> arch/loongarch/include/asm/paravirt.h | 27 ++++++++++++
>> .../include/asm/paravirt_api_clock.h | 1 +
>> arch/loongarch/kernel/Makefile | 1 +
>> arch/loongarch/kernel/paravirt.c | 41 +++++++++++++++++++
>> arch/loongarch/kernel/setup.c | 2 +
>> 7 files changed, 87 insertions(+)
>> create mode 100644 arch/loongarch/include/asm/paravirt.h
>> create mode 100644 arch/loongarch/include/asm/paravirt_api_clock.h
>> create mode 100644 arch/loongarch/kernel/paravirt.c
>>
>> diff --git a/arch/loongarch/Kconfig b/arch/loongarch/Kconfig
>> index ee123820a476..940e5960d297 100644
>> --- a/arch/loongarch/Kconfig
>> +++ b/arch/loongarch/Kconfig
>> @@ -564,6 +564,14 @@ config CPU_HAS_PREFETCH
>> bool
>> default y
>> +config PARAVIRT
>> + bool "Enable paravirtualization code"
>> + help
>> + This changes the kernel so it can modify itself when it is run
>> + under a hypervisor, potentially improving performance
>> significantly
>> + over full virtualization. However, when run without a hypervisor
>> + the kernel is theoretically slower and slightly larger.
>> +
>> config ARCH_SUPPORTS_KEXEC
>> def_bool y
>> diff --git a/arch/loongarch/include/asm/kvm_para.h
>> b/arch/loongarch/include/asm/kvm_para.h
>> index 9425d3b7e486..41200e922a82 100644
>> --- a/arch/loongarch/include/asm/kvm_para.h
>> +++ b/arch/loongarch/include/asm/kvm_para.h
>> @@ -2,6 +2,13 @@
>> #ifndef _ASM_LOONGARCH_KVM_PARA_H
>> #define _ASM_LOONGARCH_KVM_PARA_H
>> +/*
>> + * Hypcall code field
>> + */
>> +#define HYPERVISOR_KVM 1
>> +#define HYPERVISOR_VENDOR_SHIFT 8
>> +#define HYPERCALL_CODE(vendor, code) ((vendor <<
>> HYPERVISOR_VENDOR_SHIFT) + code)
>> +
>> /*
>> * LoongArch hypcall return code
>> */
>> diff --git a/arch/loongarch/include/asm/paravirt.h
>> b/arch/loongarch/include/asm/paravirt.h
>> new file mode 100644
>> index 000000000000..b64813592ba0
>> --- /dev/null
>> +++ b/arch/loongarch/include/asm/paravirt.h
>> @@ -0,0 +1,27 @@
>> +/* SPDX-License-Identifier: GPL-2.0 */
>> +#ifndef _ASM_LOONGARCH_PARAVIRT_H
>> +#define _ASM_LOONGARCH_PARAVIRT_H
>> +
>> +#ifdef CONFIG_PARAVIRT
>> +#include <linux/static_call_types.h>
>> +struct static_key;
>> +extern struct static_key paravirt_steal_enabled;
>> +extern struct static_key paravirt_steal_rq_enabled;
>> +
>> +u64 dummy_steal_clock(int cpu);
>> +DECLARE_STATIC_CALL(pv_steal_clock, dummy_steal_clock);
>> +
>> +static inline u64 paravirt_steal_clock(int cpu)
>> +{
>> + return static_call(pv_steal_clock)(cpu);
>> +}
>> +
>> +int pv_guest_init(void);
>> +#else
>> +static inline int pv_guest_init(void)
>> +{
>> + return 0;
>> +}
>> +
>> +#endif // CONFIG_PARAVIRT
>> +#endif
>> diff --git a/arch/loongarch/include/asm/paravirt_api_clock.h
>> b/arch/loongarch/include/asm/paravirt_api_clock.h
>> new file mode 100644
>> index 000000000000..65ac7cee0dad
>> --- /dev/null
>> +++ b/arch/loongarch/include/asm/paravirt_api_clock.h
>> @@ -0,0 +1 @@
>> +#include <asm/paravirt.h>
>> diff --git a/arch/loongarch/kernel/Makefile
>> b/arch/loongarch/kernel/Makefile
>> index 3c808c680370..662e6e9de12d 100644
>> --- a/arch/loongarch/kernel/Makefile
>> +++ b/arch/loongarch/kernel/Makefile
>> @@ -48,6 +48,7 @@ obj-$(CONFIG_MODULES) += module.o
>> module-sections.o
>> obj-$(CONFIG_STACKTRACE) += stacktrace.o
>> obj-$(CONFIG_PROC_FS) += proc.o
>> +obj-$(CONFIG_PARAVIRT) += paravirt.o
>> obj-$(CONFIG_SMP) += smp.o
>> diff --git a/arch/loongarch/kernel/paravirt.c
>> b/arch/loongarch/kernel/paravirt.c
>> new file mode 100644
>> index 000000000000..21d01d05791a
>> --- /dev/null
>> +++ b/arch/loongarch/kernel/paravirt.c
>> @@ -0,0 +1,41 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +#include <linux/export.h>
>> +#include <linux/types.h>
>> +#include <linux/jump_label.h>
>> +#include <linux/kvm_para.h>
>> +#include <asm/paravirt.h>
>> +#include <linux/static_call.h>
>> +
>> +struct static_key paravirt_steal_enabled;
>> +struct static_key paravirt_steal_rq_enabled;
>> +
>> +static u64 native_steal_clock(int cpu)
>> +{
>> + return 0;
>> +}
>> +
>> +DEFINE_STATIC_CALL(pv_steal_clock, native_steal_clock);
>
> This is the 4th arch with the same definition of native_steal_clock() and
> pv_steal_clock. I think we should add a common file kernel/paravirt.c and
> move the related functions from the archs into the new file.
>
> If you don't want to do that I can prepare a series.
>
>> +
>> +static bool kvm_para_available(void)
>> +{
>> + static int hypervisor_type;
>> + int config;
>> +
>> + if (!hypervisor_type) {
>> + config = read_cpucfg(CPUCFG_KVM_SIG);
>> + if (!memcmp(&config, KVM_SIGNATURE, 4))
>> + hypervisor_type = HYPERVISOR_KVM;
>> + }
>> +
>> + return hypervisor_type == HYPERVISOR_KVM;
>> +}
>> +
>> +int __init pv_guest_init(void)
>> +{
>> + if (!cpu_has_hypervisor)
>> + return 0;
>> + if (!kvm_para_available())
>> + return 0;
>> +
>> + return 1;
>> +}
>> diff --git a/arch/loongarch/kernel/setup.c
>> b/arch/loongarch/kernel/setup.c
>> index d183a745fb85..fa680bdd0bd1 100644
>> --- a/arch/loongarch/kernel/setup.c
>> +++ b/arch/loongarch/kernel/setup.c
>> @@ -43,6 +43,7 @@
>> #include <asm/efi.h>
>> #include <asm/loongson.h>
>> #include <asm/numa.h>
>> +#include <asm/paravirt.h>
>> #include <asm/pgalloc.h>
>> #include <asm/sections.h>
>> #include <asm/setup.h>
>> @@ -376,6 +377,7 @@ void __init platform_init(void)
>> pr_info("The BIOS Version: %s\n", b_info.bios_version);
>> efi_runtime_init();
>> + pv_guest_init();
>
> Any reason pv_guest_init() needs to return a value at all, seeing that
> you don't
> use the returned value?
>
>
> Juergen
Powered by blists - more mailing lists