Re: [PATCH v4 01/16] xen: Add support for VMware cpuid leaves

[Don Slutz <dslutz@verizon.com>]

On 09/17/14 00:30, Eric Shelton wrote:
> On 15.09.14 at 08:42, Jan Beulich wrote:
> >>>> On 12.09.14 at 19:46, <dslutz@verizon.com 
> <mailto:dslutz@verizon.com>> wrote:
> >> On 09/12/14 05:49, Jan Beulich wrote:
> >>>>>> On 11.09.14 at 21:49, <andrew.cooper3@citrix.com 
> <mailto:andrew.cooper3@citrix.com>> wrote:
> >>>>> +    case 0x10:
> >>>>> +        /* (Virtual) TSC frequency in kHz. */
> >>>>> +        *eax =  d->arch.tsc_khz;
> >>>>> +        /* (Virtual) Bus (local apic timer) frequency in kHz. */
> >>>>> +        *ebx = 1000000000ull / APIC_BUS_CYCLE_NS / 1000ull;
> >>>> At least 1 pair of brackets please, especially as the placement of
> >>>> brackets affects the result of this particular calculation.
> >>> Or simply eliminate one of the divisions using
> >>> "1000000ull / APIC_BUS_CYCLE_NS".
> >>
> >> I am totally confused.  I am happy to go with Jan's version.
> >>
> >> The confusion is that I get the same answer all the ways I try.
> >
> > Hmm - this ...
> >
> >>        ebx1 = 1000000000ull / APIC_BUS_CYCLE_NS / 1000ull;
> >>        ebx2 = (1000000000ull / APIC_BUS_CYCLE_NS) / 1000ull;
> >>        ebx3 = 1000000000ull / (APIC_BUS_CYCLE_NS * 1000ull);
> >
> > ... clearly indicates the contrary: You converted to mutiplication
> >here, when the respective possibility of putting parentheses here
> >would have been
> >
> > ebx3 = 1000000000ull / (APIC_BUS_CYCLE_NS / 1000ull);
> >
> >which I'm sure you agree won't produce the same result. But yes,
> >the language implies parentheses this way
> >
> > ebx2 = (1000000000ull / APIC_BUS_CYCLE_NS) / 1000ull;
> >
> >so the original expression without them was correct, just
> >slightly ambiguous.
> >
> >Jan
>
> A different approach for the bus frequency, taken in the patch I just 
> posted, is to provide the actual bus frequency, rather than a 
> hardcoded value.  Specifically, the value bus_freq in apic.c.
>

This is not the correct bus frequency for the vlapic code.

I have a server where the host:
...
(XEN) ..... host bus clock speed is 200.0108 MHz.

But the guest sees:
...
..... host bus clock speed is 99.0999 MHz.

(do to a bug in Linux, this is not displayed correctly, the real value 
is 99.999 MHz
because "CONFIG_HZ=1000" not 100)

I also see:
..... host bus clock speed is 100.0000 MHz.


So reporting the 200.0108 to the guest would be wrong.

(And on a different server I have:

(XEN) ..... host bus clock speed is 100.0010 MHz.

for the host; which is closer but still not right).

    -Don Slutz

> Additionally, I propose Xen provides the 0x40000010 timing info leaf 
> for all domains, not just those operating under a VMware compatability 
> mode.  This is the approach taken in my proposed patch.  OS X will 
> happily use the 0x40000010 leaf even if the hypervisor vendor ID does 
> not indicate VMware.  Use of this leaf ended up being the only way I 
> managed to find to get OS X to configure its timers correctly on a 
> Haswell system.
>
> Eric
>