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

[Boris Ostrovsky <boris.ostrovsky@oracle.com>]

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.


>
>> --- a/xen/include/asm-x86/domain.h
>> +++ b/xen/include/asm-x86/domain.h
>> @@ -308,6 +308,8 @@ struct arch_domain
>>                                   (possibly other cases in the future */
>>       uint64_t vtsc_kerncount; /* for hvm, counts all vtsc */
>>       uint64_t vtsc_usercount; /* not used for hvm */
>> +    uint64_t chkpt_tsc;      /* TSC value that VCPUs use to calculate their
>> +                                tsc_offset value. Used during save/restore */
> I'm sorry for not asking earlier - why does this get put in struct
> arch_domain rather than struct hvm_domain?

Right. hvm_domain seems like a better place given that this is only 
relevant to HVM.


-boris