Re: [PATCH v13 2/5] s390/pci: Improve function measurement state management

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential
issue to consider:
- [Medium] s390/pci: zpci_reenable_device() enables FMB on disabled PCI
  devices
--

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

s390/pci: Improve function measurement state management

This commit tracks the desired function measurement state in
zdev->statistics_on and restores it when enabling, disabling, or
re-enabling a zPCI device.

> diff --git a/arch/s390/pci/pci.c b/arch/s390/pci/pci.c
> index c055a9ad09724..f49736386eb7c 100644
> --- a/arch/s390/pci/pci.c
> +++ b/arch/s390/pci/pci.c

[ ... ]

> @@ -737,8 +769,14 @@ int zpci_reenable_device(struct zpci_dev *zdev)
>  	}
>  
>  	rc = zpci_iommu_register_ioat(zdev, &status);
> -	if (rc)
> +	if (rc) {
>  		zpci_disable_device(zdev);
> +		return rc;
> +	}
> +
> +	guard(mutex)(&zdev->fmb_lock);
> +	if (zdev->statistics_on)
> +		zpci_fmb_enable(zdev);
>  
>  	return rc;
>  }

[Severity: Medium]
Does this enable FMB measurement in firmware when the PCI device is
disabled?

zdev->statistics_on is initialized to 1 in zpci_create_device() and
remains 1 after pcibios_disable_device() disables FMB.  Callers such as
_do_recover() and vfio_pci_core_disable() -> ... -> zpci_hot_reset_device()
disable or remove the struct pci_dev before calling
zpci_reenable_device(zdev):

arch/s390/pci/pci_sysfs.c:_do_recover() {
	pci_stop_and_remove_bus_device(pdev);
	if (zdev_enabled(zdev)) {
		ret = zpci_disable_device(zdev);
		...
	}

	ret = zpci_reenable_device(zdev);
}

Because zdev->statistics_on is still 1, zpci_reenable_device() allocates
zdev->fmb and enables FMB DMA in firmware even though pcibios_disable_device()
already ran.  If no driver re-enables the PCI device, firmware FMB
measurement stays active on a disabled device, and at this commit
zpci_release_device() leaks zdev->fmb if the device is later deconfigured
(though commit "s390/pci: Improve FMB memory management" later in the
series frees zdev->fmb on release).  When a driver does later enable the
device, pcibios_enable_device() -> zpci_fmb_enable() sees zdev->fmb
already allocated and returns -EINVAL instead of zeroing zdev->fmb and
resetting the software IOMMU counters.

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