KVM Archive on lore.kernel.org
 help / color / Atom feed
* [PATCH v19 0/7] KVM: arm64: Add host/guest KVM-PTP support
@ 2021-03-30 14:54 Marc Zyngier
  2021-03-30 14:54 ` [PATCH v19 1/7] arm/arm64: Probe for the presence of KVM hypervisor Marc Zyngier
                   ` (6 more replies)
  0 siblings, 7 replies; 25+ messages in thread
From: Marc Zyngier @ 2021-03-30 14:54 UTC (permalink / raw)
  To: netdev, yangbo.lu, john.stultz, tglx, pbonzini, seanjc,
	richardcochran, Mark.Rutland, will, suzuki.poulose,
	Andre.Przywara, steven.price, lorenzo.pieralisi, sudeep.holla
  Cc: linux-kernel, linux-arm-kernel, kvmarm, kvm, Steve.Capper,
	justin.he, jianyong.wu, kernel-team

Given that this series[0] has languished in my Inbox for the best of the
past two years, and in an effort to eventually get it merged, I've
taken the liberty to pick it up and do the changes I wanted to see
instead of waiting to go through yet another round.

All the patches have a link to their original counterpart (though I
have squashed a couple of them where it made sense). Tested both 64
and 32bit guests for a good measure. Of course, I claim full
responsibility for any bug introduced here.

Unless someone screams now, this is going in 5.13, because I'm frankly
fed up with it! ;-)

* From v18 [2]
  - Fix kvm_hypercall2() return type
  - Rebased on top of 5.12-rc3
  - Added RBs

* From v17 [1]:
  - Fixed compilation issue on 32bit systems not selecting
    CONFIG_HAVE_ARM_SMCCC_DISCOVERY
  - Fixed KVM service discovery not properly parsing the reply
    from the hypervisor

* From v16 [0]:
  - Moved the KVM service discovery to its own file, plugged it into
    PSCI instead of the arch code, dropped the inlining, made use of
    asm/hypervisor.h.
  - Tidied-up the namespacing
  - Cleanup the hypercall handler
  - De-duplicate the guest code
  - Tidied-up arm64-specific documentation
  - Dropped the generic PTP documentation as it needs a new location,
    and some cleanup
  - Squashed hypercall documentation and capability into the
    main KVM patch
  - Rebased on top of 5.11-rc4

[0] https://lore.kernel.org/r/20201209060932.212364-1-jianyong.wu@arm.com
[1] https://lore.kernel.org/r/20210202141204.3134855-1-maz@kernel.org
[2] https://lore.kernel.org/r/20210208134029.3269384-1-maz@kernel.org

Jianyong Wu (4):
  ptp: Reorganize ptp_kvm.c to make it arch-independent
  clocksource: Add clocksource id for arm arch counter
  KVM: arm64: Add support for the KVM PTP service
  ptp: arm/arm64: Enable ptp_kvm for arm/arm64

Thomas Gleixner (1):
  time: Add mechanism to recognize clocksource in time_get_snapshot

Will Deacon (2):
  arm/arm64: Probe for the presence of KVM hypervisor
  KVM: arm64: Advertise KVM UID to guests via SMCCC

 Documentation/virt/kvm/api.rst              | 10 +++
 Documentation/virt/kvm/arm/index.rst        |  1 +
 Documentation/virt/kvm/arm/ptp_kvm.rst      | 25 ++++++
 arch/arm/include/asm/hypervisor.h           |  3 +
 arch/arm64/include/asm/hypervisor.h         |  3 +
 arch/arm64/kvm/arm.c                        |  1 +
 arch/arm64/kvm/hypercalls.c                 | 80 +++++++++++++++--
 drivers/clocksource/arm_arch_timer.c        | 36 ++++++++
 drivers/firmware/psci/psci.c                |  2 +
 drivers/firmware/smccc/Makefile             |  2 +-
 drivers/firmware/smccc/kvm_guest.c          | 50 +++++++++++
 drivers/firmware/smccc/smccc.c              |  1 +
 drivers/ptp/Kconfig                         |  2 +-
 drivers/ptp/Makefile                        |  2 +
 drivers/ptp/ptp_kvm_arm.c                   | 28 ++++++
 drivers/ptp/{ptp_kvm.c => ptp_kvm_common.c} | 84 +++++-------------
 drivers/ptp/ptp_kvm_x86.c                   | 97 +++++++++++++++++++++
 include/linux/arm-smccc.h                   | 41 +++++++++
 include/linux/clocksource.h                 |  6 ++
 include/linux/clocksource_ids.h             | 12 +++
 include/linux/ptp_kvm.h                     | 19 ++++
 include/linux/timekeeping.h                 | 12 +--
 include/uapi/linux/kvm.h                    |  1 +
 kernel/time/clocksource.c                   |  2 +
 kernel/time/timekeeping.c                   |  1 +
 25 files changed, 443 insertions(+), 78 deletions(-)
 create mode 100644 Documentation/virt/kvm/arm/ptp_kvm.rst
 create mode 100644 drivers/firmware/smccc/kvm_guest.c
 create mode 100644 drivers/ptp/ptp_kvm_arm.c
 rename drivers/ptp/{ptp_kvm.c => ptp_kvm_common.c} (60%)
 create mode 100644 drivers/ptp/ptp_kvm_x86.c
 create mode 100644 include/linux/clocksource_ids.h
 create mode 100644 include/linux/ptp_kvm.h

-- 
2.29.2


^ permalink raw reply	[flat|nested] 25+ messages in thread

* [PATCH v19 1/7] arm/arm64: Probe for the presence of KVM hypervisor
  2021-03-30 14:54 [PATCH v19 0/7] KVM: arm64: Add host/guest KVM-PTP support Marc Zyngier
@ 2021-03-30 14:54 ` Marc Zyngier
  2021-04-17  9:49   ` Zenghui Yu
  2021-03-30 14:54 ` [PATCH v19 2/7] KVM: arm64: Advertise KVM UID to guests via SMCCC Marc Zyngier
                   ` (5 subsequent siblings)
  6 siblings, 1 reply; 25+ messages in thread
From: Marc Zyngier @ 2021-03-30 14:54 UTC (permalink / raw)
  To: netdev, yangbo.lu, john.stultz, tglx, pbonzini, seanjc,
	richardcochran, Mark.Rutland, will, suzuki.poulose,
	Andre.Przywara, steven.price, lorenzo.pieralisi, sudeep.holla
  Cc: linux-kernel, linux-arm-kernel, kvmarm, kvm, Steve.Capper,
	justin.he, jianyong.wu, kernel-team

From: Will Deacon <will@kernel.org>

Although the SMCCC specification provides some limited functionality for
describing the presence of hypervisor and firmware services, this is
generally applicable only to functions designated as "Arm Architecture
Service Functions" and no portable discovery mechanism is provided for
standard hypervisor services, despite having a designated range of
function identifiers reserved by the specification.

In an attempt to avoid the need for additional firmware changes every
time a new function is added, introduce a UID to identify the service
provider as being compatible with KVM. Once this has been established,
additional services can be discovered via a feature bitmap.

Reviewed-by: Steven Price <steven.price@arm.com>
Signed-off-by: Will Deacon <will@kernel.org>
Signed-off-by: Jianyong Wu <jianyong.wu@arm.com>
[maz: move code to its own file, plug it into PSCI]
Signed-off-by: Marc Zyngier <maz@kernel.org>
Link: https://lore.kernel.org/r/20201209060932.212364-2-jianyong.wu@arm.com
---
 arch/arm/include/asm/hypervisor.h   |  3 ++
 arch/arm64/include/asm/hypervisor.h |  3 ++
 drivers/firmware/psci/psci.c        |  2 ++
 drivers/firmware/smccc/Makefile     |  2 +-
 drivers/firmware/smccc/kvm_guest.c  | 50 +++++++++++++++++++++++++++++
 drivers/firmware/smccc/smccc.c      |  1 +
 include/linux/arm-smccc.h           | 25 +++++++++++++++
 7 files changed, 85 insertions(+), 1 deletion(-)
 create mode 100644 drivers/firmware/smccc/kvm_guest.c

diff --git a/arch/arm/include/asm/hypervisor.h b/arch/arm/include/asm/hypervisor.h
index df8524365637..bd61502b9715 100644
--- a/arch/arm/include/asm/hypervisor.h
+++ b/arch/arm/include/asm/hypervisor.h
@@ -4,4 +4,7 @@
 
 #include <asm/xen/hypervisor.h>
 
+void kvm_init_hyp_services(void);
+bool kvm_arm_hyp_service_available(u32 func_id);
+
 #endif
diff --git a/arch/arm64/include/asm/hypervisor.h b/arch/arm64/include/asm/hypervisor.h
index f9cc1d021791..0ae427f352c8 100644
--- a/arch/arm64/include/asm/hypervisor.h
+++ b/arch/arm64/include/asm/hypervisor.h
@@ -4,4 +4,7 @@
 
 #include <asm/xen/hypervisor.h>
 
+void kvm_init_hyp_services(void);
+bool kvm_arm_hyp_service_available(u32 func_id);
+
 #endif
diff --git a/drivers/firmware/psci/psci.c b/drivers/firmware/psci/psci.c
index f5fc429cae3f..69e296f02902 100644
--- a/drivers/firmware/psci/psci.c
+++ b/drivers/firmware/psci/psci.c
@@ -23,6 +23,7 @@
 
 #include <asm/cpuidle.h>
 #include <asm/cputype.h>
+#include <asm/hypervisor.h>
 #include <asm/system_misc.h>
 #include <asm/smp_plat.h>
 #include <asm/suspend.h>
@@ -498,6 +499,7 @@ static int __init psci_probe(void)
 		psci_init_cpu_suspend();
 		psci_init_system_suspend();
 		psci_init_system_reset2();
+		kvm_init_hyp_services();
 	}
 
 	return 0;
diff --git a/drivers/firmware/smccc/Makefile b/drivers/firmware/smccc/Makefile
index 72ab84042832..40d19144a860 100644
--- a/drivers/firmware/smccc/Makefile
+++ b/drivers/firmware/smccc/Makefile
@@ -1,4 +1,4 @@
 # SPDX-License-Identifier: GPL-2.0
 #
-obj-$(CONFIG_HAVE_ARM_SMCCC_DISCOVERY)	+= smccc.o
+obj-$(CONFIG_HAVE_ARM_SMCCC_DISCOVERY)	+= smccc.o kvm_guest.o
 obj-$(CONFIG_ARM_SMCCC_SOC_ID)	+= soc_id.o
diff --git a/drivers/firmware/smccc/kvm_guest.c b/drivers/firmware/smccc/kvm_guest.c
new file mode 100644
index 000000000000..08836f2f39ee
--- /dev/null
+++ b/drivers/firmware/smccc/kvm_guest.c
@@ -0,0 +1,50 @@
+// SPDX-License-Identifier: GPL-2.0
+
+#define pr_fmt(fmt) "smccc: KVM: " fmt
+
+#include <linux/arm-smccc.h>
+#include <linux/bitmap.h>
+#include <linux/kernel.h>
+#include <linux/string.h>
+
+#include <asm/hypervisor.h>
+
+static DECLARE_BITMAP(__kvm_arm_hyp_services, ARM_SMCCC_KVM_NUM_FUNCS) __ro_after_init = { };
+
+void __init kvm_init_hyp_services(void)
+{
+	struct arm_smccc_res res;
+	u32 val[4];
+
+	if (arm_smccc_1_1_get_conduit() != SMCCC_CONDUIT_HVC)
+		return;
+
+	arm_smccc_1_1_invoke(ARM_SMCCC_VENDOR_HYP_CALL_UID_FUNC_ID, &res);
+	if (res.a0 != ARM_SMCCC_VENDOR_HYP_UID_KVM_REG_0 ||
+	    res.a1 != ARM_SMCCC_VENDOR_HYP_UID_KVM_REG_1 ||
+	    res.a2 != ARM_SMCCC_VENDOR_HYP_UID_KVM_REG_2 ||
+	    res.a3 != ARM_SMCCC_VENDOR_HYP_UID_KVM_REG_3)
+		return;
+
+	memset(&res, 0, sizeof(res));
+	arm_smccc_1_1_invoke(ARM_SMCCC_VENDOR_HYP_KVM_FEATURES_FUNC_ID, &res);
+
+	val[0] = lower_32_bits(res.a0);
+	val[1] = lower_32_bits(res.a1);
+	val[2] = lower_32_bits(res.a2);
+	val[3] = lower_32_bits(res.a3);
+
+	bitmap_from_arr32(__kvm_arm_hyp_services, val, ARM_SMCCC_KVM_NUM_FUNCS);
+
+	pr_info("hypervisor services detected (0x%08lx 0x%08lx 0x%08lx 0x%08lx)\n",
+		 res.a3, res.a2, res.a1, res.a0);
+}
+
+bool kvm_arm_hyp_service_available(u32 func_id)
+{
+	if (func_id >= ARM_SMCCC_KVM_NUM_FUNCS)
+		return -EINVAL;
+
+	return test_bit(func_id, __kvm_arm_hyp_services);
+}
+EXPORT_SYMBOL_GPL(kvm_arm_hyp_service_available);
diff --git a/drivers/firmware/smccc/smccc.c b/drivers/firmware/smccc/smccc.c
index d52bfc5ed5e4..028f81d702cc 100644
--- a/drivers/firmware/smccc/smccc.c
+++ b/drivers/firmware/smccc/smccc.c
@@ -8,6 +8,7 @@
 #include <linux/cache.h>
 #include <linux/init.h>
 #include <linux/arm-smccc.h>
+#include <linux/kernel.h>
 #include <asm/archrandom.h>
 
 static u32 smccc_version = ARM_SMCCC_VERSION_1_0;
