linux-kernel - Re: [PATCH v2 1/3] iommufd: Add devices_users to track the hw

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: <Y9gqFwDNd3VKQvC3@nvidia.com>
Date:   Mon, 30 Jan 2023 16:35:35 -0400
From:   Jason Gunthorpe <jgg@...dia.com>
To:     Nicolin Chen <nicolinc@...dia.com>
Cc:     kevin.tian@...el.com, yi.l.liu@...el.com, iommu@...ts.linux.dev,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2 1/3] iommufd: Add devices_users to track the
 hw_pagetable usage by device

On Mon, Jan 30, 2023 at 12:04:33PM -0800, Nicolin Chen wrote:

> I recall we've discussed this that SMMU sets up domain when it
> attaches the device to, so we made a compromise here...

The ARM driver has a problem that it doesn't know what SMMU instance
will host the domain when it is allocated so it doesn't know if it
should select a S1 or S2 page table format - which is determined by
the capabilities of the specific SMMU HW block.

However, we don't have this problem when creating the S2. The S2
should be created by a special alloc_domain_iommufd() asking for the
S2 format. Not only does the new alloc_domain_iommufd API directly
request a S2 format table, but it also specifies the struct device so
any residual details can be determined directly.

Thus there is no need to do the two stage process when working with
the S2.

So fixup the driver to create fully configured iommu_domain's
immediately and get rid of this problem.

IMHO I would structure the smmu driver so that all the different
iommu_domain formats have their own ops pointer. The special
"undecided" format would have a special ops with only attach_dev and
at first attach it would switch the ops to whatever format it
selected.

I think this could get rid of a lot of the 'if undecided/S1/S2/CD'
complexity all over the place. You know what type it is because you
were called on a op that is only called on its type.

Jason