[PATCH v5.10.y] xen: replace xen_remap() with memremap()

Teddy Astie posted 1 patch 1 month, 4 weeks ago
Failed in applying to current master (apply log)
There is a newer version of this series
arch/x86/include/asm/xen/page.h   | 3 ---
drivers/tty/hvc/hvc_xen.c         | 2 +-
drivers/xen/grant-table.c         | 6 +++---
drivers/xen/xenbus/xenbus_probe.c | 3 +--
include/xen/arm/page.h            | 3 ---
5 files changed, 5 insertions(+), 12 deletions(-)
[PATCH v5.10.y] xen: replace xen_remap() with memremap()
Posted by Teddy Astie 1 month, 4 weeks ago
From: Juergen Gross <jgross@suse.com>

From: Juergen Gross <jgross@suse.com>

[ upstream commit 41925b105e345ebc84cedb64f59d20cb14a62613 ]

xen_remap() is used to establish mappings for frames not under direct
control of the kernel: for Xenstore and console ring pages, and for
grant pages of non-PV guests.

Today xen_remap() is defined to use ioremap() on x86 (doing uncached
mappings), and ioremap_cache() on Arm (doing cached mappings).

Uncached mappings for those use cases are bad for performance, so they
should be avoided if possible. As all use cases of xen_remap() don't
require uncached mappings (the mapped area is always physical RAM),
a mapping using the standard WB cache mode is fine.

As sparse is flagging some of the xen_remap() use cases to be not
appropriate for iomem(), as the result is not annotated with the
__iomem modifier, eliminate xen_remap() completely and replace all
use cases with memremap() specifying the MEMREMAP_WB caching mode.

xen_unmap() can be replaced with memunmap().

Reported-by: kernel test robot <lkp@intel.com>
Signed-off-by: Juergen Gross <jgross@suse.com>
Reviewed-by: Boris Ostrovsky <boris.ostrovsky@oracle.com>
Acked-by: Stefano Stabellini <sstabellini@kernel.org>
Link: https://lore.kernel.org/r/20220530082634.6339-1-jgross@suse.com
Signed-off-by: Juergen Gross <jgross@suse.com>
Signed-off-by: Teddy Astie <teddy.astie@vates.tech> [backport to 5.10.y]
---
Cc: Anthoine Bourgeois <anthoine.bourgeois@vates.tech>
Cc: Juergen Gross <jgross@suse.com>
Cc: Boris Ostrovsky <boris.ostrovsky@oracle.com>
Cc: Stefano Stabellini <sstabellini@kernel.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: Borislav Petkov <bp@alien8.de>
Cc: Dave Hansen <dave.hansen@linux.intel.com>
Cc: "H. Peter Anvin" <hpa@zytor.com>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Jiri Slaby <jirislaby@kernel.org>

 arch/x86/include/asm/xen/page.h   | 3 ---
 drivers/tty/hvc/hvc_xen.c         | 2 +-
 drivers/xen/grant-table.c         | 6 +++---
 drivers/xen/xenbus/xenbus_probe.c | 3 +--
 include/xen/arm/page.h            | 3 ---
 5 files changed, 5 insertions(+), 12 deletions(-)

diff --git a/arch/x86/include/asm/xen/page.h b/arch/x86/include/asm/xen/page.h
index 5941e18edd5a..c183b7f9efef 100644
--- a/arch/x86/include/asm/xen/page.h
+++ b/arch/x86/include/asm/xen/page.h
@@ -355,9 +355,6 @@ unsigned long arbitrary_virt_to_mfn(void *vaddr);
 void make_lowmem_page_readonly(void *vaddr);
 void make_lowmem_page_readwrite(void *vaddr);
 
-#define xen_remap(cookie, size) ioremap((cookie), (size));
-#define xen_unmap(cookie) iounmap((cookie))
-
 static inline bool xen_arch_need_swiotlb(struct device *dev,
 					 phys_addr_t phys,
 					 dma_addr_t dev_addr)