diff --git a/include/linux/arm-smccc.h b/include/linux/arm-smccc.h
index 62c54234576c..1a27bd9493fe 100644
--- a/include/linux/arm-smccc.h
+++ b/include/linux/arm-smccc.h
@@ -55,6 +55,8 @@
 #define ARM_SMCCC_OWNER_TRUSTED_OS	50
 #define ARM_SMCCC_OWNER_TRUSTED_OS_END	63
 
+#define ARM_SMCCC_FUNC_QUERY_CALL_UID  0xff01
+
 #define ARM_SMCCC_QUIRK_NONE		0
 #define ARM_SMCCC_QUIRK_QCOM_A6		1 /* Save/restore register a6 */
 
@@ -87,6 +89,29 @@
 			   ARM_SMCCC_SMC_32,				\
 			   0, 0x7fff)
 
+#define ARM_SMCCC_VENDOR_HYP_CALL_UID_FUNC_ID				\
+	ARM_SMCCC_CALL_VAL(ARM_SMCCC_FAST_CALL,				\
+			   ARM_SMCCC_SMC_32,				\
+			   ARM_SMCCC_OWNER_VENDOR_HYP,			\
+			   ARM_SMCCC_FUNC_QUERY_CALL_UID)
+
+/* KVM UID value: 28b46fb6-2ec5-11e9-a9ca-4b564d003a74 */
+#define ARM_SMCCC_VENDOR_HYP_UID_KVM_REG_0	0xb66fb428U
+#define ARM_SMCCC_VENDOR_HYP_UID_KVM_REG_1	0xe911c52eU
+#define ARM_SMCCC_VENDOR_HYP_UID_KVM_REG_2	0x564bcaa9U
+#define ARM_SMCCC_VENDOR_HYP_UID_KVM_REG_3	0x743a004dU
+
+/* KVM "vendor specific" services */
+#define ARM_SMCCC_KVM_FUNC_FEATURES		0
+#define ARM_SMCCC_KVM_FUNC_FEATURES_2		127
+#define ARM_SMCCC_KVM_NUM_FUNCS			128
+
+#define ARM_SMCCC_VENDOR_HYP_KVM_FEATURES_FUNC_ID			\
+	ARM_SMCCC_CALL_VAL(ARM_SMCCC_FAST_CALL,				\
+			   ARM_SMCCC_SMC_32,				\
+			   ARM_SMCCC_OWNER_VENDOR_HYP,			\
+			   ARM_SMCCC_KVM_FUNC_FEATURES)
+
 #define SMCCC_ARCH_WORKAROUND_RET_UNAFFECTED	1
 
 /* Paravirtualised time calls (defined by ARM DEN0057A) */
-- 
2.29.2


^ permalink raw reply	[flat|nested] 25+ messages in thread

* [PATCH v19 2/7] KVM: arm64: Advertise KVM UID to guests via SMCCC
  2021-03-30 14:54 [PATCH v19 0/7] KVM: arm64: Add host/guest KVM-PTP support Marc Zyngier
  2021-03-30 14:54 ` [PATCH v19 1/7] arm/arm64: Probe for the presence of KVM hypervisor Marc Zyngier
@ 2021-03-30 14:54 ` Marc Zyngier
  2021-03-30 14:54 ` [PATCH v19 3/7] ptp: Reorganize ptp_kvm.c to make it arch-independent Marc Zyngier
                   ` (4 subsequent siblings)
  6 siblings, 0 replies; 25+ messages in thread
From: Marc Zyngier @ 2021-03-30 14:54 UTC (permalink / raw)
  To: netdev, yangbo.lu, john.stultz, tglx, pbonzini, seanjc,
	richardcochran, Mark.Rutland, will, suzuki.poulose,
	Andre.Przywara, steven.price, lorenzo.pieralisi, sudeep.holla
  Cc: linux-kernel, linux-arm-kernel, kvmarm, kvm, Steve.Capper,
	justin.he, jianyong.wu, kernel-team

From: Will Deacon <will@kernel.org>

We can advertise ourselves to guests as KVM and provide a basic features
bitmap for discoverability of future hypervisor services.

Cc: Marc Zyngier <maz@kernel.org>
Signed-off-by: Will Deacon <will@kernel.org>
Signed-off-by: Jianyong Wu <jianyong.wu@arm.com>
Signed-off-by: Marc Zyngier <maz@kernel.org>
Link: https://lore.kernel.org/r/20201209060932.212364-3-jianyong.wu@arm.com
---
 arch/arm64/kvm/hypercalls.c | 27 ++++++++++++++++++---------
 1 file changed, 18 insertions(+), 9 deletions(-)

diff --git a/arch/arm64/kvm/hypercalls.c b/arch/arm64/kvm/hypercalls.c
index ead21b98b620..78d32c34d49c 100644
--- a/arch/arm64/kvm/hypercalls.c
+++ b/arch/arm64/kvm/hypercalls.c
@@ -12,13 +12,13 @@
 int kvm_hvc_call_handler(struct kvm_vcpu *vcpu)
 {
 	u32 func_id = smccc_get_function(vcpu);
-	long val = SMCCC_RET_NOT_SUPPORTED;
+	u64 val[4] = {SMCCC_RET_NOT_SUPPORTED};
 	u32 feature;
 	gpa_t gpa;
 
 	switch (func_id) {
 	case ARM_SMCCC_VERSION_FUNC_ID:
-		val = ARM_SMCCC_VERSION_1_1;
+		val[0] = ARM_SMCCC_VERSION_1_1;
 		break;
 	case ARM_SMCCC_ARCH_FEATURES_FUNC_ID:
 		feature = smccc_get_arg1(vcpu);
@@ -28,10 +28,10 @@ int kvm_hvc_call_handler(struct kvm_vcpu *vcpu)
 			case SPECTRE_VULNERABLE:
 				break;
 			case SPECTRE_MITIGATED:
-				val = SMCCC_RET_SUCCESS;
+				val[0] = SMCCC_RET_SUCCESS;
 				break;
 			case SPECTRE_UNAFFECTED:
-				val = SMCCC_ARCH_WORKAROUND_RET_UNAFFECTED;
+				val[0] = SMCCC_ARCH_WORKAROUND_RET_UNAFFECTED;
 				break;
 			}
 			break;
@@ -54,22 +54,31 @@ int kvm_hvc_call_handler(struct kvm_vcpu *vcpu)
 					break;
 				fallthrough;
 			case SPECTRE_UNAFFECTED:
-				val = SMCCC_RET_NOT_REQUIRED;
+				val[0] = SMCCC_RET_NOT_REQUIRED;
 				break;
 			}
 			break;
 		case ARM_SMCCC_HV_PV_TIME_FEATURES:
-			val = SMCCC_RET_SUCCESS;
+			val[0] = SMCCC_RET_SUCCESS;
 			break;
 		}
 		break;
 	case ARM_SMCCC_HV_PV_TIME_FEATURES:
-		val = kvm_hypercall_pv_features(vcpu);
+		val[0] = kvm_hypercall_pv_features(vcpu);
 		break;
 	case ARM_SMCCC_HV_PV_TIME_ST:
 		gpa = kvm_init_stolen_time(vcpu);
 		if (gpa != GPA_INVALID)
-			val = gpa;
+			val[0] = gpa;
+		break;
+	case ARM_SMCCC_VENDOR_HYP_CALL_UID_FUNC_ID:
+		val[0] = ARM_SMCCC_VENDOR_HYP_UID_KVM_REG_0;
+		val[1] = ARM_SMCCC_VENDOR_HYP_UID_KVM_REG_1;
+		val[2] = ARM_SMCCC_VENDOR_HYP_UID_KVM_REG_2;
+		val[3] = ARM_SMCCC_VENDOR_HYP_UID_KVM_REG_3;
+		break;
+	case ARM_SMCCC_VENDOR_HYP_KVM_FEATURES_FUNC_ID:
+		val[0] = BIT(ARM_SMCCC_KVM_FUNC_FEATURES);
 		break;
 	case ARM_SMCCC_TRNG_VERSION:
 	case ARM_SMCCC_TRNG_FEATURES:
@@ -81,6 +90,6 @@ int kvm_hvc_call_handler(struct kvm_vcpu *vcpu)
 		return kvm_psci_call(vcpu);
 	}
 
-	smccc_set_retval(vcpu, val, 0, 0, 0);
+	smccc_set_retval(vcpu, val[0], val[1], val[2], val[3]);
 	return 1;
 }
-- 
2.29.2


^ permalink raw reply	[flat|nested] 25+ messages in thread

* [PATCH v19 3/7] ptp: Reorganize ptp_kvm.c to make it arch-independent
  2021-03-30 14:54 [PATCH v19 0/7] KVM: arm64: Add host/guest KVM-PTP support Marc Zyngier
  2021-03-30 14:54 ` [PATCH v19 1/7] arm/arm64: Probe for the presence of KVM hypervisor Marc Zyngier
  2021-03-30 14:54 ` [PATCH v19 2/7] KVM: arm64: Advertise KVM UID to guests via SMCCC Marc Zyngier
@ 2021-03-30 14:54 ` Marc Zyngier
  2021-04-07  9:28   ` Marc Zyngier
  2021-03-30 14:54 ` [PATCH v19 4/7] time: Add mechanism to recognize clocksource in time_get_snapshot Marc Zyngier
                   ` (3 subsequent siblings)
  6 siblings, 1 reply; 25+ messages in thread
From: Marc Zyngier @ 2021-03-30 14:54 UTC (permalink / raw)
  To: netdev, yangbo.lu, john.stultz, tglx, pbonzini, seanjc,
	richardcochran, Mark.Rutland, will, suzuki.poulose,
	Andre.Przywara, steven.price, lorenzo.pieralisi, sudeep.holla
  Cc: linux-kernel, linux-arm-kernel, kvmarm, kvm, Steve.Capper,
	justin.he, jianyong.wu, kernel-team, Andre Przywara

From: Jianyong Wu <jianyong.wu@arm.com>

Currently, the ptp_kvm module contains a lot of x86-specific code.
Let's move this code into a new arch-specific file in the same directory,
and rename the arch-independent file to ptp_kvm_common.c.

Reviewed-by: Andre Przywara <andre.przywara@arm.com>
Signed-off-by: Jianyong Wu <jianyong.wu@arm.com>
Signed-off-by: Marc Zyngier <maz@kernel.org>
Link: https://lore.kernel.org/r/20201209060932.212364-4-jianyong.wu@arm.com
---
 drivers/ptp/Makefile                        |  1 +
 drivers/ptp/{ptp_kvm.c => ptp_kvm_common.c} | 84 +++++-------------
 drivers/ptp/ptp_kvm_x86.c                   | 97 +++++++++++++++++++++
 include/linux/ptp_kvm.h                     | 19 ++++
 4 files changed, 139 insertions(+), 62 deletions(-)
 rename drivers/ptp/{ptp_kvm.c => ptp_kvm_common.c} (60%)
 create mode 100644 drivers/ptp/ptp_kvm_x86.c
 create mode 100644 include/linux/ptp_kvm.h

diff --git a/drivers/ptp/Makefile b/drivers/ptp/Makefile
index db5aef3bddc6..d11eeb5811d1 100644
--- a/drivers/ptp/Makefile
+++ b/drivers/ptp/Makefile
@@ -4,6 +4,7 @@
 #
 
 ptp-y					:= ptp_clock.o ptp_chardev.o ptp_sysfs.o
+ptp_kvm-$(CONFIG_X86)			:= ptp_kvm_x86.o ptp_kvm_common.o
 obj-$(CONFIG_PTP_1588_CLOCK)		+= ptp.o
 obj-$(CONFIG_PTP_1588_CLOCK_DTE)	+= ptp_dte.o
 obj-$(CONFIG_PTP_1588_CLOCK_INES)	+= ptp_ines.o
diff --git a/drivers/ptp/ptp_kvm.c b/drivers/ptp/ptp_kvm_common.c
similarity index 60%
rename from drivers/ptp/ptp_kvm.c
rename to drivers/ptp/ptp_kvm_common.c
index 658d33fc3195..721ddcede5e1 100644
--- a/drivers/ptp/ptp_kvm.c
+++ b/drivers/ptp/ptp_kvm_common.c
@@ -8,11 +8,11 @@
 #include <linux/err.h>
 #include <linux/init.h>
 #include <linux/kernel.h>
+#include <linux/slab.h>
 #include <linux/module.h>
+#include <linux/ptp_kvm.h>
 #include <uapi/linux/kvm_para.h>
 #include <asm/kvm_para.h>
-#include <asm/pvclock.h>
-#include <asm/kvmclock.h>
 #include <uapi/asm/kvm_para.h>
 
 #include <linux/ptp_clock_kernel.h>
