Re: [PATCH 10/10] vfio/qat: Add vfio_pci driver for Intel QAT VF devices

From: Jason Gunthorpe <jgg@nvidia.com>
To: "Zeng, Xin" <xin.zeng@intel.com>
Cc: Shameerali Kolothum Thodi <shameerali.kolothum.thodi@huawei.com>,
	"linux-crypto@vger.kernel.org" <linux-crypto@vger.kernel.org>,
	"kvm@vger.kernel.org" <kvm@vger.kernel.org>,
	qat-linux <qat-linux@intel.com>,
	"herbert@gondor.apana.org.au" <herbert@gondor.apana.org.au>,
	"alex.williamson@redhat.com" <alex.williamson@redhat.com>,
	"yishaih@nvidia.com" <yishaih@nvidia.com>,
	"Tian, Kevin" <kevin.tian@intel.com>,
	"Cao, Yahui" <yahui.cao@intel.com>
Subject: Re: [PATCH 10/10] vfio/qat: Add vfio_pci driver for Intel QAT VF devices
Date: Tue, 13 Feb 2024 10:55:51 -0400	[thread overview]
Message-ID: <20240213145551.GC989297@nvidia.com> (raw)
In-Reply-To: <DM4PR11MB55025C3ECE1896D9C5CA4E86884F2@DM4PR11MB5502.namprd11.prod.outlook.com>

On Tue, Feb 13, 2024 at 01:08:47PM +0000, Zeng, Xin wrote:
> > > +		ret = -ENODEV;
> > > +		goto err_rel;
> > > +	}
> > > +
> > > +	mdev = qat_vfmig_create(parent, vf_id);
> > > +	if (IS_ERR(mdev)) {
> > > +		ret = PTR_ERR(mdev);
> > > +		goto err_rel;
> > > +	}
> > > +
> > > +	ops = mdev->ops;
> > > +	if (!ops || !ops->init || !ops->cleanup ||
> > > +	    !ops->open || !ops->close ||
> > > +	    !ops->save_state || !ops->load_state ||
> > > +	    !ops->suspend || !ops->resume) {
> > > +		ret = -EIO;
> > > +		dev_err(&parent->dev, "Incomplete device migration ops
> > > structure!");
> > > +		goto err_destroy;
> > > +	}
> > 
> > If all these ops are a must why cant we move the check inside the
> > qat_vfmig_create()?
> > Or rather call them explicitly as suggested by Jason.
> 
> We can do it, but it might make sense to leave the check to the APIs' user
> as some of these ops interfaces might be optional for other QAT variant driver
> in future.

If you have patches already that need ops then you can leave them, but
otherwise they should be removed and added back if you come with
future patches that require another implementation.

Jason