[PATCH 01/22] cpufreq/acpi-cpufreq: Convert to hotplug state machine

[PATCH 01/22] cpufreq/acpi-cpufreq: Convert to hotplug state machine

[Sebastian Andrzej Siewior]

Install the callbacks via the state machine.

Cc: "Rafael J. Wysocki" <rjw@rjwysocki.net>
Cc: Viresh Kumar <viresh.kumar@linaro.org>
Cc: linux-pm@vger.kernel.org
Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
---
 drivers/cpufreq/acpi-cpufreq.c | 93 ++++++++++++++++++++----------------------
 1 file changed, 45 insertions(+), 48 deletions(-)

diff --git a/drivers/cpufreq/acpi-cpufreq.c b/drivers/cpufreq/acpi-cpufreq.c
index 297e9128fe9f..2c29cbaca7b5 100644
--- a/drivers/cpufreq/acpi-cpufreq.c
+++ b/drivers/cpufreq/acpi-cpufreq.c
@@ -536,46 +536,33 @@ static void free_acpi_perf_data(void)
 	free_percpu(acpi_perf_data);
 }
 
-static int boost_notify(struct notifier_block *nb, unsigned long action,
-		      void *hcpu)
+static int cpufreq_boost_online(unsigned int cpu)
+{
+	const struct cpumask *cpumask;
+
+	cpumask = get_cpu_mask(cpu);
+	/*
+	 * On the CPU_UP path we simply keep the boost-disable flag
+	 * in sync with the current global state.
+	 */
+	boost_set_msrs(acpi_cpufreq_driver.boost_enabled, cpumask);
+	return 0;
+}
+
+static int cpufreq_boost_down_prep(unsigned int cpu)
 {
-	unsigned cpu = (long)hcpu;
 	const struct cpumask *cpumask;
 
 	cpumask = get_cpu_mask(cpu);
 
 	/*
 	 * Clear the boost-disable bit on the CPU_DOWN path so that
-	 * this cpu cannot block the remaining ones from boosting. On
-	 * the CPU_UP path we simply keep the boost-disable flag in
-	 * sync with the current global state.
+	 * this cpu cannot block the remaining ones from boosting.
 	 */
-
-	switch (action) {
-	case CPU_DOWN_FAILED:
-	case CPU_DOWN_FAILED_FROZEN:
-	case CPU_ONLINE:
-	case CPU_ONLINE_FROZEN:
-		boost_set_msrs(acpi_cpufreq_driver.boost_enabled, cpumask);
-		break;
-
-	case CPU_DOWN_PREPARE:
-	case CPU_DOWN_PREPARE_FROZEN:
-		boost_set_msrs(1, cpumask);
-		break;
-
-	default:
-		break;
-	}
-
-	return NOTIFY_OK;
+	boost_set_msrs(1, cpumask);
+	return 0;
 }
 
-
-static struct notifier_block boost_nb = {
-	.notifier_call          = boost_notify,
-};
-
 /*
  * acpi_cpufreq_early_init - initialize ACPI P-States library
  *
@@ -922,37 +909,47 @@ static struct cpufreq_driver acpi_cpufreq_driver = {
 	.attr		= acpi_cpufreq_attr,
 };
 
+static enum cpuhp_state acpi_cpufreq_online;
+
 static void __init acpi_cpufreq_boost_init(void)
 {
-	if (boot_cpu_has(X86_FEATURE_CPB) || boot_cpu_has(X86_FEATURE_IDA)) {
-		msrs = msrs_alloc();
+	int ret;
 
-		if (!msrs)
-			return;
+	if (!(boot_cpu_has(X86_FEATURE_CPB) || boot_cpu_has(X86_FEATURE_IDA)))
+		return;
 
-		acpi_cpufreq_driver.set_boost = set_boost;
-		acpi_cpufreq_driver.boost_enabled = boost_state(0);
+	msrs = msrs_alloc();
 
-		cpu_notifier_register_begin();
+	if (!msrs)
+		return;
 
-		/* Force all MSRs to the same value */
-		boost_set_msrs(acpi_cpufreq_driver.boost_enabled,
-			       cpu_online_mask);
+	acpi_cpufreq_driver.set_boost = set_boost;
+	acpi_cpufreq_driver.boost_enabled = boost_state(0);
 
-		__register_cpu_notifier(&boost_nb);
-
-		cpu_notifier_register_done();
+	/*
+	 * This calls the online callback on all online cpu and forces all
+	 * MSRs to the same value.
+	 */
+	ret = cpuhp_setup_state(CPUHP_AP_ONLINE_DYN, "cpufreq/acpi:online",
+				cpufreq_boost_online, cpufreq_boost_down_prep);
+	if (ret < 0) {
+		pr_err("acpi_cpufreq: failed to register hotplug callbacks\n");
+		msrs_free(msrs);
+		return;
 	}
+	acpi_cpufreq_online = ret;
 }
 
 static void acpi_cpufreq_boost_exit(void)
 {
-	if (msrs) {
-		unregister_cpu_notifier(&boost_nb);
+	if (!msrs)
+		return;
 
-		msrs_free(msrs);
-		msrs = NULL;
-	}
+	if (acpi_cpufreq_online >= 0)
+		cpuhp_remove_state_nocalls(acpi_cpufreq_online);
+
+	msrs_free(msrs);
+	msrs = NULL;
 }
 
 static int __init acpi_cpufreq_init(void)
-- 
2.10.2

Re: [PATCH 01/22] cpufreq/acpi-cpufreq: Convert to hotplug state machine

[Viresh Kumar]