@@ -24,56 +24,29 @@ struct kvm_ptp_clock {
 
 static DEFINE_SPINLOCK(kvm_ptp_lock);
 
-static struct pvclock_vsyscall_time_info *hv_clock;
-
-static struct kvm_clock_pairing clock_pair;
-static phys_addr_t clock_pair_gpa;
-
 static int ptp_kvm_get_time_fn(ktime_t *device_time,
 			       struct system_counterval_t *system_counter,
 			       void *ctx)
 {
-	unsigned long ret;
+	long ret;
+	u64 cycle;
 	struct timespec64 tspec;
-	unsigned version;
-	int cpu;
-	struct pvclock_vcpu_time_info *src;
+	struct clocksource *cs;
 
 	spin_lock(&kvm_ptp_lock);
 
 	preempt_disable_notrace();
-	cpu = smp_processor_id();
-	src = &hv_clock[cpu].pvti;
-
-	do {
-		/*
-		 * We are using a TSC value read in the hosts
-		 * kvm_hc_clock_pairing handling.
-		 * So any changes to tsc_to_system_mul
-		 * and tsc_shift or any other pvclock
-		 * data invalidate that measurement.
-		 */
-		version = pvclock_read_begin(src);
-
-		ret = kvm_hypercall2(KVM_HC_CLOCK_PAIRING,
-				     clock_pair_gpa,
-				     KVM_CLOCK_PAIRING_WALLCLOCK);
-		if (ret != 0) {
-			pr_err_ratelimited("clock pairing hypercall ret %lu\n", ret);
-			spin_unlock(&kvm_ptp_lock);
-			preempt_enable_notrace();
-			return -EOPNOTSUPP;
-		}
-
-		tspec.tv_sec = clock_pair.sec;
-		tspec.tv_nsec = clock_pair.nsec;
-		ret = __pvclock_read_cycles(src, clock_pair.tsc);
-	} while (pvclock_read_retry(src, version));
+	ret = kvm_arch_ptp_get_crosststamp(&cycle, &tspec, &cs);
+	if (ret) {
+		spin_unlock(&kvm_ptp_lock);
+		preempt_enable_notrace();
+		return ret;
+	}
 
 	preempt_enable_notrace();
 
-	system_counter->cycles = ret;
-	system_counter->cs = &kvm_clock;
+	system_counter->cycles = cycle;
+	system_counter->cs = cs;
 
 	*device_time = timespec64_to_ktime(tspec);
 
@@ -111,22 +84,17 @@ static int ptp_kvm_settime(struct ptp_clock_info *ptp,
 
 static int ptp_kvm_gettime(struct ptp_clock_info *ptp, struct timespec64 *ts)
 {
-	unsigned long ret;
+	long ret;
 	struct timespec64 tspec;
 
 	spin_lock(&kvm_ptp_lock);
 
-	ret = kvm_hypercall2(KVM_HC_CLOCK_PAIRING,
-			     clock_pair_gpa,
-			     KVM_CLOCK_PAIRING_WALLCLOCK);
-	if (ret != 0) {
-		pr_err_ratelimited("clock offset hypercall ret %lu\n", ret);
+	ret = kvm_arch_ptp_get_clock(&tspec);
+	if (ret) {
 		spin_unlock(&kvm_ptp_lock);
-		return -EOPNOTSUPP;
+		return ret;
 	}
 
-	tspec.tv_sec = clock_pair.sec;
-	tspec.tv_nsec = clock_pair.nsec;
 	spin_unlock(&kvm_ptp_lock);
 
 	memcpy(ts, &tspec, sizeof(struct timespec64));
@@ -168,19 +136,11 @@ static int __init ptp_kvm_init(void)
 {
 	long ret;
 
-	if (!kvm_para_available())
-		return -ENODEV;
-
-	clock_pair_gpa = slow_virt_to_phys(&clock_pair);
-	hv_clock = pvclock_get_pvti_cpu0_va();
-
-	if (!hv_clock)
-		return -ENODEV;
-
-	ret = kvm_hypercall2(KVM_HC_CLOCK_PAIRING, clock_pair_gpa,
-			KVM_CLOCK_PAIRING_WALLCLOCK);
-	if (ret == -KVM_ENOSYS || ret == -KVM_EOPNOTSUPP)
-		return -ENODEV;
+	ret = kvm_arch_ptp_init();
+	if (ret) {
+		pr_err("fail to initialize ptp_kvm");
+		return ret;
+	}
 
 	kvm_ptp_clock.caps = ptp_kvm_caps;
 
diff --git a/drivers/ptp/ptp_kvm_x86.c b/drivers/ptp/ptp_kvm_x86.c
new file mode 100644
index 000000000000..3dd519dfc473
--- /dev/null
+++ b/drivers/ptp/ptp_kvm_x86.c
@@ -0,0 +1,97 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Virtual PTP 1588 clock for use with KVM guests
+ *
+ * Copyright (C) 2017 Red Hat Inc.
+ */
+
+#include <linux/device.h>
+#include <linux/kernel.h>
+#include <asm/pvclock.h>
+#include <asm/kvmclock.h>
+#include <linux/module.h>
+#include <uapi/asm/kvm_para.h>
+#include <uapi/linux/kvm_para.h>
+#include <linux/ptp_clock_kernel.h>
+#include <linux/ptp_kvm.h>
+
+struct pvclock_vsyscall_time_info *hv_clock;
+
+static phys_addr_t clock_pair_gpa;
+static struct kvm_clock_pairing clock_pair;
+
+int kvm_arch_ptp_init(void)
+{
+	long ret;
+
+	if (!kvm_para_available())
+		return -ENODEV;
+
+	clock_pair_gpa = slow_virt_to_phys(&clock_pair);
+	hv_clock = pvclock_get_pvti_cpu0_va();
+	if (!hv_clock)
+		return -ENODEV;
+
+	ret = kvm_hypercall2(KVM_HC_CLOCK_PAIRING, clock_pair_gpa,
+			     KVM_CLOCK_PAIRING_WALLCLOCK);
+	if (ret == -KVM_ENOSYS || ret == -KVM_EOPNOTSUPP)
+		return -ENODEV;
+
+	return 0;
+}
+
+int kvm_arch_ptp_get_clock(struct timespec64 *ts)
+{
+	long ret;
+
+	ret = kvm_hypercall2(KVM_HC_CLOCK_PAIRING,
+			     clock_pair_gpa,
+			     KVM_CLOCK_PAIRING_WALLCLOCK);
+	if (ret != 0) {
+		pr_err_ratelimited("clock offset hypercall ret %lu\n", ret);
+		return -EOPNOTSUPP;
+	}
+
+	ts->tv_sec = clock_pair.sec;
+	ts->tv_nsec = clock_pair.nsec;
+
+	return 0;
+}
+
+int kvm_arch_ptp_get_crosststamp(u64 *cycle, struct timespec64 *tspec,
+			      struct clocksource **cs)
+{
+	struct pvclock_vcpu_time_info *src;
+	unsigned int version;
+	long ret;
+	int cpu;
+
+	cpu = smp_processor_id();
+	src = &hv_clock[cpu].pvti;
+
+	do {
+		/*
+		 * We are using a TSC value read in the hosts
+		 * kvm_hc_clock_pairing handling.
+		 * So any changes to tsc_to_system_mul
+		 * and tsc_shift or any other pvclock
+		 * data invalidate that measurement.
+		 */
+		version = pvclock_read_begin(src);
+
+		ret = kvm_hypercall2(KVM_HC_CLOCK_PAIRING,
+				     clock_pair_gpa,
+				     KVM_CLOCK_PAIRING_WALLCLOCK);
+		if (ret != 0) {
+			pr_err_ratelimited("clock pairing hypercall ret %lu\n", ret);
+			return -EOPNOTSUPP;
+		}
+		tspec->tv_sec = clock_pair.sec;
+		tspec->tv_nsec = clock_pair.nsec;
+		*cycle = __pvclock_read_cycles(src, clock_pair.tsc);
+	} while (pvclock_read_retry(src, version));
+
+	*cs = &kvm_clock;
+
+	return 0;
+}
diff --git a/include/linux/ptp_kvm.h b/include/linux/ptp_kvm.h
new file mode 100644
index 000000000000..f960a719f0d5
--- /dev/null
+++ b/include/linux/ptp_kvm.h
@@ -0,0 +1,19 @@
+/* SPDX-License-Identifier: GPL-2.0-or-later */
+/*
+ * Virtual PTP 1588 clock for use with KVM guests
+ *
+ * Copyright (C) 2017 Red Hat Inc.
+ */
+
+#ifndef _PTP_KVM_H_
+#define _PTP_KVM_H_
+
+struct timespec64;
+struct clocksource;
+
+int kvm_arch_ptp_init(void);
+int kvm_arch_ptp_get_clock(struct timespec64 *ts);
+int kvm_arch_ptp_get_crosststamp(u64 *cycle,
+		struct timespec64 *tspec, struct clocksource **cs);
+
+#endif /* _PTP_KVM_H_ */
-- 
2.29.2


^ permalink raw reply	[flat|nested] 25+ messages in thread

* [PATCH v19 4/7] time: Add mechanism to recognize clocksource in time_get_snapshot
  2021-03-30 14:54 [PATCH v19 0/7] KVM: arm64: Add host/guest KVM-PTP support Marc Zyngier
                   ` (2 preceding siblings ...)
  2021-03-30 14:54 ` [PATCH v19 3/7] ptp: Reorganize ptp_kvm.c to make it arch-independent Marc Zyngier
@ 2021-03-30 14:54 ` Marc Zyngier
  2021-04-17  9:27   ` Zenghui Yu
  2021-03-30 14:54 ` [PATCH v19 5/7] clocksource: Add clocksource id for arm arch counter Marc Zyngier
                   ` (2 subsequent siblings)
  6 siblings, 1 reply; 25+ messages in thread
From: Marc Zyngier @ 2021-03-30 14:54 UTC (permalink / raw)
  To: netdev, yangbo.lu, john.stultz, tglx, pbonzini, seanjc,
	richardcochran, Mark.Rutland, will, suzuki.poulose,
	Andre.Przywara, steven.price, lorenzo.pieralisi, sudeep.holla
  Cc: linux-kernel, linux-arm-kernel, kvmarm, kvm, Steve.Capper,
	justin.he, jianyong.wu, kernel-team

From: Thomas Gleixner <tglx@linutronix.de>

System time snapshots are not conveying information about the current
clocksource which was used, but callers like the PTP KVM guest
implementation have the requirement to evaluate the clocksource type to
select the appropriate mechanism.

Introduce a clocksource id field in struct clocksource which is by default
set to CSID_GENERIC (0). Clocksource implementations can set that field to
a value which allows to identify the clocksource.

Store the clocksource id of the current clocksource in the
system_time_snapshot so callers can evaluate which clocksource was used to
take the snapshot and act accordingly.

Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Signed-off-by: Jianyong Wu <jianyong.wu@arm.com>
Signed-off-by: Marc Zyngier <maz@kernel.org>
Link: https://lore.kernel.org/r/20201209060932.212364-5-jianyong.wu@arm.com
---
 include/linux/clocksource.h     |  6 ++++++
 include/linux/clocksource_ids.h | 11 +++++++++++
 include/linux/timekeeping.h     | 12 +++++++-----
 kernel/time/clocksource.c       |  2 ++
 kernel/time/timekeeping.c       |  1 +
 5 files changed, 27 insertions(+), 5 deletions(-)
 create mode 100644 include/linux/clocksource_ids.h

diff --git a/include/linux/clocksource.h b/include/linux/clocksource.h
index 86d143db6523..1290d0dce840 100644
--- a/include/linux/clocksource.h
+++ b/include/linux/clocksource.h
@@ -17,6 +17,7 @@
 #include <linux/timer.h>
 #include <linux/init.h>
 #include <linux/of.h>
+#include <linux/clocksource_ids.h>
 #include <asm/div64.h>
 #include <asm/io.h>
 
@@ -62,6 +63,10 @@ struct module;
  *			400-499: Perfect
  *				The ideal clocksource. A must-use where
  *				available.
+ * @id:			Defaults to CSID_GENERIC. The id value is captured
+ *			in certain snapshot functions to allow callers to
+ *			validate the clocksource from which the snapshot was
+ *			taken.
  * @flags:		Flags describing special properties
  * @enable:		Optional function to enable the clocksource
  * @disable:		Optional function to disable the clocksource
@@ -100,6 +105,7 @@ struct clocksource {
 	const char		*name;
 	struct list_head	list;
 	int			rating;
+	enum clocksource_ids	id;
 	enum vdso_clock_mode	vdso_clock_mode;
 	unsigned long		flags;
 
diff --git a/include/linux/clocksource_ids.h b/include/linux/clocksource_ids.h
new file mode 100644
index 000000000000..4d8e19e05328
--- /dev/null
+++ b/include/linux/clocksource_ids.h
@@ -0,0 +1,11 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef _LINUX_CLOCKSOURCE_IDS_H
+#define _LINUX_CLOCKSOURCE_IDS_H
+
+/* Enum to give clocksources a unique identifier */
+enum clocksource_ids {
+	CSID_GENERIC		= 0,
+	CSID_MAX,
+};
+
+#endif
diff --git a/include/linux/timekeeping.h b/include/linux/timekeeping.h
index c6792cf01bc7..78a98bdff76d 100644
--- a/include/linux/timekeeping.h
+++ b/include/linux/timekeeping.h
@@ -3,6 +3,7 @@
 #define _LINUX_TIMEKEEPING_H
 
 #include <linux/errno.h>
+#include <linux/clocksource_ids.h>
 
 /* Included from linux/ktime.h */
 
@@ -243,11 +244,12 @@ struct ktime_timestamps {
  * @cs_was_changed_seq:	The sequence number of clocksource change events
  */
 struct system_time_snapshot {
-	u64		cycles;
-	ktime_t		real;
-	ktime_t		raw;
-	unsigned int	clock_was_set_seq;
-	u8		cs_was_changed_seq;
+	u64			cycles;
+	ktime_t			real;
+	ktime_t			raw;
+	enum clocksource_ids	cs_id;
+	unsigned int		clock_was_set_seq;
+	u8			cs_was_changed_seq;
 };
 
 /**
diff --git a/kernel/time/clocksource.c b/kernel/time/clocksource.c
index cce484a2cc7c..4fe1df894ee5 100644
--- a/kernel/time/clocksource.c
+++ b/kernel/time/clocksource.c
@@ -920,6 +920,8 @@ int __clocksource_register_scale(struct clocksource *cs, u32 scale, u32 freq)
 
 	clocksource_arch_init(cs);
 
+	if (WARN_ON_ONCE((unsigned int)cs->id >= CSID_MAX))
+		cs->id = CSID_GENERIC;
 	if (cs->vdso_clock_mode < 0 ||
 	    cs->vdso_clock_mode >= VDSO_CLOCKMODE_MAX) {
 		pr_warn("clocksource %s registered with invalid VDSO mode %d. Disabling VDSO support.\n",
diff --git a/kernel/time/timekeeping.c b/kernel/time/timekeeping.c
index 6aee5768c86f..06f55f9258bf 100644
--- a/kernel/time/timekeeping.c
+++ b/kernel/time/timekeeping.c
@@ -1048,6 +1048,7 @@ void ktime_get_snapshot(struct system_time_snapshot *systime_snapshot)
 	do {
 		seq = read_seqcount_begin(&tk_core.seq);
 		now = tk_clock_read(&tk->tkr_mono);
+		systime_snapshot->cs_id = tk->tkr_mono.clock->id;
 		systime_snapshot->cs_was_changed_seq = tk->cs_was_changed_seq;
 		systime_snapshot->clock_was_set_seq = tk->clock_was_set_seq;
 		base_real = ktime_add(tk->tkr_mono.base,
-- 
2.29.2


^ permalink raw reply	[flat|nested] 25+ messages in thread

* [PATCH v19 5/7] clocksource: Add clocksource id for arm arch counter
  2021-03-30 14:54 [PATCH v19 0/7] KVM: arm64: Add host/guest KVM-PTP support Marc Zyngier
                   ` (3 preceding siblings ...)
  2021-03-30 14:54 ` [PATCH v19 4/7] time: Add mechanism to recognize clocksource in time_get_snapshot Marc Zyngier
@ 2021-03-30 14:54 ` Marc Zyngier
  2021-03-30 14:54 ` [PATCH v19 6/7] KVM: arm64: Add support for the KVM PTP service Marc Zyngier
  2021-03-30 14:54 ` [PATCH v19 7/7] ptp: arm/arm64: Enable ptp_kvm for arm/arm64 Marc Zyngier
  6 siblings, 0 replies; 25+ messages in thread
From: Marc Zyngier @ 2021-03-30 14:54 UTC (permalink / raw)
  To: netdev, yangbo.lu, john.stultz, tglx, pbonzini, seanjc,
	richardcochran, Mark.Rutland, will, suzuki.poulose,
	Andre.Przywara, steven.price, lorenzo.pieralisi, sudeep.holla
  Cc: linux-kernel, linux-arm-kernel, kvmarm, kvm, Steve.Capper,
	justin.he, jianyong.wu, kernel-team, Mark Rutland,
	Andre Przywara

From: Jianyong Wu <jianyong.wu@arm.com>

Add clocksource id to the ARM generic counter so that it can be easily
identified from callers such as ptp_kvm.

Cc: Mark Rutland <mark.rutland@arm.com>
Reviewed-by: Andre Przywara <andre.przywara@arm.com>
Signed-off-by: Jianyong Wu <jianyong.wu@arm.com>
Signed-off-by: Marc Zyngier <maz@kernel.org>
Link: https://lore.kernel.org/r/20201209060932.212364-6-jianyong.wu@arm.com
---
 drivers/clocksource/arm_arch_timer.c | 2 ++
 include/linux/clocksource_ids.h      | 1 +
 2 files changed, 3 insertions(+)

diff --git a/drivers/clocksource/arm_arch_timer.c b/drivers/clocksource/arm_arch_timer.c
index d0177824c518..8f12e223703f 100644
--- a/drivers/clocksource/arm_arch_timer.c
+++ b/drivers/clocksource/arm_arch_timer.c
@@ -16,6 +16,7 @@
 #include <linux/cpu_pm.h>
 #include <linux/clockchips.h>
 #include <linux/clocksource.h>
+#include <linux/clocksource_ids.h>
 #include <linux/interrupt.h>
 #include <linux/of_irq.h>
 #include <linux/of_address.h>
@@ -191,6 +192,7 @@ static u64 arch_counter_read_cc(const struct cyclecounter *cc)
 
 static struct clocksource clocksource_counter = {
 	.name	= "arch_sys_counter",
+	.id	= CSID_ARM_ARCH_COUNTER,
 	.rating	= 400,
 	.read	= arch_counter_read,
 	.mask	= CLOCKSOURCE_MASK(56),
diff --git a/include/linux/clocksource_ids.h b/include/linux/clocksource_ids.h
index 4d8e19e05328..16775d7d8f8d 100644
--- a/include/linux/clocksource_ids.h
+++ b/include/linux/clocksource_ids.h
@@ -5,6 +5,7 @@
 /* Enum to give clocksources a unique identifier */
 enum clocksource_ids {
 	CSID_GENERIC		= 0,
+	CSID_ARM_ARCH_COUNTER,
 	CSID_MAX,
 };
 
-- 
2.29.2


^ permalink raw reply	[flat|nested] 25+ messages in thread

* [PATCH v19 6/7] KVM: arm64: Add support for the KVM PTP service
  2021-03-30 14:54 [PATCH v19 0/7] KVM: arm64: Add host/guest KVM-PTP support Marc Zyngier
                   ` (4 preceding siblings ...)
  2021-03-30 14:54 ` [PATCH v19 5/7] clocksource: Add clocksource id for arm arch counter Marc Zyngier
@ 2021-03-30 14:54 ` Marc Zyngier
  2021-04-17  8:59   ` Zenghui Yu
  2021-03-30 14:54 ` [PATCH v19 7/7] ptp: arm/arm64: Enable ptp_kvm for arm/arm64 Marc Zyngier
  6 siblings, 1 reply; 25+ messages in thread
From: Marc Zyngier @ 2021-03-30 14:54 UTC (permalink / raw)
  To: netdev, yangbo.lu, john.stultz, tglx, pbonzini, seanjc,
	richardcochran, Mark.Rutland, will, suzuki.poulose,
	Andre.Przywara, steven.price, lorenzo.pieralisi, sudeep.holla
  Cc: linux-kernel, linux-arm-kernel, kvmarm, kvm, Steve.Capper,
	justin.he, jianyong.wu, kernel-team

From: Jianyong Wu <jianyong.wu@arm.com>

Implement the hypervisor side of the KVM PTP interface.

The service offers wall time and cycle count from host to guest.
The caller must specify whether they want the host's view of
either the virtual or physical counter.

Signed-off-by: Jianyong Wu <jianyong.wu@arm.com>
Signed-off-by: Marc Zyngier <maz@kernel.org>
Link: https://lore.kernel.org/r/20201209060932.212364-7-jianyong.wu@arm.com
---
 Documentation/virt/kvm/api.rst         | 10 +++++
 Documentation/virt/kvm/arm/index.rst   |  1 +
 Documentation/virt/kvm/arm/ptp_kvm.rst | 25 ++++++++++++
 arch/arm64/kvm/arm.c                   |  1 +
 arch/arm64/kvm/hypercalls.c            | 53 ++++++++++++++++++++++++++
 include/linux/arm-smccc.h              | 16 ++++++++
 include/uapi/linux/kvm.h               |  1 +
 7 files changed, 107 insertions(+)
 create mode 100644 Documentation/virt/kvm/arm/ptp_kvm.rst

diff --git a/Documentation/virt/kvm/api.rst b/Documentation/virt/kvm/api.rst
index 38e327d4b479..987d99e39887 100644
--- a/Documentation/virt/kvm/api.rst
+++ b/Documentation/virt/kvm/api.rst
@@ -6724,3 +6724,13 @@ vcpu_info is set.
 The KVM_XEN_HVM_CONFIG_RUNSTATE flag indicates that the runstate-related
 features KVM_XEN_VCPU_ATTR_TYPE_RUNSTATE_ADDR/_CURRENT/_DATA/_ADJUST are
 supported by the KVM_XEN_VCPU_SET_ATTR/KVM_XEN_VCPU_GET_ATTR ioctls.
+
+8.31 KVM_CAP_PTP_KVM
+--------------------
+
+:Architectures: arm64
+
+This capability indicates that the KVM virtual PTP service is
+supported in the host. A VMM can check whether the service is
+available to the guest on migration.
+
diff --git a/Documentation/virt/kvm/arm/index.rst b/Documentation/virt/kvm/arm/index.rst
index 3e2b2aba90fc..78a9b670aafe 100644
--- a/Documentation/virt/kvm/arm/index.rst
+++ b/Documentation/virt/kvm/arm/index.rst
@@ -10,3 +10,4 @@ ARM
    hyp-abi
    psci
    pvtime
+   ptp_kvm
diff --git a/Documentation/virt/kvm/arm/ptp_kvm.rst b/Documentation/virt/kvm/arm/ptp_kvm.rst
new file mode 100644
index 000000000000..68cffb50d8bf
--- /dev/null
+++ b/Documentation/virt/kvm/arm/ptp_kvm.rst
@@ -0,0 +1,25 @@
+.. SPDX-License-Identifier: GPL-2.0
+
+PTP_KVM support for arm/arm64
+=============================
+
+PTP_KVM is used for high precision time sync between host and guests.
+It relies on transferring the wall clock and counter value from the
+host to the guest using a KVM-specific hypercall.
+
+* ARM_SMCCC_HYP_KVM_PTP_FUNC_ID: 0x86000001
+
+This hypercall uses the SMC32/HVC32 calling convention:
+
+ARM_SMCCC_HYP_KVM_PTP_FUNC_ID
+    =============    ==========    ==========
+    Function ID:     (uint32)      0x86000001
+    Arguments:       (uint32)      KVM_PTP_VIRT_COUNTER(0)
+                                   KVM_PTP_PHYS_COUNTER(1)
+    Return Values:   (int32)       NOT_SUPPORTED(-1) on error, or
+                     (uint32)      Upper 32 bits of wall clock time (r0)
+                     (uint32)      Lower 32 bits of wall clock time (r1)
+                     (uint32)      Upper 32 bits of counter (r2)
+                     (uint32)      Lower 32 bits of counter (r3)
+    Endianness:                    No Restrictions.
+    =============    ==========    ==========
diff --git a/arch/arm64/kvm/arm.c b/arch/arm64/kvm/arm.c
index 7f06ba76698d..46401798c644 100644
--- a/arch/arm64/kvm/arm.c
+++ b/arch/arm64/kvm/arm.c
@@ -206,6 +206,7 @@ int kvm_vm_ioctl_check_extension(struct kvm *kvm, long ext)
 	case KVM_CAP_ARM_INJECT_EXT_DABT:
 	case KVM_CAP_SET_GUEST_DEBUG:
 	case KVM_CAP_VCPU_ATTRIBUTES:
+	case KVM_CAP_PTP_KVM:
 		r = 1;
 		break;
 	case KVM_CAP_ARM_SET_DEVICE_ADDR:
diff --git a/arch/arm64/kvm/hypercalls.c b/arch/arm64/kvm/hypercalls.c
index 78d32c34d49c..30da78f72b3b 100644
--- a/arch/arm64/kvm/hypercalls.c
+++ b/arch/arm64/kvm/hypercalls.c
@@ -9,6 +9,55 @@
 #include <kvm/arm_hypercalls.h>
 #include <kvm/arm_psci.h>
 
+static void kvm_ptp_get_time(struct kvm_vcpu *vcpu, u64 *val)
+{
+	struct system_time_snapshot systime_snapshot;
+	u64 cycles = ~0UL;
+	u32 feature;
+
+	/*
+	 * system time and counter value must captured at the same
+	 * time to keep consistency and precision.
+	 */
+	ktime_get_snapshot(&systime_snapshot);
+
+	/*
+	 * This is only valid if the current clocksource is the
+	 * architected counter, as this is the only one the guest
+	 * can see.
+	 */
+	if (systime_snapshot.cs_id != CSID_ARM_ARCH_COUNTER)
+		return;
+
+	/*
+	 * The guest selects one of the two reference counters
+	 * (virtual or physical) with the first argument of the SMCCC
+	 * call. In case the identifier is not supported, error out.
+	 */
+	feature = smccc_get_arg1(vcpu);
+	switch (feature) {
+	case KVM_PTP_VIRT_COUNTER:
+		cycles = systime_snapshot.cycles - vcpu_read_sys_reg(vcpu, CNTVOFF_EL2);
+		break;
+	case KVM_PTP_PHYS_COUNTER:
+		cycles = systime_snapshot.cycles;
+		break;
+	default:
+		return;
+	}
+
+	/*
+	 * This relies on the top bit of val[0] never being set for
+	 * valid values of system time, because that is *really* far
+	 * in the future (about 292 years from 1970, and at that stage
+	 * nobody will give a damn about it).
+	 */
+	val[0] = upper_32_bits(systime_snapshot.real);
+	val[1] = lower_32_bits(systime_snapshot.real);
+	val[2] = upper_32_bits(cycles);
+	val[3] = lower_32_bits(cycles);
+}
+
 int kvm_hvc_call_handler(struct kvm_vcpu *vcpu)
 {
 	u32 func_id = smccc_get_function(vcpu);
@@ -79,6 +128,10 @@ 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);
+		val[0] |= BIT(ARM_SMCCC_KVM_FUNC_PTP);
+		break;
+	case ARM_SMCCC_VENDOR_HYP_KVM_PTP_FUNC_ID:
+		kvm_ptp_get_time(vcpu, val);
 		break;
 	case ARM_SMCCC_TRNG_VERSION:
 	case ARM_SMCCC_TRNG_FEATURES:
diff --git a/include/linux/arm-smccc.h b/include/linux/arm-smccc.h
index 1a27bd9493fe..6861489a1890 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_PTP			1
 #define ARM_SMCCC_KVM_FUNC_FEATURES_2		127
 #define ARM_SMCCC_KVM_NUM_FUNCS			128
 
@@ -114,6 +115,21 @@
 
 #define SMCCC_ARCH_WORKAROUND_RET_UNAFFECTED	1
 
+/*
+ * 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_PTP)
+
+/* ptp_kvm counter type ID */
+#define KVM_PTP_VIRT_COUNTER			0
+#define KVM_PTP_PHYS_COUNTER			1
+
 /* Paravirtualised time calls (defined by ARM DEN0057A) */
 #define ARM_SMCCC_HV_PV_TIME_FEATURES				\
 	ARM_SMCCC_CALL_VAL(ARM_SMCCC_FAST_CALL,			\
diff --git a/include/uapi/linux/kvm.h b/include/uapi/linux/kvm.h
index f6afee209620..0e0f70c0d0dc 100644
--- a/include/uapi/linux/kvm.h
+++ b/include/uapi/linux/kvm.h
@@ -1078,6 +1078,7 @@ struct kvm_ppc_resize_hpt {
 #define KVM_CAP_DIRTY_LOG_RING 192
 #define KVM_CAP_X86_BUS_LOCK_EXIT 193
 #define KVM_CAP_PPC_DAWR1 194
+#define KVM_CAP_PTP_KVM 195
 
 #ifdef KVM_CAP_IRQ_ROUTING
 
-- 
2.29.2


^ permalink raw reply	[flat|nested] 25+ messages in thread

* [PATCH v19 7/7] ptp: arm/arm64: Enable ptp_kvm for arm/arm64
  2021-03-30 14:54 [PATCH v19 0/7] KVM: arm64: Add host/guest KVM-PTP support Marc Zyngier
                   ` (5 preceding siblings ...)
  2021-03-30 14:54 ` [PATCH v19 6/7] KVM: arm64: Add support for the KVM PTP service Marc Zyngier
@ 2021-03-30 14:54 ` Marc Zyngier
  2021-04-17  8:42   ` Zenghui Yu
  2021-05-11  9:07   ` Geert Uytterhoeven
  6 siblings, 2 replies; 25+ messages in thread
From: Marc Zyngier @ 2021-03-30 14:54 UTC (permalink / raw)
  To: netdev, yangbo.lu, john.stultz, tglx, pbonzini, seanjc,
	richardcochran, Mark.Rutland, will, suzuki.poulose,
	Andre.Przywara, steven.price, lorenzo.pieralisi, sudeep.holla
  Cc: linux-kernel, linux-arm-kernel, kvmarm, kvm, Steve.Capper,
	justin.he, jianyong.wu, kernel-team, Mark Rutland

From: Jianyong Wu <jianyong.wu@arm.com>

Currently, there is no mechanism to keep time sync between guest and host
in arm/arm64 virtualization environment. Time in guest will drift compared
with host after boot up as they may both use third party time sources
to correct their time respectively. The time deviation will be in order
of milliseconds. But in some scenarios,like in cloud environment, we ask
for higher time precision.

kvm ptp clock, which chooses the host clock source as a reference
clock to sync time between guest and host, has been adopted by x86
which takes the time sync order from milliseconds to nanoseconds.

This patch enables kvm ptp clock for arm/arm64 and improves clock sync precision
significantly.

Test result comparisons between with kvm ptp clock and without it in arm/arm64
are as follows. This test derived from the result of command 'chronyc
sources'. we should take more care of the last sample column which shows
the offset between the local clock and the source at the last measurement.

no kvm ptp in guest:
MS Name/IP address   Stratum Poll Reach LastRx Last sample
========================================================================
^* dns1.synet.edu.cn      2   6   377    13  +1040us[+1581us] +/-   21ms
^* dns1.synet.edu.cn      2   6   377    21  +1040us[+1581us] +/-   21ms
^* dns1.synet.edu.cn      2   6   377    29  +1040us[+1581us] +/-   21ms
^* dns1.synet.edu.cn      2   6   377    37  +1040us[+1581us] +/-   21ms
^* dns1.synet.edu.cn      2   6   377    45  +1040us[+1581us] +/-   21ms
^* dns1.synet.edu.cn      2   6   377    53  +1040us[+1581us] +/-   21ms
^* dns1.synet.edu.cn      2   6   377    61  +1040us[+1581us] +/-   21ms
^* dns1.synet.edu.cn      2   6   377     4   -130us[ +796us] +/-   21ms
^* dns1.synet.edu.cn      2   6   377    12   -130us[ +796us] +/-   21ms
^* dns1.synet.edu.cn      2   6   377    20   -130us[ +796us] +/-   21ms

in host:
MS Name/IP address   Stratum Poll Reach LastRx Last sample
========================================================================
^* 120.25.115.20          2   7   377    72   -470us[ -603us] +/-   18ms
^* 120.25.115.20          2   7   377    92   -470us[ -603us] +/-   18ms
^* 120.25.115.20          2   7   377   112   -470us[ -603us] +/-   18ms
^* 120.25.115.20          2   7   377     2   +872ns[-6808ns] +/-   17ms
^* 120.25.115.20          2   7   377    22   +872ns[-6808ns] +/-   17ms
^* 120.25.115.20          2   7   377    43   +872ns[-6808ns] +/-   17ms
^* 120.25.115.20          2   7   377    63   +872ns[-6808ns] +/-   17ms
^* 120.25.115.20          2   7   377    83   +872ns[-6808ns] +/-   17ms
^* 120.25.115.20          2   7   377   103   +872ns[-6808ns] +/-   17ms
^* 120.25.115.20          2   7   377   123   +872ns[-6808ns] +/-   17ms

The dns1.synet.edu.cn is the network reference clock for guest and
120.25.115.20 is the network reference clock for host. we can't get the
clock error between guest and host directly, but a roughly estimated value
will be in order of hundreds of us to ms.

with kvm ptp in guest:
chrony has been disabled in host to remove the disturb by network clock.

MS Name/IP address         Stratum Poll Reach LastRx Last sample
========================================================================
* PHC0                    0   3   377     8     -7ns[   +1ns] +/-    3ns
* PHC0                    0   3   377     8     +1ns[  +16ns] +/-    3ns
* PHC0                    0   3   377     6     -4ns[   -0ns] +/-    6ns
* PHC0                    0   3   377     6     -8ns[  -12ns] +/-    5ns
* PHC0                    0   3   377     5     +2ns[   +4ns] +/-    4ns
* PHC0                    0   3   377    13     +2ns[   +4ns] +/-    4ns
* PHC0                    0   3   377    12     -4ns[   -6ns] +/-    4ns
* PHC0                    0   3   377    11     -8ns[  -11ns] +/-    6ns
* PHC0                    0   3   377    10    -14ns[  -20ns] +/-    4ns
* PHC0                    0   3   377     8     +4ns[   +5ns] +/-    4ns

The PHC0 is the ptp clock which choose the host clock as its source
clock. So we can see that the clock difference between host and guest
is in order of ns.

Cc: Mark Rutland <mark.rutland@arm.com>
Signed-off-by: Jianyong Wu <jianyong.wu@arm.com>
Signed-off-by: Marc Zyngier <maz@kernel.org>
Link: https://lore.kernel.org/r/20201209060932.212364-8-jianyong.wu@arm.com
---
 drivers/clocksource/arm_arch_timer.c | 34 ++++++++++++++++++++++++++++
 drivers/ptp/Kconfig                  |  2 +-
 drivers/ptp/Makefile                 |  1 +
 drivers/ptp/ptp_kvm_arm.c            | 28 +++++++++++++++++++++++
 4 files changed, 64 insertions(+), 1 deletion(-)
 create mode 100644 drivers/ptp/ptp_kvm_arm.c

diff --git a/drivers/clocksource/arm_arch_timer.c b/drivers/clocksource/arm_arch_timer.c
index 8f12e223703f..e0f167e5e792 100644
--- a/drivers/clocksource/arm_arch_timer.c
+++ b/drivers/clocksource/arm_arch_timer.c
@@ -25,6 +25,8 @@
 #include <linux/sched/clock.h>
 #include <linux/sched_clock.h>
 #include <linux/acpi.h>
+#include <linux/arm-smccc.h>
+#include <linux/ptp_kvm.h>
 
 #include <asm/arch_timer.h>
 #include <asm/virt.h>
@@ -1659,3 +1661,35 @@ static int __init arch_timer_acpi_init(struct acpi_table_header *table)
 }
 TIMER_ACPI_DECLARE(arch_timer, ACPI_SIG_GTDT, arch_timer_acpi_init);
 #endif
+
+int kvm_arch_ptp_get_crosststamp(u64 *cycle, struct timespec64 *ts,
+				 struct clocksource **cs)
+{
+	struct arm_smccc_res hvc_res;
+	u32 ptp_counter;
+	ktime_t ktime;
+
+	if (!IS_ENABLED(CONFIG_HAVE_ARM_SMCCC_DISCOVERY))
+		return -EOPNOTSUPP;
+
+	if (arch_timer_uses_ppi == ARCH_TIMER_VIRT_PPI)
+		ptp_counter = KVM_PTP_VIRT_COUNTER;
+	else
+		ptp_counter = KVM_PTP_PHYS_COUNTER;
+
+	arm_smccc_1_1_invoke(ARM_SMCCC_VENDOR_HYP_KVM_PTP_FUNC_ID,
+			     ptp_counter, &hvc_res);
+
+	if ((int)(hvc_res.a0) < 0)
+		return -EOPNOTSUPP;
+
+	ktime = (u64)hvc_res.a0 << 32 | hvc_res.a1;
+	*ts = ktime_to_timespec64(ktime);
+	if (cycle)
+		*cycle = (u64)hvc_res.a2 << 32 | hvc_res.a3;
+	if (cs)
+		*cs = &clocksource_counter;
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(kvm_arch_ptp_get_crosststamp);
diff --git a/drivers/ptp/Kconfig b/drivers/ptp/Kconfig
index f2edef0df40f..8c20e524e9ad 100644
--- a/drivers/ptp/Kconfig
+++ b/drivers/ptp/Kconfig
@@ -108,7 +108,7 @@ config PTP_1588_CLOCK_PCH
 config PTP_1588_CLOCK_KVM
 	tristate "KVM virtual PTP clock"
 	depends on PTP_1588_CLOCK
-	depends on KVM_GUEST && X86
+	depends on (KVM_GUEST && X86) || (HAVE_ARM_SMCCC_DISCOVERY && ARM_ARCH_TIMER)
 	default y
 	help
 	  This driver adds support for using kvm infrastructure as a PTP
diff --git a/drivers/ptp/Makefile b/drivers/ptp/Makefile
index d11eeb5811d1..8673d1743faa 100644
--- a/drivers/ptp/Makefile
+++ b/drivers/ptp/Makefile
@@ -5,6 +5,7 @@
 
 ptp-y					:= ptp_clock.o ptp_chardev.o ptp_sysfs.o
 ptp_kvm-$(CONFIG_X86)			:= ptp_kvm_x86.o ptp_kvm_common.o
+ptp_kvm-$(CONFIG_HAVE_ARM_SMCCC)	:= ptp_kvm_arm.o ptp_kvm_common.o
 obj-$(CONFIG_PTP_1588_CLOCK)		+= ptp.o
 obj-$(CONFIG_PTP_1588_CLOCK_DTE)	+= ptp_dte.o
 obj-$(CONFIG_PTP_1588_CLOCK_INES)	+= ptp_ines.o
diff --git a/drivers/ptp/ptp_kvm_arm.c b/drivers/ptp/ptp_kvm_arm.c
new file mode 100644
index 000000000000..b7d28c8dfb84
--- /dev/null
+++ b/drivers/ptp/ptp_kvm_arm.c
@@ -0,0 +1,28 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ *  Virtual PTP 1588 clock for use with KVM guests
+ *  Copyright (C) 2019 ARM Ltd.
+ *  All Rights Reserved
+ */
+
+#include <linux/arm-smccc.h>
+#include <linux/ptp_kvm.h>
+
+#include <asm/arch_timer.h>
+#include <asm/hypervisor.h>
+
+int kvm_arch_ptp_init(void)
+{
+	int ret;
+
+	ret = kvm_arm_hyp_service_available(ARM_SMCCC_KVM_FUNC_PTP);
+	if (ret <= 0)
+		return -EOPNOTSUPP;
+
+	return 0;
+}
+
+int kvm_arch_ptp_get_clock(struct timespec64 *ts)
+{
+	return kvm_arch_ptp_get_crosststamp(NULL, ts, NULL);
+}
-- 
2.29.2


^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH v19 3/7] ptp: Reorganize ptp_kvm.c to make it arch-independent
  2021-03-30 14:54 ` [PATCH v19 3/7] ptp: Reorganize ptp_kvm.c to make it arch-independent Marc Zyngier
@ 2021-04-07  9:28   ` Marc Zyngier
  2021-04-07 15:13     ` Richard Cochran
  0 siblings, 1 reply; 25+ messages in thread
From: Marc Zyngier @ 2021-04-07  9:28 UTC (permalink / raw)
  To: pbonzini, richardcochran
  Cc: netdev, yangbo.lu, john.stultz, tglx, seanjc, Mark.Rutland, will,
	suzuki.poulose, Andre.Przywara, steven.price, lorenzo.pieralisi,
	sudeep.holla, linux-kernel, linux-arm-kernel, kvmarm, kvm,
	Steve.Capper, justin.he, jianyong.wu, kernel-team,
	Andre Przywara

On Tue, 30 Mar 2021 15:54:26 +0100,
Marc Zyngier <maz@kernel.org> wrote:
> 
> From: Jianyong Wu <jianyong.wu@arm.com>
> 
> Currently, the ptp_kvm module contains a lot of x86-specific code.
> Let's move this code into a new arch-specific file in the same directory,
> and rename the arch-independent file to ptp_kvm_common.c.
> 
> Reviewed-by: Andre Przywara <andre.przywara@arm.com>
> Signed-off-by: Jianyong Wu <jianyong.wu@arm.com>
> Signed-off-by: Marc Zyngier <maz@kernel.org>
> Link: https://lore.kernel.org/r/20201209060932.212364-4-jianyong.wu@arm.com

Richard, Paolo,

Can I get an Ack on this and patch #7? We're getting pretty close to
the next merge window, and this series has been going on for a couple
of years now...

Thanks,

	M.

-- 
Without deviation from the norm, progress is not possible.

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH v19 3/7] ptp: Reorganize ptp_kvm.c to make it arch-independent
  2021-04-07  9:28   ` Marc Zyngier
@ 2021-04-07 15:13     ` Richard Cochran
  2021-04-07 15:31       ` Marc Zyngier
  0 siblings, 1 reply; 25+ messages in thread
From: Richard Cochran @ 2021-04-07 15:13 UTC (permalink / raw)
  To: Marc Zyngier
  Cc: pbonzini, netdev, yangbo.lu, john.stultz, tglx, seanjc,
	Mark.Rutland, will, suzuki.poulose, Andre.Przywara, steven.price,
	lorenzo.pieralisi, sudeep.holla, linux-kernel, linux-arm-kernel,
	kvmarm, kvm, Steve.Capper, justin.he, jianyong.wu, kernel-team

On Wed, Apr 07, 2021 at 10:28:44AM +0100, Marc Zyngier wrote:
> On Tue, 30 Mar 2021 15:54:26 +0100,
> Marc Zyngier <maz@kernel.org> wrote:
> > 
> > From: Jianyong Wu <jianyong.wu@arm.com>
> > 
> > Currently, the ptp_kvm module contains a lot of x86-specific code.
> > Let's move this code into a new arch-specific file in the same directory,
> > and rename the arch-independent file to ptp_kvm_common.c.
> > 
> > Reviewed-by: Andre Przywara <andre.przywara@arm.com>
> > Signed-off-by: Jianyong Wu <jianyong.wu@arm.com>
> > Signed-off-by: Marc Zyngier <maz@kernel.org>
> > Link: https://lore.kernel.org/r/20201209060932.212364-4-jianyong.wu@arm.com
> 
> Richard, Paolo,
> 
> Can I get an Ack on this and patch #7? We're getting pretty close to
> the next merge window, and this series has been going on for a couple
> of years now...

For both patches:

Acked-by: Richard Cochran <richardcochran@gmail.com>

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH v19 3/7] ptp: Reorganize ptp_kvm.c to make it arch-independent
  2021-04-07 15:13     ` Richard Cochran
@ 2021-04-07 15:31       ` Marc Zyngier
  0 siblings, 0 replies; 25+ messages in thread
From: Marc Zyngier @ 2021-04-07 15:31 UTC (permalink / raw)
  To: Richard Cochran
  Cc: pbonzini, netdev, yangbo.lu, john.stultz, tglx, seanjc,
	Mark.Rutland, will, suzuki.poulose, Andre.Przywara, steven.price,
	lorenzo.pieralisi, sudeep.holla, linux-kernel, linux-arm-kernel,
	kvmarm, kvm, Steve.Capper, justin.he, jianyong.wu, kernel-team

On Wed, 07 Apr 2021 16:13:34 +0100,
Richard Cochran <richardcochran@gmail.com> wrote:
> 
> On Wed, Apr 07, 2021 at 10:28:44AM +0100, Marc Zyngier wrote:
> > On Tue, 30 Mar 2021 15:54:26 +0100,
> > Marc Zyngier <maz@kernel.org> wrote:
> > > 
> > > From: Jianyong Wu <jianyong.wu@arm.com>
> > > 
> > > Currently, the ptp_kvm module contains a lot of x86-specific code.
> > > Let's move this code into a new arch-specific file in the same directory,
> > > and rename the arch-independent file to ptp_kvm_common.c.
> > > 
> > > Reviewed-by: Andre Przywara <andre.przywara@arm.com>
> > > Signed-off-by: Jianyong Wu <jianyong.wu@arm.com>
> > > Signed-off-by: Marc Zyngier <maz@kernel.org>
> > > Link: https://lore.kernel.org/r/20201209060932.212364-4-jianyong.wu@arm.com
> > 
> > Richard, Paolo,
> > 
> > Can I get an Ack on this and patch #7? We're getting pretty close to
> > the next merge window, and this series has been going on for a couple
> > of years now...
> 
> For both patches:
> 
> Acked-by: Richard Cochran <richardcochran@gmail.com>

Cheers Richard, much appreciated.

	M.

-- 
Without deviation from the norm, progress is not possible.

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH v19 7/7] ptp: arm/arm64: Enable ptp_kvm for arm/arm64
  2021-03-30 14:54 ` [PATCH v19 7/7] ptp: arm/arm64: Enable ptp_kvm for arm/arm64 Marc Zyngier
@ 2021-04-17  8:42   ` Zenghui Yu
  2021-04-17  9:05     ` Marc Zyngier
  2021-05-11  9:07   ` Geert Uytterhoeven
  1 sibling, 1 reply; 25+ messages in thread
From: Zenghui Yu @ 2021-04-17  8:42 UTC (permalink / raw)
  To: Marc Zyngier
  Cc: netdev, yangbo.lu, john.stultz, tglx, pbonzini, seanjc,
	richardcochran, Mark.Rutland, will, suzuki.poulose,
	Andre.Przywara, steven.price, lorenzo.pieralisi, sudeep.holla,
	justin.he, kvm, linux-kernel, kernel-team, kvmarm,
	linux-arm-kernel

On 2021/3/30 22:54, Marc Zyngier wrote:
> +int kvm_arch_ptp_init(void)
> +{
> +	int ret;
> +
> +	ret = kvm_arm_hyp_service_available(ARM_SMCCC_KVM_FUNC_PTP);
> +	if (ret <= 0)

kvm_arm_hyp_service_available() returns boolean. Maybe write as ?

	bool ret;

	ret = kvm_arm_hyp_service_available();
	if (!ret)
		return -ENODEV;

> +		return -EOPNOTSUPP;
> +
> +	return 0;
> +}

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH v19 6/7] KVM: arm64: Add support for the KVM PTP service
  2021-03-30 14:54 ` [PATCH v19 6/7] KVM: arm64: Add support for the KVM PTP service Marc Zyngier
@ 2021-04-17  8:59   ` Zenghui Yu
  2021-04-17  9:10     ` Marc Zyngier
  0 siblings, 1 reply; 25+ messages in thread
From: Zenghui Yu @ 2021-04-17  8:59 UTC (permalink / raw)
  To: Marc Zyngier
  Cc: netdev, yangbo.lu, john.stultz, tglx, pbonzini, seanjc,
	richardcochran, Mark.Rutland, will, suzuki.poulose,
	Andre.Przywara, steven.price, lorenzo.pieralisi, sudeep.holla,
	justin.he, kvm, linux-kernel, kernel-team, kvmarm,
	linux-arm-kernel

On 2021/3/30 22:54, Marc Zyngier wrote:
> +PTP_KVM support for arm/arm64
> +=============================
> +
> +PTP_KVM is used for high precision time sync between host and guests.
> +It relies on transferring the wall clock and counter value from the
> +host to the guest using a KVM-specific hypercall.
> +
> +* ARM_SMCCC_HYP_KVM_PTP_FUNC_ID: 0x86000001

Per include/linux/arm-smccc.h, this should be
ARM_SMCCC_VENDOR_HYP_KVM_PTP_FUNC_ID.

> +
> +This hypercall uses the SMC32/HVC32 calling convention:
> +
> +ARM_SMCCC_HYP_KVM_PTP_FUNC_ID

ditto

> +    =============    ==========    ==========
> +    Function ID:     (uint32)      0x86000001
> +    Arguments:       (uint32)      KVM_PTP_VIRT_COUNTER(0)
> +                                   KVM_PTP_PHYS_COUNTER(1)

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH v19 7/7] ptp: arm/arm64: Enable ptp_kvm for arm/arm64
  2021-04-17  8:42   ` Zenghui Yu
@ 2021-04-17  9:05     ` Marc Zyngier
  0 siblings, 0 replies; 25+ messages in thread
From: Marc Zyngier @ 2021-04-17  9:05 UTC (permalink / raw)
  To: Zenghui Yu
  Cc: netdev, yangbo.lu, john.stultz, tglx, pbonzini, seanjc,
	richardcochran, Mark.Rutland, will, suzuki.poulose,
	Andre.Przywara, steven.price, lorenzo.pieralisi, sudeep.holla,
	justin.he, kvm, linux-kernel, kernel-team, kvmarm,
	linux-arm-kernel

On Sat, 17 Apr 2021 09:42:37 +0100,
Zenghui Yu <yuzenghui@huawei.com> wrote:
> 
> On 2021/3/30 22:54, Marc Zyngier wrote:
> > +int kvm_arch_ptp_init(void)
> > +{
> > +	int ret;
> > +
> > +	ret = kvm_arm_hyp_service_available(ARM_SMCCC_KVM_FUNC_PTP);
> > +	if (ret <= 0)
> 
> kvm_arm_hyp_service_available() returns boolean. Maybe write as ?
> 
> 	bool ret;
> 
> 	ret = kvm_arm_hyp_service_available();
> 	if (!ret)
> 		return -ENODEV;

Fixed in 300bb1fe7671, as previously reported by Dan Carpenter in [1].

Thanks,

	M.

https://lore.kernel.org/r/20210331043704.GG2065@kadam

-- 
Without deviation from the norm, progress is not possible.

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH v19 6/7] KVM: arm64: Add support for the KVM PTP service
  2021-04-17  8:59   ` Zenghui Yu
@ 2021-04-17  9:10     ` Marc Zyngier
  2021-04-17 10:24       ` Zenghui Yu
  0 siblings, 1 reply; 25+ messages in thread
From: Marc Zyngier @ 2021-04-17  9:10 UTC (permalink / raw)
  To: Zenghui Yu
  Cc: netdev, yangbo.lu, john.stultz, tglx, pbonzini, seanjc,
	richardcochran, Mark.Rutland, will, suzuki.poulose,
	Andre.Przywara, steven.price, lorenzo.pieralisi, sudeep.holla,
	justin.he, kvm, linux-kernel, kernel-team, kvmarm,
	linux-arm-kernel

On Sat, 17 Apr 2021 09:59:39 +0100,
Zenghui Yu <yuzenghui@huawei.com> wrote:
> 
> On 2021/3/30 22:54, Marc Zyngier wrote:
> > +PTP_KVM support for arm/arm64
> > +=============================
> > +
> > +PTP_KVM is used for high precision time sync between host and guests.
> > +It relies on transferring the wall clock and counter value from the
> > +host to the guest using a KVM-specific hypercall.
> > +
> > +* ARM_SMCCC_HYP_KVM_PTP_FUNC_ID: 0x86000001
> 
> Per include/linux/arm-smccc.h, this should be
> ARM_SMCCC_VENDOR_HYP_KVM_PTP_FUNC_ID.

Well spotted. Care to send a patch on top of my kvm-arm64/ptp branch?

Thanks,

	M.

-- 
Without deviation from the norm, progress is not possible.

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH v19 4/7] time: Add mechanism to recognize clocksource in time_get_snapshot
  2021-03-30 14:54 ` [PATCH v19 4/7] time: Add mechanism to recognize clocksource in time_get_snapshot Marc Zyngier
@ 2021-04-17  9:27   ` Zenghui Yu
  0 siblings, 0 replies; 25+ messages in thread
From: Zenghui Yu @ 2021-04-17  9:27 UTC (permalink / raw)
  To: Marc Zyngier
  Cc: netdev, yangbo.lu, john.stultz, tglx, pbonzini, seanjc,
	richardcochran, Mark.Rutland, will, suzuki.poulose,
	Andre.Przywara, steven.price, lorenzo.pieralisi, sudeep.holla,
	justin.he, kvm, linux-kernel, kernel-team, kvmarm,
	linux-arm-kernel

On 2021/3/30 22:54, Marc Zyngier wrote:
> -	u64		cycles;
> -	ktime_t		real;
> -	ktime_t		raw;
> -	unsigned int	clock_was_set_seq;
> -	u8		cs_was_changed_seq;
> +	u64			cycles;
> +	ktime_t			real;
> +	ktime_t			raw;
> +	enum clocksource_ids	cs_id;

nit: worth adding a description for it (on top of the structure)?

> +	unsigned int		clock_was_set_seq;
> +	u8			cs_was_changed_seq;

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH v19 1/7] arm/arm64: Probe for the presence of KVM hypervisor
  2021-03-30 14:54 ` [PATCH v19 1/7] arm/arm64: Probe for the presence of KVM hypervisor Marc Zyngier
@ 2021-04-17  9:49   ` Zenghui Yu
  0 siblings, 0 replies; 25+ messages in thread
From: Zenghui Yu @ 2021-04-17  9:49 UTC (permalink / raw)
  To: Marc Zyngier
  Cc: netdev, yangbo.lu, john.stultz, tglx, pbonzini, seanjc,
	richardcochran, Mark.Rutland, will, suzuki.poulose,
	Andre.Przywara, steven.price, lorenzo.pieralisi, sudeep.holla,
	justin.he, kvm, linux-kernel, kernel-team, kvmarm,
	linux-arm-kernel

On 2021/3/30 22:54, Marc Zyngier wrote:
>  #define SMCCC_ARCH_WORKAROUND_RET_UNAFFECTED	1

I think it'd be better to keep this definition together with other
wa Function IDs. It's only a cosmetic comment anyway.


Zenghui

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH v19 6/7] KVM: arm64: Add support for the KVM PTP service
  2021-04-17  9:10     ` Marc Zyngier
@ 2021-04-17 10:24       ` Zenghui Yu
  0 siblings, 0 replies; 25+ messages in thread
From: Zenghui Yu @ 2021-04-17 10:24 UTC (permalink / raw)
  To: Marc Zyngier
  Cc: netdev, yangbo.lu, john.stultz, tglx, pbonzini, seanjc,
	richardcochran, Mark.Rutland, will, suzuki.poulose,
	Andre.Przywara, steven.price, lorenzo.pieralisi, sudeep.holla,
	justin.he, kvm, linux-kernel, kernel-team, kvmarm,
	linux-arm-kernel

On 2021/4/17 17:10, Marc Zyngier wrote:
> On Sat, 17 Apr 2021 09:59:39 +0100,
> Zenghui Yu <yuzenghui@huawei.com> wrote:
>>
>> On 2021/3/30 22:54, Marc Zyngier wrote:
>>> +PTP_KVM support for arm/arm64
>>> +=============================
>>> +
>>> +PTP_KVM is used for high precision time sync between host and guests.
>>> +It relies on transferring the wall clock and counter value from the
>>> +host to the guest using a KVM-specific hypercall.
>>> +
>>> +* ARM_SMCCC_HYP_KVM_PTP_FUNC_ID: 0x86000001
>>
>> Per include/linux/arm-smccc.h, this should be
>> ARM_SMCCC_VENDOR_HYP_KVM_PTP_FUNC_ID.
> 
> Well spotted. Care to send a patch on top of my kvm-arm64/ptp branch?

Okay. I'll have a patch for that.


Zenghui

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH v19 7/7] ptp: arm/arm64: Enable ptp_kvm for arm/arm64
  2021-03-30 14:54 ` [PATCH v19 7/7] ptp: arm/arm64: Enable ptp_kvm for arm/arm64 Marc Zyngier
  2021-04-17  8:42   ` Zenghui Yu
