linux-kernel - Re: [PATCH v1 06/12] iommufd/selftest: Implement mock_get_viommu_size and mock_viommu

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: <aEyH1YPkmW1gy2da@nvidia.com>
Date: Fri, 13 Jun 2025 13:19:33 -0700
From: Nicolin Chen <nicolinc@...dia.com>
To: Jason Gunthorpe <jgg@...dia.com>
CC: <kevin.tian@...el.com>, <will@...nel.org>, <robin.murphy@....com>,
	<joro@...tes.org>, <ddutile@...hat.com>, <yi.l.liu@...el.com>,
	<peterz@...radead.org>, <jsnitsel@...hat.com>, <praan@...gle.com>,
	<linux-arm-kernel@...ts.infradead.org>, <iommu@...ts.linux.dev>,
	<linux-kernel@...r.kernel.org>, <patches@...ts.linux.dev>,
	<baolu.lu@...ux.intel.com>
Subject: Re: [PATCH v1 06/12] iommufd/selftest: Implement
 mock_get_viommu_size and mock_viommu_init

On Fri, Jun 13, 2025 at 10:45:22AM -0300, Jason Gunthorpe wrote:
> On Mon, Jun 09, 2025 at 10:13:29AM -0700, Nicolin Chen wrote:
> > Sanitize the inputs and report the size of struct mock_viommu on success,
> > in mock_get_viommu_size().
> > 
> > The core will ensure the viommu_type is set to the core vIOMMU object, so
> > simply init the driver part in mock_viommu_init().
> > 
> > The mock_viommu_alloc() will be cleaned up once the transition is done.
> 
> Note the addition of the missed s2_parent store

I am adding a new patch in prep-v2 dropping the unused s2_parent:

-------------------------------------------------------------------
iommufd/selftest: Drop parent domain from mock_iommu_domain_nested

This is no use of this parent domain. Delete the dead code.

Note that the s2_parent in struct mock_viommu will be a deadcode too. Yet,
keep it because it will be soon used by HW queue objects, i.e. no point in
adding it back and forth in such a short window. Besides, keeping it could
cover the majority of vIOMMU use cases where a driver-level structure will
be larger in size than the core structure.

Signed-off-by: Nicolin Chen <nicolinc@...dia.com>
-------------------------------------------------------------------

In this case, we don't need to assign mock_viommu->s2_parent here
since it's a dead code. The HW queue series will assign it and add
a use case:

+/* Test iommufd_hw_queue_depend/undepend() */
+static int mock_hw_queue_init_phys(struct iommufd_hw_queue *hw_queue, u32 index,
+                                  phys_addr_t base_addr_pa)
...
+       /*
+        * Test to catch a kernel bug if the core converted the physical address
+        * incorrectly. Let mock_domain_iova_to_phys() WARN_ON if it fails.
+        */
+       if (base_addr_pa != iommu_iova_to_phys(&mock_viommu->s2_parent->domain,
+                                              hw_queue->base_addr)) {
+               rc = -EFAULT;
+               goto unlock;
+       }

> > +static int mock_viommu_init(struct iommufd_viommu *viommu,
> > +			    struct iommu_domain *parent_domain)
> > +{
> > +	struct mock_iommu_device *mock_iommu = container_of(
> > +		viommu->iommu_dev, struct mock_iommu_device, iommu_dev);
> > +	struct mock_viommu *mock_viommu = to_mock_viommu(viommu);
> > +
> > +	refcount_inc(&mock_iommu->users);
> > +	mock_viommu->s2_parent = to_mock_domain(parent_domain);
> > +
> > +	viommu->ops = &mock_viommu_ops;
> > +	return 0;
> > +}
> 
> The patches will read better if you add the call logic for init along
> side alloc based on init or alloc ops being non-NULL in the prior
> patch and then have these driver patches replace alloc with init.
> 
> Duplicating alloc into init and leaving both makes the patch harder to
> check.

I see. That will add an additional patch tentatively supporting
both ops.

Thanks
Nicolin