diff mbox

[RFC,v2,2/2] add support for Hyper-V invariant TSC

Message ID 1368947197-9033-3-git-send-email-vrozenfe@redhat.com (mailing list archive)
State New, archived
Headers show

Commit Message

Vadim Rozenfeld May 19, 2013, 7:06 a.m. UTC
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(-)

Comments

Marcelo Tosatti May 22, 2013, 12:50 a.m. UTC | #1
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.
2) TscSequence should increase? 
"This field serves as a sequence number that is incremented whenever..."
3) 0xFFFFFFFF is the value for invalid source of reference time?

> +		tsc_ref.TscScale =
> +				((10000LL << 32) / vcpu->arch.virtual_tsc_khz) << 32;
> +		tsc_ref.TscOffset = 0;
>  		if (__copy_to_user((void __user *)addr, &tsc_ref, sizeof(tsc_ref)))
>  			return 1;
>  		mark_page_dirty(kvm, gfn);
> -- 
> 1.8.1.2
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Vadim Rozenfeld May 22, 2013, 7:22 a.m. UTC | #2
----- 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.

> +		tsc_ref.TscScale =
> +				((10000LL << 32) / vcpu->arch.virtual_tsc_khz) << 32;
> +		tsc_ref.TscOffset = 0;
>  		if (__copy_to_user((void __user *)addr, &tsc_ref, sizeof(tsc_ref)))
>  			return 1;
>  		mark_page_dirty(kvm, gfn);
> -- 
> 1.8.1.2
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Marcelo Tosatti May 22, 2013, 9:23 p.m. UTC | #3
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?


--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Peter Lieven May 23, 2013, 6:18 a.m. UTC | #4
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.

Peter

--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Gleb Natapov May 23, 2013, 9:12 a.m. UTC | #5
On Wed, May 22, 2013 at 06:23:30PM -0300, 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?
> 
Please read the whole discussion, we talked about it already. We
definitely do not want to fall back to PM timer either, we want to use
reference counter instead.

--
			Gleb.
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Gleb Natapov May 23, 2013, 9:13 a.m. UTC | #6
On Thu, May 23, 2013 at 08:18:55AM +0200, Peter Lieven wrote:
> 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.
> 
Reference TSC is even faster. There should be no crashed with proper
implementation.

--
			Gleb.
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Vadim Rozenfeld May 23, 2013, 12:21 p.m. UTC | #7
----- 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: Thursday, May 23, 2013 7:23:30 AM
Subject: Re: [RFC PATCH v2 2/2] add support for Hyper-V invariant TSC

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?

[VR]
Because of different frequencies, I think. 
Hyper-V reference counters and iTSC report
performance frequency equal to 10MHz,
which is obviously is not true for PM and HPET timers.
Windows calibrates timers on boot-up and you probably 
have no chance to do it after or during resume.  


--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Vadim Rozenfeld May 23, 2013, 12:33 p.m. UTC | #8
----- 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

Vadim.

Peter

--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Peter Lieven May 23, 2013, 12:44 p.m. UTC | #9
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

--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Gleb Natapov May 23, 2013, 12:45 p.m. UTC | #10
On Thu, May 23, 2013 at 02:44:14PM +0200, Peter Lieven wrote:
> 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?
> 
irq disabling cannot account for such big difference.

--
			Gleb.
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Vadim Rozenfeld May 23, 2013, 12:54 p.m. UTC | #11
----- Original Message -----
From: "Peter Lieven" <pl@dlhnet.de>
To: "Vadim Rozenfeld" <vrozenfe@redhat.com>
Cc: "Marcelo Tosatti" <mtosatti@redhat.com>, kvm@vger.kernel.org, gleb@redhat.com, pl@dlh.net
Sent: Thursday, May 23, 2013 10:44:14 PM
Subject: Re: [RFC PATCH v2 2/2] add support for Hyper-V invariant TSC

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?
[VR]
local_irq_disable

Peter

--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Marcelo Tosatti May 23, 2013, 1:35 p.m. UTC | #12
On Thu, May 23, 2013 at 12:13:16PM +0300, Gleb Natapov wrote:
> > >
> > >"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.
> > 
> Reference TSC is even faster. There should be no crashed with proper
> implementation.
> 
> --
> 			Gleb.

