Received-SPF: pass (google.com: domain of linux-crypto+bounces-3713-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45e3:2400::1 as permitted sender) client-ip=2604:1380:45e3:2400::1;
Date: Fri, 19 Apr 2024 11:10:57 -0300
From: Jason Gunthorpe <jgg@nvidia.com>
To: "Tian, Kevin" <kevin.tian@intel.com>
Cc: Alex Williamson <alex.williamson@redhat.com>,
	"Zeng, Xin" <xin.zeng@intel.com>,
	"herbert@gondor.apana.org.au" <herbert@gondor.apana.org.au>,
	"yishaih@nvidia.com" <yishaih@nvidia.com>,
	"shameerali.kolothum.thodi@huawei.com" <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>, "Cao, Yahui" <yahui.cao@intel.com>
Subject: Re: [PATCH v6 1/1] vfio/qat: Add vfio_pci driver for Intel QAT
 SR-IOV VF devices
Message-ID: <20240419141057.GG3050601@nvidia.com>
References: <20240417143141.1909824-1-xin.zeng@intel.com>
 <20240417143141.1909824-2-xin.zeng@intel.com>
 <20240418165434.1da52cf0.alex.williamson@redhat.com>
 <BN9PR11MB52767D5F7FF5D6498C974B388C0D2@BN9PR11MB5276.namprd11.prod.outlook.com>
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <BN9PR11MB52767D5F7FF5D6498C974B388C0D2@BN9PR11MB5276.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?BS1jnSK3MN5PWHH0ANE+lSEC/tYwTejmvIT02nPCvy/y3D9FCGp+UhWk0WqD?=
 =?us-ascii?Q?oAhVGybO4n4k7MOuFHAH6UX2mYGdXHnS452+Y6PlhxUaxgR2F+J8gys/8/l0?=
 =?us-ascii?Q?O1YmBnFcj3v5rQv3rwI71DrtdY27cJWsqoutrr7WOytpsnN2dd4JHPGjooMR?=
 =?us-ascii?Q?nD/8FXYNt8bYraNDUYvCMV3fz3U+NrBmNc63Gm2v78R72xhtoJa2wW+HQHdT?=
 =?us-ascii?Q?j3CIlz7dY4g8+6ottUSRiWNOfezBewWyyOxRZEkkH/5L2qvQL3crY5y2iKc2?=
 =?us-ascii?Q?GWJfUPjWzxd34CzEBUXDk2mpqyCaXIZYl0agg/8K4HDh2aOo+1GIoQwRFNW3?=
 =?us-ascii?Q?BAeAHhDleeG+sfJri9XfnDKLwsa+qltroqkVOGd0q9xE/c7Q9/QLoDJCXDG1?=
 =?us-ascii?Q?xgRn0CCx/9XKddWdgab7sd0dx3J/cvmLtV1wre1z0xBjlDyy+o6mo7TwVOOB?=
 =?us-ascii?Q?edWnOh3jkTu7imXCPjASDMV3Osm9GrAfIwv0945nJMVdo/WisnFit6poPYJw?=
 =?us-ascii?Q?XmZ0M4etNXgmTxZcsV2Tt7S7qEvy0mbFp39hhBI1uQVBCL4cZcbOAPzcGrkz?=
 =?us-ascii?Q?RBeV/roJD2u9oWB9xHWJ9kVYFPMon5nLzleC5wmeAv+lzI7Wklr+vFCp/v0n?=
 =?us-ascii?Q?N85BF6LwTRG9lbybViTVo5fTpikMqiQuIZQ521J1YzoZc3EUKrN9MAZi6TPY?=
 =?us-ascii?Q?WdU6fs45TmYjjUOuIZvCdf7mJTzqsBvNOE2glSk9BRdIO/lXfr/eYXnf+TU2?=
 =?us-ascii?Q?8UZI3W9MdJ4+nENs87HUL0HKiCKgz/8yyhqjaOMwihxwUeHNakrJqLRqfv+B?=
 =?us-ascii?Q?xPmqKLxSDaiRQQlMoiUa3if2778CW7kxbMvYGqA+F+RP1zo/laZssjiSmwHS?=
 =?us-ascii?Q?j2eE4IwECndad4UsLJG/fWfkpo1gLY99Cu/+NT6+IUqCbDpSRg5sq2OZ0Zct?=
 =?us-ascii?Q?BVnAOjnv/1HFcO/crXTsCaxmX/MZ7ZZWwdJr9Y7djih+FAywd1qfrDyl+BcE?=
 =?us-ascii?Q?qn559Xj/m9+fZX7FIk7uBfimy5/k/dMWjqkskjf4z1LEAnpbTZ020FTi4d31?=
 =?us-ascii?Q?Ez94j8m1qeEYuo8Lq7VyCacjXwoyNdmTxnsuTqfYcIf6LogUEyepeNgJ/iC/?=
 =?us-ascii?Q?9c3z6Jko9Hu3YIP9XkQzm9nJSWL5cM7DuDIE7k0h7t3XxV0zTDoS9Ve3fp1n?=
 =?us-ascii?Q?RIFM95i+At4ZPENIXoys888QG+Janh8tPSTz7Ohtqwe8/AWtG9pq07RXJQG6?=
 =?us-ascii?Q?a+SAgrXGv76YC56c01fTCGCBzBq79vLUUR7LJE634MbGi/u3tw8cLN4UHYhj?=
 =?us-ascii?Q?OBxS1uF6g0PG0yOoqs1L5pUZo2BcokWdA7nZnAX2TYMFcDIzM/vbjIEYkCsr?=
 =?us-ascii?Q?gCS18aGUexYolLxObM2z5YKN009mo5eXOd9/tqvFmFL14/4tfQpL4I9vGyW+?=
 =?us-ascii?Q?FMS65oolC0gvSlvsQ6p/KcPCmFYjnRnNn0gVxu4jqgzurwhi8atfQRAL99Fa?=
 =?us-ascii?Q?m0HCx2IR1yb1ORoqfRy4IBPvCAce1NnoYqhMXNHfDZdcZ9U+xlD/gl4Bq2Fc?=
 =?us-ascii?Q?wbfzmwrOk5Q09C4MfoUV1vbzzqn6h5IwjwTZPugC?=
