diff mbox series

[v1,1/7] vfio/ccw: create a parent struct

Message ID 20221019162135.798901-2-farman@linux.ibm.com (mailing list archive)
State New, archived
Headers show
Series vfio-ccw parent rework | expand

Commit Message

Eric Farman Oct. 19, 2022, 4:21 p.m. UTC 
Move the stuff associated with the mdev parent (and thus the
subchannel struct) into its own struct, and leave the rest in
the existing private structure.

The subchannel will point to the parent, and the parent will point
to the private, for the areas where one or both are needed. Further
separation of these structs will follow.

Signed-off-by: Eric Farman <farman@linux.ibm.com>
---
 drivers/s390/cio/vfio_ccw_drv.c     | 104 ++++++++++++++++++++--------
 drivers/s390/cio/vfio_ccw_ops.c     |   9 ++-
 drivers/s390/cio/vfio_ccw_parent.h  |  28 ++++++++
 drivers/s390/cio/vfio_ccw_private.h |   5 --
 4 files changed, 112 insertions(+), 34 deletions(-)
 create mode 100644 drivers/s390/cio/vfio_ccw_parent.h

Comments

Eric Farman Oct. 27, 2022, 8:32 p.m. UTC | #1 
On Wed, 2022-10-19 at 18:21 +0200, Eric Farman wrote:
> Move the stuff associated with the mdev parent (and thus the
> subchannel struct) into its own struct, and leave the rest in
> the existing private structure.
> 
> The subchannel will point to the parent, and the parent will point
> to the private, for the areas where one or both are needed. Further
> separation of these structs will follow.
> 
> Signed-off-by: Eric Farman <farman@linux.ibm.com>
> ---
>  drivers/s390/cio/vfio_ccw_drv.c     | 104 ++++++++++++++++++++------
> --
>  drivers/s390/cio/vfio_ccw_ops.c     |   9 ++-
>  drivers/s390/cio/vfio_ccw_parent.h  |  28 ++++++++
>  drivers/s390/cio/vfio_ccw_private.h |   5 --
>  4 files changed, 112 insertions(+), 34 deletions(-)
>  create mode 100644 drivers/s390/cio/vfio_ccw_parent.h
> 
> diff --git a/drivers/s390/cio/vfio_ccw_drv.c
> b/drivers/s390/cio/vfio_ccw_drv.c
> index 7f5402fe857a..634760ca0dea 100644
> --- a/drivers/s390/cio/vfio_ccw_drv.c
> +++ b/drivers/s390/cio/vfio_ccw_drv.c

...snip...

> @@ -213,18 +241,28 @@ static int vfio_ccw_sch_probe(struct subchannel
> *sch)
>                 return -ENODEV;
>         }
>  
> +       parent = kzalloc(sizeof(*parent), GFP_KERNEL);
> +       if (IS_ERR(parent))
> +               return PTR_ERR(parent);
> +
> +       parent->dev.release = &vfio_ccw_free_parent;
> +       device_initialize(&parent->dev);
> +

Oops. This should either be device_register, or I needed a device_add
somewhere along the way. There's no need to separate them, so the
former is probably better, but I still need some additional logic to
link this back to the css driver.

