Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Wed, 30 Mar 2022 16:02:01 -0300
From:   Jason Gunthorpe <jgg@nvidia.com>
To:     Lu Baolu <baolu.lu@linux.intel.com>
Cc:     Joerg Roedel <joro@8bytes.org>,
        Christoph Hellwig <hch@infradead.org>,
        Kevin Tian <kevin.tian@intel.com>,
        Ashok Raj <ashok.raj@intel.com>, Will Deacon <will@kernel.org>,
        Robin Murphy <robin.murphy@arm.com>,
        Jean-Philippe Brucker <jean-philippe@linaro.com>,
        Eric Auger <eric.auger@redhat.com>,
        Liu Yi L <yi.l.liu@intel.com>,
        Jacob jun Pan <jacob.jun.pan@intel.com>,
        iommu@lists.linux-foundation.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH RFC v2 03/11] iommu/sva: Add iommu_domain type for SVA
Message-ID: <20220330190201.GB2120790@nvidia.com>
References: <20220329053800.3049561-1-baolu.lu@linux.intel.com>
 <20220329053800.3049561-4-baolu.lu@linux.intel.com>
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20220329053800.3049561-4-baolu.lu@linux.intel.com>
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?us-ascii?Q?S2TxC5UCqkyTXPR7ZT1ltYXG8Iwp2n66Mp3M3TCebz9f0qPl5DY2opiV/0Lf?=
 =?us-ascii?Q?AlTaMBpjGpCekk36z7cstDWei5rThK5JcO8FZdGBNxf4FbCLXU5MJIW6DxIT?=
 =?us-ascii?Q?BD9AF8cYcgMbq9eXYFjHDNWcEnQRNY41yCHWOYZKvMSE3myI5s7Z2YHAfv8I?=
 =?us-ascii?Q?J1zWLlOHS2rOJYf7ZtHyoVJmURUHj39kGfVurymP3SGgB2MLfWLov+DugU2G?=
 =?us-ascii?Q?HBniK863BDaN2A+b/XaSsj3Ib6uix69T0NpIBASvPsjvBAfsnLcDRlRCLXCK?=
 =?us-ascii?Q?iqzJ2f7IXXn+h/ZiFNGzpFjJ9OBeifDyWIRzBD6NLwOD7cckl78SLy95NAAT?=
 =?us-ascii?Q?7nValyvvlI+SBMS7tBMBni0i5qj3iYOSN7uDVu1kTwaSXhT2hBu+MWdpDqcS?=
 =?us-ascii?Q?j5hrqOxbC3ygblTCe/2IluYuZNfLLyGkrn9rc19Xu1BHcAbvWsYMioZhyUIx?=
 =?us-ascii?Q?voyZwhCo0SfQ1IdqrrONjDbOb+z9KVMQQLYrgV/mLdEIb1J8zck1z7yP7FI8?=
 =?us-ascii?Q?SkhblZ9YZxc6K3Xjcvwxkxy+Nh+/Egwul2x0lLXgjGL9It1GDTY5zpv0eJKs?=
 =?us-ascii?Q?3GFZGgMPtRwlU9nBNYQTI0jvpfBzFqeKXLojRCfLCtfv8Cyor7MeY2QqIl9W?=
 =?us-ascii?Q?pg3tZ9nfaDwgPtsKjCB+spcZuAgZk7SHxVehL4rEV5jnVw3w6ZODU550/ErV?=
 =?us-ascii?Q?XamxGVvu2PquQTv3baWl84Xyzl+5JqBiyu0ICSVukEeZcuyubPp6YuWhoriC?=
 =?us-ascii?Q?42ALrgG0wj1IBDDCngHCO0VlQdO9c9zsmltCl4zdA2u6GLMka2/yJKJaJeIB?=
 =?us-ascii?Q?H5FXRykfRS+wKyQjwNOwx3B5MQhF7aZ7Tac3ZGtiUxw407SGjN6vqMD0/HRd?=
 =?us-ascii?Q?JoARD6ZU1WNlshBl8oOdggMJdmsxW5huDWzFJx5/hdO0RdDIz+w4fihESYbY?=
 =?us-ascii?Q?haM+wL8fC8wgJoFWtu71I4KmKnfNBJLaLXISlZwJOxjGmAUKlJR3sC6uWhi/?=
 =?us-ascii?Q?WKzKzHXme7U9NZQEvelcum6QxIh6yW1IN/uR/lbKk2Pl8rIOXlsy24i+0hHQ?=
 =?us-ascii?Q?Bx41wtarny3nbbCVEkZJe/DoQdD+bBFEBLKr1+crSJfQALHujBIRz4xsABwl?=
 =?us-ascii?Q?XUkt+EoFBsyt2xMxDDUwKa0YlF8ol6zZXMZnqjPrl6CDs4bqxEopihAc5H6E?=
 =?us-ascii?Q?eMhstF64HOGyNvT7nOco+oSmLEWKYFo4yQPiWdU8HLKcAjsrQ+W5/iMPkdcF?=
 =?us-ascii?Q?Vy3byDmNIJT2vj7J+95pi5n1MuT1GYBG0XvjIiyYmUYxvWA1zh7YoBfEdcFB?=
 =?us-ascii?Q?zwsFdZCvxHXpIKFnrpp5Hq0V0SJWKJnhVYBL42XhQeHF6EXqDWuuPlLOxEQm?=
 =?us-ascii?Q?gRrcbrGlwFWjcgniLJfGDLdkJiR6EBk86CfUqya+9xa00hlej7ykSX9wqMCR?=
 =?us-ascii?Q?Kg7OvdwxKVL99uwAja0Vd2JemRBXBulTJRxALvoC5W2OmU06pSdPg0b3Pk7Y?=
 =?us-ascii?Q?KI8UuW7ZsEyEv3ETTy1pt2khE4ORXmbywC9Z3p6xAX3aPCg1Sa0SKKf0NieZ?=
 =?us-ascii?Q?eOkoG7EXTn5kKLT/txAcCXJrF/31tKRpQSgMFbzOvY19CXQ05uMzRoYL8OZy?=
 =?us-ascii?Q?t2LL5E1wZ2gHtUO5RSi2cKmHU8AJJmLeLMhmzedCmAJgsPWkzm2nu7zO+Nga?=
 =?us-ascii?Q?L1gi8Z2lk+ay5YspjrOtWAsH7isxR8i7qNyQbv2IJx+U41O+PUGrrU9f494n?=
 =?us-ascii?Q?xKldwYWZgw=3D=3D?=
