[v4,4/6] LoongArch: Add paravirt interface for guest kernel

Message ID 20240201031950.3225626-5-maobibo@loongson.cn
State New
Headers
Series LoongArch: Add pv ipi support on LoongArch VM |

Commit Message

maobibo Feb. 1, 2024, 3:19 a.m. UTC
  The patch adds paravirt interface for guest kernel, function
pv_guest_initi() firstly checks whether system runs on VM mode. If kernel
runs on VM mode, it will call function kvm_para_available() to detect
whether current VMM is KVM hypervisor. And the paravirt function can work
only if current VMM is KVM hypervisor, since there is only KVM hypervisor
supported on LoongArch now.

This patch only adds paravirt interface for guest kernel, however there
is not effective pv functions added here.

Signed-off-by: Bibo Mao <maobibo@loongson.cn>
---
 arch/loongarch/Kconfig                        |  9 ++++
 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, 88 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
  

Comments

Huacai Chen Feb. 19, 2024, 2:42 a.m. UTC | #1
Hi, Bibo,

On Thu, Feb 1, 2024 at 11:19 AM Bibo Mao <maobibo@loongson.cn> wrote:
>
> The patch adds paravirt interface for guest kernel, function
> pv_guest_initi() firstly checks whether system runs on VM mode. If kernel
> runs on VM mode, it will call function kvm_para_available() to detect
> whether current VMM is KVM hypervisor. And the paravirt function can work
> only if current VMM is KVM hypervisor, since there is only KVM hypervisor
> supported on LoongArch now.
>
> This patch only adds paravirt interface for guest kernel, however there
> is not effective pv functions added here.
>
> Signed-off-by: Bibo Mao <maobibo@loongson.cn>
> ---
>  arch/loongarch/Kconfig                        |  9 ++++
>  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, 88 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 10959e6c3583..817a56dff80f 100644
> --- a/arch/loongarch/Kconfig
> +++ b/arch/loongarch/Kconfig
> @@ -585,6 +585,15 @@ config CPU_HAS_PREFETCH
>         bool
>         default y
>
> +config PARAVIRT
> +       bool "Enable paravirtualization code"
> +       depends on AS_HAS_LVZ_EXTENSION
> +       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);
> +}
The steal time code can be removed in this patch, I think.

> +
> +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);
The steal time code can be removed in this patch, I think.

> +
> +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 edf2bba80130..de5c36dccc49 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>
> @@ -367,6 +368,7 @@ void __init platform_init(void)
>         pr_info("The BIOS Version: %s\n", b_info.bios_version);
>
>         efi_runtime_init();
> +       pv_guest_init();
I prefer use CONFIG_PARAVIRT here, though you have a dummy version for
!CONFIG_PARAVIRT, I think it is better to let others clearly know that
PARAVIRT is an optional feature.

Huacai


Huacai
>  }
>
>  static void __init check_kernel_sections_mem(void)
> --
> 2.39.3
>
>
  
maobibo Feb. 19, 2024, 4:11 a.m. UTC | #2
On 2024/2/19 上午10:42, Huacai Chen wrote:
> Hi, Bibo,
> 
> On Thu, Feb 1, 2024 at 11:19 AM Bibo Mao <maobibo@loongson.cn> wrote:
>>
>> The patch adds paravirt interface for guest kernel, function
>> pv_guest_initi() firstly checks whether system runs on VM mode. If kernel
>> runs on VM mode, it will call function kvm_para_available() to detect
>> whether current VMM is KVM hypervisor. And the paravirt function can work
>> only if current VMM is KVM hypervisor, since there is only KVM hypervisor
>> supported on LoongArch now.
>>
>> This patch only adds paravirt interface for guest kernel, however there
>> is not effective pv functions added here.
>>
>> Signed-off-by: Bibo Mao <maobibo@loongson.cn>
>> ---
>>   arch/loongarch/Kconfig                        |  9 ++++
>>   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, 88 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 10959e6c3583..817a56dff80f 100644
>> --- a/arch/loongarch/Kconfig
>> +++ b/arch/loongarch/Kconfig
>> @@ -585,6 +585,15 @@ config CPU_HAS_PREFETCH
>>          bool
>>          default y
>>
>> +config PARAVIRT
>> +       bool "Enable paravirtualization code"
>> +       depends on AS_HAS_LVZ_EXTENSION
>> +       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);
>> +}
> The steal time code can be removed in this patch, I think.
> 
Originally I want to remove this piece of code, but it fails to compile 
if CONFIG_PARAVIRT is selected. Here is reference code, function 
paravirt_steal_clock() must be defined if CONFIG_PARAVIRT is selected.

