* [PATCH v2] ioremap() et al
@ 2026-04-08 12:05 Jan Beulich
2026-04-08 12:07 ` [PATCH v2 1/2] make ioremap_attr() common Jan Beulich
2026-04-08 12:09 ` [PATCH v2 2/2] make ioremap_wc() x86 only (for the time being) Jan Beulich
0 siblings, 2 replies; 10+ messages in thread
From: Jan Beulich @ 2026-04-08 12:05 UTC (permalink / raw)
To: xen-devel@lists.xenproject.org
Cc: Andrew Cooper, Julien Grall, Stefano Stabellini, Anthony PERARD,
Michal Orzel, Roger Pau Monné, Alistair Francis,
Connor Davis, Oleksii Kurochko, Bertrand Marquis,
Volodymyr Babchuk
I found an old todo item, carrying out of which then made me notice
another anomaly.
1: make ioremap_attr() common
2: make ioremap_wc() x86 only (for the time being)
Jan
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH v2 1/2] make ioremap_attr() common
2026-04-08 12:05 [PATCH v2] ioremap() et al Jan Beulich
@ 2026-04-08 12:07 ` Jan Beulich
2026-04-09 7:26 ` Luca Fancellu
` (2 more replies)
2026-04-08 12:09 ` [PATCH v2 2/2] make ioremap_wc() x86 only (for the time being) Jan Beulich
1 sibling, 3 replies; 10+ messages in thread
From: Jan Beulich @ 2026-04-08 12:07 UTC (permalink / raw)
To: xen-devel@lists.xenproject.org
Cc: Andrew Cooper, Julien Grall, Stefano Stabellini, Anthony PERARD,
Michal Orzel, Roger Pau Monné, Alistair Francis,
Connor Davis, Oleksii Kurochko, Bertrand Marquis,
Volodymyr Babchuk
This core backing function is uniform; what varies across architectures
are the attributes passed and hence the wrappers around it. Yet of course
extra checking or special handling may be needed per arch, so introduce a
suitable hook.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
---
Declarations (or inline counterparts) aren't being moved around, to avoid
the need to touch source files using the functions. Quite possibly they
want to consistently go into xen/io.h and asm/io.h.
Of course ioremap.c could also go into lib/.
For RISC-V the wrappers likely should become inline functions?
PPC doesn't reference any of the functions just yet, so gets only a
declaration.
For Arm, a TODO item is deliberately retained, yet seeing the use of
ioremap_wc() in domain building (which by itself is questionable, see next
patch) I wonder if that's even feasible as long as we don't have
memremap() or alike.
---
v2: Use conditional operator in ioremap_attr()'s final return. Re-base and
leverage that to simplify ioremap_attr() itself.
--- a/xen/arch/arm/include/asm/io.h
+++ b/xen/arch/arm/include/asm/io.h
@@ -1,6 +1,8 @@
#ifndef _ASM_IO_H
#define _ASM_IO_H
+#include <xen/mm-types.h>
+
#if defined(CONFIG_ARM_32)
# include <asm/arm32/io.h>
#elif defined(CONFIG_ARM_64)
@@ -9,6 +11,16 @@
# error "unknown ARM variant"
#endif
+#ifdef CONFIG_MPU
+void __iomem *mpu_ioremap_attr(paddr_t start, size_t len, pte_attr_t flags);
+#define arch_ioremap_attr mpu_ioremap_attr
+#else
+/*
+ * ioremap_attr() should only be used to remap device address ranges.
+ * TODO: Add an arch hook to verify this assumption.
+ */
+#endif
+
#endif
/*
* Local variables:
--- a/xen/arch/arm/include/asm/mm.h
+++ b/xen/arch/arm/include/asm/mm.h
@@ -5,6 +5,7 @@
#include <asm/page.h>
#include <public/xen.h>
#include <xen/pdx.h>
+#include <xen/vmap.h>
#if defined(CONFIG_ARM_32)
# include <asm/arm32/mm.h>
@@ -200,13 +201,12 @@ extern int prepare_secondary_mm(int cpu)
extern void setup_frametable_mappings(paddr_t ps, paddr_t pe);
/* Helper function to setup memory management */
void setup_mm_helper(void);
-/* map a physical range in virtual memory */
-void __iomem *ioremap_attr(paddr_t start, size_t len, unsigned int attributes);
static inline void __iomem *ioremap_nocache(paddr_t start, size_t len)
{
return ioremap_attr(start, len, PAGE_HYPERVISOR_NOCACHE);
}
+#define ioremap ioremap_nocache
static inline void __iomem *ioremap_cache(paddr_t start, size_t len)
{
--- a/xen/arch/arm/mm.c
+++ b/xen/arch/arm/mm.c
@@ -455,11 +455,6 @@ unsigned long get_upper_mfn_bound(void)
return max_page - 1;
}
-void *ioremap(paddr_t pa, size_t len)
-{
- return ioremap_attr(pa, len, PAGE_HYPERVISOR_NOCACHE);
-}
-
/*
* Local variables:
* mode: C
--- a/xen/arch/arm/mmu/pt.c
+++ b/xen/arch/arm/mmu/pt.c
@@ -206,23 +206,6 @@ void clear_fixmap(unsigned int map)
BUG_ON(res != 0);
}
-/*
- * This function should only be used to remap device address ranges
- * TODO: add a check to verify this assumption
- */
-void *ioremap_attr(paddr_t start, size_t len, unsigned int attributes)
-{
- mfn_t mfn = _mfn(PFN_DOWN(start));
- unsigned int offs = start & (PAGE_SIZE - 1);
- unsigned int nr = PFN_UP(offs + len);
- void *ptr = __vmap(&mfn, nr, 1, 1, attributes, VMAP_DEFAULT);
-
- if ( ptr == NULL )
- return NULL;
-
- return ptr + offs;
-}
-
static int create_xen_table(lpae_t *entry)
{
mfn_t mfn;
--- a/xen/arch/arm/mpu/mm.c
+++ b/xen/arch/arm/mpu/mm.c
@@ -9,6 +9,8 @@
#include <xen/sizes.h>
#include <xen/spinlock.h>
#include <xen/types.h>
+
+#include <asm/io.h>
#include <asm/mpu.h>
#include <asm/mpu/mm.h>
#include <asm/page.h>
@@ -593,7 +595,7 @@ void free_init_memory(void)
spin_unlock(&xen_mpumap_lock);
}
-void __iomem *ioremap_attr(paddr_t start, size_t len, unsigned int flags)
+void __iomem *mpu_ioremap_attr(paddr_t start, size_t len, pte_attr_t flags)
{
paddr_t start_pg = round_pgdown(start);
paddr_t end_pg = round_pgup(start + len);
--- a/xen/arch/ppc/include/asm/io.h
+++ b/xen/arch/ppc/include/asm/io.h
@@ -13,4 +13,6 @@
#define writew(v,c) ({ (void)(v); (void)(c); BUG_ON("unimplemented"); })
#define writel(v,c) ({ (void)(v); (void)(c); BUG_ON("unimplemented"); })
+void __iomem *ioremap(paddr_t pa, size_t len);
+
#endif /* __ASM_PPC_IO_H__ */
--- a/xen/arch/riscv/include/asm/io.h
+++ b/xen/arch/riscv/include/asm/io.h
@@ -41,6 +41,7 @@
#include <xen/macros.h>
#include <xen/types.h>
+void __iomem *ioremap(paddr_t pa, size_t len);
void __iomem *ioremap_cache(paddr_t pa, size_t len);
void __iomem *ioremap_wc(paddr_t pa, size_t len);
--- a/xen/arch/riscv/mm.c
+++ b/xen/arch/riscv/mm.c
@@ -587,20 +587,6 @@ void *__init arch_vmap_virt_end(void)
return (void *)(VMAP_VIRT_START + VMAP_VIRT_SIZE);
}
-static void __iomem *ioremap_attr(paddr_t pa, size_t len,
- pte_attr_t attributes)
-{
- mfn_t mfn = _mfn(PFN_DOWN(pa));
- unsigned int offs = pa & (PAGE_SIZE - 1);
- unsigned int nr = PFN_UP(offs + len);
- void *ptr = __vmap(&mfn, nr, 1, 1, attributes, VMAP_DEFAULT);
-
- if ( ptr == NULL )
- return NULL;
-
- return ptr + offs;
-}
-
void __iomem *ioremap_cache(paddr_t pa, size_t len)
{
return ioremap_attr(pa, len, PAGE_HYPERVISOR);
--- a/xen/arch/x86/include/asm/io.h
+++ b/xen/arch/x86/include/asm/io.h
@@ -47,6 +47,9 @@ __OUT(b,"b",char)
__OUT(w,"w",short)
__OUT(l,,int)
+void __iomem *x86_ioremap_attr(paddr_t pa, size_t len, pte_attr_t attr);
+#define arch_ioremap_attr x86_ioremap_attr
+
/*
* Boolean indicator and function used to handle platform specific I/O port
* emulation.
--- a/xen/arch/x86/include/asm/mm.h
+++ b/xen/arch/x86/include/asm/mm.h
@@ -611,7 +611,15 @@ void destroy_perdomain_mapping(struct do
unsigned int nr);
void free_perdomain_mappings(struct domain *d);
-void __iomem *ioremap_wc(paddr_t pa, size_t len);
+static inline void __iomem *ioremap(paddr_t pa, size_t len)
+{
+ return ioremap_attr(pa, len, PAGE_HYPERVISOR_UCMINUS);
+}
+
+static inline void __iomem *ioremap_wc(paddr_t pa, size_t len)
+{
+ return ioremap_attr(pa, len, PAGE_HYPERVISOR_WC);
+}
extern int memory_add(unsigned long spfn, unsigned long epfn, unsigned int pxm);
--- a/xen/arch/x86/mm.c
+++ b/xen/arch/x86/mm.c
@@ -6167,41 +6167,15 @@ void *__init arch_vmap_virt_end(void)
return fix_to_virt(__end_of_fixed_addresses);
}
-void __iomem *ioremap(paddr_t pa, size_t len)
+void __iomem *x86_ioremap_attr(paddr_t pa, size_t len, pte_attr_t attr)
{
- mfn_t mfn = _mfn(PFN_DOWN(pa));
- void *va;
-
- WARN_ON(page_is_ram_type(mfn_x(mfn), RAM_TYPE_CONVENTIONAL));
+ WARN_ON(page_is_ram_type(PFN_DOWN(pa), RAM_TYPE_CONVENTIONAL));
/* The low first Mb is always mapped. */
- if ( !((pa + len - 1) >> 20) )
- va = __va(pa);
- else
- {
- unsigned int offs = pa & (PAGE_SIZE - 1);
- unsigned int nr = PFN_UP(offs + len);
-
- va = __vmap(&mfn, nr, 1, 1, PAGE_HYPERVISOR_UCMINUS, VMAP_DEFAULT);
- if ( va )
- va += offs;
- }
-
- return (void __force __iomem *)va;
-}
-
-void __iomem *__init ioremap_wc(paddr_t pa, size_t len)
-{
- mfn_t mfn = _mfn(PFN_DOWN(pa));
- unsigned int offs = pa & (PAGE_SIZE - 1);
- unsigned int nr = PFN_UP(offs + len);
- void *va;
-
- WARN_ON(page_is_ram_type(mfn_x(mfn), RAM_TYPE_CONVENTIONAL));
-
- va = __vmap(&mfn, nr, 1, 1, PAGE_HYPERVISOR_WC, VMAP_DEFAULT);
+ if ( !((pa + len - 1) >> 20) && attr == PAGE_HYPERVISOR_UCMINUS )
+ return (void __force __iomem *)__va(pa);
- return (void __force __iomem *)(va ? va + offs : NULL);
+ return NULL;
}
int create_perdomain_mapping(struct domain *d, unsigned long va,
--- a/xen/common/Makefile
+++ b/xen/common/Makefile
@@ -18,6 +18,7 @@ obj-$(CONFIG_EVTCHN_FIFO) += event_fifo.
obj-$(CONFIG_GRANT_TABLE) += grant_table.o
obj-y += gzip/
obj-$(CONFIG_HYPFS) += hypfs.o
+obj-y += ioremap.o
obj-$(CONFIG_IOREQ_SERVER) += ioreq.o
obj-y += irq.o
obj-y += kernel.o
--- /dev/null
+++ b/xen/common/ioremap.c
@@ -0,0 +1,39 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+
+#include <xen/mm.h>
+#include <xen/pfn.h>
+#include <xen/vmap.h>
+
+#include <asm/io.h>
+
+void __iomem *ioremap_attr(paddr_t pa, size_t len, pte_attr_t attr)
+{
+ void __iomem *ptr = NULL;
+ unsigned int offs = PAGE_OFFSET(pa);
+
+#ifdef arch_ioremap_attr
+ ptr = arch_ioremap_attr(pa, len, attr);
+ if ( ptr )
+ return ptr;
+#endif
+
+ if ( IS_ENABLED(CONFIG_HAS_VMAP) )
+ {
+ mfn_t mfn = _mfn(PFN_DOWN(pa));
+
+ ptr = (void __force __iomem *)__vmap(&mfn, PFN_UP(offs + len), 1, 1,
+ attr, VMAP_DEFAULT);
+ }
+
+ return ptr ? ptr + offs : NULL;
+}
+
+/*
+ * Local variables:
+ * mode: C
+ * c-file-style: "BSD"
+ * c-basic-offset: 4
+ * tab-width: 4
+ * indent-tabs-mode: nil
+ * End:
+ */
--- a/xen/include/xen/vmap.h
+++ b/xen/include/xen/vmap.h
@@ -121,9 +121,10 @@ void vfree(void *va);
*
* @param pa Physical base address of the MMIO region.
* @param len Length of the MMIO region in octets.
+ * @param attr Attributes for the mapping.
* @return Pointer to the mapped area on success; NULL otherwise.
*/
-void __iomem *ioremap(paddr_t pa, size_t len);
+void __iomem *ioremap_attr(paddr_t pa, size_t len, pte_attr_t attr);
/* Return the number of pages in the mapping starting at address 'va' */
unsigned int vmap_size(const void *va);
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH v2 2/2] make ioremap_wc() x86 only (for the time being)
2026-04-08 12:05 [PATCH v2] ioremap() et al Jan Beulich
2026-04-08 12:07 ` [PATCH v2 1/2] make ioremap_attr() common Jan Beulich
@ 2026-04-08 12:09 ` Jan Beulich
2026-04-09 7:57 ` Luca Fancellu
[not found] ` <4291DA48-87D9-491B-83C6-51CCACC0FFE7@arm.com>
1 sibling, 2 replies; 10+ messages in thread
From: Jan Beulich @ 2026-04-08 12:09 UTC (permalink / raw)
To: xen-devel@lists.xenproject.org
Cc: Andrew Cooper, Julien Grall, Stefano Stabellini, Michal Orzel,
Roger Pau Monné, Alistair Francis, Connor Davis,
Oleksii Kurochko, Bertrand Marquis, Volodymyr Babchuk
Its use in domain building is questionable: Already at the point both uses
were introduced, ioremap_cache() existed. I can't see why kernel and
initrd would need mapping WC, when at the same time other similar mappings
(in common/device-tree/) are done WB.
With those uses replaced, neither Arm nor RISC-V have a need for the
function anymore.
Amends: d8972aa9645f ("xen/arm: kernel: Rework kernel_zimage_load to use the generic copy helper")
Amends: bb7e6d565d92 ("xen/arm: domain_build: Rework initrd_load to use the generic copy helper")
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Oleksii Kurochko <oleksii.kurochko@gmail.com>
--- a/xen/arch/arm/include/asm/mm.h
+++ b/xen/arch/arm/include/asm/mm.h
@@ -213,11 +213,6 @@ static inline void __iomem *ioremap_cach
return ioremap_attr(start, len, PAGE_HYPERVISOR);
}
-static inline void __iomem *ioremap_wc(paddr_t start, size_t len)
-{
- return ioremap_attr(start, len, PAGE_HYPERVISOR_WC);
-}
-
/* XXX -- account for base */
#define mfn_valid(mfn) ({ \
unsigned long __m_f_n = mfn_x(mfn); \
--- a/xen/arch/arm/kernel.c
+++ b/xen/arch/arm/kernel.c
@@ -148,7 +148,7 @@ static void __init kernel_zimage_load(st
printk("Loading zImage from %"PRIpaddr" to %"PRIpaddr"-%"PRIpaddr"\n",
paddr, load_addr, load_addr + len);
- kernel = ioremap_wc(paddr, len);
+ kernel = ioremap_cache(paddr, len);
if ( !kernel )
panic("Unable to map the %pd kernel\n", info->bd.d);
--- a/xen/arch/riscv/include/asm/io.h
+++ b/xen/arch/riscv/include/asm/io.h
@@ -43,7 +43,6 @@
void __iomem *ioremap(paddr_t pa, size_t len);
void __iomem *ioremap_cache(paddr_t pa, size_t len);
-void __iomem *ioremap_wc(paddr_t pa, size_t len);
/* Generic IO read/write. These perform native-endian accesses. */
static inline void __raw_writeb(uint8_t val, volatile void __iomem *addr)
--- a/xen/arch/riscv/mm.c
+++ b/xen/arch/riscv/mm.c
@@ -592,11 +592,6 @@ void __iomem *ioremap_cache(paddr_t pa,
return ioremap_attr(pa, len, PAGE_HYPERVISOR);
}
-void __iomem *ioremap_wc(paddr_t pa, size_t len)
-{
- return ioremap_attr(pa, len, PAGE_HYPERVISOR_WC);
-}
-
void __iomem *ioremap(paddr_t pa, size_t len)
{
return ioremap_attr(pa, len, PAGE_HYPERVISOR_NOCACHE);
--- a/xen/common/device-tree/domain-build.c
+++ b/xen/common/device-tree/domain-build.c
@@ -395,7 +395,7 @@ void __init initrd_load(struct kernel_in
if ( res )
panic("Cannot fix up \"linux,initrd-end\" property\n");
- initrd = ioremap_wc(paddr, len);
+ initrd = ioremap_cache(paddr, len);
if ( !initrd )
panic("Unable to map the %pd initrd\n", kinfo->bd.d);
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v2 1/2] make ioremap_attr() common
2026-04-08 12:07 ` [PATCH v2 1/2] make ioremap_attr() common Jan Beulich
@ 2026-04-09 7:26 ` Luca Fancellu
2026-04-09 7:54 ` Jan Beulich
2026-04-09 7:52 ` Roger Pau Monné
2026-04-09 8:36 ` Oleksii Kurochko
2 siblings, 1 reply; 10+ messages in thread
From: Luca Fancellu @ 2026-04-09 7:26 UTC (permalink / raw)
To: Jan Beulich
Cc: xen-devel@lists.xenproject.org, Andrew Cooper, Julien Grall,
Stefano Stabellini, Anthony PERARD, Michal Orzel,
Roger Pau Monné, Alistair Francis, Connor Davis,
Oleksii Kurochko, Bertrand Marquis, Volodymyr Babchuk
Hi Jan,
> On 8 Apr 2026, at 13:07, Jan Beulich <jbeulich@suse.com> wrote:
>
> This core backing function is uniform; what varies across architectures
> are the attributes passed and hence the wrappers around it. Yet of course
> extra checking or special handling may be needed per arch, so introduce a
> suitable hook.
>
> Signed-off-by: Jan Beulich <jbeulich@suse.com>
> ---
> Declarations (or inline counterparts) aren't being moved around, to avoid
> the need to touch source files using the functions. Quite possibly they
> want to consistently go into xen/io.h and asm/io.h.
>
> Of course ioremap.c could also go into lib/.
>
> For RISC-V the wrappers likely should become inline functions?
>
> PPC doesn't reference any of the functions just yet, so gets only a
> declaration.
>
> For Arm, a TODO item is deliberately retained, yet seeing the use of
> ioremap_wc() in domain building (which by itself is questionable, see next
> patch) I wonder if that's even feasible as long as we don't have
> memremap() or alike.
> ---
> v2: Use conditional operator in ioremap_attr()'s final return. Re-base and
> leverage that to simplify ioremap_attr() itself.
>
> --- a/xen/arch/arm/include/asm/io.h
> +++ b/xen/arch/arm/include/asm/io.h
> @@ -1,6 +1,8 @@
> #ifndef _ASM_IO_H
> #define _ASM_IO_H
>
> +#include <xen/mm-types.h>
> +
> #if defined(CONFIG_ARM_32)
> # include <asm/arm32/io.h>
> #elif defined(CONFIG_ARM_64)
> @@ -9,6 +11,16 @@
> # error "unknown ARM variant"
> #endif
>
> +#ifdef CONFIG_MPU
> +void __iomem *mpu_ioremap_attr(paddr_t start, size_t len, pte_attr_t flags);
> +#define arch_ioremap_attr mpu_ioremap_attr
> +#else
> +/*
> + * ioremap_attr() should only be used to remap device address ranges.
> + * TODO: Add an arch hook to verify this assumption.
> + */
> +#endif
I find a bit strange to have an #else with only a comment, but to be fair I’m not sure where this
comment can be put otherwise.
For the Arm and common part, I’ve also tested on Arm64 MMU, Arm32 MMU, Arm64 MPU on virtual platforms:
Reviewed-by: Luca Fancellu <luca.fancellu@arm.com> # arm, common
Tested-by: Luca Fancellu <luca.fancellu@arm.com> # arm, common
Cheers,
Luca
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v2 1/2] make ioremap_attr() common
2026-04-08 12:07 ` [PATCH v2 1/2] make ioremap_attr() common Jan Beulich
2026-04-09 7:26 ` Luca Fancellu
@ 2026-04-09 7:52 ` Roger Pau Monné
2026-04-09 7:58 ` Jan Beulich
2026-04-09 8:36 ` Oleksii Kurochko
2 siblings, 1 reply; 10+ messages in thread
From: Roger Pau Monné @ 2026-04-09 7:52 UTC (permalink / raw)
To: Jan Beulich
Cc: xen-devel@lists.xenproject.org, Andrew Cooper, Julien Grall,
Stefano Stabellini, Anthony PERARD, Michal Orzel,
Alistair Francis, Connor Davis, Oleksii Kurochko,
Bertrand Marquis, Volodymyr Babchuk
On Wed, Apr 08, 2026 at 02:07:23PM +0200, Jan Beulich wrote:
> This core backing function is uniform; what varies across architectures
> are the attributes passed and hence the wrappers around it. Yet of course
> extra checking or special handling may be needed per arch, so introduce a
> suitable hook.
>
> Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Roger Pau Monné <roger.pau@citrix.com>
Just looked at the common and x86 bits, both LGTM.
> ---
> Declarations (or inline counterparts) aren't being moved around, to avoid
> the need to touch source files using the functions. Quite possibly they
> want to consistently go into xen/io.h and asm/io.h.
>
> Of course ioremap.c could also go into lib/.
Maybe I'm missing the point, but what's the benefit for it to go into
lib/? Is there any realistic scenario where we might have a
functional hypervisor build that doesn't require ioremap?
Thanks, Roger.
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v2 1/2] make ioremap_attr() common
2026-04-09 7:26 ` Luca Fancellu
@ 2026-04-09 7:54 ` Jan Beulich
0 siblings, 0 replies; 10+ messages in thread
From: Jan Beulich @ 2026-04-09 7:54 UTC (permalink / raw)
To: Luca Fancellu
Cc: xen-devel@lists.xenproject.org, Andrew Cooper, Julien Grall,
Stefano Stabellini, Anthony PERARD, Michal Orzel,
Roger Pau Monné, Alistair Francis, Connor Davis,
Oleksii Kurochko, Bertrand Marquis, Volodymyr Babchuk
On 09.04.2026 09:26, Luca Fancellu wrote:
> Hi Jan,
>
>> On 8 Apr 2026, at 13:07, Jan Beulich <jbeulich@suse.com> wrote:
>>
>> This core backing function is uniform; what varies across architectures
>> are the attributes passed and hence the wrappers around it. Yet of course
>> extra checking or special handling may be needed per arch, so introduce a
>> suitable hook.
>>
>> Signed-off-by: Jan Beulich <jbeulich@suse.com>
>> ---
>> Declarations (or inline counterparts) aren't being moved around, to avoid
>> the need to touch source files using the functions. Quite possibly they
>> want to consistently go into xen/io.h and asm/io.h.
>>
>> Of course ioremap.c could also go into lib/.
>>
>> For RISC-V the wrappers likely should become inline functions?
>>
>> PPC doesn't reference any of the functions just yet, so gets only a
>> declaration.
>>
>> For Arm, a TODO item is deliberately retained, yet seeing the use of
>> ioremap_wc() in domain building (which by itself is questionable, see next
>> patch) I wonder if that's even feasible as long as we don't have
>> memremap() or alike.
>> ---
>> v2: Use conditional operator in ioremap_attr()'s final return. Re-base and
>> leverage that to simplify ioremap_attr() itself.
>>
>> --- a/xen/arch/arm/include/asm/io.h
>> +++ b/xen/arch/arm/include/asm/io.h
>> @@ -1,6 +1,8 @@
>> #ifndef _ASM_IO_H
>> #define _ASM_IO_H
>>
>> +#include <xen/mm-types.h>
>> +
>> #if defined(CONFIG_ARM_32)
>> # include <asm/arm32/io.h>
>> #elif defined(CONFIG_ARM_64)
>> @@ -9,6 +11,16 @@
>> # error "unknown ARM variant"
>> #endif
>>
>> +#ifdef CONFIG_MPU
>> +void __iomem *mpu_ioremap_attr(paddr_t start, size_t len, pte_attr_t flags);
>> +#define arch_ioremap_attr mpu_ioremap_attr
>> +#else
>> +/*
>> + * ioremap_attr() should only be used to remap device address ranges.
>> + * TODO: Add an arch hook to verify this assumption.
>> + */
>> +#endif
>
> I find a bit strange to have an #else with only a comment, but to be fair I’m not sure where this
> comment can be put otherwise.
That's the issue: I don't want to lose the comment, but I also didn't
see a good place to put it. Having it here means it can be replaced by
a single patch hunk, whenever the TODO is going to be addressed.
> For the Arm and common part, I’ve also tested on Arm64 MMU, Arm32 MMU, Arm64 MPU on virtual platforms:
>
> Reviewed-by: Luca Fancellu <luca.fancellu@arm.com> # arm, common
> Tested-by: Luca Fancellu <luca.fancellu@arm.com> # arm, common
Thanks.
Jan
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v2 2/2] make ioremap_wc() x86 only (for the time being)
2026-04-08 12:09 ` [PATCH v2 2/2] make ioremap_wc() x86 only (for the time being) Jan Beulich
@ 2026-04-09 7:57 ` Luca Fancellu
[not found] ` <4291DA48-87D9-491B-83C6-51CCACC0FFE7@arm.com>
1 sibling, 0 replies; 10+ messages in thread
From: Luca Fancellu @ 2026-04-09 7:57 UTC (permalink / raw)
To: Jan Beulich
Cc: xen-devel@lists.xenproject.org, Andrew Cooper, Julien Grall,
Stefano Stabellini, Michal Orzel, Roger Pau Monné,
Alistair Francis, Connor Davis, Oleksii Kurochko,
Bertrand Marquis, Volodymyr Babchuk
Hi Jan,
Reviewed the arm and common part, tested on arm64 mmu, arm32 mmu, arm64 mpu:
Reviewed-by: Luca Fancellu <luca.fancellu@arm.com> # arm, common
Tested-by: Luca Fancellu <luca.fancellu@arm.com> # arm, common
P.S. I’m fighting against the spam filter to send this, hence I’ve deleted the whole code patch,
let’s see if it works...
Cheers,
Luca
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v2 1/2] make ioremap_attr() common
2026-04-09 7:52 ` Roger Pau Monné
@ 2026-04-09 7:58 ` Jan Beulich
0 siblings, 0 replies; 10+ messages in thread
From: Jan Beulich @ 2026-04-09 7:58 UTC (permalink / raw)
To: Roger Pau Monné
Cc: xen-devel@lists.xenproject.org, Andrew Cooper, Julien Grall,
Stefano Stabellini, Anthony PERARD, Michal Orzel,
Alistair Francis, Connor Davis, Oleksii Kurochko,
Bertrand Marquis, Volodymyr Babchuk
On 09.04.2026 09:52, Roger Pau Monné wrote:
> On Wed, Apr 08, 2026 at 02:07:23PM +0200, Jan Beulich wrote:
>> This core backing function is uniform; what varies across architectures
>> are the attributes passed and hence the wrappers around it. Yet of course
>> extra checking or special handling may be needed per arch, so introduce a
>> suitable hook.
>>
>> Signed-off-by: Jan Beulich <jbeulich@suse.com>
>
> Reviewed-by: Roger Pau Monné <roger.pau@citrix.com>
Thanks.
> Just looked at the common and x86 bits, both LGTM.
I'll transcribe this into
Reviewed-by: Roger Pau Monné <roger.pau@citrix.com> # x86, common
>> ---
>> Declarations (or inline counterparts) aren't being moved around, to avoid
>> the need to touch source files using the functions. Quite possibly they
>> want to consistently go into xen/io.h and asm/io.h.
>>
>> Of course ioremap.c could also go into lib/.
>
> Maybe I'm missing the point, but what's the benefit for it to go into
> lib/? Is there any realistic scenario where we might have a
> functional hypervisor build that doesn't require ioremap?
Well, the difference likely is between "in principle" vs "in practice". In
principle a lot of code could really be regarded "library" like. In practice
whether to put code in lib/ really matters for stuff which may only be
needed in certain configurations.
Jan
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v2 2/2] make ioremap_wc() x86 only (for the time being)
[not found] ` <4291DA48-87D9-491B-83C6-51CCACC0FFE7@arm.com>
@ 2026-04-09 8:01 ` Jan Beulich
0 siblings, 0 replies; 10+ messages in thread
From: Jan Beulich @ 2026-04-09 8:01 UTC (permalink / raw)
To: Luca Fancellu
Cc: xen-devel@lists.xenproject.org, Andrew Cooper, Julien Grall,
Stefano Stabellini, Michal Orzel, Roger Pau Monné,
Alistair Francis, Connor Davis, Oleksii Kurochko,
Bertrand Marquis, Volodymyr Babchuk
On 09.04.2026 09:52, Luca Fancellu wrote:
>> On 8 Apr 2026, at 13:09, Jan Beulich <jbeulich@suse.com> wrote:
>>
>> Its use in domain building is questionable: Already at the point both uses
>> were introduced, ioremap_cache() existed. I can't see why kernel and
>> initrd would need mapping WC, when at the same time other similar mappings
>> (in common/device-tree/) are done WB.
>>
>> With those uses replaced, neither Arm nor RISC-V have a need for the
>> function anymore.
>>
>> Amends: d8972aa9645f ("xen/arm: kernel: Rework kernel_zimage_load to use the generic copy helper")
>> Amends: bb7e6d565d92 ("xen/arm: domain_build: Rework initrd_load to use the generic copy helper")
>> Signed-off-by: Jan Beulich <jbeulich@suse.com>
>> Reviewed-by: Oleksii Kurochko <oleksii.kurochko@gmail.com>
>
>
> Reviewed the arm and common part, tested on arm64 mmu, arm32 mmu, arm64 mpu:
>
> Reviewed-by: Luca Fancellu <luca.fancellu@arm.com>
> Tested-by: Luca Fancellu <luca.fancellu@arm.com>
Thanks, but I guess I'll use the form you sent earlier, with the restricted tags
(matching what you say separately).
Jan
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v2 1/2] make ioremap_attr() common
2026-04-08 12:07 ` [PATCH v2 1/2] make ioremap_attr() common Jan Beulich
2026-04-09 7:26 ` Luca Fancellu
2026-04-09 7:52 ` Roger Pau Monné
@ 2026-04-09 8:36 ` Oleksii Kurochko
2 siblings, 0 replies; 10+ messages in thread
From: Oleksii Kurochko @ 2026-04-09 8:36 UTC (permalink / raw)
To: Jan Beulich, xen-devel@lists.xenproject.org
Cc: Andrew Cooper, Julien Grall, Stefano Stabellini, Anthony PERARD,
Michal Orzel, Roger Pau Monné, Alistair Francis,
Connor Davis, Bertrand Marquis, Volodymyr Babchuk
On 4/8/26 2:07 PM, Jan Beulich wrote:
> This core backing function is uniform; what varies across architectures
> are the attributes passed and hence the wrappers around it. Yet of course
> extra checking or special handling may be needed per arch, so introduce a
> suitable hook.
>
> Signed-off-by: Jan Beulich <jbeulich@suse.com>
> ---
RISC-V part LGTM:
Reviewed-by: Oleksii Kurochko <oleksii.kurochko@gmail.com> # riscv
I've also tested on my test cases this patch and the next one and it
works fine:
Tested-by: Oleksii Kurochko <oleksii.kurochko@gmail.com> # riscv
Thanks.
~ Oleksii
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2026-04-09 8:36 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-04-08 12:05 [PATCH v2] ioremap() et al Jan Beulich
2026-04-08 12:07 ` [PATCH v2 1/2] make ioremap_attr() common Jan Beulich
2026-04-09 7:26 ` Luca Fancellu
2026-04-09 7:54 ` Jan Beulich
2026-04-09 7:52 ` Roger Pau Monné
2026-04-09 7:58 ` Jan Beulich
2026-04-09 8:36 ` Oleksii Kurochko
2026-04-08 12:09 ` [PATCH v2 2/2] make ioremap_wc() x86 only (for the time being) Jan Beulich
2026-04-09 7:57 ` Luca Fancellu
[not found] ` <4291DA48-87D9-491B-83C6-51CCACC0FFE7@arm.com>
2026-04-09 8:01 ` Jan Beulich
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.