[PATCH 0/3] s390/pci: Enable CONTEXT_ANALYSIS

[PATCH 0/3] s390/pci: Enable CONTEXT_ANALYSIS

[Heiko Carstens]

Enable CONTEXT_ANALYSYS for s390's pci code.

Static code checking for acquiring and releasing locks used to be done
with sparse. That was removed with [1] and replaced with a clang based
approach [2]. The new approach requires that each subsystem needs to be
explicitly enabled for checking.

Do that for s390's pci code. In order to avoid false positives the code
has to be slightly reworked, since conditionally acquiring and releasing
locks does not work with the checker (besides that this is sub optimal
coding style).

[1] 5b63d0ae94cc ("compiler-context-analysis: Remove Sparse support")
[2] 3269701cb256 ("compiler-context-analysis: Add infrastructure for Context Analysis with Clang")

Notes:

- More patches for other s390 specific subsystems will follow

- "Pre-existing issues" reported by AI will not be addressed in the
  context of this series

Heiko Carstens (3):
  s390/pci: Rework __zpci_event_error() to remove conditional locking
  s390/pci: Rework__zpci_event_availability() to remove conditional locking
  s390/pci: Enable CONTEXT_ANALYSIS

 arch/s390/pci/Makefile    |   2 +
 arch/s390/pci/pci_event.c | 146 +++++++++++++++++++-------------------
 2 files changed, 75 insertions(+), 73 deletions(-)

-- 
2.53.0

[PATCH 1/3] s390/pci: Rework __zpci_event_error() to remove conditional locking

[Heiko Carstens]

Clang's compiler based static context analysis does not work with
locks that are conditionally taken like in __zpci_event_error():

arch/s390/pci/pci_event.c:320:2: warning: mutex 'get_zdev_by_fid(ccdf->fid).state_lock'
 is not held on every path through here [-Wthread-safety-analysis]

Given that code which takes locks conditionally can be considered
suboptimal rework __zpci_event_error() to get rid of this.

Signed-off-by: Heiko Carstens <hca@linux.ibm.com>
---
 arch/s390/pci/pci_event.c | 43 ++++++++++++++++++++++-----------------
 1 file changed, 24 insertions(+), 19 deletions(-)

diff --git a/arch/s390/pci/pci_event.c b/arch/s390/pci/pci_event.c
index 839bd91c056e..48fa26dcbee1 100644
--- a/arch/s390/pci/pci_event.c
+++ b/arch/s390/pci/pci_event.c
@@ -288,6 +288,12 @@ static void zpci_event_io_failure(struct pci_dev *pdev, pci_channel_state_t es)
 	pci_dev_unlock(pdev);
 }
 
+static void __zpci_event_print_error(struct pci_dev *pdev, struct zpci_ccdf_err *ccdf)
+{
+	pr_err("%s: Event 0x%x reports an error for PCI function 0x%x\n",
+	       pdev ? pci_name(pdev) : "n/a", ccdf->pec, ccdf->fid);
+}
+
 static void __zpci_event_error(struct zpci_ccdf_err *ccdf)
 {
 	struct zpci_dev *zdev = get_zdev_by_fid(ccdf->fid);
@@ -301,24 +307,24 @@ static void __zpci_event_error(struct zpci_ccdf_err *ccdf)
 	zpci_err("error CCDF:\n");
 	zpci_err_hex(ccdf, sizeof(*ccdf));
 
-	if (zdev) {
-		mutex_lock(&zdev->state_lock);
-		rc = clp_refresh_fh(zdev->fid, &fh);
-		if (rc)
-			goto no_pdev;
-		if (!fh || ccdf->fh != fh) {
-			/* Ignore events with stale handles */
-			zpci_dbg(3, "err fid:%x, fh:%x (stale %x)\n",
-				 ccdf->fid, fh, ccdf->fh);
-			goto no_pdev;
-		}
-		zpci_update_fh(zdev, ccdf->fh);
-		if (zdev->zbus->bus)
-			pdev = pci_get_slot(zdev->zbus->bus, zdev->devfn);
-	}
+	if (!zdev)
+		return __zpci_event_print_error(pdev, ccdf);
 
-	pr_err("%s: Event 0x%x reports an error for PCI function 0x%x\n",
-	       pdev ? pci_name(pdev) : "n/a", ccdf->pec, ccdf->fid);
+	mutex_lock(&zdev->state_lock);
+	rc = clp_refresh_fh(zdev->fid, &fh);
+	if (rc)
+		goto no_pdev;
+	if (!fh || ccdf->fh != fh) {
+		/* Ignore events with stale handles */
+		zpci_dbg(3, "err fid:%x, fh:%x (stale %x)\n",
+			 ccdf->fid, fh, ccdf->fh);
+		goto no_pdev;
+	}
+	zpci_update_fh(zdev, ccdf->fh);
+	if (zdev->zbus->bus)
+		pdev = pci_get_slot(zdev->zbus->bus, zdev->devfn);
+
+	__zpci_event_print_error(pdev, ccdf);
 
 	if (!pdev)
 		goto no_pdev;
@@ -340,8 +346,7 @@ static void __zpci_event_error(struct zpci_ccdf_err *ccdf)
 	}
 	pci_dev_put(pdev);
 no_pdev:
