Received-SPF: pass (google.com: domain of linux-kernel+bounces-90142-linux.lists.archive=gmail.com@vger.kernel.org designates 139.178.88.99 as permitted sender) client-ip=139.178.88.99;
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>, "Badger, Eric" <ebadger@purestorage.com>
CC: "iommu@lists.linux.dev" <iommu@lists.linux.dev>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Subject: RE: [PATCH v2 2/2] iommu/vt-d: Fix NULL domain on device release
Thread-Topic: [PATCH v2 2/2] iommu/vt-d: Fix NULL domain on device release
Thread-Index: AQHaavUDCy/i5tLpVk2E42aG3RqhHLEnNNfQ
Date: Mon, 4 Mar 2024 07:36:45 +0000
Message-ID: <BN9PR11MB52764189F754ABF69D24E2AE8C232@BN9PR11MB5276.namprd11.prod.outlook.com>
References: <20240229094613.121575-1-baolu.lu@linux.intel.com>
 <20240229094613.121575-3-baolu.lu@linux.intel.com>
In-Reply-To: <20240229094613.121575-3-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?gJYmiXk6PpKT+0xu65vTJVaE3Diy2bAJcMAY9hExO1iMB9iPPJ2YRaguUTdb?=
 =?us-ascii?Q?sBY+qsr6deA+UstsVd1whHZjVBejcMCWvOfEdMuk4JU2FOeDDiWCZpiRZvtJ?=
 =?us-ascii?Q?PqJY1BZioQYK+ALrxUjiHJ0zQkdXjp2hanyyY2pEFzshOHPfdnZR8z61ISXG?=
 =?us-ascii?Q?4j+uuuI4Kxiz8fUsxnJyQyB3B8RAF6fV1ijKQI6GesSF2/cB+8hnxQRQ0RK1?=
 =?us-ascii?Q?iOJX40btax1th3oSDApYDTQtGGAO/in/7ZlqODlQdAkpYyZ1/r+r5W1vatLB?=
 =?us-ascii?Q?R44VsKBCq9bcfvdI/xZfWjqKfloi6cJKj/umL/qIKmiya2IY6EXYuUcl9Ot1?=
 =?us-ascii?Q?j3iMTdFqM9EEgRyTejcdA9yVhSbkz28b9gtH5GidTSXiIsri4Him/dI/p7Y1?=
 =?us-ascii?Q?opb5i7mbiaLxEPDrCIPehaZOeFylwHl9OzMlCZx2YvNOVwfBYltPbAhVhWsP?=
 =?us-ascii?Q?VbxIdIRm9IsRVncy2yTHx+udXpPcEOGvdrUr7FpmC2tXXBXVv1PYeRJ9GFB9?=
 =?us-ascii?Q?EaT/Ksy+Lo5A3qtEN1f9E36+yOD80p/ebcDGSRXHflNFSSKktRDPmOb9SBco?=
 =?us-ascii?Q?Hv6ounP8NZHZUqBPe/XQdmAV3K9YqHkqxX5zRMVq5fzsZgXmfUzQBKaODOx7?=
 =?us-ascii?Q?oWwjn8yYssU+XYVGlhSPc9UZwaFu+C++Z6eRBq2AQwzEoSHcsiWBwIkiMlyx?=
 =?us-ascii?Q?4D+1WqNb6mWKwFwDTDXEu+cSiSKakRC++Z/0H/Fg49Tow82ntl9TJFnFPHGy?=
 =?us-ascii?Q?KXtwi9LhD4GGSDrdWUSOLi6Ln4izlSbgh5KRTWhJ8W1ri/Lsq9YsfOYtCp7Y?=
 =?us-ascii?Q?2scWlwOwOwAnFEKA/zyRLMTCV+Usrkz34Spj3ExYU2oEtrILWUlZYBlbwXFd?=
 =?us-ascii?Q?GIPk95nIf+z9qbE4IL7YFwJywRTnLywLaA8amRSY2bqyBWeBjyyspqoRZdyJ?=
 =?us-ascii?Q?qEjw9S2T7IYj25QET3OzWB2zBVkJCdwVJ/5XBLtHFTN4Fd0ycfZo8SsnYFim?=
 =?us-ascii?Q?OZMhAn8TP1gtD9+EyM14uOcAj2xNWD5o4lSNjI0cf4dbBn/rdGO0ZQL70fwt?=
 =?us-ascii?Q?A7qxQRH72tksJzOgdflhw1XkZqVirQ2FVQ5i7D5V5CRqxD6QSfhkO1XCqlJi?=
 =?us-ascii?Q?kzMEaJ7FmAR5gcIHhJCpv8kEElEAbGgXM7AYOOhjfzxcE0Oynv9VjQm5cB7F?=
 =?us-ascii?Q?nN9e5aNXNsh+rJqJsfsXRburmL0xpPgpKfUahiuoreziOLj2M/MgP6UnC6hS?=
 =?us-ascii?Q?0GKaHV2umj7sz0BhC+uw6GaY5dosQo/2O7WB6olwT0mb/Ndksjy7IbIjz49y?=
 =?us-ascii?Q?ZkT936gGZkUaK2ijBy0d+eqeFApTkeGYCFzkL/dq81y7fZxazkLVBz6SoxiB?=
 =?us-ascii?Q?0fCiDTiDEc+cMGHde/zmGMleumzy8HUWzKc6brz96lmwbQjGbBcUnEerOH+D?=
 =?us-ascii?Q?6PcIPiJRxsYTIFUNsDFNXwrfBxL0wSjuwTEKzwkWB8UQZYBGyz6mRrxGSist?=
 =?us-ascii?Q?wL/HobStrvLYJswSf2pDVV0r4DU0w7sIxjq9W+I10SBn0VOIWfeJMzj+Itlu?=
 =?us-ascii?Q?w1iiR1A2BzEf3kQZwnfIGmvaf+oXfOgTa+2sYyQY?=
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: 06a04caa-84ff-4752-990c-08dc3c1dd800
X-MS-Exchange-CrossTenant-originalarrivaltime: 04 Mar 2024 07:36:45.5072
 (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: MgnUwjoM+lxvB4SzObGmklelkdtcAtq4/a46KByigUKZATtLtJ3CLYeF8YAnHr7aiWqNiAWbYsCAKUAhLJIN8g==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: MN0PR11MB6086
X-OriginatorOrg: intel.com

> From: Lu Baolu <baolu.lu@linux.intel.com>
> Sent: Thursday, February 29, 2024 5:46 PM
>=20
> +
> +/*
> + * Cache invalidation for changes to a scalable-mode context table
> + * entry.
> + *
> + * Section 6.5.3.3 of the VT-d spec:
> + * - Device-selective context-cache invalidation;
> + * - Domain-selective PASID-cache invalidation to affected domains
> + *   (can be skipped if all PASID entries were not-present);
> + * - Domain-selective IOTLB invalidation to affected domains;

the spec talks about domain-selective but the code actually does
global invalidation.

> + * - Global Device-TLB invalidation to affected functions.
> + *
> + * Note that RWBF (Required Write-Buffer Flushing) capability has
> + * been deprecated for scable mode. Section 11.4.2 of the VT-d spec:
> + *
> + * HRWBF: Hardware implementations reporting Scalable Mode Translation
> + * Support (SMTS) as Set also report this field as Clear.

RWBF info is a bit weird given existing code doesn't touch it

> + */
> +static void sm_context_flush_caches(struct device *dev)
> +{
> +	struct device_domain_info *info =3D dev_iommu_priv_get(dev);
> +	struct intel_iommu *iommu =3D info->iommu;
> +
> +	iommu->flush.flush_context(iommu, 0, PCI_DEVID(info->bus, info-
> >devfn),
> +				   DMA_CCMD_MASK_NOBIT,
> DMA_CCMD_DEVICE_INVL);
> +	qi_flush_pasid_cache(iommu, 0, QI_PC_GLOBAL, 0);
> +	iommu->flush.flush_iotlb(iommu, 0, 0, 0, DMA_TLB_GLOBAL_FLUSH);
> +	devtlb_invalidation_with_pasid(iommu, dev, IOMMU_NO_PASID);
> +}
> +
> +static void context_entry_teardown_pasid_table(struct intel_iommu
> *iommu,
> +					       struct context_entry *context)
> +{
> +	context_clear_entry(context);
> +	if (!ecap_coherent(iommu->ecap))
> +		clflush_cache_range(context, sizeof(*context));

this is __iommu_flush_cache(). You can use it throughout this and
the 2nd series.

> +
> +void intel_pasid_teardown_sm_context(struct device *dev)
> +{

it's clearer to call it just intel_teardown_sm_context. pasid_table
is one field in the context entry. Having pasid leading is slightly
confusing.