From mboxrd@z Thu Jan 1 00:00:00 1970 From: Don Slutz Subject: Re: [PATCH v4 01/16] xen: Add support for VMware cpuid leaves Date: Mon, 22 Sep 2014 14:21:44 -0400 Message-ID: <542068B8.2020304@terremark.com> References: Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii"; Format="flowed" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Eric Shelton , Jan Beulich Cc: Kevin Tian , Keir Fraser , Ian Campbell , Stefano Stabellini , George Dunlap , Andrew Cooper , Tim Deegan , Don Slutz , "xen-devel@lists.xen.org" , Eddie Dong , Aravind Gopalakrishnan , Jun Nakajima , Suravee Suthikulpanit , Boris Ostrovsky , Ian Jackson List-Id: xen-devel@lists.xenproject.org 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, > wrote: > >> On 09/12/14 05:49, Jan Beulich wrote: > >>>>>> On 11.09.14 at 21:49, > 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 >