static __always_inline u64 steal_account_process_time(u64 maxtime)
{
#ifdef CONFIG_PARAVIRT
         if (static_key_false(&paravirt_steal_enabled)) {
                 u64 steal;

                 steal = paravirt_steal_clock(smp_processor_id());
                 steal -= this_rq()->prev_steal_time;
                 steal = min(steal, maxtime);
                 account_steal_time(steal);
                 this_rq()->prev_steal_time += steal;

                 return steal;
         }
#endif
         return 0;
}

>> +
>> +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);
> The steal time code can be removed in this patch, I think.
Ditto, the same reason with above.
> 
>> +
>> +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 edf2bba80130..de5c36dccc49 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>
>> @@ -367,6 +368,7 @@ void __init platform_init(void)
>>          pr_info("The BIOS Version: %s\n", b_info.bios_version);
>>
>>          efi_runtime_init();
>> +       pv_guest_init();
> I prefer use CONFIG_PARAVIRT here, though you have a dummy version for
> !CONFIG_PARAVIRT, I think it is better to let others clearly know that
> PARAVIRT is an optional feature.
I remember that there is rule that CONFIG_xxx had better be used in 
header files rather than c code, so that the code looks neat. Am I wrong?

Regards
Bibo Mao
> 
> Huacai
> 
> 
> Huacai
>>   }
>>
>>   static void __init check_kernel_sections_mem(void)
>> --
>> 2.39.3
>>
>>
  
Huacai Chen Feb. 19, 2024, 8:48 a.m. UTC | #3
On Mon, Feb 19, 2024 at 12:11 PM maobibo <maobibo@loongson.cn> wrote:
>
>
>
> On 2024/2/19 上午10:42, Huacai Chen wrote:
> > Hi, Bibo,
> >
> > On Thu, Feb 1, 2024 at 11:19 AM Bibo Mao <maobibo@loongson.cn> wrote:
> >>
> >> The patch adds paravirt interface for guest kernel, function
> >> pv_guest_initi() firstly checks whether system runs on VM mode. If kernel
> >> runs on VM mode, it will call function kvm_para_available() to detect
> >> whether current VMM is KVM hypervisor. And the paravirt function can work
> >> only if current VMM is KVM hypervisor, since there is only KVM hypervisor
> >> supported on LoongArch now.
> >>
> >> This patch only adds paravirt interface for guest kernel, however there
> >> is not effective pv functions added here.
> >>
> >> Signed-off-by: Bibo Mao <maobibo@loongson.cn>
> >> ---
> >>   arch/loongarch/Kconfig                        |  9 ++++
> >>   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, 88 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 10959e6c3583..817a56dff80f 100644
> >> --- a/arch/loongarch/Kconfig
> >> +++ b/arch/loongarch/Kconfig
> >> @@ -585,6 +585,15 @@ config CPU_HAS_PREFETCH
> >>          bool
> >>          default y
> >>
> >> +config PARAVIRT
> >> +       bool "Enable paravirtualization code"
> >> +       depends on AS_HAS_LVZ_EXTENSION
> >> +       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);
> >> +}
> > The steal time code can be removed in this patch, I think.
> >
> Originally I want to remove this piece of code, but it fails to compile
> if CONFIG_PARAVIRT is selected. Here is reference code, function
> paravirt_steal_clock() must be defined if CONFIG_PARAVIRT is selected.
>
> static __always_inline u64 steal_account_process_time(u64 maxtime)
> {
> #ifdef CONFIG_PARAVIRT
>          if (static_key_false(&paravirt_steal_enabled)) {
>                  u64 steal;
>
>                  steal = paravirt_steal_clock(smp_processor_id());
>                  steal -= this_rq()->prev_steal_time;
>                  steal = min(steal, maxtime);
>                  account_steal_time(steal);
>                  this_rq()->prev_steal_time += steal;
>
>                  return steal;
>          }
> #endif
>          return 0;
> }
OK, then keep it.

>
> >> +
> >> +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);
> > The steal time code can be removed in this patch, I think.
> Ditto, the same reason with above.
> >
> >> +
> >> +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 edf2bba80130..de5c36dccc49 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>
> >> @@ -367,6 +368,7 @@ void __init platform_init(void)
> >>          pr_info("The BIOS Version: %s\n", b_info.bios_version);
> >>
> >>          efi_runtime_init();
> >> +       pv_guest_init();
> > I prefer use CONFIG_PARAVIRT here, though you have a dummy version for
> > !CONFIG_PARAVIRT, I think it is better to let others clearly know that
> > PARAVIRT is an optional feature.
> I remember that there is rule that CONFIG_xxx had better be used in
> header files rather than c code, so that the code looks neat. Am I wrong?
That depends on what we want, sometimes we want to hide the details,
but sometimes we want to give others a notice.

And there is another problem: if you want to centralize all pv init
functions, it is better to use pv_features_init() rather than
pv_guest_init(); if you want to give each feature an init function,
then we don't need pv_guest_init here, and we can then add a
pv_ipi_init() in the last patch.

Huacai

>
> Regards
> Bibo Mao
> >
> > Huacai
> >
> >
> > Huacai
> >>   }
> >>
> >>   static void __init check_kernel_sections_mem(void)
> >> --
> >> 2.39.3
> >>
> >>
>
  
maobibo Feb. 19, 2024, 9:21 a.m. UTC | #4
On 2024/2/19 下午4:48, Huacai Chen wrote:
> On Mon, Feb 19, 2024 at 12:11 PM maobibo <maobibo@loongson.cn> wrote:
>>
>>
>>
>> On 2024/2/19 上午10:42, Huacai Chen wrote:
>>> Hi, Bibo,
>>>
>>> On Thu, Feb 1, 2024 at 11:19 AM Bibo Mao <maobibo@loongson.cn> wrote:
>>>>
>>>> The patch adds paravirt interface for guest kernel, function
>>>> pv_guest_initi() firstly checks whether system runs on VM mode. If kernel
>>>> runs on VM mode, it will call function kvm_para_available() to detect
>>>> whether current VMM is KVM hypervisor. And the paravirt function can work
>>>> only if current VMM is KVM hypervisor, since there is only KVM hypervisor
>>>> supported on LoongArch now.
>>>>
>>>> This patch only adds paravirt interface for guest kernel, however there
>>>> is not effective pv functions added here.
>>>>
>>>> Signed-off-by: Bibo Mao <maobibo@loongson.cn>
>>>> ---
>>>>    arch/loongarch/Kconfig                        |  9 ++++
>>>>    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, 88 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 10959e6c3583..817a56dff80f 100644
>>>> --- a/arch/loongarch/Kconfig
>>>> +++ b/arch/loongarch/Kconfig
>>>> @@ -585,6 +585,15 @@ config CPU_HAS_PREFETCH
>>>>           bool
>>>>           default y
>>>>
>>>> +config PARAVIRT
>>>> +       bool "Enable paravirtualization code"
>>>> +       depends on AS_HAS_LVZ_EXTENSION
>>>> +       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);
>>>> +}
>>> The steal time code can be removed in this patch, I think.
>>>
>> Originally I want to remove this piece of code, but it fails to compile
>> if CONFIG_PARAVIRT is selected. Here is reference code, function
>> paravirt_steal_clock() must be defined if CONFIG_PARAVIRT is selected.
>>
>> static __always_inline u64 steal_account_process_time(u64 maxtime)
>> {
>> #ifdef CONFIG_PARAVIRT
>>           if (static_key_false(&paravirt_steal_enabled)) {
>>                   u64 steal;
>>
>>                   steal = paravirt_steal_clock(smp_processor_id());
>>                   steal -= this_rq()->prev_steal_time;
>>                   steal = min(steal, maxtime);
>>                   account_steal_time(steal);
>>                   this_rq()->prev_steal_time += steal;
>>
>>                   return steal;
>>           }
>> #endif
>>           return 0;
>> }
> OK, then keep it.
> 
>>
>>>> +
>>>> +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);
>>> The steal time code can be removed in this patch, I think.
>> Ditto, the same reason with above.
>>>
>>>> +
>>>> +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 edf2bba80130..de5c36dccc49 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>
>>>> @@ -367,6 +368,7 @@ void __init platform_init(void)
>>>>           pr_info("The BIOS Version: %s\n", b_info.bios_version);
>>>>
>>>>           efi_runtime_init();
>>>> +       pv_guest_init();
>>> I prefer use CONFIG_PARAVIRT here, though you have a dummy version for
>>> !CONFIG_PARAVIRT, I think it is better to let others clearly know that
>>> PARAVIRT is an optional feature.
>> I remember that there is rule that CONFIG_xxx had better be used in
>> header files rather than c code, so that the code looks neat. Am I wrong?
> That depends on what we want, sometimes we want to hide the details,
> but sometimes we want to give others a notice.
I want to keep code clean here :)

