Messages in this thread Patch in this message | | | Subject | Re: [PATCH v14 6/6] iommu: Remove mode argument from iommu_set_dma_strict() | From | John Garry <> | Date | Mon, 21 Jun 2021 11:34:25 +0100 |
| |
On 21/06/2021 11:00, Lu Baolu wrote: >> void iommu_set_dma_strict(bool force) >> { >> if (force == true) >> iommu_dma_strict = true; >> else if (!(iommu_cmd_line & IOMMU_CMD_LINE_STRICT)) >> iommu_dma_strict = true; >> } >> >> So we would use iommu_set_dma_strict(true) for a) and b), but >> iommu_set_dma_strict(false) for c). > > Yes. We need to distinguish the "must" and "nice-to-have" cases of > setting strict mode. > >> >> Then I am not sure what you want to do with the accompanying print for >> c). It was: >> "IOMMU batching is disabled due to virtualization" >> >> And now is from this series: >> "IOMMU batching disallowed due to virtualization" >> >> Using iommu_get_dma_strict(domain) is not appropriate here to know the >> current mode (so we know whether to print). >> >> Note that this change would mean that the current series would require >> non-trivial rework, which would be unfortunate so late in the cycle. > > This patch series looks good to me and I have added by reviewed-by. > Probably we could make another patch series to improve it so that the > kernel optimization should not override the user setting.
On a personal level I would be happy with that approach, but I think it's better to not start changing things right away in a follow-up series.
So how about we add this patch (which replaces 6/6 "iommu: Remove mode argument from iommu_set_dma_strict()")?
Robin, any opinion?
------->8---------
[PATCH] iommu/vt-d: Make "iommu.strict" override batching due to virtualization
As a change in policy, make iommu.strict cmdline argument override whether we disable batching due to virtualization.
The API of iommu_set_dma_strict() is changed to accept a "force" argument, which means that we always set iommu_dma_strict true, regardless of whether we already set via cmdline. Also return a boolean, to tell whether iommu_dma_strict was set or not.
Note that in all pre-existing callsites of iommu_set_dma_strict(), argument strict was true, so this argument is dropped.
Signed-off-by: John Garry <john.garry@huawei.com>
diff --git a/drivers/iommu/intel/iommu.c b/drivers/iommu/intel/iommu.c index 06666f9d8116..e8d65239b359 100644 --- a/drivers/iommu/intel/iommu.c +++ b/drivers/iommu/intel/iommu.c @@ -4380,10 +4380,8 @@ int __init intel_iommu_init(void) * is likely to be much lower than the overhead of synchronizing * the virtual and physical IOMMU page-tables. */ - if (cap_caching_mode(iommu->cap)) { + if (cap_caching_mode(iommu->cap) && iommu_set_dma_strict(false)) pr_info_once("IOMMU batching disallowed due to virtualization\n"); - iommu_set_dma_strict(true); - } iommu_device_sysfs_add(&iommu->iommu, NULL, intel_iommu_groups, "%s", iommu->name); diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c index 60b1ec42e73b..1434bee64af3 100644 --- a/drivers/iommu/iommu.c +++ b/drivers/iommu/iommu.c @@ -349,10 +349,14 @@ static int __init iommu_dma_setup(char *str) } early_param("iommu.strict", iommu_dma_setup);
-void iommu_set_dma_strict(bool strict) +/* Return true if we set iommu_dma_strict */ +bool iommu_set_dma_strict(bool force) { - if (strict || !(iommu_cmd_line & IOMMU_CMD_LINE_STRICT)) - iommu_dma_strict = strict; + if (force || !(iommu_cmd_line & IOMMU_CMD_LINE_STRICT)) { + iommu_dma_strict = true; + return true; + } + return false; }
bool iommu_get_dma_strict(struct iommu_domain *domain) diff --git a/include/linux/iommu.h b/include/linux/iommu.h index 32d448050bf7..f17b20234296 100644 --- a/include/linux/iommu.h +++ b/include/linux/iommu.h @@ -476,7 +476,7 @@ int iommu_enable_nesting(struct iommu_domain *domain); int iommu_set_pgtable_quirks(struct iommu_domain *domain, unsigned long quirks);
-void iommu_set_dma_strict(bool val); +bool iommu_set_dma_strict(bool force); bool iommu_get_dma_strict(struct iommu_domain *domain);
extern int report_iommu_fault(struct iommu_domain *domain, struct device *dev, -- 2.26.2
| |