Re: [Xen-devel] [PATCH v3 1/1] x86/HVM: Use fixed TSC value when saving or restoring domain

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 10.04.14 at 16:33, <boris.ostrovsky@xxxxxxxxxx> wrote:
> On 04/10/2014 02:55 AM, Jan Beulich wrote:
>>
>>> --- a/xen/arch/x86/hvm/hvm.c
>>> +++ b/xen/arch/x86/hvm/hvm.c
>>> @@ -255,16 +255,20 @@ int hvm_set_guest_pat(struct vcpu *v, u64 guest_pat)
>>>       return 1;
>>>   }
>>>   
>>> -void hvm_set_guest_tsc(struct vcpu *v, u64 guest_tsc)
>>> +void hvm_set_guest_tsc_fixed(struct vcpu *v, u64 guest_tsc, u64 at_tsc)
>> What I now started wondering is (namely with the struct
>> hvm_function_table pointer also gaining the extra argument): Is
>> this ever being called with a zero at_tsc when
>> v->domain->arch.chkpt_tsc is non-zero? If not, rather than passing
>> around the value I guess the function could simply read it itself. And
>> yes, this is meant only for the "set" version, I'm relatively convinced
>> that the "get" ones would (or could easily become) different.
> 
> I couldn't convince myself that v->domain->arch.chkpt_tsc is always the 
> same as at_tsc. I think it is (and that's why arch_hvm_save_done() was 
> added) but I was worried that there may be some other path to 
> hvm_set_guest_tsc().
> 
> The other reason was for symmetry with the the "get" counterpart.

I think the symmetry should specifically not matter here: I can
imagine the need to "get" both values at the same time (if nothing
else, then for debugging purposes), but I can't see why you'd ever
want to "set" both ways at the same time.

Hence I'd like to ask that you drop the new argument, and add
ASSERT()s in places where you feel uncertain (if in doubt - at each
relevant call site).

Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel