diff mbox series

[v14,07/10] arm64/kvm: Add hypercall service for kvm ptp.

Message ID 20200904092744.167655-8-jianyong.wu@arm.com
State New, archived
Headers show
Series Enable ptp_kvm for arm64 | expand

Commit Message

Jianyong Wu Sept. 4, 2020, 9:27 a.m. UTC
ptp_kvm will get this service through smccc call.
The service offers wall time and counter cycle of host for guest.
caller must explicitly determines which cycle of virtual counter or
physical counter to return if it needs counter cycle.

Signed-off-by: Jianyong Wu <jianyong.wu@arm.com>
---
 arch/arm64/kvm/Kconfig       |  6 +++++
 arch/arm64/kvm/arch_timer.c  |  2 +-
 arch/arm64/kvm/hypercalls.c  | 49 ++++++++++++++++++++++++++++++++++++
 include/kvm/arm_arch_timer.h |  1 +
 include/linux/arm-smccc.h    | 16 ++++++++++++
 5 files changed, 73 insertions(+), 1 deletion(-)

Comments

Marc Zyngier Sept. 4, 2020, 4:15 p.m. UTC | #1
On Fri, 04 Sep 2020 10:27:41 +0100,
Jianyong Wu <jianyong.wu@arm.com> wrote:
> 
> ptp_kvm will get this service through smccc call.
> The service offers wall time and counter cycle of host for guest.
> caller must explicitly determines which cycle of virtual counter or
> physical counter to return if it needs counter cycle.
> 
> Signed-off-by: Jianyong Wu <jianyong.wu@arm.com>
> ---
>  arch/arm64/kvm/Kconfig       |  6 +++++
>  arch/arm64/kvm/arch_timer.c  |  2 +-
>  arch/arm64/kvm/hypercalls.c  | 49 ++++++++++++++++++++++++++++++++++++
>  include/kvm/arm_arch_timer.h |  1 +
>  include/linux/arm-smccc.h    | 16 ++++++++++++
>  5 files changed, 73 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/arm64/kvm/Kconfig b/arch/arm64/kvm/Kconfig
> index 318c8f2df245..bbdfacec4813 100644
> --- a/arch/arm64/kvm/Kconfig
> +++ b/arch/arm64/kvm/Kconfig
> @@ -60,6 +60,12 @@ config KVM_ARM_PMU
>  config KVM_INDIRECT_VECTORS
>  	def_bool HARDEN_BRANCH_PREDICTOR || RANDOMIZE_BASE
>  
> +config ARM64_KVM_PTP_HOST
> +	bool "KVM PTP clock host service for arm64"

The "for arm64" is not that useful.

> +	default y
> +	help
> +	  virtual kvm ptp clock hypercall service for arm64
> +

I'm not keen on making this a compile option, because whatever is not
always on ends up bit-rotting. Please drop the option.

