Re: [PATCH] x86: detect running on a HyperV system

[H. Peter Anvin]

On 05/06/2010 12:18 PM, Greg KH wrote:
> On Thu, May 06, 2010 at 12:13:17PM -0700, H. Peter Anvin wrote:
>> On 05/06/2010 12:08 PM, Greg KH wrote:
>>>
>>> x86-maintainers, we need this patch in the tree to get a -staging driver
>>> to work properly.  If you don't object, can I take it through the
>>> staging tree?  Or do you want to take it in -tip?
>>>
>>
>> I'd prefer to take it in -tip, partly because I would like to rework a
>> few things -- including preexisting braindamage like c->x86_hyper_vendor
>> (since when was hypervisor vendor a per-cpu property!?)
> 
> Heh, good point, but where should it live?
> 
> And I have no objection to taking it in -tip, thanks for doing so.
> 
>> I'll try to get to it today, if not feel free to bug me.
> 
> Will do :)
> 
> thanks again,
> 

Okay... we write something called x86_hyper_features, but it isn't
actually used for anything.  I'm assuming based on what I can see in
<asm/hyperv.h> that it is almost certainly not anything CPU-based.  I
really don't like a random field that can be stuffed with information
from random locations, and it doesn't look like it's even a flag field
in any meaningful way.  Worse, the field is just used to mix bits from
two different CPUID levels at what appears to be a completely arbitrary
boundary:

        /*
         * 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);

Why 9?  Why 6?  The only thing that I can think of that justifies those
particular numbers is what the *current* version of Hyper-V does.

It would be better to just have a global variable where these two levels
are separately stuffed away instead of arbitrarily commingled (again,
I'm assuming none of the bits are actually per CPU.)

What does this driver of yours do with them?

	-hpa
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/