From mboxrd@z Thu Jan  1 00:00:00 1970
From: Avi Kivity <avi@redhat.com>
Subject: Re: [PATCH] tsc: use kvmclock for calibration
Date: Thu, 09 Aug 2012 17:05:09 +0300
Message-ID: <5023C395.40900@redhat.com>
References: <1344513463-7329-1-git-send-email-kraxel@redhat.com>
	<5023B2C4.90302@redhat.com> <5023C1D2.5090103@redhat.com>
	<5023C2BE.5070702@redhat.com>
Mime-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
Cc: seabios@seabios.org, kvm@vger.kernel.org
To: Gerd Hoffmann <kraxel@redhat.com>
Return-path: <seabios-bounces@seabios.org>
In-Reply-To: <5023C2BE.5070702@redhat.com>
List-Unsubscribe: <http://www.seabios.org/mailman/options/seabios>,
	<mailto:seabios-request@seabios.org?subject=unsubscribe>
List-Archive: <http://www.seabios.org/pipermail/seabios>
List-Post: <mailto:seabios@seabios.org>
List-Help: <mailto:seabios-request@seabios.org?subject=help>
List-Subscribe: <http://www.seabios.org/mailman/listinfo/seabios>,
	<mailto:seabios-request@seabios.org?subject=subscribe>
Errors-To: seabios-bounces@seabios.org
Sender: seabios-bounces@seabios.org
List-Id: kvm.vger.kernel.org

On 08/09/2012 05:01 PM, Avi Kivity wrote:
> On 08/09/2012 04:57 PM, Gerd Hoffmann wrote:
>>   Hi,
>> 
>>>> +u64 kvm_tsc_khz(void)
>>>> +{
>>>> +    u32 eax, ebx, ecx, edx, msr;
>>>> +    struct pvclock_vcpu_time_info time;
>>>> +    u32 addr = (u32)(&time);
>>>> +    u64 khz;
>>>> +
>>>> +    /* check presence and figure msr number */
>>>> +    cpuid(KVM_CPUID_FEATURES, &eax, &ebx, &ecx, &edx);
>>>> +    if (eax & KVM_FEATURE_CLOCKSOURCE2) {
>>>> +        msr = MSR_KVM_SYSTEM_TIME_NEW;
>>>> +    } else if (eax & KVM_FEATURE_CLOCKSOURCE) {
>>>> +        msr = MSR_KVM_SYSTEM_TIME;
>>>> +    } else {
>>>> +        return 0;
>>>> +    }
>>>> +
>>>> +    /* ask kvm hypervisor to fill struct */
>>>> +    memset(&time, 0, sizeof(time));
>>>> +    wrmsr(msr, addr | 1);
>>> 
>>> How can this work?
>> 
>> It did in my testing, although maybe by pure luck ...
>> 
>>> There is a 64-byte alignment requirement.
>> 
>> 64 bytes?  Sure?  The whole struct is only 32 bytes in size ...
> 
> er, the documentation says 4 bytes (so stack alignment works).  I
> distinctly remember having a large alignment requirement so we don't
> cross a page or slot boundary... something's wrong here.

	case MSR_KVM_SYSTEM_TIME: {
		kvmclock_reset(vcpu);

		vcpu->arch.time = data;
		kvm_make_request(KVM_REQ_CLOCK_UPDATE, vcpu);

		/* we verify if the enable bit is set... */
		if (!(data & 1))
			break;

		/* ...but clean it before doing the actual write */
		vcpu->arch.time_offset = data & ~(PAGE_MASK | 1);

		vcpu->arch.time_page =
				gfn_to_page(vcpu->kvm, data >> PAGE_SHIFT);

		if (is_error_page(vcpu->arch.time_page))
			vcpu->arch.time_page = NULL;

		break;

So your tests worked by pure luck, but the bug is in kvm.  We need to
grab two pages here.


-- 
error compiling committee.c: too many arguments to function