2023-12-15 22:09:51

by Peter Hilber

[permalink] [raw]
Subject: [RFC PATCH v2 3/7] x86/kvm, ptp/kvm: Add clocksource ID, set system_counterval_t.cs_id

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, this will keep get_device_system_crosststamp() working, when it
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 <[email protected]>
---

Notes:
v2:

- Name clock id according to Thomas Gleixner's mockup.

arch/x86/kernel/kvmclock.c | 2 ++
drivers/ptp/ptp_kvm_common.c | 2 ++
include/linux/clocksource_ids.h | 1 +
3 files changed, 5 insertions(+)

diff --git a/arch/x86/kernel/kvmclock.c b/arch/x86/kernel/kvmclock.c
index fb8f52149be9..25d6bf743b03 100644
--- a/arch/x86/kernel/kvmclock.c
+++ b/arch/x86/kernel/kvmclock.c
@@ -4,6 +4,7 @@
*/

#include <linux/clocksource.h>
+#include <linux/clocksource_ids.h>
#include <linux/kvm_para.h>
#include <asm/pvclock.h>
#include <asm/msr.h>
@@ -160,6 +161,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



2023-12-18 10:24:14

by Andy Shevchenko

[permalink] [raw]
Subject: Re: [RFC PATCH v2 3/7] x86/kvm, ptp/kvm: Add clocksource ID, set system_counterval_t.cs_id

On Fri, Dec 15, 2023 at 11:06:08PM +0100, Peter Hilber wrote:
> 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, this will keep get_device_system_crosststamp() working, when it
> 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.

...

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

It's the second file that includes both.

I'm just wondering if it makes sense to always (?) include the latter into
the former.

--
With Best Regards,
Andy Shevchenko



2024-01-11 11:39:21

by Peter Hilber

[permalink] [raw]
Subject: Re: [RFC PATCH v2 3/7] x86/kvm, ptp/kvm: Add clocksource ID, set system_counterval_t.cs_id

On 18.12.23 11:23, Andy Shevchenko wrote:
> On Fri, Dec 15, 2023 at 11:06:08PM +0100, Peter Hilber wrote:
>> 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, this will keep get_device_system_crosststamp() working, when it
>> 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.
>
> ...
>
>> #include <linux/clocksource.h>
>> +#include <linux/clocksource_ids.h>
>
> It's the second file that includes both.
>
> I'm just wondering if it makes sense to always (?) include the latter into
> the former.
>

Actually, clocksource.h already includes clocksource_ids.h, always since
the latter was created. So I'll just omit the unnecessary clocksource_ids.h
includes in other files.

Thanks for the comment,

Peter