2024-02-01 01:12:35

by Peter Hilber

[permalink] [raw]
Subject: [PATCH v3 0/8] treewide: Use clocksource ID for get_device_system_crosststamp()

Overview
--------

This patch series changes struct system_counterval_t to identify the
clocksource through enum clocksource_ids, rather than through struct
clocksource *. The net effect of the patch series is that
get_device_system_crosststamp() callers can supply clocksource ids instead
of clocksource pointers. The pointers can be problematic to get hold of.

The series is also available at

https://github.com/OpenSynergy/linux clocksource-id-for-xtstamp-v3

Motivation
----------

The immediate motivation for this patch series is to enable the virtio_rtc
RFC driver (v3 cf. [1]) to refer to the Arm Generic Timer clocksource
without requiring new helper functions in the arm_arch_timer driver. Other
future get_device_system_crosststamp() users may profit from this change as
well.

Clocksource structs are normally private to clocksource drivers. Therefore,
get_device_system_crosststamp() callers require that clocksource drivers
expose the clocksource of interest in some way.

Drivers such as virtio_rtc could obtain all information for calling
get_device_system_crosststamp() from their bound device, except for
clocksource identification. Often, such drivers' only direct relation with
the clocksource driver is clocksource identification. So using the
clocksource enum, rather than obtaining pointers in a clocksource driver
specific way, would reduce the coupling between the
get_device_system_crosststamp() callers and clocksource drivers.

Affected Code
-------------

This series modifies code which is relevant to
get_device_system_crosststamp(), in timekeeping, ptp/kvm, x86/kvm, and
x86/tsc.

There are two sorts of get_device_system_crosststamp() callers in the
current kernel:

1) On Intel platforms, some PTP hardware clocks, and the HDA controller,
obtain the clocksource pointer for get_device_system_crosststamp() using
convert_art_to_tsc() or convert_art_ns_to_tsc() from arch/x86.

2) The ptp_kvm driver uses kvm_arch_ptp_get_crosststamp(), which is
implemented for platforms with kvm_clock (x86) or arm_arch_timer.
Amongst other things, kvm_arch_ptp_get_crosststamp() returns a clocksource
pointer. The Arm implementation is in the arm_arch_timer driver.

Changes
-------

The series does the following:

- add clocksource ID to the get_device_system_crosststamp() param type

- add required clocksource ids and set them in
get_device_system_crosststamp() users

- evaluate clocksource ID in get_device_system_crosststamp(), rather than
clocksource pointer

- remove now obsolete clocksource pointer field and related code

This series should not alter any behavior. This series is a prerequisite
for the virtio_rtc driver [1].

Verification
------------

Out of the existing get_device_system_crosststamp() users, only ptp_kvm has
been tested (on x86-64 and arm64).

For each patch, with next-20240131 and mainline 1bbb19b6eb1b, on x86-64 and
arm64:

- built allmodconfig, allyesconfig, tinyconfig with GCC and LLVM (with a
few unrelated features turned off)

- runtime-tested ptp_kvm, checking ioctl PTP_SYS_OFFSET_PRECISE return
codes and clock synchronization success (reverted unrelated
"tty: serial: amba-pl011: Remove QDF2xxx workarounds" from linux-next
on arm64, to get QEMU with console working)

Changelog
---------

v3:

- Drop RFC.

- Omit redundant clocksource_ids.h includes (Andy Shevchenko).

- Fix tsc.c kernel-doc warnings, omitting some redundant documentation
(Simon Horman).

- Document relevant verification.

- Improve commit message wording.

v2:

- Align existing changes with sketch [2] by Thomas Gleixner (omitting
additional clocksource base changes from [2]).

- Add follow-up improvements in ptp_kvm and kvmclock.

- Split patches differently (Thomas Gleixner).

- Refer to clocksource IDs as such in comments (Thomas Gleixner).

- Update comments which were still referring to clocksource pointers.

[1] https://lore.kernel.org/lkml/[email protected]/
[2] https://lore.kernel.org/lkml/87lec15i4b.ffs@tglx/


