Received-SPF: pass (google.com: domain of linux-kernel+bounces-52235-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:4601:e00::3 as permitted sender) client-ip=2604:1380:4601:e00::3;
From: "Tian, Kevin" <kevin.tian@intel.com>
To: Lu Baolu <baolu.lu@linux.intel.com>, Joerg Roedel <joro@8bytes.org>, "Will
 Deacon" <will@kernel.org>, Robin Murphy <robin.murphy@arm.com>, "Jason
 Gunthorpe" <jgg@ziepe.ca>, Jean-Philippe Brucker <jean-philippe@linaro.org>,
	Nicolin Chen <nicolinc@nvidia.com>
CC: "Liu, Yi L" <yi.l.liu@intel.com>, Jacob Pan
	<jacob.jun.pan@linux.intel.com>, Longfang Liu <liulongfang@huawei.com>,
	"Zhao, Yan Y" <yan.y.zhao@intel.com>, Joel Granados <j.granados@samsung.com>,
	"iommu@lists.linux.dev" <iommu@lists.linux.dev>, "kvm@vger.kernel.org"
	<kvm@vger.kernel.org>, "linux-kernel@vger.kernel.org"
	<linux-kernel@vger.kernel.org>, Jason Gunthorpe <jgg@nvidia.com>
Subject: RE: [PATCH v11 13/16] iommu: Improve iopf_queue_remove_device()
Thread-Topic: [PATCH v11 13/16] iommu: Improve iopf_queue_remove_device()
Thread-Index: AQHaU1RzuvzbDOxmwk6/Qx3I8sJBXLD7dmtA
Date: Mon, 5 Feb 2024 09:00:51 +0000
Message-ID: <BN9PR11MB5276E70CAB272B212977F0C98C472@BN9PR11MB5276.namprd11.prod.outlook.com>
References: <20240130080835.58921-1-baolu.lu@linux.intel.com>
 <20240130080835.58921-14-baolu.lu@linux.intel.com>
