[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [PATCH] x86: don't write_tsc() non-zero values on CPUs updating only the lower 32 bits
On 14/04/2011 19:33, "Wang, Winston L" <winston.l.wang@xxxxxxxxx> wrote: > Jan and Keir, > > Great efforts for turning the test code re-validate the processor with > (X86_FEATURE_TSC_RELIABLE) set. We know for sure that the broken old > processors any write TSC will Zero the upper 32 bit. So can we move this test > code as early as possible, say immediately after the early Processor init code > where checking CPU id and set X86_FEATURE_TSC_RELIABLE? For our meaning of TSC_RELIABLE, there's no need for its setting to rely on TSC 64-bit writability. I think leaving it where it is in init_xen_time() is probably fine. -- Keir > Thanks, > > Winston, > >> -----Original Message----- >> From: Keir Fraser [mailto:keir.xen@xxxxxxxxx] >> Sent: Thursday, April 14, 2011 9:48 AM >> To: Jan Beulich >> >> On 14/04/2011 17:28, "Jan Beulich" <JBeulich@xxxxxxxxxx> wrote: >> >>> >>>> I also simplified the actual writability check itself. I couldn't >> figure out >>>> what the benefit of your more complex approach would be. In fact it >> looked >>>> like it wouldn't work if bit 32 was set already in the TSC counter, >> as then >>>> you would write back an unmodified TSC (and in fact you would detect >> the >>>> wrong way round, as you'd see a big delta if the write silently >> cleared bit >>>> 32 (and bits 33-63)). And the final write of tsc+4*delta, wasn't >> sure what >>>> that was about either! But if you can explain why your test is >> better I'd be >>>> happy to use it as you originally wrote it. >>> >>> So you were concerned about getting the TSC slightly off, and now >>> you flush it to zero, without any attempt to restore the original >>> value? >> >> Haha, well it doesn't matter too much if we sync TSCs as we bring them >> online anyway. But I agree it makes sense to try if we are only able to >> write the lower 32 bits -- we can at least hope the write test happens >> while >> TSC counter is a 32-bit value anyway, and at least we've had a best- >> effort >> attempt to keep TSCs in sync. >> >>> As to my original test being broken - I don't think that was the case: >>> The first write used (u32)tsc as the input, so the two writes, if >>> happening completely, would be certain to be apart by >>> approximately 1<<32 (or more, depending on what was in the >>> upper half originally). >> >> Ah yes, I missed the importance of the (u32)tsc write. Fair enough, >> your way >> is better. :-) >> >>> The only case not handled was if the TSC >>> overflowed 64 bits during the process - I considered this case >>> hypothetical only. >>> >>> The final write of tsc+4*delta was basically an attempt to restore >>> the value that would have been there if we didn't fiddle with it. >> >> But the write is actually tsc + 4*(s32)(tmp-tsc), and tmp has 1U<<32 >> ORed >> into it (because it was read after your second write to the TSC. >> Perhaps we >> should just write back the full original tsc and call that good enough? >> >> -- Keir >> >> >>> The factor 4 was sort of invented, on the basis that the delta was >>> between one write and an immediate read back, with there being >>> a total of four such windows (read->write or write->read). As >>> one wouldn't get it precise anyway, the number seemed fine to >>> me, though just writing back the original values probably wouldn't >>> have been much worse. >> >>> Jan >>> >> > _______________________________________________ Xen-devel mailing list Xen-devel@xxxxxxxxxxxxxxxxxxx http://lists.xensource.com/xen-devel
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |