Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <f048b506-edae-1372-53d1-4b080fee8b9a@amd.com>
Date:   Mon, 11 Apr 2022 15:24:15 -0500
User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:91.0) Gecko/20100101
 Thunderbird/91.8.0
Subject: Re: [PATCH] amd_hsmp: Add HSMP protocol version 5 messages
Content-Language: en-US
To:     Naveen Krishna Chatradhi <nchatrad@amd.com>,
        platform-driver-x86@vger.kernel.org, linux-kernel@vger.kernel.org
Cc:     hdegoede@redhat.com, markgross@kernel.org, siva.sathappan@amd.com,
        Suma Hegde <suma.hegde@amd.com>
References: <20220411174723.547221-1-nchatrad@amd.com>
From:   Carlos Bilbao <carlos.bilbao@amd.com>
In-Reply-To: <20220411174723.547221-1-nchatrad@amd.com>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 7bit
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?d0M3VHVjY3JldVlqN2h2R053bWd6Z0g5OEVvcXFLY2JHU243ellUU2JUOTNW?=
 =?utf-8?B?WXNkdjRubSsyTzBoT2ZrbDJ4dkN6TnJmWTBBak93L2luQTFWZ1VOKzdEZnhU?=
 =?utf-8?B?ZmdyeS9abCtYcU9DS3lHbHRVUERwUTE0Nk43WDduM2FWWnQ5TDNZSms0dC8r?=
 =?utf-8?B?T3ZvcWR4cjVBTDlZUW9EcXl3bWNLbCs0MjdQbjNEVGZCdEk3ckJnVEQ2RGZW?=
 =?utf-8?B?d3lNRXdqU296dm5wTWJXR1JkVmVwY3gzVElUMlYrZ3JUSE1lVXR1R3R4WFh5?=
 =?utf-8?B?TVBZMG4vNHlVWTU2S3lTRUFaYjlaV3QyZkZMSm9ha3piZ2ExYjAyMHpidnJU?=
 =?utf-8?B?MFVxV0tZR1BtWWwyVnpxK3dOcnNoK1p1TDVCU1RVd3V2SzQrU2dlWlQzWkFS?=
 =?utf-8?B?UDhGbEV3eGlTWHlZdTRHck1rMWN2ZXNvNmVsUUlKcFlVa3I0VDdCSk1HUUtm?=
 =?utf-8?B?WEZuM2JVZW85YVVUMUJyeUN0S3lHeVVkRWR1Q3lSRlB2VWs3WjZFV2VTdUFJ?=
 =?utf-8?B?Y3dBV0JpMTdUQXdUa1J2K3liem9uSW0wWVdRM0RyMFFDOG45L3ZwK3BzeU8w?=
 =?utf-8?B?RVBqMit0ZlAxamtuSi9Tc3VSbjA4UEhNZ01iZWlGREh6VS96OTJxczhxWVF0?=
 =?utf-8?B?bFRIZ2VaKzM1N1FQN0NPVW9zN24wVjhPQ1RRMnZ3MEhiTy9kMCtVSlZ3VjVW?=
 =?utf-8?B?ODFuc2VNWlZEREs5cXp3bFh0ZWJoZ2FtWTlCL2N1MDllZW5SSWpkTDdmS0w4?=
 =?utf-8?B?aTdDTGFQNkFPcklCbXBRWTU3Z0EweXJXb1VjZlhKWUl4MUpPZEdVOGdWaWpl?=
 =?utf-8?B?dTJWNjlRcjFjQ21QZlFxanlYbFEvWU9DZ0lGckhKUjNtdE0reitrVHp5cWVx?=
 =?utf-8?B?OFMyTFZvSnBFQVBWUk1jUzhkY3dITUpPVG9KRjVKYTFDMWNnSE9PV2V2SGs4?=
 =?utf-8?B?eTNOMzM5eTZGSUhaMzJhWk5pSkNJbGhyUHRrYW9BU1JZK1BiVDJlTnNWMndv?=
 =?utf-8?B?TStaeHZrMitsYnNSWnRWSDZVSjhOeE5weDloVnh5VmlldVdWaEdIOWpBYTV3?=
 =?utf-8?B?TFVINEhYZGxuQmVjMjJMOTBCeGZnbDlLVmJLT2YxOWtwTjNnNzBvVGtiRlk0?=
 =?utf-8?B?cUMvb0NNb0Y2V2c5MUNlRU5Uejk5bmVia0xVR2loQ2JWTDB5bXEzb0VxZ0k2?=
 =?utf-8?B?WEo2a2h2MCt6YW1sN21ERVNGUGE2QlRsQWxIUEtCWnZ6eEtnMW9NbHNQbmhU?=
 =?utf-8?B?bjZTaXNTeEJ5YmRXV01ZZnoxZG9qQlBzSUl2TlVyNzNRSDNxU1FVN2xzcTJD?=
 =?utf-8?B?MFVpcXA1WXM5QVJKRE5xUXJ6U2RCTitIdUJXNjhGdEF6RmlYOVg2ays0V3ZE?=
 =?utf-8?B?NVJBVzgzb2E1MTdZYVNQMVZjK1FWK05CZVA0Z25NY3RGRko3ekhDSUhqM01C?=
 =?utf-8?B?dkRIMWpNbUtXKy93bkxKV1BmaWRWN2txZENOd2lUZjBQa2JLaUk4SnU4cyti?=
 =?utf-8?B?V1hvYWtIc0VyQnF6eVJtUFRvcmFMeUpLNmt0M2ovVTB2aDBLZlFDcGJzVDBn?=
 =?utf-8?B?dlRTRFYxMFlJYkV4STdXOEdpKzYyUU1hNng0QmdhQnlHcGJwZkZJd0Y2bmtm?=
 =?utf-8?B?NTRaUkVrcHZQR212Z2I4aGlKd2hGNENHM2dwNE9NaWo2SDgvTG02TnhJWlJm?=
 =?utf-8?B?Q3NOOERDTEJUWmoxSklMd00vaDNaczZpUjB0K3FiclQ3aGxtOGoydWtuYUZB?=
 =?utf-8?B?N09XTVNEbGdNcDJGMkJvQXJ5ajdIRWNHcCswYno1S1VKMVdCVnI2dGt4YWZx?=
 =?utf-8?B?Nk1sS0FsQzVUNEpFK3FqU2pPVW1oeVUzbWRiaUNtNGUrKzY3RW1sUFNFQStC?=
 =?utf-8?B?UHJxNW0rYXhrMU5POHJFeFRzdEd0MXZaQVRtUFNZUGZURkU2c2gvOHNScjRn?=
 =?utf-8?B?ekZHaE9qeHVEaEd5Zk5jbWZSWUlmWFdjcjhlbFNrcHUybXdnOUNwdFBVSEd5?=
 =?utf-8?B?bEpHekFhUWVyc0JhNWxXaXN0dEg3U0p1UG94YUtjK1BUT1VueGJNNzZFMXNK?=
 =?utf-8?B?QWJBU0t6c3FUbERJK1Z1enJ0d2VkVCtoZm1KVVpMWXBId1FDWVN5a2FsUmVt?=
 =?utf-8?B?d3orYXMwZXVBQytZUGNnNWwxVlZkYWlZK21xa1p6QWZMQStsTGM2SFVvQU80?=
 =?utf-8?B?MmI4VHdvM0tQSi9aUHdHczl2N0J3d05Xa1prOXNwaE5SNCtzVm13TGJZR3U2?=
 =?utf-8?B?NUFlNk5ydzZVSHA2UTA5QjFLSEdEaklCNVpGbUthR056SkdIa2ZwYmFPZUxn?=
 =?utf-8?B?M3dVOURqVmhsaHJvY2pFQTg0TElYb3ZMMlcrUGRpTExiWXp0YXB1UT09?=
X-OriginatorOrg: amd.com
X-MS-Exchange-CrossTenant-Network-Message-Id: ccc711d4-fd89-4a7e-7bf9-08da1bf940b7
X-MS-Exchange-CrossTenant-AuthSource: BL1PR12MB5874.namprd12.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 11 Apr 2022 20:24:17.5106
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 3dd8961f-e488-4e60-8e11-a82d994e183d
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: b4RZQrGRUB23cQgFpuYhMvlaPlj2lMUqnDuEJsITtXb+yZtqBXo8s8ML1AOG7Ra3uir6+8t7jJJFlN6wR43lkg==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: MWHPR12MB1774
X-Spam-Status: No, score=-3.7 required=5.0 tests=BAYES_00,DKIM_SIGNED,
        DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,NICE_REPLY_A,RCVD_IN_DNSWL_NONE,
        RCVD_IN_MSPIKE_H2,SPF_HELO_PASS,SPF_PASS,T_SCC_BODY_TEXT_LINE
        autolearn=ham autolearn_force=no version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on
        lindbergh.monkeyblade.net
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

On 4/11/2022 12:47 PM, Naveen Krishna Chatradhi wrote:
> From: Suma Hegde <suma.hegde@amd.com>
> 
> HSMP protocol version 5 is supported on AMD family 19h model 10h
> EPYC processors. This version brings new features such as
> -- DIMM statistics
> -- Bandiwdth for IO and xGMI links

