[RFC PATCH 0/2] Hyper-V timers

[RFC PATCH 0/2] Hyper-V timers

[Vadim Rozenfeld]

This RFC series adds support for two Hyper-V timer services -
a per-partition reference time counter, and a  partition reference 
time enlightenmen.

Vadim Rozenfeld (2):
  hyper-v reference counter
  Hyper-V iTSC handler

 arch/x86/include/asm/kvm_host.h    |  2 ++
 arch/x86/include/uapi/asm/hyperv.h | 13 +++++++++++++
 arch/x86/kvm/x86.c                 | 33 ++++++++++++++++++++++++++++++++-
 include/uapi/linux/kvm.h           |  1 +
 4 files changed, 48 insertions(+), 1 deletion(-)

-- 
1.8.1.2

[RFC PATCH 1/2] Hyper-H reference counter

[Vadim Rozenfeld]

Signed-off: Peter Lieven <pl@dlh.net>
Signed-off: Gleb Natapov <gleb@redhat.com>
Signed-off: Vadim Rozenfeld <vrozenfe@redhat.com>

The following patch allows to activate Hyper-V 
reference time counter 
---
 arch/x86/include/asm/kvm_host.h    |  2 ++
 arch/x86/include/uapi/asm/hyperv.h |  3 +++
 arch/x86/kvm/x86.c                 | 25 ++++++++++++++++++++++++-
 3 files changed, 29 insertions(+), 1 deletion(-)

diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
index 3741c65..f0fee35 100644
--- a/arch/x86/include/asm/kvm_host.h
+++ b/arch/x86/include/asm/kvm_host.h
@@ -575,6 +575,8 @@ struct kvm_arch {
 	/* fields used by HYPER-V emulation */
 	u64 hv_guest_os_id;
 	u64 hv_hypercall;
+	u64 hv_ref_count;
+	u64 hv_tsc_page;
 
 	#ifdef CONFIG_KVM_MMU_AUDIT
 	int audit_point;
diff --git a/arch/x86/include/uapi/asm/hyperv.h b/arch/x86/include/uapi/asm/hyperv.h
index b80420b..9711819 100644
--- a/arch/x86/include/uapi/asm/hyperv.h
+++ b/arch/x86/include/uapi/asm/hyperv.h
@@ -136,6 +136,9 @@
 /* MSR used to read the per-partition time reference counter */
 #define HV_X64_MSR_TIME_REF_COUNT		0x40000020
 
+/* A partition's reference time stamp counter (TSC) page */
+#define HV_X64_MSR_REFERENCE_TSC		0x40000021
+
 /* Define the virtual APIC registers */
 #define HV_X64_MSR_EOI				0x40000070
 #define HV_X64_MSR_ICR				0x40000071
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index 094b5d9..1a4036d 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -843,7 +843,7 @@ EXPORT_SYMBOL_GPL(kvm_rdpmc);
 static u32 msrs_to_save[] = {
 	MSR_KVM_SYSTEM_TIME, MSR_KVM_WALL_CLOCK,
 	MSR_KVM_SYSTEM_TIME_NEW, MSR_KVM_WALL_CLOCK_NEW,
-	HV_X64_MSR_GUEST_OS_ID, HV_X64_MSR_HYPERCALL,
+	HV_X64_MSR_GUEST_OS_ID, HV_X64_MSR_HYPERCALL,HV_X64_MSR_TIME_REF_COUNT,
 	HV_X64_MSR_APIC_ASSIST_PAGE, MSR_KVM_ASYNC_PF_EN, MSR_KVM_STEAL_TIME,
 	MSR_KVM_PV_EOI_EN,
 	MSR_IA32_SYSENTER_CS, MSR_IA32_SYSENTER_ESP, MSR_IA32_SYSENTER_EIP,
@@ -1764,6 +1764,8 @@ static bool kvm_hv_msr_partition_wide(u32 msr)
 	switch (msr) {
 	case HV_X64_MSR_GUEST_OS_ID:
 	case HV_X64_MSR_HYPERCALL:
+	case HV_X64_MSR_REFERENCE_TSC:
+	case HV_X64_MSR_TIME_REF_COUNT:
 		r = true;
 		break;
 	}
@@ -1803,6 +1805,21 @@ static int set_msr_hyperv_pw(struct kvm_vcpu *vcpu, u32 msr, u64 data)
 		if (__copy_to_user((void __user *)addr, instructions, 4))
 			return 1;
 		kvm->arch.hv_hypercall = data;
+		kvm->arch.hv_ref_count = get_kernel_ns();
+		break;
+	}
+	case HV_X64_MSR_REFERENCE_TSC: {
+		u64 gfn;
+		unsigned long addr;
+		u32 tsc_ref;
+		gfn = data >> HV_X64_MSR_HYPERCALL_PAGE_ADDRESS_SHIFT;
+		addr = gfn_to_hva(kvm, gfn);
+		if (kvm_is_error_hva(addr))
+			return 1;
+		tsc_ref = 0;
+		if(__copy_to_user((void __user *)addr, &tsc_ref, sizeof(tsc_ref)))
+			return 1;
+		kvm->arch.hv_tsc_page = data;
 		break;
 	}
 	default:
@@ -2229,6 +2246,12 @@ static int get_msr_hyperv_pw(struct kvm_vcpu *vcpu, u32 msr, u64 *pdata)
 	case HV_X64_MSR_HYPERCALL:
 		data = kvm->arch.hv_hypercall;
 		break;
+	case HV_X64_MSR_TIME_REF_COUNT: 
+		data = div_u64(get_kernel_ns() - kvm->arch.hv_ref_count,100); 
+		break;
+	case HV_X64_MSR_REFERENCE_TSC:
+		data = kvm->arch.hv_tsc_page;
+		break;
 	default:
 		vcpu_unimpl(vcpu, "Hyper-V unhandled rdmsr: 0x%x\n", msr);
 		return 1;
-- 
1.8.1.2

[RFC PATCH 2/2] Hyper-V iTSC handler

[Vadim Rozenfeld]

Signed-off: Vadim Rozenfeld <vrozenfe@redhat.com>

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.

---
 arch/x86/include/uapi/asm/hyperv.h | 10 ++++++++++
 arch/x86/kvm/x86.c                 | 18 +++++++++++++-----
 include/uapi/linux/kvm.h           |  1 +
 3 files changed, 24 insertions(+), 5 deletions(-)

diff --git a/arch/x86/include/uapi/asm/hyperv.h b/arch/x86/include/uapi/asm/hyperv.h
index 9711819..2d9e666 100644
--- a/arch/x86/include/uapi/asm/hyperv.h
+++ b/arch/x86/include/uapi/asm/hyperv.h
@@ -182,6 +182,9 @@
 #define HV_X64_MSR_APIC_ASSIST_PAGE_ADDRESS_MASK	\
 		(~((1ull << HV_X64_MSR_APIC_ASSIST_PAGE_ADDRESS_SHIFT) - 1))
 
+#define HV_X64_MSR_TSC_REFERENCE_ENABLE			0x00000001
+#define HV_X64_MSR_TSC_REFERENCE_ADDRESS_SHIFT		12
+
 #define HV_PROCESSOR_POWER_STATE_C0		0
 #define HV_PROCESSOR_POWER_STATE_C1		1
 #define HV_PROCESSOR_POWER_STATE_C2		2
@@ -194,4 +197,11 @@
 #define HV_STATUS_INVALID_ALIGNMENT		4
 #define HV_STATUS_INSUFFICIENT_BUFFERS		19
 
+typedef struct _HV_REFERENCE_TSC_PAGE {
+	uint32_t TscSequence;
+	uint32_t Rserved1;
+	uint64_t TscScale;
+	int64_t  TscOffset;
+} HV_REFERENCE_TSC_PAGE, * PHV_REFERENCE_TSC_PAGE;
+
 #endif
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index 1a4036d..5788e8f 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -1809,14 +1809,21 @@ static int set_msr_hyperv_pw(struct kvm_vcpu *vcpu, u32 msr, u64 data)
 		break;
 	}
 	case HV_X64_MSR_REFERENCE_TSC: {
-		u64 gfn;
 		unsigned long addr;
-		u32 tsc_ref;
-		gfn = data >> HV_X64_MSR_HYPERCALL_PAGE_ADDRESS_SHIFT;
-		addr = gfn_to_hva(kvm, gfn);
+		HV_REFERENCE_TSC_PAGE tsc_ref;
+		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 (!(data & HV_X64_MSR_TSC_REFERENCE_ENABLE)) {
+			kvm->arch.hv_tsc_page = data;
+			break;
+		}
+		addr = gfn_to_hva(vcpu->kvm, data >>
+			HV_X64_MSR_TSC_REFERENCE_ADDRESS_SHIFT);
 		if (kvm_is_error_hva(addr))
 			return 1;
-		tsc_ref = 0;
 		if(__copy_to_user((void __user *)addr, &tsc_ref, sizeof(tsc_ref)))
 			return 1;
 		kvm->arch.hv_tsc_page = data;
@@ -2553,6 +2560,7 @@ int kvm_dev_ioctl_check_extension(long ext)
 	case KVM_CAP_HYPERV:
 	case KVM_CAP_HYPERV_VAPIC:
 	case KVM_CAP_HYPERV_SPIN:
+	case KVM_CAP_HYPERV_TSC:
 	case KVM_CAP_PCI_SEGMENT:
 	case KVM_CAP_DEBUGREGS:
 	case KVM_CAP_X86_ROBUST_SINGLESTEP:
diff --git a/include/uapi/linux/kvm.h b/include/uapi/linux/kvm.h
index a5c86fc..8eff540 100644
--- a/include/uapi/linux/kvm.h
+++ b/include/uapi/linux/kvm.h
@@ -666,6 +666,7 @@ struct kvm_ppc_smmu_info {
 #define KVM_CAP_IRQ_MPIC 90
 #define KVM_CAP_PPC_RTAS 91
 #define KVM_CAP_IRQ_XICS 92
+#define KVM_CAP_HYPERV_TSC 93
 
 #ifdef KVM_CAP_IRQ_ROUTING
 
-- 
1.8.1.2

Re: [RFC PATCH 1/2] Hyper-H reference counter

[Eric Northup]

On Mon, May 13, 2013 at 4:45 AM, Vadim Rozenfeld <vrozenfe@redhat.com> wrote:
> Signed-off: Peter Lieven <pl@dlh.net>
> Signed-off: Gleb Natapov <gleb@redhat.com>
> Signed-off: Vadim Rozenfeld <vrozenfe@redhat.com>
>
> The following patch allows to activate Hyper-V
> reference time counter
> ---
>  arch/x86/include/asm/kvm_host.h    |  2 ++
>  arch/x86/include/uapi/asm/hyperv.h |  3 +++
>  arch/x86/kvm/x86.c                 | 25 ++++++++++++++++++++++++-
>  3 files changed, 29 insertions(+), 1 deletion(-)
>
> diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
> index 3741c65..f0fee35 100644
> --- a/arch/x86/include/asm/kvm_host.h
> +++ b/arch/x86/include/asm/kvm_host.h
> @@ -575,6 +575,8 @@ struct kvm_arch {
>         /* fields used by HYPER-V emulation */
>         u64 hv_guest_os_id;
>         u64 hv_hypercall;
> +       u64 hv_ref_count;
> +       u64 hv_tsc_page;
>
>         #ifdef CONFIG_KVM_MMU_AUDIT
>         int audit_point;
> diff --git a/arch/x86/include/uapi/asm/hyperv.h b/arch/x86/include/uapi/asm/hyperv.h
> index b80420b..9711819 100644
> --- a/arch/x86/include/uapi/asm/hyperv.h
> +++ b/arch/x86/include/uapi/asm/hyperv.h
> @@ -136,6 +136,9 @@
>  /* MSR used to read the per-partition time reference counter */
>  #define HV_X64_MSR_TIME_REF_COUNT              0x40000020
>
> +/* A partition's reference time stamp counter (TSC) page */
> +#define HV_X64_MSR_REFERENCE_TSC               0x40000021
> +
>  /* Define the virtual APIC registers */
>  #define HV_X64_MSR_EOI                         0x40000070
>  #define HV_X64_MSR_ICR                         0x40000071
> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> index 094b5d9..1a4036d 100644
> --- a/arch/x86/kvm/x86.c
> +++ b/arch/x86/kvm/x86.c
> @@ -843,7 +843,7 @@ EXPORT_SYMBOL_GPL(kvm_rdpmc);
>  static u32 msrs_to_save[] = {
>         MSR_KVM_SYSTEM_TIME, MSR_KVM_WALL_CLOCK,
>         MSR_KVM_SYSTEM_TIME_NEW, MSR_KVM_WALL_CLOCK_NEW,
> -       HV_X64_MSR_GUEST_OS_ID, HV_X64_MSR_HYPERCALL,
> +       HV_X64_MSR_GUEST_OS_ID, HV_X64_MSR_HYPERCALL,HV_X64_MSR_TIME_REF_COUNT,
>         HV_X64_MSR_APIC_ASSIST_PAGE, MSR_KVM_ASYNC_PF_EN, MSR_KVM_STEAL_TIME,
>         MSR_KVM_PV_EOI_EN,
>         MSR_IA32_SYSENTER_CS, MSR_IA32_SYSENTER_ESP, MSR_IA32_SYSENTER_EIP,
> @@ -1764,6 +1764,8 @@ static bool kvm_hv_msr_partition_wide(u32 msr)
>         switch (msr) {
>         case HV_X64_MSR_GUEST_OS_ID:
>         case HV_X64_MSR_HYPERCALL:
> +       case HV_X64_MSR_REFERENCE_TSC:
> +       case HV_X64_MSR_TIME_REF_COUNT:
>                 r = true;
>                 break;
>         }
> @@ -1803,6 +1805,21 @@ static int set_msr_hyperv_pw(struct kvm_vcpu *vcpu, u32 msr, u64 data)
>                 if (__copy_to_user((void __user *)addr, instructions, 4))
>                         return 1;
>                 kvm->arch.hv_hypercall = data;
> +               kvm->arch.hv_ref_count = get_kernel_ns();
> +               break;
> +       }
> +       case HV_X64_MSR_REFERENCE_TSC: {
> +               u64 gfn;
> +               unsigned long addr;
> +               u32 tsc_ref;
> +               gfn = data >> HV_X64_MSR_HYPERCALL_PAGE_ADDRESS_SHIFT;
> +               addr = gfn_to_hva(kvm, gfn);
> +               if (kvm_is_error_hva(addr))
> +                       return 1;
> +               tsc_ref = 0;
> +               if(__copy_to_user((void __user *)addr, &tsc_ref, sizeof(tsc_ref)))

Does this do the right thing when we're migrating?  How does usermode
learn that the guest page has been dirtied?

> +                       return 1;
> +               kvm->arch.hv_tsc_page = data;
>                 break;
>         }
>         default:
> @@ -2229,6 +2246,12 @@ static int get_msr_hyperv_pw(struct kvm_vcpu *vcpu, u32 msr, u64 *pdata)
>         case HV_X64_MSR_HYPERCALL:
>                 data = kvm->arch.hv_hypercall;
>                 break;
> +       case HV_X64_MSR_TIME_REF_COUNT:
> +               data = div_u64(get_kernel_ns() - kvm->arch.hv_ref_count,100);
> +               break;
> +       case HV_X64_MSR_REFERENCE_TSC:
> +               data = kvm->arch.hv_tsc_page;
> +               break;
>         default:
>                 vcpu_unimpl(vcpu, "Hyper-V unhandled rdmsr: 0x%x\n", msr);
>                 return 1;
> --
> 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

Re: [RFC PATCH 1/2] Hyper-H reference counter

[Vadim Rozenfeld]

On Mon, 2013-05-13 at 16:30 -0700, Eric Northup wrote:
> On Mon, May 13, 2013 at 4:45 AM, Vadim Rozenfeld <vrozenfe@redhat.com> wrote:
> > Signed-off: Peter Lieven <pl@dlh.net>
> > Signed-off: Gleb Natapov <gleb@redhat.com>
> > Signed-off: Vadim Rozenfeld <vrozenfe@redhat.com>
> >
> > The following patch allows to activate Hyper-V
> > reference time counter
> > ---
> >  arch/x86/include/asm/kvm_host.h    |  2 ++
> >  arch/x86/include/uapi/asm/hyperv.h |  3 +++
> >  arch/x86/kvm/x86.c                 | 25 ++++++++++++++++++++++++-
> >  3 files changed, 29 insertions(+), 1 deletion(-)
> >
> > diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
> > index 3741c65..f0fee35 100644
> > --- a/arch/x86/include/asm/kvm_host.h
> > +++ b/arch/x86/include/asm/kvm_host.h
> > @@ -575,6 +575,8 @@ struct kvm_arch {
> >         /* fields used by HYPER-V emulation */
> >         u64 hv_guest_os_id;
> >         u64 hv_hypercall;
> > +       u64 hv_ref_count;
> > +       u64 hv_tsc_page;
> >
> >         #ifdef CONFIG_KVM_MMU_AUDIT
> >         int audit_point;
> > diff --git a/arch/x86/include/uapi/asm/hyperv.h b/arch/x86/include/uapi/asm/hyperv.h
> > index b80420b..9711819 100644
> > --- a/arch/x86/include/uapi/asm/hyperv.h
> > +++ b/arch/x86/include/uapi/asm/hyperv.h
> > @@ -136,6 +136,9 @@
> >  /* MSR used to read the per-partition time reference counter */
> >  #define HV_X64_MSR_TIME_REF_COUNT              0x40000020
> >
> > +/* A partition's reference time stamp counter (TSC) page */
> > +#define HV_X64_MSR_REFERENCE_TSC               0x40000021
> > +
> >  /* Define the virtual APIC registers */
> >  #define HV_X64_MSR_EOI                         0x40000070
> >  #define HV_X64_MSR_ICR                         0x40000071
> > diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> > index 094b5d9..1a4036d 100644
> > --- a/arch/x86/kvm/x86.c
> > +++ b/arch/x86/kvm/x86.c
> > @@ -843,7 +843,7 @@ EXPORT_SYMBOL_GPL(kvm_rdpmc);
> >  static u32 msrs_to_save[] = {
> >         MSR_KVM_SYSTEM_TIME, MSR_KVM_WALL_CLOCK,
> >         MSR_KVM_SYSTEM_TIME_NEW, MSR_KVM_WALL_CLOCK_NEW,
> > -       HV_X64_MSR_GUEST_OS_ID, HV_X64_MSR_HYPERCALL,
> > +       HV_X64_MSR_GUEST_OS_ID, HV_X64_MSR_HYPERCALL,HV_X64_MSR_TIME_REF_COUNT,
> >         HV_X64_MSR_APIC_ASSIST_PAGE, MSR_KVM_ASYNC_PF_EN, MSR_KVM_STEAL_TIME,
> >         MSR_KVM_PV_EOI_EN,
> >         MSR_IA32_SYSENTER_CS, MSR_IA32_SYSENTER_ESP, MSR_IA32_SYSENTER_EIP,
> > @@ -1764,6 +1764,8 @@ static bool kvm_hv_msr_partition_wide(u32 msr)
> >         switch (msr) {
> >         case HV_X64_MSR_GUEST_OS_ID:
> >         case HV_X64_MSR_HYPERCALL:
> > +       case HV_X64_MSR_REFERENCE_TSC:
> > +       case HV_X64_MSR_TIME_REF_COUNT:
> >                 r = true;
> >                 break;
> >         }
> > @@ -1803,6 +1805,21 @@ static int set_msr_hyperv_pw(struct kvm_vcpu *vcpu, u32 msr, u64 data)
> >                 if (__copy_to_user((void __user *)addr, instructions, 4))
> >                         return 1;
> >                 kvm->arch.hv_hypercall = data;
> > +               kvm->arch.hv_ref_count = get_kernel_ns();
> > +               break;
> > +       }
> > +       case HV_X64_MSR_REFERENCE_TSC: {
> > +               u64 gfn;
> > +               unsigned long addr;
> > +               u32 tsc_ref;
> > +               gfn = data >> HV_X64_MSR_HYPERCALL_PAGE_ADDRESS_SHIFT;
> > +               addr = gfn_to_hva(kvm, gfn);
> > +               if (kvm_is_error_hva(addr))
> > +                       return 1;
> > +               tsc_ref = 0;
> > +               if(__copy_to_user((void __user *)addr, &tsc_ref, sizeof(tsc_ref)))
> 
> Does this do the right thing when we're migrating?  How does usermode
> learn that the guest page has been dirtied?
> 

No, it shouldn't be a problem for this patch. Guest allocates a page
from nonpaged physical memory, maps it to the system address space, gets
physical address and sends it to KVM. KVM sets the first DWORD
(TscSequence) to zero, which means that guest will use reference time
counter as a timestamp source even after migration.

> > +                       return 1;
> > +               kvm->arch.hv_tsc_page = data;
> >                 break;
> >         }
> >         default:
> > @@ -2229,6 +2246,12 @@ static int get_msr_hyperv_pw(struct kvm_vcpu *vcpu, u32 msr, u64 *pdata)
> >         case HV_X64_MSR_HYPERCALL:
> >                 data = kvm->arch.hv_hypercall;
> >                 break;
> > +       case HV_X64_MSR_TIME_REF_COUNT:
> > +               data = div_u64(get_kernel_ns() - kvm->arch.hv_ref_count,100);
> > +               break;
> > +       case HV_X64_MSR_REFERENCE_TSC:
> > +               data = kvm->arch.hv_tsc_page;
> > +               break;
> >         default:
> >                 vcpu_unimpl(vcpu, "Hyper-V unhandled rdmsr: 0x%x\n", msr);
> >                 return 1;
> > --
> > 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

Re: [RFC PATCH 1/2] Hyper-H reference counter

[Peter Lieven]

On 13.05.2013 13:45, Vadim Rozenfeld wrote:
> Signed-off: Peter Lieven <pl@dlh.net>
> Signed-off: Gleb Natapov <gleb@redhat.com>
> Signed-off: Vadim Rozenfeld <vrozenfe@redhat.com>
>
> The following patch allows to activate Hyper-V
> reference time counter
> ---
>   arch/x86/include/asm/kvm_host.h    |  2 ++
>   arch/x86/include/uapi/asm/hyperv.h |  3 +++
>   arch/x86/kvm/x86.c                 | 25 ++++++++++++++++++++++++-
>   3 files changed, 29 insertions(+), 1 deletion(-)
>
> diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
> index 3741c65..f0fee35 100644
> --- a/arch/x86/include/asm/kvm_host.h
> +++ b/arch/x86/include/asm/kvm_host.h
> @@ -575,6 +575,8 @@ struct kvm_arch {
>   	/* fields used by HYPER-V emulation */
>   	u64 hv_guest_os_id;
>   	u64 hv_hypercall;
> +	u64 hv_ref_count;
> +	u64 hv_tsc_page;
>
>   	#ifdef CONFIG_KVM_MMU_AUDIT
>   	int audit_point;
> diff --git a/arch/x86/include/uapi/asm/hyperv.h b/arch/x86/include/uapi/asm/hyperv.h
> index b80420b..9711819 100644
> --- a/arch/x86/include/uapi/asm/hyperv.h
> +++ b/arch/x86/include/uapi/asm/hyperv.h
> @@ -136,6 +136,9 @@
>   /* MSR used to read the per-partition time reference counter */
>   #define HV_X64_MSR_TIME_REF_COUNT		0x40000020
>
> +/* A partition's reference time stamp counter (TSC) page */
> +#define HV_X64_MSR_REFERENCE_TSC		0x40000021
> +
>   /* Define the virtual APIC registers */
>   #define HV_X64_MSR_EOI				0x40000070
>   #define HV_X64_MSR_ICR				0x40000071
> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> index 094b5d9..1a4036d 100644
> --- a/arch/x86/kvm/x86.c
> +++ b/arch/x86/kvm/x86.c
> @@ -843,7 +843,7 @@ EXPORT_SYMBOL_GPL(kvm_rdpmc);
>   static u32 msrs_to_save[] = {
>   	MSR_KVM_SYSTEM_TIME, MSR_KVM_WALL_CLOCK,
>   	MSR_KVM_SYSTEM_TIME_NEW, MSR_KVM_WALL_CLOCK_NEW,
> -	HV_X64_MSR_GUEST_OS_ID, HV_X64_MSR_HYPERCALL,
> +	HV_X64_MSR_GUEST_OS_ID, HV_X64_MSR_HYPERCALL,HV_X64_MSR_TIME_REF_COUNT,
>   	HV_X64_MSR_APIC_ASSIST_PAGE, MSR_KVM_ASYNC_PF_EN, MSR_KVM_STEAL_TIME,
>   	MSR_KVM_PV_EOI_EN,
>   	MSR_IA32_SYSENTER_CS, MSR_IA32_SYSENTER_ESP, MSR_IA32_SYSENTER_EIP,
> @@ -1764,6 +1764,8 @@ static bool kvm_hv_msr_partition_wide(u32 msr)
>   	switch (msr) {
>   	case HV_X64_MSR_GUEST_OS_ID:
>   	case HV_X64_MSR_HYPERCALL:
> +	case HV_X64_MSR_REFERENCE_TSC:
> +	case HV_X64_MSR_TIME_REF_COUNT:
>   		r = true;
>   		break;
>   	}
> @@ -1803,6 +1805,21 @@ static int set_msr_hyperv_pw(struct kvm_vcpu *vcpu, u32 msr, u64 data)
>   		if (__copy_to_user((void __user *)addr, instructions, 4))
>   			return 1;
>   		kvm->arch.hv_hypercall = data;
> +		kvm->arch.hv_ref_count = get_kernel_ns();
> +		break;
> +	}
> +	case HV_X64_MSR_REFERENCE_TSC: {
> +		u64 gfn;
> +		unsigned long addr;
> +		u32 tsc_ref;
> +		gfn = data >> HV_X64_MSR_HYPERCALL_PAGE_ADDRESS_SHIFT;
> +		addr = gfn_to_hva(kvm, gfn);
> +		if (kvm_is_error_hva(addr))
> +			return 1;
> +		tsc_ref = 0;
> +		if(__copy_to_user((void __user *)addr, &tsc_ref, sizeof(tsc_ref)))
> +			return 1;
> +		kvm->arch.hv_tsc_page = data;
>   		break;
>   	}
>   	default:
> @@ -2229,6 +2246,12 @@ static int get_msr_hyperv_pw(struct kvm_vcpu *vcpu, u32 msr, u64 *pdata)
>   	case HV_X64_MSR_HYPERCALL:
>   		data = kvm->arch.hv_hypercall;
>   		break;
> +	case HV_X64_MSR_TIME_REF_COUNT:
> +		data = div_u64(get_kernel_ns() - kvm->arch.hv_ref_count,100);
> +		break;


in an earlier version of this patch I have the following:

+       case HV_X64_MSR_TIME_REF_COUNT: {
+               u64 now_ns;
+               local_irq_disable();
+               now_ns = get_kernel_ns();
+               data = div_u64(now_ns + kvm->arch.kvmclock_offset - kvm->arch.hv_ref_count,100);
+               local_irq_enable();
+               break;
+       }

I do not know if this is right, but I can report that this one is working without any flaws
since approx. 1.5 years.

Peter

Re: [RFC PATCH 1/2] Hyper-H reference counter

[Vadim Rozenfeld]

On Tue, 2013-05-14 at 16:14 +0200, Peter Lieven wrote:
> On 13.05.2013 13:45, Vadim Rozenfeld wrote:
> > Signed-off: Peter Lieven <pl@dlh.net>
> > Signed-off: Gleb Natapov <gleb@redhat.com>
> > Signed-off: Vadim Rozenfeld <vrozenfe@redhat.com>
> >
> > The following patch allows to activate Hyper-V
> > reference time counter
> > ---
> >   arch/x86/include/asm/kvm_host.h    |  2 ++
> >   arch/x86/include/uapi/asm/hyperv.h |  3 +++
> >   arch/x86/kvm/x86.c                 | 25 ++++++++++++++++++++++++-
> >   3 files changed, 29 insertions(+), 1 deletion(-)
> >
> > diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
> > index 3741c65..f0fee35 100644
> > --- a/arch/x86/include/asm/kvm_host.h
> > +++ b/arch/x86/include/asm/kvm_host.h
> > @@ -575,6 +575,8 @@ struct kvm_arch {
> >   	/* fields used by HYPER-V emulation */
> >   	u64 hv_guest_os_id;
> >   	u64 hv_hypercall;
> > +	u64 hv_ref_count;
> > +	u64 hv_tsc_page;
> >
> >   	#ifdef CONFIG_KVM_MMU_AUDIT
> >   	int audit_point;
> > diff --git a/arch/x86/include/uapi/asm/hyperv.h b/arch/x86/include/uapi/asm/hyperv.h
> > index b80420b..9711819 100644
> > --- a/arch/x86/include/uapi/asm/hyperv.h
> > +++ b/arch/x86/include/uapi/asm/hyperv.h
> > @@ -136,6 +136,9 @@
> >   /* MSR used to read the per-partition time reference counter */
> >   #define HV_X64_MSR_TIME_REF_COUNT		0x40000020
> >
> > +/* A partition's reference time stamp counter (TSC) page */
> > +#define HV_X64_MSR_REFERENCE_TSC		0x40000021
> > +
> >   /* Define the virtual APIC registers */
> >   #define HV_X64_MSR_EOI				0x40000070
> >   #define HV_X64_MSR_ICR				0x40000071
> > diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> > index 094b5d9..1a4036d 100644
> > --- a/arch/x86/kvm/x86.c
> > +++ b/arch/x86/kvm/x86.c
> > @@ -843,7 +843,7 @@ EXPORT_SYMBOL_GPL(kvm_rdpmc);
> >   static u32 msrs_to_save[] = {
> >   	MSR_KVM_SYSTEM_TIME, MSR_KVM_WALL_CLOCK,
> >   	MSR_KVM_SYSTEM_TIME_NEW, MSR_KVM_WALL_CLOCK_NEW,
> > -	HV_X64_MSR_GUEST_OS_ID, HV_X64_MSR_HYPERCALL,
> > +	HV_X64_MSR_GUEST_OS_ID, HV_X64_MSR_HYPERCALL,HV_X64_MSR_TIME_REF_COUNT,
> >   	HV_X64_MSR_APIC_ASSIST_PAGE, MSR_KVM_ASYNC_PF_EN, MSR_KVM_STEAL_TIME,
> >   	MSR_KVM_PV_EOI_EN,
> >   	MSR_IA32_SYSENTER_CS, MSR_IA32_SYSENTER_ESP, MSR_IA32_SYSENTER_EIP,
> > @@ -1764,6 +1764,8 @@ static bool kvm_hv_msr_partition_wide(u32 msr)
> >   	switch (msr) {
> >   	case HV_X64_MSR_GUEST_OS_ID:
> >   	case HV_X64_MSR_HYPERCALL:
> > +	case HV_X64_MSR_REFERENCE_TSC:
> > +	case HV_X64_MSR_TIME_REF_COUNT:
> >   		r = true;
> >   		break;
> >   	}
> > @@ -1803,6 +1805,21 @@ static int set_msr_hyperv_pw(struct kvm_vcpu *vcpu, u32 msr, u64 data)
> >   		if (__copy_to_user((void __user *)addr, instructions, 4))
> >   			return 1;
> >   		kvm->arch.hv_hypercall = data;
> > +		kvm->arch.hv_ref_count = get_kernel_ns();
> > +		break;
> > +	}
> > +	case HV_X64_MSR_REFERENCE_TSC: {
> > +		u64 gfn;
> > +		unsigned long addr;
> > +		u32 tsc_ref;
> > +		gfn = data >> HV_X64_MSR_HYPERCALL_PAGE_ADDRESS_SHIFT;
> > +		addr = gfn_to_hva(kvm, gfn);
> > +		if (kvm_is_error_hva(addr))
> > +			return 1;
> > +		tsc_ref = 0;
> > +		if(__copy_to_user((void __user *)addr, &tsc_ref, sizeof(tsc_ref)))
> > +			return 1;
> > +		kvm->arch.hv_tsc_page = data;
> >   		break;
> >   	}
> >   	default:
> > @@ -2229,6 +2246,12 @@ static int get_msr_hyperv_pw(struct kvm_vcpu *vcpu, u32 msr, u64 *pdata)
> >   	case HV_X64_MSR_HYPERCALL:
> >   		data = kvm->arch.hv_hypercall;
> >   		break;
> > +	case HV_X64_MSR_TIME_REF_COUNT:
> > +		data = div_u64(get_kernel_ns() - kvm->arch.hv_ref_count,100);
> > +		break;
> 
> 
> in an earlier version of this patch I have the following:
> 
> +       case HV_X64_MSR_TIME_REF_COUNT: {
> +               u64 now_ns;
> +               local_irq_disable();
> +               now_ns = get_kernel_ns();
> +               data = div_u64(now_ns + kvm->arch.kvmclock_offset - kvm->arch.hv_ref_count,100);
> +               local_irq_enable();
> +               break;
> +       }
> 
> I do not know if this is right, but I can report that this one is working without any flaws
> since approx. 1.5 years.

Hi Peter,
I created this patch based on the original code posted  
in thread http://marc.info/?l=kvm&m=133278705514826
But please feel free to send your version, if you see any problem 
in the current code.
Best regards,
Vadim.


> Peter

Re: [RFC PATCH 0/2] Hyper-V timers

[Gleb Natapov]

Marcelo, please review.

On Mon, May 13, 2013 at 09:45:15PM +1000, Vadim Rozenfeld wrote:
> This RFC series adds support for two Hyper-V timer services -
> a per-partition reference time counter, and a  partition reference 
> time enlightenmen.
> 
> Vadim Rozenfeld (2):
>   hyper-v reference counter
>   Hyper-V iTSC handler
> 
>  arch/x86/include/asm/kvm_host.h    |  2 ++
>  arch/x86/include/uapi/asm/hyperv.h | 13 +++++++++++++
>  arch/x86/kvm/x86.c                 | 33 ++++++++++++++++++++++++++++++++-
>  include/uapi/linux/kvm.h           |  1 +
>  4 files changed, 48 insertions(+), 1 deletion(-)
> 
> -- 
> 1.8.1.2

--
			Gleb.

Re: [RFC PATCH 1/2] Hyper-H reference counter

[Gleb Natapov]

On Tue, May 14, 2013 at 04:14:11PM +0200, Peter Lieven wrote:
> On 13.05.2013 13:45, Vadim Rozenfeld wrote:
> >Signed-off: Peter Lieven <pl@dlh.net>
> >Signed-off: Gleb Natapov <gleb@redhat.com>
> >Signed-off: Vadim Rozenfeld <vrozenfe@redhat.com>
> >
> >The following patch allows to activate Hyper-V
> >reference time counter
> >---
> >  arch/x86/include/asm/kvm_host.h    |  2 ++
> >  arch/x86/include/uapi/asm/hyperv.h |  3 +++
> >  arch/x86/kvm/x86.c                 | 25 ++++++++++++++++++++++++-
> >  3 files changed, 29 insertions(+), 1 deletion(-)
> >
> >diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
> >index 3741c65..f0fee35 100644
> >--- a/arch/x86/include/asm/kvm_host.h
> >+++ b/arch/x86/include/asm/kvm_host.h
> >@@ -575,6 +575,8 @@ struct kvm_arch {
> >  	/* fields used by HYPER-V emulation */
> >  	u64 hv_guest_os_id;
> >  	u64 hv_hypercall;
> >+	u64 hv_ref_count;
> >+	u64 hv_tsc_page;
> >
> >  	#ifdef CONFIG_KVM_MMU_AUDIT
> >  	int audit_point;
> >diff --git a/arch/x86/include/uapi/asm/hyperv.h b/arch/x86/include/uapi/asm/hyperv.h
> >index b80420b..9711819 100644
> >--- a/arch/x86/include/uapi/asm/hyperv.h
> >+++ b/arch/x86/include/uapi/asm/hyperv.h
> >@@ -136,6 +136,9 @@
> >  /* MSR used to read the per-partition time reference counter */
> >  #define HV_X64_MSR_TIME_REF_COUNT		0x40000020
> >
> >+/* A partition's reference time stamp counter (TSC) page */
> >+#define HV_X64_MSR_REFERENCE_TSC		0x40000021
> >+
> >  /* Define the virtual APIC registers */
> >  #define HV_X64_MSR_EOI				0x40000070
> >  #define HV_X64_MSR_ICR				0x40000071
> >diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> >index 094b5d9..1a4036d 100644
> >--- a/arch/x86/kvm/x86.c
> >+++ b/arch/x86/kvm/x86.c
> >@@ -843,7 +843,7 @@ EXPORT_SYMBOL_GPL(kvm_rdpmc);
> >  static u32 msrs_to_save[] = {
> >  	MSR_KVM_SYSTEM_TIME, MSR_KVM_WALL_CLOCK,
> >  	MSR_KVM_SYSTEM_TIME_NEW, MSR_KVM_WALL_CLOCK_NEW,
> >-	HV_X64_MSR_GUEST_OS_ID, HV_X64_MSR_HYPERCALL,
> >+	HV_X64_MSR_GUEST_OS_ID, HV_X64_MSR_HYPERCALL,HV_X64_MSR_TIME_REF_COUNT,
> >  	HV_X64_MSR_APIC_ASSIST_PAGE, MSR_KVM_ASYNC_PF_EN, MSR_KVM_STEAL_TIME,
> >  	MSR_KVM_PV_EOI_EN,
> >  	MSR_IA32_SYSENTER_CS, MSR_IA32_SYSENTER_ESP, MSR_IA32_SYSENTER_EIP,
> >@@ -1764,6 +1764,8 @@ static bool kvm_hv_msr_partition_wide(u32 msr)
> >  	switch (msr) {
> >  	case HV_X64_MSR_GUEST_OS_ID:
> >  	case HV_X64_MSR_HYPERCALL:
> >+	case HV_X64_MSR_REFERENCE_TSC:
> >+	case HV_X64_MSR_TIME_REF_COUNT:
> >  		r = true;
> >  		break;
> >  	}
> >@@ -1803,6 +1805,21 @@ static int set_msr_hyperv_pw(struct kvm_vcpu *vcpu, u32 msr, u64 data)
> >  		if (__copy_to_user((void __user *)addr, instructions, 4))
> >  			return 1;
> >  		kvm->arch.hv_hypercall = data;
> >+		kvm->arch.hv_ref_count = get_kernel_ns();
> >+		break;
> >+	}
> >+	case HV_X64_MSR_REFERENCE_TSC: {
> >+		u64 gfn;
> >+		unsigned long addr;
> >+		u32 tsc_ref;
> >+		gfn = data >> HV_X64_MSR_HYPERCALL_PAGE_ADDRESS_SHIFT;
> >+		addr = gfn_to_hva(kvm, gfn);
> >+		if (kvm_is_error_hva(addr))
> >+			return 1;
> >+		tsc_ref = 0;
> >+		if(__copy_to_user((void __user *)addr, &tsc_ref, sizeof(tsc_ref)))
> >+			return 1;
> >+		kvm->arch.hv_tsc_page = data;
> >  		break;
> >  	}
> >  	default:
> >@@ -2229,6 +2246,12 @@ static int get_msr_hyperv_pw(struct kvm_vcpu *vcpu, u32 msr, u64 *pdata)
> >  	case HV_X64_MSR_HYPERCALL:
> >  		data = kvm->arch.hv_hypercall;
> >  		break;
> >+	case HV_X64_MSR_TIME_REF_COUNT:
> >+		data = div_u64(get_kernel_ns() - kvm->arch.hv_ref_count,100);
> >+		break;
> 
> 
> in an earlier version of this patch I have the following:
> 
> +       case HV_X64_MSR_TIME_REF_COUNT: {
> +               u64 now_ns;
> +               local_irq_disable();
> +               now_ns = get_kernel_ns();
> +               data = div_u64(now_ns + kvm->arch.kvmclock_offset - kvm->arch.hv_ref_count,100);
> +               local_irq_enable();
> +               break;
> +       }
> 
> I do not know if this is right, but I can report that this one is working without any flaws
> since approx. 1.5 years.
> 
get_kernel_ns() should be called at least with preemption disabled and
all other callers disable irq around it too, also without
kvmclock_offset the counter will be incorrect after migration. Marcelo,
is kvmclock_offset guaranties migration correctness or more is required?

--
			Gleb.

Re: [RFC PATCH 1/2] Hyper-H reference counter

[Gleb Natapov]

On Tue, May 14, 2013 at 07:46:36PM +1000, Vadim Rozenfeld wrote:
> On Mon, 2013-05-13 at 16:30 -0700, Eric Northup wrote:
> > On Mon, May 13, 2013 at 4:45 AM, Vadim Rozenfeld <vrozenfe@redhat.com> wrote:
> > > Signed-off: Peter Lieven <pl@dlh.net>
> > > Signed-off: Gleb Natapov <gleb@redhat.com>
> > > Signed-off: Vadim Rozenfeld <vrozenfe@redhat.com>
> > >
> > > The following patch allows to activate Hyper-V
> > > reference time counter
> > > ---
> > >  arch/x86/include/asm/kvm_host.h    |  2 ++
> > >  arch/x86/include/uapi/asm/hyperv.h |  3 +++
> > >  arch/x86/kvm/x86.c                 | 25 ++++++++++++++++++++++++-
> > >  3 files changed, 29 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
> > > index 3741c65..f0fee35 100644
> > > --- a/arch/x86/include/asm/kvm_host.h
> > > +++ b/arch/x86/include/asm/kvm_host.h
> > > @@ -575,6 +575,8 @@ struct kvm_arch {
> > >         /* fields used by HYPER-V emulation */
> > >         u64 hv_guest_os_id;
> > >         u64 hv_hypercall;
> > > +       u64 hv_ref_count;
> > > +       u64 hv_tsc_page;
> > >
> > >         #ifdef CONFIG_KVM_MMU_AUDIT
> > >         int audit_point;
> > > diff --git a/arch/x86/include/uapi/asm/hyperv.h b/arch/x86/include/uapi/asm/hyperv.h
> > > index b80420b..9711819 100644
> > > --- a/arch/x86/include/uapi/asm/hyperv.h
> > > +++ b/arch/x86/include/uapi/asm/hyperv.h
> > > @@ -136,6 +136,9 @@
> > >  /* MSR used to read the per-partition time reference counter */
> > >  #define HV_X64_MSR_TIME_REF_COUNT              0x40000020
> > >
> > > +/* A partition's reference time stamp counter (TSC) page */
> > > +#define HV_X64_MSR_REFERENCE_TSC               0x40000021
> > > +
> > >  /* Define the virtual APIC registers */
> > >  #define HV_X64_MSR_EOI                         0x40000070
> > >  #define HV_X64_MSR_ICR                         0x40000071
> > > diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> > > index 094b5d9..1a4036d 100644
> > > --- a/arch/x86/kvm/x86.c
> > > +++ b/arch/x86/kvm/x86.c
> > > @@ -843,7 +843,7 @@ EXPORT_SYMBOL_GPL(kvm_rdpmc);
> > >  static u32 msrs_to_save[] = {
> > >         MSR_KVM_SYSTEM_TIME, MSR_KVM_WALL_CLOCK,
> > >         MSR_KVM_SYSTEM_TIME_NEW, MSR_KVM_WALL_CLOCK_NEW,
> > > -       HV_X64_MSR_GUEST_OS_ID, HV_X64_MSR_HYPERCALL,
> > > +       HV_X64_MSR_GUEST_OS_ID, HV_X64_MSR_HYPERCALL,HV_X64_MSR_TIME_REF_COUNT,
> > >         HV_X64_MSR_APIC_ASSIST_PAGE, MSR_KVM_ASYNC_PF_EN, MSR_KVM_STEAL_TIME,
> > >         MSR_KVM_PV_EOI_EN,
> > >         MSR_IA32_SYSENTER_CS, MSR_IA32_SYSENTER_ESP, MSR_IA32_SYSENTER_EIP,
> > > @@ -1764,6 +1764,8 @@ static bool kvm_hv_msr_partition_wide(u32 msr)
> > >         switch (msr) {
> > >         case HV_X64_MSR_GUEST_OS_ID:
> > >         case HV_X64_MSR_HYPERCALL:
> > > +       case HV_X64_MSR_REFERENCE_TSC:
> > > +       case HV_X64_MSR_TIME_REF_COUNT:
> > >                 r = true;
> > >                 break;
> > >         }
> > > @@ -1803,6 +1805,21 @@ static int set_msr_hyperv_pw(struct kvm_vcpu *vcpu, u32 msr, u64 data)
> > >                 if (__copy_to_user((void __user *)addr, instructions, 4))
> > >                         return 1;
> > >                 kvm->arch.hv_hypercall = data;
> > > +               kvm->arch.hv_ref_count = get_kernel_ns();
> > > +               break;
> > > +       }
> > > +       case HV_X64_MSR_REFERENCE_TSC: {
> > > +               u64 gfn;
> > > +               unsigned long addr;
> > > +               u32 tsc_ref;
> > > +               gfn = data >> HV_X64_MSR_HYPERCALL_PAGE_ADDRESS_SHIFT;
> > > +               addr = gfn_to_hva(kvm, gfn);
> > > +               if (kvm_is_error_hva(addr))
> > > +                       return 1;
> > > +               tsc_ref = 0;
> > > +               if(__copy_to_user((void __user *)addr, &tsc_ref, sizeof(tsc_ref)))
> > 
> > Does this do the right thing when we're migrating?  How does usermode
> > learn that the guest page has been dirtied?
> > 
> 
> No, it shouldn't be a problem for this patch. Guest allocates a page
> from nonpaged physical memory, maps it to the system address space, gets
> physical address and sends it to KVM. KVM sets the first DWORD
> (TscSequence) to zero, which means that guest will use reference time
> counter as a timestamp source even after migration.
> 
Eric is right, we need mark_page_dirty() here and in HV_X64_MSR_HYPERCALL
too.  Without it QEMU will not know that content of the page has changed
and will not migrate it.

--
			Gleb.

Re: [RFC PATCH 2/2] Hyper-V iTSC handler

[Gleb Natapov]

On Mon, May 13, 2013 at 09:45:17PM +1000, Vadim Rozenfeld wrote:
> Signed-off: Vadim Rozenfeld <vrozenfe@redhat.com>
> 
> 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.
> 
Do you mean "will _not_ survive migration"?

> ---
>  arch/x86/include/uapi/asm/hyperv.h | 10 ++++++++++
>  arch/x86/kvm/x86.c                 | 18 +++++++++++++-----
>  include/uapi/linux/kvm.h           |  1 +
>  3 files changed, 24 insertions(+), 5 deletions(-)
> 
> diff --git a/arch/x86/include/uapi/asm/hyperv.h b/arch/x86/include/uapi/asm/hyperv.h
> index 9711819..2d9e666 100644
> --- a/arch/x86/include/uapi/asm/hyperv.h
> +++ b/arch/x86/include/uapi/asm/hyperv.h
> @@ -182,6 +182,9 @@
>  #define HV_X64_MSR_APIC_ASSIST_PAGE_ADDRESS_MASK	\
>  		(~((1ull << HV_X64_MSR_APIC_ASSIST_PAGE_ADDRESS_SHIFT) - 1))
>  
> +#define HV_X64_MSR_TSC_REFERENCE_ENABLE			0x00000001
> +#define HV_X64_MSR_TSC_REFERENCE_ADDRESS_SHIFT		12
> +
>  #define HV_PROCESSOR_POWER_STATE_C0		0
>  #define HV_PROCESSOR_POWER_STATE_C1		1
>  #define HV_PROCESSOR_POWER_STATE_C2		2
> @@ -194,4 +197,11 @@
>  #define HV_STATUS_INVALID_ALIGNMENT		4
>  #define HV_STATUS_INSUFFICIENT_BUFFERS		19
>  
> +typedef struct _HV_REFERENCE_TSC_PAGE {
> +	uint32_t TscSequence;
> +	uint32_t Rserved1;
> +	uint64_t TscScale;
> +	int64_t  TscOffset;
> +} HV_REFERENCE_TSC_PAGE, * PHV_REFERENCE_TSC_PAGE;
> +
Use kernel types: __u32/__u64/__s64.

>  #endif
> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> index 1a4036d..5788e8f 100644
> --- a/arch/x86/kvm/x86.c
> +++ b/arch/x86/kvm/x86.c
> @@ -1809,14 +1809,21 @@ static int set_msr_hyperv_pw(struct kvm_vcpu *vcpu, u32 msr, u64 data)
>  		break;
>  	}
>  	case HV_X64_MSR_REFERENCE_TSC: {
> -		u64 gfn;
>  		unsigned long addr;
> -		u32 tsc_ref;
> -		gfn = data >> HV_X64_MSR_HYPERCALL_PAGE_ADDRESS_SHIFT;
> -		addr = gfn_to_hva(kvm, gfn);
> +		HV_REFERENCE_TSC_PAGE tsc_ref;
> +		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 (!(data & HV_X64_MSR_TSC_REFERENCE_ENABLE)) {
> +			kvm->arch.hv_tsc_page = data;
> +			break;
> +		}
> +		addr = gfn_to_hva(vcpu->kvm, data >>
> +			HV_X64_MSR_TSC_REFERENCE_ADDRESS_SHIFT);
>  		if (kvm_is_error_hva(addr))
>  			return 1;
> -		tsc_ref = 0;
>  		if(__copy_to_user((void __user *)addr, &tsc_ref, sizeof(tsc_ref)))
>  			return 1;
>  		kvm->arch.hv_tsc_page = data;
> @@ -2553,6 +2560,7 @@ int kvm_dev_ioctl_check_extension(long ext)
>  	case KVM_CAP_HYPERV:
>  	case KVM_CAP_HYPERV_VAPIC:
>  	case KVM_CAP_HYPERV_SPIN:
> +	case KVM_CAP_HYPERV_TSC:
>  	case KVM_CAP_PCI_SEGMENT:
>  	case KVM_CAP_DEBUGREGS:
>  	case KVM_CAP_X86_ROBUST_SINGLESTEP:
> diff --git a/include/uapi/linux/kvm.h b/include/uapi/linux/kvm.h
> index a5c86fc..8eff540 100644
> --- a/include/uapi/linux/kvm.h
> +++ b/include/uapi/linux/kvm.h
> @@ -666,6 +666,7 @@ struct kvm_ppc_smmu_info {
>  #define KVM_CAP_IRQ_MPIC 90
>  #define KVM_CAP_PPC_RTAS 91
>  #define KVM_CAP_IRQ_XICS 92
> +#define KVM_CAP_HYPERV_TSC 93
>  
>  #ifdef KVM_CAP_IRQ_ROUTING
>  
> -- 
> 1.8.1.2

--
			Gleb.

Re: [RFC PATCH 1/2] Hyper-H reference counter

[Gleb Natapov]

On Mon, May 13, 2013 at 09:45:16PM +1000, Vadim Rozenfeld wrote:
> Signed-off: Peter Lieven <pl@dlh.net>
> Signed-off: Gleb Natapov <gleb@redhat.com>
> Signed-off: Vadim Rozenfeld <vrozenfe@redhat.com>
> 
> The following patch allows to activate Hyper-V 
> reference time counter 
> ---
>  arch/x86/include/asm/kvm_host.h    |  2 ++
>  arch/x86/include/uapi/asm/hyperv.h |  3 +++
>  arch/x86/kvm/x86.c                 | 25 ++++++++++++++++++++++++-
>  3 files changed, 29 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
> index 3741c65..f0fee35 100644
> --- a/arch/x86/include/asm/kvm_host.h
> +++ b/arch/x86/include/asm/kvm_host.h
> @@ -575,6 +575,8 @@ struct kvm_arch {
>  	/* fields used by HYPER-V emulation */
>  	u64 hv_guest_os_id;
>  	u64 hv_hypercall;
> +	u64 hv_ref_count;
> +	u64 hv_tsc_page;
>  
>  	#ifdef CONFIG_KVM_MMU_AUDIT
>  	int audit_point;
> diff --git a/arch/x86/include/uapi/asm/hyperv.h b/arch/x86/include/uapi/asm/hyperv.h
> index b80420b..9711819 100644
> --- a/arch/x86/include/uapi/asm/hyperv.h
> +++ b/arch/x86/include/uapi/asm/hyperv.h
> @@ -136,6 +136,9 @@
>  /* MSR used to read the per-partition time reference counter */
>  #define HV_X64_MSR_TIME_REF_COUNT		0x40000020
>  
> +/* A partition's reference time stamp counter (TSC) page */
> +#define HV_X64_MSR_REFERENCE_TSC		0x40000021
> +
>  /* Define the virtual APIC registers */
>  #define HV_X64_MSR_EOI				0x40000070
>  #define HV_X64_MSR_ICR				0x40000071
> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> index 094b5d9..1a4036d 100644
> --- a/arch/x86/kvm/x86.c
> +++ b/arch/x86/kvm/x86.c
> @@ -843,7 +843,7 @@ EXPORT_SYMBOL_GPL(kvm_rdpmc);
>  static u32 msrs_to_save[] = {
>  	MSR_KVM_SYSTEM_TIME, MSR_KVM_WALL_CLOCK,
>  	MSR_KVM_SYSTEM_TIME_NEW, MSR_KVM_WALL_CLOCK_NEW,
> -	HV_X64_MSR_GUEST_OS_ID, HV_X64_MSR_HYPERCALL,
> +	HV_X64_MSR_GUEST_OS_ID, HV_X64_MSR_HYPERCALL,HV_X64_MSR_TIME_REF_COUNT,
>  	HV_X64_MSR_APIC_ASSIST_PAGE, MSR_KVM_ASYNC_PF_EN, MSR_KVM_STEAL_TIME,
>  	MSR_KVM_PV_EOI_EN,
>  	MSR_IA32_SYSENTER_CS, MSR_IA32_SYSENTER_ESP, MSR_IA32_SYSENTER_EIP,
> @@ -1764,6 +1764,8 @@ static bool kvm_hv_msr_partition_wide(u32 msr)
>  	switch (msr) {
>  	case HV_X64_MSR_GUEST_OS_ID:
>  	case HV_X64_MSR_HYPERCALL:
> +	case HV_X64_MSR_REFERENCE_TSC:
> +	case HV_X64_MSR_TIME_REF_COUNT:
>  		r = true;
>  		break;
>  	}
> @@ -1803,6 +1805,21 @@ static int set_msr_hyperv_pw(struct kvm_vcpu *vcpu, u32 msr, u64 data)
>  		if (__copy_to_user((void __user *)addr, instructions, 4))
>  			return 1;
>  		kvm->arch.hv_hypercall = data;
> +		kvm->arch.hv_ref_count = get_kernel_ns();
> +		break;
> +	}
> +	case HV_X64_MSR_REFERENCE_TSC: {
> +		u64 gfn;
> +		unsigned long addr;
> +		u32 tsc_ref;
> +		gfn = data >> HV_X64_MSR_HYPERCALL_PAGE_ADDRESS_SHIFT;
Shouldn't you check HV_X64_MSR_TSC_REFERENCE_ENABLE here?

> +		addr = gfn_to_hva(kvm, gfn);
> +		if (kvm_is_error_hva(addr))
> +			return 1;
> +		tsc_ref = 0;
> +		if(__copy_to_user((void __user *)addr, &tsc_ref, sizeof(tsc_ref)))
> +			return 1;
> +		kvm->arch.hv_tsc_page = data;
>  		break;
>  	}
>  	default:
> @@ -2229,6 +2246,12 @@ static int get_msr_hyperv_pw(struct kvm_vcpu *vcpu, u32 msr, u64 *pdata)
>  	case HV_X64_MSR_HYPERCALL:
>  		data = kvm->arch.hv_hypercall;
>  		break;
> +	case HV_X64_MSR_TIME_REF_COUNT: 
> +		data = div_u64(get_kernel_ns() - kvm->arch.hv_ref_count,100); 
> +		break;
> +	case HV_X64_MSR_REFERENCE_TSC:
> +		data = kvm->arch.hv_tsc_page;
> +		break;
>  	default:
>  		vcpu_unimpl(vcpu, "Hyper-V unhandled rdmsr: 0x%x\n", msr);
>  		return 1;
> -- 
> 1.8.1.2

--
			Gleb.

Re: [RFC PATCH 1/2] Hyper-H reference counter

[Vadim Rozenfeld]

On Thu, 2013-05-16 at 11:18 +0300, Gleb Natapov wrote:
> On Tue, May 14, 2013 at 07:46:36PM +1000, Vadim Rozenfeld wrote:
> > On Mon, 2013-05-13 at 16:30 -0700, Eric Northup wrote:
> > > On Mon, May 13, 2013 at 4:45 AM, Vadim Rozenfeld <vrozenfe@redhat.com> wrote:
> > > > Signed-off: Peter Lieven <pl@dlh.net>
> > > > Signed-off: Gleb Natapov <gleb@redhat.com>
> > > > Signed-off: Vadim Rozenfeld <vrozenfe@redhat.com>
> > > >
> > > > The following patch allows to activate Hyper-V
> > > > reference time counter
> > > > ---
> > > >  arch/x86/include/asm/kvm_host.h    |  2 ++
> > > >  arch/x86/include/uapi/asm/hyperv.h |  3 +++
> > > >  arch/x86/kvm/x86.c                 | 25 ++++++++++++++++++++++++-
> > > >  3 files changed, 29 insertions(+), 1 deletion(-)
> > > >
> > > > diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
> > > > index 3741c65..f0fee35 100644
> > > > --- a/arch/x86/include/asm/kvm_host.h
> > > > +++ b/arch/x86/include/asm/kvm_host.h
> > > > @@ -575,6 +575,8 @@ struct kvm_arch {
> > > >         /* fields used by HYPER-V emulation */
> > > >         u64 hv_guest_os_id;
> > > >         u64 hv_hypercall;
> > > > +       u64 hv_ref_count;
> > > > +       u64 hv_tsc_page;
> > > >
> > > >         #ifdef CONFIG_KVM_MMU_AUDIT
> > > >         int audit_point;
> > > > diff --git a/arch/x86/include/uapi/asm/hyperv.h b/arch/x86/include/uapi/asm/hyperv.h
> > > > index b80420b..9711819 100644
> > > > --- a/arch/x86/include/uapi/asm/hyperv.h
> > > > +++ b/arch/x86/include/uapi/asm/hyperv.h
> > > > @@ -136,6 +136,9 @@
> > > >  /* MSR used to read the per-partition time reference counter */
> > > >  #define HV_X64_MSR_TIME_REF_COUNT              0x40000020
> > > >
> > > > +/* A partition's reference time stamp counter (TSC) page */
> > > > +#define HV_X64_MSR_REFERENCE_TSC               0x40000021
> > > > +
> > > >  /* Define the virtual APIC registers */
> > > >  #define HV_X64_MSR_EOI                         0x40000070
> > > >  #define HV_X64_MSR_ICR                         0x40000071
> > > > diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> > > > index 094b5d9..1a4036d 100644
> > > > --- a/arch/x86/kvm/x86.c
> > > > +++ b/arch/x86/kvm/x86.c
> > > > @@ -843,7 +843,7 @@ EXPORT_SYMBOL_GPL(kvm_rdpmc);
> > > >  static u32 msrs_to_save[] = {
> > > >         MSR_KVM_SYSTEM_TIME, MSR_KVM_WALL_CLOCK,
> > > >         MSR_KVM_SYSTEM_TIME_NEW, MSR_KVM_WALL_CLOCK_NEW,
> > > > -       HV_X64_MSR_GUEST_OS_ID, HV_X64_MSR_HYPERCALL,
> > > > +       HV_X64_MSR_GUEST_OS_ID, HV_X64_MSR_HYPERCALL,HV_X64_MSR_TIME_REF_COUNT,
> > > >         HV_X64_MSR_APIC_ASSIST_PAGE, MSR_KVM_ASYNC_PF_EN, MSR_KVM_STEAL_TIME,
> > > >         MSR_KVM_PV_EOI_EN,
> > > >         MSR_IA32_SYSENTER_CS, MSR_IA32_SYSENTER_ESP, MSR_IA32_SYSENTER_EIP,
> > > > @@ -1764,6 +1764,8 @@ static bool kvm_hv_msr_partition_wide(u32 msr)
> > > >         switch (msr) {
> > > >         case HV_X64_MSR_GUEST_OS_ID:
> > > >         case HV_X64_MSR_HYPERCALL:
> > > > +       case HV_X64_MSR_REFERENCE_TSC:
> > > > +       case HV_X64_MSR_TIME_REF_COUNT:
> > > >                 r = true;
> > > >                 break;
> > > >         }
> > > > @@ -1803,6 +1805,21 @@ static int set_msr_hyperv_pw(struct kvm_vcpu *vcpu, u32 msr, u64 data)
> > > >                 if (__copy_to_user((void __user *)addr, instructions, 4))
> > > >                         return 1;
> > > >                 kvm->arch.hv_hypercall = data;
> > > > +               kvm->arch.hv_ref_count = get_kernel_ns();
> > > > +               break;
> > > > +       }
> > > > +       case HV_X64_MSR_REFERENCE_TSC: {
> > > > +               u64 gfn;
> > > > +               unsigned long addr;
> > > > +               u32 tsc_ref;
> > > > +               gfn = data >> HV_X64_MSR_HYPERCALL_PAGE_ADDRESS_SHIFT;
> > > > +               addr = gfn_to_hva(kvm, gfn);
> > > > +               if (kvm_is_error_hva(addr))
> > > > +                       return 1;
> > > > +               tsc_ref = 0;
> > > > +               if(__copy_to_user((void __user *)addr, &tsc_ref, sizeof(tsc_ref)))
> > > 
> > > Does this do the right thing when we're migrating?  How does usermode
> > > learn that the guest page has been dirtied?
> > > 
> > 
> > No, it shouldn't be a problem for this patch. Guest allocates a page
> > from nonpaged physical memory, maps it to the system address space, gets
> > physical address and sends it to KVM. KVM sets the first DWORD
> > (TscSequence) to zero, which means that guest will use reference time
> > counter as a timestamp source even after migration.
> > 
> Eric is right, we need mark_page_dirty() here and in HV_X64_MSR_HYPERCALL
> too.  Without it QEMU will not know that content of the page has changed
> and will not migrate it.
OK. Will fix it.
> 
> --
> 			Gleb.

Re: [RFC PATCH 2/2] Hyper-V iTSC handler

[Vadim Rozenfeld]

On Thu, 2013-05-16 at 11:33 +0300, Gleb Natapov wrote:
> On Mon, May 13, 2013 at 09:45:17PM +1000, Vadim Rozenfeld wrote:
> > Signed-off: Vadim Rozenfeld <vrozenfe@redhat.com>
> > 
> > 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.
> > 
> Do you mean "will _not_ survive migration"?
Sorry for typo. Yes, it will not,
because offset, scale and sequence must be
readjusted. 
 
> 
> > ---
> >  arch/x86/include/uapi/asm/hyperv.h | 10 ++++++++++
> >  arch/x86/kvm/x86.c                 | 18 +++++++++++++-----
> >  include/uapi/linux/kvm.h           |  1 +
> >  3 files changed, 24 insertions(+), 5 deletions(-)
> > 
> > diff --git a/arch/x86/include/uapi/asm/hyperv.h b/arch/x86/include/uapi/asm/hyperv.h
> > index 9711819..2d9e666 100644
> > --- a/arch/x86/include/uapi/asm/hyperv.h
> > +++ b/arch/x86/include/uapi/asm/hyperv.h
> > @@ -182,6 +182,9 @@
> >  #define HV_X64_MSR_APIC_ASSIST_PAGE_ADDRESS_MASK	\
> >  		(~((1ull << HV_X64_MSR_APIC_ASSIST_PAGE_ADDRESS_SHIFT) - 1))
> >  
> > +#define HV_X64_MSR_TSC_REFERENCE_ENABLE			0x00000001
> > +#define HV_X64_MSR_TSC_REFERENCE_ADDRESS_SHIFT		12
> > +
> >  #define HV_PROCESSOR_POWER_STATE_C0		0
> >  #define HV_PROCESSOR_POWER_STATE_C1		1
> >  #define HV_PROCESSOR_POWER_STATE_C2		2
> > @@ -194,4 +197,11 @@
> >  #define HV_STATUS_INVALID_ALIGNMENT		4
> >  #define HV_STATUS_INSUFFICIENT_BUFFERS		19
> >  
> > +typedef struct _HV_REFERENCE_TSC_PAGE {
> > +	uint32_t TscSequence;
> > +	uint32_t Rserved1;
> > +	uint64_t TscScale;
> > +	int64_t  TscOffset;
> > +} HV_REFERENCE_TSC_PAGE, * PHV_REFERENCE_TSC_PAGE;
> > +
> Use kernel types: __u32/__u64/__s64.
> 
> >  #endif
> > diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> > index 1a4036d..5788e8f 100644
> > --- a/arch/x86/kvm/x86.c
> > +++ b/arch/x86/kvm/x86.c
> > @@ -1809,14 +1809,21 @@ static int set_msr_hyperv_pw(struct kvm_vcpu *vcpu, u32 msr, u64 data)
> >  		break;
> >  	}
> >  	case HV_X64_MSR_REFERENCE_TSC: {
> > -		u64 gfn;
> >  		unsigned long addr;
> > -		u32 tsc_ref;
> > -		gfn = data >> HV_X64_MSR_HYPERCALL_PAGE_ADDRESS_SHIFT;
> > -		addr = gfn_to_hva(kvm, gfn);
> > +		HV_REFERENCE_TSC_PAGE tsc_ref;
> > +		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 (!(data & HV_X64_MSR_TSC_REFERENCE_ENABLE)) {
> > +			kvm->arch.hv_tsc_page = data;
> > +			break;
> > +		}
> > +		addr = gfn_to_hva(vcpu->kvm, data >>
> > +			HV_X64_MSR_TSC_REFERENCE_ADDRESS_SHIFT);
> >  		if (kvm_is_error_hva(addr))
> >  			return 1;
> > -		tsc_ref = 0;
> >  		if(__copy_to_user((void __user *)addr, &tsc_ref, sizeof(tsc_ref)))
> >  			return 1;
> >  		kvm->arch.hv_tsc_page = data;
> > @@ -2553,6 +2560,7 @@ int kvm_dev_ioctl_check_extension(long ext)
> >  	case KVM_CAP_HYPERV:
> >  	case KVM_CAP_HYPERV_VAPIC:
> >  	case KVM_CAP_HYPERV_SPIN:
> > +	case KVM_CAP_HYPERV_TSC:
> >  	case KVM_CAP_PCI_SEGMENT:
> >  	case KVM_CAP_DEBUGREGS:
> >  	case KVM_CAP_X86_ROBUST_SINGLESTEP:
> > diff --git a/include/uapi/linux/kvm.h b/include/uapi/linux/kvm.h
> > index a5c86fc..8eff540 100644
> > --- a/include/uapi/linux/kvm.h
> > +++ b/include/uapi/linux/kvm.h
> > @@ -666,6 +666,7 @@ struct kvm_ppc_smmu_info {
> >  #define KVM_CAP_IRQ_MPIC 90
> >  #define KVM_CAP_PPC_RTAS 91
> >  #define KVM_CAP_IRQ_XICS 92
> > +#define KVM_CAP_HYPERV_TSC 93
> >  
> >  #ifdef KVM_CAP_IRQ_ROUTING
> >  
> > -- 
> > 1.8.1.2
> 
> --
> 			Gleb.

Re: [RFC PATCH 1/2] Hyper-H reference counter

[Vadim Rozenfeld]

On Thu, 2013-05-16 at 11:34 +0300, Gleb Natapov wrote:
> On Mon, May 13, 2013 at 09:45:16PM +1000, Vadim Rozenfeld wrote:
> > Signed-off: Peter Lieven <pl@dlh.net>
> > Signed-off: Gleb Natapov <gleb@redhat.com>
> > Signed-off: Vadim Rozenfeld <vrozenfe@redhat.com>
> > 
> > The following patch allows to activate Hyper-V 
> > reference time counter 
> > ---
> >  arch/x86/include/asm/kvm_host.h    |  2 ++
> >  arch/x86/include/uapi/asm/hyperv.h |  3 +++
> >  arch/x86/kvm/x86.c                 | 25 ++++++++++++++++++++++++-
> >  3 files changed, 29 insertions(+), 1 deletion(-)
> > 
> > diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
> > index 3741c65..f0fee35 100644
> > --- a/arch/x86/include/asm/kvm_host.h
> > +++ b/arch/x86/include/asm/kvm_host.h
> > @@ -575,6 +575,8 @@ struct kvm_arch {
> >  	/* fields used by HYPER-V emulation */
> >  	u64 hv_guest_os_id;
> >  	u64 hv_hypercall;
> > +	u64 hv_ref_count;
> > +	u64 hv_tsc_page;
> >  
> >  	#ifdef CONFIG_KVM_MMU_AUDIT
> >  	int audit_point;
> > diff --git a/arch/x86/include/uapi/asm/hyperv.h b/arch/x86/include/uapi/asm/hyperv.h
> > index b80420b..9711819 100644
> > --- a/arch/x86/include/uapi/asm/hyperv.h
> > +++ b/arch/x86/include/uapi/asm/hyperv.h
> > @@ -136,6 +136,9 @@
> >  /* MSR used to read the per-partition time reference counter */
> >  #define HV_X64_MSR_TIME_REF_COUNT		0x40000020
> >  
> > +/* A partition's reference time stamp counter (TSC) page */
> > +#define HV_X64_MSR_REFERENCE_TSC		0x40000021
> > +
> >  /* Define the virtual APIC registers */
> >  #define HV_X64_MSR_EOI				0x40000070
> >  #define HV_X64_MSR_ICR				0x40000071
> > diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> > index 094b5d9..1a4036d 100644
> > --- a/arch/x86/kvm/x86.c
> > +++ b/arch/x86/kvm/x86.c
> > @@ -843,7 +843,7 @@ EXPORT_SYMBOL_GPL(kvm_rdpmc);
> >  static u32 msrs_to_save[] = {
> >  	MSR_KVM_SYSTEM_TIME, MSR_KVM_WALL_CLOCK,
> >  	MSR_KVM_SYSTEM_TIME_NEW, MSR_KVM_WALL_CLOCK_NEW,
> > -	HV_X64_MSR_GUEST_OS_ID, HV_X64_MSR_HYPERCALL,
> > +	HV_X64_MSR_GUEST_OS_ID, HV_X64_MSR_HYPERCALL,HV_X64_MSR_TIME_REF_COUNT,
> >  	HV_X64_MSR_APIC_ASSIST_PAGE, MSR_KVM_ASYNC_PF_EN, MSR_KVM_STEAL_TIME,
> >  	MSR_KVM_PV_EOI_EN,
> >  	MSR_IA32_SYSENTER_CS, MSR_IA32_SYSENTER_ESP, MSR_IA32_SYSENTER_EIP,
> > @@ -1764,6 +1764,8 @@ static bool kvm_hv_msr_partition_wide(u32 msr)
> >  	switch (msr) {
> >  	case HV_X64_MSR_GUEST_OS_ID:
> >  	case HV_X64_MSR_HYPERCALL:
> > +	case HV_X64_MSR_REFERENCE_TSC:
> > +	case HV_X64_MSR_TIME_REF_COUNT:
> >  		r = true;
> >  		break;
> >  	}
> > @@ -1803,6 +1805,21 @@ static int set_msr_hyperv_pw(struct kvm_vcpu *vcpu, u32 msr, u64 data)
> >  		if (__copy_to_user((void __user *)addr, instructions, 4))
> >  			return 1;
> >  		kvm->arch.hv_hypercall = data;
> > +		kvm->arch.hv_ref_count = get_kernel_ns();
> > +		break;
> > +	}
> > +	case HV_X64_MSR_REFERENCE_TSC: {
> > +		u64 gfn;
> > +		unsigned long addr;
> > +		u32 tsc_ref;
> > +		gfn = data >> HV_X64_MSR_HYPERCALL_PAGE_ADDRESS_SHIFT;
> Shouldn't you check HV_X64_MSR_TSC_REFERENCE_ENABLE here?

Yes, I have this check added in the second patch.

> 
> > +		addr = gfn_to_hva(kvm, gfn);
> > +		if (kvm_is_error_hva(addr))
> > +			return 1;
> > +		tsc_ref = 0;
> > +		if(__copy_to_user((void __user *)addr, &tsc_ref, sizeof(tsc_ref)))
> > +			return 1;
> > +		kvm->arch.hv_tsc_page = data;
> >  		break;
> >  	}
> >  	default:
> > @@ -2229,6 +2246,12 @@ static int get_msr_hyperv_pw(struct kvm_vcpu *vcpu, u32 msr, u64 *pdata)
> >  	case HV_X64_MSR_HYPERCALL:
> >  		data = kvm->arch.hv_hypercall;
> >  		break;
> > +	case HV_X64_MSR_TIME_REF_COUNT: 
> > +		data = div_u64(get_kernel_ns() - kvm->arch.hv_ref_count,100); 
> > +		break;
> > +	case HV_X64_MSR_REFERENCE_TSC:
> > +		data = kvm->arch.hv_tsc_page;
> > +		break;
> >  	default:
> >  		vcpu_unimpl(vcpu, "Hyper-V unhandled rdmsr: 0x%x\n", msr);
> >  		return 1;
> > -- 
> > 1.8.1.2
> 
> --
> 			Gleb.

Re: [RFC PATCH 1/2] Hyper-H reference counter

[Gleb Natapov]

On Thu, May 16, 2013 at 07:13:41PM +1000, Vadim Rozenfeld wrote:
> On Thu, 2013-05-16 at 11:34 +0300, Gleb Natapov wrote:
> > On Mon, May 13, 2013 at 09:45:16PM +1000, Vadim Rozenfeld wrote:
> > > Signed-off: Peter Lieven <pl@dlh.net>
> > > Signed-off: Gleb Natapov <gleb@redhat.com>
> > > Signed-off: Vadim Rozenfeld <vrozenfe@redhat.com>
> > > 
> > > The following patch allows to activate Hyper-V 
> > > reference time counter 
> > > ---
> > >  arch/x86/include/asm/kvm_host.h    |  2 ++
> > >  arch/x86/include/uapi/asm/hyperv.h |  3 +++
> > >  arch/x86/kvm/x86.c                 | 25 ++++++++++++++++++++++++-
> > >  3 files changed, 29 insertions(+), 1 deletion(-)
> > > 
> > > diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
> > > index 3741c65..f0fee35 100644
> > > --- a/arch/x86/include/asm/kvm_host.h
> > > +++ b/arch/x86/include/asm/kvm_host.h
> > > @@ -575,6 +575,8 @@ struct kvm_arch {
> > >  	/* fields used by HYPER-V emulation */
> > >  	u64 hv_guest_os_id;
> > >  	u64 hv_hypercall;
> > > +	u64 hv_ref_count;
> > > +	u64 hv_tsc_page;
> > >  
> > >  	#ifdef CONFIG_KVM_MMU_AUDIT
> > >  	int audit_point;
> > > diff --git a/arch/x86/include/uapi/asm/hyperv.h b/arch/x86/include/uapi/asm/hyperv.h
> > > index b80420b..9711819 100644
> > > --- a/arch/x86/include/uapi/asm/hyperv.h
> > > +++ b/arch/x86/include/uapi/asm/hyperv.h
> > > @@ -136,6 +136,9 @@
> > >  /* MSR used to read the per-partition time reference counter */
> > >  #define HV_X64_MSR_TIME_REF_COUNT		0x40000020
> > >  
> > > +/* A partition's reference time stamp counter (TSC) page */
> > > +#define HV_X64_MSR_REFERENCE_TSC		0x40000021
> > > +
> > >  /* Define the virtual APIC registers */
> > >  #define HV_X64_MSR_EOI				0x40000070
> > >  #define HV_X64_MSR_ICR				0x40000071
> > > diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> > > index 094b5d9..1a4036d 100644
> > > --- a/arch/x86/kvm/x86.c
> > > +++ b/arch/x86/kvm/x86.c
> > > @@ -843,7 +843,7 @@ EXPORT_SYMBOL_GPL(kvm_rdpmc);
> > >  static u32 msrs_to_save[] = {
> > >  	MSR_KVM_SYSTEM_TIME, MSR_KVM_WALL_CLOCK,
> > >  	MSR_KVM_SYSTEM_TIME_NEW, MSR_KVM_WALL_CLOCK_NEW,
> > > -	HV_X64_MSR_GUEST_OS_ID, HV_X64_MSR_HYPERCALL,
> > > +	HV_X64_MSR_GUEST_OS_ID, HV_X64_MSR_HYPERCALL,HV_X64_MSR_TIME_REF_COUNT,
> > >  	HV_X64_MSR_APIC_ASSIST_PAGE, MSR_KVM_ASYNC_PF_EN, MSR_KVM_STEAL_TIME,
> > >  	MSR_KVM_PV_EOI_EN,
> > >  	MSR_IA32_SYSENTER_CS, MSR_IA32_SYSENTER_ESP, MSR_IA32_SYSENTER_EIP,
> > > @@ -1764,6 +1764,8 @@ static bool kvm_hv_msr_partition_wide(u32 msr)
> > >  	switch (msr) {
> > >  	case HV_X64_MSR_GUEST_OS_ID:
> > >  	case HV_X64_MSR_HYPERCALL:
> > > +	case HV_X64_MSR_REFERENCE_TSC:
> > > +	case HV_X64_MSR_TIME_REF_COUNT:
> > >  		r = true;
> > >  		break;
> > >  	}
> > > @@ -1803,6 +1805,21 @@ static int set_msr_hyperv_pw(struct kvm_vcpu *vcpu, u32 msr, u64 data)
> > >  		if (__copy_to_user((void __user *)addr, instructions, 4))
> > >  			return 1;
> > >  		kvm->arch.hv_hypercall = data;
> > > +		kvm->arch.hv_ref_count = get_kernel_ns();
> > > +		break;
> > > +	}
> > > +	case HV_X64_MSR_REFERENCE_TSC: {
> > > +		u64 gfn;
> > > +		unsigned long addr;
> > > +		u32 tsc_ref;
> > > +		gfn = data >> HV_X64_MSR_HYPERCALL_PAGE_ADDRESS_SHIFT;
> > Shouldn't you check HV_X64_MSR_TSC_REFERENCE_ENABLE here?
> 
> Yes, I have this check added in the second patch.
> 
Move it here please.

> > 
> > > +		addr = gfn_to_hva(kvm, gfn);
> > > +		if (kvm_is_error_hva(addr))
> > > +			return 1;
> > > +		tsc_ref = 0;
> > > +		if(__copy_to_user((void __user *)addr, &tsc_ref, sizeof(tsc_ref)))
> > > +			return 1;
> > > +		kvm->arch.hv_tsc_page = data;
> > >  		break;
> > >  	}
> > >  	default:
> > > @@ -2229,6 +2246,12 @@ static int get_msr_hyperv_pw(struct kvm_vcpu *vcpu, u32 msr, u64 *pdata)
> > >  	case HV_X64_MSR_HYPERCALL:
> > >  		data = kvm->arch.hv_hypercall;
> > >  		break;
> > > +	case HV_X64_MSR_TIME_REF_COUNT: 
> > > +		data = div_u64(get_kernel_ns() - kvm->arch.hv_ref_count,100); 
> > > +		break;
> > > +	case HV_X64_MSR_REFERENCE_TSC:
> > > +		data = kvm->arch.hv_tsc_page;
> > > +		break;
> > >  	default:
> > >  		vcpu_unimpl(vcpu, "Hyper-V unhandled rdmsr: 0x%x\n", msr);
> > >  		return 1;
> > > -- 
> > > 1.8.1.2
> > 
> > --
> > 			Gleb.
> 

--
			Gleb.

Re: [RFC PATCH 1/2] Hyper-H reference counter

[Vadim Rozenfeld]

On Thu, 2013-05-16 at 12:21 +0300, Gleb Natapov wrote:
> On Thu, May 16, 2013 at 07:13:41PM +1000, Vadim Rozenfeld wrote:
> > On Thu, 2013-05-16 at 11:34 +0300, Gleb Natapov wrote:
> > > On Mon, May 13, 2013 at 09:45:16PM +1000, Vadim Rozenfeld wrote:
> > > > Signed-off: Peter Lieven <pl@dlh.net>
> > > > Signed-off: Gleb Natapov <gleb@redhat.com>
> > > > Signed-off: Vadim Rozenfeld <vrozenfe@redhat.com>
> > > > 
> > > > The following patch allows to activate Hyper-V 
> > > > reference time counter 
> > > > ---
> > > >  arch/x86/include/asm/kvm_host.h    |  2 ++
> > > >  arch/x86/include/uapi/asm/hyperv.h |  3 +++
> > > >  arch/x86/kvm/x86.c                 | 25 ++++++++++++++++++++++++-
> > > >  3 files changed, 29 insertions(+), 1 deletion(-)
> > > > 
> > > > diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
> > > > index 3741c65..f0fee35 100644
> > > > --- a/arch/x86/include/asm/kvm_host.h
> > > > +++ b/arch/x86/include/asm/kvm_host.h
> > > > @@ -575,6 +575,8 @@ struct kvm_arch {
> > > >  	/* fields used by HYPER-V emulation */
> > > >  	u64 hv_guest_os_id;
> > > >  	u64 hv_hypercall;
> > > > +	u64 hv_ref_count;
> > > > +	u64 hv_tsc_page;
> > > >  
> > > >  	#ifdef CONFIG_KVM_MMU_AUDIT
> > > >  	int audit_point;
> > > > diff --git a/arch/x86/include/uapi/asm/hyperv.h b/arch/x86/include/uapi/asm/hyperv.h
> > > > index b80420b..9711819 100644
> > > > --- a/arch/x86/include/uapi/asm/hyperv.h
> > > > +++ b/arch/x86/include/uapi/asm/hyperv.h
> > > > @@ -136,6 +136,9 @@
> > > >  /* MSR used to read the per-partition time reference counter */
> > > >  #define HV_X64_MSR_TIME_REF_COUNT		0x40000020
> > > >  
> > > > +/* A partition's reference time stamp counter (TSC) page */
> > > > +#define HV_X64_MSR_REFERENCE_TSC		0x40000021
> > > > +
> > > >  /* Define the virtual APIC registers */
> > > >  #define HV_X64_MSR_EOI				0x40000070
> > > >  #define HV_X64_MSR_ICR				0x40000071
> > > > diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> > > > index 094b5d9..1a4036d 100644
> > > > --- a/arch/x86/kvm/x86.c
> > > > +++ b/arch/x86/kvm/x86.c
> > > > @@ -843,7 +843,7 @@ EXPORT_SYMBOL_GPL(kvm_rdpmc);
> > > >  static u32 msrs_to_save[] = {
> > > >  	MSR_KVM_SYSTEM_TIME, MSR_KVM_WALL_CLOCK,
> > > >  	MSR_KVM_SYSTEM_TIME_NEW, MSR_KVM_WALL_CLOCK_NEW,
> > > > -	HV_X64_MSR_GUEST_OS_ID, HV_X64_MSR_HYPERCALL,
> > > > +	HV_X64_MSR_GUEST_OS_ID, HV_X64_MSR_HYPERCALL,HV_X64_MSR_TIME_REF_COUNT,
> > > >  	HV_X64_MSR_APIC_ASSIST_PAGE, MSR_KVM_ASYNC_PF_EN, MSR_KVM_STEAL_TIME,
> > > >  	MSR_KVM_PV_EOI_EN,
> > > >  	MSR_IA32_SYSENTER_CS, MSR_IA32_SYSENTER_ESP, MSR_IA32_SYSENTER_EIP,
> > > > @@ -1764,6 +1764,8 @@ static bool kvm_hv_msr_partition_wide(u32 msr)
> > > >  	switch (msr) {
> > > >  	case HV_X64_MSR_GUEST_OS_ID:
> > > >  	case HV_X64_MSR_HYPERCALL:
> > > > +	case HV_X64_MSR_REFERENCE_TSC:
> > > > +	case HV_X64_MSR_TIME_REF_COUNT:
> > > >  		r = true;
> > > >  		break;
> > > >  	}
> > > > @@ -1803,6 +1805,21 @@ static int set_msr_hyperv_pw(struct kvm_vcpu *vcpu, u32 msr, u64 data)
> > > >  		if (__copy_to_user((void __user *)addr, instructions, 4))
> > > >  			return 1;
> > > >  		kvm->arch.hv_hypercall = data;
> > > > +		kvm->arch.hv_ref_count = get_kernel_ns();
> > > > +		break;
> > > > +	}
> > > > +	case HV_X64_MSR_REFERENCE_TSC: {
> > > > +		u64 gfn;
> > > > +		unsigned long addr;
> > > > +		u32 tsc_ref;
> > > > +		gfn = data >> HV_X64_MSR_HYPERCALL_PAGE_ADDRESS_SHIFT;
> > > Shouldn't you check HV_X64_MSR_TSC_REFERENCE_ENABLE here?
> > 
> > Yes, I have this check added in the second patch.
> > 
> Move it here please.
OK, will do it.
> 
> > > 
> > > > +		addr = gfn_to_hva(kvm, gfn);
> > > > +		if (kvm_is_error_hva(addr))
> > > > +			return 1;
> > > > +		tsc_ref = 0;
> > > > +		if(__copy_to_user((void __user *)addr, &tsc_ref, sizeof(tsc_ref)))
> > > > +			return 1;
> > > > +		kvm->arch.hv_tsc_page = data;
> > > >  		break;
> > > >  	}
> > > >  	default:
> > > > @@ -2229,6 +2246,12 @@ static int get_msr_hyperv_pw(struct kvm_vcpu *vcpu, u32 msr, u64 *pdata)
> > > >  	case HV_X64_MSR_HYPERCALL:
> > > >  		data = kvm->arch.hv_hypercall;
> > > >  		break;
> > > > +	case HV_X64_MSR_TIME_REF_COUNT: 
> > > > +		data = div_u64(get_kernel_ns() - kvm->arch.hv_ref_count,100); 
> > > > +		break;
> > > > +	case HV_X64_MSR_REFERENCE_TSC:
> > > > +		data = kvm->arch.hv_tsc_page;
> > > > +		break;
> > > >  	default:
> > > >  		vcpu_unimpl(vcpu, "Hyper-V unhandled rdmsr: 0x%x\n", msr);
> > > >  		return 1;
> > > > -- 
> > > > 1.8.1.2
> > > 
> > > --
> > > 			Gleb.
> > 
> 
> --
> 			Gleb.

Re: [RFC PATCH 1/2] Hyper-H reference counter

[Paolo Bonzini]

Il 16/05/2013 11:28, Vadim Rozenfeld ha scritto:
>>>>> > > > > +	case HV_X64_MSR_REFERENCE_TSC: {
>>>>> > > > > +		u64 gfn;
>>>>> > > > > +		unsigned long addr;
>>>>> > > > > +		u32 tsc_ref;
>>>>> > > > > +		gfn = data >> HV_X64_MSR_HYPERCALL_PAGE_ADDRESS_SHIFT;
>>>> > > > Shouldn't you check HV_X64_MSR_TSC_REFERENCE_ENABLE here?
>>> > > 
>>> > > Yes, I have this check added in the second patch.
>>> > > 
>> > Move it here please.
> OK, will do it.
>> > 
>>>> > > > 
>>>>> > > > > +		addr = gfn_to_hva(kvm, gfn);
>>>>> > > > > +		if (kvm_is_error_hva(addr))
>>>>> > > > > +			return 1;
>>>>> > > > > +		tsc_ref = 0;

This should write 0xFFFFFFFF.

Paolo

Re: [RFC PATCH 1/2] Hyper-H reference counter

[Paolo Bonzini]

Il 16/05/2013 11:28, Vadim Rozenfeld ha scritto:
> On Thu, 2013-05-16 at 12:21 +0300, Gleb Natapov wrote:
>> On Thu, May 16, 2013 at 07:13:41PM +1000, Vadim Rozenfeld wrote:
>>> On Thu, 2013-05-16 at 11:34 +0300, Gleb Natapov wrote:
>>>> On Mon, May 13, 2013 at 09:45:16PM +1000, Vadim Rozenfeld wrote:
>>>>> Signed-off: Peter Lieven <pl@dlh.net>
>>>>> Signed-off: Gleb Natapov <gleb@redhat.com>
>>>>> Signed-off: Vadim Rozenfeld <vrozenfe@redhat.com>
>>>>>
>>>>> The following patch allows to activate Hyper-V 
>>>>> reference time counter 
>>>>> ---
>>>>>  arch/x86/include/asm/kvm_host.h    |  2 ++
>>>>>  arch/x86/include/uapi/asm/hyperv.h |  3 +++
>>>>>  arch/x86/kvm/x86.c                 | 25 ++++++++++++++++++++++++-
>>>>>  3 files changed, 29 insertions(+), 1 deletion(-)
>>>>>
>>>>> diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
>>>>> index 3741c65..f0fee35 100644
>>>>> --- a/arch/x86/include/asm/kvm_host.h
>>>>> +++ b/arch/x86/include/asm/kvm_host.h
>>>>> @@ -575,6 +575,8 @@ struct kvm_arch {
>>>>>  	/* fields used by HYPER-V emulation */
>>>>>  	u64 hv_guest_os_id;
>>>>>  	u64 hv_hypercall;
>>>>> +	u64 hv_ref_count;
>>>>> +	u64 hv_tsc_page;
>>>>>  
>>>>>  	#ifdef CONFIG_KVM_MMU_AUDIT
>>>>>  	int audit_point;
>>>>> diff --git a/arch/x86/include/uapi/asm/hyperv.h b/arch/x86/include/uapi/asm/hyperv.h
>>>>> index b80420b..9711819 100644
>>>>> --- a/arch/x86/include/uapi/asm/hyperv.h
>>>>> +++ b/arch/x86/include/uapi/asm/hyperv.h
>>>>> @@ -136,6 +136,9 @@
>>>>>  /* MSR used to read the per-partition time reference counter */
>>>>>  #define HV_X64_MSR_TIME_REF_COUNT		0x40000020
>>>>>  
>>>>> +/* A partition's reference time stamp counter (TSC) page */
>>>>> +#define HV_X64_MSR_REFERENCE_TSC		0x40000021
>>>>> +
>>>>>  /* Define the virtual APIC registers */
>>>>>  #define HV_X64_MSR_EOI				0x40000070
>>>>>  #define HV_X64_MSR_ICR				0x40000071
>>>>> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
>>>>> index 094b5d9..1a4036d 100644
>>>>> --- a/arch/x86/kvm/x86.c
>>>>> +++ b/arch/x86/kvm/x86.c
>>>>> @@ -843,7 +843,7 @@ EXPORT_SYMBOL_GPL(kvm_rdpmc);
>>>>>  static u32 msrs_to_save[] = {
>>>>>  	MSR_KVM_SYSTEM_TIME, MSR_KVM_WALL_CLOCK,
>>>>>  	MSR_KVM_SYSTEM_TIME_NEW, MSR_KVM_WALL_CLOCK_NEW,
>>>>> -	HV_X64_MSR_GUEST_OS_ID, HV_X64_MSR_HYPERCALL,
>>>>> +	HV_X64_MSR_GUEST_OS_ID, HV_X64_MSR_HYPERCALL,HV_X64_MSR_TIME_REF_COUNT,
>>>>>  	HV_X64_MSR_APIC_ASSIST_PAGE, MSR_KVM_ASYNC_PF_EN, MSR_KVM_STEAL_TIME,
>>>>>  	MSR_KVM_PV_EOI_EN,
>>>>>  	MSR_IA32_SYSENTER_CS, MSR_IA32_SYSENTER_ESP, MSR_IA32_SYSENTER_EIP,
>>>>> @@ -1764,6 +1764,8 @@ static bool kvm_hv_msr_partition_wide(u32 msr)
>>>>>  	switch (msr) {
>>>>>  	case HV_X64_MSR_GUEST_OS_ID:
>>>>>  	case HV_X64_MSR_HYPERCALL:
>>>>> +	case HV_X64_MSR_REFERENCE_TSC:
>>>>> +	case HV_X64_MSR_TIME_REF_COUNT:
>>>>>  		r = true;
>>>>>  		break;
>>>>>  	}
>>>>> @@ -1803,6 +1805,21 @@ static int set_msr_hyperv_pw(struct kvm_vcpu *vcpu, u32 msr, u64 data)
>>>>>  		if (__copy_to_user((void __user *)addr, instructions, 4))
>>>>>  			return 1;
>>>>>  		kvm->arch.hv_hypercall = data;
>>>>> +		kvm->arch.hv_ref_count = get_kernel_ns();

Please rename to kvm->arch.hv_ref_count_base.

>>>>> +		break;
>>>>> +	}
>>>>> +	case HV_X64_MSR_REFERENCE_TSC: {
>>>>> +		u64 gfn;
>>>>> +		unsigned long addr;
>>>>> +		u32 tsc_ref;
>>>>> +		gfn = data >> HV_X64_MSR_HYPERCALL_PAGE_ADDRESS_SHIFT;
>>>> Shouldn't you check HV_X64_MSR_TSC_REFERENCE_ENABLE here?
>>>
>>> Yes, I have this check added in the second patch.
>>>
>> Move it here please.
> OK, will do it.

Or better, remove all the handling of HV_X64_MSR_REFERENCE_TSC from this
patch, and leave it all to the second.

Paolo

>>>>
>>>>> +		addr = gfn_to_hva(kvm, gfn);
>>>>> +		if (kvm_is_error_hva(addr))
>>>>> +			return 1;
>>>>> +		tsc_ref = 0;
>>>>> +		if(__copy_to_user((void __user *)addr, &tsc_ref, sizeof(tsc_ref)))
>>>>> +			return 1;
>>>>> +		kvm->arch.hv_tsc_page = data;
>>>>>  		break;
>>>>>  	}
>>>>>  	default:
>>>>> @@ -2229,6 +2246,12 @@ static int get_msr_hyperv_pw(struct kvm_vcpu *vcpu, u32 msr, u64 *pdata)
>>>>>  	case HV_X64_MSR_HYPERCALL:
>>>>>  		data = kvm->arch.hv_hypercall;
>>>>>  		break;
>>>>> +	case HV_X64_MSR_TIME_REF_COUNT: 
>>>>> +		data = div_u64(get_kernel_ns() - kvm->arch.hv_ref_count,100); 
>>>>> +		break;
>>>>> +	case HV_X64_MSR_REFERENCE_TSC:
>>>>> +		data = kvm->arch.hv_tsc_page;
>>>>> +		break;
>>>>>  	default:
>>>>>  		vcpu_unimpl(vcpu, "Hyper-V unhandled rdmsr: 0x%x\n", msr);
>>>>>  		return 1;
>>>>> -- 
>>>>> 1.8.1.2
>>>>
>>>> --
>>>> 			Gleb.
>>>
>>
>> --
>> 			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
> 

Re: [RFC PATCH 1/2] Hyper-H reference counter

[Vadim Rozenfeld]

On Thu, 2013-05-16 at 15:37 +0200, Paolo Bonzini wrote:
> Il 16/05/2013 11:28, Vadim Rozenfeld ha scritto:
> >>>>> > > > > +	case HV_X64_MSR_REFERENCE_TSC: {
> >>>>> > > > > +		u64 gfn;
> >>>>> > > > > +		unsigned long addr;
> >>>>> > > > > +		u32 tsc_ref;
> >>>>> > > > > +		gfn = data >> HV_X64_MSR_HYPERCALL_PAGE_ADDRESS_SHIFT;
> >>>> > > > Shouldn't you check HV_X64_MSR_TSC_REFERENCE_ENABLE here?
> >>> > > 
> >>> > > Yes, I have this check added in the second patch.
> >>> > > 
> >> > Move it here please.
> > OK, will do it.
> >> > 
> >>>> > > > 
> >>>>> > > > > +		addr = gfn_to_hva(kvm, gfn);
> >>>>> > > > > +		if (kvm_is_error_hva(addr))
> >>>>> > > > > +			return 1;
> >>>>> > > > > +		tsc_ref = 0;
> 
> This should write 0xFFFFFFFF.
This should write 0
Vadim.
> 
> Paolo

Re: [RFC PATCH 1/2] Hyper-H reference counter

[Vadim Rozenfeld]

On Thu, 2013-05-16 at 15:44 +0200, Paolo Bonzini wrote:
> Il 16/05/2013 11:28, Vadim Rozenfeld ha scritto:
> > On Thu, 2013-05-16 at 12:21 +0300, Gleb Natapov wrote:
> >> On Thu, May 16, 2013 at 07:13:41PM +1000, Vadim Rozenfeld wrote:
> >>> On Thu, 2013-05-16 at 11:34 +0300, Gleb Natapov wrote:
> >>>> On Mon, May 13, 2013 at 09:45:16PM +1000, Vadim Rozenfeld wrote:
> >>>>> Signed-off: Peter Lieven <pl@dlh.net>
> >>>>> Signed-off: Gleb Natapov <gleb@redhat.com>
> >>>>> Signed-off: Vadim Rozenfeld <vrozenfe@redhat.com>
> >>>>>
> >>>>> The following patch allows to activate Hyper-V 
> >>>>> reference time counter 
> >>>>> ---
> >>>>>  arch/x86/include/asm/kvm_host.h    |  2 ++
> >>>>>  arch/x86/include/uapi/asm/hyperv.h |  3 +++
> >>>>>  arch/x86/kvm/x86.c                 | 25 ++++++++++++++++++++++++-
> >>>>>  3 files changed, 29 insertions(+), 1 deletion(-)
> >>>>>
> >>>>> diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
> >>>>> index 3741c65..f0fee35 100644
> >>>>> --- a/arch/x86/include/asm/kvm_host.h
> >>>>> +++ b/arch/x86/include/asm/kvm_host.h
> >>>>> @@ -575,6 +575,8 @@ struct kvm_arch {
> >>>>>  	/* fields used by HYPER-V emulation */
> >>>>>  	u64 hv_guest_os_id;
> >>>>>  	u64 hv_hypercall;
> >>>>> +	u64 hv_ref_count;
> >>>>> +	u64 hv_tsc_page;
> >>>>>  
> >>>>>  	#ifdef CONFIG_KVM_MMU_AUDIT
> >>>>>  	int audit_point;
> >>>>> diff --git a/arch/x86/include/uapi/asm/hyperv.h b/arch/x86/include/uapi/asm/hyperv.h
> >>>>> index b80420b..9711819 100644
> >>>>> --- a/arch/x86/include/uapi/asm/hyperv.h
> >>>>> +++ b/arch/x86/include/uapi/asm/hyperv.h
> >>>>> @@ -136,6 +136,9 @@
> >>>>>  /* MSR used to read the per-partition time reference counter */
> >>>>>  #define HV_X64_MSR_TIME_REF_COUNT		0x40000020
> >>>>>  
> >>>>> +/* A partition's reference time stamp counter (TSC) page */
> >>>>> +#define HV_X64_MSR_REFERENCE_TSC		0x40000021
> >>>>> +
> >>>>>  /* Define the virtual APIC registers */
> >>>>>  #define HV_X64_MSR_EOI				0x40000070
> >>>>>  #define HV_X64_MSR_ICR				0x40000071
> >>>>> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> >>>>> index 094b5d9..1a4036d 100644
> >>>>> --- a/arch/x86/kvm/x86.c
> >>>>> +++ b/arch/x86/kvm/x86.c
> >>>>> @@ -843,7 +843,7 @@ EXPORT_SYMBOL_GPL(kvm_rdpmc);
> >>>>>  static u32 msrs_to_save[] = {
> >>>>>  	MSR_KVM_SYSTEM_TIME, MSR_KVM_WALL_CLOCK,
> >>>>>  	MSR_KVM_SYSTEM_TIME_NEW, MSR_KVM_WALL_CLOCK_NEW,
> >>>>> -	HV_X64_MSR_GUEST_OS_ID, HV_X64_MSR_HYPERCALL,
> >>>>> +	HV_X64_MSR_GUEST_OS_ID, HV_X64_MSR_HYPERCALL,HV_X64_MSR_TIME_REF_COUNT,
> >>>>>  	HV_X64_MSR_APIC_ASSIST_PAGE, MSR_KVM_ASYNC_PF_EN, MSR_KVM_STEAL_TIME,
> >>>>>  	MSR_KVM_PV_EOI_EN,
> >>>>>  	MSR_IA32_SYSENTER_CS, MSR_IA32_SYSENTER_ESP, MSR_IA32_SYSENTER_EIP,
> >>>>> @@ -1764,6 +1764,8 @@ static bool kvm_hv_msr_partition_wide(u32 msr)
> >>>>>  	switch (msr) {
> >>>>>  	case HV_X64_MSR_GUEST_OS_ID:
> >>>>>  	case HV_X64_MSR_HYPERCALL:
> >>>>> +	case HV_X64_MSR_REFERENCE_TSC:
> >>>>> +	case HV_X64_MSR_TIME_REF_COUNT:
> >>>>>  		r = true;
> >>>>>  		break;
> >>>>>  	}
> >>>>> @@ -1803,6 +1805,21 @@ static int set_msr_hyperv_pw(struct kvm_vcpu *vcpu, u32 msr, u64 data)
> >>>>>  		if (__copy_to_user((void __user *)addr, instructions, 4))
> >>>>>  			return 1;
> >>>>>  		kvm->arch.hv_hypercall = data;
> >>>>> +		kvm->arch.hv_ref_count = get_kernel_ns();
> 
> Please rename to kvm->arch.hv_ref_count_base.
> 
> >>>>> +		break;
> >>>>> +	}
> >>>>> +	case HV_X64_MSR_REFERENCE_TSC: {
> >>>>> +		u64 gfn;
> >>>>> +		unsigned long addr;
> >>>>> +		u32 tsc_ref;
> >>>>> +		gfn = data >> HV_X64_MSR_HYPERCALL_PAGE_ADDRESS_SHIFT;
> >>>> Shouldn't you check HV_X64_MSR_TSC_REFERENCE_ENABLE here?
> >>>
> >>> Yes, I have this check added in the second patch.
> >>>
> >> Move it here please.
> > OK, will do it.
> 
> Or better, remove all the handling of HV_X64_MSR_REFERENCE_TSC from this
> patch, and leave it all to the second.
> 

What for? Could you please elaborate?
Vadim. 

> Paolo
> 
> >>>>
> >>>>> +		addr = gfn_to_hva(kvm, gfn);
> >>>>> +		if (kvm_is_error_hva(addr))
> >>>>> +			return 1;
> >>>>> +		tsc_ref = 0;
> >>>>> +		if(__copy_to_user((void __user *)addr, &tsc_ref, sizeof(tsc_ref)))
> >>>>> +			return 1;
> >>>>> +		kvm->arch.hv_tsc_page = data;
> >>>>>  		break;
> >>>>>  	}
> >>>>>  	default:
> >>>>> @@ -2229,6 +2246,12 @@ static int get_msr_hyperv_pw(struct kvm_vcpu *vcpu, u32 msr, u64 *pdata)
> >>>>>  	case HV_X64_MSR_HYPERCALL:
> >>>>>  		data = kvm->arch.hv_hypercall;
> >>>>>  		break;
> >>>>> +	case HV_X64_MSR_TIME_REF_COUNT: 
> >>>>> +		data = div_u64(get_kernel_ns() - kvm->arch.hv_ref_count,100); 
> >>>>> +		break;
> >>>>> +	case HV_X64_MSR_REFERENCE_TSC:
> >>>>> +		data = kvm->arch.hv_tsc_page;
> >>>>> +		break;
> >>>>>  	default:
> >>>>>  		vcpu_unimpl(vcpu, "Hyper-V unhandled rdmsr: 0x%x\n", msr);
> >>>>>  		return 1;
> >>>>> -- 
> >>>>> 1.8.1.2
> >>>>
> >>>> --
> >>>> 			Gleb.
> >>>
> >>
> >> --
> >> 			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
> > 
> 

Re: [RFC PATCH 1/2] Hyper-H reference counter

[Paolo Bonzini]

Il 13/05/2013 13:45, Vadim Rozenfeld ha scritto:
> Signed-off: Peter Lieven <pl@dlh.net>
> Signed-off: Gleb Natapov <gleb@redhat.com>
> Signed-off: Vadim Rozenfeld <vrozenfe@redhat.com>
> 
> The following patch allows to activate Hyper-V 
> reference time counter 
> ---
>  arch/x86/include/asm/kvm_host.h    |  2 ++
>  arch/x86/include/uapi/asm/hyperv.h |  3 +++
>  arch/x86/kvm/x86.c                 | 25 ++++++++++++++++++++++++-
>  3 files changed, 29 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
> index 3741c65..f0fee35 100644
> --- a/arch/x86/include/asm/kvm_host.h
> +++ b/arch/x86/include/asm/kvm_host.h
> @@ -575,6 +575,8 @@ struct kvm_arch {
>  	/* fields used by HYPER-V emulation */
>  	u64 hv_guest_os_id;
>  	u64 hv_hypercall;
> +	u64 hv_ref_count;
> +	u64 hv_tsc_page;
>  
>  	#ifdef CONFIG_KVM_MMU_AUDIT
>  	int audit_point;
> diff --git a/arch/x86/include/uapi/asm/hyperv.h b/arch/x86/include/uapi/asm/hyperv.h
> index b80420b..9711819 100644
> --- a/arch/x86/include/uapi/asm/hyperv.h
> +++ b/arch/x86/include/uapi/asm/hyperv.h
> @@ -136,6 +136,9 @@
>  /* MSR used to read the per-partition time reference counter */
>  #define HV_X64_MSR_TIME_REF_COUNT		0x40000020
>  
> +/* A partition's reference time stamp counter (TSC) page */
> +#define HV_X64_MSR_REFERENCE_TSC		0x40000021
> +
>  /* Define the virtual APIC registers */
>  #define HV_X64_MSR_EOI				0x40000070
>  #define HV_X64_MSR_ICR				0x40000071
> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> index 094b5d9..1a4036d 100644
> --- a/arch/x86/kvm/x86.c
> +++ b/arch/x86/kvm/x86.c
> @@ -843,7 +843,7 @@ EXPORT_SYMBOL_GPL(kvm_rdpmc);
>  static u32 msrs_to_save[] = {
>  	MSR_KVM_SYSTEM_TIME, MSR_KVM_WALL_CLOCK,
>  	MSR_KVM_SYSTEM_TIME_NEW, MSR_KVM_WALL_CLOCK_NEW,
> -	HV_X64_MSR_GUEST_OS_ID, HV_X64_MSR_HYPERCALL,
> +	HV_X64_MSR_GUEST_OS_ID, HV_X64_MSR_HYPERCALL,HV_X64_MSR_TIME_REF_COUNT,
>  	HV_X64_MSR_APIC_ASSIST_PAGE, MSR_KVM_ASYNC_PF_EN, MSR_KVM_STEAL_TIME,
>  	MSR_KVM_PV_EOI_EN,
>  	MSR_IA32_SYSENTER_CS, MSR_IA32_SYSENTER_ESP, MSR_IA32_SYSENTER_EIP,
> @@ -1764,6 +1764,8 @@ static bool kvm_hv_msr_partition_wide(u32 msr)
>  	switch (msr) {
>  	case HV_X64_MSR_GUEST_OS_ID:
>  	case HV_X64_MSR_HYPERCALL:
> +	case HV_X64_MSR_REFERENCE_TSC:
> +	case HV_X64_MSR_TIME_REF_COUNT:
>  		r = true;
>  		break;
>  	}
> @@ -1803,6 +1805,21 @@ static int set_msr_hyperv_pw(struct kvm_vcpu *vcpu, u32 msr, u64 data)
>  		if (__copy_to_user((void __user *)addr, instructions, 4))
>  			return 1;
>  		kvm->arch.hv_hypercall = data;
> +		kvm->arch.hv_ref_count = get_kernel_ns();
> +		break;
> +	}
> +	case HV_X64_MSR_REFERENCE_TSC: {
> +		u64 gfn;
> +		unsigned long addr;
> +		u32 tsc_ref;
> +		gfn = data >> HV_X64_MSR_HYPERCALL_PAGE_ADDRESS_SHIFT;
> +		addr = gfn_to_hva(kvm, gfn);
> +		if (kvm_is_error_hva(addr))
> +			return 1;
> +		tsc_ref = 0;
> +		if(__copy_to_user((void __user *)addr, &tsc_ref, sizeof(tsc_ref)))
> +			return 1;
> +		kvm->arch.hv_tsc_page = data;
>  		break;
>  	}
>  	default:
> @@ -2229,6 +2246,12 @@ static int get_msr_hyperv_pw(struct kvm_vcpu *vcpu, u32 msr, u64 *pdata)
>  	case HV_X64_MSR_HYPERCALL:
>  		data = kvm->arch.hv_hypercall;
>  		break;
> +	case HV_X64_MSR_TIME_REF_COUNT: 
> +		data = div_u64(get_kernel_ns() - kvm->arch.hv_ref_count,100); 
> +		break;

For migration, this should probably be writable from userspace
(msr_info->host_initiated in kvm_set_msr_common).

However, the spec says that the reference counter must count up from 0
_when the partition is created_, not when you initialize the
enlightenments.  Perhaps we can just piggy back on the kvmclock ioctls
and remove hv_ref_count altogether.  Then userspace doesn't need to
write to HV_X64_MSR_TIME_REF_COUNT.

Paolo

> +	case HV_X64_MSR_REFERENCE_TSC:
> +		data = kvm->arch.hv_tsc_page;
> +		break;
>  	default:
>  		vcpu_unimpl(vcpu, "Hyper-V unhandled rdmsr: 0x%x\n", msr);
>  		return 1;
> 

Re: [RFC PATCH 1/2] Hyper-H reference counter

[Paolo Bonzini]

Il 16/05/2013 16:26, Vadim Rozenfeld ha scritto:
>>>>> > >>>
>>>>> > >>> Yes, I have this check added in the second patch.
>>>>> > >>>
>>>> > >> Move it here please.
>>> > > OK, will do it.
>> > 
>> > Or better, remove all the handling of HV_X64_MSR_REFERENCE_TSC from this
>> > patch, and leave it all to the second.
>> > 
> What for? Could you please elaborate?

To make code reviewable.  Add one MSR here, the other in the second patch.

Paolo

Re: [RFC PATCH 1/2] Hyper-H reference counter

[Paolo Bonzini]

Il 16/05/2013 16:22, Vadim Rozenfeld ha scritto:
>>>>>>>>>>> > >>>>> > > > > +		addr = gfn_to_hva(kvm, gfn);
>>>>>>>>>>> > >>>>> > > > > +		if (kvm_is_error_hva(addr))
>>>>>>>>>>> > >>>>> > > > > +			return 1;
>>>>>>>>>>> > >>>>> > > > > +		tsc_ref = 0;
>> > 
>> > This should write 0xFFFFFFFF.
> This should write 0

No.  You have not writing sensible values to TscScale/TscOffset, hence
the VM should fall back to another time source.

Microsoft docs say "A special value of 0xFFFFFFFF is used to indicate
that this facility is no longer a reliable source of reference time and
the virtual machine must fall back to a different source (for example,
the virtual PM timer)".

So you need to write 0xFFFFFFFF here (though my suggestion would rather
be to remove this part of the patch, and do everything in the next
patch).  The next patch should pick 0 if constant TSC is available,
0xFFFFFFFF if it is not.

If the docs are wrong, I stand corrected but then you need a comment
explaining that.

Paolo

Re: [RFC PATCH 1/2] Hyper-H reference counter

[Vadim Rozenfeld]

On Thu, 2013-05-16 at 16:45 +0200, Paolo Bonzini wrote:
> Il 16/05/2013 16:26, Vadim Rozenfeld ha scritto:
> >>>>> > >>>
> >>>>> > >>> Yes, I have this check added in the second patch.
> >>>>> > >>>
> >>>> > >> Move it here please.
> >>> > > OK, will do it.
> >> > 
> >> > Or better, remove all the handling of HV_X64_MSR_REFERENCE_TSC from this
> >> > patch, and leave it all to the second.
> >> > 
> > What for? Could you please elaborate?
> 
> To make code reviewable.  Add one MSR here, the other in the second patch.
removing HV_X64_MSR_REFERENCE_TSC will make this particular patch
completely non-functional.
> 
> Paolo

Re: [RFC PATCH 1/2] Hyper-H reference counter

[Paolo Bonzini]

Il 19/05/2013 08:37, Vadim Rozenfeld ha scritto:
> On Thu, 2013-05-16 at 16:45 +0200, Paolo Bonzini wrote:
>> Il 16/05/2013 16:26, Vadim Rozenfeld ha scritto:
>>>>>>>>>>>
>>>>>>>>>>> Yes, I have this check added in the second patch.
>>>>>>>>>>>
>>>>>>>>> Move it here please.
>>>>>>> OK, will do it.
>>>>>
>>>>> Or better, remove all the handling of HV_X64_MSR_REFERENCE_TSC from this
>>>>> patch, and leave it all to the second.
>>>>>
>>> What for? Could you please elaborate?
>
> To make code reviewable.  Add one MSR here, the other in the second patch.
> removing HV_X64_MSR_REFERENCE_TSC will make this particular patch
> completely non-functional.

Do you mean Windows guest will BSOD or just that they won't use the
reference TSC?  If the latter, it's not a problem.

Paolo

Re: [RFC PATCH 1/2] Hyper-H reference counter

[Gleb Natapov]

On Mon, May 20, 2013 at 10:05:38AM +0200, Paolo Bonzini wrote:
> Il 19/05/2013 08:37, Vadim Rozenfeld ha scritto:
> > On Thu, 2013-05-16 at 16:45 +0200, Paolo Bonzini wrote:
> >> Il 16/05/2013 16:26, Vadim Rozenfeld ha scritto:
> >>>>>>>>>>>
> >>>>>>>>>>> Yes, I have this check added in the second patch.
> >>>>>>>>>>>
> >>>>>>>>> Move it here please.
> >>>>>>> OK, will do it.
> >>>>>
> >>>>> Or better, remove all the handling of HV_X64_MSR_REFERENCE_TSC from this
> >>>>> patch, and leave it all to the second.
> >>>>>
> >>> What for? Could you please elaborate?
> >
> > To make code reviewable.  Add one MSR here, the other in the second patch.
> > removing HV_X64_MSR_REFERENCE_TSC will make this particular patch
> > completely non-functional.
> 
> Do you mean Windows guest will BSOD or just that they won't use the
> reference TSC?  If the latter, it's not a problem.
> 
I think it is. If reference counter works without TSC we have a bisect
point for the case when something will going wrong with TSC.

--
			Gleb.

Re: [RFC PATCH 1/2] Hyper-H reference counter

[Paolo Bonzini]

Il 20/05/2013 10:36, Gleb Natapov ha scritto:
> On Mon, May 20, 2013 at 10:05:38AM +0200, Paolo Bonzini wrote:
>> Il 19/05/2013 08:37, Vadim Rozenfeld ha scritto:
>>> On Thu, 2013-05-16 at 16:45 +0200, Paolo Bonzini wrote:
>>>> Il 16/05/2013 16:26, Vadim Rozenfeld ha scritto:
>>>>>>>>>>>>>
>>>>>>>>>>>>> Yes, I have this check added in the second patch.
>>>>>>>>>>>>>
>>>>>>>>>>> Move it here please.
>>>>>>>>> OK, will do it.
>>>>>>>
>>>>>>> Or better, remove all the handling of HV_X64_MSR_REFERENCE_TSC from this
>>>>>>> patch, and leave it all to the second.
>>>>>>>
>>>>> What for? Could you please elaborate?
>>>
>>> To make code reviewable.  Add one MSR here, the other in the second patch.
>>> removing HV_X64_MSR_REFERENCE_TSC will make this particular patch
>>> completely non-functional.
>>
>> Do you mean Windows guest will BSOD or just that they won't use the
>> reference TSC?  If the latter, it's not a problem.
>>
> I think it is. If reference counter works without TSC we have a bisect
> point for the case when something will going wrong with TSC.

Isn't that exactly what might happen with this patch only?  Windows will
not use the TSC because it finds invalid values in the TSC page.  If it
still uses the reference counter, we have the situation you describe.

    refcount        TSC page
        Y              Y           <= after patch 2
        Y              N           <= after patch 1
        N              Y           <= impossible
        N              N           <= removing TSC page from this patch?

Of course if the guest BSODs, it's not possible to split the patches
that way.  Perhaps in that case it's simply better to do a single patch.

Paolo

Re: [RFC PATCH 1/2] Hyper-H reference counter

[Gleb Natapov]

On Mon, May 20, 2013 at 10:42:52AM +0200, Paolo Bonzini wrote:
> Il 20/05/2013 10:36, Gleb Natapov ha scritto:
> > On Mon, May 20, 2013 at 10:05:38AM +0200, Paolo Bonzini wrote:
> >> Il 19/05/2013 08:37, Vadim Rozenfeld ha scritto:
> >>> On Thu, 2013-05-16 at 16:45 +0200, Paolo Bonzini wrote:
> >>>> Il 16/05/2013 16:26, Vadim Rozenfeld ha scritto:
> >>>>>>>>>>>>>
> >>>>>>>>>>>>> Yes, I have this check added in the second patch.
> >>>>>>>>>>>>>
> >>>>>>>>>>> Move it here please.
> >>>>>>>>> OK, will do it.
> >>>>>>>
> >>>>>>> Or better, remove all the handling of HV_X64_MSR_REFERENCE_TSC from this
> >>>>>>> patch, and leave it all to the second.
> >>>>>>>
> >>>>> What for? Could you please elaborate?
> >>>
> >>> To make code reviewable.  Add one MSR here, the other in the second patch.
> >>> removing HV_X64_MSR_REFERENCE_TSC will make this particular patch
> >>> completely non-functional.
> >>
> >> Do you mean Windows guest will BSOD or just that they won't use the
> >> reference TSC?  If the latter, it's not a problem.
> >>
> > I think it is. If reference counter works without TSC we have a bisect
> > point for the case when something will going wrong with TSC.
> 
> Isn't that exactly what might happen with this patch only?  Windows will
> not use the TSC because it finds invalid values in the TSC page.
Yes, it will use reference counter instead. Exactly what we want for a bisect point.

>                                                                  If it
> still uses the reference counter, we have the situation you describe.
> 
>     refcount        TSC page
>         Y              Y           <= after patch 2
>         Y              N           <= after patch 1
>         N              Y           <= impossible
>         N              N           <= removing TSC page from this patch?
> 
> Of course if the guest BSODs, it's not possible to split the patches
> that way.  Perhaps in that case it's simply better to do a single patch.
> 
I am not sure what you are trying to say. Your option list above shows
that there is a value to split patches like they are split now.

--
			Gleb.

Re: [RFC PATCH 1/2] Hyper-H reference counter

[Paolo Bonzini]

Il 20/05/2013 10:49, Gleb Natapov ha scritto:
> On Mon, May 20, 2013 at 10:42:52AM +0200, Paolo Bonzini wrote:
>> Il 20/05/2013 10:36, Gleb Natapov ha scritto:
>>> On Mon, May 20, 2013 at 10:05:38AM +0200, Paolo Bonzini wrote:
>>>> Il 19/05/2013 08:37, Vadim Rozenfeld ha scritto:
>>>>> On Thu, 2013-05-16 at 16:45 +0200, Paolo Bonzini wrote:
>>>>>> Il 16/05/2013 16:26, Vadim Rozenfeld ha scritto:
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> Yes, I have this check added in the second patch.
>>>>>>>>>>>>>>>
>>>>>>>>>>>>> Move it here please.
>>>>>>>>>>> OK, will do it.
>>>>>>>>>
>>>>>>>>> Or better, remove all the handling of HV_X64_MSR_REFERENCE_TSC from this
>>>>>>>>> patch, and leave it all to the second.
>>>>>>>>>
>>>>>>> What for? Could you please elaborate?
>>>>>
>>>>> To make code reviewable.  Add one MSR here, the other in the second patch.
>>>>> removing HV_X64_MSR_REFERENCE_TSC will make this particular patch
>>>>> completely non-functional.
>>>>
>>>> Do you mean Windows guest will BSOD or just that they won't use the
>>>> reference TSC?  If the latter, it's not a problem.
>>>>
>>> I think it is. If reference counter works without TSC we have a bisect
>>> point for the case when something will going wrong with TSC.
>>
>> Isn't that exactly what might happen with this patch only?  Windows will
>> not use the TSC because it finds invalid values in the TSC page.
> 
> Yes, it will use reference counter instead. Exactly what we want for a bisect point.
> 
>>                                                                  If it
>> still uses the reference counter, we have the situation you describe.
>>
>>     refcount        TSC page
>>         Y              Y           <= after patch 2
>>         Y              N           <= after patch 1
>>         N              Y           <= impossible
>>         N              N           <= removing TSC page from this patch?
>>
>> Of course if the guest BSODs, it's not possible to split the patches
>> that way.  Perhaps in that case it's simply better to do a single patch.
>>
> I am not sure what you are trying to say. Your option list above shows
> that there is a value to split patches like they are split now.

Hmm, we're talking past each other. :)

I put the "?" because that's what Vadim implied ("it would make this
particular patch non-functional"), but I don't see why it should be like
this.  To me, the obvious way of getting the desired bisect point is
implementing one MSR per patch.  So, moving the REFERENCE_TSC handling
entirely to patch 2 would still be in the "refcount=Y, TSC page=N" case.

In any case, this patch needs more comments and a better commit message.
 Microsoft docs are decent, but there are several non-obvious points in
how the patches were done, and they need to be documented.

Re: [RFC PATCH 1/2] Hyper-H reference counter

[Vadim Rozenfeld]

On Mon, 2013-05-20 at 10:05 +0200, Paolo Bonzini wrote:
> Il 19/05/2013 08:37, Vadim Rozenfeld ha scritto:
> > On Thu, 2013-05-16 at 16:45 +0200, Paolo Bonzini wrote:
> >> Il 16/05/2013 16:26, Vadim Rozenfeld ha scritto:
> >>>>>>>>>>>
> >>>>>>>>>>> Yes, I have this check added in the second patch.
> >>>>>>>>>>>
> >>>>>>>>> Move it here please.
> >>>>>>> OK, will do it.
> >>>>>
> >>>>> Or better, remove all the handling of HV_X64_MSR_REFERENCE_TSC from this
> >>>>> patch, and leave it all to the second.
> >>>>>
> >>> What for? Could you please elaborate?
> >
> > To make code reviewable.  Add one MSR here, the other in the second patch.
> > removing HV_X64_MSR_REFERENCE_TSC will make this particular patch
> > completely non-functional.
> 
> Do you mean Windows guest will BSOD or just that they won't use the
> reference TSC?  If the latter, it's not a problem.
Unfortunately, it will crash. 
> 
> Paolo

Re: [RFC PATCH 1/2] Hyper-H reference counter

[Gleb Natapov]

On Mon, May 20, 2013 at 10:56:22AM +0200, Paolo Bonzini wrote:
> Il 20/05/2013 10:49, Gleb Natapov ha scritto:
> > On Mon, May 20, 2013 at 10:42:52AM +0200, Paolo Bonzini wrote:
> >> Il 20/05/2013 10:36, Gleb Natapov ha scritto:
> >>> On Mon, May 20, 2013 at 10:05:38AM +0200, Paolo Bonzini wrote:
> >>>> Il 19/05/2013 08:37, Vadim Rozenfeld ha scritto:
> >>>>> On Thu, 2013-05-16 at 16:45 +0200, Paolo Bonzini wrote:
> >>>>>> Il 16/05/2013 16:26, Vadim Rozenfeld ha scritto:
> >>>>>>>>>>>>>>>
> >>>>>>>>>>>>>>> Yes, I have this check added in the second patch.
> >>>>>>>>>>>>>>>
> >>>>>>>>>>>>> Move it here please.
> >>>>>>>>>>> OK, will do it.
> >>>>>>>>>
> >>>>>>>>> Or better, remove all the handling of HV_X64_MSR_REFERENCE_TSC from this
> >>>>>>>>> patch, and leave it all to the second.
> >>>>>>>>>
> >>>>>>> What for? Could you please elaborate?
> >>>>>
> >>>>> To make code reviewable.  Add one MSR here, the other in the second patch.
> >>>>> removing HV_X64_MSR_REFERENCE_TSC will make this particular patch
> >>>>> completely non-functional.
> >>>>
> >>>> Do you mean Windows guest will BSOD or just that they won't use the
> >>>> reference TSC?  If the latter, it's not a problem.
> >>>>
> >>> I think it is. If reference counter works without TSC we have a bisect
> >>> point for the case when something will going wrong with TSC.
> >>
> >> Isn't that exactly what might happen with this patch only?  Windows will
> >> not use the TSC because it finds invalid values in the TSC page.
> > 
> > Yes, it will use reference counter instead. Exactly what we want for a bisect point.
> > 
> >>                                                                  If it
> >> still uses the reference counter, we have the situation you describe.
> >>
> >>     refcount        TSC page
> >>         Y              Y           <= after patch 2
> >>         Y              N           <= after patch 1
> >>         N              Y           <= impossible
> >>         N              N           <= removing TSC page from this patch?
> >>
> >> Of course if the guest BSODs, it's not possible to split the patches
> >> that way.  Perhaps in that case it's simply better to do a single patch.
> >>
> > I am not sure what you are trying to say. Your option list above shows
> > that there is a value to split patches like they are split now.
> 
> Hmm, we're talking past each other. :)
> 
> I put the "?" because that's what Vadim implied ("it would make this
> particular patch non-functional"), but I don't see why it should be like
> this.  To me, the obvious way of getting the desired bisect point is
> implementing one MSR per patch.  So, moving the REFERENCE_TSC handling
> entirely to patch 2 would still be in the "refcount=Y, TSC page=N" case.
> 
This is not the case as far as I understand from Vadim. Actually,
from past discussion I remember win8 had some problem with incomplete
implementation, but that was likely pre release versions.


> In any case, this patch needs more comments and a better commit message.
>  Microsoft docs are decent, but there are several non-obvious points in
> how the patches were done, and they need to be documented.

--
			Gleb.

Re: [RFC PATCH 1/2] Hyper-H reference counter

[Gleb Natapov]

On Mon, May 20, 2013 at 10:56:22AM +0200, Paolo Bonzini wrote:
> In any case, this patch needs more comments and a better commit message.
>  Microsoft docs are decent, but there are several non-obvious points in
> how the patches were done, and they need to be documented.
I wish you were right about Microsoft docs :) So in Hyper-V spec they
say:

  Special value of 0xFFFFFFFF is used to indicate that this facility is no
  longer a reliable source of reference time and the virtual machine must
  fall back to a different source (for example, the virtual PM timer).

May be they really mean "virtual PM timer" here and reference counter is
not considered as a fall back source, but this is not what we want.

On the other hand in API specification [1] they have:

#define HV_REFERENCE_TSC_SEQUENCE_INVALID   (0x00000000)

which is not even documented in hyper-v spec. Actually 0 is specified as
valid value there. Go figure.

[1] http://msdn.microsoft.com/en-us/library/windows/hardware/ff540244%28v=vs.85%29.aspx

--
			Gleb.

Re: [RFC PATCH 1/2] Hyper-H reference counter

[Paolo Bonzini]

Il 20/05/2013 11:25, Gleb Natapov ha scritto:
> So in Hyper-V spec they
> say:
> 
>   Special value of 0xFFFFFFFF is used to indicate that this facility is no
>   longer a reliable source of reference time and the virtual machine must
>   fall back to a different source (for example, the virtual PM timer).
> 
> May be they really mean "virtual PM timer" here and reference counter is
> not considered as a fall back source, but this is not what we want.
> 
> On the other hand in API specification [1] they have:
> 
> #define HV_REFERENCE_TSC_SEQUENCE_INVALID   (0x00000000)
> 
> which is not even documented in hyper-v spec. Actually 0 is specified as
> valid value there. Go figure.
> 
> [1] http://msdn.microsoft.com/en-us/library/windows/hardware/ff540244%28v=vs.85%29.aspx

Ok, if the API document is right then we should use
HV_REFERENCE_TSC_SEQUENCE_INVALID instead of 0, with a comment
explaining why we use 0 and not 0xFFFFFFFF.

Paolo

Re: [RFC PATCH 1/2] Hyper-H reference counter

[Gleb Natapov]

On Mon, May 20, 2013 at 11:32:27AM +0200, Paolo Bonzini wrote:
> Il 20/05/2013 11:25, Gleb Natapov ha scritto:
> > So in Hyper-V spec they
> > say:
> > 
> >   Special value of 0xFFFFFFFF is used to indicate that this facility is no
> >   longer a reliable source of reference time and the virtual machine must
> >   fall back to a different source (for example, the virtual PM timer).
> > 
> > May be they really mean "virtual PM timer" here and reference counter is
> > not considered as a fall back source, but this is not what we want.
> > 
> > On the other hand in API specification [1] they have:
> > 
> > #define HV_REFERENCE_TSC_SEQUENCE_INVALID   (0x00000000)
> > 
> > which is not even documented in hyper-v spec. Actually 0 is specified as
> > valid value there. Go figure.
> > 
> > [1] http://msdn.microsoft.com/en-us/library/windows/hardware/ff540244%28v=vs.85%29.aspx
> 
> Ok, if the API document is right then we should use
> HV_REFERENCE_TSC_SEQUENCE_INVALID instead of 0, with a comment
> explaining why we use 0 and not 0xFFFFFFFF.
> 
Using define is always a good idea no matter if API doc is right or
hyper-v spec is, it's just the "decent documentation" part that got me :)
Definitely better than nothing and thanks them for that.

--
			Gleb.

Re: [RFC PATCH 1/2] Hyper-H reference counter

[Peter Lieven]

Hi all,

sorry that I am a bit unresponsive about this series. I have a few days off and can't spend much time in this.
If I read that the REFERENCE TSC breaks migration I don't think its a good option to include it at all.
I have this hyperv_refcnt MSR in an internal patch I sent over about 1.5 years ago and its working flawlessly
with Win2k8R2, Win7, Win8 + Win2012. I set the reference TSC to 0x00 and this seems to work with all
the above Windows versions. Some of the early Alphas of Windows 8 didn't work with this patch, but the
final is running smoothly also with migration etc.

I crafted this patch to avoid the heavy calls to PM Timer during high I/O which slowed down Windows
approx. by 30% compared to Hyper-V.

I reinclude this patch for reference. Its unchanged since mid 2012 and it might not apply.

Cheers,
Peter

diff -Npur kvm-kmod-3.3/include/asm-x86/hyperv.h kvm-kmod-3.3-hyperv-refcnt/include/asm-x86/hyperv.h
--- kvm-kmod-3.3/include/asm-x86/hyperv.h	2012-03-19 23:00:49.000000000 +0100
+++ kvm-kmod-3.3-hyperv-refcnt/include/asm-x86/hyperv.h	2012-03-28 12:23:02.000000000 +0200
@@ -169,7 +169,8 @@

/* MSR used to read the per-partition time reference counter */
#define HV_X64_MSR_TIME_REF_COUNT		0x40000020
-
+#define HV_X64_MSR_REFERENCE_TSC		0x40000021
+                                        
/* Define the virtual APIC registers */
#define HV_X64_MSR_EOI				0x40000070
#define HV_X64_MSR_ICR				0x40000071
diff -Npur kvm-kmod-3.3/include/asm-x86/kvm_host.h kvm-kmod-3.3-hyperv-refcnt/include/asm-x86/kvm_host.h
--- kvm-kmod-3.3/include/asm-x86/kvm_host.h	2012-03-19 23:00:49.000000000 +0100
+++ kvm-kmod-3.3-hyperv-refcnt/include/asm-x86/kvm_host.h	2012-03-28 15:08:24.000000000 +0200
@@ -553,6 +553,8 @@ struct kvm_arch {
	/* fields used by HYPER-V emulation */
	u64 hv_guest_os_id;
	u64 hv_hypercall;
+	u64 hv_ref_count;
+	u64 hv_reference_tsc;

	atomic_t reader_counter;

diff -Npur kvm-kmod-3.3/x86/x86.c kvm-kmod-3.3-hyperv-refcnt/x86/x86.c
--- kvm-kmod-3.3/x86/x86.c	2012-03-19 23:00:56.000000000 +0100
+++ kvm-kmod-3.3-hyperv-refcnt/x86/x86.c	2012-03-28 16:27:46.000000000 +0200
@@ -826,7 +826,7 @@ EXPORT_SYMBOL_GPL(kvm_rdpmc);
static u32 msrs_to_save[] = {
	MSR_KVM_SYSTEM_TIME, MSR_KVM_WALL_CLOCK,
	MSR_KVM_SYSTEM_TIME_NEW, MSR_KVM_WALL_CLOCK_NEW,
-	HV_X64_MSR_GUEST_OS_ID, HV_X64_MSR_HYPERCALL,
+	HV_X64_MSR_GUEST_OS_ID, HV_X64_MSR_HYPERCALL, HV_X64_MSR_TIME_REF_COUNT,
	HV_X64_MSR_APIC_ASSIST_PAGE, MSR_KVM_ASYNC_PF_EN, MSR_KVM_STEAL_TIME,
	MSR_IA32_SYSENTER_CS, MSR_IA32_SYSENTER_ESP, MSR_IA32_SYSENTER_EIP,
	MSR_STAR,
@@ -1387,6 +1387,8 @@ static bool kvm_hv_msr_partition_wide(u3
	switch (msr) {
	case HV_X64_MSR_GUEST_OS_ID:
	case HV_X64_MSR_HYPERCALL:
+	case HV_X64_MSR_REFERENCE_TSC:
+	case HV_X64_MSR_TIME_REF_COUNT:
		r = true;
		break;
	}
@@ -1426,6 +1428,21 @@ static int set_msr_hyperv_pw(struct kvm_
		if (__copy_to_user((void   *)addr, instructions, 4))
			return 1;
		kvm->arch.hv_hypercall = data;
+		kvm->arch.hv_ref_count = get_kernel_ns();
+ 		break;
+	}
+	case HV_X64_MSR_REFERENCE_TSC: {
+		u64 gfn;
+		unsigned long addr;
+		u32 hv_tsc_sequence;
+		gfn = data >> HV_X64_MSR_HYPERCALL_PAGE_ADDRESS_SHIFT;
+		addr = gfn_to_hva(kvm, gfn);
+		if (kvm_is_error_hva(addr))
+			return 1;
+		hv_tsc_sequence = 0x0; //invalid
+		if (__copy_to_user((void   *)addr, (void __user *) &hv_tsc_sequence, sizeof(hv_tsc_sequence)))
+			return 1;		
+		kvm->arch.hv_reference_tsc = data;
		break;
	}
	default:
@@ -1826,6 +1843,17 @@ static int get_msr_hyperv_pw(struct kvm_
	case HV_X64_MSR_HYPERCALL:
		data = kvm->arch.hv_hypercall;
		break;
+	case HV_X64_MSR_TIME_REF_COUNT: {
+		u64 now_ns;
+		local_irq_disable();
+		now_ns = get_kernel_ns();
+		data = div_u64(now_ns + kvm->arch.kvmclock_offset - kvm->arch.hv_ref_count,100); 
+		local_irq_enable();
+		break;
+	}
+	case HV_X64_MSR_REFERENCE_TSC:
+		data = kvm->arch.hv_reference_tsc;
+		break;
	default:
		pr_unimpl(vcpu, "Hyper-V unhandled rdmsr: 0x%x\n", msr);
		return 1;


Am 20.05.2013 um 11:41 schrieb Gleb Natapov <gleb@redhat.com>:

> On Mon, May 20, 2013 at 11:32:27AM +0200, Paolo Bonzini wrote:
>> Il 20/05/2013 11:25, Gleb Natapov ha scritto:
>>> So in Hyper-V spec they
>>> say:
>>> 
>>>  Special value of 0xFFFFFFFF is used to indicate that this facility is no
>>>  longer a reliable source of reference time and the virtual machine must
>>>  fall back to a different source (for example, the virtual PM timer).
>>> 
>>> May be they really mean "virtual PM timer" here and reference counter is
>>> not considered as a fall back source, but this is not what we want.
>>> 
>>> On the other hand in API specification [1] they have:
>>> 
>>> #define HV_REFERENCE_TSC_SEQUENCE_INVALID   (0x00000000)
>>> 
>>> which is not even documented in hyper-v spec. Actually 0 is specified as
>>> valid value there. Go figure.
>>> 
>>> [1] http://msdn.microsoft.com/en-us/library/windows/hardware/ff540244%28v=vs.85%29.aspx
>> 
>> Ok, if the API document is right then we should use
>> HV_REFERENCE_TSC_SEQUENCE_INVALID instead of 0, with a comment
>> explaining why we use 0 and not 0xFFFFFFFF.
>> 
> Using define is always a good idea no matter if API doc is right or
> hyper-v spec is, it's just the "decent documentation" part that got me :)
> Definitely better than nothing and thanks them for that.
> 
> --
> 			Gleb.

Re: [RFC PATCH 1/2] Hyper-H reference counter

[Vadim Rozenfeld]

On Mon, 2013-05-20 at 10:56 +0200, Paolo Bonzini wrote:
> Il 20/05/2013 10:49, Gleb Natapov ha scritto:
> > On Mon, May 20, 2013 at 10:42:52AM +0200, Paolo Bonzini wrote:
> >> Il 20/05/2013 10:36, Gleb Natapov ha scritto:
> >>> On Mon, May 20, 2013 at 10:05:38AM +0200, Paolo Bonzini wrote:
> >>>> Il 19/05/2013 08:37, Vadim Rozenfeld ha scritto:
> >>>>> On Thu, 2013-05-16 at 16:45 +0200, Paolo Bonzini wrote:
> >>>>>> Il 16/05/2013 16:26, Vadim Rozenfeld ha scritto:
> >>>>>>>>>>>>>>>
> >>>>>>>>>>>>>>> Yes, I have this check added in the second patch.
> >>>>>>>>>>>>>>>
> >>>>>>>>>>>>> Move it here please.
> >>>>>>>>>>> OK, will do it.
> >>>>>>>>>
> >>>>>>>>> Or better, remove all the handling of HV_X64_MSR_REFERENCE_TSC from this
> >>>>>>>>> patch, and leave it all to the second.
> >>>>>>>>>
> >>>>>>> What for? Could you please elaborate?
> >>>>>
> >>>>> To make code reviewable.  Add one MSR here, the other in the second patch.
> >>>>> removing HV_X64_MSR_REFERENCE_TSC will make this particular patch
> >>>>> completely non-functional.
> >>>>
> >>>> Do you mean Windows guest will BSOD or just that they won't use the
> >>>> reference TSC?  If the latter, it's not a problem.
> >>>>
> >>> I think it is. If reference counter works without TSC we have a bisect
> >>> point for the case when something will going wrong with TSC.
> >>
> >> Isn't that exactly what might happen with this patch only?  Windows will
> >> not use the TSC because it finds invalid values in the TSC page.
> > 
> > Yes, it will use reference counter instead. Exactly what we want for a bisect point.
> > 
> >>                                                                  If it
> >> still uses the reference counter, we have the situation you describe.
> >>
> >>     refcount        TSC page
> >>         Y              Y           <= after patch 2
> >>         Y              N           <= after patch 1
> >>         N              Y           <= impossible
> >>         N              N           <= removing TSC page from this patch?
> >>
> >> Of course if the guest BSODs, it's not possible to split the patches
> >> that way.  Perhaps in that case it's simply better to do a single patch.
> >>
> > I am not sure what you are trying to say. Your option list above shows
> > that there is a value to split patches like they are split now.
> 
> Hmm, we're talking past each other. :)
> 
> I put the "?" because that's what Vadim implied ("it would make this
> particular patch non-functional"), but I don't see why it should be like
> this.  To me, the obvious way of getting the desired bisect point is
> implementing one MSR per patch.  So, moving the REFERENCE_TSC handling
> entirely to patch 2 would still be in the "refcount=Y, TSC page=N" case.
> 
> In any case, this patch needs more comments and a better commit message.
>  Microsoft docs are decent, but there are several non-obvious points in
> how the patches were done, and they need to be documented.
We need specify two partition privileges to activate reference time
enlightenment in HYPERV_CPUID_FEATURES (0x40000003)
AccessPartitionReferenceCounter and AccessPartitionReferenceTsc
otherwise VM will use HPET or PMTimer as a timestamp source.
If we specify AccessPartitionReferenceTsc but don't handle write 
request to HV_X64_MSR_REFERENCE_TSC - the system will fail with 0x78
(PHASE0_EXCEPTION) bugcheck code. If we provide HV_X64_MSR_REFERENCE_TSC
handler but don't initialize sequence to 0 - guest will probably newer
start or will be extremely slow, because in this case scale should also
be initialized. Sequence 0 is a special case, it means use reference
counter, but not TSC, as a time source. It is also a fallback solution
in a case when a VM, which using TSC has been migrated to a host, which
is not equipped with invariant TSC.  
        
 

Re: [RFC PATCH 1/2] Hyper-H reference counter

[Vadim Rozenfeld]

On Mon, 2013-05-20 at 12:25 +0300, Gleb Natapov wrote:
> On Mon, May 20, 2013 at 10:56:22AM +0200, Paolo Bonzini wrote:
> > In any case, this patch needs more comments and a better commit message.
> >  Microsoft docs are decent, but there are several non-obvious points in
> > how the patches were done, and they need to be documented.
> I wish you were right about Microsoft docs :) So in Hyper-V spec they
> say:
> 
>   Special value of 0xFFFFFFFF is used to indicate that this facility is no
>   longer a reliable source of reference time and the virtual machine must
>   fall back to a different source (for example, the virtual PM timer).
> 
> May be they really mean "virtual PM timer" here and reference counter is
> not considered as a fall back source, but this is not what we want.

As far as I know, you cannot fall back from iTSC to PMTimer or HPET,
but you can fallback to reference counters.

> 
> On the other hand in API specification [1] they have:
> 
> #define HV_REFERENCE_TSC_SEQUENCE_INVALID   (0x00000000)
> 
> which is not even documented in hyper-v spec. Actually 0 is specified as
> valid value there. Go figure.
> 
> [1] http://msdn.microsoft.com/en-us/library/windows/hardware/ff540244%28v=vs.85%29.aspx
> 
> --
> 			Gleb.

Re: [RFC PATCH 1/2] Hyper-H reference counter

[Gleb Natapov]

On Mon, May 20, 2013 at 08:25:11PM +1000, Vadim Rozenfeld wrote:
> On Mon, 2013-05-20 at 12:25 +0300, Gleb Natapov wrote:
> > On Mon, May 20, 2013 at 10:56:22AM +0200, Paolo Bonzini wrote:
> > > In any case, this patch needs more comments and a better commit message.
> > >  Microsoft docs are decent, but there are several non-obvious points in
> > > how the patches were done, and they need to be documented.
> > I wish you were right about Microsoft docs :) So in Hyper-V spec they
> > say:
> > 
> >   Special value of 0xFFFFFFFF is used to indicate that this facility is no
> >   longer a reliable source of reference time and the virtual machine must
> >   fall back to a different source (for example, the virtual PM timer).
> > 
> > May be they really mean "virtual PM timer" here and reference counter is
> > not considered as a fall back source, but this is not what we want.
> 
> As far as I know, you cannot fall back from iTSC to PMTimer or HPET,
> but you can fallback to reference counters.
> 
What if you put 0xFFFFFFFF as a sequence? Or is this another case where
the spec is wrong.

> > 
> > On the other hand in API specification [1] they have:
> > 
> > #define HV_REFERENCE_TSC_SEQUENCE_INVALID   (0x00000000)
> > 
> > which is not even documented in hyper-v spec. Actually 0 is specified as
> > valid value there. Go figure.
> > 
> > [1] http://msdn.microsoft.com/en-us/library/windows/hardware/ff540244%28v=vs.85%29.aspx
> > 
> > --
> > 			Gleb.
> 

--
			Gleb.

Re: [RFC PATCH 1/2] Hyper-H reference counter

[Vadim Rozenfeld]

On Mon, 2013-05-20 at 13:27 +0300, Gleb Natapov wrote:
> On Mon, May 20, 2013 at 08:25:11PM +1000, Vadim Rozenfeld wrote:
> > On Mon, 2013-05-20 at 12:25 +0300, Gleb Natapov wrote:
> > > On Mon, May 20, 2013 at 10:56:22AM +0200, Paolo Bonzini wrote:
> > > > In any case, this patch needs more comments and a better commit message.
> > > >  Microsoft docs are decent, but there are several non-obvious points in
> > > > how the patches were done, and they need to be documented.
> > > I wish you were right about Microsoft docs :) So in Hyper-V spec they
> > > say:
> > > 
> > >   Special value of 0xFFFFFFFF is used to indicate that this facility is no
> > >   longer a reliable source of reference time and the virtual machine must
> > >   fall back to a different source (for example, the virtual PM timer).
> > > 
> > > May be they really mean "virtual PM timer" here and reference counter is
> > > not considered as a fall back source, but this is not what we want.
> > 
> > As far as I know, you cannot fall back from iTSC to PMTimer or HPET,
> > but you can fallback to reference counters.
> > 
> 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 the other hand in API specification [1] they have:
> > > 
> > > #define HV_REFERENCE_TSC_SEQUENCE_INVALID   (0x00000000)
> > > 
> > > which is not even documented in hyper-v spec. Actually 0 is specified as
> > > valid value there. Go figure.
> > > 
> > > [1] http://msdn.microsoft.com/en-us/library/windows/hardware/ff540244%28v=vs.85%29.aspx
> > > 
> > > --
> > > 			Gleb.
> > 
> 
> --
> 			Gleb.