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:     Jason Gunthorpe <jgg@nvidia.com>
CC:     "Liu, Yi L" <yi.l.liu@intel.com>,
        "alex.williamson@redhat.com" <alex.williamson@redhat.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>,
        "david@gibson.dropbear.id.au" <david@gibson.dropbear.id.au>,
        "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+UqMJ7tQK2EdmauuvK2AgACKnbCAALw6gIAHYVYAgABthQCAABACQA==
Date:   Mon, 27 Sep 2021 13:00:08 +0000
Message-ID: <BN9PR11MB5433502FEF11940984774F278CA79@BN9PR11MB5433.namprd11.prod.outlook.com>
References: <20210919063848.1476776-1-yi.l.liu@intel.com>
 <20210919063848.1476776-7-yi.l.liu@intel.com>
 <20210921170943.GS327412@nvidia.com>
 <BN9PR11MB5433DA330D4583387B59AA7F8CA29@BN9PR11MB5433.namprd11.prod.outlook.com>
 <20210922123931.GI327412@nvidia.com>
 <BN9PR11MB5433CE19425E85E7F52093278CA79@BN9PR11MB5433.namprd11.prod.outlook.com>
 <20210927115342.GW964074@nvidia.com>
In-Reply-To: <20210927115342.GW964074@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?B7/1qKbc8b3nVfv66jVmVMluGjrN0dmMHJ2d3kUY1uA4BPxFG+ADxajdl6Ux?=
 =?us-ascii?Q?bzuSm+wTG8jFFGe2PM7bIala1lsQnFj1Z1FldXqmAeqZ1NhHhIieeBvMA5oz?=
 =?us-ascii?Q?eHfXFIZaYk8vj3OrU+x8YmUOmS7eXkun6jI9jQ3RyoDmzNq08HqDXB7m2iKV?=
 =?us-ascii?Q?TtjSzrXlC1zUlOPcI7Uk3UwriJqtvSUW+gOmPazENfcqPtte7y01GywmA6Jg?=
 =?us-ascii?Q?KKIZz01GejkClrLHN9RoIhy6B5DwInc3kqlFt4DnbQn6xf/ewFiA4O3NgRan?=
 =?us-ascii?Q?T32cMKREJwj3hxyjvDE1355ZI9cCN1gTVD963TeNpVOaTe8FsElzXtAkDBt/?=
 =?us-ascii?Q?cY+Qn9FgPTAJQ+9bd/RfA11gg6kb6scoOSUSL1M7RWM6z+IBjX4ilq2FXOod?=
 =?us-ascii?Q?cthA1tyi5EjCXixlWBNtn2U8rFZ2UznFp1YMAe2zPp6Oi9N+5LrNeoN2RO0e?=
 =?us-ascii?Q?evHdNHlSHsZ5uO5iGoMPWBoDAQ0AKx3u6wVMtu0HjjfC/4XUXtQoJWb/676V?=
 =?us-ascii?Q?KuxN4dHMVhtf8u3/JUoTb2rdJV6kDC+/no6fXjYEAgfBQNjlk0/MJmkkheZt?=
 =?us-ascii?Q?ZVDZ0fXpOjyngH1aBrK1tTYYqxW420OKG2srfk4gMmxGftaOiLUfNW2ofvnr?=
 =?us-ascii?Q?zjFyFLszYr49y1yGpwCcolujgI8tjiiOLVteYo4aG7XQT8b5lWlxyMC3Rc2M?=
 =?us-ascii?Q?FxewQAcdM/x/q3+yqi2sEy78EhRhdku88nQM7k6rcXtAQ9RoH2XNhbsfjfJD?=
 =?us-ascii?Q?0aSGzhSm6VHC+9QJHKw/mISJSyykxA2nstTlRoAT8Mc7fwQzOuj4CkNAsj4f?=
 =?us-ascii?Q?KLayVkjIj5nHZ3oiqvcuV3xOO3JuDNkrOBk1EWSdDFjV/byLZQ+yKprj+Wr1?=
 =?us-ascii?Q?3Ik0pDt0JRO88ole1sqLkeMV2tVEwU71tnHjCKY8TbUX5uxDoNaqm7jezXAL?=
 =?us-ascii?Q?6xAVdEdrijitEYvLDT3cZOMR7aZtnnJK2RMYTxrWiH6Hfs81aA0zYevEdiId?=
 =?us-ascii?Q?nVtqgNEsbNne6XHMPTkdfERqS5LKfgvMEmK0AOJFeZZHozmlUQueBEBFQ3Wi?=
 =?us-ascii?Q?HjzgjWWEqcUmR1JDoLgFGXL5yijwasEafGo8qHayoA7e3YSPATxzz5aloC8l?=
 =?us-ascii?Q?gE93v6IlQYqF+pcgOiciwMZ5GCSV6z2D2Jukx/01J19Bt8MO5dXNvUQ13syw?=
 =?us-ascii?Q?IYvMCpzT+kYrajjFgVe8LnX3aWQDVbFRK2LYA//1kGuYoNU5gecZLj8aRRWR?=
 =?us-ascii?Q?W/G1zIYf6RplqPI6/I2yqYGNnonr1BOd4rkNSDjxuV9K+oHQjJKBLxH+jTW8?=
 =?us-ascii?Q?X+b+PcWAmBXD4YI12PAI0Pft?=
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: c7534eb1-78c4-4783-0331-08d981b6bb98
X-MS-Exchange-CrossTenant-originalarrivaltime: 27 Sep 2021 13:00:08.0311
 (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: taExu+HyJHe0KQatxWih4F84vlv4gAv4W1ZW/BDs7ulO8sMI6PkH/vRD6rTj5ShOqVnf4C2irUJu9mQWx3bJuw==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: BN6PR1101MB2084
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: Monday, September 27, 2021 7:54 PM
>=20
> On Mon, Sep 27, 2021 at 09:42:58AM +0000, Tian, Kevin wrote:
> > > From: Jason Gunthorpe <jgg@nvidia.com>
> > > Sent: Wednesday, September 22, 2021 8:40 PM
> > >
> > > > > Ie the basic flow would see the driver core doing some:
> > > >
> > > > Just double confirm. Is there concern on having the driver core to
> > > > call iommu functions?
> > >
> > > It is always an interesting question, but I'd say iommu is
> > > foundantional to Linux and if it needs driver core help it shouldn't
> > > be any different from PM, pinctl, or other subsystems that have
> > > inserted themselves into the driver core.
> > >
> > > Something kind of like the below.
> > >
> > > If I recall, once it is done like this then the entire iommu notifier
> > > infrastructure can be ripped out which is a lot of code.
> >
> > Currently vfio is the only user of this notifier mechanism. Now
> > three events are handled in vfio_iommu_group_notifier():
> >
> > NOTIFY_ADD_DEVICE: this is basically for some sanity check. suppose
> > not required once we handle it cleanly in the iommu/driver core.
> >
> > NOTIFY_BOUND_DRIVER: the BUG_ON() logic to be fixed by this change.
> >
> > NOTIFY_UNBOUND_DRIVER: still needs some thoughts. Based on
> > the comments the group->unbound_list is used to avoid breaking
>=20
> I have a patch series to delete the unbound_list, the scenario you
> describe is handled by the device_lock()

that's great!

>=20
> > diff --git a/drivers/base/dd.c b/drivers/base/dd.c
> > index 68ea1f9..826a651 100644
> > +++ b/drivers/base/dd.c
> > @@ -566,6 +566,10 @@ static int really_probe(struct device *dev, struct
> device_driver *drv)
> >  		goto done;
> >  	}
> >
> > +	ret =3D iommu_device_set_dma_hint(dev, drv->dma_hint);
> > +	if (ret)
> > +		return ret;
>=20
> I think for such a narrow usage you should not change the struct
> device_driver. Just have pci_stub call a function to flip back to user
> mode.

Here we want to ensure that kernel dma should be blocked
if the group is already marked for user-dma. If we just blindly
do it for any driver at this point (as you commented earlier):

+       ret =3D iommu_set_kernel_ownership(dev);
+       if (ret)
+               return ret;

how would pci-stub reach its function to indicate that it doesn't=20
do dma and flip back?

Do you envision a simpler policy that no driver can be bound
to the group if it's already set for user-dma? what about vfio-pci
itself?

>=20
> > +static int iommu_dev_viable(struct device *dev, void *data)
> > +{
> > +	enum dma_hint hint =3D *data;
> > +	struct device_driver *drv =3D READ_ONCE(dev->driver);
>=20
> Especially since this isn't locked properly or safe.

I have the same worry when copying from vfio. Not sure how
vfio gets safe with this approach...

Thanks
Kevin