On Thu, 12 Mar 2026, David E. Box wrote:
> Allow the PMT class to read discovery headers from either PCI MMIO or
> ACPI-provided entries, depending on the discovery source. The new
> source-aware fetch helper retrieves the first two QWORDs for both paths
> while keeping the mapped discovery table available for users such as
> crashlog.
>
> Split intel_pmt_populate_entry() into source-specific resolvers:
> - pmt_resolve_access_pci(): handles both ACCESS_LOCAL and ACCESS_BARID
> for PCI-backed devices and sets entry->pcidev. Same existing
> functionality.
> - pmt_resolve_access_acpi(): handles only ACCESS_BARID for ACPI-backed
> devices, rejecting ACCESS_LOCAL which has no valid semantics without
> a physical discovery resource.
>
> This maintains existing PCI behavior and makes no functional changes
> for PCI devices.
>
> Signed-off-by: David E. Box <[email protected]>
> ---
> drivers/platform/x86/intel/pmt/class.c | 124 +++++++++++++++++++++++--
> 1 file changed, 115 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/platform/x86/intel/pmt/class.c
> b/drivers/platform/x86/intel/pmt/class.c
> index f94f51178043..ddd9c4bf7323 100644
> --- a/drivers/platform/x86/intel/pmt/class.c
> +++ b/drivers/platform/x86/intel/pmt/class.c
> @@ -205,9 +205,9 @@ struct class intel_pmt_class = {
> };
> EXPORT_SYMBOL_GPL(intel_pmt_class);
>
> -static int intel_pmt_populate_entry(struct intel_pmt_entry *entry,
> - struct intel_vsec_device *ivdev,
> - int idx)
> +static int pmt_resolve_access_pci(struct intel_pmt_entry *entry,
> + struct intel_vsec_device *ivdev,
> + int idx)
> {
> struct pci_dev *pci_dev = to_pci_dev(ivdev->dev);
> struct device *dev = &ivdev->auxdev.dev;
> @@ -287,6 +287,82 @@ static int intel_pmt_populate_entry(struct
> intel_pmt_entry *entry,
> }
>
> entry->pcidev = pci_dev;
> +
> + return 0;
> +}
> +
> +static int pmt_resolve_access_acpi(struct intel_pmt_entry *entry,
> + struct intel_vsec_device *ivdev)
> +{
> + struct pci_dev *pci_dev = NULL;
> + struct device *dev = &ivdev->auxdev.dev;
> + struct intel_pmt_header *header = &entry->header;
> + u8 bir;
> +
> + if (dev_is_pci(ivdev->dev))
> + pci_dev = to_pci_dev(ivdev->dev);
> +
> + /*
> + * The base offset should always be 8 byte aligned.
> + *
> + * For non-local access types the lower 3 bits of base offset
> + * contains the index of the base address register where the
> + * telemetry can be found.
> + */
> + bir = GET_BIR(header->base_offset);
> +
> + switch (header->access_type) {
> + case ACCESS_BARID:
> + /* ACPI platform drivers use base_addr */
> + if (ivdev->base_addr) {
> + entry->base_addr = ivdev->base_addr +
> + GET_ADDRESS(header->base_offset);
> + break;
> + }
> +
> + /* If base_addr is not provided, then this is an ACPI companion
> device */
> + if (!pci_dev) {
> + dev_err(dev,
> + "ACCESS_BARID requires PCI BAR resources or
> base_addr\n");
IMO the extra line is not very useful here.
> + return -EINVAL;
> + }
> +
> + entry->base_addr = pci_resource_start(pci_dev, bir) +
> + GET_ADDRESS(header->base_offset);
> + break;
> + default:
> + dev_err(dev, "Unsupported access type %d for ACPI based PMT\n",
> + header->access_type);
> + return -EINVAL;
> + }
> +
> + return 0;
> +}
> +
> +static int intel_pmt_populate_entry(struct intel_pmt_entry *entry,
> + struct intel_vsec_device *ivdev,
> + int idx)
> +{
> + struct intel_pmt_header *header = &entry->header;
> + struct device *dev = &ivdev->auxdev.dev;
> + int ret;
> +
> + switch (ivdev->src) {
> + case INTEL_VSEC_DISC_PCI:
> + ret = pmt_resolve_access_pci(entry, ivdev, idx);
> + break;
> + case INTEL_VSEC_DISC_ACPI:
> + ret = pmt_resolve_access_acpi(entry, ivdev);
> + break;
> + default:
> + dev_err(dev, "Unknown discovery source: %d\n", ivdev->src);
> + ret = -EINVAL;
> + break;
> + }
> +
> + if (ret)
> + return ret;
Instead of deferring to after the switch/case, I'd prefer you handle the
errors within the cases as it, while longer by a few lines, makes
following the logic easier.
> +
> entry->guid = header->guid;
> entry->size = header->size;
> entry->cb = ivdev->priv_data;
> @@ -371,18 +447,48 @@ static int intel_pmt_dev_register(struct
> intel_pmt_entry *entry,
> return ret;
> }
>
> +static int pmt_get_headers(struct intel_vsec_device *ivdev, int idx,
> + struct intel_pmt_entry *entry, u64 headers[2])
> +{
> + struct device *dev = &ivdev->auxdev.dev;
> +
> + switch (ivdev->src) {
> + case INTEL_VSEC_DISC_PCI: {
> + void __iomem *disc_table;
> +
> + disc_table = devm_ioremap_resource(dev, &ivdev->resource[idx]);
> + if (IS_ERR(disc_table))
> + return PTR_ERR(disc_table);
> +
> + memcpy_fromio(headers, disc_table, 2 * sizeof(u64));
> +
> + /* Used by crashlog driver */
> + entry->disc_table = disc_table;
> +
> + return 0;
> + }
> + case INTEL_VSEC_DISC_ACPI:
> + memcpy(headers, &ivdev->acpi_disc[idx][0], 2 * sizeof(u64));
> +
> + return 0;
> + default:
> + dev_err(dev, "Unknown discovery source type: %d\n", ivdev->src);
> + break;
> + }
> +
> + return -EINVAL;
> +}
> +
> static int pmt_read_header(struct intel_vsec_device *ivdev, int idx,
> struct intel_pmt_entry *entry)
> {
> struct intel_pmt_header *header = &entry->header;
> - struct device *dev = &ivdev->auxdev.dev;
> u64 headers[2];
> + int ret;
>
> - entry->disc_table = devm_ioremap_resource(dev, &ivdev->resource[idx]);
> - if (IS_ERR(entry->disc_table))
> - return PTR_ERR(entry->disc_table);
> -
> - memcpy_fromio(headers, entry->disc_table, 2 * sizeof(u64));
> + ret = pmt_get_headers(ivdev, idx, entry, headers);
> + if (ret)
> + return ret;
>
> header->access_type = FIELD_GET(PMT_ACCESS_TYPE, headers[0]);
> header->telem_type = FIELD_GET(PMT_TELEM_TYPE, headers[0]);
>
--
i.