On 27-11-16, 00:13, Sebastian Andrzej Siewior wrote:
> Install the callbacks via the state machine.
> 
> Cc: "Rafael J. Wysocki" <rjw@rjwysocki.net>
> Cc: Viresh Kumar <viresh.kumar@linaro.org>
> Cc: linux-pm@vger.kernel.org
> Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
> ---
>  drivers/cpufreq/acpi-cpufreq.c | 93 ++++++++++++++++++++----------------------
>  1 file changed, 45 insertions(+), 48 deletions(-)
> 
> diff --git a/drivers/cpufreq/acpi-cpufreq.c b/drivers/cpufreq/acpi-cpufreq.c
> index 297e9128fe9f..2c29cbaca7b5 100644
> --- a/drivers/cpufreq/acpi-cpufreq.c
> +++ b/drivers/cpufreq/acpi-cpufreq.c
> @@ -536,46 +536,33 @@ static void free_acpi_perf_data(void)
>  	free_percpu(acpi_perf_data);
>  }
>  
> -static int boost_notify(struct notifier_block *nb, unsigned long action,
> -		      void *hcpu)
> +static int cpufreq_boost_online(unsigned int cpu)
> +{
> +	const struct cpumask *cpumask;
> +
> +	cpumask = get_cpu_mask(cpu);
> +	/*
> +	 * On the CPU_UP path we simply keep the boost-disable flag
> +	 * in sync with the current global state.
> +	 */
> +	boost_set_msrs(acpi_cpufreq_driver.boost_enabled, cpumask);
> +	return 0;
> +}
> +
> +static int cpufreq_boost_down_prep(unsigned int cpu)
>  {
> -	unsigned cpu = (long)hcpu;
>  	const struct cpumask *cpumask;
>  
>  	cpumask = get_cpu_mask(cpu);
>  
>  	/*
>  	 * Clear the boost-disable bit on the CPU_DOWN path so that
> -	 * this cpu cannot block the remaining ones from boosting. On
> -	 * the CPU_UP path we simply keep the boost-disable flag in
> -	 * sync with the current global state.
> +	 * this cpu cannot block the remaining ones from boosting.
>  	 */
> -
> -	switch (action) {
> -	case CPU_DOWN_FAILED:
> -	case CPU_DOWN_FAILED_FROZEN:
> -	case CPU_ONLINE:
> -	case CPU_ONLINE_FROZEN:
> -		boost_set_msrs(acpi_cpufreq_driver.boost_enabled, cpumask);
> -		break;
> -
> -	case CPU_DOWN_PREPARE:
> -	case CPU_DOWN_PREPARE_FROZEN:
> -		boost_set_msrs(1, cpumask);
> -		break;
> -
> -	default:
> -		break;
> -	}
> -
> -	return NOTIFY_OK;
> +	boost_set_msrs(1, cpumask);
> +	return 0;
>  }
>  
> -
> -static struct notifier_block boost_nb = {
> -	.notifier_call          = boost_notify,
> -};
> -
>  /*
>   * acpi_cpufreq_early_init - initialize ACPI P-States library
>   *
> @@ -922,37 +909,47 @@ static struct cpufreq_driver acpi_cpufreq_driver = {
>  	.attr		= acpi_cpufreq_attr,
>  };
>  
> +static enum cpuhp_state acpi_cpufreq_online;
> +
>  static void __init acpi_cpufreq_boost_init(void)
>  {
> -	if (boot_cpu_has(X86_FEATURE_CPB) || boot_cpu_has(X86_FEATURE_IDA)) {
> -		msrs = msrs_alloc();
> +	int ret;
>  
> -		if (!msrs)
> -			return;
> +	if (!(boot_cpu_has(X86_FEATURE_CPB) || boot_cpu_has(X86_FEATURE_IDA)))
> +		return;
>  
> -		acpi_cpufreq_driver.set_boost = set_boost;
> -		acpi_cpufreq_driver.boost_enabled = boost_state(0);
> +	msrs = msrs_alloc();
>  
> -		cpu_notifier_register_begin();
> +	if (!msrs)
> +		return;
>  
> -		/* Force all MSRs to the same value */
> -		boost_set_msrs(acpi_cpufreq_driver.boost_enabled,
> -			       cpu_online_mask);

Why isn't this required anymore ?

> +	acpi_cpufreq_driver.set_boost = set_boost;
> +	acpi_cpufreq_driver.boost_enabled = boost_state(0);
>  
> -		__register_cpu_notifier(&boost_nb);
> -
> -		cpu_notifier_register_done();
> +	/*
> +	 * This calls the online callback on all online cpu and forces all
> +	 * MSRs to the same value.
> +	 */
> +	ret = cpuhp_setup_state(CPUHP_AP_ONLINE_DYN, "cpufreq/acpi:online",
> +				cpufreq_boost_online, cpufreq_boost_down_prep);
> +	if (ret < 0) {
> +		pr_err("acpi_cpufreq: failed to register hotplug callbacks\n");
> +		msrs_free(msrs);

If 'msrs = NULL' is required to be done in the _exit() routine, then it should
be done here as well. Right?

> +		return;
>  	}
> +	acpi_cpufreq_online = ret;
>  }
>  
>  static void acpi_cpufreq_boost_exit(void)
>  {
> -	if (msrs) {
> -		unregister_cpu_notifier(&boost_nb);
> +	if (!msrs)
> +		return;
>  
> -		msrs_free(msrs);
> -		msrs = NULL;
> -	}
> +	if (acpi_cpufreq_online >= 0)
> +		cpuhp_remove_state_nocalls(acpi_cpufreq_online);
> +
> +	msrs_free(msrs);
> +	msrs = NULL;
>  }
>  
>  static int __init acpi_cpufreq_init(void)
> -- 
> 2.10.2

-- 
viresh

Re: [PATCH 01/22] cpufreq/acpi-cpufreq: Convert to hotplug state machine

[Sebastian Andrzej Siewior]

On 2016-11-28 10:45:20 [+0530], Viresh Kumar wrote:
> > diff --git a/drivers/cpufreq/acpi-cpufreq.c b/drivers/cpufreq/acpi-cpufreq.c
> > index 297e9128fe9f..2c29cbaca7b5 100644
> > --- a/drivers/cpufreq/acpi-cpufreq.c
> > +++ b/drivers/cpufreq/acpi-cpufreq.c
> > @@ -922,37 +909,47 @@ static struct cpufreq_driver acpi_cpufreq_driver = {
> >  	.attr		= acpi_cpufreq_attr,
> >  };
> >  
> > +static enum cpuhp_state acpi_cpufreq_online;
> > +
> >  static void __init acpi_cpufreq_boost_init(void)
> >  {
> > -	if (boot_cpu_has(X86_FEATURE_CPB) || boot_cpu_has(X86_FEATURE_IDA)) {
> > -		msrs = msrs_alloc();
> > +	int ret;
> >  
> > -		if (!msrs)
> > -			return;
> > +	if (!(boot_cpu_has(X86_FEATURE_CPB) || boot_cpu_has(X86_FEATURE_IDA)))
> > +		return;
> >  
> > -		acpi_cpufreq_driver.set_boost = set_boost;
> > -		acpi_cpufreq_driver.boost_enabled = boost_state(0);
> > +	msrs = msrs_alloc();
> >  
> > -		cpu_notifier_register_begin();
> > +	if (!msrs)
> > +		return;
> >  
> > -		/* Force all MSRs to the same value */
> > -		boost_set_msrs(acpi_cpufreq_driver.boost_enabled,
> > -			       cpu_online_mask);
> 
> Why isn't this required anymore ?

Later there is cpuhp_setup_state() which invokes the online callback
(cpufreq_boost_online) which is doing this.

> > +	acpi_cpufreq_driver.set_boost = set_boost;
> > +	acpi_cpufreq_driver.boost_enabled = boost_state(0);
> >  
> > -		__register_cpu_notifier(&boost_nb);
> > -
> > -		cpu_notifier_register_done();
> > +	/*
> > +	 * This calls the online callback on all online cpu and forces all
> > +	 * MSRs to the same value.
> > +	 */
> > +	ret = cpuhp_setup_state(CPUHP_AP_ONLINE_DYN, "cpufreq/acpi:online",
> > +				cpufreq_boost_online, cpufreq_boost_down_prep);
> > +	if (ret < 0) {
> > +		pr_err("acpi_cpufreq: failed to register hotplug callbacks\n");
> > +		msrs_free(msrs);
> 
> If 'msrs = NULL' is required to be done in the _exit() routine, then it should
> be done here as well. Right?

It is not required in the exit path, it is just needed here. It is gone
in the following patch anyway but I will post a fixed version.

> > +		return;
> >  	}
> > +	acpi_cpufreq_online = ret;
> >  }
> >  
> >  static void acpi_cpufreq_boost_exit(void)
> >  {
> > -	if (msrs) {
> > -		unregister_cpu_notifier(&boost_nb);
> > +	if (!msrs)
> > +		return;
> >  
> > -		msrs_free(msrs);
> > -		msrs = NULL;
> > -	}
> > +	if (acpi_cpufreq_online >= 0)
> > +		cpuhp_remove_state_nocalls(acpi_cpufreq_online);
> > +
> > +	msrs_free(msrs);
> > +	msrs = NULL;
> >  }
> >  
> >  static int __init acpi_cpufreq_init(void)
> > -- 
> > 2.10.2

Sebastian

[PATCH 01/22 v2] cpufreq/acpi-cpufreq: Convert to hotplug state machine

[Sebastian Andrzej Siewior]

Install the callbacks via the state machine.

Cc: "Rafael J. Wysocki" <rjw@rjwysocki.net>
Cc: Viresh Kumar <viresh.kumar@linaro.org>
Cc: linux-pm@vger.kernel.org
Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
---
v1…v2: set msrs to NULL on error during init.

 drivers/cpufreq/acpi-cpufreq.c |   92 ++++++++++++++++++++---------------------
 1 file changed, 45 insertions(+), 47 deletions(-)

--- a/drivers/cpufreq/acpi-cpufreq.c
+++ b/drivers/cpufreq/acpi-cpufreq.c
@@ -536,46 +536,33 @@ static void free_acpi_perf_data(void)
 	free_percpu(acpi_perf_data);
 }
 
-static int boost_notify(struct notifier_block *nb, unsigned long action,
-		      void *hcpu)
+static int cpufreq_boost_online(unsigned int cpu)
 {
-	unsigned cpu = (long)hcpu;
 	const struct cpumask *cpumask;
 
 	cpumask = get_cpu_mask(cpu);
-
 	/*
-	 * Clear the boost-disable bit on the CPU_DOWN path so that
-	 * this cpu cannot block the remaining ones from boosting. On
-	 * the CPU_UP path we simply keep the boost-disable flag in
-	 * sync with the current global state.
+	 * On the CPU_UP path we simply keep the boost-disable flag
+	 * in sync with the current global state.
 	 */
+	boost_set_msrs(acpi_cpufreq_driver.boost_enabled, cpumask);
+	return 0;
+}
 
-	switch (action) {
-	case CPU_DOWN_FAILED:
-	case CPU_DOWN_FAILED_FROZEN:
-	case CPU_ONLINE:
-	case CPU_ONLINE_FROZEN:
-		boost_set_msrs(acpi_cpufreq_driver.boost_enabled, cpumask);
-		break;
-
-	case CPU_DOWN_PREPARE:
-	case CPU_DOWN_PREPARE_FROZEN:
-		boost_set_msrs(1, cpumask);
-		break;
+static int cpufreq_boost_down_prep(unsigned int cpu)
+{
+	const struct cpumask *cpumask;
 
-	default:
-		break;
-	}
+	cpumask = get_cpu_mask(cpu);
 
-	return NOTIFY_OK;
+	/*
+	 * Clear the boost-disable bit on the CPU_DOWN path so that
+	 * this cpu cannot block the remaining ones from boosting.
+	 */
+	boost_set_msrs(1, cpumask);
+	return 0;
 }
 
-
-static struct notifier_block boost_nb = {
-	.notifier_call          = boost_notify,
-};
-
 /*
  * acpi_cpufreq_early_init - initialize ACPI P-States library
  *
@@ -922,37 +909,48 @@ static struct cpufreq_driver acpi_cpufre
 	.attr		= acpi_cpufreq_attr,
 };
 
+static enum cpuhp_state acpi_cpufreq_online;
+
 static void __init acpi_cpufreq_boost_init(void)
 {
-	if (boot_cpu_has(X86_FEATURE_CPB) || boot_cpu_has(X86_FEATURE_IDA)) {
-		msrs = msrs_alloc();
+	int ret;
 
-		if (!msrs)
-			return;
+	if (!(boot_cpu_has(X86_FEATURE_CPB) || boot_cpu_has(X86_FEATURE_IDA)))
+		return;
 
-		acpi_cpufreq_driver.set_boost = set_boost;
-		acpi_cpufreq_driver.boost_enabled = boost_state(0);
+	msrs = msrs_alloc();
 
-		cpu_notifier_register_begin();
+	if (!msrs)
+		return;
 
-		/* Force all MSRs to the same value */
-		boost_set_msrs(acpi_cpufreq_driver.boost_enabled,
-			       cpu_online_mask);
+	acpi_cpufreq_driver.set_boost = set_boost;
+	acpi_cpufreq_driver.boost_enabled = boost_state(0);
 
-		__register_cpu_notifier(&boost_nb);
-
-		cpu_notifier_register_done();
+	/*
+	 * This calls the online callback on all online cpu and forces all
+	 * MSRs to the same value.
+	 */
+	ret = cpuhp_setup_state(CPUHP_AP_ONLINE_DYN, "cpufreq/acpi:online",
+				cpufreq_boost_online, cpufreq_boost_down_prep);
+	if (ret < 0) {
+		pr_err("acpi_cpufreq: failed to register hotplug callbacks\n");
+		msrs_free(msrs);
+		msrs = NULL;
+		return;
 	}
