Re: [Intel-gfx] [RE]: [PATCH v3 10/10] vfio/ccw: Move the lifecycle of the struct vfio_ccw_private to the mdev
On Mon, 2022-01-17 at 11:35 -0400, Jason Gunthorpe wrote: > On Fri, Jan 14, 2022 at 11:30:36AM -0500, Eric Farman wrote: > > On Fri, 2022-01-14 at 20:28 +0800, Liu Yi L wrote: > > > Hi Eric, > > > > > > Hope you are back from new year holiday.:-) Have you got chane to > > > consider > > > this patch? > > > > Hi Yi Liu, > > > > It's coming up the list, but it's not there yet. Haven't forgotten. > > :) > > Liu would like to see ccw use a normal lifecycle for the vfio_device > backing memory, is there a shorter path to achieve that then what I > came up with? Getting through your proposal is the task on our plate. Just not enough hours in the day at the moment, sorry. Eric > > Jason
Re: [Intel-gfx] [RE]: [PATCH v3 10/10] vfio/ccw: Move the lifecycle of the struct vfio_ccw_private to the mdev
On Fri, 2022-01-14 at 20:28 +0800, Liu Yi L wrote:
> Hi Eric,
>
> Hope you are back from new year holiday.:-) Have you got chane to
> consider
> this patch?
Hi Yi Liu,
It's coming up the list, but it's not there yet. Haven't forgotten. :)
Eric
>
> Regards,
> Yi Liu
>
> On 2021/11/30 19:32, Liu, Yi L wrote:
> > > From: Eric Farman
> > > Sent: Tuesday, November 30, 2021 1:18 AM
> > >
> > > On Wed, 2021-11-24 at 12:25 +, Liu, Yi L wrote:
> > > > > From: Jason Gunthorpe
> > > > > Sent: Fri, 1 Oct 2021 14:52:51 -0300
> > > > >
> > > > > The css_driver's main purpose is to create/destroy the mdev
> > > > > and
> > > > > relay the
> > > > > shutdown, irq, sch_event, and chp_event css_driver ops to the
> > > > > single
> > > > > created vfio_device, if it exists.
> > > > >
> > > > > Reframe the boundary where the css_driver domain switches to
> > > > > the
> > > > > vfio
> > > > > domain by using rcu to read and refcount the vfio_device out
> > > > > of the
> > > > > sch's
> > > > > drvdata. The mdev probe/remove will manage the drvdata of the
> > > > > parent.
> > > > >
> > > > > The vfio core code refcounting thus guarantees that when a
> > > > > css_driver
> > > > > callback is running the vfio_device is registered,
> > > > > simplifying the
> > > > > understanding of the whole lifecycle.
> > > > >
> > > > > Finally the vfio_ccw_private is allocated/freed during
> > > > > probe/remove
> > > > > of the
> > > > > mdev like any other vfio_device struct.
> > > >
> > > > Hi Eric,
> > > >
> > > > how about the status of this patch?
> > >
> > > Hi YiLiu,
> > >
> > > To be honest I never got this far in the series, as the middle of
> > > the
> > > series got into some more involved rework than I had the
> > > bandwidth to
> > > consider. Not sure I'll be able to do anything with it before the
> > > year
> > > end holiday, but I've noted your interest in getting this in line
> > > with
> > > the rest of vfio_device so I don't drop it too far down the list.
> >
> > thanks. look forward to your further thoughts on it. e.g. the
> > rework
> > things and gaps in this cleanup.
> >
> > Regards,
> > Yi Liu
> >
> > > Thanks,
> > > Eric
> > >
> > > > I found it is a good clean up to make
> > > > vfio ccw behave same with other vfio_device users. Also, I'd
> > > > like to
> > > > do a clean up to consolidate the vfio_device allocation which
> > > > needs the
> > > > vfio ccw private allocation happen in the mdev probe. So it
> > > > would be nice
> > > > to build the cleanup based on this patch.
> > > >
> > > > Regards,
> > > > Yi Liu
> > > >
> > > > > Signed-off-by: Jason Gunthorpe
> > > > > ---
> > > > > drivers/s390/cio/vfio_ccw_drv.c | 67 ++--
> > > > > -
> > > > >
> > > > > drivers/s390/cio/vfio_ccw_ops.c | 40 +++--
> > > > > drivers/s390/cio/vfio_ccw_private.h | 23 +-
> > > > > 3 files changed, 69 insertions(+), 61 deletions(-)
> > > > >
> > > > > diff --git a/drivers/s390/cio/vfio_ccw_drv.c
> > > > > b/drivers/s390/cio/vfio_ccw_drv.c
> > > > > index 18ad047811d111..c5582fc9c46c9e 100644
> > > > > --- a/drivers/s390/cio/vfio_ccw_drv.c
> > > > > +++ b/drivers/s390/cio/vfio_ccw_drv.c
> > > > > @@ -86,13 +86,19 @@ 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(>dev);
> > > > > + struct vfio_ccw_private *private =
> > > > > vfio_ccw_get_priv(sch);
> > > > > +
> > > > > + /* IRQ should not be delivered after the mdev is
> > > > > destroyed */
> > > > > + if (WARN_ON(!private))
> > > > > + return;
> > > > >
> > > > > inc_irq_stat(IRQIO_CIO);
> > > > > vfio_ccw_fsm_event(private, VFIO_CCW_EVENT_INTERRUPT);
> > > > > + vfio_device_put(>vdev);
> > > > > }
> > > > >
> > > > > -static struct vfio_ccw_private
> > > > > *vfio_ccw_alloc_private(struct
> > > > > subchannel *sch)
> > > > > +struct vfio_ccw_private *vfio_ccw_alloc_private(struct
> > > > > mdev_device
> > > > > *mdev,
> > > > > + struct
> > > > > subchannel *sch)
> > > > > {
> > > > > struct vfio_ccw_private *private;
> > > > >
> > > > > @@ -100,6 +106,8 @@ static struct vfio_ccw_private
> > > > > *vfio_ccw_alloc_private(struct subchannel *sch)
> > > > > if (!private)
> > > > > return ERR_PTR(-ENOMEM);
> > > > >
> > > > > + vfio_init_group_dev(>vdev, >dev,
> > > > > + _ccw_dev_ops);
> > > > > private->sch = sch;
> > > > > mutex_init(>io_mutex);
> > > > > private->state = VFIO_CCW_STATE_CLOSED;
> > > > > @@ -145,11 +153,12 @@ static struct vfio_ccw_private
> > > > > *vfio_ccw_alloc_private(struct subchannel *sch)
> > > > > kfree(private->cp.guest_cp);
> > > > > out_free_private:
> > > > > mutex_destroy(>io_mutex);
> > > > >
Re: [Intel-gfx] [RE]: [PATCH v3 10/10] vfio/ccw: Move the lifecycle of the struct vfio_ccw_private to the mdev
Hi Eric,
Hope you are back from new year holiday.:-) Have you got chane to consider
this patch?
Regards,
Yi Liu
On 2021/11/30 19:32, Liu, Yi L wrote:
From: Eric Farman
Sent: Tuesday, November 30, 2021 1:18 AM
On Wed, 2021-11-24 at 12:25 +, Liu, Yi L wrote:
From: Jason Gunthorpe
Sent: Fri, 1 Oct 2021 14:52:51 -0300
The css_driver's main purpose is to create/destroy the mdev and
relay the
shutdown, irq, sch_event, and chp_event css_driver ops to the
single
created vfio_device, if it exists.
Reframe the boundary where the css_driver domain switches to the
vfio
domain by using rcu to read and refcount the vfio_device out of the
sch's
drvdata. The mdev probe/remove will manage the drvdata of the
parent.
The vfio core code refcounting thus guarantees that when a
css_driver
callback is running the vfio_device is registered, simplifying the
understanding of the whole lifecycle.
Finally the vfio_ccw_private is allocated/freed during probe/remove
of the
mdev like any other vfio_device struct.
Hi Eric,
how about the status of this patch?
Hi YiLiu,
To be honest I never got this far in the series, as the middle of the
series got into some more involved rework than I had the bandwidth to
consider. Not sure I'll be able to do anything with it before the year
end holiday, but I've noted your interest in getting this in line with
the rest of vfio_device so I don't drop it too far down the list.
thanks. look forward to your further thoughts on it. e.g. the rework
things and gaps in this cleanup.
Regards,
Yi Liu
Thanks,
Eric
I found it is a good clean up to make
vfio ccw behave same with other vfio_device users. Also, I'd like to
do a clean up to consolidate the vfio_device allocation which needs the
vfio ccw private allocation happen in the mdev probe. So it would be nice
to build the cleanup based on this patch.
Regards,
Yi Liu
Signed-off-by: Jason Gunthorpe
---
drivers/s390/cio/vfio_ccw_drv.c | 67 ++---
drivers/s390/cio/vfio_ccw_ops.c | 40 +++--
drivers/s390/cio/vfio_ccw_private.h | 23 +-
3 files changed, 69 insertions(+), 61 deletions(-)
diff --git a/drivers/s390/cio/vfio_ccw_drv.c
b/drivers/s390/cio/vfio_ccw_drv.c
index 18ad047811d111..c5582fc9c46c9e 100644
--- a/drivers/s390/cio/vfio_ccw_drv.c
+++ b/drivers/s390/cio/vfio_ccw_drv.c
@@ -86,13 +86,19 @@ 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(>dev);
+ struct vfio_ccw_private *private = vfio_ccw_get_priv(sch);
+
+ /* IRQ should not be delivered after the mdev is destroyed */
+ if (WARN_ON(!private))
+ return;
inc_irq_stat(IRQIO_CIO);
vfio_ccw_fsm_event(private, VFIO_CCW_EVENT_INTERRUPT);
+ vfio_device_put(>vdev);
}
-static struct vfio_ccw_private *vfio_ccw_alloc_private(struct
subchannel *sch)
+struct vfio_ccw_private *vfio_ccw_alloc_private(struct mdev_device
*mdev,
+ struct subchannel *sch)
{
struct vfio_ccw_private *private;
@@ -100,6 +106,8 @@ static struct vfio_ccw_private
*vfio_ccw_alloc_private(struct subchannel *sch)
if (!private)
return ERR_PTR(-ENOMEM);
+ vfio_init_group_dev(>vdev, >dev,
+ _ccw_dev_ops);
private->sch = sch;
mutex_init(>io_mutex);
private->state = VFIO_CCW_STATE_CLOSED;
@@ -145,11 +153,12 @@ static struct vfio_ccw_private
*vfio_ccw_alloc_private(struct subchannel *sch)
kfree(private->cp.guest_cp);
out_free_private:
mutex_destroy(>io_mutex);
+ vfio_uninit_group_dev(>vdev);
kfree(private);
return ERR_PTR(-ENOMEM);
}
-static void vfio_ccw_free_private(struct vfio_ccw_private
*private)
+void vfio_ccw_free_private(struct vfio_ccw_private *private)
{
struct vfio_ccw_crw *crw, *temp;
@@ -164,14 +173,14 @@ static void vfio_ccw_free_private(struct
vfio_ccw_private *private)
kmem_cache_free(vfio_ccw_io_region, private->io_region);
kfree(private->cp.guest_cp);
mutex_destroy(>io_mutex);
- kfree(private);
+ vfio_uninit_group_dev(>vdev);
+ kfree_rcu(private, rcu);
}
static int vfio_ccw_sch_probe(struct subchannel *sch)
{
struct pmcw *pmcw = >schib.pmcw;
- struct vfio_ccw_private *private;
- int ret = -ENOMEM;
+ int ret;
if (pmcw->qf) {
dev_warn(>dev, "vfio: ccw: does not support QDIO:
%s\n",
@@ -179,15 +188,9 @@ static int vfio_ccw_sch_probe(struct
subchannel *sch)
return -ENODEV;
}
- private = vfio_ccw_alloc_private(sch);
- if (IS_ERR(private))
- return PTR_ERR(private);
-
- dev_set_drvdata(>dev, private);
-
- ret = vfio_ccw_mdev_reg(sch);
+ ret = mdev_register_device(>dev, _ccw_mdev_ops);
if (ret)
-
Re: [Intel-gfx] [RE]: [PATCH v3 10/10] vfio/ccw: Move the lifecycle of the struct vfio_ccw_private to the mdev
> From: Eric Farman
> Sent: Tuesday, November 30, 2021 1:18 AM
>
> On Wed, 2021-11-24 at 12:25 +, Liu, Yi L wrote:
> > > From: Jason Gunthorpe
> > > Sent: Fri, 1 Oct 2021 14:52:51 -0300
> > >
> > > The css_driver's main purpose is to create/destroy the mdev and
> > > relay the
> > > shutdown, irq, sch_event, and chp_event css_driver ops to the
> > > single
> > > created vfio_device, if it exists.
> > >
> > > Reframe the boundary where the css_driver domain switches to the
> > > vfio
> > > domain by using rcu to read and refcount the vfio_device out of the
> > > sch's
> > > drvdata. The mdev probe/remove will manage the drvdata of the
> > > parent.
> > >
> > > The vfio core code refcounting thus guarantees that when a
> > > css_driver
> > > callback is running the vfio_device is registered, simplifying the
> > > understanding of the whole lifecycle.
> > >
> > > Finally the vfio_ccw_private is allocated/freed during probe/remove
> > > of the
> > > mdev like any other vfio_device struct.
> >
> > Hi Eric,
> >
> > how about the status of this patch?
>
> Hi YiLiu,
>
> To be honest I never got this far in the series, as the middle of the
> series got into some more involved rework than I had the bandwidth to
> consider. Not sure I'll be able to do anything with it before the year
> end holiday, but I've noted your interest in getting this in line with
> the rest of vfio_device so I don't drop it too far down the list.
thanks. look forward to your further thoughts on it. e.g. the rework
things and gaps in this cleanup.
Regards,
Yi Liu
> Thanks,
> Eric
>
> > I found it is a good clean up to make
> > vfio ccw behave same with other vfio_device users. Also, I'd like to
> > do a clean up to consolidate the vfio_device allocation which needs the
> > vfio ccw private allocation happen in the mdev probe. So it would be nice
> > to build the cleanup based on this patch.
> >
> > Regards,
> > Yi Liu
> >
> > > Signed-off-by: Jason Gunthorpe
> > > ---
> > > drivers/s390/cio/vfio_ccw_drv.c | 67 ++---
> > >
> > > drivers/s390/cio/vfio_ccw_ops.c | 40 +++--
> > > drivers/s390/cio/vfio_ccw_private.h | 23 +-
> > > 3 files changed, 69 insertions(+), 61 deletions(-)
> > >
> > > diff --git a/drivers/s390/cio/vfio_ccw_drv.c
> > > b/drivers/s390/cio/vfio_ccw_drv.c
> > > index 18ad047811d111..c5582fc9c46c9e 100644
> > > --- a/drivers/s390/cio/vfio_ccw_drv.c
> > > +++ b/drivers/s390/cio/vfio_ccw_drv.c
> > > @@ -86,13 +86,19 @@ 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(>dev);
> > > + struct vfio_ccw_private *private = vfio_ccw_get_priv(sch);
> > > +
> > > + /* IRQ should not be delivered after the mdev is destroyed */
> > > + if (WARN_ON(!private))
> > > + return;
> > >
> > > inc_irq_stat(IRQIO_CIO);
> > > vfio_ccw_fsm_event(private, VFIO_CCW_EVENT_INTERRUPT);
> > > + vfio_device_put(>vdev);
> > > }
> > >
> > > -static struct vfio_ccw_private *vfio_ccw_alloc_private(struct
> > > subchannel *sch)
> > > +struct vfio_ccw_private *vfio_ccw_alloc_private(struct mdev_device
> > > *mdev,
> > > + struct subchannel *sch)
> > > {
> > > struct vfio_ccw_private *private;
> > >
> > > @@ -100,6 +106,8 @@ static struct vfio_ccw_private
> > > *vfio_ccw_alloc_private(struct subchannel *sch)
> > > if (!private)
> > > return ERR_PTR(-ENOMEM);
> > >
> > > + vfio_init_group_dev(>vdev, >dev,
> > > + _ccw_dev_ops);
> > > private->sch = sch;
> > > mutex_init(>io_mutex);
> > > private->state = VFIO_CCW_STATE_CLOSED;
> > > @@ -145,11 +153,12 @@ static struct vfio_ccw_private
> > > *vfio_ccw_alloc_private(struct subchannel *sch)
> > > kfree(private->cp.guest_cp);
> > > out_free_private:
> > > mutex_destroy(>io_mutex);
> > > + vfio_uninit_group_dev(>vdev);
> > > kfree(private);
> > > return ERR_PTR(-ENOMEM);
> > > }
> > >
> > > -static void vfio_ccw_free_private(struct vfio_ccw_private
> > > *private)
> > > +void vfio_ccw_free_private(struct vfio_ccw_private *private)
> > > {
> > > struct vfio_ccw_crw *crw, *temp;
> > >
> > > @@ -164,14 +173,14 @@ static void vfio_ccw_free_private(struct
> > > vfio_ccw_private *private)
> > > kmem_cache_free(vfio_ccw_io_region, private->io_region);
> > > kfree(private->cp.guest_cp);
> > > mutex_destroy(>io_mutex);
> > > - kfree(private);
> > > + vfio_uninit_group_dev(>vdev);
> > > + kfree_rcu(private, rcu);
> > > }
> > >
> > > static int vfio_ccw_sch_probe(struct subchannel *sch)
> > > {
> > > struct pmcw *pmcw = >schib.pmcw;
> > > - struct vfio_ccw_private *private;
> > > - int ret = -ENOMEM;
> > > + int ret;
> > >
> > > if (pmcw->qf) {
> > > dev_warn(>dev, "vfio: ccw: does not support QDIO:
> > > %s\n",
> > > @@ -179,15 +188,9 @@ static int vfio_ccw_sch_probe(struct
> > >
Re: [Intel-gfx] [RE]: [PATCH v3 10/10] vfio/ccw: Move the lifecycle of the struct vfio_ccw_private to the mdev
On Wed, 2021-11-24 at 12:25 +, Liu, Yi L wrote:
> > From: Jason Gunthorpe
> > Sent: Fri, 1 Oct 2021 14:52:51 -0300
> >
> > The css_driver's main purpose is to create/destroy the mdev and
> > relay the
> > shutdown, irq, sch_event, and chp_event css_driver ops to the
> > single
> > created vfio_device, if it exists.
> >
> > Reframe the boundary where the css_driver domain switches to the
> > vfio
> > domain by using rcu to read and refcount the vfio_device out of the
> > sch's
> > drvdata. The mdev probe/remove will manage the drvdata of the
> > parent.
> >
> > The vfio core code refcounting thus guarantees that when a
> > css_driver
> > callback is running the vfio_device is registered, simplifying the
> > understanding of the whole lifecycle.
> >
> > Finally the vfio_ccw_private is allocated/freed during probe/remove
> > of the
> > mdev like any other vfio_device struct.
>
> Hi Eric,
>
> how about the status of this patch?
Hi YiLiu,
To be honest I never got this far in the series, as the middle of the
series got into some more involved rework than I had the bandwidth to
consider. Not sure I'll be able to do anything with it before the year
end holiday, but I've noted your interest in getting this in line with
the rest of vfio_device so I don't drop it too far down the list.
Thanks,
Eric
> I found it is a good clean up to make
> vfio ccw behave same with other vfio_device users. Also, I'd like to
> do a
> clean up to consolidate the vfio_device allocation which needs the
> vfio
> ccw private allocation happen in the mdev probe. So it would be nice
> to
> build the cleanup based on this patch.
>
> Regards,
> Yi Liu
>
> > Signed-off-by: Jason Gunthorpe
> > ---
> > drivers/s390/cio/vfio_ccw_drv.c | 67 ++---
> >
> > drivers/s390/cio/vfio_ccw_ops.c | 40 +++--
> > drivers/s390/cio/vfio_ccw_private.h | 23 +-
> > 3 files changed, 69 insertions(+), 61 deletions(-)
> >
> > diff --git a/drivers/s390/cio/vfio_ccw_drv.c
> > b/drivers/s390/cio/vfio_ccw_drv.c
> > index 18ad047811d111..c5582fc9c46c9e 100644
> > --- a/drivers/s390/cio/vfio_ccw_drv.c
> > +++ b/drivers/s390/cio/vfio_ccw_drv.c
> > @@ -86,13 +86,19 @@ 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(>dev);
> > + struct vfio_ccw_private *private = vfio_ccw_get_priv(sch);
> > +
> > + /* IRQ should not be delivered after the mdev is destroyed */
> > + if (WARN_ON(!private))
> > + return;
> >
> > inc_irq_stat(IRQIO_CIO);
> > vfio_ccw_fsm_event(private, VFIO_CCW_EVENT_INTERRUPT);
> > + vfio_device_put(>vdev);
> > }
> >
> > -static struct vfio_ccw_private *vfio_ccw_alloc_private(struct
> > subchannel *sch)
> > +struct vfio_ccw_private *vfio_ccw_alloc_private(struct mdev_device
> > *mdev,
> > + struct subchannel *sch)
> > {
> > struct vfio_ccw_private *private;
> >
> > @@ -100,6 +106,8 @@ static struct vfio_ccw_private
> > *vfio_ccw_alloc_private(struct subchannel *sch)
> > if (!private)
> > return ERR_PTR(-ENOMEM);
> >
> > + vfio_init_group_dev(>vdev, >dev,
> > + _ccw_dev_ops);
> > private->sch = sch;
> > mutex_init(>io_mutex);
> > private->state = VFIO_CCW_STATE_CLOSED;
> > @@ -145,11 +153,12 @@ static struct vfio_ccw_private
> > *vfio_ccw_alloc_private(struct subchannel *sch)
> > kfree(private->cp.guest_cp);
> > out_free_private:
> > mutex_destroy(>io_mutex);
> > + vfio_uninit_group_dev(>vdev);
> > kfree(private);
> > return ERR_PTR(-ENOMEM);
> > }
> >
> > -static void vfio_ccw_free_private(struct vfio_ccw_private
> > *private)
> > +void vfio_ccw_free_private(struct vfio_ccw_private *private)
> > {
> > struct vfio_ccw_crw *crw, *temp;
> >
> > @@ -164,14 +173,14 @@ static void vfio_ccw_free_private(struct
> > vfio_ccw_private *private)
> > kmem_cache_free(vfio_ccw_io_region, private->io_region);
> > kfree(private->cp.guest_cp);
> > mutex_destroy(>io_mutex);
> > - kfree(private);
> > + vfio_uninit_group_dev(>vdev);
> > + kfree_rcu(private, rcu);
> > }
> >
> > static int vfio_ccw_sch_probe(struct subchannel *sch)
> > {
> > struct pmcw *pmcw = >schib.pmcw;
> > - struct vfio_ccw_private *private;
> > - int ret = -ENOMEM;
> > + int ret;
> >
> > if (pmcw->qf) {
> > dev_warn(>dev, "vfio: ccw: does not support QDIO:
> > %s\n",
> > @@ -179,15 +188,9 @@ static int vfio_ccw_sch_probe(struct
> > subchannel *sch)
> > return -ENODEV;
> > }
> >
> > - private = vfio_ccw_alloc_private(sch);
> > - if (IS_ERR(private))
> > - return PTR_ERR(private);
> > -
> > - dev_set_drvdata(>dev, private);
> > -
> > - ret = vfio_ccw_mdev_reg(sch);
> > + ret = mdev_register_device(>dev, _ccw_mdev_ops);
> > if (ret)
> > -
[Intel-gfx] [RE]: [PATCH v3 10/10] vfio/ccw: Move the lifecycle of the struct vfio_ccw_private to the mdev
> From: Jason Gunthorpe
> Sent: Fri, 1 Oct 2021 14:52:51 -0300
>
> The css_driver's main purpose is to create/destroy the mdev and relay the
> shutdown, irq, sch_event, and chp_event css_driver ops to the single
> created vfio_device, if it exists.
>
> Reframe the boundary where the css_driver domain switches to the vfio
> domain by using rcu to read and refcount the vfio_device out of the sch's
> drvdata. The mdev probe/remove will manage the drvdata of the parent.
>
> The vfio core code refcounting thus guarantees that when a css_driver
> callback is running the vfio_device is registered, simplifying the
> understanding of the whole lifecycle.
>
> Finally the vfio_ccw_private is allocated/freed during probe/remove of the
> mdev like any other vfio_device struct.
Hi Eric,
how about the status of this patch? I found it is a good clean up to make
vfio ccw behave same with other vfio_device users. Also, I'd like to do a
clean up to consolidate the vfio_device allocation which needs the vfio
ccw private allocation happen in the mdev probe. So it would be nice to
build the cleanup based on this patch.
Regards,
Yi Liu
> Signed-off-by: Jason Gunthorpe
> ---
> drivers/s390/cio/vfio_ccw_drv.c | 67 ++---
> drivers/s390/cio/vfio_ccw_ops.c | 40 +++--
> drivers/s390/cio/vfio_ccw_private.h | 23 +-
> 3 files changed, 69 insertions(+), 61 deletions(-)
>
> diff --git a/drivers/s390/cio/vfio_ccw_drv.c b/drivers/s390/cio/vfio_ccw_drv.c
> index 18ad047811d111..c5582fc9c46c9e 100644
> --- a/drivers/s390/cio/vfio_ccw_drv.c
> +++ b/drivers/s390/cio/vfio_ccw_drv.c
> @@ -86,13 +86,19 @@ 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(>dev);
> + struct vfio_ccw_private *private = vfio_ccw_get_priv(sch);
> +
> + /* IRQ should not be delivered after the mdev is destroyed */
> + if (WARN_ON(!private))
> + return;
>
> inc_irq_stat(IRQIO_CIO);
> vfio_ccw_fsm_event(private, VFIO_CCW_EVENT_INTERRUPT);
> + vfio_device_put(>vdev);
> }
>
> -static struct vfio_ccw_private *vfio_ccw_alloc_private(struct subchannel
> *sch)
> +struct vfio_ccw_private *vfio_ccw_alloc_private(struct mdev_device *mdev,
> + struct subchannel *sch)
> {
> struct vfio_ccw_private *private;
>
> @@ -100,6 +106,8 @@ static struct vfio_ccw_private
> *vfio_ccw_alloc_private(struct subchannel *sch)
> if (!private)
> return ERR_PTR(-ENOMEM);
>
> + vfio_init_group_dev(>vdev, >dev,
> + _ccw_dev_ops);
> private->sch = sch;
> mutex_init(>io_mutex);
> private->state = VFIO_CCW_STATE_CLOSED;
> @@ -145,11 +153,12 @@ static struct vfio_ccw_private
> *vfio_ccw_alloc_private(struct subchannel *sch)
> kfree(private->cp.guest_cp);
> out_free_private:
> mutex_destroy(>io_mutex);
> + vfio_uninit_group_dev(>vdev);
> kfree(private);
> return ERR_PTR(-ENOMEM);
> }
>
> -static void vfio_ccw_free_private(struct vfio_ccw_private *private)
> +void vfio_ccw_free_private(struct vfio_ccw_private *private)
> {
> struct vfio_ccw_crw *crw, *temp;
>
> @@ -164,14 +173,14 @@ static void vfio_ccw_free_private(struct
> vfio_ccw_private *private)
> kmem_cache_free(vfio_ccw_io_region, private->io_region);
> kfree(private->cp.guest_cp);
> mutex_destroy(>io_mutex);
> - kfree(private);
> + vfio_uninit_group_dev(>vdev);
> + kfree_rcu(private, rcu);
> }
>
> static int vfio_ccw_sch_probe(struct subchannel *sch)
> {
> struct pmcw *pmcw = >schib.pmcw;
> - struct vfio_ccw_private *private;
> - int ret = -ENOMEM;
> + int ret;
>
> if (pmcw->qf) {
> dev_warn(>dev, "vfio: ccw: does not support QDIO: %s\n",
> @@ -179,15 +188,9 @@ static int vfio_ccw_sch_probe(struct subchannel *sch)
> return -ENODEV;
> }
>
> - private = vfio_ccw_alloc_private(sch);
> - if (IS_ERR(private))
> - return PTR_ERR(private);
> -
> - dev_set_drvdata(>dev, private);
> -
> - ret = vfio_ccw_mdev_reg(sch);
> + ret = mdev_register_device(>dev, _ccw_mdev_ops);
> if (ret)
> - goto out_free;
> + return ret;
>
> if (dev_get_uevent_suppress(>dev)) {
> dev_set_uevent_suppress(>dev, 0);
> @@ -198,22 +201,11 @@ static int vfio_ccw_sch_probe(struct subchannel *sch)
> sch->schid.cssid, sch->schid.ssid,
> sch->schid.sch_no);
> return 0;
> -
> -out_free:
> - dev_set_drvdata(>dev, NULL);
> - vfio_ccw_free_private(private);
> - return ret;
> }
>
> static void vfio_ccw_sch_remove(struct subchannel *sch)
> {
> - struct vfio_ccw_private *private = dev_get_drvdata(>dev);
> -
> - vfio_ccw_mdev_unreg(sch);
>
