[PATCH v3 0/4] hyper-v: Introduce TSC page for root partition

[PATCH v3 0/4] hyper-v: Introduce TSC page for root partition

[Stanislav Kinsburskii]

This series does some cleanup and precursor changes to the hyper-v clock
source and introduces support for TSC page based clock in root partition.

Hyper-V root partition requires kernel to map the page, specified by the
hypervisor (instead of provide the PFN to the hypervisor like it's done in
guest partition).

v3:
1. Rebased on top of the fix to support arbitrary TSC page size by Anirudh
2. Fixed TSC physical page when remapping in root partition.

v2:
1. Addressed all the comments to the commit messages.
2. TSC page in root partition is now remapped based on size of the tsc_pg
variable.

The following series implements...

---

Stanislav Kinsburskiy (4):
      drivers/clocksource/hyper-v: Introduce a pointer to TSC page
      drivers/clocksource/hyper-v: Introduce TSC PFN getter
      drivers/clocksource/hyper-v: Use TSC PFN getter to map vvar page
      drivers/clocksource/hyper-v: Add TSC page support for root partition


 arch/x86/entry/vdso/vma.c          |    7 ++---
 arch/x86/hyperv/hv_init.c          |    2 +
 drivers/clocksource/hyperv_timer.c |   55 +++++++++++++++++++++++++++---------
 include/clocksource/hyperv_timer.h |    7 +++++
 4 files changed, 53 insertions(+), 18 deletions(-)

[PATCH v3 1/4] drivers/clocksource/hyper-v: Introduce a pointer to TSC page

[Stanislav Kinsburskii]

From: Stanislav Kinsburskiy <stanislav.kinsburskiy@gmail.com>

Will be used later keep the address of the remapped page for the root
partition as it will be Microsoft Hypervisor defined (and thus won't be a
static address).

Signed-off-by: Stanislav Kinsburskiy <stanislav.kinsburskiy@gmail.com>
CC: "K. Y. Srinivasan" <kys@microsoft.com>
CC: Haiyang Zhang <haiyangz@microsoft.com>
CC: Wei Liu <wei.liu@kernel.org>
CC: Dexuan Cui <decui@microsoft.com>
CC: Daniel Lezcano <daniel.lezcano@linaro.org>
CC: Thomas Gleixner <tglx@linutronix.de>
CC: linux-hyperv@vger.kernel.org
CC: linux-kernel@vger.kernel.org
---
 drivers/clocksource/hyperv_timer.c |    6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/clocksource/hyperv_timer.c b/drivers/clocksource/hyperv_timer.c
index 18de1f439ffd..b0b5df576e17 100644
--- a/drivers/clocksource/hyperv_timer.c
+++ b/drivers/clocksource/hyperv_timer.c
@@ -367,9 +367,11 @@ static union {
 	u8 reserved[PAGE_SIZE];
 } tsc_pg __aligned(PAGE_SIZE);
 
+static struct ms_hyperv_tsc_page *tsc_page = &tsc_pg.page;
+
 struct ms_hyperv_tsc_page *hv_get_tsc_page(void)
 {
-	return &tsc_pg.page;
+	return tsc_page;
 }
 EXPORT_SYMBOL_GPL(hv_get_tsc_page);
 
@@ -407,7 +409,7 @@ 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_pg);
+	phys_addr_t phys_addr = virt_to_phys(tsc_page);
 	union hv_reference_tsc_msr tsc_msr;
 
 	/* Re-enable the TSC page */

RE: [PATCH v3 1/4] drivers/clocksource/hyper-v: Introduce a pointer to TSC page

[Michael Kelley (LINUX)]

From: Stanislav Kinsburskii <skinsburskii@linux.microsoft.com> Sent: Thursday, November 3, 2022 10:59 AM
> 
> Will be used later keep the address of the remapped page for the root
> partition as it will be Microsoft Hypervisor defined (and thus won't be a
> static address).
> 
> Signed-off-by: Stanislav Kinsburskiy <stanislav.kinsburskiy@gmail.com>
> CC: "K. Y. Srinivasan" <kys@microsoft.com>
> CC: Haiyang Zhang <haiyangz@microsoft.com>
> CC: Wei Liu <wei.liu@kernel.org>
> CC: Dexuan Cui <decui@microsoft.com>
> CC: Daniel Lezcano <daniel.lezcano@linaro.org>
> CC: Thomas Gleixner <tglx@linutronix.de>
> CC: linux-hyperv@vger.kernel.org
> CC: linux-kernel@vger.kernel.org
> ---
>  drivers/clocksource/hyperv_timer.c |    6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/clocksource/hyperv_timer.c b/drivers/clocksource/hyperv_timer.c
> index 18de1f439ffd..b0b5df576e17 100644
> --- a/drivers/clocksource/hyperv_timer.c
> +++ b/drivers/clocksource/hyperv_timer.c
> @@ -367,9 +367,11 @@ static union {
>  	u8 reserved[PAGE_SIZE];
>  } tsc_pg __aligned(PAGE_SIZE);
> 
> +static struct ms_hyperv_tsc_page *tsc_page = &tsc_pg.page;
> +
>  struct ms_hyperv_tsc_page *hv_get_tsc_page(void)
>  {
> -	return &tsc_pg.page;
> +	return tsc_page;
>  }
>  EXPORT_SYMBOL_GPL(hv_get_tsc_page);
> 
> @@ -407,7 +409,7 @@ 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_pg);
> +	phys_addr_t phys_addr = virt_to_phys(tsc_page);
>  	union hv_reference_tsc_msr tsc_msr;
> 
>  	/* Re-enable the TSC page */
> 

Reviewed-by: Michael Kelley <mikelley@microsoft.com>

[PATCH v3 2/4] drivers/clocksource/hyper-v: Introduce TSC PFN getter

[Stanislav Kinsburskii]

From: Stanislav Kinsburskiy <stanislav.kinsburskiy@gmail.com>

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 <stanislav.kinsburskiy@gmail.com>
CC: "K. Y. Srinivasan" <kys@microsoft.com>
CC: Haiyang Zhang <haiyangz@microsoft.com>
CC: Wei Liu <wei.liu@kernel.org>
CC: Dexuan Cui <decui@microsoft.com>
CC: Daniel Lezcano <daniel.lezcano@linaro.org>
CC: Thomas Gleixner <tglx@linutronix.de>
CC: linux-hyperv@vger.kernel.org
CC: linux-kernel@vger.kernel.org
---
 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 b0b5df576e17..b7af19d06b51 100644
--- a/drivers/clocksource/hyperv_timer.c
+++ b/drivers/clocksource/hyperv_timer.c
@@ -368,6 +368,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)
 {
@@ -409,13 +415,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 = HVPFN_DOWN(phys_addr);
+	tsc_msr.pfn = tsc_pfn;
 	hv_set_register(HV_REGISTER_REFERENCE_TSC, tsc_msr.as_uint64);
 }
 