+	acpi_cpufreq_online = ret;
 }
 
 static void acpi_cpufreq_boost_exit(void)
 {
-	if (msrs) {
-		unregister_cpu_notifier(&boost_nb);
+	if (!msrs)
+		return;
 
-		msrs_free(msrs);
-		msrs = NULL;
-	}
+	if (acpi_cpufreq_online >= 0)
+		cpuhp_remove_state_nocalls(acpi_cpufreq_online);
+
+	msrs_free(msrs);
+	msrs = NULL;
 }
 
 static int __init acpi_cpufreq_init(void)

Re: [PATCH 01/22 v2] cpufreq/acpi-cpufreq: Convert to hotplug state machine

[Viresh Kumar]

On 28-11-16, 10:51, Sebastian Andrzej Siewior wrote:
> Install the callbacks via the state machine.
> 
> Cc: "Rafael J. Wysocki" <rjw@rjwysocki.net>
> Cc: Viresh Kumar <viresh.kumar@linaro.org>
> Cc: linux-pm@vger.kernel.org
> Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
> ---
> v1…v2: set msrs to NULL on error during init.
> 
>  drivers/cpufreq/acpi-cpufreq.c |   92 ++++++++++++++++++++---------------------
>  1 file changed, 45 insertions(+), 47 deletions(-)

Acked-by: Viresh Kumar <viresh.kumar@linaro.org>

-- 
viresh

Re: [PATCH 01/22 v2] cpufreq/acpi-cpufreq: Convert to hotplug state machine

[Rafael J. Wysocki]

On Mon, Nov 28, 2016 at 10:51 AM, Sebastian Andrzej Siewior
<bigeasy@linutronix.de> wrote:
> Install the callbacks via the state machine.
>
> Cc: "Rafael J. Wysocki" <rjw@rjwysocki.net>
> Cc: Viresh Kumar <viresh.kumar@linaro.org>
> Cc: linux-pm@vger.kernel.org
> Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
> ---
> v1…v2: set msrs to NULL on error during init.

OK

Let me route this and the [2/22] through the PM tree (they are first
in the series anyway).

Thanks,
Rafael

[PATCH 02/22] cpufreq/acpi-cpufreq: drop rdmsr_on_cpus() usage

[Sebastian Andrzej Siewior]

The online / pre_down callback is invoked on the target CPU since commit
1cf4f629d9d2 ("cpu/hotplug: Move online calls to hotplugged cpu") which means
for the hotplug callback we can use rmdsrl() instead of rdmsr_on_cpus().

This leaves us with set_boost() as the only user which still needs to
read/write the MSR on different CPUs. There is no point in doing that
update on all cpus with the read modify write magic via per cpu data. We
simply can issue a function call on all online CPUs which also means that we
need half that many IPIs.

Cc: "Rafael J. Wysocki" <rjw@rjwysocki.net>
Cc: Viresh Kumar <viresh.kumar@linaro.org>
Cc: linux-pm@vger.kernel.org
Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
---
 drivers/cpufreq/acpi-cpufreq.c | 58 +++++++++++++++---------------------------
 1 file changed, 20 insertions(+), 38 deletions(-)

