Re: [PATCH v3 2/6] vfio: ccw: Rework subchannel state on setup

[Pierre Morel]

On 18/12/2018 18:44, Eric Farman wrote:
> My questions to this patch from the original RFC series are still 
> outstanding. :(
>
> https://marc.info/?l=linux-s390&m=154223063716128&w=2


Hi Eric,

Thanks for the following of this patch series.

For your question about quiece during remove I do not think it should be 
a NOP, we must make sure the channel is disabled at that time.

> On 11/28/2018 07:41 AM, Pierre Morel wrote:
> > The subchannel enablement and the according setting to the
> > VFIO_CCW_STATE_STANDBY state should only be done when all
> > parts of the VFIO mediated device have been initialized
> > i.e. after the mediated device has been successfully opened.
> >
> > Let's stay in VFIO_CCW_STATE_NOT_OPER until the mediated
> > device has been opened and set the VFIO_CCW_STATE_STANDBY
> > on a successful open.
> >
> > On release the state is set back to VFIO_CCW_STATE_NOT_OPER
> > by vfio_ccw_sch_quiesce().
> >
> > When the mediated device is closed, disable the sub channel
> > by calling vfio_ccw_sch_quiesce().
> >
> > Signed-off-by: Pierre Morel <pmorel@xxxxxxxxxxxxx>
> > ---
> > Â drivers/s390/cio/vfio_ccw_async.cÂÂ | 11 +++++++++++
>
> Ah, this series is built on Connie's async changes. Okay. [1]

Yes, and after reflections I think the timing is bad so I prefer to wait 
for the series from Connie on hsch/csch to be finished before going on 
with this series.
Otherwise I fear to only add noise to the current discussions.


> > Â drivers/s390/cio/vfio_ccw_drv.cÂÂÂÂ | 10 +---------
> > Â drivers/s390/cio/vfio_ccw_ops.cÂÂÂÂ | 27 +++++++++++++++++++++------
...snip...
> > @@ -170,6 +184,7 @@ static void vfio_ccw_mdev_release(struct 
> > mdev_device *mdev)
> > ÂÂÂÂÂÂÂÂÂ dev_get_drvdata(mdev_parent_dev(mdev));
> > ÂÂÂÂÂ int i;
> > +ÂÂÂ vfio_ccw_sch_quiesce(private->sch);
> > ÂÂÂÂÂ vfio_unregister_notifier(mdev_dev(mdev), VFIO_IOMMU_NOTIFY,
> > ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ &private->nb);
>
> [1] If Connie's async patches go in first, then the stuff in your 
> "vfio_ccw_unregister_async_dev_regions" is also added here. That could 
> be removed and replaced with a call to your new function, yes?

certainly.
Thanks for your comments.

Regards,
Pierre


--
Pierre Morel
Linux/KVM/QEMU in BÃblingen - Germany