From:   "Tian, Kevin" <kevin.tian@intel.com>
To:     Jason Gunthorpe <jgg@nvidia.com>
CC:     Nicolin Chen <nicolinc@nvidia.com>,
        "joro@8bytes.org" <joro@8bytes.org>,
        "will@kernel.org" <will@kernel.org>,
        "robin.murphy@arm.com" <robin.murphy@arm.com>,
        "alex.williamson@redhat.com" <alex.williamson@redhat.com>,
        "shuah@kernel.org" <shuah@kernel.org>,
        "Liu, Yi L" <yi.l.liu@intel.com>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
        "iommu@lists.linux.dev" <iommu@lists.linux.dev>,
        "kvm@vger.kernel.org" <kvm@vger.kernel.org>,
        "linux-kselftest@vger.kernel.org" <linux-kselftest@vger.kernel.org>,
        "baolu.lu@linux.intel.com" <baolu.lu@linux.intel.com>
Subject: RE: [PATCH v2 07/10] iommufd/device: Make hwpt_list list_add/del
 symmetric
Thread-Topic: [PATCH v2 07/10] iommufd/device: Make hwpt_list list_add/del
 symmetric
Thread-Index: AQHZOzoSQMv1v33l6Eyp7LBz8BGoIq7F9D6AgACo6ACAAM2SAIABSPOAgAN3GSA=
Date:   Mon, 13 Feb 2023 02:12:49 +0000
Message-ID: <BN9PR11MB5276914C26D3063AD77DDDD98CDD9@BN9PR11MB5276.namprd11.prod.outlook.com>
References: <cover.1675802050.git.nicolinc@nvidia.com>
 <9d0d4f935c4972771f5aed4b4837d8ae35814e06.1675802050.git.nicolinc@nvidia.com>
 <BN9PR11MB5276F36D2F07209AC7155BAB8CD99@BN9PR11MB5276.namprd11.prod.outlook.com>
 <Y+Tz/kkZ68e9QWR/@nvidia.com>
 <BN9PR11MB5276CA32940B6886F8238FA38CDE9@BN9PR11MB5276.namprd11.prod.outlook.com>
 <Y+a0YUwd+MVMZ1dN@nvidia.com>
In-Reply-To: <Y+a0YUwd+MVMZ1dN@nvidia.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?g/iYpl526boituLCZmJvTeylTx9UZFMLjSWxtcpxtiQ4T4InyuqdxOceLmja?=
 =?us-ascii?Q?WMqnYyrHn6Ie+8RSroswt3g5szQJg74YU/UTp93rKwWMGQwX0Po710zs+dRQ?=
 =?us-ascii?Q?8LwGGHjRBq8cjXpHXkNVLVAe6V00dzu7QIlxSKBwvIOIm2zY/LBf8Suk8OBX?=
 =?us-ascii?Q?SW5ufmfQHuFUznvblqtfroKfAS27coF0+Gm7tIIjjy6cFmhmld8vUmGJ7hqo?=
 =?us-ascii?Q?mduUoKe8XUI6RFg5jfMNfxUWZPpw6pQWcO9aZ8TlVH5XQEkx50l3z+jXHcAt?=
 =?us-ascii?Q?i2Pie75nuQpiPdSYa42Mz1vnZypYirLnirnoaXrg9SPqzdVV8eTAJhj0X6ia?=
 =?us-ascii?Q?M34C9+oR+b3noGPu0XdZzJnaCytteJtQMDf41KDNSJfNUoNa8lYnbAhPvkXb?=
 =?us-ascii?Q?5i/RtNfgxGzWmfJ6kJDU3AclDT0FKlZHovnGV1uPxJ/KFBUKR7FKzYTGi0pa?=
 =?us-ascii?Q?MRVaaxA0s75SbuZ9Q2DoUiqM0a8QYNfHTxIbVT4dcuZik1VU0Xt31PWlIvev?=
 =?us-ascii?Q?fC5NcuOhLWolDqfFr0oTDKFV/rgcUTxVZVZEUO9TKIk40vNYktOs0K3j/RzQ?=
 =?us-ascii?Q?FqxgPmXhVOvb//HpFviBRvO+/gHhRpNm7uY0knN5ss8fbmtnGbAPbUTj6jUJ?=
 =?us-ascii?Q?SmbWIJU3uGq5Ci59TzXnRBUxQzSEedqO9caAkid6UwmWk97lS3DE27YNG/ER?=
 =?us-ascii?Q?Y+LCEIGNa2DqMI/pvrDPEsKKEam1WLter+7PMrLsHYz8HIbh8oBXVkODp+Og?=
 =?us-ascii?Q?zyGtZgzkD+P/PRxriMs4kcLTK9XdbVQpBTOyQVMxyj+KK6DXx+YoHzWWNWof?=
 =?us-ascii?Q?Bkwu8UZwyYKz98Q5lQQlXcWJ2l/fWwgydTDhXIxWoagiu4jCyyk9FyUdbDmd?=
 =?us-ascii?Q?AtfA4XGJ5nIikV8bzbSVre1WbWd2B112ZjByzRvz5KaoTaBUIr5h+Oak5Bg1?=
 =?us-ascii?Q?G3PlEXbYKA1cz+zNHmioD7hcdL33QM5MyYkkKFbN5OKmXI1fNrd+OtELNs/R?=
 =?us-ascii?Q?Clfyqyv4rugMRiLAf4Iij+yrQG6gs5RgDDVnm0RwGOs6/7w6EOIRDpJAJcMJ?=
 =?us-ascii?Q?9Pk09nPKbehUBSgfmrokVWM+xVUi1NzFeWa3lPVAf099P6G4BwHX4E25p+6q?=
 =?us-ascii?Q?18yzy1jmbkuXV6FTrse+QLsPuiS4RRT1U/pkVTxXlEV6Y3q/QvJcmM54eexH?=
 =?us-ascii?Q?rmZsuddB127OB1bkkZjqBz7eEO7YXgIbXsySvGfRH8+GjvHCzw4dMiYeLsBp?=
 =?us-ascii?Q?tx4LY0+Ib5xLr/21G5MUeyvoUKzOPxNo46tNW3rV1b/SV6gO0ZS9V46iovBp?=
 =?us-ascii?Q?rQKfHWRYnpYWk9wgVLIqBrCzdFC3/d03zORzXSQngbhFtkTKP5QGl/1x19Ft?=
 =?us-ascii?Q?RMFwJrYzCZFBJ6x+w+dhJAEocz2/1aPL63/W/i0miol4NGmFA+hw1waVPBr0?=
 =?us-ascii?Q?paTHCjKtfBDQ7ZqWuuCOoVju6VXtlTyy6OP6U9SmccMu4v800KYmZz1pKVLC?=
 =?us-ascii?Q?XZeLdq/eAfTGQuhDHLK3Tp1/bq8l4TBmGaWasq4sEiXpYN7KezU04+ctU7nC?=
 =?us-ascii?Q?MsxexP6zsi//5kzF6/cZkWm9IsfmkjVvWgl9Vtm2?=
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: quoted-printable
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: 00bc607a-d194-4b88-aa06-08db0d67ce54
X-MS-Exchange-CrossTenant-originalarrivaltime: 13 Feb 2023 02:12:49.7348
 (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: eVB77lsOKkfJYwc0Qgaiyuh8Nog4MV0OvyvwZ5VUCoaZEQFIqGJid7xYpJyv4acVqdpLaBcKDYjyGoiPX3wMaw==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: IA0PR11MB7380
X-OriginatorOrg: intel.com
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

> From: Jason Gunthorpe <jgg@nvidia.com>
> Sent: Saturday, February 11, 2023 5:17 AM
>=20
> >
> > there is semantics change.
> >
> > here is the current code:
> >
> > 	case IOMMUFD_OBJ_HW_PAGETABLE: {
> > 		struct iommufd_hw_pagetable *hwpt =3D
> > 			container_of(pt_obj, struct iommufd_hw_pagetable,
> obj);
> >
> > 		rc =3D iommufd_device_do_attach(idev, hwpt);
> > 		if (rc)
> > 			goto out_put_pt_obj;
> >
> > 		mutex_lock(&hwpt->ioas->mutex);
> > 		list_add_tail(&hwpt->hwpt_item, &hwpt->ioas->hwpt_list);
> > 		mutex_unlock(&hwpt->ioas->mutex);
> > 		break;
> > 	}
> >
> > Above means every attach to hwpt will try to add the hwpt to the
> > list tail. Isn't it a bug?
>=20
> Yes, that looks like a bug..
>=20
> But this patch isn't the right way to fix that.
>=20
> The HWPT should be permanently linked to the IOAS as long as it
> exists, and the linkage should happen when it is first created.
>=20
> So attaching a HWPT to another device should never re-link it to the
> ioas, thus delete these lines here.
>=20

yes, this makes more sense