linux-kernel - Re: [PATCH 06/17] iommufd/hw_pagetable: Use domain_alloc

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <Y+VaCV7DRR1+mLYY@nvidia.com>
Date:   Thu, 9 Feb 2023 16:39:37 -0400
From:   Jason Gunthorpe <jgg@...dia.com>
To:     Nicolin Chen <nicolinc@...dia.com>
Cc:     Yi Liu <yi.l.liu@...el.com>,
        Matthew Rosato <mjrosato@...ux.ibm.com>, joro@...tes.org,
        alex.williamson@...hat.com, kevin.tian@...el.com,
        robin.murphy@....com, cohuck@...hat.com, eric.auger@...hat.com,
        kvm@...r.kernel.org, chao.p.peng@...ux.intel.com,
        yi.y.sun@...ux.intel.com, peterx@...hat.com, jasowang@...hat.com,
        shameerali.kolothum.thodi@...wei.com, lulu@...hat.com,
        suravee.suthikulpanit@....com, iommu@...ts.linux.dev,
        linux-kernel@...r.kernel.org, linux-kselftest@...r.kernel.org,
        baolu.lu@...ux.intel.com
Subject: Re: [PATCH 06/17] iommufd/hw_pagetable: Use domain_alloc_user op for
 domain allocation

On Thu, Feb 09, 2023 at 11:51:31AM -0800, Nicolin Chen wrote:
> On Thu, Feb 09, 2023 at 02:36:49PM -0400, Jason Gunthorpe wrote:
> > On Thu, Feb 09, 2023 at 12:59:58PM -0500, Matthew Rosato wrote:
> > > really should highlight that).  Otherwise, conditionally calling
> > > iommu_domain_alloc(dev->bus) when !ops->domain_alloc_user (instead
> > > of returning -EOPNOTSUPP) seems to restore the prior functionality
> > > for me.
> > 
> > Yes, that is right if the input user data is 0 length or full of 0s
> > then we should call the normal driver function
> 
> Maybe I am wrong, yet I recall that doing ops->domain_alloc_user
> without a fallback was intentional to rule out drivers that don't
> support IOMMUFD?

I think we moved away from that to the idea of using the dma_domain
patch I suggested..

> To be backward-compatible and concern about SMMU, we can opt in
> ops->domain_alloc_user upon availability and then add a fallback:
> 
> 	if ((!ops || !ops->domain_alloc_user) && user_data) {
> 		rc = -EOPNOTSUPP;
> 		goto out_abort;
> 	}
> 
> 	if (ops->domain_alloc_user)
> 		hwpt->domain = ops->domain_alloc_user(dev, NULL, NULL);
> 	else
> 		hwpt->domain = iommu_domain_alloc(dev->bus);
> 	if (!hwpt->domain) {
> 		rc = -ENOMEM;
> 		goto out_abort;
> 	}
> 
> Yet, even by doing so, this series having the PATCH 07/17 that
> moves iopt_table_add_domain() would temporally break IOMMUFD on
> ARM platforms, until we add the ops->domain_alloc_user to SMMU
> drivers.

Drop patch 7 and 8

Change patch 12 so it has a unique flow to allocate and IOAS map a
HWPT that does not try to share so much code with the existing flow.

The HWPT flow should always just call allocate and then map with no
effort to attach first. This will fail on ARM SMMU at this point, and
that is fine.

All the existing code should work exactly as it is now and not have
any need to be changed.

Where things when wrong was trying to share
"__iommufd_hw_pagetable_alloc", I think.

Don't try to consolidate at this point. Once all the drivers are
updated then we could try to consolidate things.

Jason