* [PATCH 0/3] s390: Enable CONTEXT_ANALYSIS for cio and virtio
@ 2026-08-06 15:38 Heiko Carstens
2026-08-06 15:38 ` [PATCH 1/3] s390/vfio_ccw: Add __must_hold() attribute to vfio_ccw_sch_quiesce() Heiko Carstens
` (3 more replies)
0 siblings, 4 replies; 16+ messages in thread
From: Heiko Carstens @ 2026-08-06 15:38 UTC (permalink / raw)
To: Christian Borntraeger, Janosch Frank, Claudio Imbrenda,
Eric Farman, Matthew Rosato, Vineeth Vijayan, Peter Oberparleiter
Cc: linux-s390
Enable CONTEXT_ANALYSYS for cio and virtio.
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 cio and virtio. Add a __must_hold() attribute to
vfio_ccw_sch_quiesce() to address the only valid warning.
Then enable CONTEXT_ANALYSIS for both cio and virtio.
Christian, Claudio, Janosch, I think it would make most sense if this
goes via kvms390.
Thanks,
Heiko
[1] 5b63d0ae94cc ("compiler-context-analysis: Remove Sparse support")
[2] 3269701cb256 ("compiler-context-analysis: Add infrastructure for Context Analysis with Clang")
Heiko Carstens (3):
s390/vfio_ccw: Add __must_hold() attribute to vfio_ccw_sch_quiesce()
s390/cio: Enable CONTEXT_ANALYSIS
s390/virtio: Enable CONTEXT_ANALYSIS
drivers/s390/cio/Makefile | 2 ++
drivers/s390/cio/vfio_ccw_drv.c | 1 +
drivers/s390/cio/vfio_ccw_private.h | 3 ++-
drivers/s390/virtio/Makefile | 2 ++
4 files changed, 7 insertions(+), 1 deletion(-)
--
2.53.0
^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH 1/3] s390/vfio_ccw: Add __must_hold() attribute to vfio_ccw_sch_quiesce()
2026-08-06 15:38 [PATCH 0/3] s390: Enable CONTEXT_ANALYSIS for cio and virtio Heiko Carstens
@ 2026-08-06 15:38 ` Heiko Carstens
2026-08-06 15:52 ` sashiko-bot
` (3 more replies)
2026-08-06 15:38 ` [PATCH 2/3] s390/cio: Enable CONTEXT_ANALYSIS Heiko Carstens
` (2 subsequent siblings)
3 siblings, 4 replies; 16+ messages in thread
From: Heiko Carstens @ 2026-08-06 15:38 UTC (permalink / raw)
To: Christian Borntraeger, Janosch Frank, Claudio Imbrenda,
Eric Farman, Matthew Rosato, Vineeth Vijayan, Peter Oberparleiter
Cc: linux-s390
Add __must_hold() attribute to vfio_ccw_sch_quiesce() in order to let
clang's context analysis know that sch->lock must be held on function
entry. This can also be easily verified when inspecting the function.
Without this annotation this leads to a valid warning when context
analysis is enabled:
drivers/s390/cio/vfio_ccw_drv.c:55:9: warning:
expecting spinlock 'sch->lock' to be held at start of each loop [-Wthread-safety-analysis]
55 | ret = cio_cancel_halt_clear(sch, &iretry);
| ^
Signed-off-by: Heiko Carstens <hca@linux.ibm.com>
---
drivers/s390/cio/vfio_ccw_drv.c | 1 +
drivers/s390/cio/vfio_ccw_private.h | 3 ++-
2 files changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/s390/cio/vfio_ccw_drv.c b/drivers/s390/cio/vfio_ccw_drv.c
index 1a095085bc72..ed9ca77c5b23 100644
--- a/drivers/s390/cio/vfio_ccw_drv.c
+++ b/drivers/s390/cio/vfio_ccw_drv.c
@@ -35,6 +35,7 @@ debug_info_t *vfio_ccw_debug_trace_id;
* Helpers
*/
int vfio_ccw_sch_quiesce(struct subchannel *sch)
+ __must_hold(&sch->lock);
{
struct vfio_ccw_parent *parent = dev_get_drvdata(&sch->dev);
struct vfio_ccw_private *private = dev_get_drvdata(&parent->dev);
diff --git a/drivers/s390/cio/vfio_ccw_private.h b/drivers/s390/cio/vfio_ccw_private.h
index 0501d4bbcdbd..8db29519dbfd 100644
--- a/drivers/s390/cio/vfio_ccw_private.h
+++ b/drivers/s390/cio/vfio_ccw_private.h
@@ -127,7 +127,8 @@ struct vfio_ccw_private {
struct work_struct crw_work;
} __aligned(8);
-int vfio_ccw_sch_quiesce(struct subchannel *sch);
+int vfio_ccw_sch_quiesce(struct subchannel *sch)
+ __must_hold(&sch->lock);
void vfio_ccw_sch_io_todo(struct work_struct *work);
void vfio_ccw_crw_todo(struct work_struct *work);
--
2.53.0
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH 2/3] s390/cio: Enable CONTEXT_ANALYSIS
2026-08-06 15:38 [PATCH 0/3] s390: Enable CONTEXT_ANALYSIS for cio and virtio Heiko Carstens
2026-08-06 15:38 ` [PATCH 1/3] s390/vfio_ccw: Add __must_hold() attribute to vfio_ccw_sch_quiesce() Heiko Carstens
@ 2026-08-06 15:38 ` Heiko Carstens
2026-08-06 15:42 ` sashiko-bot
2026-08-06 15:38 ` [PATCH 3/3] s390/virtio: " Heiko Carstens
2026-08-07 12:31 ` [PATCH 0/3] s390: Enable CONTEXT_ANALYSIS for cio and virtio Christian Borntraeger
3 siblings, 1 reply; 16+ messages in thread
From: Heiko Carstens @ 2026-08-06 15:38 UTC (permalink / raw)
To: Christian Borntraeger, Janosch Frank, Claudio Imbrenda,
Eric Farman, Matthew Rosato, Vineeth Vijayan, Peter Oberparleiter
Cc: linux-s390
All cio code passes clang's compile time context analysis.
Therefore enable CONTEXT_ANALYSIS.
Signed-off-by: Heiko Carstens <hca@linux.ibm.com>
---
drivers/s390/cio/Makefile | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/s390/cio/Makefile b/drivers/s390/cio/Makefile
index 3bd1c245183f..611a39103df9 100644
--- a/drivers/s390/cio/Makefile
+++ b/drivers/s390/cio/Makefile
@@ -3,6 +3,8 @@
# Makefile for the S/390 common i/o drivers
#
+CONTEXT_ANALYSIS := y
+
# The following is required for define_trace.h to find ./trace.h
CFLAGS_trace.o := -I$(src)
CFLAGS_vfio_ccw_trace.o := -I$(src)
--
2.53.0
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH 3/3] s390/virtio: Enable CONTEXT_ANALYSIS
2026-08-06 15:38 [PATCH 0/3] s390: Enable CONTEXT_ANALYSIS for cio and virtio Heiko Carstens
2026-08-06 15:38 ` [PATCH 1/3] s390/vfio_ccw: Add __must_hold() attribute to vfio_ccw_sch_quiesce() Heiko Carstens
2026-08-06 15:38 ` [PATCH 2/3] s390/cio: Enable CONTEXT_ANALYSIS Heiko Carstens
@ 2026-08-06 15:38 ` Heiko Carstens
2026-08-06 15:40 ` sashiko-bot
2026-08-06 17:35 ` Matthew Rosato
2026-08-07 12:31 ` [PATCH 0/3] s390: Enable CONTEXT_ANALYSIS for cio and virtio Christian Borntraeger
3 siblings, 2 replies; 16+ messages in thread
From: Heiko Carstens @ 2026-08-06 15:38 UTC (permalink / raw)
To: Christian Borntraeger, Janosch Frank, Claudio Imbrenda,
Eric Farman, Matthew Rosato, Vineeth Vijayan, Peter Oberparleiter
Cc: linux-s390
All virtio code passes clang's compile time context analysis.
Therefore enable CONTEXT_ANALYSIS.
Signed-off-by: Heiko Carstens <hca@linux.ibm.com>
---
drivers/s390/virtio/Makefile | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/s390/virtio/Makefile b/drivers/s390/virtio/Makefile
index 2dc4d9aab634..cab140fe0b29 100644
--- a/drivers/s390/virtio/Makefile
+++ b/drivers/s390/virtio/Makefile
@@ -3,4 +3,6 @@
#
# Copyright IBM Corp. 2008
+CONTEXT_ANALYSIS := y
+
obj-$(CONFIG_S390_GUEST) += virtio_ccw.o
--
2.53.0
^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: [PATCH 3/3] s390/virtio: Enable CONTEXT_ANALYSIS
2026-08-06 15:38 ` [PATCH 3/3] s390/virtio: " Heiko Carstens
@ 2026-08-06 15:40 ` sashiko-bot
2026-08-06 17:35 ` Matthew Rosato
1 sibling, 0 replies; 16+ messages in thread
From: sashiko-bot @ 2026-08-06 15:40 UTC (permalink / raw)
To: Heiko Carstens
Cc: Alexander Gordeev, linux-s390, Vasily Gorbik, Heiko Carstens
> All virtio code passes clang's compile time context analysis.
> Therefore enable CONTEXT_ANALYSIS.
>
> 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/20260806153849.2622576-1-hca@linux.ibm.com?part=3
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 2/3] s390/cio: Enable CONTEXT_ANALYSIS
2026-08-06 15:38 ` [PATCH 2/3] s390/cio: Enable CONTEXT_ANALYSIS Heiko Carstens
@ 2026-08-06 15:42 ` sashiko-bot
0 siblings, 0 replies; 16+ messages in thread
From: sashiko-bot @ 2026-08-06 15:42 UTC (permalink / raw)
To: Heiko Carstens
Cc: Alexander Gordeev, linux-s390, Vasily Gorbik, Heiko Carstens
> All cio code passes clang's compile time context analysis.
> Therefore enable CONTEXT_ANALYSIS.
>
> 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/20260806153849.2622576-1-hca@linux.ibm.com?part=2
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 1/3] s390/vfio_ccw: Add __must_hold() attribute to vfio_ccw_sch_quiesce()
2026-08-06 15:38 ` [PATCH 1/3] s390/vfio_ccw: Add __must_hold() attribute to vfio_ccw_sch_quiesce() Heiko Carstens
@ 2026-08-06 15:52 ` sashiko-bot
2026-08-06 16:02 ` Heiko Carstens
2026-08-06 17:34 ` Matthew Rosato
` (2 subsequent siblings)
3 siblings, 1 reply; 16+ messages in thread
From: sashiko-bot @ 2026-08-06 15:52 UTC (permalink / raw)
To: Heiko Carstens
Cc: linux-s390, Heiko Carstens, Alexander Gordeev, Vasily Gorbik
Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:
Pre-existing issues:
- [High] Premature loop termination in `vfio_ccw_sch_quiesce` due to re-used completion without re-initialization.
- [High] Unconditional NULL pointer dereference in asynchronous subchannel callbacks.
- [High] Leaked workqueue items (`io_work` and `crw_work`) during device teardown leading to Use-After-Free.
- [High] Lockless state access in `vfio_ccw_sch_io_todo` causes a double-free of channel program (`cp_free`).
--
--- Patch [1]: [PATCH 1/3] s390/vfio_ccw: Add __must_hold() attribute to vfio_ccw_sch_quiesce() ---
commit c7b4889a75b6cf11527a57bff8438c3c08dac9c4
Author: Heiko Carstens <hca@linux.ibm.com>
Subject: s390/vfio_ccw: Add __must_hold() attribute to vfio_ccw_sch_quiesce()
This patch adds the __must_hold attribute to vfio_ccw_sch_quiesce() to
inform context analysis that sch->lock must be held on function entry.
This resolves a thread-safety-analysis warning during compilation.
> diff --git a/drivers/s390/cio/vfio_ccw_drv.c b/drivers/s390/cio/vfio_ccw_drv.c
> index 1a095085bc728..ed9ca77c5b23f 100644
> --- a/drivers/s390/cio/vfio_ccw_drv.c
> +++ b/drivers/s390/cio/vfio_ccw_drv.c
> @@ -35,6 +35,7 @@ debug_info_t *vfio_ccw_debug_trace_id;
> * Helpers
> */
> int vfio_ccw_sch_quiesce(struct subchannel *sch)
> + __must_hold(&sch->lock);
> {
> struct vfio_ccw_parent *parent = dev_get_drvdata(&sch->dev);
> struct vfio_ccw_private *private = dev_get_drvdata(&parent->dev);
[Severity: High]
This is a pre-existing issue, but does the completion handling in this loop
allow premature termination?
If wait_for_completion_timeout() times out, but a late interrupt arrives
and calls complete(), the completion's done counter becomes greater than 0.
On the next iteration of the do/while (ret == -EBUSY) loop in
vfio_ccw_sch_quiesce():
do {
ret = cio_cancel_halt_clear(sch, &iretry);
...
if (ret == -EBUSY)
wait_for_completion_timeout(&completion, 3*HZ);
...
} while (ret == -EBUSY);
Could wait_for_completion_timeout() see done > 0 and return immediately
without waiting, causing a tight spin loop calling cio_disable_subchannel()
since there is no reinit_completion() inside the loop?
[Severity: High]
This isn't a bug introduced by this patch, but is there a risk of a NULL
pointer dereference when fetching the parent pointer?
During device removal, vfio_ccw_sch_remove() sets the drvdata to NULL:
dev_set_drvdata(&sch->dev, NULL);
If an asynchronous event like an interrupt triggers concurrently before the
CSS core fully detaches the driver, could vfio_ccw_sch_irq() or
vfio_ccw_sch_event() fetch a NULL parent and immediately dereference it
in this sequence?
struct vfio_ccw_parent *parent = dev_get_drvdata(&sch->dev);
struct vfio_ccw_private *private = dev_get_drvdata(&parent->dev);
[ ... ]
> diff --git a/drivers/s390/cio/vfio_ccw_private.h b/drivers/s390/cio/vfio_ccw_private.h
> index 0501d4bbcdbd6..8db29519dbfde 100644
> --- a/drivers/s390/cio/vfio_ccw_private.h
> +++ b/drivers/s390/cio/vfio_ccw_private.h
> @@ -127,7 +127,8 @@ struct vfio_ccw_private {
> struct work_struct crw_work;
> } __aligned(8);
[Severity: High]
This is a pre-existing issue, but does the device teardown path leak these
pending work items?
When a mediated device is removed, vfio_ccw_mdev_release_dev() frees the
private structure without calling cancel_work_sync() on io_work or crw_work:
static void vfio_ccw_mdev_release_dev(struct vfio_device *vdev)
{
...
kfree(private);
}
Additionally, fsm_close() can skip the global flush_workqueue() if
cio_disable_subchannel() immediately returns 0:
ret = cio_disable_subchannel(sch);
if (ret == -EBUSY)
ret = vfio_ccw_sch_quiesce(sch);
If a work item is left pending on the workqueue, could it execute after
the private structure has been freed, resulting in a use-after-free?
> -int vfio_ccw_sch_quiesce(struct subchannel *sch);
> +int vfio_ccw_sch_quiesce(struct subchannel *sch)
> + __must_hold(&sch->lock);
> void vfio_ccw_sch_io_todo(struct work_struct *work);
> void vfio_ccw_crw_todo(struct work_struct *work);
[Severity: High]
This isn't a bug introduced by this patch, but does the lockless state check
in vfio_ccw_sch_io_todo() allow a double-free of the channel program?
The workqueue function vfio_ccw_sch_io_todo() reads private->state
locklessly:
if (is_final && private->state == VFIO_CCW_STATE_CP_PENDING) {
cp_free(&private->cp);
cp_is_finished = true;
}
Concurrently, fsm_close() holds sch->lock, changes the state to STANDBY,
unlocks, and also calls cp_free():
spin_unlock_irq(&sch->lock);
cp_free(&private->cp);
Since vfio_ccw_sch_io_todo() operates outside sch->lock, could both threads
evaluate their conditions simultaneously and concurrently execute cp_free()
on the same channel program?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260806153849.2622576-1-hca@linux.ibm.com?part=1
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 1/3] s390/vfio_ccw: Add __must_hold() attribute to vfio_ccw_sch_quiesce()
2026-08-06 15:52 ` sashiko-bot
@ 2026-08-06 16:02 ` Heiko Carstens
2026-08-06 17:26 ` Matthew Rosato
0 siblings, 1 reply; 16+ messages in thread
From: Heiko Carstens @ 2026-08-06 16:02 UTC (permalink / raw)
To: Eric Farman, Matthew Rosato; +Cc: linux-s390, Alexander Gordeev, Vasily Gorbik
Eric, Matthew,
not sure if you are aware of the below (nothing I would address). Please handle :)
On Thu, Aug 06, 2026 at 03:52:23PM +0000, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:
>
> Pre-existing issues:
> - [High] Premature loop termination in `vfio_ccw_sch_quiesce` due to re-used completion without re-initialization.
> - [High] Unconditional NULL pointer dereference in asynchronous subchannel callbacks.
> - [High] Leaked workqueue items (`io_work` and `crw_work`) during device teardown leading to Use-After-Free.
> - [High] Lockless state access in `vfio_ccw_sch_io_todo` causes a double-free of channel program (`cp_free`).
> --
>
> --- Patch [1]: [PATCH 1/3] s390/vfio_ccw: Add __must_hold() attribute to vfio_ccw_sch_quiesce() ---
> commit c7b4889a75b6cf11527a57bff8438c3c08dac9c4
> Author: Heiko Carstens <hca@linux.ibm.com>
> Subject: s390/vfio_ccw: Add __must_hold() attribute to vfio_ccw_sch_quiesce()
>
> This patch adds the __must_hold attribute to vfio_ccw_sch_quiesce() to
> inform context analysis that sch->lock must be held on function entry.
> This resolves a thread-safety-analysis warning during compilation.
>
> > diff --git a/drivers/s390/cio/vfio_ccw_drv.c b/drivers/s390/cio/vfio_ccw_drv.c
> > index 1a095085bc728..ed9ca77c5b23f 100644
> > --- a/drivers/s390/cio/vfio_ccw_drv.c
> > +++ b/drivers/s390/cio/vfio_ccw_drv.c
> > @@ -35,6 +35,7 @@ debug_info_t *vfio_ccw_debug_trace_id;
> > * Helpers
> > */
> > int vfio_ccw_sch_quiesce(struct subchannel *sch)
> > + __must_hold(&sch->lock);
> > {
> > struct vfio_ccw_parent *parent = dev_get_drvdata(&sch->dev);
> > struct vfio_ccw_private *private = dev_get_drvdata(&parent->dev);
>
> [Severity: High]
> This is a pre-existing issue, but does the completion handling in this loop
> allow premature termination?
>
> If wait_for_completion_timeout() times out, but a late interrupt arrives
> and calls complete(), the completion's done counter becomes greater than 0.
>
> On the next iteration of the do/while (ret == -EBUSY) loop in
> vfio_ccw_sch_quiesce():
>
> do {
> ret = cio_cancel_halt_clear(sch, &iretry);
> ...
> if (ret == -EBUSY)
> wait_for_completion_timeout(&completion, 3*HZ);
> ...
> } while (ret == -EBUSY);
>
> Could wait_for_completion_timeout() see done > 0 and return immediately
> without waiting, causing a tight spin loop calling cio_disable_subchannel()
> since there is no reinit_completion() inside the loop?
>
> [Severity: High]
> This isn't a bug introduced by this patch, but is there a risk of a NULL
> pointer dereference when fetching the parent pointer?
>
> During device removal, vfio_ccw_sch_remove() sets the drvdata to NULL:
> dev_set_drvdata(&sch->dev, NULL);
>
> If an asynchronous event like an interrupt triggers concurrently before the
> CSS core fully detaches the driver, could vfio_ccw_sch_irq() or
> vfio_ccw_sch_event() fetch a NULL parent and immediately dereference it
> in this sequence?
>
> struct vfio_ccw_parent *parent = dev_get_drvdata(&sch->dev);
> struct vfio_ccw_private *private = dev_get_drvdata(&parent->dev);
>
> [ ... ]
> > diff --git a/drivers/s390/cio/vfio_ccw_private.h b/drivers/s390/cio/vfio_ccw_private.h
> > index 0501d4bbcdbd6..8db29519dbfde 100644
> > --- a/drivers/s390/cio/vfio_ccw_private.h
> > +++ b/drivers/s390/cio/vfio_ccw_private.h
> > @@ -127,7 +127,8 @@ struct vfio_ccw_private {
> > struct work_struct crw_work;
> > } __aligned(8);
>
> [Severity: High]
> This is a pre-existing issue, but does the device teardown path leak these
> pending work items?
>
> When a mediated device is removed, vfio_ccw_mdev_release_dev() frees the
> private structure without calling cancel_work_sync() on io_work or crw_work:
>
> static void vfio_ccw_mdev_release_dev(struct vfio_device *vdev)
> {
> ...
> kfree(private);
> }
>
> Additionally, fsm_close() can skip the global flush_workqueue() if
> cio_disable_subchannel() immediately returns 0:
>
> ret = cio_disable_subchannel(sch);
> if (ret == -EBUSY)
> ret = vfio_ccw_sch_quiesce(sch);
>
> If a work item is left pending on the workqueue, could it execute after
> the private structure has been freed, resulting in a use-after-free?
>
> > -int vfio_ccw_sch_quiesce(struct subchannel *sch);
> > +int vfio_ccw_sch_quiesce(struct subchannel *sch)
> > + __must_hold(&sch->lock);
> > void vfio_ccw_sch_io_todo(struct work_struct *work);
> > void vfio_ccw_crw_todo(struct work_struct *work);
>
> [Severity: High]
> This isn't a bug introduced by this patch, but does the lockless state check
> in vfio_ccw_sch_io_todo() allow a double-free of the channel program?
>
> The workqueue function vfio_ccw_sch_io_todo() reads private->state
> locklessly:
>
> if (is_final && private->state == VFIO_CCW_STATE_CP_PENDING) {
> cp_free(&private->cp);
> cp_is_finished = true;
> }
>
> Concurrently, fsm_close() holds sch->lock, changes the state to STANDBY,
> unlocks, and also calls cp_free():
>
> spin_unlock_irq(&sch->lock);
> cp_free(&private->cp);
>
> Since vfio_ccw_sch_io_todo() operates outside sch->lock, could both threads
> evaluate their conditions simultaneously and concurrently execute cp_free()
> on the same channel program?
>
> --
> Sashiko AI review · https://sashiko.dev/#/patchset/20260806153849.2622576-1-hca@linux.ibm.com?part=1
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 1/3] s390/vfio_ccw: Add __must_hold() attribute to vfio_ccw_sch_quiesce()
2026-08-06 16:02 ` Heiko Carstens
@ 2026-08-06 17:26 ` Matthew Rosato
0 siblings, 0 replies; 16+ messages in thread
From: Matthew Rosato @ 2026-08-06 17:26 UTC (permalink / raw)
To: Heiko Carstens, Eric Farman; +Cc: linux-s390, Alexander Gordeev, Vasily Gorbik
On 8/6/26 12:02 PM, Heiko Carstens wrote:
> Eric, Matthew,
>
> not sure if you are aware of the below (nothing I would address). Please handle :)
>
Thanks. At least some of these are handled by fixes in the following
kvm PR which hasn't landed in master yet:
https://lore.kernel.org/linux-s390/20260803160924.236807-1-imbrenda@linux.ibm.com/
But anyway: ACK - will check w/ Eric, if any of these aren't already
addressed we will follow up on them independent of this series.
Thanks,
Matt
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 1/3] s390/vfio_ccw: Add __must_hold() attribute to vfio_ccw_sch_quiesce()
2026-08-06 15:38 ` [PATCH 1/3] s390/vfio_ccw: Add __must_hold() attribute to vfio_ccw_sch_quiesce() Heiko Carstens
2026-08-06 15:52 ` sashiko-bot
@ 2026-08-06 17:34 ` Matthew Rosato
2026-08-07 10:55 ` Christian Borntraeger
2026-08-07 12:14 ` Christian Borntraeger
3 siblings, 0 replies; 16+ messages in thread
From: Matthew Rosato @ 2026-08-06 17:34 UTC (permalink / raw)
To: Heiko Carstens, Christian Borntraeger, Janosch Frank,
Claudio Imbrenda, Eric Farman, Vineeth Vijayan,
Peter Oberparleiter
Cc: linux-s390
On 8/6/26 11:38 AM, Heiko Carstens wrote:
> Add __must_hold() attribute to vfio_ccw_sch_quiesce() in order to let
> clang's context analysis know that sch->lock must be held on function
> entry. This can also be easily verified when inspecting the function.
>
> Without this annotation this leads to a valid warning when context
> analysis is enabled:
>
> drivers/s390/cio/vfio_ccw_drv.c:55:9: warning:
> expecting spinlock 'sch->lock' to be held at start of each loop [-Wthread-safety-analysis]
> 55 | ret = cio_cancel_halt_clear(sch, &iretry);
> | ^
>
> Signed-off-by: Heiko Carstens <hca@linux.ibm.com>
Reviewed-by: Matthew Rosato <mjrosato@linux.ibm.com>
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 3/3] s390/virtio: Enable CONTEXT_ANALYSIS
2026-08-06 15:38 ` [PATCH 3/3] s390/virtio: " Heiko Carstens
2026-08-06 15:40 ` sashiko-bot
@ 2026-08-06 17:35 ` Matthew Rosato
1 sibling, 0 replies; 16+ messages in thread
From: Matthew Rosato @ 2026-08-06 17:35 UTC (permalink / raw)
To: Heiko Carstens, Christian Borntraeger, Janosch Frank,
Claudio Imbrenda, Eric Farman, Vineeth Vijayan,
Peter Oberparleiter
Cc: linux-s390
On 8/6/26 11:38 AM, Heiko Carstens wrote:
> All virtio code passes clang's compile time context analysis.
> Therefore enable CONTEXT_ANALYSIS.
>
> Signed-off-by: Heiko Carstens <hca@linux.ibm.com>
Acked-by: Matthew Rosato <mjrosato@linux.ibm.com>
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 1/3] s390/vfio_ccw: Add __must_hold() attribute to vfio_ccw_sch_quiesce()
2026-08-06 15:38 ` [PATCH 1/3] s390/vfio_ccw: Add __must_hold() attribute to vfio_ccw_sch_quiesce() Heiko Carstens
2026-08-06 15:52 ` sashiko-bot
2026-08-06 17:34 ` Matthew Rosato
@ 2026-08-07 10:55 ` Christian Borntraeger
2026-08-07 11:15 ` Heiko Carstens
2026-08-07 12:14 ` Christian Borntraeger
3 siblings, 1 reply; 16+ messages in thread
From: Christian Borntraeger @ 2026-08-07 10:55 UTC (permalink / raw)
To: Heiko Carstens, Janosch Frank, Claudio Imbrenda, Eric Farman,
Matthew Rosato, Vineeth Vijayan, Peter Oberparleiter
Cc: linux-s390
Am 06.08.26 um 17:38 schrieb Heiko Carstens:
> Add __must_hold() attribute to vfio_ccw_sch_quiesce() in order to let
> clang's context analysis know that sch->lock must be held on function
> entry. This can also be easily verified when inspecting the function.
>
> Without this annotation this leads to a valid warning when context
> analysis is enabled:
>
> drivers/s390/cio/vfio_ccw_drv.c:55:9: warning:
> expecting spinlock 'sch->lock' to be held at start of each loop [-Wthread-safety-analysis]
> 55 | ret = cio_cancel_halt_clear(sch, &iretry);
> | ^
>
> Signed-off-by: Heiko Carstens <hca@linux.ibm.com>
Acked-by: Christian Borntraeger <borntraeger@linux.ibm.com>
I assume this series should go via the s390 tree with the others?
> ---
> drivers/s390/cio/vfio_ccw_drv.c | 1 +
> drivers/s390/cio/vfio_ccw_private.h | 3 ++-
> 2 files changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/s390/cio/vfio_ccw_drv.c b/drivers/s390/cio/vfio_ccw_drv.c
> index 1a095085bc72..ed9ca77c5b23 100644
> --- a/drivers/s390/cio/vfio_ccw_drv.c
> +++ b/drivers/s390/cio/vfio_ccw_drv.c
> @@ -35,6 +35,7 @@ debug_info_t *vfio_ccw_debug_trace_id;
> * Helpers
> */
> int vfio_ccw_sch_quiesce(struct subchannel *sch)
> + __must_hold(&sch->lock);
> {
> struct vfio_ccw_parent *parent = dev_get_drvdata(&sch->dev);
> struct vfio_ccw_private *private = dev_get_drvdata(&parent->dev);
> diff --git a/drivers/s390/cio/vfio_ccw_private.h b/drivers/s390/cio/vfio_ccw_private.h
> index 0501d4bbcdbd..8db29519dbfd 100644
> --- a/drivers/s390/cio/vfio_ccw_private.h
> +++ b/drivers/s390/cio/vfio_ccw_private.h
> @@ -127,7 +127,8 @@ struct vfio_ccw_private {
> struct work_struct crw_work;
> } __aligned(8);
>
> -int vfio_ccw_sch_quiesce(struct subchannel *sch);
> +int vfio_ccw_sch_quiesce(struct subchannel *sch)
> + __must_hold(&sch->lock);
> void vfio_ccw_sch_io_todo(struct work_struct *work);
> void vfio_ccw_crw_todo(struct work_struct *work);
>
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 1/3] s390/vfio_ccw: Add __must_hold() attribute to vfio_ccw_sch_quiesce()
2026-08-07 10:55 ` Christian Borntraeger
@ 2026-08-07 11:15 ` Heiko Carstens
0 siblings, 0 replies; 16+ messages in thread
From: Heiko Carstens @ 2026-08-07 11:15 UTC (permalink / raw)
To: Christian Borntraeger
Cc: Janosch Frank, Claudio Imbrenda, Eric Farman, Matthew Rosato,
Vineeth Vijayan, Peter Oberparleiter, linux-s390
On Fri, Aug 07, 2026 at 12:55:28PM +0200, Christian Borntraeger wrote:
>
>
> Am 06.08.26 um 17:38 schrieb Heiko Carstens:
> > Add __must_hold() attribute to vfio_ccw_sch_quiesce() in order to let
> > clang's context analysis know that sch->lock must be held on function
> > entry. This can also be easily verified when inspecting the function.
> >
> > Without this annotation this leads to a valid warning when context
> > analysis is enabled:
> >
> > drivers/s390/cio/vfio_ccw_drv.c:55:9: warning:
> > expecting spinlock 'sch->lock' to be held at start of each loop [-Wthread-safety-analysis]
> > 55 | ret = cio_cancel_halt_clear(sch, &iretry);
> > | ^
> >
> > Signed-off-by: Heiko Carstens <hca@linux.ibm.com>
>
> Acked-by: Christian Borntraeger <borntraeger@linux.ibm.com>
>
> I assume this series should go via the s390 tree with the others?
As written in the cover-letter:
"Christian, Claudio, Janosch, I think it would make most sense if this
goes via kvms390."
:)
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 1/3] s390/vfio_ccw: Add __must_hold() attribute to vfio_ccw_sch_quiesce()
2026-08-06 15:38 ` [PATCH 1/3] s390/vfio_ccw: Add __must_hold() attribute to vfio_ccw_sch_quiesce() Heiko Carstens
` (2 preceding siblings ...)
2026-08-07 10:55 ` Christian Borntraeger
@ 2026-08-07 12:14 ` Christian Borntraeger
2026-08-07 12:29 ` Heiko Carstens
3 siblings, 1 reply; 16+ messages in thread
From: Christian Borntraeger @ 2026-08-07 12:14 UTC (permalink / raw)
To: Heiko Carstens, Janosch Frank, Claudio Imbrenda, Eric Farman,
Matthew Rosato, Vineeth Vijayan, Peter Oberparleiter
Cc: linux-s390
Am 06.08.26 um 17:38 schrieb Heiko Carstens:
> Add __must_hold() attribute to vfio_ccw_sch_quiesce() in order to let
> clang's context analysis know that sch->lock must be held on function
> entry. This can also be easily verified when inspecting the function.
>
> Without this annotation this leads to a valid warning when context
> analysis is enabled:
>
> drivers/s390/cio/vfio_ccw_drv.c:55:9: warning:
> expecting spinlock 'sch->lock' to be held at start of each loop [-Wthread-safety-analysis]
> 55 | ret = cio_cancel_halt_clear(sch, &iretry);
> | ^
>
> Signed-off-by: Heiko Carstens <hca@linux.ibm.com>
> ---
> drivers/s390/cio/vfio_ccw_drv.c | 1 +
> drivers/s390/cio/vfio_ccw_private.h | 3 ++-
> 2 files changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/s390/cio/vfio_ccw_drv.c b/drivers/s390/cio/vfio_ccw_drv.c
> index 1a095085bc72..ed9ca77c5b23 100644
> --- a/drivers/s390/cio/vfio_ccw_drv.c
> +++ b/drivers/s390/cio/vfio_ccw_drv.c
> @@ -35,6 +35,7 @@ debug_info_t *vfio_ccw_debug_trace_id;
> * Helpers
> */
> int vfio_ccw_sch_quiesce(struct subchannel *sch)
> + __must_hold(&sch->lock);
Looks like this ; is wrong ^
will fixup when applying.
> {
> struct vfio_ccw_parent *parent = dev_get_drvdata(&sch->dev);
> struct vfio_ccw_private *private = dev_get_drvdata(&parent->dev);
> diff --git a/drivers/s390/cio/vfio_ccw_private.h b/drivers/s390/cio/vfio_ccw_private.h
> index 0501d4bbcdbd..8db29519dbfd 100644
> --- a/drivers/s390/cio/vfio_ccw_private.h
> +++ b/drivers/s390/cio/vfio_ccw_private.h
> @@ -127,7 +127,8 @@ struct vfio_ccw_private {
> struct work_struct crw_work;
> } __aligned(8);
>
> -int vfio_ccw_sch_quiesce(struct subchannel *sch);
> +int vfio_ccw_sch_quiesce(struct subchannel *sch)
> + __must_hold(&sch->lock);
> void vfio_ccw_sch_io_todo(struct work_struct *work);
> void vfio_ccw_crw_todo(struct work_struct *work);
>
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 1/3] s390/vfio_ccw: Add __must_hold() attribute to vfio_ccw_sch_quiesce()
2026-08-07 12:14 ` Christian Borntraeger
@ 2026-08-07 12:29 ` Heiko Carstens
0 siblings, 0 replies; 16+ messages in thread
From: Heiko Carstens @ 2026-08-07 12:29 UTC (permalink / raw)
To: Christian Borntraeger
Cc: Janosch Frank, Claudio Imbrenda, Eric Farman, Matthew Rosato,
Vineeth Vijayan, Peter Oberparleiter, linux-s390
On Fri, Aug 07, 2026 at 02:14:27PM +0200, Christian Borntraeger wrote:
> > int vfio_ccw_sch_quiesce(struct subchannel *sch)
> > + __must_hold(&sch->lock);
> Looks like this ; is wrong ^
>
> will fixup when applying.
Right. Thanks!
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 0/3] s390: Enable CONTEXT_ANALYSIS for cio and virtio
2026-08-06 15:38 [PATCH 0/3] s390: Enable CONTEXT_ANALYSIS for cio and virtio Heiko Carstens
` (2 preceding siblings ...)
2026-08-06 15:38 ` [PATCH 3/3] s390/virtio: " Heiko Carstens
@ 2026-08-07 12:31 ` Christian Borntraeger
3 siblings, 0 replies; 16+ messages in thread
From: Christian Borntraeger @ 2026-08-07 12:31 UTC (permalink / raw)
To: Heiko Carstens, Janosch Frank, Claudio Imbrenda, Eric Farman,
Matthew Rosato, Vineeth Vijayan, Peter Oberparleiter
Cc: linux-s390
Am 06.08.26 um 17:38 schrieb Heiko Carstens:
> Enable CONTEXT_ANALYSYS for cio and virtio.
>
> 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 cio and virtio. Add a __must_hold() attribute to
> vfio_ccw_sch_quiesce() to address the only valid warning.
>
> Then enable CONTEXT_ANALYSIS for both cio and virtio.
>
> Christian, Claudio, Janosch, I think it would make most sense if this
> goes via kvms390.
applied with the small ";" fixup and scheduled for next.
Thanks you.
^ permalink raw reply [flat|nested] 16+ messages in thread
end of thread, other threads:[~2026-08-07 12:31 UTC | newest]
Thread overview: 16+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-08-06 15:38 [PATCH 0/3] s390: Enable CONTEXT_ANALYSIS for cio and virtio Heiko Carstens
2026-08-06 15:38 ` [PATCH 1/3] s390/vfio_ccw: Add __must_hold() attribute to vfio_ccw_sch_quiesce() Heiko Carstens
2026-08-06 15:52 ` sashiko-bot
2026-08-06 16:02 ` Heiko Carstens
2026-08-06 17:26 ` Matthew Rosato
2026-08-06 17:34 ` Matthew Rosato
2026-08-07 10:55 ` Christian Borntraeger
2026-08-07 11:15 ` Heiko Carstens
2026-08-07 12:14 ` Christian Borntraeger
2026-08-07 12:29 ` Heiko Carstens
2026-08-06 15:38 ` [PATCH 2/3] s390/cio: Enable CONTEXT_ANALYSIS Heiko Carstens
2026-08-06 15:42 ` sashiko-bot
2026-08-06 15:38 ` [PATCH 3/3] s390/virtio: " Heiko Carstens
2026-08-06 15:40 ` sashiko-bot
2026-08-06 17:35 ` Matthew Rosato
2026-08-07 12:31 ` [PATCH 0/3] s390: Enable CONTEXT_ANALYSIS for cio and virtio Christian Borntraeger
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox