From:   "Tian, Kevin" <kevin.tian@intel.com>
To:     Nicolin Chen <nicolinc@nvidia.com>
CC:     "jgg@nvidia.com" <jgg@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 08/10] iommufd/device: Use iommu_group_replace_domain()
Thread-Topic: [PATCH v2 08/10] iommufd/device: Use
 iommu_group_replace_domain()
Thread-Index: AQHZOzoV+UAbwLQwnEuO17sJd5b+0K7F+NzggAEnS4CAAE2/cIAG4ngAgAD1Q2A=
Date:   Wed, 15 Feb 2023 01:38:32 +0000
Message-ID: <BN9PR11MB5276A2CCAB714977F07AD0758CA39@BN9PR11MB5276.namprd11.prod.outlook.com>
References: <cover.1675802050.git.nicolinc@nvidia.com>
 <4653f009c3dacae8ebf3a4865aaa944aa9c7cc7e.1675802050.git.nicolinc@nvidia.com>
 <BN9PR11MB5276C1807B710CAD3E5820D78CD99@BN9PR11MB5276.namprd11.prod.outlook.com>
 <Y+Vh479cDD7LX2x/@Asurada-Nvidia>
 <BN9PR11MB5276268D3ED0360913A05C368CDE9@BN9PR11MB5276.namprd11.prod.outlook.com>
 <Y+tpkpNYil3duTIP@Asurada-Nvidia>
