Re: `io{re,un}map()` build error in s390 under `!CONFIG_HAS_IOMEM`
"Gary Guo" <[email protected]> Tue, 04 Aug 2026 13:26:15 +0100
| Newsgroups | org.kernel.vger.rust-for-linux,dev.linux.lists.driver-core,org.kernel.vger.linux-arch,org.kernel.vger.linux-s390 |
|---|---|
| Message-ID | <[email protected]> |
On Tue Aug 4, 2026 at 1:02 PM BST, Arnd Bergmann wrote:
> On Tue, Aug 4, 2026, at 13:10, Danilo Krummrich wrote:
>> On Tue Aug 4, 2026 at 12:36 PM CEST, Arnd Bergmann wrote:
>>> On Tue, Aug 4, 2026, at 09:13, Heiko Carstens wrote:
>>>>
>>>>> I just sent out a fix [1]; the only annoying part is [2], but we shou=
ld change
>>>>> those doc-tests anyway. For the one in rust/kernel/io.rs we already d=
id in
>>>>> driver-core-next.
>>>>>=20
>>>>> [1] https://lore.kernel.org/driver-core/20260803200249.3494259-1-dakr=
@kernel.org/
>>>
>>> This looks like you still provide the rust version of ioremap(),
>>> turning what is supposed to be a link failure into a runtime
>>> error.
>>
>> Which is the standard for many core APIs, such as [1]. However, I do agr=
ee that
>> in this case the correct fix would be to have all architectures provide =
the
>> stubs rather than the Rust code.
>
> We have both types of interfaces in the kernel. For HAS_IOMEM and HAS_IOP=
ORT,
> the link failure is intentional, as it helps identify drivers that need
> a Kconfig dependency and are either unusable or potentially harmful if lo=
aded
> without this.
>
> Having empty stubs only really makes sense for things like LED support
> where a driver calling the interfaces can continue to work
> correctly when the interface is compile-time disabled.
We can still have link failures if we always provide the signatures, just d=
on't
provide implementation?
Best,
Gary
>
>> However, there's already a precedent for this in the kernel, e.g. in [2]=
. Of
>> course, it would be better to clean this up, but depending on whether th=
ere's
>> more architectures having this issue (I didn't check) that's separate fr=
om a
>> fix.
>
> arch/um is the only other one that does not always enable HAS_IOMEM,
> though most m68k targets don't have any support for ISA/PCI style
> MMIO or PIO and probably should not enable it in theory.
>
>> [2]=20
>> https://elixir.bootlin.com/linux/v7.1.5/source/include/linux/device/devr=
es.h#L115
>
> Right, we are definitely already inconsistent here.
>
>>> The simple change below would just extend that behavior to !PCI
>>> and make that consistent with CONFIG_PCI=3Dy on machines without
>>> actual PCI hardware. Of course any code that might rely on this
>>> is now a bug that likely never gets caught at build time.
>>>
>>> This still relies on implementing the __raw_* helpers as nop
>>> to have the same behavior as the PCI=3Dy version, as the generic
>>> version would just end up dereferencing the invalid pointers.
>>
>> As mentioned, I didn't check, but if this is the only architecture causi=
ng those
>> issues that'd be the better fix of course.
>>
>> However, IIUC, your patch below would make ioremap() and friends silenty=
succeed
>> and only the accessors would prevent undefined behavior?
>>
>> In this case I still think ioremap() should just fail.
>
> In that case, it would make sense to also change the CONFIG_PCI=3Dy
> version to fail the same way when the address points outside of
> the PCI memory space range. The current version in
> arch/s390/pci/pci.c just falls back to generic_ioremap_prot(),
> which is what I would use here directly:
>
> void __iomem *ioremap_prot(phys_addr_t phys_addr, size_t size,
> pgprot_t prot)
> {
> if (!static_branch_unlikely(&have_mio))
> return (void __iomem *)phys_addr;
> return generic_ioremap_prot(phys_addr, size, prot);
> }
>
> The two methods here (generic_ioremap_prot() and the cast)
> are machine specific to refer to two different ways that PCI
> devices can be accessed if present, but there is no case
> for PCI being unavailable altogether.
>
> Arnd