On Thu, Jun 18, 2026 at 05:21:20PM +0800, Ruidong Tian wrote:
> From: Tong Tiangen <[email protected]>
> 
> Currently, many scenarios that can tolerate memory errors when copying page
> have been supported in the kernel[1~9], all of which are implemented by
> copy_mc_[user]_highpage(). arm64 should also support this mechanism.
> 
> Due to mte, arm64 needs to have its own copy_mc_[user]_highpage()
> architecture implementation, macros __HAVE_ARCH_COPY_MC_HIGHPAGE and
> __HAVE_ARCH_COPY_MC_USER_HIGHPAGE have been added to control it.
> 
> Add new helper copy_mc_page() which provide a page copy implementation with
> hardware memory error safe. The code logic of copy_mc_page() is the same as
> copy_page(), the main difference is that the ldp insn of copy_mc_page()
> contains the fixup type EX_TYPE_KACCESS_SEA, therefore, the
> main logic is extracted to copy_page_template.S. In addition, the fixup of
> MOPS insn is not considered at present.
> 

Hi Tong,

[snip]

> diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
> index fe60738e5943..831b20d45893 100644
> --- a/arch/arm64/Kconfig
> +++ b/arch/arm64/Kconfig
> @@ -21,6 +21,7 @@ config ARM64
>       select ARCH_HAS_CACHE_LINE_SIZE
>       select ARCH_HAS_CC_PLATFORM
>       select ARCH_HAS_CPU_CACHE_INVALIDATE_MEMREGION
> +     select ARCH_HAS_COPY_MC if ACPI_APEI_GHES

ACPI_APEI_GHES seems like the wrong gate. The recovery this series enables 
hinges
on the in-kernel SEA claim path, and that is gated on CONFIG_ACPI_APEI_SEA,
not ACPI_APEI_GHES. From your own patch 2, include/acpi/ghes.h:

        #ifdef CONFIG_ACPI_APEI_SEA
        int ghes_notify_sea(enum ghes_exec_ctx context);
        #else
        static inline int ghes_notify_sea(enum ghes_exec_ctx context)
        { return -ENOENT; }
        #endif

ACPI_APEI_SEA is "bool, depends on ARM64 && ACPI_APEI_GHES, default y", so
the config ACPI_APEI_GHES=y, ACPI_APEI_SEA=n is reachable. In that build we
still select ARCH_HAS_COPY_MC, but ghes_notify_sea() is the -ENOENT stub,
apei_claim_sea() never claims the abort in do_sea(), and every KERNEL_SEA
fixup added below is unreachable dead code. 

Maybe:

select ARCH_HAS_COPY_MC if ACPI_APEI_SEA