In-Reply-To: <20240130080835.58921-14-baolu.lu@linux.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?YvlwzyoqXvvNSVveR1kM7j5JwwUWBs+0w89hnB1727Q/CxpTQOG9SO6Trs8Y?=
 =?us-ascii?Q?Ib3m8SXNUM0m8tmZWW8oZtBxNmR+Adr0i0NO2MwsdysyXqaUIfgAa23NKeyH?=
 =?us-ascii?Q?rUuWSZS12Z52aICDBe38wPgHvGQIVM4xKLjR73jVWIa8QZwBmqluEv0BSM4R?=
 =?us-ascii?Q?MThjG2g5liYS4PYfkNCsTJ9PyF7IkHwx29B4iWGvOjZ1OUIGg1xNrNMiIRhS?=
 =?us-ascii?Q?ZP3driLhc28UN6REu4UGsIsilx1bLsbSpW40QlNqhxp2A2YB9bu86cbY+Hef?=
 =?us-ascii?Q?OjO8qIub6Bma04tPgnY6AwrZglsiGQpdAFXYeWw7NoJMgbuAiFtT29hle6s9?=
 =?us-ascii?Q?2oO3HZCenJqUCahevEgSEWC3oCitvQV/6lNG+HZKTjRN7++Yo7jS9ajLQFlo?=
 =?us-ascii?Q?iKRqK0X5I5EkNqaJZHPe6pR+t8vfyhgY7AmFg4C7xL0IZYGEXHB26M2Eid95?=
 =?us-ascii?Q?o7mZH7ZFNXgY7A5ehECb2u6PIFc4opbjuG0zKrQ5MHcJ4Q58eOYD9NYu2RLN?=
 =?us-ascii?Q?juNLH97yAKSiYk8qdxGXJoZdf1I9spfRWqjHAp3MegETNizgXRtELyPt0Ldy?=
 =?us-ascii?Q?lLUr5TvQEFFsaq3plROQb6iKkmC5tKXyaMvEVhzyZU+mAUnNdN0HIamiaUFv?=
 =?us-ascii?Q?eh34x/G0aWOBoK4U0mW/U9yEdzhQRLMj6Ex6TfDf38LpiwYKH4f0XKdKjEtU?=
 =?us-ascii?Q?AG90ZCNWNwm7h0bJINWSq9DA8V2dI255WHmVvEKlxTBdwNBzVG5WhExiInSg?=
 =?us-ascii?Q?/KDS4uN/8BENyj0jxJKeu3o2E1rH1OLE4Fas0WbcrUyee9qqz3rREIPNTkn3?=
 =?us-ascii?Q?K+i+VpW2dS0fcs4Fu/ecSMlA0IDMnXtYScwjJc8pFB1BVOVq/IrGKec7QOP3?=
 =?us-ascii?Q?MaiUezRlg2ghyF2echmh5LEYoqfH59/oXkMvl9hDaEa0ThrjDOXTDZSjFQHS?=
 =?us-ascii?Q?rTC085dmVQZhytdj2J84ZCmbklLAhU4p7eefIVcLKomqwquTC5b/a47LCsTv?=
 =?us-ascii?Q?r90qQcRXK2IUoV1YjwaMlBafWlKpOUiC0cNvLq/6fu6FDLHM4weJcpxloCuN?=
 =?us-ascii?Q?IIzQvhp5ZG7bKH+mvBS/SduzOz2dqfjeEyj08Lz6RQL1NMWqMpwNW+XPixgD?=
 =?us-ascii?Q?i1b5qzQd8ovCtxk3Q2Xi5IPdyHv9Pnms6dBrQnLBp7mbcf3uDFTMhoJJfT0e?=
 =?us-ascii?Q?Uz9nN72eMvCcKGSh1fYdqAKkNvjFdCPmANUuWeoTUrr9D/rkeViQt7ctvUFE?=
 =?us-ascii?Q?eD7zBhGtHKZgwgilc/0HKL0eBlsHaipsofw1r0i2BqyUfpCHxjH1rUYwt2cY?=
 =?us-ascii?Q?xkjNqeRWWOLyxkmut5Smzrw3P04298TuDmfxZpnpOKZYwhkLURDeQen/rARs?=
 =?us-ascii?Q?tYU1sp0aV7FzumHgnqDHMWIEjJyMypatvUx6IUioUmgekn9NdJunvad3QUPW?=
 =?us-ascii?Q?3nwm4ER7/QXvtrlTq+k5ka9FkbXIeu+CgoIVJT1rQUXWYwALLIt6FYjSeWnh?=
 =?us-ascii?Q?s1+4Fyopn/LaYvNLSkXDnC0k7LTchlb1NduzES9qQD+yzyeba535B8lyOv2F?=
 =?us-ascii?Q?Vo4cnvy//87Podua5XjrT8jmP7IBSK2QeSFtc/+R?=
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: quoted-printable
Precedence: bulk
X-Mailing-List: linux-kernel@vger.kernel.org
List-Id: <linux-kernel.vger.kernel.org>
List-Subscribe: <mailto:linux-kernel+subscribe@vger.kernel.org>
List-Unsubscribe: <mailto:linux-kernel+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: ea811b2e-c16e-4869-a69a-08dc2628f449
X-MS-Exchange-CrossTenant-originalarrivaltime: 05 Feb 2024 09:00:51.8201
 (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: vBssPuqWgGgdX2Z9gF3yIKT3EUf8r3aXqwdRQj/3R5zp3pZ6loS9rbdq4QuwSUXqSKfafAznCNjTZAjWc2bEsA==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: CH3PR11MB8519
X-OriginatorOrg: intel.com

> From: Lu Baolu <baolu.lu@linux.intel.com>
> Sent: Tuesday, January 30, 2024 4:09 PM
>   *
> - * Caller makes sure that no more faults are reported for this device.
> + * Removing a device from an iopf_queue. It's recommended to follow
> these
> + * steps when removing a device:
>   *
> - * Return: 0 on success and <0 on error.
> + * - Disable new PRI reception: Turn off PRI generation in the IOMMU
> hardware
> + *   and flush any hardware page request queues. This should be done
> before
> + *   calling into this helper.

this 1st step is already not followed by intel-iommu driver. The Page
Request Enable (PRE) bit is set in the context entry when a device
is attached to the default domain and cleared only in
intel_iommu_release_device().

but iopf_queue_remove_device() is called when IOMMU_DEV_FEAT_IOPF
is disabled e.g. when idxd driver is unbound from the device.

so the order is already violated.

> + * - Acknowledge all outstanding PRQs to the device: Respond to all
> outstanding
> + *   page requests with IOMMU_PAGE_RESP_INVALID, indicating the device
> should
> + *   not retry. This helper function handles this.
> + * - Disable PRI on the device: After calling this helper, the caller co=
uld
> + *   then disable PRI on the device.

intel_iommu_disable_iopf() disables PRI cap before calling this helper.

> + * - Tear down the iopf infrastructure: Calling iopf_queue_remove_device=
()
> + *   essentially disassociates the device. The fault_param might still e=
xist,
> + *   but iommu_page_response() will do nothing. The device fault paramet=
er
> + *   reference count has been properly passed from
> iommu_report_device_fault()
> + *   to the fault handling work, and will eventually be released after
> + *   iommu_page_response().

it's unclear what 'tear down' means here.=20