On 2021/6/21 16:12, John Garry wrote:
On 21/06/2021 06:17, Lu Baolu wrote:
On 2021/6/18 19:34, John Garry wrote:
diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
index 60b1ec42e73b..ff221d3ddcbc 100644
--- a/drivers/iommu/iommu.c
+++ b/drivers/iommu/iommu.c
@@ -349,10 +349,9 @@ static int __init iommu_dma_setup(char *str)
}
early_param("iommu.strict", iommu_dma_setup);
-void iommu_set_dma_strict(bool strict)
+void iommu_set_dma_strict(void)
{
- if (strict || !(iommu_cmd_line & IOMMU_CMD_LINE_STRICT))
- iommu_dma_strict = strict;
+ iommu_dma_strict = true;
}
Hi baolu,
Hi John,
Sorry for this late comment.
> Normally the cache invalidation policy should come from the user. We
have pre-build kernel option and also a kernel boot command iommu.strict
to override it. These seem reasonable.
We also have a helper (iommu_set_dma_strict()) so that the vendor iommu
driver could squeeze in and change the previous settings mostly due to:
a) vendor iommu driver specific kernel boot command. (We are about to
deprecate those.)
b) quirky hardware.
c) kernel optimization (e.x. strict mode in VM environment).
a) and b) are mandatory, while c) is optional. In any instance should c)
override the flush mode specified by the user. Hence, probably we should
also have another helper like:
void iommu_set_dma_strict_optional()
{
if (!(iommu_cmd_line & IOMMU_CMD_LINE_STRICT))
iommu_dma_strict = true;
}
Any thoughts?
What you are suggesting is a change in policy from mainline code.
Currently for c) we always set strict enabled, regardless of any user
cmdline input. But now you are saying that you want iommu.strict to
override in particular scenario, right?
In that case I would think it's better to rework the current API, like
adding an option to "force" strict mode:
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.
Best regards,
baolu