s/Bandiwdth/Bandwidth  

> -- Monitor socket and core frequency limits
> -- Configure power efficiency modes, df pstate range etc
> 
> Briefly, describe the new messages.

I think this line should go.

> 
> Signed-off-by: Suma Hegde <suma.hegde@amd.com>
> Reviewed-by: Carlos Bilbao <carlos.bilbao@amd.com>
> Signed-off-by: Naveen Krishna Chatradhi <nchatrad@amd.com>
> ---
>  arch/x86/include/uapi/asm/amd_hsmp.h | 114 +++++++++++++++++++++++++--
>  1 file changed, 109 insertions(+), 5 deletions(-)
> 
> diff --git a/arch/x86/include/uapi/asm/amd_hsmp.h b/arch/x86/include/uapi/asm/amd_hsmp.h
> index 7ee7ba0d63a3..769b939444ae 100644
> --- a/arch/x86/include/uapi/asm/amd_hsmp.h
> +++ b/arch/x86/include/uapi/asm/amd_hsmp.h
> @@ -31,9 +31,22 @@ enum hsmp_message_ids {
>  	HSMP_GET_CCLK_THROTTLE_LIMIT,	/* 10h Get CCLK frequency limit in socket */
>  	HSMP_GET_C0_PERCENT,		/* 11h Get average C0 residency in socket */
>  	HSMP_SET_NBIO_DPM_LEVEL,	/* 12h Set max/min LCLK DPM Level for a given NBIO */
> -					/* 13h Reserved */
> -	HSMP_GET_DDR_BANDWIDTH = 0x14,	/* 14h Get theoretical maximum and current DDR Bandwidth */
> -	HSMP_GET_TEMP_MONITOR,		/* 15h Get per-DIMM temperature and refresh rates */
> +	HSMP_GET_NBIO_DPM_LEVEL,	/* 13h Get LCLK DPM level min and max for a given NBIO */
> +	HSMP_GET_DDR_BANDWIDTH,		/* 14h Get theoretical maximum and current DDR Bandwidth */
> +	HSMP_GET_TEMP_MONITOR,		/* 15h Get socket temperature */
> +	HSMP_GET_DIMM_TEMP_RANGE,	/* 16h Get per-DIMM temperature range and refresh rate */
> +	HSMP_GET_DIMM_POWER,		/* 17h Get per-DIMM power consumption */
> +	HSMP_GET_DIMM_THERMAL,		/* 18h Get per-DIMM thermal sensors */
> +	HSMP_GET_SOCKET_FREQ_LIMIT,	/* 19h Get current active frequency per socket */
> +	HSMP_GET_CCLK_CORE_LIMIT,	/* 1Ah Get CCLK frequency limit per core */
> +	HSMP_GET_RAILS_SVI,		/* 1Bh Get SVI-based Telemetry for all rails */
> +	HSMP_GET_SOCKET_FMAX_FMIN,	/* 1Ch Get Fmax and Fmin per socket */
> +	HSMP_GET_IOLINK_BANDWITH,	/* 1Dh Get current bandwidth on IO Link */
> +	HSMP_GET_XGMI_BANDWITH,		/* 1Eh Get current bandwidth on xGMI Link */
> +	HSMP_SET_GMI3_WIDTH,		/* 1Fh Set max and min GMI3 Link width */
> +	HSMP_SET_PCI_RATE,		/* 20h Control link rate on PCIe devices */
> +	HSMP_SET_POWER_MODE,		/* 21h Select power efficiency profile policy */
> +	HSMP_SET_PSTATE_MAX_MIN,	/* 22h Set the max and min DF P-State  */
>  	HSMP_MSG_ID_MAX,
>  };
>  
> @@ -175,8 +188,12 @@ static const struct hsmp_msg_desc hsmp_msg_desc_table[] = {
>  	 */
>  	{1, 0, HSMP_SET},
>  
> -	/* RESERVED message */
> -	{0, 0, HSMP_RSVD},
> +	/*
> +	 * HSMP_GET_NBIO_DPM_LEVEL, num_args = 1, response_sz = 1
> +	 * input: args[0] = nbioid[23:16]
> +	 * output: args[0] = max dpm level[15:8] + min dpm level[7:0]
> +	 */
> +	{1, 1, HSMP_GET},
>  
>  	/*
>  	 * HSMP_GET_DDR_BANDWIDTH, num_args = 0, response_sz = 1
> @@ -191,6 +208,93 @@ static const struct hsmp_msg_desc hsmp_msg_desc_table[] = {
>  	 * [7:5] fractional part
>  	 */
>  	{0, 1, HSMP_GET},
> +
> +	/*
> +	 * HSMP_GET_DIMM_TEMP_RANGE, num_args = 1, response_sz = 1
> +	 * input: args[0] = DIMM address[7:0]
> +	 * output: args[0] = refresh rate[3] + temperature range[2:0]
> +	 */
> +	{1, 1, HSMP_GET},
> +
> +	/*
> +	 * HSMP_GET_DIMM_POWER, num_args = 1, response_sz = 1
> +	 * input: args[0] = DIMM address[7:0]
> +	 * output: args[0] = DIMM power in mW[31:17] + update rate in ms[16:8] +
> +	 * DIMM address[7:0]
> +	 */
> +	{1, 1, HSMP_GET},
> +
> +	/*
> +	 * HSMP_GET_DIMM_THERMAL, num_args = 1, response_sz = 1
> +	 * input: args[0] = DIMM address[7:0]
> +	 * output: args[0] = temperature in degree celcius[31:21] + update rate in ms[16:8] +
> +	 * DIMM address[7:0]
> +	 */
> +	{1, 1, HSMP_GET},
> +
> +	/*
> +	 * HSMP_GET_SOCKET_FREQ_LIMIT, num_args = 0, response_sz = 1
> +	 * output: args[0] = frequency in MHz[31:16] + frequency source[15:0]
> +	 */
> +	{0, 1, HSMP_GET},
> +
> +	/*
> +	 * HSMP_GET_CCLK_CORE_LIMIT, num_args = 1, response_sz = 1
> +	 * input: args[0] = apic id [31:0]
> +	 * output: args[0] = frequency in MHz[31:0]
> +	 */
> +	{1, 1, HSMP_GET},
> +
> +	/*
> +	 * HSMP_GET_RAILS_SVI, num_args = 0, response_sz = 1
> +	 * output: args[0] = power in mW[31:0]
> +	 */
> +	{0, 1, HSMP_GET},
> +
> +	/*
> +	 * HSMP_GET_SOCKET_FMAX_FMIN, num_args = 0, response_sz = 1
> +	 * output: args[0] = fmax in MHz[31:16] + fmin in MHz[15:0]
> +	 */
> +	{0, 1, HSMP_GET},
> +
> +	/*
> +	 * HSMP_GET_IOLINK_BANDWITH, num_args = 1, response_sz = 1
> +	 * input: args[0] = link id[15:8] + bw type[2:0]
> +	 * output: args[0] = io bandwidth in Mbps[31:0]
> +	 */
> +	{1, 1, HSMP_GET},
> +
> +	/*
> +	 * HSMP_GET_XGMI_BANDWITH, num_args = 1, response_sz = 1
> +	 * input: args[0] = link id[15:8] + bw type[2:0]
> +	 * output: args[0] = xgmi bandwidth in Mbps[31:0]
> +	 */
> +	{1, 1, HSMP_GET},
> +
> +	/*
> +	 * HSMP_SET_GMI3_WIDTH, num_args = 1, response_sz = 0
> +	 * input: args[0] = min link width[15:8] + max link width[7:0]
> +	 */
> +	{1, 0, HSMP_SET},
> +
> +	/*
> +	 * HSMP_SET_PCI_RATE, num_args = 1, response_sz = 1
> +	 * input: args[0] = link rate control value
> +	 * output: args[0] = previous link rate control value
> +	 */
> +	{1, 1, HSMP_SET},
> +
> +	/*
> +	 * HSMP_SET_POWER_MODE, num_args = 1, response_sz = 0
> +	 * input: args[0] = power efficiency mode[2:0]
> +	 */
> +	{1, 0, HSMP_SET},
> +
> +	/*
> +	 * HSMP_SET_PSTATE_MAX_MIN, num_args = 1, response_sz = 0
> +	 * input: args[0] = min df pstate[15:8] + max df pstate[7:0]
> +	 */
> +	{1, 0, HSMP_SET},
>  };
>  
>  /* Reset to default packing */

Other than that I stand by my Reviewed-by!

Thanks,
Carlos