powerpc/pseries: energy driver only print message when LPAR guest

PATCH Applied Nicholas Piggin 4 participants 7 messages Jul 20, 2017 – Aug 11, 2017 PowerPC

Share & tools

Nicholas Piggin

author reviewer

2017-07-20 11:59 UTC #0 lore raw

PATCH 1/1 powerpc/pseries: energy driver only print message when LPAR guest

This driver currently reports the H_BEST_ENERGY is unsupported even when booting in a non-LPAR environment (e.g., powernv). Prevent it.

Signed-off-by Nicholas Piggin <[email protected]>

---
 arch/powerpc/platforms/pseries/pseries_energy.c | 3 +++
 1 file changed, 3 insertions(+)

arch/powerpc/platforms/pseries/pseries_energy.c +3 -0

index 164a13d3998a..832b3d7898a3 100644
@@ -229,6 +229,9 @@ static int __init pseries_energy_init(void)
 	int cpu, err;
 	struct device *cpu_dev;
 
+	if (!firmware_has_feature(FW_FEATURE_LPAR))
+		return 0;
+
 	if (!firmware_has_feature(FW_FEATURE_BEST_ENERGY)) {
 		printk(KERN_INFO "Hypercall H_BEST_ENERGY not supported\n");
 		return 0;

Signature · 1 line

2.11.0

Michael Ellerman

reviewer

2017-07-20 13:03 UTC #1 lore raw

Quoted 2 lines · Nicholas Piggin

This driver currently reports the H_BEST_ENERGY is unsupported even when booting in a non-LPAR environment (e.g., powernv). Prevent it.

Just delete the printk(). Users don't know what that means, and developers have other better ways to detect that the hcall is missing if anyone cares.

cheers

Nicholas Piggin

author reviewer

2017-07-21 01:16 UTC #2 lore raw

Quoted 10 lines · On Thu, 20 Jul 2017 23:03:21 +1000 Michael Ellerman <[email protected]> wrote:

Quoted 2 lines · Nicholas Piggin
This driver currently reports the H_BEST_ENERGY is unsupported even when booting in a non-LPAR environment (e.g., powernv). Prevent it.
Just delete the printk(). Users don't know what that means, and developers have other better ways to detect that the hcall is missing if anyone cares.
cheers

powerpc/pseries: energy driver do not print failure message

This driver currently reports the H_BEST_ENERGY is unsupported (even when booting in a non-LPAR environment). This is not something the administrator can do much with, and not significant for debugging.

Remove it.

Signed-off-by Nicholas Piggin <[email protected]>

---
 arch/powerpc/platforms/pseries/pseries_energy.c | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

arch/powerpc/platforms/pseries/pseries_energy.c +3 -4

index 164a13d3998a..35c891aabef0 100644
@@ -229,10 +229,9 @@ static int __init pseries_energy_init(void)
 	int cpu, err;
 	struct device *cpu_dev;
 
-	if (!firmware_has_feature(FW_FEATURE_BEST_ENERGY)) {
-		printk(KERN_INFO "Hypercall H_BEST_ENERGY not supported\n");
-		return 0;
-	}
+	if (!firmware_has_feature(FW_FEATURE_BEST_ENERGY))
+		return 0; /* H_BEST_ENERGY hcall not supported */
+
 	/* Create the sysfs files */
 	err = device_create_file(cpu_subsys.dev_root,
 				&attr_cpu_activate_hint_list);

Signature · 1 line

2.11.0

Vaidyanathan Srinivasan

Reviewed-by

2017-07-21 04:35 UTC #3 lore raw

Quoted 16 lines · Nicholas Piggin

Quoted 10 lines · On Thu, 20 Jul 2017 23:03:21 +1000 Michael Ellerman <[email protected]> wrote:
Quoted 2 lines · Nicholas Piggin
This driver currently reports the H_BEST_ENERGY is unsupported even when booting in a non-LPAR environment (e.g., powernv). Prevent it.
Just delete the printk(). Users don't know what that means, and developers have other better ways to detect that the hcall is missing if anyone cares.
cheers
powerpc/pseries: energy driver do not print failure message

Quoted 7 lines

This driver currently reports the H_BEST_ENERGY is unsupported (even when booting in a non-LPAR environment). This is not something the administrator can do much with, and not significant for debugging.
Remove it.
Signed-off-by Nicholas Piggin <[email protected]>

Reviewed-by Vaidyanathan Srinivasan <[email protected]>

Quoted 9 lines

---
 arch/powerpc/platforms/pseries/pseries_energy.c | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

arch/powerpc/platforms/pseries/pseries_energy.c +0 -0

index 164a13d3998a..35c891aabef0 100644

arch/powerpc/platforms/pseries/pseries_energy.c static int __init pseries_energy_init(void) · line 234

Quoted 10 lines

arch/powerpc/platforms/pseries/pseries_energy.c +3 -4

@@ -229,10 +229,9 @@ static int __init pseries_energy_init(void)
 	int cpu, err;
 	struct device *cpu_dev;
 
-	if (!firmware_has_feature(FW_FEATURE_BEST_ENERGY)) {
-		printk(KERN_INFO "Hypercall H_BEST_ENERGY not supported\n");
-		return 0;
-	}
+	if (!firmware_has_feature(FW_FEATURE_BEST_ENERGY))
+		return 0; /* H_BEST_ENERGY hcall not supported */
+

The first patch (!firmware_has_feature(FW_FEATURE_LPAR)) would be ideal, but we do not have this in KVM guest case also. Hence I take mpe's suggestion. Removing the print is fine.

--Vaidy

Michael Ellerman

reviewer

2017-07-21 06:33 UTC #4 lore raw

Quoted 9 lines · Vaidyanathan Srinivasan

Quoted 8 lines · Nicholas Piggin
arch/powerpc/platforms/pseries/pseries_energy.c +0 -0
index 164a13d3998a..35c891aabef0 100644
@@ -229,10 +229,9 @@ static int __init pseries_energy_init(void)
 	int cpu, err;
 	struct device *cpu_dev;
 

Quoted 10 lines

arch/powerpc/platforms/pseries/pseries_energy.c static int __init pseries_energy_init(void) · line 234
Quoted 7 lines
arch/powerpc/platforms/pseries/pseries_energy.c +3 -4
-	if (!firmware_has_feature(FW_FEATURE_BEST_ENERGY)) {
-		printk(KERN_INFO "Hypercall H_BEST_ENERGY not supported\n");
-		return 0;
-	}
+	if (!firmware_has_feature(FW_FEATURE_BEST_ENERGY))
+		return 0; /* H_BEST_ENERGY hcall not supported */
+
The first patch (!firmware_has_feature(FW_FEATURE_LPAR)) would be ideal, but we do not have this in KVM guest case also.

Yeah we do.

It should really be called FW_FEATURE_RUNNING_UNDER_PAPR_HYPERVISOR.

static int __init probe_fw_features(unsigned long node, const char *uname, int
depth, void *data)

{
	....
	if (!strcmp(uname, "rtas") || !strcmp(uname, "rtas@0")) {
		prop = of_get_flat_dt_prop(node, "ibm,hypertas-functions", &len);
		if (prop) {
			powerpc_firmware_features |= FW_FEATURE_LPAR;

Qemu initialises that property unconditionally in spapr_dt_rtas().

cheers

Vaidyanathan Srinivasan

2017-07-21 08:02 UTC #5 lore raw

Quoted 35 lines · Michael Ellerman

Quoted 9 lines · Vaidyanathan Srinivasan
Quoted 8 lines · Nicholas Piggin
arch/powerpc/platforms/pseries/pseries_energy.c +0 -0
index 164a13d3998a..35c891aabef0 100644
@@ -229,10 +229,9 @@ static int __init pseries_energy_init(void)
 	int cpu, err;
 	struct device *cpu_dev;
 
Quoted 10 lines
arch/powerpc/platforms/pseries/pseries_energy.c static int __init pseries_energy_init(void) · line 234
Quoted 7 lines
arch/powerpc/platforms/pseries/pseries_energy.c +3 -4
-	if (!firmware_has_feature(FW_FEATURE_BEST_ENERGY)) {
-		printk(KERN_INFO "Hypercall H_BEST_ENERGY not supported\n");
-		return 0;
-	}
+	if (!firmware_has_feature(FW_FEATURE_BEST_ENERGY))
+		return 0; /* H_BEST_ENERGY hcall not supported */
+
The first patch (!firmware_has_feature(FW_FEATURE_LPAR)) would be ideal, but we do not have this in KVM guest case also.
Yeah we do.
It should really be called FW_FEATURE_RUNNING_UNDER_PAPR_HYPERVISOR.
static int __init probe_fw_features(unsigned long node, const char *uname, int
depth, void *data)
{
	....
	if (!strcmp(uname, "rtas") || !strcmp(uname, "rtas@0")) {
		prop = of_get_flat_dt_prop(node, "ibm,hypertas-functions", &len);
		if (prop) {
			powerpc_firmware_features |= FW_FEATURE_LPAR;

Quoted 1 line

Qemu initialises that property unconditionally in spapr_dt_rtas().

oops... I meant that FW_FEATURE_BEST_ENERGY is not found in KVM and we will see the print needlessly.

If we have a check for phyp LPAR, then we can enable the print "H_BEST_ENERGY hcall not supported"

Since the FW_FEATURE_LPAR is common for all PAPR guest (both pHyp and KVM), I agree that deleting the print is the right thing to do since we see it on both powernv and KVM where it is not supported and there is no point reporting it.

--Vaidy

Michael Ellerman

Applied

2017-08-11 12:19 UTC #6 lore raw

In reply to Nicholas Piggin

Quoted 16 lines · Nicholas Piggin

Quoted 10 lines · On Thu, 20 Jul 2017 23:03:21 +1000 Michael Ellerman <[email protected]> wrote:
Quoted 2 lines · Nicholas Piggin
This driver currently reports the H_BEST_ENERGY is unsupported even when booting in a non-LPAR environment (e.g., powernv). Prevent it.
Just delete the printk(). Users don't know what that means, and developers have other better ways to detect that the hcall is missing if anyone cares.
cheers
powerpc/pseries: energy driver do not print failure message

Quoted 8 lines

This driver currently reports the H_BEST_ENERGY is unsupported (even when booting in a non-LPAR environment). This is not something the administrator can do much with, and not significant for debugging.
Remove it.
Signed-off-by Nicholas Piggin <[email protected]>
Reviewed-by Vaidyanathan Srinivasan <[email protected]>

Applied to powerpc next, thanks.

https://git.kernel.org/powerpc/c/a70a0b9f4404d8edb72ca0e0272731

cheers