From mboxrd@z Thu Jan 1 00:00:00 1970 From: Keir Fraser Subject: Re: [PATCH] x86: don't write_tsc() non-zero values on CPUs updating only the lower 32 bits Date: Thu, 14 Apr 2011 17:48:26 +0100 Message-ID: References: <4DA73CBD020000780003BBB0@vpn.id2.novell.com> Mime-Version: 1.0 Content-Type: text/plain; charset="US-ASCII" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <4DA73CBD020000780003BBB0@vpn.id2.novell.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xensource.com Errors-To: xen-devel-bounces@lists.xensource.com To: Jan Beulich Cc: "xen-devel@lists.xensource.com" , "winston.l.wang" List-Id: xen-devel@lists.xenproject.org On 14/04/2011 17:28, "Jan Beulich" 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 >