* [PATCH v5 0/6] platform/x86/intel/vsec: Prep for ACPI PMT discovery
@ 2026-02-25 1:50 David E. Box
2026-02-25 1:50 ` [PATCH v5 1/6] platform/x86/intel/vsec: Refactor base_addr handling David E. Box
` (6 more replies)
0 siblings, 7 replies; 14+ messages in thread
From: David E. Box @ 2026-02-25 1:50 UTC (permalink / raw)
To: thomas.hellstrom, rodrigo.vivi, irenic.rajneesh, ilpo.jarvinen,
srinivas.pandruvada, intel-xe, dri-devel, xi.pardee
Cc: david.e.box, hansg, linux-kernel, platform-driver-x86
This series updates intel_vsec to improve const-correctness, decouple
helper APIs from PCI, enhance error handling, and plumb ACPI-based Intel
Platform Monitoring Technology (PMT) discovery through the vsec layer. It
is preparatory infrastructure for follow-on PMT core/telemetry and
PMC/SSRAM series that add ACPI discovery and support for new platforms.
The series is organized as follows:
Patches 1-2 refactor and improve const-correctness of base_addr handling.
Patch 1 makes base_addr an explicit parameter throughout the call chain,
clarifying ownership and removing conditional logic. Patch 2 then makes the
platform info data structure read-only, preventing unintended modifications
to shared driver data.
Patches 3-4 decouple the vsec layer from PCI-specific types, updating
helper APIs and data structures to use generic struct device instead of
struct pci_dev. This enables vsec to work with both PCI and ACPI parent
devices.
Patch 5 enhances error visibility by returning meaningful error codes from
the registration path instead of collapsing to boolean success/failure.
Patch 6 adds infrastructure for ACPI-based PMT discovery, allowing client
drivers to consume discovery data from either PCI or ACPI sources.
Signed-off-by: David E. Box <david.e.box@linux.intel.com>
David E. Box (6):
platform/x86/intel/vsec: Refactor base_addr handling
platform/x86/intel/vsec: Make driver_data info const
platform/x86/intel/vsec: Decouple add/link helpers from PCI
platform/x86/intel/vsec: Switch exported helpers from pci_dev to device
platform/x86/intel/vsec: Return real error codes from registration path
platform/x86/intel/vsec: Plumb ACPI PMT discovery tables through vsec
drivers/gpu/drm/xe/xe_debugfs.c | 2 +-
drivers/gpu/drm/xe/xe_hwmon.c | 2 +-
drivers/gpu/drm/xe/xe_vsec.c | 7 +-
drivers/gpu/drm/xe/xe_vsec.h | 4 +-
drivers/platform/x86/intel/pmc/core.c | 4 +-
drivers/platform/x86/intel/pmc/ssram_telemetry.c | 2 +-
drivers/platform/x86/intel/pmt/class.c | 8 +-
drivers/platform/x86/intel/pmt/class.h | 5 +-
drivers/platform/x86/intel/pmt/discovery.c | 4 +-
drivers/platform/x86/intel/pmt/telemetry.c | 13 +--
drivers/platform/x86/intel/pmt/telemetry.h | 12 +--
drivers/platform/x86/intel/sdsi.c | 5 +-
drivers/platform/x86/intel/vsec.c | 121 ++++++++++++++---------
drivers/platform/x86/intel/vsec_tpmi.c | 10 +-
include/linux/intel_vsec.h | 39 ++++++--
15 files changed, 144 insertions(+), 94 deletions(-)
---
base-commit: 6de23f81a5e08be8fbf5e8d7e9febc72a5b5f27f
--
^ permalink raw reply [flat|nested] 14+ messages in thread* [PATCH v5 1/6] platform/x86/intel/vsec: Refactor base_addr handling 2026-02-25 1:50 [PATCH v5 0/6] platform/x86/intel/vsec: Prep for ACPI PMT discovery David E. Box @ 2026-02-25 1:50 ` David E. Box 2026-02-27 4:12 ` Claude review: " Claude Code Review Bot 2026-02-25 1:50 ` [PATCH v5 2/6] platform/x86/intel/vsec: Make driver_data info const David E. Box ` (5 subsequent siblings) 6 siblings, 1 reply; 14+ messages in thread From: David E. Box @ 2026-02-25 1:50 UTC (permalink / raw) To: thomas.hellstrom, rodrigo.vivi, irenic.rajneesh, ilpo.jarvinen, srinivas.pandruvada, intel-xe, dri-devel, xi.pardee Cc: david.e.box, hansg, linux-kernel, platform-driver-x86 The base_addr field in intel_vsec_platform_info was originally added to support devices that emulate PCI VSEC capabilities in MMIO. Previously, the code would check at registration time whether base_addr was set, falling back to the PCI BAR if not. Refactor this by making base_addr an explicit function parameter. This clarifies ownership of the value and removes conditional logic from intel_vsec_add_dev(). It also enables making intel_vsec_platform_info const in a later patch, since the function no longer needs to write to info->base_addr. No functional change intended. Signed-off-by: David E. Box <david.e.box@linux.intel.com> --- Changes in v5: - No change Changes in v4: - No change Changes in v3: - No change Changes in v2: - Use pci_resource_start() macro instead of direct pdev->resource array access (suggested by Ilpo) drivers/platform/x86/intel/vsec.c | 23 ++++++++++------------- 1 file changed, 10 insertions(+), 13 deletions(-) diff --git a/drivers/platform/x86/intel/vsec.c b/drivers/platform/x86/intel/vsec.c index 5059d320edf8..46966edca03b 100644 --- a/drivers/platform/x86/intel/vsec.c +++ b/drivers/platform/x86/intel/vsec.c @@ -271,14 +271,13 @@ 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, - unsigned long cap_id) + unsigned long cap_id, u64 base_addr) { struct intel_vsec_device __free(kfree) *intel_vsec_dev = NULL; struct resource __free(kfree) *res = NULL; struct resource *tmp; struct device *parent; unsigned long quirks = info->quirks; - u64 base_addr; int i; if (info->parent) @@ -310,11 +309,6 @@ static int intel_vsec_add_dev(struct pci_dev *pdev, struct intel_vsec_header *he if (quirks & VSEC_QUIRK_TABLE_SHIFT) header->offset >>= TABLE_OFFSET_SHIFT; - if (info->base_addr) - base_addr = info->base_addr; - else - base_addr = pdev->resource[header->tbir].start; - /* * The DVSEC/VSEC contains the starting offset and count for a block of * discovery tables. Create a resource array of these tables to the @@ -412,7 +406,8 @@ static int get_cap_id(u32 header_id, unsigned long *cap_id) static int intel_vsec_register_device(struct pci_dev *pdev, struct intel_vsec_header *header, - struct intel_vsec_platform_info *info) + struct intel_vsec_platform_info *info, + u64 base_addr) { const struct vsec_feature_dependency *consumer_deps; struct vsec_priv *priv; @@ -428,7 +423,7 @@ static int intel_vsec_register_device(struct pci_dev *pdev, * 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); + return intel_vsec_add_dev(pdev, header, info, cap_id, base_addr); priv = pci_get_drvdata(pdev); if (priv->state[cap_id] == STATE_REGISTERED || @@ -444,7 +439,7 @@ static int intel_vsec_register_device(struct pci_dev *pdev, 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); + ret = intel_vsec_add_dev(pdev, header, info, cap_id, base_addr); if (ret) priv->state[cap_id] = STATE_SKIP; else @@ -464,7 +459,7 @@ static bool intel_vsec_walk_header(struct pci_dev *pdev, int ret; for ( ; *header; header++) { - ret = intel_vsec_register_device(pdev, *header, info); + ret = intel_vsec_register_device(pdev, *header, info, info->base_addr); if (!ret) have_devices = true; } @@ -512,7 +507,8 @@ 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_register_device(pdev, &header, info); + ret = intel_vsec_register_device(pdev, &header, info, + pci_resource_start(pdev, header.tbir)); if (ret) continue; @@ -557,7 +553,8 @@ 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_register_device(pdev, &header, info); + ret = intel_vsec_register_device(pdev, &header, info, + pci_resource_start(pdev, header.tbir)); if (ret) continue; -- 2.43.0 ^ permalink raw reply related [flat|nested] 14+ messages in thread
* Claude review: platform/x86/intel/vsec: Refactor base_addr handling 2026-02-25 1:50 ` [PATCH v5 1/6] platform/x86/intel/vsec: Refactor base_addr handling David E. Box @ 2026-02-27 4:12 ` Claude Code Review Bot 0 siblings, 0 replies; 14+ messages in thread From: Claude Code Review Bot @ 2026-02-27 4:12 UTC (permalink / raw) To: dri-devel-reviews Patch Review **Overall: Clean, straightforward refactor.** This patch removes the conditional `info->base_addr` vs `pdev->resource[header->tbir].start` logic from `intel_vsec_add_dev()` and instead passes `base_addr` as an explicit parameter all the way down the call chain. The callers now resolve the address: `intel_vsec_walk_header()` passes `info->base_addr`, while `intel_vsec_walk_dvsec()` and `intel_vsec_walk_vsec()` use `pci_resource_start()`. This is a good preparation for making `info` const later and for ACPI paths where BAR lookups don't apply. No issues found. --- --- Generated by Claude Code Patch Reviewer ^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH v5 2/6] platform/x86/intel/vsec: Make driver_data info const 2026-02-25 1:50 [PATCH v5 0/6] platform/x86/intel/vsec: Prep for ACPI PMT discovery David E. Box 2026-02-25 1:50 ` [PATCH v5 1/6] platform/x86/intel/vsec: Refactor base_addr handling David E. Box @ 2026-02-25 1:50 ` David E. Box 2026-02-27 4:12 ` Claude review: " Claude Code Review Bot 2026-02-25 1:50 ` [PATCH v5 3/6] platform/x86/intel/vsec: Decouple add/link helpers from PCI David E. Box ` (4 subsequent siblings) 6 siblings, 1 reply; 14+ messages in thread From: David E. Box @ 2026-02-25 1:50 UTC (permalink / raw) To: thomas.hellstrom, rodrigo.vivi, irenic.rajneesh, ilpo.jarvinen, srinivas.pandruvada, intel-xe, dri-devel, xi.pardee Cc: david.e.box, hansg, linux-kernel, platform-driver-x86 Treat PCI id->driver_data (intel_vsec_platform_info) as read-only by making vsec_priv->info a const pointer and updating all function signatures to accept const intel_vsec_platform_info *. This improves const-correctness and clarifies that the platform info data from the driver_data table is not meant to be modified at runtime. No functional changes intended. Signed-off-by: David E. Box <david.e.box@linux.intel.com> --- Changes in v5: - No change Changes in v4: - No change Changes in v3: - No change Changes in v2: - New patch drivers/platform/x86/intel/vsec.c | 20 ++++++++++---------- include/linux/intel_vsec.h | 4 ++-- 2 files changed, 12 insertions(+), 12 deletions(-) diff --git a/drivers/platform/x86/intel/vsec.c b/drivers/platform/x86/intel/vsec.c index 46966edca03b..e0096be605d9 100644 --- a/drivers/platform/x86/intel/vsec.c +++ b/drivers/platform/x86/intel/vsec.c @@ -42,7 +42,7 @@ enum vsec_device_state { }; struct vsec_priv { - struct intel_vsec_platform_info *info; + const struct intel_vsec_platform_info *info; struct device *suppliers[VSEC_FEATURE_COUNT]; struct oobmsm_plat_info plat_info; enum vsec_device_state state[VSEC_FEATURE_COUNT]; @@ -270,7 +270,7 @@ 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, + const struct intel_vsec_platform_info *info, unsigned long cap_id, u64 base_addr) { struct intel_vsec_device __free(kfree) *intel_vsec_dev = NULL; @@ -406,7 +406,7 @@ static int get_cap_id(u32 header_id, unsigned long *cap_id) static int intel_vsec_register_device(struct pci_dev *pdev, struct intel_vsec_header *header, - struct intel_vsec_platform_info *info, + const struct intel_vsec_platform_info *info, u64 base_addr) { const struct vsec_feature_dependency *consumer_deps; @@ -452,7 +452,7 @@ static int intel_vsec_register_device(struct pci_dev *pdev, } static bool intel_vsec_walk_header(struct pci_dev *pdev, - struct intel_vsec_platform_info *info) + const struct intel_vsec_platform_info *info) { struct intel_vsec_header **header = info->headers; bool have_devices = false; @@ -468,7 +468,7 @@ static bool intel_vsec_walk_header(struct pci_dev *pdev, } static bool intel_vsec_walk_dvsec(struct pci_dev *pdev, - struct intel_vsec_platform_info *info) + const struct intel_vsec_platform_info *info) { bool have_devices = false; int pos = 0; @@ -519,7 +519,7 @@ static bool intel_vsec_walk_dvsec(struct pci_dev *pdev, } static bool intel_vsec_walk_vsec(struct pci_dev *pdev, - struct intel_vsec_platform_info *info) + const struct intel_vsec_platform_info *info) { bool have_devices = false; int pos = 0; @@ -565,7 +565,7 @@ static bool intel_vsec_walk_vsec(struct pci_dev *pdev, } int intel_vsec_register(struct pci_dev *pdev, - struct intel_vsec_platform_info *info) + const struct intel_vsec_platform_info *info) { if (!pdev || !info || !info->headers) return -EINVAL; @@ -578,7 +578,7 @@ int intel_vsec_register(struct pci_dev *pdev, EXPORT_SYMBOL_NS_GPL(intel_vsec_register, "INTEL_VSEC"); static bool intel_vsec_get_features(struct pci_dev *pdev, - struct intel_vsec_platform_info *info) + const struct intel_vsec_platform_info *info) { bool found = false; @@ -622,7 +622,7 @@ static void intel_vsec_skip_missing_dependencies(struct pci_dev *pdev) static int intel_vsec_pci_probe(struct pci_dev *pdev, const struct pci_device_id *id) { - struct intel_vsec_platform_info *info; + const struct intel_vsec_platform_info *info; struct vsec_priv *priv; int num_caps, ret; int run_once = 0; @@ -633,7 +633,7 @@ static int intel_vsec_pci_probe(struct pci_dev *pdev, const struct pci_device_id return ret; pci_save_state(pdev); - info = (struct intel_vsec_platform_info *)id->driver_data; + info = (const struct intel_vsec_platform_info *)id->driver_data; if (!info) return -EINVAL; diff --git a/include/linux/intel_vsec.h b/include/linux/intel_vsec.h index 1a0f357c2427..d551174b0049 100644 --- a/include/linux/intel_vsec.h +++ b/include/linux/intel_vsec.h @@ -200,13 +200,13 @@ static inline struct intel_vsec_device *auxdev_to_ivdev(struct auxiliary_device #if IS_ENABLED(CONFIG_INTEL_VSEC) int intel_vsec_register(struct pci_dev *pdev, - struct intel_vsec_platform_info *info); + const struct intel_vsec_platform_info *info); int intel_vsec_set_mapping(struct oobmsm_plat_info *plat_info, struct intel_vsec_device *vsec_dev); struct oobmsm_plat_info *intel_vsec_get_mapping(struct pci_dev *pdev); #else static inline int intel_vsec_register(struct pci_dev *pdev, - struct intel_vsec_platform_info *info) + const struct intel_vsec_platform_info *info) { return -ENODEV; } -- 2.43.0 ^ permalink raw reply related [flat|nested] 14+ messages in thread
* Claude review: platform/x86/intel/vsec: Make driver_data info const 2026-02-25 1:50 ` [PATCH v5 2/6] platform/x86/intel/vsec: Make driver_data info const David E. Box @ 2026-02-27 4:12 ` Claude Code Review Bot 0 siblings, 0 replies; 14+ messages in thread From: Claude Code Review Bot @ 2026-02-27 4:12 UTC (permalink / raw) To: dri-devel-reviews Patch Review **Overall: Clean const-correctness improvement.** Makes `vsec_priv->info` a `const` pointer and updates all function signatures that accept `intel_vsec_platform_info *` to `const`. The cast in `intel_vsec_pci_probe()`: ```c info = (const struct intel_vsec_platform_info *)id->driver_data; ``` is correct since `driver_data` is `unsigned long` and this is a standard PCI ID table pattern. Minor nit: the alignment of `const` in the `intel_vsec_register` declaration differs slightly between the two hunks but this is cosmetic. No issues found. --- --- Generated by Claude Code Patch Reviewer ^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH v5 3/6] platform/x86/intel/vsec: Decouple add/link helpers from PCI 2026-02-25 1:50 [PATCH v5 0/6] platform/x86/intel/vsec: Prep for ACPI PMT discovery David E. Box 2026-02-25 1:50 ` [PATCH v5 1/6] platform/x86/intel/vsec: Refactor base_addr handling David E. Box 2026-02-25 1:50 ` [PATCH v5 2/6] platform/x86/intel/vsec: Make driver_data info const David E. Box @ 2026-02-25 1:50 ` David E. Box 2026-02-27 4:12 ` Claude review: " Claude Code Review Bot 2026-02-25 1:50 ` [PATCH v5 4/6] platform/x86/intel/vsec: Switch exported helpers from pci_dev to device David E. Box ` (3 subsequent siblings) 6 siblings, 1 reply; 14+ messages in thread From: David E. Box @ 2026-02-25 1:50 UTC (permalink / raw) To: thomas.hellstrom, rodrigo.vivi, irenic.rajneesh, ilpo.jarvinen, srinivas.pandruvada, intel-xe, dri-devel, xi.pardee Cc: david.e.box, hansg, linux-kernel, platform-driver-x86 This refactor is prepares for adding ACPI-enumerated PMT endpoints. While intel_vsec is bound to PCI today, some helpers are used by code that will also register PMT endpoints from non-PCI (ACPI) paths. Clean up PCI-specific plumbing where it isn’t strictly required and rely on generic struct device where possible. Reviewed-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com> Signed-off-by: David E. Box <david.e.box@linux.intel.com> --- Changes in v5: - No change Changes in v4: - No change Changes in v3: - No change Changes in v2: - No chnage (previous patch 1) drivers/platform/x86/intel/vsec.c | 13 +++++++++---- drivers/platform/x86/intel/vsec_tpmi.c | 2 +- include/linux/intel_vsec.h | 2 +- 3 files changed, 11 insertions(+), 6 deletions(-) diff --git a/drivers/platform/x86/intel/vsec.c b/drivers/platform/x86/intel/vsec.c index e0096be605d9..938648b9ef09 100644 --- a/drivers/platform/x86/intel/vsec.c +++ b/drivers/platform/x86/intel/vsec.c @@ -158,18 +158,23 @@ static bool vsec_driver_present(int cap_id) */ static const struct pci_device_id intel_vsec_pci_ids[]; -static int intel_vsec_link_devices(struct pci_dev *pdev, struct device *dev, +static int intel_vsec_link_devices(struct device *parent, struct device *dev, int consumer_id) { const struct vsec_feature_dependency *deps; enum vsec_device_state *state; struct device **suppliers; struct vsec_priv *priv; + struct pci_dev *pdev; int supplier_id; if (!consumer_id) return 0; + if (!dev_is_pci(parent)) + return 0; + + pdev = to_pci_dev(parent); if (!pci_match_id(intel_vsec_pci_ids, pdev)) return 0; @@ -204,7 +209,7 @@ static int intel_vsec_link_devices(struct pci_dev *pdev, struct device *dev, return 0; } -int intel_vsec_add_aux(struct pci_dev *pdev, struct device *parent, +int intel_vsec_add_aux(struct device *parent, struct intel_vsec_device *intel_vsec_dev, const char *name) { @@ -252,7 +257,7 @@ int intel_vsec_add_aux(struct pci_dev *pdev, struct device *parent, if (ret) goto cleanup_aux; - ret = intel_vsec_link_devices(pdev, &auxdev->dev, intel_vsec_dev->cap_id); + ret = intel_vsec_link_devices(parent, &auxdev->dev, intel_vsec_dev->cap_id); if (ret) goto cleanup_aux; @@ -343,7 +348,7 @@ 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() */ - return intel_vsec_add_aux(pdev, parent, no_free_ptr(intel_vsec_dev), + return intel_vsec_add_aux(parent, no_free_ptr(intel_vsec_dev), intel_vsec_name(header->id)); } diff --git a/drivers/platform/x86/intel/vsec_tpmi.c b/drivers/platform/x86/intel/vsec_tpmi.c index 98846e88d3d0..e08af071fe87 100644 --- a/drivers/platform/x86/intel/vsec_tpmi.c +++ b/drivers/platform/x86/intel/vsec_tpmi.c @@ -655,7 +655,7 @@ static int tpmi_create_device(struct intel_tpmi_info *tpmi_info, * feature_vsec_dev and res memory are also freed as part of * device deletion. */ - return intel_vsec_add_aux(vsec_dev->pcidev, &vsec_dev->auxdev.dev, + return intel_vsec_add_aux(&vsec_dev->pcidev->dev, feature_vsec_dev, feature_id_name); } diff --git a/include/linux/intel_vsec.h b/include/linux/intel_vsec.h index d551174b0049..49a746ec0128 100644 --- a/include/linux/intel_vsec.h +++ b/include/linux/intel_vsec.h @@ -184,7 +184,7 @@ struct pmt_feature_group { struct telemetry_region regions[]; }; -int intel_vsec_add_aux(struct pci_dev *pdev, struct device *parent, +int intel_vsec_add_aux(struct device *parent, struct intel_vsec_device *intel_vsec_dev, const char *name); -- 2.43.0 ^ permalink raw reply related [flat|nested] 14+ messages in thread
* Claude review: platform/x86/intel/vsec: Decouple add/link helpers from PCI 2026-02-25 1:50 ` [PATCH v5 3/6] platform/x86/intel/vsec: Decouple add/link helpers from PCI David E. Box @ 2026-02-27 4:12 ` Claude Code Review Bot 0 siblings, 0 replies; 14+ messages in thread From: Claude Code Review Bot @ 2026-02-27 4:12 UTC (permalink / raw) To: dri-devel-reviews Patch Review **Overall: Good refactor, clean separation.** This removes the `struct pci_dev *pdev` parameter from `intel_vsec_add_aux()` and `intel_vsec_link_devices()`, replacing them with `struct device *parent`. The key safety addition is: ```c if (!dev_is_pci(parent)) return 0; ``` in `intel_vsec_link_devices()`, which correctly short-circuits the dependency-tracking logic (which relies on PCI ID matching) for non-PCI devices. The dependency/linking machinery is only relevant to the VSEC PCI driver's own probe path, so returning 0 for non-PCI parents is correct. The `vsec_tpmi.c` caller update from `vsec_dev->pcidev` to `&vsec_dev->pcidev->dev` is correct given that at this point in the series, `pcidev` still exists. No issues found. --- --- Generated by Claude Code Patch Reviewer ^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH v5 4/6] platform/x86/intel/vsec: Switch exported helpers from pci_dev to device 2026-02-25 1:50 [PATCH v5 0/6] platform/x86/intel/vsec: Prep for ACPI PMT discovery David E. Box ` (2 preceding siblings ...) 2026-02-25 1:50 ` [PATCH v5 3/6] platform/x86/intel/vsec: Decouple add/link helpers from PCI David E. Box @ 2026-02-25 1:50 ` David E. Box 2026-02-27 4:12 ` Claude review: " Claude Code Review Bot 2026-02-25 1:50 ` [PATCH v5 5/6] platform/x86/intel/vsec: Return real error codes from registration path David E. Box ` (2 subsequent siblings) 6 siblings, 1 reply; 14+ messages in thread From: David E. Box @ 2026-02-25 1:50 UTC (permalink / raw) To: thomas.hellstrom, rodrigo.vivi, irenic.rajneesh, ilpo.jarvinen, srinivas.pandruvada, intel-xe, dri-devel, xi.pardee Cc: david.e.box, hansg, linux-kernel, platform-driver-x86 Preparatory refactor for ACPI-enumerated PMT endpoints. Several exported PMT/VSEC interfaces and structs carried struct pci_dev * even though callers only need a generic struct device. Move those to struct device * so the same APIs work for PCI and ACPI parents. Acked-by: Rodrigo Vivi <rodrigo.vivi@intel.com> Signed-off-by: David E. Box <david.e.box@linux.intel.com> --- Changes in v5: - xe_vsec.h: Remove forward declaration for pci_dev and add for device Changes in v4: - No change Changes in v3: - No change Changes in v2: - Add forward declarations for struct device in class.h, telemetry.h - Restore struct pci_dev forward declaration in intel_vsec.h that was removed - Remove base_addr parameter changes (moved to separate patch) - Remove erroneous hunk in intel_vsec_pci_probe() (review comments by Ilpo Järvinen) drivers/gpu/drm/xe/xe_debugfs.c | 2 +- drivers/gpu/drm/xe/xe_hwmon.c | 2 +- drivers/gpu/drm/xe/xe_vsec.c | 7 ++-- drivers/gpu/drm/xe/xe_vsec.h | 4 +-- drivers/platform/x86/intel/pmc/core.c | 4 +-- drivers/platform/x86/intel/pmc/ssram_telemetry.c | 2 +- drivers/platform/x86/intel/pmt/class.c | 8 ++--- drivers/platform/x86/intel/pmt/class.h | 5 +-- drivers/platform/x86/intel/pmt/discovery.c | 4 +-- drivers/platform/x86/intel/pmt/telemetry.c | 13 +++---- drivers/platform/x86/intel/pmt/telemetry.h | 12 +++---- drivers/platform/x86/intel/sdsi.c | 5 +-- drivers/platform/x86/intel/vsec.c | 44 ++++++++++++++---------- drivers/platform/x86/intel/vsec_tpmi.c | 10 +++--- include/linux/intel_vsec.h | 13 +++---- 15 files changed, 73 insertions(+), 62 deletions(-) diff --git a/drivers/gpu/drm/xe/xe_debugfs.c b/drivers/gpu/drm/xe/xe_debugfs.c index 844cfafe1ec7..ad2d8f179eb6 100644 --- a/drivers/gpu/drm/xe/xe_debugfs.c +++ b/drivers/gpu/drm/xe/xe_debugfs.c @@ -45,7 +45,7 @@ static void read_residency_counter(struct xe_device *xe, struct xe_mmio *mmio, u64 residency = 0; int ret; - ret = xe_pmt_telem_read(to_pci_dev(xe->drm.dev), + ret = xe_pmt_telem_read(xe->drm.dev, xe_mmio_read32(mmio, PUNIT_TELEMETRY_GUID), &residency, offset, sizeof(residency)); if (ret != sizeof(residency)) { diff --git a/drivers/gpu/drm/xe/xe_hwmon.c b/drivers/gpu/drm/xe/xe_hwmon.c index 0fd4d4f1014a..92e423a339f1 100644 --- a/drivers/gpu/drm/xe/xe_hwmon.c +++ b/drivers/gpu/drm/xe/xe_hwmon.c @@ -506,7 +506,7 @@ xe_hwmon_energy_get(struct xe_hwmon *hwmon, int channel, long *energy) if (hwmon->xe->info.platform == XE_BATTLEMAGE) { u64 pmt_val; - ret = xe_pmt_telem_read(to_pci_dev(hwmon->xe->drm.dev), + ret = xe_pmt_telem_read(hwmon->xe->drm.dev, xe_mmio_read32(mmio, PUNIT_TELEMETRY_GUID), &pmt_val, BMG_ENERGY_STATUS_PMT_OFFSET, sizeof(pmt_val)); if (ret != sizeof(pmt_val)) { diff --git a/drivers/gpu/drm/xe/xe_vsec.c b/drivers/gpu/drm/xe/xe_vsec.c index 4ebb4dbe1c9b..a9baf0bfe572 100644 --- a/drivers/gpu/drm/xe/xe_vsec.c +++ b/drivers/gpu/drm/xe/xe_vsec.c @@ -140,10 +140,10 @@ static int xe_guid_decode(u32 guid, int *index, u32 *offset) return 0; } -int xe_pmt_telem_read(struct pci_dev *pdev, u32 guid, u64 *data, loff_t user_offset, +int xe_pmt_telem_read(struct device *dev, u32 guid, u64 *data, loff_t user_offset, u32 count) { - struct xe_device *xe = pdev_to_xe_device(pdev); + struct xe_device *xe = kdev_to_xe_device(dev); void __iomem *telem_addr = xe->mmio.regs + BMG_TELEMETRY_OFFSET; u32 mem_region; u32 offset; @@ -198,7 +198,6 @@ void xe_vsec_init(struct xe_device *xe) { struct intel_vsec_platform_info *info; struct device *dev = xe->drm.dev; - struct pci_dev *pdev = to_pci_dev(dev); enum xe_vsec platform; platform = get_platform_info(xe); @@ -221,6 +220,6 @@ void xe_vsec_init(struct xe_device *xe) * Register a VSEC. Cleanup is handled using device managed * resources. */ - intel_vsec_register(pdev, info); + intel_vsec_register(dev, info); } MODULE_IMPORT_NS("INTEL_VSEC"); diff --git a/drivers/gpu/drm/xe/xe_vsec.h b/drivers/gpu/drm/xe/xe_vsec.h index dabfb4e02d70..a25b4e6e681b 100644 --- a/drivers/gpu/drm/xe/xe_vsec.h +++ b/drivers/gpu/drm/xe/xe_vsec.h @@ -6,10 +6,10 @@ #include <linux/types.h> -struct pci_dev; +struct device; struct xe_device; void xe_vsec_init(struct xe_device *xe); -int xe_pmt_telem_read(struct pci_dev *pdev, u32 guid, u64 *data, loff_t user_offset, u32 count); +int xe_pmt_telem_read(struct device *dev, u32 guid, u64 *data, loff_t user_offset, u32 count); #endif diff --git a/drivers/platform/x86/intel/pmc/core.c b/drivers/platform/x86/intel/pmc/core.c index 02b303418d18..d91e1ab842d6 100644 --- a/drivers/platform/x86/intel/pmc/core.c +++ b/drivers/platform/x86/intel/pmc/core.c @@ -1315,7 +1315,7 @@ static struct telem_endpoint *pmc_core_register_endpoint(struct pci_dev *pcidev, unsigned int i; for (i = 0; guids[i]; i++) { - ep = pmt_telem_find_and_register_endpoint(pcidev, guids[i], 0); + ep = pmt_telem_find_and_register_endpoint(&pcidev->dev, guids[i], 0); if (!IS_ERR(ep)) return ep; } @@ -1600,7 +1600,7 @@ static int pmc_core_get_telem_info(struct pmc_dev *pmcdev, struct pmc_dev_info * if (!pmc->map->lpm_req_guid) return -ENXIO; - ep = pmt_telem_find_and_register_endpoint(pcidev, pmc->map->lpm_req_guid, 0); + ep = pmt_telem_find_and_register_endpoint(&pcidev->dev, pmc->map->lpm_req_guid, 0); if (IS_ERR(ep)) { dev_dbg(&pmcdev->pdev->dev, "couldn't get telem endpoint %pe", ep); return -EPROBE_DEFER; diff --git a/drivers/platform/x86/intel/pmc/ssram_telemetry.c b/drivers/platform/x86/intel/pmc/ssram_telemetry.c index 03fad9331fc0..6f6e83e70fc5 100644 --- a/drivers/platform/x86/intel/pmc/ssram_telemetry.c +++ b/drivers/platform/x86/intel/pmc/ssram_telemetry.c @@ -60,7 +60,7 @@ pmc_ssram_telemetry_add_pmt(struct pci_dev *pcidev, u64 ssram_base, void __iomem info.base_addr = ssram_base; info.parent = &pcidev->dev; - return intel_vsec_register(pcidev, &info); + return intel_vsec_register(&pcidev->dev, &info); } static inline u64 get_base(void __iomem *addr, u32 offset) diff --git a/drivers/platform/x86/intel/pmt/class.c b/drivers/platform/x86/intel/pmt/class.c index be3c8d9e4fff..b4c9964df807 100644 --- a/drivers/platform/x86/intel/pmt/class.c +++ b/drivers/platform/x86/intel/pmt/class.c @@ -60,11 +60,11 @@ pmt_memcpy64_fromio(void *to, const u64 __iomem *from, size_t count) return count; } -int pmt_telem_read_mmio(struct pci_dev *pdev, struct pmt_callbacks *cb, u32 guid, void *buf, +int pmt_telem_read_mmio(struct device *dev, struct pmt_callbacks *cb, u32 guid, void *buf, void __iomem *addr, loff_t off, u32 count) { if (cb && cb->read_telem) - return cb->read_telem(pdev, guid, buf, off, count); + return cb->read_telem(dev, guid, buf, off, count); addr += off; @@ -99,7 +99,7 @@ intel_pmt_read(struct file *filp, struct kobject *kobj, if (count > entry->size - off) count = entry->size - off; - count = pmt_telem_read_mmio(entry->pcidev, entry->cb, entry->header.guid, buf, + count = pmt_telem_read_mmio(entry->ep->dev, entry->cb, entry->header.guid, buf, entry->base, off, count); return count; @@ -208,7 +208,7 @@ static int intel_pmt_populate_entry(struct intel_pmt_entry *entry, struct intel_vsec_device *ivdev, struct resource *disc_res) { - struct pci_dev *pci_dev = ivdev->pcidev; + struct pci_dev *pci_dev = to_pci_dev(ivdev->dev); struct device *dev = &ivdev->auxdev.dev; struct intel_pmt_header *header = &entry->header; u8 bir; diff --git a/drivers/platform/x86/intel/pmt/class.h b/drivers/platform/x86/intel/pmt/class.h index 3c5ad5f52bca..1ae56a5baad2 100644 --- a/drivers/platform/x86/intel/pmt/class.h +++ b/drivers/platform/x86/intel/pmt/class.h @@ -19,11 +19,12 @@ #define GET_BIR(v) ((v) & GENMASK(2, 0)) #define GET_ADDRESS(v) ((v) & GENMASK(31, 3)) +struct device; struct pci_dev; extern struct class intel_pmt_class; struct telem_endpoint { - struct pci_dev *pcidev; + struct device *dev; struct telem_header header; struct pmt_callbacks *cb; void __iomem *base; @@ -65,7 +66,7 @@ struct intel_pmt_namespace { struct intel_pmt_entry *entry); }; -int pmt_telem_read_mmio(struct pci_dev *pdev, struct pmt_callbacks *cb, u32 guid, void *buf, +int pmt_telem_read_mmio(struct device *dev, struct pmt_callbacks *cb, u32 guid, void *buf, void __iomem *addr, loff_t off, u32 count); bool intel_pmt_is_early_client_hw(struct device *dev); int intel_pmt_dev_create(struct intel_pmt_entry *entry, diff --git a/drivers/platform/x86/intel/pmt/discovery.c b/drivers/platform/x86/intel/pmt/discovery.c index e500aa327d23..c482368bfaae 100644 --- a/drivers/platform/x86/intel/pmt/discovery.c +++ b/drivers/platform/x86/intel/pmt/discovery.c @@ -542,7 +542,7 @@ static int pmt_features_probe(struct auxiliary_device *auxdev, const struct auxi if (!priv) return -ENOMEM; - priv->parent = &ivdev->pcidev->dev; + priv->parent = ivdev->dev; auxiliary_set_drvdata(auxdev, priv); priv->dev = device_create(&intel_pmt_class, &auxdev->dev, MKDEV(0, 0), priv, @@ -609,7 +609,7 @@ void intel_pmt_get_features(struct intel_pmt_entry *entry) mutex_lock(&feature_list_lock); list_for_each_entry(feature, &pmt_feature_list, list) { - if (feature->priv->parent != &entry->ep->pcidev->dev) + if (feature->priv->parent != entry->ep->dev) continue; pmt_get_features(entry, feature); diff --git a/drivers/platform/x86/intel/pmt/telemetry.c b/drivers/platform/x86/intel/pmt/telemetry.c index a52803bfe124..bdc7c24a3678 100644 --- a/drivers/platform/x86/intel/pmt/telemetry.c +++ b/drivers/platform/x86/intel/pmt/telemetry.c @@ -112,7 +112,7 @@ static int pmt_telem_add_endpoint(struct intel_vsec_device *ivdev, return -ENOMEM; ep = entry->ep; - ep->pcidev = ivdev->pcidev; + ep->dev = ivdev->dev; ep->header.access_type = entry->header.access_type; ep->header.guid = entry->header.guid; ep->header.base_offset = entry->header.base_offset; @@ -204,7 +204,7 @@ int pmt_telem_get_endpoint_info(int devid, struct telem_endpoint_info *info) goto unlock; } - info->pdev = entry->ep->pcidev; + info->dev = entry->ep->dev; info->header = entry->ep->header; unlock: @@ -218,9 +218,10 @@ static int pmt_copy_region(struct telemetry_region *region, struct intel_pmt_entry *entry) { + struct pci_dev *pdev = to_pci_dev(entry->ep->dev); struct oobmsm_plat_info *plat_info; - plat_info = intel_vsec_get_mapping(entry->ep->pcidev); + plat_info = intel_vsec_get_mapping(pdev); if (IS_ERR(plat_info)) return PTR_ERR(plat_info); @@ -308,7 +309,7 @@ int pmt_telem_read(struct telem_endpoint *ep, u32 id, u64 *data, u32 count) if (offset + NUM_BYTES_QWORD(count) > size) return -EINVAL; - pmt_telem_read_mmio(ep->pcidev, ep->cb, ep->header.guid, data, ep->base, offset, + pmt_telem_read_mmio(ep->dev, ep->cb, ep->header.guid, data, ep->base, offset, NUM_BYTES_QWORD(count)); return ep->present ? 0 : -EPIPE; @@ -335,7 +336,7 @@ int pmt_telem_read32(struct telem_endpoint *ep, u32 id, u32 *data, u32 count) EXPORT_SYMBOL_NS_GPL(pmt_telem_read32, "INTEL_PMT_TELEMETRY"); struct telem_endpoint * -pmt_telem_find_and_register_endpoint(struct pci_dev *pcidev, u32 guid, u16 pos) +pmt_telem_find_and_register_endpoint(struct device *dev, u32 guid, u16 pos) { int devid = 0; int inst = 0; @@ -348,7 +349,7 @@ pmt_telem_find_and_register_endpoint(struct pci_dev *pcidev, u32 guid, u16 pos) if (err) return ERR_PTR(err); - if (ep_info.header.guid == guid && ep_info.pdev == pcidev) { + if (ep_info.header.guid == guid && ep_info.dev == dev) { if (inst == pos) return pmt_telem_register_endpoint(devid); ++inst; diff --git a/drivers/platform/x86/intel/pmt/telemetry.h b/drivers/platform/x86/intel/pmt/telemetry.h index d45af5512b4e..0f88c5e7d90e 100644 --- a/drivers/platform/x86/intel/pmt/telemetry.h +++ b/drivers/platform/x86/intel/pmt/telemetry.h @@ -6,8 +6,8 @@ #define PMT_TELEM_TELEMETRY 0 #define PMT_TELEM_CRASHLOG 1 +struct device; struct telem_endpoint; -struct pci_dev; struct telem_header { u8 access_type; @@ -17,7 +17,7 @@ struct telem_header { }; struct telem_endpoint_info { - struct pci_dev *pdev; + struct device *dev; struct telem_header header; }; @@ -71,8 +71,8 @@ int pmt_telem_get_endpoint_info(int devid, struct telem_endpoint_info *info); /** * pmt_telem_find_and_register_endpoint() - Get a telemetry endpoint from - * pci_dev device, guid and pos - * @pdev: PCI device inside the Intel vsec + * device, guid and pos + * @dev: device inside the Intel vsec * @guid: GUID of the telemetry space * @pos: Instance of the guid * @@ -80,8 +80,8 @@ int pmt_telem_get_endpoint_info(int devid, struct telem_endpoint_info *info); * * endpoint - On success returns pointer to the telemetry endpoint * * -ENXIO - telemetry endpoint not found */ -struct telem_endpoint *pmt_telem_find_and_register_endpoint(struct pci_dev *pcidev, - u32 guid, u16 pos); +struct telem_endpoint * +pmt_telem_find_and_register_endpoint(struct device *dev, u32 guid, u16 pos); /** * pmt_telem_read() - Read qwords from counter sram using sample id diff --git a/drivers/platform/x86/intel/sdsi.c b/drivers/platform/x86/intel/sdsi.c index da75f53d0bcc..d7e37d4ace23 100644 --- a/drivers/platform/x86/intel/sdsi.c +++ b/drivers/platform/x86/intel/sdsi.c @@ -599,13 +599,14 @@ static int sdsi_get_layout(struct sdsi_priv *priv, struct disc_table *table) return 0; } -static int sdsi_map_mbox_registers(struct sdsi_priv *priv, struct pci_dev *parent, +static int sdsi_map_mbox_registers(struct sdsi_priv *priv, struct device *dev, struct disc_table *disc_table, struct resource *disc_res) { u32 access_type = FIELD_GET(DT_ACCESS_TYPE, disc_table->access_info); u32 size = FIELD_GET(DT_SIZE, disc_table->access_info); u32 tbir = FIELD_GET(DT_TBIR, disc_table->offset); u32 offset = DT_OFFSET(disc_table->offset); + struct pci_dev *parent = to_pci_dev(dev); struct resource res = {}; /* Starting location of SDSi MMIO region based on access type */ @@ -681,7 +682,7 @@ static int sdsi_probe(struct auxiliary_device *auxdev, const struct auxiliary_de return ret; /* Map the SDSi mailbox registers */ - ret = sdsi_map_mbox_registers(priv, intel_cap_dev->pcidev, &disc_table, disc_res); + ret = sdsi_map_mbox_registers(priv, intel_cap_dev->dev, &disc_table, disc_res); if (ret) return ret; diff --git a/drivers/platform/x86/intel/vsec.c b/drivers/platform/x86/intel/vsec.c index 938648b9ef09..a547e4b98245 100644 --- a/drivers/platform/x86/intel/vsec.c +++ b/drivers/platform/x86/intel/vsec.c @@ -274,7 +274,7 @@ int intel_vsec_add_aux(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, +static int intel_vsec_add_dev(struct device *dev, struct intel_vsec_header *header, const struct intel_vsec_platform_info *info, unsigned long cap_id, u64 base_addr) { @@ -288,18 +288,18 @@ static int intel_vsec_add_dev(struct pci_dev *pdev, struct intel_vsec_header *he if (info->parent) parent = info->parent; else - parent = &pdev->dev; + parent = dev; if (!intel_vsec_supported(header->id, info->caps)) return -EINVAL; if (!header->num_entries) { - dev_dbg(&pdev->dev, "Invalid 0 entry count for header id %d\n", header->id); + dev_dbg(dev, "Invalid 0 entry count for header id %d\n", header->id); return -EINVAL; } if (!header->entry_size) { - dev_dbg(&pdev->dev, "Invalid 0 entry size for header id %d\n", header->id); + dev_dbg(dev, "Invalid 0 entry size for header id %d\n", header->id); return -EINVAL; } @@ -331,7 +331,7 @@ static int intel_vsec_add_dev(struct pci_dev *pdev, struct intel_vsec_header *he release_mem_region(tmp->start, resource_size(tmp)); } - intel_vsec_dev->pcidev = pdev; + intel_vsec_dev->dev = dev; intel_vsec_dev->resource = no_free_ptr(res); intel_vsec_dev->num_resources = header->num_entries; intel_vsec_dev->quirks = info->quirks; @@ -409,13 +409,14 @@ static int get_cap_id(u32 header_id, unsigned long *cap_id) return 0; } -static int intel_vsec_register_device(struct pci_dev *pdev, +static int intel_vsec_register_device(struct device *dev, struct intel_vsec_header *header, const struct intel_vsec_platform_info *info, u64 base_addr) { const struct vsec_feature_dependency *consumer_deps; struct vsec_priv *priv; + struct pci_dev *pdev; unsigned long cap_id; int ret; @@ -427,8 +428,12 @@ static int intel_vsec_register_device(struct pci_dev *pdev, * Only track dependencies for devices probed by the VSEC driver. * For others using the exported APIs, add the device directly. */ + if (!dev_is_pci(dev)) + return intel_vsec_add_dev(dev, header, info, cap_id, base_addr); + + pdev = to_pci_dev(dev); if (!pci_match_id(intel_vsec_pci_ids, pdev)) - return intel_vsec_add_dev(pdev, header, info, cap_id, base_addr); + return intel_vsec_add_dev(dev, header, info, cap_id, base_addr); priv = pci_get_drvdata(pdev); if (priv->state[cap_id] == STATE_REGISTERED || @@ -444,7 +449,7 @@ static int intel_vsec_register_device(struct pci_dev *pdev, 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, base_addr); + ret = intel_vsec_add_dev(dev, header, info, cap_id, base_addr); if (ret) priv->state[cap_id] = STATE_SKIP; else @@ -456,7 +461,7 @@ static int intel_vsec_register_device(struct pci_dev *pdev, return -EAGAIN; } -static bool intel_vsec_walk_header(struct pci_dev *pdev, +static bool intel_vsec_walk_header(struct device *dev, const struct intel_vsec_platform_info *info) { struct intel_vsec_header **header = info->headers; @@ -464,7 +469,7 @@ static bool intel_vsec_walk_header(struct pci_dev *pdev, int ret; for ( ; *header; header++) { - ret = intel_vsec_register_device(pdev, *header, info, info->base_addr); + ret = intel_vsec_register_device(dev, *header, info, info->base_addr); if (!ret) have_devices = true; } @@ -512,7 +517,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_register_device(pdev, &header, info, + ret = intel_vsec_register_device(&pdev->dev, &header, info, pci_resource_start(pdev, header.tbir)); if (ret) continue; @@ -558,7 +563,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_register_device(pdev, &header, info, + ret = intel_vsec_register_device(&pdev->dev, &header, info, pci_resource_start(pdev, header.tbir)); if (ret) continue; @@ -569,13 +574,13 @@ static bool intel_vsec_walk_vsec(struct pci_dev *pdev, return have_devices; } -int intel_vsec_register(struct pci_dev *pdev, +int intel_vsec_register(struct device *dev, const struct intel_vsec_platform_info *info) { - if (!pdev || !info || !info->headers) + if (!dev || !info || !info->headers) return -EINVAL; - if (!intel_vsec_walk_header(pdev, info)) + if (!intel_vsec_walk_header(dev, info)) return -ENODEV; else return 0; @@ -601,7 +606,7 @@ static bool intel_vsec_get_features(struct pci_dev *pdev, found = true; if (info && (info->quirks & VSEC_QUIRK_NO_DVSEC) && - intel_vsec_walk_header(pdev, info)) + intel_vsec_walk_header(&pdev->dev, info)) found = true; return found; @@ -673,7 +678,10 @@ int intel_vsec_set_mapping(struct oobmsm_plat_info *plat_info, { struct vsec_priv *priv; - priv = pci_get_drvdata(vsec_dev->pcidev); + if (!dev_is_pci(vsec_dev->dev)) + return -ENODEV; + + priv = pci_get_drvdata(to_pci_dev(vsec_dev->dev)); if (!priv) return -EINVAL; @@ -821,7 +829,7 @@ static pci_ers_result_t intel_vsec_pci_slot_reset(struct pci_dev *pdev) xa_for_each(&auxdev_array, index, intel_vsec_dev) { /* check if pdev doesn't match */ - if (pdev != intel_vsec_dev->pcidev) + if (&pdev->dev != intel_vsec_dev->dev) continue; devm_release_action(&pdev->dev, intel_vsec_remove_aux, &intel_vsec_dev->auxdev); diff --git a/drivers/platform/x86/intel/vsec_tpmi.c b/drivers/platform/x86/intel/vsec_tpmi.c index e08af071fe87..259addf664e8 100644 --- a/drivers/platform/x86/intel/vsec_tpmi.c +++ b/drivers/platform/x86/intel/vsec_tpmi.c @@ -530,7 +530,7 @@ static const struct file_operations mem_write_ops = { .release = single_release, }; -#define tpmi_to_dev(info) (&info->vsec_dev->pcidev->dev) +#define tpmi_to_dev(info) ((info)->vsec_dev->dev) static void tpmi_dbgfs_register(struct intel_tpmi_info *tpmi_info) { @@ -642,7 +642,7 @@ static int tpmi_create_device(struct intel_tpmi_info *tpmi_info, tmp->flags = IORESOURCE_MEM; } - feature_vsec_dev->pcidev = vsec_dev->pcidev; + feature_vsec_dev->dev = vsec_dev->dev; feature_vsec_dev->resource = res; feature_vsec_dev->num_resources = pfs->pfs_header.num_entries; feature_vsec_dev->priv_data = &tpmi_info->plat_info; @@ -655,8 +655,8 @@ static int tpmi_create_device(struct intel_tpmi_info *tpmi_info, * feature_vsec_dev and res memory are also freed as part of * device deletion. */ - return intel_vsec_add_aux(&vsec_dev->pcidev->dev, - feature_vsec_dev, feature_id_name); + return intel_vsec_add_aux(vsec_dev->dev, feature_vsec_dev, + feature_id_name); } static int tpmi_create_devices(struct intel_tpmi_info *tpmi_info) @@ -742,7 +742,7 @@ static int tpmi_fetch_pfs_header(struct intel_tpmi_pm_feature *pfs, u64 start, i static int intel_vsec_tpmi_init(struct auxiliary_device *auxdev) { struct intel_vsec_device *vsec_dev = auxdev_to_ivdev(auxdev); - struct pci_dev *pci_dev = vsec_dev->pcidev; + struct pci_dev *pci_dev = to_pci_dev(vsec_dev->dev); struct intel_tpmi_info *tpmi_info; u64 pfs_start = 0; int ret, i; diff --git a/include/linux/intel_vsec.h b/include/linux/intel_vsec.h index 49a746ec0128..4eecb2a6bac4 100644 --- a/include/linux/intel_vsec.h +++ b/include/linux/intel_vsec.h @@ -29,6 +29,7 @@ #define INTEL_DVSEC_TABLE_OFFSET(x) ((x) & GENMASK(31, 3)) #define TABLE_OFFSET_SHIFT 3 +struct device; struct pci_dev; struct resource; @@ -82,14 +83,14 @@ enum intel_vsec_quirks { * struct pmt_callbacks - Callback infrastructure for PMT devices * @read_telem: when specified, called by client driver to access PMT * data (instead of direct copy). - * * pdev: PCI device reference for the callback's use + * * dev: device reference for the callback's use * * guid: ID of data to acccss * * data: buffer for the data to be copied * * off: offset into the requested buffer * * count: size of buffer */ struct pmt_callbacks { - int (*read_telem)(struct pci_dev *pdev, u32 guid, u64 *data, loff_t off, u32 count); + int (*read_telem)(struct device *dev, u32 guid, u64 *data, loff_t off, u32 count); }; struct vsec_feature_dependency { @@ -122,7 +123,7 @@ struct intel_vsec_platform_info { /** * struct intel_vsec_device - Auxbus specific device information * @auxdev: auxbus device struct for auxbus access - * @pcidev: pci device associated with the device + * @dev: struct device associated with the device * @resource: any resources shared by the parent * @ida: id reference * @num_resources: number of resources @@ -135,7 +136,7 @@ struct intel_vsec_platform_info { */ struct intel_vsec_device { struct auxiliary_device auxdev; - struct pci_dev *pcidev; + struct device *dev; struct resource *resource; struct ida *ida; int num_resources; @@ -199,13 +200,13 @@ static inline struct intel_vsec_device *auxdev_to_ivdev(struct auxiliary_device } #if IS_ENABLED(CONFIG_INTEL_VSEC) -int intel_vsec_register(struct pci_dev *pdev, +int intel_vsec_register(struct device *dev, const struct intel_vsec_platform_info *info); int intel_vsec_set_mapping(struct oobmsm_plat_info *plat_info, struct intel_vsec_device *vsec_dev); struct oobmsm_plat_info *intel_vsec_get_mapping(struct pci_dev *pdev); #else -static inline int intel_vsec_register(struct pci_dev *pdev, +static inline int intel_vsec_register(struct device *dev, const struct intel_vsec_platform_info *info) { return -ENODEV; -- 2.43.0 ^ permalink raw reply related [flat|nested] 14+ messages in thread
* Claude review: platform/x86/intel/vsec: Switch exported helpers from pci_dev to device 2026-02-25 1:50 ` [PATCH v5 4/6] platform/x86/intel/vsec: Switch exported helpers from pci_dev to device David E. Box @ 2026-02-27 4:12 ` Claude Code Review Bot 0 siblings, 0 replies; 14+ messages in thread From: Claude Code Review Bot @ 2026-02-27 4:12 UTC (permalink / raw) To: dri-devel-reviews Patch Review **Overall: The largest patch, mostly mechanical. Has a potential safety concern.** This is the core conversion: `struct intel_vsec_device` gains `struct device *dev` replacing `struct pci_dev *pcidev`. All consumers are updated. **Concern — unchecked `to_pci_dev()` in downstream code after ACPI support is added:** Several downstream consumers now do `to_pci_dev(ivdev->dev)` or `to_pci_dev(entry->ep->dev)` without first checking `dev_is_pci()`. While this is safe *today* because only PCI parents exist, patch 6 adds ACPI discovery support, and future ACPI-enumerated PMT endpoints could reach this code. Specifically: 1. In `class.c`, `intel_pmt_populate_entry()`: ```c struct pci_dev *pci_dev = to_pci_dev(ivdev->dev); ``` This function uses `pci_resource_start()` which will crash on a non-PCI device. 2. In `telemetry.c`, `pmt_copy_region()`: ```c struct pci_dev *pdev = to_pci_dev(entry->ep->dev); ``` 3. In `sdsi.c`, `sdsi_map_mbox_registers()`: ```c struct pci_dev *parent = to_pci_dev(dev); ``` 4. In `vsec_tpmi.c`, `intel_vsec_tpmi_init()`: ```c struct pci_dev *pci_dev = to_pci_dev(vsec_dev->dev); ``` While the commit message says "same APIs work for PCI and ACPI parents", these unconverted sites will crash with non-PCI devices. It would be prudent to add `dev_is_pci()` guards or at minimum a comment at each `to_pci_dev()` site asserting that only PCI devices reach that path. The `intel_vsec_set_mapping()` function correctly adds such a guard: ```c if (!dev_is_pci(vsec_dev->dev)) return -ENODEV; ``` This pattern should be applied more consistently. **Minor: `xe_pmt_telem_read` uses `kdev_to_xe_device()`** — this needs `kdev_to_xe_device()` to exist in the xe driver. It does (it's a standard xe helper), so this is fine. --- --- Generated by Claude Code Patch Reviewer ^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH v5 5/6] platform/x86/intel/vsec: Return real error codes from registration path 2026-02-25 1:50 [PATCH v5 0/6] platform/x86/intel/vsec: Prep for ACPI PMT discovery David E. Box ` (3 preceding siblings ...) 2026-02-25 1:50 ` [PATCH v5 4/6] platform/x86/intel/vsec: Switch exported helpers from pci_dev to device David E. Box @ 2026-02-25 1:50 ` David E. Box 2026-02-27 4:12 ` Claude review: " Claude Code Review Bot 2026-02-25 1:50 ` [PATCH v5 6/6] platform/x86/intel/vsec: Plumb ACPI PMT discovery tables through vsec David E. Box 2026-02-27 4:12 ` Claude review: platform/x86/intel/vsec: Prep for ACPI PMT discovery Claude Code Review Bot 6 siblings, 1 reply; 14+ messages in thread From: David E. Box @ 2026-02-25 1:50 UTC (permalink / raw) To: thomas.hellstrom, rodrigo.vivi, irenic.rajneesh, ilpo.jarvinen, srinivas.pandruvada, intel-xe, dri-devel, xi.pardee Cc: david.e.box, hansg, linux-kernel, platform-driver-x86 Stop collapsing registration results into booleans. Make intel_vsec_walk_header() return int and propagate the first non-zero error from intel_vsec_register_device(). intel_vsec_register() now returns that error directly and 0 on success. This preserves success behavior while surfacing meaningful errors instead of hiding them behind a bool/-ENODEV, which makes debugging and probe ordering issues clearer. Reviewed-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com> Signed-off-by: David E. Box <david.e.box@linux.intel.com> --- Changes in v5: - No change Changes in v4: - No change Changes in v3: - No change Changes in v2: - No chnage (previous patch 3) drivers/platform/x86/intel/vsec.c | 16 ++++++---------- 1 file changed, 6 insertions(+), 10 deletions(-) diff --git a/drivers/platform/x86/intel/vsec.c b/drivers/platform/x86/intel/vsec.c index a547e4b98245..34b2c19ecff0 100644 --- a/drivers/platform/x86/intel/vsec.c +++ b/drivers/platform/x86/intel/vsec.c @@ -461,20 +461,19 @@ static int intel_vsec_register_device(struct device *dev, return -EAGAIN; } -static bool intel_vsec_walk_header(struct device *dev, - const struct intel_vsec_platform_info *info) +static int intel_vsec_walk_header(struct device *dev, + const struct intel_vsec_platform_info *info) { struct intel_vsec_header **header = info->headers; - bool have_devices = false; int ret; for ( ; *header; header++) { ret = intel_vsec_register_device(dev, *header, info, info->base_addr); - if (!ret) - have_devices = true; + if (ret) + return ret; } - return have_devices; + return 0; } static bool intel_vsec_walk_dvsec(struct pci_dev *pdev, @@ -580,10 +579,7 @@ int intel_vsec_register(struct device *dev, if (!dev || !info || !info->headers) return -EINVAL; - if (!intel_vsec_walk_header(dev, info)) - return -ENODEV; - else - return 0; + return intel_vsec_walk_header(dev, info); } EXPORT_SYMBOL_NS_GPL(intel_vsec_register, "INTEL_VSEC"); -- 2.43.0 ^ permalink raw reply related [flat|nested] 14+ messages in thread
* Claude review: platform/x86/intel/vsec: Return real error codes from registration path 2026-02-25 1:50 ` [PATCH v5 5/6] platform/x86/intel/vsec: Return real error codes from registration path David E. Box @ 2026-02-27 4:12 ` Claude Code Review Bot 0 siblings, 0 replies; 14+ messages in thread From: Claude Code Review Bot @ 2026-02-27 4:12 UTC (permalink / raw) To: dri-devel-reviews Patch Review **Overall: Good intent but introduces a semantic change in error behavior.** The patch converts `intel_vsec_walk_header()` from returning `bool` (any device registered successfully) to `int` (first error encountered). This is a meaningful behavioral change: ```c for ( ; *header; header++) { ret = intel_vsec_register_device(dev, *header, info, info->base_addr); - if (!ret) - have_devices = true; + if (ret) + return ret; } -return have_devices; +return 0; ``` **The old behavior**: iterate all headers, register as many devices as possible, return success if *any* device was registered. Individual failures were silently ignored. **The new behavior**: return the *first* error encountered and *stop processing remaining headers*. This is a significant semantic change that the commit message describes as "preserves success behavior" but it doesn't fully: - If the first header fails but the second would succeed, the old code would register the second device and return success. The new code fails immediately. - For cases where `intel_vsec_register_device()` returns `-EAGAIN` (deferred dependency), the old code would continue and register other devices. The new code aborts. Consider whether this short-circuit on first error is intentional. If only partial registration is acceptable, the old approach of continuing was more robust. If the intent is truly "all-or-nothing", the already-registered devices from earlier iterations should be cleaned up (which isn't done here). **Suggestion**: Either restore the iterate-all-and-track-first-error approach (return first error only if *no* devices succeeded), or document that all-or-nothing semantics is the new intended behavior. The `-EAGAIN` case seems particularly problematic with the new short-circuit. --- --- Generated by Claude Code Patch Reviewer ^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH v5 6/6] platform/x86/intel/vsec: Plumb ACPI PMT discovery tables through vsec 2026-02-25 1:50 [PATCH v5 0/6] platform/x86/intel/vsec: Prep for ACPI PMT discovery David E. Box ` (4 preceding siblings ...) 2026-02-25 1:50 ` [PATCH v5 5/6] platform/x86/intel/vsec: Return real error codes from registration path David E. Box @ 2026-02-25 1:50 ` David E. Box 2026-02-27 4:12 ` Claude review: " Claude Code Review Bot 2026-02-27 4:12 ` Claude review: platform/x86/intel/vsec: Prep for ACPI PMT discovery Claude Code Review Bot 6 siblings, 1 reply; 14+ messages in thread From: David E. Box @ 2026-02-25 1:50 UTC (permalink / raw) To: thomas.hellstrom, rodrigo.vivi, irenic.rajneesh, ilpo.jarvinen, srinivas.pandruvada, intel-xe, dri-devel, xi.pardee Cc: david.e.box, hansg, linux-kernel, platform-driver-x86 Some platforms expose PMT discovery via ACPI instead of PCI BARs. Add a generic discovery source flag and carry ACPI discovery entries alongside the existing PCI resource path so PMT clients can consume either. Changes: - Add enum intel_vsec_disc_source { _PCI, _ACPI }. - Extend intel_vsec_platform_info and intel_vsec_device with source enum and ACPI discovery table pointer/ - When src==ACPI, skip BAR resource setup and copy the ACPI discovery entries into the aux device. No user-visible behavior change yet; this only wires ACPI data through vsec in preparation for ACPI-enumerated PMT clients. Signed-off-by: David E. Box <david.e.box@linux.intel.com> --- Changes in v5: - No change Changes in v4: - Use check_mul_overflow() instead of array_size() which was incorrectly checking for 0 anyway. Changes in v3: - Re-send with all changes intended for v2 which was sent without them being applied. Changes in v2: - Improve comment to clarify BAR resource setup doesn't apply to ACPI discovery - Add missing #include for kmemdup() - Use array_size() for overflow protection (review comments by Ilpo Järvinen) drivers/platform/x86/intel/vsec.c | 23 +++++++++++++++++++++++ include/linux/intel_vsec.h | 20 +++++++++++++++++++- 2 files changed, 42 insertions(+), 1 deletion(-) diff --git a/drivers/platform/x86/intel/vsec.c b/drivers/platform/x86/intel/vsec.c index 34b2c19ecff0..7907505a365c 100644 --- a/drivers/platform/x86/intel/vsec.c +++ b/drivers/platform/x86/intel/vsec.c @@ -24,7 +24,9 @@ #include <linux/intel_vsec.h> #include <linux/kernel.h> #include <linux/module.h> +#include <linux/overflow.h> #include <linux/pci.h> +#include <linux/string.h> #include <linux/types.h> #define PMT_XA_START 0 @@ -109,6 +111,7 @@ static void intel_vsec_dev_release(struct device *dev) ida_free(intel_vsec_dev->ida, intel_vsec_dev->auxdev.id); + kfree(intel_vsec_dev->acpi_disc); kfree(intel_vsec_dev->resource); kfree(intel_vsec_dev); } @@ -320,6 +323,13 @@ static int intel_vsec_add_dev(struct device *dev, struct intel_vsec_header *head * auxiliary device driver. */ for (i = 0, tmp = res; i < header->num_entries; i++, tmp++) { + /* + * Skip resource mapping check for ACPI-based discovery + * since those tables are read from _DSD, not MMIO. + */ + if (info->src == INTEL_VSEC_DISC_ACPI) + break; + tmp->start = base_addr + header->offset + i * (header->entry_size * sizeof(u32)); tmp->end = tmp->start + (header->entry_size * sizeof(u32)) - 1; tmp->flags = IORESOURCE_MEM; @@ -338,6 +348,19 @@ static int intel_vsec_add_dev(struct device *dev, struct intel_vsec_header *head intel_vsec_dev->base_addr = info->base_addr; intel_vsec_dev->priv_data = info->priv_data; intel_vsec_dev->cap_id = cap_id; + intel_vsec_dev->src = info->src; + + if (info->src == INTEL_VSEC_DISC_ACPI) { + size_t bytes; + + if (check_mul_overflow(intel_vsec_dev->num_resources, + sizeof(*info->acpi_disc), &bytes)) + return -EOVERFLOW; + + intel_vsec_dev->acpi_disc = kmemdup(info->acpi_disc, bytes, GFP_KERNEL); + if (!intel_vsec_dev->acpi_disc) + return -ENOMEM; + } if (header->id == VSEC_ID_SDSI) intel_vsec_dev->ida = &intel_vsec_sdsi_ida; diff --git a/include/linux/intel_vsec.h b/include/linux/intel_vsec.h index 4eecb2a6bac4..1fe5665a9d02 100644 --- a/include/linux/intel_vsec.h +++ b/include/linux/intel_vsec.h @@ -33,6 +33,11 @@ struct device; struct pci_dev; struct resource; +enum intel_vsec_disc_source { + INTEL_VSEC_DISC_PCI, /* PCI, default */ + INTEL_VSEC_DISC_ACPI, /* ACPI */ +}; + enum intel_vsec_id { VSEC_ID_TELEMETRY = 2, VSEC_ID_WATCHER = 3, @@ -103,6 +108,10 @@ struct vsec_feature_dependency { * @parent: parent device in the auxbus chain * @headers: list of headers to define the PMT client devices to create * @deps: array of feature dependencies + * @acpi_disc: ACPI discovery tables, each entry is two QWORDs + * in little-endian format as defined by the PMT ACPI spec. + * Valid only when @provider == INTEL_VSEC_DISC_ACPI. + * @src: source of discovery table data * @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 @@ -113,6 +122,8 @@ struct intel_vsec_platform_info { struct device *parent; struct intel_vsec_header **headers; const struct vsec_feature_dependency *deps; + u32 (*acpi_disc)[4]; + enum intel_vsec_disc_source src; void *priv_data; unsigned long caps; unsigned long quirks; @@ -124,7 +135,12 @@ struct intel_vsec_platform_info { * struct intel_vsec_device - Auxbus specific device information * @auxdev: auxbus device struct for auxbus access * @dev: struct device associated with the device - * @resource: any resources shared by the parent + * @resource: PCI discovery resources (BAR windows), one per discovery + * instance. Valid only when @src == INTEL_VSEC_DISC_PCI + * @acpi_disc: ACPI discovery tables, each entry is two QWORDs + * in little-endian format as defined by the PMT ACPI spec. + * Valid only when @src == INTEL_VSEC_DISC_ACPI. + * @src: source of discovery table data * @ida: id reference * @num_resources: number of resources * @id: xarray id @@ -138,6 +154,8 @@ struct intel_vsec_device { struct auxiliary_device auxdev; struct device *dev; struct resource *resource; + u32 (*acpi_disc)[4]; + enum intel_vsec_disc_source src; struct ida *ida; int num_resources; int id; /* xa */ -- 2.43.0 ^ permalink raw reply related [flat|nested] 14+ messages in thread
* Claude review: platform/x86/intel/vsec: Plumb ACPI PMT discovery tables through vsec 2026-02-25 1:50 ` [PATCH v5 6/6] platform/x86/intel/vsec: Plumb ACPI PMT discovery tables through vsec David E. Box @ 2026-02-27 4:12 ` Claude Code Review Bot 0 siblings, 0 replies; 14+ messages in thread From: Claude Code Review Bot @ 2026-02-27 4:12 UTC (permalink / raw) To: dri-devel-reviews Patch Review **Overall: Core feature patch. Has a resource leak issue and a minor doc typo.** This adds the ACPI discovery infrastructure: `enum intel_vsec_disc_source`, `acpi_disc` arrays in both `intel_vsec_platform_info` and `intel_vsec_device`, and the `src` field. **Issue 1 — Potential resource leak when `kmemdup` succeeds but device registration fails:** In `intel_vsec_add_dev()`: ```c intel_vsec_dev->acpi_disc = kmemdup(info->acpi_disc, bytes, GFP_KERNEL); if (!intel_vsec_dev->acpi_disc) return -ENOMEM; ``` The `intel_vsec_dev` is declared with `__free(kfree)` cleanup, but `acpi_disc` is only freed in `intel_vsec_dev_release()` (the device release callback). If `intel_vsec_add_aux()` fails later in this function *before* the device is fully registered (before `device_add` succeeds), the `__free(kfree)` cleanup will free `intel_vsec_dev` but not `acpi_disc`, since `intel_vsec_dev_release` won't be called. This mirrors the existing pattern for `resource` though — `resource` has the same issue with `no_free_ptr(res)` — so this may be acceptable if the existing pattern is considered safe. But it's worth verifying. Actually, looking more carefully: `intel_vsec_dev` uses `__free(kfree)` and is later passed via `no_free_ptr()` to `intel_vsec_add_aux()`. If `no_free_ptr()` has already been called, ownership has been transferred. But the `acpi_disc` allocation happens *before* `no_free_ptr()`, so if the function returns early between the `kmemdup` and `no_free_ptr`, `acpi_disc` will leak since `kfree(intel_vsec_dev)` doesn't also free `intel_vsec_dev->acpi_disc`. **Issue 2 — `check_mul_overflow` alignment:** ```c if (check_mul_overflow(intel_vsec_dev->num_resources, sizeof(*info->acpi_disc), &bytes)) ``` The `sizeof(*info->acpi_disc)` evaluates to `sizeof(u32[4])` = 16 bytes, and `num_resources` is `int`. The `check_mul_overflow` macro should handle the type promotion correctly, but `bytes` is `size_t` while `num_resources` is `int` — if `num_resources` were somehow negative, this could behave unexpectedly. It won't happen in practice since `num_resources` is validated earlier, but using `(size_t)intel_vsec_dev->num_resources` would be more defensive. **Issue 3 — Comment typo in the header:** ```c * @acpi_disc: ACPI discovery tables, each entry is two QWORDs * in little-endian format as defined by the PMT ACPI spec. * Valid only when @provider == INTEL_VSEC_DISC_ACPI. ``` `@provider` should be `@src` to match the actual field name. **Issue 4 — Breaking the resource loop early:** ```c for (i = 0, tmp = res; i < header->num_entries; i++, tmp++) { if (info->src == INTEL_VSEC_DISC_ACPI) break; ``` This `break` on ACPI effectively leaves `res` allocated but empty (all zeroed). The `num_resources` is still set to `header->num_entries`, so downstream code might expect valid resources. It might be cleaner to skip the allocation and loop entirely for ACPI, or set `num_resources` to 0 for ACPI devices. --- Generated by Claude Code Patch Reviewer ^ permalink raw reply [flat|nested] 14+ messages in thread
* Claude review: platform/x86/intel/vsec: Prep for ACPI PMT discovery 2026-02-25 1:50 [PATCH v5 0/6] platform/x86/intel/vsec: Prep for ACPI PMT discovery David E. Box ` (5 preceding siblings ...) 2026-02-25 1:50 ` [PATCH v5 6/6] platform/x86/intel/vsec: Plumb ACPI PMT discovery tables through vsec David E. Box @ 2026-02-27 4:12 ` Claude Code Review Bot 6 siblings, 0 replies; 14+ messages in thread From: Claude Code Review Bot @ 2026-02-27 4:12 UTC (permalink / raw) To: dri-devel-reviews Overall Series Review Subject: platform/x86/intel/vsec: Prep for ACPI PMT discovery Author: "David E. Box" <david.e.box@linux.intel.com> Patches: 7 Reviewed: 2026-02-27T14:12:35.971128 --- This is a well-structured 6-patch series by David E. Box that prepares the Intel VSEC subsystem for ACPI-based PMT discovery. The series progressively decouples the vsec layer from PCI-specific types, improves const-correctness and error handling, and finally adds ACPI discovery table plumbing. The patch ordering is logical and each patch has a clear purpose. The series is mostly clean with good incremental refactoring. However, there are a few issues worth addressing, particularly around safety in patch 4's PCI-to-device conversion (unchecked `to_pci_dev()` calls in downstream consumers), a semantic change in error handling in patch 5, and a potential resource leak in patch 6. --- --- Generated by Claude Code Patch Reviewer ^ permalink raw reply [flat|nested] 14+ messages in thread
end of thread, other threads:[~2026-02-27 4:12 UTC | newest] Thread overview: 14+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2026-02-25 1:50 [PATCH v5 0/6] platform/x86/intel/vsec: Prep for ACPI PMT discovery David E. Box 2026-02-25 1:50 ` [PATCH v5 1/6] platform/x86/intel/vsec: Refactor base_addr handling David E. Box 2026-02-27 4:12 ` Claude review: " Claude Code Review Bot 2026-02-25 1:50 ` [PATCH v5 2/6] platform/x86/intel/vsec: Make driver_data info const David E. Box 2026-02-27 4:12 ` Claude review: " Claude Code Review Bot 2026-02-25 1:50 ` [PATCH v5 3/6] platform/x86/intel/vsec: Decouple add/link helpers from PCI David E. Box 2026-02-27 4:12 ` Claude review: " Claude Code Review Bot 2026-02-25 1:50 ` [PATCH v5 4/6] platform/x86/intel/vsec: Switch exported helpers from pci_dev to device David E. Box 2026-02-27 4:12 ` Claude review: " Claude Code Review Bot 2026-02-25 1:50 ` [PATCH v5 5/6] platform/x86/intel/vsec: Return real error codes from registration path David E. Box 2026-02-27 4:12 ` Claude review: " Claude Code Review Bot 2026-02-25 1:50 ` [PATCH v5 6/6] platform/x86/intel/vsec: Plumb ACPI PMT discovery tables through vsec David E. Box 2026-02-27 4:12 ` Claude review: " Claude Code Review Bot 2026-02-27 4:12 ` Claude review: platform/x86/intel/vsec: Prep for ACPI PMT discovery Claude Code Review Bot
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox