[PATCH 0/3] intel_pstate: allow to be built as module and handle Sun server power capping.

[PATCH 0/3] intel_pstate: allow to be built as module and handle Sun server power capping.

[Ethan Zhao]

  Oracle Sun servers(X86) have power capping features that work via ACPI _PPC method,
patch No.1 is used to skip this driver if Oracle Sun server and _PPC detected. patch
No.2 is used to allow the driver to be configured and built as a module, so provide
the flexibility of configuration by userland. patch No.3 introduce a module parameter
and a kernel command line parameter, let user could force it loaded even on Oracle Sun
Servers(X86), that will be useful for debug\test\workaround etc purpose.

These patches have been tested on Oracle Sun server X4-2 series with following
cases on stable v3.18-rc3.

  a. Configure and build  intel_pstate as builtin.
     Boot without any kernel line parameter.
     Boot with intel_pstate=ignore_acpi_ppc.
  b. Configure and build intel_pstate as module.
     Load intel_pstate drive without any module parameter.
     Load intel_pstate driver with ignore_acpi_ppc=1

These cases passed and work fine.
--
Brian Maly (1):
  intel_pstate: allow driver to be built as a module

Ethan Zhao (2):
  intel_pstate: skip the driver if Sun server has ACPI _PPC method
  intel_pstate: add module and kernel command line parameter to ignore
    ACPI _PPC

 Documentation/kernel-parameters.txt |  3 +++
 drivers/cpufreq/Kconfig.x86         |  2 +-
 drivers/cpufreq/intel_pstate.c      | 32 ++++++++++++++++++++++++++++++++
 3 files changed, 36 insertions(+), 1 deletion(-)

-- 
1.8.3.1

[PATCH 1/3] intel_pstate: skip the driver if Sun server has ACPI _PPC method

[Ethan Zhao]

Oracle Sun X86 servers have dynamic power capping capability that works via
ACPI _PPC method etc, so skip loading this driver if Sun server has ACPI _PPC
enabled.

Signed-off-by: Ethan Zhao <ethan.zhao@oracle.com>
---
 drivers/cpufreq/intel_pstate.c | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

diff --git a/drivers/cpufreq/intel_pstate.c b/drivers/cpufreq/intel_pstate.c
index 27bb6d3..5498eb0 100644
--- a/drivers/cpufreq/intel_pstate.c
+++ b/drivers/cpufreq/intel_pstate.c
@@ -943,6 +943,21 @@ static bool intel_pstate_no_acpi_pss(void)
 	return true;
 }
 
+static bool intel_pstate_has_acpi_ppc(void)
+{
+	int i;
+
+	for_each_possible_cpu(i) {
+		struct acpi_processor *pr = per_cpu(processors, i);
+
+		if (!pr)
+			continue;
+		if (acpi_has_method(pr->handle, "_PPC"))
+			return true;
+	}
+	return false;
+}
+
 struct hw_vendor_info {
 	u16  valid;
 	char oem_id[ACPI_OEM_ID_SIZE];
@@ -952,6 +967,7 @@ struct hw_vendor_info {
 /* Hardware vendor-specific info that has its own power management modes */
 static struct hw_vendor_info vendor_info[] = {
 	{1, "HP    ", "ProLiant"},
+	{1, "ORACLE", ""},
 	{0, "", ""},
 };
 
@@ -969,12 +985,16 @@ static bool intel_pstate_platform_pwr_mgmt_exists(void)
 		    !strncmp(hdr.oem_table_id, v_info->oem_table_id, ACPI_OEM_TABLE_ID_SIZE) &&
 		    intel_pstate_no_acpi_pss())
 			return true;
+		if (!strncmp(hdr.oem_id, v_info->oem_id, ACPI_OEM_ID_SIZE) &&
+			intel_pstate_has_acpi_ppc())
+			return true;
 	}
 
 	return false;
 }
 #else /* CONFIG_ACPI not enabled */
 static inline bool intel_pstate_platform_pwr_mgmt_exists(void) { return false; }
+static inline bool intel_pstate_has_acpi_ppc(void) { return false; }
 #endif /* CONFIG_ACPI */
 
 static int __init intel_pstate_init(void)
-- 
1.8.3.1

[PATCH 2/3] intel_pstate: allow driver to be built as a module

[Ethan Zhao]

From: Brian Maly <brian.maly@oracle.com>

To provide the flexibility of module, allow this driver to
be configured and built as a module.

Signed-off-by: Brian Maly <brian.maly@oracle.com>
Signed-off-by: Ethan Zhao <ethan.zhao@oracle.com>
---
 drivers/cpufreq/Kconfig.x86    | 2 +-
 drivers/cpufreq/intel_pstate.c | 6 ++++++
 2 files changed, 7 insertions(+), 1 deletion(-)

diff --git a/drivers/cpufreq/Kconfig.x86 b/drivers/cpufreq/Kconfig.x86
index 89ae88f..94c9e6b 100644
--- a/drivers/cpufreq/Kconfig.x86
+++ b/drivers/cpufreq/Kconfig.x86
@@ -3,7 +3,7 @@
 #
 
 config X86_INTEL_PSTATE
-       bool "Intel P state control"
+       tristate "Intel P state control"
        depends on X86
        help
           This driver provides a P state for Intel core processors.
diff --git a/drivers/cpufreq/intel_pstate.c b/drivers/cpufreq/intel_pstate.c
index 5498eb0..7c5faea 100644
--- a/drivers/cpufreq/intel_pstate.c
+++ b/drivers/cpufreq/intel_pstate.c
@@ -590,7 +590,9 @@ static void intel_pstate_set_pstate(struct cpudata *cpu, int pstate)
 	if (pstate == cpu->pstate.current_pstate)
 		return;
 
+#ifndef MODULE
 	trace_cpu_frequency(pstate * cpu->pstate.scaling, cpu->cpu);
+#endif
 
 	cpu->pstate.current_pstate = pstate;
 
@@ -705,12 +707,14 @@ static void intel_pstate_timer_func(unsigned long __data)
 
 	intel_pstate_adjust_busy_pstate(cpu);
 
+#ifndef MODULE
 	trace_pstate_sample(fp_toint(sample->core_pct_busy),
 			fp_toint(intel_pstate_get_scaled_busy(cpu)),
 			cpu->pstate.current_pstate,
 			sample->mperf,
 			sample->aperf,
 			sample->freq);
+#endif
 
 	intel_pstate_set_sample_time(cpu);
 }
@@ -1054,6 +1058,7 @@ out:
 }
 device_initcall(intel_pstate_init);
 
+#ifndef MODULE
 static int __init intel_pstate_setup(char *str)
 {
 	if (!str)
@@ -1064,6 +1069,7 @@ static int __init intel_pstate_setup(char *str)
 	return 0;
 }
 early_param("intel_pstate", intel_pstate_setup);
+#endif
 
 MODULE_AUTHOR("Dirk Brandewie <dirk.j.brandewie@intel.com>");
 MODULE_DESCRIPTION("'intel_pstate' - P state driver Intel Core processors");
-- 
1.8.3.1

[PATCH 3/3] intel_pstate: add module and kernel command line parameter to ignore ACPI _PPC

[Ethan Zhao]

Add kernel command line parameter
 intel_pstate = ignore_acpi_ppc
and module parameter
 ignore_acpi_ppc = 1
to allow driver to ignore the ACPI _PPC existence even for Sun x86 servers.
These parameter could be used for debug\test\workaround etc purpose.

Signed-off-by: Ethan Zhao <ethan.zhao@oracle.com>
---
 Documentation/kernel-parameters.txt | 3 +++
 drivers/cpufreq/intel_pstate.c      | 8 +++++++-
 2 files changed, 10 insertions(+), 1 deletion(-)

diff --git a/Documentation/kernel-parameters.txt b/Documentation/kernel-parameters.txt
index 4c81a86..f502b85 100644
--- a/Documentation/kernel-parameters.txt
+++ b/Documentation/kernel-parameters.txt
@@ -1446,6 +1446,9 @@ bytes respectively. Such letter suffixes can also be entirely omitted.
 		       disable
 		         Do not enable intel_pstate as the default
 		         scaling driver for the supported processors
+		       ignore_acpi_ppc
+			 Ignore the existence of ACPI method _PPC for Sun x86 servers
+			 and load the driver.
 
 	intremap=	[X86-64, Intel-IOMMU]
 			on	enable Interrupt Remapping (default)
diff --git a/drivers/cpufreq/intel_pstate.c b/drivers/cpufreq/intel_pstate.c
index 7c5faea..388387b 100644
--- a/drivers/cpufreq/intel_pstate.c
+++ b/drivers/cpufreq/intel_pstate.c
@@ -870,6 +870,7 @@ static struct cpufreq_driver intel_pstate_driver = {
 };
 
 static int __initdata no_load;
+static unsigned int  ignore_acpi_ppc;
 
 static int intel_pstate_msrs_not_valid(void)
 {
@@ -990,7 +991,7 @@ static bool intel_pstate_platform_pwr_mgmt_exists(void)
 		    intel_pstate_no_acpi_pss())
 			return true;
 		if (!strncmp(hdr.oem_id, v_info->oem_id, ACPI_OEM_ID_SIZE) &&
-			intel_pstate_has_acpi_ppc())
+			intel_pstate_has_acpi_ppc() && !ignore_acpi_ppc)
 			return true;
 	}
 
@@ -1066,11 +1067,16 @@ static int __init intel_pstate_setup(char *str)
 
 	if (!strcmp(str, "disable"))
 		no_load = 1;
+	if (!strcmp(str, "ignore_acpi_ppc"))
+		ignore_acpi_ppc = 1;
 	return 0;
 }
 early_param("intel_pstate", intel_pstate_setup);
 #endif
 
+module_param(ignore_acpi_ppc, uint, 0644);
+MODULE_PARM_DESC(ignore_acpi_ppc,
+	"value 0 or non-zero. non-zero -> ignore ACPI _PPC and load this driver");
 MODULE_AUTHOR("Dirk Brandewie <dirk.j.brandewie@intel.com>");
 MODULE_DESCRIPTION("'intel_pstate' - P state driver Intel Core processors");
 MODULE_LICENSE("GPL");
-- 
1.8.3.1

[PATCH 0/3] intel_pstate: allow to be built as module and handle Sun server power capping.

[Ethan Zhao]

  Oracle Sun servers(X86) have power capping features that work via ACPI _PPC method,
patch No.1 is used to skip this driver if Oracle Sun server and _PPC detected. patch
No.2 is used to allow the driver to be configured and built as a module, so provide
the flexibility of configuration by userland. patch No.3 introduce a module parameter
and a kernel command line parameter, let user could force it loaded even on Oracle Sun
Servers(X86), that will be useful for debug\test\workaround etc purpose.

These patches have been tested on Oracle Sun server X4-2 series with following
cases on stable v3.18-rc3.

  a. Configure and build  intel_pstate as builtin.
     Boot without any kernel line parameter.
     Boot with intel_pstate=ignore_acpi_ppc.
  b. Configure and build intel_pstate as module.
     Load intel_pstate drive without any module parameter.
     Load intel_pstate driver with ignore_acpi_ppc=1

These cases passed and work fine.
--
Brian Maly (1):
  intel_pstate: allow driver to be built as a module

Ethan Zhao (2):
  intel_pstate: skip the driver if Sun server has ACPI _PPC method
  intel_pstate: add module and kernel command line parameter to ignore
    ACPI _PPC

 Documentation/kernel-parameters.txt |  3 +++
 drivers/cpufreq/Kconfig.x86         |  2 +-
 drivers/cpufreq/intel_pstate.c      | 32 ++++++++++++++++++++++++++++++++
 3 files changed, 36 insertions(+), 1 deletion(-)

-- 
1.8.3.1

[PATCH 1/3] intel_pstate: skip the driver if Sun server has ACPI _PPC method

[Ethan Zhao]

Oracle Sun X86 servers have dynamic power capping capability that works via
ACPI _PPC method etc, so skip loading this driver if Sun server has ACPI _PPC
enabled.

Signed-off-by: Ethan Zhao <ethan.zhao@oracle.com>
---
 drivers/cpufreq/intel_pstate.c | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

diff --git a/drivers/cpufreq/intel_pstate.c b/drivers/cpufreq/intel_pstate.c
index 27bb6d3..5498eb0 100644
--- a/drivers/cpufreq/intel_pstate.c
+++ b/drivers/cpufreq/intel_pstate.c
@@ -943,6 +943,21 @@ static bool intel_pstate_no_acpi_pss(void)
 	return true;
 }
 
+static bool intel_pstate_has_acpi_ppc(void)
+{
+	int i;
+
+	for_each_possible_cpu(i) {
+		struct acpi_processor *pr = per_cpu(processors, i);
+
+		if (!pr)
+			continue;
+		if (acpi_has_method(pr->handle, "_PPC"))
+			return true;
+	}
+	return false;
+}
+
 struct hw_vendor_info {
 	u16  valid;
 	char oem_id[ACPI_OEM_ID_SIZE];
@@ -952,6 +967,7 @@ struct hw_vendor_info {
 /* Hardware vendor-specific info that has its own power management modes */
 static struct hw_vendor_info vendor_info[] = {
 	{1, "HP    ", "ProLiant"},
+	{1, "ORACLE", ""},
 	{0, "", ""},
 };
 
@@ -969,12 +985,16 @@ static bool intel_pstate_platform_pwr_mgmt_exists(void)
 		    !strncmp(hdr.oem_table_id, v_info->oem_table_id, ACPI_OEM_TABLE_ID_SIZE) &&
 		    intel_pstate_no_acpi_pss())
 			return true;
+		if (!strncmp(hdr.oem_id, v_info->oem_id, ACPI_OEM_ID_SIZE) &&
+			intel_pstate_has_acpi_ppc())
+			return true;
 	}
 
 	return false;
 }
 #else /* CONFIG_ACPI not enabled */
 static inline bool intel_pstate_platform_pwr_mgmt_exists(void) { return false; }
+static inline bool intel_pstate_has_acpi_ppc(void) { return false; }
 #endif /* CONFIG_ACPI */
 
 static int __init intel_pstate_init(void)
-- 
1.8.3.1

[PATCH 2/3] intel_pstate: allow driver to be built as a module

[Ethan Zhao]

From: Brian Maly <brian.maly@oracle.com>

To provide the flexibility of module, allow this driver to
be configured and built as a module.

Signed-off-by: Brian Maly <brian.maly@oracle.com>
Signed-off-by: Ethan Zhao <ethan.zhao@oracle.com>
---
 drivers/cpufreq/Kconfig.x86    | 2 +-
 drivers/cpufreq/intel_pstate.c | 6 ++++++
 2 files changed, 7 insertions(+), 1 deletion(-)

diff --git a/drivers/cpufreq/Kconfig.x86 b/drivers/cpufreq/Kconfig.x86
index 89ae88f..94c9e6b 100644
--- a/drivers/cpufreq/Kconfig.x86
+++ b/drivers/cpufreq/Kconfig.x86
@@ -3,7 +3,7 @@
 #
 
 config X86_INTEL_PSTATE
-       bool "Intel P state control"
+       tristate "Intel P state control"
        depends on X86
        help
           This driver provides a P state for Intel core processors.
diff --git a/drivers/cpufreq/intel_pstate.c b/drivers/cpufreq/intel_pstate.c
index 5498eb0..7c5faea 100644
--- a/drivers/cpufreq/intel_pstate.c
+++ b/drivers/cpufreq/intel_pstate.c
@@ -590,7 +590,9 @@ static void intel_pstate_set_pstate(struct cpudata *cpu, int pstate)
 	if (pstate == cpu->pstate.current_pstate)
 		return;
 
+#ifndef MODULE
 	trace_cpu_frequency(pstate * cpu->pstate.scaling, cpu->cpu);
+#endif
 
 	cpu->pstate.current_pstate = pstate;
 
@@ -705,12 +707,14 @@ static void intel_pstate_timer_func(unsigned long __data)
 
 	intel_pstate_adjust_busy_pstate(cpu);
 
+#ifndef MODULE
 	trace_pstate_sample(fp_toint(sample->core_pct_busy),
 			fp_toint(intel_pstate_get_scaled_busy(cpu)),
 			cpu->pstate.current_pstate,
 			sample->mperf,
 			sample->aperf,
 			sample->freq);
+#endif
 
 	intel_pstate_set_sample_time(cpu);
 }
@@ -1054,6 +1058,7 @@ out:
 }
 device_initcall(intel_pstate_init);
 
+#ifndef MODULE
 static int __init intel_pstate_setup(char *str)
 {
 	if (!str)
@@ -1064,6 +1069,7 @@ static int __init intel_pstate_setup(char *str)
 	return 0;
 }
 early_param("intel_pstate", intel_pstate_setup);
+#endif
 
 MODULE_AUTHOR("Dirk Brandewie <dirk.j.brandewie@intel.com>");
 MODULE_DESCRIPTION("'intel_pstate' - P state driver Intel Core processors");
-- 
1.8.3.1

[PATCH 3/3] intel_pstate: add module and kernel command line parameter to ignore ACPI _PPC

[Ethan Zhao]

Add kernel command line parameter
 intel_pstate = ignore_acpi_ppc
and module parameter
 ignore_acpi_ppc = 1
to allow driver to ignore the ACPI _PPC existence even for Sun x86 servers.
These parameter could be used for debug\test\workaround etc purpose.

Signed-off-by: Ethan Zhao <ethan.zhao@oracle.com>
---
 Documentation/kernel-parameters.txt | 3 +++
 drivers/cpufreq/intel_pstate.c      | 8 +++++++-
 2 files changed, 10 insertions(+), 1 deletion(-)

diff --git a/Documentation/kernel-parameters.txt b/Documentation/kernel-parameters.txt
index 4c81a86..f502b85 100644
--- a/Documentation/kernel-parameters.txt
+++ b/Documentation/kernel-parameters.txt
@@ -1446,6 +1446,9 @@ bytes respectively. Such letter suffixes can also be entirely omitted.
 		       disable
 		         Do not enable intel_pstate as the default
 		         scaling driver for the supported processors
+		       ignore_acpi_ppc
+			 Ignore the existence of ACPI method _PPC for Sun x86 servers
+			 and load the driver.
 
 	intremap=	[X86-64, Intel-IOMMU]
 			on	enable Interrupt Remapping (default)
diff --git a/drivers/cpufreq/intel_pstate.c b/drivers/cpufreq/intel_pstate.c
index 7c5faea..388387b 100644
--- a/drivers/cpufreq/intel_pstate.c
+++ b/drivers/cpufreq/intel_pstate.c
@@ -870,6 +870,7 @@ static struct cpufreq_driver intel_pstate_driver = {
 };
 
 static int __initdata no_load;
+static unsigned int  ignore_acpi_ppc;
 
 static int intel_pstate_msrs_not_valid(void)
 {
@@ -990,7 +991,7 @@ static bool intel_pstate_platform_pwr_mgmt_exists(void)
 		    intel_pstate_no_acpi_pss())
 			return true;
 		if (!strncmp(hdr.oem_id, v_info->oem_id, ACPI_OEM_ID_SIZE) &&
-			intel_pstate_has_acpi_ppc())
+			intel_pstate_has_acpi_ppc() && !ignore_acpi_ppc)
 			return true;
 	}
 
@@ -1066,11 +1067,16 @@ static int __init intel_pstate_setup(char *str)
 
 	if (!strcmp(str, "disable"))
 		no_load = 1;
+	if (!strcmp(str, "ignore_acpi_ppc"))
+		ignore_acpi_ppc = 1;
 	return 0;
 }
 early_param("intel_pstate", intel_pstate_setup);
 #endif
 
+module_param(ignore_acpi_ppc, uint, 0644);
+MODULE_PARM_DESC(ignore_acpi_ppc,
+	"value 0 or non-zero. non-zero -> ignore ACPI _PPC and load this driver");
 MODULE_AUTHOR("Dirk Brandewie <dirk.j.brandewie@intel.com>");
 MODULE_DESCRIPTION("'intel_pstate' - P state driver Intel Core processors");
 MODULE_LICENSE("GPL");
-- 
1.8.3.1

Re: [PATCH 2/3] intel_pstate: allow driver to be built as a module

[Rafael J. Wysocki]

On Tuesday, November 18, 2014 05:37:01 PM Ethan Zhao wrote:
> From: Brian Maly <brian.maly@oracle.com>
> 
> To provide the flexibility of module, allow this driver to
> be configured and built as a module.
> 
> Signed-off-by: Brian Maly <brian.maly@oracle.com>
> Signed-off-by: Ethan Zhao <ethan.zhao@oracle.com>
> ---
>  drivers/cpufreq/Kconfig.x86    | 2 +-
>  drivers/cpufreq/intel_pstate.c | 6 ++++++
>  2 files changed, 7 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/cpufreq/Kconfig.x86 b/drivers/cpufreq/Kconfig.x86
> index 89ae88f..94c9e6b 100644
> --- a/drivers/cpufreq/Kconfig.x86
> +++ b/drivers/cpufreq/Kconfig.x86
> @@ -3,7 +3,7 @@
>  #
>  
>  config X86_INTEL_PSTATE
> -       bool "Intel P state control"
> +       tristate "Intel P state control"
>         depends on X86
>         help
>            This driver provides a P state for Intel core processors.
> diff --git a/drivers/cpufreq/intel_pstate.c b/drivers/cpufreq/intel_pstate.c
> index 5498eb0..7c5faea 100644
> --- a/drivers/cpufreq/intel_pstate.c
> +++ b/drivers/cpufreq/intel_pstate.c
> @@ -590,7 +590,9 @@ static void intel_pstate_set_pstate(struct cpudata *cpu, int pstate)
>  	if (pstate == cpu->pstate.current_pstate)
>  		return;
>  
> +#ifndef MODULE

This is not acceptable in any code that I maintain, sorry.

>  	trace_cpu_frequency(pstate * cpu->pstate.scaling, cpu->cpu);
> +#endif
>  
>  	cpu->pstate.current_pstate = pstate;
>  
> @@ -705,12 +707,14 @@ static void intel_pstate_timer_func(unsigned long __data)
>  
>  	intel_pstate_adjust_busy_pstate(cpu);
>  
> +#ifndef MODULE
>  	trace_pstate_sample(fp_toint(sample->core_pct_busy),
>  			fp_toint(intel_pstate_get_scaled_busy(cpu)),
>  			cpu->pstate.current_pstate,
>  			sample->mperf,
>  			sample->aperf,
>  			sample->freq);
> +#endif
>  
>  	intel_pstate_set_sample_time(cpu);
>  }
> @@ -1054,6 +1058,7 @@ out:
>  }
>  device_initcall(intel_pstate_init);
>  
> +#ifndef MODULE
>  static int __init intel_pstate_setup(char *str)
>  {
>  	if (!str)
> @@ -1064,6 +1069,7 @@ static int __init intel_pstate_setup(char *str)
>  	return 0;
>  }
>  early_param("intel_pstate", intel_pstate_setup);
> +#endif
>  
>  MODULE_AUTHOR("Dirk Brandewie <dirk.j.brandewie@intel.com>");
>  MODULE_DESCRIPTION("'intel_pstate' - P state driver Intel Core processors");
> 

-- 
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.

Re: [PATCH 1/3] intel_pstate: skip the driver if Sun server has ACPI _PPC method

[Dirk Brandewie]

On 11/18/2014 12:37 AM, Ethan Zhao wrote:
> Oracle Sun X86 servers have dynamic power capping capability that works via
> ACPI _PPC method etc, so skip loading this driver if Sun server has ACPI _PPC
> enabled.
>
> Signed-off-by: Ethan Zhao <ethan.zhao@oracle.com>
> ---
>   drivers/cpufreq/intel_pstate.c | 20 ++++++++++++++++++++
>   1 file changed, 20 insertions(+)
>
> diff --git a/drivers/cpufreq/intel_pstate.c b/drivers/cpufreq/intel_pstate.c
> index 27bb6d3..5498eb0 100644
> --- a/drivers/cpufreq/intel_pstate.c
> +++ b/drivers/cpufreq/intel_pstate.c
> @@ -943,6 +943,21 @@ static bool intel_pstate_no_acpi_pss(void)
>   	return true;
>   }
>
> +static bool intel_pstate_has_acpi_ppc(void)
> +{
> +	int i;
> +
> +	for_each_possible_cpu(i) {
> +		struct acpi_processor *pr = per_cpu(processors, i);
> +
> +		if (!pr)
> +			continue;
> +		if (acpi_has_method(pr->handle, "_PPC"))
> +			return true;
> +	}
> +	return false;
> +}
> +
>   struct hw_vendor_info {
>   	u16  valid;
>   	char oem_id[ACPI_OEM_ID_SIZE];
> @@ -952,6 +967,7 @@ struct hw_vendor_info {
>   /* Hardware vendor-specific info that has its own power management modes */
>   static struct hw_vendor_info vendor_info[] = {
>   	{1, "HP    ", "ProLiant"},
> +	{1, "ORACLE", ""},
>   	{0, "", ""},
>   };
>

Does this apply to ALL oracle systems?

Is the presence or absense of the _PPC method configurable in the oracle BIOS?

> @@ -969,12 +985,16 @@ static bool intel_pstate_platform_pwr_mgmt_exists(void)
>   		    !strncmp(hdr.oem_table_id, v_info->oem_table_id, ACPI_OEM_TABLE_ID_SIZE) &&
>   		    intel_pstate_no_acpi_pss())
>   			return true;
> +		if (!strncmp(hdr.oem_id, v_info->oem_id, ACPI_OEM_ID_SIZE) &&
> +			intel_pstate_has_acpi_ppc())
> +			return true;
>   	}
>
>   	return false;
>   }
>   #else /* CONFIG_ACPI not enabled */
>   static inline bool intel_pstate_platform_pwr_mgmt_exists(void) { return false; }
> +static inline bool intel_pstate_has_acpi_ppc(void) { return false; }
>   #endif /* CONFIG_ACPI */
>
>   static int __init intel_pstate_init(void)
>

Re: [PATCH 2/3] intel_pstate: allow driver to be built as a module

[Kristen Carlson Accardi]

On Tue, 18 Nov 2014 17:37:05 +0900
Ethan Zhao <ethan.zhao@oracle.com> wrote:

> From: Brian Maly <brian.maly@oracle.com>
> 
> To provide the flexibility of module, allow this driver to
> be configured and built as a module.
> 
> Signed-off-by: Brian Maly <brian.maly@oracle.com>
> Signed-off-by: Ethan Zhao <ethan.zhao@oracle.com>

I believe the entire concept of being able to use intel_pstate as a
module just isn't going to work.  There are load order issues - and
additionally the driver doesn't clean up after itself in any way.


> ---
>  drivers/cpufreq/Kconfig.x86    | 2 +-
>  drivers/cpufreq/intel_pstate.c | 6 ++++++
>  2 files changed, 7 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/cpufreq/Kconfig.x86 b/drivers/cpufreq/Kconfig.x86
> index 89ae88f..94c9e6b 100644
> --- a/drivers/cpufreq/Kconfig.x86
> +++ b/drivers/cpufreq/Kconfig.x86
> @@ -3,7 +3,7 @@
>  #
>  
>  config X86_INTEL_PSTATE
> -       bool "Intel P state control"
> +       tristate "Intel P state control"
>         depends on X86
>         help
>            This driver provides a P state for Intel core processors.
> diff --git a/drivers/cpufreq/intel_pstate.c b/drivers/cpufreq/intel_pstate.c
> index 5498eb0..7c5faea 100644
> --- a/drivers/cpufreq/intel_pstate.c
> +++ b/drivers/cpufreq/intel_pstate.c
> @@ -590,7 +590,9 @@ static void intel_pstate_set_pstate(struct cpudata *cpu, int pstate)
>  	if (pstate == cpu->pstate.current_pstate)
>  		return;
>  
> +#ifndef MODULE
>  	trace_cpu_frequency(pstate * cpu->pstate.scaling, cpu->cpu);
> +#endif
>  
>  	cpu->pstate.current_pstate = pstate;
>  
> @@ -705,12 +707,14 @@ static void intel_pstate_timer_func(unsigned long __data)
>  
>  	intel_pstate_adjust_busy_pstate(cpu);
>  
> +#ifndef MODULE
>  	trace_pstate_sample(fp_toint(sample->core_pct_busy),
>  			fp_toint(intel_pstate_get_scaled_busy(cpu)),
>  			cpu->pstate.current_pstate,
>  			sample->mperf,
>  			sample->aperf,
>  			sample->freq);
> +#endif
>  
>  	intel_pstate_set_sample_time(cpu);
>  }
> @@ -1054,6 +1058,7 @@ out:
>  }
>  device_initcall(intel_pstate_init);
>  
> +#ifndef MODULE
>  static int __init intel_pstate_setup(char *str)
>  {
>  	if (!str)
> @@ -1064,6 +1069,7 @@ static int __init intel_pstate_setup(char *str)
>  	return 0;
>  }
>  early_param("intel_pstate", intel_pstate_setup);
> +#endif
>  
>  MODULE_AUTHOR("Dirk Brandewie <dirk.j.brandewie@intel.com>");
>  MODULE_DESCRIPTION("'intel_pstate' - P state driver Intel Core processors");

