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] [thread-next>] [day] [month] [year] [list]
Message-ID: <BN9PR11MB5276D10BD480FE66881870B08CFBA@BN9PR11MB5276.namprd11.prod.outlook.com>
Date: Thu, 30 Oct 2025 08:47:18 +0000
From: "Tian, Kevin" <kevin.tian@...el.com>
To: Nicolin Chen <nicolinc@...dia.com>, "joro@...tes.org" <joro@...tes.org>,
	"jgg@...dia.com" <jgg@...dia.com>
CC: "suravee.suthikulpanit@....com" <suravee.suthikulpanit@....com>,
	"will@...nel.org" <will@...nel.org>, "robin.murphy@....com"
	<robin.murphy@....com>, "sven@...nel.org" <sven@...nel.org>, "j@...nau.net"
	<j@...nau.net>, "jean-philippe@...aro.org" <jean-philippe@...aro.org>,
	"robin.clark@....qualcomm.com" <robin.clark@....qualcomm.com>,
	"dwmw2@...radead.org" <dwmw2@...radead.org>, "baolu.lu@...ux.intel.com"
	<baolu.lu@...ux.intel.com>, "yong.wu@...iatek.com" <yong.wu@...iatek.com>,
	"matthias.bgg@...il.com" <matthias.bgg@...il.com>,
	"angelogioacchino.delregno@...labora.com"
	<angelogioacchino.delregno@...labora.com>, "tjeznach@...osinc.com"
	<tjeznach@...osinc.com>, "pjw@...nel.org" <pjw@...nel.org>,
	"palmer@...belt.com" <palmer@...belt.com>, "aou@...s.berkeley.edu"
	<aou@...s.berkeley.edu>, "heiko@...ech.de" <heiko@...ech.de>,
	"schnelle@...ux.ibm.com" <schnelle@...ux.ibm.com>, "mjrosato@...ux.ibm.com"
	<mjrosato@...ux.ibm.com>, "wens@...e.org" <wens@...e.org>,
	"jernej.skrabec@...il.com" <jernej.skrabec@...il.com>, "samuel@...lland.org"
	<samuel@...lland.org>, "thierry.reding@...il.com" <thierry.reding@...il.com>,
	"jonathanh@...dia.com" <jonathanh@...dia.com>, "iommu@...ts.linux.dev"
	<iommu@...ts.linux.dev>, "linux-kernel@...r.kernel.org"
	<linux-kernel@...r.kernel.org>, "asahi@...ts.linux.dev"
	<asahi@...ts.linux.dev>, "linux-arm-kernel@...ts.infradead.org"
	<linux-arm-kernel@...ts.infradead.org>, "linux-arm-msm@...r.kernel.org"
	<linux-arm-msm@...r.kernel.org>, "linux-mediatek@...ts.infradead.org"
	<linux-mediatek@...ts.infradead.org>, "linux-riscv@...ts.infradead.org"
	<linux-riscv@...ts.infradead.org>, "linux-rockchip@...ts.infradead.org"
	<linux-rockchip@...ts.infradead.org>, "linux-s390@...r.kernel.org"
	<linux-s390@...r.kernel.org>, "linux-sunxi@...ts.linux.dev"
	<linux-sunxi@...ts.linux.dev>, "linux-tegra@...r.kernel.org"
	<linux-tegra@...r.kernel.org>, "virtualization@...ts.linux.dev"
	<virtualization@...ts.linux.dev>, "patches@...ts.linux.dev"
	<patches@...ts.linux.dev>
Subject: RE: [PATCH v1 02/20] iommu: Introduce a test_dev domain op and an
 internal helper

> From: Nicolin Chen <nicolinc@...dia.com>
> Sent: Monday, October 13, 2025 8:05 AM
> @@ -714,7 +714,12 @@ struct iommu_ops {
> 
>  /**
>   * 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
> + *            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.
> + *            It's suggested to avoid the kernel prints in this op.
>   *  Return:
>   * * 0		- success
>   * * EINVAL	- can indicate that device and domain are incompatible due
> to
> @@ -722,11 +727,15 @@ struct iommu_ops {
>   *		  driver shouldn't log an error, since it is legitimate for a
>   *		  caller to test reuse of existing domains. Otherwise, it may
>   *		  still represent some other fundamental problem
> - * * ENOMEM	- out of memory
> - * * ENOSPC	- non-ENOMEM type of resource allocation failures
>   * * EBUSY	- device is attached to a domain and cannot be changed
>   * * ENODEV	- device specific errors, not able to be attached
>   * * <others>	- treated as ENODEV by the caller. Use is discouraged
> + * @attach_dev: attach an iommu domain to a device
> + *  Return:
> + * * 0		- success
> + * * ENOMEM	- out of memory
> + * * ENOSPC	- non-ENOMEM type of resource allocation failures
> + * * <others>	- Use is discouraged

It might need more work to meet this requirement. e.g. after patch4
I could still spot other errors easily in the attach path:

intel_iommu_attach_device()
  iopf_for_domain_set()
    intel_iommu_enable_iopf():

        if (!info->pri_enabled)
                return -ENODEV;

intel_iommu_attach_device()
  dmar_domain_attach_device()
    domain_attach_iommu():
      
       curr = xa_cmpxchg(&domain->iommu_array, iommu->seq_id,
                          NULL, info, GFP_KERNEL);
        if (curr) {
                ret = xa_err(curr) ? : -EBUSY;
                goto err_clear;
        }

intel_iommu_attach_device()
  dmar_domain_attach_device()
    domain_setup_first_level()
      __domain_setup_first_level()
        intel_pasid_setup_first_level():

        pte = intel_pasid_get_entry(dev, pasid);
        if (!pte) {
                spin_unlock(&iommu->lock);
                return -ENODEV;
        }

        if (pasid_pte_is_present(pte)) {
                spin_unlock(&iommu->lock);
                return -EBUSY;
        }

On the other hand, how do we communicate whatever errors returned
by attach_dev in the reset_done path back to userspace? As noted above
resource allocation failures could still occur in attach_dev, but userspace
may think the requested attach in middle of a reset has succeeded as
long as it passes the test_dev check.

Does it work better to block the attaching process upon ongoing reset
and wake it up later upon reset_done to resume attach?