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

From: Robin Murphy <robin.murphy@arm.com>
To: John Garry <john.garry@huawei.com>, joro@8bytes.org, will@kernel.org
Cc: iommu@lists.linux-foundation.org, linux-kernel@vger.kernel.org,
	hch@lst.de
Subject: Re: [PATCH] iommu: Print default strict or lazy mode at init time
Date: Tue, 1 Jun 2021 17:42:27 +0100	[thread overview]
Message-ID: <58ea001c-4c37-9c5a-77be-38ac3bea2476@arm.com> (raw)
In-Reply-To: <e478aea8-54d3-bfc6-1bc8-b0c7709e353a@huawei.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.
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu