* [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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.