X-OriginatorOrg: Nvidia.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 3e2f8ef3-702e-4946-0a05-08dc607a8984
X-MS-Exchange-CrossTenant-AuthSource: DM6PR12MB3849.namprd12.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 19 Apr 2024 14:10:59.0474
 (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: ZP9vOOaHyOJtHVHtVYIxO94dl3iHOk/Fxd/M/nRXFupHR07/shL0mBuwfpXCPfsN
X-MS-Exchange-Transport-CrossTenantHeadersStamped: DM4PR12MB9071

On Fri, Apr 19, 2024 at 05:23:30AM +0000, Tian, Kevin wrote:
> > From: Alex Williamson <alex.williamson@redhat.com>
> > Sent: Friday, April 19, 2024 6:55 AM
> > 
> > On Wed, 17 Apr 2024 22:31:41 +0800
> > Xin Zeng <xin.zeng@intel.com> wrote:
> > 
> > > +
> > > +	/*
> > > +	 * As the device is not capable of just stopping P2P DMAs, suspend
> > the
> > > +	 * device completely once any of the P2P states are reached.
> > > +	 * On the opposite direction, resume the device after transiting from
> > > +	 * the P2P state.
> > > +	 */
> > > +	if ((cur == VFIO_DEVICE_STATE_RUNNING && new ==
> > VFIO_DEVICE_STATE_RUNNING_P2P) ||
> > > +	    (cur == VFIO_DEVICE_STATE_PRE_COPY && new ==
> > VFIO_DEVICE_STATE_PRE_COPY_P2P)) {
> > > +		ret = qat_vfmig_suspend(qat_vdev->mdev);
> > > +		if (ret)
> > > +			return ERR_PTR(ret);
> > > +		return NULL;
> > > +	}
> > 
> > This doesn't appear to be a valid way to support P2P, the P2P states
> > are defined as running states.  The guest driver may legitimately
> > access and modify the device state during P2P states. 
> 
> yes it's a conceptual violation of the definition of the P2P states.

It depends what suspend actually does.

Like if it halts all queues and keeps them halted, while still
allowing queue head/tail pointer updats then it would be a fine
implementation for P2P.

> > Should this device be advertising support for P2P?
> 
> Jason suggests all new migration drivers must support P2P state.
> In an old discussion [1]

I did? I don't think that is what the link says..

We've been saying for a while that devices should try hard to
implement P2P because if they don't then multi VFIO VMM's won't work
and people will be unhappy..

> But obviously we overlooked that by definition RUNNING_P2P is
> a running state so could still see state changed from either CPU
> or other devices.

Yes, incoming MMIO writes that change the state must be recorded, but
as long as DMA does not start and remains suspended then it is OK.

Jason