Lack of invariant TSC support in the host.

--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Marcelo Tosatti May 23, 2013, 1:47 p.m. UTC | #13
On Thu, May 23, 2013 at 08:21:29AM -0400, Vadim Rozenfeld wrote:
> > > @@ -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?
> 
> [VR]
> Because of different frequencies, I think. 
> Hyper-V reference counters and iTSC report
> performance frequency equal to 10MHz,
> which is obviously is not true for PM and HPET timers.

Windows has to convert from the native hardware clock frequency to
internal system frequency, so i don't believe this is a problem.

> Windows calibrates timers on boot-up and you probably 
> have no chance to do it after or during resume.  

It is documented as such, it has been designed to fallback
to other hardware clock devices. Is there evidence for any 
problem on fallback?

Earlier you said:

"> What if you put 0xFFFFFFFF as a sequence? Or is this another case where
> the spec is wrong.
>
it will use PMTimer (maybe HPET if you have it) if you specify it on
VM's start up. But I'm not sure if it will work if you migrate from TSC
or reference counter to 0xFFFFFFFF
"


--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Marcelo Tosatti May 23, 2013, 1:53 p.m. UTC | #14
On Thu, May 23, 2013 at 12:12:29PM +0300, Gleb Natapov wrote:
> > 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?
> > 
> Please read the whole discussion, we talked about it already. We
> definitely do not want to fall back to PM timer either, we want to use
> reference counter instead.

Case 1) On migration of TSC page enabled Windows guest, from invariant TSC host,
to non-invariant TSC host, Windows guests fallback to PMTimer
and not to reference timer via MSR. 

This is suboptimal because pmtimer emulation is excessively slow.

Is there a better option?

Case 2)
Reference timer (via MSR) support is interesting for the case of non invariant TSC
host.

--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Gleb Natapov May 23, 2013, 3:14 p.m. UTC | #15
On Thu, May 23, 2013 at 10:35:59AM -0300, Marcelo Tosatti wrote:
> On Thu, May 23, 2013 at 12:13:16PM +0300, Gleb Natapov wrote:
> > > >
> > > >"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.
> > > 
> > Reference TSC is even faster. There should be no crashed with proper
> > implementation.
> > 
> > --
> > 			Gleb.
> 
> Lack of invariant TSC support in the host.
Proper implementation will disable reference TSC in this case.

--
			Gleb.
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Gleb Natapov May 23, 2013, 3:31 p.m. UTC | #16
On Thu, May 23, 2013 at 10:53:38AM -0300, Marcelo Tosatti wrote:
> On Thu, May 23, 2013 at 12:12:29PM +0300, Gleb Natapov wrote:
> > > 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?
> > > 
> > Please read the whole discussion, we talked about it already. We
> > definitely do not want to fall back to PM timer either, we want to use
> > reference counter instead.
> 
> Case 1) On migration of TSC page enabled Windows guest, from invariant TSC host,
> to non-invariant TSC host, Windows guests fallback to PMTimer
> and not to reference timer via MSR. 
> 
> This is suboptimal because pmtimer emulation is excessively slow.
> 
> Is there a better option?
> 
If setting TscSequence to zero makes Windows fall back to the MSR this is a
better option.

> Case 2)
> Reference timer (via MSR) support is interesting for the case of non invariant TSC
> host.

--
			Gleb.
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Paolo Bonzini May 23, 2013, 4:44 p.m. UTC | #17
Il 19/05/2013 09:06, Vadim Rozenfeld ha scritto:
> 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;

Thinking more of migration, could we increment whatever sequence value
we found (or better, do (x|3)+2 to skip over 0 and 0xFFFFFFFF), instead
of forcing it to 1?

Add HV_X64_MSR_REFERENCE_TSC to msrs_to_save, and migration should just
work.

Paolo

> +		tsc_ref.TscScale =
> +				((10000LL << 32) / vcpu->arch.virtual_tsc_khz) << 32;
> +		tsc_ref.TscOffset = 0;
>  		if (__copy_to_user((void __user *)addr, &tsc_ref, sizeof(tsc_ref)))
>  			return 1;
>  		mark_page_dirty(kvm, gfn);
> 

