Re: [PATCH] iommu: Print default strict or lazy mode at init time

[Robin Murphy <robin.murphy@arm.com>]

On 2021-06-01 16:50, John Garry wrote:
> On 01/06/2021 10:09, Robin Murphy wrote:
>>> diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
>>> index 808ab70d5df5..f25fae62f077 100644
>>> --- a/drivers/iommu/iommu.c
>>> +++ b/drivers/iommu/iommu.c
>>> @@ -138,6 +138,11 @@ static int __init iommu_subsys_init(void)
>>>           (iommu_cmd_line & IOMMU_CMD_LINE_DMA_API) ?
>>>               "(set via kernel command line)" : "");
>>> +    pr_info("Default DMA domain mode: %s %s\n",
>>
>> Nit: I think this might be a little unclear for end-users - *I'm* not 
>> even sure whether "Default" here is meant to refer to the mode setting 
>> itself or to default domains (of DMA type). Maybe something like "DMA 
>> domain TLB invalidation policy"? Certainly it seems like a good idea 
>> to explicitly mention invalidation to correlate with the documentation 
>> of the "iommu.strict" parameter.
>>
>> Ack to the general idea though.
> 
> ok, so I'll go with this:
> 
> pr_info("DMA domain default TLB invalidation policy: %s mode %s\n",
>                iommu_dma_strict ? "strict" : "lazy",
>                 (iommu_cmd_line & IOMMU_CMD_LINE_STRICT) ?
>                         "(set via kernel command line)" : "");
> 
> I think it's worth mentioning "default" somewhere, as not all IOMMUs or 
> devices will use lazy mode even if it's default.

But that's part of what I think is misleading - I boot and see that the 
default is something, so I reboot with iommu.strict to explicitly set it 
the other way, but now that's the default... huh?

The way I see it, we're saying what the current IOMMU API policy is - 
the value of iommu_dma_strict at any given time is fact - but we're not 
necessarily saying how widely that policy is enforced. We similarly 
report the type for default domains from global policy even though that 
may also be overridden per-group by drivers and/or userspace later; we 
don't say it's the *default* default domain type.

However, having now debugged the AMD issue from another thread, I think 
doing this at subsys_initcall is in fact going to be too early to be 
meaningful, since it ignores drivers' ability to change the global policy :(

Robin.