Re: [PATCH v5 10/16] platform/x86/intel/pmc/ssram: Use fixed-size static pmc array

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

[-- Attachment #1: Type: text/plain, Size: 2715 bytes --]

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

> From: Xi Pardee <xi.pardee@linux.intel.com>
> 
> Switch pmc_ssram_telems from a devm-allocated pointer to a fixed-size
> static array, eliminating per-probe allocation overhead and simplifying
> lifetime management.
> 
> Correspondingly simplify pmc_ssram_telemetry_get_pmc_info() validation to
> check devid availability and tighten input bounds checking. Drop
> null-pointer checks now that the storage is static.
> 
> Signed-off-by: Xi Pardee <xi.pardee@linux.intel.com>
> Signed-off-by: David E. Box <david.e.box@linux.intel.com>
> Reviewed-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
> ---

Sashiko has noted some corner case issues related probe/init paths and 
missing cleanup which may have some merit, especially if this series makes 
the issue worse (for patches 10-14 but all those seem to fundamendally 
relate to same reprobe corner case and leaks of stale setups through 
static vars).

It has some false positives too, such as not realizing smp_*mb() implies 
a compiler barrier so those READ/WRITE_ONCE() requests look bogus to me.

-- 
 i.

> V5 - No changes
> 
> V4 - No changes
> 
> V3 - No changes
> 
> V2 changes:
>   - Replaced hardcoded array size [3] with MAX_NUM_PMC constant
> 
>  drivers/platform/x86/intel/pmc/ssram_telemetry.c | 10 ++--------
>  1 file changed, 2 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/platform/x86/intel/pmc/ssram_telemetry.c b/drivers/platform/x86/intel/pmc/ssram_telemetry.c
> index 1deb4d71da3f..4bfe60ee55ca 100644
> --- a/drivers/platform/x86/intel/pmc/ssram_telemetry.c
> +++ b/drivers/platform/x86/intel/pmc/ssram_telemetry.c
> @@ -24,7 +24,7 @@
>  
>  DEFINE_FREE(pmc_ssram_telemetry_iounmap, void __iomem *, if (_T) iounmap(_T))
>  
> -static struct pmc_ssram_telemetry *pmc_ssram_telems;
> +static struct pmc_ssram_telemetry pmc_ssram_telems[MAX_NUM_PMC];
>  static bool device_probed;
>  
>  static int
> @@ -140,7 +140,7 @@ int pmc_ssram_telemetry_get_pmc_info(unsigned int pmc_idx,
>  	if (pmc_idx >= MAX_NUM_PMC)
>  		return -EINVAL;
>  
> -	if (!pmc_ssram_telems || !pmc_ssram_telems[pmc_idx].devid)
> +	if (!pmc_ssram_telems[pmc_idx].devid)
>  		return -ENODEV;
>  
>  	pmc_ssram_telemetry->devid = pmc_ssram_telems[pmc_idx].devid;
> @@ -153,12 +153,6 @@ static int pmc_ssram_telemetry_probe(struct pci_dev *pcidev, const struct pci_de
>  {
>  	int ret;
>  
> -	pmc_ssram_telems = devm_kzalloc(&pcidev->dev, sizeof(*pmc_ssram_telems) * MAX_NUM_PMC,
> -					GFP_KERNEL);
> -	if (!pmc_ssram_telems) {
> -		ret = -ENOMEM;
> -		goto probe_finish;
> -	}
>  
>  	ret = pcim_enable_device(pcidev);
>  	if (ret) {
>