Re: [PATCH] softtsc (was RE: [Xen-devel] Guest TSC and Xen (Intel and AMD feedback please))

[Keir Fraser <keir.fraser@xxxxxxxxxxxxx>]

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



_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel