platform/x86/intel/vsec: Add device links to enforce dependencies

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@linux.intel.com>
Link: https://lore.kernel.org/r/20250703022832.1302928-5-david.e.box@linux.intel.com
Reviewed-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>

diff --git a/drivers/platform/x86/intel/vsec.c b/drivers/platform/x86/intel/vsec.c
index 8bdb74d86f240200e76967a5a623d952872310be..aa1f7e63039d0ae6731fed7c7d6efc15b4faa792 100644 (file)
--- a/drivers/platform/x86/intel/vsec.c
+++ b/drivers/platform/x86/intel/vsec.c
@@ -15,9 +15,12 @@
 
 #include <linux/auxiliary_bus.h>
 #include <linux/bits.h>
+#include <linux/bitops.h>
+#include <linux/bug.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 +35,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 +107,74 @@ 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))
+                       return &deps[consumer_id];
+
+       return NULL;
+}
+
+/*
+ * 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) {
+               struct device_link *link;
+
+               if (state[supplier_id] != STATE_REGISTERED)
+                       continue;
+
+               if (!suppliers[supplier_id]) {
+                       dev_err(dev, "Bad supplier list\n");
+                       return -EINVAL;
+               }
+
+               link = device_link_add(dev, suppliers[supplier_id],
+                                      DL_FLAG_AUTOPROBE_CONSUMER);
+               if (!link)
+                       return -EINVAL;
+       }
+
+       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,19 +212,37 @@ 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
+        * supplies 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)
+               goto cleanup_aux;
+
+       ret = intel_vsec_link_devices(pdev, &auxdev->dev, intel_vsec_dev->cap_id);
+       if (ret)
+               goto cleanup_aux;
+
        ret = auxiliary_device_add(auxdev);
-       if (ret < 0) {
-               auxiliary_device_uninit(auxdev);
-               return ret;
-       }
+       if (ret)
+               goto cleanup_aux;
 
        return devm_add_action_or_reset(parent, intel_vsec_remove_aux,
                                       auxdev);
+
+cleanup_aux:
+       auxiliary_device_uninit(auxdev);
+       return ret;
 }
 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 +309,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;
@@ -225,6 +324,101 @@ static int intel_vsec_add_dev(struct pci_dev *pdev, struct intel_vsec_header *he
                                  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 (WARN_ON_ONCE(consumer_deps->feature != BIT(cap_id)))
+               return false;
+
+       /*
+        * Verify that all required suppliers have been found. Return false
+        * immediately if any are still missing.
+        */
+       for_each_set_bit(supplier_id, &consumer_deps->supplier_bitmap, VSEC_FEATURE_COUNT) {
+               if (state[supplier_id] == STATE_SKIP)
+                       continue;
+
+               if (state[supplier_id] == STATE_NOT_FOUND)
+                       return false;
+       }
+
+       /*
+        * 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;
 
@@ -378,7 +572,8 @@ 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;
-       int ret;
+       int num_caps, ret;
+       bool found_any = false;
 
        ret = pcim_enable_device(pdev);
        if (ret)
@@ -396,7 +591,15 @@ static int intel_vsec_pci_probe(struct pci_dev *pdev, const struct pci_device_id
        priv->info = info;
        pci_set_drvdata(pdev, priv);
 
-       if (!intel_vsec_get_features(pdev, info))
+       num_caps = hweight_long(info->caps);
+       while (num_caps--) {
+               found_any |= intel_vsec_get_features(pdev, info);
+
+               if (priv->found_caps == info->caps)
+                       break;
+       }
+
+       if (!found_any)
                return -ENODEV;
 
        return 0;

diff --git a/include/linux/intel_vsec.h b/include/linux/intel_vsec.h
index bc95821f1bfb2abfb60bf253d721dd157cde8792..71067afaca9922894d8ea933e56067475ddf1fa7 100644 (file)
--- 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,