Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
Subject: Re: [PATCH AUTOSEL 4.19 022/146] cpupower: Fix coredump on VMWare
To:     Sasha Levin <sashal@kernel.org>, stable@vger.kernel.org,
        linux-kernel@vger.kernel.org
Cc:     Shuah Khan <shuah@kernel.org>, Stafford Horne <shorne@gmail.com>
References: <20181031230541.28822-1-sashal@kernel.org>
 <20181031230541.28822-22-sashal@kernel.org>
From:   Prarit Bhargava <prarit@redhat.com>
Message-ID: <7c08983b-a8c5-0dbc-c8aa-2c6a6799e1ee@redhat.com>
Date:   Thu, 1 Nov 2018 07:43:35 -0400
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101
 Thunderbird/52.9.1
MIME-Version: 1.0
In-Reply-To: <20181031230541.28822-22-sashal@kernel.org>
Content-Type: text/plain; charset=utf-8
Content-Language: en-GB
Content-Transfer-Encoding: 7bit
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk


On 10/31/2018 07:03 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.
> 

Hi Sasha,

This patch, f69ffc5d3db8, depends on 8c22e2f69592 ("cpupower: Fix AMD Family
0x17 msr_pstate size").  Without 8c22e2f69592 the patch below will always read a
value of "0" and not output the correct data.

8c22e2f69592 must be applied to any stable branch that f69ffc5d3db8 is applied to.

P.


> 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 df43cd45d810..ccd08dd00996 100644
> --- a/tools/power/cpupower/utils/cpufreq-info.c
> +++ b/tools/power/cpupower/utils/cpufreq-info.c
> @@ -200,6 +200,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 bb41cdd0df6b..58d23997424d 100644
> --- a/tools/power/cpupower/utils/helpers/amd.c
> +++ b/tools/power/cpupower/utils/helpers/amd.c
> @@ -119,6 +119,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;
>