Peter Hilber (8):
x86/tsc: Fix major kernel-doc warnings for tsc.c
timekeeping: Add clocksource ID to struct system_counterval_t
x86/tsc: Add clocksource ID, set system_counterval_t.cs_id
x86/kvm, ptp/kvm: Add clocksource ID, set system_counterval_t.cs_id
ptp/kvm, arm_arch_timer: Set system_counterval_t.cs_id to constant
timekeeping: Evaluate system_counterval_t.cs_id instead of .cs
treewide: Remove system_counterval_t.cs, which is never read
kvmclock: Unexport kvmclock clocksource

arch/x86/include/asm/kvmclock.h | 2 --
arch/x86/kernel/kvmclock.c | 4 ++--
arch/x86/kernel/tsc.c | 28 +++++++++++++++-------------
drivers/clocksource/arm_arch_timer.c | 6 +++---
drivers/ptp/ptp_kvm_common.c | 10 +++++-----
drivers/ptp/ptp_kvm_x86.c | 4 ++--
include/linux/clocksource_ids.h | 3 +++
include/linux/ptp_kvm.h | 4 ++--
include/linux/timekeeping.h | 10 ++++++----
kernel/time/timekeeping.c | 9 +++++----
10 files changed, 43 insertions(+), 37 deletions(-)


base-commit: 06f658aadff0e483ee4f807b0b46c9e5cba62bfa
--
2.40.1



2024-02-01 01:13:45

by Peter Hilber

[permalink] [raw]
Subject: [PATCH v3 6/8] timekeeping: Evaluate system_counterval_t.cs_id instead of .cs

Clocksource pointers can be problematic to obtain for drivers which are not
clocksource drivers themselves. In particular, the RFC virtio_rtc driver
[1] would require a new helper function to obtain a pointer to the Arm
Generic Timer clocksource. The ptp_kvm driver also required a similar
workaround.

Address this by evaluating the clocksource ID, rather than the clocksource
pointer, of struct system_counterval_t. By this, setting the clocksource
pointer becomes unneeded, and it will be dropped from struct
system_counterval_t in the future. By this, get_device_system_crosststamp()
callers (such as virtio_rtc and ptp_kvm) will no longer need to supply
clocksource pointers.

This change should not alter any behavior, as the struct
system_counterval_t clocksource ID is already being set wherever the
clocksource pointer is set. get_device_system_crosststamp() will now fail
if the clocksource has ID CSID_GENERIC, but all currently relevant
clocksources have a custom clocksource ID.

[1] https://lore.kernel.org/lkml/[email protected]/

Signed-off-by: Peter Hilber <[email protected]>
---

Notes:
v2:

- Refer to clocksource IDs as such in comments (Thomas Gleixner).

- Update comments which were still referring to clocksource pointers.

include/linux/timekeeping.h | 10 +++++-----
kernel/time/timekeeping.c | 9 +++++----
2 files changed, 10 insertions(+), 9 deletions(-)

diff --git a/include/linux/timekeeping.h b/include/linux/timekeeping.h
index ca234fa4cc04..3538c5bdf9ee 100644
--- a/include/linux/timekeeping.h
+++ b/include/linux/timekeeping.h
@@ -268,13 +268,13 @@ struct system_device_crosststamp {
};