X-OriginatorOrg: Nvidia.com
X-MS-Exchange-CrossTenant-Network-Message-Id: a4f7a1f9-7f2f-4e22-c25c-08da127fc67f
X-MS-Exchange-CrossTenant-AuthSource: MN2PR12MB4192.namprd12.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 30 Mar 2022 19:02:02.8316
 (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: /KRBBUforC8wWAztlsOTSjukXQpFk9OQ6kc4RIkEk0XM2kwCLDzmyeiTH7eGoPXT
X-MS-Exchange-Transport-CrossTenantHeadersStamped: MN2PR12MB2943
X-Spam-Status: No, score=-1.1 required=5.0 tests=BAYES_00,DKIMWL_WL_HIGH,
        DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,FORGED_SPF_HELO,
        RCVD_IN_DNSWL_NONE,RCVD_IN_MSPIKE_H2,SPF_HELO_PASS,SPF_NONE,
        T_SCC_BODY_TEXT_LINE autolearn=no autolearn_force=no version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on
        lindbergh.monkeyblade.net
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

On Tue, Mar 29, 2022 at 01:37:52PM +0800, Lu Baolu wrote:
> @@ -95,6 +101,7 @@ struct iommu_domain {
>  	void *handler_token;
>  	struct iommu_domain_geometry geometry;
>  	struct iommu_dma_cookie *iova_cookie;
> +	struct iommu_sva_cookie *sva_cookie;

Cookie is still the wrong word to use here

> +struct iommu_sva_cookie {
> +	struct mm_struct *mm;
> +	ioasid_t pasid;
> +	refcount_t users;

Really surprised to see a refcount buried inside the iommu_domain..

This design seems inside out, the SVA struct should 'enclose' the domain, not
be a pointer inside it.

struct iommu_sva_domain {
       struct kref_t kref;
       struct mm_struct *mm;
       ioasid_t pasid;

       /* All the domains that are linked to this */
       struct xarray domain_list;
};

And then you could have a pointer to that inside the mm_struct instead
of just the naked pasid.

> +static __maybe_unused struct iommu_domain *

Why maybe unused?

> +iommu_sva_get_domain(struct device *dev, struct mm_struct *mm)
> +{
> +	struct iommu_domain *domain;
> +	ioasid_t pasid = mm->pasid;
> +
> +	if (pasid == INVALID_IOASID)
> +		return NULL;
> +
> +	domain = xa_load(&sva_domain_array, pasid);
> +	if (!domain)
> +		return iommu_sva_alloc_domain(dev, mm);
> +	iommu_sva_domain_get_user(domain);

This assumes any domain is interchangeable with any device, which is
not the iommu model. We need a domain op to check if a device is
compatiable with the domain for vfio an iommufd, this should do the
same.

It means each mm can have a list of domains associated with it and a
new domain is auto-created if the device doesn't work with any of the
existing domains.

Jason