Re: [PATCH 3/8] iommu/vt-d: Use ops->blocked_domain
On 9/25/23 7:41 PM, Jason Gunthorpe wrote: On Mon, Sep 25, 2023 at 10:29:52AM +0800, Baolu Lu wrote: On 9/23/23 1:07 AM, Jason Gunthorpe wrote: Trivially migrate to the ops->blocked_domain for the existing global static. Signed-off-by: Jason Gunthorpe --- drivers/iommu/intel/iommu.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) Reviewed-by: Lu Baolu P.S. We can further do the same thing to the identity domain. I will clean it up after all patches are landed. I looked at that, and it is not trivial.. Both the Intel and virtio-iommu drivers create an "identity" domain out of a paging domain and pass that off as a true "identity" domain. So neither can set the global static since the determination is at runtime.. Emm, yes. The early hardware requires a real 1:1 mapped page table. The recent implementations are no longer needed. What I was thinking about doing is consolidating that code so that the core logic is the thing turning a paging domain into an identity domain. Yes. It's not trivial. Needs a separated series with some refactoring efforts. Best regards, baolu
Re: [PATCH 3/8] iommu/vt-d: Use ops->blocked_domain
On 9/23/23 1:07 AM, Jason Gunthorpe wrote: Trivially migrate to the ops->blocked_domain for the existing global static. Signed-off-by: Jason Gunthorpe --- drivers/iommu/intel/iommu.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) Reviewed-by: Lu Baolu P.S. We can further do the same thing to the identity domain. I will clean it up after all patches are landed. Best regards, baolu
Re: [PATCH 2/8] iommu/vt-d: Update the definition of the blocking domain
On 9/23/23 1:07 AM, Jason Gunthorpe wrote: The global static should pre-define the type and the NOP free function can be now left as NULL. Signed-off-by: Jason Gunthorpe --- drivers/iommu/intel/iommu.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) Reviewed-by: Lu Baolu Best regards, baolu
Re: [PATCH 1/8] iommu: Move IOMMU_DOMAIN_BLOCKED global statics to ops->blocked_domain
On 9/23/23 1:07 AM, Jason Gunthorpe wrote: Following the pattern of identity domains, just assign the BLOCKED domain global statics to a value in ops. Update the core code to use the global static directly. Update powerpc to use the new scheme and remove its empty domain_alloc callback. Signed-off-by: Jason Gunthorpe --- arch/powerpc/kernel/iommu.c | 9 + drivers/iommu/iommu.c | 2 ++ include/linux/iommu.h | 3 +++ 3 files changed, 6 insertions(+), 8 deletions(-) Reviewed-by: Lu Baolu Best regards, baolu
Re: [PATCH v6 00/25] iommu: Make default_domain's mandatory
On 2023/8/15 1:30, Jason Gunthorpe wrote:
On Mon, Aug 14, 2023 at 04:43:23PM +0800, Baolu Lu wrote:
This is on github:https://github.com/jgunthorpe/linux/commits/iommu_all_defdom
It seems that after this series, all ARM iommu drivers are able to
support the IDENTITY default domain, hence perhaps we can remove below
code?
Yes, but this code is still used
If I remember it correctly, the background of this part of code is
that some arm drivers didn't support IDENTITY domain, so fall back to
DMA domain if IDENTITY domain allocation fails.
Not quite..
if (req_type)
return __iommu_group_alloc_default_domain(group, req_type);
req_type == 0 can still happen because it depends on what
def_domain_type returns, which is still 0 in alot of cases
/* The driver gave no guidance on what type to use, try the default */
dom = __iommu_group_alloc_default_domain(group, iommu_def_domain_type);
if (dom)
return dom;
So we try the default which might be IDENTITY/DMA/DMA_FQ - still have
to do this.
/* Otherwise IDENTITY and DMA_FQ defaults will try DMA */
if (iommu_def_domain_type == IOMMU_DOMAIN_DMA)
return NULL;
dom = __iommu_group_alloc_default_domain(group, IOMMU_DOMAIN_DMA);
if (!dom)
return NULL;
pr_warn("Failed to allocate default IOMMU domain of type %u for group %s -
Falling back to IOMMU_DOMAIN_DMA",
iommu_def_domain_type, group->name);
And this hunk is primarily a fallback in case the DMA_FQ didn't
work. Then we try normal DMA.
That it also protected against not implementing IDENTITY is a side
effect, so I think we have to keep all of this still.
Okay, fair enough. Thanks for the explanation.
Best regards,
baolu
Re: [PATCH v6 08/25] iommu: Reorganize iommu_get_default_domain_type() to respect def_domain_type()
On 2023/8/15 1:25, Jason Gunthorpe wrote:
Ah, I went over all this again and decided to try again, it is too
complicated. This patch can do what the commit message says and the
following patches are even simpler:
Yes. This method is more concise. Some nits below.
/*
* Combine the driver's choosen def_domain_type across all the devices in a
* group. Drivers must give a consistent result.
*/
static int iommu_get_def_domain_type(struct iommu_group *group,
struct device *dev, int cur_type)
{
const struct iommu_ops *ops = group_iommu_ops(group);
int type;
if (!ops->def_domain_type)
return cur_type;
type = ops->def_domain_type(dev);
if (!type || cur_type == type)
return cur_type;
if (!cur_type)
return type;
dev_err_ratelimited(
dev,
"IOMMU driver error, requesting conflicting def_domain_type, %s and
%s, for devices in group %u.\n",
iommu_domain_type_str(cur_type), iommu_domain_type_str(type),
group->id);
/*
* Try to recover, drivers are allowed to force IDENITY or DMA, IDENTITY
* takes precedence.
*/
if (cur_type || type == IOMMU_DOMAIN_IDENTITY)
return IOMMU_DOMAIN_IDENTITY;
No need to check cur_type. It already returned if cur_type is 0.
return cur_type;
}
/*
* A target_type of 0 will select the best domain type. 0 can be returned in
* this case meaning the global default should be used.
*/
static int iommu_get_default_domain_type(struct iommu_group *group,
int target_type)
{
struct device *untrusted = NULL;
struct group_device *gdev;
int driver_type = 0;
lockdep_assert_held(>mutex);
/*
* ARM32 drivers supporting CONFIG_ARM_DMA_USE_IOMMU can declare an
* identity_domain and it will automatically become their default
* domain. Later on ARM_DMA_USE_IOMMU will install its UNMANAGED domain.
* Override the selection to IDENTITY.
*/
if (IS_ENABLED(CONFIG_ARM_DMA_USE_IOMMU)) {
static_assert(!(IS_ENABLED(CONFIG_ARM_DMA_USE_IOMMU) &&
IS_ENABLED(CONFIG_IOMMU_DMA)));
IS_ENABLED(CONFIG_ARM_DMA_USE_IOMMU) is duplicate with the condition in
the if statement. So only
static_assert(!IS_ENABLED(CONFIG_IOMMU_DMA));
?
driver_type = IOMMU_DOMAIN_IDENTITY;
}
for_each_group_device(group, gdev) {
driver_type = iommu_get_def_domain_type(group, gdev->dev,
driver_type);
No need to call this in the loop body?
if (dev_is_pci(gdev->dev) && to_pci_dev(gdev->dev)->untrusted) {
/*
* No ARM32 using systems will set untrusted, it cannot
* work.
*/
if (WARN_ON(IS_ENABLED(CONFIG_ARM_DMA_USE_IOMMU)))
return -1;
untrusted = gdev->dev;
}
}
if (untrusted) {
if (driver_type && driver_type != IOMMU_DOMAIN_DMA) {
dev_err_ratelimited(
untrusted,
"Device is not trusted, but driver is overriding
group %u to %s, refusing to probe.\n",
group->id, iommu_domain_type_str(driver_type));
return -1;
}
driver_type = IOMMU_DOMAIN_DMA;
}
if (target_type) {
if (driver_type && target_type != driver_type)
return -1;
return target_type;
}
return driver_type;
}
Best regards,
baolu
Re: [PATCH v6 17/25] iommu/qcom_iommu: Add an IOMMU_IDENTITIY_DOMAIN
On 2023/8/14 23:36, Jason Gunthorpe wrote:
On Mon, Aug 14, 2023 at 02:32:33PM +0800, Baolu Lu wrote:
+ pm_runtime_get_sync(qcom_iommu->dev);
+ for (i = 0; i < fwspec->num_ids; i++) {
+ struct qcom_iommu_ctx *ctx = to_ctx(qcom_domain,
fwspec->ids[i]);
+
+ /* Disable the context bank: */
+ iommu_writel(ctx, ARM_SMMU_CB_SCTLR, 0);
+
+ ctx->domain = NULL;
Does setting ctx->domain to NULL still match this semantics?
Yes, I did not try to fix this driver. NULL means identity in the
ctx->domain.
Okay.
Best regards,
baolu
Re: [PATCH v6 07/25] iommu/mtk_iommu_v1: Implement an IDENTITY domain
On 2023/8/14 22:34, Jason Gunthorpe wrote:
@@ -443,7 +459,7 @@ static int mtk_iommu_v1_create_mapping(struct device *dev,
struct of_phandle_arg
static int mtk_iommu_v1_def_domain_type(struct device *dev)
{
- return IOMMU_DOMAIN_UNMANAGED;
+ return IOMMU_DOMAIN_IDENTITY;
def_domain_type can't be used for this purpose. But this seems to be a
temporary code, as it will be removed in patch 09/25.
It looked OK when I checked it, mkt_v1 is really confusing what it
tries to do, but it should call probe_finalize and basically do the
same hacky thing as what UNMANAGED was trying to accomplish.
Did you see something else?
No.
Best regards,
baolu
Re: [PATCH v6 00/25] iommu: Make default_domain's mandatory
On 2023/8/3 8:07, Jason Gunthorpe wrote:
[ It would be good to get this in linux-next, we have some good test
coverage on the ARM side already, thanks! ]
It has been a long time coming, this series completes the default_domain
transition and makes it so that the core IOMMU code will always have a
non-NULL default_domain for every driver on every
platform. set_platform_dma_ops() turned out to be a bad idea, and so
completely remove it.
This is achieved by changing each driver to either:
1 - Convert the existing (or deleted) ops->detach_dev() into an
op->attach_dev() of an IDENTITY domain.
This is based on the theory that the ARM32 HW is able to function when
the iommu is turned off and so the turned off state is an IDENTITY
translation.
2 - Use a new PLATFORM domain type. This is a hack to accommodate drivers
that we don't really know WTF they do. S390 is legitimately using this
to switch to it's platform dma_ops implementation, which is where the
name comes from.
3 - Do #1 and force the default domain to be IDENTITY, this corrects
the tegra-smmu case where even an ARM64 system would have a NULL
default_domain.
Using this we can apply the rules:
a) ARM_DMA_USE_IOMMU mode always uses either the driver's
ops->default_domain, ops->def_domain_type(), or an IDENTITY domain.
All ARM32 drivers provide one of these three options.
b) dma-iommu.c mode uses either the driver's ops->default_domain,
ops->def_domain_type or the usual DMA API policy logic based on the
command line/etc to pick IDENTITY/DMA domain types
c) All other arch's (PPC/S390) use ops->default_domain always.
See the patch "Require a default_domain for all iommu drivers" for a
per-driver breakdown.
The conversion broadly teaches a bunch of ARM32 drivers that they can do
IDENTITY domains. There is some educated guessing involved that these are
actual IDENTITY domains. If this turns out to be wrong the driver can be
trivially changed to use a BLOCKING domain type instead. Further, the
domain type only matters for drivers using ARM64's dma-iommu.c mode as it
will select IDENTITY based on the command line and expect IDENTITY to
work. For ARM32 and other arch cases it is purely documentation.
Finally, based on all the analysis in this series, we can purge
IOMMU_DOMAIN_UNMANAGED/DMA constants from most of the drivers. This
greatly simplifies understanding the driver contract to the core
code. IOMMU drivers should not be involved in policy for how the DMA API
works, that should be a core core decision.
The main gain from this work is to remove alot of ARM_DMA_USE_IOMMU
specific code and behaviors from drivers. All that remains in iommu
drivers after this series is the calls to arm_iommu_create_mapping().
This is a step toward removing ARM_DMA_USE_IOMMU.
The IDENTITY domains added to the ARM64 supporting drivers can be tested
by booting in ARM64 mode and enabling CONFIG_IOMMU_DEFAULT_PASSTHROUGH. If
the system still boots then most likely the implementation is an IDENTITY
domain. If not we can trivially change it to BLOCKING or at worst PLATFORM
if there is no detail what is going on in the HW.
I think this is pretty safe for the ARM32 drivers as they don't really
change, the code that was in detach_dev continues to be called in the same
places it was called before.
This is on github:https://github.com/jgunthorpe/linux/commits/iommu_all_defdom
It seems that after this series, all ARM iommu drivers are able to
support the IDENTITY default domain, hence perhaps we can remove below
code?
If I remember it correctly, the background of this part of code is
that some arm drivers didn't support IDENTITY domain, so fall back to
DMA domain if IDENTITY domain allocation fails.
diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
index ddbba3ffbfbd..ee1fa63f0612 100644
--- a/drivers/iommu/iommu.c
+++ b/drivers/iommu/iommu.c
@@ -1798,7 +1798,6 @@ iommu_group_alloc_default_domain(struct
iommu_group *group, int req_type)
list_first_entry(>devices, struct group_device, list)
->dev;
const struct iommu_ops *ops = dev_iommu_ops(dev);
- struct iommu_domain *dom;
lockdep_assert_held(>mutex);
@@ -1817,20 +1816,7 @@ iommu_group_alloc_default_domain(struct
iommu_group *group, int req_type)
return __iommu_group_alloc_default_domain(group, req_type);
/* The driver gave no guidance on what type to use, try the default */
- dom = __iommu_group_alloc_default_domain(group, iommu_def_domain_type);
- if (dom)
- return dom;
-
- /* Otherwise IDENTITY and DMA_FQ defaults will try DMA */
- if (iommu_def_domain_type == IOMMU_DOMAIN_DMA)
- return NULL;
- dom = __iommu_group_alloc_default_domain(group, IOMMU_DOMAIN_DMA);
- if (!dom)
- return NULL;
-
- pr_warn("Failed to allocate default IOMMU domain of type %u for group
%s - Falling back
Re: [PATCH v6 25/25] iommu: Convert remaining simple drivers to domain_alloc_paging()
On 2023/8/3 8:08, Jason Gunthorpe wrote: These drivers don't support IOMMU_DOMAIN_DMA, so this commit effectively allows them to support that mode. The prior work to require default_domains makes this safe because every one of these drivers is either compilation incompatible with dma-iommu.c, or already establishing a default_domain. In both cases alloc_domain() will never be called with IOMMU_DOMAIN_DMA for these drivers so it is safe to drop the test. Removing these tests clarifies that the domain allocation path is only about the functionality of a paging domain and has nothing to do with policy of how the paging domain is used for UNMANAGED/DMA/DMA_FQ. Tested-by: Niklas Schnelle Tested-by: Steven Price Tested-by: Marek Szyprowski Tested-by: Nicolin Chen Signed-off-by: Jason Gunthorpe --- drivers/iommu/msm_iommu.c| 7 ++- drivers/iommu/mtk_iommu_v1.c | 7 ++- drivers/iommu/omap-iommu.c | 7 ++- drivers/iommu/s390-iommu.c | 7 ++- 4 files changed, 8 insertions(+), 20 deletions(-) Reviewed-by: Lu Baolu Best regards, baolu
Re: [PATCH v6 24/25] iommu: Convert simple drivers with DOMAIN_DMA to domain_alloc_paging()
On 2023/8/3 8:08, Jason Gunthorpe wrote:
These drivers are all trivially converted since the function is only
called if the domain type is going to be
IOMMU_DOMAIN_UNMANAGED/DMA.
Tested-by: Heiko Stuebner
Tested-by: Steven Price
Tested-by: Marek Szyprowski
Tested-by: Nicolin Chen
Signed-off-by: Jason Gunthorpe
---
drivers/iommu/arm/arm-smmu/qcom_iommu.c | 6 ++
drivers/iommu/exynos-iommu.c| 7 ++-
drivers/iommu/ipmmu-vmsa.c | 7 ++-
drivers/iommu/mtk_iommu.c | 7 ++-
drivers/iommu/rockchip-iommu.c | 7 ++-
drivers/iommu/sprd-iommu.c | 7 ++-
drivers/iommu/sun50i-iommu.c| 9 +++--
drivers/iommu/tegra-smmu.c | 7 ++-
8 files changed, 17 insertions(+), 40 deletions(-)
[...]
diff --git a/drivers/iommu/sun50i-iommu.c b/drivers/iommu/sun50i-iommu.c
index 0bf08b120cf105..056832a367c2af 100644
--- a/drivers/iommu/sun50i-iommu.c
+++ b/drivers/iommu/sun50i-iommu.c
@@ -667,14 +667,11 @@ static phys_addr_t sun50i_iommu_iova_to_phys(struct
iommu_domain *domain,
sun50i_iova_get_page_offset(iova);
}
-static struct iommu_domain *sun50i_iommu_domain_alloc(unsigned type)
+static struct iommu_domain *
+sun50i_iommu_domain_alloc_paging(struct device *paging)
Why not "struct device *dev"?
Typo? Or anything I missed?
{
struct sun50i_iommu_domain *sun50i_domain;
- if (type != IOMMU_DOMAIN_DMA &&
- type != IOMMU_DOMAIN_UNMANAGED)
- return NULL;
-
sun50i_domain = kzalloc(sizeof(*sun50i_domain), GFP_KERNEL);
if (!sun50i_domain)
return NULL;
@@ -840,7 +837,7 @@ static const struct iommu_ops sun50i_iommu_ops = {
.identity_domain = _iommu_identity_domain,
.pgsize_bitmap = SZ_4K,
.device_group = sun50i_iommu_device_group,
- .domain_alloc = sun50i_iommu_domain_alloc,
+ .domain_alloc_paging = sun50i_iommu_domain_alloc_paging,
.of_xlate = sun50i_iommu_of_xlate,
.probe_device = sun50i_iommu_probe_device,
.default_domain_ops = &(const struct iommu_domain_ops) {
diff --git a/drivers/iommu/tegra-smmu.c b/drivers/iommu/tegra-smmu.c
index 6cba034905edbf..69c40c191ce4f0 100644
--- a/drivers/iommu/tegra-smmu.c
+++ b/drivers/iommu/tegra-smmu.c
@@ -272,13 +272,10 @@ static void tegra_smmu_free_asid(struct tegra_smmu *smmu,
unsigned int id)
clear_bit(id, smmu->asids);
}
-static struct iommu_domain *tegra_smmu_domain_alloc(unsigned type)
+static struct iommu_domain *tegra_smmu_domain_alloc_paging(struct device *dev)
{
struct tegra_smmu_as *as;
- if (type != IOMMU_DOMAIN_UNMANAGED && type != IOMMU_DOMAIN_DMA)
- return NULL;
-
as = kzalloc(sizeof(*as), GFP_KERNEL);
if (!as)
return NULL;
@@ -997,7 +994,7 @@ static const struct iommu_ops tegra_smmu_ops = {
.default_domain = _smmu_identity_domain,
.identity_domain = _smmu_identity_domain,
.def_domain_type = _smmu_def_domain_type,
- .domain_alloc = tegra_smmu_domain_alloc,
+ .domain_alloc_paging = tegra_smmu_domain_alloc_paging,
.probe_device = tegra_smmu_probe_device,
.device_group = tegra_smmu_device_group,
.of_xlate = tegra_smmu_of_xlate,
Anyway,
Reviewed-by: Lu Baolu
Best regards,
baolu
Re: [PATCH v6 20/25] iommu/sun50i: Add an IOMMU_IDENTITIY_DOMAIN
On 2023/8/3 8:08, Jason Gunthorpe wrote:
Prior to commit 1b932ceddd19 ("iommu: Remove detach_dev callbacks") the
sun50i_iommu_detach_device() function was being called by
ops->detach_dev().
This is an IDENTITY domain so convert sun50i_iommu_detach_device() into
sun50i_iommu_identity_attach() and a full IDENTITY domain and thus hook it
back up the same was as the old ops->detach_dev().
Signed-off-by: Jason Gunthorpe
---
drivers/iommu/sun50i-iommu.c | 26 +++---
1 file changed, 19 insertions(+), 7 deletions(-)
diff --git a/drivers/iommu/sun50i-iommu.c b/drivers/iommu/sun50i-iommu.c
index 74c5cb93e90027..0bf08b120cf105 100644
--- a/drivers/iommu/sun50i-iommu.c
+++ b/drivers/iommu/sun50i-iommu.c
@@ -757,21 +757,32 @@ static void sun50i_iommu_detach_domain(struct
sun50i_iommu *iommu,
iommu->domain = NULL;
}
-static void sun50i_iommu_detach_device(struct iommu_domain *domain,
- struct device *dev)
+static int sun50i_iommu_identity_attach(struct iommu_domain *identity_domain,
+ struct device *dev)
{
- struct sun50i_iommu_domain *sun50i_domain = to_sun50i_domain(domain);
struct sun50i_iommu *iommu = dev_iommu_priv_get(dev);
+ struct sun50i_iommu_domain *sun50i_domain;
dev_dbg(dev, "Detaching from IOMMU domain\n");
- if (iommu->domain != domain)
- return;
+ if (iommu->domain == identity_domain)
+ return 0;
+ sun50i_domain = to_sun50i_domain(iommu->domain);
if (refcount_dec_and_test(_domain->refcnt))
sun50i_iommu_detach_domain(iommu, sun50i_domain);
+ return 0;
}
Does iommu->domain need to be set to identity_domain before returning?
Best regards,
baolu
Re: [PATCH v6 19/25] iommu/mtk_iommu: Add an IOMMU_IDENTITIY_DOMAIN
On 2023/8/3 8:08, Jason Gunthorpe wrote:
This brings back the ops->detach_dev() code that commit
1b932ceddd19 ("iommu: Remove detach_dev callbacks") deleted and turns it
into an IDENTITY domain.
Signed-off-by: Jason Gunthorpe
---
drivers/iommu/mtk_iommu.c | 23 +++
1 file changed, 23 insertions(+)
Reviewed-by: Lu Baolu
Best regards,
baolu
Re: [PATCH v6 18/25] iommu/ipmmu: Add an IOMMU_IDENTITIY_DOMAIN
On 2023/8/3 8:08, Jason Gunthorpe wrote:
This brings back the ops->detach_dev() code that commit
1b932ceddd19 ("iommu: Remove detach_dev callbacks") deleted and turns it
into an IDENTITY domain.
Also reverts commit 584d334b1393 ("iommu/ipmmu-vmsa: Remove
ipmmu_utlb_disable()")
Signed-off-by: Jason Gunthorpe
---
drivers/iommu/ipmmu-vmsa.c | 43 ++
1 file changed, 43 insertions(+)
Reviewed-by: Lu Baolu
Best regards,
baolu
Re: [PATCH v6 17/25] iommu/qcom_iommu: Add an IOMMU_IDENTITIY_DOMAIN
On 2023/8/3 8:08, Jason Gunthorpe wrote:
This brings back the ops->detach_dev() code that commit
1b932ceddd19 ("iommu: Remove detach_dev callbacks") deleted and turns it
into an IDENTITY domain.
Signed-off-by: Jason Gunthorpe
---
drivers/iommu/arm/arm-smmu/qcom_iommu.c | 39 +
1 file changed, 39 insertions(+)
diff --git a/drivers/iommu/arm/arm-smmu/qcom_iommu.c
b/drivers/iommu/arm/arm-smmu/qcom_iommu.c
index a503ed758ec302..9d7b9d8b4386d4 100644
--- a/drivers/iommu/arm/arm-smmu/qcom_iommu.c
+++ b/drivers/iommu/arm/arm-smmu/qcom_iommu.c
@@ -387,6 +387,44 @@ static int qcom_iommu_attach_dev(struct iommu_domain
*domain, struct device *dev
return 0;
}
+static int qcom_iommu_identity_attach(struct iommu_domain *identity_domain,
+ struct device *dev)
+{
+ struct iommu_domain *domain = iommu_get_domain_for_dev(dev);
+ struct qcom_iommu_domain *qcom_domain;
+ struct iommu_fwspec *fwspec = dev_iommu_fwspec_get(dev);
+ struct qcom_iommu_dev *qcom_iommu = to_iommu(dev);
+ unsigned int i;
+
+ if (domain == identity_domain || !domain)
+ return 0;
+
+ qcom_domain = to_qcom_iommu_domain(domain);
+ if (WARN_ON(!qcom_domain->iommu))
+ return -EINVAL;
+
+ pm_runtime_get_sync(qcom_iommu->dev);
+ for (i = 0; i < fwspec->num_ids; i++) {
+ struct qcom_iommu_ctx *ctx = to_ctx(qcom_domain,
fwspec->ids[i]);
+
+ /* Disable the context bank: */
+ iommu_writel(ctx, ARM_SMMU_CB_SCTLR, 0);
+
+ ctx->domain = NULL;
Does setting ctx->domain to NULL still match this semantics?
+ }
+ pm_runtime_put_sync(qcom_iommu->dev);
+ return 0;
+}
+
+static struct iommu_domain_ops qcom_iommu_identity_ops = {
+ .attach_dev = qcom_iommu_identity_attach,
+};
+
+static struct iommu_domain qcom_iommu_identity_domain = {
+ .type = IOMMU_DOMAIN_IDENTITY,
+ .ops = _iommu_identity_ops,
+};
+
static int qcom_iommu_map(struct iommu_domain *domain, unsigned long iova,
phys_addr_t paddr, size_t pgsize, size_t pgcount,
int prot, gfp_t gfp, size_t *mapped)
@@ -553,6 +591,7 @@ static int qcom_iommu_of_xlate(struct device *dev, struct
of_phandle_args *args)
}
static const struct iommu_ops qcom_iommu_ops = {
+ .identity_domain = _iommu_identity_domain,
.capable= qcom_iommu_capable,
.domain_alloc = qcom_iommu_domain_alloc,
.probe_device = qcom_iommu_probe_device,
Anyway,
Reviewed-by: Lu Baolu
Best regards,
baolu
Re: [PATCH v6 14/25] iommu/msm: Implement an IDENTITY domain
On 2023/8/3 8:08, Jason Gunthorpe wrote: What msm does during msm_iommu_set_platform_dma() is actually putting the iommu into identity mode. Move to the new core support for ARM_DMA_USE_IOMMU by defining ops->identity_domain. This driver does not support IOMMU_DOMAIN_DMA, however it cannot be compiled on ARM64 either. Most likely it is fine to support dma-iommu.c Signed-off-by: Jason Gunthorpe --- drivers/iommu/msm_iommu.c | 23 +++ 1 file changed, 19 insertions(+), 4 deletions(-) Reviewed-by: Lu Baolu Best regards, baolu
Re: [PATCH v6 13/25] iommu/omap: Implement an IDENTITY domain
On 2023/8/3 8:08, Jason Gunthorpe wrote: What omap does during omap_iommu_set_platform_dma() is actually putting the iommu into identity mode. Move to the new core support for ARM_DMA_USE_IOMMU by defining ops->identity_domain. This driver does not support IOMMU_DOMAIN_DMA, however it cannot be compiled on ARM64 either. Most likely it is fine to support dma-iommu.c Signed-off-by: Jason Gunthorpe --- drivers/iommu/omap-iommu.c | 21 ++--- 1 file changed, 18 insertions(+), 3 deletions(-) Reviewed-by: Lu Baolu Best regards, baolu
Re: [PATCH v6 12/25] iommu/tegra-smmu: Support DMA domains in tegra
On 2023/8/3 8:07, Jason Gunthorpe wrote:
All ARM64 iommu drivers should support IOMMU_DOMAIN_DMA to enable
dma-iommu.c.
tegra is blocking dma-iommu usage, and also default_domain's, because it
wants an identity translation. This is needed for some device quirk. The
correct way to do this is to support IDENTITY domains and use
ops->def_domain_type() to return IOMMU_DOMAIN_IDENTITY for only the quirky
devices.
Add support for IOMMU_DOMAIN_DMA and force IOMMU_DOMAIN_IDENTITY mode for
everything so no behavior changes.
Signed-off-by: Jason Gunthorpe
---
drivers/iommu/tegra-smmu.c | 8 +++-
1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/drivers/iommu/tegra-smmu.c b/drivers/iommu/tegra-smmu.c
index f63f1d4f0bd10f..6cba034905edbf 100644
--- a/drivers/iommu/tegra-smmu.c
+++ b/drivers/iommu/tegra-smmu.c
@@ -276,7 +276,7 @@ static struct iommu_domain
*tegra_smmu_domain_alloc(unsigned type)
{
struct tegra_smmu_as *as;
- if (type != IOMMU_DOMAIN_UNMANAGED)
+ if (type != IOMMU_DOMAIN_UNMANAGED && type != IOMMU_DOMAIN_DMA)
return NULL;
as = kzalloc(sizeof(*as), GFP_KERNEL);
@@ -989,6 +989,12 @@ static int tegra_smmu_def_domain_type(struct device *dev)
}
static const struct iommu_ops tegra_smmu_ops = {
+ /*
+* FIXME: For now we want to run all translation in IDENTITY mode,
+* better would be to have a def_domain_type op do this for just the
+* quirky device.
+*/
+ .default_domain = _smmu_identity_domain,
tegra_smmu_def_domain_type() has already forced the core to use
ops->identity_domain, why do we still need ops->default_domain?
.identity_domain = _smmu_identity_domain,
.def_domain_type = _smmu_def_domain_type,
.domain_alloc = tegra_smmu_domain_alloc,
Best regards,
baolu
Re: [PATCH v6 11/25] iommu/tegra-smmu: Implement an IDENTITY domain
On 2023/8/3 8:07, Jason Gunthorpe wrote:
What tegra-smmu does during tegra_smmu_set_platform_dma() is actually
putting the iommu into identity mode.
Move to the new core support for ARM_DMA_USE_IOMMU by defining
ops->identity_domain.
Signed-off-by: Jason Gunthorpe
---
drivers/iommu/tegra-smmu.c | 37 -
1 file changed, 32 insertions(+), 5 deletions(-)
diff --git a/drivers/iommu/tegra-smmu.c b/drivers/iommu/tegra-smmu.c
index 1cbf063ccf147a..f63f1d4f0bd10f 100644
--- a/drivers/iommu/tegra-smmu.c
+++ b/drivers/iommu/tegra-smmu.c
@@ -511,23 +511,39 @@ static int tegra_smmu_attach_dev(struct iommu_domain
*domain,
return err;
}
-static void tegra_smmu_set_platform_dma(struct device *dev)
+static int tegra_smmu_identity_attach(struct iommu_domain *identity_domain,
+ struct device *dev)
{
struct iommu_domain *domain = iommu_get_domain_for_dev(dev);
struct iommu_fwspec *fwspec = dev_iommu_fwspec_get(dev);
- struct tegra_smmu_as *as = to_smmu_as(domain);
- struct tegra_smmu *smmu = as->smmu;
+ struct tegra_smmu_as *as;
+ struct tegra_smmu *smmu;
unsigned int index;
if (!fwspec)
- return;
+ return -ENODEV;
+ if (domain == identity_domain || !domain)
+ return 0;
+
+ as = to_smmu_as(domain);
+ smmu = as->smmu;
for (index = 0; index < fwspec->num_ids; index++) {
tegra_smmu_disable(smmu, fwspec->ids[index], as->id);
tegra_smmu_as_unprepare(smmu, as);
}
+ return 0;
}
+static struct iommu_domain_ops tegra_smmu_identity_ops = {
+ .attach_dev = tegra_smmu_identity_attach,
+};
+
+static struct iommu_domain tegra_smmu_identity_domain = {
+ .type = IOMMU_DOMAIN_IDENTITY,
+ .ops = _smmu_identity_ops,
+};
+
static void tegra_smmu_set_pde(struct tegra_smmu_as *as, unsigned long iova,
u32 value)
{
@@ -962,11 +978,22 @@ static int tegra_smmu_of_xlate(struct device *dev,
return iommu_fwspec_add_ids(dev, , 1);
}
+static int tegra_smmu_def_domain_type(struct device *dev)
+{
+ /*
+* FIXME: For now we want to run all translation in IDENTITY mode, due
+* to some device quirks. Better would be to just quirk the troubled
+* devices.
+*/
+ return IOMMU_DOMAIN_IDENTITY;
+}
Hope somebody can fix this later on.
Reviewed-by: Lu Baolu
Best regards,
baolu
Re: [PATCH v6 10/25] iommu/exynos: Implement an IDENTITY domain
On 2023/8/3 8:07, Jason Gunthorpe wrote:
What exynos calls exynos_iommu_detach_device is actually putting the iommu
into identity mode.
Move to the new core support for ARM_DMA_USE_IOMMU by defining
ops->identity_domain.
Tested-by: Marek Szyprowski
Acked-by: Marek Szyprowski
Signed-off-by: Jason Gunthorpe
---
drivers/iommu/exynos-iommu.c | 66 +---
1 file changed, 32 insertions(+), 34 deletions(-)
diff --git a/drivers/iommu/exynos-iommu.c b/drivers/iommu/exynos-iommu.c
index c275fe71c4db32..5e12b85dfe8705 100644
--- a/drivers/iommu/exynos-iommu.c
+++ b/drivers/iommu/exynos-iommu.c
@@ -24,6 +24,7 @@
typedef u32 sysmmu_iova_t;
typedef u32 sysmmu_pte_t;
+static struct iommu_domain exynos_identity_domain;
Is there a conflict between above and below?
[...]
+static struct iommu_domain_ops exynos_identity_ops = {
+ .attach_dev = exynos_iommu_identity_attach,
+};
+
+static struct iommu_domain exynos_identity_domain = {
here.
Both define the same.
+ .type = IOMMU_DOMAIN_IDENTITY,
+ .ops = _identity_ops,
+};
Best regards,
baolu
Re: [PATCH v6 09/25] iommu: Allow an IDENTITY domain as the default_domain in ARM32
On 2023/8/3 8:07, Jason Gunthorpe wrote: Even though dma-iommu.c and CONFIG_ARM_DMA_USE_IOMMU do approximately the same stuff, the way they relate to the IOMMU core is quiet different. dma-iommu.c expects the core code to setup an UNMANAGED domain (of type IOMMU_DOMAIN_DMA) and then configures itself to use that domain. This becomes the default_domain for the group. ARM_DMA_USE_IOMMU does not use the default_domain, instead it directly allocates an UNMANAGED domain and operates it just like an external driver. In this case group->default_domain is NULL. If the driver provides a global static identity_domain then automatically use it as the default_domain when in ARM_DMA_USE_IOMMU mode. This allows drivers that implemented default_domain == NULL as an IDENTITY translation to trivially get a properly labeled non-NULL default_domain on ARM32 configs. With this arrangment when ARM_DMA_USE_IOMMU wants to disconnect from the device the normal detach_domain flow will restore the IDENTITY domain as the default domain. Overall this makes attach_dev() of the IDENTITY domain called in the same places as detach_dev(). This effectively migrates these drivers to default_domain mode. For drivers that support ARM64 they will gain support for the IDENTITY translation mode for the dma_api and behave in a uniform way. Drivers use this by setting ops->identity_domain to a static singleton iommu_domain that implements the identity attach. If the core detects ARM_DMA_USE_IOMMU mode then it automatically attaches the IDENTITY domain during probe. Drivers can continue to prevent the use of DMA translation by returning IOMMU_DOMAIN_IDENTITY from def_domain_type, this will completely prevent IOMMU_DMA from running but will not impact ARM_DMA_USE_IOMMU. This allows removing the set_platform_dma_ops() from every remaining driver. Remove the set_platform_dma_ops from rockchip and mkt_v1 as all it does is set an existing global static identity domain. mkt_v1 does not support IOMMU_DOMAIN_DMA and it does not compile on ARM64 so this transformation is safe. Tested-by: Steven Price Tested-by: Marek Szyprowski Tested-by: Nicolin Chen Signed-off-by: Jason Gunthorpe --- drivers/iommu/iommu.c | 26 +++--- drivers/iommu/mtk_iommu_v1.c | 12 drivers/iommu/rockchip-iommu.c | 10 -- 3 files changed, 23 insertions(+), 25 deletions(-) Reviewed-by: Lu Baolu Best regards, baolu
Re: [PATCH v6 07/25] iommu/mtk_iommu_v1: Implement an IDENTITY domain
On 2023/8/3 8:07, Jason Gunthorpe wrote:
What mtk does during mtk_iommu_v1_set_platform_dma() is actually putting
the iommu into identity mode. Make this available as a proper IDENTITY
domain.
The mtk_iommu_v1_def_domain_type() from
commit 8bbe13f52cb7 ("iommu/mediatek-v1: Add def_domain_type") explains
this was needed to allow probe_finalize() to be called, but now the
IDENTITY domain will do the same job so change the returned
def_domain_type.
mkt_v1 is the only driver that returns IOMMU_DOMAIN_UNMANAGED from
def_domain_type(). This allows the next patch to enforce an IDENTITY
domain policy for this driver.
This code seems to be not working properly.
* @def_domain_type: device default domain type, return value:
* - IOMMU_DOMAIN_IDENTITY: must use an identity domain
* - IOMMU_DOMAIN_DMA: must use a dma domain
* - 0: use the default setting
The core code is not ready to accept any other return value.
Signed-off-by: Jason Gunthorpe
---
drivers/iommu/mtk_iommu_v1.c | 21 +++--
1 file changed, 19 insertions(+), 2 deletions(-)
diff --git a/drivers/iommu/mtk_iommu_v1.c b/drivers/iommu/mtk_iommu_v1.c
index 8a0a5e5d049f4a..cc3e7d53d33ad9 100644
--- a/drivers/iommu/mtk_iommu_v1.c
+++ b/drivers/iommu/mtk_iommu_v1.c
@@ -319,11 +319,27 @@ static int mtk_iommu_v1_attach_device(struct iommu_domain
*domain, struct device
return 0;
}
-static void mtk_iommu_v1_set_platform_dma(struct device *dev)
+static int mtk_iommu_v1_identity_attach(struct iommu_domain *identity_domain,
+ struct device *dev)
{
struct mtk_iommu_v1_data *data = dev_iommu_priv_get(dev);
mtk_iommu_v1_config(data, dev, false);
+ return 0;
+}
+
+static struct iommu_domain_ops mtk_iommu_v1_identity_ops = {
+ .attach_dev = mtk_iommu_v1_identity_attach,
+};
+
+static struct iommu_domain mtk_iommu_v1_identity_domain = {
+ .type = IOMMU_DOMAIN_IDENTITY,
+ .ops = _iommu_v1_identity_ops,
+};
+
+static void mtk_iommu_v1_set_platform_dma(struct device *dev)
+{
+ mtk_iommu_v1_identity_attach(_iommu_v1_identity_domain, dev);
This callback seems to be a dead code now.
}
static int mtk_iommu_v1_map(struct iommu_domain *domain, unsigned long iova,
@@ -443,7 +459,7 @@ static int mtk_iommu_v1_create_mapping(struct device *dev,
struct of_phandle_arg
static int mtk_iommu_v1_def_domain_type(struct device *dev)
{
- return IOMMU_DOMAIN_UNMANAGED;
+ return IOMMU_DOMAIN_IDENTITY;
def_domain_type can't be used for this purpose. But this seems to be a
temporary code, as it will be removed in patch 09/25.
}
static struct iommu_device *mtk_iommu_v1_probe_device(struct device *dev)
@@ -578,6 +594,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 = _iommu_v1_identity_domain,
.domain_alloc = mtk_iommu_v1_domain_alloc,
.probe_device = mtk_iommu_v1_probe_device,
.probe_finalize = mtk_iommu_v1_probe_finalize,
Best regards,
baolu
Re: [PATCH v6 05/25] iommu/fsl_pamu: Implement a PLATFORM domain
On 2023/8/3 8:07, Jason Gunthorpe wrote: This driver is nonsensical. To not block migrating the core API away from NULL default_domains give it a hacky of a PLATFORM domain that keeps it working exactly as it always did. Leave some comments around to warn away any future people looking at this. Signed-off-by: Jason Gunthorpe --- drivers/iommu/fsl_pamu_domain.c | 41 ++--- 1 file changed, 38 insertions(+), 3 deletions(-) Reviewed-by: Lu Baolu Best regards, baolu
Re: [PATCH v6 04/25] iommu: Add IOMMU_DOMAIN_PLATFORM for S390
On 2023/8/3 8:07, Jason Gunthorpe wrote: The PLATFORM domain will be set as the default domain and attached as normal during probe. The driver will ignore the initial attach from a NULL domain to the PLATFORM domain. After this, the PLATFORM domain's attach_dev will be called whenever we detach from an UNMANAGED domain (eg for VFIO). This is the same time the original design would have called op->detach_dev(). This is temporary until the S390 dma-iommu.c conversion is merged. Tested-by: Heiko Stuebner Tested-by: Niklas Schnelle Signed-off-by: Jason Gunthorpe --- drivers/iommu/s390-iommu.c | 21 +++-- 1 file changed, 19 insertions(+), 2 deletions(-) Reviewed-by: Lu Baolu Best regards, baolu
Re: [PATCH v6 03/25] powerpc/iommu: Setup a default domain and remove set_platform_dma_ops
On 2023/8/3 8:07, Jason Gunthorpe wrote: POWER is using the set_platform_dma_ops() callback to hook up its private dma_ops, but this is buired under some indirection and is weirdly happening for a BLOCKED domain as well. For better documentation create a PLATFORM domain to manage the dma_ops, since that is what it is for, and make the BLOCKED domain an alias for it. BLOCKED is required for VFIO. Also removes the leaky allocation of the BLOCKED domain by using a global static. Signed-off-by: Jason Gunthorpe --- arch/powerpc/kernel/iommu.c | 38 + 1 file changed, 17 insertions(+), 21 deletions(-) Not sure how to fix the fake blocking domain in this driver. But it's not the purpose of this patch, so Reviewed-by: Lu Baolu Best regards, baolu
Re: [PATCH v6 02/25] iommu: Add IOMMU_DOMAIN_PLATFORM
On 2023/8/12 19:28, Jason Gunthorpe wrote:
On Sat, Aug 12, 2023 at 09:36:33AM +0800, Baolu Lu wrote:
@@ -290,6 +295,7 @@ struct iommu_ops {
unsigned long pgsize_bitmap;
struct module *owner;
struct iommu_domain *identity_domain;
+ struct iommu_domain *default_domain;
I am imaging whether we can merge above two pointers into a single one.
It is either an IDENTITY or PLATFORM domain and the core will choose it
as the default domain of a group if iommu_group_alloc_default_domain()
fails to allocate one through the iommu dev_ops.
I think that would be the wrong direction..
identity_domain is a pointer that is always, ALWAYS an identity
domain. It is the shortcut for drivers (and all drivers should do
this) that implement a global static identity domain.
I see. I originally thought this was special for arm32.
default_domain is a shortcut to avoid implementing the entire flow
around def_domain_type/domain_alloc for special cases. For this patch
the specialc ase is the IOMMU_DOMAIN_PLATFORM.
I think this is special for drivers like s390. You don't want it to be
used beyond those special drivers, right?
If so, the naming of default_domain seems to be a bit generic. I can't
think of a better one, hence I am fine if you keep as it-is. After all,
the comment for this field has already explained it very clearly.
We'll probably also get a blocking_domain pointer here too.
Yes.
All of this is removing the type multiplexor in alloc_domain so we can
so alloc_domain_paging()
Agreed with you. The dummy domains like identity and blocking could be
avoided from calling ops->domain_alloc.
Probably we could give it a more meaningful name? For example,
supplemental_domain or rescue_domain?
But that isn't what it is for, default_domain is the operational
domain for attached drivers..
Best regards,
baolu
Re: [PATCH v6 15/25] iommufd/selftest: Make the mock iommu driver into a real driver
On 2023/8/3 8:08, Jason Gunthorpe wrote:
+/*
+ * Register an iommu driver against a single bus. This is only used by iommufd
+ * selftest to create a mock iommu driver. The caller must provide
+ * some memory to hold a notifier_block.
+ */
+int iommu_device_register_bus(struct iommu_device *iommu,
+ const struct iommu_ops *ops, struct bus_type *bus,
+ struct notifier_block *nb)
+{
+ int err;
+
+ iommu->ops = ops;
+ nb->notifier_call = iommu_bus_notifier;
+ err = bus_register_notifier(bus, nb);
+ if (err)
+ return err;
+
+ spin_lock(_device_lock);
+ list_add_tail(>list, _device_list);
+ spin_unlock(_device_lock);
+
+ bus->iommu_ops = ops;
+ err = bus_iommu_probe(bus);
By the way, bus_iommu_probe() has been changed in iommu-next, so it
needs to be rebased here.
Best regards,
baolu
Re: [PATCH v6 15/25] iommufd/selftest: Make the mock iommu driver into a real driver
On 2023/8/3 8:08, Jason Gunthorpe wrote:
I've avoided doing this because there is no way to make this happen
without an intrusion into the core code. Up till now this has avoided
needing the core code's probe path with some hackery - but now that
default domains are becoming mandatory it is unavoidable. The core probe
path must be run to set the default_domain, only it can do it. Without
a default domain iommufd can't use the group.
Make it so that iommufd selftest can create a real iommu driver and bind
it only to is own private bus. Add iommu_device_register_bus() as a core
code helper to make this possible. It simply sets the right pointers and
registers the notifier block. The mock driver then works like any normal
driver should, with probe triggered by the bus ops
When the bus->iommu_ops stuff is fully unwound we can probably do better
here and remove this special case.
Remove set_platform_dma_ops from selftest and make it use a BLOCKED
default domain.
Signed-off-by: Jason Gunthorpe
---
drivers/iommu/iommu-priv.h | 16 +++
drivers/iommu/iommu.c | 43 +++
drivers/iommu/iommufd/iommufd_private.h | 5 +-
drivers/iommu/iommufd/main.c| 8 +-
drivers/iommu/iommufd/selftest.c| 149 +---
5 files changed, 152 insertions(+), 69 deletions(-)
create mode 100644 drivers/iommu/iommu-priv.h
diff --git a/drivers/iommu/iommu-priv.h b/drivers/iommu/iommu-priv.h
new file mode 100644
index 00..1cbc04b9cf7297
--- /dev/null
+++ b/drivers/iommu/iommu-priv.h
@@ -0,0 +1,16 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/* Copyright (c) 2023, NVIDIA CORPORATION & AFFILIATES.
+ */
+#ifndef __IOMMU_PRIV_H
+#define __IOMMU_PRIV_H
+
+#include
+
+int iommu_device_register_bus(struct iommu_device *iommu,
+ const struct iommu_ops *ops, struct bus_type *bus,
+ struct notifier_block *nb);
+void iommu_device_unregister_bus(struct iommu_device *iommu,
+struct bus_type *bus,
+struct notifier_block *nb);
+
+#endif
diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
index a1a93990b3a211..7fae866af0db7a 100644
--- a/drivers/iommu/iommu.c
+++ b/drivers/iommu/iommu.c
@@ -36,6 +36,7 @@
#include "dma-iommu.h"
#include "iommu-sva.h"
+#include "iommu-priv.h"
static struct kset *iommu_group_kset;
static DEFINE_IDA(iommu_group_ida);
@@ -290,6 +291,48 @@ void iommu_device_unregister(struct iommu_device *iommu)
}
EXPORT_SYMBOL_GPL(iommu_device_unregister);
+#if IS_ENABLED(CONFIG_IOMMUFD_TEST)
+void iommu_device_unregister_bus(struct iommu_device *iommu,
+struct bus_type *bus,
+struct notifier_block *nb)
+{
+ bus_unregister_notifier(bus, nb);
+ iommu_device_unregister(iommu);
+}
+EXPORT_SYMBOL_GPL(iommu_device_unregister_bus);
+
+/*
+ * Register an iommu driver against a single bus. This is only used by iommufd
+ * selftest to create a mock iommu driver. The caller must provide
+ * some memory to hold a notifier_block.
+ */
+int iommu_device_register_bus(struct iommu_device *iommu,
+ const struct iommu_ops *ops, struct bus_type *bus,
+ struct notifier_block *nb)
+{
+ int err;
+
+ iommu->ops = ops;
+ nb->notifier_call = iommu_bus_notifier;
+ err = bus_register_notifier(bus, nb);
+ if (err)
+ return err;
+
+ spin_lock(_device_lock);
+ list_add_tail(>list, _device_list);
+ spin_unlock(_device_lock);
+
+ bus->iommu_ops = ops;
+ err = bus_iommu_probe(bus);
+ if (err) {
+ iommu_device_unregister_bus(iommu, bus, nb);
+ return err;
+ }
+ return 0;
+}
+EXPORT_SYMBOL_GPL(iommu_device_register_bus);
+#endif
Although
#if IS_ENABLED(CONFIG_IOMMUFD_TEST)
... ...
#endif
doesn't look so comfortable, this is also the most concise method that I
can think of.
So,
Reviewed-by: Lu Baolu
Best regards,
baolu
Re: [PATCH v6 08/25] iommu: Reorganize iommu_get_default_domain_type() to respect def_domain_type()
On 2023/8/3 8:07, Jason Gunthorpe wrote:
Except for dart every driver returns 0 or IDENTITY from def_domain_type().
The drivers that return IDENTITY have some kind of good reason, typically
that quirky hardware really can't support anything other than IDENTITY.
Arrange things so that if the driver says it needs IDENTITY then
iommu_get_default_domain_type() either fails or returns IDENTITY. It will
never reject the driver's override to IDENTITY.
The only real functional difference is that the PCI untrusted flag is now
ignored for quirky HW instead of overriding the IOMMU driver.
This makes the next patch cleaner that wants to force IDENTITY always for
ARM_IOMMU because there is no support for DMA.
Tested-by: Steven Price
Tested-by: Marek Szyprowski
Tested-by: Nicolin Chen
Reviewed-by: Lu Baolu
Signed-off-by: Jason Gunthorpe
---
drivers/iommu/iommu.c | 66 +--
1 file changed, 33 insertions(+), 33 deletions(-)
diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
index c64365169b678d..53174179102d17 100644
--- a/drivers/iommu/iommu.c
+++ b/drivers/iommu/iommu.c
@@ -1669,19 +1669,6 @@ struct iommu_group *fsl_mc_device_group(struct device
*dev)
}
EXPORT_SYMBOL_GPL(fsl_mc_device_group);
-static int iommu_get_def_domain_type(struct device *dev)
-{
- const struct iommu_ops *ops = dev_iommu_ops(dev);
-
- if (dev_is_pci(dev) && to_pci_dev(dev)->untrusted)
- return IOMMU_DOMAIN_DMA;
-
- if (ops->def_domain_type)
- return ops->def_domain_type(dev);
-
- return 0;
-}
-
static struct iommu_domain *
__iommu_group_alloc_default_domain(const struct bus_type *bus,
struct iommu_group *group, int req_type)
@@ -1775,36 +1762,49 @@ static int iommu_bus_notifier(struct notifier_block *nb,
static int iommu_get_default_domain_type(struct iommu_group *group,
int target_type)
{
+ const struct iommu_ops *ops = dev_iommu_ops(
+ list_first_entry(>devices, struct group_device, list)
+ ->dev);
How about consolidating above into a single helper?
--- a/drivers/iommu/iommu.c
+++ b/drivers/iommu/iommu.c
@@ -1787,6 +1787,21 @@ __iommu_group_alloc_default_domain(struct
iommu_group *group, int req_type)
return __iommu_group_domain_alloc(group, req_type);
}
+/*
+ * Returns the iommu_ops for the devices in an iommu group.
+ *
+ * It is assumed that all devices in an iommu group are managed by a single
+ * IOMMU unit. Therefore, this returns the dev_iommu_ops of the first
device
+ * in the group.
+ */
+static const struct iommu_ops *group_iommu_ops(struct iommu_group *group)
+{
+ struct group_device *device;
+
+ device = list_first_entry(>devices, struct group_device, list);
+ return dev_iommu_ops(device->dev);
+}
+
/*
* req_type of 0 means "auto" which means to select a domain based on
* iommu_def_domain_type or what the driver actually supports.
@@ -1794,10 +1809,7 @@ __iommu_group_alloc_default_domain(struct
iommu_group *group, int req_type)
static struct iommu_domain *
iommu_group_alloc_default_domain(struct iommu_group *group, int req_type)
{
- struct device *dev =
- list_first_entry(>devices, struct group_device, list)
- ->dev;
- const struct iommu_ops *ops = dev_iommu_ops(dev);
+ const struct iommu_ops *ops = group_iommu_ops(group);
struct iommu_domain *dom;
lockdep_assert_held(>mutex);
@@ -1888,9 +1900,7 @@ static int iommu_bus_notifier(struct
notifier_block *nb,
static int iommu_get_default_domain_type(struct iommu_group *group,
int target_type)
{
- const struct iommu_ops *ops = dev_iommu_ops(
- list_first_entry(>devices, struct group_device, list)
- ->dev);
+ const struct iommu_ops *ops = group_iommu_ops(group);
int best_type = target_type;
struct group_device *gdev;
struct device *last_dev;
@@ -2124,13 +2134,9 @@ static struct iommu_domain
*__iommu_domain_alloc(const struct iommu_ops *ops,
static struct iommu_domain *
__iommu_group_domain_alloc(struct iommu_group *group, unsigned int type)
{
- struct device *dev =
- list_first_entry(>devices, struct group_device, list)
- ->dev;
-
lockdep_assert_held(>mutex);
- return __iommu_domain_alloc(dev_iommu_ops(dev), dev, type);
+ return __iommu_domain_alloc(group_iommu_ops(group), dev, type);
}
struct iommu_domain *iommu_domain_alloc(const struct bus_type *bus)
int best_type = target_type;
struct group_device *gdev;
struct device *last_dev;
+ int type;
lockdep_assert_held(>mutex);
-
for_each_group_device(group, gdev) {
- unsigned int type = iommu_get_def_domain_type(gdev->dev);
-
-
Re: [PATCH v6 06/25] iommu/tegra-gart: Remove tegra-gart
On 2023/8/3 8:07, Jason Gunthorpe wrote: Thierry says this is not used anymore, and doesn't think it makes sense as an iommu driver. The HW it supports is about 10 years old now and newer HW uses different IOMMU drivers. As this is the only driver with a GART approach, and it doesn't really meet the driver expectations from the IOMMU core, let's just remove it so we don't have to think about how to make it fit in. It has a number of identified problems: - The assignment of iommu_groups doesn't match the HW behavior - It claims to have an UNMANAGED domain but it is really an IDENTITY domain with a translation aperture. This is inconsistent with the core expectation for security sensitive operations - It doesn't implement a SW page table under struct iommu_domain so * It can't accept a map until the domain is attached * It forgets about all maps after the domain is detached * It doesn't clear the HW of maps once the domain is detached (made worse by having the wrong groups) Cc: Thierry Reding Cc: Dmitry Osipenko Acked-by: Thierry Reding Signed-off-by: Jason Gunthorpe --- arch/arm/configs/multi_v7_defconfig | 1 - arch/arm/configs/tegra_defconfig| 1 - drivers/iommu/Kconfig | 11 - drivers/iommu/Makefile | 1 - drivers/iommu/tegra-gart.c | 371 drivers/memory/tegra/mc.c | 34 --- drivers/memory/tegra/tegra20.c | 28 --- include/soc/tegra/mc.h | 26 -- 8 files changed, 473 deletions(-) delete mode 100644 drivers/iommu/tegra-gart.c Reviewed-by: Lu Baolu Best regards, baolu
Re: [PATCH v6 02/25] iommu: Add IOMMU_DOMAIN_PLATFORM
On 2023/8/3 8:07, Jason Gunthorpe wrote:
@@ -1967,7 +1978,8 @@ void iommu_domain_free(struct iommu_domain *domain)
if (domain->type == IOMMU_DOMAIN_SVA)
mmdrop(domain->mm);
iommu_put_dma_cookie(domain);
- domain->ops->free(domain);
+ if (domain->ops->free)
+ domain->ops->free(domain);
}
EXPORT_SYMBOL_GPL(iommu_domain_free);
diff --git a/include/linux/iommu.h b/include/linux/iommu.h
index e05c93b6c37fba..87aebba474e093 100644
--- a/include/linux/iommu.h
+++ b/include/linux/iommu.h
@@ -64,6 +64,7 @@ struct iommu_domain_geometry {
#define __IOMMU_DOMAIN_DMA_FQ (1U << 3) /* DMA-API uses flush queue*/
#define __IOMMU_DOMAIN_SVA (1U << 4) /* Shared process address space */
+#define __IOMMU_DOMAIN_PLATFORM(1U << 5)
#define IOMMU_DOMAIN_ALLOC_FLAGS ~__IOMMU_DOMAIN_DMA_FQ
/*
@@ -81,6 +82,8 @@ struct iommu_domain_geometry {
* invalidation.
*IOMMU_DOMAIN_SVA- DMA addresses are shared process addresses
* represented by mm_struct's.
+ * IOMMU_DOMAIN_PLATFORM - Legacy domain for drivers that do their own
+ * dma_api stuff. Do not use in new drivers.
*/
#define IOMMU_DOMAIN_BLOCKED (0U)
#define IOMMU_DOMAIN_IDENTITY (__IOMMU_DOMAIN_PT)
@@ -91,6 +94,7 @@ struct iommu_domain_geometry {
__IOMMU_DOMAIN_DMA_API | \
__IOMMU_DOMAIN_DMA_FQ)
#define IOMMU_DOMAIN_SVA (__IOMMU_DOMAIN_SVA)
+#define IOMMU_DOMAIN_PLATFORM (__IOMMU_DOMAIN_PLATFORM)
Nit: As a default domain could be the type of IOMMU_DOMAIN_PLATFORM,
static const char *iommu_domain_type_str(unsigned int t)
needs to be updated, so that users can get a right string when reading
/sys/.../[group_id]/type.
Best regards,
baolu
Re: [PATCH v6 02/25] iommu: Add IOMMU_DOMAIN_PLATFORM
On 2023/8/3 8:07, Jason Gunthorpe wrote:
This is used when the iommu driver is taking control of the dma_ops,
currently only on S390 and power spapr. It is designed to preserve the
original ops->detach_dev() semantic that these S390 was built around.
Provide an opaque domain type and a 'default_domain' ops value that allows
the driver to trivially force any single domain as the default domain.
Reviewed-by: Lu Baolu
Signed-off-by: Jason Gunthorpe
---
drivers/iommu/iommu.c | 14 +-
include/linux/iommu.h | 6 ++
2 files changed, 19 insertions(+), 1 deletion(-)
diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
index 5e3cdc9f3a9e78..c64365169b678d 100644
--- a/drivers/iommu/iommu.c
+++ b/drivers/iommu/iommu.c
@@ -1705,6 +1705,17 @@ iommu_group_alloc_default_domain(struct iommu_group
*group, int req_type)
lockdep_assert_held(>mutex);
+ /*
+* Allow legacy drivers to specify the domain that will be the default
+* domain. This should always be either an IDENTITY or PLATFORM domain.
+* Do not use in new drivers.
+*/
+ if (bus->iommu_ops->default_domain) {
+ if (req_type)
+ return ERR_PTR(-EINVAL);
+ return bus->iommu_ops->default_domain;
+ }
+
if (req_type)
return __iommu_group_alloc_default_domain(bus, group, req_type);
@@ -1967,7 +1978,8 @@ void iommu_domain_free(struct iommu_domain *domain)
if (domain->type == IOMMU_DOMAIN_SVA)
mmdrop(domain->mm);
iommu_put_dma_cookie(domain);
- domain->ops->free(domain);
+ if (domain->ops->free)
+ domain->ops->free(domain);
}
EXPORT_SYMBOL_GPL(iommu_domain_free);
diff --git a/include/linux/iommu.h b/include/linux/iommu.h
index e05c93b6c37fba..87aebba474e093 100644
--- a/include/linux/iommu.h
+++ b/include/linux/iommu.h
@@ -64,6 +64,7 @@ struct iommu_domain_geometry {
#define __IOMMU_DOMAIN_DMA_FQ (1U << 3) /* DMA-API uses flush queue*/
#define __IOMMU_DOMAIN_SVA (1U << 4) /* Shared process address space */
+#define __IOMMU_DOMAIN_PLATFORM(1U << 5)
#define IOMMU_DOMAIN_ALLOC_FLAGS ~__IOMMU_DOMAIN_DMA_FQ
/*
@@ -81,6 +82,8 @@ struct iommu_domain_geometry {
* invalidation.
*IOMMU_DOMAIN_SVA- DMA addresses are shared process addresses
* represented by mm_struct's.
+ * IOMMU_DOMAIN_PLATFORM - Legacy domain for drivers that do their own
+ * dma_api stuff. Do not use in new drivers.
*/
#define IOMMU_DOMAIN_BLOCKED (0U)
#define IOMMU_DOMAIN_IDENTITY (__IOMMU_DOMAIN_PT)
@@ -91,6 +94,7 @@ struct iommu_domain_geometry {
__IOMMU_DOMAIN_DMA_API | \
__IOMMU_DOMAIN_DMA_FQ)
#define IOMMU_DOMAIN_SVA (__IOMMU_DOMAIN_SVA)
+#define IOMMU_DOMAIN_PLATFORM (__IOMMU_DOMAIN_PLATFORM)
struct iommu_domain {
unsigned type;
@@ -256,6 +260,7 @@ struct iommu_iotlb_gather {
* @owner: Driver module providing these ops
* @identity_domain: An always available, always attachable identity
* translation.
+ * @default_domain: If not NULL this will always be set as the default domain.
*/
struct iommu_ops {
bool (*capable)(struct device *dev, enum iommu_cap);
@@ -290,6 +295,7 @@ struct iommu_ops {
unsigned long pgsize_bitmap;
struct module *owner;
struct iommu_domain *identity_domain;
+ struct iommu_domain *default_domain;
I am imaging whether we can merge above two pointers into a single one.
It is either an IDENTITY or PLATFORM domain and the core will choose it
as the default domain of a group if iommu_group_alloc_default_domain()
fails to allocate one through the iommu dev_ops.
Those iommu drivers that could result in a NULL default domain could
provide such domain and guarantee that this domain is always usable for
devices.
Probably we could give it a more meaningful name? For example,
supplemental_domain or rescue_domain?
I am not sure whether this can address the NULL-default-domain issues
of all drivers this series tries to address. So just for discussion
purpose.
Best regards,
baolu
Re: [PATCH v5 23/25] iommu: Add ops->domain_alloc_paging()
On 2023/7/25 1:22, Jason Gunthorpe wrote: This callback requests the driver to create only a __IOMMU_DOMAIN_PAGING domain, so it saves a few lines in a lot of drivers needlessly checking the type. More critically, this allows us to sweep out all the IOMMU_DOMAIN_UNMANAGED and IOMMU_DOMAIN_DMA checks from a lot of the drivers, simplifying what is going on in the code and ultimately removing the now-unused special cases in drivers where they did not support IOMMU_DOMAIN_DMA. domain_alloc_paging() should return a struct iommu_domain that is functionally compatible with ARM_DMA_USE_IOMMU, dma-iommu.c and iommufd. Be forwards looking and pass in a 'struct device *' argument. We can provide this when allocating the default_domain. No drivers will look at this. Tested-by: Steven Price Tested-by: Marek Szyprowski Tested-by: Nicolin Chen Signed-off-by: Jason Gunthorpe --- drivers/iommu/iommu.c | 13 ++--- include/linux/iommu.h | 3 +++ 2 files changed, 13 insertions(+), 3 deletions(-) Reviewed-by: Lu Baolu
Re: [PATCH v5 22/25] iommu: Add __iommu_group_domain_alloc()
On 2023/7/25 1:22, Jason Gunthorpe wrote: Allocate a domain from a group. Automatically obtains the iommu_ops to use from the device list of the group. Convert the internal callers to use it. Tested-by: Steven Price Tested-by: Marek Szyprowski Tested-by: Nicolin Chen Signed-off-by: Jason Gunthorpe --- drivers/iommu/iommu.c | 66 --- 1 file changed, 37 insertions(+), 29 deletions(-) Reviewed-by: Lu Baolu
Re: [PATCH v5 21/25] iommu: Require a default_domain for all iommu drivers
On 2023/7/25 1:22, Jason Gunthorpe wrote: At this point every iommu driver will cause a default_domain to be selected, so we can finally remove this gap from the core code. The following table explains what each driver supports and what the resulting default_domain will be: ops->defaut_domain IDENTITY DMA PLATFORMv ARM32 dma-iommu ARCH amd/iommu.c Y Y N/A either apple-dart.cY Y N/A either arm-smmu.c Y Y IDENTITYeither qcom_iommu.cG Y IDENTITYeither arm-smmu-v3.c Y Y N/A either exynos-iommu.c G Y IDENTITYeither fsl_pamu_domain.c Y N/A N/A PLATFORM intel/iommu.c Y Y N/A either ipmmu-vmsa.cG Y IDENTITYeither msm_iommu.c G IDENTITYN/A mtk_iommu.c G Y IDENTITYeither mtk_iommu_v1.c G IDENTITYN/A omap-iommu.cG IDENTITYN/A rockchip-iommu.cG Y IDENTITYeither s390-iommu.cY Y N/A N/A PLATFORM sprd-iommu.cY N/A DMA sun50i-iommu.c G Y IDENTITYeither tegra-smmu.cG Y IDENTITYIDENTITY virtio-iommu.c Y Y N/A either spapr Y Y N/A N/A PLATFORM * G means ops->identity_domain is used * N/A means the driver will not compile in this configuration ARM32 drivers select an IDENTITY default domain through either the ops->identity_domain or directly requesting an IDENTIY domain through alloc_domain(). In ARM64 mode tegra-smmu will still block the use of dma-iommu.c and forces an IDENTITY domain. S390 uses a PLATFORM domain to represent when the dma_ops are set to the s390 iommu code. fsl_pamu uses an IDENTITY domain. POWER SPAPR uses PLATFORM and blocking to enable its weird VFIO mode. The x86 drivers continue unchanged. After this patch group->default_domain is only NULL for a short period during bus iommu probing while all the groups are constituted. Otherwise it is always !NULL. This completes changing the iommu subsystem driver contract to a system where the current iommu_domain always represents some form of translation and the driver is continuously asserting a definable translation mode. It resolves the confusion that the original ops->detach_dev() caused around what translation, exactly, is the IOMMU performing after detach. There were at least three different answers to that question in the tree, they are all now clearly named with domain types. Tested-by: Heiko Stuebner Tested-by: Niklas Schnelle Tested-by: Steven Price Tested-by: Marek Szyprowski Tested-by: Nicolin Chen Signed-off-by: Jason Gunthorpe --- drivers/iommu/iommu.c | 21 +++-- 1 file changed, 7 insertions(+), 14 deletions(-) Reviewed-by: Lu Baolu
Re: [PATCH v5 16/25] iommu: Remove ops->set_platform_dma_ops()
On 2023/7/25 1:22, Jason Gunthorpe wrote: All drivers are now using IDENTITY or PLATFORM domains for what this did, we can remove it now. It is no longer possible to attach to a NULL domain. Tested-by: Heiko Stuebner Tested-by: Niklas Schnelle Tested-by: Steven Price Tested-by: Marek Szyprowski Tested-by: Nicolin Chen Signed-off-by: Jason Gunthorpe --- drivers/iommu/iommu.c | 30 +- include/linux/iommu.h | 4 2 files changed, 5 insertions(+), 29 deletions(-) Reviewed-by: Lu Baolu
Re: [PATCH v5 08/25] iommu: Reorganize iommu_get_default_domain_type() to respect def_domain_type()
On 2023/7/25 1:21, Jason Gunthorpe wrote: Except for dart every driver returns 0 or IDENTITY from def_domain_type(). The drivers that return IDENTITY have some kind of good reason, typically that quirky hardware really can't support anything other than IDENTITY. Arrange things so that if the driver says it needs IDENTITY then iommu_get_default_domain_type() either fails or returns IDENTITY. It will never reject the driver's override to IDENTITY. The only real functional difference is that the PCI untrusted flag is now ignored for quirky HW instead of overriding the IOMMU driver. This makes the next patch cleaner that wants to force IDENTITY always for ARM_IOMMU because there is no support for DMA. Tested-by: Steven Price Tested-by: Marek Szyprowski Tested-by: Nicolin Chen Signed-off-by: Jason Gunthorpe --- drivers/iommu/iommu.c | 66 +-- 1 file changed, 33 insertions(+), 33 deletions(-) Reviewed-by: Lu Baolu
Re: [PATCH v5 02/25] iommu: Add IOMMU_DOMAIN_PLATFORM
On 2023/7/25 1:21, Jason Gunthorpe wrote: This is used when the iommu driver is taking control of the dma_ops, currently only on S390 and power spapr. It is designed to preserve the original ops->detach_dev() semantic that these S390 was built around. Provide an opaque domain type and a 'default_domain' ops value that allows the driver to trivially force any single domain as the default domain. Signed-off-by: Jason Gunthorpe --- drivers/iommu/iommu.c | 14 +- include/linux/iommu.h | 6 ++ 2 files changed, 19 insertions(+), 1 deletion(-) Reviewed-by: Lu Baolu
Re: [PATCH v5 01/25] iommu: Add iommu_ops->identity_domain
On 2023/7/25 1:21, Jason Gunthorpe wrote: This allows a driver to set a global static to an IDENTITY domain and the core code will automatically use it whenever an IDENTITY domain is requested. By making it always available it means the IDENTITY can be used in error handling paths to force the iommu driver into a known state. Devices implementing global static identity domains should avoid failing their attach_dev ops. Convert rockchip to use the new mechanism. Tested-by: Steven Price Tested-by: Marek Szyprowski Tested-by: Nicolin Chen Signed-off-by: Jason Gunthorpe --- drivers/iommu/iommu.c | 3 +++ drivers/iommu/rockchip-iommu.c | 9 + include/linux/iommu.h | 3 +++ 3 files changed, 7 insertions(+), 8 deletions(-) I will later convert the VT-d driver to use iommu_ops->identity_domain. Reviewed-by: Lu Baolu
Re: [PATCH v3 23/25] iommu: Add ops->domain_alloc_paging()
On 6/10/23 3:56 AM, Jason Gunthorpe wrote:
This callback requests the driver to create only a __IOMMU_DOMAIN_PAGING
domain, so it saves a few lines in a lot of drivers needlessly checking
the type.
More critically, this allows us to sweep out all the
IOMMU_DOMAIN_UNMANAGED and IOMMU_DOMAIN_DMA checks from a lot of the
drivers, simplifying what is going on in the code and ultimately removing
the now-unused special cases in drivers where they did not support
IOMMU_DOMAIN_DMA.
domain_alloc_paging() should return a struct iommu_domain that is
functionally compatible with ARM_DMA_USE_IOMMU, dma-iommu.c and iommufd.
Be forwards looking and pass in a 'struct device *' argument. We can
provide this when allocating the default_domain. No drivers will look at
this.
I like this idea. :-)
Tested-by: Steven Price
Tested-by: Marek Szyprowski
Tested-by: Nicolin Chen
Signed-off-by: Jason Gunthorpe
---
drivers/iommu/iommu.c | 13 ++---
include/linux/iommu.h | 3 +++
2 files changed, 13 insertions(+), 3 deletions(-)
diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
index 0346c05e108438..2cf523ff9c6f55 100644
--- a/drivers/iommu/iommu.c
+++ b/drivers/iommu/iommu.c
@@ -1985,6 +1985,7 @@ void iommu_set_fault_handler(struct iommu_domain *domain,
EXPORT_SYMBOL_GPL(iommu_set_fault_handler);
static struct iommu_domain *__iommu_domain_alloc(const struct iommu_ops *ops,
+struct device *dev,
unsigned int type)
{
struct iommu_domain *domain;
@@ -1992,8 +1993,13 @@ static struct iommu_domain *__iommu_domain_alloc(const
struct iommu_ops *ops,
if (alloc_type == IOMMU_DOMAIN_IDENTITY && ops->identity_domain)
return ops->identity_domain;
+ else if (type & __IOMMU_DOMAIN_PAGING) {
+ domain = ops->domain_alloc_paging(dev);
This might be problematic because not all IOMMU drivers implement this
callback now. In the missing cases, the code will always result in a
null pointer reference issue?
+ } else if (ops->domain_alloc)
+ domain = ops->domain_alloc(alloc_type);
+ else
+ return NULL;
- domain = ops->domain_alloc(alloc_type);
if (!domain)
return NULL;
@@ -2024,14 +2030,15 @@ __iommu_group_domain_alloc(struct iommu_group *group, unsigned int type)
lockdep_assert_held(>mutex);
- return __iommu_domain_alloc(dev_iommu_ops(dev), type);
+ return __iommu_domain_alloc(dev_iommu_ops(dev), dev, type);
}
struct iommu_domain *iommu_domain_alloc(const struct bus_type *bus)
{
if (bus == NULL || bus->iommu_ops == NULL)
return NULL;
- return __iommu_domain_alloc(bus->iommu_ops, IOMMU_DOMAIN_UNMANAGED);
+ return __iommu_domain_alloc(bus->iommu_ops, NULL,
+ IOMMU_DOMAIN_UNMANAGED);
Suppose that iommu_domain_alloc() is always called from device drivers
where device pointer is always available. Is it possible to convert it
to a real device pointer?
}
EXPORT_SYMBOL_GPL(iommu_domain_alloc);
diff --git a/include/linux/iommu.h b/include/linux/iommu.h
index 49331573f1d1f5..8e4d178c49c417 100644
--- a/include/linux/iommu.h
+++ b/include/linux/iommu.h
@@ -233,6 +233,8 @@ struct iommu_iotlb_gather {
* struct iommu_ops - iommu ops and capabilities
* @capable: check capability
* @domain_alloc: allocate iommu domain
+ * @domain_alloc_paging: Allocate an iommu_domain that can be used for
+ * UNMANAGED, DMA, and DMA_FQ domain types.
* @probe_device: Add device to iommu driver handling
* @release_device: Remove device from iommu driver handling
* @probe_finalize: Do final setup work after the device is added to an IOMMU
@@ -264,6 +266,7 @@ struct iommu_ops {
/* Domain allocation and freeing by the iommu driver */
struct iommu_domain *(*domain_alloc)(unsigned iommu_domain_type);
+ struct iommu_domain *(*domain_alloc_paging)(struct device *dev);
struct iommu_device *(*probe_device)(struct device *dev);
void (*release_device)(struct device *dev);
Best regards,
baolu
Re: [PATCH 10/11] iommu: Split iommu_group_add_device()
On 4/20/23 12:11 AM, Jason Gunthorpe wrote:
@@ -451,16 +454,17 @@ static int __iommu_probe_device(struct device *dev,
struct list_head *group_list
goto out_unlock;
group = dev->iommu_group;
- ret = iommu_group_add_device(group, dev);
+ gdev = iommu_group_alloc_device(group, dev);
mutex_lock(>mutex);
- if (ret)
+ if (IS_ERR(gdev)) {
+ ret = PTR_ERR(gdev);
goto err_put_group;
+ }
+ list_add_tail(>list, >devices);
Do we need to put
dev->iommu_group = group;
here?
if (group_list && !group->default_domain && list_empty(>entry))
list_add_tail(>entry, group_list);
mutex_unlock(>mutex);
- iommu_group_put(group);
-
mutex_unlock(_probe_device_lock);
return 0;
Best regards,
baolu
Re: [PATCH 08/11] iommu: Always destroy the iommu_group during iommu_release_device()
On 4/20/23 12:11 AM, Jason Gunthorpe wrote: diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c index dbaf3ed9012c45..a82516c8ea87ad 100644 --- a/drivers/iommu/iommu.c +++ b/drivers/iommu/iommu.c @@ -569,7 +569,6 @@ static void __iommu_group_remove_device(struct device *dev) dev->iommu_group = NULL; goto out; Nit, given that below line has been removed, can above simply be a loop break? } - WARN(true, "Corrupted iommu_group device_list"); out: mutex_unlock(>mutex); Best regards, baolu
