Linux s390 Architecture development
 help / color / mirror / Atom feed
* [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