/**
- * struct system_counterval_t - system counter value with the pointer to the
+ * struct system_counterval_t - system counter value with the ID of the
* corresponding clocksource
* @cycles: System counter value
- * @cs: Clocksource corresponding to system counter value. Used by
- * timekeeping code to verify comparibility of two cycle values
- * @cs_id: Clocksource ID corresponding to system counter value. To be
- * used instead of cs in the future.
+ * @cs: Clocksource corresponding to system counter value. Timekeeping
+ * code now evaluates cs_id instead.
+ * @cs_id: Clocksource ID corresponding to system counter value. Used by
+ * timekeeping code to verify comparability of two cycle values.
* The default ID, CSID_GENERIC, does not identify a specific
* clocksource.
*/
diff --git a/kernel/time/timekeeping.c b/kernel/time/timekeeping.c
index 266d02809dbb..0ff065c5d25b 100644
--- a/kernel/time/timekeeping.c
+++ b/kernel/time/timekeeping.c
@@ -1232,11 +1232,12 @@ int get_device_system_crosststamp(int (*get_time_fn)
return ret;

/*
- * Verify that the clocksource associated with the captured
- * system counter value is the same as the currently installed
- * timekeeper clocksource
+ * Verify that the clocksource ID associated with the captured
+ * system counter value is the same as for the currently
+ * installed timekeeper clocksource
*/
- if (tk->tkr_mono.clock != system_counterval.cs)
+ if (system_counterval.cs_id == CSID_GENERIC ||
+ tk->tkr_mono.clock->id != system_counterval.cs_id)
return -ENODEV;
cycles = system_counterval.cycles;

--
2.40.1


2024-02-01 01:30:50

by Peter Hilber

[permalink] [raw]
Subject: [PATCH v3 8/8] kvmclock: Unexport kvmclock clocksource

The KVM PTP driver now refers to the clocksource ID CSID_X86_KVM_CLK, not
to the clocksource itself any more. There are no remaining users of the
clocksource export.

Therefore, make the clocksource static again.

Signed-off-by: Peter Hilber <[email protected]>
---

Notes:
v2:

Added in v2.

arch/x86/include/asm/kvmclock.h | 2 --
arch/x86/kernel/kvmclock.c | 3 +--
2 files changed, 1 insertion(+), 4 deletions(-)

diff --git a/arch/x86/include/asm/kvmclock.h b/arch/x86/include/asm/kvmclock.h
index 511b35069187..f163176d6f7f 100644
--- a/arch/x86/include/asm/kvmclock.h
+++ b/arch/x86/include/asm/kvmclock.h
@@ -4,8 +4,6 @@

#include <linux/percpu.h>

-extern struct clocksource kvm_clock;
-
DECLARE_PER_CPU(struct pvclock_vsyscall_time_info *, hv_clock_per_cpu);

static __always_inline struct pvclock_vcpu_time_info *this_cpu_pvti(void)
diff --git a/arch/x86/kernel/kvmclock.c b/arch/x86/kernel/kvmclock.c
index 2f1bbf730f45..5b2c15214a6b 100644
--- a/arch/x86/kernel/kvmclock.c
+++ b/arch/x86/kernel/kvmclock.c
@@ -154,7 +154,7 @@ static int kvm_cs_enable(struct clocksource *cs)
return 0;
}

