Re: [PATCH AUTOSEL 4.9 11/48] cpupower: Fix coredump on VMWare

From: Rafael David Tinoco <rafael.tinoco@linaro.org>
To: Sasha Levin <sashal@kernel.org>,
	stable@vger.kernel.org, linux-kernel@vger.kernel.org
Cc: Prarit Bhargava <prarit@redhat.com>,
	Shuah Khan <shuah@kernel.org>, Stafford Horne <shorne@gmail.com>,
	gregkh@linuxfoundation.org
Subject: Re: [PATCH AUTOSEL 4.9 11/48] cpupower: Fix coredump on VMWare
Date: Mon, 12 Nov 2018 08:31:57 -0200	[thread overview]
Message-ID: <87a22cde-f6a8-0103-c20e-150f2d8157ba@linaro.org> (raw)
In-Reply-To: <20181031231054.29333-11-sashal@kernel.org>

On 10/31/18 9:10 PM, Sasha Levin wrote:
> From: Prarit Bhargava <prarit@redhat.com>
> 
> [ Upstream commit f69ffc5d3db8f1f03fd6d1df5930f9a1fbd787b6 ]
> 
> cpupower crashes on VMWare guests.  The guests have the AMD PStateDef MSR
> (0xC0010064 + state number) set to zero.  As a result fid and did are zero
> and the crash occurs because of a divide by zero (cof = fid/did).  This
> can be prevented by checking the enable bit in the PStateDef MSR before
> calculating cof.  By doing this the value of pstate[i] remains zero and
> the value can be tested before displaying the active Pstates.
> 
> Check the enable bit in the PstateDef register for all supported families
> and only print out enabled Pstates.
> 
> Signed-off-by: Prarit Bhargava <prarit@redhat.com>
> Cc: Shuah Khan <shuah@kernel.org>
> Cc: Stafford Horne <shorne@gmail.com>
> Signed-off-by: Shuah Khan (Samsung OSG) <shuah@kernel.org>
> Signed-off-by: Sasha Levin <sashal@kernel.org>
> ---
>   tools/power/cpupower/utils/cpufreq-info.c | 2 ++
>   tools/power/cpupower/utils/helpers/amd.c  | 5 +++++
>   2 files changed, 7 insertions(+)
> 
> diff --git a/tools/power/cpupower/utils/cpufreq-info.c b/tools/power/cpupower/utils/cpufreq-info.c
> index 590d12a25f6e..8d7580e4b299 100644
> --- a/tools/power/cpupower/utils/cpufreq-info.c
> +++ b/tools/power/cpupower/utils/cpufreq-info.c
> @@ -202,6 +202,8 @@ static int get_boost_mode(unsigned int cpu)
>   		printf(_("    Boost States: %d\n"), b_states);
>   		printf(_("    Total States: %d\n"), pstate_no);
>   		for (i = 0; i < pstate_no; i++) {
> +			if (!pstates[i])
> +				continue;
>   			if (i < b_states)
>   				printf(_("    Pstate-Pb%d: %luMHz (boost state)"
>   					 "\n"), i, pstates[i]);
> diff --git a/tools/power/cpupower/utils/helpers/amd.c b/tools/power/cpupower/utils/helpers/amd.c
> index 6437ef39aeea..82adfb33d7a5 100644
> --- a/tools/power/cpupower/utils/helpers/amd.c
> +++ b/tools/power/cpupower/utils/helpers/amd.c
> @@ -103,6 +103,11 @@ int decode_pstates(unsigned int cpu, unsigned int cpu_family,
>   		}
>   		if (read_msr(cpu, MSR_AMD_PSTATE + i, &pstate.val))
>   			return -1;
> +		if ((cpu_family == 0x17) && (!pstate.fam17h_bits.en))
> +			continue;
> +		else if (!pstate.bits.en)
> +			continue;
> +
>   		pstates[i] = get_cof(cpu_family, pstate);
>   	}
>   	*no = i;
> 

Sasha,

This commit is causing:

$ make V=1 -C tools/power/cpupower all

gcc -fPIC -DVERSION=\"4.9.136.142.g36c7703\" -DPACKAGE=\"cpupower\" 
-DPACKAGE_BUGREPORT=\"linux-pm@vger.kernel.org\" -D_GNU_SOURCE -pipe 
-DNLS -Wall -Wchar-subscripts -Wpointer-arith -Wsign-compare 
-Wno-pointer-sign -Wdeclaration-after-statement -Wshadow -O1 -g -DDEBUG 
-I./lib -I ./utils -o utils/helpers/amd.o -c utils/helpers/amd.c
utils/helpers/amd.c: In function ‘decode_pstates’:
utils/helpers/amd.c:106:39: error: ‘union msr_pstate’ has no member 
named ‘fam17h_bits’
    if ((cpu_family == 0x17) && (!pstate.fam17h_bits.en))

I think it should be dropped from v4.4 and v4.9 RCs, since this is a fix 
for a CPU that isn't supported for those 2.

Thanks
--
Rafael D. Tinoco
Linaro Kernel Validation