-	if (zdev)
-		mutex_unlock(&zdev->state_lock);
+	mutex_unlock(&zdev->state_lock);
 	zpci_zdev_put(zdev);
 }
 
-- 
2.53.0

Re: [PATCH 1/3] s390/pci: Rework __zpci_event_error() to remove conditional locking

[sashiko-bot]

> Clang's compiler based static context analysis does not work with
> locks that are conditionally taken like in __zpci_event_error():
> 
> arch/s390/pci/pci_event.c:320:2: warning: mutex 'get_zdev_by_fid(ccdf->fid).state_lock'
>  is not held on every path through here [-Wthread-safety-analysis]
> 
> Given that code which takes locks conditionally can be considered
> suboptimal rework __zpci_event_error() to get rid of this.
> 
> Signed-off-by: Heiko Carstens <hca@linux.ibm.com>

Sashiko has reviewed this patch and found no issues. It looks great!

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

[PATCH 2/3] s390/pci: Rework__zpci_event_availability() to remove conditional locking

[Heiko Carstens]

Clang's compiler based static context analysis does not work with locks
that are conditionally taken like in __zpci_event_availability():

arch/s390/pci/pci_event.c:402:10: warning: mutex 'get_zdev_by_fid(ccdf->fid).state_lock'
  is not held on every path through here [-Wthread-safety-analysis]

Given that code which takes locks conditionally can be considered
suboptimal rework __zpci_event_availability() to get rid of this.

Signed-off-by: Heiko Carstens <hca@linux.ibm.com>
---
 arch/s390/pci/pci_event.c | 103 ++++++++++++++++++--------------------
 1 file changed, 49 insertions(+), 54 deletions(-)

