| Message ID | 20191210034026.45229-7-jianyong.wu@arm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Enable ptp_kvm for arm64 | expand |
On 2019-12-10 03:40, Jianyong Wu wrote: > ptp_kvm modules will call hvc to get this service. > The service offers real time and counter cycle of host for guest. > > Signed-off-by: Jianyong Wu <jianyong.wu@arm.com> > --- > include/linux/arm-smccc.h | 12 ++++++++++++ > virt/kvm/arm/psci.c | 22 ++++++++++++++++++++++ > 2 files changed, 34 insertions(+) > > diff --git a/include/linux/arm-smccc.h b/include/linux/arm-smccc.h > index 6f82c87308ed..aafb6bac167d 100644 > --- a/include/linux/arm-smccc.h > +++ b/include/linux/arm-smccc.h > @@ -94,6 +94,7 @@ > > /* KVM "vendor specific" services */ > #define ARM_SMCCC_KVM_FUNC_FEATURES 0 > +#define ARM_SMCCC_KVM_PTP 1 > #define ARM_SMCCC_KVM_FUNC_FEATURES_2 127 > #define ARM_SMCCC_KVM_NUM_FUNCS 128 > > @@ -103,6 +104,17 @@ > ARM_SMCCC_OWNER_VENDOR_HYP, \ > ARM_SMCCC_KVM_FUNC_FEATURES) > > +/* > + * This ID used for virtual ptp kvm clock and it will pass second > value > + * and nanosecond value of host real time and system counter by vcpu > + * register to guest. > + */ > +#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_PTP) > + All of this depends on patches that have never need posted to any ML, and just linger in Will's tree. You need to pick them up and post them as part of this series so that they can at least be reviewed. > #ifndef __ASSEMBLY__ > > #include <linux/linkage.h> > diff --git a/virt/kvm/arm/psci.c b/virt/kvm/arm/psci.c > index 0debf49bf259..682d892d6717 100644 > --- a/virt/kvm/arm/psci.c > +++ b/virt/kvm/arm/psci.c > @@ -9,6 +9,7 @@ > #include <linux/kvm_host.h> > #include <linux/uaccess.h> > #include <linux/wait.h> > +#include <linux/clocksource_ids.h> > > #include <asm/cputype.h> > #include <asm/kvm_emulate.h> > @@ -389,6 +390,8 @@ static int kvm_psci_call(struct kvm_vcpu *vcpu) > > int kvm_hvc_call_handler(struct kvm_vcpu *vcpu) > { > + struct system_time_snapshot systime_snapshot; > + u64 cycles; > u32 func_id = smccc_get_function(vcpu); > u32 val[4] = {}; > u32 option; > @@ -431,6 +434,25 @@ int kvm_hvc_call_handler(struct kvm_vcpu *vcpu) > case ARM_SMCCC_VENDOR_HYP_KVM_FEATURES_FUNC_ID: > val[0] = BIT(ARM_SMCCC_KVM_FUNC_FEATURES); > break; > + /* > + * This will used for virtual ptp kvm clock. three > + * 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. That's either two or four values, and not three as you claim above. Also, I fail to understand the meaning of the host cycle vs cntvoff comparison. This is something that guest can perform on its own (it has access to both physical and virtual timers, and can compute cntvoff without intervention of the hypervisor). Finally, how does it work with nested virt, where cntvoff is for the the vEL2 guest? > + */ > + case ARM_SMCCC_VENDOR_HYP_KVM_PTP_FUNC_ID: > + ktime_get_snapshot(&systime_snapshot); > + if (systime_snapshot.cs_id != CSID_ARM_ARCH_COUNTER) > + return kvm_psci_call(vcpu); What does this mean? Calling PSCI because you've failed to identify the clock source? What result do you expect from this? Hint: this isn't a PSCI call. Cosmetic comments below: > + val[0] = systime_snapshot.real >> 32; val[0] = upper_32_bits(systime_snapshot.real); > + val[1] = systime_snapshot.real << 32 >> 32; val[1] = lower_32_bits(systime_snapshot.real); > + cycles = systime_snapshot.cycles - > + vcpu_vtimer(vcpu)->cntvoff; On a single line please. > + val[2] = cycles >> 32; > + val[3] = cycles << 32 >> 32; Same as above. > + break; > default: > return kvm_psci_call(vcpu); > } Thanks, M.
Hi Marc, > -----Original Message----- > From: Marc Zyngier <maz@kernel.org> > Sent: Tuesday, January 7, 2020 5:16 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>; Kaly Xin > <Kaly.Xin@arm.com>; Justin He <Justin.He@arm.com>; nd <nd@arm.com> > Subject: Re: [RFC PATCH v9 6/8] psci: Add hvc call service for ptp_kvm. > > On 2019-12-10 03:40, Jianyong Wu wrote: > > ptp_kvm modules will call hvc to get this service. > > The service offers real time and counter cycle of host for guest. > > > > Signed-off-by: Jianyong Wu <jianyong.wu@arm.com> > > --- > > include/linux/arm-smccc.h | 12 ++++++++++++ > > virt/kvm/arm/psci.c | 22 ++++++++++++++++++++++ > > 2 files changed, 34 insertions(+) > > > > diff --git a/include/linux/arm-smccc.h b/include/linux/arm-smccc.h > > index 6f82c87308ed..aafb6bac167d 100644 > > --- a/include/linux/arm-smccc.h > > +++ b/include/linux/arm-smccc.h > > @@ -94,6 +94,7 @@ > > > > /* KVM "vendor specific" services */ > > #define ARM_SMCCC_KVM_FUNC_FEATURES 0 > > +#define ARM_SMCCC_KVM_PTP 1 > > #define ARM_SMCCC_KVM_FUNC_FEATURES_2 127 > > #define ARM_SMCCC_KVM_NUM_FUNCS 128 > > > > @@ -103,6 +104,17 @@ > > ARM_SMCCC_OWNER_VENDOR_HYP, > \ > > ARM_SMCCC_KVM_FUNC_FEATURES) > > > > +/* > > + * This ID used for virtual ptp kvm clock and it will pass second > > value > > + * and nanosecond value of host real time and system counter by vcpu > > + * register to guest. > > + */ > > +#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_PTP) > > + > > All of this depends on patches that have never need posted to any ML, and > just linger in Will's tree. You need to pick them up and post them as part of > this series so that they can at least be reviewed. > Ok, I will add them next version. > > #ifndef __ASSEMBLY__ > > > > #include <linux/linkage.h> > > diff --git a/virt/kvm/arm/psci.c b/virt/kvm/arm/psci.c index > > 0debf49bf259..682d892d6717 100644 > > --- a/virt/kvm/arm/psci.c > > +++ b/virt/kvm/arm/psci.c > > @@ -9,6 +9,7 @@ > > #include <linux/kvm_host.h> > > #include <linux/uaccess.h> > > #include <linux/wait.h> > > +#include <linux/clocksource_ids.h> > > > > #include <asm/cputype.h> > > #include <asm/kvm_emulate.h> > > @@ -389,6 +390,8 @@ static int kvm_psci_call(struct kvm_vcpu *vcpu) > > > > int kvm_hvc_call_handler(struct kvm_vcpu *vcpu) { > > + struct system_time_snapshot systime_snapshot; > > + u64 cycles; > > u32 func_id = smccc_get_function(vcpu); > > u32 val[4] = {}; > > u32 option; > > @@ -431,6 +434,25 @@ int kvm_hvc_call_handler(struct kvm_vcpu *vcpu) > > case ARM_SMCCC_VENDOR_HYP_KVM_FEATURES_FUNC_ID: > > val[0] = BIT(ARM_SMCCC_KVM_FUNC_FEATURES); > > break; > > + /* > > + * This will used for virtual ptp kvm clock. three > > + * 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. > > That's either two or four values, and not three as you claim above. > Sorry, I'm not sure what do you mean "three", the registers here is 4 from reg0 to reg3. > Also, I fail to understand the meaning of the host cycle vs cntvoff comparison. > This is something that guest can perform on its own (it has access to both > physical and virtual timers, and can compute cntvoff without intervention of > the hypervisor). > To keep consistency and precision, clock time and counter cycle must captured at the same time. It will perform at ktime_get_snapshot. > Finally, how does it work with nested virt, where cntvoff is for the the vEL2 > guest? > For now, I have not considered ptp_kvm in nested virtualization. Also I'm not sure about if nested virtualization is ready on arm64 , as I need test ptp_kvm on it. If so, I can consider it. > > + */ > > + case ARM_SMCCC_VENDOR_HYP_KVM_PTP_FUNC_ID: > > + ktime_get_snapshot(&systime_snapshot); > > + if (systime_snapshot.cs_id != CSID_ARM_ARCH_COUNTER) > > + return kvm_psci_call(vcpu); > > What does this mean? Calling PSCI because you've failed to identify the clock > source? What result do you expect from this? Hint: this isn't a PSCI call. > Sorry, what I want to do here is that return to guest with the error info. Maybe I should set val[0] to -1 and break to let the guest know that error comes, as the guest will check if val[0] is positive to determine the next step. > Cosmetic comments below: > > > + val[0] = systime_snapshot.real >> 32; > > val[0] = upper_32_bits(systime_snapshot.real); Ok > > > + val[1] = systime_snapshot.real << 32 >> 32; > > val[1] = lower_32_bits(systime_snapshot.real); > Yeah > > + cycles = systime_snapshot.cycles - > > + vcpu_vtimer(vcpu)->cntvoff; > > On a single line please. > ok > > + val[2] = cycles >> 32; > > + val[3] = cycles << 32 >> 32; > > Same as above. > yeah > > + break; > > default: > > return kvm_psci_call(vcpu); > > } > Thanks Jianyong > Thanks, > > M. > -- > Jazz is not dead. It just smells funny...
On 2020-01-09 05:45, Jianyong Wu wrote: > Hi Marc, > >> -----Original Message----- >> From: Marc Zyngier <maz@kernel.org> >> Sent: Tuesday, January 7, 2020 5:16 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>; Kaly Xin >> <Kaly.Xin@arm.com>; Justin He <Justin.He@arm.com>; nd <nd@arm.com> >> Subject: Re: [RFC PATCH v9 6/8] psci: Add hvc call service for >> ptp_kvm. >> >> On 2019-12-10 03:40, Jianyong Wu wrote: >> > ptp_kvm modules will call hvc to get this service. >> > The service offers real time and counter cycle of host for guest. >> > >> > Signed-off-by: Jianyong Wu <jianyong.wu@arm.com> >> > --- >> > include/linux/arm-smccc.h | 12 ++++++++++++ >> > virt/kvm/arm/psci.c | 22 ++++++++++++++++++++++ >> > 2 files changed, 34 insertions(+) >> > >> > diff --git a/include/linux/arm-smccc.h b/include/linux/arm-smccc.h >> > index 6f82c87308ed..aafb6bac167d 100644 >> > --- a/include/linux/arm-smccc.h >> > +++ b/include/linux/arm-smccc.h >> > @@ -94,6 +94,7 @@ >> > >> > /* KVM "vendor specific" services */ >> > #define ARM_SMCCC_KVM_FUNC_FEATURES 0 >> > +#define ARM_SMCCC_KVM_PTP 1 >> > #define ARM_SMCCC_KVM_FUNC_FEATURES_2 127 >> > #define ARM_SMCCC_KVM_NUM_FUNCS 128 >> > >> > @@ -103,6 +104,17 @@ >> > ARM_SMCCC_OWNER_VENDOR_HYP, >> \ >> > ARM_SMCCC_KVM_FUNC_FEATURES) >> > >> > +/* >> > + * This ID used for virtual ptp kvm clock and it will pass second >> > value >> > + * and nanosecond value of host real time and system counter by vcpu >> > + * register to guest. >> > + */ >> > +#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_PTP) >> > + >> >> All of this depends on patches that have never need posted to any ML, >> and >> just linger in Will's tree. You need to pick them up and post them as >> part of >> this series so that they can at least be reviewed. >> > Ok, I will add them next version. > >> > #ifndef __ASSEMBLY__ >> > >> > #include <linux/linkage.h> >> > diff --git a/virt/kvm/arm/psci.c b/virt/kvm/arm/psci.c index >> > 0debf49bf259..682d892d6717 100644 >> > --- a/virt/kvm/arm/psci.c >> > +++ b/virt/kvm/arm/psci.c >> > @@ -9,6 +9,7 @@ >> > #include <linux/kvm_host.h> >> > #include <linux/uaccess.h> >> > #include <linux/wait.h> >> > +#include <linux/clocksource_ids.h> >> > >> > #include <asm/cputype.h> >> > #include <asm/kvm_emulate.h> >> > @@ -389,6 +390,8 @@ static int kvm_psci_call(struct kvm_vcpu *vcpu) >> > >> > int kvm_hvc_call_handler(struct kvm_vcpu *vcpu) { >> > + struct system_time_snapshot systime_snapshot; >> > + u64 cycles; >> > u32 func_id = smccc_get_function(vcpu); >> > u32 val[4] = {}; >> > u32 option; >> > @@ -431,6 +434,25 @@ int kvm_hvc_call_handler(struct kvm_vcpu *vcpu) >> > case ARM_SMCCC_VENDOR_HYP_KVM_FEATURES_FUNC_ID: >> > val[0] = BIT(ARM_SMCCC_KVM_FUNC_FEATURES); >> > break; >> > + /* >> > + * This will used for virtual ptp kvm clock. three >> > + * 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. >> >> That's either two or four values, and not three as you claim above. >> > Sorry, I'm not sure what do you mean "three", the registers here is 4 > from reg0 to reg3. Please read the comment you have written above... >> Also, I fail to understand the meaning of the host cycle vs cntvoff >> comparison. >> This is something that guest can perform on its own (it has access to >> both >> physical and virtual timers, and can compute cntvoff without >> intervention of >> the hypervisor). >> > To keep consistency and precision, clock time and counter cycle must > captured at the same time. It will perform at ktime_get_snapshot. Fair enough. It would vertainly help if you documented it. It would also help if you explained why it is so much worse to read the counter in the guest before *and* after the call, and assume that the clock time read happened right in the middle? That aside, what you are returning is something that *looks* like the virtual counter. What if the guest is using the physical counter, which is likely to be the case with nested virt? Do you expect the guest to always use the virtual counter? This isn't going to fly. >> Finally, how does it work with nested virt, where cntvoff is for the >> the vEL2 >> guest? >> > For now, I have not considered ptp_kvm in nested virtualization. Also > I'm not sure about if nested virtualization is > ready on arm64 , as I need test ptp_kvm on it. If so, I can consider > it. It is not about testing. It is about taking the architecture into account. And ready or not doesn't come into play here. What you're defining here is an ABI, and it better be totally future proof. But if you want to test, help yourself to [1] and have fun! > >> > + */ >> > + case ARM_SMCCC_VENDOR_HYP_KVM_PTP_FUNC_ID: >> > + ktime_get_snapshot(&systime_snapshot); >> > + if (systime_snapshot.cs_id != CSID_ARM_ARCH_COUNTER) >> > + return kvm_psci_call(vcpu); >> >> What does this mean? Calling PSCI because you've failed to identify >> the clock >> source? What result do you expect from this? Hint: this isn't a PSCI >> call. >> > Sorry, what I want to do here is that return to guest with the error > info. > Maybe I should set val[0] to -1 and break to let the guest know that > error comes, as > the guest will check if val[0] is positive to determine the next step. What you should do is handle it like a normal SMCCC failure. M. [1] https://git.kernel.org/pub/scm/linux/kernel/git/maz/arm-platforms.git/log/?h=kvm-arm64/nv-5.5-rc4-WIP
Hi Marc, > -----Original Message----- > From: Marc Zyngier <maz@kernel.org> > Sent: Thursday, January 9, 2020 5:16 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>; Kaly Xin > <Kaly.Xin@arm.com>; Justin He <Justin.He@arm.com>; nd <nd@arm.com> > Subject: Re: [RFC PATCH v9 6/8] psci: Add hvc call service for ptp_kvm. > > On 2020-01-09 05:45, Jianyong Wu wrote: > > Hi Marc, > > > >> -----Original Message----- > >> From: Marc Zyngier <maz@kernel.org> > >> Sent: Tuesday, January 7, 2020 5:16 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>; Kaly Xin <Kaly.Xin@arm.com>; Justin He > >> <Justin.He@arm.com>; nd <nd@arm.com> > >> Subject: Re: [RFC PATCH v9 6/8] psci: Add hvc call service for > >> ptp_kvm. > >> > >> On 2019-12-10 03:40, Jianyong Wu wrote: > >> > ptp_kvm modules will call hvc to get this service. > >> > The service offers real time and counter cycle of host for guest. > >> > > >> > Signed-off-by: Jianyong Wu <jianyong.wu@arm.com> > >> > --- > >> > include/linux/arm-smccc.h | 12 ++++++++++++ > >> > virt/kvm/arm/psci.c | 22 ++++++++++++++++++++++ > >> > 2 files changed, 34 insertions(+) > >> > > >> > diff --git a/include/linux/arm-smccc.h b/include/linux/arm-smccc.h > >> > index 6f82c87308ed..aafb6bac167d 100644 > >> > --- a/include/linux/arm-smccc.h > >> > +++ b/include/linux/arm-smccc.h > >> > @@ -94,6 +94,7 @@ > >> > > >> > /* KVM "vendor specific" services */ > >> > #define ARM_SMCCC_KVM_FUNC_FEATURES 0 > >> > +#define ARM_SMCCC_KVM_PTP 1 > >> > #define ARM_SMCCC_KVM_FUNC_FEATURES_2 127 > >> > #define ARM_SMCCC_KVM_NUM_FUNCS 128 > >> > > >> > @@ -103,6 +104,17 @@ > >> > ARM_SMCCC_OWNER_VENDOR_HYP, > >> \ > >> > ARM_SMCCC_KVM_FUNC_FEATURES) > >> > > >> > +/* > >> > + * This ID used for virtual ptp kvm clock and it will pass second > >> > value > >> > + * and nanosecond value of host real time and system counter by > >> > +vcpu > >> > + * register to guest. > >> > + */ > >> > +#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_PTP) > >> > + > >> > >> All of this depends on patches that have never need posted to any ML, > >> and just linger in Will's tree. You need to pick them up and post > >> them as part of this series so that they can at least be reviewed. > >> > > Ok, I will add them next version. > > > >> > #ifndef __ASSEMBLY__ > >> > > >> > #include <linux/linkage.h> > >> > diff --git a/virt/kvm/arm/psci.c b/virt/kvm/arm/psci.c index > >> > 0debf49bf259..682d892d6717 100644 > >> > --- a/virt/kvm/arm/psci.c > >> > +++ b/virt/kvm/arm/psci.c > >> > @@ -9,6 +9,7 @@ > >> > #include <linux/kvm_host.h> > >> > #include <linux/uaccess.h> > >> > #include <linux/wait.h> > >> > +#include <linux/clocksource_ids.h> > >> > > >> > #include <asm/cputype.h> > >> > #include <asm/kvm_emulate.h> > >> > @@ -389,6 +390,8 @@ static int kvm_psci_call(struct kvm_vcpu *vcpu) > >> > > >> > int kvm_hvc_call_handler(struct kvm_vcpu *vcpu) { > >> > + struct system_time_snapshot systime_snapshot; > >> > + u64 cycles; > >> > u32 func_id = smccc_get_function(vcpu); > >> > u32 val[4] = {}; > >> > u32 option; > >> > @@ -431,6 +434,25 @@ int kvm_hvc_call_handler(struct kvm_vcpu > *vcpu) > >> > case ARM_SMCCC_VENDOR_HYP_KVM_FEATURES_FUNC_ID: > >> > val[0] = BIT(ARM_SMCCC_KVM_FUNC_FEATURES); > >> > break; > >> > + /* > >> > + * This will used for virtual ptp kvm clock. three > >> > + * 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. > >> > >> That's either two or four values, and not three as you claim above. > >> > > Sorry, I'm not sure what do you mean "three", the registers here is 4 > > from reg0 to reg3. > > Please read the comment you have written above... oh, I see it. > > >> Also, I fail to understand the meaning of the host cycle vs cntvoff > >> comparison. > >> This is something that guest can perform on its own (it has access to > >> both physical and virtual timers, and can compute cntvoff without > >> intervention of the hypervisor). > >> > > To keep consistency and precision, clock time and counter cycle must > > captured at the same time. It will perform at ktime_get_snapshot. > > Fair enough. It would vertainly help if you documented it. It would also help if > you explained why it is so much worse to read the counter in the guest > before *and* after the call, and assume that the clock time read happened > right in the middle? > ok, I will give explain in comments. > That aside, what you are returning is something that *looks* like the virtual > counter. What if the guest is using the physical counter, which is likely to be > the case with nested virt? Do you expect the guest to always use the virtual > counter? This isn't going to fly. To be honest, I have little knowledge of nested virtualization for arm and I'm confused with that guest'guest will use physical counter. IMO, ptp_kvm will call hvc to trap to its hypervisor adjacent to it. So guest'guest will trap to hypervisor in guest and will get guest's counter cycle then calculate guest'guest's counter cycle by something like offset to sync time with it. So only if the guest's hypervisor can calculate the guest'guest's counter value, can ptp_kvm works. the implementation of calculating the return value of counter cycle vary with the way deriving counter cycle from hypervisor to guest. If considering nested virt here, we need the basic knowledge of how guest'guest's counter cycle is calculated from its hypervisor and how to determine we are in guest's hypervisor or guest'guest's hypervisor. If it is the case, can you give me some knowledge, something like a document, about that? > > >> Finally, how does it work with nested virt, where cntvoff is for the > >> the vEL2 guest? > >> > > For now, I have not considered ptp_kvm in nested virtualization. Also > > I'm not sure about if nested virtualization is ready on arm64 , as I > > need test ptp_kvm on it. If so, I can consider it. > > It is not about testing. It is about taking the architecture into account. > And ready or not doesn't come into play here. What you're defining here is > an ABI, and it better be totally future proof. > Yeah, should included it in design. > But if you want to test, help yourself to [1] and have fun! > > Thanks > >> > + */ > >> > + case ARM_SMCCC_VENDOR_HYP_KVM_PTP_FUNC_ID: > >> > + ktime_get_snapshot(&systime_snapshot); > >> > + if (systime_snapshot.cs_id != CSID_ARM_ARCH_COUNTER) > >> > + return kvm_psci_call(vcpu); > >> > >> What does this mean? Calling PSCI because you've failed to identify > >> the clock source? What result do you expect from this? Hint: this > >> isn't a PSCI call. > >> > > Sorry, what I want to do here is that return to guest with the error > > info. > > Maybe I should set val[0] to -1 and break to let the guest know that > > error comes, as the guest will check if val[0] is positive to > > determine the next step. > > What you should do is handle it like a normal SMCCC failure. > Yeah, I will fix it. Thanks Jianyong > M. > > [1] > https://git.kernel.org/pub/scm/linux/kernel/git/maz/arm- > platforms.git/log/?h=kvm-arm64/nv-5.5-rc4-WIP > -- > Jazz is not dead. It just smells funny...
On 2020-01-10 09:51, Jianyong Wu wrote: > Hi Marc, > >> -----Original Message----- >> From: Marc Zyngier <maz@kernel.org> >> Sent: Thursday, January 9, 2020 5:16 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>; Kaly Xin >> <Kaly.Xin@arm.com>; Justin He <Justin.He@arm.com>; nd <nd@arm.com> >> Subject: Re: [RFC PATCH v9 6/8] psci: Add hvc call service for >> ptp_kvm. >> >> On 2020-01-09 05:45, Jianyong Wu wrote: >> > Hi Marc, >> > >> >> -----Original Message----- >> >> From: Marc Zyngier <maz@kernel.org> >> >> Sent: Tuesday, January 7, 2020 5:16 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>; Kaly Xin <Kaly.Xin@arm.com>; Justin He >> >> <Justin.He@arm.com>; nd <nd@arm.com> >> >> Subject: Re: [RFC PATCH v9 6/8] psci: Add hvc call service for >> >> ptp_kvm. >> >> >> >> On 2019-12-10 03:40, Jianyong Wu wrote: >> >> > ptp_kvm modules will call hvc to get this service. >> >> > The service offers real time and counter cycle of host for guest. >> >> > >> >> > Signed-off-by: Jianyong Wu <jianyong.wu@arm.com> >> >> > --- >> >> > include/linux/arm-smccc.h | 12 ++++++++++++ >> >> > virt/kvm/arm/psci.c | 22 ++++++++++++++++++++++ >> >> > 2 files changed, 34 insertions(+) >> >> > >> >> > diff --git a/include/linux/arm-smccc.h b/include/linux/arm-smccc.h >> >> > index 6f82c87308ed..aafb6bac167d 100644 >> >> > --- a/include/linux/arm-smccc.h >> >> > +++ b/include/linux/arm-smccc.h >> >> > @@ -94,6 +94,7 @@ >> >> > >> >> > /* KVM "vendor specific" services */ >> >> > #define ARM_SMCCC_KVM_FUNC_FEATURES 0 >> >> > +#define ARM_SMCCC_KVM_PTP 1 >> >> > #define ARM_SMCCC_KVM_FUNC_FEATURES_2 127 >> >> > #define ARM_SMCCC_KVM_NUM_FUNCS 128 >> >> > >> >> > @@ -103,6 +104,17 @@ >> >> > ARM_SMCCC_OWNER_VENDOR_HYP, >> >> \ >> >> > ARM_SMCCC_KVM_FUNC_FEATURES) >> >> > >> >> > +/* >> >> > + * This ID used for virtual ptp kvm clock and it will pass second >> >> > value >> >> > + * and nanosecond value of host real time and system counter by >> >> > +vcpu >> >> > + * register to guest. >> >> > + */ >> >> > +#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_PTP) >> >> > + >> >> >> >> All of this depends on patches that have never need posted to any ML, >> >> and just linger in Will's tree. You need to pick them up and post >> >> them as part of this series so that they can at least be reviewed. >> >> >> > Ok, I will add them next version. >> > >> >> > #ifndef __ASSEMBLY__ >> >> > >> >> > #include <linux/linkage.h> >> >> > diff --git a/virt/kvm/arm/psci.c b/virt/kvm/arm/psci.c index >> >> > 0debf49bf259..682d892d6717 100644 >> >> > --- a/virt/kvm/arm/psci.c >> >> > +++ b/virt/kvm/arm/psci.c >> >> > @@ -9,6 +9,7 @@ >> >> > #include <linux/kvm_host.h> >> >> > #include <linux/uaccess.h> >> >> > #include <linux/wait.h> >> >> > +#include <linux/clocksource_ids.h> >> >> > >> >> > #include <asm/cputype.h> >> >> > #include <asm/kvm_emulate.h> >> >> > @@ -389,6 +390,8 @@ static int kvm_psci_call(struct kvm_vcpu *vcpu) >> >> > >> >> > int kvm_hvc_call_handler(struct kvm_vcpu *vcpu) { >> >> > + struct system_time_snapshot systime_snapshot; >> >> > + u64 cycles; >> >> > u32 func_id = smccc_get_function(vcpu); >> >> > u32 val[4] = {}; >> >> > u32 option; >> >> > @@ -431,6 +434,25 @@ int kvm_hvc_call_handler(struct kvm_vcpu >> *vcpu) >> >> > case ARM_SMCCC_VENDOR_HYP_KVM_FEATURES_FUNC_ID: >> >> > val[0] = BIT(ARM_SMCCC_KVM_FUNC_FEATURES); >> >> > break; >> >> > + /* >> >> > + * This will used for virtual ptp kvm clock. three >> >> > + * 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. >> >> >> >> That's either two or four values, and not three as you claim above. >> >> >> > Sorry, I'm not sure what do you mean "three", the registers here is 4 >> > from reg0 to reg3. >> >> Please read the comment you have written above... > > oh, I see it. > >> >> >> Also, I fail to understand the meaning of the host cycle vs cntvoff >> >> comparison. >> >> This is something that guest can perform on its own (it has access to >> >> both physical and virtual timers, and can compute cntvoff without >> >> intervention of the hypervisor). >> >> >> > To keep consistency and precision, clock time and counter cycle must >> > captured at the same time. It will perform at ktime_get_snapshot. >> >> Fair enough. It would vertainly help if you documented it. It would >> also help if >> you explained why it is so much worse to read the counter in the guest >> before *and* after the call, and assume that the clock time read >> happened >> right in the middle? >> > ok, I will give explain in comments. > >> That aside, what you are returning is something that *looks* like the >> virtual >> counter. What if the guest is using the physical counter, which is >> likely to be >> the case with nested virt? Do you expect the guest to always use the >> virtual >> counter? This isn't going to fly. > > To be honest, I have little knowledge of nested virtualization for arm > and I'm confused with that guest'guest will use physical counter. Not the guest's guest (L2), but L1. Just look at what counter the KVM host uses: that's the physical counter. Now imagine you run that host as a guest, no other change. > IMO, ptp_kvm will call hvc to trap to its hypervisor adjacent to it. > So guest'guest will trap to hypervisor in guest and will > get guest's counter cycle then calculate guest'guest's counter cycle > by something like offset to sync time with it. So only if the > guest's hypervisor can calculate the guest'guest's counter value, can > ptp_kvm works. Sure, but that's not the problem we're trying to solve. The issue is that of the reference counter value you're including in the hypercall response. It needs to be a value that makes sense to the guest, and so far you're assuming virtual. NV breaks that assumtion, because the guest hypervisor is using the physical counter. Also, let's not forget that the hypercall isn't Linux specific. I can write my own non-Linux guest and still use this hypercall. Nothing in there says that I can't use the physical counter if I want to. So somehow, you need to convey the the hypervisor the notion of *which* counter the guest uses. Does it make sense? Or am I missing something? Thanks, M.
Hi Marc, > -----Original Message----- > From: Marc Zyngier <maz@kernel.org> > Sent: Friday, January 10, 2020 6:56 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>; Kaly Xin > <Kaly.Xin@arm.com>; Justin He <Justin.He@arm.com>; nd <nd@arm.com> > Subject: Re: [RFC PATCH v9 6/8] psci: Add hvc call service for ptp_kvm. > > On 2020-01-10 09:51, Jianyong Wu wrote: > > Hi Marc, > > > >> -----Original Message----- > >> From: Marc Zyngier <maz@kernel.org> > >> Sent: Thursday, January 9, 2020 5:16 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>; Kaly Xin <Kaly.Xin@arm.com>; Justin He > >> <Justin.He@arm.com>; nd <nd@arm.com> > >> Subject: Re: [RFC PATCH v9 6/8] psci: Add hvc call service for > >> ptp_kvm. > >> > >> On 2020-01-09 05:45, Jianyong Wu wrote: > >> > Hi Marc, > >> > > >> >> -----Original Message----- > >> >> From: Marc Zyngier <maz@kernel.org> > >> >> Sent: Tuesday, January 7, 2020 5:16 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>; Kaly > Xin > >> >> <Kaly.Xin@arm.com>; Justin He <Justin.He@arm.com>; nd > <nd@arm.com> > >> >> Subject: Re: [RFC PATCH v9 6/8] psci: Add hvc call service for > >> >> ptp_kvm. > >> >> > >> >> On 2019-12-10 03:40, Jianyong Wu wrote: > >> >> > ptp_kvm modules will call hvc to get this service. > >> >> > The service offers real time and counter cycle of host for guest. > >> >> > > >> >> > Signed-off-by: Jianyong Wu <jianyong.wu@arm.com> > >> >> > --- > >> >> > include/linux/arm-smccc.h | 12 ++++++++++++ > >> >> > virt/kvm/arm/psci.c | 22 ++++++++++++++++++++++ > >> >> > 2 files changed, 34 insertions(+) > >> >> > > >> >> > diff --git a/include/linux/arm-smccc.h > >> >> > b/include/linux/arm-smccc.h index 6f82c87308ed..aafb6bac167d > >> >> > 100644 > >> >> > --- a/include/linux/arm-smccc.h > >> >> > +++ b/include/linux/arm-smccc.h > >> >> > @@ -94,6 +94,7 @@ > >> >> > > >> >> > /* KVM "vendor specific" services */ > >> >> > #define ARM_SMCCC_KVM_FUNC_FEATURES 0 > >> >> > +#define ARM_SMCCC_KVM_PTP 1 > >> >> > #define ARM_SMCCC_KVM_FUNC_FEATURES_2 127 > >> >> > #define ARM_SMCCC_KVM_NUM_FUNCS 128 > >> >> > > >> >> > @@ -103,6 +104,17 @@ > >> >> > ARM_SMCCC_OWNER_VENDOR_HYP, > >> >> \ > >> >> > ARM_SMCCC_KVM_FUNC_FEATURES) > >> >> > > >> >> > +/* > >> >> > + * This ID used for virtual ptp kvm clock and it will pass > >> >> > +second > >> >> > value > >> >> > + * and nanosecond value of host real time and system counter by > >> >> > +vcpu > >> >> > + * register to guest. > >> >> > + */ > >> >> > +#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_PTP) > >> >> > + > >> >> > >> >> All of this depends on patches that have never need posted to any > >> >> ML, and just linger in Will's tree. You need to pick them up and > >> >> post them as part of this series so that they can at least be reviewed. > >> >> > >> > Ok, I will add them next version. > >> > > >> >> > #ifndef __ASSEMBLY__ > >> >> > > >> >> > #include <linux/linkage.h> > >> >> > diff --git a/virt/kvm/arm/psci.c b/virt/kvm/arm/psci.c index > >> >> > 0debf49bf259..682d892d6717 100644 > >> >> > --- a/virt/kvm/arm/psci.c > >> >> > +++ b/virt/kvm/arm/psci.c > >> >> > @@ -9,6 +9,7 @@ > >> >> > #include <linux/kvm_host.h> > >> >> > #include <linux/uaccess.h> > >> >> > #include <linux/wait.h> > >> >> > +#include <linux/clocksource_ids.h> > >> >> > > >> >> > #include <asm/cputype.h> > >> >> > #include <asm/kvm_emulate.h> > >> >> > @@ -389,6 +390,8 @@ static int kvm_psci_call(struct kvm_vcpu > >> >> > *vcpu) > >> >> > > >> >> > int kvm_hvc_call_handler(struct kvm_vcpu *vcpu) { > >> >> > + struct system_time_snapshot systime_snapshot; > >> >> > + u64 cycles; > >> >> > u32 func_id = smccc_get_function(vcpu); > >> >> > u32 val[4] = {}; > >> >> > u32 option; > >> >> > @@ -431,6 +434,25 @@ int kvm_hvc_call_handler(struct kvm_vcpu > >> *vcpu) > >> >> > case > ARM_SMCCC_VENDOR_HYP_KVM_FEATURES_FUNC_ID: > >> >> > val[0] = BIT(ARM_SMCCC_KVM_FUNC_FEATURES); > >> >> > break; > >> >> > + /* > >> >> > + * This will used for virtual ptp kvm clock. three > >> >> > + * 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. > >> >> > >> >> That's either two or four values, and not three as you claim above. > >> >> > >> > Sorry, I'm not sure what do you mean "three", the registers here is > >> > 4 from reg0 to reg3. > >> > >> Please read the comment you have written above... > > > > oh, I see it. > > > >> > >> >> Also, I fail to understand the meaning of the host cycle vs > >> >> cntvoff comparison. > >> >> This is something that guest can perform on its own (it has access > >> >> to both physical and virtual timers, and can compute cntvoff > >> >> without intervention of the hypervisor). > >> >> > >> > To keep consistency and precision, clock time and counter cycle > >> > must captured at the same time. It will perform at ktime_get_snapshot. > >> > >> Fair enough. It would vertainly help if you documented it. It would > >> also help if you explained why it is so much worse to read the > >> counter in the guest before *and* after the call, and assume that the > >> clock time read happened right in the middle? > >> > > ok, I will give explain in comments. > > > >> That aside, what you are returning is something that *looks* like the > >> virtual counter. What if the guest is using the physical counter, > >> which is likely to be the case with nested virt? Do you expect the > >> guest to always use the virtual counter? This isn't going to fly. > > > > To be honest, I have little knowledge of nested virtualization for arm > > and I'm confused with that guest'guest will use physical counter. > > Not the guest's guest (L2), but L1. Just look at what counter the KVM host > uses: that's the physical counter. Now imagine you run that host as a guest, > no other change. > Ok, > > IMO, ptp_kvm will call hvc to trap to its hypervisor adjacent to it. > > So guest'guest will trap to hypervisor in guest and will get guest's > > counter cycle then calculate guest'guest's counter cycle by something > > like offset to sync time with it. So only if the guest's hypervisor > > can calculate the guest'guest's counter value, can ptp_kvm works. > > Sure, but that's not the problem we're trying to solve. The issue is that of the > reference counter value you're including in the hypercall response. > It needs to be a value that makes sense to the guest, and so far you're > assuming virtual. > Get it. > NV breaks that assumtion, because the guest hypervisor is using the physical > counter. Also, let's not forget that the hypercall isn't Linux specific. > I can write my own non-Linux guest and still use this hypercall. Nothing in > there says that I can't use the physical counter if I want to. > > So somehow, you need to convey the the hypervisor the notion of *which* > counter the guest uses. > > Does it make sense? Or am I missing something? > I know what you say. Let me try to solve this problem. Step 0, summary out all the conditions we should process, which will sever as branch condition.(now only normal virt and nested virt, I think) Step 1, figure out the set of reference counter value used by guest in all condition. Step 2, determine which reference counter value will be used by guest in a certain condition in hypercall. In step 1, can we give the set only 2 elements that one is physical counter the other is virtual counter? For step 2, I have no idea for that now. can you give me some hint about it? Thanks Jianyong > Thanks, > > M. > -- > Jazz is not dead. It just smells funny...
Hi Jianyong, On 2020-01-13 10:30, Jianyong Wu wrote: > Hi Marc, > >> -----Original Message----- >> From: Marc Zyngier <maz@kernel.org> >> Sent: Friday, January 10, 2020 6:56 PM >> NV breaks that assumtion, because the guest hypervisor is using the >> physical >> counter. Also, let's not forget that the hypercall isn't Linux >> specific. >> I can write my own non-Linux guest and still use this hypercall. >> Nothing in >> there says that I can't use the physical counter if I want to. >> >> So somehow, you need to convey the the hypervisor the notion of >> *which* >> counter the guest uses. >> >> Does it make sense? Or am I missing something? >> > I know what you say. Let me try to solve this problem. > Step 0, summary out all the conditions we should process, which will > sever as branch condition.(now only normal virt and nested virt, I > think) No. You shouldn't think of the various use cases, but of which time references a guest can use. You don't need nested virt to use the physical counter, for example. > Step 1, figure out the set of reference counter value used by guest > in all condition. That should be for the guest to tell you when it calls into the PV service. > Step 2, determine which reference counter value will be used by guest > in a certain condition in hypercall. > In step 1, can we give the set only 2 elements that one is physical > counter the other is virtual counter? I don't think returning the two values is useful. Just return what the guest asks for. > For step 2, I have no idea for that now. can you give me some hint > about it? Just expand your SMC call to take a parameter indicating the reference counter, and return the sampled (or computed) value corresponding to that counter. M.
Hi Marc, > -----Original Message----- > From: Marc Zyngier <maz@kernel.org> > Sent: Monday, January 13, 2020 7:16 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>; Kaly Xin > <Kaly.Xin@arm.com>; Justin He <Justin.He@arm.com>; nd <nd@arm.com> > Subject: Re: [RFC PATCH v9 6/8] psci: Add hvc call service for ptp_kvm. > > Hi Jianyong, > > On 2020-01-13 10:30, Jianyong Wu wrote: > > Hi Marc, > > > >> -----Original Message----- > >> From: Marc Zyngier <maz@kernel.org> > >> Sent: Friday, January 10, 2020 6:56 PM NV breaks that assumtion, > >> because the guest hypervisor is using the physical counter. Also, > >> let's not forget that the hypercall isn't Linux specific. > >> I can write my own non-Linux guest and still use this hypercall. > >> Nothing in > >> there says that I can't use the physical counter if I want to. > >> > >> So somehow, you need to convey the the hypervisor the notion of > >> *which* > >> counter the guest uses. > >> > >> Does it make sense? Or am I missing something? > >> > > I know what you say. Let me try to solve this problem. > > Step 0, summary out all the conditions we should process, which will > > sever as branch condition.(now only normal virt and nested virt, I > > think) > > No. You shouldn't think of the various use cases, but of which time > references a guest can use. You don't need nested virt to use the physical > counter, for example. Ok, > > > Step 1, figure out the set of reference counter value used by guest > > in all condition. > > That should be for the guest to tell you when it calls into the PV service. > Yeah > > Step 2, determine which reference counter value will be used by > guest > > in a certain condition in hypercall. > > In step 1, can we give the set only 2 elements that one is physical > > counter the other is virtual counter? > > I don't think returning the two values is useful. Just return what the guest > asks for. > > > For step 2, I have no idea for that now. can you give me some hint > > about it? > > Just expand your SMC call to take a parameter indicating the reference > counter, and return the sampled (or computed) value corresponding to that > counter. Get it, I'll try it. Thanks Jianyong > > M. > -- > Jazz is not dead. It just smells funny...
diff --git a/include/linux/arm-smccc.h b/include/linux/arm-smccc.h index 6f82c87308ed..aafb6bac167d 100644 --- a/include/linux/arm-smccc.h +++ b/include/linux/arm-smccc.h @@ -94,6 +94,7 @@ /* KVM "vendor specific" services */ #define ARM_SMCCC_KVM_FUNC_FEATURES 0 +#define ARM_SMCCC_KVM_PTP 1 #define ARM_SMCCC_KVM_FUNC_FEATURES_2 127 #define ARM_SMCCC_KVM_NUM_FUNCS 128 @@ -103,6 +104,17 @@ ARM_SMCCC_OWNER_VENDOR_HYP, \ ARM_SMCCC_KVM_FUNC_FEATURES) +/* + * This ID used for virtual ptp kvm clock and it will pass second value + * and nanosecond value of host real time and system counter by vcpu + * register to guest. + */ +#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_PTP) + #ifndef __ASSEMBLY__ #include <linux/linkage.h> diff --git a/virt/kvm/arm/psci.c b/virt/kvm/arm/psci.c index 0debf49bf259..682d892d6717 100644 --- a/virt/kvm/arm/psci.c +++ b/virt/kvm/arm/psci.c @@ -9,6 +9,7 @@ #include <linux/kvm_host.h> #include <linux/uaccess.h> #include <linux/wait.h> +#include <linux/clocksource_ids.h> #include <asm/cputype.h> #include <asm/kvm_emulate.h> @@ -389,6 +390,8 @@ static int kvm_psci_call(struct kvm_vcpu *vcpu) int kvm_hvc_call_handler(struct kvm_vcpu *vcpu) { + struct system_time_snapshot systime_snapshot; + u64 cycles; u32 func_id = smccc_get_function(vcpu); u32 val[4] = {}; u32 option; @@ -431,6 +434,25 @@ int kvm_hvc_call_handler(struct kvm_vcpu *vcpu) case ARM_SMCCC_VENDOR_HYP_KVM_FEATURES_FUNC_ID: val[0] = BIT(ARM_SMCCC_KVM_FUNC_FEATURES); break; + /* + * This will used for virtual ptp kvm clock. three + * 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: + ktime_get_snapshot(&systime_snapshot); + if (systime_snapshot.cs_id != CSID_ARM_ARCH_COUNTER) + return kvm_psci_call(vcpu); + val[0] = systime_snapshot.real >> 32; + val[1] = systime_snapshot.real << 32 >> 32; + cycles = systime_snapshot.cycles - + vcpu_vtimer(vcpu)->cntvoff; + val[2] = cycles >> 32; + val[3] = cycles << 32 >> 32; + break; default: return kvm_psci_call(vcpu); }
ptp_kvm modules will call hvc to get this service. The service offers real time and counter cycle of host for guest. Signed-off-by: Jianyong Wu <jianyong.wu@arm.com> --- include/linux/arm-smccc.h | 12 ++++++++++++ virt/kvm/arm/psci.c | 22 ++++++++++++++++++++++ 2 files changed, 34 insertions(+)