lkml.org 
[lkml]   [2008]   [Sep]   [26]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    SubjectRe: Use CPUID to communicate with the hypervisor.
    From
    Date
    Hi Peter,

    Thanks for the comments, please find my replies below.

    On Fri, 2008-09-26 at 17:09 -0700, H. Peter Anvin wrote:
    > Alok Kataria wrote:

    > >
    > > Leaf 0x40000000, Hypervisor CPUID information
    > > # EAX: The maximum input value for hypervisor CPUID info (0x40000010).
    > > # EBX, ECX, EDX: Hypervisor vendor ID signature. E.g. "VMwareVMware"
    > >
    >
    > This is great, obviously... although we'll have to deal with legacy
    > methods for a while if not indefinitely (just as we have to for
    > pre-CPUID processors).

    Ok, do you think we should keep those (legacy) interfaces separate so
    that they can be phased out whenever the time is right.

    >
    > >
    > > +static void __init detect_hypervisor(void)
    > > +{
    > > + if (cpu_has_hypervisor) {
    > > + unsigned int eax, ebx, ecx, edx;
    > > + char hyper_vendor_id[13];
    > > +
    > > + cpuid(HYPERVISOR_INFO_LEAF, &eax, &ebx, &ecx, &edx);
    > > + memcpy(hyper_vendor_id + 0, &ebx, 4);
    > > + memcpy(hyper_vendor_id + 4, &ecx, 4);
    > > + memcpy(hyper_vendor_id + 8, &edx, 4);
    > > + hyper_vendor_id[12] = '\0';
    > > + printk(KERN_INFO "Hypervisor vendor id %s\n", hyper_vendor_id);
    > > + }
    > > +}
    > > +
    >
    > This should be broken out into a separate file in cpu/*, because we
    > *will* need to detect hypervisors by other means.
    Ok, i will do that.

    > > --- a/arch/x86/kernel/tsc.c
    > > +++ b/arch/x86/kernel/tsc.c
    > > @@ -345,16 +345,38 @@ failed:
    > > return 0;
    > > }
    > >
    > > +unsigned long hypervisor_tsc_freq(void)
    > > +{
    > > + unsigned long tsc_khz;
    > > + unsigned int max_cpuid_leaf;
    > > +
    > > + if (cpu_has_hypervisor) {
    > > + max_cpuid_leaf = cpuid_eax(HYPERVISOR_INFO_LEAF);
    > > + if (max_cpuid_leaf >= HYPERVISOR_TIMING_LEAF) {
    > > + tsc_khz = cpuid_eax(HYPERVISOR_TIMING_LEAF);
    > > + printk(KERN_INFO
    > > + "TSC frequency read from hypervisor\n");
    > > + return tsc_khz;
    > > + }
    > > + }
    > > + return 0;
    > > +}
    > > +
    >
    > I would call this "vmware_tsc_freq()" because it is a VMWare-defined
    > interface... you can't just poke at 0x40000010 and assume it is using
    > the VMWare definition.

    I would like to see this as a generic hypervisor way to get frequency
    rather than a VMware specific thingy.
    >
    > In order for *that* to be safe, you'd have to have well-defined ranges
    > for different virtualization vendors where each of them can define their
    > own stuff.

    My motivation for doing this is to have a standard across all the
    hypervisor's. If all the different hypervisor guys can come to some
    sought of consensus on the various hypervisor leafs that would help keep
    this simple and a lot more maintainable.

    >
    > > diff --git a/include/asm-x86/processor.h b/include/asm-x86/processor.h
    > > index ee7cbb3..70ca49b 100644
    > > --- a/include/asm-x86/processor.h
    > > +++ b/include/asm-x86/processor.h
    > > @@ -124,6 +124,27 @@ struct cpuinfo_x86 {
    > > #define X86_VENDOR_UNKNOWN 0xff
    > >
    > > /*
    > > + * Intel & AMD have reserved the cpuid levels 0x40000000 - 0x400000FF for
    > > + * software use. Hypervisors can use these levels to provide an interface
    > > + * to pass information from the hypervisor to the guest. This is similar
    > > + * to how we extract information about a physical cpu by using cpuid.
    > > + */
    > > +
    > > +/*
    > > + * This CPUID leaf returns the information about the hypervisor.
    > > + * EAX : maximum input value for CPUID supported by the hypervisor.
    > > + * EBX, ECX, EDX : Hypervisor vendor ID signature. E.g. VMwareVMware.
    > > + */
    > > +#define HYPERVISOR_INFO_LEAF 0x40000000
    > > +/*
    > > + * This leaf gets timing information from the hypervisor.
    > > + * EAX: (Virtual) TSC frequency in kHz.
    > > + * EBX: (Virtual) Bus (local apic timer) frequency in kHz.
    > > + * ECX, EDX: RESERVED
    > > + */
    > > +#define HYPERVISOR_TIMING_LEAF 0x40000010
    > > +
    >
    > <asm/processor.h> is the wrong place for this, and these constants
    > should have CPUID_ in them to tell what they fundamentally are.
    >
    > My preference would be for <asm/cpuid.h>, but otherwise
    > <asm/cpufeature.h> wouldn't be entirely wrong.

    Ok makes sense, will do that.

    Thanks,
    Alok
    >
    > -hpa



    \
     
     \ /
      Last update: 2008-09-27 02:33    [W:0.031 / U:31.500 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site