Re: [patch 3/7] x86/kvmclock: Decrapify kvm_register_clock()
In addition to what Paolo's noticed, may be change printk to pr_info()
while editing around it:
> -
> printk(KERN_INFO "kvm-clock: Using msrs %x and %x",
> msr_kvm_system_time, msr_kvm_wall_clock);
>
> + hv_clock = (struct pvclock_vsyscall_time_info *)hv_clock_mem;
> + kvm_register_clock("primary cpu clock");
Otherwise,
Reviewed-by: Pavel Tatashin
Re: [patch 3/7] x86/kvmclock: Decrapify kvm_register_clock()
In addition to what Paolo's noticed, may be change printk to pr_info()
while editing around it:
> -
> printk(KERN_INFO "kvm-clock: Using msrs %x and %x",
> msr_kvm_system_time, msr_kvm_wall_clock);
>
> + hv_clock = (struct pvclock_vsyscall_time_info *)hv_clock_mem;
> + kvm_register_clock("primary cpu clock");
Otherwise,
Reviewed-by: Pavel Tatashin
Re: [patch 3/7] x86/kvmclock: Decrapify kvm_register_clock()
On Fri, 6 Jul 2018, Paolo Bonzini wrote: > Decrapification went a bit too far... > > On 06/07/2018 18:13, Thomas Gleixner wrote: > > - int cpu = smp_processor_id(); > > - int low, high, ret; > > struct pvclock_vcpu_time_info *src; > > + int cpu = smp_processor_id(); > > + u64 pa; > > > > if (!hv_clock) > > - return 0; > > - > > - src = _clock[cpu].pvti; > > ... the line above should not have been a "-". That, or initialize it > in the declaration. Whoever applies can fix it, no need to repost. > I'll test with the above fix. Indeed.
Re: [patch 3/7] x86/kvmclock: Decrapify kvm_register_clock()
On Fri, 6 Jul 2018, Paolo Bonzini wrote: > Decrapification went a bit too far... > > On 06/07/2018 18:13, Thomas Gleixner wrote: > > - int cpu = smp_processor_id(); > > - int low, high, ret; > > struct pvclock_vcpu_time_info *src; > > + int cpu = smp_processor_id(); > > + u64 pa; > > > > if (!hv_clock) > > - return 0; > > - > > - src = _clock[cpu].pvti; > > ... the line above should not have been a "-". That, or initialize it > in the declaration. Whoever applies can fix it, no need to repost. > I'll test with the above fix. Indeed.
Re: [patch 3/7] x86/kvmclock: Decrapify kvm_register_clock()
Decrapification went a bit too far...
On 06/07/2018 18:13, Thomas Gleixner wrote:
> - int cpu = smp_processor_id();
> - int low, high, ret;
> struct pvclock_vcpu_time_info *src;
> + int cpu = smp_processor_id();
> + u64 pa;
>
> if (!hv_clock)
> - return 0;
> -
> - src = _clock[cpu].pvti;
... the line above should not have been a "-". That, or initialize it
in the declaration. Whoever applies can fix it, no need to repost.
I'll test with the above fix.
Paolo
> - low = (int)slow_virt_to_phys(src) | 1;
> - high = ((u64)slow_virt_to_phys(src) >> 32);
> - ret = native_write_msr_safe(msr_kvm_system_time, low, high);
> - printk(KERN_INFO "kvm-clock: cpu %d, msr %x:%x, %s\n",
> -cpu, high, low, txt);
> + return;
>
> - return ret;
> + pa = slow_virt_to_phys(src) | 0x01ULL;
> + wrmsrl(msr_kvm_system_time, pa);
> + pr_info("kvm-clock: cpu %d, msr %llx, %s\n", cpu, pa, txt);
> }
Re: [patch 3/7] x86/kvmclock: Decrapify kvm_register_clock()
Decrapification went a bit too far...
On 06/07/2018 18:13, Thomas Gleixner wrote:
> - int cpu = smp_processor_id();
> - int low, high, ret;
> struct pvclock_vcpu_time_info *src;
> + int cpu = smp_processor_id();
> + u64 pa;
>
> if (!hv_clock)
> - return 0;
> -
> - src = _clock[cpu].pvti;
... the line above should not have been a "-". That, or initialize it
in the declaration. Whoever applies can fix it, no need to repost.
I'll test with the above fix.
Paolo
> - low = (int)slow_virt_to_phys(src) | 1;
> - high = ((u64)slow_virt_to_phys(src) >> 32);
> - ret = native_write_msr_safe(msr_kvm_system_time, low, high);
> - printk(KERN_INFO "kvm-clock: cpu %d, msr %x:%x, %s\n",
> -cpu, high, low, txt);
> + return;
>
> - return ret;
> + pa = slow_virt_to_phys(src) | 0x01ULL;
> + wrmsrl(msr_kvm_system_time, pa);
> + pr_info("kvm-clock: cpu %d, msr %llx, %s\n", cpu, pa, txt);
> }
[patch 3/7] x86/kvmclock: Decrapify kvm_register_clock()
The return value is pointless because the wrmsr cannot fail if
KVM_FEATURE_CLOCKSOURCE or KVM_FEATURE_CLOCKSOURCE2 are set.
kvm_register_clock() is only called locally so wants to be static.
Signed-off-by: Thomas Gleixner
Cc: Paolo Bonzini
Cc: Radim Krcmar
Cc: Peter Zijlstra
Cc: Juergen Gross
Cc: Pavel Tatashin
Cc: steven.sist...@oracle.com
Cc: daniel.m.jor...@oracle.com
Cc: x...@kernel.org
Cc: k...@vger.kernel.org
---
arch/x86/include/asm/kvm_para.h |1 -
arch/x86/kernel/kvmclock.c | 35 +++
2 files changed, 11 insertions(+), 25 deletions(-)
--- a/arch/x86/include/asm/kvm_para.h
+++ b/arch/x86/include/asm/kvm_para.h
@@ -7,7 +7,6 @@
#include
extern void kvmclock_init(void);
-extern int kvm_register_clock(char *txt);
#ifdef CONFIG_KVM_GUEST
bool kvm_check_and_clear_guest_paused(void);
--- a/arch/x86/kernel/kvmclock.c
+++ b/arch/x86/kernel/kvmclock.c
@@ -186,23 +186,18 @@ struct clocksource kvm_clock = {
};
EXPORT_SYMBOL_GPL(kvm_clock);
-int kvm_register_clock(char *txt)
+static void kvm_register_clock(char *txt)
{
- int cpu = smp_processor_id();
- int low, high, ret;
struct pvclock_vcpu_time_info *src;
+ int cpu = smp_processor_id();
+ u64 pa;
if (!hv_clock)
- return 0;
-
- src = _clock[cpu].pvti;
- low = (int)slow_virt_to_phys(src) | 1;
- high = ((u64)slow_virt_to_phys(src) >> 32);
- ret = native_write_msr_safe(msr_kvm_system_time, low, high);
- printk(KERN_INFO "kvm-clock: cpu %d, msr %x:%x, %s\n",
- cpu, high, low, txt);
+ return;
- return ret;
+ pa = slow_virt_to_phys(src) | 0x01ULL;
+ wrmsrl(msr_kvm_system_time, pa);
+ pr_info("kvm-clock: cpu %d, msr %llx, %s\n", cpu, pa, txt);
}
static void kvm_save_sched_clock_state(void)
@@ -217,11 +212,7 @@ static void kvm_restore_sched_clock_stat
#ifdef CONFIG_X86_LOCAL_APIC
static void kvm_setup_secondary_clock(void)
{
- /*
-* Now that the first cpu already had this clocksource initialized,
-* we shouldn't fail.
-*/
- WARN_ON(kvm_register_clock("secondary cpu clock"));
+ kvm_register_clock("secondary cpu clock");
}
#endif
@@ -264,16 +255,12 @@ void __init kvmclock_init(void)
} else if (!(kvmclock && kvm_para_has_feature(KVM_FEATURE_CLOCKSOURCE)))
return;
- hv_clock = (struct pvclock_vsyscall_time_info *)hv_clock_mem;
-
- if (kvm_register_clock("primary cpu clock")) {
- hv_clock = NULL;
- return;
- }
-
printk(KERN_INFO "kvm-clock: Using msrs %x and %x",
msr_kvm_system_time, msr_kvm_wall_clock);
+ hv_clock = (struct pvclock_vsyscall_time_info *)hv_clock_mem;
+ kvm_register_clock("primary cpu clock");
+
if (kvm_para_has_feature(KVM_FEATURE_CLOCKSOURCE_STABLE_BIT))
pvclock_set_flags(PVCLOCK_TSC_STABLE_BIT);
[patch 3/7] x86/kvmclock: Decrapify kvm_register_clock()
The return value is pointless because the wrmsr cannot fail if
KVM_FEATURE_CLOCKSOURCE or KVM_FEATURE_CLOCKSOURCE2 are set.
kvm_register_clock() is only called locally so wants to be static.
Signed-off-by: Thomas Gleixner
Cc: Paolo Bonzini
Cc: Radim Krcmar
Cc: Peter Zijlstra
Cc: Juergen Gross
Cc: Pavel Tatashin
Cc: steven.sist...@oracle.com
Cc: daniel.m.jor...@oracle.com
Cc: x...@kernel.org
Cc: k...@vger.kernel.org
---
arch/x86/include/asm/kvm_para.h |1 -
arch/x86/kernel/kvmclock.c | 35 +++
2 files changed, 11 insertions(+), 25 deletions(-)
--- a/arch/x86/include/asm/kvm_para.h
+++ b/arch/x86/include/asm/kvm_para.h
@@ -7,7 +7,6 @@
#include
extern void kvmclock_init(void);
-extern int kvm_register_clock(char *txt);
#ifdef CONFIG_KVM_GUEST
bool kvm_check_and_clear_guest_paused(void);
--- a/arch/x86/kernel/kvmclock.c
+++ b/arch/x86/kernel/kvmclock.c
@@ -186,23 +186,18 @@ struct clocksource kvm_clock = {
};
EXPORT_SYMBOL_GPL(kvm_clock);
-int kvm_register_clock(char *txt)
+static void kvm_register_clock(char *txt)
{
- int cpu = smp_processor_id();
- int low, high, ret;
struct pvclock_vcpu_time_info *src;
+ int cpu = smp_processor_id();
+ u64 pa;
if (!hv_clock)
- return 0;
-
- src = _clock[cpu].pvti;
- low = (int)slow_virt_to_phys(src) | 1;
- high = ((u64)slow_virt_to_phys(src) >> 32);
- ret = native_write_msr_safe(msr_kvm_system_time, low, high);
- printk(KERN_INFO "kvm-clock: cpu %d, msr %x:%x, %s\n",
- cpu, high, low, txt);
+ return;
- return ret;
+ pa = slow_virt_to_phys(src) | 0x01ULL;
+ wrmsrl(msr_kvm_system_time, pa);
+ pr_info("kvm-clock: cpu %d, msr %llx, %s\n", cpu, pa, txt);
}
static void kvm_save_sched_clock_state(void)
@@ -217,11 +212,7 @@ static void kvm_restore_sched_clock_stat
#ifdef CONFIG_X86_LOCAL_APIC
static void kvm_setup_secondary_clock(void)
{
- /*
-* Now that the first cpu already had this clocksource initialized,
-* we shouldn't fail.
-*/
- WARN_ON(kvm_register_clock("secondary cpu clock"));
+ kvm_register_clock("secondary cpu clock");
}
#endif
@@ -264,16 +255,12 @@ void __init kvmclock_init(void)
} else if (!(kvmclock && kvm_para_has_feature(KVM_FEATURE_CLOCKSOURCE)))
return;
- hv_clock = (struct pvclock_vsyscall_time_info *)hv_clock_mem;
-
- if (kvm_register_clock("primary cpu clock")) {
- hv_clock = NULL;
- return;
- }
-
printk(KERN_INFO "kvm-clock: Using msrs %x and %x",
msr_kvm_system_time, msr_kvm_wall_clock);
+ hv_clock = (struct pvclock_vsyscall_time_info *)hv_clock_mem;
+ kvm_register_clock("primary cpu clock");
+
if (kvm_para_has_feature(KVM_FEATURE_CLOCKSOURCE_STABLE_BIT))
pvclock_set_flags(PVCLOCK_TSC_STABLE_BIT);