@ 2021-05-11  9:07   ` Geert Uytterhoeven
  2021-05-11  9:13     ` Marc Zyngier
  1 sibling, 1 reply; 25+ messages in thread
From: Geert Uytterhoeven @ 2021-05-11  9:07 UTC (permalink / raw)
  To: Marc Zyngier, jianyong.wu
  Cc: netdev, Yangbo Lu, John Stultz, Thomas Gleixner, Paolo Bonzini,
	seanjc, Richard Cochran, Mark Rutland, Will Deacon,
	Suzuki K Poulose, Andre Przywara, Steven Price,
	Lorenzo Pieralisi, Sudeep Holla, Linux Kernel Mailing List,
	Linux ARM, kvmarm, KVM list, Steve Capper, justin.he,
	Android Kernel Team

Hi Marc, Jianyong,

On Tue, Mar 30, 2021 at 4:56 PM Marc Zyngier <maz@kernel.org> wrote:
> From: Jianyong Wu <jianyong.wu@arm.com>
>
> Currently, there is no mechanism to keep time sync between guest and host
> in arm/arm64 virtualization environment. Time in guest will drift compared
> with host after boot up as they may both use third party time sources
> to correct their time respectively. The time deviation will be in order
> of milliseconds. But in some scenarios,like in cloud environment, we ask
> for higher time precision.
>
> kvm ptp clock, which chooses the host clock source as a reference
> clock to sync time between guest and host, has been adopted by x86
> which takes the time sync order from milliseconds to nanoseconds.
>
> This patch enables kvm ptp clock for arm/arm64 and improves clock sync precision
> significantly.

