Re: [PATCH v4 1/2] platforms/x86: Add AMD system management interface

[Nathan Fontenot <nafonten@amd.com>]

On 2/14/22 11:21, Song Liu wrote:
> On Mon, Feb 14, 2022 at 7:32 AM Chatradhi, Naveen Krishna
> <nchatrad@amd.com> wrote:
>>
>> Hi Song,
>>
>> On 2/11/2022 3:10 AM, Song Liu wrote:
> [...]
>>
>> HSMP mailbox messages are evolving and each platform defines a supported
>> list of messages.
>>
>> On a given platform these messages are described in the PPR.
>>
>> Eg: Milan PPR has "7 Host System Management Port (HSMP)", is made public
>>
>> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fwww.amd.com%2Fsystem%2Ffiles%2FTechDocs%2F55898_B1_pub_0.50.zip&amp;data=04%7C01%7CNathan.Fontenot%40amd.com%7C6026990f93ed41d04d0608d9efde79a1%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637804561503265585%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000&amp;sdata=o5BR%2FgHpjnNVa9zVu75I3RyzO8WNB4L7FMEvLNbzZVY%3D&amp;reserved=0
>>
>>
>> Bringing detailed description of these messages from PPR into the kernel
>> would be a
>>
>> duplicating effort. Which will also bring in challenges such as
>> maintaining the details
>>
>> for every supported platform and submitting kernel patches for every
>> platform.
>>
>>
>> We would like to avoid bringing more details of these messages to the
>> kernel documentation.
>>
>> Such a structure can be described as part of esmi_oob_library to ease
>> user space tool development.
> 
> I agree there is  extra effort to adding extra logic and documentations to
> the kernel. How about we ship current version with a few minor changes:
> 1). msg_desc_table is in the header, so please prefix it with hsmp_
> 2) as Nathan suggested, add more comments to msg_desc_table. Maybe
> something like:
> /*
>  * HSMP_GET_FCLK_MCLK,
>  * output arg0 = fclk (MHz); arg1 = mclk (MHz)
>  */

Agreed, this is the type of update I was thinking of.

-Nathan

> {0, 2, GET},
> 
> Besides these, I have a question. Per the hardware design, args and
> reponse in hsmp_message share the same registers. Shall we make
> them in/out argments in hsmp_message and save 256 bytes per
> hsmp_message?
> 
> Thanks,
> Song