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

[David Box]

On Fri, May 22, 2026 at 01:37:07PM +0300, Ilpo Järvinen wrote:
> On Thu, 21 May 2026, David E. Box wrote:
> 
> > From: Xi Pardee <xi.pardee@xxxxxxxxxxxxxxx>
> > 
> > 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@xxxxxxxxxxxxxxx>
> > Signed-off-by: David E. Box <david.e.box@xxxxxxxxxxxxxxx>
> > Reviewed-by: Ilpo Järvinen <ilpo.jarvinen@xxxxxxxxxxxxxxx>
> > ---
> 
> 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.

Okay thanks. This will be the first time I try it myself.

David

> 
> -- 
>  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) {
> >