| Message ID | 20210330145430.996981-8-maz@kernel.org (mailing list archive) |
|---|---|
| State | Not Applicable |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | KVM: arm64: Add host/guest KVM-PTP support | expand |
| Context | Check | Description |
|---|---|---|
| netdev/tree_selection | success | Not a local patch |
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; > +}
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
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
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.
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
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.
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
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.
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
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); +}