From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 3590CC32772 for ; Tue, 23 Aug 2022 12:40:48 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender: Content-Transfer-Encoding:Content-Type:List-Subscribe:List-Help:List-Post: List-Archive:List-Unsubscribe:List-Id:In-Reply-To:MIME-Version:References: Message-ID:Subject:Cc:To:From:Date:Reply-To:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=g4B06Ttk7evjDj/FUVJFJUtm+pxJz4AmhASpV6pfku8=; b=EOrlfowXprArO1 5JC4xJkI3ctBrcIt7D7HSQQcyFwCHIUMFzlpbq6XFwhNoKxKrShpJaqWELlfkcMYjuCGVcq1icHRe Lef8psaWGYcO2ElqJ8xa4A1owNKnajBeH79esJl+ZCrGhLxDYqFNIJa1Qgr0qtZ0IKEL0Ad3aVwUT 2sIQcWn5MW2ozVJaVquq2UyDVZNpOYF0Vd8+7zv5TbSxm0G6lsgV0pbc5igF0uVFFi01aqIxWX+tg fW6wMOI2lG6mn701Z/m5bR71ob/9sLtC9MmngYUHZ/kKx3GC7dGtX/jjKAOPXm9Bauo0iCSZ1jK5C sa37D0/Fx36BLV0IbTwA==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.94.2 #2 (Red Hat Linux)) id 1oQTBO-005wfO-Ee; Tue, 23 Aug 2022 12:39:04 +0000 Received: from us-smtp-delivery-124.mimecast.com ([170.10.133.124]) by bombadil.infradead.org with esmtps (Exim 4.94.2 #2 (Red Hat Linux)) id 1oQT5P-005u3L-Jd for linux-arm-kernel@lists.infradead.org; Tue, 23 Aug 2022 12:33:05 +0000 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1661257970; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=JFZpUrH9edUOuu2AArKyngbcz1gI3F8tDFjZRMkRQFs=; b=Ui+eZfqHhvNr5Nv3x+b5NMJVXJCILL/clpbEKfGwsdFqA6TiPc5zWbyvMK/4DJsR177nyz Rc1KAEiwygrI8mUk6lRCtZiq06lFa5FUlS/pn1m1IRlzq1uqp0YYKQxUuY6pmyByXebRnR qy+kSNpYyDMQoByBtrKwN4INyS3GuOo= Received: from mimecast-mx02.redhat.com (mimecast-mx02.redhat.com [66.187.233.88]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id us-mta-470-cQTcaUiDMyaszofSEUWFVQ-1; Tue, 23 Aug 2022 08:32:47 -0400 X-MC-Unique: cQTcaUiDMyaszofSEUWFVQ-1 Received: from smtp.corp.redhat.com (int-mx02.intmail.prod.int.rdu2.redhat.com [10.11.54.2]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx02.redhat.com (Postfix) with ESMTPS id 4C03918E5341; Tue, 23 Aug 2022 12:32:47 +0000 (UTC) Received: from localhost (ovpn-12-31.pek2.redhat.com [10.72.12.31]) by smtp.corp.redhat.com (Postfix) with ESMTPS id F022940C141D; Tue, 23 Aug 2022 12:32:45 +0000 (UTC) Date: Tue, 23 Aug 2022 20:32:42 +0800 From: Baoquan He To: Christophe Leroy Cc: "linux-kernel@vger.kernel.org" , "linux-mm@kvack.org" , "akpm@linux-foundation.org" , "hch@infradead.org" , "agordeev@linux.ibm.com" , "wangkefeng.wang@huawei.com" , "linux-arm-kernel@lists.infradead.org" Subject: Re: [PATCH v2 02/11] mm: ioremap: fixup the physical address and page prot Message-ID: References: <20220820003125.353570-1-bhe@redhat.com> <20220820003125.353570-3-bhe@redhat.com> <54b7afcc-056d-7f33-6858-d451a3222c70@csgroup.eu> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <54b7afcc-056d-7f33-6858-d451a3222c70@csgroup.eu> X-Scanned-By: MIMEDefang 2.84 on 10.11.54.2 X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20220823_053303_989474_279D3BAD X-CRM114-Status: GOOD ( 27.05 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: quoted-printable Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org On 08/23/22 at 05:33am, Christophe Leroy wrote: > = > = > Le 23/08/2022 =E0 03:19, Baoquan He a =E9crit=A0: > > On 08/22/22 at 06:30am, Christophe Leroy wrote: > >> > >> > >> Le 20/08/2022 =E0 02:31, Baoquan He a =E9crit=A0: > >>> On some architectures, the physical address need be fixed up before > >>> doing mapping, e.g, parisc. And on architectures, e.g arc, the > >>> parameter 'prot' passed into ioremap_prot() need be adjusted too. > >>> > >>> In oder to convert them to take GENERIC_IOREMAP method, we need wrap > >>> the address fixing up code and page prot adjusting code into arch_ior= emap() > >>> and pass the new address and 'prot' out for ioremap_prot() handling. > >> > >> Is it really the best approach ? Wouldn't it be better to have helpers > >> to do that, those helpers being called by the ioremap_prot(), instead = of > >> doing it inside the arch_ioremap() function ? > > = > > This is suggested too by Alexander during his v1 reviewing. I tried, but > > feel the current way taken in this patchset is better. Because not all > > architecutres need the address fix up, only parisc, and only few need > > adjust the 'prot'. Introducing other helpers seems too much, that only > > increases the complexity of of ioremap() and the generic GENERIC_IOREMAP > > method for people to understand and take. > = > I can't understand. Why is it difficult to do something like: > = > #ifndef ioremap_adjust_prot > static inline unsigned long ioremap_adjust_prot(unsigned long flags) > { > return flags; > } > #endif > = > Then for arc you do > = > static inline unsigned long ioremap_adjust_prot(unsigned long flags) > { > return pgprot_val(pgprot_noncached(__pgprot(flags))); > } > #define ioremap_adjust_prot ioremap_adjust_prot My thinking is we have four things to do in the added hookers. 1) check if we should do ioremap on ARCHes. If not, return NULL from ioremap_prot(); 2) handling the mapping io address specifically on ARCHes, e.g arc, ia64, s390; 3) the original physical address passed into ioremap_prot() need be fixed up, e.g arc; 4) the 'prot' passed into ioremap_prot() need be adjusted, e.g on arc and xtensa. With Kefeng's patches, the case 1) is handled with introduced ioremap_allowed()/iounmap_allowed(). In this patchset, what I do is rename the hooks as arch_ioremap() and arch_iounmap(), then put case 1), 2), 3), 4) handling into arch_ioremap(). Adding helpers to cover each case is not difficult from my side. I worry that as time goes by, those several hooks my cause issue, e.g if a new adjustment need be done, should we introduce a new helper or make do with the existed hook; how = When I investigated this, one arch_ioremap() looks not complicated = since not all ARCHes need cover all above 4 cases. That's why I finally choose one hook. I am open to new idea, please let me know if we should change it to introduce several different helpers. > = > = > By the way, could be a good opportunity to change ioremap_prot() flags = > type from unsigned long to pgprot_t Tend to agree, I will give it a shot. _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel