Hi,
On 2/11/23 07:32, Srinivas Pandruvada wrote:
> Enumerate TPMI SST driver and create basic framework to add more
> features.
>
> The basic user space interface is still same as the legacy using
> /dev/isst_interface. Users of "intel-speed-select" utility should
> be able to use same commands as prior gens without being aware
> of new underlying hardware interface.
>
> TPMI SST driver enumerates on device "intel_vsec.tpmi-sst". Since there
> can be multiple instances and there is one common SST core, split
> implementation into two parts: A common core part and an enumeration
> part. The enumeration driver is loaded for each device instance and
> register with the TPMI SST core driver.
>
> On very first enumeration the TPMI SST core driver register with SST
> core driver to get IOCTL callbacks. The api_version is incremented
> for IOCTL ISST_IF_GET_PLATFORM_INFO, so that user space can issue
> new IOCTLs.
>
> Each TPMI package contains multiple power domains. Each power domain
> has its own set of SST controls. For each domain map the MMIO memory
> and update per domain struct tpmi_per_power_domain_info. This information
> will be used to implement other SST interfaces.
>
> Implement first IOCTL commands to get number of TPMI SST instances
> and instance mask as some of the power domains may not have any
> SST controls.
>
> Signed-off-by: Srinivas Pandruvada <srinivas.pandruvada@xxxxxxxxxxxxxxx>
Thanks, patch looks good to me:
Reviewed-by: Hans de Goede <hdegoede@xxxxxxxxxx>
But this is a somewhat larger patch and IMHO it would be good
to get an extra pair of eyes on this, can you get someone else
from Intel to also review this patch ?
This (getting someone else to review the patches) especially
applies to patches 7-11, where I don't really feel myself
qualifeed to review them. I have given them a quick lookover
and nothing stood out, but they really need a Reviewed-by or
at minimum an Ack from someone else @Intel.
Regards,
Hans
> ---
> .../x86/intel/speed_select_if/Kconfig | 4 +
> .../x86/intel/speed_select_if/Makefile | 2 +
> .../x86/intel/speed_select_if/isst_tpmi.c | 53 ++++
> .../intel/speed_select_if/isst_tpmi_core.c | 274 ++++++++++++++++++
> .../intel/speed_select_if/isst_tpmi_core.h | 16 +
> include/uapi/linux/isst_if.h | 18 ++
> 6 files changed, 367 insertions(+)
> create mode 100644 drivers/platform/x86/intel/speed_select_if/isst_tpmi.c
> create mode 100644 drivers/platform/x86/intel/speed_select_if/isst_tpmi_core.c
> create mode 100644 drivers/platform/x86/intel/speed_select_if/isst_tpmi_core.h
>
> diff --git a/drivers/platform/x86/intel/speed_select_if/Kconfig b/drivers/platform/x86/intel/speed_select_if/Kconfig
> index ce3e3dc076d2..4eb3ad299db0 100644
> --- a/drivers/platform/x86/intel/speed_select_if/Kconfig
> +++ b/drivers/platform/x86/intel/speed_select_if/Kconfig
> @@ -2,8 +2,12 @@ menu "Intel Speed Select Technology interface support"
> depends on PCI
> depends on X86_64 || COMPILE_TEST
>
> +config INTEL_SPEED_SELECT_TPMI
> + tristate
> +
> config INTEL_SPEED_SELECT_INTERFACE
> tristate "Intel(R) Speed Select Technology interface drivers"
> + select INTEL_SPEED_SELECT_TPMI if INTEL_TPMI
> help
> This config enables the Intel(R) Speed Select Technology interface
> drivers. The Intel(R) speed select technology features are non
> diff --git a/drivers/platform/x86/intel/speed_select_if/Makefile b/drivers/platform/x86/intel/speed_select_if/Makefile
> index 856076206f35..1d878a36d0ab 100644
> --- a/drivers/platform/x86/intel/speed_select_if/Makefile
> +++ b/drivers/platform/x86/intel/speed_select_if/Makefile
> @@ -8,3 +8,5 @@ obj-$(CONFIG_INTEL_SPEED_SELECT_INTERFACE) += isst_if_common.o
> obj-$(CONFIG_INTEL_SPEED_SELECT_INTERFACE) += isst_if_mmio.o
> obj-$(CONFIG_INTEL_SPEED_SELECT_INTERFACE) += isst_if_mbox_pci.o
> obj-$(CONFIG_INTEL_SPEED_SELECT_INTERFACE) += isst_if_mbox_msr.o
> +obj-$(CONFIG_INTEL_SPEED_SELECT_TPMI) += isst_tpmi_core.o
> +obj-$(CONFIG_INTEL_SPEED_SELECT_TPMI) += isst_tpmi.o
> diff --git a/drivers/platform/x86/intel/speed_select_if/isst_tpmi.c b/drivers/platform/x86/intel/speed_select_if/isst_tpmi.c
> new file mode 100644
> index 000000000000..7b4bdeefb8bc
> --- /dev/null
> +++ b/drivers/platform/x86/intel/speed_select_if/isst_tpmi.c
> @@ -0,0 +1,53 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * isst_tpmi.c: SST TPMI interface
> + *
> + * Copyright (c) 2023, Intel Corporation.
> + * All Rights Reserved.
> + *
> + */
> +
> +#include <linux/auxiliary_bus.h>
> +#include <linux/module.h>
> +#include <linux/intel_tpmi.h>
> +
> +#include "isst_tpmi_core.h"
> +
> +static int intel_sst_probe(struct auxiliary_device *auxdev, const struct auxiliary_device_id *id)
> +{
> + int ret;
> +
> + ret = tpmi_sst_init();
> + if (ret)
> + return ret;
> +
> + ret = tpmi_sst_dev_add(auxdev);
> + if (ret)
> + tpmi_sst_exit();
> +
> + return ret;
> +}
> +
> +static void intel_sst_remove(struct auxiliary_device *auxdev)
> +{
> + tpmi_sst_dev_remove(auxdev);
> + tpmi_sst_exit();
> +}
> +
> +static const struct auxiliary_device_id intel_sst_id_table[] = {
> + { .name = "intel_vsec.tpmi-sst" },
> + {}
> +};
> +MODULE_DEVICE_TABLE(auxiliary, intel_sst_id_table);
> +
> +static struct auxiliary_driver intel_sst_aux_driver = {
> + .id_table = intel_sst_id_table,
> + .remove = intel_sst_remove,
> + .probe = intel_sst_probe,
> +};
> +
> +module_auxiliary_driver(intel_sst_aux_driver);
> +
> +MODULE_IMPORT_NS(INTEL_TPMI_SST);
> +MODULE_DESCRIPTION("Intel TPMI SST Driver");
> +MODULE_LICENSE("GPL");
> diff --git a/drivers/platform/x86/intel/speed_select_if/isst_tpmi_core.c b/drivers/platform/x86/intel/speed_select_if/isst_tpmi_core.c
> new file mode 100644
> index 000000000000..6b37016c0417
> --- /dev/null
> +++ b/drivers/platform/x86/intel/speed_select_if/isst_tpmi_core.c
> @@ -0,0 +1,274 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * isst_tpmi.c: SST TPMI interface core
> + *
> + * Copyright (c) 2023, Intel Corporation.
> + * All Rights Reserved.
> + *
> + * This information will be useful to understand flows:
> + * In the current generation of platforms, TPMI is supported via OOB
> + * PCI device. This PCI device has one instance per CPU package.
> + * There is a unique TPMI ID for SST. Each TPMI ID also has multiple
> + * entries, representing per power domain information.
> + *
> + * There is one dev file for complete SST information and control same as the
> + * prior generation of hardware. User spaces don't need to know how the
> + * information is presented by the hardware. The TPMI core module implements
> + * the hardware mapping.
> + */
> +
> +#include <linux/auxiliary_bus.h>
> +#include <linux/intel_tpmi.h>
> +#include <linux/fs.h>
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <uapi/linux/isst_if.h>
> +
> +#include "isst_tpmi_core.h"
> +#include "isst_if_common.h"
> +
> +/**
> + * struct tpmi_per_power_domain_info - Store per power_domain SST info
> + * @package_id: Package id for this power_domain
> + * @power_domain_id: Power domain id, Each entry from the SST-TPMI instance is a power_domain.
> + * @sst_base: Mapped SST base IO memory
> + * @auxdev: Auxiliary device instance enumerated this instance
> + *
> + * This structure is used store complete SST information for a power_domain. This information
> + * is used to read/write request for any SST IOCTL. Each physical CPU package can have multiple
> + * power_domains. Each power domain describes its own SST information and has its own controls.
> + */
> +struct tpmi_per_power_domain_info {
> + int package_id;
> + int power_domain_id;
> + void __iomem *sst_base;
> + struct auxiliary_device *auxdev;
> +};
> +
> +/**
> + * struct tpmi_sst_struct - Store sst info for a package
> + * @package_id: Package id for this aux device instance
> + * @number_of_power_domains: Number of power_domains pointed by power_domain_info pointer
> + * @power_domain_info: Pointer to power domains information
> + *
> + * This structure is used store full SST information for a package.
> + * Each package has a unique OOB PCI device, which enumerates TPMI.
> + * Each Package will have multiple power_domains.
> + */
> +struct tpmi_sst_struct {
> + int package_id;
> + int number_of_power_domains;
> + struct tpmi_per_power_domain_info *power_domain_info;
> +};
> +
> +/**
> + * struct tpmi_sst_common_struct - Store all SST instances
> + * @max_index: Maximum instances currently present
> + * @sst_inst: Pointer to per package instance
> + *
> + * Stores every SST Package instance.
> + */
> +struct tpmi_sst_common_struct {
> + int max_index;
> + struct tpmi_sst_struct **sst_inst;
> +};
> +
> +/*
> + * Each IOCTL request is processed under this lock. Also used to protect
> + * registration functions and common data structures.
> + */
> +static DEFINE_MUTEX(isst_tpmi_dev_lock);
> +
> +/* Usage count to track, number of TPMI SST instances registered to this core. */
> +static int isst_core_usage_count;
> +
> +/* Stores complete SST information for every package and power_domain */
> +static struct tpmi_sst_common_struct isst_common;
> +
> +static int isst_if_get_tpmi_instance_count(void __user *argp)
> +{
> + struct isst_tpmi_instance_count tpmi_inst;
> + struct tpmi_sst_struct *sst_inst;
> + int i;
> +
> + if (copy_from_user(&tpmi_inst, argp, sizeof(tpmi_inst)))
> + return -EFAULT;
> +
> + if (tpmi_inst.socket_id >= topology_max_packages())
> + return -EINVAL;
> +
> + tpmi_inst.count = isst_common.sst_inst[tpmi_inst.socket_id]->number_of_power_domains;
> +
> + sst_inst = isst_common.sst_inst[tpmi_inst.socket_id];
> + tpmi_inst.valid_mask = 0;
> + for (i = 0; i < sst_inst->number_of_power_domains; ++i) {
> + struct tpmi_per_power_domain_info *power_domain_info;
> +
> + power_domain_info = &sst_inst->power_domain_info[i];
> + if (power_domain_info->sst_base)
> + tpmi_inst.valid_mask |= BIT(i);
> + }
> +
> + if (copy_to_user(argp, &tpmi_inst, sizeof(tpmi_inst)))
> + return -EFAULT;
> +
> + return 0;
> +}
> +
> +static long isst_if_def_ioctl(struct file *file, unsigned int cmd,
> + unsigned long arg)
> +{
> + void __user *argp = (void __user *)arg;
> + long ret = -ENOTTY;
> +
> + mutex_lock(&isst_tpmi_dev_lock);
> + switch (cmd) {
> + case ISST_IF_COUNT_TPMI_INSTANCES:
> + ret = isst_if_get_tpmi_instance_count(argp);
> + break;
> + default:
> + break;
> + }
> + mutex_unlock(&isst_tpmi_dev_lock);
> +
> + return ret;
> +}
> +
> +int tpmi_sst_dev_add(struct auxiliary_device *auxdev)
> +{
> + struct intel_tpmi_plat_info *plat_info;
> + struct tpmi_sst_struct *tpmi_sst;
> + int i, pkg = 0, inst = 0;
> + int num_resources;
> +
> + plat_info = tpmi_get_platform_data(auxdev);
> + if (!plat_info) {
> + dev_err(&auxdev->dev, "No platform info\n");
> + return -EINVAL;
> + }
> +
> + pkg = plat_info->package_id;
> + if (pkg >= topology_max_packages()) {
> + dev_err(&auxdev->dev, "Invalid package id :%x\n", pkg);
> + return -EINVAL;
> + }
> +
> + if (isst_common.sst_inst[pkg])
> + return -EEXIST;
> +
> + num_resources = tpmi_get_resource_count(auxdev);
> +
> + if (!num_resources)
> + return -EINVAL;
> +
> + tpmi_sst = devm_kzalloc(&auxdev->dev, sizeof(*tpmi_sst), GFP_KERNEL);
> + if (!tpmi_sst)
> + return -ENOMEM;
> +
> + tpmi_sst->power_domain_info = devm_kcalloc(&auxdev->dev, num_resources,
> + sizeof(*tpmi_sst->power_domain_info),
> + GFP_KERNEL);
> + if (!tpmi_sst->power_domain_info)
> + return -ENOMEM;
> +
> + tpmi_sst->number_of_power_domains = num_resources;
> +
> + for (i = 0; i < num_resources; ++i) {
> + struct resource *res;
> +
> + res = tpmi_get_resource_at_index(auxdev, i);
> + if (!res) {
> + tpmi_sst->power_domain_info[i].sst_base = NULL;
> + continue;
> + }
> +
> + tpmi_sst->power_domain_info[i].package_id = pkg;
> + tpmi_sst->power_domain_info[i].power_domain_id = i;
> + tpmi_sst->power_domain_info[i].auxdev = auxdev;
> + tpmi_sst->power_domain_info[i].sst_base = devm_ioremap_resource(&auxdev->dev, res);
> + if (IS_ERR(tpmi_sst->power_domain_info[i].sst_base))
> + return PTR_ERR(tpmi_sst->power_domain_info[i].sst_base);
> +
> + ++inst;
> + }
> +
> + if (!inst)
> + return -ENODEV;
> +
> + tpmi_sst->package_id = pkg;
> + auxiliary_set_drvdata(auxdev, tpmi_sst);
> +
> + mutex_lock(&isst_tpmi_dev_lock);
> + if (isst_common.max_index < pkg)
> + isst_common.max_index = pkg;
> + isst_common.sst_inst[pkg] = tpmi_sst;
> + mutex_unlock(&isst_tpmi_dev_lock);
> +
> + return 0;
> +}
> +EXPORT_SYMBOL_NS_GPL(tpmi_sst_dev_add, INTEL_TPMI_SST);
> +
> +void tpmi_sst_dev_remove(struct auxiliary_device *auxdev)
> +{
> + struct tpmi_sst_struct *tpmi_sst = auxiliary_get_drvdata(auxdev);
> +
> + mutex_lock(&isst_tpmi_dev_lock);
> + isst_common.sst_inst[tpmi_sst->package_id] = NULL;
> + mutex_unlock(&isst_tpmi_dev_lock);
> +}
> +EXPORT_SYMBOL_NS_GPL(tpmi_sst_dev_remove, INTEL_TPMI_SST);
> +
> +#define ISST_TPMI_API_VERSION 0x02
> +
> +int tpmi_sst_init(void)
> +{
> + struct isst_if_cmd_cb cb;
> + int ret = 0;
> +
> + mutex_lock(&isst_tpmi_dev_lock);
> +
> + if (isst_core_usage_count) {
> + ++isst_core_usage_count;
> + goto init_done;
> + }
> +
> + isst_common.sst_inst = kcalloc(topology_max_packages(),
> + sizeof(*isst_common.sst_inst),
> + GFP_KERNEL);
> + if (!isst_common.sst_inst)
> + return -ENOMEM;
> +
> + memset(&cb, 0, sizeof(cb));
> + cb.cmd_size = sizeof(struct isst_if_io_reg);
> + cb.offset = offsetof(struct isst_if_io_regs, io_reg);
> + cb.cmd_callback = NULL;
> + cb.api_version = ISST_TPMI_API_VERSION;
> + cb.def_ioctl = isst_if_def_ioctl;
> + cb.owner = THIS_MODULE;
> + ret = isst_if_cdev_register(ISST_IF_DEV_TPMI, &cb);
> + if (ret)
> + kfree(isst_common.sst_inst);
> +init_done:
> + mutex_unlock(&isst_tpmi_dev_lock);
> + return ret;
> +}
> +EXPORT_SYMBOL_NS_GPL(tpmi_sst_init, INTEL_TPMI_SST);
> +
> +void tpmi_sst_exit(void)
> +{
> + mutex_lock(&isst_tpmi_dev_lock);
> + if (isst_core_usage_count)
> + --isst_core_usage_count;
> +
> + if (!isst_core_usage_count) {
> + isst_if_cdev_unregister(ISST_IF_DEV_TPMI);
> + kfree(isst_common.sst_inst);
> + }
> + mutex_unlock(&isst_tpmi_dev_lock);
> +}
> +EXPORT_SYMBOL_NS_GPL(tpmi_sst_exit, INTEL_TPMI_SST);
> +
> +MODULE_IMPORT_NS(INTEL_TPMI);
> +MODULE_IMPORT_NS(INTEL_TPMI_POWER_DOMAIN);
> +
> +MODULE_LICENSE("GPL");
> diff --git a/drivers/platform/x86/intel/speed_select_if/isst_tpmi_core.h b/drivers/platform/x86/intel/speed_select_if/isst_tpmi_core.h
> new file mode 100644
> index 000000000000..356cb02273b1
> --- /dev/null
> +++ b/drivers/platform/x86/intel/speed_select_if/isst_tpmi_core.h
> @@ -0,0 +1,16 @@
> +/* SPDX-License-Identifier: GPL-2.0-only */
> +/*
> + * Intel Speed Select Interface: Drivers Internal defines
> + * Copyright (c) 2023, Intel Corporation.
> + * All rights reserved.
> + *
> + */
> +
> +#ifndef _ISST_TPMI_CORE_H
> +#define _ISST_TPMI_CORE_H
> +
> +int tpmi_sst_init(void);
> +void tpmi_sst_exit(void);
> +int tpmi_sst_dev_add(struct auxiliary_device *auxdev);
> +void tpmi_sst_dev_remove(struct auxiliary_device *auxdev);
> +#endif
> diff --git a/include/uapi/linux/isst_if.h b/include/uapi/linux/isst_if.h
> index ba078f8e9add..bf32d959f6e8 100644
> --- a/include/uapi/linux/isst_if.h
> +++ b/include/uapi/linux/isst_if.h
> @@ -163,10 +163,28 @@ struct isst_if_msr_cmds {
> struct isst_if_msr_cmd msr_cmd[1];
> };
>
> +/**
> + * struct isst_tpmi_instance_count - Get number of TPMI instances per socket
> + * @socket_id: Socket/package id
> + * @count: Number of instances
> + * @valid_mask: Mask of instances as there can be holes
> + *
> + * Structure used to get TPMI instances information using
> + * IOCTL ISST_IF_COUNT_TPMI_INSTANCES.
> + */
> +struct isst_tpmi_instance_count {
> + __u8 socket_id;
> + __u8 count;
> + __u16 valid_mask;
> +};
> +
> #define ISST_IF_MAGIC 0xFE
> #define ISST_IF_GET_PLATFORM_INFO _IOR(ISST_IF_MAGIC, 0, struct isst_if_platform_info *)
> #define ISST_IF_GET_PHY_ID _IOWR(ISST_IF_MAGIC, 1, struct isst_if_cpu_map *)
> #define ISST_IF_IO_CMD _IOW(ISST_IF_MAGIC, 2, struct isst_if_io_regs *)
> #define ISST_IF_MBOX_COMMAND _IOWR(ISST_IF_MAGIC, 3, struct isst_if_mbox_cmds *)
> #define ISST_IF_MSR_COMMAND _IOWR(ISST_IF_MAGIC, 4, struct isst_if_msr_cmds *)
> +
> +#define ISST_IF_COUNT_TPMI_INSTANCES _IOR(ISST_IF_MAGIC, 5, struct isst_tpmi_instance_count *)
> +
> #endif