diff --git a/drivers/cpufreq/acpi-cpufreq.c b/drivers/cpufreq/acpi-cpufreq.c
index 2c29cbaca7b5..3a98702b7445 100644
--- a/drivers/cpufreq/acpi-cpufreq.c
+++ b/drivers/cpufreq/acpi-cpufreq.c
@@ -84,7 +84,6 @@ static inline struct acpi_processor_performance *to_perf_data(struct acpi_cpufre
 static struct cpufreq_driver acpi_cpufreq_driver;
 
 static unsigned int acpi_pstate_strict;
-static struct msr __percpu *msrs;
 
 static bool boost_state(unsigned int cpu)
 {
@@ -104,11 +103,10 @@ static bool boost_state(unsigned int cpu)
 	return false;
 }
 
-static void boost_set_msrs(bool enable, const struct cpumask *cpumask)
+static int boost_set_msr(bool enable)
 {
-	u32 cpu;
 	u32 msr_addr;
-	u64 msr_mask;
+	u64 msr_mask, val;
 
 	switch (boot_cpu_data.x86_vendor) {
 	case X86_VENDOR_INTEL:
@@ -120,26 +118,31 @@ static void boost_set_msrs(bool enable, const struct cpumask *cpumask)
 		msr_mask = MSR_K7_HWCR_CPB_DIS;
 		break;
 	default:
-		return;
+		return -EINVAL;
 	}
 
-	rdmsr_on_cpus(cpumask, msr_addr, msrs);
+	rdmsrl(msr_addr, val);
 
-	for_each_cpu(cpu, cpumask) {
-		struct msr *reg = per_cpu_ptr(msrs, cpu);
-		if (enable)
-			reg->q &= ~msr_mask;
-		else
-			reg->q |= msr_mask;
-	}
+	if (enable)
+		val &= ~msr_mask;
+	else
+		val |= msr_mask;
 
-	wrmsr_on_cpus(cpumask, msr_addr, msrs);
+	wrmsrl(msr_addr, val);
+	return 0;
+}
+
+static void boost_set_msr_each(void *p_en)
+{
+	bool enable = (bool) p_en;
+
+	boost_set_msr(enable);
 }
 
 static int set_boost(int val)
 {
 	get_online_cpus();
-	boost_set_msrs(val, cpu_online_mask);
+	on_each_cpu(boost_set_msr_each, (void *)(long)val, 1);
 	put_online_cpus();
 	pr_debug("Core Boosting %sabled.\n", val ? "en" : "dis");
 
@@ -538,29 +541,20 @@ static void free_acpi_perf_data(void)
 
 static int cpufreq_boost_online(unsigned int cpu)
 {
-	const struct cpumask *cpumask;
-
-	cpumask = get_cpu_mask(cpu);
 	/*
 	 * On the CPU_UP path we simply keep the boost-disable flag
 	 * in sync with the current global state.
 	 */
-	boost_set_msrs(acpi_cpufreq_driver.boost_enabled, cpumask);
-	return 0;
+	return boost_set_msr(acpi_cpufreq_driver.boost_enabled);
 }
 
 static int cpufreq_boost_down_prep(unsigned int cpu)
 {
-	const struct cpumask *cpumask;
-
-	cpumask = get_cpu_mask(cpu);
-
 	/*
 	 * Clear the boost-disable bit on the CPU_DOWN path so that
 	 * this cpu cannot block the remaining ones from boosting.
 	 */
-	boost_set_msrs(1, cpumask);
-	return 0;
+	return boost_set_msr(1);
 }
 
 /*
@@ -918,11 +912,6 @@ static void __init acpi_cpufreq_boost_init(void)
 	if (!(boot_cpu_has(X86_FEATURE_CPB) || boot_cpu_has(X86_FEATURE_IDA)))
 		return;
 
-	msrs = msrs_alloc();
-
-	if (!msrs)
-		return;
-
 	acpi_cpufreq_driver.set_boost = set_boost;
 	acpi_cpufreq_driver.boost_enabled = boost_state(0);
 
@@ -934,7 +923,6 @@ static void __init acpi_cpufreq_boost_init(void)
 				cpufreq_boost_online, cpufreq_boost_down_prep);
 	if (ret < 0) {
 		pr_err("acpi_cpufreq: failed to register hotplug callbacks\n");
-		msrs_free(msrs);
 		return;
 	}
 	acpi_cpufreq_online = ret;
@@ -942,14 +930,8 @@ static void __init acpi_cpufreq_boost_init(void)
 
 static void acpi_cpufreq_boost_exit(void)
 {
-	if (!msrs)
-		return;
-
 	if (acpi_cpufreq_online >= 0)
 		cpuhp_remove_state_nocalls(acpi_cpufreq_online);
-
-	msrs_free(msrs);
-	msrs = NULL;
 }
 
 static int __init acpi_cpufreq_init(void)
-- 
2.10.2

[PATCH 02/22 v2] cpufreq/acpi-cpufreq: drop rdmsr_on_cpus() usage

[Sebastian Andrzej Siewior]

The online / pre_down callback is invoked on the target CPU since commit
1cf4f629d9d2 ("cpu/hotplug: Move online calls to hotplugged cpu") which means
for the hotplug callback we can use rmdsrl() instead of rdmsr_on_cpus().

This leaves us with set_boost() as the only user which still needs to
read/write the MSR on different CPUs. There is no point in doing that
update on all cpus with the read modify write magic via per cpu data. We
simply can issue a function call on all online CPUs which also means that we
need half that many IPIs.

Cc: "Rafael J. Wysocki" <rjw@rjwysocki.net>
Cc: Viresh Kumar <viresh.kumar@linaro.org>
Cc: linux-pm@vger.kernel.org
Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
---
v1…v2: rebase ontop of the previous patch.

 drivers/cpufreq/acpi-cpufreq.c |   59 +++++++++++++----------------------------
 1 file changed, 20 insertions(+), 39 deletions(-)

--- a/drivers/cpufreq/acpi-cpufreq.c
+++ b/drivers/cpufreq/acpi-cpufreq.c
@@ -84,7 +84,6 @@ static inline struct acpi_processor_perf
 static struct cpufreq_driver acpi_cpufreq_driver;
 
 static unsigned int acpi_pstate_strict;
-static struct msr __percpu *msrs;
 
 static bool boost_state(unsigned int cpu)
 {
@@ -104,11 +103,10 @@ static bool boost_state(unsigned int cpu
 	return false;
 }
 
-static void boost_set_msrs(bool enable, const struct cpumask *cpumask)
+static int boost_set_msr(bool enable)
 {
-	u32 cpu;
 	u32 msr_addr;
-	u64 msr_mask;
+	u64 msr_mask, val;
 
 	switch (boot_cpu_data.x86_vendor) {
 	case X86_VENDOR_INTEL:
@@ -120,26 +118,31 @@ static void boost_set_msrs(bool enable,
 		msr_mask = MSR_K7_HWCR_CPB_DIS;
 		break;
 	default:
-		return;
+		return -EINVAL;
 	}
 
-	rdmsr_on_cpus(cpumask, msr_addr, msrs);
+	rdmsrl(msr_addr, val);
 
-	for_each_cpu(cpu, cpumask) {
-		struct msr *reg = per_cpu_ptr(msrs, cpu);
-		if (enable)
-			reg->q &= ~msr_mask;
-		else
-			reg->q |= msr_mask;
-	}
+	if (enable)
+		val &= ~msr_mask;
+	else
+		val |= msr_mask;
 
-	wrmsr_on_cpus(cpumask, msr_addr, msrs);
+	wrmsrl(msr_addr, val);
+	return 0;
+}
+
+static void boost_set_msr_each(void *p_en)
+{
+	bool enable = (bool) p_en;
+
+	boost_set_msr(enable);
 }
 
 static int set_boost(int val)
 {
 	get_online_cpus();
-	boost_set_msrs(val, cpu_online_mask);
+	on_each_cpu(boost_set_msr_each, (void *)(long)val, 1);
 	put_online_cpus();
 	pr_debug("Core Boosting %sabled.\n", val ? "en" : "dis");
 
@@ -538,29 +541,20 @@ static void free_acpi_perf_data(void)
 
 static int cpufreq_boost_online(unsigned int cpu)
 {
-	const struct cpumask *cpumask;
-
-	cpumask = get_cpu_mask(cpu);
 	/*
 	 * On the CPU_UP path we simply keep the boost-disable flag
 	 * in sync with the current global state.
 	 */
-	boost_set_msrs(acpi_cpufreq_driver.boost_enabled, cpumask);
-	return 0;
+	return boost_set_msr(acpi_cpufreq_driver.boost_enabled);
 }
 
 static int cpufreq_boost_down_prep(unsigned int cpu)
 {
-	const struct cpumask *cpumask;
-
-	cpumask = get_cpu_mask(cpu);
-
 	/*
 	 * Clear the boost-disable bit on the CPU_DOWN path so that
 	 * this cpu cannot block the remaining ones from boosting.
 	 */
-	boost_set_msrs(1, cpumask);
-	return 0;
+	return boost_set_msr(1);
 }
 
 /*
@@ -918,11 +912,6 @@ static void __init acpi_cpufreq_boost_in
 	if (!(boot_cpu_has(X86_FEATURE_CPB) || boot_cpu_has(X86_FEATURE_IDA)))
 		return;
 
-	msrs = msrs_alloc();
-
-	if (!msrs)
-		return;
-
 	acpi_cpufreq_driver.set_boost = set_boost;
 	acpi_cpufreq_driver.boost_enabled = boost_state(0);
 
@@ -934,8 +923,6 @@ static void __init acpi_cpufreq_boost_in
 				cpufreq_boost_online, cpufreq_boost_down_prep);
 	if (ret < 0) {
 		pr_err("acpi_cpufreq: failed to register hotplug callbacks\n");
-		msrs_free(msrs);
-		msrs = NULL;
 		return;
 	}
 	acpi_cpufreq_online = ret;
@@ -943,14 +930,8 @@ static void __init acpi_cpufreq_boost_in
 
 static void acpi_cpufreq_boost_exit(void)
 {
-	if (!msrs)
-		return;
-
 	if (acpi_cpufreq_online >= 0)
 		cpuhp_remove_state_nocalls(acpi_cpufreq_online);
-
-	msrs_free(msrs);
-	msrs = NULL;
 }
 
 static int __init acpi_cpufreq_init(void)

Re: [PATCH 02/22 v2] cpufreq/acpi-cpufreq: drop rdmsr_on_cpus() usage

[Viresh Kumar]

On 28-11-16, 10:52, Sebastian Andrzej Siewior wrote:
> The online / pre_down callback is invoked on the target CPU since commit
> 1cf4f629d9d2 ("cpu/hotplug: Move online calls to hotplugged cpu") which means
> for the hotplug callback we can use rmdsrl() instead of rdmsr_on_cpus().
> 
> This leaves us with set_boost() as the only user which still needs to
> read/write the MSR on different CPUs. There is no point in doing that
> update on all cpus with the read modify write magic via per cpu data. We
> simply can issue a function call on all online CPUs which also means that we
> need half that many IPIs.
> 
> Cc: "Rafael J. Wysocki" <rjw@rjwysocki.net>
> Cc: Viresh Kumar <viresh.kumar@linaro.org>
> Cc: linux-pm@vger.kernel.org
> Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
> ---
> v1…v2: rebase ontop of the previous patch.

Acked-by: Viresh Kumar <viresh.kumar@linaro.org>

-- 
viresh

[PATCH 03/22] idle/intel: Remove superfluous SMP fuction call

[Sebastian Andrzej Siewior]

From: Anna-Maria Gleixner <anna-maria@linutronix.de>

Since commit 1cf4f629d9d2 ("cpu/hotplug: Move online calls to
hotplugged cpu") the CPU_ONLINE and CPU_DOWN_PREPARE notifiers are
always run on the hot plugged CPU, and as of commit 3b9d6da67e11
("cpu/hotplug: Fix rollback during error-out in __cpu_disable()") the
CPU_DOWN_FAILED notifier also runs on the hot plugged CPU. This patch
converts the SMP functional calls into direct calls.

smp_function_call_single() executes the function with interrupts
disabled. This calling convention is not preserved, because
tick_broadcast_enable() and tick_braodcast_disable() handle
interrupts themselves.

Cc: Len Brown <lenb@kernel.org>
Cc: linux-pm@vger.kernel.org
Signed-off-by: Anna-Maria Gleixner <anna-maria@linutronix.de>
Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
---
 drivers/idle/intel_idle.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/idle/intel_idle.c b/drivers/idle/intel_idle.c
index 4466a2f969d7..f53b42a78186 100644
--- a/drivers/idle/intel_idle.c
+++ b/drivers/idle/intel_idle.c
@@ -927,8 +927,7 @@ static int cpu_hotplug_notify(struct notifier_block *n,
 	case CPU_ONLINE:
 
 		if (lapic_timer_reliable_states != LAPIC_TIMER_ALWAYS_RELIABLE)
-			smp_call_function_single(hotcpu, __setup_broadcast_timer,
-						 (void *)true, 1);
+			__setup_broadcast_timer((void *)true);
 
 		/*
 		 * Some systems can hotplug a cpu at runtime after
-- 
2.10.2

[PATCH 04/22] idle/intel: Convert to hotplug state machine

[Sebastian Andrzej Siewior]

Install the callbacks via the state machine and let the core invoke the
callbacks on the already online CPUs.

The two smp_call_function_single() invocations in intel_idle_cpu_init() have
been removed because intel_idle_cpu_init() is now invoked via the hotplug
callback which runs on the target CPU. The IRQ-off calling convention for
auto_demotion_disable() and c1e_promotion_disable() has not been preserved
because only those two modify the MSR during CPU intialization.

Cc: Len Brown <lenb@kernel.org>
Cc: linux-pm@vger.kernel.org
Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
---
 drivers/idle/intel_idle.c | 106 ++++++++++++++++++----------------------------
 1 file changed, 42 insertions(+), 64 deletions(-)

diff --git a/drivers/idle/intel_idle.c b/drivers/idle/intel_idle.c
index f53b42a78186..d9631db1b4f5 100644
--- a/drivers/idle/intel_idle.c
+++ b/drivers/idle/intel_idle.c
@@ -98,8 +98,6 @@ static int intel_idle(struct cpuidle_device *dev,
 			struct cpuidle_driver *drv, int index);
 static void intel_idle_freeze(struct cpuidle_device *dev,
 			      struct cpuidle_driver *drv, int index);
-static int intel_idle_cpu_init(int cpu);
-
 static struct cpuidle_state *cpuidle_state_table;
 
 /*
@@ -907,50 +905,15 @@ static void intel_idle_freeze(struct cpuidle_device *dev,
 	mwait_idle_with_hints(eax, ecx);
 }
 
-static void __setup_broadcast_timer(void *arg)
+static void __setup_broadcast_timer(bool on)
 {
-	unsigned long on = (unsigned long)arg;
-
 	if (on)
 		tick_broadcast_enable();
 	else
 		tick_broadcast_disable();
 }
 
-static int cpu_hotplug_notify(struct notifier_block *n,
-			      unsigned long action, void *hcpu)
-{
-	int hotcpu = (unsigned long)hcpu;
-	struct cpuidle_device *dev;
-
-	switch (action & ~CPU_TASKS_FROZEN) {
-	case CPU_ONLINE:
-
-		if (lapic_timer_reliable_states != LAPIC_TIMER_ALWAYS_RELIABLE)
-			__setup_broadcast_timer((void *)true);
-
-		/*
-		 * Some systems can hotplug a cpu at runtime after
-		 * the kernel has booted, we have to initialize the
-		 * driver in this case
-		 */
-		dev = per_cpu_ptr(intel_idle_cpuidle_devices, hotcpu);
-		if (dev->registered)
-			break;
-
-		if (intel_idle_cpu_init(hotcpu))
-			return NOTIFY_BAD;
-
-		break;
-	}
-	return NOTIFY_OK;
-}
-
-static struct notifier_block cpu_hotplug_notifier = {
-	.notifier_call = cpu_hotplug_notify,
-};
-
-static void auto_demotion_disable(void *dummy)
+static void auto_demotion_disable(void)
 {
 	unsigned long long msr_bits;
 
@@ -958,7 +921,7 @@ static void auto_demotion_disable(void *dummy)
 	msr_bits &= ~(icpu->auto_demotion_disable_flags);
 	wrmsrl(MSR_NHM_SNB_PKG_CST_CFG_CTL, msr_bits);
 }
