Re: [RFC PATCH v11 5/9] psci: Add hypercall service for ptp_kvm.

[Jianyong Wu <Jianyong.Wu@arm.com>]

On 2020/4/30 6:36 PM, Mark Rutland wrote:
> On Tue, Apr 28, 2020 at 07:14:52AM +0100, Jianyong Wu wrote:
>> On 2020/4/24 6:39 PM, Mark Rutland wrote:
>>> On Fri, Apr 24, 2020 at 03:50:22AM +0100, Jianyong Wu wrote:
>>>> On 2020/4/21 5:57 PM, Mark Rutland wrote:
>>>>> On Tue, Apr 21, 2020 at 11:23:00AM +0800, Jianyong Wu wrote:
>>>>>> diff --git a/virt/kvm/arm/hypercalls.c b/virt/kvm/arm/hypercalls.c
>>>>>> index 550dfa3e53cd..a5309c28d4dc 100644
>>>>>> --- a/virt/kvm/arm/hypercalls.c
>>>>>> +++ b/virt/kvm/arm/hypercalls.c
>>>>>> @@ -62,6 +66,44 @@ int kvm_hvc_call_handler(struct kvm_vcpu *vcpu)
>>>>>>     if (gpa != GPA_INVALID)
>>>>>>     val = gpa;
>>>>>>     break;
>>>>>> +/*
>>>>>> + * This serves virtual kvm_ptp.
>>>>>> + * Four values will be passed back.
>>>>>> + * reg0 stores high 32-bit host ktime;
>>>>>> + * reg1 stores low 32-bit host ktime;
>>>>>> + * reg2 stores high 32-bit difference of host cycles and cntvoff;
>>>>>> + * reg3 stores low 32-bit difference of host cycles and cntvoff.
>>>>>> + */
>>>>>> +case ARM_SMCCC_HYP_KVM_PTP_FUNC_ID:
>>>>> Shouldn't the host opt-in to providing this to the guest, as with other
>>>>> features?
>>>> er, do you mean that "ARM_SMCCC_HV_PV_TIME_XXX" as "opt-in"? if so, I
>>>> think this
>>>>
>>>> kvm_ptp doesn't need a buddy. the driver in guest will call this service
>>>> in a definite way.
>>> I mean that when creating the VM, userspace should be able to choose
>>> whether the PTP service is provided to the guest. The host shouldn't
>>> always provide it as there may be cases where doing so is undesireable.
>>>
>> I think I have implemented in patch 9/9 that userspace can get the info
>> that if the host offers the kvm_ptp service. But for now, the host
>> kernel will always offer the kvm_ptp capability in the current
>> implementation. I think x86 follow the same behavior (see [1]). so I
>> have not considered when and how to disable this kvm_ptp service in
>> host. Do you think we should offer this opt-in?
> 
> I think taht should be opt-in, yes.

ok, what about adding "CONFIG_ARM64_KVM_PTP_HOST" in kvm_ptp host 
service to implement this "opt-in"?
> 
> [...]
> 
>>> It's also not clear to me what notion of host time is being exposed to
>>> the guest (and consequently how this would interact with time changes on
>>> the host, time namespaces, etc). Having some description of that would
>>> be very helpful.
>>
>> sorry to have not made it clear.
>>
>> Time will not change in host and only time in guest will change to sync
>> with host. host time is the target that time in guest want to adjust to.
>> guest need to get the host time then compute the different of the time
>> in guest and host, so the guest can adjust the time base on the difference.
> 
> I understood that host time wouldn't change here, but what was not clear
> is which notion of host time is being exposed to the guest.
> 
> e.g. is that a raw monotonic clock, or one subject to periodic adjument,
> or wall time in the host? What is the epoch of the host time?

sorry, I misunderstood your last comment.
I think it is one of the key part of kvm_ptp. I have confused with these 
clock time and expect to hear the comments from experts of kernel time 
subsystem like you.
IMO,kvm_ptp targets to sync time in VM with host and if all the VMs in 
the same host do so, they can get time sync from each other. with no 
kvm_ptp, both host and guest may affected by NTP, the time will diverge 
between them. kvm_ptp can avoid this issue. if the host time vary with 
something like NTP adjustment, guest will track this variation with the 
help of kvm_ptp. I acquire these knowledge originally from [2]. also ptp 
driver will compare the wall time(see [3]). so I think wall time clock 
which subject to NTP adjustment is a good choice for kvm_ptp. in the 
current implementation I get the wall time clock from ktime_get_snapshot.

I'm not sure if I give the correct knowledge of kvm_ptp and make a right 
choice of host clock time. So WDYT?

[2]https://opensource.com/article/17/6/timekeeping-linux-vms
[3] see PTP_SYS_OFFSET2 in ptp_ioctl in 
https://github.com/torvalds/linux/blob/master/drivers/ptp/ptp_chardev.c, 
it uses ktime_get_real_ts64 as the host time to calculate the difference 
between ptp time and host time.

Thanks
Jianyong

> 
>> I will add the base principle of time sync service in guest using
>> kvm_ptp in commit message.
> 
> That would be great; thanks!
> 
> Mark.
>