From mboxrd@z Thu Jan 1 00:00:00 1970 From: "Dan Magenheimer" Subject: RE: [PATCH] softtsc (was RE: Guest TSC and Xen (Intel and AMD feedback please)) Date: Thu, 10 Jul 2008 08:19:31 -0600 Message-ID: <20080710081931031.00000003744@djm-pc> References: Reply-To: "dan.magenheimer@oracle.com" Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Transfer-Encoding: quoted-printable Return-path: In-Reply-To: List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xensource.com Errors-To: xen-devel-bounces@lists.xensource.com To: Keir Fraser , "Zhang, Li" , "Xen-Devel (E-mail)" Cc: "Tian, Kevin" List-Id: xen-devel@lists.xenproject.org > Yes, I will take it, but have the following comments. > 2. Your change in common/keyhandler.c breaks ia64. :-) Oops! ;-) What's the best way to handle this? It would be unfortunate to lose valuable debug data just because its arch-dependent but I don't see any other arch-dependent code in keyhandler.c and I'll bet you don't want to start adding ifdef's nor introduce arch/xxx/keyhandler.c just for this. > 1. Why do you define new boolean 'constant_tsc'? Can you just use > test_bit(X86_FEATURE_CONSTANT_TSC)? > 3. Your change to arch/x86/time.c looks unnecessary. I was thinking that the tests for these features should probably be abstracted (e.g. static inline in a header file or a global function), but wasn't sure about the best way to deal with the datatypes (e.g. struct cpuinfo_x86) so defaulted to global variables. Both globals are simply for debug output in keyhandler.c so depending on the answer to (2) above, those patch-parts could just go away. > 4. Should you catch SVM's RDTSCP vmexit as well as RDTSC? I thought I remembered seeing code that reported/lied to guests that the rdtscp feature was not present? Thanks, Dan