> --- a/drivers/ptp/Kconfig
> +++ b/drivers/ptp/Kconfig
> @@ -108,7 +108,7 @@ config PTP_1588_CLOCK_PCH
>  config PTP_1588_CLOCK_KVM
>         tristate "KVM virtual PTP clock"
>         depends on PTP_1588_CLOCK
> -       depends on KVM_GUEST && X86
> +       depends on (KVM_GUEST && X86) || (HAVE_ARM_SMCCC_DISCOVERY && ARM_ARCH_TIMER)

Why does this not depend on KVM_GUEST on ARM?
I.e. shouldn't the dependency be:

    KVM_GUEST && (X86 || (HAVE_ARM_SMCCC_DISCOVERY && ARM_ARCH_TIMER))

?

>         default y
>         help
>           This driver adds support for using kvm infrastructure as a PTP

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH v19 7/7] ptp: arm/arm64: Enable ptp_kvm for arm/arm64
  2021-05-11  9:07   ` Geert Uytterhoeven
@ 2021-05-11  9:13     ` Marc Zyngier
  2021-05-26  7:52       ` Geert Uytterhoeven
  0 siblings, 1 reply; 25+ messages in thread
From: Marc Zyngier @ 2021-05-11  9:13 UTC (permalink / raw)
  To: Geert Uytterhoeven
  Cc: jianyong.wu, netdev, Yangbo Lu, John Stultz, Thomas Gleixner,
	Paolo Bonzini, seanjc, Richard Cochran, Mark Rutland,
	Will Deacon, Suzuki K Poulose, Andre Przywara, Steven Price,
	Lorenzo Pieralisi, Sudeep Holla, Linux Kernel Mailing List,
	Linux ARM, kvmarm, KVM list, Steve Capper, justin.he,
	Android Kernel Team

Hi Geert,

On 2021-05-11 10:07, Geert Uytterhoeven wrote:
> Hi Marc, Jianyong,
> 
> On Tue, Mar 30, 2021 at 4:56 PM Marc Zyngier <maz@kernel.org> wrote:
>> From: Jianyong Wu <jianyong.wu@arm.com>
>> 
>> Currently, there is no mechanism to keep time sync between guest and 
>> host
>> in arm/arm64 virtualization environment. Time in guest will drift 
>> compared
>> with host after boot up as they may both use third party time sources
>> to correct their time respectively. The time deviation will be in 
>> order
>> of milliseconds. But in some scenarios,like in cloud environment, we 
>> ask
>> for higher time precision.
>> 
>> kvm ptp clock, which chooses the host clock source as a reference
>> clock to sync time between guest and host, has been adopted by x86
>> which takes the time sync order from milliseconds to nanoseconds.
>> 
>> This patch enables kvm ptp clock for arm/arm64 and improves clock sync 
>> precision
>> significantly.
> 
>> --- a/drivers/ptp/Kconfig
>> +++ b/drivers/ptp/Kconfig
>> @@ -108,7 +108,7 @@ config PTP_1588_CLOCK_PCH
>>  config PTP_1588_CLOCK_KVM
>>         tristate "KVM virtual PTP clock"
>>         depends on PTP_1588_CLOCK
>> -       depends on KVM_GUEST && X86
>> +       depends on (KVM_GUEST && X86) || (HAVE_ARM_SMCCC_DISCOVERY && 
>> ARM_ARCH_TIMER)
> 
> Why does this not depend on KVM_GUEST on ARM?
> I.e. shouldn't the dependency be:
> 
>     KVM_GUEST && (X86 || (HAVE_ARM_SMCCC_DISCOVERY && ARM_ARCH_TIMER))
> 
> ?

arm/arm64 do not select KVM_GUEST. Any kernel can be used for a guest,
and KVM/arm64 doesn't know about this configuration symbol.

Thanks,

         M.
-- 
Jazz is not dead. It just smells funny...

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH v19 7/7] ptp: arm/arm64: Enable ptp_kvm for arm/arm64
  2021-05-11  9:13     ` Marc Zyngier