@@ -499,7 +504,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;
@@ -524,7 +528,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 = HVPFN_DOWN(virt_to_phys(tsc_page));
 
 	/*
 	 * The Hyper-V TLFS specifies to preserve the value of reserved
@@ -535,7 +539,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 = HVPFN_DOWN(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);

RE: [PATCH v3 2/4] drivers/clocksource/hyper-v: Introduce TSC PFN getter

[Michael Kelley (LINUX)]

From: Stanislav Kinsburskii <skinsburskii@linux.microsoft.com> Sent: Thursday, November 3, 2022 10:59 AM
> 
> 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 <stanislav.kinsburskiy@gmail.com>
> CC: "K. Y. Srinivasan" <kys@microsoft.com>
> CC: Haiyang Zhang <haiyangz@microsoft.com>
> CC: Wei Liu <wei.liu@kernel.org>
> CC: Dexuan Cui <decui@microsoft.com>
> CC: Daniel Lezcano <daniel.lezcano@linaro.org>
> CC: Thomas Gleixner <tglx@linutronix.de>
> CC: linux-hyperv@vger.kernel.org
> CC: linux-kernel@vger.kernel.org
> ---
>  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 b0b5df576e17..b7af19d06b51 100644
> --- a/drivers/clocksource/hyperv_timer.c
> +++ b/drivers/clocksource/hyperv_timer.c
> @@ -368,6 +368,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)
>  {
> @@ -409,13 +415,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 = HVPFN_DOWN(phys_addr);
> +	tsc_msr.pfn = tsc_pfn;
>  	hv_set_register(HV_REGISTER_REFERENCE_TSC, tsc_msr.as_uint64);
>  }
> 
> @@ -499,7 +504,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;
> @@ -524,7 +528,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 = HVPFN_DOWN(virt_to_phys(tsc_page));
> 
>  	/*
>  	 * The Hyper-V TLFS specifies to preserve the value of reserved
> @@ -535,7 +539,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 = HVPFN_DOWN(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);
> 

Reviewed-by: Michael Kelley <mikelley@microsoft.com>

[PATCH v3 3/4] drivers/clocksource/hyper-v: Use TSC PFN getter to map vvar page

[Stanislav Kinsburskii]

From: Stanislav Kinsburskiy <stanislav.kinsburskiy@gmail.com>

Instead of converting the virtual address to physical directly.

This is a precursor patch for the upcoming support for TSC page mapping into
Microsoft Hypervisor root partition, where TSC PFN will be defined by the
hypervisor and thus can't be obtained by linear translation of the physical
address.

Signed-off-by: Stanislav Kinsburskiy <stanislav.kinsburskiy@gmail.com>
CC: Andy Lutomirski <luto@kernel.org>
CC: Thomas Gleixner <tglx@linutronix.de>
CC: Ingo Molnar <mingo@redhat.com>
CC: Borislav Petkov <bp@alien8.de>
CC: Dave Hansen <dave.hansen@linux.intel.com>
CC: x86@kernel.org
CC: "H. Peter Anvin" <hpa@zytor.com>
CC: "K. Y. Srinivasan" <kys@microsoft.com>
CC: Haiyang Zhang <haiyangz@microsoft.com>
CC: Wei Liu <wei.liu@kernel.org>
CC: Dexuan Cui <decui@microsoft.com>
CC: Daniel Lezcano <daniel.lezcano@linaro.org>
CC: linux-kernel@vger.kernel.org
CC: linux-hyperv@vger.kernel.org
---
 arch/x86/entry/vdso/vma.c          |    7 +++----
 drivers/clocksource/hyperv_timer.c |    3 ++-
 include/clocksource/hyperv_timer.h |    6 ++++++
 3 files changed, 11 insertions(+), 5 deletions(-)

diff --git a/arch/x86/entry/vdso/vma.c b/arch/x86/entry/vdso/vma.c
index 311eae30e089..6976416b2c9f 100644
--- a/arch/x86/entry/vdso/vma.c
+++ b/arch/x86/entry/vdso/vma.c
@@ -210,11 +210,10 @@ static vm_fault_t vvar_fault(const struct vm_special_mapping *sm,
 					pgprot_decrypted(vma->vm_page_prot));
 		}
 	} else if (sym_offset == image->sym_hvclock_page) {
-		struct ms_hyperv_tsc_page *tsc_pg = hv_get_tsc_page();
+		pfn = hv_get_tsc_pfn();
 
-		if (tsc_pg && vclock_was_used(VDSO_CLOCKMODE_HVCLOCK))
-			return vmf_insert_pfn(vma, vmf->address,
-					virt_to_phys(tsc_pg) >> PAGE_SHIFT);
+		if (pfn && vclock_was_used(VDSO_CLOCKMODE_HVCLOCK))
+			return vmf_insert_pfn(vma, vmf->address, pfn);
 	} else if (sym_offset == image->sym_timens_page) {
 		struct page *timens_page = find_timens_vvar_page(vma);
 
diff --git a/drivers/clocksource/hyperv_timer.c b/drivers/clocksource/hyperv_timer.c
index b7af19d06b51..9445a1558fe9 100644
--- a/drivers/clocksource/hyperv_timer.c
+++ b/drivers/clocksource/hyperv_timer.c
@@ -370,10 +370,11 @@ static union {
 static struct ms_hyperv_tsc_page *tsc_page = &tsc_pg.page;
 static unsigned long tsc_pfn;
 
-static unsigned long hv_get_tsc_pfn(void)
+unsigned long hv_get_tsc_pfn(void)
 {
 	return tsc_pfn;
 }
+EXPORT_SYMBOL_GPL(hv_get_tsc_pfn);
 
 struct ms_hyperv_tsc_page *hv_get_tsc_page(void)
 {
diff --git a/include/clocksource/hyperv_timer.h b/include/clocksource/hyperv_timer.h
index b3f5d73ae1d6..3078d23faaea 100644
--- a/include/clocksource/hyperv_timer.h
+++ b/include/clocksource/hyperv_timer.h
@@ -32,6 +32,7 @@ extern void hv_stimer0_isr(void);
 
 extern void hv_init_clocksource(void);
 
+extern unsigned long hv_get_tsc_pfn(void);
 extern struct ms_hyperv_tsc_page *hv_get_tsc_page(void);
 
 static inline notrace u64
@@ -90,6 +91,11 @@ hv_read_tsc_page(const struct ms_hyperv_tsc_page *tsc_pg)
 }
 
 #else /* CONFIG_HYPERV_TIMER */
+static inline unsigned long hv_get_tsc_pfn(void)
+{
+	return 0;
+}
+
 static inline struct ms_hyperv_tsc_page *hv_get_tsc_page(void)
 {
 	return NULL;

RE: [PATCH v3 3/4] drivers/clocksource/hyper-v: Use TSC PFN getter to map vvar page

[Michael Kelley (LINUX)]

From: Stanislav Kinsburskii <skinsburskii@linux.microsoft.com> Sent: Thursday, November 3, 2022 10:59 AM
> 
> Instead of converting the virtual address to physical directly.
> 
> This is a precursor patch for the upcoming support for TSC page mapping into
> Microsoft Hypervisor root partition, where TSC PFN will be defined by the
> hypervisor and thus can't be obtained by linear translation of the physical
> address.
> 
> Signed-off-by: Stanislav Kinsburskiy <stanislav.kinsburskiy@gmail.com>
> CC: Andy Lutomirski <luto@kernel.org>
> CC: Thomas Gleixner <tglx@linutronix.de>
> CC: Ingo Molnar <mingo@redhat.com>
> CC: Borislav Petkov <bp@alien8.de>
> CC: Dave Hansen <dave.hansen@linux.intel.com>
> CC: x86@kernel.org
> CC: "H. Peter Anvin" <hpa@zytor.com>
> CC: "K. Y. Srinivasan" <kys@microsoft.com>
> CC: Haiyang Zhang <haiyangz@microsoft.com>
> CC: Wei Liu <wei.liu@kernel.org>
> CC: Dexuan Cui <decui@microsoft.com>
> CC: Daniel Lezcano <daniel.lezcano@linaro.org>
> CC: linux-kernel@vger.kernel.org
> CC: linux-hyperv@vger.kernel.org
> ---
>  arch/x86/entry/vdso/vma.c          |    7 +++----
>  drivers/clocksource/hyperv_timer.c |    3 ++-
>  include/clocksource/hyperv_timer.h |    6 ++++++
>  3 files changed, 11 insertions(+), 5 deletions(-)
> 
> diff --git a/arch/x86/entry/vdso/vma.c b/arch/x86/entry/vdso/vma.c
> index 311eae30e089..6976416b2c9f 100644
> --- a/arch/x86/entry/vdso/vma.c
> +++ b/arch/x86/entry/vdso/vma.c
> @@ -210,11 +210,10 @@ static vm_fault_t vvar_fault(const struct
> vm_special_mapping *sm,
>  					pgprot_decrypted(vma->vm_page_prot));
>  		}
>  	} else if (sym_offset == image->sym_hvclock_page) {
> -		struct ms_hyperv_tsc_page *tsc_pg = hv_get_tsc_page();
> +		pfn = hv_get_tsc_pfn();
> 
> -		if (tsc_pg && vclock_was_used(VDSO_CLOCKMODE_HVCLOCK))
> -			return vmf_insert_pfn(vma, vmf->address,
> -					virt_to_phys(tsc_pg) >> PAGE_SHIFT);
> +		if (pfn && vclock_was_used(VDSO_CLOCKMODE_HVCLOCK))
> +			return vmf_insert_pfn(vma, vmf->address, pfn);
>  	} else if (sym_offset == image->sym_timens_page) {
>  		struct page *timens_page = find_timens_vvar_page(vma);
> 
> diff --git a/drivers/clocksource/hyperv_timer.c b/drivers/clocksource/hyperv_timer.c
> index b7af19d06b51..9445a1558fe9 100644
> --- a/drivers/clocksource/hyperv_timer.c
> +++ b/drivers/clocksource/hyperv_timer.c
> @@ -370,10 +370,11 @@ static union {
>  static struct ms_hyperv_tsc_page *tsc_page = &tsc_pg.page;
>  static unsigned long tsc_pfn;
> 
> -static unsigned long hv_get_tsc_pfn(void)
> +unsigned long hv_get_tsc_pfn(void)
>  {
>  	return tsc_pfn;
>  }
> +EXPORT_SYMBOL_GPL(hv_get_tsc_pfn);
> 
>  struct ms_hyperv_tsc_page *hv_get_tsc_page(void)
>  {
> diff --git a/include/clocksource/hyperv_timer.h
> b/include/clocksource/hyperv_timer.h
> index b3f5d73ae1d6..3078d23faaea 100644
> --- a/include/clocksource/hyperv_timer.h
> +++ b/include/clocksource/hyperv_timer.h
> @@ -32,6 +32,7 @@ extern void hv_stimer0_isr(void);
> 
>  extern void hv_init_clocksource(void);
> 
> +extern unsigned long hv_get_tsc_pfn(void);
>  extern struct ms_hyperv_tsc_page *hv_get_tsc_page(void);
> 
>  static inline notrace u64
> @@ -90,6 +91,11 @@ hv_read_tsc_page(const struct ms_hyperv_tsc_page *tsc_pg)
>  }
> 
>  #else /* CONFIG_HYPERV_TIMER */
> +static inline unsigned long hv_get_tsc_pfn(void)
> +{
> +	return 0;
> +}
> +
>  static inline struct ms_hyperv_tsc_page *hv_get_tsc_page(void)
>  {
>  	return NULL;
> 

Reviewed-by: Michael Kelley <mikelley@microsoft.com>

[PATCH v3 4/4] drivers/clocksource/hyper-v: Add TSC page support for root partition

[Stanislav Kinsburskii]

From: Stanislav Kinsburskiy <stanislav.kinsburskiy@gmail.com>

Microsoft Hypervisor root partition has to map the TSC page specified
by the hypervisor, instead of providing the page to the hypervisor like
it's done in the guest partitions.

However, it's too early to map the page when the clock is initialized, so, the
actual mapping is happening later.

Signed-off-by: Stanislav Kinsburskiy <stanislav.kinsburskiy@gmail.com>
CC: "K. Y. Srinivasan" <kys@microsoft.com>
CC: Haiyang Zhang <haiyangz@microsoft.com>
CC: Wei Liu <wei.liu@kernel.org>
CC: Dexuan Cui <decui@microsoft.com>
CC: Thomas Gleixner <tglx@linutronix.de>
CC: Ingo Molnar <mingo@redhat.com>
CC: Borislav Petkov <bp@alien8.de>
CC: Dave Hansen <dave.hansen@linux.intel.com>
CC: x86@kernel.org
CC: "H. Peter Anvin" <hpa@zytor.com>
CC: Daniel Lezcano <daniel.lezcano@linaro.org>
CC: linux-hyperv@vger.kernel.org
CC: linux-kernel@vger.kernel.org
---
 arch/x86/hyperv/hv_init.c          |    2 ++
 drivers/clocksource/hyperv_timer.c |   38 +++++++++++++++++++++++++++---------
 include/clocksource/hyperv_timer.h |    1 +
 3 files changed, 32 insertions(+), 9 deletions(-)

diff --git a/arch/x86/hyperv/hv_init.c b/arch/x86/hyperv/hv_init.c
index f49bc3ec76e6..89954490af93 100644
--- a/arch/x86/hyperv/hv_init.c
+++ b/arch/x86/hyperv/hv_init.c
@@ -464,6 +464,8 @@ void __init hyperv_init(void)
 		BUG_ON(!src);
 		memcpy_to_page(pg, 0, src, HV_HYP_PAGE_SIZE);
 		memunmap(src);
+
+		hv_remap_tsc_clocksource();
 	} else {
 		hypercall_msr.guest_physical_address = vmalloc_to_pfn(hv_hypercall_pg);
 		wrmsrl(HV_X64_MSR_HYPERCALL, hypercall_msr.as_uint64);
diff --git a/drivers/clocksource/hyperv_timer.c b/drivers/clocksource/hyperv_timer.c
index 9445a1558fe9..dec7ad3b85ba 100644
--- a/drivers/clocksource/hyperv_timer.c
+++ b/drivers/clocksource/hyperv_timer.c
@@ -509,9 +509,6 @@ static bool __init hv_init_tsc_clocksource(void)
 	if (!(ms_hyperv.features & HV_MSR_REFERENCE_TSC_AVAILABLE))
 		return false;
 
-	if (hv_root_partition)
-		return false;
-
 	/*
 	 * If Hyper-V offers TSC_INVARIANT, then the virtualized TSC correctly
 	 * handles frequency and offset changes due to live migration,
@@ -529,16 +526,22 @@ static bool __init hv_init_tsc_clocksource(void)
 	}
 
 	hv_read_reference_counter = read_hv_clock_tsc;
-	tsc_pfn = HVPFN_DOWN(virt_to_phys(tsc_page));
 
 	/*
-	 * The Hyper-V TLFS specifies to preserve the value of reserved
-	 * bits in registers. So read the existing value, preserve the
-	 * low order 12 bits, and add in the guest physical address
-	 * (which already has at least the low 12 bits set to zero since
-	 * it is page aligned). Also set the "enable" bit, which is bit 0.
+	 * TSC page mapping works differently in root compared to guest.
+	 * - In guest partition the guest PFN has to be passed to the
+	 *   hypervisor.
+	 * - In root partition it's other way around: it has to map the PFN
+	 *   provided by the hypervisor.
+	 *   But it can't be mapped right here as it's too early and MMU isn't
+	 *   ready yet. So, we only set the enable bit here and will remap the
+	 *   page later in hv_remap_tsc_clocksource().
 	 */
 	tsc_msr.as_uint64 = hv_get_register(HV_REGISTER_REFERENCE_TSC);
+	if (hv_root_partition)
+		tsc_pfn = tsc_msr.pfn;
+	else
+		tsc_pfn = HVPFN_DOWN(virt_to_phys(tsc_page));
 	tsc_msr.enable = 1;
 	tsc_msr.pfn = tsc_pfn;
 	hv_set_register(HV_REGISTER_REFERENCE_TSC, tsc_msr.as_uint64);
@@ -573,3 +576,20 @@ void __init hv_init_clocksource(void)
 	hv_sched_clock_offset = hv_read_reference_counter();
 	hv_setup_sched_clock(read_hv_sched_clock_msr);
 }
+
+void __init hv_remap_tsc_clocksource(void)
+{
+	if (!(ms_hyperv.features & HV_MSR_REFERENCE_TSC_AVAILABLE))
+		return;
+
+	if (!hv_root_partition) {
+		WARN(1, "%s: attempt to remap TSC page in guest partition\n",
+		     __func__);
+		return;
+	}
+
+	tsc_page = memremap(tsc_pfn << HV_HYP_PAGE_SHIFT, sizeof(tsc_pg),
+			    MEMREMAP_WB);
+	if (!tsc_page)
+		pr_err("Failed to remap Hyper-V TSC page.\n");
+}
diff --git a/include/clocksource/hyperv_timer.h b/include/clocksource/hyperv_timer.h
index 3078d23faaea..783701a2102d 100644
--- a/include/clocksource/hyperv_timer.h
+++ b/include/clocksource/hyperv_timer.h
@@ -31,6 +31,7 @@ extern void hv_stimer_global_cleanup(void);
 extern void hv_stimer0_isr(void);
 
 extern void hv_init_clocksource(void);
+extern void hv_remap_tsc_clocksource(void);
 
 extern unsigned long hv_get_tsc_pfn(void);
 extern struct ms_hyperv_tsc_page *hv_get_tsc_page(void);

RE: [PATCH v3 4/4] drivers/clocksource/hyper-v: Add TSC page support for root partition

[Michael Kelley (LINUX)]

From: Stanislav Kinsburskii <skinsburskii@linux.microsoft.com> Sent: Thursday, November 3, 2022 10:59 AM
> 
> Microsoft Hypervisor root partition has to map the TSC page specified
> by the hypervisor, instead of providing the page to the hypervisor like
> it's done in the guest partitions.
> 
> However, it's too early to map the page when the clock is initialized, so, the
> actual mapping is happening later.
> 
> Signed-off-by: Stanislav Kinsburskiy <stanislav.kinsburskiy@gmail.com>
> CC: "K. Y. Srinivasan" <kys@microsoft.com>
> CC: Haiyang Zhang <haiyangz@microsoft.com>
> CC: Wei Liu <wei.liu@kernel.org>
> CC: Dexuan Cui <decui@microsoft.com>
> CC: Thomas Gleixner <tglx@linutronix.de>
> CC: Ingo Molnar <mingo@redhat.com>
> CC: Borislav Petkov <bp@alien8.de>
> CC: Dave Hansen <dave.hansen@linux.intel.com>
> CC: x86@kernel.org
> CC: "H. Peter Anvin" <hpa@zytor.com>
> CC: Daniel Lezcano <daniel.lezcano@linaro.org>
> CC: linux-hyperv@vger.kernel.org
> CC: linux-kernel@vger.kernel.org
> ---
>  arch/x86/hyperv/hv_init.c          |    2 ++
>  drivers/clocksource/hyperv_timer.c |   38 +++++++++++++++++++++++++++---------
>  include/clocksource/hyperv_timer.h |    1 +
>  3 files changed, 32 insertions(+), 9 deletions(-)
> 
> diff --git a/arch/x86/hyperv/hv_init.c b/arch/x86/hyperv/hv_init.c
> index f49bc3ec76e6..89954490af93 100644
> --- a/arch/x86/hyperv/hv_init.c
> +++ b/arch/x86/hyperv/hv_init.c
> @@ -464,6 +464,8 @@ void __init hyperv_init(void)
>  		BUG_ON(!src);
>  		memcpy_to_page(pg, 0, src, HV_HYP_PAGE_SIZE);
>  		memunmap(src);
> +
> +		hv_remap_tsc_clocksource();
>  	} else {
>  		hypercall_msr.guest_physical_address =
> vmalloc_to_pfn(hv_hypercall_pg);
>  		wrmsrl(HV_X64_MSR_HYPERCALL, hypercall_msr.as_uint64);
> diff --git a/drivers/clocksource/hyperv_timer.c b/drivers/clocksource/hyperv_timer.c
> index 9445a1558fe9..dec7ad3b85ba 100644
> --- a/drivers/clocksource/hyperv_timer.c
> +++ b/drivers/clocksource/hyperv_timer.c
> @@ -509,9 +509,6 @@ static bool __init hv_init_tsc_clocksource(void)
>  	if (!(ms_hyperv.features & HV_MSR_REFERENCE_TSC_AVAILABLE))
>  		return false;
> 
> -	if (hv_root_partition)
> -		return false;
> -
>  	/*
>  	 * If Hyper-V offers TSC_INVARIANT, then the virtualized TSC correctly
>  	 * handles frequency and offset changes due to live migration,
> @@ -529,16 +526,22 @@ static bool __init hv_init_tsc_clocksource(void)
>  	}
> 
>  	hv_read_reference_counter = read_hv_clock_tsc;
> -	tsc_pfn = HVPFN_DOWN(virt_to_phys(tsc_page));
> 
>  	/*
> -	 * The Hyper-V TLFS specifies to preserve the value of reserved
> -	 * bits in registers. So read the existing value, preserve the
> -	 * low order 12 bits, and add in the guest physical address
> -	 * (which already has at least the low 12 bits set to zero since
> -	 * it is page aligned). Also set the "enable" bit, which is bit 0.
> +	 * TSC page mapping works differently in root compared to guest.
> +	 * - In guest partition the guest PFN has to be passed to the
> +	 *   hypervisor.
> +	 * - In root partition it's other way around: it has to map the PFN
> +	 *   provided by the hypervisor.
> +	 *   But it can't be mapped right here as it's too early and MMU isn't
> +	 *   ready yet. So, we only set the enable bit here and will remap the
> +	 *   page later in hv_remap_tsc_clocksource().
>  	 */
>  	tsc_msr.as_uint64 = hv_get_register(HV_REGISTER_REFERENCE_TSC);
> +	if (hv_root_partition)
> +		tsc_pfn = tsc_msr.pfn;
> +	else
> +		tsc_pfn = HVPFN_DOWN(virt_to_phys(tsc_page));
>  	tsc_msr.enable = 1;
>  	tsc_msr.pfn = tsc_pfn;
>  	hv_set_register(HV_REGISTER_REFERENCE_TSC, tsc_msr.as_uint64);

There's a subtlety here that was nagging me, and I think I see it now.

At this point, the code has enabled the Reference TSC, and if we're the root
partition, the Reference TSC Page is the page supplied by the hypervisor.
tsc_pfn has been updated to reflect that hypervisor supplied page.

But tsc_page has not been updated to be in sync with tsc_pfn because we
can't do the memremap() here.  tsc_page still points to tsc_pg, which is a
global variable in Linux.  tsc_page and tsc_pfn will be out-of- sync until
hv_remap_tsc_clocksource() is called later in the boot process.  During
this interval, calls to get the Hyper-V Reference TSC value will use tsc_pg,
not on the Reference TSC Page that the hypervisor is using.  Fortunately,
the function hv_read_tsc_page_tsc(), which actually reads the Reference
TSC Page, treats a zero value for tsc_sequence as a special case meaning
that the Reference TSC page isn't valid.  read_hv_clock_tsc() then falls
back to reading a hypervisor provided synthetic MSR to get the correct
Reference TSC value.  That fallback is fine -- it's just slower because it
traps to the hypervisor.  And the fallback will no longer be used once 
tsc_page is updated by hv_remap_tsc_clocksource().

So the code works. Presumably this subtlety was already understood, but
it really should be called out in a comment, as it is far from obvious.  I
know this code pretty well and I just figured it out. :-(

Michael

> @@ -573,3 +576,20 @@ void __init hv_init_clocksource(void)
>  	hv_sched_clock_offset = hv_read_reference_counter();
>  	hv_setup_sched_clock(read_hv_sched_clock_msr);
>  }
> +
> +void __init hv_remap_tsc_clocksource(void)
> +{
> +	if (!(ms_hyperv.features & HV_MSR_REFERENCE_TSC_AVAILABLE))
> +		return;
> +
> +	if (!hv_root_partition) {
> +		WARN(1, "%s: attempt to remap TSC page in guest partition\n",
> +		     __func__);
> +		return;
> +	}
> +
> +	tsc_page = memremap(tsc_pfn << HV_HYP_PAGE_SHIFT, sizeof(tsc_pg),
> +			    MEMREMAP_WB);
> +	if (!tsc_page)
> +		pr_err("Failed to remap Hyper-V TSC page.\n");
> +}
> diff --git a/include/clocksource/hyperv_timer.h
> b/include/clocksource/hyperv_timer.h
> index 3078d23faaea..783701a2102d 100644
> --- a/include/clocksource/hyperv_timer.h
> +++ b/include/clocksource/hyperv_timer.h
> @@ -31,6 +31,7 @@ extern void hv_stimer_global_cleanup(void);
>  extern void hv_stimer0_isr(void);
> 
>  extern void hv_init_clocksource(void);
> +extern void hv_remap_tsc_clocksource(void);
> 
>  extern unsigned long hv_get_tsc_pfn(void);
>  extern struct ms_hyperv_tsc_page *hv_get_tsc_page(void);
> 

Re: [PATCH v3 4/4] drivers/clocksource/hyper-v: Add TSC page support for root partition

[Stanislav Kinsburskii]

On Thu, Nov 03, 2022 at 08:33:40PM +0000, Michael Kelley (LINUX) wrote:
> From: Stanislav Kinsburskii <skinsburskii@linux.microsoft.com> Sent: Thursday, November 3, 2022 10:59 AM
> > 
> > Microsoft Hypervisor root partition has to map the TSC page specified
> > by the hypervisor, instead of providing the page to the hypervisor like
> > it's done in the guest partitions.
> > 
> > However, it's too early to map the page when the clock is initialized, so, the
> > actual mapping is happening later.
> > 
> > Signed-off-by: Stanislav Kinsburskiy <stanislav.kinsburskiy@gmail.com>
> > CC: "K. Y. Srinivasan" <kys@microsoft.com>
> > CC: Haiyang Zhang <haiyangz@microsoft.com>
> > CC: Wei Liu <wei.liu@kernel.org>
> > CC: Dexuan Cui <decui@microsoft.com>
> > CC: Thomas Gleixner <tglx@linutronix.de>
> > CC: Ingo Molnar <mingo@redhat.com>
> > CC: Borislav Petkov <bp@alien8.de>
> > CC: Dave Hansen <dave.hansen@linux.intel.com>
> > CC: x86@kernel.org
> > CC: "H. Peter Anvin" <hpa@zytor.com>
> > CC: Daniel Lezcano <daniel.lezcano@linaro.org>
> > CC: linux-hyperv@vger.kernel.org
> > CC: linux-kernel@vger.kernel.org
> > ---
> >  arch/x86/hyperv/hv_init.c          |    2 ++
> >  drivers/clocksource/hyperv_timer.c |   38 +++++++++++++++++++++++++++---------
> >  include/clocksource/hyperv_timer.h |    1 +
> >  3 files changed, 32 insertions(+), 9 deletions(-)
> > 
> > diff --git a/arch/x86/hyperv/hv_init.c b/arch/x86/hyperv/hv_init.c
> > index f49bc3ec76e6..89954490af93 100644
> > --- a/arch/x86/hyperv/hv_init.c
> > +++ b/arch/x86/hyperv/hv_init.c
> > @@ -464,6 +464,8 @@ void __init hyperv_init(void)
> >  		BUG_ON(!src);
> >  		memcpy_to_page(pg, 0, src, HV_HYP_PAGE_SIZE);
> >  		memunmap(src);
> > +
> > +		hv_remap_tsc_clocksource();
> >  	} else {
> >  		hypercall_msr.guest_physical_address =
> > vmalloc_to_pfn(hv_hypercall_pg);
> >  		wrmsrl(HV_X64_MSR_HYPERCALL, hypercall_msr.as_uint64);
> > diff --git a/drivers/clocksource/hyperv_timer.c b/drivers/clocksource/hyperv_timer.c
> > index 9445a1558fe9..dec7ad3b85ba 100644
> > --- a/drivers/clocksource/hyperv_timer.c
> > +++ b/drivers/clocksource/hyperv_timer.c
> > @@ -509,9 +509,6 @@ static bool __init hv_init_tsc_clocksource(void)
> >  	if (!(ms_hyperv.features & HV_MSR_REFERENCE_TSC_AVAILABLE))
> >  		return false;
> > 
> > -	if (hv_root_partition)
> > -		return false;
> > -
> >  	/*
> >  	 * If Hyper-V offers TSC_INVARIANT, then the virtualized TSC correctly
> >  	 * handles frequency and offset changes due to live migration,
> > @@ -529,16 +526,22 @@ static bool __init hv_init_tsc_clocksource(void)
> >  	}
> > 
> >  	hv_read_reference_counter = read_hv_clock_tsc;
> > -	tsc_pfn = HVPFN_DOWN(virt_to_phys(tsc_page));
> > 
> >  	/*
> > -	 * The Hyper-V TLFS specifies to preserve the value of reserved
> > -	 * bits in registers. So read the existing value, preserve the
> > -	 * low order 12 bits, and add in the guest physical address
> > -	 * (which already has at least the low 12 bits set to zero since
> > -	 * it is page aligned). Also set the "enable" bit, which is bit 0.
> > +	 * TSC page mapping works differently in root compared to guest.
> > +	 * - In guest partition the guest PFN has to be passed to the
> > +	 *   hypervisor.
> > +	 * - In root partition it's other way around: it has to map the PFN
> > +	 *   provided by the hypervisor.
> > +	 *   But it can't be mapped right here as it's too early and MMU isn't
> > +	 *   ready yet. So, we only set the enable bit here and will remap the
> > +	 *   page later in hv_remap_tsc_clocksource().
> >  	 */
> >  	tsc_msr.as_uint64 = hv_get_register(HV_REGISTER_REFERENCE_TSC);
> > +	if (hv_root_partition)
> > +		tsc_pfn = tsc_msr.pfn;
> > +	else
> > +		tsc_pfn = HVPFN_DOWN(virt_to_phys(tsc_page));
> >  	tsc_msr.enable = 1;
> >  	tsc_msr.pfn = tsc_pfn;
> >  	hv_set_register(HV_REGISTER_REFERENCE_TSC, tsc_msr.as_uint64);
> 
> There's a subtlety here that was nagging me, and I think I see it now.
> 
> At this point, the code has enabled the Reference TSC, and if we're the root
> partition, the Reference TSC Page is the page supplied by the hypervisor.
> tsc_pfn has been updated to reflect that hypervisor supplied page.
> 
> But tsc_page has not been updated to be in sync with tsc_pfn because we
> can't do the memremap() here.  tsc_page still points to tsc_pg, which is a
> global variable in Linux.  tsc_page and tsc_pfn will be out-of- sync until
> hv_remap_tsc_clocksource() is called later in the boot process.  During
> this interval, calls to get the Hyper-V Reference TSC value will use tsc_pg,
> not on the Reference TSC Page that the hypervisor is using.  Fortunately,
> the function hv_read_tsc_page_tsc(), which actually reads the Reference
> TSC Page, treats a zero value for tsc_sequence as a special case meaning
> that the Reference TSC page isn't valid.  read_hv_clock_tsc() then falls
> back to reading a hypervisor provided synthetic MSR to get the correct
> Reference TSC value.  That fallback is fine -- it's just slower because it
> traps to the hypervisor.  And the fallback will no longer be used once 
> tsc_page is updated by hv_remap_tsc_clocksource().
> 
> So the code works. Presumably this subtlety was already understood, but
> it really should be called out in a comment, as it is far from obvious.  I
> know this code pretty well and I just figured it out. :-(
> 

You are absolutely right in everything above.
Moreover, this imlementation will update the tsc_pfn early and will keep
it the same regardless of the result of the memremap call in
hv_remap_tsc_clocksource().

This in turn can lead to an interesting (although quite unprobable)
situation: kernel fails to remap TSC page (and thus use MSR registers as
fallback), while user space process can successfully map the TSC page
and use it instead.

The code can be changed to be, I'd say, more evident (by assigning
tsc_pfn to the hypervisor PFN only if remapping succeede), but the current
implementation is the most efficient from the performance point of view,
so I'd keep it as is (even so it's not very obvious).

Stas

> Michael
> 

RE: [PATCH v3 4/4] drivers/clocksource/hyper-v: Add TSC page support for root partition

[Michael Kelley (LINUX)]

From: Stanislav Kinsburskii <skinsburskii@linux.microsoft.com> Sent: Wednesday, November 2, 2022 6:37 PM
> 
> On Thu, Nov 03, 2022 at 08:33:40PM +0000, Michael Kelley (LINUX) wrote:
> > From: Stanislav Kinsburskii <skinsburskii@linux.microsoft.com> Sent: Thursday,
> November 3, 2022 10:59 AM
> > >
> > > Microsoft Hypervisor root partition has to map the TSC page specified
> > > by the hypervisor, instead of providing the page to the hypervisor like
> > > it's done in the guest partitions.
> > >
> > > However, it's too early to map the page when the clock is initialized, so, the
> > > actual mapping is happening later.
> > >
> > > Signed-off-by: Stanislav Kinsburskiy <stanislav.kinsburskiy@gmail.com>
> > > CC: "K. Y. Srinivasan" <kys@microsoft.com>
> > > CC: Haiyang Zhang <haiyangz@microsoft.com>
> > > CC: Wei Liu <wei.liu@kernel.org>
> > > CC: Dexuan Cui <decui@microsoft.com>
> > > CC: Thomas Gleixner <tglx@linutronix.de>
> > > CC: Ingo Molnar <mingo@redhat.com>
> > > CC: Borislav Petkov <bp@alien8.de>
> > > CC: Dave Hansen <dave.hansen@linux.intel.com>
> > > CC: x86@kernel.org
> > > CC: "H. Peter Anvin" <hpa@zytor.com>
> > > CC: Daniel Lezcano <daniel.lezcano@linaro.org>
> > > CC: linux-hyperv@vger.kernel.org
> > > CC: linux-kernel@vger.kernel.org
> > > ---
> > >  arch/x86/hyperv/hv_init.c          |    2 ++
> > >  drivers/clocksource/hyperv_timer.c |   38 +++++++++++++++++++++++++++------
> ---
> > >  include/clocksource/hyperv_timer.h |    1 +
> > >  3 files changed, 32 insertions(+), 9 deletions(-)
> > >
> > > diff --git a/arch/x86/hyperv/hv_init.c b/arch/x86/hyperv/hv_init.c
> > > index f49bc3ec76e6..89954490af93 100644
> > > --- a/arch/x86/hyperv/hv_init.c
> > > +++ b/arch/x86/hyperv/hv_init.c
> > > @@ -464,6 +464,8 @@ void __init hyperv_init(void)
> > >  		BUG_ON(!src);
> > >  		memcpy_to_page(pg, 0, src, HV_HYP_PAGE_SIZE);
> > >  		memunmap(src);
> > > +
> > > +		hv_remap_tsc_clocksource();
> > >  	} else {
> > >  		hypercall_msr.guest_physical_address =
> > > vmalloc_to_pfn(hv_hypercall_pg);
> > >  		wrmsrl(HV_X64_MSR_HYPERCALL, hypercall_msr.as_uint64);
> > > diff --git a/drivers/clocksource/hyperv_timer.c
> b/drivers/clocksource/hyperv_timer.c
> > > index 9445a1558fe9..dec7ad3b85ba 100644
> > > --- a/drivers/clocksource/hyperv_timer.c
> > > +++ b/drivers/clocksource/hyperv_timer.c
> > > @@ -509,9 +509,6 @@ static bool __init hv_init_tsc_clocksource(void)
> > >  	if (!(ms_hyperv.features & HV_MSR_REFERENCE_TSC_AVAILABLE))
> > >  		return false;
> > >
> > > -	if (hv_root_partition)
> > > -		return false;
> > > -
> > >  	/*
> > >  	 * If Hyper-V offers TSC_INVARIANT, then the virtualized TSC correctly
> > >  	 * handles frequency and offset changes due to live migration,
> > > @@ -529,16 +526,22 @@ static bool __init hv_init_tsc_clocksource(void)
> > >  	}
> > >
> > >  	hv_read_reference_counter = read_hv_clock_tsc;
> > > -	tsc_pfn = HVPFN_DOWN(virt_to_phys(tsc_page));
> > >
> > >  	/*
> > > -	 * The Hyper-V TLFS specifies to preserve the value of reserved
> > > -	 * bits in registers. So read the existing value, preserve the
> > > -	 * low order 12 bits, and add in the guest physical address
> > > -	 * (which already has at least the low 12 bits set to zero since
> > > -	 * it is page aligned). Also set the "enable" bit, which is bit 0.
> > > +	 * TSC page mapping works differently in root compared to guest.
> > > +	 * - In guest partition the guest PFN has to be passed to the
> > > +	 *   hypervisor.
> > > +	 * - In root partition it's other way around: it has to map the PFN
> > > +	 *   provided by the hypervisor.
> > > +	 *   But it can't be mapped right here as it's too early and MMU isn't
> > > +	 *   ready yet. So, we only set the enable bit here and will remap the
> > > +	 *   page later in hv_remap_tsc_clocksource().
> > >  	 */
> > >  	tsc_msr.as_uint64 = hv_get_register(HV_REGISTER_REFERENCE_TSC);
> > > +	if (hv_root_partition)
> > > +		tsc_pfn = tsc_msr.pfn;
> > > +	else
> > > +		tsc_pfn = HVPFN_DOWN(virt_to_phys(tsc_page));
> > >  	tsc_msr.enable = 1;
> > >  	tsc_msr.pfn = tsc_pfn;
> > >  	hv_set_register(HV_REGISTER_REFERENCE_TSC, tsc_msr.as_uint64);
> >
> > There's a subtlety here that was nagging me, and I think I see it now.
> >
> > At this point, the code has enabled the Reference TSC, and if we're the root
> > partition, the Reference TSC Page is the page supplied by the hypervisor.
> > tsc_pfn has been updated to reflect that hypervisor supplied page.
> >
> > But tsc_page has not been updated to be in sync with tsc_pfn because we
> > can't do the memremap() here.  tsc_page still points to tsc_pg, which is a
> > global variable in Linux.  tsc_page and tsc_pfn will be out-of- sync until
> > hv_remap_tsc_clocksource() is called later in the boot process.  During
> > this interval, calls to get the Hyper-V Reference TSC value will use tsc_pg,
> > not on the Reference TSC Page that the hypervisor is using.  Fortunately,
> > the function hv_read_tsc_page_tsc(), which actually reads the Reference
> > TSC Page, treats a zero value for tsc_sequence as a special case meaning
> > that the Reference TSC page isn't valid.  read_hv_clock_tsc() then falls
> > back to reading a hypervisor provided synthetic MSR to get the correct
> > Reference TSC value.  That fallback is fine -- it's just slower because it
> > traps to the hypervisor.  And the fallback will no longer be used once
> > tsc_page is updated by hv_remap_tsc_clocksource().
> >
> > So the code works. Presumably this subtlety was already understood, but
> > it really should be called out in a comment, as it is far from obvious.  I
> > know this code pretty well and I just figured it out. :-(
> >
> 
> You are absolutely right in everything above.
> Moreover, this imlementation will update the tsc_pfn early and will keep
> it the same regardless of the result of the memremap call in
> hv_remap_tsc_clocksource().
> 
> This in turn can lead to an interesting (although quite unprobable)
> situation: kernel fails to remap TSC page (and thus use MSR registers as
> fallback), while user space process can successfully map the TSC page
> and use it instead.

I'm not really worried about this scenario.  If the remap fails,  there's
a broader problem somewhere and the VM isn't likely to live long.

> 
> The code can be changed to be, I'd say, more evident (by assigning
> tsc_pfn to the hypervisor PFN only if remapping succeede), but the current
> implementation is the most efficient from the performance point of view,
> so I'd keep it as is (even so it's not very obvious).
> 