diff --git a/drivers/tty/hvc/hvc_xen.c b/drivers/tty/hvc/hvc_xen.c
index 4886cad0fde6..7b472ab2f34f 100644
--- a/drivers/tty/hvc/hvc_xen.c
+++ b/drivers/tty/hvc/hvc_xen.c
@@ -270,7 +270,7 @@ static int xen_hvm_console_init(void)
 	if (r < 0 || v == 0)
 		goto err;
 	gfn = v;
-	info->intf = xen_remap(gfn << XEN_PAGE_SHIFT, XEN_PAGE_SIZE);
+	info->intf = memremap(gfn << XEN_PAGE_SHIFT, XEN_PAGE_SIZE, MEMREMAP_WB);
 	if (info->intf == NULL)
 		goto err;
 	info->vtermno = HVC_COOKIE;
diff --git a/drivers/xen/grant-table.c b/drivers/xen/grant-table.c
index 0a2d24d6ac6f..a10e0741bec5 100644
--- a/drivers/xen/grant-table.c
+++ b/drivers/xen/grant-table.c
@@ -743,7 +743,7 @@ int gnttab_setup_auto_xlat_frames(phys_addr_t addr)
 	if (xen_auto_xlat_grant_frames.count)
 		return -EINVAL;
 
-	vaddr = xen_remap(addr, XEN_PAGE_SIZE * max_nr_gframes);
+	vaddr = memremap(addr, XEN_PAGE_SIZE * max_nr_gframes, MEMREMAP_WB);
 	if (vaddr == NULL) {
 		pr_warn("Failed to ioremap gnttab share frames (addr=%pa)!\n",
 			&addr);
@@ -751,7 +751,7 @@ int gnttab_setup_auto_xlat_frames(phys_addr_t addr)
 	}
 	pfn = kcalloc(max_nr_gframes, sizeof(pfn[0]), GFP_KERNEL);
 	if (!pfn) {
-		xen_unmap(vaddr);
+		memunmap(vaddr);
 		return -ENOMEM;
 	}
 	for (i = 0; i < max_nr_gframes; i++)
@@ -770,7 +770,7 @@ void gnttab_free_auto_xlat_frames(void)
 	if (!xen_auto_xlat_grant_frames.count)
 		return;
 	kfree(xen_auto_xlat_grant_frames.pfn);
-	xen_unmap(xen_auto_xlat_grant_frames.vaddr);
+	memunmap(xen_auto_xlat_grant_frames.vaddr);
 
 	xen_auto_xlat_grant_frames.pfn = NULL;
 	xen_auto_xlat_grant_frames.count = 0;
diff --git a/drivers/xen/xenbus/xenbus_probe.c b/drivers/xen/xenbus/xenbus_probe.c
index fb5358a73820..23595fdd053d 100644
--- a/drivers/xen/xenbus/xenbus_probe.c
+++ b/drivers/xen/xenbus/xenbus_probe.c
@@ -919,8 +919,7 @@ static int __init xenbus_init(void)
 #endif
 		xen_store_gfn = (unsigned long)v;
 		xen_store_interface =
-			xen_remap(xen_store_gfn << XEN_PAGE_SHIFT,
-				  XEN_PAGE_SIZE);
+			memremap(xen_store_gfn << XEN_PAGE_SHIFT, XEN_PAGE_SIZE, MEMREMAP_WB);
 		break;
 	default:
 		pr_warn("Xenstore state unknown\n");
diff --git a/include/xen/arm/page.h b/include/xen/arm/page.h
index ac1b65470563..f831cfeca000 100644
--- a/include/xen/arm/page.h
+++ b/include/xen/arm/page.h
@@ -109,9 +109,6 @@ static inline bool set_phys_to_machine(unsigned long pfn, unsigned long mfn)
 	return __set_phys_to_machine(pfn, mfn);
 }
 
-#define xen_remap(cookie, size) ioremap_cache((cookie), (size))
-#define xen_unmap(cookie) iounmap((cookie))
-
 bool xen_arch_need_swiotlb(struct device *dev,
 			   phys_addr_t phys,
 			   dma_addr_t dev_addr);
-- 
2.51.0



--
Teddy Astie | Vates XCP-ng Developer

XCP-ng & Xen Orchestra - Vates solutions

