Re: [PATCH v5 05/16] platform/x86/intel/pmt: Cache the telemetry discovery header

["Ilpo Järvinen" <ilpo.jarvinen@linux.intel.com>]

On Thu, 21 May 2026, David E. Box wrote:

> pmt_telem_header_decode() only needs the discovery header dwords, but it
> currently decodes them by reading directly from entry->disc_table.
> 
> Cache the discovery header in intel_pmt_entry when the device is created
> and have telemetry decode use the cached values instead of performing MMIO
> reads at decode time.
> 
> The DVSEC discovery resource for a namespace is sized by its per-entry
> entry_size (in dwords), which can be less than the 4-dword cache (e.g.
> telemetry uses entry_size = 3, i.e. 12 bytes). Cap the memcpy_fromio()
> to resource_size(disc_res) so the new cache does not read past the
> mapped region. Any unread dwords stay zero from the zero-initialized
> allocation of the containing struct.

sashiko seems to not agree with it being always zero-initialized and 
to me it looks there's some merit to that because of pmt_telem_probe() 
code reusing entry:

        for (i = 0; i < intel_vsec_dev->num_resources; i++) {
                struct intel_pmt_entry *entry = &priv->entry[priv->num_entries];

                mutex_lock(&ep_lock);
                ret = intel_pmt_dev_create(entry, &pmt_telem_ns, intel_vsec_dev, i);
                mutex_unlock(&ep_lock);
                if (ret < 0)
                        goto abort_probe;
                if (ret)
                        continue;

                priv->num_entries++;

With the subsequent change to u32 headers[PMT_DISC_HEADER_DWORDS];
in the next patch, the array will not be in zero-initialized state at 
all AFAICT.

> This keeps the telemetry header decode path independent of how the
> discovery data is backed and avoids baking a direct MMIO assumption into
> the feature-specific decode logic.
> 
> Assisted-by: GitHub-Copilot:claude-opus-4.7
> Signed-off-by: David E. Box <david.e.box@linux.intel.com>
> ---
> V5 changes:
>   - Cap memcpy_fromio() of the cached discovery header to
>     resource_size(disc_res) so the newly introduced cache does not
>     over-read namespaces whose DVSEC entry_size is smaller than the
>     cache (e.g. telemetry has entry_size = 3, 12 bytes).
> 
> V4 - No changes
> 
> V3 changes:
>   - New patch split out from PMT header-fetch rework to cache discovery
>     header data before downstream decode/population.
>   - Added to carry the post-v3 bug fix while preserving the original series
>     ordering intent.
> 
>  drivers/platform/x86/intel/pmt/class.c     | 11 +++++++++++
>  drivers/platform/x86/intel/pmt/class.h     |  1 +
>  drivers/platform/x86/intel/pmt/telemetry.c | 12 ++++++------
>  3 files changed, 18 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/platform/x86/intel/pmt/class.c b/drivers/platform/x86/intel/pmt/class.c
> index 7da8279b54f8..246e11837800 100644
> --- a/drivers/platform/x86/intel/pmt/class.c
> +++ b/drivers/platform/x86/intel/pmt/class.c
> @@ -383,6 +383,17 @@ int intel_pmt_dev_create(struct intel_pmt_entry *entry, struct intel_pmt_namespa
>  	if (IS_ERR(entry->disc_table))
>  		return PTR_ERR(entry->disc_table);
>  
> +	/*
> +	 * The mapped discovery resource may be smaller than disc_header (its
> +	 * size is the namespace's DVSEC entry_size in dwords, which can be
> +	 * less than 4). Cap the copy to the actual resource size to avoid
> +	 * reading past the mapped region; any unread dwords stay zero from
> +	 * the zero-initialized allocation of the containing struct.
> +	 */
> +	memcpy_fromio(entry->disc_header, entry->disc_table,
> +		      min_t(size_t, sizeof(entry->disc_header),

You forgot to add include for min_t(). Please add it before module.h (even 
if the includes are not fully in alphabetical order atm).

> +			    resource_size(disc_res)));
> +
>  	if (ns->pmt_pre_decode) {
>  		ret = ns->pmt_pre_decode(intel_vsec_dev, entry);
>  		if (ret)
> diff --git a/drivers/platform/x86/intel/pmt/class.h b/drivers/platform/x86/intel/pmt/class.h
> index 8a0db0ef58c1..84202fc7920c 100644
> --- a/drivers/platform/x86/intel/pmt/class.h
> +++ b/drivers/platform/x86/intel/pmt/class.h
> @@ -44,6 +44,7 @@ struct intel_pmt_entry {
>  	struct telem_endpoint	*ep;
>  	struct pci_dev		*pcidev;
>  	struct intel_pmt_header	header;
> +	u32			disc_header[4];
>  	struct bin_attribute	pmt_bin_attr;
>  	const struct attribute_group *attr_grp;
>  	struct kobject		*kobj;
> diff --git a/drivers/platform/x86/intel/pmt/telemetry.c b/drivers/platform/x86/intel/pmt/telemetry.c
> index d22f633638be..953f35b6daec 100644
> --- a/drivers/platform/x86/intel/pmt/telemetry.c
> +++ b/drivers/platform/x86/intel/pmt/telemetry.c
> @@ -72,16 +72,16 @@ static bool pmt_telem_region_overlaps(struct device *dev, u32 guid, u32 type)
>  static int pmt_telem_header_decode(struct intel_pmt_entry *entry,
>  				   struct device *dev)
>  {
> -	void __iomem *disc_table = entry->disc_table;
>  	struct intel_pmt_header *header = &entry->header;
> +	u32 *disc_header = entry->disc_header;
>  
> -	header->access_type = TELEM_ACCESS(readl(disc_table));
> -	header->guid = readl(disc_table + TELEM_GUID_OFFSET);
> -	header->base_offset = readl(disc_table + TELEM_BASE_OFFSET);
> +	header->access_type = TELEM_ACCESS(disc_header[0]);
> +	header->guid = disc_header[1];
> +	header->base_offset = disc_header[2];
>  
>  	/* Size is measured in DWORDS, but accessor returns bytes */
> -	header->size = TELEM_SIZE(readl(disc_table));
> -	header->telem_type = TELEM_TYPE(readl(entry->disc_table));
> +	header->size = TELEM_SIZE(disc_header[0]);
> +	header->telem_type = TELEM_TYPE(disc_header[0]);
>  
>  	return 0;
>  }
> 

-- 
 i.