Re: A clocksource driver for HyperV

Re: A clocksource driver for HyperV

[Ky Srinivasan]

[-- Attachment #1: Type: text/plain, Size: 5489 bytes --]



>>> On 4/5/2010 at  5:36 PM, in message <  4BBA57FB.2000406@goop.org  >, Jeremy
Fitzhardinge <  jeremy@goop.org  > wrote: 
> On 04/05/2010 01:30 PM, Ky Srinivasan wrote:
>> +static cycle_t read_hv_clock(struct clocksource *arg)
>> +{
>> +	cycle_t current_tick;
>> +	/*
>> +	 * Read the partition counter to get the current tick count. This count
>> +	 * is set to 0 when the partition is created and is incremented in
>> +	 * 100 nanosecond units.
>> +	 */
>> +	rdmsrl(HV_X64_MSR_TIME_REF_COUNT, current_tick);
>> +	return current_tick;
>> +}
>> +
>> +static struct clocksource clocksource_hyperv = {
>> +	.name           = "hyperv_clocksource",
>>    
> 
> Seems like a redundantly long name; any use of this string is going to 
> be in a context where it is obviously a clocksource.  How about just 
> "hyperv"


> 
>> +	.rating         = 400, /* use this when running on Hyperv*/
>> +	.read           = read_hv_clock,
>> +	.mask           = CLOCKSOURCE_MASK(64),
>> +	.shift          = HV_CLOCK_SHIFT,
>> +};
>> +
>> +static struct dmi_system_id __initconst
>> +hv_timesource_dmi_table[] __maybe_unused  = {
>> +	{
>> +		.ident = "Hyper-V",
>> +		.matches = {
>> +			DMI_MATCH(DMI_SYS_VENDOR, "Microsoft Corporation"),
>> +			DMI_MATCH(DMI_PRODUCT_NAME, "Virtual Machine"),
>> +			DMI_MATCH(DMI_BOARD_NAME, "Virtual Machine"),
>> +		},
>> +	},
>> +	{ },
>> +};
>> +MODULE_DEVICE_TABLE(dmi, hv_timesource_dmi_table);
>>    
> 
> So you use the DMI signatures to determine whether the module is needed, 
> but cpuid to work out if the feature is present?
> 
>> +
>> +static struct pci_device_id __initconst
>> +hv_timesource_pci_table[] __maybe_unused = {
>> +	{ PCI_DEVICE(0x1414, 0x5353) }, /* VGA compatible controller */
>> +	{ 0 }
>> +};
>> +MODULE_DEVICE_TABLE(pci, hv_timesource_pci_table);
>>    
> 
> And/or PCI?
> 
> Seems a bit... ad-hoc?  Is this the official way to determine the 
> presence of Hyper-V?

The presence of HyperV and our ability to use the partition-wide counter obviously is checked via probing the cpuid leaves. The DMI/PCI signatures are used in auto-loading these modules.
> 
>> +
>> +
>> +static int __init hv_detect_hyperv(void)
>>    
> 
> This looks generally useful.  Should it be hidden away in the 
> clocksource driver, or in some common hyper-v code?  Do other hyper-v 
> drivers have versions of this?
Good point. Right now, I can think of  multiple drivers replicating this code. We could include hyperV detection code in cpu/hypervisor.c . I will spin up a patch for doing that shortly. 
> 
>> +{
>> +	u32 eax, ebx, ecx, edx;
>> +	static char hyp_signature[20];
>>    
> 20?  static?
> 
>> +
>> +	cpuid(1,&eax,&ebx,&ecx,&edx);
>> +	if (!(ecx&  HV_HYPERVISOR_PRESENT_BIT)) {
>> +		printk(KERN_WARNING
>> +			"Not on a Hypervisor\n");
>>    
> This just looks like noise, especially since it doesn't identify what is 
> generating the message.  And if you compile this code in as =y 
> (non-modular) then it will complain every boot.
> 
>> +		return 1;
>> +	}
>> +	cpuid(HV_CPUID_SIGNATURE,&eax,&ebx,&ecx,&edx);
>> +	*(u32 *)(hyp_signature + 0) = ebx;
>> +	*(u32 *)(hyp_signature + 4) = ecx;
>> +	*(u32 *)(hyp_signature + 8) = edx;
>> +	hyp_signature[12] = 0;
>> +
>> +	if ((eax<  HV_CPUID_MIN) || (strcmp("Microsoft Hv", hyp_signature))) {
>>    
> 
> memcmp, surely?
> 
>> +		printk(KERN_WARNING
>> +			"Not on HyperV; signature %s, eax %x\n",
>> +			hyp_signature, eax);
>> +		return 1;
>> +	}
>> +	/*
>> +	 * Extract the features, recommendations etc.
>> +	 */
>> +	cpuid(HV_CPUID_FEATURES,&eax,&ebx,&ecx,&edx);
>> +	if (!(eax&  0x10)) {
>> +		printk(KERN_WARNING "HyperV Time Ref Counter not available!\n");
>> +		return 1;
>> +	}
>> +
>> +	cpuid(HV_CPUID_RECOMMENDATIONS,&eax,&ebx,&ecx,&edx);
>> +	printk(KERN_INFO "HyperV recommendations: %x\n", eax);
>> +	printk(KERN_INFO "HyperV spin count: %x\n", ebx);
>> +	return 0;
>> +}
>> +
>> +
>> +static int __init init_hv_clocksource(void)
>> +{
>> +	if (hv_detect_hyperv())
>> +		return -ENODEV;


>> +	/*
>> +	 * The time ref counter in HyperV is in 100ns units.
>> +	 * The definition of mult is:
>> +	 * mult/2^shift = ns/cyc = 100
>> +	 * mult = (100<<  shift)
>> +	 */
>> +	clocksource_hyperv.mult = (100<<  HV_CLOCK_SHIFT);
>>    
> 
> Why not initialize this in the structure?  It's just 100<<22 isn't it?
> 
>> +	printk(KERN_INFO "Registering HyperV clock source\n");
>> +	return clocksource_register(&clocksource_hyperv);
>> +}
>> +
>> +module_init(init_hv_clocksource);
>> +MODULE_DESCRIPTION("HyperV based clocksource");
>> +MODULE_AUTHOR("K. Y. Srinivasan<  ksrinivasan@novell.com  >");
>> +MODULE_LICENSE("GPL");
>> Index: linux/drivers/staging/hv/Makefile
>> ===================================================================
>> --- linux.orig/drivers/staging/hv/Makefile	2010-04-05 13:02:06.000000000 -0600
>> +++ linux/drivers/staging/hv/Makefile	2010-04-05 13:02:13.000000000 -0600
>> @@ -1,4 +1,4 @@
>> -obj-$(CONFIG_HYPERV)		+= hv_vmbus.o
>> +obj-$(CONFIG_HYPERV)		+= hv_vmbus.o hv_timesource.o
>>   obj-$(CONFIG_HYPERV_STORAGE)	+= hv_storvsc.o
>>   obj-$(CONFIG_HYPERV_BLOCK)	+= hv_blkvsc.o
>>   obj-$(CONFIG_HYPERV_NET)	+= hv_netvsc.o
>>    

Jeremy, thank you for your comments. I am attaching the next version of this patch that addresses the comments I have gotten thus far.  

Regards,

K. Y 


[-- Attachment #2: hyperv_clocksource.patch --]
[-- Type: text/plain, Size: 5525 bytes --]

From: K. Y. Srinivasan <ksrinivasan@novell.com>
Subject:  A clocksource for Linux guests hosted on HyperV.
References: None
Patch-mainline: 

This patch is a clocksource implementation suitable for guests hosted on HyperV.
Time keeping in Linux guests hosted on HyperV is unstable. This clocksource 
driver fixes the problem. 

Signed-off-by: K. Y. Srinivasan <ksrinivasan@novell.com>

Index: linux/drivers/staging/hv/hv_timesource.c
===================================================================
--- /dev/null	1970-01-01 00:00:00.000000000 +0000
+++ linux/drivers/staging/hv/hv_timesource.c	2010-04-07 12:17:29.000000000 -0600
@@ -0,0 +1,147 @@
+/*
+ * A clocksource for Linux running on HyperV.
+ *
+ *
+ * Copyright (C) 2010, Novell, Inc.
+ * Author : K. Y. Srinivasan <ksrinivasan@novell.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful, but
+ * WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY OR FITNESS FOR A PARTICULAR PURPOSE, GOOD TITLE or
+ * NON INFRINGEMENT.  See the GNU General Public License for more
+ * details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
+ *
+ */
+
+#include <linux/version.h>
+#include <linux/clocksource.h>
+#include <linux/init.h>
+#include <linux/module.h>
+#include <linux/pci.h>
+#include <linux/dmi.h>
+
+#define HV_CLOCK_SHIFT	22
+/*
+ * HyperV defined synthetic CPUID leaves:
+ */
+#define HV_CPUID_SIGNATURE	0x40000000
+#define HV_CPUID_MIN		0x40000005
+#define HV_HYPERVISOR_PRESENT_BIT	0x80000000
+#define HV_CPUID_FEATURES	0x40000003
+#define HV_CPUID_RECOMMENDATIONS	0x40000004
+
+/*
+ * HyperV defined synthetic MSRs
+ */
+
+#define HV_X64_MSR_TIME_REF_COUNT	0x40000020
+
+
+static cycle_t read_hv_clock(struct clocksource *arg)
+{
+	cycle_t current_tick;
+	/*
+	 * Read the partition counter to get the current tick count. This count
+	 * is set to 0 when the partition is created and is incremented in
+	 * 100 nanosecond units.
+	 */
+	rdmsrl(HV_X64_MSR_TIME_REF_COUNT, current_tick);
+	return current_tick;
+}
+
+static struct clocksource hyperv_cs = {
+	.name           = "hyperv_clocksource",
+	.rating         = 400, /* use this when running on Hyperv*/
+	.read           = read_hv_clock,
+	.mask           = CLOCKSOURCE_MASK(64),
+	/*
+	 * The time ref counter in HyperV is in 100ns units.
+	 * The definition of mult is:
+	 * mult/2^shift = ns/cyc = 100
+	 * mult = (100 << shift)
+	 */
+	.mult           = (100 << HV_CLOCK_SHIFT),
+	.shift          = HV_CLOCK_SHIFT,
+};
+
+static const struct dmi_system_id __initconst
+hv_timesource_dmi_table[] __maybe_unused  = {
+	{
+		.ident = "Hyper-V",
+		.matches = {
+			DMI_MATCH(DMI_SYS_VENDOR, "Microsoft Corporation"),
+			DMI_MATCH(DMI_PRODUCT_NAME, "Virtual Machine"),
+			DMI_MATCH(DMI_BOARD_NAME, "Virtual Machine"),
+		},
+	},
+	{ },
+};
+MODULE_DEVICE_TABLE(dmi, hv_timesource_dmi_table);
+
+static const struct pci_device_id __initconst
+hv_timesource_pci_table[] __maybe_unused = {
+	{ PCI_DEVICE(0x1414, 0x5353) }, /* VGA compatible controller */
+	{ 0 }
+};
+MODULE_DEVICE_TABLE(pci, hv_timesource_pci_table);
+
+
+static int __init hv_detect_hyperv(void)
+{
+	u32 eax, ebx, ecx, edx;
+	char hyp_signature[20];
+
+	cpuid(1, &eax, &ebx, &ecx, &edx);
+
+	if (!(ecx & HV_HYPERVISOR_PRESENT_BIT))
+		return 1;
+
+	cpuid(HV_CPUID_SIGNATURE, &eax, &ebx, &ecx, &edx);
+	*(u32 *)(hyp_signature + 0) = ebx;
+	*(u32 *)(hyp_signature + 4) = ecx;
+	*(u32 *)(hyp_signature + 8) = edx;
+
+	if ((eax < HV_CPUID_MIN) ||
+	    (memcmp("Microsoft Hv", hyp_signature, 12))) {
+		printk(KERN_WARNING
+			"Not on HyperV; signature %s, eax %x\n",
+			hyp_signature, eax);
+		return 1;
+	}
+	/*
+	 * Extract the features, recommendations etc.
+	 */
+	cpuid(HV_CPUID_FEATURES, &eax, &ebx, &ecx, &edx);
+	if (!(eax & 0x10)) {
+		printk(KERN_WARNING "HyperV Time Ref Counter not available!\n");
+		return 1;
+	}
+
+	cpuid(HV_CPUID_RECOMMENDATIONS, &eax, &ebx, &ecx, &edx);
+	printk(KERN_INFO "HyperV recommendations: %x\n", eax);
+	printk(KERN_INFO "HyperV spin count: %x\n", ebx);
+	return 0;
+}
+
+
+static int __init init_hv_clocksource(void)
+{
+	if (hv_detect_hyperv())
+		return -ENODEV;
+	printk(KERN_INFO "Registering HyperV clock source\n");
+	return clocksource_register(&hyperv_cs);
+}
+
+module_init(init_hv_clocksource);
+MODULE_DESCRIPTION("HyperV based clocksource");
+MODULE_AUTHOR("K. Y. Srinivasan <ksrinivasan@novell.com>");
+MODULE_LICENSE("GPL");
Index: linux/drivers/staging/hv/Makefile
===================================================================
--- linux.orig/drivers/staging/hv/Makefile	2010-04-07 12:17:25.000000000 -0600
+++ linux/drivers/staging/hv/Makefile	2010-04-07 12:17:29.000000000 -0600
@@ -1,4 +1,4 @@
-obj-$(CONFIG_HYPERV)		+= hv_vmbus.o
+obj-$(CONFIG_HYPERV)		+= hv_vmbus.o hv_timesource.o
 obj-$(CONFIG_HYPERV_STORAGE)	+= hv_storvsc.o
 obj-$(CONFIG_HYPERV_BLOCK)	+= hv_blkvsc.o
 obj-$(CONFIG_HYPERV_NET)	+= hv_netvsc.o

[-- Attachment #3: Type: text/plain, Size: 184 bytes --]

_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linux-foundation.org/mailman/listinfo/virtualization

Re: A clocksource driver for HyperV

[Ky Srinivasan]

[-- Attachment #1: Type: text/plain, Size: 5489 bytes --]



>>> On 4/5/2010 at  5:36 PM, in message <  4BBA57FB.2000406@goop.org  >, Jeremy
Fitzhardinge <  jeremy@goop.org  > wrote: 
> On 04/05/2010 01:30 PM, Ky Srinivasan wrote:
>> +static cycle_t read_hv_clock(struct clocksource *arg)
>> +{
>> +	cycle_t current_tick;
>> +	/*
>> +	 * Read the partition counter to get the current tick count. This count
>> +	 * is set to 0 when the partition is created and is incremented in
>> +	 * 100 nanosecond units.
>> +	 */
>> +	rdmsrl(HV_X64_MSR_TIME_REF_COUNT, current_tick);
>> +	return current_tick;
>> +}
>> +
>> +static struct clocksource clocksource_hyperv = {
>> +	.name           = "hyperv_clocksource",
>>    
> 
> Seems like a redundantly long name; any use of this string is going to 
> be in a context where it is obviously a clocksource.  How about just 
> "hyperv"


> 
>> +	.rating         = 400, /* use this when running on Hyperv*/
>> +	.read           = read_hv_clock,
>> +	.mask           = CLOCKSOURCE_MASK(64),
>> +	.shift          = HV_CLOCK_SHIFT,
>> +};
>> +
>> +static struct dmi_system_id __initconst
>> +hv_timesource_dmi_table[] __maybe_unused  = {
>> +	{
>> +		.ident = "Hyper-V",
>> +		.matches = {
>> +			DMI_MATCH(DMI_SYS_VENDOR, "Microsoft Corporation"),
>> +			DMI_MATCH(DMI_PRODUCT_NAME, "Virtual Machine"),
>> +			DMI_MATCH(DMI_BOARD_NAME, "Virtual Machine"),
>> +		},
>> +	},
>> +	{ },
>> +};
>> +MODULE_DEVICE_TABLE(dmi, hv_timesource_dmi_table);
>>    
> 
> So you use the DMI signatures to determine whether the module is needed, 
> but cpuid to work out if the feature is present?
> 
>> +
>> +static struct pci_device_id __initconst
>> +hv_timesource_pci_table[] __maybe_unused = {
>> +	{ PCI_DEVICE(0x1414, 0x5353) }, /* VGA compatible controller */
>> +	{ 0 }
>> +};
>> +MODULE_DEVICE_TABLE(pci, hv_timesource_pci_table);
>>    
> 
> And/or PCI?
> 
> Seems a bit... ad-hoc?  Is this the official way to determine the 
> presence of Hyper-V?

The presence of HyperV and our ability to use the partition-wide counter obviously is checked via probing the cpuid leaves. The DMI/PCI signatures are used in auto-loading these modules.
> 
>> +
>> +
>> +static int __init hv_detect_hyperv(void)
>>    
> 
> This looks generally useful.  Should it be hidden away in the 
> clocksource driver, or in some common hyper-v code?  Do other hyper-v 
> drivers have versions of this?
Good point. Right now, I can think of  multiple drivers replicating this code. We could include hyperV detection code in cpu/hypervisor.c . I will spin up a patch for doing that shortly. 
> 
>> +{
>> +	u32 eax, ebx, ecx, edx;
>> +	static char hyp_signature[20];
>>    
> 20?  static?
> 
>> +
>> +	cpuid(1,&eax,&ebx,&ecx,&edx);
>> +	if (!(ecx&  HV_HYPERVISOR_PRESENT_BIT)) {
>> +		printk(KERN_WARNING
>> +			"Not on a Hypervisor\n");
>>    
> This just looks like noise, especially since it doesn't identify what is 
> generating the message.  And if you compile this code in as =y 
> (non-modular) then it will complain every boot.
> 
>> +		return 1;
>> +	}
>> +	cpuid(HV_CPUID_SIGNATURE,&eax,&ebx,&ecx,&edx);
>> +	*(u32 *)(hyp_signature + 0) = ebx;
>> +	*(u32 *)(hyp_signature + 4) = ecx;
>> +	*(u32 *)(hyp_signature + 8) = edx;
>> +	hyp_signature[12] = 0;
>> +
>> +	if ((eax<  HV_CPUID_MIN) || (strcmp("Microsoft Hv", hyp_signature))) {
>>    
> 
> memcmp, surely?
> 
>> +		printk(KERN_WARNING
>> +			"Not on HyperV; signature %s, eax %x\n",
>> +			hyp_signature, eax);
>> +		return 1;
>> +	}
>> +	/*
>> +	 * Extract the features, recommendations etc.
>> +	 */
>> +	cpuid(HV_CPUID_FEATURES,&eax,&ebx,&ecx,&edx);
>> +	if (!(eax&  0x10)) {
>> +		printk(KERN_WARNING "HyperV Time Ref Counter not available!\n");
>> +		return 1;
>> +	}
>> +
>> +	cpuid(HV_CPUID_RECOMMENDATIONS,&eax,&ebx,&ecx,&edx);
>> +	printk(KERN_INFO "HyperV recommendations: %x\n", eax);
>> +	printk(KERN_INFO "HyperV spin count: %x\n", ebx);
>> +	return 0;
>> +}
>> +
>> +
>> +static int __init init_hv_clocksource(void)
>> +{
>> +	if (hv_detect_hyperv())
>> +		return -ENODEV;


>> +	/*
>> +	 * The time ref counter in HyperV is in 100ns units.
>> +	 * The definition of mult is:
>> +	 * mult/2^shift = ns/cyc = 100
>> +	 * mult = (100<<  shift)
>> +	 */
>> +	clocksource_hyperv.mult = (100<<  HV_CLOCK_SHIFT);
>>    
> 
> Why not initialize this in the structure?  It's just 100<<22 isn't it?
> 
>> +	printk(KERN_INFO "Registering HyperV clock source\n");
>> +	return clocksource_register(&clocksource_hyperv);
>> +}
>> +
>> +module_init(init_hv_clocksource);
>> +MODULE_DESCRIPTION("HyperV based clocksource");
>> +MODULE_AUTHOR("K. Y. Srinivasan<  ksrinivasan@novell.com  >");
>> +MODULE_LICENSE("GPL");
>> Index: linux/drivers/staging/hv/Makefile
>> ===================================================================
>> --- linux.orig/drivers/staging/hv/Makefile	2010-04-05 13:02:06.000000000 -0600
>> +++ linux/drivers/staging/hv/Makefile	2010-04-05 13:02:13.000000000 -0600
>> @@ -1,4 +1,4 @@
>> -obj-$(CONFIG_HYPERV)		+= hv_vmbus.o
>> +obj-$(CONFIG_HYPERV)		+= hv_vmbus.o hv_timesource.o
>>   obj-$(CONFIG_HYPERV_STORAGE)	+= hv_storvsc.o
>>   obj-$(CONFIG_HYPERV_BLOCK)	+= hv_blkvsc.o
>>   obj-$(CONFIG_HYPERV_NET)	+= hv_netvsc.o
>>    

Jeremy, thank you for your comments. I am attaching the next version of this patch that addresses the comments I have gotten thus far.  

Regards,

K. Y 


[-- Attachment #2: hyperv_clocksource.patch --]
[-- Type: text/plain, Size: 5525 bytes --]

From: K. Y. Srinivasan <ksrinivasan@novell.com>
Subject:  A clocksource for Linux guests hosted on HyperV.
References: None
Patch-mainline: 

This patch is a clocksource implementation suitable for guests hosted on HyperV.
Time keeping in Linux guests hosted on HyperV is unstable. This clocksource 
driver fixes the problem. 

Signed-off-by: K. Y. Srinivasan <ksrinivasan@novell.com>

Index: linux/drivers/staging/hv/hv_timesource.c
===================================================================
--- /dev/null	1970-01-01 00:00:00.000000000 +0000
+++ linux/drivers/staging/hv/hv_timesource.c	2010-04-07 12:17:29.000000000 -0600
@@ -0,0 +1,147 @@
+/*
+ * A clocksource for Linux running on HyperV.
+ *
+ *
+ * Copyright (C) 2010, Novell, Inc.
+ * Author : K. Y. Srinivasan <ksrinivasan@novell.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful, but
+ * WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY OR FITNESS FOR A PARTICULAR PURPOSE, GOOD TITLE or
+ * NON INFRINGEMENT.  See the GNU General Public License for more
+ * details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
+ *
+ */
+
+#include <linux/version.h>
+#include <linux/clocksource.h>
+#include <linux/init.h>
+#include <linux/module.h>
+#include <linux/pci.h>
+#include <linux/dmi.h>
+
+#define HV_CLOCK_SHIFT	22
+/*
+ * HyperV defined synthetic CPUID leaves:
+ */
+#define HV_CPUID_SIGNATURE	0x40000000
+#define HV_CPUID_MIN		0x40000005
+#define HV_HYPERVISOR_PRESENT_BIT	0x80000000
+#define HV_CPUID_FEATURES	0x40000003
+#define HV_CPUID_RECOMMENDATIONS	0x40000004
+
+/*
+ * HyperV defined synthetic MSRs
+ */
+
+#define HV_X64_MSR_TIME_REF_COUNT	0x40000020
+
+
+static cycle_t read_hv_clock(struct clocksource *arg)
+{
+	cycle_t current_tick;
+	/*
+	 * Read the partition counter to get the current tick count. This count
+	 * is set to 0 when the partition is created and is incremented in
+	 * 100 nanosecond units.
+	 */
+	rdmsrl(HV_X64_MSR_TIME_REF_COUNT, current_tick);
+	return current_tick;
+}
+
+static struct clocksource hyperv_cs = {
+	.name           = "hyperv_clocksource",
+	.rating         = 400, /* use this when running on Hyperv*/
+	.read           = read_hv_clock,
+	.mask           = CLOCKSOURCE_MASK(64),
+	/*
+	 * The time ref counter in HyperV is in 100ns units.
+	 * The definition of mult is:
+	 * mult/2^shift = ns/cyc = 100
+	 * mult = (100 << shift)
+	 */
+	.mult           = (100 << HV_CLOCK_SHIFT),
+	.shift          = HV_CLOCK_SHIFT,
+};
+
+static const struct dmi_system_id __initconst
+hv_timesource_dmi_table[] __maybe_unused  = {
+	{
+		.ident = "Hyper-V",
+		.matches = {
+			DMI_MATCH(DMI_SYS_VENDOR, "Microsoft Corporation"),
+			DMI_MATCH(DMI_PRODUCT_NAME, "Virtual Machine"),
+			DMI_MATCH(DMI_BOARD_NAME, "Virtual Machine"),
+		},
+	},
+	{ },
+};
+MODULE_DEVICE_TABLE(dmi, hv_timesource_dmi_table);
+
+static const struct pci_device_id __initconst
+hv_timesource_pci_table[] __maybe_unused = {
+	{ PCI_DEVICE(0x1414, 0x5353) }, /* VGA compatible controller */
+	{ 0 }
+};
+MODULE_DEVICE_TABLE(pci, hv_timesource_pci_table);
+
+
+static int __init hv_detect_hyperv(void)
+{
+	u32 eax, ebx, ecx, edx;
+	char hyp_signature[20];
+
+	cpuid(1, &eax, &ebx, &ecx, &edx);
+
+	if (!(ecx & HV_HYPERVISOR_PRESENT_BIT))
+		return 1;
+
+	cpuid(HV_CPUID_SIGNATURE, &eax, &ebx, &ecx, &edx);
+	*(u32 *)(hyp_signature + 0) = ebx;
+	*(u32 *)(hyp_signature + 4) = ecx;
+	*(u32 *)(hyp_signature + 8) = edx;
+
+	if ((eax < HV_CPUID_MIN) ||
+	    (memcmp("Microsoft Hv", hyp_signature, 12))) {
+		printk(KERN_WARNING
+			"Not on HyperV; signature %s, eax %x\n",
+			hyp_signature, eax);
+		return 1;
+	}
+	/*
+	 * Extract the features, recommendations etc.
+	 */
+	cpuid(HV_CPUID_FEATURES, &eax, &ebx, &ecx, &edx);
+	if (!(eax & 0x10)) {
+		printk(KERN_WARNING "HyperV Time Ref Counter not available!\n");
+		return 1;
+	}
+
+	cpuid(HV_CPUID_RECOMMENDATIONS, &eax, &ebx, &ecx, &edx);
+	printk(KERN_INFO "HyperV recommendations: %x\n", eax);
+	printk(KERN_INFO "HyperV spin count: %x\n", ebx);
+	return 0;
+}
+
+
+static int __init init_hv_clocksource(void)
+{
+	if (hv_detect_hyperv())
+		return -ENODEV;
+	printk(KERN_INFO "Registering HyperV clock source\n");
+	return clocksource_register(&hyperv_cs);
+}
+
+module_init(init_hv_clocksource);
+MODULE_DESCRIPTION("HyperV based clocksource");
+MODULE_AUTHOR("K. Y. Srinivasan <ksrinivasan@novell.com>");
+MODULE_LICENSE("GPL");
Index: linux/drivers/staging/hv/Makefile
===================================================================
--- linux.orig/drivers/staging/hv/Makefile	2010-04-07 12:17:25.000000000 -0600
+++ linux/drivers/staging/hv/Makefile	2010-04-07 12:17:29.000000000 -0600
@@ -1,4 +1,4 @@
-obj-$(CONFIG_HYPERV)		+= hv_vmbus.o
+obj-$(CONFIG_HYPERV)		+= hv_vmbus.o hv_timesource.o
 obj-$(CONFIG_HYPERV_STORAGE)	+= hv_storvsc.o
 obj-$(CONFIG_HYPERV_BLOCK)	+= hv_blkvsc.o
 obj-$(CONFIG_HYPERV_NET)	+= hv_netvsc.o

[-- Attachment #3: Type: text/plain, Size: 184 bytes --]

_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linux-foundation.org/mailman/listinfo/virtualization

Re: A clocksource driver for HyperV

[Jeremy Fitzhardinge]

On 04/07/2010 11:20 AM, Ky Srinivasan wrote:
> Jeremy, thank you for your comments. I am attaching the next version of this patch that addresses the comments I have gotten thus far.
>    

It looks like you overlooked a couple of comments.

> +}
> +
> +static struct clocksource hyperv_cs = {
> +	.name           = "hyperv_clocksource",
> +	.rating         = 400, /* use this when running on Hyperv*/
> +	.read           = read_hv_clock,
>    
Did you not want to call it "hyperv" for some reason?

> +static int __init hv_detect_hyperv(void)
> +{
> +	u32 eax, ebx, ecx, edx;
> +	char hyp_signature[20];
>    
Why 20?  You're only putting 12 bytes into it.

> +
> +	cpuid(1,&eax,&ebx,&ecx,&edx);
> +
> +	if (!(ecx&  HV_HYPERVISOR_PRESENT_BIT))
> +		return 1;
> +
> +	cpuid(HV_CPUID_SIGNATURE,&eax,&ebx,&ecx,&edx);
> +	*(u32 *)(hyp_signature + 0) = ebx;
> +	*(u32 *)(hyp_signature + 4) = ecx;
> +	*(u32 *)(hyp_signature + 8) = edx;
> +
> +	if ((eax<  HV_CPUID_MIN) ||
> +	    (memcmp("Microsoft Hv", hyp_signature, 12))) {
> +		printk(KERN_WARNING
> +			"Not on HyperV; signature %s, eax %x\n",
> +			hyp_signature, eax);
>    

The message is pointless.  There's nothing to error or warn about 
booting under some other
hypervisor.

Thanks,
     J

Re: A clocksource driver for HyperV

[Ky Srinivasan]

[-- Attachment #1: Type: text/plain, Size: 305 bytes --]

Jeremy,

As suggested by you, I am enclosing a patch that integrates HyperV detection with the  current hypervisor detection mechanism in the kernel. Also attached is a HyperV clocksource patch that uses this new functionality.

Signed-off-by: K. Y. Srinivasan <ksrinivasan@novell.com>








[-- Attachment #2: hyperv_detection.patch --]
[-- Type: text/plain, Size: 7013 bytes --]

From: K. Y. Srinivasan <ksrinivasan@novell.com>
Subject:  HyperV Detection code.
References: None
Patch-mainline:

This patch integrates HyperV detection within the framework currently used by
VmWare. With this patch, we can avoid having to replicate the HyperV detection
code in each of the HyperV specific Linux drivers - MSFT Linux Integration 
Components. 

Signed-off-by: K. Y. Srinivasan <ksrinivasan@novell.com>

Index: linux-2.6.34-rc4/arch/x86/include/asm/processor.h
===================================================================
--- linux-2.6.34-rc4.orig/arch/x86/include/asm/processor.h	2010-04-12 19:41:35.000000000 -0600
+++ linux-2.6.34-rc4/arch/x86/include/asm/processor.h	2010-04-15 10:00:18.000000000 -0600
@@ -114,6 +114,8 @@ struct cpuinfo_x86 {
 	u16			cpu_index;
 #endif
 	unsigned int		x86_hyper_vendor;
+	/* The layout of this field is hypervisor specific */
+	unsigned int		x86_hyper_features;
 } __attribute__((__aligned__(SMP_CACHE_BYTES)));
 
 #define X86_VENDOR_INTEL	0
@@ -129,6 +131,7 @@ struct cpuinfo_x86 {
 
 #define X86_HYPER_VENDOR_NONE  0
 #define X86_HYPER_VENDOR_VMWARE 1
+#define X86_HYPER_VENDOR_MSFT 2
 
 /*
  * capabilities of CPUs
Index: linux-2.6.34-rc4/arch/x86/kernel/cpu/Makefile
===================================================================
--- linux-2.6.34-rc4.orig/arch/x86/kernel/cpu/Makefile	2010-04-12 19:41:35.000000000 -0600
+++ linux-2.6.34-rc4/arch/x86/kernel/cpu/Makefile	2010-04-15 10:00:18.000000000 -0600
@@ -14,7 +14,7 @@ CFLAGS_common.o		:= $(nostackp)
 
 obj-y			:= intel_cacheinfo.o addon_cpuid_features.o
 obj-y			+= proc.o capflags.o powerflags.o common.o
-obj-y			+= vmware.o hypervisor.o sched.o
+obj-y			+= vmware.o hypervisor.o sched.o hyperv.o
 
 obj-$(CONFIG_X86_32)	+= bugs.o cmpxchg.o
 obj-$(CONFIG_X86_64)	+= bugs_64.o
Index: linux-2.6.34-rc4/arch/x86/kernel/cpu/hyperv.c
===================================================================
--- /dev/null	1970-01-01 00:00:00.000000000 +0000
+++ linux-2.6.34-rc4/arch/x86/kernel/cpu/hyperv.c	2010-04-15 10:00:18.000000000 -0600
@@ -0,0 +1,66 @@
+/*
+ * HyperV  Detection code.
+ *
+ * Copyright (C) 2010, Novell, Inc.
+ * Author : K. Y. Srinivasan <ksrinivasan@novell.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful, but
+ * WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY OR FITNESS FOR A PARTICULAR PURPOSE, GOOD TITLE or
+ * NON INFRINGEMENT.  See the GNU General Public License for more
+ * details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
+ *
+ */
+
+#include <linux/types.h>
+#include <asm/processor.h>
+#include <asm/hyperv.h>
+
+
+int hyperv_platform(void)
+{
+	u32 eax, ebx, ecx, edx;
+	char hyp_signature[13];
+
+	cpuid(1, &eax, &ebx, &ecx, &edx);
+	if (!(ecx & HYPERV_HYPERVISOR_PRESENT_BIT))
+		return 0;
+
+	cpuid(HYPERV_CPUID_VENDOR_AND_MAX_FUNCTIONS, &eax, &ebx, &ecx, &edx);
+	*(u32 *)(hyp_signature + 0) = ebx;
+	*(u32 *)(hyp_signature + 4) = ecx;
+	*(u32 *)(hyp_signature + 8) = edx;
+
+	if ((eax < HYPERV_CPUID_MIN) || (memcmp("Microsoft Hv", hyp_signature, 12)))
+		return 0;
+	return 1;
+}
+
+void __cpuinit hyperv_set_feature_bits(struct cpuinfo_x86 *c)
+{
+	u32 eax, ebx, ecx, edx;
+
+	c->x86_hyper_features = 0;
+	/*
+	 * Extract the features, recommendations etc.
+	 * The first 9 bits will be used to track hypervisor features.
+	 * The next 6 bits will be used to track the hypervisor
+	 * recommendations.
+	 */
+	cpuid(HYPERV_CPUID_FEATURES, &eax, &ebx, &ecx, &edx);
+	c->x86_hyper_features |= (eax & 0x1ff);
+
+	cpuid(HYPERV_CPUID_ENLIGHTMENT_INFO, &eax, &ebx, &ecx, &edx);
+	c->x86_hyper_features |= ((eax & 0x3f) << 9);
+	printk(KERN_INFO "Detected HyperV with features: %x\n",
+		c->x86_hyper_features);
+}
Index: linux-2.6.34-rc4/arch/x86/kernel/cpu/hypervisor.c
===================================================================
--- linux-2.6.34-rc4.orig/arch/x86/kernel/cpu/hypervisor.c	2010-04-12 19:41:35.000000000 -0600
+++ linux-2.6.34-rc4/arch/x86/kernel/cpu/hypervisor.c	2010-04-15 10:00:18.000000000 -0600
@@ -23,6 +23,7 @@
 
 #include <asm/processor.h>
 #include <asm/vmware.h>
+#include <asm/hyperv.h>
 #include <asm/hypervisor.h>
 
 static inline void __cpuinit
@@ -30,6 +31,8 @@ detect_hypervisor_vendor(struct cpuinfo_
 {
 	if (vmware_platform())
 		c->x86_hyper_vendor = X86_HYPER_VENDOR_VMWARE;
+	else if (hyperv_platform())
+		c->x86_hyper_vendor = X86_HYPER_VENDOR_MSFT;
 	else
 		c->x86_hyper_vendor = X86_HYPER_VENDOR_NONE;
 }
@@ -37,10 +40,11 @@ detect_hypervisor_vendor(struct cpuinfo_
 static inline void __cpuinit
 hypervisor_set_feature_bits(struct cpuinfo_x86 *c)
 {
-	if (boot_cpu_data.x86_hyper_vendor == X86_HYPER_VENDOR_VMWARE) {
+	if (boot_cpu_data.x86_hyper_vendor == X86_HYPER_VENDOR_VMWARE)
 		vmware_set_feature_bits(c);
-		return;
-	}
+	else if (boot_cpu_data.x86_hyper_vendor == X86_HYPER_VENDOR_MSFT)
+		hyperv_set_feature_bits(c);
+	return;
 }
 
 void __cpuinit init_hypervisor(struct cpuinfo_x86 *c)
Index: linux-2.6.34-rc4/arch/x86/include/asm/hyperv.h
===================================================================
--- linux-2.6.34-rc4.orig/arch/x86/include/asm/hyperv.h	2010-04-12 19:41:35.000000000 -0600
+++ linux-2.6.34-rc4/arch/x86/include/asm/hyperv.h	2010-04-15 13:06:58.000000000 -0600
@@ -2,6 +2,8 @@
 #define _ASM_X86_KVM_HYPERV_H
 
 #include <linux/types.h>
+#include <linux/init.h>
+#include <asm/processor.h>
 
 /*
  * The below CPUID leaves are present if VersionAndFeatures.HypervisorPresent
@@ -14,6 +16,9 @@
 #define HYPERV_CPUID_ENLIGHTMENT_INFO		0x40000004
 #define HYPERV_CPUID_IMPLEMENT_LIMITS		0x40000005
 
+#define HYPERV_HYPERVISOR_PRESENT_BIT		0x80000000
+#define HYPERV_CPUID_MIN			0x40000005
+
 /*
  * Feature identification. EAX indicates which features are available
  * to the partition based upon the current partition privileges.
@@ -129,6 +134,9 @@
 /* MSR used to provide vcpu index */
 #define HV_X64_MSR_VP_INDEX			0x40000002
 
+/* MSR used to read the per-partition time reference counter */
+#define HV_X64_MSR_TIME_REF_COUNT       0x40000020
+
 /* Define the virtual APIC registers */
 #define HV_X64_MSR_EOI				0x40000070
 #define HV_X64_MSR_ICR				0x40000071
@@ -183,4 +191,6 @@
 #define HV_STATUS_INVALID_HYPERCALL_INPUT	3
 #define HV_STATUS_INVALID_ALIGNMENT		4
 
+int hyperv_platform(void);
+void __cpuinit hyperv_set_feature_bits(struct cpuinfo_x86 *c);
 #endif

[-- Attachment #3: hyperv_clocksource.patch --]
[-- Type: text/plain, Size: 4366 bytes --]

From: K. Y. Srinivasan <ksrinivasan@novell.com>
Subject:  A clocksource for Linux guests hosted on HyperV.
References: None
Patch-mainline: 

This patch is a clocksource implementation suitable for guests hosted on HyperV.
Time keeping in Linux guests hosted on HyperV is unstable. This clocksource 
driver fixes the problem. 

Signed-off-by: K. Y. Srinivasan <ksrinivasan@novell.com>

Index: linux-2.6.34-rc4/drivers/staging/hv/hv_timesource.c
===================================================================
--- /dev/null	1970-01-01 00:00:00.000000000 +0000
+++ linux-2.6.34-rc4/drivers/staging/hv/hv_timesource.c	2010-04-15 13:45:46.000000000 -0600
@@ -0,0 +1,97 @@
+/*
+ * A clocksource for Linux running on HyperV.
+ *
+ *
+ * Copyright (C) 2010, Novell, Inc.
+ * Author : K. Y. Srinivasan <ksrinivasan@novell.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful, but
+ * WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY OR FITNESS FOR A PARTICULAR PURPOSE, GOOD TITLE or
+ * NON INFRINGEMENT.  See the GNU General Public License for more
+ * details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
+ *
+ */
+
+#include <linux/version.h>
+#include <linux/clocksource.h>
+#include <linux/init.h>
+#include <linux/module.h>
+#include <linux/pci.h>
+#include <linux/dmi.h>
+#include <asm/hyperv.h>
+
+#define HV_CLOCK_SHIFT	22
+
+static cycle_t read_hv_clock(struct clocksource *arg)
+{
+	cycle_t current_tick;
+	/*
+	 * Read the partition counter to get the current tick count. This count
+	 * is set to 0 when the partition is created and is incremented in
+	 * 100 nanosecond units.
+	 */
+	rdmsrl(HV_X64_MSR_TIME_REF_COUNT, current_tick);
+	return current_tick;
+}
+
+static struct clocksource hyperv_cs = {
+	.name           = "hyperv_clocksource",
+	.rating         = 400, /* use this when running on Hyperv*/
+	.read           = read_hv_clock,
+	.mask           = CLOCKSOURCE_MASK(64),
+	/*
+	 * The time ref counter in HyperV is in 100ns units.
+	 * The definition of mult is:
+	 * mult/2^shift = ns/cyc = 100
+	 * mult = (100 << shift)
+	 */
+	.mult           = (100 << HV_CLOCK_SHIFT),
+	.shift          = HV_CLOCK_SHIFT,
+};
+
+static const struct dmi_system_id __initconst
+hv_timesource_dmi_table[] __maybe_unused  = {
+	{
+		.ident = "Hyper-V",
+		.matches = {
+			DMI_MATCH(DMI_SYS_VENDOR, "Microsoft Corporation"),
+			DMI_MATCH(DMI_PRODUCT_NAME, "Virtual Machine"),
+			DMI_MATCH(DMI_BOARD_NAME, "Virtual Machine"),
+		},
+	},
+	{ },
+};
+MODULE_DEVICE_TABLE(dmi, hv_timesource_dmi_table);
+
+static const struct pci_device_id __initconst
+hv_timesource_pci_table[] __maybe_unused = {
+	{ PCI_DEVICE(0x1414, 0x5353) }, /* VGA compatible controller */
+	{ 0 }
+};
+MODULE_DEVICE_TABLE(pci, hv_timesource_pci_table);
+
+
+static int __init init_hv_clocksource(void)
+{
+	if ((boot_cpu_data.x86_hyper_vendor != X86_HYPER_VENDOR_MSFT) ||
+		!(boot_cpu_data.x86_hyper_features &
+		HV_X64_MSR_TIME_REF_COUNT_AVAILABLE))
+		return -ENODEV;
+	printk(KERN_INFO "Registering HyperV clock source\n");
+	return clocksource_register(&hyperv_cs);
+}
+
+module_init(init_hv_clocksource);
+MODULE_DESCRIPTION("HyperV based clocksource");
+MODULE_AUTHOR("K. Y. Srinivasan <ksrinivasan@novell.com>");
+MODULE_LICENSE("GPL");
Index: linux-2.6.34-rc4/drivers/staging/hv/Makefile
===================================================================
--- linux-2.6.34-rc4.orig/drivers/staging/hv/Makefile	2010-04-12 19:41:35.000000000 -0600
+++ linux-2.6.34-rc4/drivers/staging/hv/Makefile	2010-04-15 13:09:07.000000000 -0600
@@ -1,4 +1,4 @@
-obj-$(CONFIG_HYPERV)		+= hv_vmbus.o
+obj-$(CONFIG_HYPERV)		+= hv_vmbus.o hv_timesource.o
 obj-$(CONFIG_HYPERV_STORAGE)	+= hv_storvsc.o
 obj-$(CONFIG_HYPERV_BLOCK)	+= hv_blkvsc.o
 obj-$(CONFIG_HYPERV_NET)	+= hv_netvsc.o

[-- Attachment #4: Type: text/plain, Size: 184 bytes --]

_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linux-foundation.org/mailman/listinfo/virtualization

Re: A clocksource driver for HyperV

[Greg KH]

On Thu, Apr 15, 2010 at 02:04:53PM -0600, Ky Srinivasan wrote:
> Jeremy,
> 
> As suggested by you, I am enclosing a patch that integrates HyperV detection with the  current hypervisor detection mechanism in the kernel. Also attached is a HyperV clocksource patch that uses this new functionality.
> 
> Signed-off-by: K. Y. Srinivasan <ksrinivasan@novell.com>

What's the status of this patch?  I think it's needed for the
clocksource HV patch, right?  Who's tree is it going to go through?

I'll be glad to take it in the staging tree if I can get some ACKs.

thanks,

greg k-h

Re: A clocksource driver for HyperV

[Greg KH]

On Mon, Apr 05, 2010 at 02:36:59PM -0700, Jeremy Fitzhardinge wrote:
> >+static struct dmi_system_id __initconst
> >+hv_timesource_dmi_table[] __maybe_unused  = {
> >+	{
> >+		.ident = "Hyper-V",
> >+		.matches = {
> >+			DMI_MATCH(DMI_SYS_VENDOR, "Microsoft Corporation"),
> >+			DMI_MATCH(DMI_PRODUCT_NAME, "Virtual Machine"),
> >+			DMI_MATCH(DMI_BOARD_NAME, "Virtual Machine"),
> >+		},
> >+	},
> >+	{ },
> >+};
> >+MODULE_DEVICE_TABLE(dmi, hv_timesource_dmi_table);
> 
> So you use the DMI signatures to determine whether the module is
> needed, but cpuid to work out if the feature is present?

The DMI and PCI stuff is to handle module autoloading.  Ideally we would
just use the DMI matching, but some distro installers can't handle that,
so we also add the PCI stuff.  The hyperv core already has this in it.

> >+
> >+static struct pci_device_id __initconst
> >+hv_timesource_pci_table[] __maybe_unused = {
> >+	{ PCI_DEVICE(0x1414, 0x5353) }, /* VGA compatible controller */
> >+	{ 0 }
> >+};
> >+MODULE_DEVICE_TABLE(pci, hv_timesource_pci_table);
> 
> And/or PCI?
> 
> Seems a bit... ad-hoc?  Is this the official way to determine the
> presence of Hyper-V?

Yeah, it is :(

thanks,

greg k-h

Re: A clocksource driver for HyperV

[Jeremy Fitzhardinge]

On 04/05/2010 01:30 PM, Ky Srinivasan wrote:
> +static cycle_t read_hv_clock(struct clocksource *arg)
> +{
> +	cycle_t current_tick;
> +	/*
> +	 * Read the partition counter to get the current tick count. This count
> +	 * is set to 0 when the partition is created and is incremented in
> +	 * 100 nanosecond units.
> +	 */
> +	rdmsrl(HV_X64_MSR_TIME_REF_COUNT, current_tick);
> +	return current_tick;
> +}
> +
> +static struct clocksource clocksource_hyperv = {
> +	.name           = "hyperv_clocksource",
>    

Seems like a redundantly long name; any use of this string is going to 
be in a context where it is obviously a clocksource.  How about just 
"hyperv"?

> +	.rating         = 400, /* use this when running on Hyperv*/
> +	.read           = read_hv_clock,
> +	.mask           = CLOCKSOURCE_MASK(64),
> +	.shift          = HV_CLOCK_SHIFT,
> +};
> +
> +static struct dmi_system_id __initconst
> +hv_timesource_dmi_table[] __maybe_unused  = {
> +	{
> +		.ident = "Hyper-V",
> +		.matches = {
> +			DMI_MATCH(DMI_SYS_VENDOR, "Microsoft Corporation"),
> +			DMI_MATCH(DMI_PRODUCT_NAME, "Virtual Machine"),
> +			DMI_MATCH(DMI_BOARD_NAME, "Virtual Machine"),
> +		},
> +	},
> +	{ },
> +};
> +MODULE_DEVICE_TABLE(dmi, hv_timesource_dmi_table);
>    

So you use the DMI signatures to determine whether the module is needed, 
but cpuid to work out if the feature is present?

> +
> +static struct pci_device_id __initconst
> +hv_timesource_pci_table[] __maybe_unused = {
> +	{ PCI_DEVICE(0x1414, 0x5353) }, /* VGA compatible controller */
> +	{ 0 }
> +};
> +MODULE_DEVICE_TABLE(pci, hv_timesource_pci_table);
>    

And/or PCI?

Seems a bit... ad-hoc?  Is this the official way to determine the 
presence of Hyper-V?

> +
> +
> +static int __init hv_detect_hyperv(void)
>    

This looks generally useful.  Should it be hidden away in the 
clocksource driver, or in some common hyper-v code?  Do other hyper-v 
drivers have versions of this?

> +{
> +	u32 eax, ebx, ecx, edx;
> +	static char hyp_signature[20];
>    
20?  static?

> +
> +	cpuid(1,&eax,&ebx,&ecx,&edx);
> +	if (!(ecx&  HV_HYPERVISOR_PRESENT_BIT)) {
> +		printk(KERN_WARNING
> +			"Not on a Hypervisor\n");
>    
This just looks like noise, especially since it doesn't identify what is 
generating the message.  And if you compile this code in as =y 
(non-modular) then it will complain every boot.

> +		return 1;
> +	}
> +	cpuid(HV_CPUID_SIGNATURE,&eax,&ebx,&ecx,&edx);
> +	*(u32 *)(hyp_signature + 0) = ebx;
> +	*(u32 *)(hyp_signature + 4) = ecx;
> +	*(u32 *)(hyp_signature + 8) = edx;
> +	hyp_signature[12] = 0;
> +
> +	if ((eax<  HV_CPUID_MIN) || (strcmp("Microsoft Hv", hyp_signature))) {
>    

memcmp, surely?

> +		printk(KERN_WARNING
> +			"Not on HyperV; signature %s, eax %x\n",
> +			hyp_signature, eax);
> +		return 1;
> +	}
> +	/*
> +	 * Extract the features, recommendations etc.
> +	 */
> +	cpuid(HV_CPUID_FEATURES,&eax,&ebx,&ecx,&edx);
> +	if (!(eax&  0x10)) {
> +		printk(KERN_WARNING "HyperV Time Ref Counter not available!\n");
> +		return 1;
> +	}
> +
> +	cpuid(HV_CPUID_RECOMMENDATIONS,&eax,&ebx,&ecx,&edx);
> +	printk(KERN_INFO "HyperV recommendations: %x\n", eax);
> +	printk(KERN_INFO "HyperV spin count: %x\n", ebx);
> +	return 0;
> +}
> +
> +
> +static int __init init_hv_clocksource(void)
> +{
> +	if (hv_detect_hyperv())
> +		return -ENODEV;
> +	/*
> +	 * The time ref counter in HyperV is in 100ns units.
> +	 * The definition of mult is:
> +	 * mult/2^shift = ns/cyc = 100
> +	 * mult = (100<<  shift)
> +	 */
> +	clocksource_hyperv.mult = (100<<  HV_CLOCK_SHIFT);
>    

Why not initialize this in the structure?  It's just 100<<22 isn't it?

> +	printk(KERN_INFO "Registering HyperV clock source\n");
> +	return clocksource_register(&clocksource_hyperv);
> +}
> +
> +module_init(init_hv_clocksource);
> +MODULE_DESCRIPTION("HyperV based clocksource");
> +MODULE_AUTHOR("K. Y. Srinivasan<ksrinivasan@novell.com>");
> +MODULE_LICENSE("GPL");
> Index: linux/drivers/staging/hv/Makefile
> ===================================================================
> --- linux.orig/drivers/staging/hv/Makefile	2010-04-05 13:02:06.000000000 -0600
> +++ linux/drivers/staging/hv/Makefile	2010-04-05 13:02:13.000000000 -0600
> @@ -1,4 +1,4 @@
> -obj-$(CONFIG_HYPERV)		+= hv_vmbus.o
> +obj-$(CONFIG_HYPERV)		+= hv_vmbus.o hv_timesource.o
>   obj-$(CONFIG_HYPERV_STORAGE)	+= hv_storvsc.o
>   obj-$(CONFIG_HYPERV_BLOCK)	+= hv_blkvsc.o
>   obj-$(CONFIG_HYPERV_NET)	+= hv_netvsc.o
>    

A clocksource driver for HyperV

[Ky Srinivasan]

[-- Attachment #1: Type: text/plain, Size: 115 bytes --]

I am attaching a clocksource driver for HyperV.

Signed-off-by: K. Y. Srinivasan <ksrinivasan@novell.com>




[-- Attachment #2: hyperv_clocksource.patch --]
[-- Type: text/plain, Size: 5617 bytes --]

From: K. Y. Srinivasan <ksrinivasan@novell.com>
Subject:  A clocksource for Linux guests hosted on HyperV.
References: None
Patch-mainline: 

This patch is a clocksource implementation suitable for guests hosted on HyperV.
Time keeping in Linux guests hosted on HyperV is unstable. This clocksource 
driver fixes the problem. 

Signed-off-by: K. Y. Srinivasan <ksrinivasan@novell.com>

Index: linux/drivers/staging/hv/hv_timesource.c
===================================================================
--- /dev/null	1970-01-01 00:00:00.000000000 +0000
+++ linux/drivers/staging/hv/hv_timesource.c	2010-04-05 13:33:36.000000000 -0600
@@ -0,0 +1,148 @@
+/*
+ * A clocksource for Linux running on HyperV.
+ *
+ *
+ * Copyright (C) 2010, Novell, Inc.
+ * Author : K. Y. Srinivasan <ksrinivasan@novell.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful, but
+ * WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY OR FITNESS FOR A PARTICULAR PURPOSE, GOOD TITLE or
+ * NON INFRINGEMENT.  See the GNU General Public License for more
+ * details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
+ *
+ */
+
+#include <linux/version.h>
+#include <linux/clocksource.h>
+#include <linux/init.h>
+#include <linux/module.h>
+#include <linux/pci.h>
+#include <linux/dmi.h>
+
+#define HV_CLOCK_SHIFT	22
+/*
+ * HyperV defined synthetic CPUID leaves:
+ */
+#define HV_CPUID_SIGNATURE	0x40000000
+#define HV_CPUID_MIN		0x40000005
+#define HV_HYPERVISOR_PRESENT_BIT	0x80000000
+#define HV_CPUID_FEATURES	0x40000003
+#define HV_CPUID_RECOMMENDATIONS	0x40000004
+
+/*
+ * HyperV defined synthetic MSRs
+ */
+
+#define HV_X64_MSR_TIME_REF_COUNT	0x40000020
+
+
+static cycle_t read_hv_clock(struct clocksource *arg)
+{
+	cycle_t current_tick;
+	/*
+	 * Read the partition counter to get the current tick count. This count
+	 * is set to 0 when the partition is created and is incremented in
+	 * 100 nanosecond units.
+	 */
+	rdmsrl(HV_X64_MSR_TIME_REF_COUNT, current_tick);
+	return current_tick;
+}
+
+static struct clocksource clocksource_hyperv = {
+	.name           = "hyperv_clocksource",
+	.rating         = 400, /* use this when running on Hyperv*/
+	.read           = read_hv_clock,
+	.mask           = CLOCKSOURCE_MASK(64),
+	.shift          = HV_CLOCK_SHIFT,
+};
+
+static struct dmi_system_id __initconst
+hv_timesource_dmi_table[] __maybe_unused  = {
+	{
+		.ident = "Hyper-V",
+		.matches = {
+			DMI_MATCH(DMI_SYS_VENDOR, "Microsoft Corporation"),
+			DMI_MATCH(DMI_PRODUCT_NAME, "Virtual Machine"),
+			DMI_MATCH(DMI_BOARD_NAME, "Virtual Machine"),
+		},
+	},
+	{ },
+};
+MODULE_DEVICE_TABLE(dmi, hv_timesource_dmi_table);
+
+static struct pci_device_id __initconst
+hv_timesource_pci_table[] __maybe_unused = {
+	{ PCI_DEVICE(0x1414, 0x5353) }, /* VGA compatible controller */
+	{ 0 }
+};
+MODULE_DEVICE_TABLE(pci, hv_timesource_pci_table);
+
+
+static int __init hv_detect_hyperv(void)
+{
+	u32 eax, ebx, ecx, edx;
+	static char hyp_signature[20];
+
+	cpuid(1, &eax, &ebx, &ecx, &edx);
+	if (!(ecx & HV_HYPERVISOR_PRESENT_BIT)) {
+		printk(KERN_WARNING
+			"Not on a Hypervisor\n");
+		return 1;
+	}
+	cpuid(HV_CPUID_SIGNATURE, &eax, &ebx, &ecx, &edx);
+	*(u32 *)(hyp_signature + 0) = ebx;
+	*(u32 *)(hyp_signature + 4) = ecx;
+	*(u32 *)(hyp_signature + 8) = edx;
+	hyp_signature[12] = 0;
+
+	if ((eax < HV_CPUID_MIN) || (strcmp("Microsoft Hv", hyp_signature))) {
+		printk(KERN_WARNING
+			"Not on HyperV; signature %s, eax %x\n",
+			hyp_signature, eax);
+		return 1;
+	}
+	/*
+	 * Extract the features, recommendations etc.
+	 */
+	cpuid(HV_CPUID_FEATURES, &eax, &ebx, &ecx, &edx);
+	if (!(eax & 0x10)) {
+		printk(KERN_WARNING "HyperV Time Ref Counter not available!\n");
+		return 1;
+	}
+
+	cpuid(HV_CPUID_RECOMMENDATIONS, &eax, &ebx, &ecx, &edx);
+	printk(KERN_INFO "HyperV recommendations: %x\n", eax);
+	printk(KERN_INFO "HyperV spin count: %x\n", ebx);
+	return 0;
+}
+
+
+static int __init init_hv_clocksource(void)
+{
+	if (hv_detect_hyperv())
+		return -ENODEV;
+	/*
+	 * The time ref counter in HyperV is in 100ns units.
+	 * The definition of mult is:
+	 * mult/2^shift = ns/cyc = 100
+	 * mult = (100 << shift)
+	 */
+	clocksource_hyperv.mult = (100 << HV_CLOCK_SHIFT);
+	printk(KERN_INFO "Registering HyperV clock source\n");
+	return clocksource_register(&clocksource_hyperv);
+}
+
+module_init(init_hv_clocksource);
+MODULE_DESCRIPTION("HyperV based clocksource");
+MODULE_AUTHOR("K. Y. Srinivasan <ksrinivasan@novell.com>");
+MODULE_LICENSE("GPL");
Index: linux/drivers/staging/hv/Makefile
===================================================================
--- linux.orig/drivers/staging/hv/Makefile	2010-04-05 13:02:06.000000000 -0600
+++ linux/drivers/staging/hv/Makefile	2010-04-05 13:02:13.000000000 -0600
@@ -1,4 +1,4 @@
-obj-$(CONFIG_HYPERV)		+= hv_vmbus.o
+obj-$(CONFIG_HYPERV)		+= hv_vmbus.o hv_timesource.o
 obj-$(CONFIG_HYPERV_STORAGE)	+= hv_storvsc.o
 obj-$(CONFIG_HYPERV_BLOCK)	+= hv_blkvsc.o
 obj-$(CONFIG_HYPERV_NET)	+= hv_netvsc.o

[-- Attachment #3: Type: text/plain, Size: 184 bytes --]

_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linux-foundation.org/mailman/listinfo/virtualization

A clocksource driver for HyperV

[Ky Srinivasan]

[-- Attachment #1: Type: text/plain, Size: 115 bytes --]

I am attaching a clocksource driver for HyperV.

Signed-off-by: K. Y. Srinivasan <ksrinivasan@novell.com>




[-- Attachment #2: hyperv_clocksource.patch --]
[-- Type: text/plain, Size: 5617 bytes --]

From: K. Y. Srinivasan <ksrinivasan@novell.com>
Subject:  A clocksource for Linux guests hosted on HyperV.
References: None
Patch-mainline: 

This patch is a clocksource implementation suitable for guests hosted on HyperV.
Time keeping in Linux guests hosted on HyperV is unstable. This clocksource 
driver fixes the problem. 

Signed-off-by: K. Y. Srinivasan <ksrinivasan@novell.com>

Index: linux/drivers/staging/hv/hv_timesource.c
===================================================================
--- /dev/null	1970-01-01 00:00:00.000000000 +0000
+++ linux/drivers/staging/hv/hv_timesource.c	2010-04-05 13:33:36.000000000 -0600
@@ -0,0 +1,148 @@
+/*
+ * A clocksource for Linux running on HyperV.
+ *
+ *
+ * Copyright (C) 2010, Novell, Inc.
+ * Author : K. Y. Srinivasan <ksrinivasan@novell.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful, but
+ * WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY OR FITNESS FOR A PARTICULAR PURPOSE, GOOD TITLE or
+ * NON INFRINGEMENT.  See the GNU General Public License for more
+ * details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
+ *
+ */
+
+#include <linux/version.h>
+#include <linux/clocksource.h>
+#include <linux/init.h>
+#include <linux/module.h>
+#include <linux/pci.h>
+#include <linux/dmi.h>
+
+#define HV_CLOCK_SHIFT	22
+/*
+ * HyperV defined synthetic CPUID leaves:
+ */
+#define HV_CPUID_SIGNATURE	0x40000000
+#define HV_CPUID_MIN		0x40000005
+#define HV_HYPERVISOR_PRESENT_BIT	0x80000000
+#define HV_CPUID_FEATURES	0x40000003
+#define HV_CPUID_RECOMMENDATIONS	0x40000004
+
+/*
+ * HyperV defined synthetic MSRs
+ */
+
+#define HV_X64_MSR_TIME_REF_COUNT	0x40000020
+
+
+static cycle_t read_hv_clock(struct clocksource *arg)
+{
+	cycle_t current_tick;
+	/*
+	 * Read the partition counter to get the current tick count. This count
+	 * is set to 0 when the partition is created and is incremented in
+	 * 100 nanosecond units.
+	 */
+	rdmsrl(HV_X64_MSR_TIME_REF_COUNT, current_tick);
+	return current_tick;
+}
+
+static struct clocksource clocksource_hyperv = {
+	.name           = "hyperv_clocksource",
+	.rating         = 400, /* use this when running on Hyperv*/
+	.read           = read_hv_clock,
+	.mask           = CLOCKSOURCE_MASK(64),
+	.shift          = HV_CLOCK_SHIFT,
+};
+
+static struct dmi_system_id __initconst
+hv_timesource_dmi_table[] __maybe_unused  = {
+	{
+		.ident = "Hyper-V",
+		.matches = {
+			DMI_MATCH(DMI_SYS_VENDOR, "Microsoft Corporation"),
+			DMI_MATCH(DMI_PRODUCT_NAME, "Virtual Machine"),
+			DMI_MATCH(DMI_BOARD_NAME, "Virtual Machine"),
+		},
+	},
+	{ },
+};
+MODULE_DEVICE_TABLE(dmi, hv_timesource_dmi_table);
+
+static struct pci_device_id __initconst
+hv_timesource_pci_table[] __maybe_unused = {
+	{ PCI_DEVICE(0x1414, 0x5353) }, /* VGA compatible controller */
+	{ 0 }
+};
+MODULE_DEVICE_TABLE(pci, hv_timesource_pci_table);
+
+
+static int __init hv_detect_hyperv(void)
+{
+	u32 eax, ebx, ecx, edx;
+	static char hyp_signature[20];
+
+	cpuid(1, &eax, &ebx, &ecx, &edx);
+	if (!(ecx & HV_HYPERVISOR_PRESENT_BIT)) {
+		printk(KERN_WARNING
+			"Not on a Hypervisor\n");
+		return 1;
+	}
+	cpuid(HV_CPUID_SIGNATURE, &eax, &ebx, &ecx, &edx);
+	*(u32 *)(hyp_signature + 0) = ebx;
+	*(u32 *)(hyp_signature + 4) = ecx;
+	*(u32 *)(hyp_signature + 8) = edx;
+	hyp_signature[12] = 0;
+
+	if ((eax < HV_CPUID_MIN) || (strcmp("Microsoft Hv", hyp_signature))) {
+		printk(KERN_WARNING
+			"Not on HyperV; signature %s, eax %x\n",
+			hyp_signature, eax);
+		return 1;
+	}
+	/*
+	 * Extract the features, recommendations etc.
+	 */
+	cpuid(HV_CPUID_FEATURES, &eax, &ebx, &ecx, &edx);
+	if (!(eax & 0x10)) {
+		printk(KERN_WARNING "HyperV Time Ref Counter not available!\n");
+		return 1;
+	}
+
+	cpuid(HV_CPUID_RECOMMENDATIONS, &eax, &ebx, &ecx, &edx);
+	printk(KERN_INFO "HyperV recommendations: %x\n", eax);
+	printk(KERN_INFO "HyperV spin count: %x\n", ebx);
+	return 0;
+}
+
+
+static int __init init_hv_clocksource(void)
+{
+	if (hv_detect_hyperv())
+		return -ENODEV;
+	/*
+	 * The time ref counter in HyperV is in 100ns units.
+	 * The definition of mult is:
+	 * mult/2^shift = ns/cyc = 100
+	 * mult = (100 << shift)
+	 */
+	clocksource_hyperv.mult = (100 << HV_CLOCK_SHIFT);
+	printk(KERN_INFO "Registering HyperV clock source\n");
+	return clocksource_register(&clocksource_hyperv);
+}
+
+module_init(init_hv_clocksource);
+MODULE_DESCRIPTION("HyperV based clocksource");
+MODULE_AUTHOR("K. Y. Srinivasan <ksrinivasan@novell.com>");
+MODULE_LICENSE("GPL");
Index: linux/drivers/staging/hv/Makefile
===================================================================
--- linux.orig/drivers/staging/hv/Makefile	2010-04-05 13:02:06.000000000 -0600
+++ linux/drivers/staging/hv/Makefile	2010-04-05 13:02:13.000000000 -0600
@@ -1,4 +1,4 @@
-obj-$(CONFIG_HYPERV)		+= hv_vmbus.o
+obj-$(CONFIG_HYPERV)		+= hv_vmbus.o hv_timesource.o
 obj-$(CONFIG_HYPERV_STORAGE)	+= hv_storvsc.o
 obj-$(CONFIG_HYPERV_BLOCK)	+= hv_blkvsc.o
 obj-$(CONFIG_HYPERV_NET)	+= hv_netvsc.o

[-- Attachment #3: Type: text/plain, Size: 184 bytes --]

_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linux-foundation.org/mailman/listinfo/virtualization