2022-11-02 22:54:29

by Stanislav Kinsburskii

[permalink] [raw]
Subject: [PATCH v2 2/4] drivers/clocksource/hyper-v: Introduce TSC PFN getter

From: Stanislav Kinsburskiy <[email protected]>

And rework the code to use it instead of the physical address, which isn't
required by itself.

This is a cleanup and precursor patch for upcoming support for TSC page
mapping into Microsoft Hypervisor root partition, where TSC PFN will be
defined by the hypervisor and not by the kernel.

Signed-off-by: Stanislav Kinsburskiy <[email protected]>
CC: "K. Y. Srinivasan" <[email protected]>
CC: Haiyang Zhang <[email protected]>
CC: Wei Liu <[email protected]>
CC: Dexuan Cui <[email protected]>
CC: Daniel Lezcano <[email protected]>
CC: Thomas Gleixner <[email protected]>
CC: [email protected]
CC: [email protected]
---
drivers/clocksource/hyperv_timer.c | 14 +++++++++-----
1 file changed, 9 insertions(+), 5 deletions(-)

diff --git a/drivers/clocksource/hyperv_timer.c b/drivers/clocksource/hyperv_timer.c
index c4dbf40a3d3e..d447bc99a399 100644
--- a/drivers/clocksource/hyperv_timer.c
+++ b/drivers/clocksource/hyperv_timer.c
@@ -367,6 +367,12 @@ static union {
} tsc_pg __aligned(PAGE_SIZE);

static struct ms_hyperv_tsc_page *tsc_page = &tsc_pg.page;
+static unsigned long tsc_pfn;
+
+static unsigned long hv_get_tsc_pfn(void)
+{
+ return tsc_pfn;
+}

struct ms_hyperv_tsc_page *hv_get_tsc_page(void)
{
@@ -408,13 +414,12 @@ static void suspend_hv_clock_tsc(struct clocksource *arg)

static void resume_hv_clock_tsc(struct clocksource *arg)
{
- phys_addr_t phys_addr = virt_to_phys(tsc_page);
union hv_reference_tsc_msr tsc_msr;

/* Re-enable the TSC page */
tsc_msr.as_uint64 = hv_get_register(HV_REGISTER_REFERENCE_TSC);
tsc_msr.enable = 1;
- tsc_msr.pfn = __phys_to_pfn(phys_addr);
+ tsc_msr.pfn = tsc_pfn;
hv_set_register(HV_REGISTER_REFERENCE_TSC, tsc_msr.as_uint64);
}

@@ -498,7 +503,6 @@ static __always_inline void hv_setup_sched_clock(void *sched_clock) {}
static bool __init hv_init_tsc_clocksource(void)
{
union hv_reference_tsc_msr tsc_msr;
- phys_addr_t phys_addr;

if (!(ms_hyperv.features & HV_MSR_REFERENCE_TSC_AVAILABLE))
return false;
@@ -523,7 +527,7 @@ static bool __init hv_init_tsc_clocksource(void)
}

hv_read_reference_counter = read_hv_clock_tsc;
- phys_addr = virt_to_phys(hv_get_tsc_page());
+ tsc_pfn = __phys_to_pfn(virt_to_phys(tsc_page));

/*
* The Hyper-V TLFS specifies to preserve the value of reserved
@@ -534,7 +538,7 @@ static bool __init hv_init_tsc_clocksource(void)
*/
tsc_msr.as_uint64 = hv_get_register(HV_REGISTER_REFERENCE_TSC);
tsc_msr.enable = 1;
- tsc_msr.pfn = __phys_to_pfn(phys_addr);
+ tsc_msr.pfn = tsc_pfn;
hv_set_register(HV_REGISTER_REFERENCE_TSC, tsc_msr.as_uint64);

clocksource_register_hz(&hyperv_cs_tsc, NSEC_PER_SEC/100);




2022-11-03 00:16:09

by Michael Kelley (LINUX)

[permalink] [raw]
Subject: RE: [PATCH v2 2/4] drivers/clocksource/hyper-v: Introduce TSC PFN getter

From: Stanislav Kinsburskii <[email protected]> Sent: Wednesday, November 2, 2022 3:07 PM
>
> And rework the code to use it instead of the physical address, which isn't
> required by itself.
>
> This is a cleanup and precursor patch for upcoming support for TSC page
> mapping into Microsoft Hypervisor root partition, where TSC PFN will be
> defined by the hypervisor and not by the kernel.
>
> Signed-off-by: Stanislav Kinsburskiy <[email protected]>
> CC: "K. Y. Srinivasan" <[email protected]>
> CC: Haiyang Zhang <[email protected]>
> CC: Wei Liu <[email protected]>
> CC: Dexuan Cui <[email protected]>
> CC: Daniel Lezcano <[email protected]>
> CC: Thomas Gleixner <[email protected]>
> CC: [email protected]
> CC: [email protected]
> ---
> drivers/clocksource/hyperv_timer.c | 14 +++++++++-----
> 1 file changed, 9 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/clocksource/hyperv_timer.c b/drivers/clocksource/hyperv_timer.c
> index c4dbf40a3d3e..d447bc99a399 100644
> --- a/drivers/clocksource/hyperv_timer.c
> +++ b/drivers/clocksource/hyperv_timer.c
> @@ -367,6 +367,12 @@ static union {
> } tsc_pg __aligned(PAGE_SIZE);
>
> static struct ms_hyperv_tsc_page *tsc_page = &tsc_pg.page;
> +static unsigned long tsc_pfn;
> +
> +static unsigned long hv_get_tsc_pfn(void)
> +{
> + return tsc_pfn;
> +}
>
> struct ms_hyperv_tsc_page *hv_get_tsc_page(void)
> {
> @@ -408,13 +414,12 @@ static void suspend_hv_clock_tsc(struct clocksource *arg)
>
> static void resume_hv_clock_tsc(struct clocksource *arg)
> {
> - phys_addr_t phys_addr = virt_to_phys(tsc_page);
> union hv_reference_tsc_msr tsc_msr;
>
> /* Re-enable the TSC page */
> tsc_msr.as_uint64 = hv_get_register(HV_REGISTER_REFERENCE_TSC);
> tsc_msr.enable = 1;
> - tsc_msr.pfn = __phys_to_pfn(phys_addr);
> + tsc_msr.pfn = tsc_pfn;
> hv_set_register(HV_REGISTER_REFERENCE_TSC, tsc_msr.as_uint64);
> }
>
> @@ -498,7 +503,6 @@ static __always_inline void hv_setup_sched_clock(void
> *sched_clock) {}
> static bool __init hv_init_tsc_clocksource(void)
> {
> union hv_reference_tsc_msr tsc_msr;
> - phys_addr_t phys_addr;
>
> if (!(ms_hyperv.features & HV_MSR_REFERENCE_TSC_AVAILABLE))
> return false;
> @@ -523,7 +527,7 @@ static bool __init hv_init_tsc_clocksource(void)
> }
>
> hv_read_reference_counter = read_hv_clock_tsc;
> - phys_addr = virt_to_phys(hv_get_tsc_page());
> + tsc_pfn = __phys_to_pfn(virt_to_phys(tsc_page));

I probably wasn't specific enough when I said "your choice" in my previous
comments. This sets tsc_pfn to a guest PFN. In a guest with page size
other than 4096, it will be wrong when stored into tsc_msr.pfn a few
lines below. hyperv_timer.c is currently architecture and page size
independent, and it needs to stay that way.

> /*
> * The Hyper-V TLFS specifies to preserve the value of reserved
> @@ -534,7 +538,7 @@ static bool __init hv_init_tsc_clocksource(void)
> */
> tsc_msr.as_uint64 = hv_get_register(HV_REGISTER_REFERENCE_TSC);
> tsc_msr.enable = 1;
> - tsc_msr.pfn = __phys_to_pfn(phys_addr);
> + tsc_msr.pfn = tsc_pfn;
> hv_set_register(HV_REGISTER_REFERENCE_TSC, tsc_msr.as_uint64);
>
> clocksource_register_hz(&hyperv_cs_tsc, NSEC_PER_SEC/100);
>