I'm good with the code in your patch in its current form.  But add a
comment in the code (maybe where tsc_pfn is set) explaining what's going
on and that correct operation is dependent on the empty TSC page being
treated as invalid so that the fallback to the MSR occurs.  The next new
person who looks at this code will thank you. :-)  Then I'll give my
"Reviewed-by:".

Michael

[PATCH v4] drivers/clocksource/hyper-v: Add TSC page support for root partition

[Stanislav Kinsburskii]

From: Stanislav Kinsburskiy <stanislav.kinsburskiy@gmail.com>

Microsoft Hypervisor root partition has to map the TSC page specified
by the hypervisor, instead of providing the page to the hypervisor like
it's done in the guest partitions.

However, it's too early to map the page when the clock is initialized, so, the
actual mapping is happening later.

Signed-off-by: Stanislav Kinsburskiy <stanislav.kinsburskiy@gmail.com>
CC: "K. Y. Srinivasan" <kys@microsoft.com>
CC: Haiyang Zhang <haiyangz@microsoft.com>
CC: Wei Liu <wei.liu@kernel.org>
CC: Dexuan Cui <decui@microsoft.com>
CC: Thomas Gleixner <tglx@linutronix.de>
CC: Ingo Molnar <mingo@redhat.com>
CC: Borislav Petkov <bp@alien8.de>
CC: Dave Hansen <dave.hansen@linux.intel.com>
CC: x86@kernel.org
CC: "H. Peter Anvin" <hpa@zytor.com>
CC: Daniel Lezcano <daniel.lezcano@linaro.org>
CC: linux-hyperv@vger.kernel.org
CC: linux-kernel@vger.kernel.org
---
 arch/x86/hyperv/hv_init.c          |    2 ++
 drivers/clocksource/hyperv_timer.c |   44 +++++++++++++++++++++++++++++-------
 include/clocksource/hyperv_timer.h |    1 +
 3 files changed, 38 insertions(+), 9 deletions(-)