Re: [PATCH 2/3] intel_pstate: allow driver to be built as a module

[Kristen Carlson Accardi]

On Tue, 18 Nov 2014 17:37:05 +0900
Ethan Zhao <ethan.zhao@oracle.com> wrote:

> From: Brian Maly <brian.maly@oracle.com>
> 
> To provide the flexibility of module, allow this driver to
> be configured and built as a module.
> 
> Signed-off-by: Brian Maly <brian.maly@oracle.com>
> Signed-off-by: Ethan Zhao <ethan.zhao@oracle.com>

I believe the entire concept of being able to use intel_pstate as a
module just isn't going to work.  There are load order issues - and
additionally the driver doesn't clean up after itself in any way.


> ---
>  drivers/cpufreq/Kconfig.x86    | 2 +-
>  drivers/cpufreq/intel_pstate.c | 6 ++++++
>  2 files changed, 7 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/cpufreq/Kconfig.x86 b/drivers/cpufreq/Kconfig.x86
> index 89ae88f..94c9e6b 100644
> --- a/drivers/cpufreq/Kconfig.x86
> +++ b/drivers/cpufreq/Kconfig.x86
> @@ -3,7 +3,7 @@
>  #
>  
>  config X86_INTEL_PSTATE
> -       bool "Intel P state control"
> +       tristate "Intel P state control"
>         depends on X86
>         help
>            This driver provides a P state for Intel core processors.
> diff --git a/drivers/cpufreq/intel_pstate.c b/drivers/cpufreq/intel_pstate.c
> index 5498eb0..7c5faea 100644
> --- a/drivers/cpufreq/intel_pstate.c
> +++ b/drivers/cpufreq/intel_pstate.c
> @@ -590,7 +590,9 @@ static void intel_pstate_set_pstate(struct cpudata *cpu, int pstate)
>  	if (pstate == cpu->pstate.current_pstate)
>  		return;
>  
> +#ifndef MODULE
>  	trace_cpu_frequency(pstate * cpu->pstate.scaling, cpu->cpu);
> +#endif
>  
>  	cpu->pstate.current_pstate = pstate;
>  
> @@ -705,12 +707,14 @@ static void intel_pstate_timer_func(unsigned long __data)
>  
>  	intel_pstate_adjust_busy_pstate(cpu);
>  
> +#ifndef MODULE
>  	trace_pstate_sample(fp_toint(sample->core_pct_busy),
>  			fp_toint(intel_pstate_get_scaled_busy(cpu)),
>  			cpu->pstate.current_pstate,
>  			sample->mperf,
>  			sample->aperf,
>  			sample->freq);
> +#endif
>  
>  	intel_pstate_set_sample_time(cpu);
>  }
> @@ -1054,6 +1058,7 @@ out:
>  }
>  device_initcall(intel_pstate_init);
>  
> +#ifndef MODULE
>  static int __init intel_pstate_setup(char *str)
>  {
>  	if (!str)
> @@ -1064,6 +1069,7 @@ static int __init intel_pstate_setup(char *str)
>  	return 0;
>  }
>  early_param("intel_pstate", intel_pstate_setup);
> +#endif
>  
>  MODULE_AUTHOR("Dirk Brandewie <dirk.j.brandewie@intel.com>");
>  MODULE_DESCRIPTION("'intel_pstate' - P state driver Intel Core processors");

Re: [PATCH 3/3] intel_pstate: add module and kernel command line parameter to ignore ACPI _PPC

[Kristen Carlson Accardi]

On Tue, 18 Nov 2014 17:37:06 +0900
Ethan Zhao <ethan.zhao@oracle.com> wrote:

> Add kernel command line parameter
>  intel_pstate = ignore_acpi_ppc
> and module parameter
>  ignore_acpi_ppc = 1
> to allow driver to ignore the ACPI _PPC existence even for Sun x86 servers.
> These parameter could be used for debug\test\workaround etc purpose.
> 
> Signed-off-by: Ethan Zhao <ethan.zhao@oracle.com>

What if we used a more generic parameter like "force" that would bypass
any vendor specific checks and just load anyway?  This way we don't have
to add new parameters everything some new thing shows up that we want to
ignore.

> ---
>  Documentation/kernel-parameters.txt | 3 +++
>  drivers/cpufreq/intel_pstate.c      | 8 +++++++-
>  2 files changed, 10 insertions(+), 1 deletion(-)
> 
> diff --git a/Documentation/kernel-parameters.txt b/Documentation/kernel-parameters.txt
> index 4c81a86..f502b85 100644
> --- a/Documentation/kernel-parameters.txt
> +++ b/Documentation/kernel-parameters.txt
> @@ -1446,6 +1446,9 @@ bytes respectively. Such letter suffixes can also be entirely omitted.
>  		       disable
>  		         Do not enable intel_pstate as the default
>  		         scaling driver for the supported processors
> +		       ignore_acpi_ppc
> +			 Ignore the existence of ACPI method _PPC for Sun x86 servers
> +			 and load the driver.
>  
>  	intremap=	[X86-64, Intel-IOMMU]
>  			on	enable Interrupt Remapping (default)
> diff --git a/drivers/cpufreq/intel_pstate.c b/drivers/cpufreq/intel_pstate.c
> index 7c5faea..388387b 100644
> --- a/drivers/cpufreq/intel_pstate.c
> +++ b/drivers/cpufreq/intel_pstate.c
> @@ -870,6 +870,7 @@ static struct cpufreq_driver intel_pstate_driver = {
>  };
>  
>  static int __initdata no_load;
> +static unsigned int  ignore_acpi_ppc;
>  
>  static int intel_pstate_msrs_not_valid(void)
>  {
> @@ -990,7 +991,7 @@ static bool intel_pstate_platform_pwr_mgmt_exists(void)
>  		    intel_pstate_no_acpi_pss())
>  			return true;
>  		if (!strncmp(hdr.oem_id, v_info->oem_id, ACPI_OEM_ID_SIZE) &&
> -			intel_pstate_has_acpi_ppc())
> +			intel_pstate_has_acpi_ppc() && !ignore_acpi_ppc)
>  			return true;
>  	}
>  
> @@ -1066,11 +1067,16 @@ static int __init intel_pstate_setup(char *str)
>  
>  	if (!strcmp(str, "disable"))
>  		no_load = 1;
> +	if (!strcmp(str, "ignore_acpi_ppc"))
> +		ignore_acpi_ppc = 1;
>  	return 0;
>  }
>  early_param("intel_pstate", intel_pstate_setup);
>  #endif
>  
> +module_param(ignore_acpi_ppc, uint, 0644);
> +MODULE_PARM_DESC(ignore_acpi_ppc,
> +	"value 0 or non-zero. non-zero -> ignore ACPI _PPC and load this driver");
>  MODULE_AUTHOR("Dirk Brandewie <dirk.j.brandewie@intel.com>");
>  MODULE_DESCRIPTION("'intel_pstate' - P state driver Intel Core processors");
>  MODULE_LICENSE("GPL");

Re: [PATCH 3/3] intel_pstate: add module and kernel command line parameter to ignore ACPI _PPC

[Kristen Carlson Accardi]

On Tue, 18 Nov 2014 17:37:06 +0900
Ethan Zhao <ethan.zhao@oracle.com> wrote:

> Add kernel command line parameter
>  intel_pstate = ignore_acpi_ppc
> and module parameter
>  ignore_acpi_ppc = 1
> to allow driver to ignore the ACPI _PPC existence even for Sun x86 servers.
> These parameter could be used for debug\test\workaround etc purpose.
> 
> Signed-off-by: Ethan Zhao <ethan.zhao@oracle.com>

What if we used a more generic parameter like "force" that would bypass
any vendor specific checks and just load anyway?  This way we don't have
to add new parameters everything some new thing shows up that we want to
ignore.

> ---
>  Documentation/kernel-parameters.txt | 3 +++
>  drivers/cpufreq/intel_pstate.c      | 8 +++++++-
>  2 files changed, 10 insertions(+), 1 deletion(-)
> 
> diff --git a/Documentation/kernel-parameters.txt b/Documentation/kernel-parameters.txt
> index 4c81a86..f502b85 100644
> --- a/Documentation/kernel-parameters.txt
> +++ b/Documentation/kernel-parameters.txt
> @@ -1446,6 +1446,9 @@ bytes respectively. Such letter suffixes can also be entirely omitted.
>  		       disable
>  		         Do not enable intel_pstate as the default
>  		         scaling driver for the supported processors
> +		       ignore_acpi_ppc
> +			 Ignore the existence of ACPI method _PPC for Sun x86 servers
> +			 and load the driver.
>  
>  	intremap=	[X86-64, Intel-IOMMU]
>  			on	enable Interrupt Remapping (default)
> diff --git a/drivers/cpufreq/intel_pstate.c b/drivers/cpufreq/intel_pstate.c
> index 7c5faea..388387b 100644
> --- a/drivers/cpufreq/intel_pstate.c
> +++ b/drivers/cpufreq/intel_pstate.c
> @@ -870,6 +870,7 @@ static struct cpufreq_driver intel_pstate_driver = {
>  };
>  
>  static int __initdata no_load;
> +static unsigned int  ignore_acpi_ppc;
>  
>  static int intel_pstate_msrs_not_valid(void)
>  {
> @@ -990,7 +991,7 @@ static bool intel_pstate_platform_pwr_mgmt_exists(void)
>  		    intel_pstate_no_acpi_pss())
>  			return true;
>  		if (!strncmp(hdr.oem_id, v_info->oem_id, ACPI_OEM_ID_SIZE) &&
> -			intel_pstate_has_acpi_ppc())
> +			intel_pstate_has_acpi_ppc() && !ignore_acpi_ppc)
>  			return true;
>  	}
>  
> @@ -1066,11 +1067,16 @@ static int __init intel_pstate_setup(char *str)
>  
>  	if (!strcmp(str, "disable"))
>  		no_load = 1;
> +	if (!strcmp(str, "ignore_acpi_ppc"))
> +		ignore_acpi_ppc = 1;
>  	return 0;
>  }
>  early_param("intel_pstate", intel_pstate_setup);
>  #endif
>  
> +module_param(ignore_acpi_ppc, uint, 0644);
> +MODULE_PARM_DESC(ignore_acpi_ppc,
> +	"value 0 or non-zero. non-zero -> ignore ACPI _PPC and load this driver");
>  MODULE_AUTHOR("Dirk Brandewie <dirk.j.brandewie@intel.com>");
>  MODULE_DESCRIPTION("'intel_pstate' - P state driver Intel Core processors");
>  MODULE_LICENSE("GPL");

Re: [PATCH 1/3] intel_pstate: skip the driver if Sun server has ACPI _PPC method

[Linda Knippers]

On 11/18/2014 3:37 AM, Ethan Zhao wrote:
> Oracle Sun X86 servers have dynamic power capping capability that works via
> ACPI _PPC method etc, so skip loading this driver if Sun server has ACPI _PPC
> enabled.
> 
> Signed-off-by: Ethan Zhao <ethan.zhao@oracle.com>
> ---
>  drivers/cpufreq/intel_pstate.c | 20 ++++++++++++++++++++
>  1 file changed, 20 insertions(+)
> 
> diff --git a/drivers/cpufreq/intel_pstate.c b/drivers/cpufreq/intel_pstate.c
> index 27bb6d3..5498eb0 100644
> --- a/drivers/cpufreq/intel_pstate.c
> +++ b/drivers/cpufreq/intel_pstate.c
> @@ -943,6 +943,21 @@ static bool intel_pstate_no_acpi_pss(void)
>  	return true;
>  }
>  
> +static bool intel_pstate_has_acpi_ppc(void)
> +{
> +	int i;
> +
> +	for_each_possible_cpu(i) {
> +		struct acpi_processor *pr = per_cpu(processors, i);
> +
> +		if (!pr)
> +			continue;
> +		if (acpi_has_method(pr->handle, "_PPC"))
> +			return true;
> +	}
> +	return false;
> +}
> +
>  struct hw_vendor_info {
>  	u16  valid;
>  	char oem_id[ACPI_OEM_ID_SIZE];
> @@ -952,6 +967,7 @@ struct hw_vendor_info {
>  /* Hardware vendor-specific info that has its own power management modes */
>  static struct hw_vendor_info vendor_info[] = {
>  	{1, "HP    ", "ProLiant"},
> +	{1, "ORACLE", ""},
>  	{0, "", ""},
>  };
>  
> @@ -969,12 +985,16 @@ static bool intel_pstate_platform_pwr_mgmt_exists(void)
>  		    !strncmp(hdr.oem_table_id, v_info->oem_table_id, ACPI_OEM_TABLE_ID_SIZE) &&
>  		    intel_pstate_no_acpi_pss())
>  			return true;
> +		if (!strncmp(hdr.oem_id, v_info->oem_id, ACPI_OEM_ID_SIZE) &&
> +			intel_pstate_has_acpi_ppc())

We need try this on a few platforms to make sure this patch doesn't break the
HP platforms that may or may not need this driver, depending on the BIOS settings.

I don't suppose you tested on a ProLiant too?

-- ljk

> +			return true;
>  	}
>  
>  	return false;
>  }
>  #else /* CONFIG_ACPI not enabled */
>  static inline bool intel_pstate_platform_pwr_mgmt_exists(void) { return false; }
> +static inline bool intel_pstate_has_acpi_ppc(void) { return false; }
>  #endif /* CONFIG_ACPI */
>  
>  static int __init intel_pstate_init(void)
> 

Re: [PATCH 1/3] intel_pstate: skip the driver if Sun server has ACPI _PPC method

[ethan]

Linda，

