linux-kernel - Re: [PATCH v1 02/20] iommu: Introduce a test

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] [day] [month] [year] [list]

Message-ID: <21f4afb4069489729cf90469abf2d1321ed17bec.camel@linux.ibm.com>
Date: Tue, 14 Oct 2025 16:20:44 +0200
From: Niklas Schnelle <schnelle@...ux.ibm.com>
To: Nicolin Chen <nicolinc@...dia.com>
Cc: joro@...tes.org, jgg@...dia.com, kevin.tian@...el.com,
        suravee.suthikulpanit@....com, will@...nel.org, robin.murphy@....com,
        sven@...nel.org, j@...nau.net, jean-philippe@...aro.org,
        robin.clark@....qualcomm.com, dwmw2@...radead.org,
        baolu.lu@...ux.intel.com, yong.wu@...iatek.com, matthias.bgg@...il.com,
        angelogioacchino.delregno@...labora.com, tjeznach@...osinc.com,
        pjw@...nel.org, palmer@...belt.com, aou@...s.berkeley.edu,
        heiko@...ech.de, mjrosato@...ux.ibm.com, wens@...e.org,
        jernej.skrabec@...il.com, samuel@...lland.org,
        thierry.reding@...il.com, jonathanh@...dia.com, iommu@...ts.linux.dev,
        linux-kernel@...r.kernel.org, asahi@...ts.linux.dev,
        linux-arm-kernel@...ts.infradead.org, linux-arm-msm@...r.kernel.org,
        linux-mediatek@...ts.infradead.org, linux-riscv@...ts.infradead.org,
        linux-rockchip@...ts.infradead.org, linux-s390@...r.kernel.org,
        linux-sunxi@...ts.linux.dev, linux-tegra@...r.kernel.org,
        virtualization@...ts.linux.dev, patches@...ts.linux.dev
Subject: Re: [PATCH v1 02/20] iommu: Introduce a test_dev domain op and an
 internal helper

On Mon, 2025-10-13 at 10:22 -0700, Nicolin Chen wrote:
> Hi Niklas,
> 
> On Mon, Oct 13, 2025 at 11:53:55AM +0200, Niklas Schnelle wrote:
> > On Sun, 2025-10-12 at 17:04 -0700, Nicolin Chen wrote:
> > > Add a new test_dev domain op for driver to test the compatibility between
> > > a domain and a device at the driver level, before calling into the actual
> > > attachment/replacement of a domain. Support pasid for set_dev_pasid call.
> > > 
> > > Move existing core-level compatibility tests to a helper function. Invoke
> > > it prior to:
> > >  * __iommu_attach_device() or its wrapper __iommu_device_set_domain()
> > >  * __iommu_set_group_pasid()
> > 
> > Should this list also include iommu_deferred_attach()? The code does
> > include it.
> 
> iommu_deferred_attach() invokes __iommu_attach_device(), so it is
> already included in the list :)

Ok makes sense, though it does list __iommu_device_set_domain()
separately. Either way is fine for me.

> 
> > >  /**
> > >   * struct iommu_domain_ops - domain specific operations
> > > - * @attach_dev: attach an iommu domain to a device
> > > + * @test_dev: Test compatibility prior to an @attach_dev or @set_dev_pasid call.
> > > + *            A driver-level callback of this op should do a thorough sanity, to
> > 
> > You're missing the word "check" above.
> 
> Ack.
> 
> > > + *            make sure a device is compatible with the domain. So the following
> > > + *            @attach_dev and @set_dev_pasid functions would likely succeed with
> > > + *            only one exception due to a temporary failure like out of memory.
> > 
> > Nit: "… only one exception …" / "… like out of memory …" this sounds a
> > bit odd to me because on the one hand it's one exception but then also
> > a group (temporary failures).
> > 
> > Maybe better:
> > "… would likely succeed with only the exception of temporary failures
> > like out of memory."?
> 
> Sure. I can do that. Fixing both parts, it would be:
> 
>  * @test_dev: Test compatibility prior to an @attach_dev or @set_dev_pasid call.
>  *            A driver callback of this op should do a thorough sanity check, to
>  *            make sure a device is compatible with the domain, so the following
>  *            @attach_dev and @set_dev_pasid functions would likely succeed with
>  *            only the exception of temporary failures like out of memory.
> 

Sounds good, thanks!