2020-09-14 11:45:06

by Wei Liu

Subject: [PATCH RFC v1 05/18] clocksource/hyperv: use MSR-based access if running as root

Signed-off-by: Wei Liu <[email protected]>
---
drivers/clocksource/hyperv_timer.c | 3 +++
1 file changed, 3 insertions(+)

diff --git a/drivers/clocksource/hyperv_timer.c b/drivers/clocksource/hyperv_timer.c
index 09aa44cb8a91..fe96082ce85e 100644
--- a/drivers/clocksource/hyperv_timer.c
+++ b/drivers/clocksource/hyperv_timer.c
@@ -426,6 +426,9 @@ 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;
+
hv_read_reference_counter = read_hv_clock_tsc;
phys_addr = virt_to_phys(hv_get_tsc_page());

--
2.20.1

2020-09-15 10:13:23

by Vitaly Kuznetsov

Subject: Re: [PATCH RFC v1 05/18] clocksource/hyperv: use MSR-based access if running as root

Wei Liu <[email protected]> writes:

> Signed-off-by: Wei Liu <[email protected]>
> ---
> drivers/clocksource/hyperv_timer.c | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/drivers/clocksource/hyperv_timer.c b/drivers/clocksource/hyperv_timer.c
> index 09aa44cb8a91..fe96082ce85e 100644
> --- a/drivers/clocksource/hyperv_timer.c
> +++ b/drivers/clocksource/hyperv_timer.c
> @@ -426,6 +426,9 @@ 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;
> +

Out of pure curiosity,

TSC page clocksource seems to be available to the root partition (as
HV_MSR_REFERENCE_TSC_AVAILABLE is set), why don't we use it? (I
understand that with TSC scaling support in modern CPUs even migration
is a no-issue and we can use raw TSC but this all seems to be
independent from root/non-root partition question).

> hv_read_reference_counter = read_hv_clock_tsc;
> phys_addr = virt_to_phys(hv_get_tsc_page());

--
Vitaly

2020-09-15 10:35:08

by Wei Liu

Subject: Re: [PATCH RFC v1 05/18] clocksource/hyperv: use MSR-based access if running as root

On Tue, Sep 15, 2020 at 12:10:04PM +0200, Vitaly Kuznetsov wrote:
> Wei Liu <[email protected]> writes:
>
> > Signed-off-by: Wei Liu <[email protected]>
> > ---
> > drivers/clocksource/hyperv_timer.c | 3 +++
> > 1 file changed, 3 insertions(+)
> >
> > diff --git a/drivers/clocksource/hyperv_timer.c b/drivers/clocksource/hyperv_timer.c
> > index 09aa44cb8a91..fe96082ce85e 100644
> > --- a/drivers/clocksource/hyperv_timer.c
> > +++ b/drivers/clocksource/hyperv_timer.c
> > @@ -426,6 +426,9 @@ 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;
> > +
>
> Out of pure curiosity,
>
> TSC page clocksource seems to be available to the root partition (as
> HV_MSR_REFERENCE_TSC_AVAILABLE is set), why don't we use it? (I
> understand that with TSC scaling support in modern CPUs even migration
> is a no-issue and we can use raw TSC but this all seems to be
> independent from root/non-root partition question).
>

Yes. It is available to the root partition. An earlier version of this
patch did use that. With some major restructuring of the code between
4.19 and 5.6 (splitting out the clocksource to and getting initialized
earlier), the old code wouldn't work anymore.

The catch is that the page will have been set up by the hypervisor, so
in the function we will need to take a different path. And making that
work again on 5.8 requires a bit more code churn.

Like I said in the cover letter, this is the bare minimum to get things
going. I try not distract people too much with less important stuff at
this stage, but making the reference TSC work again is definitely
something we want to look into in the future.

Wei.

> > hv_read_reference_counter = read_hv_clock_tsc;
> > phys_addr = virt_to_phys(hv_get_tsc_page());
>
> --
> Vitaly
>