--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Vadim Rozenfeld May 24, 2013, 9:57 a.m. UTC | #18
----- Original Message -----
From: "Marcelo Tosatti" <mtosatti@redhat.com>
To: "Gleb Natapov" <gleb@redhat.com>
Cc: "Peter Lieven" <pl@dlhnet.de>, "Vadim Rozenfeld" <vrozenfe@redhat.com>, kvm@vger.kernel.org, pl@dlh.net
Sent: Thursday, May 23, 2013 11:35:59 PM
Subject: Re: [RFC PATCH v2 2/2] add support for Hyper-V invariant TSC

On Thu, May 23, 2013 at 12:13:16PM +0300, Gleb Natapov wrote:
> > >
> > >"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.
> > 
> Reference TSC is even faster. There should be no crashed with proper
> implementation.
> 
> --
> 			Gleb.

Lack of invariant TSC support in the host.

if there is no iTSC in the host -> set sequence to 0 and go with
reference counter. It is why they both scaled to 10 MHz, and it's
why reference counters is a fall-back for iTSC. 

--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Vadim Rozenfeld May 24, 2013, 10:01 a.m. UTC | #19
----- 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: Thursday, May 23, 2013 11:47:46 PM
Subject: Re: [RFC PATCH v2 2/2] add support for Hyper-V invariant TSC

On Thu, May 23, 2013 at 08:21:29AM -0400, Vadim Rozenfeld wrote:
> > > @@ -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?
> 
> [VR]
> Because of different frequencies, I think. 
> Hyper-V reference counters and iTSC report
> performance frequency equal to 10MHz,
> which is obviously is not true for PM and HPET timers.

Windows has to convert from the native hardware clock frequency to
internal system frequency, so i don't believe this is a problem.

> Windows calibrates timers on boot-up and you probably 
> have no chance to do it after or during resume.  

It is documented as such, it has been designed to fallback
to other hardware clock devices. Is there evidence for any 
problem on fallback?

Earlier you said:

"> What if you put 0xFFFFFFFF as a sequence? Or is this another case where
> the spec is wrong.
>
it will use PMTimer (maybe HPET if you have it) if you specify it on
VM's start up. But I'm not sure if it will work if you migrate from TSC
or reference counter to 0xFFFFFFFF
"
On startup, not after migration, when you migrate to host w/o iTSC and/or 
reference counters support.


--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Vadim Rozenfeld May 24, 2013, 10:11 a.m. UTC | #20
----- Original Message -----
From: "Gleb Natapov" <gleb@redhat.com>
To: "Marcelo Tosatti" <mtosatti@redhat.com>
Cc: "Vadim Rozenfeld" <vrozenfe@redhat.com>, kvm@vger.kernel.org, pl@dlh.net
Sent: Friday, May 24, 2013 1:31:10 AM
Subject: Re: [RFC PATCH v2 2/2] add support for Hyper-V invariant TSC

On Thu, May 23, 2013 at 10:53:38AM -0300, Marcelo Tosatti wrote:
> On Thu, May 23, 2013 at 12:12:29PM +0300, Gleb Natapov wrote:
> > > 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?
> > > 
> > Please read the whole discussion, we talked about it already. We
> > definitely do not want to fall back to PM timer either, we want to use
> > reference counter instead.
> 
> Case 1) On migration of TSC page enabled Windows guest, from invariant TSC host,
> to non-invariant TSC host, Windows guests fallback to PMTimer
> and not to reference timer via MSR. 
> 
> This is suboptimal because pmtimer emulation is excessively slow.
> 
> Is there a better option?
> 
If setting TscSequence to zero makes Windows fall back to the MSR this is a
better option.

+1 
This is why MS has two different mechanisms:
iTSC as a primary, reference counters as a fall-back.

  
> Case 2)
> Reference timer (via MSR) support is interesting for the case of non invariant TSC
> host.

