On 08/02/2010 04:43 AM, Glauber Costa wrote:
On Sat, Jul 31, 2010 at 01:55:10PM -1000, Zachary Amsden wrote:
On 07/31/2010 06:36 AM, Arjan Koers wrote:
On 2010-07-31 13:53, Arjan Koers wrote:
The kernel boots successfully when CONFIG_PRINTK_TIME is not set.
The problem occurs when this message is printed:
[ 0.016000] kvm-clock: cpu 1, msr 0:1511c01, secondary cpu clock
When I disable that printk, the kernel boots with
CONFIG_PRINTK_TIME=y
--- a/arch/x86/kernel/kvmclock.c
+++ b/arch/x86/kernel/kvmclock.c
@@ -131,8 +131,8 @@ static int kvm_register_clock(char *txt)
int low, high;
low = (int)__pa(&per_cpu(hv_clock, cpu)) | 1;
high = ((u64)__pa(&per_cpu(hv_clock, cpu))>> 32);
- printk(KERN_INFO "kvm-clock: cpu %d, msr %x:%x, %s\n",
- cpu, high, low, txt);
+ /*printk(KERN_INFO "kvm-clock: cpu %d, msr %x:%x, %s\n",
+ cpu, high, low, txt);*/
return native_write_msr_safe(msr_kvm_system_time, low, high);
}
So the problem appears to be that the clock of the second CPU
is used too soon (or that clock setup should finish earlier).
That's almost hilarious. The printk from setting up the kvm clock
is invoking the kvm clock before it is setup.
There's no reason other printks couldn't do the same thing, however.
I think it's safest to keep an initialized flag and check for it
before attempting to return a meaningful value.
I was on vacations, just got back.
I think it is safe to just patch our own use of it. Before that, all other
printks will be handled by the main cpu anyway, since it'll be the only one active
at the moment. The only possible offenders for this are us, and the cpu initialization
code, which is already fragile in multiple ways anyway.
A flag would only make things more complicated and dirty
Can we just do this?
Initialize hv_clock to zero
This stops callers from getting random values if data is accessed before
clock is initialized; instead they will get zeroed clock values (because
computation involves a multiplication by a factor in hv_clock).
Signed-off-by: Zachary Amsden <zamsden@xxxxxxxxxx>
diff --git a/arch/x86/kernel/kvmclock.c b/arch/x86/kernel/kvmclock.c
index eb9b76c..e7acd0d 100644
--- a/arch/x86/kernel/kvmclock.c
+++ b/arch/x86/kernel/kvmclock.c
@@ -40,7 +40,7 @@ static int parse_no_kvmclock(char *arg)
early_param("no-kvmclock", parse_no_kvmclock);
/* The hypervisor will put information about time periodically here */
-static DEFINE_PER_CPU_SHARED_ALIGNED(struct pvclock_vcpu_time_info, hv_clock);
+static DEFINE_PER_CPU_SHARED_ALIGNED(struct pvclock_vcpu_time_info, hv_clock) = {0};
static struct pvclock_wall_clock wall_clock;
/*