> 在 2014年11月20日，04:22，Linda Knippers <linda.knippers@hp.com> 写道：
> 
>> On 11/18/2014 3:37 AM, Ethan Zhao wrote:
>> Oracle Sun X86 servers have dynamic power capping capability that works via
>> ACPI _PPC method etc, so skip loading this driver if Sun server has ACPI _PPC
>> enabled.
>> 
>> Signed-off-by: Ethan Zhao <ethan.zhao@oracle.com>
>> ---
>> drivers/cpufreq/intel_pstate.c | 20 ++++++++++++++++++++
>> 1 file changed, 20 insertions(+)
>> 
>> diff --git a/drivers/cpufreq/intel_pstate.c b/drivers/cpufreq/intel_pstate.c
>> index 27bb6d3..5498eb0 100644
>> --- a/drivers/cpufreq/intel_pstate.c
>> +++ b/drivers/cpufreq/intel_pstate.c
>> @@ -943,6 +943,21 @@ static bool intel_pstate_no_acpi_pss(void)
>>    return true;
>> }
>> 
>> +static bool intel_pstate_has_acpi_ppc(void)
>> +{
>> +    int i;
>> +
>> +    for_each_possible_cpu(i) {
>> +        struct acpi_processor *pr = per_cpu(processors, i);
>> +
>> +        if (!pr)
>> +            continue;
>> +        if (acpi_has_method(pr->handle, "_PPC"))
>> +            return true;
>> +    }
>> +    return false;
>> +}
>> +
>> struct hw_vendor_info {
>>    u16  valid;
>>    char oem_id[ACPI_OEM_ID_SIZE];
>> @@ -952,6 +967,7 @@ struct hw_vendor_info {
>> /* Hardware vendor-specific info that has its own power management modes */
>> static struct hw_vendor_info vendor_info[] = {
>>    {1, "HP    ", "ProLiant"},
>> +    {1, "ORACLE", ""},
>>    {0, "", ""},
>> };
>> 
>> @@ -969,12 +985,16 @@ static bool intel_pstate_platform_pwr_mgmt_exists(void)
>>            !strncmp(hdr.oem_table_id, v_info->oem_table_id, ACPI_OEM_TABLE_ID_SIZE) &&
>>            intel_pstate_no_acpi_pss())
>>            return true;
>> +        if (!strncmp(hdr.oem_id, v_info->oem_id, ACPI_OEM_ID_SIZE) &&
>> +            intel_pstate_has_acpi_ppc())
> 
> We need try this on a few platforms to make sure this patch doesn't break the
> HP platforms that may or may not need this driver, depending on the BIOS settings.
> 
> I don't suppose you tested on a ProLiant too?
I  am very glad you could catch this though I missed to CC you.
The code only touch oem_id 'ORACLE' platform, wouldn't break HP's.
But it is good you could test on ProLiant.(not find one around yet)

Thanks,
Ethan

> 
> -- ljk
> 
>> +            return true;
>>    }
>> 
>>    return false;
>> }
>> #else /* CONFIG_ACPI not enabled */
>> static inline bool intel_pstate_platform_pwr_mgmt_exists(void) { return false; }
>> +static inline bool intel_pstate_has_acpi_ppc(void) { return false; }
>> #endif /* CONFIG_ACPI */
>> 
>> static int __init intel_pstate_init(void)
>> 
> 

Re: [PATCH 3/3] intel_pstate: add module and kernel command line parameter to ignore ACPI _PPC

[ethan]

> 在 2014年11月20日，03:05，Kristen Carlson Accardi <kristen@linux.intel.com> 写道：
> 
> On Tue, 18 Nov 2014 17:37:06 +0900
> Ethan Zhao <ethan.zhao@oracle.com> wrote:
> 
>> Add kernel command line parameter
>> intel_pstate = ignore_acpi_ppc
>> and module parameter
>> ignore_acpi_ppc = 1
>> to allow driver to ignore the ACPI _PPC existence even for Sun x86 servers.
>> These parameter could be used for debug\test\workaround etc purpose.
>> 
>> Signed-off-by: Ethan Zhao <ethan.zhao@oracle.com>
> 
> What if we used a more generic parameter like "force" that would bypass
> any vendor specific checks and just load anyway?  This way we don't have
> to add new parameters everything some new thing shows up that we want to
> ignore.
> 
To be honest, I prefer more generic parameter. But to avoid the possible negative affect 
To another vendors. I back to this way.

Thanks,
Ethan
>> ---
>> Documentation/kernel-parameters.txt | 3 +++
>> drivers/cpufreq/intel_pstate.c      | 8 +++++++-
>> 2 files changed, 10 insertions(+), 1 deletion(-)
>> 
>> diff --git a/Documentation/kernel-parameters.txt b/Documentation/kernel-parameters.txt
>> index 4c81a86..f502b85 100644
>> --- a/Documentation/kernel-parameters.txt
>> +++ b/Documentation/kernel-parameters.txt
>> @@ -1446,6 +1446,9 @@ bytes respectively. Such letter suffixes can also be entirely omitted.
>>               disable
>>                 Do not enable intel_pstate as the default
>>                 scaling driver for the supported processors
>> +               ignore_acpi_ppc
>> +             Ignore the existence of ACPI method _PPC for Sun x86 servers
>> +             and load the driver.
>> 
>>    intremap=    [X86-64, Intel-IOMMU]
>>            on    enable Interrupt Remapping (default)
>> diff --git a/drivers/cpufreq/intel_pstate.c b/drivers/cpufreq/intel_pstate.c
>> index 7c5faea..388387b 100644
>> --- a/drivers/cpufreq/intel_pstate.c
>> +++ b/drivers/cpufreq/intel_pstate.c
>> @@ -870,6 +870,7 @@ static struct cpufreq_driver intel_pstate_driver = {
>> };
>> 
>> static int __initdata no_load;
>> +static unsigned int  ignore_acpi_ppc;
>> 
>> static int intel_pstate_msrs_not_valid(void)
>> {
>> @@ -990,7 +991,7 @@ static bool intel_pstate_platform_pwr_mgmt_exists(void)
>>            intel_pstate_no_acpi_pss())
>>            return true;
>>        if (!strncmp(hdr.oem_id, v_info->oem_id, ACPI_OEM_ID_SIZE) &&
>> -            intel_pstate_has_acpi_ppc())
>> +            intel_pstate_has_acpi_ppc() && !ignore_acpi_ppc)
>>            return true;
>>    }
>> 
>> @@ -1066,11 +1067,16 @@ static int __init intel_pstate_setup(char *str)
>> 
>>    if (!strcmp(str, "disable"))
>>        no_load = 1;
>> +    if (!strcmp(str, "ignore_acpi_ppc"))
>> +        ignore_acpi_ppc = 1;
>>    return 0;
>> }
>> early_param("intel_pstate", intel_pstate_setup);
>> #endif
>> 
>> +module_param(ignore_acpi_ppc, uint, 0644);
>> +MODULE_PARM_DESC(ignore_acpi_ppc,
>> +    "value 0 or non-zero. non-zero -> ignore ACPI _PPC and load this driver");
>> MODULE_AUTHOR("Dirk Brandewie <dirk.j.brandewie@intel.com>");
>> MODULE_DESCRIPTION("'intel_pstate' - P state driver Intel Core processors");
>> MODULE_LICENSE("GPL");
> 

Re: [PATCH 2/3] intel_pstate: allow driver to be built as a module

[ethan]

Kristen,
> 在 2014年11月20日，02:58，Kristen Carlson Accardi <kristen@linux.intel.com> 写道：
> 
> On Tue, 18 Nov 2014 17:37:05 +0900
> Ethan Zhao <ethan.zhao@oracle.com> wrote:
> 
>> From: Brian Maly <brian.maly@oracle.com>
>> 
>> To provide the flexibility of module, allow this driver to
>> be configured and built as a module.
>> 
>> Signed-off-by: Brian Maly <brian.maly@oracle.com>
>> Signed-off-by: Ethan Zhao <ethan.zhao@oracle.com>
> 
> I believe the entire concept of being able to use intel_pstate as a
> module just isn't going to work.  There are load order issues - and
> additionally the driver doesn't clean up after itself in any way.
> 
Roger.

Thanks,
Ethan
>> ---
>> drivers/cpufreq/Kconfig.x86    | 2 +-
>> drivers/cpufreq/intel_pstate.c | 6 ++++++
>> 2 files changed, 7 insertions(+), 1 deletion(-)
>> 
>> diff --git a/drivers/cpufreq/Kconfig.x86 b/drivers/cpufreq/Kconfig.x86
>> index 89ae88f..94c9e6b 100644
>> --- a/drivers/cpufreq/Kconfig.x86
>> +++ b/drivers/cpufreq/Kconfig.x86
>> @@ -3,7 +3,7 @@
>> #
>> 
>> config X86_INTEL_PSTATE
>> -       bool "Intel P state control"
>> +       tristate "Intel P state control"
>>        depends on X86
>>        help
>>           This driver provides a P state for Intel core processors.
>> diff --git a/drivers/cpufreq/intel_pstate.c b/drivers/cpufreq/intel_pstate.c
>> index 5498eb0..7c5faea 100644
>> --- a/drivers/cpufreq/intel_pstate.c
>> +++ b/drivers/cpufreq/intel_pstate.c
>> @@ -590,7 +590,9 @@ static void intel_pstate_set_pstate(struct cpudata *cpu, int pstate)
>>    if (pstate == cpu->pstate.current_pstate)
>>        return;
>> 
>> +#ifndef MODULE
>>    trace_cpu_frequency(pstate * cpu->pstate.scaling, cpu->cpu);
>> +#endif
>> 
>>    cpu->pstate.current_pstate = pstate;
>> 
>> @@ -705,12 +707,14 @@ static void intel_pstate_timer_func(unsigned long __data)
>> 
>>    intel_pstate_adjust_busy_pstate(cpu);
>> 
>> +#ifndef MODULE
>>    trace_pstate_sample(fp_toint(sample->core_pct_busy),
>>            fp_toint(intel_pstate_get_scaled_busy(cpu)),
>>            cpu->pstate.current_pstate,
>>            sample->mperf,
>>            sample->aperf,
>>            sample->freq);
>> +#endif
>> 
>>    intel_pstate_set_sample_time(cpu);
>> }
>> @@ -1054,6 +1058,7 @@ out:
>> }
>> device_initcall(intel_pstate_init);
>> 
>> +#ifndef MODULE
>> static int __init intel_pstate_setup(char *str)
>> {
>>    if (!str)
>> @@ -1064,6 +1069,7 @@ static int __init intel_pstate_setup(char *str)
>>    return 0;
>> }
>> early_param("intel_pstate", intel_pstate_setup);
>> +#endif
>> 
>> MODULE_AUTHOR("Dirk Brandewie <dirk.j.brandewie@intel.com>");
>> MODULE_DESCRIPTION("'intel_pstate' - P state driver Intel Core processors");
> 

Re: [PATCH 1/3] intel_pstate: skip the driver if Sun server has ACPI _PPC method

[ethan]

Dirk,

> 在 2014年11月19日，22:59，Dirk Brandewie <dirk.brandewie@gmail.com> 写道：
> 
>> On 11/18/2014 12:37 AM, Ethan Zhao wrote:
>> Oracle Sun X86 servers have dynamic power capping capability that works via
>> ACPI _PPC method etc, so skip loading this driver if Sun server has ACPI _PPC
>> enabled.
>> 
>> Signed-off-by: Ethan Zhao <ethan.zhao@oracle.com>
>> ---
>>  drivers/cpufreq/intel_pstate.c | 20 ++++++++++++++++++++
>>  1 file changed, 20 insertions(+)
>> 
>> diff --git a/drivers/cpufreq/intel_pstate.c b/drivers/cpufreq/intel_pstate.c
>> index 27bb6d3..5498eb0 100644
>> --- a/drivers/cpufreq/intel_pstate.c
>> +++ b/drivers/cpufreq/intel_pstate.c
>> @@ -943,6 +943,21 @@ static bool intel_pstate_no_acpi_pss(void)
>>      return true;
>>  }
>> 
>> +static bool intel_pstate_has_acpi_ppc(void)
>> +{
>> +    int i;
>> +
>> +    for_each_possible_cpu(i) {
>> +        struct acpi_processor *pr = per_cpu(processors, i);
>> +
>> +        if (!pr)
>> +            continue;
>> +        if (acpi_has_method(pr->handle, "_PPC"))
>> +            return true;
>> +    }
>> +    return false;
>> +}
>> +
>>  struct hw_vendor_info {
>>      u16  valid;
>>      char oem_id[ACPI_OEM_ID_SIZE];
>> @@ -952,6 +967,7 @@ struct hw_vendor_info {
>>  /* Hardware vendor-specific info that has its own power management modes */
>>  static struct hw_vendor_info vendor_info[] = {
>>      {1, "HP    ", "ProLiant"},
>> +    {1, "ORACLE", ""},
>>      {0, "", ""},
>>  };
> 
> Does this apply to ALL oracle systems?
Yes, as I know, all recent oracle x86 servers have power capping functions.
> 
> Is the presence or absense of the _PPC method configurable in the oracle BIOS?
There is no option in BIOS to enable or disable _PPC. But they configure power limit
Functions via SP(service processors).

Thanks,
Ethan
> 
>> @@ -969,12 +985,16 @@ static bool intel_pstate_platform_pwr_mgmt_exists(void)
>>              !strncmp(hdr.oem_table_id, v_info->oem_table_id, ACPI_OEM_TABLE_ID_SIZE) &&
>>              intel_pstate_no_acpi_pss())
>>              return true;
>> +        if (!strncmp(hdr.oem_id, v_info->oem_id, ACPI_OEM_ID_SIZE) &&
>> +            intel_pstate_has_acpi_ppc())
>> +            return true;
>>      }
>> 
>>      return false;
>>  }
>>  #else /* CONFIG_ACPI not enabled */
>>  static inline bool intel_pstate_platform_pwr_mgmt_exists(void) { return false; }
>> +static inline bool intel_pstate_has_acpi_ppc(void) { return false; }
>>  #endif /* CONFIG_ACPI */
>> 
>>  static int __init intel_pstate_init(void)
> 

Re: [PATCH 1/3] intel_pstate: skip the driver if Sun server has ACPI _PPC method

[Dirk Brandewie]

On 11/19/2014 12:22 PM, Linda Knippers wrote:
> On 11/18/2014 3:37 AM, Ethan Zhao wrote:
>> Oracle Sun X86 servers have dynamic power capping capability that works via
>> ACPI _PPC method etc, so skip loading this driver if Sun server has ACPI _PPC
>> enabled.
>>
>> Signed-off-by: Ethan Zhao <ethan.zhao@oracle.com>
>> ---
>>   drivers/cpufreq/intel_pstate.c | 20 ++++++++++++++++++++
>>   1 file changed, 20 insertions(+)
>>
>> diff --git a/drivers/cpufreq/intel_pstate.c b/drivers/cpufreq/intel_pstate.c
>> index 27bb6d3..5498eb0 100644
>> --- a/drivers/cpufreq/intel_pstate.c
>> +++ b/drivers/cpufreq/intel_pstate.c
>> @@ -943,6 +943,21 @@ static bool intel_pstate_no_acpi_pss(void)
>>   	return true;
>>   }
>>
>> +static bool intel_pstate_has_acpi_ppc(void)
>> +{
>> +	int i;
>> +
>> +	for_each_possible_cpu(i) {
>> +		struct acpi_processor *pr = per_cpu(processors, i);
>> +
>> +		if (!pr)
>> +			continue;
>> +		if (acpi_has_method(pr->handle, "_PPC"))
>> +			return true;
>> +	}
>> +	return false;
>> +}
>> +
>>   struct hw_vendor_info {
>>   	u16  valid;
>>   	char oem_id[ACPI_OEM_ID_SIZE];
>> @@ -952,6 +967,7 @@ struct hw_vendor_info {
>>   /* Hardware vendor-specific info that has its own power management modes */
>>   static struct hw_vendor_info vendor_info[] = {
>>   	{1, "HP    ", "ProLiant"},
>> +	{1, "ORACLE", ""},
>>   	{0, "", ""},
>>   };
>>
>> @@ -969,12 +985,16 @@ static bool intel_pstate_platform_pwr_mgmt_exists(void)
>>   		    !strncmp(hdr.oem_table_id, v_info->oem_table_id, ACPI_OEM_TABLE_ID_SIZE) &&
>>   		    intel_pstate_no_acpi_pss())
>>   			return true;
>> +		if (!strncmp(hdr.oem_id, v_info->oem_id, ACPI_OEM_ID_SIZE) &&
>> +			intel_pstate_has_acpi_ppc())
>
> We need try this on a few platforms to make sure this patch doesn't break the
> HP platforms that may or may not need this driver, depending on the BIOS settings.
>

It looks like HP systems would get swept up in this check too if they have _PPC

What about extending the hw_vendor_info struct to include whether _PSS or
_PPC should be done for the platform since it appears that oracle and HP
have implemented similar functionality using two different methods.


> I don't suppose you tested on a ProLiant too?
>
> -- ljk
>
>> +			return true;
>>   	}
>>
>>   	return false;
>>   }
>>   #else /* CONFIG_ACPI not enabled */
>>   static inline bool intel_pstate_platform_pwr_mgmt_exists(void) { return false; }
>> +static inline bool intel_pstate_has_acpi_ppc(void) { return false; }
>>   #endif /* CONFIG_ACPI */
>>
>>   static int __init intel_pstate_init(void)
>>
>

Re: [PATCH 3/3] intel_pstate: add module and kernel command line parameter to ignore ACPI _PPC

[Kristen Carlson Accardi]

On Thu, 20 Nov 2014 08:57:34 +0800
ethan <ethan.kernel@gmail.com> wrote:

> 
> 
> > 在 2014年11月20日，03:05，Kristen Carlson Accardi <kristen@linux.intel.com> 写道：
> > 
> > On Tue, 18 Nov 2014 17:37:06 +0900
> > Ethan Zhao <ethan.zhao@oracle.com> wrote:
> > 
> >> Add kernel command line parameter
> >> intel_pstate = ignore_acpi_ppc
> >> and module parameter
> >> ignore_acpi_ppc = 1
> >> to allow driver to ignore the ACPI _PPC existence even for Sun x86 servers.
> >> These parameter could be used for debug\test\workaround etc purpose.
> >> 
> >> Signed-off-by: Ethan Zhao <ethan.zhao@oracle.com>
> > 
> > What if we used a more generic parameter like "force" that would bypass
> > any vendor specific checks and just load anyway?  This way we don't have
> > to add new parameters everything some new thing shows up that we want to
> > ignore.
> > 
> To be honest, I prefer more generic parameter. But to avoid the possible negative affect 
> To another vendors. I back to this way.

Well, your parameter can still impact other vendors as it is.  it
is pretty typical to assume that using a parameter like "force" means
you know what you are doing and accept the risks.  Especially if its
documented as such.

> 
> Thanks,
> Ethan
> >> ---
> >> Documentation/kernel-parameters.txt | 3 +++
> >> drivers/cpufreq/intel_pstate.c      | 8 +++++++-
> >> 2 files changed, 10 insertions(+), 1 deletion(-)
> >> 
> >> diff --git a/Documentation/kernel-parameters.txt b/Documentation/kernel-parameters.txt
> >> index 4c81a86..f502b85 100644
> >> --- a/Documentation/kernel-parameters.txt
> >> +++ b/Documentation/kernel-parameters.txt
> >> @@ -1446,6 +1446,9 @@ bytes respectively. Such letter suffixes can also be entirely omitted.
> >>               disable
> >>                 Do not enable intel_pstate as the default
> >>                 scaling driver for the supported processors
> >> +               ignore_acpi_ppc
> >> +             Ignore the existence of ACPI method _PPC for Sun x86 servers
> >> +             and load the driver.
> >> 
> >>    intremap=    [X86-64, Intel-IOMMU]
> >>            on    enable Interrupt Remapping (default)
> >> diff --git a/drivers/cpufreq/intel_pstate.c b/drivers/cpufreq/intel_pstate.c
> >> index 7c5faea..388387b 100644
> >> --- a/drivers/cpufreq/intel_pstate.c
> >> +++ b/drivers/cpufreq/intel_pstate.c
> >> @@ -870,6 +870,7 @@ static struct cpufreq_driver intel_pstate_driver = {
> >> };
> >> 
> >> static int __initdata no_load;
> >> +static unsigned int  ignore_acpi_ppc;
> >> 
> >> static int intel_pstate_msrs_not_valid(void)
> >> {
> >> @@ -990,7 +991,7 @@ static bool intel_pstate_platform_pwr_mgmt_exists(void)
> >>            intel_pstate_no_acpi_pss())
> >>            return true;
> >>        if (!strncmp(hdr.oem_id, v_info->oem_id, ACPI_OEM_ID_SIZE) &&
> >> -            intel_pstate_has_acpi_ppc())
> >> +            intel_pstate_has_acpi_ppc() && !ignore_acpi_ppc)
> >>            return true;
> >>    }
> >> 
> >> @@ -1066,11 +1067,16 @@ static int __init intel_pstate_setup(char *str)
> >> 
> >>    if (!strcmp(str, "disable"))
> >>        no_load = 1;
> >> +    if (!strcmp(str, "ignore_acpi_ppc"))
> >> +        ignore_acpi_ppc = 1;
> >>    return 0;
> >> }
> >> early_param("intel_pstate", intel_pstate_setup);
> >> #endif
> >> 
> >> +module_param(ignore_acpi_ppc, uint, 0644);
> >> +MODULE_PARM_DESC(ignore_acpi_ppc,
> >> +    "value 0 or non-zero. non-zero -> ignore ACPI _PPC and load this driver");
> >> MODULE_AUTHOR("Dirk Brandewie <dirk.j.brandewie@intel.com>");
> >> MODULE_DESCRIPTION("'intel_pstate' - P state driver Intel Core processors");
> >> MODULE_LICENSE("GPL");
> > 

Re: [PATCH 1/3] intel_pstate: skip the driver if Sun server has ACPI _PPC method

[ethan zhao]

Dirk,

On 2014/11/21 0:50, Dirk Brandewie wrote:
> On 11/19/2014 12:22 PM, Linda Knippers wrote:
>> On 11/18/2014 3:37 AM, Ethan Zhao wrote:
>>> Oracle Sun X86 servers have dynamic power capping capability that 
>>> works via
>>> ACPI _PPC method etc, so skip loading this driver if Sun server has 
>>> ACPI _PPC
>>> enabled.
>>>
>>> Signed-off-by: Ethan Zhao <ethan.zhao@oracle.com>
>>> ---
>>>   drivers/cpufreq/intel_pstate.c | 20 ++++++++++++++++++++
>>>   1 file changed, 20 insertions(+)
>>>
>>> diff --git a/drivers/cpufreq/intel_pstate.c 
>>> b/drivers/cpufreq/intel_pstate.c
>>> index 27bb6d3..5498eb0 100644
>>> --- a/drivers/cpufreq/intel_pstate.c
>>> +++ b/drivers/cpufreq/intel_pstate.c
>>> @@ -943,6 +943,21 @@ static bool intel_pstate_no_acpi_pss(void)
>>>       return true;
>>>   }
>>>
>>> +static bool intel_pstate_has_acpi_ppc(void)
>>> +{
>>> +    int i;
>>> +
>>> +    for_each_possible_cpu(i) {
>>> +        struct acpi_processor *pr = per_cpu(processors, i);
>>> +
>>> +        if (!pr)
>>> +            continue;
>>> +        if (acpi_has_method(pr->handle, "_PPC"))
>>> +            return true;
>>> +    }
>>> +    return false;
>>> +}
>>> +
>>>   struct hw_vendor_info {
>>>       u16  valid;
>>>       char oem_id[ACPI_OEM_ID_SIZE];
>>> @@ -952,6 +967,7 @@ struct hw_vendor_info {
>>>   /* Hardware vendor-specific info that has its own power management 
>>> modes */
>>>   static struct hw_vendor_info vendor_info[] = {
>>>       {1, "HP    ", "ProLiant"},
>>> +    {1, "ORACLE", ""},
>>>       {0, "", ""},
>>>   };
>>>
>>> @@ -969,12 +985,16 @@ static bool 
>>> intel_pstate_platform_pwr_mgmt_exists(void)
>>>               !strncmp(hdr.oem_table_id, v_info->oem_table_id, 
>>> ACPI_OEM_TABLE_ID_SIZE) &&
>>>               intel_pstate_no_acpi_pss())
>>>               return true;
>>> +        if (!strncmp(hdr.oem_id, v_info->oem_id, ACPI_OEM_ID_SIZE) &&
>>> +            intel_pstate_has_acpi_ppc())
>>
>> We need try this on a few platforms to make sure this patch doesn't 
>> break the
>> HP platforms that may or may not need this driver, depending on the 
>> BIOS settings.
>>
>
> It looks like HP systems would get swept up in this check too if they 
> have _PPC
    No , this patch checks the oem_id against 'ORACLE" first, will not 
affect other vendors
   even they have _PPC.

>
> What about extending the hw_vendor_info struct to include whether _PSS or
  Except refer to ACPI DSDT, I don't know how to fill such info.
> _PPC should be done for the platform since it appears that oracle and HP
> have implemented similar functionality using two different methods.
   Maybe Linda could answer this whether HP also has _PPC and should be 
wept out.
   But that doesn't happen with on the same box at the same time.

   Thanks,
   Ethan
>
>
>> I don't suppose you tested on a ProLiant too?
>>
>> -- ljk
>>
>>> +            return true;
>>>       }
>>>
>>>       return false;
>>>   }
>>>   #else /* CONFIG_ACPI not enabled */
>>>   static inline bool intel_pstate_platform_pwr_mgmt_exists(void) { 
>>> return false; }
>>> +static inline bool intel_pstate_has_acpi_ppc(void) { return false; }
>>>   #endif /* CONFIG_ACPI */
>>>
>>>   static int __init intel_pstate_init(void)
>>>
>>
>

Re: [PATCH 3/3] intel_pstate: add module and kernel command line parameter to ignore ACPI _PPC

[Ethan Zhao]

Kristen,
   Whatever I would like there is a way to load intel_pstate and give
it a try even it does not support all the PM features.
   I think 'force' is OK.
Linda,
  Do you like it ? if the 'intel_pstate=force' would force the driver
to be loaded on to HP too ?

Thanks,
Ethan

On Fri, Nov 21, 2014 at 5:23 AM, Kristen Carlson Accardi
<kristen@linux.intel.com> wrote:
> On Thu, 20 Nov 2014 08:57:34 +0800
> ethan <ethan.kernel@gmail.com> wrote:
>
>>
>>
>> > 在 2014年11月20日，03:05，Kristen Carlson Accardi <kristen@linux.intel.com> 写道：
>> >
>> > On Tue, 18 Nov 2014 17:37:06 +0900
>> > Ethan Zhao <ethan.zhao@oracle.com> wrote:
>> >
>> >> Add kernel command line parameter
>> >> intel_pstate = ignore_acpi_ppc
>> >> and module parameter
>> >> ignore_acpi_ppc = 1
>> >> to allow driver to ignore the ACPI _PPC existence even for Sun x86 servers.
>> >> These parameter could be used for debug\test\workaround etc purpose.
>> >>
>> >> Signed-off-by: Ethan Zhao <ethan.zhao@oracle.com>
>> >
>> > What if we used a more generic parameter like "force" that would bypass
>> > any vendor specific checks and just load anyway?  This way we don't have
>> > to add new parameters everything some new thing shows up that we want to
>> > ignore.
>> >
>> To be honest, I prefer more generic parameter. But to avoid the possible negative affect
>> To another vendors. I back to this way.
>
> Well, your parameter can still impact other vendors as it is.  it
> is pretty typical to assume that using a parameter like "force" means
> you know what you are doing and accept the risks.  Especially if its
> documented as such.
>
>>
>> Thanks,
>> Ethan
>> >> ---
>> >> Documentation/kernel-parameters.txt | 3 +++
>> >> drivers/cpufreq/intel_pstate.c      | 8 +++++++-
>> >> 2 files changed, 10 insertions(+), 1 deletion(-)
>> >>
>> >> diff --git a/Documentation/kernel-parameters.txt b/Documentation/kernel-parameters.txt
>> >> index 4c81a86..f502b85 100644
>> >> --- a/Documentation/kernel-parameters.txt
>> >> +++ b/Documentation/kernel-parameters.txt
>> >> @@ -1446,6 +1446,9 @@ bytes respectively. Such letter suffixes can also be entirely omitted.
>> >>               disable
>> >>                 Do not enable intel_pstate as the default
>> >>                 scaling driver for the supported processors
>> >> +               ignore_acpi_ppc
>> >> +             Ignore the existence of ACPI method _PPC for Sun x86 servers
>> >> +             and load the driver.
>> >>
>> >>    intremap=    [X86-64, Intel-IOMMU]
>> >>            on    enable Interrupt Remapping (default)
>> >> diff --git a/drivers/cpufreq/intel_pstate.c b/drivers/cpufreq/intel_pstate.c
>> >> index 7c5faea..388387b 100644
>> >> --- a/drivers/cpufreq/intel_pstate.c
>> >> +++ b/drivers/cpufreq/intel_pstate.c
>> >> @@ -870,6 +870,7 @@ static struct cpufreq_driver intel_pstate_driver = {
>> >> };
>> >>
>> >> static int __initdata no_load;
>> >> +static unsigned int  ignore_acpi_ppc;
>> >>
>> >> static int intel_pstate_msrs_not_valid(void)
>> >> {
>> >> @@ -990,7 +991,7 @@ static bool intel_pstate_platform_pwr_mgmt_exists(void)
>> >>            intel_pstate_no_acpi_pss())
>> >>            return true;
>> >>        if (!strncmp(hdr.oem_id, v_info->oem_id, ACPI_OEM_ID_SIZE) &&
>> >> -            intel_pstate_has_acpi_ppc())
>> >> +            intel_pstate_has_acpi_ppc() && !ignore_acpi_ppc)
>> >>            return true;
>> >>    }
>> >>
>> >> @@ -1066,11 +1067,16 @@ static int __init intel_pstate_setup(char *str)
>> >>
>> >>    if (!strcmp(str, "disable"))
>> >>        no_load = 1;
>> >> +    if (!strcmp(str, "ignore_acpi_ppc"))
>> >> +        ignore_acpi_ppc = 1;
>> >>    return 0;
>> >> }
>> >> early_param("intel_pstate", intel_pstate_setup);
>> >> #endif
>> >>
>> >> +module_param(ignore_acpi_ppc, uint, 0644);
>> >> +MODULE_PARM_DESC(ignore_acpi_ppc,
>> >> +    "value 0 or non-zero. non-zero -> ignore ACPI _PPC and load this driver");
>> >> MODULE_AUTHOR("Dirk Brandewie <dirk.j.brandewie@intel.com>");
>> >> MODULE_DESCRIPTION("'intel_pstate' - P state driver Intel Core processors");
>> >> MODULE_LICENSE("GPL");
>> >
>

