Hi Ilpo Jarvinen, Thanks for the suggestions. On 8/22/2023 2:27 PM, Ilpo Järvinen wrote:
Caution: This message originated from an External Source. Use proper caution when opening attachments, clicking links, or responding. On Fri, 18 Aug 2023, Suma Hegde wrote:From: Suma Hegde <suma.hegde@xxxxxxx> AMD MI300 MCM provides GET_METRICS_TABLE message with which all the system management information from SMU can be retrieved in just one message. The metrics table is available as hexadecimal sysfs binary file in /sys/devices/platform/amd_hsmp/socket%d_metrics_bin Metrics table definitions will be documented as part of PPR which available in public domain. The same is defined in the amd_hsmp.h header file as well. Signed-off-by: Suma Hegde <suma.hegde@xxxxxxx> Reviewed-by: Naveen Krishna Chatradhi <nchatrad@xxxxxxx> --- +#define HSMP_ATRR_NAME_SIZE 25 +static int hsmp_create_metric_tbl_sysfs_file(int sock_ind) +{ + struct bin_attribute *hattr = &plat_dev.sock[sock_ind].hsmp_attr; + char *name; + + sysfs_attr_init(&plat_dev.sock[sock_ind].hsmp_attr); + name = devm_kzalloc(plat_dev.dev, HSMP_ATRR_NAME_SIZE, GFP_KERNEL); + if (!name) + return -ENOMEM; + sprintf(name, "socket%d_metrics_bin", sock_ind);The structure would be more future-proof if you do socket%d/metrics_bin instead. Are you expected to add at some point in the future more files which are socket specific? You didn't provide any documentation for this interface btw, you should add it.
We are not expecting any new socket files right now, but in future we might create files for some more socket specific info.
So I will add directories for sockets and then will create files. I will update documentation too.
+ hattr->attr.name = name; + hattr->attr.mode = 0444; + hattr->read = hsmp_metric_tbl_read; + hattr->size = sizeof(struct hsmp_metric_table); + hattr->private = &plat_dev.sock[sock_ind]; + + return device_create_bin_file(plat_dev.dev, hattr); +} +static int hsmp_create_sysfs_file(void) +{ + int ret, i; + + for (i = 0; i < num_sockets; i++) { + ret = hsmp_get_tbl_dram_base(i); + if (ret) + goto cleanup; + + ret = hsmp_create_metric_tbl_sysfs_file(i); + if (ret) { + dev_err(plat_dev.dev, "Unable to create sysfs file for metric table\n"); + goto cleanup; + } + } + + return 0; + +cleanup: + while (i > 0) + device_remove_bin_file(plat_dev.dev, &plat_dev.sock[--i].hsmp_attr); + return ret; +} + +static int hsmp_cache_proto_ver(void) +{ + struct hsmp_message msg = { 0 }; + int ret; + + msg.msg_id = HSMP_GET_PROTO_VER; + msg.sock_ind = 0; + msg.response_sz = hsmp_msg_desc_table[HSMP_GET_PROTO_VER].response_sz; + + ret = hsmp_send_message(&msg); + if (!ret) + plat_dev.proto_ver = msg.args[0]; + + return ret; +} + static int hsmp_pltdrv_probe(struct platform_device *pdev) { - int i; + int ret, i; plat_dev.sock = devm_kzalloc(&pdev->dev, (num_sockets * sizeof(struct hsmp_socket)), @@ -344,18 +463,44 @@ static int hsmp_pltdrv_probe(struct platform_device *pdev) plat_dev.sock[i].sock_ind = i; } - plat_dev.hsmp_device.name = "hsmp_cdev"; + plat_dev.hsmp_device.name = HSMP_CDEV_NAME; plat_dev.hsmp_device.minor = MISC_DYNAMIC_MINOR; plat_dev.hsmp_device.fops = &hsmp_fops; plat_dev.hsmp_device.parent = &pdev->dev; - plat_dev.hsmp_device.nodename = "hsmp"; + plat_dev.hsmp_device.nodename = HSMP_DEVNODE_NAME; plat_dev.hsmp_device.mode = 0644; - return misc_register(&plat_dev.hsmp_device); + ret = misc_register(&plat_dev.hsmp_device); + if (ret) + return ret; + + ret = hsmp_cache_proto_ver(); + if (ret) { + dev_err(plat_dev.dev, "Failed to read HSMP protocol version\n"); + goto cleanup; + } + + /* metrics table is supported only from proto ver6 EPYCs */ + if (plat_dev.proto_ver >= HSMP_PROTO_VER6) { + ret = hsmp_create_sysfs_file();For new things, driver/device attribute_group should be used where possible. .is_visible() can handle that condition check for you.
Ok, will do the required modification.
-- i.
Thank you, Suma