-struct clocksource kvm_clock = {
+static struct clocksource kvm_clock = {
.name = "kvm-clock",
.read = kvm_clock_get_cycles,
.rating = 400,
@@ -163,7 +163,6 @@ struct clocksource kvm_clock = {
.id = CSID_X86_KVM_CLK,
.enable = kvm_cs_enable,
};
-EXPORT_SYMBOL_GPL(kvm_clock);

static void kvm_register_clock(char *txt)
{
--
2.40.1


2024-02-01 01:44:12

by Peter Hilber

[permalink] [raw]
Subject: [PATCH v3 5/8] ptp/kvm, arm_arch_timer: Set system_counterval_t.cs_id to constant

Identify the clocksources used by ptp_kvm by setting clocksource ID enum
constants. This avoids dereferencing struct clocksource. Once the
system_counterval_t.cs member will be removed, this will also avoid the
need to obtain clocksource pointers from kvm_arch_ptp_get_crosststamp().

The clocksource IDs are associated to timestamps requested from the KVM
hypervisor, so the proper clocksource ID is known at the ptp_kvm request
site.

While at it, also rectify the ptp_kvm_get_time_fn() ret type.

Signed-off-by: Peter Hilber <[email protected]>
---

Notes:
v2:

Added in v2.

drivers/clocksource/arm_arch_timer.c | 5 ++++-
drivers/ptp/ptp_kvm_arm.c | 2 +-
drivers/ptp/ptp_kvm_common.c | 10 +++++-----
drivers/ptp/ptp_kvm_x86.c | 4 +++-
include/linux/ptp_kvm.h | 4 +++-
5 files changed, 16 insertions(+), 9 deletions(-)

diff --git a/drivers/clocksource/arm_arch_timer.c b/drivers/clocksource/arm_arch_timer.c
index e054de92de91..45a02872669e 100644
--- a/drivers/clocksource/arm_arch_timer.c
+++ b/drivers/clocksource/arm_arch_timer.c
@@ -1807,7 +1807,8 @@ 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 clocksource **cs,
+ enum clocksource_ids *cs_id)
{
struct arm_smccc_res hvc_res;
u32 ptp_counter;
@@ -1833,6 +1834,8 @@ int kvm_arch_ptp_get_crosststamp(u64 *cycle, struct timespec64 *ts,
*cycle = (u64)hvc_res.a2 << 32 | hvc_res.a3;
if (cs)
*cs = &clocksource_counter;
+ if (cs_id)
+ *cs_id = CSID_ARM_ARCH_COUNTER;

return 0;
}
diff --git a/drivers/ptp/ptp_kvm_arm.c b/drivers/ptp/ptp_kvm_arm.c
index e68e6943167b..017bb5f03b14 100644
--- a/drivers/ptp/ptp_kvm_arm.c
+++ b/drivers/ptp/ptp_kvm_arm.c
@@ -28,5 +28,5 @@ void kvm_arch_ptp_exit(void)

int kvm_arch_ptp_get_clock(struct timespec64 *ts)
{
- return kvm_arch_ptp_get_crosststamp(NULL, ts, NULL);
+ return kvm_arch_ptp_get_crosststamp(NULL, ts, NULL, NULL);
}
diff --git a/drivers/ptp/ptp_kvm_common.c b/drivers/ptp/ptp_kvm_common.c
index b0b36f135347..f6683ba0ab3c 100644
--- a/drivers/ptp/ptp_kvm_common.c
+++ b/drivers/ptp/ptp_kvm_common.c
@@ -4,7 +4,6 @@
*
* Copyright (C) 2017 Red Hat Inc.
*/
-#include <linux/clocksource.h>
#include <linux/device.h>
#include <linux/err.h>
#include <linux/init.h>
@@ -29,15 +28,16 @@ static int ptp_kvm_get_time_fn(ktime_t *device_time,
struct system_counterval_t *system_counter,
void *ctx)
{
- long ret;
- u64 cycle;
+ enum clocksource_ids cs_id;
struct timespec64 tspec;
struct clocksource *cs;
+ u64 cycle;
+ int ret;

spin_lock(&kvm_ptp_lock);

preempt_disable_notrace();
- ret = kvm_arch_ptp_get_crosststamp(&cycle, &tspec, &cs);
+ ret = kvm_arch_ptp_get_crosststamp(&cycle, &tspec, &cs, &cs_id);
if (ret) {
spin_unlock(&kvm_ptp_lock);
preempt_enable_notrace();
@@ -48,7 +48,7 @@ static int ptp_kvm_get_time_fn(ktime_t *device_time,

system_counter->cycles = cycle;
system_counter->cs = cs;
- system_counter->cs_id = cs->id;
+ system_counter->cs_id = cs_id;

*device_time = timespec64_to_ktime(tspec);

diff --git a/drivers/ptp/ptp_kvm_x86.c b/drivers/ptp/ptp_kvm_x86.c
index 902844cc1a17..2782442922cb 100644
--- a/drivers/ptp/ptp_kvm_x86.c
+++ b/drivers/ptp/ptp_kvm_x86.c
@@ -93,7 +93,8 @@ int kvm_arch_ptp_get_clock(struct timespec64 *ts)
}

int kvm_arch_ptp_get_crosststamp(u64 *cycle, struct timespec64 *tspec,
- struct clocksource **cs)
+ struct clocksource **cs,
+ enum clocksource_ids *cs_id)
{
struct pvclock_vcpu_time_info *src;
unsigned int version;
@@ -124,6 +125,7 @@ int kvm_arch_ptp_get_crosststamp(u64 *cycle, struct timespec64 *tspec,
} while (pvclock_read_retry(src, version));

*cs = &kvm_clock;
+ *cs_id = CSID_X86_KVM_CLK;

return 0;
}
diff --git a/include/linux/ptp_kvm.h b/include/linux/ptp_kvm.h
index 746fd67c3480..95b3d4d0d7dd 100644
--- a/include/linux/ptp_kvm.h
+++ b/include/linux/ptp_kvm.h
@@ -8,6 +8,7 @@
#ifndef _PTP_KVM_H_
#define _PTP_KVM_H_

+#include <linux/clocksource_ids.h>
#include <linux/types.h>

struct timespec64;
@@ -17,6 +18,7 @@ int kvm_arch_ptp_init(void);
void kvm_arch_ptp_exit(void);
int kvm_arch_ptp_get_clock(struct timespec64 *ts);
int kvm_arch_ptp_get_crosststamp(u64 *cycle,
- struct timespec64 *tspec, struct clocksource **cs);
+ struct timespec64 *tspec, struct clocksource **cs,
+ enum clocksource_ids *cs_id);

#endif /* _PTP_KVM_H_ */
--
2.40.1


2024-02-01 13:52:48

by Marc Zyngier

[permalink] [raw]
Subject: Re: [PATCH v3 5/8] ptp/kvm, arm_arch_timer: Set system_counterval_t.cs_id to constant

On Thu, 01 Feb 2024 01:04:50 +0000,
Peter Hilber <[email protected]> wrote:
>
> Identify the clocksources used by ptp_kvm by setting clocksource ID enum
> constants. This avoids dereferencing struct clocksource. Once the
> system_counterval_t.cs member will be removed, this will also avoid the
> need to obtain clocksource pointers from kvm_arch_ptp_get_crosststamp().
>
> The clocksource IDs are associated to timestamps requested from the KVM
> hypervisor, so the proper clocksource ID is known at the ptp_kvm request
> site.
>
> While at it, also rectify the ptp_kvm_get_time_fn() ret type.

Not sure what is wrong with that return type, but this patch doesn't
seem to affect it.

M.

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

2024-02-01 14:09:01

by Peter Hilber

[permalink] [raw]
Subject: Re: [PATCH v3 5/8] ptp/kvm, arm_arch_timer: Set system_counterval_t.cs_id to constant

On 01.02.24 14:52, Marc Zyngier wrote:
> On Thu, 01 Feb 2024 01:04:50 +0000,
> Peter Hilber <[email protected]> wrote:
>>
>> Identify the clocksources used by ptp_kvm by setting clocksource ID enum
>> constants. This avoids dereferencing struct clocksource. Once the
>> system_counterval_t.cs member will be removed, this will also avoid the
>> need to obtain clocksource pointers from kvm_arch_ptp_get_crosststamp().
>>
>> The clocksource IDs are associated to timestamps requested from the KVM
>> hypervisor, so the proper clocksource ID is known at the ptp_kvm request
>> site.
>>
>> While at it, also rectify the ptp_kvm_get_time_fn() ret type.
>
> Not sure what is wrong with that return type, but this patch doesn't
> seem to affect it.
>

I meant to refer to the type of the variable ret declared in
ptp_kvm_get_time_fn(). I will reword the commit message to make this clear.

Thanks for the comment,

Peter

2024-02-11 08:41:39

by tip-bot2 for Jacob Pan

[permalink] [raw]
Subject: [tip: timers/ptp] kvmclock: Unexport kvmclock clocksource

The following commit has been merged into the timers/ptp branch of tip:

Commit-ID: 27f6a9c87a97f5ea7459be08d5be231af6b32c20
Gitweb: https://git.kernel.org/tip/27f6a9c87a97f5ea7459be08d5be231af6b32c20
Author: Peter Hilber <[email protected]>
AuthorDate: Thu, 01 Feb 2024 02:04:53 +01:00
Committer: Thomas Gleixner <[email protected]>
CommitterDate: Wed, 07 Feb 2024 17:05:21 +01:00

kvmclock: Unexport kvmclock clocksource

The KVM PTP driver now refers to the clocksource ID CSID_X86_KVM_CLK, not
to the clocksource itself any more. There are no remaining users of the
clocksource export.

Therefore, make the clocksource static again.

Signed-off-by: Peter Hilber <[email protected]>
Signed-off-by: Thomas Gleixner <[email protected]>
Link: https://lore.kernel.org/r/[email protected]

---
arch/x86/include/asm/kvmclock.h | 2 --
arch/x86/kernel/kvmclock.c | 3 +--
2 files changed, 1 insertion(+), 4 deletions(-)

diff --git a/arch/x86/include/asm/kvmclock.h b/arch/x86/include/asm/kvmclock.h
index 511b350..f163176 100644
--- a/arch/x86/include/asm/kvmclock.h
+++ b/arch/x86/include/asm/kvmclock.h
@@ -4,8 +4,6 @@

#include <linux/percpu.h>

-extern struct clocksource kvm_clock;
-
DECLARE_PER_CPU(struct pvclock_vsyscall_time_info *, hv_clock_per_cpu);

static __always_inline struct pvclock_vcpu_time_info *this_cpu_pvti(void)
diff --git a/arch/x86/kernel/kvmclock.c b/arch/x86/kernel/kvmclock.c
index 2f1bbf7..5b2c152 100644
--- a/arch/x86/kernel/kvmclock.c
+++ b/arch/x86/kernel/kvmclock.c
@@ -154,7 +154,7 @@ static int kvm_cs_enable(struct clocksource *cs)
return 0;
}

-struct clocksource kvm_clock = {
+static struct clocksource kvm_clock = {
.name = "kvm-clock",
.read = kvm_clock_get_cycles,
.rating = 400,
@@ -163,7 +163,6 @@ struct clocksource kvm_clock = {
.id = CSID_X86_KVM_CLK,
.enable = kvm_cs_enable,
};
-EXPORT_SYMBOL_GPL(kvm_clock);

static void kvm_register_clock(char *txt)
{

2024-02-11 08:41:58

by tip-bot2 for Jacob Pan

[permalink] [raw]
Subject: [tip: timers/ptp] timekeeping: Evaluate system_counterval_t.cs_id instead of .cs

The following commit has been merged into the timers/ptp branch of tip:

Commit-ID: 4b7f521229ef4eee06848427d865954e6e0e3675
Gitweb: https://git.kernel.org/tip/4b7f521229ef4eee06848427d865954e6e0e3675
Author: Peter Hilber <[email protected]>
AuthorDate: Thu, 01 Feb 2024 02:04:51 +01:00
Committer: Thomas Gleixner <[email protected]>
CommitterDate: Wed, 07 Feb 2024 17:05:21 +01:00

timekeeping: Evaluate system_counterval_t.cs_id instead of .cs

Clocksource pointers can be problematic to obtain for drivers which are not
clocksource drivers themselves. In particular, the RFC virtio_rtc driver
[1] would require a new helper function to obtain a pointer to the ARM
Generic Timer clocksource. The ptp_kvm driver also required a similar
workaround.

Address this by evaluating the clocksource ID, rather than the clocksource
pointer, of struct system_counterval_t. By this, setting the clocksource
pointer becomes unneeded, and get_device_system_crosststamp() callers will
no longer need to supply clocksource pointers.

All relevant clocksource drivers provide the ID, so this change is not
changing the behaviour.

[1] https://lore.kernel.org/lkml/[email protected]/

Signed-off-by: Peter Hilber <[email protected]>
Signed-off-by: Thomas Gleixner <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
---
include/linux/timekeeping.h | 10 +++++-----
kernel/time/timekeeping.c | 9 +++++----
2 files changed, 10 insertions(+), 9 deletions(-)

diff --git a/include/linux/timekeeping.h b/include/linux/timekeeping.h
index ca234fa..3538c5b 100644
--- a/include/linux/timekeeping.h
+++ b/include/linux/timekeeping.h
@@ -268,13 +268,13 @@ struct system_device_crosststamp {
};

/**
- * struct system_counterval_t - system counter value with the pointer to the
+ * struct system_counterval_t - system counter value with the ID of the
* corresponding clocksource
* @cycles: System counter value
- * @cs: Clocksource corresponding to system counter value. Used by
- * timekeeping code to verify comparibility of two cycle values
- * @cs_id: Clocksource ID corresponding to system counter value. To be
- * used instead of cs in the future.
+ * @cs: Clocksource corresponding to system counter value. Timekeeping
+ * code now evaluates cs_id instead.
+ * @cs_id: Clocksource ID corresponding to system counter value. Used by
+ * timekeeping code to verify comparability of two cycle values.
* The default ID, CSID_GENERIC, does not identify a specific
* clocksource.
*/
diff --git a/kernel/time/timekeeping.c b/kernel/time/timekeeping.c
index 266d028..0ff065c 100644
--- a/kernel/time/timekeeping.c
+++ b/kernel/time/timekeeping.c
@@ -1232,11 +1232,12 @@ int get_device_system_crosststamp(int (*get_time_fn)
return ret;

/*
- * Verify that the clocksource associated with the captured
- * system counter value is the same as the currently installed
- * timekeeper clocksource
+ * Verify that the clocksource ID associated with the captured
+ * system counter value is the same as for the currently
+ * installed timekeeper clocksource
*/
- if (tk->tkr_mono.clock != system_counterval.cs)
+ if (system_counterval.cs_id == CSID_GENERIC ||
+ tk->tkr_mono.clock->id != system_counterval.cs_id)
return -ENODEV;
cycles = system_counterval.cycles;


2024-02-11 08:42:07

by tip-bot2 for Jacob Pan

[permalink] [raw]
Subject: [tip: timers/ptp] ptp/kvm, arm_arch_timer: Set system_counterval_t.cs_id to constant

The following commit has been merged into the timers/ptp branch of tip:

Commit-ID: 9be3b2f057d7a6752e8cf25c1d456198b4d3bd6a
Gitweb: https://git.kernel.org/tip/9be3b2f057d7a6752e8cf25c1d456198b4d3bd6a
Author: Peter Hilber <[email protected]>
AuthorDate: Thu, 01 Feb 2024 02:04:50 +01:00
Committer: Thomas Gleixner <[email protected]>
CommitterDate: Wed, 07 Feb 2024 17:05:21 +01:00

ptp/kvm, arm_arch_timer: Set system_counterval_t.cs_id to constant

Identify the clocksources used by ptp_kvm by setting the clocksource ID
enum constants. This avoids dereferencing struct clocksource. Once the
system_counterval_t.cs member will be removed, this will also avoid the
need to obtain clocksource pointers from kvm_arch_ptp_get_crosststamp().

The clocksource IDs are associated to timestamps requested from the KVM
hypervisor, so the proper clocksource ID is known at the ptp_kvm request
site.

While at it, also make the ptp_kvm_get_time_fn() 'ret' variable type int as
that's what the function return value is.

Signed-off-by: Peter Hilber <[email protected]>
Signed-off-by: Thomas Gleixner <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
---
drivers/clocksource/arm_arch_timer.c | 5 ++++-
drivers/ptp/ptp_kvm_arm.c | 2 +-
drivers/ptp/ptp_kvm_common.c | 10 +++++-----
drivers/ptp/ptp_kvm_x86.c | 4 +++-
include/linux/ptp_kvm.h | 4 +++-
5 files changed, 16 insertions(+), 9 deletions(-)

diff --git a/drivers/clocksource/arm_arch_timer.c b/drivers/clocksource/arm_arch_timer.c
index e054de9..45a0287 100644
--- a/drivers/clocksource/arm_arch_timer.c
+++ b/drivers/clocksource/arm_arch_timer.c
@@ -1807,7 +1807,8 @@ 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 clocksource **cs,
+ enum clocksource_ids *cs_id)
{
struct arm_smccc_res hvc_res;
u32 ptp_counter;
@@ -1833,6 +1834,8 @@ int kvm_arch_ptp_get_crosststamp(u64 *cycle, struct timespec64 *ts,
*cycle = (u64)hvc_res.a2 << 32 | hvc_res.a3;
if (cs)
*cs = &clocksource_counter;
+ if (cs_id)
+ *cs_id = CSID_ARM_ARCH_COUNTER;

return 0;
}
diff --git a/drivers/ptp/ptp_kvm_arm.c b/drivers/ptp/ptp_kvm_arm.c
index e68e694..017bb5f 100644
--- a/drivers/ptp/ptp_kvm_arm.c
+++ b/drivers/ptp/ptp_kvm_arm.c
@@ -28,5 +28,5 @@ void kvm_arch_ptp_exit(void)

int kvm_arch_ptp_get_clock(struct timespec64 *ts)
{
- return kvm_arch_ptp_get_crosststamp(NULL, ts, NULL);
+ return kvm_arch_ptp_get_crosststamp(NULL, ts, NULL, NULL);
}
diff --git a/drivers/ptp/ptp_kvm_common.c b/drivers/ptp/ptp_kvm_common.c
index b0b36f1..f6683ba 100644
--- a/drivers/ptp/ptp_kvm_common.c
+++ b/drivers/ptp/ptp_kvm_common.c
@@ -4,7 +4,6 @@
*
* Copyright (C) 2017 Red Hat Inc.
*/
-#include <linux/clocksource.h>
#include <linux/device.h>
#include <linux/err.h>
#include <linux/init.h>
@@ -29,15 +28,16 @@ static int ptp_kvm_get_time_fn(ktime_t *device_time,
struct system_counterval_t *system_counter,
void *ctx)
{
- long ret;
- u64 cycle;
+ enum clocksource_ids cs_id;
struct timespec64 tspec;
struct clocksource *cs;
+ u64 cycle;
+ int ret;

spin_lock(&kvm_ptp_lock);

preempt_disable_notrace();
- ret = kvm_arch_ptp_get_crosststamp(&cycle, &tspec, &cs);
+ ret = kvm_arch_ptp_get_crosststamp(&cycle, &tspec, &cs, &cs_id);
if (ret) {
spin_unlock(&kvm_ptp_lock);
preempt_enable_notrace();
@@ -48,7 +48,7 @@ static int ptp_kvm_get_time_fn(ktime_t *device_time,

system_counter->cycles = cycle;
system_counter->cs = cs;
- system_counter->cs_id = cs->id;
+ system_counter->cs_id = cs_id;

*device_time = timespec64_to_ktime(tspec);

diff --git a/drivers/ptp/ptp_kvm_x86.c b/drivers/ptp/ptp_kvm_x86.c
index 902844c..2782442 100644
--- a/drivers/ptp/ptp_kvm_x86.c
+++ b/drivers/ptp/ptp_kvm_x86.c
@@ -93,7 +93,8 @@ int kvm_arch_ptp_get_clock(struct timespec64 *ts)
}

int kvm_arch_ptp_get_crosststamp(u64 *cycle, struct timespec64 *tspec,
- struct clocksource **cs)
+ struct clocksource **cs,
+ enum clocksource_ids *cs_id)
{
struct pvclock_vcpu_time_info *src;
unsigned int version;
@@ -124,6 +125,7 @@ int kvm_arch_ptp_get_crosststamp(u64 *cycle, struct timespec64 *tspec,
} while (pvclock_read_retry(src, version));

*cs = &kvm_clock;
+ *cs_id = CSID_X86_KVM_CLK;

return 0;
}
diff --git a/include/linux/ptp_kvm.h b/include/linux/ptp_kvm.h
index 746fd67..95b3d4d 100644
--- a/include/linux/ptp_kvm.h
+++ b/include/linux/ptp_kvm.h
@@ -8,6 +8,7 @@
#ifndef _PTP_KVM_H_
#define _PTP_KVM_H_

+#include <linux/clocksource_ids.h>
#include <linux/types.h>

struct timespec64;
@@ -17,6 +18,7 @@ int kvm_arch_ptp_init(void);
void kvm_arch_ptp_exit(void);
int kvm_arch_ptp_get_clock(struct timespec64 *ts);
int kvm_arch_ptp_get_crosststamp(u64 *cycle,
- struct timespec64 *tspec, struct clocksource **cs);
+ struct timespec64 *tspec, struct clocksource **cs,
+ enum clocksource_ids *cs_id);

#endif /* _PTP_KVM_H_ */