| 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
| ||
|
Message-ID: <20240410152606.GF223006@ziepe.ca>
Date: Wed, 10 Apr 2024 12:26:06 -0300
From: Jason Gunthorpe <jgg@...pe.ca>
To: Lu Baolu <baolu.lu@...ux.intel.com>
Cc: Joerg Roedel <joro@...tes.org>, Will Deacon <will@...nel.org>,
Robin Murphy <robin.murphy@....com>,
Kevin Tian <kevin.tian@...el.com>,
Eric Badger <ebadger@...estorage.com>, iommu@...ts.linux.dev,
linux-kernel@...r.kernel.org
Subject: Re: [PATCH v3 1/5] iommu: Add static iommu_ops->release_domain
On Tue, Mar 05, 2024 at 09:33:01AM +0800, Lu Baolu wrote:
> The current device_release callback for individual iommu drivers does the
> following:
>
> 1) Silent IOMMU DMA translation: It detaches any existing domain from the
> device and puts it into a blocking state (some drivers might use the
> identity state).
> 2) Resource release: It releases resources allocated during the
> device_probe callback and restores the device to its pre-probe state.
>
> Step 1 is challenging for individual iommu drivers because each must check
> if a domain is already attached to the device. Additionally, if a deferred
> attach never occurred, the device_release should avoid modifying hardware
> configuration regardless of the reason for its call.
>
> To simplify this process, introduce a static release_domain within the
> iommu_ops structure. It can be either a blocking or identity domain
> depending on the iommu hardware. The iommu core will decide whether to
> attach this domain before the device_release callback, eliminating the
> need for repetitive code in various drivers.
>
> Consequently, the device_release callback can focus solely on the opposite
> operations of device_probe, including releasing all resources allocated
> during that callback.
>
> Co-developed-by: Jason Gunthorpe <jgg@...dia.com>
> Signed-off-by: Jason Gunthorpe <jgg@...dia.com>
> Signed-off-by: Lu Baolu <baolu.lu@...ux.intel.com>
> Reviewed-by: Kevin Tian <kevin.tian@...el.com>
> ---
> include/linux/iommu.h | 1 +
> drivers/iommu/iommu.c | 19 +++++++++++++++----
> 2 files changed, 16 insertions(+), 4 deletions(-)
I looked at all the drivers:
1) Didn't spend time to guess what ipmmu-vmss is doing
2) Several drivers are obviously missing the release_domain behavior
and now be trivially fixed
3) amd, s390, virtio-iommu and arm-smmu should probably support
blocked_domain (I assume that is what their detach does)
4) arm-smmuv3 can use it too once disable_bypass is removed
5) Several drivers don't even have release_device functions.
Probably all of them can do their identiy domains too.
This seems like a pretty reasonable thing to add to this series too:
diff --git a/drivers/iommu/apple-dart.c b/drivers/iommu/apple-dart.c
index eb1e62cd499a58..3ddc4b4418a049 100644
--- a/drivers/iommu/apple-dart.c
+++ b/drivers/iommu/apple-dart.c
@@ -979,6 +979,7 @@ static void apple_dart_get_resv_regions(struct device *dev,
static const struct iommu_ops apple_dart_iommu_ops = {
.identity_domain = &apple_dart_identity_domain,
.blocked_domain = &apple_dart_blocked_domain,
+ .release_domain = &apple_dart_blocked_domain,
.domain_alloc_paging = apple_dart_domain_alloc_paging,
.probe_device = apple_dart_probe_device,
.release_device = apple_dart_release_device,
diff --git a/drivers/iommu/exynos-iommu.c b/drivers/iommu/exynos-iommu.c
index d98c9161948a25..902dc4da44f987 100644
--- a/drivers/iommu/exynos-iommu.c
+++ b/drivers/iommu/exynos-iommu.c
@@ -1424,8 +1424,6 @@ static void exynos_iommu_release_device(struct device *dev)
struct exynos_iommu_owner *owner = dev_iommu_priv_get(dev);
struct sysmmu_drvdata *data;
- WARN_ON(exynos_iommu_identity_attach(&exynos_identity_domain, dev));
-
list_for_each_entry(data, &owner->controllers, owner_node)
device_link_del(data->link);
}
@@ -1471,6 +1469,7 @@ static int exynos_iommu_of_xlate(struct device *dev,
static const struct iommu_ops exynos_iommu_ops = {
.identity_domain = &exynos_identity_domain,
+ .release_domain = &exynos_identity_domain,
.domain_alloc_paging = exynos_iommu_domain_alloc_paging,
.device_group = generic_device_group,
.probe_device = exynos_iommu_probe_device,
diff --git a/drivers/iommu/mtk_iommu.c b/drivers/iommu/mtk_iommu.c
index b8c47f18bc2612..b5693041b18dd4 100644
--- a/drivers/iommu/mtk_iommu.c
+++ b/drivers/iommu/mtk_iommu.c
@@ -1012,6 +1012,7 @@ static void mtk_iommu_get_resv_regions(struct device *dev,
static const struct iommu_ops mtk_iommu_ops = {
.identity_domain = &mtk_iommu_identity_domain,
+ .release_domain = &mtk_iommu_identity_domain,
.domain_alloc_paging = mtk_iommu_domain_alloc_paging,
.probe_device = mtk_iommu_probe_device,
.release_device = mtk_iommu_release_device,
diff --git a/drivers/iommu/mtk_iommu_v1.c b/drivers/iommu/mtk_iommu_v1.c
index a9fa2a54dc9b39..9e7205af7d7316 100644
--- a/drivers/iommu/mtk_iommu_v1.c
+++ b/drivers/iommu/mtk_iommu_v1.c
@@ -580,6 +580,7 @@ static int mtk_iommu_v1_hw_init(const struct mtk_iommu_v1_data *data)
static const struct iommu_ops mtk_iommu_v1_ops = {
.identity_domain = &mtk_iommu_v1_identity_domain,
+ .release_domain = &mtk_iommu_v1_identity_domain,
.domain_alloc_paging = mtk_iommu_v1_domain_alloc_paging,
.probe_device = mtk_iommu_v1_probe_device,
.probe_finalize = mtk_iommu_v1_probe_finalize,
diff --git a/drivers/iommu/omap-iommu.c b/drivers/iommu/omap-iommu.c
index c9528065a59afa..c4c76aaec19e50 100644
--- a/drivers/iommu/omap-iommu.c
+++ b/drivers/iommu/omap-iommu.c
@@ -1725,6 +1725,7 @@ static void omap_iommu_release_device(struct device *dev)
static const struct iommu_ops omap_iommu_ops = {
.identity_domain = &omap_iommu_identity_domain,
+ .release_domain = &omap_iommu_identity_domain,
.domain_alloc_paging = omap_iommu_domain_alloc_paging,
.probe_device = omap_iommu_probe_device,
.release_device = omap_iommu_release_device,
diff --git a/drivers/iommu/rockchip-iommu.c b/drivers/iommu/rockchip-iommu.c
index da79d9f4cf6371..e551c5b143bbd3 100644
--- a/drivers/iommu/rockchip-iommu.c
+++ b/drivers/iommu/rockchip-iommu.c
@@ -1162,6 +1162,7 @@ static int rk_iommu_of_xlate(struct device *dev,
static const struct iommu_ops rk_iommu_ops = {
.identity_domain = &rk_identity_domain,
+ .release_domain = &rk_identity_domain,
.domain_alloc_paging = rk_iommu_domain_alloc_paging,
.probe_device = rk_iommu_probe_device,
.release_device = rk_iommu_release_device,
> + if (!dev->iommu->attach_deferred && ops->release_domain)
> + ops->release_domain->ops->attach_dev(ops->release_domain, dev);
We should probably be sensitive to the
dev->iommu->require_direct flag - generally drivers should prefer the
blocked for the release domain, but in case the FW ias asking for
require_direct we need to switch to identity.
Also, may as well avoid switching a domain if the group is already
correct and use the common attach function to get the tracing.. So
like this?
if (!dev->iommu->attach_deferred) {
struct iommu_domain *release_domain = ops->release_domain;
if (dev->iommu->require_direct && ops->identity_domain)
release_domain = ops->identity_domain;
if (release_domain && group->domain != release_domain)
WARN_ON(__iommu_attach_device(release_domain, dev));
}
Jason
Powered by blists - more mailing lists