From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from fhigh-a2-smtp.messagingengine.com (fhigh-a2-smtp.messagingengine.com [103.168.172.153]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id B16B5200CB for ; Mon, 24 Mar 2025 08:43:00 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=103.168.172.153 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1742805782; cv=none; b=Wl9SedIHfqNgZvsU/Rm6C97W3bJv+yGN9upagjuRxj8N3h0jf154RU6E1dqlIPKsznWBVLaGTwoLhi9U9YEK/lv/VCfH153EAgXveuiVdhviPCTBfDu703MLTh5zoH5y6PrXelxmmoe/mSqlT9d+B+o6jClY2r4R8lW+wvFhWyo= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1742805782; c=relaxed/simple; bh=ycOg5IKMHPKpgJbedMyST9/3B0t5g8eFieS6PBgUfn8=; h=MIME-Version:Date:From:To:Cc:Message-Id:In-Reply-To:References: Subject:Content-Type; b=uKM7SaznO7kRJe53pglWZ3rbcmV8FuvaXVYYoBXuFLkZQ5XmNhFIoznw1SVLcL+L0a7mbW41g+7v6syoRMEFnzC4PMNRDKnizggqmRcCy09TU/x4THjR+Tfe4BjB/QCLYmr/XtZhb62aA9rKLitRKol0dGIfAhxSLaxLvYeSHtg= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=arndb.de; spf=pass smtp.mailfrom=arndb.de; dkim=pass (2048-bit key) header.d=arndb.de header.i=@arndb.de header.b=eMJ3g99Y; dkim=pass (2048-bit key) header.d=messagingengine.com header.i=@messagingengine.com header.b=BSfO6oxA; arc=none smtp.client-ip=103.168.172.153 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=arndb.de Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=arndb.de Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=arndb.de header.i=@arndb.de header.b="eMJ3g99Y"; dkim=pass (2048-bit key) header.d=messagingengine.com header.i=@messagingengine.com header.b="BSfO6oxA" Received: from phl-compute-07.internal (phl-compute-07.phl.internal [10.202.2.47]) by mailfhigh.phl.internal (Postfix) with ESMTP id BC9A211400C4; Mon, 24 Mar 2025 04:42:59 -0400 (EDT) Received: from phl-imap-11 ([10.202.2.101]) by phl-compute-07.internal (MEProxy); Mon, 24 Mar 2025 04:42:59 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=arndb.de; h=cc :cc:content-transfer-encoding:content-type:content-type:date :date:from:from:in-reply-to:in-reply-to:message-id:mime-version :references:reply-to:subject:subject:to:to; s=fm1; t=1742805779; x=1742892179; bh=ooYuA0uM53YxnENt+atSUcEUD65s+aUhDXD6jeflgG8=; b= eMJ3g99Yzd9c4BoOVLfrKG/FuaQnaGtQaOKsWzqgQUJYektzgB4mNYbHM5hPL/Y2 yMNIGARDPEK9gtWnk6b/1Wd6GUQvcLhbjcRedyCLpBXz+M3pkgKZtg3fW2/N26Oq CwTHAZ5Q2Oe5XRtx6pzRFDidKQJvHMrx+nddzru9/VO6CoaPXkenn4fCSMav3ZYf ZkE6qtAFGaEgQkNFfi+8XWBYEC+5zYFGhllX/tpsBXw9LCOgoMCFWJKgAKWu1SVC orisL8qdIkyuYGbrg+Gk/10yQgnyNaQrn/Zn2gzG8xtwAm3KT0CQn2eAhlCtMwO9 Uvr+jg4KF79afpCOv3oGaQ== DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d= messagingengine.com; h=cc:cc:content-transfer-encoding :content-type:content-type:date:date:feedback-id:feedback-id :from:from:in-reply-to:in-reply-to:message-id:mime-version :references:reply-to:subject:subject:to:to:x-me-proxy :x-me-sender:x-me-sender:x-sasl-enc; s=fm2; t=1742805779; x= 1742892179; bh=ooYuA0uM53YxnENt+atSUcEUD65s+aUhDXD6jeflgG8=; b=B SfO6oxApSVgj0TrkS6W3wVKULQ6hAQa3lp77kCWvQ8e/xZOtuIQyRAmW3rpe4QVQ yLUsoZiaB1QXnBg7VkR4iYL8cFoFdDKVoXBjZ0RPeE7UBRBcfdW8olZfRhKxXu6Q LsxvF/tDPh+EfCewAc8TdJoaX7MabBNBxPsouqJS/UhzggXgeSdY+BU1wx13Dn88 am7JAV0bi3HFpu9cGjb2QkCdC9ZTOw00XEWoGYBFjRtRd0sO5wnhPyemBaG9N2i2 msdFnm6cxxzBl2uTtZCBwKD4ScxlF7LyjWLRa45Q9erytJiTNhwArL9qA51j6nwa anon86oFjBrId9t5uGP8A== X-ME-Sender: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgeefvddrtddtgdduheelfedvucetufdoteggodetrf dotffvucfrrhhofhhilhgvmecuhfgrshhtofgrihhlpdggtfgfnhhsuhgsshgtrhhisggv pdfurfetoffkrfgpnffqhgenuceurghilhhouhhtmecufedttdenucesvcftvggtihhpih gvnhhtshculddquddttddmnecujfgurhepofggfffhvfevkfgjfhfutgfgsehtqhertder tdejnecuhfhrohhmpedftehrnhguuceuvghrghhmrghnnhdfuceorghrnhgusegrrhhnug gsrdguvgeqnecuggftrfgrthhtvghrnhepvdfhvdekueduveffffetgfdvveefvdelhedv vdegjedvfeehtdeggeevheefleejnecuvehluhhsthgvrhfuihiivgeptdenucfrrghrrg hmpehmrghilhhfrhhomheprghrnhgusegrrhhnuggsrdguvgdpnhgspghrtghpthhtohep vddvpdhmohguvgepshhmthhpohhuthdprhgtphhtthhopegrnhhshhhumhgrnhdrkhhhrg hnughurghlsegrrhhmrdgtohhmpdhrtghpthhtoheptggrthgrlhhinhdrmhgrrhhinhgr shesrghrmhdrtghomhdprhgtphhtthhopehrhigrnhdrrhhosggvrhhtshesrghrmhdrtg homhdprhgtphhtthhopehvihhntggvnhiiohdrfhhrrghstghinhhosegrrhhmrdgtohhm pdhrtghpthhtoheptghuihihuhhnhhhuihessgihthgvuggrnhgtvgdrtghomhdprhgtph htthhopehluhiguhdrkhgvrhhnvghlsegshihtvggurghntggvrdgtohhmpdhrtghpthht ohepshhtuhgrrhhtrdhmvghnvghfhiestghouggrshhiphdrtghomhdprhgtphhtthhope gthhhrihhsthhophhhvgdrlhgvrhhohiestghsghhrohhuphdrvghupdhrtghpthhtohep phgrlhhmvghrsegurggssggvlhhtrdgtohhm X-ME-Proxy: Feedback-ID: i56a14606:Fastmail Received: by mailuser.phl.internal (Postfix, from userid 501) id 1BD7F2220072; Mon, 24 Mar 2025 04:42:57 -0400 (EDT) X-Mailer: MessagingEngine.com Webmail Interface Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 X-ThreadId: T4d23c9949082f1fa Date: Mon, 24 Mar 2025 09:42:35 +0100 From: "Arnd Bergmann" To: "Yunhui Cui" Cc: "Paul Walmsley" , "Palmer Dabbelt" , "Albert Ou" , "Alexandre Ghiti" , "Anshuman Khandual" , "Andrew Morton" , "Ingo Molnar" , "Catalin Marinas" , "Ryan Roberts" , "Kirill A. Shutemov" , "Nam Cao" , =?UTF-8?Q?Bj=C3=B6rn_T=C3=B6pel?= , "Stuart Menefy" , "Xu Lu" , "Vincenzo Frascino" , "Samuel Holland" , "Christophe Leroy" , "Dawei Li" , "Mike Rapoport" , linux-riscv@lists.infradead.org, linux-kernel@vger.kernel.org Message-Id: <68644786-ed4b-49b1-9a4c-3318a8fc0c7d@app.fastmail.com> In-Reply-To: References: <20250320084428.51151-1-cuiyunhui@bytedance.com> <9256ca10-323d-41b4-b935-5281f925d50c@app.fastmail.com> Subject: Re: [External] Re: [PATCH] riscv: introduce the ioremap_prot() function Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable On Mon, Mar 24, 2025, at 02:30, yunhui cui wrote: > On Thu, Mar 20, 2025 at 5:22=E2=80=AFPM Arnd Bergmann = wrote: >> > diff --git a/arch/riscv/include/asm/io.h b/arch/riscv/include/asm/i= o.h >> > index a0e51840b9db..736c5557bd06 100644 >> > --- a/arch/riscv/include/asm/io.h >> > +++ b/arch/riscv/include/asm/io.h >> > @@ -133,6 +133,8 @@ __io_writes_outs(outs, u64, q, __io_pbr(), __io= _paw()) >> > #define outsq(addr, buffer, count) __outsq(PCI_IOBASE + (addr), bu= ffer, count) >> > #endif >> > >> > +#define ioremap_prot ioremap_prot >> > + >> > #include >> > >> > #ifdef CONFIG_MMU >> >> This feels slightly wrong to me, the "#define foo foo" method >> is normally used to override a declaration or inline function with >> another one, but this one only overrides the implementation, not >> the declaration. >> >> I see the same is done on arc, arm64, parisc, powerpc, s390, >> sh and xtensa, so we can keep this one as well, but it would be >> nice to change all of these to a less surprising approach. >> >> Maybe we should just remove these macros from asm/io.h and >> the trivial wrapper from mm/ioremap.c, and instead change the >> other architectures that have GENERIC_IOREMAP to use >> >> #define ioremap_prot generic_ioremap_prot >> >> It seems this would be only csky, hexagon, (some) loongarch >> and openrisc. > > It seems that we can't simply use #define ioremap_prot > generic_ioremap_prot because some architectures have certain special > behaviors before calling generic_ioremap_prot(). For example, there's > the ioremap_prot_hook() logic on ARM64 and the CONFIG_EISA logic on > PA-RISC, among others. I meant only the four that I have listed above should do the "#define ioremap_prot generic_ioremap_prot", while the ones that have some special case would continue to provide their own implementation. > Regarding the check of whether the address is a memory address, I > think we can directly incorporate pfn_valid() into > generic_ioremap_prot. This probably won't affect architectures that > directly use generic_ioremap_prot(), such as C-SKY, Hexagon, and > LoongArch. > > So, my next plan is to add pfn_valid() to generic_ioremap_prot(). Ah right, I see now that x86 does not use CONFIG_GENERIC_IOREMAP, so it would still be able to have its memremap() fall back to ioremap_cache(). You should probably still go through the list of drivers calling ioremap_cache() or ioremap_wt() to see if any of them might be used on an architecture that defines those two functions through ioremap_prot(), as that would be broken when they get called on normal memory: arch/loongarch/kernel/acpi.c: return ioremap_cache(phys, size); arch/mips/include/asm/dmi.h:#define dmi_remap(x, l) = ioremap_cache(x, l) arch/powerpc/kernel/crash_dump.c: vaddr =3D ioremap_cache(= paddr, PAGE_SIZE); arch/powerpc/platforms/pasemi/dma_lib.c: dma_status =3D ioremap_c= ache(res.start, resource_size(&res)); arch/powerpc/platforms/powernv/vas-window.c: map =3D ioremap_cache(st= art, len); arch/x86/hyperv/hv_init.c: ghcb_va =3D (void *)ioremap_cache(ghcb_g= pa, HV_HYP_PAGE_SIZE); arch/x86/kernel/acpi/boot.c: return ioremap_cache(phys, size); arch/x86/platform/efi/efi_32.c: va =3D ioremap_cache(md->phys_ad= dr, size); drivers/acpi/apei/bert.c: boot_error_region =3D ioremap_cache(bert= _tab->address, region_len); drivers/acpi/apei/einj-core.c: trigger_tab =3D ioremap_cache(trigger_pa= ddr, sizeof(*trigger_tab)); drivers/acpi/apei/einj-core.c: trigger_tab =3D ioremap_cache(trigger_pa= ddr, table_size); drivers/acpi/apei/erst.c: erst_erange.vaddr =3D ioremap_cache(erst= _erange.base, drivers/firmware/meson/meson_sm.c: return ioremap_cache(sm_phy_base= , size); drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c: adev->mman.aper_= base_kaddr =3D ioremap_cache(adev->gmc.aper_base, drivers/gpu/drm/hyperv/hyperv_drm_drv.c: hv->vram =3D ioremap_cac= he(hv->mem->start, hv->fb_size); drivers/gpu/drm/ttm/ttm_bo_util.c: map->virtual =3D= ioremap_cache(res, size); drivers/gpu/drm/ttm/ttm_bo_util.c: vaddr_iomem =3D = ioremap_cache(mem->bus.offset, drivers/hv/hv.c: /* Mask out vTOM bit. ioremap_cache() ma= ps decrypted */ drivers/hv/hv.c: (void *)ioremap_cache(base, HV_H= YP_PAGE_SIZE); drivers/hv/hv.c: /* Mask out vTOM bit. ioremap_cache() ma= ps decrypted */ drivers/hv/hv.c: (void *)ioremap_cache(base, HV_H= YP_PAGE_SIZE); drivers/mtd/devices/bcm47xxsflash.c: b47s->window =3D ioremap= _cache(res->start, resource_size(res)); drivers/mtd/maps/pxa2xx-flash.c: info->map.cached =3D ioremap_cac= he(info->map.phys, info->map.size); drivers/soc/fsl/qbman/qman_ccsr.c: void __iomem *tmpp =3D ioremap_c= ache(addr, sz); drivers/video/fbdev/hyperv_fb.c: fb_virt =3D ioremap_cache(par->m= em->start, screen_fb_size); include/acpi/acpi_io.h: return ioremap_cache(phys, size); drivers/block/z2ram.c: vaddr =3D (unsigned long)ioremap= _wt(paddr, size); drivers/video/fbdev/amifb.c: videomemory =3D (u_long)ioremap_wt(info-= >fix.smem_start, drivers/video/fbdev/atafb.c: external_screen_base =3D ioremap= _wt(external_addr, external_len); drivers/video/fbdev/controlfb.c: p->frame_buffer =3D ioremap_wt(p= ->frame_buffer_phys, 0x800000); drivers/video/fbdev/hpfb.c: fb_start =3D (unsigned long)ioremap_wt(f= b_info.fix.smem_start, drivers/video/fbdev/platinumfb.c: pinfo->frame_buffer =3D ioremap_= wt(pinfo->rsrc_fb.start, 0x400000); drivers/video/fbdev/valkyriefb.c: p->frame_buffer =3D ioremap_wt(f= rame_buffer_phys, p->total_vram); Similar, any architecture that doesn't have arch_memremap_wb() falls back to ioremap_cache() or ioremap(), which then have that check. Arnd