Re: [RFC PATCH v2 2/2] add support for Hyper-V invariant TSC

[Peter Lieven <pl@dlhnet.de>]

On 23.05.2013 14:33, Vadim Rozenfeld wrote:
>
>
> ----- Original Message -----
> From: "Peter Lieven" <pl@dlhnet.de>
> To: "Marcelo Tosatti" <mtosatti@redhat.com>
> Cc: "Vadim Rozenfeld" <vrozenfe@redhat.com>, kvm@vger.kernel.org, gleb@redhat.com, pl@dlh.net
> Sent: Thursday, May 23, 2013 4:18:55 PM
> Subject: Re: [RFC PATCH v2 2/2] add support for Hyper-V invariant TSC
>
> On 22.05.2013 23:23, Marcelo Tosatti wrote:
>> On Wed, May 22, 2013 at 03:22:55AM -0400, Vadim Rozenfeld wrote:
>>>
>>>
>>> ----- Original Message -----
>>> From: "Marcelo Tosatti" <mtosatti@redhat.com>
>>> To: "Vadim Rozenfeld" <vrozenfe@redhat.com>
>>> Cc: kvm@vger.kernel.org, gleb@redhat.com, pl@dlh.net
>>> Sent: Wednesday, May 22, 2013 10:50:46 AM
>>> Subject: Re: [RFC PATCH v2 2/2] add support for Hyper-V invariant TSC
>>>
>>> On Sun, May 19, 2013 at 05:06:37PM +1000, Vadim Rozenfeld wrote:
>>>> The following patch allows to activate a partition reference
>>>> time enlightenment that is based on the host platform's support
>>>> for an Invariant Time Stamp Counter (iTSC).
>>>> NOTE: This code will survive migration due to lack of VM stop/resume
>>>> handlers, when offset, scale and sequence should be
>>>> readjusted.
>>>>
>>>> ---
>>>>    arch/x86/kvm/x86.c | 6 +++++-
>>>>    1 file changed, 5 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
>>>> index 9645dab..b423fe4 100644
>>>> --- a/arch/x86/kvm/x86.c
>>>> +++ b/arch/x86/kvm/x86.c
>>>> @@ -1838,7 +1838,6 @@ static int set_msr_hyperv_pw(struct kvm_vcpu *vcpu, u32 msr, u64 data)
>>>>    		u64 gfn;
>>>>    		unsigned long addr;
>>>>    		HV_REFERENCE_TSC_PAGE tsc_ref;
>>>> -		tsc_ref.TscSequence = 0;
>>>>    		if (!(data & HV_X64_MSR_TSC_REFERENCE_ENABLE)) {
>>>>    			kvm->arch.hv_tsc_page = data;
>>>>    			break;
>>>> @@ -1848,6 +1847,11 @@ static int set_msr_hyperv_pw(struct kvm_vcpu *vcpu, u32 msr, u64 data)
>>>>    				HV_X64_MSR_TSC_REFERENCE_ADDRESS_SHIFT);
>>>>    		if (kvm_is_error_hva(addr))
>>>>    			return 1;
>>>> +		tsc_ref.TscSequence =
>>>> +				boot_cpu_has(X86_FEATURE_CONSTANT_TSC) ? 1 : 0;
>>>
>>> 1) You want NONSTOP_TSC (see 40fb1715 commit) which matches INVARIANT TSC.
>>> [VR]
>>> Thank you for reviewing. Will fix it.
>>> 2) TscSequence should increase?
>>> "This field serves as a sequence number that is incremented whenever..."
>>> [VR]
>>> Yes, on every VM resume, including migration. After migration we also need
>>> to recalculate scale and adjust offset.
>>> 3) 0xFFFFFFFF is the value for invalid source of reference time?
>>> [VR] Yes, on boot-up. In this case guest will go with PMTimer (not sure about HPET
>>> but I can check). But if we set sequence to 0xFFFFFFFF after migration - it's probably will not work.
>>
>> "Reference TSC during Save and Restore and Migration
>>
>> To address migration scenarios to physical platforms that do not support
>> iTSC, the TscSequence field is used. In the event that a guest partition
>> is  migrated from an iTSC capable host to a non-iTSC capable host, the
>> hypervisor sets TscSequence to the special value of 0xFFFFFFFF, which
>> directs the guest operating system to fall back to a different clock
>> source (for example, the virtual PM timer)."
>>
>> Why it would not/does not work after migration?
>>
>>
>
> what exactly do we heed the reference TSC for? the reference counter alone works great and it seems
> that there is a lot of trouble and crash possibilities involved with the referece tsc.
>
> [VR]
> Because it is incredibly light and fast.
> The simple test which calls QueryPerformanceCounter in a
> loop 10 millions times gives we the following results:
> PMTimer     32269 ms
> HPET        38466 ms
> Ref Count   6499 ms
> iTSC        1169 ms

is the ref_count with local_irq_disable or preempt_disable?

Peter