linux-kernel - Re: [PATCH v2 01/11] mm/ioremap: change the return value of io[re|un]map

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: <429cb367-923f-bb3d-ccf0-57dce0c7f35b@csgroup.eu>
Date:   Tue, 23 Aug 2022 15:26:07 +0000
From:   Christophe Leroy <christophe.leroy@...roup.eu>
To:     Baoquan He <bhe@...hat.com>
CC:     "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "linux-mm@...ck.org" <linux-mm@...ck.org>,
        "akpm@...ux-foundation.org" <akpm@...ux-foundation.org>,
        "hch@...radead.org" <hch@...radead.org>,
        "agordeev@...ux.ibm.com" <agordeev@...ux.ibm.com>,
        "wangkefeng.wang@...wei.com" <wangkefeng.wang@...wei.com>,
        "linux-arm-kernel@...ts.infradead.org" 
        <linux-arm-kernel@...ts.infradead.org>
Subject: Re: [PATCH v2 01/11] mm/ioremap: change the return value of
 io[re|un]map_allowed and rename



Le 23/08/2022 à 17:14, Baoquan He a écrit :
> On 08/23/22 at 05:24am, Christophe Leroy wrote:
>>
>>
>> Le 23/08/2022 à 02:20, Baoquan He a écrit :
>>> On 08/22/22 at 06:25am, Christophe Leroy wrote:
>>>>
>>>>
>>>> Le 20/08/2022 à 02:31, Baoquan He a écrit :
>>>>> In some architectures, there are ARCH specifici io address mapping
>>>>> handling when calling ioremap() or ioremap_prot(), e.g, arc, ia64,
>>>>> openrisc, s390, sh.
>>>>>
>>>>> In oder to convert them to take GENERIC_IOREMAP method, we need change
>>>>> the return value of hook ioremap_allowed() and iounmap_allowed().
>>>>> Meanwhile, rename them to arch_ioremap() and arch_iounmap() to reflect
>>>>> their current behaviour.
>>>
>>> Thanks for reviewing.
>>>
>>>>
>>>> Please don't just say you need to change the return value. Explain why.
>>>
>>> The 1st paragraph and the sentence 'In oder to convert them to take
>>> GENERIC_IOREMAP method' tell the reason, no?
>>
>> What I would like to read is _why_ you need to change the return value
>> in order to convert to GENERIC_IOREMAP
> 
> I rephrase the log as below, it's OK to you? Or please help check and
> tell what I need to improve to better explain the reason.
> 
> ====
> The current io[re|un]map_allowed() hooks are used to check if the
> io[re|un]map() actions are qualified to proceed when taking
> GENERIC_IOREMAP way to do ioremap()/iounmap(). Otherwise io[re|un]map()
> will return NULL.
> 
> On some architectures like arc, ia64, openris, s390, sh, there are
> ARCH specific io address mapping to translate the passed in physical
> address to io address when calling ioremap(). In order to convert
> these architectures to take GENERIC_IOREMAP way to ioremap(), we need
> change the return value of hook ioremap_allowed() and iounmap_allowed().
> With the change, we can move the architecture specific io address
> mapping into ioremap_allowed() hook, and give the mapped io address
> out to let ioremap_prot() return it. While at it, rename the hooks to
> arch_ioremap() and arch_iounmap() to reflect their new behaviour.
> ====
> 

That looks more in line with the type of explanation I foresee in the 
commit message, thanks.


Christophe