[PATCH 04/15] platform/x86/intel/vsec: Add device links to enforce dependencies
From: David E. Box
Date: Wed Apr 30 2025 - 17:22:17 EST
New Intel VSEC features will have dependencies on other features, requiring
certain supplier drivers to be probed before their consumers. To enforce
this dependency ordering, introduce device links using device_link_add(),
ensuring that suppliers are fully registered before consumers are probed.
- Add device link tracking by storing supplier devices and tracking their
state.
- Implement intel_vsec_link_devices() to establish links between suppliers
and consumers based on feature dependencies.
- Add get_consumer_dependencies() to retrieve supplier-consumer
relationships.
- Modify feature registration logic:
* Consumers now check that all required suppliers are registered before
being initialized.
* suppliers_ready() verifies that all required supplier devices are
available.
- Prevent potential null consumer name issue in sysfs:
- Use dev_set_name() when creating auxiliary devices to ensure a
unique, non-null consumer name.
- Update intel_vsec_pci_probe() to loop up to the number of possible
features or when all devices are registered, whichever comes first.
- Introduce VSEC_CAP_UNUSED to prevent sub-features (registered via
exported APIs) from being mistakenly linked.
Signed-off-by: David E. Box <david.e.box@xxxxxxxxxxxxxxx>
---
drivers/platform/x86/intel/vsec.c | 214 +++++++++++++++++++++++++++++-
include/linux/intel_vsec.h | 28 +++-
2 files changed, 230 insertions(+), 12 deletions(-)
diff --git a/drivers/platform/x86/intel/vsec.c b/drivers/platform/x86/intel/vsec.c
index f01651f498ca..8700ed7656df 100644
--- a/drivers/platform/x86/intel/vsec.c
+++ b/drivers/platform/x86/intel/vsec.c
@@ -15,9 +15,11 @@
#include <linux/auxiliary_bus.h>
#include <linux/bits.h>
+#include <linux/bitops.h>
#include <linux/cleanup.h>
#include <linux/delay.h>
#include <linux/idr.h>
+#include <linux/log2.h>
#include <linux/intel_vsec.h>
#include <linux/kernel.h>
#include <linux/module.h>
@@ -32,8 +34,17 @@ static DEFINE_IDA(intel_vsec_ida);
static DEFINE_IDA(intel_vsec_sdsi_ida);
static DEFINE_XARRAY_ALLOC(auxdev_array);
+enum vsec_device_state {
+ STATE_NOT_FOUND,
+ STATE_REGISTERED,
+ STATE_SKIP,
+};
+
struct vsec_priv {
struct intel_vsec_platform_info *info;
+ struct device *suppliers[VSEC_FEATURE_COUNT];
+ enum vsec_device_state state[VSEC_FEATURE_COUNT];
+ unsigned long found_caps;
};
static const char *intel_vsec_name(enum intel_vsec_id id)
@@ -95,6 +106,72 @@ static void intel_vsec_dev_release(struct device *dev)
kfree(intel_vsec_dev);
}
+static const struct vsec_feature_dependency *
+get_consumer_dependencies(struct vsec_priv *priv, int cap_id)
+{
+ const struct vsec_feature_dependency *deps = priv->info->deps;
+ int consumer_id = priv->info->num_deps;
+
+ if (!deps)
+ return NULL;
+
+ while (consumer_id--)
+ if (deps[consumer_id].feature == BIT(cap_id))
+ break;
+
+ if (consumer_id < 0)
+ return NULL;
+
+ return &deps[consumer_id];
+}
+
+/*
+ * Although pci_device_id table is available in the pdev, this prototype is
+ * necessary because the code using it can be called by an exported API that
+ * might pass a different pdev.
+ */
+static const struct pci_device_id intel_vsec_pci_ids[];
+
+static int intel_vsec_link_devices(struct pci_dev *pdev, struct device *dev,
+ int consumer_id)
+{
+ const struct vsec_feature_dependency *deps;
+ enum vsec_device_state *state;
+ struct device **suppliers;
+ struct vsec_priv *priv;
+ int supplier_id;
+
+ if (!consumer_id)
+ return 0;
+
+ if (!pci_match_id(intel_vsec_pci_ids, pdev))
+ return 0;
+
+ priv = pci_get_drvdata(pdev);
+ state = priv->state;
+ suppliers = priv->suppliers;
+
+ priv->suppliers[consumer_id] = dev;
+
+ deps = get_consumer_dependencies(priv, consumer_id);
+ if (!deps)
+ return 0;
+
+ for_each_set_bit(supplier_id, &deps->supplier_bitmap, VSEC_FEATURE_COUNT) {
+ if (state[supplier_id] != STATE_REGISTERED)
+ continue;
+
+ if (!suppliers[supplier_id]) {
+ dev_err(dev, "Bad supplier list\n");
+ return -EINVAL;
+ }
+
+ device_link_add(dev, suppliers[supplier_id], DL_FLAG_AUTOPROBE_CONSUMER);
+ }
+
+ return 0;
+}
+
int intel_vsec_add_aux(struct pci_dev *pdev, struct device *parent,
struct intel_vsec_device *intel_vsec_dev,
const char *name)
@@ -132,6 +209,25 @@ int intel_vsec_add_aux(struct pci_dev *pdev, struct device *parent,
return ret;
}
+ /*
+ * Assign a name now to ensure that the device link doesn't contain
+ * a null string for the consumer name. This is a problem when a supplier
+ * supplys more than one consumer and can lead to a duplicate name error
+ * when the link is created in sysfs.
+ */
+ ret = dev_set_name(&auxdev->dev, "%s.%s.%d", KBUILD_MODNAME, auxdev->name,
+ auxdev->id);
+ if (ret) {
+ auxiliary_device_uninit(auxdev);
+ return ret;
+ }
+
+ ret = intel_vsec_link_devices(pdev, &auxdev->dev, intel_vsec_dev->cap_id);
+ if (ret) {
+ auxiliary_device_uninit(auxdev);
+ return ret;
+ }
+
ret = auxiliary_device_add(auxdev);
if (ret < 0) {
auxiliary_device_uninit(auxdev);
@@ -144,7 +240,8 @@ int intel_vsec_add_aux(struct pci_dev *pdev, struct device *parent,
EXPORT_SYMBOL_NS_GPL(intel_vsec_add_aux, "INTEL_VSEC");
static int intel_vsec_add_dev(struct pci_dev *pdev, struct intel_vsec_header *header,
- struct intel_vsec_platform_info *info)
+ struct intel_vsec_platform_info *info,
+ unsigned long cap_id)
{
struct intel_vsec_device __free(kfree) *intel_vsec_dev = NULL;
struct resource __free(kfree) *res = NULL;
@@ -211,6 +308,7 @@ static int intel_vsec_add_dev(struct pci_dev *pdev, struct intel_vsec_header *he
intel_vsec_dev->quirks = info->quirks;
intel_vsec_dev->base_addr = info->base_addr;
intel_vsec_dev->priv_data = info->priv_data;
+ intel_vsec_dev->cap_id = cap_id;
if (header->id == VSEC_ID_SDSI)
intel_vsec_dev->ida = &intel_vsec_sdsi_ida;
@@ -219,12 +317,108 @@ static int intel_vsec_add_dev(struct pci_dev *pdev, struct intel_vsec_header *he
/*
* Pass the ownership of intel_vsec_dev and resource within it to
- * intel_vsec_add_aux()
+ * intel_vsec_add_aux().
*/
return intel_vsec_add_aux(pdev, parent, no_free_ptr(intel_vsec_dev),
intel_vsec_name(header->id));
}
+static bool suppliers_ready(struct vsec_priv *priv,
+ const struct vsec_feature_dependency *consumer_deps,
+ int cap_id)
+{
+ enum vsec_device_state *state = priv->state;
+ int supplier_id;
+
+ if (consumer_deps->feature != BIT(cap_id))
+ return false; /* Should not happen */
+
+ /*
+ * Find all features that are suppliers and check their state.
+ * Only suppliers that have been successfully registered will be linked.
+ */
+ for_each_set_bit(supplier_id, &consumer_deps->supplier_bitmap, VSEC_FEATURE_COUNT) {
+ if (state[supplier_id] == STATE_SKIP)
+ continue;
+
+ /* If any supplier is not yet found, return immediately */
+ if (state[supplier_id] == STATE_NOT_FOUND)
+ return false;
+ }
+
+ /*
+ * If we get here, all suppliers have been found and the consumer
+ * is ready to be registered.
+ */
+ return true;
+}
+
+static int get_cap_id(u32 header_id, unsigned long *cap_id)
+{
+ switch (header_id) {
+ case VSEC_ID_TELEMETRY:
+ *cap_id = ilog2(VSEC_CAP_TELEMETRY);
+ break;
+ case VSEC_ID_WATCHER:
+ *cap_id = ilog2(VSEC_CAP_WATCHER);
+ break;
+ case VSEC_ID_CRASHLOG:
+ *cap_id = ilog2(VSEC_CAP_CRASHLOG);
+ break;
+ case VSEC_ID_SDSI:
+ *cap_id = ilog2(VSEC_CAP_SDSI);
+ break;
+ case VSEC_ID_TPMI:
+ *cap_id = ilog2(VSEC_CAP_TPMI);
+ break;
+ default:
+ return -EINVAL;
+ }
+
+ return 0;
+}
+
+static int intel_vsec_register_device(struct pci_dev *pdev,
+ struct intel_vsec_header *header,
+ struct intel_vsec_platform_info *info)
+{
+ const struct vsec_feature_dependency *consumer_deps;
+ struct vsec_priv *priv;
+ unsigned long cap_id;
+ int ret;
+
+ ret = get_cap_id(header->id, &cap_id);
+ if (ret)
+ return ret;
+
+ /*
+ * Only track dependencies for devices probed by the VSEC driver.
+ * For others using the exported APIs, add the device directly.
+ */
+ if (!pci_match_id(intel_vsec_pci_ids, pdev))
+ return intel_vsec_add_dev(pdev, header, info, cap_id);
+
+ priv = pci_get_drvdata(pdev);
+ if (priv->state[cap_id] == STATE_REGISTERED ||
+ priv->state[cap_id] == STATE_SKIP)
+ return -EEXIST;
+
+ priv->found_caps |= BIT(cap_id);
+
+ consumer_deps = get_consumer_dependencies(priv, cap_id);
+ if (!consumer_deps || suppliers_ready(priv, consumer_deps, cap_id)) {
+ ret = intel_vsec_add_dev(pdev, header, info, cap_id);
+ if (ret)
+ priv->state[cap_id] = STATE_SKIP;
+ else
+ priv->state[cap_id] = STATE_REGISTERED;
+
+ return ret;
+ }
+
+ return -EAGAIN;
+}
+
static bool intel_vsec_walk_header(struct pci_dev *pdev,
struct intel_vsec_platform_info *info)
{
@@ -233,7 +427,7 @@ static bool intel_vsec_walk_header(struct pci_dev *pdev,
int ret;
for ( ; *header; header++) {
- ret = intel_vsec_add_dev(pdev, *header, info);
+ ret = intel_vsec_register_device(pdev, *header, info);
if (!ret)
have_devices = true;
}
@@ -281,7 +475,7 @@ static bool intel_vsec_walk_dvsec(struct pci_dev *pdev,
pci_read_config_dword(pdev, pos + PCI_DVSEC_HEADER2, &hdr);
header.id = PCI_DVSEC_HEADER2_ID(hdr);
- ret = intel_vsec_add_dev(pdev, &header, info);
+ ret = intel_vsec_register_device(pdev, &header, info);
if (ret)
continue;
@@ -326,7 +520,7 @@ static bool intel_vsec_walk_vsec(struct pci_dev *pdev,
header.tbir = INTEL_DVSEC_TABLE_BAR(table);
header.offset = INTEL_DVSEC_TABLE_OFFSET(table);
- ret = intel_vsec_add_dev(pdev, &header, info);
+ ret = intel_vsec_register_device(pdev, &header, info);
if (ret)
continue;
@@ -375,7 +569,7 @@ static int intel_vsec_pci_probe(struct pci_dev *pdev, const struct pci_device_id
struct intel_vsec_platform_info *info;
struct vsec_priv *priv;
bool have_devices = false;
- int ret;
+ int num_caps, ret;
ret = pcim_enable_device(pdev);
if (ret)
@@ -393,7 +587,13 @@ static int intel_vsec_pci_probe(struct pci_dev *pdev, const struct pci_device_id
priv->info = info;
pci_set_drvdata(pdev, priv);
- intel_vsec_feature_walk(pdev, &have_devices, info);
+ num_caps = hweight_long(info->caps);
+ while (num_caps--) {
+ intel_vsec_feature_walk(pdev, &have_devices, info);
+
+ if (priv->found_caps == priv->info->caps)
+ break;
+ }
if (!have_devices)
return -ENODEV;
diff --git a/include/linux/intel_vsec.h b/include/linux/intel_vsec.h
index bc95821f1bfb..71067afaca99 100644
--- a/include/linux/intel_vsec.h
+++ b/include/linux/intel_vsec.h
@@ -5,11 +5,18 @@
#include <linux/auxiliary_bus.h>
#include <linux/bits.h>
-#define VSEC_CAP_TELEMETRY BIT(0)
-#define VSEC_CAP_WATCHER BIT(1)
-#define VSEC_CAP_CRASHLOG BIT(2)
-#define VSEC_CAP_SDSI BIT(3)
-#define VSEC_CAP_TPMI BIT(4)
+/*
+ * VSEC_CAP_UNUSED is reserved. It exists to prevent zero initialized
+ * intel_vsec devices from being automatically set to a known
+ * capability with ID 0
+ */
+#define VSEC_CAP_UNUSED BIT(0)
+#define VSEC_CAP_TELEMETRY BIT(1)
+#define VSEC_CAP_WATCHER BIT(2)
+#define VSEC_CAP_CRASHLOG BIT(3)
+#define VSEC_CAP_SDSI BIT(4)
+#define VSEC_CAP_TPMI BIT(5)
+#define VSEC_FEATURE_COUNT 6
/* Intel DVSEC offsets */
#define INTEL_DVSEC_ENTRIES 0xA
@@ -81,22 +88,31 @@ struct pmt_callbacks {
int (*read_telem)(struct pci_dev *pdev, u32 guid, u64 *data, loff_t off, u32 count);
};
+struct vsec_feature_dependency {
+ unsigned long feature;
+ unsigned long supplier_bitmap;
+};
+
/**
* struct intel_vsec_platform_info - Platform specific data
* @parent: parent device in the auxbus chain
* @headers: list of headers to define the PMT client devices to create
+ * @deps: array of feature dependencies
* @priv_data: private data, usable by parent devices, currently a callback
* @caps: bitmask of PMT capabilities for the given headers
* @quirks: bitmask of VSEC device quirks
* @base_addr: allow a base address to be specified (rather than derived)
+ * @num_deps: Count feature dependencies
*/
struct intel_vsec_platform_info {
struct device *parent;
struct intel_vsec_header **headers;
+ const struct vsec_feature_dependency *deps;
void *priv_data;
unsigned long caps;
unsigned long quirks;
u64 base_addr;
+ int num_deps;
};
/**
@@ -110,6 +126,7 @@ struct intel_vsec_platform_info {
* @priv_data: any private data needed
* @quirks: specified quirks
* @base_addr: base address of entries (if specified)
+ * @cap_id: the enumerated id of the vsec feature
*/
struct intel_vsec_device {
struct auxiliary_device auxdev;
@@ -122,6 +139,7 @@ struct intel_vsec_device {
size_t priv_data_size;
unsigned long quirks;
u64 base_addr;
+ unsigned long cap_id;
};
int intel_vsec_add_aux(struct pci_dev *pdev, struct device *parent,
--
2.43.0