>  endif # KVM
>  
>  endif # VIRTUALIZATION
> diff --git a/arch/arm64/kvm/arch_timer.c b/arch/arm64/kvm/arch_timer.c
> index 32ba6fbc3814..eb85f6701845 100644
> --- a/arch/arm64/kvm/arch_timer.c
> +++ b/arch/arm64/kvm/arch_timer.c
> @@ -81,7 +81,7 @@ u64 timer_get_cval(struct arch_timer_context *ctxt)
>  	}
>  }
>  
> -static u64 timer_get_offset(struct arch_timer_context *ctxt)
> +u64 timer_get_offset(struct arch_timer_context *ctxt)
>  {
>  	struct kvm_vcpu *vcpu = ctxt->vcpu;
>  
> diff --git a/arch/arm64/kvm/hypercalls.c b/arch/arm64/kvm/hypercalls.c
> index 901c60f119c2..2628ddc13abd 100644
> --- a/arch/arm64/kvm/hypercalls.c
> +++ b/arch/arm64/kvm/hypercalls.c
> @@ -3,6 +3,7 @@
>  
>  #include <linux/arm-smccc.h>
>  #include <linux/kvm_host.h>
> +#include <linux/clocksource_ids.h>
>  
>  #include <asm/kvm_emulate.h>
>  
> @@ -11,6 +12,10 @@
>  
>  int kvm_hvc_call_handler(struct kvm_vcpu *vcpu)
>  {
> +#ifdef CONFIG_ARM64_KVM_PTP_HOST
> +	struct system_time_snapshot systime_snapshot;
> +	u64 cycles = -1;
> +#endif

Please move all the PTP-related code to its own function, rather than
keeping it in the main HVC dispatcher. Also assigning a negative value
to something that is unsigned hurts my eyes. Consider using ~0UL instead.
See the comment below though.

>  	u32 func_id = smccc_get_function(vcpu);
>  	u64 val[4] = {SMCCC_RET_NOT_SUPPORTED};
>  	u32 feature;
> @@ -21,6 +26,10 @@ int kvm_hvc_call_handler(struct kvm_vcpu *vcpu)
>  		val[0] = ARM_SMCCC_VERSION_1_1;
>  		break;
>  	case ARM_SMCCC_ARCH_FEATURES_FUNC_ID:
> +		/*
> +		 * Note: keep in mind that feature is u32 and smccc_get_arg1
> +		 * will return u64, so need auto cast here.
> +		 */
>  		feature = smccc_get_arg1(vcpu);
>  		switch (feature) {
>  		case ARM_SMCCC_ARCH_WORKAROUND_1:
> @@ -70,7 +79,47 @@ int kvm_hvc_call_handler(struct kvm_vcpu *vcpu)
>  		break;
>  	case ARM_SMCCC_VENDOR_HYP_KVM_FEATURES_FUNC_ID:
>  		val[0] = BIT(ARM_SMCCC_KVM_FUNC_FEATURES);
> +#ifdef CONFIG_ARM64_KVM_PTP_HOST
> +		val[0] |= BIT(ARM_SMCCC_KVM_FUNC_KVM_PTP);
> +#endif
>  		break;
> +#ifdef CONFIG_ARM64_KVM_PTP_HOST
> +	/*
> +	 * 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.

This comment doesn't match what I read below.

> +	 */
> +	case ARM_SMCCC_VENDOR_HYP_KVM_PTP_FUNC_ID:
> +		/*
> +		 * system time and counter value must captured in the same
> +		 * time to keep consistency and precision.
> +		 */
> +		ktime_get_snapshot(&systime_snapshot);
> +		if (systime_snapshot.cs_id != CSID_ARM_ARCH_COUNTER)
> +			break;
> +		val[0] = systime_snapshot.real;
> +		/*
> +		 * which of virtual counter or physical counter being
> +		 * asked for is decided by the r1 value of smccc

nit: s/smccc/SMCCC/

> +		 * call. If no invalid r1 value offered, default cycle

nit: If r1 is an invalid value...

> +		 * value(-1) will return.

nit: will be returned.

> +		 */
> +		feature = smccc_get_arg1(vcpu);
> +		switch (feature) {
> +		case ARM_PTP_VIRT_COUNTER:
> +			cycles = systime_snapshot.cycles -
> +				 vcpu_read_sys_reg(vcpu, CNTVOFF_EL2);

nit: On a single line, please.

> +			break;
> +		case ARM_PTP_PHY_COUNTER:
> +			cycles = systime_snapshot.cycles;
> +			break;

It'd be a lot clearer if you had a default: case here, handling the
invalid case.

> +		}
> +		val[1] = cycles;

Given that cycles is a 64bit value, how does it work for a 32bit
guest? Or have you removed support for 32bit guests altogether?

> +		break;
> +#endif
>  	default:
>  		return kvm_psci_call(vcpu);
>  	}
> diff --git a/include/kvm/arm_arch_timer.h b/include/kvm/arm_arch_timer.h
> index 51c19381108c..5a2b6da9be7a 100644
> --- a/include/kvm/arm_arch_timer.h
> +++ b/include/kvm/arm_arch_timer.h
> @@ -105,5 +105,6 @@ void kvm_arm_timer_write_sysreg(struct kvm_vcpu *vcpu,
>  /* Needed for tracing */
>  u32 timer_get_ctl(struct arch_timer_context *ctxt);
>  u64 timer_get_cval(struct arch_timer_context *ctxt);
> +u64 timer_get_offset(struct arch_timer_context *ctxt);
>  
>  #endif
> diff --git a/include/linux/arm-smccc.h b/include/linux/arm-smccc.h
> index f7b5dd7dbf9f..0724840eb5f7 100644
> --- a/include/linux/arm-smccc.h
> +++ b/include/linux/arm-smccc.h
> @@ -103,6 +103,7 @@
>  
>  /* KVM "vendor specific" services */
>  #define ARM_SMCCC_KVM_FUNC_FEATURES		0
> +#define ARM_SMCCC_KVM_FUNC_KVM_PTP		1
>  #define ARM_SMCCC_KVM_FUNC_FEATURES_2		127
>  #define ARM_SMCCC_KVM_NUM_FUNCS			128
>  
> @@ -112,6 +113,21 @@
>  			   ARM_SMCCC_OWNER_VENDOR_HYP,			\
>  			   ARM_SMCCC_KVM_FUNC_FEATURES)
>  
> +/*
> + * ptp_kvm is a feature used for time sync between vm and host.
> + * ptp_kvm module in guest kernel will get service from host using
> + * this hypercall ID.
> + */
> +#define ARM_SMCCC_VENDOR_HYP_KVM_PTP_FUNC_ID                           \
> +	ARM_SMCCC_CALL_VAL(ARM_SMCCC_FAST_CALL,                         \
> +			   ARM_SMCCC_SMC_32,                            \
> +			   ARM_SMCCC_OWNER_VENDOR_HYP,                  \
> +			   ARM_SMCCC_KVM_FUNC_KVM_PTP)
> +
> +/* ptp_kvm counter type ID */
> +#define ARM_PTP_VIRT_COUNTER			0
> +#define ARM_PTP_PHY_COUNTER			1
> +
>  /* Paravirtualised time calls (defined by ARM DEN0057A) */
>  #define ARM_SMCCC_HV_PV_TIME_FEATURES				\
>  	ARM_SMCCC_CALL_VAL(ARM_SMCCC_FAST_CALL,			\
> -- 
> 2.17.1
> 
> 

Thanks,

	M.
Marc Zyngier Sept. 5, 2020, 11:04 a.m. UTC | #2
On Fri, 04 Sep 2020 10:27:41 +0100,
Jianyong Wu <jianyong.wu@arm.com> wrote:
> 
> ptp_kvm will get this service through smccc call.
> The service offers wall time and counter cycle of host for guest.
> caller must explicitly determines which cycle of virtual counter or
> physical counter to return if it needs counter cycle.
> 
> Signed-off-by: Jianyong Wu <jianyong.wu@arm.com>
> ---
>  arch/arm64/kvm/Kconfig       |  6 +++++
>  arch/arm64/kvm/arch_timer.c  |  2 +-
>  arch/arm64/kvm/hypercalls.c  | 49 ++++++++++++++++++++++++++++++++++++
>  include/kvm/arm_arch_timer.h |  1 +
>  include/linux/arm-smccc.h    | 16 ++++++++++++
>  5 files changed, 73 insertions(+), 1 deletion(-)
>

[...]

On top of what I said yesterday:

> index 32ba6fbc3814..eb85f6701845 100644
> --- a/arch/arm64/kvm/arch_timer.c
> +++ b/arch/arm64/kvm/arch_timer.c
> @@ -81,7 +81,7 @@ u64 timer_get_cval(struct arch_timer_context *ctxt)
>  	}
>  }
>  
> -static u64 timer_get_offset(struct arch_timer_context *ctxt)
> +u64 timer_get_offset(struct arch_timer_context *ctxt)
>  {
>  	struct kvm_vcpu *vcpu = ctxt->vcpu;
>

Why has this become global? I can't see a reason why we would want to
expose this purely KVM internal helper.

	M.
Jianyong Wu Sept. 7, 2020, 8:10 a.m. UTC | #3
Hi Marc,

> -----Original Message-----
> From: Marc Zyngier <maz@kernel.org>
> Sent: Saturday, September 5, 2020 12:15 AM
> To: Jianyong Wu <Jianyong.Wu@arm.com>
> Cc: netdev@vger.kernel.org; yangbo.lu@nxp.com; john.stultz@linaro.org;
> tglx@linutronix.de; pbonzini@redhat.com; sean.j.christopherson@intel.com;
> richardcochran@gmail.com; Mark Rutland <Mark.Rutland@arm.com>;
> will@kernel.org; Suzuki Poulose <Suzuki.Poulose@arm.com>; Steven Price
> <Steven.Price@arm.com>; linux-kernel@vger.kernel.org; linux-arm-
> kernel@lists.infradead.org; kvmarm@lists.cs.columbia.edu;
> kvm@vger.kernel.org; Steve Capper <Steve.Capper@arm.com>; Justin He
> <Justin.He@arm.com>; nd <nd@arm.com>
> Subject: Re: [PATCH v14 07/10] arm64/kvm: Add hypercall service for kvm
> ptp.
> 
> On Fri, 04 Sep 2020 10:27:41 +0100,
> Jianyong Wu <jianyong.wu@arm.com> wrote:
> >
> > ptp_kvm will get this service through smccc call.
> > The service offers wall time and counter cycle of host for guest.
> > caller must explicitly determines which cycle of virtual counter or
> > physical counter to return if it needs counter cycle.
> >
> > Signed-off-by: Jianyong Wu <jianyong.wu@arm.com>
> > ---
> >  arch/arm64/kvm/Kconfig       |  6 +++++
> >  arch/arm64/kvm/arch_timer.c  |  2 +-
> >  arch/arm64/kvm/hypercalls.c  | 49
> > ++++++++++++++++++++++++++++++++++++
> >  include/kvm/arm_arch_timer.h |  1 +
> >  include/linux/arm-smccc.h    | 16 ++++++++++++
> >  5 files changed, 73 insertions(+), 1 deletion(-)
> >
> > diff --git a/arch/arm64/kvm/Kconfig b/arch/arm64/kvm/Kconfig index
> > 318c8f2df245..bbdfacec4813 100644
> > --- a/arch/arm64/kvm/Kconfig
> > +++ b/arch/arm64/kvm/Kconfig
> > @@ -60,6 +60,12 @@ config KVM_ARM_PMU
> >  config KVM_INDIRECT_VECTORS
> >  	def_bool HARDEN_BRANCH_PREDICTOR || RANDOMIZE_BASE
> >
> > +config ARM64_KVM_PTP_HOST
> > +	bool "KVM PTP clock host service for arm64"
> 
> The "for arm64" is not that useful.
Yeah,

> 
> > +	default y
> > +	help
> > +	  virtual kvm ptp clock hypercall service for arm64
> > +
> 
> I'm not keen on making this a compile option, because whatever is not
> always on ends up bit-rotting. Please drop the option.
> 
Ok, I will remove this option next time.

> >  endif # KVM
> >
> >  endif # VIRTUALIZATION
> > diff --git a/arch/arm64/kvm/arch_timer.c b/arch/arm64/kvm/arch_timer.c
> > index 32ba6fbc3814..eb85f6701845 100644
> > --- a/arch/arm64/kvm/arch_timer.c
> > +++ b/arch/arm64/kvm/arch_timer.c
> > @@ -81,7 +81,7 @@ u64 timer_get_cval(struct arch_timer_context *ctxt)
> >  	}
> >  }
> >
> > -static u64 timer_get_offset(struct arch_timer_context *ctxt)
> > +u64 timer_get_offset(struct arch_timer_context *ctxt)
> >  {
> >  	struct kvm_vcpu *vcpu = ctxt->vcpu;
> >
> > diff --git a/arch/arm64/kvm/hypercalls.c b/arch/arm64/kvm/hypercalls.c
> > index 901c60f119c2..2628ddc13abd 100644
> > --- a/arch/arm64/kvm/hypercalls.c
> > +++ b/arch/arm64/kvm/hypercalls.c
> > @@ -3,6 +3,7 @@
> >
> >  #include <linux/arm-smccc.h>
> >  #include <linux/kvm_host.h>
> > +#include <linux/clocksource_ids.h>
> >
> >  #include <asm/kvm_emulate.h>
> >
> > @@ -11,6 +12,10 @@
> >
> >  int kvm_hvc_call_handler(struct kvm_vcpu *vcpu)  {
> > +#ifdef CONFIG_ARM64_KVM_PTP_HOST
> > +	struct system_time_snapshot systime_snapshot;
> > +	u64 cycles = -1;
> > +#endif
> 
> Please move all the PTP-related code to its own function, rather than
> keeping it in the main HVC dispatcher. Also assigning a negative value to
> something that is unsigned hurts my eyes. Consider using ~0UL instead.
> See the comment below though.

Ok, much better.

> 
> >  	u32 func_id = smccc_get_function(vcpu);
> >  	u64 val[4] = {SMCCC_RET_NOT_SUPPORTED};
> >  	u32 feature;
> > @@ -21,6 +26,10 @@ int kvm_hvc_call_handler(struct kvm_vcpu *vcpu)
> >  		val[0] = ARM_SMCCC_VERSION_1_1;
> >  		break;
> >  	case ARM_SMCCC_ARCH_FEATURES_FUNC_ID:
> > +		/*
> > +		 * Note: keep in mind that feature is u32 and smccc_get_arg1
> > +		 * will return u64, so need auto cast here.
> > +		 */
> >  		feature = smccc_get_arg1(vcpu);
> >  		switch (feature) {
> >  		case ARM_SMCCC_ARCH_WORKAROUND_1:
> > @@ -70,7 +79,47 @@ int kvm_hvc_call_handler(struct kvm_vcpu *vcpu)
> >  		break;
> >  	case ARM_SMCCC_VENDOR_HYP_KVM_FEATURES_FUNC_ID:
> >  		val[0] = BIT(ARM_SMCCC_KVM_FUNC_FEATURES);
> > +#ifdef CONFIG_ARM64_KVM_PTP_HOST
> > +		val[0] |= BIT(ARM_SMCCC_KVM_FUNC_KVM_PTP); #endif
> >  		break;
> > +#ifdef CONFIG_ARM64_KVM_PTP_HOST
> > +	/*
> > +	 * 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.
> 
> This comment doesn't match what I read below.
>
Sorry, should have changed according this time. But should keep this next time as
we really need use 32-bits value to support HVC32.
 
> > +	 */
> > +	case ARM_SMCCC_VENDOR_HYP_KVM_PTP_FUNC_ID:
> > +		/*
> > +		 * system time and counter value must captured in the same
> > +		 * time to keep consistency and precision.
> > +		 */
> > +		ktime_get_snapshot(&systime_snapshot);
> > +		if (systime_snapshot.cs_id != CSID_ARM_ARCH_COUNTER)
> > +			break;
> > +		val[0] = systime_snapshot.real;
> > +		/*
> > +		 * which of virtual counter or physical counter being
> > +		 * asked for is decided by the r1 value of smccc
> 
> nit: s/smccc/SMCCC/
Thanks

> 
> > +		 * call. If no invalid r1 value offered, default cycle
> 
> nit: If r1 is an invalid value...
> 
> > +		 * value(-1) will return.
> 
> nit: will be returned.
> 
Yeah.

> > +		 */
> > +		feature = smccc_get_arg1(vcpu);
> > +		switch (feature) {
> > +		case ARM_PTP_VIRT_COUNTER:
> > +			cycles = systime_snapshot.cycles -
> > +				 vcpu_read_sys_reg(vcpu, CNTVOFF_EL2);
> 
> nit: On a single line, please.
Ok.
> 
> > +			break;
> > +		case ARM_PTP_PHY_COUNTER:
> > +			cycles = systime_snapshot.cycles;
> > +			break;
> 
> It'd be a lot clearer if you had a default: case here, handling the invalid case.

Ok, much better.

> 
> > +		}
> > +		val[1] = cycles;
> 
> Given that cycles is a 64bit value, how does it work for a 32bit guest? Or have
> you removed support for 32bit guests altogether?
> 
Yeah, I will arm32 support back.

Thanks
Jianyong 
> > +		break;
> > +#endif
> >  	default:
> >  		return kvm_psci_call(vcpu);
> >  	}
> > diff --git a/include/kvm/arm_arch_timer.h
> > b/include/kvm/arm_arch_timer.h index 51c19381108c..5a2b6da9be7a
> 100644
> > --- a/include/kvm/arm_arch_timer.h
> > +++ b/include/kvm/arm_arch_timer.h
> > @@ -105,5 +105,6 @@ void kvm_arm_timer_write_sysreg(struct
> kvm_vcpu
> > *vcpu,
> >  /* Needed for tracing */
> >  u32 timer_get_ctl(struct arch_timer_context *ctxt);
> >  u64 timer_get_cval(struct arch_timer_context *ctxt);
> > +u64 timer_get_offset(struct arch_timer_context *ctxt);
> >
> >  #endif
> > diff --git a/include/linux/arm-smccc.h b/include/linux/arm-smccc.h
> > index f7b5dd7dbf9f..0724840eb5f7 100644
> > --- a/include/linux/arm-smccc.h
> > +++ b/include/linux/arm-smccc.h
> > @@ -103,6 +103,7 @@
> >
> >  /* KVM "vendor specific" services */
> >  #define ARM_SMCCC_KVM_FUNC_FEATURES		0
> > +#define ARM_SMCCC_KVM_FUNC_KVM_PTP		1
> >  #define ARM_SMCCC_KVM_FUNC_FEATURES_2		127
> >  #define ARM_SMCCC_KVM_NUM_FUNCS			128
> >
> > @@ -112,6 +113,21 @@
> >  			   ARM_SMCCC_OWNER_VENDOR_HYP,
> 		\
> >  			   ARM_SMCCC_KVM_FUNC_FEATURES)
> >
> > +/*
> > + * ptp_kvm is a feature used for time sync between vm and host.
> > + * ptp_kvm module in guest kernel will get service from host using
> > + * this hypercall ID.
> > + */
> > +#define ARM_SMCCC_VENDOR_HYP_KVM_PTP_FUNC_ID                           \
> > +	ARM_SMCCC_CALL_VAL(ARM_SMCCC_FAST_CALL,                         \
> > +			   ARM_SMCCC_SMC_32,                            \
> > +			   ARM_SMCCC_OWNER_VENDOR_HYP,                  \
> > +			   ARM_SMCCC_KVM_FUNC_KVM_PTP)
> > +
> > +/* ptp_kvm counter type ID */
> > +#define ARM_PTP_VIRT_COUNTER			0
> > +#define ARM_PTP_PHY_COUNTER			1
> > +
> >  /* Paravirtualised time calls (defined by ARM DEN0057A) */
> >  #define ARM_SMCCC_HV_PV_TIME_FEATURES
> 	\
> >  	ARM_SMCCC_CALL_VAL(ARM_SMCCC_FAST_CALL,
> 	\
> > --
> > 2.17.1
> >
> >
> 
> Thanks,
> 
> 	M.
> 
> --
> Without deviation from the norm, progress is not possible.
Jianyong Wu Sept. 7, 2020, 8:13 a.m. UTC | #4
Hi Marc,

> -----Original Message-----
> From: Marc Zyngier <maz@kernel.org>
> Sent: Saturday, September 5, 2020 7:04 PM
> To: Jianyong Wu <Jianyong.Wu@arm.com>
> Cc: netdev@vger.kernel.org; yangbo.lu@nxp.com; john.stultz@linaro.org;
> tglx@linutronix.de; pbonzini@redhat.com; sean.j.christopherson@intel.com;
> richardcochran@gmail.com; Mark Rutland <Mark.Rutland@arm.com>;
> will@kernel.org; Suzuki Poulose <Suzuki.Poulose@arm.com>; Steven Price
> <Steven.Price@arm.com>; linux-kernel@vger.kernel.org; linux-arm-
> kernel@lists.infradead.org; kvmarm@lists.cs.columbia.edu;
> kvm@vger.kernel.org; Steve Capper <Steve.Capper@arm.com>; Justin He
> <Justin.He@arm.com>; nd <nd@arm.com>
> Subject: Re: [PATCH v14 07/10] arm64/kvm: Add hypercall service for kvm
> ptp.
> 
> On Fri, 04 Sep 2020 10:27:41 +0100,
> Jianyong Wu <jianyong.wu@arm.com> wrote:
> >
> > ptp_kvm will get this service through smccc call.
> > The service offers wall time and counter cycle of host for guest.
> > caller must explicitly determines which cycle of virtual counter or
> > physical counter to return if it needs counter cycle.
> >
> > Signed-off-by: Jianyong Wu <jianyong.wu@arm.com>
> > ---
> >  arch/arm64/kvm/Kconfig       |  6 +++++
> >  arch/arm64/kvm/arch_timer.c  |  2 +-
> >  arch/arm64/kvm/hypercalls.c  | 49
> > ++++++++++++++++++++++++++++++++++++
> >  include/kvm/arm_arch_timer.h |  1 +
> >  include/linux/arm-smccc.h    | 16 ++++++++++++
> >  5 files changed, 73 insertions(+), 1 deletion(-)
> >
> 
> [...]
> 
> On top of what I said yesterday:
> 
> > index 32ba6fbc3814..eb85f6701845 100644
> > --- a/arch/arm64/kvm/arch_timer.c
> > +++ b/arch/arm64/kvm/arch_timer.c
> > @@ -81,7 +81,7 @@ u64 timer_get_cval(struct arch_timer_context *ctxt)
> >  	}
> >  }
> >
> > -static u64 timer_get_offset(struct arch_timer_context *ctxt)
> > +u64 timer_get_offset(struct arch_timer_context *ctxt)
> >  {
> >  	struct kvm_vcpu *vcpu = ctxt->vcpu;
> >
> 
> Why has this become global? I can't see a reason why we would want to
> expose this purely KVM internal helper.
> 
Sorry to have forgotten remove this change.

Thanks
Jianyong 
> 	M.
> 
> --
> Without deviation from the norm, progress is not possible.
diff mbox series

Patch

diff --git a/arch/arm64/kvm/Kconfig b/arch/arm64/kvm/Kconfig
index 318c8f2df245..bbdfacec4813 100644
--- a/arch/arm64/kvm/Kconfig
+++ b/arch/arm64/kvm/Kconfig
@@ -60,6 +60,12 @@  config KVM_ARM_PMU
 config KVM_INDIRECT_VECTORS
 	def_bool HARDEN_BRANCH_PREDICTOR || RANDOMIZE_BASE
 
+config ARM64_KVM_PTP_HOST
+	bool "KVM PTP clock host service for arm64"
+	default y
+	help
+	  virtual kvm ptp clock hypercall service for arm64
+
 endif # KVM
 
 endif # VIRTUALIZATION
diff --git a/arch/arm64/kvm/arch_timer.c b/arch/arm64/kvm/arch_timer.c
index 32ba6fbc3814..eb85f6701845 100644
--- a/arch/arm64/kvm/arch_timer.c
+++ b/arch/arm64/kvm/arch_timer.c
@@ -81,7 +81,7 @@  u64 timer_get_cval(struct arch_timer_context *ctxt)
 	}
 }
 
