RE: [RFC PATCH v3 4/6] psci: Add hvc call service for ptp_kvm.

[Jianyong Wu (Arm Technology China)]

Hi Paolo,

> -----Original Message-----
> From: Paolo Bonzini <pbonzini@xxxxxxxxxx>
> Sent: Wednesday, September 18, 2019 4:26 PM
> To: Jianyong Wu (Arm Technology China) <Jianyong.Wu@xxxxxxx>;
> netdev@xxxxxxxxxxxxxxx; yangbo.lu@xxxxxxx; john.stultz@xxxxxxxxxx;
> tglx@xxxxxxxxxxxxx; sean.j.christopherson@xxxxxxxxx; maz@xxxxxxxxxx;
> richardcochran@xxxxxxxxx; Mark Rutland <Mark.Rutland@xxxxxxx>; Will
> Deacon <Will.Deacon@xxxxxxx>; Suzuki Poulose
> <Suzuki.Poulose@xxxxxxx>
> Cc: linux-kernel@xxxxxxxxxxxxxxx; kvm@xxxxxxxxxxxxxxx; Steve Capper
> <Steve.Capper@xxxxxxx>; Kaly Xin (Arm Technology China)
> <Kaly.Xin@xxxxxxx>; Justin He (Arm Technology China)
> <Justin.He@xxxxxxx>; nd <nd@xxxxxxx>; linux-arm-
> kernel@xxxxxxxxxxxxxxxxxxx
> Subject: Re: [RFC PATCH v3 4/6] psci: Add hvc call service for ptp_kvm.
> 
> On 18/09/19 10:07, Jianyong Wu wrote:
> > +	case ARM_SMCCC_VENDOR_HYP_KVM_PTP_FUNC_ID:
> > +		getnstimeofday(ts);
> 
> This is not Y2038-safe.  Please use ktime_get_real_ts64 instead, and split the
> 64-bit seconds value between val[0] and val[1].
> 
As far as I know, y2038-safe will only affect signed 32-bit integer, how does it affect 64-bit integer?
And why split 64-bit number into two blocks is necessary?
Also, split number will lead to shortage of return value.
 
> However, it seems to me that the new function is not needed and you can
> just use ktime_get_snapshot.  You'll get the time in systime_snapshot->real
> and the cycles value in systime_snapshot->cycles.

See patch 5/6, I need both counter cycle and clocksource, ktime_get_snapshot seems only offer cycles.
  
> 
> > +		get_current_counterval(&sc);
> > +		val[0] = ts->tv_sec;
> > +		val[1] = ts->tv_nsec;
> > +		val[2] = sc.cycles;
> > +		val[3] = 0;
> > +		break;
> 
> This should return a guest-cycles value.  If the cycles values always the same
> between the host and the guest on ARM, then okay.  If not, you have to
> apply whatever offset exists.
> 
In my opinion, when use ptp_kvm as clock sources to sync time between host and guest, user should promise the guest and host has no
clock offset. So we can be sure that the cycle between guest and host should be keep consistent. But I need check it.
I think host cycle should be returned to guest as we should promise we get clock and counter in the same time.

Thanks
Jianyong Wu

> Thanks,
> 
> Paolo