Received-SPF: pass (google.com: domain of linux-kernel+bounces-134767-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>, "iommu@lists.linux.dev"
	<iommu@lists.linux.dev>
CC: Jason Gunthorpe <jgg@ziepe.ca>, Joerg Roedel <joro@8bytes.org>, "Will
 Deacon" <will@kernel.org>, Robin Murphy <robin.murphy@arm.com>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Subject: RE: [PATCH 1/1] iommu/vt-d: Fix WARN_ON in iommu probe path
Thread-Topic: [PATCH 1/1] iommu/vt-d: Fix WARN_ON in iommu probe path
Thread-Index: AQHaiIkfDCiYXSWkLUGWnjJZsYe/DLFdt7FQ
Date: Mon, 8 Apr 2024 03:52:54 +0000
Message-ID: <BN9PR11MB52760913F36DACBC612088668C002@BN9PR11MB5276.namprd11.prod.outlook.com>
References: <20240407011429.136282-1-baolu.lu@linux.intel.com>
In-Reply-To: <20240407011429.136282-1-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?0Rm6h5zatkXY/qcbXaoAXDuYA5T0ON++YeL+2s2cj4VxIMqRF8jcVDrpN1Rg?=
 =?us-ascii?Q?4x/JWGIyJKdzAKdkE+MhkwYwQFzAdEnhG/WC3wqzkU4J2Khe75IAqd8afyfM?=
 =?us-ascii?Q?hEgIWc7Bu3zkBucfB3u01c41azyi1sTwS/JuRHdkmvQxWGS/tLmhxsHqaLnx?=
 =?us-ascii?Q?N5mc/2SRHcQTkk6wdAh5oqwcxIQxrXpQahLT8T/MUCyHHgNDXl36iGVVQyGc?=
 =?us-ascii?Q?wmeIvSiGEczP781qMFbtMexyps+2IL40AE8i9KxiuLxYbWDbIU/pjqX4TBsO?=
 =?us-ascii?Q?VdgqjnGRwtinEUOWexZ08wn+CuulPOlOPCZvAN1//1Rn2lcz7wX9mSuAKmmG?=
 =?us-ascii?Q?t1sTefLoZ4NBQBAuOkUQaI7iTlHd9GqvYUtTUN3wxtAT/+clHnxSX+cFIjS3?=
 =?us-ascii?Q?ceT3fqMm1YGpldaX+9Lk/BDiU74t/Z+ybxJCVevD6hrgzdIW5pTsEBp9nd+d?=
 =?us-ascii?Q?OzrXTdjhySdZuwlTQGpsejblKLYCuQ9Czfw+G9BQkvhfC9BVwo4T7SDI+mqQ?=
 =?us-ascii?Q?+ba+v4YM3iiiUb4BjsR+8XjhuNRE4rm9ox4xjWwK7dA/Ml+J4DMKQPKGablF?=
 =?us-ascii?Q?ViahfhTZ6W7wdRU7UkvJ4/dcmGBRTl8G1ccdiVFxkEXC6jS0RCcsumsb3Gc0?=
 =?us-ascii?Q?2IxjR82CE9AQIEbwRsaROAFBqcj2G/8sju1dopaSeyzFyAW9MdkPcC3oflGe?=
 =?us-ascii?Q?SpWIVIVeaPhU+ih6c/XpPofsmJbdj/tm5RbBIkPMxw5AA/637i2+Qp9P1OEA?=
 =?us-ascii?Q?8hv+AjVMPrgZ+ib9lkHN3LG7AupaHBJstN9QXEXYXcuuAZ3enX+xZqBhW9pS?=
 =?us-ascii?Q?f8qNk2ss9Qx9zxohuz1S/Oo03LrveMSP2+/mn9qPAXRryS3Vh/Qfj309raES?=
 =?us-ascii?Q?lU2kgEvznbFdIIrdE2hM5rKPPIZKih7MB2d9RxVoVv2ZQXD2GadBeHwsKdcY?=
 =?us-ascii?Q?UfmZITo4Lhye3/p2pfd+z8S5mtyPVJLTMkXSx1A5fTpPjZOlm4J9Hxip3F+X?=
 =?us-ascii?Q?3f5h18YPXcJH2wjBzMGwuxHRoe+eyUMJwQpZKVmDwFWfR1yhfNpWKCneHHiD?=
 =?us-ascii?Q?8Oc/3QD9+osNb2C+s/ytk66UeZ9OdpFnu+w00ezC7bRlQTQXiOYbgho6+Oec?=
 =?us-ascii?Q?uHrMbIJ9GhG23gQ2SKj5rms1dl2sn0kc783LrKEIzRQT1B9xz8LsoPnRKVzw?=
 =?us-ascii?Q?4Y4xTANzo+HOZLd29Vf7TpIEN2cc1o5I2L3NOWzRW9lu+V15OZrMxc59lN+5?=
 =?us-ascii?Q?jXEWb/PI1X6R5bRGK7mffsMo+h0O4IeAuI9pc+cpTj+9AQuBLhaa1uD5gJGQ?=
 =?us-ascii?Q?ZSWTqZ3+32XaGUzZuj/BATr41N/60P5KwcBDoAFAYYMDULRFMi7oZZbS7pA6?=
 =?us-ascii?Q?iRim2RhNb9RvAXw0hd71ZqNdCiZAwEPKaNwdHIQn/ytAcR+uhdtiMvVcLPTb?=
 =?us-ascii?Q?MMnTqTaXq1w5Fetu/A+oAqaDVcIkWnMjDxoO01D6kVkwgAGUjBPauGEu2DeH?=
 =?us-ascii?Q?Xyqx3i90rtZYGeTSS6Kpe5L2yFGk/2qsQSHAcnw1pQ7irBftkX2xGJjlz5D6?=
 =?us-ascii?Q?LEOSRKu6sv+EmmlHFjxQO/4bK9ys9S8jJZHr+c7+?=
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: e7fc723f-ae26-41a4-573a-08dc577f5f34
X-MS-Exchange-CrossTenant-originalarrivaltime: 08 Apr 2024 03:52:54.9292
 (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: rpLmdwe33DyQlk+WE+FP2PIrN/oyNTB7qbdW5sdvWovWm3MHbhNeyGIkfRLmhEgYbt5jwyhf7CrsELwVzHHvHQ==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: DS0PR11MB8069
X-OriginatorOrg: intel.com

> From: Lu Baolu <baolu.lu@linux.intel.com>
> Sent: Sunday, April 7, 2024 9:14 AM
>=20
> Commit 1a75cc710b95 ("iommu/vt-d: Use rbtree to track iommu probed
> devices") adds all devices probed by the iommu driver in a rbtree
> indexed by the source ID of each device. It assumes that each device
> has a unique source ID. This assumption is incorrect and the VT-d
> spec doesn't state this requirement either.
>=20
> The reason for using a rbtree to track devices is to look up the device
> with PCI bus and devfunc in the paths of handling ATS invalidation time
> out error and the PRI I/O page faults. Both are PCI ATS feature related.
>=20
> Only track the devices that have PCI ATS capabilities in the rbtree to
> avoid unnecessary WARN_ON in the iommu probe path. Otherwise, on some
> platforms below kernel splat will be displayed and the iommu probe result=
s
> in failure.

Just be curious. What about two ATS capable devices putting behind
a PCI-to-PCIe bridge?

>=20
>  WARNING: CPU: 3 PID: 166 at drivers/iommu/intel/iommu.c:158
> intel_iommu_probe_device+0x319/0xd90
>  Call Trace:
>   <TASK>
>   ? __warn+0x7e/0x180
>   ? intel_iommu_probe_device+0x319/0xd90
>   ? report_bug+0x1f8/0x200
>   ? handle_bug+0x3c/0x70
>   ? exc_invalid_op+0x18/0x70
>   ? asm_exc_invalid_op+0x1a/0x20
>   ? intel_iommu_probe_device+0x319/0xd90
>   ? debug_mutex_init+0x37/0x50
>   __iommu_probe_device+0xf2/0x4f0
>   iommu_probe_device+0x22/0x70
>   iommu_bus_notifier+0x1e/0x40
>   notifier_call_chain+0x46/0x150
>   blocking_notifier_call_chain+0x42/0x60
>   bus_notify+0x2f/0x50
>   device_add+0x5ed/0x7e0
>   platform_device_add+0xf5/0x240
>   mfd_add_devices+0x3f9/0x500
>   ? preempt_count_add+0x4c/0xa0
>   ? up_write+0xa2/0x1b0
>   ? __debugfs_create_file+0xe3/0x150
>   intel_lpss_probe+0x49f/0x5b0
>   ? pci_conf1_write+0xa3/0xf0
>   intel_lpss_pci_probe+0xcf/0x110 [intel_lpss_pci]
>   pci_device_probe+0x95/0x120
>   really_probe+0xd9/0x370
>   ? __pfx___driver_attach+0x10/0x10
>   __driver_probe_device+0x73/0x150
>   driver_probe_device+0x19/0xa0
>   __driver_attach+0xb6/0x180
>   ? __pfx___driver_attach+0x10/0x10
>   bus_for_each_dev+0x77/0xd0
>   bus_add_driver+0x114/0x210
>   driver_register+0x5b/0x110
>   ? __pfx_intel_lpss_pci_driver_init+0x10/0x10 [intel_lpss_pci]
>   do_one_initcall+0x57/0x2b0
>   ? kmalloc_trace+0x21e/0x280
>   ? do_init_module+0x1e/0x210
>   do_init_module+0x5f/0x210
>   load_module+0x1d37/0x1fc0
>   ? init_module_from_file+0x86/0xd0
>   init_module_from_file+0x86/0xd0
>   idempotent_init_module+0x17c/0x230
>   __x64_sys_finit_module+0x56/0xb0
>   do_syscall_64+0x6e/0x140
>   entry_SYSCALL_64_after_hwframe+0x71/0x79
>=20
> Fixes: 1a75cc710b95 ("iommu/vt-d: Use rbtree to track iommu probed
> devices")
> Signed-off-by: Lu Baolu <baolu.lu@linux.intel.com>
> ---
>  drivers/iommu/intel/iommu.c | 11 +++++++----
>  1 file changed, 7 insertions(+), 4 deletions(-)
>=20
> diff --git a/drivers/iommu/intel/iommu.c b/drivers/iommu/intel/iommu.c
> index 50eb9aed47cc..a7ecd90303dc 100644
> --- a/drivers/iommu/intel/iommu.c
> +++ b/drivers/iommu/intel/iommu.c
> @@ -4299,9 +4299,11 @@ static struct iommu_device
> *intel_iommu_probe_device(struct device *dev)
>  	}
>=20
>  	dev_iommu_priv_set(dev, info);
> -	ret =3D device_rbtree_insert(iommu, info);
> -	if (ret)
> -		goto free;
> +	if (pdev && pci_ats_supported(pdev)) {
> +		ret =3D device_rbtree_insert(iommu, info);
> +		if (ret)
> +			goto free;
> +	}

probably replace device_rbtree with ats_rbtree?

>=20
>  	if (sm_supported(iommu) && !dev_is_real_dma_subdevice(dev)) {
>  		ret =3D intel_pasid_alloc_table(dev);
> @@ -4336,7 +4338,8 @@ static void intel_iommu_release_device(struct
> device *dev)
>  	struct intel_iommu *iommu =3D info->iommu;
>=20
>  	mutex_lock(&iommu->iopf_lock);
> -	device_rbtree_remove(info);
> +	if (dev_is_pci(dev) && pci_ats_supported(to_pci_dev(dev)))
> +		device_rbtree_remove(info);
>  	mutex_unlock(&iommu->iopf_lock);
>=20
>  	if (sm_supported(iommu) && !dev_is_real_dma_subdevice(dev) &&
> --
> 2.34.1