Re: [PATCH 1/3] intel_pstate: skip the driver if Sun server has ACPI _PPC method

[Linda Knippers]

On 11/20/2014 07:37 PM, ethan zhao wrote:
> Dirk,
> 
> On 2014/11/21 0:50, Dirk Brandewie wrote:
>> On 11/19/2014 12:22 PM, Linda Knippers wrote:
>>> On 11/18/2014 3:37 AM, Ethan Zhao wrote:
>>>> Oracle Sun X86 servers have dynamic power capping capability that
>>>> works via
>>>> ACPI _PPC method etc, so skip loading this driver if Sun server has
>>>> ACPI _PPC
>>>> enabled.
>>>>
>>>> Signed-off-by: Ethan Zhao <ethan.zhao@oracle.com>
>>>> ---
>>>>   drivers/cpufreq/intel_pstate.c | 20 ++++++++++++++++++++
>>>>   1 file changed, 20 insertions(+)
>>>>
>>>> diff --git a/drivers/cpufreq/intel_pstate.c
>>>> b/drivers/cpufreq/intel_pstate.c
>>>> index 27bb6d3..5498eb0 100644
>>>> --- a/drivers/cpufreq/intel_pstate.c
>>>> +++ b/drivers/cpufreq/intel_pstate.c
>>>> @@ -943,6 +943,21 @@ static bool intel_pstate_no_acpi_pss(void)
>>>>       return true;
>>>>   }
>>>>
>>>> +static bool intel_pstate_has_acpi_ppc(void)
>>>> +{
>>>> +    int i;
>>>> +
>>>> +    for_each_possible_cpu(i) {
>>>> +        struct acpi_processor *pr = per_cpu(processors, i);
>>>> +
>>>> +        if (!pr)
>>>> +            continue;
>>>> +        if (acpi_has_method(pr->handle, "_PPC"))
>>>> +            return true;
>>>> +    }
>>>> +    return false;
>>>> +}
>>>> +
>>>>   struct hw_vendor_info {
>>>>       u16  valid;
>>>>       char oem_id[ACPI_OEM_ID_SIZE];
>>>> @@ -952,6 +967,7 @@ struct hw_vendor_info {
>>>>   /* Hardware vendor-specific info that has its own power management
>>>> modes */
>>>>   static struct hw_vendor_info vendor_info[] = {
>>>>       {1, "HP    ", "ProLiant"},
>>>> +    {1, "ORACLE", ""},
>>>>       {0, "", ""},
>>>>   };
>>>>
>>>> @@ -969,12 +985,16 @@ static bool
>>>> intel_pstate_platform_pwr_mgmt_exists(void)
>>>>               !strncmp(hdr.oem_table_id, v_info->oem_table_id,
>>>> ACPI_OEM_TABLE_ID_SIZE) &&
>>>>               intel_pstate_no_acpi_pss())
>>>>               return true;
>>>> +        if (!strncmp(hdr.oem_id, v_info->oem_id, ACPI_OEM_ID_SIZE) &&
>>>> +            intel_pstate_has_acpi_ppc())
>>>
>>> We need try this on a few platforms to make sure this patch doesn't
>>> break the
>>> HP platforms that may or may not need this driver, depending on the
>>> BIOS settings.
>>>
>>
>> It looks like HP systems would get swept up in this check too if they
>> have _PPC

Right.  This patch breaks HP ProLiant platforms when they are
configured to have the OS do power management.  In that case,
the firmware exposes _PPC information.

>    No , this patch checks the oem_id against 'ORACLE" first, will not
> affect other vendors even they have _PPC.

I don't think that's how your code works.  This patch will match any
vendor that is in the table, not just "ORACLE".
> 
>>
>> What about extending the hw_vendor_info struct to include whether _PSS or
>  Except refer to ACPI DSDT, I don't know how to fill such info.
>> _PPC should be done for the platform since it appears that oracle and HP
>> have implemented similar functionality using two different methods.
>   Maybe Linda could answer this whether HP also has _PPC and should be
> wept out.
>   But that doesn't happen with on the same box at the same time.

I don't know how an Oracle box works but on a ProLiant, customers can
choose to have platform power management or OS power management.
When the platform is managing the power, we don't provide the _PSS
information.  Since our oem information is in the table and there is
no _PSS, the intel_pstate driver doesn't stay loaded.  That's what we want.

When the platform configured to have the OS do the power management,
the firmware has _PSS and _PPC and we want the intel_pstate driver,
That's what your patch breaks.  With your patch, the driver won't
stay loaded because our platform is in the table and the check for
_PPC passes.

How does an Oracle box work?

-- ljk

> 
>   Thanks,
>   Ethan
>>
>>
>>> I don't suppose you tested on a ProLiant too?
>>>
>>> -- ljk
>>>
>>>> +            return true;
>>>>       }
>>>>
>>>>       return false;
>>>>   }
>>>>   #else /* CONFIG_ACPI not enabled */
>>>>   static inline bool intel_pstate_platform_pwr_mgmt_exists(void) {
>>>> return false; }
>>>> +static inline bool intel_pstate_has_acpi_ppc(void) { return false; }
>>>>   #endif /* CONFIG_ACPI */
>>>>
>>>>   static int __init intel_pstate_init(void)
>>>>
>>>
>>
> 
> -- 
> To unsubscribe from this list: send the line "unsubscribe linux-pm" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 3/3] intel_pstate: add module and kernel command line parameter to ignore ACPI _PPC

[Linda Knippers]

On 11/20/2014 10:07 PM, Ethan Zhao wrote:
> Kristen,
>    Whatever I would like there is a way to load intel_pstate and give
> it a try even it does not support all the PM features.
>    I think 'force' is OK.
> Linda,
>   Do you like it ? if the 'intel_pstate=force' would force the driver
> to be loaded on to HP too ?

I'd prefer that it didn't.  If you force the intel_pstate driver when
the platform thinks it's doing power management, then the OS and the
firmware are trying to manage the power at the same time.  That's a
mess.  If you want that for testing or debugging, what are you actually
testing or debugging?  On an Oracle box, the firmware wouldn't stop
doing whatever it's doing just because the intel_pstate driver is
loaded, would it?

I also wonder what it means to "force" the intel_pstate driver
on systems with processors that aren't supported by the intel_pstate
driver.  It wouldn't really be forced, would it?

-- ljk

> 
> Thanks,
> Ethan
> 
> On Fri, Nov 21, 2014 at 5:23 AM, Kristen Carlson Accardi
> <kristen@linux.intel.com> wrote:
>> On Thu, 20 Nov 2014 08:57:34 +0800
>> ethan <ethan.kernel@gmail.com> wrote:
>>
>>>
>>>
>>>> 在 2014年11月20日，03:05，Kristen Carlson Accardi <kristen@linux.intel.com> 写道：
>>>>
>>>> On Tue, 18 Nov 2014 17:37:06 +0900
>>>> Ethan Zhao <ethan.zhao@oracle.com> wrote:
>>>>
>>>>> Add kernel command line parameter
>>>>> intel_pstate = ignore_acpi_ppc
>>>>> and module parameter
>>>>> ignore_acpi_ppc = 1
>>>>> to allow driver to ignore the ACPI _PPC existence even for Sun x86 servers.
>>>>> These parameter could be used for debug\test\workaround etc purpose.
>>>>>
>>>>> Signed-off-by: Ethan Zhao <ethan.zhao@oracle.com>
>>>>
>>>> What if we used a more generic parameter like "force" that would bypass
>>>> any vendor specific checks and just load anyway?  This way we don't have
>>>> to add new parameters everything some new thing shows up that we want to
>>>> ignore.
>>>>
>>> To be honest, I prefer more generic parameter. But to avoid the possible negative affect
>>> To another vendors. I back to this way.
>>
>> Well, your parameter can still impact other vendors as it is.  it
>> is pretty typical to assume that using a parameter like "force" means
>> you know what you are doing and accept the risks.  Especially if its
>> documented as such.
>>
>>>
>>> Thanks,
>>> Ethan
>>>>> ---
>>>>> Documentation/kernel-parameters.txt | 3 +++
>>>>> drivers/cpufreq/intel_pstate.c      | 8 +++++++-
>>>>> 2 files changed, 10 insertions(+), 1 deletion(-)
>>>>>
>>>>> diff --git a/Documentation/kernel-parameters.txt b/Documentation/kernel-parameters.txt
>>>>> index 4c81a86..f502b85 100644
>>>>> --- a/Documentation/kernel-parameters.txt
>>>>> +++ b/Documentation/kernel-parameters.txt
>>>>> @@ -1446,6 +1446,9 @@ bytes respectively. Such letter suffixes can also be entirely omitted.
>>>>>               disable
>>>>>                 Do not enable intel_pstate as the default
>>>>>                 scaling driver for the supported processors
>>>>> +               ignore_acpi_ppc
>>>>> +             Ignore the existence of ACPI method _PPC for Sun x86 servers
>>>>> +             and load the driver.
>>>>>
>>>>>    intremap=    [X86-64, Intel-IOMMU]
>>>>>            on    enable Interrupt Remapping (default)
>>>>> diff --git a/drivers/cpufreq/intel_pstate.c b/drivers/cpufreq/intel_pstate.c
>>>>> index 7c5faea..388387b 100644
>>>>> --- a/drivers/cpufreq/intel_pstate.c
>>>>> +++ b/drivers/cpufreq/intel_pstate.c
>>>>> @@ -870,6 +870,7 @@ static struct cpufreq_driver intel_pstate_driver = {
>>>>> };
>>>>>
>>>>> static int __initdata no_load;
>>>>> +static unsigned int  ignore_acpi_ppc;
>>>>>
>>>>> static int intel_pstate_msrs_not_valid(void)
>>>>> {
>>>>> @@ -990,7 +991,7 @@ static bool intel_pstate_platform_pwr_mgmt_exists(void)
>>>>>            intel_pstate_no_acpi_pss())
>>>>>            return true;
>>>>>        if (!strncmp(hdr.oem_id, v_info->oem_id, ACPI_OEM_ID_SIZE) &&
>>>>> -            intel_pstate_has_acpi_ppc())
>>>>> +            intel_pstate_has_acpi_ppc() && !ignore_acpi_ppc)
>>>>>            return true;
>>>>>    }
>>>>>
>>>>> @@ -1066,11 +1067,16 @@ static int __init intel_pstate_setup(char *str)
>>>>>
>>>>>    if (!strcmp(str, "disable"))
>>>>>        no_load = 1;
>>>>> +    if (!strcmp(str, "ignore_acpi_ppc"))
>>>>> +        ignore_acpi_ppc = 1;
>>>>>    return 0;
>>>>> }
>>>>> early_param("intel_pstate", intel_pstate_setup);
>>>>> #endif
>>>>>
>>>>> +module_param(ignore_acpi_ppc, uint, 0644);
>>>>> +MODULE_PARM_DESC(ignore_acpi_ppc,
>>>>> +    "value 0 or non-zero. non-zero -> ignore ACPI _PPC and load this driver");
>>>>> MODULE_AUTHOR("Dirk Brandewie <dirk.j.brandewie@intel.com>");
>>>>> MODULE_DESCRIPTION("'intel_pstate' - P state driver Intel Core processors");
>>>>> MODULE_LICENSE("GPL");
>>>>
>>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-pm" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

Re: [PATCH 1/3] intel_pstate: skip the driver if Sun server has ACPI _PPC method

[ethan zhao]

Linda,

