Re: A clocksource driver for HyperV

[Jeremy Fitzhardinge <jeremy@goop.org>]

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