diff --git a/arch/x86/hyperv/hv_init.c b/arch/x86/hyperv/hv_init.c
index f49bc3ec76e6..89954490af93 100644
--- a/arch/x86/hyperv/hv_init.c
+++ b/arch/x86/hyperv/hv_init.c
@@ -464,6 +464,8 @@ void __init hyperv_init(void)
 		BUG_ON(!src);
 		memcpy_to_page(pg, 0, src, HV_HYP_PAGE_SIZE);
 		memunmap(src);
+
+		hv_remap_tsc_clocksource();
 	} else {
 		hypercall_msr.guest_physical_address = vmalloc_to_pfn(hv_hypercall_pg);
 		wrmsrl(HV_X64_MSR_HYPERCALL, hypercall_msr.as_uint64);
diff --git a/drivers/clocksource/hyperv_timer.c b/drivers/clocksource/hyperv_timer.c
index 9445a1558fe9..c0cef92b12b8 100644
--- a/drivers/clocksource/hyperv_timer.c
+++ b/drivers/clocksource/hyperv_timer.c
@@ -509,9 +509,6 @@ static bool __init hv_init_tsc_clocksource(void)
 	if (!(ms_hyperv.features & HV_MSR_REFERENCE_TSC_AVAILABLE))
 		return false;
 
-	if (hv_root_partition)
-		return false;
-
 	/*
 	 * If Hyper-V offers TSC_INVARIANT, then the virtualized TSC correctly
 	 * handles frequency and offset changes due to live migration,
@@ -529,16 +526,28 @@ static bool __init hv_init_tsc_clocksource(void)
 	}
 
 	hv_read_reference_counter = read_hv_clock_tsc;
-	tsc_pfn = HVPFN_DOWN(virt_to_phys(tsc_page));
 
 	/*
-	 * The Hyper-V TLFS specifies to preserve the value of reserved
-	 * bits in registers. So read the existing value, preserve the
-	 * low order 12 bits, and add in the guest physical address
-	 * (which already has at least the low 12 bits set to zero since
-	 * it is page aligned). Also set the "enable" bit, which is bit 0.
+	 * TSC page mapping works differently in root compared to guest.
+	 * - In guest partition the guest PFN has to be passed to the
+	 *   hypervisor.
+	 * - In root partition it's other way around: it has to map the PFN
+	 *   provided by the hypervisor.
+	 *   But it can't be mapped right here as it's too early and MMU isn't
+	 *   ready yet. So, we only set the enable bit here and will remap the
+	 *   page later in hv_remap_tsc_clocksource().
+	 *
+	 * It worth mentioning, that TSC clocksource read function
+	 * (read_hv_clock_tsc) has a MSR-based fallback mechanism, used when
+	 * TSC page is zeroed (which is the case until the PFN is remapped) and
+	 * thus TSC clocksource will work even without the real TSC page
+	 * mapped.
 	 */
 	tsc_msr.as_uint64 = hv_get_register(HV_REGISTER_REFERENCE_TSC);
+	if (hv_root_partition)
+		tsc_pfn = tsc_msr.pfn;
+	else
+		tsc_pfn = HVPFN_DOWN(virt_to_phys(tsc_page));
 	tsc_msr.enable = 1;
 	tsc_msr.pfn = tsc_pfn;
 	hv_set_register(HV_REGISTER_REFERENCE_TSC, tsc_msr.as_uint64);
@@ -573,3 +582,20 @@ void __init hv_init_clocksource(void)
 	hv_sched_clock_offset = hv_read_reference_counter();
 	hv_setup_sched_clock(read_hv_sched_clock_msr);
 }
