Hi Song, Carlos
On 2/17/2022 3:54 AM, Carlos Bilbao wrote:
Hello,
On 2/16/2022 4:06 PM, Song Liu wrote:
On Wed, Feb 16, 2022 at 5:34 AM Naveen Krishna Chatradhi
<nchatrad@xxxxxxx> wrote:
From: Suma Hegde <suma.hegde@xxxxxxx>
Recent Fam19h EPYC server line of processors from AMD support system
management functionality via HSMP (Host System Management Port) interface.
The Host System Management Port (HSMP) is an interface to provide
OS-level software with access to system management functions via a
set of mailbox registers.
More details on the interface can be found in chapter
"7 Host System Management Port (HSMP)" of the following PPR
https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fwww.amd.com%2Fsystem%2Ffiles%2FTechDocs%2F55898_B1_pub_0.50.zip&data=04%7C01%7Ccarlos.bilbao%40amd.com%7Cb33402053aa6443bf4f308d9f198a7fe%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637806460908911848%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000&sdata=F%2BQgWoHlueI1CW14pJHhY4%2FwM5rOTOj0D3IO%2B7yhrbQ%3D&reserved=0
This patch adds new amd_hsmp module under the drivers/platforms/x86/
which creates miscdevice with an IOCTL interface to the user space.
/dev/hsmp is for running the hsmp mailbox commands.
Signed-off-by: Suma Hegde <suma.hegde@xxxxxxx>
Signed-off-by: Naveen Krishna Chatradhi <nchatrad@xxxxxxx>
Reviewed-by: Carlos Bilbao <carlos.bilbao@xxxxxxx>
Acked-by: Song Liu <song@xxxxxxxxxx>
With a couple minor comments below.
---
[...]
+
+struct hsmp_message {
+ __u32 msg_id; /* Message ID */
+ __u16 num_args; /* Number of input argument words in message */
+ __u16 response_sz; /* Number of expected output/response words */
+ __u32 buf[HSMP_MAX_MSG_LEN]; /* argument/response buffer */
How about we call these args instead of buf?
Now that we are using this member for both input arguments and output
response, I've changed it to a
generic name, no problem will change it back to args.
[...]
+
+static long hsmp_ioctl(struct file *fp, unsigned int cmd, unsigned long arg)
+{
+ int __user *arguser = (int __user *)arg;
+ struct hsmp_message msg = { 0 };
+ int ret;
+
+ if (copy_struct_from_user(&msg, sizeof(msg), arguser, sizeof(struct hsmp_message)))
+ return -EFAULT;
+
+ ret = validate_message(&msg);
We call validate_message() twice in this path. This is not a big issue, but it
will be nice if we can avoid the extra check.
Yes, we can probably just rely on hsmp_send_message. We shouldn't remove it
from there since we export that function.
We are using msg_id to reference elements from the table. so, i will
validate the msg.msg_id
is with in the array bounds of hsmp_msg_desc_table[] here.
[...]
I have reviewed the patch, compiled with several configurations (including
allyes) and functionally tested. Also not a single hole on pahole.
Everything looks in great shape to me.
Thank you.
Thanks,
Carlos