@ 2021-05-26  7:52       ` Geert Uytterhoeven
  2021-05-26  8:01         ` Marc Zyngier
  0 siblings, 1 reply; 25+ messages in thread
From: Geert Uytterhoeven @ 2021-05-26  7:52 UTC (permalink / raw)
  To: Marc Zyngier
  Cc: jianyong.wu, netdev, Yangbo Lu, John Stultz, Thomas Gleixner,
	Paolo Bonzini, seanjc, Richard Cochran, Mark Rutland,
	Will Deacon, Suzuki K Poulose, Andre Przywara, Steven Price,
	Lorenzo Pieralisi, Sudeep Holla, Linux Kernel Mailing List,
	Linux ARM, kvmarm, KVM list, Steve Capper, justin.he,
	Android Kernel Team

Hi Marc,

On Tue, May 11, 2021 at 11:13 AM Marc Zyngier <maz@kernel.org> wrote:
> On 2021-05-11 10:07, Geert Uytterhoeven wrote:
> > On Tue, Mar 30, 2021 at 4:56 PM Marc Zyngier <maz@kernel.org> wrote:
> >> From: Jianyong Wu <jianyong.wu@arm.com>
> >>
> >> Currently, there is no mechanism to keep time sync between guest and
> >> host
> >> in arm/arm64 virtualization environment. Time in guest will drift
> >> compared
> >> with host after boot up as they may both use third party time sources
> >> to correct their time respectively. The time deviation will be in
> >> order
> >> of milliseconds. But in some scenarios,like in cloud environment, we
> >> ask
> >> for higher time precision.
> >>
> >> kvm ptp clock, which chooses the host clock source as a reference
> >> clock to sync time between guest and host, has been adopted by x86
> >> which takes the time sync order from milliseconds to nanoseconds.
> >>
> >> This patch enables kvm ptp clock for arm/arm64 and improves clock sync
> >> precision
> >> significantly.
> >
> >> --- a/drivers/ptp/Kconfig
> >> +++ b/drivers/ptp/Kconfig
> >> @@ -108,7 +108,7 @@ config PTP_1588_CLOCK_PCH
> >>  config PTP_1588_CLOCK_KVM
> >>         tristate "KVM virtual PTP clock"
> >>         depends on PTP_1588_CLOCK
> >> -       depends on KVM_GUEST && X86
> >> +       depends on (KVM_GUEST && X86) || (HAVE_ARM_SMCCC_DISCOVERY &&
> >> ARM_ARCH_TIMER)
> >
> > Why does this not depend on KVM_GUEST on ARM?
> > I.e. shouldn't the dependency be:
> >
> >     KVM_GUEST && (X86 || (HAVE_ARM_SMCCC_DISCOVERY && ARM_ARCH_TIMER))
> >
> > ?
>
> arm/arm64 do not select KVM_GUEST. Any kernel can be used for a guest,
> and KVM/arm64 doesn't know about this configuration symbol.

