* [PATCH v3 2/8] timekeeping: Add clocksource ID to struct system_counterval_t
2024-02-01 1:04 [PATCH v3 0/8] treewide: Use clocksource ID for get_device_system_crosststamp() Peter Hilber
@ 2024-02-01 1:04 ` Peter Hilber
2024-02-01 1:04 ` [PATCH v3 3/8] x86/tsc: Add clocksource ID, set system_counterval_t.cs_id Peter Hilber
` (5 subsequent siblings)
6 siblings, 0 replies; 10+ messages in thread
From: Peter Hilber @ 2024-02-01 1:04 UTC (permalink / raw)
To: linux-kernel
Cc: Peter Hilber, D, Lakshmi Sowjanya, Thomas Gleixner, jstultz,
giometti, corbet, Dong, Eddie, Hall, Christopher S, Simon Horman,
Andy Shevchenko, Marc Zyngier, linux-arm-kernel,
Sean Christopherson, Paolo Bonzini, Ingo Molnar, Borislav Petkov,
Dave Hansen, x86, H. Peter Anvin, Wanpeng Li, Vitaly Kuznetsov,
Mark Rutland, Daniel Lezcano, Richard Cochran, kvm, netdev,
Stephen Boyd
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.
Add a clocksource ID member to struct system_counterval_t, which in the
future shall identify the clocksource, and which shall replace the struct
clocksource * member. By this, get_device_system_crosststamp() callers
(such as virtio_rtc and ptp_kvm) will be able to supply easily accessible
clocksource ids instead of clocksource pointers.
[1] https://lore.kernel.org/lkml/20231218073849.35294-1-peter.hilber@opensynergy.com/
Signed-off-by: Peter Hilber <peter.hilber@opensynergy.com>
---
Notes:
v2:
- Refer to clocksource IDs as such in comments (Thomas Gleixner).
include/linux/timekeeping.h | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/include/linux/timekeeping.h b/include/linux/timekeeping.h
index 7c43e98cf211..ca234fa4cc04 100644
--- a/include/linux/timekeeping.h
+++ b/include/linux/timekeeping.h
@@ -273,10 +273,15 @@ struct system_device_crosststamp {
* @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.
+ * The default ID, CSID_GENERIC, does not identify a specific
+ * clocksource.
*/
struct system_counterval_t {
u64 cycles;
struct clocksource *cs;
+ enum clocksource_ids cs_id;
};
/*
--
2.40.1
^ permalink raw reply related [flat|nested] 10+ messages in thread* [PATCH v3 3/8] x86/tsc: Add clocksource ID, set system_counterval_t.cs_id
2024-02-01 1:04 [PATCH v3 0/8] treewide: Use clocksource ID for get_device_system_crosststamp() Peter Hilber
2024-02-01 1:04 ` [PATCH v3 2/8] timekeeping: Add clocksource ID to struct system_counterval_t Peter Hilber
@ 2024-02-01 1:04 ` Peter Hilber
2024-02-01 1:04 ` [PATCH v3 4/8] x86/kvm, ptp/kvm: " Peter Hilber
` (4 subsequent siblings)
6 siblings, 0 replies; 10+ messages in thread
From: Peter Hilber @ 2024-02-01 1:04 UTC (permalink / raw)
To: linux-kernel
Cc: Peter Hilber, D, Lakshmi Sowjanya, Thomas Gleixner, jstultz,
giometti, corbet, Dong, Eddie, Hall, Christopher S, Simon Horman,
Andy Shevchenko, Sean Christopherson, Paolo Bonzini, Ingo Molnar,
Borislav Petkov, Dave Hansen, x86, H. Peter Anvin, Wanpeng Li,
Vitaly Kuznetsov, Mark Rutland, Marc Zyngier, Daniel Lezcano,
Richard Cochran, kvm, netdev
Add a clocksource ID for TSC and a distinct one for the early TSC.
Use distinct IDs for TSC and early TSC, since those also have distinct
clocksource structs. This should help to keep existing semantics when
comparing clocksources.
Also, set the recently added struct system_counterval_t member cs_id to the
TSC ID in the cases where the clocksource member is being set to the TSC
clocksource. In the future, get_device_system_crosststamp() will compare
the clocksource ID in struct system_counterval_t, rather than the
clocksource.
For the x86 ART related code, system_counterval_t.cs == NULL corresponds to
system_counterval_t.cs_id == CSID_GENERIC (0).
Signed-off-by: Peter Hilber <peter.hilber@opensynergy.com>
---
Notes:
v3:
- Omit redundant clocksource_ids.h include (Andy Shevchenko).
- Omit struct system_counterval_t member documentation, to resolve
kernel-doc warning pointed out by Simon Horman.
v2:
- Name clock id according to Thomas Gleixner's mockup.
- Refer to clocksource IDs as such in comments (Thomas Gleixner).
- Update comments which were still referring to clocksource pointers.
arch/x86/kernel/tsc.c | 27 ++++++++++++++++++++-------
include/linux/clocksource_ids.h | 2 ++
2 files changed, 22 insertions(+), 7 deletions(-)
diff --git a/arch/x86/kernel/tsc.c b/arch/x86/kernel/tsc.c
index 42328f9653c1..9f164aad5e94 100644
--- a/arch/x86/kernel/tsc.c
+++ b/arch/x86/kernel/tsc.c
@@ -54,6 +54,7 @@ static u32 art_to_tsc_numerator;
static u32 art_to_tsc_denominator;
static u64 art_to_tsc_offset;
static struct clocksource *art_related_clocksource;
+static bool have_art;
struct cyc2ns {
struct cyc2ns_data data[2]; /* 0 + 2*16 = 32 */
@@ -1168,6 +1169,7 @@ static struct clocksource clocksource_tsc_early = {
.mask = CLOCKSOURCE_MASK(64),
.flags = CLOCK_SOURCE_IS_CONTINUOUS |
CLOCK_SOURCE_MUST_VERIFY,
+ .id = CSID_X86_TSC_EARLY,
.vdso_clock_mode = VDSO_CLOCKMODE_TSC,
.enable = tsc_cs_enable,
.resume = tsc_resume,
@@ -1190,6 +1192,7 @@ static struct clocksource clocksource_tsc = {
CLOCK_SOURCE_VALID_FOR_HRES |
CLOCK_SOURCE_MUST_VERIFY |
CLOCK_SOURCE_VERIFY_PERCPU,
+ .id = CSID_X86_TSC,
.vdso_clock_mode = VDSO_CLOCKMODE_TSC,
.enable = tsc_cs_enable,
.resume = tsc_resume,
@@ -1309,8 +1312,11 @@ struct system_counterval_t convert_art_to_tsc(u64 art)
do_div(tmp, art_to_tsc_denominator);
res += tmp + art_to_tsc_offset;
- return (struct system_counterval_t) {.cs = art_related_clocksource,
- .cycles = res};
+ return (struct system_counterval_t) {
+ .cs = art_related_clocksource,
+ .cs_id = have_art ? CSID_X86_TSC : CSID_GENERIC,
+ .cycles = res
+ };
}
EXPORT_SYMBOL(convert_art_to_tsc);
@@ -1327,7 +1333,7 @@ EXPORT_SYMBOL(convert_art_to_tsc);
* that this flag is set before conversion to TSC is attempted.
*
* Return:
- * 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
*/
@@ -1343,8 +1349,11 @@ struct system_counterval_t convert_art_ns_to_tsc(u64 art_ns)
do_div(tmp, USEC_PER_SEC);
res += tmp;
- return (struct system_counterval_t) { .cs = art_related_clocksource,
- .cycles = res};
+ return (struct system_counterval_t) {
+ .cs = art_related_clocksource,
+ .cs_id = have_art ? CSID_X86_TSC : CSID_GENERIC,
+ .cycles = res
+ };
}
EXPORT_SYMBOL(convert_art_ns_to_tsc);
@@ -1450,8 +1459,10 @@ static void tsc_refine_calibration_work(struct work_struct *work)
if (tsc_unstable)
goto unreg;
- if (boot_cpu_has(X86_FEATURE_ART))
+ if (boot_cpu_has(X86_FEATURE_ART)) {
art_related_clocksource = &clocksource_tsc;
+ have_art = true;
+ }
clocksource_register_khz(&clocksource_tsc, tsc_khz);
unreg:
clocksource_unregister(&clocksource_tsc_early);
@@ -1476,8 +1487,10 @@ static int __init init_tsc_clocksource(void)
* the refined calibration and directly register it as a clocksource.
*/
if (boot_cpu_has(X86_FEATURE_TSC_KNOWN_FREQ)) {
- if (boot_cpu_has(X86_FEATURE_ART))
+ if (boot_cpu_has(X86_FEATURE_ART)) {
art_related_clocksource = &clocksource_tsc;
+ have_art = true;
+ }
clocksource_register_khz(&clocksource_tsc, tsc_khz);
clocksource_unregister(&clocksource_tsc_early);
diff --git a/include/linux/clocksource_ids.h b/include/linux/clocksource_ids.h
index 16775d7d8f8d..f8467946e9ee 100644
--- a/include/linux/clocksource_ids.h
+++ b/include/linux/clocksource_ids.h
@@ -6,6 +6,8 @@
enum clocksource_ids {
CSID_GENERIC = 0,
CSID_ARM_ARCH_COUNTER,
+ CSID_X86_TSC_EARLY,
+ CSID_X86_TSC,
CSID_MAX,
};
--
2.40.1
^ permalink raw reply related [flat|nested] 10+ messages in thread* [PATCH v3 4/8] x86/kvm, ptp/kvm: Add clocksource ID, set system_counterval_t.cs_id
2024-02-01 1:04 [PATCH v3 0/8] treewide: Use clocksource ID for get_device_system_crosststamp() Peter Hilber
2024-02-01 1:04 ` [PATCH v3 2/8] timekeeping: Add clocksource ID to struct system_counterval_t Peter Hilber
2024-02-01 1:04 ` [PATCH v3 3/8] x86/tsc: Add clocksource ID, set system_counterval_t.cs_id Peter Hilber
@ 2024-02-01 1:04 ` Peter Hilber
2024-02-01 1:04 ` [PATCH v3 5/8] ptp/kvm, arm_arch_timer: Set system_counterval_t.cs_id to constant Peter Hilber
` (3 subsequent siblings)
6 siblings, 0 replies; 10+ messages in thread
From: Peter Hilber @ 2024-02-01 1:04 UTC (permalink / raw)
To: linux-kernel
Cc: Peter Hilber, D, Lakshmi Sowjanya, Thomas Gleixner, jstultz,
giometti, corbet, Dong, Eddie, Hall, Christopher S, Simon Horman,
Andy Shevchenko, Marc Zyngier, linux-arm-kernel,
Sean Christopherson, Paolo Bonzini, Ingo Molnar, Borislav Petkov,
Dave Hansen, x86, H. Peter Anvin, Wanpeng Li, Vitaly Kuznetsov,
Mark Rutland, Daniel Lezcano, Richard Cochran, kvm, netdev
Add a clocksource ID for the x86 kvmclock.
Also, for ptp_kvm, set the recently added struct system_counterval_t member
cs_id to the clocksource ID (x86 kvmclock or Arm Generic Timer). In the
future, get_device_system_crosststamp() will compare the clocksource ID in
struct system_counterval_t, rather than the clocksource.
For now, to avoid touching too many subsystems at once, extract the
clocksource ID from the clocksource. The clocksource dereference will be
removed in the following.
Signed-off-by: Peter Hilber <peter.hilber@opensynergy.com>
---
Notes:
v3:
- Omit redundant clocksource_ids.h include (Andy Shevchenko).
v2:
- Name clock id according to Thomas Gleixner's mockup.
arch/x86/kernel/kvmclock.c | 1 +
drivers/ptp/ptp_kvm_common.c | 2 ++
include/linux/clocksource_ids.h | 1 +
3 files changed, 4 insertions(+)
diff --git a/arch/x86/kernel/kvmclock.c b/arch/x86/kernel/kvmclock.c
index 5bb395551c44..2f1bbf730f45 100644
--- a/arch/x86/kernel/kvmclock.c
+++ b/arch/x86/kernel/kvmclock.c
@@ -160,6 +160,7 @@ struct clocksource kvm_clock = {
.rating = 400,
.mask = CLOCKSOURCE_MASK(64),
.flags = CLOCK_SOURCE_IS_CONTINUOUS,
+ .id = CSID_X86_KVM_CLK,
.enable = kvm_cs_enable,
};
EXPORT_SYMBOL_GPL(kvm_clock);
diff --git a/drivers/ptp/ptp_kvm_common.c b/drivers/ptp/ptp_kvm_common.c
index 2418977989be..b0b36f135347 100644
--- a/drivers/ptp/ptp_kvm_common.c
+++ b/drivers/ptp/ptp_kvm_common.c
@@ -4,6 +4,7 @@
*
* Copyright (C) 2017 Red Hat Inc.
*/
+#include <linux/clocksource.h>
#include <linux/device.h>
#include <linux/err.h>
#include <linux/init.h>
@@ -47,6 +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;
*device_time = timespec64_to_ktime(tspec);
diff --git a/include/linux/clocksource_ids.h b/include/linux/clocksource_ids.h
index f8467946e9ee..a4fa3436940c 100644
--- a/include/linux/clocksource_ids.h
+++ b/include/linux/clocksource_ids.h
@@ -8,6 +8,7 @@ enum clocksource_ids {
CSID_ARM_ARCH_COUNTER,
CSID_X86_TSC_EARLY,
CSID_X86_TSC,
+ CSID_X86_KVM_CLK,
CSID_MAX,
};
--
2.40.1
^ permalink raw reply related [flat|nested] 10+ messages in thread* [PATCH v3 5/8] ptp/kvm, arm_arch_timer: Set system_counterval_t.cs_id to constant
2024-02-01 1:04 [PATCH v3 0/8] treewide: Use clocksource ID for get_device_system_crosststamp() Peter Hilber
` (2 preceding siblings ...)
2024-02-01 1:04 ` [PATCH v3 4/8] x86/kvm, ptp/kvm: " Peter Hilber
@ 2024-02-01 1:04 ` Peter Hilber
2024-02-01 13:52 ` Marc Zyngier
2024-02-01 1:04 ` [PATCH v3 6/8] timekeeping: Evaluate system_counterval_t.cs_id instead of .cs Peter Hilber
` (2 subsequent siblings)
6 siblings, 1 reply; 10+ messages in thread
From: Peter Hilber @ 2024-02-01 1:04 UTC (permalink / raw)
To: linux-kernel
Cc: Peter Hilber, D, Lakshmi Sowjanya, Thomas Gleixner, jstultz,
giometti, corbet, Dong, Eddie, Hall, Christopher S, Simon Horman,
Andy Shevchenko, Marc Zyngier, linux-arm-kernel,
Sean Christopherson, Paolo Bonzini, Ingo Molnar, Borislav Petkov,
Dave Hansen, x86, H. Peter Anvin, Wanpeng Li, Vitaly Kuznetsov,
Mark Rutland, Daniel Lezcano, Richard Cochran, kvm, netdev
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 <peter.hilber@opensynergy.com>
---
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
^ permalink raw reply related [flat|nested] 10+ messages in thread* Re: [PATCH v3 5/8] ptp/kvm, arm_arch_timer: Set system_counterval_t.cs_id to constant
2024-02-01 1:04 ` [PATCH v3 5/8] ptp/kvm, arm_arch_timer: Set system_counterval_t.cs_id to constant Peter Hilber
@ 2024-02-01 13:52 ` Marc Zyngier
2024-02-01 14:04 ` Peter Hilber
0 siblings, 1 reply; 10+ messages in thread
From: Marc Zyngier @ 2024-02-01 13:52 UTC (permalink / raw)
To: Peter Hilber
Cc: linux-kernel, D, Lakshmi Sowjanya, Thomas Gleixner, jstultz,
giometti, corbet, Dong, Eddie, Hall, Christopher S, Simon Horman,
Andy Shevchenko, linux-arm-kernel, Sean Christopherson,
Paolo Bonzini, Ingo Molnar, Borislav Petkov, Dave Hansen, x86,
H. Peter Anvin, Wanpeng Li, Vitaly Kuznetsov, Mark Rutland,
Daniel Lezcano, Richard Cochran, kvm, netdev
On Thu, 01 Feb 2024 01:04:50 +0000,
Peter Hilber <peter.hilber@opensynergy.com> 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.
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v3 5/8] ptp/kvm, arm_arch_timer: Set system_counterval_t.cs_id to constant
2024-02-01 13:52 ` Marc Zyngier
@ 2024-02-01 14:04 ` Peter Hilber
0 siblings, 0 replies; 10+ messages in thread
From: Peter Hilber @ 2024-02-01 14:04 UTC (permalink / raw)
To: Marc Zyngier
Cc: linux-kernel, D, Lakshmi Sowjanya, Thomas Gleixner, jstultz,
giometti, corbet, Dong, Eddie, Hall, Christopher S, Simon Horman,
Andy Shevchenko, linux-arm-kernel, Sean Christopherson,
Paolo Bonzini, Ingo Molnar, Borislav Petkov, Dave Hansen, x86,
H. Peter Anvin, Wanpeng Li, Vitaly Kuznetsov, Mark Rutland,
Daniel Lezcano, Richard Cochran, kvm, netdev
On 01.02.24 14:52, Marc Zyngier wrote:
> On Thu, 01 Feb 2024 01:04:50 +0000,
> Peter Hilber <peter.hilber@opensynergy.com> 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
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH v3 6/8] timekeeping: Evaluate system_counterval_t.cs_id instead of .cs
2024-02-01 1:04 [PATCH v3 0/8] treewide: Use clocksource ID for get_device_system_crosststamp() Peter Hilber
` (3 preceding siblings ...)
2024-02-01 1:04 ` [PATCH v3 5/8] ptp/kvm, arm_arch_timer: Set system_counterval_t.cs_id to constant Peter Hilber
@ 2024-02-01 1:04 ` Peter Hilber
2024-02-01 1:04 ` [PATCH v3 7/8] treewide: Remove system_counterval_t.cs, which is never read Peter Hilber
2024-02-01 1:04 ` [PATCH v3 8/8] kvmclock: Unexport kvmclock clocksource Peter Hilber
6 siblings, 0 replies; 10+ messages in thread
From: Peter Hilber @ 2024-02-01 1:04 UTC (permalink / raw)
To: linux-kernel
Cc: Peter Hilber, D, Lakshmi Sowjanya, Thomas Gleixner, jstultz,
giometti, corbet, Dong, Eddie, Hall, Christopher S, Simon Horman,
Andy Shevchenko, Marc Zyngier, linux-arm-kernel,
Sean Christopherson, Paolo Bonzini, Ingo Molnar, Borislav Petkov,
Dave Hansen, x86, H. Peter Anvin, Wanpeng Li, Vitaly Kuznetsov,
Mark Rutland, Daniel Lezcano, Richard Cochran, kvm, netdev,
Stephen Boyd
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/20231218073849.35294-1-peter.hilber@opensynergy.com/
Signed-off-by: Peter Hilber <peter.hilber@opensynergy.com>
---
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
^ permalink raw reply related [flat|nested] 10+ messages in thread* [PATCH v3 7/8] treewide: Remove system_counterval_t.cs, which is never read
2024-02-01 1:04 [PATCH v3 0/8] treewide: Use clocksource ID for get_device_system_crosststamp() Peter Hilber
` (4 preceding siblings ...)
2024-02-01 1:04 ` [PATCH v3 6/8] timekeeping: Evaluate system_counterval_t.cs_id instead of .cs Peter Hilber
@ 2024-02-01 1:04 ` Peter Hilber
2024-02-01 1:04 ` [PATCH v3 8/8] kvmclock: Unexport kvmclock clocksource Peter Hilber
6 siblings, 0 replies; 10+ messages in thread
From: Peter Hilber @ 2024-02-01 1:04 UTC (permalink / raw)
To: linux-kernel
Cc: Peter Hilber, D, Lakshmi Sowjanya, Thomas Gleixner, jstultz,
giometti, corbet, Dong, Eddie, Hall, Christopher S, Simon Horman,
Andy Shevchenko, Marc Zyngier, linux-arm-kernel,
Sean Christopherson, Paolo Bonzini, Ingo Molnar, Borislav Petkov,
Dave Hansen, x86, H. Peter Anvin, Wanpeng Li, Vitaly Kuznetsov,
Mark Rutland, Daniel Lezcano, Richard Cochran, kvm, netdev,
Stephen Boyd
The clocksource pointer in struct system_counterval_t is not evaluated any
more. Remove the code setting the member, and the member itself.
Signed-off-by: Peter Hilber <peter.hilber@opensynergy.com>
---
arch/x86/kernel/tsc.c | 11 ++---------
drivers/clocksource/arm_arch_timer.c | 3 ---
drivers/ptp/ptp_kvm_arm.c | 2 +-
drivers/ptp/ptp_kvm_common.c | 4 +---
drivers/ptp/ptp_kvm_x86.c | 2 --
include/linux/ptp_kvm.h | 4 +---
include/linux/timekeeping.h | 3 ---
7 files changed, 5 insertions(+), 24 deletions(-)
diff --git a/arch/x86/kernel/tsc.c b/arch/x86/kernel/tsc.c
index 9f164aad5e94..693148adca22 100644
--- a/arch/x86/kernel/tsc.c
+++ b/arch/x86/kernel/tsc.c
@@ -53,7 +53,6 @@ static int __read_mostly tsc_force_recalibrate;
static u32 art_to_tsc_numerator;
static u32 art_to_tsc_denominator;
static u64 art_to_tsc_offset;
-static struct clocksource *art_related_clocksource;
static bool have_art;
struct cyc2ns {
@@ -1313,7 +1312,6 @@ struct system_counterval_t convert_art_to_tsc(u64 art)
res += tmp + art_to_tsc_offset;
return (struct system_counterval_t) {
- .cs = art_related_clocksource,
.cs_id = have_art ? CSID_X86_TSC : CSID_GENERIC,
.cycles = res
};
@@ -1350,7 +1348,6 @@ struct system_counterval_t convert_art_ns_to_tsc(u64 art_ns)
res += tmp;
return (struct system_counterval_t) {
- .cs = art_related_clocksource,
.cs_id = have_art ? CSID_X86_TSC : CSID_GENERIC,
.cycles = res
};
@@ -1459,10 +1456,8 @@ static void tsc_refine_calibration_work(struct work_struct *work)
if (tsc_unstable)
goto unreg;
- if (boot_cpu_has(X86_FEATURE_ART)) {
- art_related_clocksource = &clocksource_tsc;
+ if (boot_cpu_has(X86_FEATURE_ART))
have_art = true;
- }
clocksource_register_khz(&clocksource_tsc, tsc_khz);
unreg:
clocksource_unregister(&clocksource_tsc_early);
@@ -1487,10 +1482,8 @@ static int __init init_tsc_clocksource(void)
* the refined calibration and directly register it as a clocksource.
*/
if (boot_cpu_has(X86_FEATURE_TSC_KNOWN_FREQ)) {
- if (boot_cpu_has(X86_FEATURE_ART)) {
- art_related_clocksource = &clocksource_tsc;
+ if (boot_cpu_has(X86_FEATURE_ART))
have_art = true;
- }
clocksource_register_khz(&clocksource_tsc, tsc_khz);
clocksource_unregister(&clocksource_tsc_early);
diff --git a/drivers/clocksource/arm_arch_timer.c b/drivers/clocksource/arm_arch_timer.c
index 45a02872669e..8d4a52056684 100644
--- a/drivers/clocksource/arm_arch_timer.c
+++ b/drivers/clocksource/arm_arch_timer.c
@@ -1807,7 +1807,6 @@ 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,
enum clocksource_ids *cs_id)
{
struct arm_smccc_res hvc_res;
@@ -1832,8 +1831,6 @@ int kvm_arch_ptp_get_crosststamp(u64 *cycle, struct timespec64 *ts,
*ts = ktime_to_timespec64(ktime);
if (cycle)
*cycle = (u64)hvc_res.a2 << 32 | hvc_res.a3;
- if (cs)
- *cs = &clocksource_counter;
if (cs_id)
*cs_id = CSID_ARM_ARCH_COUNTER;
diff --git a/drivers/ptp/ptp_kvm_arm.c b/drivers/ptp/ptp_kvm_arm.c
index 017bb5f03b14..e68e6943167b 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, NULL);
+ return kvm_arch_ptp_get_crosststamp(NULL, ts, NULL);
}
diff --git a/drivers/ptp/ptp_kvm_common.c b/drivers/ptp/ptp_kvm_common.c
index f6683ba0ab3c..15ccb7dd2ed0 100644
--- a/drivers/ptp/ptp_kvm_common.c
+++ b/drivers/ptp/ptp_kvm_common.c
@@ -30,14 +30,13 @@ static int ptp_kvm_get_time_fn(ktime_t *device_time,
{
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, &cs_id);
+ ret = kvm_arch_ptp_get_crosststamp(&cycle, &tspec, &cs_id);
if (ret) {
spin_unlock(&kvm_ptp_lock);
preempt_enable_notrace();
@@ -47,7 +46,6 @@ static int ptp_kvm_get_time_fn(ktime_t *device_time,
preempt_enable_notrace();
system_counter->cycles = cycle;
- system_counter->cs = cs;
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 2782442922cb..617c8d6706d3 100644
--- a/drivers/ptp/ptp_kvm_x86.c
+++ b/drivers/ptp/ptp_kvm_x86.c
@@ -93,7 +93,6 @@ int kvm_arch_ptp_get_clock(struct timespec64 *ts)
}
int kvm_arch_ptp_get_crosststamp(u64 *cycle, struct timespec64 *tspec,
- struct clocksource **cs,
enum clocksource_ids *cs_id)
{
struct pvclock_vcpu_time_info *src;
@@ -124,7 +123,6 @@ int kvm_arch_ptp_get_crosststamp(u64 *cycle, struct timespec64 *tspec,
*cycle = __pvclock_read_cycles(src, clock_pair->tsc);
} 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 95b3d4d0d7dd..e8c74fa3f455 100644
--- a/include/linux/ptp_kvm.h
+++ b/include/linux/ptp_kvm.h
@@ -12,13 +12,11 @@
#include <linux/types.h>
struct timespec64;
-struct clocksource;
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,
- enum clocksource_ids *cs_id);
+ struct timespec64 *tspec, enum clocksource_ids *cs_id);
#endif /* _PTP_KVM_H_ */
diff --git a/include/linux/timekeeping.h b/include/linux/timekeeping.h
index 3538c5bdf9ee..7e50cbd97f86 100644
--- a/include/linux/timekeeping.h
+++ b/include/linux/timekeeping.h
@@ -271,8 +271,6 @@ struct system_device_crosststamp {
* 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. 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
@@ -280,7 +278,6 @@ struct system_device_crosststamp {
*/
struct system_counterval_t {
u64 cycles;
- struct clocksource *cs;
enum clocksource_ids cs_id;
};
--
2.40.1
^ permalink raw reply related [flat|nested] 10+ messages in thread* [PATCH v3 8/8] kvmclock: Unexport kvmclock clocksource
2024-02-01 1:04 [PATCH v3 0/8] treewide: Use clocksource ID for get_device_system_crosststamp() Peter Hilber
` (5 preceding siblings ...)
2024-02-01 1:04 ` [PATCH v3 7/8] treewide: Remove system_counterval_t.cs, which is never read Peter Hilber
@ 2024-02-01 1:04 ` Peter Hilber
6 siblings, 0 replies; 10+ messages in thread
From: Peter Hilber @ 2024-02-01 1:04 UTC (permalink / raw)
To: linux-kernel
Cc: Peter Hilber, D, Lakshmi Sowjanya, Thomas Gleixner, jstultz,
giometti, corbet, Dong, Eddie, Hall, Christopher S, Simon Horman,
Andy Shevchenko, Sean Christopherson, Paolo Bonzini, Ingo Molnar,
Borislav Petkov, Dave Hansen, x86, H. Peter Anvin, Wanpeng Li,
Vitaly Kuznetsov, Mark Rutland, Marc Zyngier, Daniel Lezcano,
Richard Cochran, kvm, netdev
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 <peter.hilber@opensynergy.com>
---
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
^ permalink raw reply related [flat|nested] 10+ messages in thread