Date:   Wed, 15 Feb 2023 14:46:43 -0400
From:   Jason Gunthorpe <jgg@nvidia.com>
To:     Jacob Pan <jacob.jun.pan@linux.intel.com>
Cc:     "Tian, Kevin" <kevin.tian@intel.com>,
        LKML <linux-kernel@vger.kernel.org>,
        "iommu@lists.linux.dev" <iommu@lists.linux.dev>,
        Lu Baolu <baolu.lu@linux.intel.com>,
        Joerg Roedel <joro@8bytes.org>,
        Jean-Philippe Brucker <jean-philippe@linaro.com>,
        Robin Murphy <robin.murphy@arm.com>,
        Will Deacon <will@kernel.org>,
        David Woodhouse <dwmw2@infradead.org>,
        "Raj, Ashok" <ashok.raj@intel.com>,
        "Liu, Yi L" <yi.l.liu@intel.com>
Subject: Re: [PATCH v2 2/3] iommu/sva: Stop using ioasid_set for SVA
Message-ID: <Y+0okzkbTh6mw0EM@nvidia.com>
References: <20230214064414.1038058-1-jacob.jun.pan@linux.intel.com>
 <20230214064414.1038058-3-jacob.jun.pan@linux.intel.com>
 <BN9PR11MB527670F77FADCB351E41F8B78CA39@BN9PR11MB5276.namprd11.prod.outlook.com>
 <20230215103822.6235ada9@jacob-builder>
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20230215103822.6235ada9@jacob-builder>
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?us-ascii?Q?q12mA/+eZuhICir0Y2NrIAFtIvn7XYnAfHNslEGs6PvzqhtNv9t6UMHV0YSs?=
 =?us-ascii?Q?ZEyV7Q8blhIBr8BwmlGPY2Q+H/GQG8JA7fOW9HgLIELR5HTYvbCT2i/CNoiM?=
 =?us-ascii?Q?/d1MtUd4yJnw0gVD5uR71iktmvHXktxBg4cKhcx458nhLlhJsyNUseAYqfqo?=
 =?us-ascii?Q?8IPk8Rx3od8bJUXeIdKVU0NR9mXLMXIaGNpA9gmhmihGejHhdGk3QJ/2MfZk?=
 =?us-ascii?Q?xUItk2SxUx6kCVPJA28q/Rkzgk3Tom3qY5nPf6iqKhomjIetgJB9xKz4mq2e?=
 =?us-ascii?Q?FebGrIyPHhaZ7w4joZdiwZd/+jEFJ2o0T9dyGUOwrB598cn6sh2U9J8eqxPB?=
 =?us-ascii?Q?ULdVb33jDvqQkLzfq0pMvDD2X614xHZBk3MvJY6oIbZ4QLzWX7OZAlWrLgfk?=
 =?us-ascii?Q?tz4/l3o8hKZxbJ4RpM4D4gNw/RowkbwvItCrnTq3AFutNkZHw+RFiYUZPy8o?=
 =?us-ascii?Q?1suX5I1JhwiQ8VVgc0xGILSKTJv3T8xqxC1tP/6jq3hVUjwWPtmVr7zbVqgN?=
 =?us-ascii?Q?UGl5TJED2aHQ3V663Xel2erprj2CzfcJxGctdN9yRVi0SbyvTuCgeS3trNNr?=
 =?us-ascii?Q?lwPiCWzNBMmE3Bhq29YP0lSO7MmIQdF+JzYSaVBbPE1JS9runl4CnHGwZjdk?=
 =?us-ascii?Q?aIO2ZvK3GBZFFrOoeZEyUHQt+5M4ssTJZ8NKmOaWXEz0ZgC0xOLqahAr+vDp?=
 =?us-ascii?Q?+qIu2RIRT7t+B7CivwykQxb6odpC9OU2/0QATnJRFQpgJkEOEdTakrxBnY0f?=
 =?us-ascii?Q?hTzkjncFhGNPTC0zfnDLRo/HQd9pKafkT2x2xyID33QUzqk/9dbyitCdBuQB?=
 =?us-ascii?Q?nqL1iWSHZgkrvGI3lXyAn1Z7joTKS2JDke4Raho+A6O3UjI8+Ifk4nHf3dUw?=
 =?us-ascii?Q?GP45UL1m1a1VQK+ZzxtdHQonAyebQf+MY/BhYWi19vgZex4vupZ4kULI4ceN?=
 =?us-ascii?Q?Q5yeDSZrJJuDa/q+SeJ5nWA7n3lWAunV2K3yjtJGnwH7ozSEpdl+1l875Hb5?=
 =?us-ascii?Q?x0ytZytn1lMHwxbE0F+kYl7H7t71LoNRgKqYvFfm9OOEthp7jFFOeSDtg27W?=
 =?us-ascii?Q?KM6Lzk25UjOPSxnpPcaUwhFHz3qwNVyhzw9OAZPGbXn1py0k3fFEcPZ555MS?=
 =?us-ascii?Q?xbDhxvSIxhdMwhOb3bf4VRUrwk4K5F8p4R1N6dd/NQr2H0shtnPEWu+ZgdfN?=
 =?us-ascii?Q?JWQESDHl+MD8oU4E8xjf1rDwKMSGySlTHM2nY+7Rshpb1SiuCu9hK5MHFWP8?=
 =?us-ascii?Q?+3/XjpVEG4dJr02FhAYo6JauhC/72EeU2y65KCK6JvpayYTGGv0Q6tHc0d0h?=
 =?us-ascii?Q?ATNIJ2yj/qaOx51OxTAXNoDJYowbfTEeYNFlgvtoP1Xaf18PllWkpXdw7o6M?=
 =?us-ascii?Q?wzAYTHP/HLNs7C/gXgRgcXlN9cr7B/6wALf5NADaY/TDwpVMDRKRW8xfVA4l?=
 =?us-ascii?Q?40d1AQmXvVk+zMPJLiCQQ7S6toUyT5GaaXdolOuYFUmHvq9NXdXJ3wWyegiB?=
 =?us-ascii?Q?16vCk54mhbBzsTuc5xynhJUIS1rfb5BDLYzbEu/ml23OyT8yRsYxvNMK3EZt?=
 =?us-ascii?Q?C88gDGhbyujxRbkpRZ8qpPaQhiJ6q9l151mOts/Q?=
X-OriginatorOrg: Nvidia.com
X-MS-Exchange-CrossTenant-Network-Message-Id: a9696bfe-20c2-4faa-d896-08db0f84fc46
X-MS-Exchange-CrossTenant-AuthSource: LV2PR12MB5869.namprd12.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 15 Feb 2023 18:46:44.6908
 (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: hQF0SXo4RSpm3edxpqINQ12zAYa9jgcpeksfo9V9TcOqBKnlk+fufVF+RHaaaFKV
X-MS-Exchange-Transport-CrossTenantHeadersStamped: PH0PR12MB8032
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

On Wed, Feb 15, 2023 at 10:38:22AM -0800, Jacob Pan wrote:
> Hi Kevin,
> 
> On Wed, 15 Feb 2023 03:24:18 +0000, "Tian, Kevin" <kevin.tian@intel.com>
> wrote:
> 
> > > From: Jacob Pan <jacob.jun.pan@linux.intel.com>
> > > Sent: Tuesday, February 14, 2023 2:44 PM
> > > 
> > > From: Jason Gunthorpe <jgg@nvidia.com>
> > > 
> > > Instead SVA drivers can use a simple global IDA to allocate PASIDs for
> > > each mm_struct.
> > > 
> > > Future work would be to allow drivers using the SVA APIs to reserve
> > > global PASIDs from this IDA for their internal use, eg with the DMA API
> > > PASID support.  
> > 
> > I think DMA API PASID will need new API around this IDA. SVA APIs should
> > be just for SVA.
> > 
> > > +void mm_pasid_drop(struct mm_struct *mm)
> > >  {
> > > -	return mmget_not_zero(mm);
> > > -}
> > > +	pr_alert("%s %d", __func__, mm->pasid);
> > > +	if (likely(!pasid_valid(mm->pasid)))
> > > +		return;
> > > 
> > > -/**
> > > - * iommu_sva_find() - Find mm associated to the given PASID
> > > - * @pasid: Process Address Space ID assigned to the mm
> > > - *
> > > - * On success a reference to the mm is taken, and must be released with
> > > mmput().
> > > - *
> > > - * Returns the mm corresponding to this PASID, or an error if not
> > > found.
> > > - */
> > > -struct mm_struct *iommu_sva_find(ioasid_t pasid)
> > > -{
> > > -	return ioasid_find(&iommu_sva_pasid, pasid, __mmget_not_zero);
> > > +	ida_free(&iommu_global_pasid_ida, mm->pasid);
> > >  }  
> > 
> > when moving this function following line is missed:
> > 
> > 	mm->pasid = INVALID_IOASID;
> > 
> not needed, mm is gone by now.
> 
> > btw the current placement looks asymmetric. We now have
> > mm_pasid_init() and mm_pasid_set() in mm.h while mm_pasid_drop()
> > in iommu-sva.c.
> > Is it cleaner to introduce a iommu_sva_free_pasid() helper and then
> > call it from mm_pasid_drop() in mm.h?
> That is what we had before, it seems logical to me. Jason?

I don't much like a maze of functions calling each other.

Renaming the helper might help, especially if it comes from iommu.h

Jason