> 
> And there is another problem: if you want to centralize all pv init
> functions, it is better to use pv_features_init() rather than
> pv_guest_init(); if you want to give each feature an init function,
> then we don't need pv_guest_init here, and we can then add a
> pv_ipi_init() in the last patch.
Currently I have no idea how to add other pv features like pv 
stealtimer, I will consider this when adding other pv features. 
pv_ipi_init/pv_guest_init is both ok for me, pv_ipi_init is better for now.

Regards
Bibo Mao

> 
> Huacai
> 
>>
>> Regards
>> Bibo Mao
>>>
>>> Huacai
>>>
>>>
>>> Huacai
>>>>    }
>>>>
>>>>    static void __init check_kernel_sections_mem(void)
>>>> --
>>>> 2.39.3
>>>>
>>>>
>>
  
Huacai Chen Feb. 19, 2024, 9:38 a.m. UTC | #5
On Mon, Feb 19, 2024 at 5:21 PM maobibo <maobibo@loongson.cn> wrote:
>
>
>
> On 2024/2/19 下午4:48, Huacai Chen wrote:
> > On Mon, Feb 19, 2024 at 12:11 PM maobibo <maobibo@loongson.cn> wrote:
> >>
> >>
> >>
> >> On 2024/2/19 上午10:42, Huacai Chen wrote:
> >>> Hi, Bibo,
> >>>
> >>> On Thu, Feb 1, 2024 at 11:19 AM Bibo Mao <maobibo@loongson.cn> wrote:
> >>>>
> >>>> The patch adds paravirt interface for guest kernel, function
> >>>> pv_guest_initi() firstly checks whether system runs on VM mode. If kernel
> >>>> runs on VM mode, it will call function kvm_para_available() to detect
> >>>> whether current VMM is KVM hypervisor. And the paravirt function can work
> >>>> only if current VMM is KVM hypervisor, since there is only KVM hypervisor
> >>>> supported on LoongArch now.
> >>>>
> >>>> This patch only adds paravirt interface for guest kernel, however there
> >>>> is not effective pv functions added here.
> >>>>
> >>>> Signed-off-by: Bibo Mao <maobibo@loongson.cn>
> >>>> ---
> >>>>    arch/loongarch/Kconfig                        |  9 ++++
> >>>>    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, 88 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 10959e6c3583..817a56dff80f 100644
> >>>> --- a/arch/loongarch/Kconfig
> >>>> +++ b/arch/loongarch/Kconfig
> >>>> @@ -585,6 +585,15 @@ config CPU_HAS_PREFETCH
> >>>>           bool
> >>>>           default y
> >>>>
> >>>> +config PARAVIRT
> >>>> +       bool "Enable paravirtualization code"
> >>>> +       depends on AS_HAS_LVZ_EXTENSION
> >>>> +       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);
> >>>> +}
> >>> The steal time code can be removed in this patch, I think.
> >>>
> >> Originally I want to remove this piece of code, but it fails to compile
> >> if CONFIG_PARAVIRT is selected. Here is reference code, function
> >> paravirt_steal_clock() must be defined if CONFIG_PARAVIRT is selected.
> >>
> >> static __always_inline u64 steal_account_process_time(u64 maxtime)
> >> {
> >> #ifdef CONFIG_PARAVIRT
> >>           if (static_key_false(&paravirt_steal_enabled)) {
> >>                   u64 steal;
> >>
> >>                   steal = paravirt_steal_clock(smp_processor_id());
> >>                   steal -= this_rq()->prev_steal_time;
> >>                   steal = min(steal, maxtime);
> >>                   account_steal_time(steal);
> >>                   this_rq()->prev_steal_time += steal;
> >>
> >>                   return steal;
> >>           }
> >> #endif
> >>           return 0;
> >> }
> > OK, then keep it.
> >
> >>
> >>>> +
> >>>> +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);
> >>> The steal time code can be removed in this patch, I think.
> >> Ditto, the same reason with above.
> >>>
> >>>> +
> >>>> +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 edf2bba80130..de5c36dccc49 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>
> >>>> @@ -367,6 +368,7 @@ void __init platform_init(void)
> >>>>           pr_info("The BIOS Version: %s\n", b_info.bios_version);
> >>>>
> >>>>           efi_runtime_init();
> >>>> +       pv_guest_init();
> >>> I prefer use CONFIG_PARAVIRT here, though you have a dummy version for
> >>> !CONFIG_PARAVIRT, I think it is better to let others clearly know that
> >>> PARAVIRT is an optional feature.
> >> I remember that there is rule that CONFIG_xxx had better be used in
> >> header files rather than c code, so that the code looks neat. Am I wrong?
> > That depends on what we want, sometimes we want to hide the details,
> > but sometimes we want to give others a notice.
> I want to keep code clean here :)
>
> >
> > And there is another problem: if you want to centralize all pv init
> > functions, it is better to use pv_features_init() rather than
> > pv_guest_init(); if you want to give each feature an init function,
> > then we don't need pv_guest_init here, and we can then add a
> > pv_ipi_init() in the last patch.
> Currently I have no idea how to add other pv features like pv
> stealtimer, I will consider this when adding other pv features.
> pv_ipi_init/pv_guest_init is both ok for me, pv_ipi_init is better for now.
Then you want to add an init function for each feature, so please
rename to pv_ipi_init(), move to the last patch and in
loongson_smp_setup().