diff --git a/arch/s390/pci/pci_event.c b/arch/s390/pci/pci_event.c
index 48fa26dcbee1..37acee2c7d25 100644
--- a/arch/s390/pci/pci_event.c
+++ b/arch/s390/pci/pci_event.c
@@ -390,18 +390,21 @@ static void zpci_event_reappear(struct zpci_dev *zdev)
 static void __zpci_event_availability(struct zpci_ccdf_avail *ccdf)
 {
 	struct zpci_dev *zdev = get_zdev_by_fid(ccdf->fid);
-	bool existing_zdev = !!zdev;
 	enum zpci_state state;
 
 	zpci_dbg(3, "avl fid:%x, fh:%x, pec:%x\n",
 		 ccdf->fid, ccdf->fh, ccdf->pec);
 
-	if (existing_zdev)
-		mutex_lock(&zdev->state_lock);
+	if (ccdf->pec == 0x0306) {
+		/* 0x308 or 0x302 for multiple devices */
+		zpci_remove_reserved_devices();
+		zpci_scan_devices();
+		return;
+	}
 
-	switch (ccdf->pec) {
-	case 0x0301: /* Reserved|Standby -> Configured */
-		if (!zdev) {
+	if (!zdev) {
+		switch (ccdf->pec) {
+		case 0x0301: /* Reserved|Standby -> Configured */
 			zdev = zpci_create_device(ccdf->fid, ccdf->fh, ZPCI_FN_STATE_CONFIGURED);
 			if (IS_ERR(zdev))
 				break;
@@ -409,18 +412,9 @@ static void __zpci_event_availability(struct zpci_ccdf_avail *ccdf)
 				kfree(zdev);
 				break;
 			}
-		} else {
-			if (zdev->state == ZPCI_FN_STATE_RESERVED)
-				zpci_event_reappear(zdev);
-			/* the configuration request may be stale */
-			else if (zdev->state != ZPCI_FN_STATE_STANDBY)
-				break;
-			zdev->state = ZPCI_FN_STATE_CONFIGURED;
-		}
-		zpci_scan_configured_device(zdev, ccdf->fh);
-		break;
-	case 0x0302: /* Reserved -> Standby */
-		if (!zdev) {
+			zpci_scan_configured_device(zdev, ccdf->fh);
+			break;
+		case 0x0302: /* Reserved -> Standby */
 			zdev = zpci_create_device(ccdf->fid, ccdf->fh, ZPCI_FN_STATE_STANDBY);
 			if (IS_ERR(zdev))
 				break;
@@ -428,53 +422,54 @@ static void __zpci_event_availability(struct zpci_ccdf_avail *ccdf)
 				kfree(zdev);
 				break;
 			}
-		} else {
-			if (zdev->state == ZPCI_FN_STATE_RESERVED)
-				zpci_event_reappear(zdev);
-			zpci_update_fh(zdev, ccdf->fh);
+			break;
 		}
+		return;
+	}
+
+	mutex_lock(&zdev->state_lock);
+	switch (ccdf->pec) {
+	case 0x0301: /* Reserved|Standby -> Configured */
+		if (zdev->state == ZPCI_FN_STATE_RESERVED)
+			zpci_event_reappear(zdev);
+		/* the configuration request may be stale */
+		else if (zdev->state != ZPCI_FN_STATE_STANDBY)
+			break;
+		zdev->state = ZPCI_FN_STATE_CONFIGURED;
+		zpci_scan_configured_device(zdev, ccdf->fh);
+		break;
+	case 0x0302: /* Reserved -> Standby */
+		if (zdev->state == ZPCI_FN_STATE_RESERVED)
+			zpci_event_reappear(zdev);
+		zpci_update_fh(zdev, ccdf->fh);
 		break;
 	case 0x0303: /* Deconfiguration requested */
-		if (zdev) {
-			/* The event may have been queued before we configured
-			 * the device.
-			 */
-			if (zdev->state != ZPCI_FN_STATE_CONFIGURED)
-				break;
-			zpci_update_fh(zdev, ccdf->fh);
-			zpci_deconfigure_device(zdev);
-		}
+		/* The event may have been queued before we configured
+		 * the device.
+		 */
+		if (zdev->state != ZPCI_FN_STATE_CONFIGURED)
+			break;
+		zpci_update_fh(zdev, ccdf->fh);
+		zpci_deconfigure_device(zdev);
 		break;
 	case 0x0304: /* Configured -> Standby|Reserved */
-		if (zdev) {
-			/* The event may have been queued before we configured
-			 * the device.:
-			 */
-			if (zdev->state == ZPCI_FN_STATE_CONFIGURED)
-				zpci_event_hard_deconfigured(zdev, ccdf->fh);
-			/* The 0x0304 event may immediately reserve the device */
-			if (!clp_get_state(zdev->fid, &state) &&
-			    state == ZPCI_FN_STATE_RESERVED) {
-				zpci_device_reserved(zdev);
-			}
+		/* The event may have been queued before we configured
+		 * the device.:
+		 */
+		if (zdev->state == ZPCI_FN_STATE_CONFIGURED)
+			zpci_event_hard_deconfigured(zdev, ccdf->fh);
+		/* The 0x0304 event may immediately reserve the device */
+		if (!clp_get_state(zdev->fid, &state) &&
+		    state == ZPCI_FN_STATE_RESERVED) {
+			zpci_device_reserved(zdev);
 		}
 		break;
-	case 0x0306: /* 0x308 or 0x302 for multiple devices */
-		zpci_remove_reserved_devices();
-		zpci_scan_devices();
-		break;
 	case 0x0308: /* Standby -> Reserved */
-		if (!zdev)
-			break;
 		zpci_device_reserved(zdev);
 		break;
-	default:
-		break;
-	}
-	if (existing_zdev) {
-		mutex_unlock(&zdev->state_lock);
-		zpci_zdev_put(zdev);
 	}
+	mutex_unlock(&zdev->state_lock);
+	zpci_zdev_put(zdev);
 }
 
 void zpci_event_availability(void *data)
-- 
2.53.0

Re: [PATCH 2/3] s390/pci: Rework__zpci_event_availability() to remove conditional locking

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] The `zdev` reference obtained via `get_zdev_by_fid()` is leaked when handling the `0x0306` PCI event.
--

--- Patch [2]: [PATCH 2/3] s390/pci: Rework__zpci_event_availability() to remove conditional locking ---
commit abeb29e9ac48aa95001741be64e50a68bcadf6fd
Author: Heiko Carstens <hca@linux.ibm.com>