-static void c1e_promotion_disable(void *dummy)
+static void c1e_promotion_disable(void)
 {
 	unsigned long long msr_bits;
 
@@ -1372,12 +1335,11 @@ static void __init intel_idle_cpuidle_driver_init(void)
  * allocate, initialize, register cpuidle_devices
  * @cpu: cpu/core to initialize
  */
-static int intel_idle_cpu_init(int cpu)
+static int intel_idle_cpu_init(unsigned int cpu)
 {
 	struct cpuidle_device *dev;
 
 	dev = per_cpu_ptr(intel_idle_cpuidle_devices, cpu);
-
 	dev->cpu = cpu;
 
 	if (cpuidle_register_device(dev)) {
@@ -1386,17 +1348,38 @@ static int intel_idle_cpu_init(int cpu)
 	}
 
 	if (icpu->auto_demotion_disable_flags)
-		smp_call_function_single(cpu, auto_demotion_disable, NULL, 1);
+		auto_demotion_disable();
 
 	if (icpu->disable_promotion_to_c1e)
-		smp_call_function_single(cpu, c1e_promotion_disable, NULL, 1);
+		c1e_promotion_disable();
 
 	return 0;
 }
 
+static int intel_idle_cpu_online(unsigned int cpu)
+{
+	struct cpuidle_device *dev;
+
+	if (lapic_timer_reliable_states != LAPIC_TIMER_ALWAYS_RELIABLE)
+		__setup_broadcast_timer(true);
+
+	/*
+	 * Some systems can hotplug a cpu at runtime after
+	 * the kernel has booted, we have to initialize the
+	 * driver in this case
+	 */
+	dev = per_cpu_ptr(intel_idle_cpuidle_devices, cpu);
+	if (!dev->registered)
+		return intel_idle_cpu_init(cpu);
+
+	return 0;
+}
+
+static enum cpuhp_state hp_online;
+
 static int __init intel_idle_init(void)
 {
-	int retval, i;
+	int retval;
 
 	/* Do not load intel_idle at all for now if idle= is passed */
 	if (boot_option_idle_override != IDLE_NO_OVERRIDE)
@@ -1416,35 +1399,30 @@ static int __init intel_idle_init(void)
 		struct cpuidle_driver *drv = cpuidle_get_driver();
 		printk(KERN_DEBUG PREFIX "intel_idle yielding to %s",
 			drv ? drv->name : "none");
-		free_percpu(intel_idle_cpuidle_devices);
-		return retval;
+		goto init_driver_fail;
 	}
 
-	cpu_notifier_register_begin();
-
-	for_each_online_cpu(i) {
-		retval = intel_idle_cpu_init(i);
-		if (retval) {
-			intel_idle_cpuidle_devices_uninit();
-			cpu_notifier_register_done();
-			cpuidle_unregister_driver(&intel_idle_driver);
-			free_percpu(intel_idle_cpuidle_devices);
-			return retval;
-		}
-	}
-	__register_cpu_notifier(&cpu_hotplug_notifier);
-
 	if (boot_cpu_has(X86_FEATURE_ARAT))	/* Always Reliable APIC Timer */
 		lapic_timer_reliable_states = LAPIC_TIMER_ALWAYS_RELIABLE;
-	else
-		on_each_cpu(__setup_broadcast_timer, (void *)true, 1);
 
-	cpu_notifier_register_done();
+	retval = cpuhp_setup_state(CPUHP_AP_ONLINE_DYN, "idle/intel:online",
+				   intel_idle_cpu_online, NULL);
+	if (retval < 0)
+		goto hp_setup_fail;
+	hp_online = retval;
 
 	pr_debug(PREFIX "lapic_timer_reliable_states 0x%x\n",
 		lapic_timer_reliable_states);
 
 	return 0;
+
+hp_setup_fail:
+	intel_idle_cpuidle_devices_uninit();
+	cpuidle_unregister_driver(&intel_idle_driver);
+init_driver_fail:
+	free_percpu(intel_idle_cpuidle_devices);
+	return retval;
+
 }
 device_initcall(intel_idle_init);
 
