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

[Ilpo Järvinen]

On Sun, 31 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.
> 
> 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@xxxxxxxxxxxxxxx>
> ---
> V6 changes:
>   - Added #include <linux/minmax.h> for min_t() macro used in
>     memcpy_fromio() size calculation (Ilpo).
> 
> 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     | 12 ++++++++++++
>  drivers/platform/x86/intel/pmt/class.h     |  1 +
>  drivers/platform/x86/intel/pmt/telemetry.c | 12 ++++++------
>  3 files changed, 19 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/platform/x86/intel/pmt/class.c b/drivers/platform/x86/intel/pmt/class.c
> index 7da8279b54f8..44a0a52014fc 100644
> --- a/drivers/platform/x86/intel/pmt/class.c
> +++ b/drivers/platform/x86/intel/pmt/class.c
> @@ -12,6 +12,7 @@
>  #include <linux/log2.h>
>  #include <linux/intel_vsec.h>
>  #include <linux/io-64-nonatomic-lo-hi.h>
> +#include <linux/minmax.h>
>  #include <linux/module.h>
>  #include <linux/mm.h>
>  #include <linux/pci.h>
> @@ -383,6 +384,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),
> +			    resource_size(disc_res)));

I seem to have missed earlier that both of these are size_t already, so 
min() should suffice.

> +
>  	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.