Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
From:   "Tian, Kevin" <kevin.tian@intel.com>
To:     David Gibson <david@gibson.dropbear.id.au>,
        "Liu, Yi L" <yi.l.liu@intel.com>
CC:     "alex.williamson@redhat.com" <alex.williamson@redhat.com>,
        "jgg@nvidia.com" <jgg@nvidia.com>, "hch@lst.de" <hch@lst.de>,
        "jasowang@redhat.com" <jasowang@redhat.com>,
        "joro@8bytes.org" <joro@8bytes.org>,
        "jean-philippe@linaro.org" <jean-philippe@linaro.org>,
        "parav@mellanox.com" <parav@mellanox.com>,
        "lkml@metux.net" <lkml@metux.net>,
        "pbonzini@redhat.com" <pbonzini@redhat.com>,
        "lushenming@huawei.com" <lushenming@huawei.com>,
        "eric.auger@redhat.com" <eric.auger@redhat.com>,
        "corbet@lwn.net" <corbet@lwn.net>,
        "Raj, Ashok" <ashok.raj@intel.com>,
        "yi.l.liu@linux.intel.com" <yi.l.liu@linux.intel.com>,
        "Tian, Jun J" <jun.j.tian@intel.com>, "Wu, Hao" <hao.wu@intel.com>,
        "Jiang, Dave" <dave.jiang@intel.com>,
        "jacob.jun.pan@linux.intel.com" <jacob.jun.pan@linux.intel.com>,
        "kwankhede@nvidia.com" <kwankhede@nvidia.com>,
        "robin.murphy@arm.com" <robin.murphy@arm.com>,
        "kvm@vger.kernel.org" <kvm@vger.kernel.org>,
        "iommu@lists.linux-foundation.org" <iommu@lists.linux-foundation.org>,
        "dwmw2@infradead.org" <dwmw2@infradead.org>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
        "baolu.lu@linux.intel.com" <baolu.lu@linux.intel.com>,
        "nicolinc@nvidia.com" <nicolinc@nvidia.com>
Subject: RE: [RFC 06/20] iommu: Add iommu_device_init[exit]_user_dma
 interfaces
Thread-Topic: [RFC 06/20] iommu: Add iommu_device_init[exit]_user_dma
 interfaces
Thread-Index: AQHXrSF9WDj+Z+DU+UqMJ7tQK2Edmau6gj6AgAAJv4A=
Date:   Wed, 29 Sep 2021 05:38:56 +0000
Message-ID: <BN9PR11MB5433E1BF538C7D3632F4C6188CA99@BN9PR11MB5433.namprd11.prod.outlook.com>
References: <20210919063848.1476776-1-yi.l.liu@intel.com>
 <20210919063848.1476776-7-yi.l.liu@intel.com> <YVPxzad5TYHAc1H/@yekko>
In-Reply-To: <YVPxzad5TYHAc1H/@yekko>
Accept-Language: en-US
Content-Language: en-US
x-ms-exchange-antispam-messagedata-chunkcount: 1
x-ms-exchange-antispam-messagedata-0: =?us-ascii?Q?UIX+QJbyS/k+PN7PVEO1qvsvrhhw7HcWdY+5Ql4F1038d637xfFi3ifWNhIs?=
 =?us-ascii?Q?H55UXXhK2fpa6CwSGxPvurThlgkT0EIF79y+Kxc3OuGxUaEAMuJp/sFAS8kY?=
 =?us-ascii?Q?U1NYDAH/4vZFuWHxH2K/cPx3s2L0xRrlcsLjpwqsMT4t0LMqSYs7y1Dczus+?=
 =?us-ascii?Q?u9MFe56UJJ+WixKsiyTMECQQsy19tYkteD0Z16IjupuQjbHqAAJjKGg3VRg9?=
 =?us-ascii?Q?93zz9LNxjvXEKHBGEEN+U+4z4cgS4o9dUIAzSnW3ObiOakQ0pnfUjvRHWmBK?=
 =?us-ascii?Q?F/ECn6VhKXAtteaIrWyq5qAZLuTUp3gzOceBcMqjdHNIQiETIfJPzg+gz+I+?=
 =?us-ascii?Q?5sayVDngHJ6MW48KYtVkF3vJn90jWeDGBAcirHnIqkC3BXwfb956DBZhs5o6?=
 =?us-ascii?Q?e5+snMch/riMJHTrKT8EhwNYA6siZHpq03STHrM4JkwWzwF+UEg/gCyXX+dX?=
 =?us-ascii?Q?pVz8BHkDRjvKucItBQzhOBxDZ9C0O5AlVNaR1JXeY4fv0hcmML1fcLsC4Rg/?=
 =?us-ascii?Q?v06JHBK3w2GGXM2cZv3AIfjJyEsnuiKxoQEAebHeVQcgk0HDfOSxUPachiba?=
 =?us-ascii?Q?1x5RpP4oSWyijmjsfVdSHIzeXHJOAyETNVQhZPAi86Y3OvRk2Yz7r6b/+37i?=
 =?us-ascii?Q?5EARU9OEUWaKGXr0kTMHeQ6TTDarJH5rB3tLvNuTXr5hWZsBDjt7cyvQNp2Y?=
 =?us-ascii?Q?UgT8gutgSyNcfsbQifCuo0chEHOagz4l0ilbEL6n0NmLL3JbdDxTiSoYSJpp?=
 =?us-ascii?Q?G18CQSf+rCvJAsiTAwu4blm5/mta/DdlzDTm/a+zI85bS6kRVJIszrUoPvb5?=
 =?us-ascii?Q?TNKkceUJhYsMDLJK86YGY3ignau3mcxHxYCya00dpsyvjDdhCKFNkL9IkT4Q?=
 =?us-ascii?Q?3dEL8LWX18RfSvMkyVUvCPW/HyCr7d69irW1+FG/lxeTczn5qfv5BmvnbC6o?=
 =?us-ascii?Q?hBJ5Bz3tMcmTo+YTqwEKDVJNatQZRN8OTwtYHQQ0HCHa8SEyjX8rbodD8qLj?=
 =?us-ascii?Q?8BfIJgQcqlcvsofyrUkUd0elOIp2zmVqD7PyN6lRPeH+nFw+H5sfvgDk1OHC?=
 =?us-ascii?Q?EzP6f3YVwDGa/kVgZAZs73+5F1YZlT3SSg/JH/y8KsEJk/72a8dzsk/zslVv?=
 =?us-ascii?Q?vG67y9m6pKpxEJCVMOiL447M3FD//4gV6P9+yVerbJs5giRC9p5kNVNeRVkS?=
 =?us-ascii?Q?0bJet/NqGhpaznh2rosnMjgcFUUbyHoultHfgRzf6LLzTz5+HP7vJfZuSYR8?=
 =?us-ascii?Q?6WHR1506qbQt/wY6oBKTiDaJAk/NC8Cuj7Qwea9YGjHOqAU7lxI0DifsD2q9?=
 =?us-ascii?Q?RZVrxthPXIvqWX+/Nsm4vo03?=
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: BN9PR11MB5433.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 699210bd-34a4-4367-66e0-08d9830b6e00
X-MS-Exchange-CrossTenant-originalarrivaltime: 29 Sep 2021 05:38:56.0326
 (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: E5qJggzBb8vwaqRAhRMZN9gAGtQaJhbQPWvWfV2nKkVbQeHFj69wSzYjPuuXXVBUYeSqeHa1W1Jpnp8U4NIZ4g==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: BN6PR11MB3891
X-OriginatorOrg: intel.com
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

> From: David Gibson <david@gibson.dropbear.id.au>
> Sent: Wednesday, September 29, 2021 12:56 PM
>=20
> >
> > Unlike vfio, iommufd adopts a device-centric design with all group
> > logistics hidden behind the fd. Binding a device to iommufd serves
> > as the contract to get security context established (and vice versa
> > for unbinding). One additional requirement in iommufd is to manage the
> > switch between multiple security contexts due to decoupled bind/attach:
> >
> > 1)  Open a device in "/dev/vfio/devices" with user access blocked;
>=20
> Probably worth clarifying that (1) must happen for *all* devices in
> the group before (2) happens for any device in the group.

No. User access is naturally blocked for other devices as long as they
are not opened yet.

>=20
> > 2)  Bind the device to an iommufd with an initial security context
> >     (an empty iommu domain which blocks dma) established for its
> >     group, with user access unblocked;
> >
> > 3)  Attach the device to a user-specified ioasid (shared by all devices
> >     attached to this ioasid). Before attaching, the device should be fi=
rst
> >     detached from the initial context;
>=20
> So, this step can implicitly but observably change the behaviour for
> other devices in the group as well.  I don't love that kind of
> difficult to predict side effect, which is why I'm *still* not totally
> convinced by the device-centric model.

which side-effect is predicted here? The user anyway needs to be
aware of such group restriction regardless whether it uses group
or nongroup interface.

> >
> > diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
> > index 5ea3a007fd7c..bffd84e978fb 100644
> > --- a/drivers/iommu/iommu.c
> > +++ b/drivers/iommu/iommu.c
> > @@ -45,6 +45,8 @@ struct iommu_group {
> >  	struct iommu_domain *default_domain;
> >  	struct iommu_domain *domain;
> >  	struct list_head entry;
> > +	unsigned long user_dma_owner_id;
>=20
> Using an opaque integer doesn't seem like a good idea.  I think you
> probably want a pointer to a suitable struct dma_owner or the like
> (you could have one embedded in each iommufd struct, plus a global
> static kernel_default_owner).
>=20

For remaining comments you may want to look at the latest discussion
here:

https://lore.kernel.org/kvm/20210928140712.GL964074@nvidia.com/

It relies on driver core change to manage group ownership gracefully.
No BUG_ON() is triggered any more for driver binding. There a fd will
be passed in to mark the ownership.

Thanks
Kevin