| Message ID | 20231218073849.35294-7-peter.hilber@opensynergy.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Add virtio_rtc module and related changes | expand |
On Mon, 2023-12-18 at 08:38 +0100, Peter Hilber wrote: > > +int viortc_hw_xtstamp_params(u16 *hw_counter, enum clocksource_ids *cs_id) > +{ > + *hw_counter = VIRTIO_RTC_COUNTER_ARM_VIRT; Hm, but what if it isn't? I think you need to put this in drivers/clocksource/arm_arch_timer.c where it can do something like kvm_arch_ptp_get_crosststamp() does to decide: if (arch_timer_uses_ppi == ARCH_TIMER_VIRT_PPI) ptp_counter = KVM_PTP_VIRT_COUNTER; else ptp_counter = KVM_PTP_PHYS_COUNTER; > + *cs_id = CSID_ARM_ARCH_COUNTER; > + > + return 0; > +}
Changing virtio-dev address to the new one. On 15.06.24 09:50, David Woodhouse wrote: > On Mon, 2023-12-18 at 08:38 +0100, Peter Hilber wrote: >> >> +int viortc_hw_xtstamp_params(u16 *hw_counter, enum clocksource_ids *cs_id) >> +{ >> + *hw_counter = VIRTIO_RTC_COUNTER_ARM_VIRT; > > Hm, but what if it isn't? I think you need to put this in > drivers/clocksource/arm_arch_timer.c where it can do something like > kvm_arch_ptp_get_crosststamp() does to decide: > > if (arch_timer_uses_ppi == ARCH_TIMER_VIRT_PPI) > ptp_counter = KVM_PTP_VIRT_COUNTER; > else > ptp_counter = KVM_PTP_PHYS_COUNTER; > > >> + *cs_id = CSID_ARM_ARCH_COUNTER; >> + >> + return 0; >> +} > I had such a check in v1, but Marc Zyngier didn't like the distinction [1, 2] - maybe also exposing the timer through a generic helper was not desired. Quoting from [2]: >> This was the rationale to come up with the physical/virtual counter >> distinction for the Virtio RTC device. Looking at extensions such as >> FEAT_ECV, where the CNTPCT_EL0 value can depend on the EL, or FEAT_NV*, >> it might be a bit simplistic. > > Not just simplistic. It doesn't make sense. For this to work, you'd > need to know the global offset that KVM applies to the global counter, > plus the *virtualised* CNTPOFF/CNTVOFF values that the guest can > change at any time on a *per-CPU* basis. None of that is available > outside of KVM, nor would it make any sense anyway. > >> Does this physical/virtual counter distinction sound like a good idea? >> Otherwise I would drop the arch_timer_counter_get_type() in the next >> iteration. > > My take on this is that only the global counter value makes any sense. > That value is already available from the host as the virtual counter, > because we guarantee that CNTVOFF is 0 when outside of the guest > (well, technically, outside of the vcpu_load/vcpu_put section). So I put the assumption that the virtual counter will always be the right choice for current Linux kernels, from the Virtio device POV. Thanks for the comment, Peter [1] https://lore.kernel.org/lkml/20230630171052.985577-1-peter.hilber@opensynergy.com/T/#ma8d596de1cbc8f4a78a18b2aa995db18423494a7 [2] https://lore.kernel.org/lkml/20230630171052.985577-1-peter.hilber@opensynergy.com/T/#m65fa1d715933360498c4e33d7225e4220215a9d6
diff --git a/drivers/virtio/Kconfig b/drivers/virtio/Kconfig index 8542b2f20201..d35c728778d2 100644 --- a/drivers/virtio/Kconfig +++ b/drivers/virtio/Kconfig @@ -205,6 +205,19 @@ config VIRTIO_RTC_PTP If unsure, say Y. +config VIRTIO_RTC_ARM + bool "Virtio RTC cross-timestamping using Arm Generic Timer" + default y + depends on VIRTIO_RTC_PTP && ARM_ARCH_TIMER + help + This enables Virtio RTC cross-timestamping using the Arm Generic Timer. + It only has an effect if the Virtio RTC device also supports this. The + cross-timestamp is available through the PTP clock driver precise + cross-timestamp ioctl (PTP_SYS_OFFSET_PRECISE2 or + PTP_SYS_OFFSET_PRECISE). + + If unsure, say Y. + endif # VIRTIO_RTC endif # VIRTIO_MENU diff --git a/drivers/virtio/Makefile b/drivers/virtio/Makefile index 4d48cbcae6bb..781dff9f8822 100644 --- a/drivers/virtio/Makefile +++ b/drivers/virtio/Makefile @@ -15,3 +15,4 @@ obj-$(CONFIG_VIRTIO_DMA_SHARED_BUFFER) += virtio_dma_buf.o obj-$(CONFIG_VIRTIO_RTC) += virtio_rtc.o virtio_rtc-y := virtio_rtc_driver.o virtio_rtc-$(CONFIG_VIRTIO_RTC_PTP) += virtio_rtc_ptp.o +virtio_rtc-$(CONFIG_VIRTIO_RTC_ARM) += virtio_rtc_arm.o diff --git a/drivers/virtio/virtio_rtc_arm.c b/drivers/virtio/virtio_rtc_arm.c new file mode 100644 index 000000000000..5185b130b3f1 --- /dev/null +++ b/drivers/virtio/virtio_rtc_arm.c @@ -0,0 +1,22 @@ +// SPDX-License-Identifier: GPL-2.0-or-later +/* + * Provides cross-timestamp params for Arm. + * + * Copyright (C) 2022-2023 OpenSynergy GmbH + */ + +#include <linux/clocksource_ids.h> + +#include <uapi/linux/virtio_rtc.h> + +#include "virtio_rtc_internal.h" + +/* see header for doc */ + +int viortc_hw_xtstamp_params(u16 *hw_counter, enum clocksource_ids *cs_id) +{ + *hw_counter = VIRTIO_RTC_COUNTER_ARM_VIRT; + *cs_id = CSID_ARM_ARCH_COUNTER; + + return 0; +}
Add PTP_SYS_OFFSET_PRECISE2 support on platforms using the Arm Generic Timer. Always report the CP15 virtual counter as the HW counter in use by arm_arch_timer, since the Linux kernel's usage of the Arm Generic Timer should always be compatible with this. Signed-off-by: Peter Hilber <peter.hilber@opensynergy.com> --- Notes: v2: - Depend on prerequisite patch series "treewide: Use clocksource id for get_device_system_crosststamp()". - Return clocksource id instead of calling dropped arm_arch_timer helpers. - Always report the CP15 virtual counter to be in use by arm_arch_timer, since distinction of Arm physical and virtual counter appears unneeded after discussion with Marc Zyngier. drivers/virtio/Kconfig | 13 +++++++++++++ drivers/virtio/Makefile | 1 + drivers/virtio/virtio_rtc_arm.c | 22 ++++++++++++++++++++++ 3 files changed, 36 insertions(+) create mode 100644 drivers/virtio/virtio_rtc_arm.c