>         private = vfio_ccw_alloc_private(sch);
> -       if (IS_ERR(private))
> +       if (IS_ERR(private)) {
> +               put_device(&parent->dev);
>                 return PTR_ERR(private);
> +       }
>  
> -       dev_set_drvdata(&sch->dev, private);
> +       dev_set_drvdata(&sch->dev, parent);
> +       dev_set_drvdata(&parent->dev, private);
>  
> -       private->mdev_type.sysfs_name = "io";
> -       private->mdev_type.pretty_name = "I/O subchannel (Non-QDIO)";
> -       private->mdev_types[0] = &private->mdev_type;
> -       ret = mdev_register_parent(&private->parent, &sch->dev,
> +       parent->mdev_type.sysfs_name = "io";
> +       parent->mdev_type.pretty_name = "I/O subchannel (Non-QDIO)";
> +       parent->mdev_types[0] = &parent->mdev_type;
> +       ret = mdev_register_parent(&parent->parent, &sch->dev,
>                                    &vfio_ccw_mdev_driver,
> -                                  private->mdev_types, 1);
> +                                  parent->mdev_types, 1);
>         if (ret)
>                 goto out_free;
>  
> @@ -234,20 +272,24 @@ static int vfio_ccw_sch_probe(struct subchannel
> *sch)
>         return 0;
>  
>  out_free:
> +       dev_set_drvdata(&parent->dev, NULL);
>         dev_set_drvdata(&sch->dev, NULL);
>         vfio_ccw_free_private(private);
> +       put_device(&parent->dev);
>         return ret;
>  }
>  
>  static void vfio_ccw_sch_remove(struct subchannel *sch)
>  {
> -       struct vfio_ccw_private *private = dev_get_drvdata(&sch-
> >dev);
> +       struct vfio_ccw_parent *parent = dev_get_drvdata(&sch->dev);
> +       struct vfio_ccw_private *private = dev_get_drvdata(&parent-
> >dev);
>  
> -       mdev_unregister_parent(&private->parent);
> +       mdev_unregister_parent(&parent->parent);
>  
>         dev_set_drvdata(&sch->dev, NULL);
>  
>         vfio_ccw_free_private(private);
> +       put_device(&parent->dev);
>  
>         VFIO_CCW_MSG_EVENT(4, "unbound from subchannel %x.%x.%04x\n",
>                            sch->schid.cssid, sch->schid.ssid,
> @@ -256,7 +298,11 @@ static void vfio_ccw_sch_remove(struct
> subchannel *sch)
>  
>  static void vfio_ccw_sch_shutdown(struct subchannel *sch)
>  {
> -       struct vfio_ccw_private *private = dev_get_drvdata(&sch-
> >dev);
> +       struct vfio_ccw_parent *parent = dev_get_drvdata(&sch->dev);
> +       struct vfio_ccw_private *private = dev_get_drvdata(&parent-
> >dev);
> +
> +       if (WARN_ON(!private))
> +               return;
>  
>         vfio_ccw_fsm_event(private, VFIO_CCW_EVENT_CLOSE);
>         vfio_ccw_fsm_event(private, VFIO_CCW_EVENT_NOT_OPER);
> @@ -274,7 +320,8 @@ static void vfio_ccw_sch_shutdown(struct
> subchannel *sch)
>   */
>  static int vfio_ccw_sch_event(struct subchannel *sch, int process)
>  {
> -       struct vfio_ccw_private *private = dev_get_drvdata(&sch-
> >dev);
> +       struct vfio_ccw_parent *parent = dev_get_drvdata(&sch->dev);
> +       struct vfio_ccw_private *private = dev_get_drvdata(&parent-
> >dev);
>         unsigned long flags;
>         int rc = -EAGAIN;
>  
> @@ -287,8 +334,10 @@ static int vfio_ccw_sch_event(struct subchannel
> *sch, int process)
>  
>         rc = 0;
>  
> -       if (cio_update_schib(sch))
> -               vfio_ccw_fsm_event(private, VFIO_CCW_EVENT_NOT_OPER);
> +       if (cio_update_schib(sch)) {
> +               if (private)
> +                       vfio_ccw_fsm_event(private,
> VFIO_CCW_EVENT_NOT_OPER);
> +       }
>  
>  out_unlock:
>         spin_unlock_irqrestore(sch->lock, flags);
> @@ -326,7 +375,8 @@ static void vfio_ccw_queue_crw(struct
> vfio_ccw_private *private,
>  static int vfio_ccw_chp_event(struct subchannel *sch,
>                               struct chp_link *link, int event)
>  {
> -       struct vfio_ccw_private *private = dev_get_drvdata(&sch-
> >dev);
> +       struct vfio_ccw_parent *parent = dev_get_drvdata(&sch->dev);
> +       struct vfio_ccw_private *private = dev_get_drvdata(&parent-
> >dev);
>         int mask = chp_ssd_get_mask(&sch->ssd_info, link);
>         int retry = 255;
>  
> diff --git a/drivers/s390/cio/vfio_ccw_ops.c
> b/drivers/s390/cio/vfio_ccw_ops.c
> index 6ae4d012d800..b1cd89d900ab 100644
> --- a/drivers/s390/cio/vfio_ccw_ops.c
> +++ b/drivers/s390/cio/vfio_ccw_ops.c
> @@ -14,6 +14,7 @@
>  #include <linux/nospec.h>
>  #include <linux/slab.h>
>  
> +#include "vfio_ccw_parent.h"
>  #include "vfio_ccw_private.h"
>  
>  static const struct vfio_device_ops vfio_ccw_dev_ops;
> @@ -55,7 +56,9 @@ static int vfio_ccw_mdev_init_dev(struct
> vfio_device *vdev)
>  
>  static int vfio_ccw_mdev_probe(struct mdev_device *mdev)
>  {
> -       struct vfio_ccw_private *private = dev_get_drvdata(mdev-
> >dev.parent);
> +       struct subchannel *sch = to_subchannel(mdev->dev.parent);
> +       struct vfio_ccw_parent *parent = dev_get_drvdata(&sch->dev);
> +       struct vfio_ccw_private *private = dev_get_drvdata(&parent-
> >dev);
>         int ret;
>  
>         if (private->state == VFIO_CCW_STATE_NOT_OPER)
> @@ -100,7 +103,9 @@ static void vfio_ccw_mdev_release_dev(struct
> vfio_device *vdev)
>  
>  static void vfio_ccw_mdev_remove(struct mdev_device *mdev)
>  {
> -       struct vfio_ccw_private *private = dev_get_drvdata(mdev-
> >dev.parent);
> +       struct subchannel *sch = to_subchannel(mdev->dev.parent);
> +       struct vfio_ccw_parent *parent = dev_get_drvdata(&sch->dev);
> +       struct vfio_ccw_private *private = dev_get_drvdata(&parent-
> >dev);
>  
>         VFIO_CCW_MSG_EVENT(2, "sch %x.%x.%04x: remove\n",
>                            private->sch->schid.cssid,
> diff --git a/drivers/s390/cio/vfio_ccw_parent.h
> b/drivers/s390/cio/vfio_ccw_parent.h
> new file mode 100644
> index 000000000000..834c00077802
> --- /dev/null
> +++ b/drivers/s390/cio/vfio_ccw_parent.h
> @@ -0,0 +1,28 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/*
> + * MDEV Parent contents for vfio_ccw driver
> + *
> + * Copyright IBM Corp. 2022
> + */
> +
> +#ifndef _VFIO_CCW_PARENT_H_
> +#define _VFIO_CCW_PARENT_H_
> +
> +#include <linux/mdev.h>
> +
> +/**
> + * struct vfio_ccw_parent
> + *
> + * @dev: embedded device struct
> + * @parent: parent data structures for mdevs created
> + * @mdev_type(s): identifying information for mdevs created
> + */
> +struct vfio_ccw_parent {
> +       struct device           dev;
> +
> +       struct mdev_parent      parent;
> +       struct mdev_type        mdev_type;
> +       struct mdev_type        *mdev_types[1];
> +};
> +
> +#endif
> diff --git a/drivers/s390/cio/vfio_ccw_private.h
> b/drivers/s390/cio/vfio_ccw_private.h
> index bd5fb81456af..673e9a81a172 100644
> --- a/drivers/s390/cio/vfio_ccw_private.h
> +++ b/drivers/s390/cio/vfio_ccw_private.h
> @@ -89,7 +89,6 @@ struct vfio_ccw_crw {
>   * @io_work: work for deferral process of I/O handling
>   * @crw_work: work for deferral process of CRW handling
>   * @release_comp: synchronization helper for vfio device release
> - * @parent: parent data structures for mdevs created
>   */
>  struct vfio_ccw_private {
>         struct vfio_device vdev;
> @@ -116,10 +115,6 @@ struct vfio_ccw_private {
>         struct work_struct      crw_work;
>  
>         struct completion       release_comp;
> -
> -       struct mdev_parent      parent;
> -       struct mdev_type        mdev_type;
> -       struct mdev_type        *mdev_types[1];
>  } __aligned(8);
>  
>  int vfio_ccw_sch_quiesce(struct subchannel *sch);
Matthew Rosato Oct. 28, 2022, 4:51 p.m. UTC | #2 
On 10/19/22 12:21 PM, Eric Farman wrote:
> Move the stuff associated with the mdev parent (and thus the
> subchannel struct) into its own struct, and leave the rest in
> the existing private structure.
> 
> The subchannel will point to the parent, and the parent will point
> to the private, for the areas where one or both are needed. Further
> separation of these structs will follow.
> 
> Signed-off-by: Eric Farman <farman@linux.ibm.com>
> ---
>  drivers/s390/cio/vfio_ccw_drv.c     | 104 ++++++++++++++++++++--------
>  drivers/s390/cio/vfio_ccw_ops.c     |   9 ++-
>  drivers/s390/cio/vfio_ccw_parent.h  |  28 ++++++++
>  drivers/s390/cio/vfio_ccw_private.h |   5 --
>  4 files changed, 112 insertions(+), 34 deletions(-)
>  create mode 100644 drivers/s390/cio/vfio_ccw_parent.h
> 
> diff --git a/drivers/s390/cio/vfio_ccw_drv.c b/drivers/s390/cio/vfio_ccw_drv.c
> index 7f5402fe857a..634760ca0dea 100644
> --- a/drivers/s390/cio/vfio_ccw_drv.c
> +++ b/drivers/s390/cio/vfio_ccw_drv.c
> @@ -20,6 +20,7 @@
>  #include "chp.h"
>  #include "ioasm.h"
>  #include "css.h"
> +#include "vfio_ccw_parent.h"
>  #include "vfio_ccw_private.h"
>  
>  struct workqueue_struct *vfio_ccw_work_q;
> @@ -36,7 +37,8 @@ debug_info_t *vfio_ccw_debug_trace_id;
>   */
>  int vfio_ccw_sch_quiesce(struct subchannel *sch)
>  {
> -	struct vfio_ccw_private *private = dev_get_drvdata(&sch->dev);
> +	struct vfio_ccw_parent *parent = dev_get_drvdata(&sch->dev);
> +	struct vfio_ccw_private *private = dev_get_drvdata(&parent->dev);
>  	DECLARE_COMPLETION_ONSTACK(completion);
>  	int iretry, ret = 0;
>  
> @@ -51,19 +53,21 @@ int vfio_ccw_sch_quiesce(struct subchannel *sch)
>  			break;
>  		}
>  
> -		/*
> -		 * Flush all I/O and wait for
> -		 * cancel/halt/clear completion.
> -		 */
> -		private->completion = &completion;
> -		spin_unlock_irq(sch->lock);
> +		if (private) {

Is it valid to ever reach this code with private == NULL?  If no, then this should probably be a WARN_ON upfront?

> +			/*
> +			 * Flush all I/O and wait for
> +			 * cancel/halt/clear completion.
> +			 */
> +			private->completion = &completion;
> +			spin_unlock_irq(sch->lock);
>  
> -		if (ret == -EBUSY)
> -			wait_for_completion_timeout(&completion, 3*HZ);
> +			if (ret == -EBUSY)
> +				wait_for_completion_timeout(&completion, 3*HZ);
>  
> -		private->completion = NULL;
> -		flush_workqueue(vfio_ccw_work_q);
> -		spin_lock_irq(sch->lock);
> +			private->completion = NULL;
> +			flush_workqueue(vfio_ccw_work_q);
> +			spin_lock_irq(sch->lock);
> +		}
>  		ret = cio_disable_subchannel(sch);
>  	} while (ret == -EBUSY);
>  

.. snip ..

> diff --git a/drivers/s390/cio/vfio_ccw_parent.h b/drivers/s390/cio/vfio_ccw_parent.h
> new file mode 100644
> index 000000000000..834c00077802
> --- /dev/null
> +++ b/drivers/s390/cio/vfio_ccw_parent.h
> @@ -0,0 +1,28 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/*
> + * MDEV Parent contents for vfio_ccw driver
> + *
> + * Copyright IBM Corp. 2022
> + */
> +
> +#ifndef _VFIO_CCW_PARENT_H_
> +#define _VFIO_CCW_PARENT_H_
> +
> +#include <linux/mdev.h>
> +
> +/**
> + * struct vfio_ccw_parent
> + *
> + * @dev: embedded device struct
> + * @parent: parent data structures for mdevs created
> + * @mdev_type(s): identifying information for mdevs created
> + */
> +struct vfio_ccw_parent {
> +	struct device		dev;
> +
> +	struct mdev_parent	parent;
> +	struct mdev_type	mdev_type;
> +	struct mdev_type	*mdev_types[1];
> +};

Structure itself seems fine, but any reason we need a new file for it?
Eric Farman Oct. 28, 2022, 5:21 p.m. UTC | #3 
On Fri, 2022-10-28 at 12:51 -0400, Matthew Rosato wrote:
> On 10/19/22 12:21 PM, Eric Farman wrote:
> > Move the stuff associated with the mdev parent (and thus the
> > subchannel struct) into its own struct, and leave the rest in
> > the existing private structure.
> > 
> > The subchannel will point to the parent, and the parent will point
> > to the private, for the areas where one or both are needed. Further
> > separation of these structs will follow.
> > 
> > Signed-off-by: Eric Farman <farman@linux.ibm.com>
> > ---
> >  drivers/s390/cio/vfio_ccw_drv.c     | 104 ++++++++++++++++++++----
> > ----
> >  drivers/s390/cio/vfio_ccw_ops.c     |   9 ++-
> >  drivers/s390/cio/vfio_ccw_parent.h  |  28 ++++++++
> >  drivers/s390/cio/vfio_ccw_private.h |   5 --
> >  4 files changed, 112 insertions(+), 34 deletions(-)
> >  create mode 100644 drivers/s390/cio/vfio_ccw_parent.h
> > 
> > diff --git a/drivers/s390/cio/vfio_ccw_drv.c
> > b/drivers/s390/cio/vfio_ccw_drv.c
> > index 7f5402fe857a..634760ca0dea 100644
> > --- a/drivers/s390/cio/vfio_ccw_drv.c
> > +++ b/drivers/s390/cio/vfio_ccw_drv.c
> > @@ -20,6 +20,7 @@
> >  #include "chp.h"
> >  #include "ioasm.h"
> >  #include "css.h"
> > +#include "vfio_ccw_parent.h"
> >  #include "vfio_ccw_private.h"
> >  
> >  struct workqueue_struct *vfio_ccw_work_q;
> > @@ -36,7 +37,8 @@ debug_info_t *vfio_ccw_debug_trace_id;
> >   */
> >  int vfio_ccw_sch_quiesce(struct subchannel *sch)
> >  {
> > -       struct vfio_ccw_private *private = dev_get_drvdata(&sch-
> > >dev);
> > +       struct vfio_ccw_parent *parent = dev_get_drvdata(&sch-
> > >dev);
> > +       struct vfio_ccw_private *private = dev_get_drvdata(&parent-
> > >dev);
> >         DECLARE_COMPLETION_ONSTACK(completion);
> >         int iretry, ret = 0;
> >  
> > @@ -51,19 +53,21 @@ int vfio_ccw_sch_quiesce(struct subchannel
> > *sch)
> >                         break;
> >                 }
> >  
> > -               /*
> > -                * Flush all I/O and wait for
> > -                * cancel/halt/clear completion.
> > -                */
> > -               private->completion = &completion;
> > -               spin_unlock_irq(sch->lock);
> > +               if (private) {
> 
> Is it valid to ever reach this code with private == NULL?  If no,
> then this should probably be a WARN_ON upfront?

Hrm, the caller jumps from private -> subchannel, so it would be weird
if we couldn't then go back the other way. Probably impossible, I'll
unwind these whitespace changes and put the WARN_ON on top. Thanks for
the tip.

> 
> > +                       /*
> > +                        * Flush all I/O and wait for
> > +                        * cancel/halt/clear completion.
> > +                        */
> > +                       private->completion = &completion;
> > +                       spin_unlock_irq(sch->lock);
> >  
> > -               if (ret == -EBUSY)
> > -                       wait_for_completion_timeout(&completion,
> > 3*HZ);
> > +                       if (ret == -EBUSY)
> > +                               wait_for_completion_timeout(&comple
> > tion, 3*HZ);
> >  
> > -               private->completion = NULL;
> > -               flush_workqueue(vfio_ccw_work_q);
> > -               spin_lock_irq(sch->lock);
> > +                       private->completion = NULL;
> > +                       flush_workqueue(vfio_ccw_work_q);
> > +                       spin_lock_irq(sch->lock);
> > +               }
> >                 ret = cio_disable_subchannel(sch);
> >         } while (ret == -EBUSY);
> >  
> 
> .. snip ..
> 
> > diff --git a/drivers/s390/cio/vfio_ccw_parent.h
> > b/drivers/s390/cio/vfio_ccw_parent.h
> > new file mode 100644
> > index 000000000000..834c00077802
> > --- /dev/null
> > +++ b/drivers/s390/cio/vfio_ccw_parent.h
> > @@ -0,0 +1,28 @@
> > +/* SPDX-License-Identifier: GPL-2.0 */
> > +/*
> > + * MDEV Parent contents for vfio_ccw driver
> > + *
> > + * Copyright IBM Corp. 2022
> > + */
> > +
> > +#ifndef _VFIO_CCW_PARENT_H_
> > +#define _VFIO_CCW_PARENT_H_
> > +
> > +#include <linux/mdev.h>
> > +
> > +/**
> > + * struct vfio_ccw_parent
> > + *
> > + * @dev: embedded device struct
> > + * @parent: parent data structures for mdevs created
> > + * @mdev_type(s): identifying information for mdevs created
> > + */
> > +struct vfio_ccw_parent {
> > +       struct device           dev;
> > +
> > +       struct mdev_parent      parent;
> > +       struct mdev_type        mdev_type;
> > +       struct mdev_type        *mdev_types[1];
> > +};
> 
> Structure itself seems fine, but any reason we need a new file for
> it?
> 

Not really. I could leave it in _private.h, but that file is just a
dumping ground for everything so I thought this would be a good
opportunity to start to cleaning that up. But it wouldn't bother me to
leave that whole process to another day too.
diff mbox series

Patch

diff --git a/drivers/s390/cio/vfio_ccw_drv.c b/drivers/s390/cio/vfio_ccw_drv.c
index 7f5402fe857a..634760ca0dea 100644
--- a/drivers/s390/cio/vfio_ccw_drv.c
+++ b/drivers/s390/cio/vfio_ccw_drv.c
@@ -20,6 +20,7 @@ 
 #include "chp.h"
 #include "ioasm.h"
 #include "css.h"
+#include "vfio_ccw_parent.h"
 #include "vfio_ccw_private.h"
 
 struct workqueue_struct *vfio_ccw_work_q;
@@ -36,7 +37,8 @@  debug_info_t *vfio_ccw_debug_trace_id;
  */
 int vfio_ccw_sch_quiesce(struct subchannel *sch)
 {
-	struct vfio_ccw_private *private = dev_get_drvdata(&sch->dev);
+	struct vfio_ccw_parent *parent = dev_get_drvdata(&sch->dev);
+	struct vfio_ccw_private *private = dev_get_drvdata(&parent->dev);
 	DECLARE_COMPLETION_ONSTACK(completion);
 	int iretry, ret = 0;
 
@@ -51,19 +53,21 @@  int vfio_ccw_sch_quiesce(struct subchannel *sch)
 			break;
 		}
 
-		/*
-		 * Flush all I/O and wait for
-		 * cancel/halt/clear completion.
-		 */
-		private->completion = &completion;
-		spin_unlock_irq(sch->lock);
+		if (private) {
+			/*
+			 * Flush all I/O and wait for
+			 * cancel/halt/clear completion.
+			 */
+			private->completion = &completion;
+			spin_unlock_irq(sch->lock);
 
-		if (ret == -EBUSY)
-			wait_for_completion_timeout(&completion, 3*HZ);
+			if (ret == -EBUSY)
+				wait_for_completion_timeout(&completion, 3*HZ);
 
-		private->completion = NULL;
-		flush_workqueue(vfio_ccw_work_q);
-		spin_lock_irq(sch->lock);
+			private->completion = NULL;
+			flush_workqueue(vfio_ccw_work_q);
+			spin_lock_irq(sch->lock);
+		}
 		ret = cio_disable_subchannel(sch);
 	} while (ret == -EBUSY);
 
@@ -121,7 +125,22 @@  static void vfio_ccw_crw_todo(struct work_struct *work)
  */
 static void vfio_ccw_sch_irq(struct subchannel *sch)
 {
-	struct vfio_ccw_private *private = dev_get_drvdata(&sch->dev);
+	struct vfio_ccw_parent *parent = dev_get_drvdata(&sch->dev);
+	struct vfio_ccw_private *private = dev_get_drvdata(&parent->dev);
+
+	/*
+	 * The subchannel should still be disabled at this point,
+	 * so an interrupt would be quite surprising. As with an
+	 * interrupt while the FSM is closed, let's attempt to
+	 * disable the subchannel again.
+	 */
+	if (!private) {
+		VFIO_CCW_MSG_EVENT(2, "sch %x.%x.%04x: unexpected interrupt\n",
+			sch->schid.cssid, sch->schid.ssid, sch->schid.sch_no);
+
+		cio_disable_subchannel(sch);
+		return;
+	}
 
 	inc_irq_stat(IRQIO_CIO);
 	vfio_ccw_fsm_event(private, VFIO_CCW_EVENT_INTERRUPT);
@@ -201,10 +220,19 @@  static void vfio_ccw_free_private(struct vfio_ccw_private *private)
 	mutex_destroy(&private->io_mutex);
 	kfree(private);
 }
+
+static void vfio_ccw_free_parent(struct device *dev)
+{
+	struct vfio_ccw_parent *parent = container_of(dev, struct vfio_ccw_parent, dev);
+
+	kfree(parent);
+}
+
 static int vfio_ccw_sch_probe(struct subchannel *sch)
 {
 	struct pmcw *pmcw = &sch->schib.pmcw;
 	struct vfio_ccw_private *private;
+	struct vfio_ccw_parent *parent;
 	int ret = -ENOMEM;
 
 	if (pmcw->qf) {
@@ -213,18 +241,28 @@  static int vfio_ccw_sch_probe(struct subchannel *sch)
 		return -ENODEV;
 	}
 
+	parent = kzalloc(sizeof(*parent), GFP_KERNEL);
+	if (IS_ERR(parent))
+		return PTR_ERR(parent);
+
+	parent->dev.release = &vfio_ccw_free_parent;
+	device_initialize(&parent->dev);
+
 	private = vfio_ccw_alloc_private(sch);
-	if (IS_ERR(private))
+	if (IS_ERR(private)) {
+		put_device(&parent->dev);
 		return PTR_ERR(private);
+	}
 
-	dev_set_drvdata(&sch->dev, private);
+	dev_set_drvdata(&sch->dev, parent);
+	dev_set_drvdata(&parent->dev, private);
 
-	private->mdev_type.sysfs_name = "io";
-	private->mdev_type.pretty_name = "I/O subchannel (Non-QDIO)";
-	private->mdev_types[0] = &private->mdev_type;
-	ret = mdev_register_parent(&private->parent, &sch->dev,
+	parent->mdev_type.sysfs_name = "io";
+	parent->mdev_type.pretty_name = "I/O subchannel (Non-QDIO)";
+	parent->mdev_types[0] = &parent->mdev_type;
+	ret = mdev_register_parent(&parent->parent, &sch->dev,
 				   &vfio_ccw_mdev_driver,
-				   private->mdev_types, 1);
+				   parent->mdev_types, 1);
 	if (ret)
 		goto out_free;
 
@@ -234,20 +272,24 @@  static int vfio_ccw_sch_probe(struct subchannel *sch)
 	return 0;
 
 out_free:
+	dev_set_drvdata(&parent->dev, NULL);
 	dev_set_drvdata(&sch->dev, NULL);
 	vfio_ccw_free_private(private);
+	put_device(&parent->dev);
 	return ret;
 }
 
 static void vfio_ccw_sch_remove(struct subchannel *sch)
 {
-	struct vfio_ccw_private *private = dev_get_drvdata(&sch->dev);
+	struct vfio_ccw_parent *parent = dev_get_drvdata(&sch->dev);
+	struct vfio_ccw_private *private = dev_get_drvdata(&parent->dev);
 
-	mdev_unregister_parent(&private->parent);
+	mdev_unregister_parent(&parent->parent);
 
 	dev_set_drvdata(&sch->dev, NULL);
 
 	vfio_ccw_free_private(private);
+	put_device(&parent->dev);
 
 	VFIO_CCW_MSG_EVENT(4, "unbound from subchannel %x.%x.%04x\n",
 			   sch->schid.cssid, sch->schid.ssid,
@@ -256,7 +298,11 @@  static void vfio_ccw_sch_remove(struct subchannel *sch)
 
 static void vfio_ccw_sch_shutdown(struct subchannel *sch)
 {
-	struct vfio_ccw_private *private = dev_get_drvdata(&sch->dev);
+	struct vfio_ccw_parent *parent = dev_get_drvdata(&sch->dev);
+	struct vfio_ccw_private *private = dev_get_drvdata(&parent->dev);
+
+	if (WARN_ON(!private))
+		return;
 
 	vfio_ccw_fsm_event(private, VFIO_CCW_EVENT_CLOSE);
 	vfio_ccw_fsm_event(private, VFIO_CCW_EVENT_NOT_OPER);
@@ -274,7 +320,8 @@  static void vfio_ccw_sch_shutdown(struct subchannel *sch)
  */
 static int vfio_ccw_sch_event(struct subchannel *sch, int process)
 {
-	struct vfio_ccw_private *private = dev_get_drvdata(&sch->dev);
+	struct vfio_ccw_parent *parent = dev_get_drvdata(&sch->dev);
+	struct vfio_ccw_private *private = dev_get_drvdata(&parent->dev);
 	unsigned long flags;
 	int rc = -EAGAIN;
 
@@ -287,8 +334,10 @@  static int vfio_ccw_sch_event(struct subchannel *sch, int process)
 
 	rc = 0;
 
-	if (cio_update_schib(sch))
-		vfio_ccw_fsm_event(private, VFIO_CCW_EVENT_NOT_OPER);
+	if (cio_update_schib(sch)) {
+		if (private)
+			vfio_ccw_fsm_event(private, VFIO_CCW_EVENT_NOT_OPER);
+	}
 
 out_unlock:
 	spin_unlock_irqrestore(sch->lock, flags);
@@ -326,7 +375,8 @@  static void vfio_ccw_queue_crw(struct vfio_ccw_private *private,
 static int vfio_ccw_chp_event(struct subchannel *sch,
 			      struct chp_link *link, int event)
 {
-	struct vfio_ccw_private *private = dev_get_drvdata(&sch->dev);
+	struct vfio_ccw_parent *parent = dev_get_drvdata(&sch->dev);
+	struct vfio_ccw_private *private = dev_get_drvdata(&parent->dev);
 	int mask = chp_ssd_get_mask(&sch->ssd_info, link);
 	int retry = 255;
 
diff --git a/drivers/s390/cio/vfio_ccw_ops.c b/drivers/s390/cio/vfio_ccw_ops.c
index 6ae4d012d800..b1cd89d900ab 100644
--- a/drivers/s390/cio/vfio_ccw_ops.c
+++ b/drivers/s390/cio/vfio_ccw_ops.c
@@ -14,6 +14,7 @@ 
 #include <linux/nospec.h>
 #include <linux/slab.h>
 
+#include "vfio_ccw_parent.h"
 #include "vfio_ccw_private.h"
 
 static const struct vfio_device_ops vfio_ccw_dev_ops;
@@ -55,7 +56,9 @@  static int vfio_ccw_mdev_init_dev(struct vfio_device *vdev)
 
 static int vfio_ccw_mdev_probe(struct mdev_device *mdev)
 {
-	struct vfio_ccw_private *private = dev_get_drvdata(mdev->dev.parent);
+	struct subchannel *sch = to_subchannel(mdev->dev.parent);
+	struct vfio_ccw_parent *parent = dev_get_drvdata(&sch->dev);
+	struct vfio_ccw_private *private = dev_get_drvdata(&parent->dev);
 	int ret;
 
 	if (private->state == VFIO_CCW_STATE_NOT_OPER)
@@ -100,7 +103,9 @@  static void vfio_ccw_mdev_release_dev(struct vfio_device *vdev)
 
 static void vfio_ccw_mdev_remove(struct mdev_device *mdev)
 {
-	struct vfio_ccw_private *private = dev_get_drvdata(mdev->dev.parent);
+	struct subchannel *sch = to_subchannel(mdev->dev.parent);
+	struct vfio_ccw_parent *parent = dev_get_drvdata(&sch->dev);
+	struct vfio_ccw_private *private = dev_get_drvdata(&parent->dev);
 
 	VFIO_CCW_MSG_EVENT(2, "sch %x.%x.%04x: remove\n",
 			   private->sch->schid.cssid,
diff --git a/drivers/s390/cio/vfio_ccw_parent.h b/drivers/s390/cio/vfio_ccw_parent.h
new file mode 100644
index 000000000000..834c00077802
--- /dev/null
+++ b/drivers/s390/cio/vfio_ccw_parent.h
@@ -0,0 +1,28 @@ 
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * MDEV Parent contents for vfio_ccw driver
+ *
+ * Copyright IBM Corp. 2022
+ */
+
+#ifndef _VFIO_CCW_PARENT_H_
+#define _VFIO_CCW_PARENT_H_
+
+#include <linux/mdev.h>
+
+/**
+ * struct vfio_ccw_parent
+ *
+ * @dev: embedded device struct
+ * @parent: parent data structures for mdevs created
+ * @mdev_type(s): identifying information for mdevs created
+ */
+struct vfio_ccw_parent {
+	struct device		dev;
+
+	struct mdev_parent	parent;
+	struct mdev_type	mdev_type;
+	struct mdev_type	*mdev_types[1];
+};
+
+#endif
diff --git a/drivers/s390/cio/vfio_ccw_private.h b/drivers/s390/cio/vfio_ccw_private.h
index bd5fb81456af..673e9a81a172 100644
--- a/drivers/s390/cio/vfio_ccw_private.h
+++ b/drivers/s390/cio/vfio_ccw_private.h
@@ -89,7 +89,6 @@  struct vfio_ccw_crw {
  * @io_work: work for deferral process of I/O handling
  * @crw_work: work for deferral process of CRW handling
  * @release_comp: synchronization helper for vfio device release
- * @parent: parent data structures for mdevs created
  */
 struct vfio_ccw_private {
 	struct vfio_device vdev;
@@ -116,10 +115,6 @@  struct vfio_ccw_private {
 	struct work_struct	crw_work;
 
 	struct completion	release_comp;
-
-	struct mdev_parent	parent;
-	struct mdev_type	mdev_type;
-	struct mdev_type	*mdev_types[1];
 } __aligned(8);
 
 int vfio_ccw_sch_quiesce(struct subchannel *sch);