+
+void __init hv_remap_tsc_clocksource(void)
+{
+	if (!(ms_hyperv.features & HV_MSR_REFERENCE_TSC_AVAILABLE))
+		return;
+
+	if (!hv_root_partition) {
+		WARN(1, "%s: attempt to remap TSC page in guest partition\n",
+		     __func__);
+		return;
+	}
+
+	tsc_page = memremap(tsc_pfn << HV_HYP_PAGE_SHIFT, sizeof(tsc_pg),
+			    MEMREMAP_WB);
+	if (!tsc_page)
+		pr_err("Failed to remap Hyper-V TSC page.\n");
+}
diff --git a/include/clocksource/hyperv_timer.h b/include/clocksource/hyperv_timer.h
index 3078d23faaea..783701a2102d 100644
--- a/include/clocksource/hyperv_timer.h
+++ b/include/clocksource/hyperv_timer.h
@@ -31,6 +31,7 @@ extern void hv_stimer_global_cleanup(void);
 extern void hv_stimer0_isr(void);
 
 extern void hv_init_clocksource(void);
+extern void hv_remap_tsc_clocksource(void);
 
 extern unsigned long hv_get_tsc_pfn(void);
 extern struct ms_hyperv_tsc_page *hv_get_tsc_page(void);

RE: [PATCH v4] drivers/clocksource/hyper-v: Add TSC page support for root partition

