[lucid PATCH] fix smp kvm guests on AMD

Stefan Bader stefan.bader at canonical.com
Mon Mar 7 18:04:30 UTC 2011


On 03/02/2011 10:57 PM, Serge E. Hallyn wrote:
>> Serge - Can we get this as 3 different patches, each with an
>> appropriate 'backported from' or 'cherry-picked from' SHA1 ID.
> 
> Attached to this email.  (If you prefer 3 separate inline emails
> pls let me know - I'm still getting familiar with the preferences
> here).  The result compiled fine.
> 
> thanks,
> -serge
> 

I looked through the patches and have a few questions/remarks:

Patch #2 seems to work around some changes missing from

commit 759379dd68c2885d1fafa433083d4487e710a685
Author: Zachary Amsden <zamsden at redhat.com>
Date:   Thu Aug 19 22:07:25 2010 -1000

    KVM: x86: Add helper functions for time computation

that patch replaces
 	ktime_get_ts(&ts);
 	monotonic_to_bootbased(&ts);
	[timespec_to_ns(&ts);]
by
	kernel_ns = get_kernel_ns()

which is all done within the section of disabled interrupts, so I probably would
move the assignment of kernel_ns up before local_irq_restore().

Also in patch#2, the upstream patch does this:

/* With all the info we got, fill in the values */
vcpu->hv_clock.tsc_timestamp = tsc_timestamp;

This seems to be missing in the backport.

And finally in patch#3 last_guest_tsc is set right at the beginning but the
upstream patch had that later in the section that was assigning last_kernel_ns.

@@ -1095,6 +1095,7 @@ static int kvm_write_guest_time(struct kvm_vcpu *v)
        vcpu->hv_clock.tsc_timestamp = tsc_timestamp;
        vcpu->hv_clock.system_time = kernel_ns + v->kvm->arch.kvmclock_offset;
        vcpu->last_kernel_ns = kernel_ns;
+       vcpu->last_guest_tsc = tsc_timestamp;
        vcpu->hv_clock.flags = 0;

One not directly related note: when checking whether this could have effects on
the lucid-ec2 branch, I noted that the xen code also has some scale_delta in its
code. It seems separated enough to be unaffected. But then maybe we actually
want to look closer there. Maybe there is the same problem in that code (I did
not look closer, yet).

-Stefan





More information about the kernel-team mailing list