s390/pci: Rework__zpci_event_availability() to remove conditional locking

This commit refactors the __zpci_event_availability() function to avoid
taking the device state lock conditionally. The restructuring handles
different PCI event codes explicitly to resolve static analysis warnings.

> diff --git a/arch/s390/pci/pci_event.c b/arch/s390/pci/pci_event.c
> index 48fa26dcbee14..37acee2c7d257 100644
> --- a/arch/s390/pci/pci_event.c
> +++ b/arch/s390/pci/pci_event.c
> @@ -390,18 +390,21 @@ static void zpci_event_reappear(struct zpci_dev *zdev)
>  static void __zpci_event_availability(struct zpci_ccdf_avail *ccdf)
>  {
>  	struct zpci_dev *zdev = get_zdev_by_fid(ccdf->fid);
> -	bool existing_zdev = !!zdev;
>  	enum zpci_state state;
>  
>  	zpci_dbg(3, "avl fid:%x, fh:%x, pec:%x\n",
>  		 ccdf->fid, ccdf->fh, ccdf->pec);
>  
> -	if (existing_zdev)
> -		mutex_lock(&zdev->state_lock);
> +	if (ccdf->pec == 0x0306) {
> +		/* 0x308 or 0x302 for multiple devices */
> +		zpci_remove_reserved_devices();
> +		zpci_scan_devices();
> +		return;

[Severity: High]
Does this early return leak the zdev reference?

If get_zdev_by_fid() finds an existing device, it increments its reference
count. By returning directly here when the event code is 0x0306, it seems we
skip calling zpci_zdev_put() at the end of the function. Could this prevent
the device from being properly freed during hot-unplug over time?

> +	}
>  
> -	switch (ccdf->pec) {
> -	case 0x0301: /* Reserved|Standby -> Configured */
> -		if (!zdev) {

[ ... ]

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

Re: [PATCH 2/3] s390/pci: Rework__zpci_event_availability() to remove conditional locking

[Heiko Carstens]

On Mon, Aug 03, 2026 at 10:17:59AM +0000, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [High] The `zdev` reference obtained via `get_zdev_by_fid()` is leaked when handling the `0x0306` PCI event.
> > -	if (existing_zdev)
> > -		mutex_lock(&zdev->state_lock);
> > +	if (ccdf->pec == 0x0306) {
> > +		/* 0x308 or 0x302 for multiple devices */
> > +		zpci_remove_reserved_devices();
> > +		zpci_scan_devices();
> > +		return;
> 
> [Severity: High]
> Does this early return leak the zdev reference?
> 
> If get_zdev_by_fid() finds an existing device, it increments its reference
> count. By returning directly here when the event code is 0x0306, it seems we
> skip calling zpci_zdev_put() at the end of the function. Could this prevent
> the device from being properly freed during hot-unplug over time?

This is correct, but trying to fix this I also looked a bit deeper
into the code and found a "pre-existing" issue. Oh, well..

[PATCH 3/3] s390/pci: Enable CONTEXT_ANALYSIS

[Heiko Carstens]

Enable CONTEXT_ANALYSIS since s390's pci code compiles now without warnings.

Signed-off-by: Heiko Carstens <hca@linux.ibm.com>
---
 arch/s390/pci/Makefile | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/arch/s390/pci/Makefile b/arch/s390/pci/Makefile
index 1810e0944a4e..b8be1316491b 100644
--- a/arch/s390/pci/Makefile
+++ b/arch/s390/pci/Makefile
@@ -3,6 +3,8 @@
 # Makefile for the s390 PCI subsystem.
 #
 
+CONTEXT_ANALYSIS := y
+
 obj-$(CONFIG_PCI)	+= pci.o pci_irq.o pci_clp.o \
 			   pci_event.o pci_debug.o pci_insn.o pci_mmio.o \
 			   pci_bus.o pci_kvm_hook.o pci_report.o pci_fixup.o
-- 
2.53.0

Re: [PATCH 3/3] s390/pci: Enable CONTEXT_ANALYSIS

[sashiko-bot]

> Enable CONTEXT_ANALYSIS since s390's pci code compiles now without warnings.
> 
> Signed-off-by: Heiko Carstens <hca@linux.ibm.com>

Sashiko has reviewed this patch and found no issues. It looks great!

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260803100723.2343688-1-hca@linux.ibm.com?part=3