Re: [PATCH v3 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] The reused hardware Function Measurement Block (`zdev->fmb`) is not zeroed before re-enablement, exposing stale performance counters.
--

commit 9bd944cc7f231486ddb6f89d0d8243e0febeb494
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 function
measurement block (FMB) buffer across device re-enablements. It also
separates the clearing of software counters so they can be reset when
re-enabling, allowing users to see counters start from zero.

> diff --git a/arch/s390/pci/pci.c b/arch/s390/pci/pci.c
> --- 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);
> +
> +	rc = zpci_fmb_do_enable(zdev);

[Severity: Medium]
The commit message mentions that "a program resetting an FMB sees all
counters start from zero as expected". 

While zpci_fmb_clear_iommu_ctrs() successfully clears the software
counters, does the reused hardware buffer (zdev->fmb) also need to be
cleared before calling zpci_fmb_do_enable()?

Since the original buffer in zpci_fmb_enable_device() was allocated with
kmem_cache_zalloc(), reusing it without explicitly zeroing might leave stale
hardware counter values in place. Could this result in userspace observing
non-zero hardware counters from before the re-enablement until the hardware
performs its first asynchronous DMA update?

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

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