--
			Gleb.
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Vadim Rozenfeld May 24, 2013, 10:16 a.m. UTC | #21
----- Original Message -----
From: "Paolo Bonzini" <pbonzini@redhat.com>
To: "Vadim Rozenfeld" <vrozenfe@redhat.com>
Cc: kvm@vger.kernel.org, gleb@redhat.com, mtosatti@redhat.com, pl@dlh.net
Sent: Friday, May 24, 2013 2:44:50 AM
Subject: Re: [RFC PATCH v2 2/2] add support for Hyper-V invariant TSC

Il 19/05/2013 09:06, Vadim Rozenfeld ha scritto:
> 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;

Thinking more of migration, could we increment whatever sequence value
we found (or better, do (x|3)+2 to skip over 0 and 0xFFFFFFFF), instead
of forcing it to 1?

[VR]
Yes, it should work.
We need to keep sequence between 1 and 0xFFFFFFFF and increment it every time
the VM was migrated or paused/resumed.

Add HV_X64_MSR_REFERENCE_TSC to msrs_to_save, and migration should just
work.

Paolo

> +		tsc_ref.TscScale =
> +				((10000LL << 32) / vcpu->arch.virtual_tsc_khz) << 32;
> +		tsc_ref.TscOffset = 0;
>  		if (__copy_to_user((void __user *)addr, &tsc_ref, sizeof(tsc_ref)))
>  			return 1;
>  		mark_page_dirty(kvm, gfn);
> 

--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Marcelo Tosatti May 24, 2013, 7:41 p.m. UTC | #22
On Fri, May 24, 2013 at 06:11:16AM -0400, Vadim Rozenfeld wrote:
> > Is there a better option?
> > 
> If setting TscSequence to zero makes Windows fall back to the MSR this is a
> better option.
> 
> +1 
> This is why MS has two different mechanisms:
> iTSC as a primary, reference counters as a fall-back.

Ok, is it documented that transition

iTSC valid (Sequence != 0 and != 0xFFFFFFFF) -> 
iTSC not valid but ref MSR valid (Sequence = 0), 

is a valid transition?

It was not obvious for me. Can you point to documentation?


--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Vadim Rozenfeld May 27, 2013, 12:33 p.m. UTC | #23
On Fri, 2013-05-24 at 16:41 -0300, Marcelo Tosatti wrote:
> On Fri, May 24, 2013 at 06:11:16AM -0400, Vadim Rozenfeld wrote:
> > > Is there a better option?
> > > 
> > If setting TscSequence to zero makes Windows fall back to the MSR this is a
> > better option.
> > 
> > +1 
> > This is why MS has two different mechanisms:
> > iTSC as a primary, reference counters as a fall-back.
> 
> Ok, is it documented that transition
> 
> iTSC valid (Sequence != 0 and != 0xFFFFFFFF) -> 
> iTSC not valid but ref MSR valid (Sequence = 0), 
> 
> is a valid transition?
> 
Yes, it's true.
> It was not obvious for me. Can you point to documentation?
> 
> 
Hypervisor Functional Specification v2.0a: For Windows Server 2008 R2
http://www.microsoft.com/download/en/details.aspx?displaylang=en&id=18673

"15.4.3.3 Reference TSC during Save/Restore and Migration
To address migration scenarios to physical platforms which do not
support iTSC, the TscSequence field is used. In the event 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 0x0, which
directs the guest operating system to fall back to a different clock
source (the virtual PM timer)."

Now what the virtual PM timer is - if hypervisor provides PM Timer
assist support (HvPartitionPropertyPmTimerAssist partition property),
it will use partition reference counters to calculate PM Timer value.
If partition has no HvPartitionPropertyPmTimerAssist - guest will use
reference counters MSR directly.
Currently we don't support PM timer assist, so TscSequence 0x0 means
fallback to reference counters.






--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

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;
+		tsc_ref.TscScale =
+				((10000LL << 32) / vcpu->arch.virtual_tsc_khz) << 32;
+		tsc_ref.TscOffset = 0;
 		if (__copy_to_user((void __user *)addr, &tsc_ref, sizeof(tsc_ref)))
 			return 1;
 		mark_page_dirty(kvm, gfn);