[PATCH 0/2] Fix ptp_kvm_get_time_fn infinite loop and remove redundant EXPORT_SYMBOL_GPL

[PATCH 0/2] Fix ptp_kvm_get_time_fn infinite loop and remove redundant EXPORT_SYMBOL_GPL

[Chao Wu]

We fix the infinite loop bug in ptp_kvm_get_time_fn function and removes the redundant EXPORT_SYMBOL_GPL for pvclock_get_pvti_cpu0_va.

Chao Wu (2):
  ptp_kvm: fix an infinite loop in ptp_kvm_get_time_fn when vm has more than 64 vcpus 
  pvclock: remove EXPORT_SYMBOL_GPL for pvclock_get_pvti_cpu0_va

 arch/x86/include/asm/kvmclock.h | 16 ++++++++++++++++
 arch/x86/kernel/kvmclock.c      | 12 ++----------
 arch/x86/kernel/pvclock.c       |  1 -
 drivers/ptp/ptp_kvm.c           |  6 ++----
 4 files changed, 20 insertions(+), 15 deletions(-)

-- 
2.24.3 (Apple Git-128)

[PATCH 1/2] ptp_kvm: fix an infinite loop in ptp_kvm_get_time_fn when vm has more than 64 vcpus

[Chao Wu]

Commit 95a3d4454bb1 ("x86/kvmclock: Switch kvmclock data to a PER_CPU
variable") removes the static data array sized 64bytes * CONFIG_NR_CPUS
and uses a static page sized array to store pvclock data.
For the 64bytes * CONFIG_NR_CPUS size design, the address is consecutive
for all vcpus.
But for the static page sized array design, if page size is 4kB and
struct pvclock_vsyscall_time_info size is 64Byte (cache line aligned),
the maximum length of hv_clock_boot is 64.
For vcpus after 64, kernel will dynamically allocate pages for their
pvclock data in kvmclock_init_mem. So the address is not consecutive for
all vcpus.

Commit 95a3d4454bb1 ("x86/kvmclock: Switch kvmclock data to a PER_CPU
variable") uses per-cpu variable to store the pvclock_vsyscall_time_info
struct pointer to avoid the use of inconsecutive memory address. But
ptp_kvm_get_time_fn in ptp_kvm initiate hv_clock as the cpu0’s 
pvclock_vsyscall_time_info virtual address and uses &hv_clock[cpu].pvti 
to get specified cpu’s pvclock date. When the vcpu number is beyond 63,
&hv_clock[cpu].pvti will point to an incorrect virtual address and will 
cause infinite loop in the following do while code area.

Commit 95a3d4454bb1 ("x86/kvmclock: Switch kvmclock data to a PER_CPU
variable") has removed all &hv_clock[cpu].pvti usage with per-cpu in kvmclock
but the revelant code in ptp_kvm was ignored.

We use this_cpu_pvti() to get pvclock data in ptp_kvm and put per-cpu
related declaration in kvmclock.c to kvmclock.h for ptp_kvm to use.

Signed-off-by: Chao Wu <chaowu@linux.alibaba.com>
Signed-off-by: Jiang Liu <gerry@linux.alibaba.com>
Signed-off-by: Zha Bin <zhabin@linux.alibaba.com>
---
 arch/x86/include/asm/kvmclock.h | 16 ++++++++++++++++
 arch/x86/kernel/kvmclock.c      | 12 ++----------
 drivers/ptp/ptp_kvm.c           |  6 ++----
 3 files changed, 20 insertions(+), 14 deletions(-)

diff --git a/arch/x86/include/asm/kvmclock.h b/arch/x86/include/asm/kvmclock.h
index eceea9299097..69b09839e199 100644
--- a/arch/x86/include/asm/kvmclock.h
+++ b/arch/x86/include/asm/kvmclock.h
@@ -2,6 +2,22 @@
 #ifndef _ASM_X86_KVM_CLOCK_H
 #define _ASM_X86_KVM_CLOCK_H
 
+#include <asm/pvclock.h>
+
 extern struct clocksource kvm_clock;
 
+#ifdef CONFIG_KVM_GUEST
+DECLARE_PER_CPU(struct pvclock_vsyscall_time_info *, hv_clock_per_cpu);
+
+static inline struct pvclock_vcpu_time_info *this_cpu_pvti(void)
+{
+	return &this_cpu_read(hv_clock_per_cpu)->pvti;
+}
+
+static inline struct pvclock_vsyscall_time_info *this_cpu_hvclock(void)
+{
+	return this_cpu_read(hv_clock_per_cpu);
+}
+
+#endif /* CONFIG_KVM_GUEST */
 #endif /* _ASM_X86_KVM_CLOCK_H */
diff --git a/arch/x86/kernel/kvmclock.c b/arch/x86/kernel/kvmclock.c
index 2ec202cb9dfd..006551cccdac 100644
--- a/arch/x86/kernel/kvmclock.c
+++ b/arch/x86/kernel/kvmclock.c
@@ -64,18 +64,10 @@ early_param("no-kvmclock-vsyscall", parse_no_kvmclock_vsyscall);
 static struct pvclock_vsyscall_time_info
 			hv_clock_boot[HVC_BOOT_ARRAY_SIZE] __bss_decrypted __aligned(PAGE_SIZE);
 static struct pvclock_wall_clock wall_clock __bss_decrypted;
-static DEFINE_PER_CPU(struct pvclock_vsyscall_time_info *, hv_clock_per_cpu);
 static struct pvclock_vsyscall_time_info *hvclock_mem;
 
-static inline struct pvclock_vcpu_time_info *this_cpu_pvti(void)
-{
-	return &this_cpu_read(hv_clock_per_cpu)->pvti;
-}
-
-static inline struct pvclock_vsyscall_time_info *this_cpu_hvclock(void)
-{
-	return this_cpu_read(hv_clock_per_cpu);
-}
+DEFINE_PER_CPU(struct pvclock_vsyscall_time_info *, hv_clock_per_cpu);
+EXPORT_PER_CPU_SYMBOL_GPL(hv_clock_per_cpu);
 
 /*
  * The wallclock is the time of day when we booted. Since then, some time may
diff --git a/drivers/ptp/ptp_kvm.c b/drivers/ptp/ptp_kvm.c
index c67dd11e08b1..985e3728016b 100644
--- a/drivers/ptp/ptp_kvm.c
+++ b/drivers/ptp/ptp_kvm.c
@@ -34,7 +34,6 @@ struct kvm_ptp_clock {
 
 DEFINE_SPINLOCK(kvm_ptp_lock);
 
-static struct pvclock_vsyscall_time_info *hv_clock;
 
 static struct kvm_clock_pairing clock_pair;
 static phys_addr_t clock_pair_gpa;
@@ -53,7 +52,7 @@ static int ptp_kvm_get_time_fn(ktime_t *device_time,
 
 	preempt_disable_notrace();
 	cpu = smp_processor_id();
-	src = &hv_clock[cpu].pvti;
+	src = this_cpu_pvti();
 
 	do {
 		/*
@@ -182,9 +181,8 @@ static int __init ptp_kvm_init(void)
 		return -ENODEV;
 
 	clock_pair_gpa = slow_virt_to_phys(&clock_pair);
-	hv_clock = pvclock_get_pvti_cpu0_va();
 
-	if (!hv_clock)
+	if (!this_cpu_pvti())
 		return -ENODEV;
 
 	ret = kvm_hypercall2(KVM_HC_CLOCK_PAIRING, clock_pair_gpa,
-- 
2.24.3 (Apple Git-128)

[PATCH 2/2] pvclock: remove EXPORT_SYMBOL_GPL for pvclock_get_pvti_cpu0_va

[Chao Wu]

In order to avoid abusing the "pvclock_get_pvti_cpu0_va” interface, we
should remove EXPORT_SYMBOL_GPL for that function.

Signed-off-by: Chao Wu <chaowu@linux.alibaba.com>
Signed-off-by: Jiang Liu <gerry@linux.alibaba.com>
---
 arch/x86/kernel/pvclock.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/arch/x86/kernel/pvclock.c b/arch/x86/kernel/pvclock.c
index 637982efecd8..e26940c33d99 100644
--- a/arch/x86/kernel/pvclock.c
+++ b/arch/x86/kernel/pvclock.c
@@ -164,4 +164,3 @@ struct pvclock_vsyscall_time_info *pvclock_get_pvti_cpu0_va(void)
 {
 	return pvti_cpu0_va;
 }
-EXPORT_SYMBOL_GPL(pvclock_get_pvti_cpu0_va);
-- 
2.24.3 (Apple Git-128)

Re: [PATCH 0/2] Fix ptp_kvm_get_time_fn infinite loop and remove redundant EXPORT_SYMBOL_GPL

[Chao Wu]

Ping

在 2021/5/25 下午4:44, Chao Wu 写道:
> We fix the infinite loop bug in ptp_kvm_get_time_fn function and removes the redundant EXPORT_SYMBOL_GPL for pvclock_get_pvti_cpu0_va.
>
> Chao Wu (2):
>    ptp_kvm: fix an infinite loop in ptp_kvm_get_time_fn when vm has more than 64 vcpus
>    pvclock: remove EXPORT_SYMBOL_GPL for pvclock_get_pvti_cpu0_va
>
>   arch/x86/include/asm/kvmclock.h | 16 ++++++++++++++++
>   arch/x86/kernel/kvmclock.c      | 12 ++----------
>   arch/x86/kernel/pvclock.c       |  1 -
>   drivers/ptp/ptp_kvm.c           |  6 ++----
>   4 files changed, 20 insertions(+), 15 deletions(-)
>