On 2014/11/21 12:44, Linda Knippers wrote:
>
> On 11/20/2014 07:37 PM, ethan zhao wrote:
>> Dirk,
>>
>> On 2014/11/21 0:50, Dirk Brandewie wrote:
>>> On 11/19/2014 12:22 PM, Linda Knippers wrote:
>>>> On 11/18/2014 3:37 AM, Ethan Zhao wrote:
>>>>> Oracle Sun X86 servers have dynamic power capping capability that
>>>>> works via
>>>>> ACPI _PPC method etc, so skip loading this driver if Sun server has
>>>>> ACPI _PPC
>>>>> enabled.
>>>>>
>>>>> Signed-off-by: Ethan Zhao <ethan.zhao@oracle.com>
>>>>> ---
>>>>>    drivers/cpufreq/intel_pstate.c | 20 ++++++++++++++++++++
>>>>>    1 file changed, 20 insertions(+)
>>>>>
>>>>> diff --git a/drivers/cpufreq/intel_pstate.c
>>>>> b/drivers/cpufreq/intel_pstate.c
>>>>> index 27bb6d3..5498eb0 100644
>>>>> --- a/drivers/cpufreq/intel_pstate.c
>>>>> +++ b/drivers/cpufreq/intel_pstate.c
>>>>> @@ -943,6 +943,21 @@ static bool intel_pstate_no_acpi_pss(void)
>>>>>        return true;
>>>>>    }
>>>>>
>>>>> +static bool intel_pstate_has_acpi_ppc(void)
>>>>> +{
>>>>> +    int i;
>>>>> +
>>>>> +    for_each_possible_cpu(i) {
>>>>> +        struct acpi_processor *pr = per_cpu(processors, i);
>>>>> +
>>>>> +        if (!pr)
>>>>> +            continue;
>>>>> +        if (acpi_has_method(pr->handle, "_PPC"))
>>>>> +            return true;
>>>>> +    }
>>>>> +    return false;
>>>>> +}
>>>>> +
>>>>>    struct hw_vendor_info {
>>>>>        u16  valid;
>>>>>        char oem_id[ACPI_OEM_ID_SIZE];
>>>>> @@ -952,6 +967,7 @@ struct hw_vendor_info {
>>>>>    /* Hardware vendor-specific info that has its own power management
>>>>> modes */
>>>>>    static struct hw_vendor_info vendor_info[] = {
>>>>>        {1, "HP    ", "ProLiant"},
>>>>> +    {1, "ORACLE", ""},
>>>>>        {0, "", ""},
>>>>>    };
>>>>>
>>>>> @@ -969,12 +985,16 @@ static bool
>>>>> intel_pstate_platform_pwr_mgmt_exists(void)
>>>>>                !strncmp(hdr.oem_table_id, v_info->oem_table_id,
>>>>> ACPI_OEM_TABLE_ID_SIZE) &&
>>>>>                intel_pstate_no_acpi_pss())
>>>>>                return true;
>>>>> +        if (!strncmp(hdr.oem_id, v_info->oem_id, ACPI_OEM_ID_SIZE) &&
>>>>> +            intel_pstate_has_acpi_ppc())
>>>> We need try this on a few platforms to make sure this patch doesn't
>>>> break the
>>>> HP platforms that may or may not need this driver, depending on the
>>>> BIOS settings.
>>>>
>>> It looks like HP systems would get swept up in this check too if they
>>> have _PPC
> Right.  This patch breaks HP ProLiant platforms when they are
> configured to have the OS do power management.  In that case,
> the firmware exposes _PPC information.
   Okay, got it, The HP ProLiant has an option in BIOS could be enabled 
to "OS PM", so
  will export _PSS, _PPC, and  this patch break this case.

>
>>     No , this patch checks the oem_id against 'ORACLE" first, will not
>> affect other vendors even they have _PPC.
> I don't think that's how your code works.  This patch will match any
> vendor that is in the table, not just "ORACLE".
  Will change patch to match the oem-id out of the loop, such as 
following , how about it ?

  static bool intel_pstate_platform_pwr_mgmt_exists(void)
{
         struct acpi_table_header hdr;
         struct hw_vendor_info *v_info;

         if (acpi_disabled
             || ACPI_FAILURE(acpi_get_table_header(ACPI_SIG_FADT, 0, &hdr)))
                 return false;

         for (v_info = vendor_info; v_info->valid; v_info++) {
                 if (!strncmp(hdr.oem_id, v_info->oem_id, ACPI_OEM_ID_SIZE)
                     && !strncmp(hdr.oem_table_id, v_info->oem_table_id, 
ACPI_OEM_TABLE_ID_SIZE)
                     && intel_pstate_no_acpi_pss())
                         return true;
         }

    if (!strncmp(hdr.oem_id, v_info[1]->oem_id, ACPI_OEM_ID_SIZE) &&
           intel_pstate_has_acpi_ppc())
            return true;

         return false;
}

>>> What about extending the hw_vendor_info struct to include whether _PSS or
>>   Except refer to ACPI DSDT, I don't know how to fill such info.
>>> _PPC should be done for the platform since it appears that oracle and HP
>>> have implemented similar functionality using two different methods.
>>    Maybe Linda could answer this whether HP also has _PPC and should be
>> wept out.
>>    But that doesn't happen with on the same box at the same time.
> I don't know how an Oracle box works but on a ProLiant, customers can
> choose to have platform power management or OS power management.
> When the platform is managing the power, we don't provide the _PSS
> information.  Since our oem information is in the table and there is
> no _PSS, the intel_pstate driver doesn't stay loaded.  That's what we want.
>
> When the platform configured to have the OS do the power management,
> the firmware has _PSS and _PPC and we want the intel_pstate driver,
> That's what your patch breaks.  With your patch, the driver won't
> stay loaded because our platform is in the table and the check for
> _PPC passes.
>
> How does an Oracle box work?
   Oracle Sun servers (X86) don't have the option in BIOS to change the 
PM mode to firmware/OS,
   The BIOS always has _PSS and _PPC exported to OS whatever 'soft power 
capping' or 'hard power capping' enabled
   in SP configuration web page. if the power policy violation happened, 
firmware will notify OS via SCI with the changed _PPC
   number.

   Thanks,
   Ethan
>
> -- ljk
>
>>    Thanks,
>>    Ethan
>>>
>>>> I don't suppose you tested on a ProLiant too?
>>>>
>>>> -- ljk
>>>>
>>>>> +            return true;
>>>>>        }
>>>>>
>>>>>        return false;
>>>>>    }
>>>>>    #else /* CONFIG_ACPI not enabled */
>>>>>    static inline bool intel_pstate_platform_pwr_mgmt_exists(void) {
>>>>> return false; }
>>>>> +static inline bool intel_pstate_has_acpi_ppc(void) { return false; }
>>>>>    #endif /* CONFIG_ACPI */
>>>>>
>>>>>    static int __init intel_pstate_init(void)
>>>>>
>> -- 
>> To unsubscribe from this list: send the line "unsubscribe linux-pm" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 3/3] intel_pstate: add module and kernel command line parameter to ignore ACPI _PPC

[ethan zhao]

Linda,

On 2014/11/21 13:00, Linda Knippers wrote:
>
> On 11/20/2014 10:07 PM, Ethan Zhao wrote:
>> Kristen,
>>     Whatever I would like there is a way to load intel_pstate and give
>> it a try even it does not support all the PM features.
>>     I think 'force' is OK.
>> Linda,
>>    Do you like it ? if the 'intel_pstate=force' would force the driver
>> to be loaded on to HP too ?
> I'd prefer that it didn't.  If you force the intel_pstate driver when
> the platform thinks it's doing power management, then the OS and the
> firmware are trying to manage the power at the same time.  That's a
> mess.  If you want that for testing or debugging, what are you actually
> testing or debugging?  On an Oracle box, the firmware wouldn't stop
> doing whatever it's doing just because the intel_pstate driver is
> loaded, would it?
  Yes, the platform wouldn't stop doing PM and the SCI even the intel_pstate
  was loaded.
  That option just give someone a chance who said "whatever I like 
intel_pstate
  On Oracle Servers"
>
> I also wonder what it means to "force" the intel_pstate driver
> on systems with processors that aren't supported by the intel_pstate
> driver.  It wouldn't really be forced, would it?
  Yes, wouldn't, such as AMD cpu-id and some old intel CPUs released 
before SandyBridge.
  So the best way to to do the 'force' should be specific enough for 
"Oracle Sun server"

  Thanks,
  Ethan
>
> -- ljk
>
>> Thanks,
>> Ethan
>>
>> On Fri, Nov 21, 2014 at 5:23 AM, Kristen Carlson Accardi
>> <kristen@linux.intel.com> wrote:
>>> On Thu, 20 Nov 2014 08:57:34 +0800
>>> ethan <ethan.kernel@gmail.com> wrote:
>>>
>>>>
>>>>> 在 2014年11月20日，03:05，Kristen Carlson Accardi <kristen@linux.intel.com> 写道：
>>>>>
>>>>> On Tue, 18 Nov 2014 17:37:06 +0900
>>>>> Ethan Zhao <ethan.zhao@oracle.com> wrote:
>>>>>
>>>>>> Add kernel command line parameter
>>>>>> intel_pstate = ignore_acpi_ppc
>>>>>> and module parameter
>>>>>> ignore_acpi_ppc = 1
>>>>>> to allow driver to ignore the ACPI _PPC existence even for Sun x86 servers.
>>>>>> These parameter could be used for debug\test\workaround etc purpose.
>>>>>>
>>>>>> Signed-off-by: Ethan Zhao <ethan.zhao@oracle.com>
>>>>> What if we used a more generic parameter like "force" that would bypass
>>>>> any vendor specific checks and just load anyway?  This way we don't have
>>>>> to add new parameters everything some new thing shows up that we want to
>>>>> ignore.
>>>>>
>>>> To be honest, I prefer more generic parameter. But to avoid the possible negative affect
>>>> To another vendors. I back to this way.
>>> Well, your parameter can still impact other vendors as it is.  it
>>> is pretty typical to assume that using a parameter like "force" means
>>> you know what you are doing and accept the risks.  Especially if its
>>> documented as such.
>>>
>>>> Thanks,
>>>> Ethan
>>>>>> ---
>>>>>> Documentation/kernel-parameters.txt | 3 +++
>>>>>> drivers/cpufreq/intel_pstate.c      | 8 +++++++-
>>>>>> 2 files changed, 10 insertions(+), 1 deletion(-)
>>>>>>
>>>>>> diff --git a/Documentation/kernel-parameters.txt b/Documentation/kernel-parameters.txt
>>>>>> index 4c81a86..f502b85 100644
>>>>>> --- a/Documentation/kernel-parameters.txt
>>>>>> +++ b/Documentation/kernel-parameters.txt
>>>>>> @@ -1446,6 +1446,9 @@ bytes respectively. Such letter suffixes can also be entirely omitted.
>>>>>>                disable
>>>>>>                  Do not enable intel_pstate as the default
>>>>>>                  scaling driver for the supported processors
>>>>>> +               ignore_acpi_ppc
>>>>>> +             Ignore the existence of ACPI method _PPC for Sun x86 servers
>>>>>> +             and load the driver.
>>>>>>
>>>>>>     intremap=    [X86-64, Intel-IOMMU]
>>>>>>             on    enable Interrupt Remapping (default)
>>>>>> diff --git a/drivers/cpufreq/intel_pstate.c b/drivers/cpufreq/intel_pstate.c
>>>>>> index 7c5faea..388387b 100644
>>>>>> --- a/drivers/cpufreq/intel_pstate.c
>>>>>> +++ b/drivers/cpufreq/intel_pstate.c
>>>>>> @@ -870,6 +870,7 @@ static struct cpufreq_driver intel_pstate_driver = {
>>>>>> };
>>>>>>
>>>>>> static int __initdata no_load;
>>>>>> +static unsigned int  ignore_acpi_ppc;
>>>>>>
>>>>>> static int intel_pstate_msrs_not_valid(void)
>>>>>> {
>>>>>> @@ -990,7 +991,7 @@ static bool intel_pstate_platform_pwr_mgmt_exists(void)
>>>>>>             intel_pstate_no_acpi_pss())
>>>>>>             return true;
>>>>>>         if (!strncmp(hdr.oem_id, v_info->oem_id, ACPI_OEM_ID_SIZE) &&
>>>>>> -            intel_pstate_has_acpi_ppc())
>>>>>> +            intel_pstate_has_acpi_ppc() && !ignore_acpi_ppc)
>>>>>>             return true;
>>>>>>     }
>>>>>>
>>>>>> @@ -1066,11 +1067,16 @@ static int __init intel_pstate_setup(char *str)
>>>>>>
>>>>>>     if (!strcmp(str, "disable"))
>>>>>>         no_load = 1;
>>>>>> +    if (!strcmp(str, "ignore_acpi_ppc"))
>>>>>> +        ignore_acpi_ppc = 1;
>>>>>>     return 0;
>>>>>> }
>>>>>> early_param("intel_pstate", intel_pstate_setup);
>>>>>> #endif
>>>>>>
>>>>>> +module_param(ignore_acpi_ppc, uint, 0644);
>>>>>> +MODULE_PARM_DESC(ignore_acpi_ppc,
>>>>>> +    "value 0 or non-zero. non-zero -> ignore ACPI _PPC and load this driver");
>>>>>> MODULE_AUTHOR("Dirk Brandewie <dirk.j.brandewie@intel.com>");
>>>>>> MODULE_DESCRIPTION("'intel_pstate' - P state driver Intel Core processors");
>>>>>> MODULE_LICENSE("GPL");
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-pm" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>>

Re: [PATCH 1/3] intel_pstate: skip the driver if Sun server has ACPI _PPC method

[Linda Knippers]

