linux-kernel - Re: [PATCH v5 1/6] iommu: add generic boot option iommu.dma

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]

Date:   Wed, 17 Apr 2019 10:36:58 +0800
From:   "Leizhen (ThunderTown)" <thunder.leizhen@...wei.com>
To:     Will Deacon <will.deacon@....com>,
        Robin Murphy <robin.murphy@....com>
CC:     John Garry <john.garry@...wei.com>,
        Jean-Philippe Brucker <jean-philippe.brucker@....com>,
        Joerg Roedel <joro@...tes.org>,
        "Jonathan Corbet" <corbet@....net>,
        linux-doc <linux-doc@...r.kernel.org>,
        Sebastian Ott <sebott@...ux.ibm.com>,
        Gerald Schaefer <gerald.schaefer@...ibm.com>,
        "Martin Schwidefsky" <schwidefsky@...ibm.com>,
        Heiko Carstens <heiko.carstens@...ibm.com>,
        Benjamin Herrenschmidt <benh@...nel.crashing.org>,
        Paul Mackerras <paulus@...ba.org>,
        "Michael Ellerman" <mpe@...erman.id.au>,
        Tony Luck <tony.luck@...el.com>,
        Fenghua Yu <fenghua.yu@...el.com>,
        Thomas Gleixner <tglx@...utronix.de>,
        Ingo Molnar <mingo@...hat.com>, Borislav Petkov <bp@...en8.de>,
        "H . Peter Anvin" <hpa@...or.com>,
        David Woodhouse <dwmw2@...radead.org>,
        iommu <iommu@...ts.linux-foundation.org>,
        linux-kernel <linux-kernel@...r.kernel.org>,
        linux-s390 <linux-s390@...r.kernel.org>,
        linuxppc-dev <linuxppc-dev@...ts.ozlabs.org>,
        x86 <x86@...nel.org>, linux-ia64 <linux-ia64@...r.kernel.org>,
        Hanjun Guo <guohanjun@...wei.com>
Subject: Re: [PATCH v5 1/6] iommu: add generic boot option iommu.dma_mode



On 2019/4/16 23:21, Will Deacon wrote:
> On Fri, Apr 12, 2019 at 02:11:31PM +0100, Robin Murphy wrote:
>> On 12/04/2019 11:26, John Garry wrote:
>>> On 09/04/2019 13:53, Zhen Lei wrote:
>>>> +static int __init iommu_dma_mode_setup(char *str)
>>>> +{
>>>> +    if (!str)
>>>> +        goto fail;
>>>> +
>>>> +    if (!strncmp(str, "passthrough", 11))
>>>> +        iommu_default_dma_mode = IOMMU_DMA_MODE_PASSTHROUGH;
>>>> +    else if (!strncmp(str, "lazy", 4))
>>>> +        iommu_default_dma_mode = IOMMU_DMA_MODE_LAZY;
>>>> +    else if (!strncmp(str, "strict", 6))
>>>> +        iommu_default_dma_mode = IOMMU_DMA_MODE_STRICT;
>>>> +    else
>>>> +        goto fail;
>>>> +
>>>> +    pr_info("Force dma mode to be %d\n", iommu_default_dma_mode);
>>>
>>> What happens if the cmdline option iommu.dma_mode is passed multiple
>>> times? We get mutliple - possibily conflicting - prints, right?
>>
>> Indeed; we ended up removing such prints for the existing options here,
>> specifically because multiple messages seemed more likely to be confusing
>> than useful.

I originally intended to be compatible with X86 printing.

 		} else if (!strncmp(str, "strict", 6)) {
 			pr_info("Disable batched IOTLB flush\n");
			intel_iommu_strict = 1;
 		}

>>
>>> And do we need to have backwards compatibility, such that the setting
>>> for iommu.strict or iommu.passthrough trumps iommu.dma_mode, regardless
>>> of order?
>>
>> As above I think it would be preferable to just keep using the existing
>> options anyway. The current behaviour works out as:
>>
>> iommu.passthrough |      Y	|	  N
>> iommu.strict	  |      x	|    Y         N
>> ------------------|-------------|---------|--------
>> MODE		  | PASSTHROUGH | STRICT  |  LAZY
>>
>> which seems intuitive enough that a specific dma_mode option doesn't add
>> much value, and would more likely just overcomplicate things for users as
>> well as our implementation.
> 
> Agreed. We can't remove the existing options, and they do the job perfectly
> well so I don't see the need to add more options on top.

OK, I will remove the iommu.dma_mode option in the next version. Thanks for you three.

I didn't want to add it at first, but later found that the boot options on
each ARCH are different, then want to normalize it.

In addition, do we need to compatible the build option name IOMMU_DEFAULT_PASSTHROUGH? or
change it to IOMMU_DEFAULT_DMA_MODE_PASSTHROUGH or IOMMU_DEFAULT_MODE_PASSTHROUGH?

> 
> Will
> 
> .
> 

-- 
Thanks!
BestRegards