diff mbox

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

Message ID 20170721111644.2a6a6610@roar.ozlabs.ibm.com (mailing list archive)
State Accepted
Commit a70a0b9f4404d8edb72ca0e0272731dc75bd00fb
Headers show

Commit Message

Nicholas Piggin July 21, 2017, 1:16 a.m. UTC 
On Thu, 20 Jul 2017 23:03:21 +1000
Michael Ellerman <mpe@ellerman.id.au> wrote:

> Nicholas Piggin <npiggin@gmail.com> writes:
> 
> > 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 <npiggin@gmail.com>
---
 arch/powerpc/platforms/pseries/pseries_energy.c | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

Comments

Vaidyanathan Srinivasan July 21, 2017, 4:35 a.m. UTC | #1 
* Nicholas Piggin <npiggin@gmail.com> [2017-07-21 11:16:44]:

> On Thu, 20 Jul 2017 23:03:21 +1000
> Michael Ellerman <mpe@ellerman.id.au> wrote:
> 
> > Nicholas Piggin <npiggin@gmail.com> writes:
> > 
> > > 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 <npiggin@gmail.com>

Reviewed-by: Vaidyanathan Srinivasan <svaidy@linux.vnet.ibm.com>


> ---
>  arch/powerpc/platforms/pseries/pseries_energy.c | 7 +++----
>  1 file changed, 3 insertions(+), 4 deletions(-)
> 
> diff --git a/arch/powerpc/platforms/pseries/pseries_energy.c b/arch/powerpc/platforms/pseries/pseries_energy.c
> index 164a13d3998a..35c891aabef0 100644
> --- a/arch/powerpc/platforms/pseries/pseries_energy.c
> +++ b/arch/powerpc/platforms/pseries/pseries_energy.c
> @@ -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 July 21, 2017, 6:33 a.m. UTC | #2 
Vaidyanathan Srinivasan <svaidy@linux.vnet.ibm.com> writes:
> * Nicholas Piggin <npiggin@gmail.com> [2017-07-21 11:16:44]:
>> diff --git a/arch/powerpc/platforms/pseries/pseries_energy.c b/arch/powerpc/platforms/pseries/pseries_energy.c
>> index 164a13d3998a..35c891aabef0 100644
>> --- a/arch/powerpc/platforms/pseries/pseries_energy.c
>> +++ b/arch/powerpc/platforms/pseries/pseries_energy.c
>> @@ -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.

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 July 21, 2017, 8:02 a.m. UTC | #3 
* Michael Ellerman <mpe@ellerman.id.au> [2017-07-21 16:33:07]:

> Vaidyanathan Srinivasan <svaidy@linux.vnet.ibm.com> writes:
> > * Nicholas Piggin <npiggin@gmail.com> [2017-07-21 11:16:44]:
> >> diff --git a/arch/powerpc/platforms/pseries/pseries_energy.c b/arch/powerpc/platforms/pseries/pseries_energy.c
> >> index 164a13d3998a..35c891aabef0 100644
> >> --- a/arch/powerpc/platforms/pseries/pseries_energy.c
> >> +++ b/arch/powerpc/platforms/pseries/pseries_energy.c
> >> @@ -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.
> 
> 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().

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 Aug. 11, 2017, 12:19 p.m. UTC | #4 
On Fri, 2017-07-21 at 01:16:44 UTC, Nicholas Piggin wrote:
> On Thu, 20 Jul 2017 23:03:21 +1000
> Michael Ellerman <mpe@ellerman.id.au> wrote:
> 
> > Nicholas Piggin <npiggin@gmail.com> writes:
> > 
> > > 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 <npiggin@gmail.com>
> Reviewed-by: Vaidyanathan Srinivasan <svaidy@linux.vnet.ibm.com>

Applied to powerpc next, thanks.

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

cheers
diff mbox

Patch

diff --git a/arch/powerpc/platforms/pseries/pseries_energy.c b/arch/powerpc/platforms/pseries/pseries_energy.c
index 164a13d3998a..35c891aabef0 100644
--- a/arch/powerpc/platforms/pseries/pseries_energy.c
+++ b/arch/powerpc/platforms/pseries/pseries_energy.c
@@ -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);