Re: [PATCH v14 6/6] iommu: Remove mode argument from iommu_set_dma_strict()

[Lu Baolu]

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;
>   }

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?

Best regards,
baolu