| Message ID | 20240903123830.2717196-8-suma.hegde@amd.com (mailing list archive) |
|---|---|
| State | Changes Requested, archived |
| Headers | show |
| Series | [v7,01/10] platform/x86/amd/hsmp: Create hsmp/ directory | expand |
On Tue, 3 Sep 2024, Suma Hegde wrote: > Separate the probes for HSMP ACPI and platform device drivers. > > Provide a Kconfig option to choose between > ACPI or the platform device based driver. > The common code which is the core part of the HSMP driver > maintained at hsmp.c is guarded by AMD_HSMP config and is selected by > these two driver configs. > > Also add "|| COMPILE_TEST" clause in Kconfig to get build coverage > for HSMP. > > Signed-off-by: Suma Hegde <suma.hegde@amd.com> > Reviewed-by: Naveen Krishna Chatradhi <naveenkrishna.chatradhi@amd.com> Reviewed-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com> This series looks ready as far as I can see. Thanks for all the work you've done to improve it! -- i.
Hi, On 9/3/24 2:38 PM, Suma Hegde wrote: > Separate the probes for HSMP ACPI and platform device drivers. > > Provide a Kconfig option to choose between > ACPI or the platform device based driver. > The common code which is the core part of the HSMP driver > maintained at hsmp.c is guarded by AMD_HSMP config and is selected by > these two driver configs. <snip> > diff --git a/drivers/platform/x86/amd/hsmp/Kconfig b/drivers/platform/x86/amd/hsmp/Kconfig > index b55d4ed9bceb..b10ff91e9f5a 100644 > --- a/drivers/platform/x86/amd/hsmp/Kconfig > +++ b/drivers/platform/x86/amd/hsmp/Kconfig > @@ -4,14 +4,45 @@ > # > > config AMD_HSMP > - tristate "AMD HSMP Driver" > - depends on AMD_NB && X86_64 && ACPI > + tristate > + > +menu "AMD HSMP Driver" > + depends on AMD_NB || COMPILE_TEST > + > +config AMD_HSMP_ACPI > + tristate "AMD HSMP ACPI device driver" > + depends on ACPI > + select AMD_HSMP > help > + Host System Management Port (HSMP) interface is a mailbox interface > + between the x86 core and the System Management Unit (SMU) firmware. > The driver provides a way for user space tools to monitor and manage > - system management functionality on EPYC server CPUs from AMD. > + system management functionality on EPYC and MI300A server CPUs > + from AMD. > + > + This option supports ACPI based probing. > + You may enable this, if your platform BIOS provides an ACPI object > + as described in amd_hsmp.rst document. > + > + If you choose to compile this driver as a module the module will be > + called amd_hsmp. > > +config AMD_HSMP_PLAT > + tristate "AMD HSMP platform device driver" > + depends on AMD_HSMP_ACPI=n I was about to merge this series, but this is going to cause a regression for users running distro kernels which rely on the old legacy probing. So before this we had 1 driver which would auto-load on systems which have the new ACPI description of the HSMP and which could be manually modprobed on systems which require the legacy enumeration. But now if linux distributions enable AMD_HSMP_ACPI then there will be no way for users with systems which lack the ACPI description of the HSMP to still get HSMP support. I guess what you want here is to avoid the legacy driver loading on systems which do have the ACPI description. But the way to do that would be to do a runtime check for the ACPI description, not disallow building the legacy driver altogether. E.g. you could do the following in hsmp_plt_init() : if (acpi_dev_present(ACPI_HSMP_DEVICE_HID, NULL, -1)) { pr_err("The legacy HSMP driver cannot load on this system, please use hsmp_acpi instead\n"); return -ENODEV; } I see that you also build both drivers into a single module, yes then you cannot have both because you cannot have 2 init functions. Please split things into 3 modules, one shared hsmp_common.ko + a hsmp_acpi.ko + hsmp_legacy.ko Regards, Hans > + select AMD_HSMP > + help > Host System Management Port (HSMP) interface is a mailbox interface > between the x86 core and the System Management Unit (SMU) firmware. > + The driver provides a way for user space tools to monitor and manage > + system management functionality on EPYC and MI300A server CPUs > + from AMD. > + > + This option supports platform device based probing. > + You may enable this, if your platform BIOS does not provide > + HSMP ACPI object. > > If you choose to compile this driver as a module the module will be > called amd_hsmp. > + > +endmenu > diff --git a/drivers/platform/x86/amd/hsmp/Makefile b/drivers/platform/x86/amd/hsmp/Makefile > index 0cc92865c0a2..18d9a0d1e8c5 100644 > --- a/drivers/platform/x86/amd/hsmp/Makefile > +++ b/drivers/platform/x86/amd/hsmp/Makefile > @@ -4,5 +4,7 @@ > # AMD HSMP Driver > # > > -obj-$(CONFIG_AMD_HSMP) += amd_hsmp.o > -amd_hsmp-objs := hsmp.o plat.o acpi.o > +obj-$(CONFIG_AMD_HSMP) += amd_hsmp.o > +amd_hsmp-objs := hsmp.o > +amd_hsmp-$(CONFIG_AMD_HSMP_PLAT) += plat.o > +amd_hsmp-$(CONFIG_AMD_HSMP_ACPI) += acpi.o > diff --git a/drivers/platform/x86/amd/hsmp/acpi.c b/drivers/platform/x86/amd/hsmp/acpi.c > index 6f8e7962266a..766617e6adc7 100644 > --- a/drivers/platform/x86/amd/hsmp/acpi.c > +++ b/drivers/platform/x86/amd/hsmp/acpi.c > @@ -9,11 +9,15 @@ > > #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt > > +#include <asm/amd_nb.h> > + > #include <linux/acpi.h> > #include <linux/device.h> > #include <linux/dev_printk.h> > #include <linux/ioport.h> > #include <linux/kstrtox.h> > +#include <linux/module.h> > +#include <linux/platform_device.h> > #include <linux/sysfs.h> > #include <linux/uuid.h> > > @@ -21,6 +25,10 @@ > > #include "hsmp.h" > > +#define DRIVER_NAME "amd_hsmp" > +#define DRIVER_VERSION "2.3" > +#define ACPI_HSMP_DEVICE_HID "AMDI0097" > + > /* These are the strings specified in ACPI table */ > #define MSG_IDOFF_STR "MsgIdOffset" > #define MSG_ARGOFF_STR "MsgArgOffset" > @@ -200,7 +208,6 @@ static int hsmp_parse_acpi_table(struct device *dev, u16 sock_ind) > sock->sock_ind = sock_ind; > sock->dev = dev; > sock->amd_hsmp_rdwr = amd_hsmp_acpi_rdwr; > - hsmp_pdev.is_acpi_device = true; > > sema_init(&sock->hsmp_sem, 1); > > @@ -213,7 +220,7 @@ static int hsmp_parse_acpi_table(struct device *dev, u16 sock_ind) > return hsmp_read_acpi_dsd(sock); > } > > -int hsmp_create_acpi_sysfs_if(struct device *dev) > +static int hsmp_create_acpi_sysfs_if(struct device *dev) > { > struct attribute_group *attr_grp; > u16 sock_ind; > @@ -236,7 +243,7 @@ int hsmp_create_acpi_sysfs_if(struct device *dev) > return devm_device_add_group(dev, attr_grp); > } > > -int init_acpi(struct device *dev) > +static int init_acpi(struct device *dev) > { > u16 sock_ind; > int ret; > @@ -270,3 +277,72 @@ int init_acpi(struct device *dev) > > return ret; > } > + > +static const struct acpi_device_id amd_hsmp_acpi_ids[] = { > + {ACPI_HSMP_DEVICE_HID, 0}, > + {} > +}; > +MODULE_DEVICE_TABLE(acpi, amd_hsmp_acpi_ids); > + > +static int hsmp_acpi_probe(struct platform_device *pdev) > +{ > + int ret; > + > + if (!hsmp_pdev.is_probed) { > + hsmp_pdev.num_sockets = amd_nb_num(); > + if (hsmp_pdev.num_sockets == 0 || hsmp_pdev.num_sockets > MAX_AMD_SOCKETS) > + return -ENODEV; > + > + hsmp_pdev.sock = devm_kcalloc(&pdev->dev, hsmp_pdev.num_sockets, > + sizeof(*hsmp_pdev.sock), > + GFP_KERNEL); > + if (!hsmp_pdev.sock) > + return -ENOMEM; > + } > + > + ret = init_acpi(&pdev->dev); > + if (ret) { > + dev_err(&pdev->dev, "Failed to initialize HSMP interface.\n"); > + return ret; > + } > + > + ret = hsmp_create_acpi_sysfs_if(&pdev->dev); > + if (ret) > + dev_err(&pdev->dev, "Failed to create HSMP sysfs interface\n"); > + > + if (!hsmp_pdev.is_probed) { > + ret = hsmp_misc_register(&pdev->dev); > + if (ret) > + return ret; > + hsmp_pdev.is_probed = true; > + } > + > + return 0; > +} > + > +static void hsmp_acpi_remove(struct platform_device *pdev) > +{ > + /* > + * We register only one misc_device even on multi-socket system. > + * So, deregister should happen only once. > + */ > + if (hsmp_pdev.is_probed) { > + hsmp_misc_deregister(); > + hsmp_pdev.is_probed = false; > + } > +} > + > +static struct platform_driver amd_hsmp_driver = { > + .probe = hsmp_acpi_probe, > + .remove_new = hsmp_acpi_remove, > + .driver = { > + .name = DRIVER_NAME, > + .acpi_match_table = amd_hsmp_acpi_ids, > + }, > +}; > + > +module_platform_driver(amd_hsmp_driver); > + > +MODULE_DESCRIPTION("AMD HSMP Platform Interface Driver"); > +MODULE_VERSION(DRIVER_VERSION); > +MODULE_LICENSE("GPL"); > diff --git a/drivers/platform/x86/amd/hsmp/hsmp.c b/drivers/platform/x86/amd/hsmp/hsmp.c > index 78945750d590..5e0c9c36f435 100644 > --- a/drivers/platform/x86/amd/hsmp/hsmp.c > +++ b/drivers/platform/x86/amd/hsmp/hsmp.c > @@ -15,17 +15,11 @@ > #include <linux/acpi.h> > #include <linux/delay.h> > #include <linux/device.h> > -#include <linux/module.h> > -#include <linux/platform_device.h> > #include <linux/semaphore.h> > #include <linux/sysfs.h> > > #include "hsmp.h" > > -#define DRIVER_NAME "amd_hsmp" > -#define DRIVER_VERSION "2.2" > -#define ACPI_HSMP_DEVICE_HID "AMDI0097" > - > /* HSMP Status / Error codes */ > #define HSMP_STATUS_NOT_READY 0x00 > #define HSMP_STATUS_OK 0x01 > @@ -228,7 +222,7 @@ int hsmp_test(u16 sock_ind, u32 value) > return ret; > } > > -static long hsmp_ioctl(struct file *fp, unsigned int cmd, unsigned long arg) > +long hsmp_ioctl(struct file *fp, unsigned int cmd, unsigned long arg) > { > int __user *arguser = (int __user *)arg; > struct hsmp_message msg = { 0 }; > @@ -284,12 +278,6 @@ static long hsmp_ioctl(struct file *fp, unsigned int cmd, unsigned long arg) > return 0; > } > > -static const struct file_operations hsmp_fops = { > - .owner = THIS_MODULE, > - .unlocked_ioctl = hsmp_ioctl, > - .compat_ioctl = hsmp_ioctl, > -}; > - > ssize_t hsmp_metric_tbl_read(struct file *filp, struct kobject *kobj, > struct bin_attribute *bin_attr, char *buf, > loff_t off, size_t count) > @@ -414,193 +402,25 @@ int hsmp_cache_proto_ver(u16 sock_ind) > return ret; > } > > -static const struct acpi_device_id amd_hsmp_acpi_ids[] = { > - {ACPI_HSMP_DEVICE_HID, 0}, > - {} > -}; > -MODULE_DEVICE_TABLE(acpi, amd_hsmp_acpi_ids); > - > -static bool check_acpi_support(struct device *dev) > -{ > - struct acpi_device *adev = ACPI_COMPANION(dev); > - > - if (adev && !acpi_match_device_ids(adev, amd_hsmp_acpi_ids)) > - return true; > - > - return false; > -} > - > -static int hsmp_pltdrv_probe(struct platform_device *pdev) > -{ > - int ret; > - > - /* > - * On ACPI supported BIOS, there is an ACPI HSMP device added for > - * each socket, so the per socket probing, but the memory allocated for > - * sockets should be contiguous to access it as an array, > - * Hence allocate memory for all the sockets at once instead of allocating > - * on each probe. > - */ > - if (!hsmp_pdev.is_probed) { > - hsmp_pdev.sock = devm_kcalloc(&pdev->dev, hsmp_pdev.num_sockets, > - sizeof(*hsmp_pdev.sock), > - GFP_KERNEL); > - if (!hsmp_pdev.sock) > - return -ENOMEM; > - } > - if (check_acpi_support(&pdev->dev)) { > - ret = init_acpi(&pdev->dev); > - if (ret) { > - dev_err(&pdev->dev, "Failed to init HSMP mailbox\n"); > - return ret; > - } > - ret = hsmp_create_acpi_sysfs_if(&pdev->dev); > - if (ret) > - dev_err(&pdev->dev, "Failed to create HSMP sysfs interface\n"); > - } else { > - ret = init_platform_device(&pdev->dev); > - if (ret) { > - dev_err(&pdev->dev, "Failed to init HSMP mailbox\n"); > - return ret; > - } > - ret = hsmp_create_non_acpi_sysfs_if(&pdev->dev); > - if (ret) > - dev_err(&pdev->dev, "Failed to create HSMP sysfs interface\n"); > - } > - > - if (!hsmp_pdev.is_probed) { > - hsmp_pdev.mdev.name = HSMP_CDEV_NAME; > - hsmp_pdev.mdev.minor = MISC_DYNAMIC_MINOR; > - hsmp_pdev.mdev.fops = &hsmp_fops; > - hsmp_pdev.mdev.parent = &pdev->dev; > - hsmp_pdev.mdev.nodename = HSMP_DEVNODE_NAME; > - hsmp_pdev.mdev.mode = 0644; > - > - ret = misc_register(&hsmp_pdev.mdev); > - if (ret) > - return ret; > - > - hsmp_pdev.is_probed = true; > - } > - > - return 0; > - > -} > - > -static void hsmp_pltdrv_remove(struct platform_device *pdev) > -{ > - /* > - * We register only one misc_device even on multi socket system. > - * So, deregister should happen only once. > - */ > - if (hsmp_pdev.is_probed) { > - misc_deregister(&hsmp_pdev.mdev); > - hsmp_pdev.is_probed = false; > - } > -} > - > -static struct platform_driver amd_hsmp_driver = { > - .probe = hsmp_pltdrv_probe, > - .remove_new = hsmp_pltdrv_remove, > - .driver = { > - .name = DRIVER_NAME, > - .acpi_match_table = amd_hsmp_acpi_ids, > - }, > +static const struct file_operations hsmp_fops = { > + .owner = THIS_MODULE, > + .unlocked_ioctl = hsmp_ioctl, > + .compat_ioctl = hsmp_ioctl, > }; > > -static struct platform_device *amd_hsmp_platdev; > - > -static int hsmp_plat_dev_register(void) > +int hsmp_misc_register(struct device *dev) > { > - int ret; > - > - amd_hsmp_platdev = platform_device_alloc(DRIVER_NAME, PLATFORM_DEVID_NONE); > - if (!amd_hsmp_platdev) > - return -ENOMEM; > - > - ret = platform_device_add(amd_hsmp_platdev); > - if (ret) > - platform_device_put(amd_hsmp_platdev); > - > - return ret; > + hsmp_pdev.mdev.name = HSMP_CDEV_NAME; > + hsmp_pdev.mdev.minor = MISC_DYNAMIC_MINOR; > + hsmp_pdev.mdev.fops = &hsmp_fops; > + hsmp_pdev.mdev.parent = dev; > + hsmp_pdev.mdev.nodename = HSMP_DEVNODE_NAME; > + hsmp_pdev.mdev.mode = 0644; > + > + return misc_register(&hsmp_pdev.mdev); > } > > -/* > - * This check is only needed for backward compatibility of previous platforms. > - * All new platforms are expected to support ACPI based probing. > - */ > -static bool legacy_hsmp_support(void) > +void hsmp_misc_deregister(void) > { > - if (boot_cpu_data.x86_vendor != X86_VENDOR_AMD) > - return false; > - > - switch (boot_cpu_data.x86) { > - case 0x19: > - switch (boot_cpu_data.x86_model) { > - case 0x00 ... 0x1F: > - case 0x30 ... 0x3F: > - case 0x90 ... 0x9F: > - case 0xA0 ... 0xAF: > - return true; > - default: > - return false; > - } > - case 0x1A: > - switch (boot_cpu_data.x86_model) { > - case 0x00 ... 0x1F: > - return true; > - default: > - return false; > - } > - default: > - return false; > - } > - > - return false; > + misc_deregister(&hsmp_pdev.mdev); > } > - > -static int __init hsmp_plt_init(void) > -{ > - int ret = -ENODEV; > - > - /* > - * amd_nb_num() returns number of SMN/DF interfaces present in the system > - * if we have N SMN/DF interfaces that ideally means N sockets > - */ > - hsmp_pdev.num_sockets = amd_nb_num(); > - if (hsmp_pdev.num_sockets == 0 || hsmp_pdev.num_sockets > MAX_AMD_SOCKETS) > - return ret; > - > - ret = platform_driver_register(&amd_hsmp_driver); > - if (ret) > - return ret; > - > - if (!hsmp_pdev.is_acpi_device) { > - if (legacy_hsmp_support()) { > - /* Not ACPI device, but supports HSMP, register a plat_dev */ > - ret = hsmp_plat_dev_register(); > - } else { > - /* Not ACPI, Does not support HSMP */ > - pr_info("HSMP is not supported on Family:%x model:%x\n", > - boot_cpu_data.x86, boot_cpu_data.x86_model); > - ret = -ENODEV; > - } > - if (ret) > - platform_driver_unregister(&amd_hsmp_driver); > - } > - > - return ret; > -} > - > -static void __exit hsmp_plt_exit(void) > -{ > - platform_device_unregister(amd_hsmp_platdev); > - platform_driver_unregister(&amd_hsmp_driver); > -} > - > -device_initcall(hsmp_plt_init); > -module_exit(hsmp_plt_exit); > - > -MODULE_DESCRIPTION("AMD HSMP Platform Interface Driver"); > -MODULE_VERSION(DRIVER_VERSION); > -MODULE_LICENSE("GPL v2"); > diff --git a/drivers/platform/x86/amd/hsmp/hsmp.h b/drivers/platform/x86/amd/hsmp/hsmp.h > index 9c5b9c263fc1..9ab50bc74676 100644 > --- a/drivers/platform/x86/amd/hsmp/hsmp.h > +++ b/drivers/platform/x86/amd/hsmp/hsmp.h > @@ -52,7 +52,6 @@ struct hsmp_plat_device { > struct hsmp_socket *sock; > u32 proto_ver; > u16 num_sockets; > - bool is_acpi_device; > bool is_probed; > }; > > @@ -61,14 +60,13 @@ extern struct hsmp_plat_device hsmp_pdev; > ssize_t hsmp_metric_tbl_read(struct file *filp, struct kobject *kobj, > struct bin_attribute *bin_attr, char *buf, > loff_t off, size_t count); > -int hsmp_create_non_acpi_sysfs_if(struct device *dev); > -int hsmp_create_acpi_sysfs_if(struct device *dev); > int hsmp_cache_proto_ver(u16 sock_ind); > umode_t hsmp_is_sock_attr_visible(struct kobject *kobj, > struct bin_attribute *battr, int id); > int hsmp_create_attr_list(struct attribute_group *attr_grp, > struct device *dev, u16 sock_ind); > int hsmp_test(u16 sock_ind, u32 value); > -int init_platform_device(struct device *dev); > -int init_acpi(struct device *dev); > +long hsmp_ioctl(struct file *fp, unsigned int cmd, unsigned long arg); > +void hsmp_misc_deregister(void); > +int hsmp_misc_register(struct device *dev); > #endif /* HSMP_H */ > diff --git a/drivers/platform/x86/amd/hsmp/plat.c b/drivers/platform/x86/amd/hsmp/plat.c > index e18cf82478a0..e49b53f8c5e3 100644 > --- a/drivers/platform/x86/amd/hsmp/plat.c > +++ b/drivers/platform/x86/amd/hsmp/plat.c > @@ -12,11 +12,16 @@ > #include <asm/amd_nb.h> > > #include <linux/device.h> > +#include <linux/module.h> > #include <linux/pci.h> > +#include <linux/platform_device.h> > #include <linux/sysfs.h> > > #include "hsmp.h" > > +#define DRIVER_NAME "amd_hsmp" > +#define DRIVER_VERSION "2.3" > + > /* > * To access specific HSMP mailbox register, s/w writes the SMN address of HSMP mailbox > * register into the SMN_INDEX register, and reads/writes the SMN_DATA reg. > @@ -50,7 +55,13 @@ static int amd_hsmp_pci_rdwr(struct hsmp_socket *sock, u32 offset, > return ret; > } > > -int hsmp_create_non_acpi_sysfs_if(struct device *dev) > +static const struct file_operations hsmp_fops = { > + .owner = THIS_MODULE, > + .unlocked_ioctl = hsmp_ioctl, > + .compat_ioctl = hsmp_ioctl, > +}; > + > +static int hsmp_create_non_acpi_sysfs_if(struct device *dev) > { > const struct attribute_group **hsmp_attr_grps; > struct attribute_group *attr_grp; > @@ -88,7 +99,7 @@ static inline bool is_f1a_m0h(void) > return false; > } > > -int init_platform_device(struct device *dev) > +static int init_platform_device(struct device *dev) > { > struct hsmp_socket *sock; > int ret, i; > @@ -134,3 +145,132 @@ int init_platform_device(struct device *dev) > > return 0; > } > + > +static int hsmp_pltdrv_probe(struct platform_device *pdev) > +{ > + int ret; > + > + hsmp_pdev.sock = devm_kcalloc(&pdev->dev, hsmp_pdev.num_sockets, > + sizeof(*hsmp_pdev.sock), > + GFP_KERNEL); > + if (!hsmp_pdev.sock) > + return -ENOMEM; > + > + ret = init_platform_device(&pdev->dev); > + if (ret) { > + dev_err(&pdev->dev, "Failed to init HSMP mailbox\n"); > + return ret; > + } > + > + ret = hsmp_create_non_acpi_sysfs_if(&pdev->dev); > + if (ret) > + dev_err(&pdev->dev, "Failed to create HSMP sysfs interface\n"); > + > + return hsmp_misc_register(&pdev->dev); > +} > + > +static void hsmp_pltdrv_remove(struct platform_device *pdev) > +{ > + hsmp_misc_deregister(); > +} > + > +static struct platform_driver amd_hsmp_driver = { > + .probe = hsmp_pltdrv_probe, > + .remove_new = hsmp_pltdrv_remove, > + .driver = { > + .name = DRIVER_NAME, > + }, > +}; > + > +static struct platform_device *amd_hsmp_platdev; > + > +static int hsmp_plat_dev_register(void) > +{ > + int ret; > + > + amd_hsmp_platdev = platform_device_alloc(DRIVER_NAME, PLATFORM_DEVID_NONE); > + if (!amd_hsmp_platdev) > + return -ENOMEM; > + > + ret = platform_device_add(amd_hsmp_platdev); > + if (ret) > + platform_device_put(amd_hsmp_platdev); > + > + return ret; > +} > + > +/* > + * This check is only needed for backward compatibility of previous platforms. > + * All new platforms are expected to support ACPI based probing. > + */ > +static bool legacy_hsmp_support(void) > +{ > + if (boot_cpu_data.x86_vendor != X86_VENDOR_AMD) > + return false; > + > + switch (boot_cpu_data.x86) { > + case 0x19: > + switch (boot_cpu_data.x86_model) { > + case 0x00 ... 0x1F: > + case 0x30 ... 0x3F: > + case 0x90 ... 0x9F: > + case 0xA0 ... 0xAF: > + return true; > + default: > + return false; > + } > + case 0x1A: > + switch (boot_cpu_data.x86_model) { > + case 0x00 ... 0x1F: > + return true; > + default: > + return false; > + } > + default: > + return false; > + } > + > + return false; > +} > + > +static int __init hsmp_plt_init(void) > +{ > + int ret = -ENODEV; > + > + if (!legacy_hsmp_support()) { > + pr_info("HSMP is not supported on Family:%x model:%x\n", > + boot_cpu_data.x86, boot_cpu_data.x86_model); > + return ret; > + } > + > + /* > + * amd_nb_num() returns number of SMN/DF interfaces present in the system > + * if we have N SMN/DF interfaces that ideally means N sockets > + */ > + hsmp_pdev.num_sockets = amd_nb_num(); > + if (hsmp_pdev.num_sockets == 0 || hsmp_pdev.num_sockets > MAX_AMD_SOCKETS) > + return ret; > + > + ret = platform_driver_register(&amd_hsmp_driver); > + if (ret) > + return ret; > + > + ret = hsmp_plat_dev_register(); > + if (ret) > + platform_driver_unregister(&amd_hsmp_driver); > + > + return ret; > +} > + > +static void __exit hsmp_plt_exit(void) > +{ > + platform_device_unregister(amd_hsmp_platdev); > + platform_driver_unregister(&amd_hsmp_driver); > +} > + > +device_initcall(hsmp_plt_init); > +module_exit(hsmp_plt_exit); > + > +MODULE_DESCRIPTION("AMD HSMP Platform Interface Driver"); > +MODULE_VERSION(DRIVER_VERSION); > +MODULE_LICENSE("GPL");
Hi Hans, On 9/5/2024 12:22 AM, Hans de Goede wrote: > Caution: This message originated from an External Source. Use proper caution when opening attachments, clicking links, or responding. > > > Hi, > > On 9/3/24 2:38 PM, Suma Hegde wrote: >> Separate the probes for HSMP ACPI and platform device drivers. >> >> Provide a Kconfig option to choose between >> ACPI or the platform device based driver. >> The common code which is the core part of the HSMP driver >> maintained at hsmp.c is guarded by AMD_HSMP config and is selected by >> these two driver configs. > <snip> > >> diff --git a/drivers/platform/x86/amd/hsmp/Kconfig b/drivers/platform/x86/amd/hsmp/Kconfig >> index b55d4ed9bceb..b10ff91e9f5a 100644 >> --- a/drivers/platform/x86/amd/hsmp/Kconfig >> +++ b/drivers/platform/x86/amd/hsmp/Kconfig >> @@ -4,14 +4,45 @@ >> # >> >> config AMD_HSMP >> - tristate "AMD HSMP Driver" >> - depends on AMD_NB && X86_64 && ACPI >> + tristate >> + >> +menu "AMD HSMP Driver" >> + depends on AMD_NB || COMPILE_TEST >> + >> +config AMD_HSMP_ACPI >> + tristate "AMD HSMP ACPI device driver" >> + depends on ACPI >> + select AMD_HSMP >> help >> + Host System Management Port (HSMP) interface is a mailbox interface >> + between the x86 core and the System Management Unit (SMU) firmware. >> The driver provides a way for user space tools to monitor and manage >> - system management functionality on EPYC server CPUs from AMD. >> + system management functionality on EPYC and MI300A server CPUs >> + from AMD. >> + >> + This option supports ACPI based probing. >> + You may enable this, if your platform BIOS provides an ACPI object >> + as described in amd_hsmp.rst document. >> + >> + If you choose to compile this driver as a module the module will be >> + called amd_hsmp. >> >> +config AMD_HSMP_PLAT >> + tristate "AMD HSMP platform device driver" >> + depends on AMD_HSMP_ACPI=n > I was about to merge this series, but this is going to cause > a regression for users running distro kernels which rely on > the old legacy probing. > > So before this we had 1 driver which would auto-load on systems > which have the new ACPI description of the HSMP and which could > be manually modprobed on systems which require the legacy enumeration. > > But now if linux distributions enable AMD_HSMP_ACPI then there > will be no way for users with systems which lack the ACPI description > of the HSMP to still get HSMP support. > > I guess what you want here is to avoid the legacy driver loading > on systems which do have the ACPI description. But the way to do > that would be to do a runtime check for the ACPI description, > not disallow building the legacy driver altogether. > > E.g. you could do the following in hsmp_plt_init() : > > if (acpi_dev_present(ACPI_HSMP_DEVICE_HID, NULL, -1)) { > pr_err("The legacy HSMP driver cannot load on this system, please use hsmp_acpi instead\n"); > return -ENODEV; > } > > I see that you also build both drivers into a single module, > yes then you cannot have both because you cannot have 2 init > functions. > > Please split things into 3 modules, one shared hsmp_common.ko + > a hsmp_acpi.ko + hsmp_legacy.ko Ok, I will keep Kconfig as is with 3 config symbols, but will remove "depends on AMD_HSMP_ACPI=n" and modify the Makefile to build 3 modules. hsmp_common.ko, + amd_hsmp.ko (instead of hsmp_legacy.ko, keeping name as amd_hsmp.ko to keep legacy name for the legacy module). + hsmp_acpi.ko. Also will add a check acpi_dev_present() as mentioned above in hsmp_plt_init(). Because of the change in config symbols, Linux distributions have to enable either AMD_HSMP_ACPI or AMD_HSMP_PLAT or both to get HSMP modules compiled. > Regards, > > Hans Thanks and Regards, Suma > > > > >> + select AMD_HSMP >> + help >> Host System Management Port (HSMP) interface is a mailbox interface >> between the x86 core and the System Management Unit (SMU) firmware. >> + The driver provides a way for user space tools to monitor and manage >> + system management functionality on EPYC and MI300A server CPUs >> + from AMD. >> + >> + This option supports platform device based probing. >> + You may enable this, if your platform BIOS does not provide >> + HSMP ACPI object. >> >> If you choose to compile this driver as a module the module will be >> called amd_hsmp. >> + >> +endmenu >> diff --git a/drivers/platform/x86/amd/hsmp/Makefile b/drivers/platform/x86/amd/hsmp/Makefile >> index 0cc92865c0a2..18d9a0d1e8c5 100644 >> --- a/drivers/platform/x86/amd/hsmp/Makefile >> +++ b/drivers/platform/x86/amd/hsmp/Makefile >> @@ -4,5 +4,7 @@ >> # AMD HSMP Driver >> # >> >> -obj-$(CONFIG_AMD_HSMP) += amd_hsmp.o >> -amd_hsmp-objs := hsmp.o plat.o acpi.o >> +obj-$(CONFIG_AMD_HSMP) += amd_hsmp.o >> +amd_hsmp-objs := hsmp.o >> +amd_hsmp-$(CONFIG_AMD_HSMP_PLAT) += plat.o >> +amd_hsmp-$(CONFIG_AMD_HSMP_ACPI) += acpi.o >> diff --git a/drivers/platform/x86/amd/hsmp/acpi.c b/drivers/platform/x86/amd/hsmp/acpi.c >> index 6f8e7962266a..766617e6adc7 100644 >> --- a/drivers/platform/x86/amd/hsmp/acpi.c >> +++ b/drivers/platform/x86/amd/hsmp/acpi.c >> @@ -9,11 +9,15 @@ >> >> #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt >> >> +#include <asm/amd_nb.h> >> + >> #include <linux/acpi.h> >> #include <linux/device.h> >> #include <linux/dev_printk.h> >> #include <linux/ioport.h> >> #include <linux/kstrtox.h> >> +#include <linux/module.h> >> +#include <linux/platform_device.h> >> #include <linux/sysfs.h> >> #include <linux/uuid.h> >> >> @@ -21,6 +25,10 @@ >> >> #include "hsmp.h" >> >> +#define DRIVER_NAME "amd_hsmp" >> +#define DRIVER_VERSION "2.3" >> +#define ACPI_HSMP_DEVICE_HID "AMDI0097" >> + >> /* These are the strings specified in ACPI table */ >> #define MSG_IDOFF_STR "MsgIdOffset" >> #define MSG_ARGOFF_STR "MsgArgOffset" >> @@ -200,7 +208,6 @@ static int hsmp_parse_acpi_table(struct device *dev, u16 sock_ind) >> sock->sock_ind = sock_ind; >> sock->dev = dev; >> sock->amd_hsmp_rdwr = amd_hsmp_acpi_rdwr; >> - hsmp_pdev.is_acpi_device = true; >> >> sema_init(&sock->hsmp_sem, 1); >> >> @@ -213,7 +220,7 @@ static int hsmp_parse_acpi_table(struct device *dev, u16 sock_ind) >> return hsmp_read_acpi_dsd(sock); >> } >> >> -int hsmp_create_acpi_sysfs_if(struct device *dev) >> +static int hsmp_create_acpi_sysfs_if(struct device *dev) >> { >> struct attribute_group *attr_grp; >> u16 sock_ind; >> @@ -236,7 +243,7 @@ int hsmp_create_acpi_sysfs_if(struct device *dev) >> return devm_device_add_group(dev, attr_grp); >> } >> >> -int init_acpi(struct device *dev) >> +static int init_acpi(struct device *dev) >> { >> u16 sock_ind; >> int ret; >> @@ -270,3 +277,72 @@ int init_acpi(struct device *dev) >> >> return ret; >> } >> + >> +static const struct acpi_device_id amd_hsmp_acpi_ids[] = { >> + {ACPI_HSMP_DEVICE_HID, 0}, >> + {} >> +}; >> +MODULE_DEVICE_TABLE(acpi, amd_hsmp_acpi_ids); >> + >> +static int hsmp_acpi_probe(struct platform_device *pdev) >> +{ >> + int ret; >> + >> + if (!hsmp_pdev.is_probed) { >> + hsmp_pdev.num_sockets = amd_nb_num(); >> + if (hsmp_pdev.num_sockets == 0 || hsmp_pdev.num_sockets > MAX_AMD_SOCKETS) >> + return -ENODEV; >> + >> + hsmp_pdev.sock = devm_kcalloc(&pdev->dev, hsmp_pdev.num_sockets, >> + sizeof(*hsmp_pdev.sock), >> + GFP_KERNEL); >> + if (!hsmp_pdev.sock) >> + return -ENOMEM; >> + } >> + >> + ret = init_acpi(&pdev->dev); >> + if (ret) { >> + dev_err(&pdev->dev, "Failed to initialize HSMP interface.\n"); >> + return ret; >> + } >> + >> + ret = hsmp_create_acpi_sysfs_if(&pdev->dev); >> + if (ret) >> + dev_err(&pdev->dev, "Failed to create HSMP sysfs interface\n"); >> + >> + if (!hsmp_pdev.is_probed) { >> + ret = hsmp_misc_register(&pdev->dev); >> + if (ret) >> + return ret; >> + hsmp_pdev.is_probed = true; >> + } >> + >> + return 0; >> +} >> + >> +static void hsmp_acpi_remove(struct platform_device *pdev) >> +{ >> + /* >> + * We register only one misc_device even on multi-socket system. >> + * So, deregister should happen only once. >> + */ >> + if (hsmp_pdev.is_probed) { >> + hsmp_misc_deregister(); >> + hsmp_pdev.is_probed = false; >> + } >> +} >> + >> +static struct platform_driver amd_hsmp_driver = { >> + .probe = hsmp_acpi_probe, >> + .remove_new = hsmp_acpi_remove, >> + .driver = { >> + .name = DRIVER_NAME, >> + .acpi_match_table = amd_hsmp_acpi_ids, >> + }, >> +}; >> + >> +module_platform_driver(amd_hsmp_driver); >> + >> +MODULE_DESCRIPTION("AMD HSMP Platform Interface Driver"); >> +MODULE_VERSION(DRIVER_VERSION); >> +MODULE_LICENSE("GPL"); >> diff --git a/drivers/platform/x86/amd/hsmp/hsmp.c b/drivers/platform/x86/amd/hsmp/hsmp.c >> index 78945750d590..5e0c9c36f435 100644 >> --- a/drivers/platform/x86/amd/hsmp/hsmp.c >> +++ b/drivers/platform/x86/amd/hsmp/hsmp.c >> @@ -15,17 +15,11 @@ >> #include <linux/acpi.h> >> #include <linux/delay.h> >> #include <linux/device.h> >> -#include <linux/module.h> >> -#include <linux/platform_device.h> >> #include <linux/semaphore.h> >> #include <linux/sysfs.h> >> >> #include "hsmp.h" >> >> -#define DRIVER_NAME "amd_hsmp" >> -#define DRIVER_VERSION "2.2" >> -#define ACPI_HSMP_DEVICE_HID "AMDI0097" >> - >> /* HSMP Status / Error codes */ >> #define HSMP_STATUS_NOT_READY 0x00 >> #define HSMP_STATUS_OK 0x01 >> @@ -228,7 +222,7 @@ int hsmp_test(u16 sock_ind, u32 value) >> return ret; >> } >> >> -static long hsmp_ioctl(struct file *fp, unsigned int cmd, unsigned long arg) >> +long hsmp_ioctl(struct file *fp, unsigned int cmd, unsigned long arg) >> { >> int __user *arguser = (int __user *)arg; >> struct hsmp_message msg = { 0 }; >> @@ -284,12 +278,6 @@ static long hsmp_ioctl(struct file *fp, unsigned int cmd, unsigned long arg) >> return 0; >> } >> >> -static const struct file_operations hsmp_fops = { >> - .owner = THIS_MODULE, >> - .unlocked_ioctl = hsmp_ioctl, >> - .compat_ioctl = hsmp_ioctl, >> -}; >> - >> ssize_t hsmp_metric_tbl_read(struct file *filp, struct kobject *kobj, >> struct bin_attribute *bin_attr, char *buf, >> loff_t off, size_t count) >> @@ -414,193 +402,25 @@ int hsmp_cache_proto_ver(u16 sock_ind) >> return ret; >> } >> >> -static const struct acpi_device_id amd_hsmp_acpi_ids[] = { >> - {ACPI_HSMP_DEVICE_HID, 0}, >> - {} >> -}; >> -MODULE_DEVICE_TABLE(acpi, amd_hsmp_acpi_ids); >> - >> -static bool check_acpi_support(struct device *dev) >> -{ >> - struct acpi_device *adev = ACPI_COMPANION(dev); >> - >> - if (adev && !acpi_match_device_ids(adev, amd_hsmp_acpi_ids)) >> - return true; >> - >> - return false; >> -} >> - >> -static int hsmp_pltdrv_probe(struct platform_device *pdev) >> -{ >> - int ret; >> - >> - /* >> - * On ACPI supported BIOS, there is an ACPI HSMP device added for >> - * each socket, so the per socket probing, but the memory allocated for >> - * sockets should be contiguous to access it as an array, >> - * Hence allocate memory for all the sockets at once instead of allocating >> - * on each probe. >> - */ >> - if (!hsmp_pdev.is_probed) { >> - hsmp_pdev.sock = devm_kcalloc(&pdev->dev, hsmp_pdev.num_sockets, >> - sizeof(*hsmp_pdev.sock), >> - GFP_KERNEL); >> - if (!hsmp_pdev.sock) >> - return -ENOMEM; >> - } >> - if (check_acpi_support(&pdev->dev)) { >> - ret = init_acpi(&pdev->dev); >> - if (ret) { >> - dev_err(&pdev->dev, "Failed to init HSMP mailbox\n"); >> - return ret; >> - } >> - ret = hsmp_create_acpi_sysfs_if(&pdev->dev); >> - if (ret) >> - dev_err(&pdev->dev, "Failed to create HSMP sysfs interface\n"); >> - } else { >> - ret = init_platform_device(&pdev->dev); >> - if (ret) { >> - dev_err(&pdev->dev, "Failed to init HSMP mailbox\n"); >> - return ret; >> - } >> - ret = hsmp_create_non_acpi_sysfs_if(&pdev->dev); >> - if (ret) >> - dev_err(&pdev->dev, "Failed to create HSMP sysfs interface\n"); >> - } >> - >> - if (!hsmp_pdev.is_probed) { >> - hsmp_pdev.mdev.name = HSMP_CDEV_NAME; >> - hsmp_pdev.mdev.minor = MISC_DYNAMIC_MINOR; >> - hsmp_pdev.mdev.fops = &hsmp_fops; >> - hsmp_pdev.mdev.parent = &pdev->dev; >> - hsmp_pdev.mdev.nodename = HSMP_DEVNODE_NAME; >> - hsmp_pdev.mdev.mode = 0644; >> - >> - ret = misc_register(&hsmp_pdev.mdev); >> - if (ret) >> - return ret; >> - >> - hsmp_pdev.is_probed = true; >> - } >> - >> - return 0; >> - >> -} >> - >> -static void hsmp_pltdrv_remove(struct platform_device *pdev) >> -{ >> - /* >> - * We register only one misc_device even on multi socket system. >> - * So, deregister should happen only once. >> - */ >> - if (hsmp_pdev.is_probed) { >> - misc_deregister(&hsmp_pdev.mdev); >> - hsmp_pdev.is_probed = false; >> - } >> -} >> - >> -static struct platform_driver amd_hsmp_driver = { >> - .probe = hsmp_pltdrv_probe, >> - .remove_new = hsmp_pltdrv_remove, >> - .driver = { >> - .name = DRIVER_NAME, >> - .acpi_match_table = amd_hsmp_acpi_ids, >> - }, >> +static const struct file_operations hsmp_fops = { >> + .owner = THIS_MODULE, >> + .unlocked_ioctl = hsmp_ioctl, >> + .compat_ioctl = hsmp_ioctl, >> }; >> >> -static struct platform_device *amd_hsmp_platdev; >> - >> -static int hsmp_plat_dev_register(void) >> +int hsmp_misc_register(struct device *dev) >> { >> - int ret; >> - >> - amd_hsmp_platdev = platform_device_alloc(DRIVER_NAME, PLATFORM_DEVID_NONE); >> - if (!amd_hsmp_platdev) >> - return -ENOMEM; >> - >> - ret = platform_device_add(amd_hsmp_platdev); >> - if (ret) >> - platform_device_put(amd_hsmp_platdev); >> - >> - return ret; >> + hsmp_pdev.mdev.name = HSMP_CDEV_NAME; >> + hsmp_pdev.mdev.minor = MISC_DYNAMIC_MINOR; >> + hsmp_pdev.mdev.fops = &hsmp_fops; >> + hsmp_pdev.mdev.parent = dev; >> + hsmp_pdev.mdev.nodename = HSMP_DEVNODE_NAME; >> + hsmp_pdev.mdev.mode = 0644; >> + >> + return misc_register(&hsmp_pdev.mdev); >> } >> >> -/* >> - * This check is only needed for backward compatibility of previous platforms. >> - * All new platforms are expected to support ACPI based probing. >> - */ >> -static bool legacy_hsmp_support(void) >> +void hsmp_misc_deregister(void) >> { >> - if (boot_cpu_data.x86_vendor != X86_VENDOR_AMD) >> - return false; >> - >> - switch (boot_cpu_data.x86) { >> - case 0x19: >> - switch (boot_cpu_data.x86_model) { >> - case 0x00 ... 0x1F: >> - case 0x30 ... 0x3F: >> - case 0x90 ... 0x9F: >> - case 0xA0 ... 0xAF: >> - return true; >> - default: >> - return false; >> - } >> - case 0x1A: >> - switch (boot_cpu_data.x86_model) { >> - case 0x00 ... 0x1F: >> - return true; >> - default: >> - return false; >> - } >> - default: >> - return false; >> - } >> - >> - return false; >> + misc_deregister(&hsmp_pdev.mdev); >> } >> - >> -static int __init hsmp_plt_init(void) >> -{ >> - int ret = -ENODEV; >> - >> - /* >> - * amd_nb_num() returns number of SMN/DF interfaces present in the system >> - * if we have N SMN/DF interfaces that ideally means N sockets >> - */ >> - hsmp_pdev.num_sockets = amd_nb_num(); >> - if (hsmp_pdev.num_sockets == 0 || hsmp_pdev.num_sockets > MAX_AMD_SOCKETS) >> - return ret; >> - >> - ret = platform_driver_register(&amd_hsmp_driver); >> - if (ret) >> - return ret; >> - >> - if (!hsmp_pdev.is_acpi_device) { >> - if (legacy_hsmp_support()) { >> - /* Not ACPI device, but supports HSMP, register a plat_dev */ >> - ret = hsmp_plat_dev_register(); >> - } else { >> - /* Not ACPI, Does not support HSMP */ >> - pr_info("HSMP is not supported on Family:%x model:%x\n", >> - boot_cpu_data.x86, boot_cpu_data.x86_model); >> - ret = -ENODEV; >> - } >> - if (ret) >> - platform_driver_unregister(&amd_hsmp_driver); >> - } >> - >> - return ret; >> -} >> - >> -static void __exit hsmp_plt_exit(void) >> -{ >> - platform_device_unregister(amd_hsmp_platdev); >> - platform_driver_unregister(&amd_hsmp_driver); >> -} >> - >> -device_initcall(hsmp_plt_init); >> -module_exit(hsmp_plt_exit); >> - >> -MODULE_DESCRIPTION("AMD HSMP Platform Interface Driver"); >> -MODULE_VERSION(DRIVER_VERSION); >> -MODULE_LICENSE("GPL v2"); >> diff --git a/drivers/platform/x86/amd/hsmp/hsmp.h b/drivers/platform/x86/amd/hsmp/hsmp.h >> index 9c5b9c263fc1..9ab50bc74676 100644 >> --- a/drivers/platform/x86/amd/hsmp/hsmp.h >> +++ b/drivers/platform/x86/amd/hsmp/hsmp.h >> @@ -52,7 +52,6 @@ struct hsmp_plat_device { >> struct hsmp_socket *sock; >> u32 proto_ver; >> u16 num_sockets; >> - bool is_acpi_device; >> bool is_probed; >> }; >> >> @@ -61,14 +60,13 @@ extern struct hsmp_plat_device hsmp_pdev; >> ssize_t hsmp_metric_tbl_read(struct file *filp, struct kobject *kobj, >> struct bin_attribute *bin_attr, char *buf, >> loff_t off, size_t count); >> -int hsmp_create_non_acpi_sysfs_if(struct device *dev); >> -int hsmp_create_acpi_sysfs_if(struct device *dev); >> int hsmp_cache_proto_ver(u16 sock_ind); >> umode_t hsmp_is_sock_attr_visible(struct kobject *kobj, >> struct bin_attribute *battr, int id); >> int hsmp_create_attr_list(struct attribute_group *attr_grp, >> struct device *dev, u16 sock_ind); >> int hsmp_test(u16 sock_ind, u32 value); >> -int init_platform_device(struct device *dev); >> -int init_acpi(struct device *dev); >> +long hsmp_ioctl(struct file *fp, unsigned int cmd, unsigned long arg); >> +void hsmp_misc_deregister(void); >> +int hsmp_misc_register(struct device *dev); >> #endif /* HSMP_H */ >> diff --git a/drivers/platform/x86/amd/hsmp/plat.c b/drivers/platform/x86/amd/hsmp/plat.c >> index e18cf82478a0..e49b53f8c5e3 100644 >> --- a/drivers/platform/x86/amd/hsmp/plat.c >> +++ b/drivers/platform/x86/amd/hsmp/plat.c >> @@ -12,11 +12,16 @@ >> #include <asm/amd_nb.h> >> >> #include <linux/device.h> >> +#include <linux/module.h> >> #include <linux/pci.h> >> +#include <linux/platform_device.h> >> #include <linux/sysfs.h> >> >> #include "hsmp.h" >> >> +#define DRIVER_NAME "amd_hsmp" >> +#define DRIVER_VERSION "2.3" >> + >> /* >> * To access specific HSMP mailbox register, s/w writes the SMN address of HSMP mailbox >> * register into the SMN_INDEX register, and reads/writes the SMN_DATA reg. >> @@ -50,7 +55,13 @@ static int amd_hsmp_pci_rdwr(struct hsmp_socket *sock, u32 offset, >> return ret; >> } >> >> -int hsmp_create_non_acpi_sysfs_if(struct device *dev) >> +static const struct file_operations hsmp_fops = { >> + .owner = THIS_MODULE, >> + .unlocked_ioctl = hsmp_ioctl, >> + .compat_ioctl = hsmp_ioctl, >> +}; >> + >> +static int hsmp_create_non_acpi_sysfs_if(struct device *dev) >> { >> const struct attribute_group **hsmp_attr_grps; >> struct attribute_group *attr_grp; >> @@ -88,7 +99,7 @@ static inline bool is_f1a_m0h(void) >> return false; >> } >> >> -int init_platform_device(struct device *dev) >> +static int init_platform_device(struct device *dev) >> { >> struct hsmp_socket *sock; >> int ret, i; >> @@ -134,3 +145,132 @@ int init_platform_device(struct device *dev) >> >> return 0; >> } >> + >> +static int hsmp_pltdrv_probe(struct platform_device *pdev) >> +{ >> + int ret; >> + >> + hsmp_pdev.sock = devm_kcalloc(&pdev->dev, hsmp_pdev.num_sockets, >> + sizeof(*hsmp_pdev.sock), >> + GFP_KERNEL); >> + if (!hsmp_pdev.sock) >> + return -ENOMEM; >> + >> + ret = init_platform_device(&pdev->dev); >> + if (ret) { >> + dev_err(&pdev->dev, "Failed to init HSMP mailbox\n"); >> + return ret; >> + } >> + >> + ret = hsmp_create_non_acpi_sysfs_if(&pdev->dev); >> + if (ret) >> + dev_err(&pdev->dev, "Failed to create HSMP sysfs interface\n"); >> + >> + return hsmp_misc_register(&pdev->dev); >> +} >> + >> +static void hsmp_pltdrv_remove(struct platform_device *pdev) >> +{ >> + hsmp_misc_deregister(); >> +} >> + >> +static struct platform_driver amd_hsmp_driver = { >> + .probe = hsmp_pltdrv_probe, >> + .remove_new = hsmp_pltdrv_remove, >> + .driver = { >> + .name = DRIVER_NAME, >> + }, >> +}; >> + >> +static struct platform_device *amd_hsmp_platdev; >> + >> +static int hsmp_plat_dev_register(void) >> +{ >> + int ret; >> + >> + amd_hsmp_platdev = platform_device_alloc(DRIVER_NAME, PLATFORM_DEVID_NONE); >> + if (!amd_hsmp_platdev) >> + return -ENOMEM; >> + >> + ret = platform_device_add(amd_hsmp_platdev); >> + if (ret) >> + platform_device_put(amd_hsmp_platdev); >> + >> + return ret; >> +} >> + >> +/* >> + * This check is only needed for backward compatibility of previous platforms. >> + * All new platforms are expected to support ACPI based probing. >> + */ >> +static bool legacy_hsmp_support(void) >> +{ >> + if (boot_cpu_data.x86_vendor != X86_VENDOR_AMD) >> + return false; >> + >> + switch (boot_cpu_data.x86) { >> + case 0x19: >> + switch (boot_cpu_data.x86_model) { >> + case 0x00 ... 0x1F: >> + case 0x30 ... 0x3F: >> + case 0x90 ... 0x9F: >> + case 0xA0 ... 0xAF: >> + return true; >> + default: >> + return false; >> + } >> + case 0x1A: >> + switch (boot_cpu_data.x86_model) { >> + case 0x00 ... 0x1F: >> + return true; >> + default: >> + return false; >> + } >> + default: >> + return false; >> + } >> + >> + return false; >> +} >> + >> +static int __init hsmp_plt_init(void) >> +{ >> + int ret = -ENODEV; >> + >> + if (!legacy_hsmp_support()) { >> + pr_info("HSMP is not supported on Family:%x model:%x\n", >> + boot_cpu_data.x86, boot_cpu_data.x86_model); >> + return ret; >> + } >> + >> + /* >> + * amd_nb_num() returns number of SMN/DF interfaces present in the system >> + * if we have N SMN/DF interfaces that ideally means N sockets >> + */ >> + hsmp_pdev.num_sockets = amd_nb_num(); >> + if (hsmp_pdev.num_sockets == 0 || hsmp_pdev.num_sockets > MAX_AMD_SOCKETS) >> + return ret; >> + >> + ret = platform_driver_register(&amd_hsmp_driver); >> + if (ret) >> + return ret; >> + >> + ret = hsmp_plat_dev_register(); >> + if (ret) >> + platform_driver_unregister(&amd_hsmp_driver); >> + >> + return ret; >> +} >> + >> +static void __exit hsmp_plt_exit(void) >> +{ >> + platform_device_unregister(amd_hsmp_platdev); >> + platform_driver_unregister(&amd_hsmp_driver); >> +} >> + >> +device_initcall(hsmp_plt_init); >> +module_exit(hsmp_plt_exit); >> + >> +MODULE_DESCRIPTION("AMD HSMP Platform Interface Driver"); >> +MODULE_VERSION(DRIVER_VERSION); >> +MODULE_LICENSE("GPL");
Hi Suma, On 9/11/24 8:12 AM, Suma Hegde wrote: > Hi Hans, > > On 9/5/2024 12:22 AM, Hans de Goede wrote: >> Caution: This message originated from an External Source. Use proper caution when opening attachments, clicking links, or responding. >> >> >> Hi, >> >> On 9/3/24 2:38 PM, Suma Hegde wrote: >>> Separate the probes for HSMP ACPI and platform device drivers. >>> >>> Provide a Kconfig option to choose between >>> ACPI or the platform device based driver. >>> The common code which is the core part of the HSMP driver >>> maintained at hsmp.c is guarded by AMD_HSMP config and is selected by >>> these two driver configs. >> <snip> >> >>> diff --git a/drivers/platform/x86/amd/hsmp/Kconfig b/drivers/platform/x86/amd/hsmp/Kconfig >>> index b55d4ed9bceb..b10ff91e9f5a 100644 >>> --- a/drivers/platform/x86/amd/hsmp/Kconfig >>> +++ b/drivers/platform/x86/amd/hsmp/Kconfig >>> @@ -4,14 +4,45 @@ >>> # >>> >>> config AMD_HSMP >>> - tristate "AMD HSMP Driver" >>> - depends on AMD_NB && X86_64 && ACPI >>> + tristate >>> + >>> +menu "AMD HSMP Driver" >>> + depends on AMD_NB || COMPILE_TEST >>> + >>> +config AMD_HSMP_ACPI >>> + tristate "AMD HSMP ACPI device driver" >>> + depends on ACPI >>> + select AMD_HSMP >>> help >>> + Host System Management Port (HSMP) interface is a mailbox interface >>> + between the x86 core and the System Management Unit (SMU) firmware. >>> The driver provides a way for user space tools to monitor and manage >>> - system management functionality on EPYC server CPUs from AMD. >>> + system management functionality on EPYC and MI300A server CPUs >>> + from AMD. >>> + >>> + This option supports ACPI based probing. >>> + You may enable this, if your platform BIOS provides an ACPI object >>> + as described in amd_hsmp.rst document. >>> + >>> + If you choose to compile this driver as a module the module will be >>> + called amd_hsmp. >>> >>> +config AMD_HSMP_PLAT >>> + tristate "AMD HSMP platform device driver" >>> + depends on AMD_HSMP_ACPI=n >> I was about to merge this series, but this is going to cause >> a regression for users running distro kernels which rely on >> the old legacy probing. >> >> So before this we had 1 driver which would auto-load on systems >> which have the new ACPI description of the HSMP and which could >> be manually modprobed on systems which require the legacy enumeration. >> >> But now if linux distributions enable AMD_HSMP_ACPI then there >> will be no way for users with systems which lack the ACPI description >> of the HSMP to still get HSMP support. >> >> I guess what you want here is to avoid the legacy driver loading >> on systems which do have the ACPI description. But the way to do >> that would be to do a runtime check for the ACPI description, >> not disallow building the legacy driver altogether. >> >> E.g. you could do the following in hsmp_plt_init() : >> >> if (acpi_dev_present(ACPI_HSMP_DEVICE_HID, NULL, -1)) { >> pr_err("The legacy HSMP driver cannot load on this system, please use hsmp_acpi instead\n"); >> return -ENODEV; >> } >> >> I see that you also build both drivers into a single module, >> yes then you cannot have both because you cannot have 2 init >> functions. >> >> Please split things into 3 modules, one shared hsmp_common.ko + >> a hsmp_acpi.ko + hsmp_legacy.ko > > Ok, I will keep Kconfig as is with 3 config symbols, but will remove "depends on AMD_HSMP_ACPI=n" and modify the Makefile to build 3 modules. > > hsmp_common.ko, + amd_hsmp.ko (instead of hsmp_legacy.ko, keeping name as amd_hsmp.ko to keep legacy name for the legacy module). Ok, keeping the legacy module name the same sounds good to me. > + hsmp_acpi.ko. > > Also will add a check acpi_dev_present() as mentioned above in hsmp_plt_init(). > > Because of the change in config symbols, Linux distributions have to enable either AMD_HSMP_ACPI or AMD_HSMP_PLAT or both to get HSMP modules compiled. Ack, distros need to change the config symbols all the time, so that is fine. Regards, Hans >>> + select AMD_HSMP >>> + help >>> Host System Management Port (HSMP) interface is a mailbox interface >>> between the x86 core and the System Management Unit (SMU) firmware. >>> + The driver provides a way for user space tools to monitor and manage >>> + system management functionality on EPYC and MI300A server CPUs >>> + from AMD. >>> + >>> + This option supports platform device based probing. >>> + You may enable this, if your platform BIOS does not provide >>> + HSMP ACPI object. >>> >>> If you choose to compile this driver as a module the module will be >>> called amd_hsmp. >>> + >>> +endmenu >>> diff --git a/drivers/platform/x86/amd/hsmp/Makefile b/drivers/platform/x86/amd/hsmp/Makefile >>> index 0cc92865c0a2..18d9a0d1e8c5 100644 >>> --- a/drivers/platform/x86/amd/hsmp/Makefile >>> +++ b/drivers/platform/x86/amd/hsmp/Makefile >>> @@ -4,5 +4,7 @@ >>> # AMD HSMP Driver >>> # >>> >>> -obj-$(CONFIG_AMD_HSMP) += amd_hsmp.o >>> -amd_hsmp-objs := hsmp.o plat.o acpi.o >>> +obj-$(CONFIG_AMD_HSMP) += amd_hsmp.o >>> +amd_hsmp-objs := hsmp.o >>> +amd_hsmp-$(CONFIG_AMD_HSMP_PLAT) += plat.o >>> +amd_hsmp-$(CONFIG_AMD_HSMP_ACPI) += acpi.o >>> diff --git a/drivers/platform/x86/amd/hsmp/acpi.c b/drivers/platform/x86/amd/hsmp/acpi.c >>> index 6f8e7962266a..766617e6adc7 100644 >>> --- a/drivers/platform/x86/amd/hsmp/acpi.c >>> +++ b/drivers/platform/x86/amd/hsmp/acpi.c >>> @@ -9,11 +9,15 @@ >>> >>> #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt >>> >>> +#include <asm/amd_nb.h> >>> + >>> #include <linux/acpi.h> >>> #include <linux/device.h> >>> #include <linux/dev_printk.h> >>> #include <linux/ioport.h> >>> #include <linux/kstrtox.h> >>> +#include <linux/module.h> >>> +#include <linux/platform_device.h> >>> #include <linux/sysfs.h> >>> #include <linux/uuid.h> >>> >>> @@ -21,6 +25,10 @@ >>> >>> #include "hsmp.h" >>> >>> +#define DRIVER_NAME "amd_hsmp" >>> +#define DRIVER_VERSION "2.3" >>> +#define ACPI_HSMP_DEVICE_HID "AMDI0097" >>> + >>> /* These are the strings specified in ACPI table */ >>> #define MSG_IDOFF_STR "MsgIdOffset" >>> #define MSG_ARGOFF_STR "MsgArgOffset" >>> @@ -200,7 +208,6 @@ static int hsmp_parse_acpi_table(struct device *dev, u16 sock_ind) >>> sock->sock_ind = sock_ind; >>> sock->dev = dev; >>> sock->amd_hsmp_rdwr = amd_hsmp_acpi_rdwr; >>> - hsmp_pdev.is_acpi_device = true; >>> >>> sema_init(&sock->hsmp_sem, 1); >>> >>> @@ -213,7 +220,7 @@ static int hsmp_parse_acpi_table(struct device *dev, u16 sock_ind) >>> return hsmp_read_acpi_dsd(sock); >>> } >>> >>> -int hsmp_create_acpi_sysfs_if(struct device *dev) >>> +static int hsmp_create_acpi_sysfs_if(struct device *dev) >>> { >>> struct attribute_group *attr_grp; >>> u16 sock_ind; >>> @@ -236,7 +243,7 @@ int hsmp_create_acpi_sysfs_if(struct device *dev) >>> return devm_device_add_group(dev, attr_grp); >>> } >>> >>> -int init_acpi(struct device *dev) >>> +static int init_acpi(struct device *dev) >>> { >>> u16 sock_ind; >>> int ret; >>> @@ -270,3 +277,72 @@ int init_acpi(struct device *dev) >>> >>> return ret; >>> } >>> + >>> +static const struct acpi_device_id amd_hsmp_acpi_ids[] = { >>> + {ACPI_HSMP_DEVICE_HID, 0}, >>> + {} >>> +}; >>> +MODULE_DEVICE_TABLE(acpi, amd_hsmp_acpi_ids); >>> + >>> +static int hsmp_acpi_probe(struct platform_device *pdev) >>> +{ >>> + int ret; >>> + >>> + if (!hsmp_pdev.is_probed) { >>> + hsmp_pdev.num_sockets = amd_nb_num(); >>> + if (hsmp_pdev.num_sockets == 0 || hsmp_pdev.num_sockets > MAX_AMD_SOCKETS) >>> + return -ENODEV; >>> + >>> + hsmp_pdev.sock = devm_kcalloc(&pdev->dev, hsmp_pdev.num_sockets, >>> + sizeof(*hsmp_pdev.sock), >>> + GFP_KERNEL); >>> + if (!hsmp_pdev.sock) >>> + return -ENOMEM; >>> + } >>> + >>> + ret = init_acpi(&pdev->dev); >>> + if (ret) { >>> + dev_err(&pdev->dev, "Failed to initialize HSMP interface.\n"); >>> + return ret; >>> + } >>> + >>> + ret = hsmp_create_acpi_sysfs_if(&pdev->dev); >>> + if (ret) >>> + dev_err(&pdev->dev, "Failed to create HSMP sysfs interface\n"); >>> + >>> + if (!hsmp_pdev.is_probed) { >>> + ret = hsmp_misc_register(&pdev->dev); >>> + if (ret) >>> + return ret; >>> + hsmp_pdev.is_probed = true; >>> + } >>> + >>> + return 0; >>> +} >>> + >>> +static void hsmp_acpi_remove(struct platform_device *pdev) >>> +{ >>> + /* >>> + * We register only one misc_device even on multi-socket system. >>> + * So, deregister should happen only once. >>> + */ >>> + if (hsmp_pdev.is_probed) { >>> + hsmp_misc_deregister(); >>> + hsmp_pdev.is_probed = false; >>> + } >>> +} >>> + >>> +static struct platform_driver amd_hsmp_driver = { >>> + .probe = hsmp_acpi_probe, >>> + .remove_new = hsmp_acpi_remove, >>> + .driver = { >>> + .name = DRIVER_NAME, >>> + .acpi_match_table = amd_hsmp_acpi_ids, >>> + }, >>> +}; >>> + >>> +module_platform_driver(amd_hsmp_driver); >>> + >>> +MODULE_DESCRIPTION("AMD HSMP Platform Interface Driver"); >>> +MODULE_VERSION(DRIVER_VERSION); >>> +MODULE_LICENSE("GPL"); >>> diff --git a/drivers/platform/x86/amd/hsmp/hsmp.c b/drivers/platform/x86/amd/hsmp/hsmp.c >>> index 78945750d590..5e0c9c36f435 100644 >>> --- a/drivers/platform/x86/amd/hsmp/hsmp.c >>> +++ b/drivers/platform/x86/amd/hsmp/hsmp.c >>> @@ -15,17 +15,11 @@ >>> #include <linux/acpi.h> >>> #include <linux/delay.h> >>> #include <linux/device.h> >>> -#include <linux/module.h> >>> -#include <linux/platform_device.h> >>> #include <linux/semaphore.h> >>> #include <linux/sysfs.h> >>> >>> #include "hsmp.h" >>> >>> -#define DRIVER_NAME "amd_hsmp" >>> -#define DRIVER_VERSION "2.2" >>> -#define ACPI_HSMP_DEVICE_HID "AMDI0097" >>> - >>> /* HSMP Status / Error codes */ >>> #define HSMP_STATUS_NOT_READY 0x00 >>> #define HSMP_STATUS_OK 0x01 >>> @@ -228,7 +222,7 @@ int hsmp_test(u16 sock_ind, u32 value) >>> return ret; >>> } >>> >>> -static long hsmp_ioctl(struct file *fp, unsigned int cmd, unsigned long arg) >>> +long hsmp_ioctl(struct file *fp, unsigned int cmd, unsigned long arg) >>> { >>> int __user *arguser = (int __user *)arg; >>> struct hsmp_message msg = { 0 }; >>> @@ -284,12 +278,6 @@ static long hsmp_ioctl(struct file *fp, unsigned int cmd, unsigned long arg) >>> return 0; >>> } >>> >>> -static const struct file_operations hsmp_fops = { >>> - .owner = THIS_MODULE, >>> - .unlocked_ioctl = hsmp_ioctl, >>> - .compat_ioctl = hsmp_ioctl, >>> -}; >>> - >>> ssize_t hsmp_metric_tbl_read(struct file *filp, struct kobject *kobj, >>> struct bin_attribute *bin_attr, char *buf, >>> loff_t off, size_t count) >>> @@ -414,193 +402,25 @@ int hsmp_cache_proto_ver(u16 sock_ind) >>> return ret; >>> } >>> >>> -static const struct acpi_device_id amd_hsmp_acpi_ids[] = { >>> - {ACPI_HSMP_DEVICE_HID, 0}, >>> - {} >>> -}; >>> -MODULE_DEVICE_TABLE(acpi, amd_hsmp_acpi_ids); >>> - >>> -static bool check_acpi_support(struct device *dev) >>> -{ >>> - struct acpi_device *adev = ACPI_COMPANION(dev); >>> - >>> - if (adev && !acpi_match_device_ids(adev, amd_hsmp_acpi_ids)) >>> - return true; >>> - >>> - return false; >>> -} >>> - >>> -static int hsmp_pltdrv_probe(struct platform_device *pdev) >>> -{ >>> - int ret; >>> - >>> - /* >>> - * On ACPI supported BIOS, there is an ACPI HSMP device added for >>> - * each socket, so the per socket probing, but the memory allocated for >>> - * sockets should be contiguous to access it as an array, >>> - * Hence allocate memory for all the sockets at once instead of allocating >>> - * on each probe. >>> - */ >>> - if (!hsmp_pdev.is_probed) { >>> - hsmp_pdev.sock = devm_kcalloc(&pdev->dev, hsmp_pdev.num_sockets, >>> - sizeof(*hsmp_pdev.sock), >>> - GFP_KERNEL); >>> - if (!hsmp_pdev.sock) >>> - return -ENOMEM; >>> - } >>> - if (check_acpi_support(&pdev->dev)) { >>> - ret = init_acpi(&pdev->dev); >>> - if (ret) { >>> - dev_err(&pdev->dev, "Failed to init HSMP mailbox\n"); >>> - return ret; >>> - } >>> - ret = hsmp_create_acpi_sysfs_if(&pdev->dev); >>> - if (ret) >>> - dev_err(&pdev->dev, "Failed to create HSMP sysfs interface\n"); >>> - } else { >>> - ret = init_platform_device(&pdev->dev); >>> - if (ret) { >>> - dev_err(&pdev->dev, "Failed to init HSMP mailbox\n"); >>> - return ret; >>> - } >>> - ret = hsmp_create_non_acpi_sysfs_if(&pdev->dev); >>> - if (ret) >>> - dev_err(&pdev->dev, "Failed to create HSMP sysfs interface\n"); >>> - } >>> - >>> - if (!hsmp_pdev.is_probed) { >>> - hsmp_pdev.mdev.name = HSMP_CDEV_NAME; >>> - hsmp_pdev.mdev.minor = MISC_DYNAMIC_MINOR; >>> - hsmp_pdev.mdev.fops = &hsmp_fops; >>> - hsmp_pdev.mdev.parent = &pdev->dev; >>> - hsmp_pdev.mdev.nodename = HSMP_DEVNODE_NAME; >>> - hsmp_pdev.mdev.mode = 0644; >>> - >>> - ret = misc_register(&hsmp_pdev.mdev); >>> - if (ret) >>> - return ret; >>> - >>> - hsmp_pdev.is_probed = true; >>> - } >>> - >>> - return 0; >>> - >>> -} >>> - >>> -static void hsmp_pltdrv_remove(struct platform_device *pdev) >>> -{ >>> - /* >>> - * We register only one misc_device even on multi socket system. >>> - * So, deregister should happen only once. >>> - */ >>> - if (hsmp_pdev.is_probed) { >>> - misc_deregister(&hsmp_pdev.mdev); >>> - hsmp_pdev.is_probed = false; >>> - } >>> -} >>> - >>> -static struct platform_driver amd_hsmp_driver = { >>> - .probe = hsmp_pltdrv_probe, >>> - .remove_new = hsmp_pltdrv_remove, >>> - .driver = { >>> - .name = DRIVER_NAME, >>> - .acpi_match_table = amd_hsmp_acpi_ids, >>> - }, >>> +static const struct file_operations hsmp_fops = { >>> + .owner = THIS_MODULE, >>> + .unlocked_ioctl = hsmp_ioctl, >>> + .compat_ioctl = hsmp_ioctl, >>> }; >>> >>> -static struct platform_device *amd_hsmp_platdev; >>> - >>> -static int hsmp_plat_dev_register(void) >>> +int hsmp_misc_register(struct device *dev) >>> { >>> - int ret; >>> - >>> - amd_hsmp_platdev = platform_device_alloc(DRIVER_NAME, PLATFORM_DEVID_NONE); >>> - if (!amd_hsmp_platdev) >>> - return -ENOMEM; >>> - >>> - ret = platform_device_add(amd_hsmp_platdev); >>> - if (ret) >>> - platform_device_put(amd_hsmp_platdev); >>> - >>> - return ret; >>> + hsmp_pdev.mdev.name = HSMP_CDEV_NAME; >>> + hsmp_pdev.mdev.minor = MISC_DYNAMIC_MINOR; >>> + hsmp_pdev.mdev.fops = &hsmp_fops; >>> + hsmp_pdev.mdev.parent = dev; >>> + hsmp_pdev.mdev.nodename = HSMP_DEVNODE_NAME; >>> + hsmp_pdev.mdev.mode = 0644; >>> + >>> + return misc_register(&hsmp_pdev.mdev); >>> } >>> >>> -/* >>> - * This check is only needed for backward compatibility of previous platforms. >>> - * All new platforms are expected to support ACPI based probing. >>> - */ >>> -static bool legacy_hsmp_support(void) >>> +void hsmp_misc_deregister(void) >>> { >>> - if (boot_cpu_data.x86_vendor != X86_VENDOR_AMD) >>> - return false; >>> - >>> - switch (boot_cpu_data.x86) { >>> - case 0x19: >>> - switch (boot_cpu_data.x86_model) { >>> - case 0x00 ... 0x1F: >>> - case 0x30 ... 0x3F: >>> - case 0x90 ... 0x9F: >>> - case 0xA0 ... 0xAF: >>> - return true; >>> - default: >>> - return false; >>> - } >>> - case 0x1A: >>> - switch (boot_cpu_data.x86_model) { >>> - case 0x00 ... 0x1F: >>> - return true; >>> - default: >>> - return false; >>> - } >>> - default: >>> - return false; >>> - } >>> - >>> - return false; >>> + misc_deregister(&hsmp_pdev.mdev); >>> } >>> - >>> -static int __init hsmp_plt_init(void) >>> -{ >>> - int ret = -ENODEV; >>> - >>> - /* >>> - * amd_nb_num() returns number of SMN/DF interfaces present in the system >>> - * if we have N SMN/DF interfaces that ideally means N sockets >>> - */ >>> - hsmp_pdev.num_sockets = amd_nb_num(); >>> - if (hsmp_pdev.num_sockets == 0 || hsmp_pdev.num_sockets > MAX_AMD_SOCKETS) >>> - return ret; >>> - >>> - ret = platform_driver_register(&amd_hsmp_driver); >>> - if (ret) >>> - return ret; >>> - >>> - if (!hsmp_pdev.is_acpi_device) { >>> - if (legacy_hsmp_support()) { >>> - /* Not ACPI device, but supports HSMP, register a plat_dev */ >>> - ret = hsmp_plat_dev_register(); >>> - } else { >>> - /* Not ACPI, Does not support HSMP */ >>> - pr_info("HSMP is not supported on Family:%x model:%x\n", >>> - boot_cpu_data.x86, boot_cpu_data.x86_model); >>> - ret = -ENODEV; >>> - } >>> - if (ret) >>> - platform_driver_unregister(&amd_hsmp_driver); >>> - } >>> - >>> - return ret; >>> -} >>> - >>> -static void __exit hsmp_plt_exit(void) >>> -{ >>> - platform_device_unregister(amd_hsmp_platdev); >>> - platform_driver_unregister(&amd_hsmp_driver); >>> -} >>> - >>> -device_initcall(hsmp_plt_init); >>> -module_exit(hsmp_plt_exit); >>> - >>> -MODULE_DESCRIPTION("AMD HSMP Platform Interface Driver"); >>> -MODULE_VERSION(DRIVER_VERSION); >>> -MODULE_LICENSE("GPL v2"); >>> diff --git a/drivers/platform/x86/amd/hsmp/hsmp.h b/drivers/platform/x86/amd/hsmp/hsmp.h >>> index 9c5b9c263fc1..9ab50bc74676 100644 >>> --- a/drivers/platform/x86/amd/hsmp/hsmp.h >>> +++ b/drivers/platform/x86/amd/hsmp/hsmp.h >>> @@ -52,7 +52,6 @@ struct hsmp_plat_device { >>> struct hsmp_socket *sock; >>> u32 proto_ver; >>> u16 num_sockets; >>> - bool is_acpi_device; >>> bool is_probed; >>> }; >>> >>> @@ -61,14 +60,13 @@ extern struct hsmp_plat_device hsmp_pdev; >>> ssize_t hsmp_metric_tbl_read(struct file *filp, struct kobject *kobj, >>> struct bin_attribute *bin_attr, char *buf, >>> loff_t off, size_t count); >>> -int hsmp_create_non_acpi_sysfs_if(struct device *dev); >>> -int hsmp_create_acpi_sysfs_if(struct device *dev); >>> int hsmp_cache_proto_ver(u16 sock_ind); >>> umode_t hsmp_is_sock_attr_visible(struct kobject *kobj, >>> struct bin_attribute *battr, int id); >>> int hsmp_create_attr_list(struct attribute_group *attr_grp, >>> struct device *dev, u16 sock_ind); >>> int hsmp_test(u16 sock_ind, u32 value); >>> -int init_platform_device(struct device *dev); >>> -int init_acpi(struct device *dev); >>> +long hsmp_ioctl(struct file *fp, unsigned int cmd, unsigned long arg); >>> +void hsmp_misc_deregister(void); >>> +int hsmp_misc_register(struct device *dev); >>> #endif /* HSMP_H */ >>> diff --git a/drivers/platform/x86/amd/hsmp/plat.c b/drivers/platform/x86/amd/hsmp/plat.c >>> index e18cf82478a0..e49b53f8c5e3 100644 >>> --- a/drivers/platform/x86/amd/hsmp/plat.c >>> +++ b/drivers/platform/x86/amd/hsmp/plat.c >>> @@ -12,11 +12,16 @@ >>> #include <asm/amd_nb.h> >>> >>> #include <linux/device.h> >>> +#include <linux/module.h> >>> #include <linux/pci.h> >>> +#include <linux/platform_device.h> >>> #include <linux/sysfs.h> >>> >>> #include "hsmp.h" >>> >>> +#define DRIVER_NAME "amd_hsmp" >>> +#define DRIVER_VERSION "2.3" >>> + >>> /* >>> * To access specific HSMP mailbox register, s/w writes the SMN address of HSMP mailbox >>> * register into the SMN_INDEX register, and reads/writes the SMN_DATA reg. >>> @@ -50,7 +55,13 @@ static int amd_hsmp_pci_rdwr(struct hsmp_socket *sock, u32 offset, >>> return ret; >>> } >>> >>> -int hsmp_create_non_acpi_sysfs_if(struct device *dev) >>> +static const struct file_operations hsmp_fops = { >>> + .owner = THIS_MODULE, >>> + .unlocked_ioctl = hsmp_ioctl, >>> + .compat_ioctl = hsmp_ioctl, >>> +}; >>> + >>> +static int hsmp_create_non_acpi_sysfs_if(struct device *dev) >>> { >>> const struct attribute_group **hsmp_attr_grps; >>> struct attribute_group *attr_grp; >>> @@ -88,7 +99,7 @@ static inline bool is_f1a_m0h(void) >>> return false; >>> } >>> >>> -int init_platform_device(struct device *dev) >>> +static int init_platform_device(struct device *dev) >>> { >>> struct hsmp_socket *sock; >>> int ret, i; >>> @@ -134,3 +145,132 @@ int init_platform_device(struct device *dev) >>> >>> return 0; >>> } >>> + >>> +static int hsmp_pltdrv_probe(struct platform_device *pdev) >>> +{ >>> + int ret; >>> + >>> + hsmp_pdev.sock = devm_kcalloc(&pdev->dev, hsmp_pdev.num_sockets, >>> + sizeof(*hsmp_pdev.sock), >>> + GFP_KERNEL); >>> + if (!hsmp_pdev.sock) >>> + return -ENOMEM; >>> + >>> + ret = init_platform_device(&pdev->dev); >>> + if (ret) { >>> + dev_err(&pdev->dev, "Failed to init HSMP mailbox\n"); >>> + return ret; >>> + } >>> + >>> + ret = hsmp_create_non_acpi_sysfs_if(&pdev->dev); >>> + if (ret) >>> + dev_err(&pdev->dev, "Failed to create HSMP sysfs interface\n"); >>> + >>> + return hsmp_misc_register(&pdev->dev); >>> +} >>> + >>> +static void hsmp_pltdrv_remove(struct platform_device *pdev) >>> +{ >>> + hsmp_misc_deregister(); >>> +} >>> + >>> +static struct platform_driver amd_hsmp_driver = { >>> + .probe = hsmp_pltdrv_probe, >>> + .remove_new = hsmp_pltdrv_remove, >>> + .driver = { >>> + .name = DRIVER_NAME, >>> + }, >>> +}; >>> + >>> +static struct platform_device *amd_hsmp_platdev; >>> + >>> +static int hsmp_plat_dev_register(void) >>> +{ >>> + int ret; >>> + >>> + amd_hsmp_platdev = platform_device_alloc(DRIVER_NAME, PLATFORM_DEVID_NONE); >>> + if (!amd_hsmp_platdev) >>> + return -ENOMEM; >>> + >>> + ret = platform_device_add(amd_hsmp_platdev); >>> + if (ret) >>> + platform_device_put(amd_hsmp_platdev); >>> + >>> + return ret; >>> +} >>> + >>> +/* >>> + * This check is only needed for backward compatibility of previous platforms. >>> + * All new platforms are expected to support ACPI based probing. >>> + */ >>> +static bool legacy_hsmp_support(void) >>> +{ >>> + if (boot_cpu_data.x86_vendor != X86_VENDOR_AMD) >>> + return false; >>> + >>> + switch (boot_cpu_data.x86) { >>> + case 0x19: >>> + switch (boot_cpu_data.x86_model) { >>> + case 0x00 ... 0x1F: >>> + case 0x30 ... 0x3F: >>> + case 0x90 ... 0x9F: >>> + case 0xA0 ... 0xAF: >>> + return true; >>> + default: >>> + return false; >>> + } >>> + case 0x1A: >>> + switch (boot_cpu_data.x86_model) { >>> + case 0x00 ... 0x1F: >>> + return true; >>> + default: >>> + return false; >>> + } >>> + default: >>> + return false; >>> + } >>> + >>> + return false; >>> +} >>> + >>> +static int __init hsmp_plt_init(void) >>> +{ >>> + int ret = -ENODEV; >>> + >>> + if (!legacy_hsmp_support()) { >>> + pr_info("HSMP is not supported on Family:%x model:%x\n", >>> + boot_cpu_data.x86, boot_cpu_data.x86_model); >>> + return ret; >>> + } >>> + >>> + /* >>> + * amd_nb_num() returns number of SMN/DF interfaces present in the system >>> + * if we have N SMN/DF interfaces that ideally means N sockets >>> + */ >>> + hsmp_pdev.num_sockets = amd_nb_num(); >>> + if (hsmp_pdev.num_sockets == 0 || hsmp_pdev.num_sockets > MAX_AMD_SOCKETS) >>> + return ret; >>> + >>> + ret = platform_driver_register(&amd_hsmp_driver); >>> + if (ret) >>> + return ret; >>> + >>> + ret = hsmp_plat_dev_register(); >>> + if (ret) >>> + platform_driver_unregister(&amd_hsmp_driver); >>> + >>> + return ret; >>> +} >>> + >>> +static void __exit hsmp_plt_exit(void) >>> +{ >>> + platform_device_unregister(amd_hsmp_platdev); >>> + platform_driver_unregister(&amd_hsmp_driver); >>> +} >>> + >>> +device_initcall(hsmp_plt_init); >>> +module_exit(hsmp_plt_exit); >>> + >>> +MODULE_DESCRIPTION("AMD HSMP Platform Interface Driver"); >>> +MODULE_VERSION(DRIVER_VERSION); >>> +MODULE_LICENSE("GPL"); >
diff --git a/Documentation/arch/x86/amd_hsmp.rst b/Documentation/arch/x86/amd_hsmp.rst index 1e499ecf5f4e..e4b4bbeed643 100644 --- a/Documentation/arch/x86/amd_hsmp.rst +++ b/Documentation/arch/x86/amd_hsmp.rst @@ -4,8 +4,9 @@ AMD HSMP interface ============================================ -Newer Fam19h EPYC server line of processors from AMD support system -management functionality via HSMP (Host System Management Port). +Newer Fam19h(model 0x00-0x1f, 0x30-0x3f, 0x90-0x9f, 0xa0-0xaf), +Fam1Ah(model 0x00-0x1f) EPYC server line of processors from AMD support +system management functionality via HSMP (Host System Management Port). The Host System Management Port (HSMP) is an interface to provide OS-level software with access to system management functions via a @@ -16,14 +17,24 @@ More details on the interface can be found in chapter Eg: https://www.amd.com/content/dam/amd/en/documents/epyc-technical-docs/programmer-references/55898_B1_pub_0_50.zip -HSMP interface is supported on EPYC server CPU models only. +HSMP interface is supported on EPYC line of server CPUs and MI300A (APU). HSMP device ============================================ -amd_hsmp driver under the drivers/platforms/x86/ creates miscdevice -/dev/hsmp to let user space programs run hsmp mailbox commands. +amd_hsmp driver under drivers/platforms/x86/amd/hsmp/ has separate driver files +for ACPI object based probing and platform device based probing. These two +drivers are mutually exclusive drivers with compile time Kconfig options to +choose between them. Both the drivers create the miscdevice /dev/hsmp to let +user space programs run hsmp mailbox commands. + +The ACPI object format supported by the driver is defined below. + +Kconfig option CONFIG_AMD_HSMP compiles plat.c + hsmp.c +for platform device based probing. +Kconfig option CONFIG_AMD_HSMP_ACPI compiles acpi.c + hsmp.c +for ACPI object based probing. $ ls -al /dev/hsmp crw-r--r-- 1 root root 10, 123 Jan 21 21:41 /dev/hsmp @@ -59,6 +70,51 @@ Note: lseek() is not supported as entire metrics table is read. Metrics table definitions will be documented as part of Public PPR. The same is defined in the amd_hsmp.h header. +ACPI device object format +========================= +The ACPI object format expected from the amd_hsmp driver +for socket with ID00 is given below. + +Device(HSMP) + { + Name(_HID, "AMDI0097") + Name(_UID, "ID00") + Name(HSE0, 0x00000001) + Name(RBF0, ResourceTemplate() + { + Memory32Fixed(ReadWrite, 0xxxxxxx, 0x00100000) + }) + Method(_CRS, 0, NotSerialized) + { + Return(RBF0) + } + Method(_STA, 0, NotSerialized) + { + If(LEqual(HSE0, One)) + { + Return(0x0F) + } + Else + { + Return(Zero) + } + } + Name(_DSD, Package(2) + { + Buffer(0x10) + { + 0x9D, 0x61, 0x4D, 0xB7, 0x07, 0x57, 0xBD, 0x48, + 0xA6, 0x9F, 0x4E, 0xA2, 0x87, 0x1F, 0xC2, 0xF6 + }, + Package(3) + { + Package(2) {"MsgIdOffset", 0x00010934}, + Package(2) {"MsgRspOffset", 0x00010980}, + Package(2) {"MsgArgOffset", 0x000109E0} + } + }) + } + An example ========== diff --git a/arch/x86/include/asm/amd_hsmp.h b/arch/x86/include/asm/amd_hsmp.h index 03c2ce3edaf5..ada14e55f9f4 100644 --- a/arch/x86/include/asm/amd_hsmp.h +++ b/arch/x86/include/asm/amd_hsmp.h @@ -5,7 +5,7 @@ #include <uapi/asm/amd_hsmp.h> -#if IS_ENABLED(CONFIG_AMD_HSMP) +#if IS_ENABLED(CONFIG_AMD_HSMP) || IS_ENABLED(CONFIG_AMD_HSMP_ACPI) int hsmp_send_message(struct hsmp_message *msg); #else static inline int hsmp_send_message(struct hsmp_message *msg) diff --git a/drivers/platform/x86/amd/Makefile b/drivers/platform/x86/amd/Makefile index 96ec24c8701b..f0b2fe81c685 100644 --- a/drivers/platform/x86/amd/Makefile +++ b/drivers/platform/x86/amd/Makefile @@ -5,6 +5,6 @@ # obj-$(CONFIG_AMD_PMC) += pmc/ -obj-y += hsmp/ +obj-$(CONFIG_AMD_HSMP) += hsmp/ obj-$(CONFIG_AMD_PMF) += pmf/ obj-$(CONFIG_AMD_WBRF) += wbrf.o diff --git a/drivers/platform/x86/amd/hsmp/Kconfig b/drivers/platform/x86/amd/hsmp/Kconfig index b55d4ed9bceb..b10ff91e9f5a 100644 --- a/drivers/platform/x86/amd/hsmp/Kconfig +++ b/drivers/platform/x86/amd/hsmp/Kconfig @@ -4,14 +4,45 @@ # config AMD_HSMP - tristate "AMD HSMP Driver" - depends on AMD_NB && X86_64 && ACPI + tristate + +menu "AMD HSMP Driver" + depends on AMD_NB || COMPILE_TEST + +config AMD_HSMP_ACPI + tristate "AMD HSMP ACPI device driver" + depends on ACPI + select AMD_HSMP help + Host System Management Port (HSMP) interface is a mailbox interface + between the x86 core and the System Management Unit (SMU) firmware. The driver provides a way for user space tools to monitor and manage - system management functionality on EPYC server CPUs from AMD. + system management functionality on EPYC and MI300A server CPUs + from AMD. + + This option supports ACPI based probing. + You may enable this, if your platform BIOS provides an ACPI object + as described in amd_hsmp.rst document. + + If you choose to compile this driver as a module the module will be + called amd_hsmp. +config AMD_HSMP_PLAT + tristate "AMD HSMP platform device driver" + depends on AMD_HSMP_ACPI=n + select AMD_HSMP + help Host System Management Port (HSMP) interface is a mailbox interface between the x86 core and the System Management Unit (SMU) firmware. + The driver provides a way for user space tools to monitor and manage + system management functionality on EPYC and MI300A server CPUs + from AMD. + + This option supports platform device based probing. + You may enable this, if your platform BIOS does not provide + HSMP ACPI object. If you choose to compile this driver as a module the module will be called amd_hsmp. + +endmenu diff --git a/drivers/platform/x86/amd/hsmp/Makefile b/drivers/platform/x86/amd/hsmp/Makefile index 0cc92865c0a2..18d9a0d1e8c5 100644 --- a/drivers/platform/x86/amd/hsmp/Makefile +++ b/drivers/platform/x86/amd/hsmp/Makefile @@ -4,5 +4,7 @@ # AMD HSMP Driver # -obj-$(CONFIG_AMD_HSMP) += amd_hsmp.o -amd_hsmp-objs := hsmp.o plat.o acpi.o +obj-$(CONFIG_AMD_HSMP) += amd_hsmp.o +amd_hsmp-objs := hsmp.o +amd_hsmp-$(CONFIG_AMD_HSMP_PLAT) += plat.o +amd_hsmp-$(CONFIG_AMD_HSMP_ACPI) += acpi.o diff --git a/drivers/platform/x86/amd/hsmp/acpi.c b/drivers/platform/x86/amd/hsmp/acpi.c index 6f8e7962266a..766617e6adc7 100644 --- a/drivers/platform/x86/amd/hsmp/acpi.c +++ b/drivers/platform/x86/amd/hsmp/acpi.c @@ -9,11 +9,15 @@ #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt +#include <asm/amd_nb.h> + #include <linux/acpi.h> #include <linux/device.h> #include <linux/dev_printk.h> #include <linux/ioport.h> #include <linux/kstrtox.h> +#include <linux/module.h> +#include <linux/platform_device.h> #include <linux/sysfs.h> #include <linux/uuid.h> @@ -21,6 +25,10 @@ #include "hsmp.h" +#define DRIVER_NAME "amd_hsmp" +#define DRIVER_VERSION "2.3" +#define ACPI_HSMP_DEVICE_HID "AMDI0097" + /* These are the strings specified in ACPI table */ #define MSG_IDOFF_STR "MsgIdOffset" #define MSG_ARGOFF_STR "MsgArgOffset" @@ -200,7 +208,6 @@ static int hsmp_parse_acpi_table(struct device *dev, u16 sock_ind) sock->sock_ind = sock_ind; sock->dev = dev; sock->amd_hsmp_rdwr = amd_hsmp_acpi_rdwr; - hsmp_pdev.is_acpi_device = true; sema_init(&sock->hsmp_sem, 1); @@ -213,7 +220,7 @@ static int hsmp_parse_acpi_table(struct device *dev, u16 sock_ind) return hsmp_read_acpi_dsd(sock); } -int hsmp_create_acpi_sysfs_if(struct device *dev) +static int hsmp_create_acpi_sysfs_if(struct device *dev) { struct attribute_group *attr_grp; u16 sock_ind; @@ -236,7 +243,7 @@ int hsmp_create_acpi_sysfs_if(struct device *dev) return devm_device_add_group(dev, attr_grp); } -int init_acpi(struct device *dev) +static int init_acpi(struct device *dev) { u16 sock_ind; int ret; @@ -270,3 +277,72 @@ int init_acpi(struct device *dev) return ret; } + +static const struct acpi_device_id amd_hsmp_acpi_ids[] = { + {ACPI_HSMP_DEVICE_HID, 0}, + {} +}; +MODULE_DEVICE_TABLE(acpi, amd_hsmp_acpi_ids); + +static int hsmp_acpi_probe(struct platform_device *pdev) +{ + int ret; + + if (!hsmp_pdev.is_probed) { + hsmp_pdev.num_sockets = amd_nb_num(); + if (hsmp_pdev.num_sockets == 0 || hsmp_pdev.num_sockets > MAX_AMD_SOCKETS) + return -ENODEV; + + hsmp_pdev.sock = devm_kcalloc(&pdev->dev, hsmp_pdev.num_sockets, + sizeof(*hsmp_pdev.sock), + GFP_KERNEL); + if (!hsmp_pdev.sock) + return -ENOMEM; + } + + ret = init_acpi(&pdev->dev); + if (ret) { + dev_err(&pdev->dev, "Failed to initialize HSMP interface.\n"); + return ret; + } + + ret = hsmp_create_acpi_sysfs_if(&pdev->dev); + if (ret) + dev_err(&pdev->dev, "Failed to create HSMP sysfs interface\n"); + + if (!hsmp_pdev.is_probed) { + ret = hsmp_misc_register(&pdev->dev); + if (ret) + return ret; + hsmp_pdev.is_probed = true; + } + + return 0; +} + +static void hsmp_acpi_remove(struct platform_device *pdev) +{ + /* + * We register only one misc_device even on multi-socket system. + * So, deregister should happen only once. + */ + if (hsmp_pdev.is_probed) { + hsmp_misc_deregister(); + hsmp_pdev.is_probed = false; + } +} + +static struct platform_driver amd_hsmp_driver = { + .probe = hsmp_acpi_probe, + .remove_new = hsmp_acpi_remove, + .driver = { + .name = DRIVER_NAME, + .acpi_match_table = amd_hsmp_acpi_ids, + }, +}; + +module_platform_driver(amd_hsmp_driver); + +MODULE_DESCRIPTION("AMD HSMP Platform Interface Driver"); +MODULE_VERSION(DRIVER_VERSION); +MODULE_LICENSE("GPL"); diff --git a/drivers/platform/x86/amd/hsmp/hsmp.c b/drivers/platform/x86/amd/hsmp/hsmp.c index 78945750d590..5e0c9c36f435 100644 --- a/drivers/platform/x86/amd/hsmp/hsmp.c +++ b/drivers/platform/x86/amd/hsmp/hsmp.c @@ -15,17 +15,11 @@ #include <linux/acpi.h> #include <linux/delay.h> #include <linux/device.h> -#include <linux/module.h> -#include <linux/platform_device.h> #include <linux/semaphore.h> #include <linux/sysfs.h> #include "hsmp.h" -#define DRIVER_NAME "amd_hsmp" -#define DRIVER_VERSION "2.2" -#define ACPI_HSMP_DEVICE_HID "AMDI0097" - /* HSMP Status / Error codes */ #define HSMP_STATUS_NOT_READY 0x00 #define HSMP_STATUS_OK 0x01 @@ -228,7 +222,7 @@ int hsmp_test(u16 sock_ind, u32 value) return ret; } -static long hsmp_ioctl(struct file *fp, unsigned int cmd, unsigned long arg) +long hsmp_ioctl(struct file *fp, unsigned int cmd, unsigned long arg) { int __user *arguser = (int __user *)arg; struct hsmp_message msg = { 0 }; @@ -284,12 +278,6 @@ static long hsmp_ioctl(struct file *fp, unsigned int cmd, unsigned long arg) return 0; } -static const struct file_operations hsmp_fops = { - .owner = THIS_MODULE, - .unlocked_ioctl = hsmp_ioctl, - .compat_ioctl = hsmp_ioctl, -}; - ssize_t hsmp_metric_tbl_read(struct file *filp, struct kobject *kobj, struct bin_attribute *bin_attr, char *buf, loff_t off, size_t count) @@ -414,193 +402,25 @@ int hsmp_cache_proto_ver(u16 sock_ind) return ret; } -static const struct acpi_device_id amd_hsmp_acpi_ids[] = { - {ACPI_HSMP_DEVICE_HID, 0}, - {} -}; -MODULE_DEVICE_TABLE(acpi, amd_hsmp_acpi_ids); - -static bool check_acpi_support(struct device *dev) -{ - struct acpi_device *adev = ACPI_COMPANION(dev); - - if (adev && !acpi_match_device_ids(adev, amd_hsmp_acpi_ids)) - return true; - - return false; -} - -static int hsmp_pltdrv_probe(struct platform_device *pdev) -{ - int ret; - - /* - * On ACPI supported BIOS, there is an ACPI HSMP device added for - * each socket, so the per socket probing, but the memory allocated for - * sockets should be contiguous to access it as an array, - * Hence allocate memory for all the sockets at once instead of allocating - * on each probe. - */ - if (!hsmp_pdev.is_probed) { - hsmp_pdev.sock = devm_kcalloc(&pdev->dev, hsmp_pdev.num_sockets, - sizeof(*hsmp_pdev.sock), - GFP_KERNEL); - if (!hsmp_pdev.sock) - return -ENOMEM; - } - if (check_acpi_support(&pdev->dev)) { - ret = init_acpi(&pdev->dev); - if (ret) { - dev_err(&pdev->dev, "Failed to init HSMP mailbox\n"); - return ret; - } - ret = hsmp_create_acpi_sysfs_if(&pdev->dev); - if (ret) - dev_err(&pdev->dev, "Failed to create HSMP sysfs interface\n"); - } else { - ret = init_platform_device(&pdev->dev); - if (ret) { - dev_err(&pdev->dev, "Failed to init HSMP mailbox\n"); - return ret; - } - ret = hsmp_create_non_acpi_sysfs_if(&pdev->dev); - if (ret) - dev_err(&pdev->dev, "Failed to create HSMP sysfs interface\n"); - } - - if (!hsmp_pdev.is_probed) { - hsmp_pdev.mdev.name = HSMP_CDEV_NAME; - hsmp_pdev.mdev.minor = MISC_DYNAMIC_MINOR; - hsmp_pdev.mdev.fops = &hsmp_fops; - hsmp_pdev.mdev.parent = &pdev->dev; - hsmp_pdev.mdev.nodename = HSMP_DEVNODE_NAME; - hsmp_pdev.mdev.mode = 0644; - - ret = misc_register(&hsmp_pdev.mdev); - if (ret) - return ret; - - hsmp_pdev.is_probed = true; - } - - return 0; - -} - -static void hsmp_pltdrv_remove(struct platform_device *pdev) -{ - /* - * We register only one misc_device even on multi socket system. - * So, deregister should happen only once. - */ - if (hsmp_pdev.is_probed) { - misc_deregister(&hsmp_pdev.mdev); - hsmp_pdev.is_probed = false; - } -} - -static struct platform_driver amd_hsmp_driver = { - .probe = hsmp_pltdrv_probe, - .remove_new = hsmp_pltdrv_remove, - .driver = { - .name = DRIVER_NAME, - .acpi_match_table = amd_hsmp_acpi_ids, - }, +static const struct file_operations hsmp_fops = { + .owner = THIS_MODULE, + .unlocked_ioctl = hsmp_ioctl, + .compat_ioctl = hsmp_ioctl, }; -static struct platform_device *amd_hsmp_platdev; - -static int hsmp_plat_dev_register(void) +int hsmp_misc_register(struct device *dev) { - int ret; - - amd_hsmp_platdev = platform_device_alloc(DRIVER_NAME, PLATFORM_DEVID_NONE); - if (!amd_hsmp_platdev) - return -ENOMEM; - - ret = platform_device_add(amd_hsmp_platdev); - if (ret) - platform_device_put(amd_hsmp_platdev); - - return ret; + hsmp_pdev.mdev.name = HSMP_CDEV_NAME; + hsmp_pdev.mdev.minor = MISC_DYNAMIC_MINOR; + hsmp_pdev.mdev.fops = &hsmp_fops; + hsmp_pdev.mdev.parent = dev; + hsmp_pdev.mdev.nodename = HSMP_DEVNODE_NAME; + hsmp_pdev.mdev.mode = 0644; + + return misc_register(&hsmp_pdev.mdev); } -/* - * This check is only needed for backward compatibility of previous platforms. - * All new platforms are expected to support ACPI based probing. - */ -static bool legacy_hsmp_support(void) +void hsmp_misc_deregister(void) { - if (boot_cpu_data.x86_vendor != X86_VENDOR_AMD) - return false; - - switch (boot_cpu_data.x86) { - case 0x19: - switch (boot_cpu_data.x86_model) { - case 0x00 ... 0x1F: - case 0x30 ... 0x3F: - case 0x90 ... 0x9F: - case 0xA0 ... 0xAF: - return true; - default: - return false; - } - case 0x1A: - switch (boot_cpu_data.x86_model) { - case 0x00 ... 0x1F: - return true; - default: - return false; - } - default: - return false; - } - - return false; + misc_deregister(&hsmp_pdev.mdev); } - -static int __init hsmp_plt_init(void) -{ - int ret = -ENODEV; - - /* - * amd_nb_num() returns number of SMN/DF interfaces present in the system - * if we have N SMN/DF interfaces that ideally means N sockets - */ - hsmp_pdev.num_sockets = amd_nb_num(); - if (hsmp_pdev.num_sockets == 0 || hsmp_pdev.num_sockets > MAX_AMD_SOCKETS) - return ret; - - ret = platform_driver_register(&amd_hsmp_driver); - if (ret) - return ret; - - if (!hsmp_pdev.is_acpi_device) { - if (legacy_hsmp_support()) { - /* Not ACPI device, but supports HSMP, register a plat_dev */ - ret = hsmp_plat_dev_register(); - } else { - /* Not ACPI, Does not support HSMP */ - pr_info("HSMP is not supported on Family:%x model:%x\n", - boot_cpu_data.x86, boot_cpu_data.x86_model); - ret = -ENODEV; - } - if (ret) - platform_driver_unregister(&amd_hsmp_driver); - } - - return ret; -} - -static void __exit hsmp_plt_exit(void) -{ - platform_device_unregister(amd_hsmp_platdev); - platform_driver_unregister(&amd_hsmp_driver); -} - -device_initcall(hsmp_plt_init); -module_exit(hsmp_plt_exit); - -MODULE_DESCRIPTION("AMD HSMP Platform Interface Driver"); -MODULE_VERSION(DRIVER_VERSION); -MODULE_LICENSE("GPL v2"); diff --git a/drivers/platform/x86/amd/hsmp/hsmp.h b/drivers/platform/x86/amd/hsmp/hsmp.h index 9c5b9c263fc1..9ab50bc74676 100644 --- a/drivers/platform/x86/amd/hsmp/hsmp.h +++ b/drivers/platform/x86/amd/hsmp/hsmp.h @@ -52,7 +52,6 @@ struct hsmp_plat_device { struct hsmp_socket *sock; u32 proto_ver; u16 num_sockets; - bool is_acpi_device; bool is_probed; }; @@ -61,14 +60,13 @@ extern struct hsmp_plat_device hsmp_pdev; ssize_t hsmp_metric_tbl_read(struct file *filp, struct kobject *kobj, struct bin_attribute *bin_attr, char *buf, loff_t off, size_t count); -int hsmp_create_non_acpi_sysfs_if(struct device *dev); -int hsmp_create_acpi_sysfs_if(struct device *dev); int hsmp_cache_proto_ver(u16 sock_ind); umode_t hsmp_is_sock_attr_visible(struct kobject *kobj, struct bin_attribute *battr, int id); int hsmp_create_attr_list(struct attribute_group *attr_grp, struct device *dev, u16 sock_ind); int hsmp_test(u16 sock_ind, u32 value); -int init_platform_device(struct device *dev); -int init_acpi(struct device *dev); +long hsmp_ioctl(struct file *fp, unsigned int cmd, unsigned long arg); +void hsmp_misc_deregister(void); +int hsmp_misc_register(struct device *dev); #endif /* HSMP_H */ diff --git a/drivers/platform/x86/amd/hsmp/plat.c b/drivers/platform/x86/amd/hsmp/plat.c index e18cf82478a0..e49b53f8c5e3 100644 --- a/drivers/platform/x86/amd/hsmp/plat.c +++ b/drivers/platform/x86/amd/hsmp/plat.c @@ -12,11 +12,16 @@ #include <asm/amd_nb.h> #include <linux/device.h> +#include <linux/module.h> #include <linux/pci.h> +#include <linux/platform_device.h> #include <linux/sysfs.h> #include "hsmp.h" +#define DRIVER_NAME "amd_hsmp" +#define DRIVER_VERSION "2.3" + /* * To access specific HSMP mailbox register, s/w writes the SMN address of HSMP mailbox * register into the SMN_INDEX register, and reads/writes the SMN_DATA reg. @@ -50,7 +55,13 @@ static int amd_hsmp_pci_rdwr(struct hsmp_socket *sock, u32 offset, return ret; } -int hsmp_create_non_acpi_sysfs_if(struct device *dev) +static const struct file_operations hsmp_fops = { + .owner = THIS_MODULE, + .unlocked_ioctl = hsmp_ioctl, + .compat_ioctl = hsmp_ioctl, +}; + +static int hsmp_create_non_acpi_sysfs_if(struct device *dev) { const struct attribute_group **hsmp_attr_grps; struct attribute_group *attr_grp; @@ -88,7 +99,7 @@ static inline bool is_f1a_m0h(void) return false; } -int init_platform_device(struct device *dev) +static int init_platform_device(struct device *dev) { struct hsmp_socket *sock; int ret, i; @@ -134,3 +145,132 @@ int init_platform_device(struct device *dev) return 0; } + +static int hsmp_pltdrv_probe(struct platform_device *pdev) +{ + int ret; + + hsmp_pdev.sock = devm_kcalloc(&pdev->dev, hsmp_pdev.num_sockets, + sizeof(*hsmp_pdev.sock), + GFP_KERNEL); + if (!hsmp_pdev.sock) + return -ENOMEM; + + ret = init_platform_device(&pdev->dev); + if (ret) { + dev_err(&pdev->dev, "Failed to init HSMP mailbox\n"); + return ret; + } + + ret = hsmp_create_non_acpi_sysfs_if(&pdev->dev); + if (ret) + dev_err(&pdev->dev, "Failed to create HSMP sysfs interface\n"); + + return hsmp_misc_register(&pdev->dev); +} + +static void hsmp_pltdrv_remove(struct platform_device *pdev) +{ + hsmp_misc_deregister(); +} + +static struct platform_driver amd_hsmp_driver = { + .probe = hsmp_pltdrv_probe, + .remove_new = hsmp_pltdrv_remove, + .driver = { + .name = DRIVER_NAME, + }, +}; + +static struct platform_device *amd_hsmp_platdev; + +static int hsmp_plat_dev_register(void) +{ + int ret; + + amd_hsmp_platdev = platform_device_alloc(DRIVER_NAME, PLATFORM_DEVID_NONE); + if (!amd_hsmp_platdev) + return -ENOMEM; + + ret = platform_device_add(amd_hsmp_platdev); + if (ret) + platform_device_put(amd_hsmp_platdev); + + return ret; +} + +/* + * This check is only needed for backward compatibility of previous platforms. + * All new platforms are expected to support ACPI based probing. + */ +static bool legacy_hsmp_support(void) +{ + if (boot_cpu_data.x86_vendor != X86_VENDOR_AMD) + return false; + + switch (boot_cpu_data.x86) { + case 0x19: + switch (boot_cpu_data.x86_model) { + case 0x00 ... 0x1F: + case 0x30 ... 0x3F: + case 0x90 ... 0x9F: + case 0xA0 ... 0xAF: + return true; + default: + return false; + } + case 0x1A: + switch (boot_cpu_data.x86_model) { + case 0x00 ... 0x1F: + return true; + default: + return false; + } + default: + return false; + } + + return false; +} + +static int __init hsmp_plt_init(void) +{ + int ret = -ENODEV; + + if (!legacy_hsmp_support()) { + pr_info("HSMP is not supported on Family:%x model:%x\n", + boot_cpu_data.x86, boot_cpu_data.x86_model); + return ret; + } + + /* + * amd_nb_num() returns number of SMN/DF interfaces present in the system + * if we have N SMN/DF interfaces that ideally means N sockets + */ + hsmp_pdev.num_sockets = amd_nb_num(); + if (hsmp_pdev.num_sockets == 0 || hsmp_pdev.num_sockets > MAX_AMD_SOCKETS) + return ret; + + ret = platform_driver_register(&amd_hsmp_driver); + if (ret) + return ret; + + ret = hsmp_plat_dev_register(); + if (ret) + platform_driver_unregister(&amd_hsmp_driver); + + return ret; +} + +static void __exit hsmp_plt_exit(void) +{ + platform_device_unregister(amd_hsmp_platdev); + platform_driver_unregister(&amd_hsmp_driver); +} + +device_initcall(hsmp_plt_init); +module_exit(hsmp_plt_exit); + +MODULE_DESCRIPTION("AMD HSMP Platform Interface Driver"); +MODULE_VERSION(DRIVER_VERSION); +MODULE_LICENSE("GPL");