From: Borislav Petkov <[email protected]>
Issue the CPU-not-supported message only on Intel machines as this
driver is Intel-only. Which means, the print statement can remain
KERN_INFO for ease of debugging (no need to enable it first in dynamic
debug).
While at it, correct it to say CPU "model" which is what that test does.
Fixes: 076b862c7e44 ("cpufreq: intel_pstate: Add reasons for failure and debug messages")
Signed-off-by: Borislav Petkov <[email protected]>
Cc: Erwan Velu <[email protected]>
Cc: Len Brown <[email protected]>
Cc: [email protected]
Cc: Rafael J. Wysocki <[email protected]>
CC: Srinivas Pandruvada <[email protected]>
Cc: Viresh Kumar <[email protected]>
---
drivers/cpufreq/intel_pstate.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/cpufreq/intel_pstate.c b/drivers/cpufreq/intel_pstate.c
index ea62e3f02d56..19854f01e2fa 100644
--- a/drivers/cpufreq/intel_pstate.c
+++ b/drivers/cpufreq/intel_pstate.c
@@ -2608,7 +2608,9 @@ static int __init intel_pstate_init(void)
} else {
id = x86_match_cpu(intel_pstate_cpu_ids);
if (!id) {
- pr_info("CPU ID not supported\n");
+ if (boot_cpu_data.x86_vendor == X86_VENDOR_INTEL)
+ pr_info("CPU model not supported\n");
+
return -ENODEV;
}
--
2.21.0
> index ea62e3f02d56..19854f01e2fa 100644
> --- a/drivers/cpufreq/intel_pstate.c
> +++ b/drivers/cpufreq/intel_pstate.c
> @@ -2608,7 +2608,9 @@ static int __init intel_pstate_init(void)
> } else {
> id = x86_match_cpu(intel_pstate_cpu_ids);
> if (!id) {
> - pr_info("CPU ID not supported\n");
> + if (boot_cpu_data.x86_vendor == X86_VENDOR_INTEL)
> + pr_info("CPU model not supported\n");
> +
> return -ENODEV;
> }
That's a good catch but I was wondering why not putting this vendor
condition at the initial "if (noload)" statement.
I mean, if we don't run an intel CPU there is no need of making the
x86_match_cpu().
This commit is also killing the case of reporting an unsupported intel
processor.
I'd suggest something like this and keep the 'CPUID not supported' part
untouched.
if (no_load) || (boot_cpu_data.x86_vendor != X86_VENDOR_INTEL)
return -ENODEV
On Mon, Apr 01, 2019 at 08:11:51AM +0000, Erwan Velu wrote:
> I'd suggest something like this and keep the 'CPUID not supported' part
> untouched.
>
> if (no_load) || (boot_cpu_data.x86_vendor != X86_VENDOR_INTEL)
Yeah, that's the usual thing we do in such cases and the better idea,
I'll do that in v2.
Thx.
--
Regards/Gruss,
Boris.
Good mailing practices for 400: avoid top-posting and trim the reply.
From: Borislav Petkov <[email protected]>
This driver is Intel-only so loading on anything which is not Intel is
pointless. Prevent it from doing so.
While at it, correct the "not supported" print statement to say CPU
"model" which is what that test does.
Suggested-by: Erwan Velu <[email protected]>
Signed-off-by: Borislav Petkov <[email protected]>
Cc: Len Brown <[email protected]>
Cc: [email protected]
Cc: Rafael J. Wysocki <[email protected]>
CC: Srinivas Pandruvada <[email protected]>
Cc: Viresh Kumar <[email protected]>
---
drivers/cpufreq/intel_pstate.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/drivers/cpufreq/intel_pstate.c b/drivers/cpufreq/intel_pstate.c
index b599c7318aab..2986119dd31f 100644
--- a/drivers/cpufreq/intel_pstate.c
+++ b/drivers/cpufreq/intel_pstate.c
@@ -2596,6 +2596,9 @@ static int __init intel_pstate_init(void)
const struct x86_cpu_id *id;
int rc;
+ if (boot_cpu_data.x86_vendor != X86_VENDOR_INTEL)
+ return -ENODEV;
+
if (no_load)
return -ENODEV;
@@ -2611,7 +2614,7 @@ static int __init intel_pstate_init(void)
} else {
id = x86_match_cpu(intel_pstate_cpu_ids);
if (!id) {
- pr_info("CPU ID not supported\n");
+ pr_info("CPU model not supported\n");
return -ENODEV;
}
--
2.21.0
--
Regards/Gruss,
Boris.
Good mailing practices for 400: avoid top-posting and trim the reply.
Le 01/04/2019 à 17:03, Borislav Petkov a écrit :
> From: Borislav Petkov <[email protected]>
>
> This driver is Intel-only so loading on anything which is not Intel is
> pointless. Prevent it from doing so.
>
> While at it, correct the "not supported" print statement to say CPU
> "model" which is what that test does.
>
Looks good to me !
Thanks !
Erwan,
On Monday, April 1, 2019 5:03:45 PM CEST Borislav Petkov wrote:
> From: Borislav Petkov <[email protected]>
>
> This driver is Intel-only so loading on anything which is not Intel is
> pointless. Prevent it from doing so.
Nice.
I wondered whether there are more of these to find by review, instead
of waiting for the next message to show up.
I ended up in the "not so straight forward" IOMMU init macros... and
continued with daily work again.
Anyway there are a lot files showing up when grepping the kernel for
intel files/drivers, maybe someone who is involved in the one or other comes
up with something similar...
Thomas
FWIW:
Reviewed-by: Thomas Renninger <[email protected]>
> While at it, correct the "not supported" print statement to say CPU
> "model" which is what that test does.
>
> Suggested-by: Erwan Velu <[email protected]>
> Signed-off-by: Borislav Petkov <[email protected]>
> Cc: Len Brown <[email protected]>
> Cc: [email protected]
> Cc: Rafael J. Wysocki <[email protected]>
> CC: Srinivas Pandruvada <[email protected]>
> Cc: Viresh Kumar <[email protected]>
> ---
> drivers/cpufreq/intel_pstate.c | 5 ++++-
> 1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/cpufreq/intel_pstate.c b/drivers/cpufreq/intel_pstate.c
> index b599c7318aab..2986119dd31f 100644
> --- a/drivers/cpufreq/intel_pstate.c
> +++ b/drivers/cpufreq/intel_pstate.c
> @@ -2596,6 +2596,9 @@ static int __init intel_pstate_init(void)
> const struct x86_cpu_id *id;
> int rc;
>
> + if (boot_cpu_data.x86_vendor != X86_VENDOR_INTEL)
> + return -ENODEV;
> +
...
On Monday, April 1, 2019 5:03:45 PM CEST Borislav Petkov wrote:
> From: Borislav Petkov <[email protected]>
>
> This driver is Intel-only so loading on anything which is not Intel is
> pointless. Prevent it from doing so.
>
> While at it, correct the "not supported" print statement to say CPU
> "model" which is what that test does.
>
> Suggested-by: Erwan Velu <[email protected]>
> Signed-off-by: Borislav Petkov <[email protected]>
> Cc: Len Brown <[email protected]>
> Cc: [email protected]
> Cc: Rafael J. Wysocki <[email protected]>
> CC: Srinivas Pandruvada <[email protected]>
> Cc: Viresh Kumar <[email protected]>
> ---
> drivers/cpufreq/intel_pstate.c | 5 ++++-
> 1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/cpufreq/intel_pstate.c b/drivers/cpufreq/intel_pstate.c
> index b599c7318aab..2986119dd31f 100644
> --- a/drivers/cpufreq/intel_pstate.c
> +++ b/drivers/cpufreq/intel_pstate.c
> @@ -2596,6 +2596,9 @@ static int __init intel_pstate_init(void)
> const struct x86_cpu_id *id;
> int rc;
>
> + if (boot_cpu_data.x86_vendor != X86_VENDOR_INTEL)
> + return -ENODEV;
> +
> if (no_load)
> return -ENODEV;
>
> @@ -2611,7 +2614,7 @@ static int __init intel_pstate_init(void)
> } else {
> id = x86_match_cpu(intel_pstate_cpu_ids);
> if (!id) {
> - pr_info("CPU ID not supported\n");
> + pr_info("CPU model not supported\n");
> return -ENODEV;
> }
>
>
Applied, thanks!