[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-Id: <68644786-ed4b-49b1-9a4c-3318a8fc0c7d@app.fastmail.com>
Date: Mon, 24 Mar 2025 09:42:35 +0100
From: "Arnd Bergmann" <arnd@...db.de>
To: "Yunhui Cui" <cuiyunhui@...edance.com>
Cc: "Paul Walmsley" <paul.walmsley@...ive.com>,
"Palmer Dabbelt" <palmer@...belt.com>, "Albert Ou" <aou@...s.berkeley.edu>,
"Alexandre Ghiti" <alex@...ti.fr>,
"Anshuman Khandual" <anshuman.khandual@....com>,
"Andrew Morton" <akpm@...ux-foundation.org>,
"Ingo Molnar" <mingo@...nel.org>,
"Catalin Marinas" <catalin.marinas@....com>,
"Ryan Roberts" <ryan.roberts@....com>,
"Kirill A. Shutemov" <kirill.shutemov@...ux.intel.com>,
"Nam Cao" <namcao@...utronix.de>,
Björn Töpel <bjorn@...osinc.com>,
"Stuart Menefy" <stuart.menefy@...asip.com>,
"Xu Lu" <luxu.kernel@...edance.com>,
"Vincenzo Frascino" <vincenzo.frascino@....com>,
"Samuel Holland" <samuel.holland@...ive.com>,
"Christophe Leroy" <christophe.leroy@...roup.eu>,
"Dawei Li" <dawei.li@...ngroup.cn>, "Mike Rapoport" <rppt@...nel.org>,
linux-riscv@...ts.infradead.org, linux-kernel@...r.kernel.org
Subject: Re: [External] Re: [PATCH] riscv: introduce the ioremap_prot() function
On Mon, Mar 24, 2025, at 02:30, yunhui cui wrote:
> On Thu, Mar 20, 2025 at 5:22 PM Arnd Bergmann <arnd@...db.de> wrote:
>> > diff --git a/arch/riscv/include/asm/io.h b/arch/riscv/include/asm/io.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), buffer, count)
>> > #endif
>> >
>> > +#define ioremap_prot ioremap_prot
>> > +
>> > #include <asm-generic/io.h>
>> >
>> > #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 = ioremap_cache(paddr, PAGE_SIZE);
arch/powerpc/platforms/pasemi/dma_lib.c: dma_status = ioremap_cache(res.start, resource_size(&res));
arch/powerpc/platforms/powernv/vas-window.c: map = ioremap_cache(start, len);
arch/x86/hyperv/hv_init.c: ghcb_va = (void *)ioremap_cache(ghcb_gpa, HV_HYP_PAGE_SIZE);
arch/x86/kernel/acpi/boot.c: return ioremap_cache(phys, size);
arch/x86/platform/efi/efi_32.c: va = ioremap_cache(md->phys_addr, size);
drivers/acpi/apei/bert.c: boot_error_region = ioremap_cache(bert_tab->address, region_len);
drivers/acpi/apei/einj-core.c: trigger_tab = ioremap_cache(trigger_paddr, sizeof(*trigger_tab));
drivers/acpi/apei/einj-core.c: trigger_tab = ioremap_cache(trigger_paddr, table_size);
drivers/acpi/apei/erst.c: erst_erange.vaddr = 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 = ioremap_cache(adev->gmc.aper_base,
drivers/gpu/drm/hyperv/hyperv_drm_drv.c: hv->vram = ioremap_cache(hv->mem->start, hv->fb_size);
drivers/gpu/drm/ttm/ttm_bo_util.c: map->virtual = ioremap_cache(res, size);
drivers/gpu/drm/ttm/ttm_bo_util.c: vaddr_iomem = ioremap_cache(mem->bus.offset,
drivers/hv/hv.c: /* Mask out vTOM bit. ioremap_cache() maps decrypted */
drivers/hv/hv.c: (void *)ioremap_cache(base, HV_HYP_PAGE_SIZE);
drivers/hv/hv.c: /* Mask out vTOM bit. ioremap_cache() maps decrypted */
drivers/hv/hv.c: (void *)ioremap_cache(base, HV_HYP_PAGE_SIZE);
drivers/mtd/devices/bcm47xxsflash.c: b47s->window = ioremap_cache(res->start, resource_size(res));
drivers/mtd/maps/pxa2xx-flash.c: info->map.cached = ioremap_cache(info->map.phys, info->map.size);
drivers/soc/fsl/qbman/qman_ccsr.c: void __iomem *tmpp = ioremap_cache(addr, sz);
drivers/video/fbdev/hyperv_fb.c: fb_virt = ioremap_cache(par->mem->start, screen_fb_size);
include/acpi/acpi_io.h: return ioremap_cache(phys, size);
drivers/block/z2ram.c: vaddr = (unsigned long)ioremap_wt(paddr, size);
drivers/video/fbdev/amifb.c: videomemory = (u_long)ioremap_wt(info->fix.smem_start,
drivers/video/fbdev/atafb.c: external_screen_base = ioremap_wt(external_addr, external_len);
drivers/video/fbdev/controlfb.c: p->frame_buffer = ioremap_wt(p->frame_buffer_phys, 0x800000);
drivers/video/fbdev/hpfb.c: fb_start = (unsigned long)ioremap_wt(fb_info.fix.smem_start,
drivers/video/fbdev/platinumfb.c: pinfo->frame_buffer = ioremap_wt(pinfo->rsrc_fb.start, 0x400000);
drivers/video/fbdev/valkyriefb.c: p->frame_buffer = ioremap_wt(frame_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
Powered by blists - more mailing lists