Received-SPF: pass (google.com: domain of linux-crypto+bounces-2038-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:4601:e00::3 as permitted sender) client-ip=2604:1380:4601:e00::3;
Date: Tue, 13 Feb 2024 10:55:51 -0400
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
Message-ID: <20240213145551.GC989297@nvidia.com>
References: <20240201153337.4033490-1-xin.zeng@intel.com>
 <20240201153337.4033490-11-xin.zeng@intel.com>
 <972cc8a41a8549d19ed897ee7335f9e0@huawei.com>
 <DM4PR11MB55025C3ECE1896D9C5CA4E86884F2@DM4PR11MB5502.namprd11.prod.outlook.com>
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <DM4PR11MB55025C3ECE1896D9C5CA4E86884F2@DM4PR11MB5502.namprd11.prod.outlook.com>
Precedence: bulk
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0:
	=?us-ascii?Q?TFqgGXNjvkOJSI8eP95OKZu6l2EQQIaW58bolfJyauVcTmhglKXzXtWy9e2r?=
 =?us-ascii?Q?y7QMpLVwVqWnZe4jk3wLBYflXKW1NkelPg0WFSxr+CdkVVr8tEa/rYh3Q9ES?=
 =?us-ascii?Q?/9qn1fo4tQ+NeEnONbyrEj825KQOsXTdkAuLZtYIzhcSJHsMKrNpniLY17dW?=
 =?us-ascii?Q?iB08e53LLIAH05mIqb/vtwmRM91UfBvkcedfvt1jrgB0tUOSrxAyS4iw1COK?=
 =?us-ascii?Q?JcJbo6eV3K4Lh9eeTL+Flju8UigBygdVJTdWvpsCkSJuw/FBt0Wc6N44nz8u?=
 =?us-ascii?Q?4eP9vvTFZTYjxy6YNX0nag3AnA05lNjc6UsYtap02lzgvXtKOGij89UK+5Xn?=
 =?us-ascii?Q?6p9mKb/8N2kDfr/Sb9V7Smv43M0/zGjnheKjVK6ddxcflNB442pOXzxHjMsm?=
 =?us-ascii?Q?NDbRcTu+tX6heczQpRijHkCm2MJz4AkBhGepsoAsH4MzGnSc5OSVQZZ9taab?=
 =?us-ascii?Q?iTtzeMswq/pimJ3BXFd2b6Ea1DUrDum/hA3n3XZk6rz4oIE1EQCKmlU764Pz?=
 =?us-ascii?Q?YxFWEjiLCjpmApd5b4ccN28Wb5Ryrlr/e86P0eBS9NXNelMqMBPunxkzFMBc?=
 =?us-ascii?Q?957B6Sa9n+qVnYFYSkEPL3ZHZJ0CmUd5dux+ZfeXFslTtOrtg68FJU8oVRQj?=
 =?us-ascii?Q?xTJXLkwpeESG4PU2MfKbAWIycRtnKZXUNX7E7pHlacSt07tzCGdpqgufzgOB?=
 =?us-ascii?Q?53mpTEkxH7VFeMsU0Nk8ZeuJ2l9tBnXynT/Ltv/u5AptRfKLq3eB7+MHUxu0?=
 =?us-ascii?Q?Utf+kncFukKH+vMrTMgv40x5p+LbF2A8koPeHBR89aZw8XUQBV2NmvSuE+Xf?=
 =?us-ascii?Q?2z1aDoVHyndI2P8IUN2/EUvYUrb7oa6Y41f4bCDPuHoe54UBEypk6lq+LLUU?=
 =?us-ascii?Q?hhKiLf8B3z4V4oQKnxiZ/TGF9SdXdPlj2MjiBKn2tNnNrz7iNwkAGVQNSH/P?=
 =?us-ascii?Q?FCXdEqu1vYhE2BzQNZ4I8oG4Kkt2RyrLQXh2NoOkjSA4F09KFsOKJHPVJ5zc?=
 =?us-ascii?Q?50vUGBszN+G5bDHIh9ggoT+QGJiWDeB+YiTPIiR9keozxkZa8fSndHoG+jTb?=
 =?us-ascii?Q?RdZvwXBN+tLuBliqqilUiFAYnFqQQn7z7rz0KtKV+27JkhTzZyuexLZT+Txl?=
 =?us-ascii?Q?0NaaiPLMzdNGHGhYZ7bwOaxqhQNff5ev7UO9WnRhCQ4+xUvOt+w34gBDUiKk?=
 =?us-ascii?Q?4rQT7xwDvcdArJvRIIwhZcsC9t4f/LatoDqVityny70b1v2aV787ISBPnEKe?=
 =?us-ascii?Q?phQfI95sOUfh0aOoYTNk2mMNimM7xDec80kadi7YSMOneYHoiqe2pszsdrWK?=
 =?us-ascii?Q?Jzry7c61HiH1Iq7ePM03YUycmVLwEIYT3TSA71ppIWqRsKwnHlZM07aSn993?=
 =?us-ascii?Q?qBGzr7sphAgv1Z/x5+Co6ZrRn+fQstrtzvRs6yUxbyslyXr1bGjKx1QHhFrI?=
 =?us-ascii?Q?rEB6eq+sRjpa6fMA1llMSuUr8/J64VaLK82OokkEpx2hIoP/SzgrfZj9vq16?=
 =?us-ascii?Q?Qb3u1TIvaepVS1bqzH5RTSd4wQ9h0J2aitWOXijN96F1xaQb7CNEHveeYEdm?=
 =?us-ascii?Q?0eeEfiVLR+F7PRNZWb4gL3O/qWTYrKDewB5dPcRZ?=
X-OriginatorOrg: Nvidia.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 5d35d6ba-5f16-4dd3-0118-08dc2ca3e00a
X-MS-Exchange-CrossTenant-AuthSource: LV2PR12MB5869.namprd12.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 13 Feb 2024 14:55:53.0738
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 43083d15-7273-40c1-b7db-39efd9ccc17a
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: ilD59X0o5dhzJHoP6jUp9uLsCkhSou5ThU0OWwTy4h4zvvobv5ZDFv+uQFn0QnE6
X-MS-Exchange-Transport-CrossTenantHeadersStamped: IA1PR12MB6233

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