Received-SPF: Pass (protection.outlook.com: domain of nvidia.com designates
 216.228.118.233 as permitted sender) receiver=protection.outlook.com;
 client-ip=216.228.118.233; helo=mail.nvidia.com; pr=C
Date:   Thu, 9 Feb 2023 12:28:45 -0800
From:   Nicolin Chen <nicolinc@nvidia.com>
To:     "jgg@nvidia.com" <jgg@nvidia.com>,
        "Tian, Kevin" <kevin.tian@intel.com>
CC:     "joro@8bytes.org" <joro@8bytes.org>,
        "will@kernel.org" <will@kernel.org>,
        "robin.murphy@arm.com" <robin.murphy@arm.com>,
        "alex.williamson@redhat.com" <alex.williamson@redhat.com>,
        "shuah@kernel.org" <shuah@kernel.org>,
        "Liu, Yi L" <yi.l.liu@intel.com>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
        "iommu@lists.linux.dev" <iommu@lists.linux.dev>,
        "kvm@vger.kernel.org" <kvm@vger.kernel.org>,
        "linux-kselftest@vger.kernel.org" <linux-kselftest@vger.kernel.org>,
        "baolu.lu@linux.intel.com" <baolu.lu@linux.intel.com>
Subject: Re: [PATCH v2 05/10] iommufd: Add replace support in
 iommufd_access_set_ioas()
Message-ID: <Y+VXfQbXakNSHSLw@Asurada-Nvidia>
References: <cover.1675802050.git.nicolinc@nvidia.com>
 <931be169ff4a1f4d4f1ed060d722c2dc17ce6667.1675802050.git.nicolinc@nvidia.com>
 <BN9PR11MB5276AE27E37866B82A6E03608CD99@BN9PR11MB5276.namprd11.prod.outlook.com>
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Disposition: inline
In-Reply-To: <BN9PR11MB5276AE27E37866B82A6E03608CD99@BN9PR11MB5276.namprd11.prod.outlook.com>
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 09 Feb 2023 20:29:01.6215
 (UTC)
X-MS-Exchange-CrossTenant-Network-Message-Id: 3f56a5a0-ae73-4c90-0926-08db0adc47d3
X-MS-Exchange-CrossTenant-Id: 43083d15-7273-40c1-b7db-39efd9ccc17a
X-MS-Exchange-CrossTenant-OriginalAttributedTenantConnectingIp: TenantId=43083d15-7273-40c1-b7db-39efd9ccc17a;Ip=[216.228.118.233];Helo=[mail.nvidia.com]
X-MS-Exchange-CrossTenant-AuthSource: CO1PEPF00001A64.namprd05.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Anonymous
X-MS-Exchange-CrossTenant-FromEntityHeader: HybridOnPrem
X-MS-Exchange-Transport-CrossTenantHeadersStamped: CY5PR12MB6131
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

On Thu, Feb 09, 2023 at 03:13:08AM +0000, Tian, Kevin wrote:
 
> > --- a/drivers/iommu/iommufd/device.c
> > +++ b/drivers/iommu/iommufd/device.c
> > @@ -509,11 +509,23 @@ int iommufd_access_set_ioas(struct
> > iommufd_access *access, u32 ioas_id)
> >               iommufd_ref_to_users(obj);
> >       }
> >
> > +     /*
> > +      * Set ioas to NULL to block any further iommufd_access_pin_pages().
> > +      * iommufd_access_unpin_pages() can continue using access-
> > >ioas_unpin.
> > +      */
> > +     access->ioas = NULL;
> > +
> >       if (cur_ioas) {
> > +             if (new_ioas) {
> > +                     mutex_unlock(&access->ioas_lock);
> > +                     access->ops->unmap(access->data, 0, ULONG_MAX);
> > +                     mutex_lock(&access->ioas_lock);
> > +             }
> 
> why does above only apply to a valid new_ioas? this is the cleanup on
> cur_ioas then required even when new_ioas=NULL.
  
Though it'd make sense to put it in the common path, our current
detach routine doesn't call this unmap. If we do so, it'd become
something new to the normal detach routine. Or does this mean the
detach routine has been missing an unmap call so far?

Thanks
Nic