Re: [PATCH v4 2/4] s390/pci: Preserve FMB state in device re-enablement

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] zpci_fmb_reenable_device() reuses the existing FMB hardware buffer without zeroing it, exposing stale hardware counter values until the first periodic hardware update occurs.
--

commit a81b1b0a067d09df54d661ddbb8985e9220d9a88
Author: Omar Elghoul <oelghoul@linux.ibm.com>

s390/pci: Preserve FMB state in device re-enablement

This commit introduces zpci_fmb_reenable_device() to reuse the existing FMB
buffer during device re-enablement instead of allocating a new one. It also
refactors the FMB enablement and software counter clearing logic into shared
helpers.

> diff --git a/arch/s390/pci/pci.c b/arch/s390/pci/pci.c
> index 2910d4038d39c..21d3fccac7894 100644
> --- a/arch/s390/pci/pci.c
> +++ b/arch/s390/pci/pci.c

[ ... ]

> +int zpci_fmb_reenable_device(struct zpci_dev *zdev)
> +{
> +	u64 req = ZPCI_CREATE_REQ(zdev->fh, 0, ZPCI_MOD_FC_SET_MEASURE);
> +	struct zpci_fib fib = {0};
> +	u8 cc, status;
> +	int rc;
> +
> +	lockdep_assert_held(&zdev->fmb_lock);
> +
> +	if (!zdev->fmb)
> +		return zpci_fmb_enable_device(zdev);
> +
> +	fib.gd = zdev->gisa;
> +	cc = zpci_mod_fc(req, &fib, &status); /* Disable function measurement */
> +
> +	/* Unlike in zpci_fmb_disable_device(), cc == 3 is not a valid state here
> +	 * because we are re-enabling function measurement for the same function
> +	 * handle.
> +	 */
> +	if (cc)
> +		return -EIO;
> +
> +	zpci_fmb_clear_iommu_ctrs(zdev);
> +

[Severity: Medium]
Does this code need to zero the reused zdev->fmb buffer before enabling it?

The commit message states that a program resetting an FMB should see all
counters start from zero. While zpci_fmb_clear_iommu_ctrs() clears the
software-maintained IOMMU counters, the hardware-maintained FMB buffer
(zdev->fmb) is reused here without being cleared.

Could this expose stale hardware counter values to userspace until the
first asynchronous hardware update occurs? The previous behavior in
zpci_fmb_enable_device() allocated a fresh, zero-initialized buffer via
kmem_cache_zalloc().

> +	rc = zpci_fmb_do_enable(zdev);
> +	if (rc) {
> +		kmem_cache_free(zdev_fmb_cache, zdev->fmb);
> +		zdev->fmb = NULL;
> +	}
> +
> +	return rc;
> +}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260612181048.91548-1-oelghoul@linux.ibm.com?part=2