In-Reply-To: <Y+tpkpNYil3duTIP@Asurada-Nvidia>
Accept-Language: en-US
Content-Language: en-US
x-ms-exchange-antispam-messagedata-chunkcount: 1
x-ms-exchange-antispam-messagedata-0: =?us-ascii?Q?wzqBmPJEp4rtxthM6EJoa8GIq/AsEVYbvtVzPoGDpLUPAiUXN/fgEMpKu0GG?=
 =?us-ascii?Q?GNGBFxl3/x5kdtQ1aFx4QlNJaEOMT4c4ARXzO9+If3xnMi6eyQ/tWMmOVb/a?=
 =?us-ascii?Q?NEOgsqyH8IThpEAjuI4Uk3sNHc42XvW0ECG5llL/TCaMVtbdueLv3GCU15Ma?=
 =?us-ascii?Q?7V4GrgNRrovW6yNrVFyC/YIuWGAk4r/WaBW+F56hjP9bpaWJf4ZZkjobqrM8?=
 =?us-ascii?Q?qIJsXXyErBEamUG3ZnBbfEupG0qE+oJJ2N+8wyzF1wSeIEPUTAav6FDRZBxE?=
 =?us-ascii?Q?ADXJFaEsbjnWp9GDh/UjafOGfvXx+1FoqkOSAiK/JC7fU7Wp75jBPwehxg0M?=
 =?us-ascii?Q?puvdXrC3FczmHFbK1cKWh3MuD9ynaMFfqT2IxHpecmJuOWSkO2Gd7XF+vi+9?=
 =?us-ascii?Q?N4MX8pWLiJjECRiUOj02GiXHZ4K3j4QpDDuM0GbZIll8/RFMMAlkoH8R4qmQ?=
 =?us-ascii?Q?X4O2fjak4YRvYi9X07BPw8oZPRnSefndkk1wA+cspaweTrMLtoiXy4ffHdPp?=
 =?us-ascii?Q?CQEjJFRlkzBTLboyNCOrC3LMft0bqe0z1MxhK4LeNmqVing9YKw/XpidscSC?=
 =?us-ascii?Q?FarC6Z7U3dH1akv5kgnbrGZt91IyGELA97BO04+2Oyy/fA1KdgGrPh96Xveh?=
 =?us-ascii?Q?2kHcws0E8FDOUYDtQPCBKqUTo7dseeFWqZsLPKSCt/6ONZZn83jTlzMq6ZtO?=
 =?us-ascii?Q?T30J0ZNUo9nz6L+pMgyHmySIMwZu0vAfyvUPvJ3P5nCkgNfO4cFMyV2a7ka+?=
 =?us-ascii?Q?tSM1lWgDEV7WNz6LWNytZyIIZf/lwPeBocB3CyuxaQE5pxOTKwW4qAAecFEe?=
 =?us-ascii?Q?ydzp5eXesrw1M9ybihpfvTZjKoX4hLfNTWgD7k7wYMjPxvbMGvhqn5K2D/9g?=
 =?us-ascii?Q?5I0BX7zCVI9f59OGOZm29CNbJ6mDbSFgb3bcYsTmef3TKTVAkpab1hMfsNgG?=
 =?us-ascii?Q?pAxzYkTrEiumxDdVsmVxTEZhJzx+EryR1ScmjLGc4tXqGkD/6rQW3maND9vo?=
 =?us-ascii?Q?qAcsE3ykxFze2ydSfv5jWXg/fXKAmGRs2xKaaMyAuKPz4jcxBCzrnhMhjSJh?=
 =?us-ascii?Q?7yyE0n3CAJZnKpgzSP2BRp3bYzSo0WB19TUtWk7dXFbeTMS2RZEtzkJmqr6o?=
 =?us-ascii?Q?ia/FiiN5u0t3HoS1/lTBPa/rCDNiUHXojbomnlkVqc9l5dh+ap5BgAqJlEY3?=
 =?us-ascii?Q?t/MQbDyDXuL2/GJFYyGga5tLTnOPfsWpNa66+jY7IbBvaAGJ/YmEnFlHz1o8?=
 =?us-ascii?Q?EpsVQY2BPORqMgfunx0Kp1md9OQOadpqsd92gIXHPGjlHC61jzL47gdh6lwt?=
 =?us-ascii?Q?ln01z5kg557kQlNGa/A4PePmT6VpJyhiawDyTS+L5ql71mRl/+P5BpWMUfLz?=
 =?us-ascii?Q?ukqgJtboOOUQ6eD4NdeTs8HUGjrk+LEyUZM9rdcF+olTzTKbab8mdEhSJxWY?=
 =?us-ascii?Q?/MA5XCEfmhy+yjzbzcE9rrdVpuaZanWq5cKhtB0HhYFY8eSx7KGBGpdOOvUe?=
 =?us-ascii?Q?FlUxPC5rrFZ7mar/XFJVtsXGG82wUiBOye8xSF+EGBXXmc/xLs3VODkMHomM?=
 =?us-ascii?Q?iYFTdVXDBWMVUPDSgr0hQFadrA7V39qZfhVz+Ntn?=
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: f5603b8e-8689-4c80-a1f8-08db0ef55922
X-MS-Exchange-CrossTenant-originalarrivaltime: 15 Feb 2023 01:38:32.7650
 (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: j8X39JTtghaq8D4nutp4+6E6pFIwv4aLQUICJAxUJvUHXmwkFSlRDECWxvsLFCGckSRbyW4WubpkyqIv/Bn7Rw==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: DS0PR11MB7459
X-OriginatorOrg: intel.com
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

> From: Nicolin Chen <nicolinc@nvidia.com>
> Sent: Tuesday, February 14, 2023 7:00 PM
>=20
> On Fri, Feb 10, 2023 at 02:11:23AM +0000, Tian, Kevin wrote:
>=20
> > My confusion is that we have different flows between detach/attach
> > and replace.
> >
> > today with separate detach+attach we have following flow:
> >
> >         Remove device from current hwpt;
> >         if (last_device in hwpt) {
> >                 Remove hwpt domain from current iopt;
> >                 if (last_device in group)
> >                         detach group from hwpt domain;
> >         }
> >
> >         if (first device in group) {
> >                 attach group to new hwpt domain;
> >                 if (first_device in hwpt)
> >                         Add hwpt domain to new iopt;
> >         Add device to new hwpt;
> >
> > but replace flow is different on the detach part:
> >
> >         if (first device in group) {
> >                 replace group's domain from current hwpt to new hwpt;
> >                 if (first_device in hwpt)
> >                         Add hwpt domain to new iopt;
> >         }
> >
> >         Remove device from old hwpt;
> >         if (last_device in old hwpt)
> >                 Remove hwpt domain from old iopt;
> >
> >         Add device to new hwpt;
> >
> > I'm yet to figure out whether we have sufficient lock protection to
> > prevent other paths from using old iopt/hwpt to find the device
> > which is already attached to a different domain.
>=20
> With Jason's new series, the detach() routine is lighter now.
>=20
> I wonder if it'd be safer now to do the detach() call after
> iommu_group_replace_domain()?
>=20

yes, looks so.