Date:   Wed, 17 Nov 2021 09:35:17 -0400
From:   Jason Gunthorpe <jgg@nvidia.com>
To:     Lu Baolu <baolu.lu@linux.intel.com>
Cc:     Christoph Hellwig <hch@infradead.org>,
        Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
        Joerg Roedel <joro@8bytes.org>,
        Alex Williamson <alex.williamson@redhat.com>,
        Bjorn Helgaas <bhelgaas@google.com>,
        Kevin Tian <kevin.tian@intel.com>,
        Ashok Raj <ashok.raj@intel.com>,
        Chaitanya Kulkarni <kch@nvidia.com>, kvm@vger.kernel.org,
        rafael@kernel.org, linux-pci@vger.kernel.org,
        Cornelia Huck <cohuck@redhat.com>,
        linux-kernel@vger.kernel.org, iommu@lists.linux-foundation.org,
        Jacob jun Pan <jacob.jun.pan@intel.com>,
        Diana Craciun <diana.craciun@oss.nxp.com>,
        Will Deacon <will@kernel.org>
Subject: Re: [PATCH 01/11] iommu: Add device dma ownership set/release
 interfaces
Message-ID: <20211117133517.GJ2105516@nvidia.com>
References: <20211115020552.2378167-1-baolu.lu@linux.intel.com>
 <20211115020552.2378167-2-baolu.lu@linux.intel.com>
 <YZJdJH4AS+vm0j06@infradead.org>
 <cc7ce6f4-b1ec-49ef-e245-ab6c330154c2@linux.intel.com>
 <20211116134603.GA2105516@nvidia.com>
 <d79acc01-eeaf-e6ac-0415-af498c355a00@linux.intel.com>
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <d79acc01-eeaf-e6ac-0415-af498c355a00@linux.intel.com>
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?us-ascii?Q?TqhqSp7JrzWeadA8/37WIBnJ2gyG0aFICHIbx672RsUcuNNj/0qhbRTjaO8Y?=
 =?us-ascii?Q?zlk/b4n3l1+3ZF9FeOLmnMzhKjSJ6Z7t0UYBLH492wP8PS0DBXcS4GcmBkrq?=
 =?us-ascii?Q?AQAKveySsF/PAx4KT+S5kW7sOtz1xHLvyTMkZb7bgqCK3anMBdw3rgww9WUM?=
 =?us-ascii?Q?j2QJ/uVOUWe+1seHGvnnwFkpAkxV4gkUIv8bMd6N9kwFK1/zdDPkGOBBQzVX?=
 =?us-ascii?Q?qXknW6ieSB7aUOa+/nPtmEhKRow5FZHjxnvO4Q8ccT3uUv/38PwryEfw21z4?=
 =?us-ascii?Q?o/IJLxOTzyoCPBFCeA7UFdQ4RMoyygIB98v1OHW7HqvmAjDQZbvSPGC34coZ?=
 =?us-ascii?Q?xQrH5R7Rdf92+1eCT8EfzTq6as5gRTYTJIxPbMzS+6IX0uoPPcY+x+z8DUsr?=
 =?us-ascii?Q?MlwMErAFts2819nGWeI38Uz5tqbI+O4FO0QJ86zBOfzbm6kuH2tuzDMVLYxn?=
 =?us-ascii?Q?Qw3ajfvkdz1439DkC0oCRWT/uVXECN9+KMeF4wBlMIj6ZxBO5L1sfhrQhbib?=
 =?us-ascii?Q?Cjeq6mD7faD3y+hikZWdtOcZ4UYcmpSlZBZxYrdyH5eFJPLAXVNG8ygydqNB?=
 =?us-ascii?Q?Bw+wPjMxBAkhqxTa/yUkpeuZuE6X94mfJDzcxZWCuZYwhDtJQCMV/E2fQ90U?=
 =?us-ascii?Q?Ed2oDnzhYtDE8k70ev2fPXuLCy73L7FUc0aKcWSqqFZWDRt4z50yJIhVCrF/?=
 =?us-ascii?Q?OxBLKECKlPObQaiYkPAftjVnRd0KgK7E2ppPq5DC5+T5taWQYDfAdtQ8D3Jt?=
 =?us-ascii?Q?IbG8BdXzVOLl7NkG3N8+ft4oUrI5Eo8thGkoOp6incTQezHqOcFaqRwyZq+M?=
 =?us-ascii?Q?a80mgtU7o4ppUxAZ/WVdLVybsO7GPh7/EtRNh+LdYUhxbBd60hqRC9HrCwwd?=
 =?us-ascii?Q?jfWZirur3x2wNrL1trcrywBX12V23rqUEtGLwbgrTAfgj2xfrxo9kN9IXgiX?=
 =?us-ascii?Q?fiiG7wbciXZu011Y+kSPTujSdkrx9J8OQ39dj3bOnQkMw7faeFmDKWarV2tM?=
 =?us-ascii?Q?nSfsHdD7B6cPi67n3cnwfEpWKZj8bI69iSaSJYGS4AZdFkJn0XoO1LUYtHCy?=
 =?us-ascii?Q?66LVC0+SxwBTjUnMAfikkYar55Ya86c3+7ZSUzBR5fP4Undiy49HpwtGIdaE?=
 =?us-ascii?Q?AUjJmo05l/T8r7sG/VyuCyAlTM6wI36KcZ0xDKXNwLk/CRz7xRTXQeuLtZb5?=
 =?us-ascii?Q?xGezt+weiabmdD4Jh55m9+QAYbxWg9kK4tc2ngvTJ3H12LmaMi6NTsSL37eV?=
 =?us-ascii?Q?TZ4rSRByAIMdVNhHYxS5PgzWuo0GhJzee3TDu5ceGufgVAvxp1/9GF3obT32?=
 =?us-ascii?Q?fzcupJ8ngH4Lu1deFljDwU2/E73bPbq8D69zDFlvEVuXZo4docXeJbx/rbRb?=
 =?us-ascii?Q?zY/suhzeNzLKdmMRFSozCJIPRterdEMKHO4YL40onc+zW94nvDuFUbG01sk9?=
 =?us-ascii?Q?XNUBpp/nbJqinmUaxkYXNXrbeEO0snMasW98RGcsmEmGV7uv2qrdlTLvrzu4?=
 =?us-ascii?Q?8fG3LNUCdGIrZFlXoXyhdZmgxWo4MagYIqYwLWDYv6/4ZO2VxpcnpbwiwxwE?=
 =?us-ascii?Q?WKN7oqBxv5yc95fsK2Y=3D?=
X-OriginatorOrg: Nvidia.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 1d486066-5db4-49ea-5459-08d9a9cf187d
X-MS-Exchange-CrossTenant-AuthSource: BL0PR12MB5506.namprd12.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 17 Nov 2021 13:35:18.5240
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 43083d15-7273-40c1-b7db-39efd9ccc17a
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: 4RbPVBxuHDWpwRKhJ1+M2rLsJRrBvez6dQh/BSwR6AFVHUEPb+2S8sd/3YeP8DpA
X-MS-Exchange-Transport-CrossTenantHeadersStamped: BL1PR12MB5144
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

On Wed, Nov 17, 2021 at 01:22:19PM +0800, Lu Baolu wrote:
> Hi Jason,
> 
> On 11/16/21 9:46 PM, Jason Gunthorpe wrote:
> > On Tue, Nov 16, 2021 at 09:57:30AM +0800, Lu Baolu wrote:
> > > Hi Christoph,
> > > 
> > > On 11/15/21 9:14 PM, Christoph Hellwig wrote:
> > > > On Mon, Nov 15, 2021 at 10:05:42AM +0800, Lu Baolu wrote:
> > > > > +enum iommu_dma_owner {
> > > > > +	DMA_OWNER_NONE,
> > > > > +	DMA_OWNER_KERNEL,
> > > > > +	DMA_OWNER_USER,
> > > > > +};
> > > > > +
> > > > 
> > > > > +	enum iommu_dma_owner dma_owner;
> > > > > +	refcount_t owner_cnt;
> > > > > +	struct file *owner_user_file;
> > > > 
> > > > I'd just overload the ownership into owner_user_file,
> > > > 
> > > >    NULL			-> no owner
> > > >    (struct file *)1UL)	-> kernel
> > > >    real pointer		-> user
> > > > 
> > > > Which could simplify a lot of the code dealing with the owner.
> > > > 
> > > 
> > > Yeah! Sounds reasonable. I will make this in the next version.
> > 
> > It would be good to figure out how to make iommu_attach_device()
> > enforce no other driver binding as a kernel user without a file *, as
> > Robin pointed to, before optimizing this.
> > 
> > This fixes an existing bug where iommu_attach_device() only checks the
> > group size and is vunerable to a hot plug increasing the group size
> > after it returns. That check should be replaced by this series's logic
> > instead.
> 
> As my my understanding, the essence of this problem is that only the
> user owner of the iommu_group could attach an UNMANAGED domain to it.
> If I understand it right, how about introducing a new interface to
> allocate a user managed domain and storing the user file pointer in it.

For iommu_attach_device() the semantic is simple non-sharing, so there
is no need for the file * at all, it can just be NULL.

> Does above help here?

No, iommu_attach_device() is kernel only and should not interact with
userspace.

I'm also going to see if I can learn what Tegra is doing with
iommu_attach_group()

Jason