web: https://vates.tech
Re: [PATCH v5.10.y] xen: replace xen_remap() with memremap()
Posted by Greg Kroah-Hartman 1 month, 4 weeks ago
On Tue, Sep 02, 2025 at 09:28:32AM +0000, Teddy Astie wrote:
> From: Juergen Gross <jgross@suse.com>
> 
> From: Juergen Gross <jgross@suse.com>
> 
> [ upstream commit 41925b105e345ebc84cedb64f59d20cb14a62613 ]
> 
> xen_remap() is used to establish mappings for frames not under direct
> control of the kernel: for Xenstore and console ring pages, and for
> grant pages of non-PV guests.
> 
> Today xen_remap() is defined to use ioremap() on x86 (doing uncached
> mappings), and ioremap_cache() on Arm (doing cached mappings).
> 
> Uncached mappings for those use cases are bad for performance, so they
> should be avoided if possible. As all use cases of xen_remap() don't
> require uncached mappings (the mapped area is always physical RAM),
> a mapping using the standard WB cache mode is fine.
> 
> As sparse is flagging some of the xen_remap() use cases to be not
> appropriate for iomem(), as the result is not annotated with the
> __iomem modifier, eliminate xen_remap() completely and replace all
> use cases with memremap() specifying the MEMREMAP_WB caching mode.
> 
> xen_unmap() can be replaced with memunmap().
> 
> Reported-by: kernel test robot <lkp@intel.com>
> Signed-off-by: Juergen Gross <jgross@suse.com>
> Reviewed-by: Boris Ostrovsky <boris.ostrovsky@oracle.com>
> Acked-by: Stefano Stabellini <sstabellini@kernel.org>
> Link: https://lore.kernel.org/r/20220530082634.6339-1-jgross@suse.com
> Signed-off-by: Juergen Gross <jgross@suse.com>
> Signed-off-by: Teddy Astie <teddy.astie@vates.tech> [backport to 5.10.y]
> ---

Why is this needed for 5.10.y at all?  What bug does it fix?  And why
are you still using Xen on a 5.10.y kernel?  What prevents you from
moving to a newer one?

thanks,

greg k-h
Re: [PATCH v5.10.y] xen: replace xen_remap() with memremap()
Posted by Teddy Astie 1 month, 4 weeks ago
Le 02/09/2025 à 13:18, Greg Kroah-Hartman a écrit :
> On Tue, Sep 02, 2025 at 09:28:32AM +0000, Teddy Astie wrote:
>> From: Juergen Gross <jgross@suse.com>
>>
>> From: Juergen Gross <jgross@suse.com>
>>
>> [ upstream commit 41925b105e345ebc84cedb64f59d20cb14a62613 ]
>>
>> xen_remap() is used to establish mappings for frames not under direct
>> control of the kernel: for Xenstore and console ring pages, and for
>> grant pages of non-PV guests.
>>
>> Today xen_remap() is defined to use ioremap() on x86 (doing uncached
>> mappings), and ioremap_cache() on Arm (doing cached mappings).
>>
>> Uncached mappings for those use cases are bad for performance, so they
>> should be avoided if possible. As all use cases of xen_remap() don't
>> require uncached mappings (the mapped area is always physical RAM),
>> a mapping using the standard WB cache mode is fine.
>>
>> As sparse is flagging some of the xen_remap() use cases to be not
>> appropriate for iomem(), as the result is not annotated with the
>> __iomem modifier, eliminate xen_remap() completely and replace all
>> use cases with memremap() specifying the MEMREMAP_WB caching mode.
>>
>> xen_unmap() can be replaced with memunmap().
>>
>> Reported-by: kernel test robot <lkp@intel.com>
>> Signed-off-by: Juergen Gross <jgross@suse.com>
>> Reviewed-by: Boris Ostrovsky <boris.ostrovsky@oracle.com>
>> Acked-by: Stefano Stabellini <sstabellini@kernel.org>
>> Link: https://lore.kernel.org/r/20220530082634.6339-1-jgross@suse.com
>> Signed-off-by: Juergen Gross <jgross@suse.com>
>> Signed-off-by: Teddy Astie <teddy.astie@vates.tech> [backport to 5.10.y]
>> ---
> 
> Why is this needed for 5.10.y at all?  What bug does it fix?  And why
> are you still using Xen on a 5.10.y kernel?  What prevents you from
> moving to a newer one?
> 

This patch is only useful for virtual machines (DomU) that runs this 
Linux version (a notable Linux distribution with this kernel branch is 
Debian 11); it's not useful for Dom0 kernels.

On AMD platforms (and future Intel ones with TME); this patch along with 
[1] makes the caching attribute for access as WB instead of falling back 
to UC due to ioremap (within xen_remap) being used improving the 
performance as explained in the commit.

[1] x86/hvmloader: select xen platform pci MMIO BAR UC or WB MTRR cache 
attribute
https://xenbits.xen.org/gitweb/?p=xen.git;a=commit;h=22650d6054625be10172fe0c78b9cadd1a39bd63

> thanks,
> 
> greg k-h
> 

Teddy


--
Teddy Astie | Vates XCP-ng Developer

XCP-ng & Xen Orchestra - Vates solutions

web: https://vates.tech
Re: [PATCH v5.10.y] xen: replace xen_remap() with memremap()
Posted by Greg Kroah-Hartman 1 month, 3 weeks ago
On Tue, Sep 02, 2025 at 04:24:33PM +0000, Teddy Astie wrote:
> Le 02/09/2025 à 13:18, Greg Kroah-Hartman a écrit :
> > On Tue, Sep 02, 2025 at 09:28:32AM +0000, Teddy Astie wrote:
> >> From: Juergen Gross <jgross@suse.com>
> >>
> >> From: Juergen Gross <jgross@suse.com>
> >>
> >> [ upstream commit 41925b105e345ebc84cedb64f59d20cb14a62613 ]
> >>
> >> xen_remap() is used to establish mappings for frames not under direct
> >> control of the kernel: for Xenstore and console ring pages, and for
> >> grant pages of non-PV guests.
> >>
> >> Today xen_remap() is defined to use ioremap() on x86 (doing uncached
> >> mappings), and ioremap_cache() on Arm (doing cached mappings).
> >>
> >> Uncached mappings for those use cases are bad for performance, so they
> >> should be avoided if possible. As all use cases of xen_remap() don't
> >> require uncached mappings (the mapped area is always physical RAM),
> >> a mapping using the standard WB cache mode is fine.
> >>
> >> As sparse is flagging some of the xen_remap() use cases to be not
> >> appropriate for iomem(), as the result is not annotated with the
> >> __iomem modifier, eliminate xen_remap() completely and replace all
> >> use cases with memremap() specifying the MEMREMAP_WB caching mode.
> >>
> >> xen_unmap() can be replaced with memunmap().
> >>
> >> Reported-by: kernel test robot <lkp@intel.com>
> >> Signed-off-by: Juergen Gross <jgross@suse.com>
> >> Reviewed-by: Boris Ostrovsky <boris.ostrovsky@oracle.com>
> >> Acked-by: Stefano Stabellini <sstabellini@kernel.org>
> >> Link: https://lore.kernel.org/r/20220530082634.6339-1-jgross@suse.com
> >> Signed-off-by: Juergen Gross <jgross@suse.com>
> >> Signed-off-by: Teddy Astie <teddy.astie@vates.tech> [backport to 5.10.y]
> >> ---
> >
> > Why is this needed for 5.10.y at all?  What bug does it fix?  And why
> > are you still using Xen on a 5.10.y kernel?  What prevents you from
> > moving to a newer one?
> >
> 
> This patch is only useful for virtual machines (DomU) that runs this
> Linux version (a notable Linux distribution with this kernel branch is
> Debian 11); it's not useful for Dom0 kernels.
> 
> On AMD platforms (and future Intel ones with TME); this patch along with
> [1] makes the caching attribute for access as WB instead of falling back
> to UC due to ioremap (within xen_remap) being used improving the
> performance as explained in the commit.

So this is only a performance improvement?  One that people have not
noticed in over 3 years?  That does not feel like a real bugfix that
stable kernels should have to me.

Again, what is preventing you from just running 5.15.y in your system
instead?  Debian 11 is quite old as well, why not use Debian 13 or 12?
You only have one more year left of 5.10.y kernels so you need to
consider moving off of that as soon as possible.

thanks,

greg k-h