From mboxrd@z Thu Jan  1 00:00:00 1970
From: Keir Fraser <keir.fraser@eu.citrix.com>
Subject: Re: [PATCH] softtsc (was RE: Guest TSC and Xen (Intel
	and AMD feedback please))
Date: Thu, 10 Jul 2008 15:29:56 +0100
Message-ID: <C49BDB74.23E0B%keir.fraser@eu.citrix.com>
References: <20080710081931031.00000003744@djm-pc>
Mime-Version: 1.0
Content-Type: text/plain;
	charset="US-ASCII"
Content-Transfer-Encoding: 7bit
Return-path: <xen-devel-bounces@lists.xensource.com>
In-Reply-To: <20080710081931031.00000003744@djm-pc>
List-Unsubscribe: <http://lists.xensource.com/mailman/listinfo/xen-devel>,
	<mailto:xen-devel-request@lists.xensource.com?subject=unsubscribe>
List-Post: <mailto:xen-devel@lists.xensource.com>
List-Help: <mailto:xen-devel-request@lists.xensource.com?subject=help>
List-Subscribe: <http://lists.xensource.com/mailman/listinfo/xen-devel>,
	<mailto:xen-devel-request@lists.xensource.com?subject=subscribe>
Sender: xen-devel-bounces@lists.xensource.com
Errors-To: xen-devel-bounces@lists.xensource.com
To: "dan.magenheimer@oracle.com" <dan.magenheimer@oracle.com>, "Zhang, Li" <li.zhang@intel.com>, "Xen-Devel (E-mail)" <xen-devel@lists.xensource.com>
Cc: "Tian, Kevin" <kevin.tian@intel.com>
List-Id: xen-devel@lists.xenproject.org

Hmm.. okay I think I will throw away the debug-output pieces of the patch.
They are logically separate and distinct anyway.

 -- Keir

On 10/7/08 15:19, "Dan Magenheimer" <dan.magenheimer@oracle.com> wrote:

>> 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
>