-- 
2.10.2

Re: [PATCH 04/22] idle/intel: Convert to hotplug state machine

[Thomas Gleixner]

On Sun, 27 Nov 2016, Sebastian Andrzej Siewior wrote:
> +static enum cpuhp_state hp_online;
> +
>  static int __init intel_idle_init(void)
>  {
...

> -	cpu_notifier_register_done();
> +	retval = cpuhp_setup_state(CPUHP_AP_ONLINE_DYN, "idle/intel:online",
> +				   intel_idle_cpu_online, NULL);
> +	if (retval < 0)
> +		goto hp_setup_fail;
> +	hp_online = retval;

Why do you want to store that. There is no exit function AFAICT.

Thanks,

	tglx

Re: [PATCH 04/22] idle/intel: Convert to hotplug state machine

[Sebastian Andrzej Siewior]

On 2016-11-28 18:29:41 [+0100], Thomas Gleixner wrote:
> Why do you want to store that. There is no exit function AFAICT.

There was one until 02c4fae9ea68 ("drivers/idle: make intel_idle.c
driver more explicitly non-modular"). I am going to remove it…

> Thanks,
> 
> 	tglx

Sebastian

[PATCH 04/22 v2] idle/intel: Convert to hotplug state machine

[Sebastian Andrzej Siewior]

Install the callbacks via the state machine and let the core invoke the
callbacks on the already online CPUs.

The two smp_call_function_single() invocations in intel_idle_cpu_init() have
been removed because intel_idle_cpu_init() is now invoked via the hotplug
callback which runs on the target CPU. The IRQ-off calling convention for
auto_demotion_disable() and c1e_promotion_disable() has not been preserved
because only those two modify the MSR during CPU intialization.

Cc: Len Brown <lenb@kernel.org>
Cc: linux-pm@vger.kernel.org
Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
---
v1…v2: drop the superfluous saving of the hotplug state (not a module
anymore).

 drivers/idle/intel_idle.c | 103 ++++++++++++++++++----------------------------
 1 file changed, 39 insertions(+), 64 deletions(-)

diff --git a/drivers/idle/intel_idle.c b/drivers/idle/intel_idle.c
index f53b42a78186..df532c4398e5 100644
--- a/drivers/idle/intel_idle.c
+++ b/drivers/idle/intel_idle.c
@@ -98,8 +98,6 @@ static int intel_idle(struct cpuidle_device *dev,
 			struct cpuidle_driver *drv, int index);
 static void intel_idle_freeze(struct cpuidle_device *dev,
 			      struct cpuidle_driver *drv, int index);
-static int intel_idle_cpu_init(int cpu);
-
 static struct cpuidle_state *cpuidle_state_table;
 
 /*
@@ -907,50 +905,15 @@ static void intel_idle_freeze(struct cpuidle_device *dev,
 	mwait_idle_with_hints(eax, ecx);
 }
 
-static void __setup_broadcast_timer(void *arg)
+static void __setup_broadcast_timer(bool on)
 {
-	unsigned long on = (unsigned long)arg;
-
 	if (on)
 		tick_broadcast_enable();
 	else
 		tick_broadcast_disable();
 }
 
-static int cpu_hotplug_notify(struct notifier_block *n,
-			      unsigned long action, void *hcpu)
-{
-	int hotcpu = (unsigned long)hcpu;
-	struct cpuidle_device *dev;
-
-	switch (action & ~CPU_TASKS_FROZEN) {
-	case CPU_ONLINE:
-
-		if (lapic_timer_reliable_states != LAPIC_TIMER_ALWAYS_RELIABLE)
-			__setup_broadcast_timer((void *)true);
-
-		/*
-		 * Some systems can hotplug a cpu at runtime after
-		 * the kernel has booted, we have to initialize the
-		 * driver in this case
-		 */
-		dev = per_cpu_ptr(intel_idle_cpuidle_devices, hotcpu);
-		if (dev->registered)
-			break;
-
-		if (intel_idle_cpu_init(hotcpu))
-			return NOTIFY_BAD;
-
-		break;
-	}
-	return NOTIFY_OK;
-}
-
-static struct notifier_block cpu_hotplug_notifier = {
-	.notifier_call = cpu_hotplug_notify,
-};
-
-static void auto_demotion_disable(void *dummy)
+static void auto_demotion_disable(void)
 {
 	unsigned long long msr_bits;
 
@@ -958,7 +921,7 @@ static void auto_demotion_disable(void *dummy)
 	msr_bits &= ~(icpu->auto_demotion_disable_flags);
 	wrmsrl(MSR_NHM_SNB_PKG_CST_CFG_CTL, msr_bits);
 }
-static void c1e_promotion_disable(void *dummy)
+static void c1e_promotion_disable(void)
 {
 	unsigned long long msr_bits;
 
@@ -1372,12 +1335,11 @@ static void __init intel_idle_cpuidle_driver_init(void)
  * allocate, initialize, register cpuidle_devices
  * @cpu: cpu/core to initialize
  */
-static int intel_idle_cpu_init(int cpu)
+static int intel_idle_cpu_init(unsigned int cpu)
 {
 	struct cpuidle_device *dev;
 
 	dev = per_cpu_ptr(intel_idle_cpuidle_devices, cpu);
-
 	dev->cpu = cpu;
 
 	if (cpuidle_register_device(dev)) {
@@ -1386,17 +1348,36 @@ static int intel_idle_cpu_init(int cpu)
 	}
 
 	if (icpu->auto_demotion_disable_flags)
-		smp_call_function_single(cpu, auto_demotion_disable, NULL, 1);
+		auto_demotion_disable();
 
 	if (icpu->disable_promotion_to_c1e)
-		smp_call_function_single(cpu, c1e_promotion_disable, NULL, 1);
+		c1e_promotion_disable();
+
+	return 0;
+}
+
+static int intel_idle_cpu_online(unsigned int cpu)
+{
+	struct cpuidle_device *dev;
+
+	if (lapic_timer_reliable_states != LAPIC_TIMER_ALWAYS_RELIABLE)
+		__setup_broadcast_timer(true);
+
+	/*
+	 * Some systems can hotplug a cpu at runtime after
+	 * the kernel has booted, we have to initialize the
+	 * driver in this case
+	 */
+	dev = per_cpu_ptr(intel_idle_cpuidle_devices, cpu);
+	if (!dev->registered)
+		return intel_idle_cpu_init(cpu);
 
 	return 0;
 }
 
 static int __init intel_idle_init(void)
 {
-	int retval, i;
+	int retval;
 
 	/* Do not load intel_idle at all for now if idle= is passed */
 	if (boot_option_idle_override != IDLE_NO_OVERRIDE)
@@ -1416,35 +1397,29 @@ static int __init intel_idle_init(void)
 		struct cpuidle_driver *drv = cpuidle_get_driver();
 		printk(KERN_DEBUG PREFIX "intel_idle yielding to %s",
 			drv ? drv->name : "none");
-		free_percpu(intel_idle_cpuidle_devices);
-		return retval;
+		goto init_driver_fail;
 	}
 
-	cpu_notifier_register_begin();
-
-	for_each_online_cpu(i) {
-		retval = intel_idle_cpu_init(i);
-		if (retval) {
-			intel_idle_cpuidle_devices_uninit();
-			cpu_notifier_register_done();
-			cpuidle_unregister_driver(&intel_idle_driver);
-			free_percpu(intel_idle_cpuidle_devices);
-			return retval;
-		}
-	}
-	__register_cpu_notifier(&cpu_hotplug_notifier);
-
 	if (boot_cpu_has(X86_FEATURE_ARAT))	/* Always Reliable APIC Timer */
 		lapic_timer_reliable_states = LAPIC_TIMER_ALWAYS_RELIABLE;
-	else
-		on_each_cpu(__setup_broadcast_timer, (void *)true, 1);
 
-	cpu_notifier_register_done();
+	retval = cpuhp_setup_state(CPUHP_AP_ONLINE_DYN, "idle/intel:online",
+				   intel_idle_cpu_online, NULL);
+	if (retval < 0)
+		goto hp_setup_fail;
 
 	pr_debug(PREFIX "lapic_timer_reliable_states 0x%x\n",
 		lapic_timer_reliable_states);
 
 	return 0;
+
+hp_setup_fail:
+	intel_idle_cpuidle_devices_uninit();
+	cpuidle_unregister_driver(&intel_idle_driver);
+init_driver_fail:
+	free_percpu(intel_idle_cpuidle_devices);
+	return retval;
+
 }
 device_initcall(intel_idle_init);
 
-- 
2.10.2