On 11/23/2014 8:41 PM, ethan zhao wrote:
> Linda,
> 
> On 2014/11/21 12:44, Linda Knippers wrote:
>>
>> On 11/20/2014 07:37 PM, ethan zhao wrote:
>>> Dirk,
>>>
>>> On 2014/11/21 0:50, Dirk Brandewie wrote:
>>>> On 11/19/2014 12:22 PM, Linda Knippers wrote:
>>>>> On 11/18/2014 3:37 AM, Ethan Zhao wrote:
>>>>>> Oracle Sun X86 servers have dynamic power capping capability that
>>>>>> works via
>>>>>> ACPI _PPC method etc, so skip loading this driver if Sun server has
>>>>>> ACPI _PPC
>>>>>> enabled.
>>>>>>
>>>>>> Signed-off-by: Ethan Zhao <ethan.zhao@oracle.com>
>>>>>> ---
>>>>>>    drivers/cpufreq/intel_pstate.c | 20 ++++++++++++++++++++
>>>>>>    1 file changed, 20 insertions(+)
>>>>>>
>>>>>> diff --git a/drivers/cpufreq/intel_pstate.c
>>>>>> b/drivers/cpufreq/intel_pstate.c
>>>>>> index 27bb6d3..5498eb0 100644
>>>>>> --- a/drivers/cpufreq/intel_pstate.c
>>>>>> +++ b/drivers/cpufreq/intel_pstate.c
>>>>>> @@ -943,6 +943,21 @@ static bool intel_pstate_no_acpi_pss(void)
>>>>>>        return true;
>>>>>>    }
>>>>>>
>>>>>> +static bool intel_pstate_has_acpi_ppc(void)
>>>>>> +{
>>>>>> +    int i;
>>>>>> +
>>>>>> +    for_each_possible_cpu(i) {
>>>>>> +        struct acpi_processor *pr = per_cpu(processors, i);
>>>>>> +
>>>>>> +        if (!pr)
>>>>>> +            continue;
>>>>>> +        if (acpi_has_method(pr->handle, "_PPC"))
>>>>>> +            return true;
>>>>>> +    }
>>>>>> +    return false;
>>>>>> +}
>>>>>> +
>>>>>>    struct hw_vendor_info {
>>>>>>        u16  valid;
>>>>>>        char oem_id[ACPI_OEM_ID_SIZE];
>>>>>> @@ -952,6 +967,7 @@ struct hw_vendor_info {
>>>>>>    /* Hardware vendor-specific info that has its own power management
>>>>>> modes */
>>>>>>    static struct hw_vendor_info vendor_info[] = {
>>>>>>        {1, "HP    ", "ProLiant"},
>>>>>> +    {1, "ORACLE", ""},
>>>>>>        {0, "", ""},
>>>>>>    };
>>>>>>
>>>>>> @@ -969,12 +985,16 @@ static bool
>>>>>> intel_pstate_platform_pwr_mgmt_exists(void)
>>>>>>                !strncmp(hdr.oem_table_id, v_info->oem_table_id,
>>>>>> ACPI_OEM_TABLE_ID_SIZE) &&
>>>>>>                intel_pstate_no_acpi_pss())
>>>>>>                return true;
>>>>>> +        if (!strncmp(hdr.oem_id, v_info->oem_id, ACPI_OEM_ID_SIZE) &&
>>>>>> +            intel_pstate_has_acpi_ppc())
>>>>> We need try this on a few platforms to make sure this patch doesn't
>>>>> break the
>>>>> HP platforms that may or may not need this driver, depending on the
>>>>> BIOS settings.
>>>>>
>>>> It looks like HP systems would get swept up in this check too if they
>>>> have _PPC
>> Right.  This patch breaks HP ProLiant platforms when they are
>> configured to have the OS do power management.  In that case,
>> the firmware exposes _PPC information.
>   Okay, got it, The HP ProLiant has an option in BIOS could be enabled to "OS
> PM", so
>  will export _PSS, _PPC, and  this patch break this case.
> 
>>
>>>     No , this patch checks the oem_id against 'ORACLE" first, will not
>>> affect other vendors even they have _PPC.
>> I don't think that's how your code works.  This patch will match any
>> vendor that is in the table, not just "ORACLE".
>  Will change patch to match the oem-id out of the loop, such as following , how
> about it ?
> 
>  static bool intel_pstate_platform_pwr_mgmt_exists(void)
> {
>         struct acpi_table_header hdr;
>         struct hw_vendor_info *v_info;
> 
>         if (acpi_disabled
>             || ACPI_FAILURE(acpi_get_table_header(ACPI_SIG_FADT, 0, &hdr)))
>                 return false;
> 
>         for (v_info = vendor_info; v_info->valid; v_info++) {
>                 if (!strncmp(hdr.oem_id, v_info->oem_id, ACPI_OEM_ID_SIZE)
>                     && !strncmp(hdr.oem_table_id, v_info->oem_table_id,
> ACPI_OEM_TABLE_ID_SIZE)
>                     && intel_pstate_no_acpi_pss())
>                         return true;
>         }
> 
>    if (!strncmp(hdr.oem_id, v_info[1]->oem_id, ACPI_OEM_ID_SIZE) &&
>           intel_pstate_has_acpi_ppc())

I really don't think you want to hard code a 1 there.

I think you need to do what Dirk suggested, which is to expand the
hw_vendor_info structure to specify the check that needs to be done
for each entry.  For a ProLiant, it would be to call intel_pstate_no_acpi_pss()
and for an Oracle box, it would be to call intel_pstate_has_acpi_ppc().

-- ljk

>            return true;
> 
>         return false;
> }
> 
>>>> What about extending the hw_vendor_info struct to include whether _PSS or
>>>   Except refer to ACPI DSDT, I don't know how to fill such info.
>>>> _PPC should be done for the platform since it appears that oracle and HP
>>>> have implemented similar functionality using two different methods.
>>>    Maybe Linda could answer this whether HP also has _PPC and should be
>>> wept out.
>>>    But that doesn't happen with on the same box at the same time.
>> I don't know how an Oracle box works but on a ProLiant, customers can
>> choose to have platform power management or OS power management.
>> When the platform is managing the power, we don't provide the _PSS
>> information.  Since our oem information is in the table and there is
>> no _PSS, the intel_pstate driver doesn't stay loaded.  That's what we want.
>>
>> When the platform configured to have the OS do the power management,
>> the firmware has _PSS and _PPC and we want the intel_pstate driver,
>> That's what your patch breaks.  With your patch, the driver won't
>> stay loaded because our platform is in the table and the check for
>> _PPC passes.
>>
>> How does an Oracle box work?
>   Oracle Sun servers (X86) don't have the option in BIOS to change the PM mode
> to firmware/OS,
>   The BIOS always has _PSS and _PPC exported to OS whatever 'soft power capping'
> or 'hard power capping' enabled
>   in SP configuration web page. if the power policy violation happened, firmware
> will notify OS via SCI with the changed _PPC
>   number.
> 
>   Thanks,
>   Ethan
>>
>> -- ljk
>>
>>>    Thanks,
>>>    Ethan
>>>>
>>>>> I don't suppose you tested on a ProLiant too?
>>>>>
>>>>> -- ljk
>>>>>
>>>>>> +            return true;
>>>>>>        }
>>>>>>
>>>>>>        return false;
>>>>>>    }
>>>>>>    #else /* CONFIG_ACPI not enabled */
>>>>>>    static inline bool intel_pstate_platform_pwr_mgmt_exists(void) {
>>>>>> return false; }
>>>>>> +static inline bool intel_pstate_has_acpi_ppc(void) { return false; }
>>>>>>    #endif /* CONFIG_ACPI */
>>>>>>
>>>>>>    static int __init intel_pstate_init(void)
>>>>>>
>>> -- 
>>> To unsubscribe from this list: send the line "unsubscribe linux-pm" in
>>> the body of a message to majordomo@vger.kernel.org
>>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

Re: [PATCH 1/3] intel_pstate: skip the driver if Sun server has ACPI _PPC method

[ethan]

Linda，
> 在 2014年11月24日，23:54，Linda Knippers <linda.knippers@hp.com> 写道：
> 
>> On 11/23/2014 8:41 PM, ethan zhao wrote:
>> Linda,
>> 
>>> On 2014/11/21 12:44, Linda Knippers wrote:
>>> 
>>>> On 11/20/2014 07:37 PM, ethan zhao wrote:
>>>> Dirk,
>>>> 
>>>>> On 2014/11/21 0:50, Dirk Brandewie wrote:
>>>>>> On 11/19/2014 12:22 PM, Linda Knippers wrote:
>>>>>>> On 11/18/2014 3:37 AM, Ethan Zhao wrote:
>>>>>>> Oracle Sun X86 servers have dynamic power capping capability that
>>>>>>> works via
>>>>>>> ACPI _PPC method etc, so skip loading this driver if Sun server has
>>>>>>> ACPI _PPC
>>>>>>> enabled.
>>>>>>> 
>>>>>>> Signed-off-by: Ethan Zhao <ethan.zhao@oracle.com>
>>>>>>> ---
>>>>>>>   drivers/cpufreq/intel_pstate.c | 20 ++++++++++++++++++++
>>>>>>>   1 file changed, 20 insertions(+)
>>>>>>> 
>>>>>>> diff --git a/drivers/cpufreq/intel_pstate.c
>>>>>>> b/drivers/cpufreq/intel_pstate.c
>>>>>>> index 27bb6d3..5498eb0 100644
>>>>>>> --- a/drivers/cpufreq/intel_pstate.c
>>>>>>> +++ b/drivers/cpufreq/intel_pstate.c
>>>>>>> @@ -943,6 +943,21 @@ static bool intel_pstate_no_acpi_pss(void)
>>>>>>>       return true;
>>>>>>>   }
>>>>>>> 
>>>>>>> +static bool intel_pstate_has_acpi_ppc(void)
>>>>>>> +{
>>>>>>> +    int i;
>>>>>>> +
>>>>>>> +    for_each_possible_cpu(i) {
>>>>>>> +        struct acpi_processor *pr = per_cpu(processors, i);
>>>>>>> +
>>>>>>> +        if (!pr)
>>>>>>> +            continue;
>>>>>>> +        if (acpi_has_method(pr->handle, "_PPC"))
>>>>>>> +            return true;
>>>>>>> +    }
>>>>>>> +    return false;
>>>>>>> +}
>>>>>>> +
>>>>>>>   struct hw_vendor_info {
>>>>>>>       u16  valid;
>>>>>>>       char oem_id[ACPI_OEM_ID_SIZE];
>>>>>>> @@ -952,6 +967,7 @@ struct hw_vendor_info {
>>>>>>>   /* Hardware vendor-specific info that has its own power management
>>>>>>> modes */
>>>>>>>   static struct hw_vendor_info vendor_info[] = {
>>>>>>>       {1, "HP    ", "ProLiant"},
>>>>>>> +    {1, "ORACLE", ""},
>>>>>>>       {0, "", ""},
>>>>>>>   };
>>>>>>> 
>>>>>>> @@ -969,12 +985,16 @@ static bool
>>>>>>> intel_pstate_platform_pwr_mgmt_exists(void)
>>>>>>>               !strncmp(hdr.oem_table_id, v_info->oem_table_id,
>>>>>>> ACPI_OEM_TABLE_ID_SIZE) &&
>>>>>>>               intel_pstate_no_acpi_pss())
>>>>>>>               return true;
>>>>>>> +        if (!strncmp(hdr.oem_id, v_info->oem_id, ACPI_OEM_ID_SIZE) &&
>>>>>>> +            intel_pstate_has_acpi_ppc())
>>>>>> We need try this on a few platforms to make sure this patch doesn't
>>>>>> break the
>>>>>> HP platforms that may or may not need this driver, depending on the
>>>>>> BIOS settings.
>>>>> It looks like HP systems would get swept up in this check too if they
>>>>> have _PPC
>>> Right.  This patch breaks HP ProLiant platforms when they are
>>> configured to have the OS do power management.  In that case,
>>> the firmware exposes _PPC information.
>>  Okay, got it, The HP ProLiant has an option in BIOS could be enabled to "OS
>> PM", so
>> will export _PSS, _PPC, and  this patch break this case.
>> 
>>> 
>>>>    No , this patch checks the oem_id against 'ORACLE" first, will not
>>>> affect other vendors even they have _PPC.
>>> I don't think that's how your code works.  This patch will match any
>>> vendor that is in the table, not just "ORACLE".
>> Will change patch to match the oem-id out of the loop, such as following , how
>> about it ?
>> 
>> static bool intel_pstate_platform_pwr_mgmt_exists(void)
>> {
>>        struct acpi_table_header hdr;
>>        struct hw_vendor_info *v_info;
>> 
>>        if (acpi_disabled
>>            || ACPI_FAILURE(acpi_get_table_header(ACPI_SIG_FADT, 0, &hdr)))
>>                return false;
>> 
>>        for (v_info = vendor_info; v_info->valid; v_info++) {
>>                if (!strncmp(hdr.oem_id, v_info->oem_id, ACPI_OEM_ID_SIZE)
>>                    && !strncmp(hdr.oem_table_id, v_info->oem_table_id,
>> ACPI_OEM_TABLE_ID_SIZE)
>>                    && intel_pstate_no_acpi_pss())
>>                        return true;
>>        }
>> 
>>   if (!strncmp(hdr.oem_id, v_info[1]->oem_id, ACPI_OEM_ID_SIZE) &&
>>          intel_pstate_has_acpi_ppc())
> 
> I really don't think you want to hard code a 1 there.
> 
> I think you need to do what Dirk suggested, which is to expand the
> hw_vendor_info structure to specify the check that needs to be done
> for each entry.  For a ProLiant, it would be to call intel_pstate_no_acpi_pss()
> and for an Oracle box, it would be to call intel_pstate_has_acpi_ppc().
> 

thanks，will do that.
Ethan

> -- ljk
> 
>>           return true;
>> 
>>        return false;
>> }
>> 
>>>>> What about extending the hw_vendor_info struct to include whether _PSS or
>>>>  Except refer to ACPI DSDT, I don't know how to fill such info.
>>>>> _PPC should be done for the platform since it appears that oracle and HP
>>>>> have implemented similar functionality using two different methods.
>>>>   Maybe Linda could answer this whether HP also has _PPC and should be
>>>> wept out.
>>>>   But that doesn't happen with on the same box at the same time.
>>> I don't know how an Oracle box works but on a ProLiant, customers can
>>> choose to have platform power management or OS power management.
>>> When the platform is managing the power, we don't provide the _PSS
>>> information.  Since our oem information is in the table and there is
>>> no _PSS, the intel_pstate driver doesn't stay loaded.  That's what we want.
>>> 
>>> When the platform configured to have the OS do the power management,
>>> the firmware has _PSS and _PPC and we want the intel_pstate driver,
>>> That's what your patch breaks.  With your patch, the driver won't
>>> stay loaded because our platform is in the table and the check for
>>> _PPC passes.
>>> 
>>> How does an Oracle box work?
>>  Oracle Sun servers (X86) don't have the option in BIOS to change the PM mode
>> to firmware/OS,
>>  The BIOS always has _PSS and _PPC exported to OS whatever 'soft power capping'
>> or 'hard power capping' enabled
>>  in SP configuration web page. if the power policy violation happened, firmware
>> will notify OS via SCI with the changed _PPC
>>  number.
>> 
>>  Thanks,
>>  Ethan
>>> 
>>> -- ljk
>>> 
>>>>   Thanks,
>>>>   Ethan
>>>>> 
>>>>>> I don't suppose you tested on a ProLiant too?
>>>>>> 
>>>>>> -- ljk
>>>>>> 
>>>>>>> +            return true;
>>>>>>>       }
>>>>>>> 
>>>>>>>       return false;
>>>>>>>   }
>>>>>>>   #else /* CONFIG_ACPI not enabled */
>>>>>>>   static inline bool intel_pstate_platform_pwr_mgmt_exists(void) {
>>>>>>> return false; }
>>>>>>> +static inline bool intel_pstate_has_acpi_ppc(void) { return false; }
>>>>>>>   #endif /* CONFIG_ACPI */
>>>>>>> 
>>>>>>>   static int __init intel_pstate_init(void)
>>>> -- 
>>>> To unsubscribe from this list: send the line "unsubscribe linux-pm" in
>>>> the body of a message to majordomo@vger.kernel.org
>>>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>