[Michael Kelley (LINUX)]

From: Stanislav Kinsburskii <skinsburskii@linux.microsoft.com> Sent: Friday, November 4, 2022 1:41 PM
> 
> Microsoft Hypervisor root partition has to map the TSC page specified
> by the hypervisor, instead of providing the page to the hypervisor like
> it's done in the guest partitions.
> 
> However, it's too early to map the page when the clock is initialized, so, the
> actual mapping is happening later.
> 
> Signed-off-by: Stanislav Kinsburskiy <stanislav.kinsburskiy@gmail.com>
> CC: "K. Y. Srinivasan" <kys@microsoft.com>
> CC: Haiyang Zhang <haiyangz@microsoft.com>
> CC: Wei Liu <wei.liu@kernel.org>
> CC: Dexuan Cui <decui@microsoft.com>
> CC: Thomas Gleixner <tglx@linutronix.de>
> CC: Ingo Molnar <mingo@redhat.com>
> CC: Borislav Petkov <bp@alien8.de>
> CC: Dave Hansen <dave.hansen@linux.intel.com>
> CC: x86@kernel.org
> CC: "H. Peter Anvin" <hpa@zytor.com>
> CC: Daniel Lezcano <daniel.lezcano@linaro.org>
> CC: linux-hyperv@vger.kernel.org
> CC: linux-kernel@vger.kernel.org
> ---
>  arch/x86/hyperv/hv_init.c          |    2 ++
>  drivers/clocksource/hyperv_timer.c |   44 +++++++++++++++++++++++++++++-------
>  include/clocksource/hyperv_timer.h |    1 +
>  3 files changed, 38 insertions(+), 9 deletions(-)
> 
> diff --git a/arch/x86/hyperv/hv_init.c b/arch/x86/hyperv/hv_init.c
> index f49bc3ec76e6..89954490af93 100644
> --- a/arch/x86/hyperv/hv_init.c
> +++ b/arch/x86/hyperv/hv_init.c
> @@ -464,6 +464,8 @@ void __init hyperv_init(void)
>  		BUG_ON(!src);
>  		memcpy_to_page(pg, 0, src, HV_HYP_PAGE_SIZE);
>  		memunmap(src);
> +
> +		hv_remap_tsc_clocksource();
>  	} else {
>  		hypercall_msr.guest_physical_address =
> vmalloc_to_pfn(hv_hypercall_pg);
>  		wrmsrl(HV_X64_MSR_HYPERCALL, hypercall_msr.as_uint64);
> diff --git a/drivers/clocksource/hyperv_timer.c b/drivers/clocksource/hyperv_timer.c
> index 9445a1558fe9..c0cef92b12b8 100644
> --- a/drivers/clocksource/hyperv_timer.c
> +++ b/drivers/clocksource/hyperv_timer.c
> @@ -509,9 +509,6 @@ static bool __init hv_init_tsc_clocksource(void)
>  	if (!(ms_hyperv.features & HV_MSR_REFERENCE_TSC_AVAILABLE))
>  		return false;
> 
> -	if (hv_root_partition)
> -		return false;
> -
>  	/*
>  	 * If Hyper-V offers TSC_INVARIANT, then the virtualized TSC correctly
>  	 * handles frequency and offset changes due to live migration,
> @@ -529,16 +526,28 @@ static bool __init hv_init_tsc_clocksource(void)
>  	}
> 
>  	hv_read_reference_counter = read_hv_clock_tsc;
> -	tsc_pfn = HVPFN_DOWN(virt_to_phys(tsc_page));
> 
>  	/*
> -	 * The Hyper-V TLFS specifies to preserve the value of reserved
> -	 * bits in registers. So read the existing value, preserve the
> -	 * low order 12 bits, and add in the guest physical address
> -	 * (which already has at least the low 12 bits set to zero since
> -	 * it is page aligned). Also set the "enable" bit, which is bit 0.
> +	 * TSC page mapping works differently in root compared to guest.
> +	 * - In guest partition the guest PFN has to be passed to the
> +	 *   hypervisor.
> +	 * - In root partition it's other way around: it has to map the PFN
> +	 *   provided by the hypervisor.
> +	 *   But it can't be mapped right here as it's too early and MMU isn't
> +	 *   ready yet. So, we only set the enable bit here and will remap the
> +	 *   page later in hv_remap_tsc_clocksource().
> +	 *
> +	 * It worth mentioning, that TSC clocksource read function
> +	 * (read_hv_clock_tsc) has a MSR-based fallback mechanism, used when
> +	 * TSC page is zeroed (which is the case until the PFN is remapped) and
> +	 * thus TSC clocksource will work even without the real TSC page
> +	 * mapped.
>  	 */
>  	tsc_msr.as_uint64 = hv_get_register(HV_REGISTER_REFERENCE_TSC);
> +	if (hv_root_partition)
> +		tsc_pfn = tsc_msr.pfn;
> +	else
> +		tsc_pfn = HVPFN_DOWN(virt_to_phys(tsc_page));
>  	tsc_msr.enable = 1;
>  	tsc_msr.pfn = tsc_pfn;
>  	hv_set_register(HV_REGISTER_REFERENCE_TSC, tsc_msr.as_uint64);
> @@ -573,3 +582,20 @@ void __init hv_init_clocksource(void)
>  	hv_sched_clock_offset = hv_read_reference_counter();
>  	hv_setup_sched_clock(read_hv_sched_clock_msr);
>  }
> +
> +void __init hv_remap_tsc_clocksource(void)
> +{
> +	if (!(ms_hyperv.features & HV_MSR_REFERENCE_TSC_AVAILABLE))
> +		return;
> +
> +	if (!hv_root_partition) {
> +		WARN(1, "%s: attempt to remap TSC page in guest partition\n",
> +		     __func__);
> +		return;
> +	}
> +
> +	tsc_page = memremap(tsc_pfn << HV_HYP_PAGE_SHIFT, sizeof(tsc_pg),
> +			    MEMREMAP_WB);
> +	if (!tsc_page)
> +		pr_err("Failed to remap Hyper-V TSC page.\n");
> +}
> diff --git a/include/clocksource/hyperv_timer.h
> b/include/clocksource/hyperv_timer.h
> index 3078d23faaea..783701a2102d 100644
> --- a/include/clocksource/hyperv_timer.h
> +++ b/include/clocksource/hyperv_timer.h
> @@ -31,6 +31,7 @@ extern void hv_stimer_global_cleanup(void);
>  extern void hv_stimer0_isr(void);
> 
>  extern void hv_init_clocksource(void);
> +extern void hv_remap_tsc_clocksource(void);
> 
>  extern unsigned long hv_get_tsc_pfn(void);
>  extern struct ms_hyperv_tsc_page *hv_get_tsc_page(void);
> 

Reviewed-by: Michael Kelley <mikelley@microsoft.com>

Re: [PATCH v3 0/4] hyper-v: Introduce TSC page for root partition

[Wei Liu]

On Thu, Nov 03, 2022 at 05:58:43PM +0000, Stanislav Kinsburskii wrote:
> 
> Stanislav Kinsburskiy (4):
>       drivers/clocksource/hyper-v: Introduce a pointer to TSC page
>       drivers/clocksource/hyper-v: Introduce TSC PFN getter
>       drivers/clocksource/hyper-v: Use TSC PFN getter to map vvar page
>       drivers/clocksource/hyper-v: Add TSC page support for root partition

Applied to hyeprv-next. Thanks.

Re: [PATCH v3 0/4] hyper-v: Introduce TSC page for root partition

[Daniel Lezcano]

On 14/11/2022 14:54, Wei Liu wrote:
> On Thu, Nov 03, 2022 at 05:58:43PM +0000, Stanislav Kinsburskii wrote:
>>
>> Stanislav Kinsburskiy (4):
>>        drivers/clocksource/hyper-v: Introduce a pointer to TSC page
>>        drivers/clocksource/hyper-v: Introduce TSC PFN getter
>>        drivers/clocksource/hyper-v: Use TSC PFN getter to map vvar page
>>        drivers/clocksource/hyper-v: Add TSC page support for root partition
> 
> Applied to hyeprv-next. Thanks.

The series should have go through the clocksource/timer tree or at least 
should have specified the targeted tree for an Acked-by from the 
clocksource maintainers.

In the future, please follow the process

That said,

Acked-by: Daniel Lezcano <daniel.lezcano@linaro.org>

-- 
<http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs

Follow Linaro:  <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog

Re: [PATCH v3 0/4] hyper-v: Introduce TSC page for root partition

[Stanislav Kinsburskii]

On Fri, Dec 02, 2022 at 01:55:35PM +0100, Daniel Lezcano wrote:
> On 14/11/2022 14:54, Wei Liu wrote:
> > On Thu, Nov 03, 2022 at 05:58:43PM +0000, Stanislav Kinsburskii wrote:
> > > 
> > > Stanislav Kinsburskiy (4):
> > >        drivers/clocksource/hyper-v: Introduce a pointer to TSC page
> > >        drivers/clocksource/hyper-v: Introduce TSC PFN getter
> > >        drivers/clocksource/hyper-v: Use TSC PFN getter to map vvar page
> > >        drivers/clocksource/hyper-v: Add TSC page support for root partition
> > 
> > Applied to hyeprv-next. Thanks.
> 
> The series should have go through the clocksource/timer tree or at least
> should have specified the targeted tree for an Acked-by from the clocksource
> maintainers.
> 
> In the future, please follow the process
> 

I will.
Thank you for your feedback and aknowledgement, Daniel.

> That said,
> 
> Acked-by: Daniel Lezcano <daniel.lezcano@linaro.org>
> 
> -- 
> <http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs
> 
> Follow Linaro:  <http://www.facebook.com/pages/Linaro> Facebook |
> <http://twitter.com/#!/linaroorg> Twitter |
> <http://www.linaro.org/linaro-blog/> Blog

Re: [PATCH v3 0/4] hyper-v: Introduce TSC page for root partition

[Wei Liu]

On Fri, Dec 02, 2022 at 01:55:35PM +0100, Daniel Lezcano wrote:
> On 14/11/2022 14:54, Wei Liu wrote:
> > On Thu, Nov 03, 2022 at 05:58:43PM +0000, Stanislav Kinsburskii wrote:
> > > 
> > > Stanislav Kinsburskiy (4):
> > >        drivers/clocksource/hyper-v: Introduce a pointer to TSC page
> > >        drivers/clocksource/hyper-v: Introduce TSC PFN getter
> > >        drivers/clocksource/hyper-v: Use TSC PFN getter to map vvar page
> > >        drivers/clocksource/hyper-v: Add TSC page support for root partition
> > 
> > Applied to hyeprv-next. Thanks.
> 
> The series should have go through the clocksource/timer tree or at least
> should have specified the targeted tree for an Acked-by from the clocksource
> maintainers.
> 
> In the future, please follow the process

Sure thing.

> 
> That said,
> 
> Acked-by: Daniel Lezcano <daniel.lezcano@linaro.org>

Thank you very much!

Wei.