OK.

Does PTP_1588_CLOCK_KVM need to default to yes?
Perhaps only on X86, to maintain the status quo?

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH v19 7/7] ptp: arm/arm64: Enable ptp_kvm for arm/arm64
  2021-05-26  7:52       ` Geert Uytterhoeven
@ 2021-05-26  8:01         ` Marc Zyngier
  2021-05-26  8:18           ` Geert Uytterhoeven
  0 siblings, 1 reply; 25+ messages in thread
From: Marc Zyngier @ 2021-05-26  8:01 UTC (permalink / raw)
  To: Geert Uytterhoeven
  Cc: jianyong.wu, netdev, Yangbo Lu, John Stultz, Thomas Gleixner,
	Paolo Bonzini, seanjc, Richard Cochran, Mark Rutland,
	Will Deacon, Suzuki K Poulose, Andre Przywara, Steven Price,
	Lorenzo Pieralisi, Sudeep Holla, Linux Kernel Mailing List,
	Linux ARM, kvmarm, KVM list, Steve Capper, justin.he,
	Android Kernel Team

Hi Geert,

On Wed, 26 May 2021 08:52:42 +0100,
Geert Uytterhoeven <geert@linux-m68k.org> wrote:
> 
> Hi Marc,
> 
> On Tue, May 11, 2021 at 11:13 AM Marc Zyngier <maz@kernel.org> wrote:
> > On 2021-05-11 10:07, Geert Uytterhoeven wrote:
> > > On Tue, Mar 30, 2021 at 4:56 PM Marc Zyngier <maz@kernel.org> wrote:
> > >> From: Jianyong Wu <jianyong.wu@arm.com>
> > >
> > >> --- a/drivers/ptp/Kconfig
> > >> +++ b/drivers/ptp/Kconfig
> > >> @@ -108,7 +108,7 @@ config PTP_1588_CLOCK_PCH
> > >>  config PTP_1588_CLOCK_KVM
> > >>         tristate "KVM virtual PTP clock"
> > >>         depends on PTP_1588_CLOCK
> > >> -       depends on KVM_GUEST && X86
> > >> +       depends on (KVM_GUEST && X86) || (HAVE_ARM_SMCCC_DISCOVERY &&
> > >> ARM_ARCH_TIMER)
> > >
> > > Why does this not depend on KVM_GUEST on ARM?
> > > I.e. shouldn't the dependency be:
> > >
> > >     KVM_GUEST && (X86 || (HAVE_ARM_SMCCC_DISCOVERY && ARM_ARCH_TIMER))
> > >
> > > ?
> >
> > arm/arm64 do not select KVM_GUEST. Any kernel can be used for a guest,
> > and KVM/arm64 doesn't know about this configuration symbol.
> 
> OK.
> 
> Does PTP_1588_CLOCK_KVM need to default to yes?
> Perhaps only on X86, to maintain the status quo?

I think I don't really understand the problem you are trying to
solve. Is it that 'make oldconfig' now asks you about this new driver?
Why is that an issue?

Thanks,

	M.

-- 
Without deviation from the norm, progress is not possible.

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH v19 7/7] ptp: arm/arm64: Enable ptp_kvm for arm/arm64
  2021-05-26  8:01         ` Marc Zyngier
@ 2021-05-26  8:18           ` Geert Uytterhoeven
  2021-05-26  8:32             ` Marc Zyngier
  0 siblings, 1 reply; 25+ messages in thread
