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

[Eric Shelton <eshelton@xxxxxxxxx>]

On 15.09.14 at 08:42, Jan Beulich wrote:

>>>> On 12.09.14 at 19:46, <dslutz@verizon.com> wrote:

>> On 09/12/14 05:49, Jan Beulich wrote:
>>>>>> On 11.09.14 at 21:49, <andrew.cooper3@xxxxxxxxxx> 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.

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

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel