Received-SPF: pass (google.com: domain of linux-crypto+bounces-2506-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.199.223 as permitted sender) client-ip=147.75.199.223;
From: "Tian, Kevin" <kevin.tian@intel.com>
To: "Zeng, Xin" <xin.zeng@intel.com>, "herbert@gondor.apana.org.au"
	<herbert@gondor.apana.org.au>, "alex.williamson@redhat.com"
	<alex.williamson@redhat.com>, "jgg@nvidia.com" <jgg@nvidia.com>,
	"yishaih@nvidia.com" <yishaih@nvidia.com>,
	"shameerali.kolothum.thodi@huawei.com" <shameerali.kolothum.thodi@huawei.com>
CC: "linux-crypto@vger.kernel.org" <linux-crypto@vger.kernel.org>,
	"kvm@vger.kernel.org" <kvm@vger.kernel.org>, qat-linux <qat-linux@intel.com>,
	"Cao, Yahui" <yahui.cao@intel.com>
Subject: RE: [PATCH v4 10/10] vfio/qat: Add vfio_pci driver for Intel QAT VF
 devices
Thread-Topic: [PATCH v4 10/10] vfio/qat: Add vfio_pci driver for Intel QAT VF
 devices
Thread-Index: AQHaalR95YlyUWWE9kKxaDXGuT4g97EoxSgg
Date: Tue, 5 Mar 2024 07:37:30 +0000
Message-ID: <BN9PR11MB5276492EDE7F9CA57117FE518C222@BN9PR11MB5276.namprd11.prod.outlook.com>
References: <20240228143402.89219-1-xin.zeng@intel.com>
 <20240228143402.89219-11-xin.zeng@intel.com>
In-Reply-To: <20240228143402.89219-11-xin.zeng@intel.com>
Accept-Language: en-US
Content-Language: en-US
x-ms-exchange-antispam-messagedata-chunkcount: 1
x-ms-exchange-antispam-messagedata-0: =?us-ascii?Q?gYNhj1Set+JDK/xUurnT7AyAF3T7KDYWEOzUbZuM6eDRdPrXPwhcz7rv9hnH?=
 =?us-ascii?Q?MTY6EuTsuqyADrRTRF7c8sr85NQfV3UiRYVciaLRpimOfEbNj6w+GbELJXXZ?=
 =?us-ascii?Q?0DW9iyK8nfB5DzBf0uU9vwZMe4vARsx+IX2c9VFhtGVEzBMfeqz9jxlv+2M4?=
 =?us-ascii?Q?FaQ4XmoLZzppgZecKHniYHa4OvazTKeWDNDlZcUZeSjuv63kIQrvGBVh7te3?=
 =?us-ascii?Q?KrX1Cra1w6tYMTBylH1nQgdSzoD8hky3tDOjVxfUtcvEps01xBalpFcmdj0A?=
 =?us-ascii?Q?rWbixsBAhtAYfQimYCmk2x6oDkhH3JSrdka//bmkueezs7WdTVB2WT+KfIW4?=
 =?us-ascii?Q?eLFk41Ik2TudPXgwxNLeZ2xF4Ff+QBItFu7WJFLxQF0RH87aK9LbKzuaO9EV?=
 =?us-ascii?Q?a9gJpHB9zvTHZT23oVQwDjfAPCDS501y9f7aLq/Dml7Sy9+lPFNNdcsfpfkp?=
 =?us-ascii?Q?W0AS8tLT86UOjNUAKzsUGbKsOLNpdZSG0YCRjl+scRwCvXmSwDAt8eVcs/AL?=
 =?us-ascii?Q?f4R1dPPu5+Xgn+FA161uo4bM71csQEUBxeV4Krrdeet7UGcSy8bdrKxf8z48?=
 =?us-ascii?Q?YUD1LLN6nY0C12XUsxHwEVal9uEGPMH1d9g5ifaSeyQyfHt09z/YoKM5iwRe?=
 =?us-ascii?Q?wumndEfqvzDWsC9aUCJvG7TgxmyfJd9Vm88O+1X7GgHL5/CeDiW0wiiUMgTP?=
 =?us-ascii?Q?nYQ3mYkQv3kUH9SoyqvS5uOEMegkfjxJ2ALZgY6DTJ/0Dr+a/7/LPkLyC8dc?=
 =?us-ascii?Q?MDUMivAK8lzR83U0RXqK/JJUz2j+RQJiqMIxcMQVIx7RwRXjZ2lsN9sDYsdi?=
 =?us-ascii?Q?LNu/nSJgidT67CTS5f4XosbuPp0RhyOub0qklccothzPkY6klgbeHuA4fdrW?=
 =?us-ascii?Q?rKubqK1p5817peFJ2sU5XQ106p/QMfGz+jfqEdyDyk+fFFfPV3rXeS41NOdI?=
 =?us-ascii?Q?ZLJqYOM76S76Fbb14VGXlIwLQwrVxaFYz83GF7bXqopDEdO0UJIvNAmZLAsp?=
 =?us-ascii?Q?yNb77PAP1lJqfHZyPAiE4kASAJZc13ZHjJgh+p+a/kD9pMGoV8q7JSNTReQu?=
 =?us-ascii?Q?zqyIjENuaPyCn5x4pEzuRTTsSJ6oCQquDn4Eu5xnq8qhnvVBMPNofplmm8wm?=
 =?us-ascii?Q?6KBIoJvLWEPLv/FSGDB6rjt/QAdkZplIHBazwgGBqco8RiWzV1WwxqhWs4hx?=
 =?us-ascii?Q?bdRFAwedE1Z1UlYTcRsWRlURaYiPtgK6ZJu/HtNDtA4eh6VLGK58Y9KytqJP?=
 =?us-ascii?Q?izYqUemjzPgRR762D74s9e93svCI4MxYTwcuBOdNnmVxgNlQ9PA7TdmMvKSg?=
 =?us-ascii?Q?rqwE6fQsKB+5ZR+g9VNFzY63ESaj53wrFlysXtLfiiu2k6UO2SSzT+kLNm+r?=
 =?us-ascii?Q?htmtFKEIK6FVew2t3ppw231t3F9gHKVR9xQZ4t5vb/s1Gv7xcWsd9oKJRmbC?=
 =?us-ascii?Q?N35FQz0uDVsgjcFuuM9R+y11FXaRclgPZftFSmMBbMusn+qq2+H8nBuMRVlz?=
 =?us-ascii?Q?okn3MYiSSQewiuVuDaFQmi2KhD7w4tn0SjrzBXvvQCVDxwoOLMelrq0MhicY?=
 =?us-ascii?Q?GHm9s4Wn3yqOWWOLSLeLGVRKd5PMe8t1yDE5iLAV?=
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: quoted-printable
Precedence: bulk
X-Mailing-List: linux-crypto@vger.kernel.org
List-Id: <linux-crypto.vger.kernel.org>
List-Subscribe: <mailto:linux-crypto+subscribe@vger.kernel.org>
List-Unsubscribe: <mailto:linux-crypto+unsubscribe@vger.kernel.org>
MIME-Version: 1.0
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-AuthSource: BN9PR11MB5276.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 2657763f-0d2a-46c7-6f49-08dc3ce71d18
X-MS-Exchange-CrossTenant-originalarrivaltime: 05 Mar 2024 07:37:30.2399
 (UTC)
X-MS-Exchange-CrossTenant-fromentityheader: Hosted
X-MS-Exchange-CrossTenant-id: 46c98d88-e344-4ed4-8496-4ed7712e255d
X-MS-Exchange-CrossTenant-mailboxtype: HOSTED
X-MS-Exchange-CrossTenant-userprincipalname: YIzWG+Y7tRpxG4OsRC2apcEuP+YAPD/3b8hFNBYnw4LFd3M3GHnfmw/zdtJY67wWriEMjuQuaTdNiDS5DAm2tQ==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: IA1PR11MB7248
X-OriginatorOrg: intel.com

> From: Zeng, Xin <xin.zeng@intel.com>
> Sent: Wednesday, February 28, 2024 10:34 PM
> +
> +static long qat_vf_precopy_ioctl(struct file *filp, unsigned int cmd,
> +				 unsigned long arg)
> +{
> +	struct qat_vf_migration_file *migf =3D filp->private_data;
> +	struct qat_vf_core_device *qat_vdev =3D migf->qat_vdev;
> +	struct qat_mig_dev *mig_dev =3D qat_vdev->mdev;
> +	struct vfio_precopy_info info;
> +	loff_t *pos =3D &filp->f_pos;
> +	unsigned long minsz;
> +	int ret =3D 0;
> +
> +	if (cmd !=3D VFIO_MIG_GET_PRECOPY_INFO)
> +		return -ENOTTY;
> +
> +	minsz =3D offsetofend(struct vfio_precopy_info, dirty_bytes);
> +
> +	if (copy_from_user(&info, (void __user *)arg, minsz))
> +		return -EFAULT;
> +	if (info.argsz < minsz)
> +		return -EINVAL;
> +
> +	mutex_lock(&qat_vdev->state_mutex);
> +	if (qat_vdev->mig_state !=3D VFIO_DEVICE_STATE_PRE_COPY) {
> +		mutex_unlock(&qat_vdev->state_mutex);
> +		return -EINVAL;
> +	}

what about PRE_COPY_P2P?

> +static struct qat_vf_migration_file *
> +qat_vf_save_device_data(struct qat_vf_core_device *qat_vdev, bool
> pre_copy)
> +{
> +	struct qat_vf_migration_file *migf;
> +	int ret;
> +
> +	migf =3D kzalloc(sizeof(*migf), GFP_KERNEL);
> +	if (!migf)
> +		return ERR_PTR(-ENOMEM);
> +
> +	migf->filp =3D anon_inode_getfile("qat_vf_mig", &qat_vf_save_fops,
> +					migf, O_RDONLY);
> +	ret =3D PTR_ERR_OR_ZERO(migf->filp);
> +	if (ret) {
> +		kfree(migf);
> +		return ERR_PTR(ret);
> +	}
> +
> +	stream_open(migf->filp->f_inode, migf->filp);
> +	mutex_init(&migf->lock);
> +
> +	if (pre_copy)
> +		ret =3D qat_vf_save_setup(qat_vdev, migf);
> +	else
> +		ret =3D qat_vf_save_state(qat_vdev, migf);
> +	if (ret) {
> +		fput(migf->filp);
> +		return ERR_PTR(ret);
> +	}

lack of kfree(migf). Using goto can avoid such typo in error unwind.

> +
> +static struct file *qat_vf_pci_step_device_state(struct qat_vf_core_devi=
ce
> *qat_vdev, u32 new)
> +{
> +	u32 cur =3D qat_vdev->mig_state;
> +	int ret;
> +
> +	if ((cur =3D=3D VFIO_DEVICE_STATE_RUNNING && new =3D=3D
> VFIO_DEVICE_STATE_RUNNING_P2P) ||
> +	    (cur =3D=3D VFIO_DEVICE_STATE_PRE_COPY && new =3D=3D
> VFIO_DEVICE_STATE_PRE_COPY_P2P)) {
> +		ret =3D qat_vfmig_suspend(qat_vdev->mdev);
> +		if (ret)
> +			return ERR_PTR(ret);
> +		return NULL;
> +	}

could you arrange the transitions in pair as mlx does e.g. following
above is the reverse one calling qat_vfmig_resume()?

> +
> +MODULE_LICENSE("GPL");
> +MODULE_AUTHOR("Intel Corporation");

please use your name as the author.