From: Geert Uytterhoeven @ 2021-05-26  8:18 UTC (permalink / raw)
  To: Marc Zyngier
  Cc: jianyong.wu, netdev, Yangbo Lu, John Stultz, Thomas Gleixner,
	Paolo Bonzini, seanjc, Richard Cochran, Mark Rutland,
	Will Deacon, Suzuki K Poulose, Andre Przywara, Steven Price,
	Lorenzo Pieralisi, Sudeep Holla, Linux Kernel Mailing List,
	Linux ARM, kvmarm, KVM list, Steve Capper, justin.he,
	Android Kernel Team

Hi Marc,

On Wed, May 26, 2021 at 10:01 AM Marc Zyngier <maz@kernel.org> wrote:
> On Wed, 26 May 2021 08:52:42 +0100,
> Geert Uytterhoeven <geert@linux-m68k.org> wrote:
> > On Tue, May 11, 2021 at 11:13 AM Marc Zyngier <maz@kernel.org> wrote:
> > > On 2021-05-11 10:07, Geert Uytterhoeven wrote:
> > > > On Tue, Mar 30, 2021 at 4:56 PM Marc Zyngier <maz@kernel.org> wrote:
> > > >> From: Jianyong Wu <jianyong.wu@arm.com>
> > > >
> > > >> --- a/drivers/ptp/Kconfig
> > > >> +++ b/drivers/ptp/Kconfig
> > > >> @@ -108,7 +108,7 @@ config PTP_1588_CLOCK_PCH
> > > >>  config PTP_1588_CLOCK_KVM
> > > >>         tristate "KVM virtual PTP clock"
> > > >>         depends on PTP_1588_CLOCK
> > > >> -       depends on KVM_GUEST && X86
> > > >> +       depends on (KVM_GUEST && X86) || (HAVE_ARM_SMCCC_DISCOVERY &&
> > > >> ARM_ARCH_TIMER)
> > > >
> > > > Why does this not depend on KVM_GUEST on ARM?
> > > > I.e. shouldn't the dependency be:
> > > >
> > > >     KVM_GUEST && (X86 || (HAVE_ARM_SMCCC_DISCOVERY && ARM_ARCH_TIMER))
> > > >
> > > > ?
> > >
> > > arm/arm64 do not select KVM_GUEST. Any kernel can be used for a guest,
> > > and KVM/arm64 doesn't know about this configuration symbol.
> >
> > OK.
> >
> > Does PTP_1588_CLOCK_KVM need to default to yes?
> > Perhaps only on X86, to maintain the status quo?
>
> I think I don't really understand the problem you are trying to
> solve. Is it that 'make oldconfig' now asks you about this new driver?
> Why is that an issue?

My first "problem" was that it asked about this new driver on
arm/arm64, while I assumed there were some missing dependencies
(configuring a kernel should not ask useless questions).  That turned
out to be a wrong assumption, so there is no such problem here.

The second problem is "default y": code that is not critical should
not be enabled by default.  Hence my last question.

Thanks!

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH v19 7/7] ptp: arm/arm64: Enable ptp_kvm for arm/arm64
  2021-05-26  8:18           ` Geert Uytterhoeven
@ 2021-05-26  8:32             ` Marc Zyngier
  2021-05-26  8:50               ` Geert Uytterhoeven
  0 siblings, 1 reply; 25+ messages in thread
From: Marc Zyngier @ 2021-05-26  8:32 UTC (permalink / raw)
  To: Geert Uytterhoeven
  Cc: jianyong.wu, netdev, Yangbo Lu, John Stultz, Thomas Gleixner,
	Paolo Bonzini, seanjc, Richard Cochran, Mark Rutland,
	Will Deacon, Suzuki K Poulose, Andre Przywara, Steven Price,
	Lorenzo Pieralisi, Sudeep Holla, Linux Kernel Mailing List,
	Linux ARM, kvmarm, KVM list, Steve Capper, justin.he,
	Android Kernel Team

On Wed, 26 May 2021 09:18:27 +0100,
Geert Uytterhoeven <geert@linux-m68k.org> wrote:
> 
> Hi Marc,
> 
> On Wed, May 26, 2021 at 10:01 AM Marc Zyngier <maz@kernel.org> wrote:
> > On Wed, 26 May 2021 08:52:42 +0100,
> > Geert Uytterhoeven <geert@linux-m68k.org> wrote:
> > > On Tue, May 11, 2021 at 11:13 AM Marc Zyngier <maz@kernel.org> wrote:
> > > > On 2021-05-11 10:07, Geert Uytterhoeven wrote:
> > > > > On Tue, Mar 30, 2021 at 4:56 PM Marc Zyngier <maz@kernel.org> wrote:
> > > > >> From: Jianyong Wu <jianyong.wu@arm.com>
> > > > >
> > > > >> --- a/drivers/ptp/Kconfig
> > > > >> +++ b/drivers/ptp/Kconfig
> > > > >> @@ -108,7 +108,7 @@ config PTP_1588_CLOCK_PCH
> > > > >>  config PTP_1588_CLOCK_KVM
> > > > >>         tristate "KVM virtual PTP clock"
> > > > >>         depends on PTP_1588_CLOCK
> > > > >> -       depends on KVM_GUEST && X86
> > > > >> +       depends on (KVM_GUEST && X86) || (HAVE_ARM_SMCCC_DISCOVERY &&
> > > > >> ARM_ARCH_TIMER)
> > > > >
> > > > > Why does this not depend on KVM_GUEST on ARM?
> > > > > I.e. shouldn't the dependency be:
> > > > >
> > > > >     KVM_GUEST && (X86 || (HAVE_ARM_SMCCC_DISCOVERY && ARM_ARCH_TIMER))
> > > > >
> > > > > ?
> > > >
> > > > arm/arm64 do not select KVM_GUEST. Any kernel can be used for a guest,
> > > > and KVM/arm64 doesn't know about this configuration symbol.
> > >
> > > OK.
> > >
> > > Does PTP_1588_CLOCK_KVM need to default to yes?
> > > Perhaps only on X86, to maintain the status quo?
> >
> > I think I don't really understand the problem you are trying to
> > solve. Is it that 'make oldconfig' now asks you about this new driver?
> > Why is that an issue?
> 
> My first "problem" was that it asked about this new driver on
> arm/arm64, while I assumed there were some missing dependencies
> (configuring a kernel should not ask useless questions).  That turned
> out to be a wrong assumption, so there is no such problem here.
> 
> The second problem is "default y": code that is not critical should
> not be enabled by default.  Hence my last question.

I think consistency between architectures is important. Certainly,
distributions depend on that, and we otherwise end-up with distro
kernels missing functionalities.

The notion of "critical" is also pretty relative. defconfig contains a
gazillion of things that are not critical to most people, for example,
and yet misses a bunch of things that are needed to boot on some of my
systems.

That's just to say that I find it difficult to make that choice from
the PoV of a kernel hacker. I'm personally more inclined to leave
things enabled and let people *disable* things if they want to reduce
the footprint of their kernel.

Thanks,

	M.

-- 
Without deviation from the norm, progress is not possible.

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH v19 7/7] ptp: arm/arm64: Enable ptp_kvm for arm/arm64
  2021-05-26  8:32             ` Marc Zyngier
@ 2021-05-26  8:50               ` Geert Uytterhoeven
  0 siblings, 0 replies; 25+ messages in thread
From: Geert Uytterhoeven @ 2021-05-26  8:50 UTC (permalink / raw)
  To: Marc Zyngier
  Cc: jianyong.wu, netdev, Yangbo Lu, John Stultz, Thomas Gleixner,
	Paolo Bonzini, seanjc, Richard Cochran, Mark Rutland,
	Will Deacon, Suzuki K Poulose, Andre Przywara, Steven Price,
	Lorenzo Pieralisi, Sudeep Holla, Linux Kernel Mailing List,
	Linux ARM, kvmarm, KVM list, Steve Capper, justin.he,
	Android Kernel Team

Hi Marc,

On Wed, May 26, 2021 at 10:32 AM Marc Zyngier <maz@kernel.org> wrote:
> On Wed, 26 May 2021 09:18:27 +0100,
> Geert Uytterhoeven <geert@linux-m68k.org> wrote:
> > On Wed, May 26, 2021 at 10:01 AM Marc Zyngier <maz@kernel.org> wrote:
> > > On Wed, 26 May 2021 08:52:42 +0100,
> > > Geert Uytterhoeven <geert@linux-m68k.org> wrote:
> > > > On Tue, May 11, 2021 at 11:13 AM Marc Zyngier <maz@kernel.org> wrote:
> > > > > On 2021-05-11 10:07, Geert Uytterhoeven wrote:
> > > > > > On Tue, Mar 30, 2021 at 4:56 PM Marc Zyngier <maz@kernel.org> wrote:
> > > > > >> From: Jianyong Wu <jianyong.wu@arm.com>
> > > > > >
> > > > > >> --- a/drivers/ptp/Kconfig
> > > > > >> +++ b/drivers/ptp/Kconfig
> > > > > >> @@ -108,7 +108,7 @@ config PTP_1588_CLOCK_PCH
> > > > > >>  config PTP_1588_CLOCK_KVM
> > > > > >>         tristate "KVM virtual PTP clock"
> > > > > >>         depends on PTP_1588_CLOCK
> > > > > >> -       depends on KVM_GUEST && X86
> > > > > >> +       depends on (KVM_GUEST && X86) || (HAVE_ARM_SMCCC_DISCOVERY &&
> > > > > >> ARM_ARCH_TIMER)
> > > > > >
> > > > > > Why does this not depend on KVM_GUEST on ARM?
> > > > > > I.e. shouldn't the dependency be:
> > > > > >
> > > > > >     KVM_GUEST && (X86 || (HAVE_ARM_SMCCC_DISCOVERY && ARM_ARCH_TIMER))
> > > > > >
> > > > > > ?
> > > > >
> > > > > arm/arm64 do not select KVM_GUEST. Any kernel can be used for a guest,
> > > > > and KVM/arm64 doesn't know about this configuration symbol.
> > > >
> > > > OK.
> > > >
> > > > Does PTP_1588_CLOCK_KVM need to default to yes?
> > > > Perhaps only on X86, to maintain the status quo?
> > >
> > > I think I don't really understand the problem you are trying to
> > > solve. Is it that 'make oldconfig' now asks you about this new driver?
> > > Why is that an issue?
> >
> > My first "problem" was that it asked about this new driver on
> > arm/arm64, while I assumed there were some missing dependencies
> > (configuring a kernel should not ask useless questions).  That turned
> > out to be a wrong assumption, so there is no such problem here.
> >
> > The second problem is "default y": code that is not critical should
> > not be enabled by default.  Hence my last question.
>
> I think consistency between architectures is important. Certainly,
> distributions depend on that, and we otherwise end-up with distro
> kernels missing functionalities.
>
> The notion of "critical" is also pretty relative. defconfig contains a

I'm not talking about defconfig, but about "default y" in defconfig.

> gazillion of things that are not critical to most people, for example,
> and yet misses a bunch of things that are needed to boot on some of my
> systems.

Perhaps those should be added, so those systems can be tested using
defconfig?  At least for arm64, I think that's aligned with the
arm64 defconfig policy.

> That's just to say that I find it difficult to make that choice from
> the PoV of a kernel hacker. I'm personally more inclined to leave
> things enabled and let people *disable* things if they want to reduce
> the footprint of their kernel.

The standard question to respond to w.r.t. "default y" is: "Why is
your feature so special that it needs to be enabled by default?",
which implies "default y" is the exception, not the rule.

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

^ permalink raw reply	[flat|nested] 25+ messages in thread

end of thread, back to index

Thread overview: 25+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-03-30 14:54 [PATCH v19 0/7] KVM: arm64: Add host/guest KVM-PTP support Marc Zyngier
2021-03-30 14:54 ` [PATCH v19 1/7] arm/arm64: Probe for the presence of KVM hypervisor Marc Zyngier
2021-04-17  9:49   ` Zenghui Yu
2021-03-30 14:54 ` [PATCH v19 2/7] KVM: arm64: Advertise KVM UID to guests via SMCCC Marc Zyngier
2021-03-30 14:54 ` [PATCH v19 3/7] ptp: Reorganize ptp_kvm.c to make it arch-independent Marc Zyngier
2021-04-07  9:28   ` Marc Zyngier
2021-04-07 15:13     ` Richard Cochran
2021-04-07 15:31       ` Marc Zyngier
2021-03-30 14:54 ` [PATCH v19 4/7] time: Add mechanism to recognize clocksource in time_get_snapshot Marc Zyngier
2021-04-17  9:27   ` Zenghui Yu
2021-03-30 14:54 ` [PATCH v19 5/7] clocksource: Add clocksource id for arm arch counter Marc Zyngier
2021-03-30 14:54 ` [PATCH v19 6/7] KVM: arm64: Add support for the KVM PTP service Marc Zyngier
2021-04-17  8:59   ` Zenghui Yu
2021-04-17  9:10     ` Marc Zyngier
2021-04-17 10:24       ` Zenghui Yu
2021-03-30 14:54 ` [PATCH v19 7/7] ptp: arm/arm64: Enable ptp_kvm for arm/arm64 Marc Zyngier
2021-04-17  8:42   ` Zenghui Yu
2021-04-17  9:05     ` Marc Zyngier
2021-05-11  9:07   ` Geert Uytterhoeven
2021-05-11  9:13     ` Marc Zyngier
2021-05-26  7:52       ` Geert Uytterhoeven
2021-05-26  8:01         ` Marc Zyngier
2021-05-26  8:18           ` Geert Uytterhoeven
2021-05-26  8:32             ` Marc Zyngier
2021-05-26  8:50               ` Geert Uytterhoeven

KVM Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/kvm/0 kvm/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 kvm kvm/ https://lore.kernel.org/kvm \
		kvm@vger.kernel.org
	public-inbox-index kvm

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.kvm


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git