-static u64 timer_get_offset(struct arch_timer_context *ctxt)
+u64 timer_get_offset(struct arch_timer_context *ctxt)
 {
 	struct kvm_vcpu *vcpu = ctxt->vcpu;
 
diff --git a/arch/arm64/kvm/hypercalls.c b/arch/arm64/kvm/hypercalls.c
index 901c60f119c2..2628ddc13abd 100644
--- a/arch/arm64/kvm/hypercalls.c
+++ b/arch/arm64/kvm/hypercalls.c
@@ -3,6 +3,7 @@ 
 
 #include <linux/arm-smccc.h>
 #include <linux/kvm_host.h>
+#include <linux/clocksource_ids.h>
 
 #include <asm/kvm_emulate.h>
 
@@ -11,6 +12,10 @@ 
 
 int kvm_hvc_call_handler(struct kvm_vcpu *vcpu)
 {
+#ifdef CONFIG_ARM64_KVM_PTP_HOST
+	struct system_time_snapshot systime_snapshot;
+	u64 cycles = -1;
+#endif
 	u32 func_id = smccc_get_function(vcpu);
 	u64 val[4] = {SMCCC_RET_NOT_SUPPORTED};
 	u32 feature;
@@ -21,6 +26,10 @@  int kvm_hvc_call_handler(struct kvm_vcpu *vcpu)
 		val[0] = ARM_SMCCC_VERSION_1_1;
 		break;
 	case ARM_SMCCC_ARCH_FEATURES_FUNC_ID:
+		/*
+		 * Note: keep in mind that feature is u32 and smccc_get_arg1
+		 * will return u64, so need auto cast here.
+		 */
 		feature = smccc_get_arg1(vcpu);
 		switch (feature) {
 		case ARM_SMCCC_ARCH_WORKAROUND_1:
@@ -70,7 +79,47 @@  int kvm_hvc_call_handler(struct kvm_vcpu *vcpu)
 		break;
 	case ARM_SMCCC_VENDOR_HYP_KVM_FEATURES_FUNC_ID:
 		val[0] = BIT(ARM_SMCCC_KVM_FUNC_FEATURES);
+#ifdef CONFIG_ARM64_KVM_PTP_HOST
+		val[0] |= BIT(ARM_SMCCC_KVM_FUNC_KVM_PTP);
+#endif
 		break;
+#ifdef CONFIG_ARM64_KVM_PTP_HOST
+	/*
+	 * 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_VENDOR_HYP_KVM_PTP_FUNC_ID:
+		/*
+		 * system time and counter value must captured in the same
+		 * time to keep consistency and precision.
+		 */
+		ktime_get_snapshot(&systime_snapshot);
+		if (systime_snapshot.cs_id != CSID_ARM_ARCH_COUNTER)
+			break;
+		val[0] = systime_snapshot.real;
+		/*
+		 * which of virtual counter or physical counter being
+		 * asked for is decided by the r1 value of smccc
+		 * call. If no invalid r1 value offered, default cycle
+		 * value(-1) will return.
+		 */
+		feature = smccc_get_arg1(vcpu);
+		switch (feature) {
+		case ARM_PTP_VIRT_COUNTER:
+			cycles = systime_snapshot.cycles -
+				 vcpu_read_sys_reg(vcpu, CNTVOFF_EL2);
+			break;
+		case ARM_PTP_PHY_COUNTER:
+			cycles = systime_snapshot.cycles;
+			break;
+		}
+		val[1] = cycles;
+		break;
+#endif
 	default:
 		return kvm_psci_call(vcpu);
 	}
diff --git a/include/kvm/arm_arch_timer.h b/include/kvm/arm_arch_timer.h
index 51c19381108c..5a2b6da9be7a 100644
--- a/include/kvm/arm_arch_timer.h
+++ b/include/kvm/arm_arch_timer.h
@@ -105,5 +105,6 @@  void kvm_arm_timer_write_sysreg(struct kvm_vcpu *vcpu,
 /* Needed for tracing */
 u32 timer_get_ctl(struct arch_timer_context *ctxt);
 u64 timer_get_cval(struct arch_timer_context *ctxt);
+u64 timer_get_offset(struct arch_timer_context *ctxt);
 
 #endif
diff --git a/include/linux/arm-smccc.h b/include/linux/arm-smccc.h
index f7b5dd7dbf9f..0724840eb5f7 100644
--- a/include/linux/arm-smccc.h
+++ b/include/linux/arm-smccc.h
@@ -103,6 +103,7 @@ 
 
 /* KVM "vendor specific" services */
 #define ARM_SMCCC_KVM_FUNC_FEATURES		0
+#define ARM_SMCCC_KVM_FUNC_KVM_PTP		1
 #define ARM_SMCCC_KVM_FUNC_FEATURES_2		127
 #define ARM_SMCCC_KVM_NUM_FUNCS			128
 
@@ -112,6 +113,21 @@ 
 			   ARM_SMCCC_OWNER_VENDOR_HYP,			\
 			   ARM_SMCCC_KVM_FUNC_FEATURES)
 
+/*
+ * ptp_kvm is a feature used for time sync between vm and host.
+ * ptp_kvm module in guest kernel will get service from host using
+ * this hypercall ID.
+ */
+#define ARM_SMCCC_VENDOR_HYP_KVM_PTP_FUNC_ID                           \
+	ARM_SMCCC_CALL_VAL(ARM_SMCCC_FAST_CALL,                         \
+			   ARM_SMCCC_SMC_32,                            \
+			   ARM_SMCCC_OWNER_VENDOR_HYP,                  \
+			   ARM_SMCCC_KVM_FUNC_KVM_PTP)
+
+/* ptp_kvm counter type ID */
+#define ARM_PTP_VIRT_COUNTER			0
+#define ARM_PTP_PHY_COUNTER			1
+
 /* Paravirtualised time calls (defined by ARM DEN0057A) */
 #define ARM_SMCCC_HV_PV_TIME_FEATURES				\
 	ARM_SMCCC_CALL_VAL(ARM_SMCCC_FAST_CALL,			\