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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <58ea001c-4c37-9c5a-77be-38ac3bea2476@arm.com>
Date:   Tue, 1 Jun 2021 17:42:27 +0100
From:   Robin Murphy <robin.murphy@....com>
To:     John Garry <john.garry@...wei.com>, joro@...tes.org,
        will@...nel.org
Cc:     iommu@...ts.linux-foundation.org, linux-kernel@...r.kernel.org,
        hch@....de
Subject: Re: [PATCH] iommu: Print default strict or lazy mode at init time

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.