>       select ARCH_HAS_CURRENT_STACK_POINTER
>       select ARCH_HAS_DEBUG_VIRTUAL
>       select ARCH_HAS_DEBUG_VM_PGTABLE
> diff --git a/arch/arm64/include/asm/asm-extable.h 
> b/arch/arm64/include/asm/asm-extable.h
> index 8450ec5a3af6..9305ea77482a 100644
> --- a/arch/arm64/include/asm/asm-extable.h
> +++ b/arch/arm64/include/asm/asm-extable.h
> @@ -10,6 +10,10 @@
>  #define EX_TYPE_ACCESS_ERR_ZERO              2
>  #define EX_TYPE_UACCESS_CPY          3
>  #define EX_TYPE_LOAD_UNALIGNED_ZEROPAD       4
> +/*
> + * Kernel access: used in kernel context for both regular load/store
> + * instructions and MOPS (memory copy/set) instructions.
> + */
>  #define EX_TYPE_KACCESS_SEA          5
>  
>  /* Data fields for EX_TYPE_ACCESS_ERR_ZERO */
> diff --git a/arch/arm64/include/asm/mte.h b/arch/arm64/include/asm/mte.h
> index 7f7b97e09996..a0b1757f4847 100644
> --- a/arch/arm64/include/asm/mte.h
> +++ b/arch/arm64/include/asm/mte.h
> @@ -98,6 +98,11 @@ static inline bool try_page_mte_tagging(struct page *page)
>  void mte_zero_clear_page_tags(void *addr);
>  void mte_sync_tags(pte_t pte, unsigned int nr_pages);
>  void mte_copy_page_tags(void *kto, const void *kfrom);
> +
> +#ifdef CONFIG_ARCH_HAS_COPY_MC
> +int mte_copy_mc_page_tags(void *kto, const void *kfrom);
> +#endif
> +
>  void mte_thread_init_user(void);
>  void mte_thread_switch(struct task_struct *next);
>  void mte_cpu_setup(void);
> @@ -134,6 +139,10 @@ static inline void mte_sync_tags(pte_t pte, unsigned int 
> nr_pages)
>  static inline void mte_copy_page_tags(void *kto, const void *kfrom)
>  {
>  }
> +static inline int mte_copy_mc_page_tags(void *kto, const void *kfrom)
> +{
> +     return 0;
> +}
>  static inline void mte_thread_init_user(void)
>  {
>  }
> diff --git a/arch/arm64/include/asm/page.h b/arch/arm64/include/asm/page.h
> index e25d0d18f6d7..5c4c9f974b68 100644
> --- a/arch/arm64/include/asm/page.h
> +++ b/arch/arm64/include/asm/page.h
> @@ -29,6 +29,18 @@ void copy_user_highpage(struct page *to, struct page *from,
>  void copy_highpage(struct page *to, struct page *from);
>  #define __HAVE_ARCH_COPY_HIGHPAGE
>  
> +#ifdef CONFIG_ARCH_HAS_COPY_MC
> +int copy_mc_page(void *to, const void *from);
> +#define __HAVE_ARCH_COPY_MC_PAGE
> +
> +int copy_mc_highpage(struct page *to, struct page *from);
> +#define __HAVE_ARCH_COPY_MC_HIGHPAGE
> +
> +int copy_mc_user_highpage(struct page *to, struct page *from,
> +             unsigned long vaddr, struct vm_area_struct *vma);
> +#define __HAVE_ARCH_COPY_MC_USER_HIGHPAGE
> +#endif
> +
>  struct folio *vma_alloc_zeroed_movable_folio(struct vm_area_struct *vma,
>                                               unsigned long vaddr);
>  #define vma_alloc_zeroed_movable_folio vma_alloc_zeroed_movable_folio
> diff --git a/arch/arm64/lib/Makefile b/arch/arm64/lib/Makefile
> index 448c917494f3..1f4c3f743a20 100644
> --- a/arch/arm64/lib/Makefile
> +++ b/arch/arm64/lib/Makefile
> @@ -7,6 +7,8 @@ lib-y         := clear_user.o delay.o copy_from_user.o        
>         \
>  
>  lib-$(CONFIG_ARCH_HAS_UACCESS_FLUSHCACHE) += uaccess_flushcache.o
>  
> +lib-$(CONFIG_ARCH_HAS_COPY_MC) += copy_mc_page.o
> +
>  obj-$(CONFIG_FUNCTION_ERROR_INJECTION) += error-inject.o
>  
>  obj-$(CONFIG_ARM64_MTE) += mte.o
> diff --git a/arch/arm64/lib/copy_mc_page.S b/arch/arm64/lib/copy_mc_page.S
> new file mode 100644
> index 000000000000..f936e0c98611
> --- /dev/null
> +++ b/arch/arm64/lib/copy_mc_page.S
> @@ -0,0 +1,44 @@
> +/* SPDX-License-Identifier: GPL-2.0-only */
> +
> +#include <linux/linkage.h>
> +#include <linux/const.h>
> +#include <asm/assembler.h>
> +#include <asm/page.h>
> +#include <asm/cpufeature.h>
> +#include <asm/alternative.h>
> +#include <asm/asm-extable.h>
> +#include <asm/asm-uaccess.h>
> +
> +/*
> + * Copy a page from src to dest (both are page aligned) with memory error 
> safe
> + *
> + * Parameters:
> + *   x0 - dest
> + *   x1 - src
> + * Returns:
> + *   x0 - Return 0 if copy success, or -EFAULT if anything goes wrong
> + *        while copying.
> + */
> +     .macro ldp1 reg1, reg2, ptr, val
> +     KERNEL_SEA(9998f, ldp \reg1, \reg2, [\ptr, \val])
> +     .endm
> +
> +     .macro cpy1 dst, src, count
> +     .arch_extension mops
> +     KERNEL_SEA(9998f, cpypwn [\dst]!, [\src]!, \count!)
> +     KERNEL_SEA(9998f, cpymwn [\dst]!, [\src]!, \count!)
> +     KERNEL_SEA(9998f, cpyewn [\dst]!, [\src]!, \count!)
> +     .endm

You wrap the MOPS cpy* here (and in copy_page_template.S) in KERNEL_SEA, but
the commit message still says "the fixup of MOPS insn is not considered at
present." 

please reconcile the changelog with the code, and mention the FEAT_MOPS 
fixup support explicitly.

> +
> +SYM_FUNC_START(__pi_copy_mc_page)
> +#include "copy_page_template.S"
> +
> +     mov x0, #0
> +     ret
> +
> +9998:        mov x0, #-EFAULT
> +     ret
> +
> +SYM_FUNC_END(__pi_copy_mc_page)
> +SYM_FUNC_ALIAS(copy_mc_page, __pi_copy_mc_page)
> +EXPORT_SYMBOL(copy_mc_page)
> diff --git a/arch/arm64/lib/copy_page.S b/arch/arm64/lib/copy_page.S
> index e6374e7e5511..e520777b5150 100644
> --- a/arch/arm64/lib/copy_page.S
> +++ b/arch/arm64/lib/copy_page.S
> @@ -17,65 +17,20 @@
>   *   x0 - dest
>   *   x1 - src
>   */
> -SYM_FUNC_START(__pi_copy_page)
> -#ifdef CONFIG_AS_HAS_MOPS
> -     .arch_extension mops
> -alternative_if_not ARM64_HAS_MOPS
> -     b       .Lno_mops
> -alternative_else_nop_endif
> -
> -     mov     x2, #PAGE_SIZE
> -     cpypwn  [x0]!, [x1]!, x2!
> -     cpymwn  [x0]!, [x1]!, x2!
> -     cpyewn  [x0]!, [x1]!, x2!
> -     ret
> -.Lno_mops:
> -#endif
> -     ldp     x2, x3, [x1]
> -     ldp     x4, x5, [x1, #16]
> -     ldp     x6, x7, [x1, #32]
> -     ldp     x8, x9, [x1, #48]
> -     ldp     x10, x11, [x1, #64]
> -     ldp     x12, x13, [x1, #80]
> -     ldp     x14, x15, [x1, #96]
> -     ldp     x16, x17, [x1, #112]
> -
> -     add     x0, x0, #256
> -     add     x1, x1, #128
> -1:
> -     tst     x0, #(PAGE_SIZE - 1)
>  
> -     stnp    x2, x3, [x0, #-256]
> -     ldp     x2, x3, [x1]
> -     stnp    x4, x5, [x0, #16 - 256]
> -     ldp     x4, x5, [x1, #16]
> -     stnp    x6, x7, [x0, #32 - 256]
> -     ldp     x6, x7, [x1, #32]
> -     stnp    x8, x9, [x0, #48 - 256]
> -     ldp     x8, x9, [x1, #48]
> -     stnp    x10, x11, [x0, #64 - 256]
> -     ldp     x10, x11, [x1, #64]
> -     stnp    x12, x13, [x0, #80 - 256]
> -     ldp     x12, x13, [x1, #80]
> -     stnp    x14, x15, [x0, #96 - 256]
> -     ldp     x14, x15, [x1, #96]
> -     stnp    x16, x17, [x0, #112 - 256]
> -     ldp     x16, x17, [x1, #112]
> +     .macro ldp1 reg1, reg2, ptr, val
> +     ldp \reg1, \reg2, [\ptr, \val]
> +     .endm
>  
> -     add     x0, x0, #128
> -     add     x1, x1, #128
> -
> -     b.ne    1b
> -
> -     stnp    x2, x3, [x0, #-256]
> -     stnp    x4, x5, [x0, #16 - 256]
> -     stnp    x6, x7, [x0, #32 - 256]
> -     stnp    x8, x9, [x0, #48 - 256]
> -     stnp    x10, x11, [x0, #64 - 256]
> -     stnp    x12, x13, [x0, #80 - 256]
> -     stnp    x14, x15, [x0, #96 - 256]
> -     stnp    x16, x17, [x0, #112 - 256]
> +     .macro cpy1 dst, src, count
> +     .arch_extension mops
> +     cpypwn [\dst]!, [\src]!, \count!
> +     cpymwn [\dst]!, [\src]!, \count!
> +     cpyewn [\dst]!, [\src]!, \count!
> +     .endm
>  
> +SYM_FUNC_START(__pi_copy_page)
> +#include "copy_page_template.S"
>       ret
>  SYM_FUNC_END(__pi_copy_page)
> new file mode 100644
> index 000000000000..e5afbeaaad25
> --- /dev/null
> +++ b/arch/arm64/lib/copy_page_template.S
> @@ -0,0 +1,70 @@
> +/* SPDX-License-Identifier: GPL-2.0-only */
> +/*
> + * Copyright (C) 2012 ARM Ltd.
> + */
> +
> +/*
> + * Copy a page from src to dest (both are page aligned)
> + *
> + * Parameters:
> + *   x0 - dest
> + *   x1 - src
> + */
> +dstin        .req    x0
> +src  .req    x1
> +
> +#ifdef CONFIG_AS_HAS_MOPS
> +alternative_if_not ARM64_HAS_MOPS
> +     b       .Lno_mops
> +alternative_else_nop_endif
> +     mov     x2, #PAGE_SIZE
> +     cpy1    dst, src, x2

Should dst above be dstin instead?

Also the cpy1 instruction sits outside the alternatives block

Have you compiled with a toolchain that doesn't have MOPS? I suspect it
might break builds.

> +     b       .Lexitfunc
> +.Lno_mops:
> +#endif
> +
> +     ldp1    x2, x3, x1, #0
> +     ldp1    x4, x5, x1, #16
> +     ldp1    x6, x7, x1, #32
> +     ldp1    x8, x9, x1, #48
> +     ldp1    x10, x11, x1, #64
> +     ldp1    x12, x13, x1, #80
> +     ldp1    x14, x15, x1, #96
> +     ldp1    x16, x17, x1, #112
> +
> +     add     x0, x0, #256
> +     add     x1, x1, #128
> +1:
> +     tst     x0, #(PAGE_SIZE - 1)
> +
> +     stnp    x2, x3, [x0, #-256]
> +     ldp1    x2, x3, x1, #0
> +     stnp    x4, x5, [x0, #16 - 256]
> +     ldp1    x4, x5, x1, #16
> +     stnp    x6, x7, [x0, #32 - 256]
> +     ldp1    x6, x7, x1, #32
> +     stnp    x8, x9, [x0, #48 - 256]
> +     ldp1    x8, x9, x1, #48
> +     stnp    x10, x11, [x0, #64 - 256]
> +     ldp1    x10, x11, x1, #64
> +     stnp    x12, x13, [x0, #80 - 256]
> +     ldp1    x12, x13, x1, #80
> +     stnp    x14, x15, [x0, #96 - 256]
> +     ldp1    x14, x15, x1, #96
> +     stnp    x16, x17, [x0, #112 - 256]
> +     ldp1    x16, x17, x1, #112
> +
> +     add     x0, x0, #128
> +     add     x1, x1, #128
> +
> +     b.ne    1b
> +
> +     stnp    x2, x3, [x0, #-256]
> +     stnp    x4, x5, [x0, #16 - 256]
> +     stnp    x6, x7, [x0, #32 - 256]
> +     stnp    x8, x9, [x0, #48 - 256]
> +     stnp    x10, x11, [x0, #64 - 256]
> +     stnp    x12, x13, [x0, #80 - 256]
> +     stnp    x14, x15, [x0, #96 - 256]
> +     stnp    x16, x17, [x0, #112 - 256]
> +.Lexitfunc:
> diff --git a/arch/arm64/lib/mte.S b/arch/arm64/lib/mte.S
> index 5018ac03b6bf..1afe3ef1502c 100644
> --- a/arch/arm64/lib/mte.S
> +++ b/arch/arm64/lib/mte.S
> @@ -80,6 +80,35 @@ SYM_FUNC_START(mte_copy_page_tags)
>       ret
>  SYM_FUNC_END(mte_copy_page_tags)
>  
> +#ifdef CONFIG_ARCH_HAS_COPY_MC
> +/*
> + * Copy the tags from the source page to the destination one with memory 
> error safe
> + *   x0 - address of the destination page
> + *   x1 - address of the source page
> + * Returns:
> + *   x0 - Return 0 if copy success, or
> + *        -EFAULT if anything goes wrong while copying.
> + */
> +SYM_FUNC_START(mte_copy_mc_page_tags)
> +     mov     x2, x0
> +     mov     x3, x1
> +     multitag_transfer_size x5, x6
> +1:
> +KERNEL_SEA(2f, ldgm  x4, [x3])
> +     stgm    x4, [x2]
> +     add     x2, x2, x5
> +     add     x3, x3, x5
> +     tst     x2, #(PAGE_SIZE - 1)
> +     b.ne    1b
> +
> +     mov x0, #0
> +     ret
> +
> +2:   mov x0, #-EFAULT
> +     ret
> +SYM_FUNC_END(mte_copy_mc_page_tags)
> +#endif
> +
>  /*
>   * Read tags from a user buffer (one tag per byte) and set the corresponding
>   * tags at the given kernel address. Used by PTRACE_POKEMTETAGS.
> diff --git a/arch/arm64/mm/copypage.c b/arch/arm64/mm/copypage.c
> index cd5912ba617b..c22918ed0f3c 100644
> --- a/arch/arm64/mm/copypage.c
> +++ b/arch/arm64/mm/copypage.c
> @@ -72,3 +72,83 @@ void copy_user_highpage(struct page *to, struct page *from,
>       flush_dcache_page(to);
>  }
>  EXPORT_SYMBOL_GPL(copy_user_highpage);
> +
> +#ifdef CONFIG_ARCH_HAS_COPY_MC
> +/*
> + * Return -EFAULT if anything goes wrong while copying page or mte.
> + */
> +int copy_mc_highpage(struct page *to, struct page *from)
> +{
> +     void *kto = page_address(to);
> +     void *kfrom = page_address(from);
> +     struct folio *src = page_folio(from);
> +     struct folio *dst = page_folio(to);
> +     unsigned int i, nr_pages;
> +     int ret;
> +
> +     ret = copy_mc_page(kto, kfrom);
> +     if (ret)
> +             return -EFAULT;
> +
> +     if (kasan_hw_tags_enabled())
> +             page_kasan_tag_reset(to);
> +
> +     if (!system_supports_mte())
> +             return 0;
> +
> +     if (folio_test_hugetlb(src)) {
> +             if (!folio_test_hugetlb_mte_tagged(src) ||
> +                 from != folio_page(src, 0))
> +                     return 0;
> +
> +             WARN_ON_ONCE(!folio_try_hugetlb_mte_tagging(dst));
> +
> +             /*
> +              * Populate tags for all subpages.
> +              *
> +              * Don't assume the first page is head page since
> +              * huge page copy may start from any subpage.
> +              */
> +             nr_pages = folio_nr_pages(src);
> +             for (i = 0; i < nr_pages; i++) {
> +                     kfrom = page_address(folio_page(src, i));
> +                     kto = page_address(folio_page(dst, i));
> +                     ret = mte_copy_mc_page_tags(kto, kfrom);
> +                     if (ret)
> +                             return -EFAULT;
> +             }
> +             folio_set_hugetlb_mte_tagged(dst);
> +     } else if (page_mte_tagged(from)) {
> +             /* It's a new page, shouldn't have been tagged yet */
> +             WARN_ON_ONCE(!try_page_mte_tagging(to));
> +
> +             ret = mte_copy_mc_page_tags(kto, kfrom);
> +             if (ret)
> +                     return -EFAULT;
> +             set_page_mte_tagged(to);
> +     }

I'm not certain, but this is a verbatim copy of copy_highpage()'s MTE
body? The only delta's are copy_mc_page()/mte_opy_mc_page_tags() return
an error instead of void. Leaves 2 copies.. is it possible to refactor
them?

Also on the -EFAULT paths, is the MTE tag state left half-updated?
like set_page_mte_tagged()/folio_set_hugetlb_mte() is never reached.

I'm not an MTE expert.. just checking, love to be wrong.

> +     /*
> +      * memory_failure_queue() is not called here because on arm64
> +      * the firmware (GHES) has already reported the hardware memory
> +      * error and queued the page for memory_failure() handling via
> +      * ghes_do_memory_failure().
> +      */
> +     return 0;
> +}

So copy_mc_page()'s -EFAULT can only be produced by the KERNEL_SEA fixup
which is only reachable via fixup_exception_me() on the do_sea() path
after apei_claim_sea() has already queued memory_failure(). 

Is the ordering gauranteed? otherwise there might be a case where its
not offlined.

Cheers,
Ashok

Reply via email to