Huacai

>
> Regards
> Bibo Mao
>
> >
> > Huacai
> >
> >>
> >> Regards
> >> Bibo Mao
> >>>
> >>> Huacai
> >>>
> >>>
> >>> Huacai
> >>>>    }
> >>>>
> >>>>    static void __init check_kernel_sections_mem(void)
> >>>> --
> >>>> 2.39.3
> >>>>
> >>>>
> >>
>
  
maobibo Feb. 19, 2024, 9:50 a.m. UTC | #6
On 2024/2/19 下午5:38, Huacai Chen wrote:
> On Mon, Feb 19, 2024 at 5:21 PM maobibo <maobibo@loongson.cn> wrote:
>>
>>
>>
>> On 2024/2/19 下午4:48, Huacai Chen wrote:
>>> On Mon, Feb 19, 2024 at 12:11 PM maobibo <maobibo@loongson.cn> wrote:
>>>>
>>>>
>>>>
>>>> On 2024/2/19 上午10:42, Huacai Chen wrote:
>>>>> Hi, Bibo,
>>>>>
>>>>> On Thu, Feb 1, 2024 at 11:19 AM Bibo Mao <maobibo@loongson.cn> wrote:
>>>>>>
>>>>>> The patch adds paravirt interface for guest kernel, function
>>>>>> pv_guest_initi() firstly checks whether system runs on VM mode. If kernel
>>>>>> runs on VM mode, it will call function kvm_para_available() to detect
>>>>>> whether current VMM is KVM hypervisor. And the paravirt function can work
>>>>>> only if current VMM is KVM hypervisor, since there is only KVM hypervisor
>>>>>> supported on LoongArch now.
>>>>>>
>>>>>> This patch only adds paravirt interface for guest kernel, however there
>>>>>> is not effective pv functions added here.
>>>>>>
>>>>>> Signed-off-by: Bibo Mao <maobibo@loongson.cn>
>>>>>> ---
>>>>>>     arch/loongarch/Kconfig                        |  9 ++++
>>>>>>     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, 88 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 10959e6c3583..817a56dff80f 100644
>>>>>> --- a/arch/loongarch/Kconfig
>>>>>> +++ b/arch/loongarch/Kconfig
>>>>>> @@ -585,6 +585,15 @@ config CPU_HAS_PREFETCH
>>>>>>            bool
>>>>>>            default y
>>>>>>
>>>>>> +config PARAVIRT
>>>>>> +       bool "Enable paravirtualization code"
>>>>>> +       depends on AS_HAS_LVZ_EXTENSION
>>>>>> +       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);
>>>>>> +}
>>>>> The steal time code can be removed in this patch, I think.
>>>>>
>>>> Originally I want to remove this piece of code, but it fails to compile
>>>> if CONFIG_PARAVIRT is selected. Here is reference code, function
>>>> paravirt_steal_clock() must be defined if CONFIG_PARAVIRT is selected.
>>>>
>>>> static __always_inline u64 steal_account_process_time(u64 maxtime)
>>>> {
>>>> #ifdef CONFIG_PARAVIRT
>>>>            if (static_key_false(&paravirt_steal_enabled)) {
>>>>                    u64 steal;
>>>>
>>>>                    steal = paravirt_steal_clock(smp_processor_id());
>>>>                    steal -= this_rq()->prev_steal_time;
>>>>                    steal = min(steal, maxtime);
>>>>                    account_steal_time(steal);
>>>>                    this_rq()->prev_steal_time += steal;
>>>>
>>>>                    return steal;
>>>>            }
>>>> #endif
>>>>            return 0;
>>>> }
>>> OK, then keep it.
>>>
>>>>
>>>>>> +
>>>>>> +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);
>>>>> The steal time code can be removed in this patch, I think.
>>>> Ditto, the same reason with above.
>>>>>
>>>>>> +
>>>>>> +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 edf2bba80130..de5c36dccc49 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>
>>>>>> @@ -367,6 +368,7 @@ void __init platform_init(void)
>>>>>>            pr_info("The BIOS Version: %s\n", b_info.bios_version);
>>>>>>
>>>>>>            efi_runtime_init();
>>>>>> +       pv_guest_init();
>>>>> I prefer use CONFIG_PARAVIRT here, though you have a dummy version for
>>>>> !CONFIG_PARAVIRT, I think it is better to let others clearly know that
>>>>> PARAVIRT is an optional feature.
>>>> I remember that there is rule that CONFIG_xxx had better be used in
>>>> header files rather than c code, so that the code looks neat. Am I wrong?
>>> That depends on what we want, sometimes we want to hide the details,
>>> but sometimes we want to give others a notice.
>> I want to keep code clean here :)
>>
>>>
>>> And there is another problem: if you want to centralize all pv init
>>> functions, it is better to use pv_features_init() rather than
>>> pv_guest_init(); if you want to give each feature an init function,
>>> then we don't need pv_guest_init here, and we can then add a
>>> pv_ipi_init() in the last patch.
>> Currently I have no idea how to add other pv features like pv
>> stealtimer, I will consider this when adding other pv features.
>> pv_ipi_init/pv_guest_init is both ok for me, pv_ipi_init is better for now.
> Then you want to add an init function for each feature, so please
> rename to pv_ipi_init(), move to the last patch and in
> loongson_smp_setup().
Sure, will change function name with pv_ipi_init() and move sentences 
calling this function to last patch.

Regards
Bibo Mao

> 
> Huacai
> 
>>
>> Regards
>> Bibo Mao
>>
>>>
>>> Huacai
>>>
>>>>
>>>> Regards
>>>> Bibo Mao
>>>>>
>>>>> Huacai
>>>>>
>>>>>
>>>>> Huacai
>>>>>>     }
>>>>>>
>>>>>>     static void __init check_kernel_sections_mem(void)
>>>>>> --
>>>>>> 2.39.3
>>>>>>
>>>>>>
>>>>
>>
  

Patch

diff --git a/arch/loongarch/Kconfig b/arch/loongarch/Kconfig
index 10959e6c3583..817a56dff80f 100644
--- a/arch/loongarch/Kconfig
+++ b/arch/loongarch/Kconfig
@@ -585,6 +585,15 @@  config CPU_HAS_PREFETCH
 	bool
 	default y
 
+config PARAVIRT
+	bool "Enable paravirtualization code"
+	depends on AS_HAS_LVZ_EXTENSION
+	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);
+
+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 edf2bba80130..de5c36dccc49 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>
@@ -367,6 +368,7 @@  void __init platform_init(void)
 	pr_info("The BIOS Version: %s\n", b_info.bios_version);
 
 	efi_runtime_init();
+	pv_guest_init();
 }
 
 static void __init check_kernel_sections_mem(void)