[GIT PULL] arm64 updates for 6.13-rc1

[GIT PULL] arm64 updates for 6.13-rc1

[Catalin Marinas]

Hi Linus,

Here are the arm64 updates for 6.13. The major features are support to
run Linux in a protected VM (a.k.a. realm) under the Arm CCA and the
user Guarded Control Stack (GCS). There are a few smaller scale
additions for in-kernel memcpy instructions, MTE hugetlbfs support,
optimised CRC32, non-leaf pmd_young(), the usual perf/PMU updates and
various fixes and cleanups. We are also introducing HWCAP3 as we'll
likely run out of HWCAP2 bits in a year or so.

There's a trivial conflict with mainline due to a recent upstream fix:
commit 5baf8b037deb ("mm: refactor arch_calc_vm_flag_bits() and arm64
MTE handling") and the MTE hugetlbfs support. My fixup:

diff --cc arch/arm64/include/asm/mman.h
index 798d965760d4,1dbfb56cb313..e1572482fae8
--- a/arch/arm64/include/asm/mman.h
+++ b/arch/arm64/include/asm/mman.h
@@@ -42,7 -39,7 +42,7 @@@ static inline unsigned long arch_calc_v
  	 * filesystem supporting MTE (RAM-based).
  	 */
  	if (system_supports_mte() &&
- 	    ((flags & MAP_ANONYMOUS) || shmem_file(file)))
 -	    (flags & (MAP_ANONYMOUS | MAP_HUGETLB)))
++	    ((flags & (MAP_ANONYMOUS | MAP_HUGETLB)) || shmem_file(file)))
  		return VM_MTE_ALLOWED;
  
  	return 0;

Depending on the order you pull other requests in, there are a couple
more conflicts with the trace and KVM trees. The slightly more
complicated one is in arch/arm64/kvm/guest.c - the MTE hugetlbfs support
in the arm64 tree conflicting with the __gfn_to_page() use in
kvm_vm_ioctl_mte_copy_tags() from the KVM tree. The resolution in -next
is correct.

Since the arm64 tree has two bases, -rc1 mostly but -rc3 for the perf
branch, the diffstat below is generated against 6.12.

Thanks.

The following changes since commit 2e8a1acea8597ff42189ea94f0a63fa58640223d:

  arm64: signal: Improve POR_EL0 handling to avoid uaccess failures (2024-10-29 17:59:12 +0000)

are available in the Git repository at:

  git://git.kernel.org/pub/scm/linux/kernel/git/arm64/linux tags/arm64-upstream

for you to fetch changes up to 83ef4a378e563d085ddd7214c2a393116b5f3435:

  Merge branch 'for-next/pkey-signal' into for-next/core (2024-11-14 12:07:30 +0000)

----------------------------------------------------------------
arm64 updates for 6.13:

* Support for running Linux in a protected VM under the Arm Confidential
  Compute Architecture (CCA)

* Guarded Control Stack user-space support. Current patches follow the
  x86 ABI of implicitly creating a shadow stack on clone(). Subsequent
  patches (already on the list) will add support for clone3() allowing
  finer-grained control of the shadow stack size and placement from libc

* AT_HWCAP3 support (not running out of HWCAP2 bits yet but we are
  getting close with the upcoming dpISA support)

* Other arch features:

  - In-kernel use of the memcpy instructions, FEAT_MOPS (previously only
    exposed to user; uaccess support not merged yet)

  - MTE: hugetlbfs support and the corresponding kselftests

  - Optimise CRC32 using the PMULL instructions

  - Support for FEAT_HAFT enabling ARCH_HAS_NONLEAF_PMD_YOUNG

  - Optimise the kernel TLB flushing to use the range operations

  - POE/pkey (permission overlays): further cleanups after bringing the
    signal handler in line with the x86 behaviour for 6.12

* arm64 perf updates:

  - Support for the NXP i.MX91 PMU in the existing IMX driver

  - Support for Ampere SoCs in the Designware PCIe PMU driver

  - Support for Marvell's 'PEM' PCIe PMU present in the 'Odyssey' SoC

  - Support for Samsung's 'Mongoose' CPU PMU

  - Support for PMUv3.9 finer-grained userspace counter access control

  - Switch back to platform_driver::remove() now that it returns 'void'

  - Add some missing events for the CXL PMU driver

* Miscellaneous arm64 fixes/cleanups:

  - Page table accessors cleanup: type updates, drop unused macros,
    reorganise arch_make_huge_pte() and clean up pte_mkcont(), sanity
    check addresses before runtime P4D/PUD folding

  - Command line override for ID_AA64MMFR0_EL1.ECV (advertising the
    FEAT_ECV for the generic timers) allowing Linux to boot with
    firmware deployments that don't set SCTLR_EL3.ECVEn

  - ACPI/arm64: tighten the check for the array of platform timer
    structures and adjust the error handling procedure in
    gtdt_parse_timer_block()

  - Optimise the cache flush for the uprobes xol slot (skip if no
    change) and other uprobes/kprobes cleanups

  - Fix the context switching of tpidrro_el0 when kpti is enabled

  - Dynamic shadow call stack fixes

  - Sysreg updates

  - Various arm64 kselftest improvements

----------------------------------------------------------------
 Documentation/admin-guide/kernel-parameters.txt    |   3 +
 Documentation/admin-guide/perf/index.rst           |   1 +
 Documentation/admin-guide/perf/mrvl-pem-pmu.rst    |  56 ++
 Documentation/arch/arm64/arm-cca.rst               |  69 ++
 Documentation/arch/arm64/booting.rst               |  38 ++
 Documentation/arch/arm64/elf_hwcaps.rst            |  10 +-
 Documentation/arch/arm64/gcs.rst                   | 227 +++++++
 Documentation/arch/arm64/index.rst                 |   3 +
 Documentation/arch/arm64/mops.rst                  |  44 ++
 Documentation/arch/arm64/sme.rst                   |   4 +
 Documentation/arch/arm64/sve.rst                   |   4 +
 Documentation/devicetree/bindings/arm/pmu.yaml     |   1 +
 .../devicetree/bindings/perf/fsl-imx-ddr.yaml      |   4 +-
 Documentation/filesystems/proc.rst                 |   2 +-
 MAINTAINERS                                        |   6 +
 arch/arm/include/asm/arm_pmuv3.h                   |   8 +
 arch/arm64/Kconfig                                 |  43 ++
 arch/arm64/include/asm/arm_pmuv3.h                 |  10 +
 arch/arm64/include/asm/assembler.h                 |   7 -
 arch/arm64/include/asm/cpucaps.h                   |   2 +
 arch/arm64/include/asm/cpufeature.h                |  18 +-
 arch/arm64/include/asm/daifflags.h                 |   2 +-
 arch/arm64/include/asm/debug-monitors.h            |   1 +
 arch/arm64/include/asm/el2_setup.h                 |  30 +
 arch/arm64/include/asm/esr.h                       |  28 +-
 arch/arm64/include/asm/exception.h                 |   3 +
 arch/arm64/include/asm/gcs.h                       | 107 +++
 arch/arm64/include/asm/hugetlb.h                   |   8 +
 arch/arm64/include/asm/hwcap.h                     |   7 +-
 arch/arm64/include/asm/insn.h                      |   6 +
 arch/arm64/include/asm/io.h                        |   8 +
 arch/arm64/include/asm/kernel-pgtable.h            |   1 -
 arch/arm64/include/asm/mem_encrypt.h               |   9 +
 arch/arm64/include/asm/mman.h                      |  25 +-
 arch/arm64/include/asm/mmu_context.h               |   9 +
 arch/arm64/include/asm/mte.h                       |  67 ++
 arch/arm64/include/asm/pgalloc.h                   |  12 +-
 arch/arm64/include/asm/pgtable-hwdef.h             |   4 +
 arch/arm64/include/asm/pgtable-prot.h              |  19 +-
 arch/arm64/include/asm/pgtable.h                   |  31 +-
 arch/arm64/include/asm/probes.h                    |  11 +-
 arch/arm64/include/asm/processor.h                 |  57 +-
 arch/arm64/include/asm/ptrace.h                    |  22 +-
 arch/arm64/include/asm/rsi.h                       |  68 ++
 arch/arm64/include/asm/rsi_cmds.h                  | 160 +++++
 arch/arm64/include/asm/rsi_smc.h                   | 193 ++++++
 arch/arm64/include/asm/scs.h                       |   8 +-
 arch/arm64/include/asm/set_memory.h                |   3 +
 arch/arm64/include/asm/stacktrace/common.h         |  74 ++-
 arch/arm64/include/asm/stacktrace/frame.h          |  48 ++
 arch/arm64/include/asm/sysreg.h                    |  20 +
 arch/arm64/include/asm/tlbflush.h                  |  43 +-
 arch/arm64/include/asm/uaccess.h                   |  40 ++
 arch/arm64/include/uapi/asm/hwcap.h                |   7 +-
 arch/arm64/include/uapi/asm/ptrace.h               |   8 +
 arch/arm64/include/uapi/asm/sigcontext.h           |   9 +
 arch/arm64/kernel/Makefile                         |   3 +-
 arch/arm64/kernel/asm-offsets.c                    |  27 +-
 arch/arm64/kernel/cpufeature.c                     |  45 ++
 arch/arm64/kernel/cpuinfo.c                        |   1 +
 arch/arm64/kernel/debug-monitors.c                 |  10 +-
 arch/arm64/kernel/efi.c                            |  12 +-
 arch/arm64/kernel/entry-common.c                   |  35 +
 arch/arm64/kernel/entry.S                          |  16 +-
 arch/arm64/kernel/fpsimd.c                         |   2 +-
 arch/arm64/kernel/head.S                           |   3 +
 arch/arm64/kernel/hibernate.c                      |   6 +
 arch/arm64/kernel/module.c                         |  10 +-
 arch/arm64/kernel/mte.c                            |  27 +-
 arch/arm64/kernel/pi/idreg-override.c              |  12 +
 arch/arm64/kernel/pi/map_range.c                   |   2 +-
 arch/arm64/kernel/pi/patch-scs.c                   |  95 ++-
 arch/arm64/kernel/probes/decode-insn.c             |  22 +-
 arch/arm64/kernel/probes/decode-insn.h             |   2 +-
 arch/arm64/kernel/probes/kprobes.c                 |  39 +-
 arch/arm64/kernel/probes/simulate-insn.c           |   6 +
 arch/arm64/kernel/probes/simulate-insn.h           |   1 +
 arch/arm64/kernel/probes/uprobes.c                 |  12 +-
 arch/arm64/kernel/process.c                        | 101 ++-
 arch/arm64/kernel/ptrace.c                         |  74 ++-
 arch/arm64/kernel/rsi.c                            | 142 ++++
 arch/arm64/kernel/setup.c                          |   3 +
 arch/arm64/kernel/signal.c                         | 235 ++++++-
 arch/arm64/kernel/stacktrace.c                     | 176 ++++-
 arch/arm64/kernel/traps.c                          |  18 +
 arch/arm64/kernel/vmlinux.lds.S                    |   6 +-
 arch/arm64/kvm/guest.c                             |  16 +-
 arch/arm64/kvm/mmu.c                               |  11 +
 arch/arm64/lib/Makefile                            |   2 +-
 arch/arm64/lib/clear_page.S                        |  13 +
 arch/arm64/lib/copy_page.S                         |  13 +
 arch/arm64/lib/crc32-glue.c                        |  82 +++
 arch/arm64/lib/crc32.S                             | 346 ++++++++--
 arch/arm64/lib/memcpy.S                            |  19 +-
 arch/arm64/lib/memset.S                            |  20 +-
 arch/arm64/mm/Makefile                             |   1 +
 arch/arm64/mm/copypage.c                           |  27 +-
 arch/arm64/mm/fault.c                              |  40 ++
 arch/arm64/mm/fixmap.c                             |   9 +-
 arch/arm64/mm/gcs.c                                | 254 +++++++
 arch/arm64/mm/hugetlbpage.c                        |  21 +-
 arch/arm64/mm/init.c                               |  10 +-
 arch/arm64/mm/mmap.c                               |   9 +-
 arch/arm64/mm/mmu.c                                |  10 +-
 arch/arm64/mm/pageattr.c                           |  98 ++-
 arch/arm64/mm/proc.S                               |  19 +-
 arch/arm64/mm/ptdump.c                             |   8 +-
 arch/arm64/tools/cpucaps                           |   2 +
 arch/arm64/tools/sysreg                            |  12 +
 arch/x86/Kconfig                                   |   1 +
 arch/x86/include/uapi/asm/mman.h                   |   3 -
 drivers/acpi/arm64/gtdt.c                          |  33 +-
 drivers/perf/Kconfig                               |   7 +
 drivers/perf/Makefile                              |   1 +
 drivers/perf/alibaba_uncore_drw_pmu.c              |   2 +-
 drivers/perf/amlogic/meson_g12_ddr_pmu.c           |   2 +-
 drivers/perf/arm-cci.c                             |   2 +-
 drivers/perf/arm-ccn.c                             |   2 +-
 drivers/perf/arm-cmn.c                             |   2 +-
 drivers/perf/arm_cspmu/arm_cspmu.c                 |   2 +-
 drivers/perf/arm_dmc620_pmu.c                      |   2 +-
 drivers/perf/arm_dsu_pmu.c                         |   2 +-
 drivers/perf/arm_pmuv3.c                           |  32 +-
 drivers/perf/arm_smmuv3_pmu.c                      |   2 +-
 drivers/perf/arm_spe_pmu.c                         |   2 +-
 drivers/perf/cxl_pmu.c                             |   9 +-
 drivers/perf/dwc_pcie_pmu.c                        |  16 +-
 drivers/perf/fsl_imx8_ddr_perf.c                   |   2 +-
 drivers/perf/fsl_imx9_ddr_perf.c                   |   7 +-
 drivers/perf/hisilicon/hisi_uncore_cpa_pmu.c       |   2 +-
 drivers/perf/hisilicon/hisi_uncore_ddrc_pmu.c      |   2 +-
 drivers/perf/hisilicon/hisi_uncore_hha_pmu.c       |   2 +-
 drivers/perf/hisilicon/hisi_uncore_l3c_pmu.c       |   2 +-
 drivers/perf/hisilicon/hisi_uncore_pa_pmu.c        |   2 +-
 drivers/perf/hisilicon/hisi_uncore_sllc_pmu.c      |   2 +-
 drivers/perf/marvell_cn10k_ddr_pmu.c               |   2 +-
 drivers/perf/marvell_cn10k_tad_pmu.c               |   2 +-
 drivers/perf/marvell_pem_pmu.c                     | 425 ++++++++++++
 drivers/perf/qcom_l2_pmu.c                         |   2 +-
 drivers/perf/thunderx2_pmu.c                       |   2 +-
 drivers/perf/xgene_pmu.c                           |   2 +-
 drivers/virt/coco/Kconfig                          |   2 +
 drivers/virt/coco/Makefile                         |   1 +
 drivers/virt/coco/arm-cca-guest/Kconfig            |  11 +
 drivers/virt/coco/arm-cca-guest/Makefile           |   2 +
 drivers/virt/coco/arm-cca-guest/arm-cca-guest.c    | 224 +++++++
 fs/binfmt_elf.c                                    |   6 +
 fs/binfmt_elf_fdpic.c                              |   6 +
 fs/compat_binfmt_elf.c                             |  10 +
 fs/hugetlbfs/inode.c                               |   2 +-
 fs/proc/task_mmu.c                                 |   2 +-
 include/linux/cpuhotplug.h                         |   1 +
 include/linux/mm.h                                 |  18 +-
 include/linux/perf/arm_pmuv3.h                     |   1 +
 include/uapi/asm-generic/mman.h                    |   4 +
 include/uapi/linux/elf.h                           |   1 +
 include/uapi/linux/prctl.h                         |  22 +
 kernel/sys.c                                       |  30 +
 mm/Kconfig                                         |   6 +
 tools/testing/selftests/arm64/Makefile             |   2 +-
 tools/testing/selftests/arm64/abi/hwcap.c          |  25 +-
 tools/testing/selftests/arm64/abi/syscall-abi.c    |   8 +-
 tools/testing/selftests/arm64/fp/assembler.h       |  15 +
 tools/testing/selftests/arm64/fp/fp-ptrace-asm.S   |  41 +-
 tools/testing/selftests/arm64/fp/fp-ptrace.c       | 161 ++++-
 tools/testing/selftests/arm64/fp/fp-ptrace.h       |  12 +
 tools/testing/selftests/arm64/fp/fp-stress.c       |  49 +-
 tools/testing/selftests/arm64/fp/fpsimd-test.S     |   6 +-
 tools/testing/selftests/arm64/fp/kernel-test.c     |   4 +
 tools/testing/selftests/arm64/fp/sme-inst.h        |   2 +
 tools/testing/selftests/arm64/fp/sve-ptrace.c      |  16 +-
 tools/testing/selftests/arm64/fp/sve-test.S        |  10 +-
 tools/testing/selftests/arm64/fp/za-ptrace.c       |   8 +-
 tools/testing/selftests/arm64/fp/za-test.S         |  15 +-
 tools/testing/selftests/arm64/fp/zt-ptrace.c       |   8 +-
 tools/testing/selftests/arm64/fp/zt-test.S         |  15 +-
 tools/testing/selftests/arm64/gcs/.gitignore       |   7 +
 tools/testing/selftests/arm64/gcs/Makefile         |  30 +
 tools/testing/selftests/arm64/gcs/asm-offsets.h    |   0
 tools/testing/selftests/arm64/gcs/basic-gcs.c      | 357 ++++++++++
 tools/testing/selftests/arm64/gcs/gcs-locking.c    | 200 ++++++
 .../selftests/arm64/gcs/gcs-stress-thread.S        | 311 +++++++++
 tools/testing/selftests/arm64/gcs/gcs-stress.c     | 530 +++++++++++++++
 tools/testing/selftests/arm64/gcs/gcs-util.h       | 100 +++
 tools/testing/selftests/arm64/gcs/gcspushm.S       |  96 +++
 tools/testing/selftests/arm64/gcs/gcsstr.S         |  99 +++
 tools/testing/selftests/arm64/gcs/libc-gcs.c       | 728 +++++++++++++++++++++
 .../selftests/arm64/mte/check_buffer_fill.c        |   4 +-
 .../selftests/arm64/mte/check_hugetlb_options.c    | 285 ++++++++
 tools/testing/selftests/arm64/mte/check_prctl.c    |   2 +-
 .../selftests/arm64/mte/check_tags_inclusion.c     |   4 +-
 .../testing/selftests/arm64/mte/mte_common_util.c  |  29 +-
 .../testing/selftests/arm64/mte/mte_common_util.h  |   6 +-
 tools/testing/selftests/arm64/pauth/Makefile       |   6 +
 tools/testing/selftests/arm64/pauth/pac.c          |   5 +-
 tools/testing/selftests/arm64/signal/.gitignore    |   1 +
 tools/testing/selftests/arm64/signal/Makefile      |   2 +-
 tools/testing/selftests/arm64/signal/sve_helpers.h |  13 +
 .../testing/selftests/arm64/signal/test_signals.c  |  17 +-
 .../testing/selftests/arm64/signal/test_signals.h  |   6 +
 .../selftests/arm64/signal/test_signals_utils.c    |  32 +-
 .../selftests/arm64/signal/test_signals_utils.h    |  39 ++
 .../arm64/signal/testcases/gcs_exception_fault.c   |  62 ++
 .../selftests/arm64/signal/testcases/gcs_frame.c   |  88 +++
 .../arm64/signal/testcases/gcs_write_fault.c       |  67 ++
 .../selftests/arm64/signal/testcases/ssve_regs.c   |   5 +
 .../selftests/arm64/signal/testcases/testcases.c   |   7 +
 .../selftests/arm64/signal/testcases/testcases.h   |   1 +
 .../selftests/arm64/signal/testcases/za_regs.c     |   5 +
 tools/testing/selftests/mm/Makefile                |   8 +-
 tools/testing/selftests/mm/pkey-arm64.h            |   3 +-
 tools/testing/selftests/mm/pkey-helpers.h          |   7 +
 tools/testing/selftests/mm/pkey-x86.h              |   2 +
 tools/testing/selftests/mm/pkey_sighandler_tests.c | 115 +++-
 214 files changed, 7911 insertions(+), 585 deletions(-)

Re: [GIT PULL] arm64 updates for 6.13-rc1

[pr-tracker-bot]

The pull request you sent on Mon, 18 Nov 2024 10:06:23 +0000:

> git://git.kernel.org/pub/scm/linux/kernel/git/arm64/linux tags/arm64-upstream

has been merged into torvalds/linux.git:
https://git.kernel.org/torvalds/c/ba1f9c8fe3d443a78814cdf8ac8f9829b5ca7095

Thank you!

-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/prtracker.html

Re: [GIT PULL] arm64 updates for 6.13-rc1

[Sasha Levin]

On Mon, Nov 18, 2024 at 10:06:23AM +0000, Catalin Marinas wrote:
>  - MTE: hugetlbfs support and the corresponding kselftests

Hi Catalin,

It looks like with the new feature above, LTP manages to trigger the
following warning on linus-next:

[  100.133691] hugefork01 (362): drop_caches: 3
tst_hugepage.c:84: TINFO: 2 hugepage(s) reserved
tst_tmpdir.c:316: TINFO: Using /scratch/ltp-CckaqgMrC1/LTP_hug5PSMw8 as tmpdir (ext2/ext3/ext4 filesystem)
tst_test.c:1085: TINFO: Mounting none to /scratch/ltp-CckaqgMrC1/LTP_hug5PSMw8/hugetlbfs fstyp=hugetlbfs flags=0
tst_test.c:1860: TINFO: LTP version: 20240930
tst_test.c:1864: TINFO: Tested kernel: 6.12.0 #1 SMP PREEMPT @1732504538 aarch64
tst_test.c:1703: TINFO: Timeout per run is 0h 02m 30s
<4>[  100.355230] ------------[ cut here ]------------
<4>[  100.356888] WARNING: CPU: 0 PID: 363 at arch/arm64/include/asm/mte.h:58 copy_highpage+0x1d4/0x2d8
<4>[  100.359160] Modules linked in: crct10dif_ce sm3_ce sm3 sha3_ce sha512_ce sha512_arm64 fuse drm backlight ip_tables x_tables
<4>[  100.363578] CPU: 0 UID: 0 PID: 363 Comm: hugefork01 Not tainted 6.12.0 #1
<4>[  100.365113] Hardware name: linux,dummy-virt (DT)
<4>[  100.365966] pstate: 63402009 (nZCv daif +PAN -UAO +TCO +DIT -SSBS BTYPE=--)
<4>[  100.366468] pc : copy_highpage+0x1d4/0x2d8
<4>[  100.366780] lr : copy_highpage+0x78/0x2d8
<4>[  100.367090] sp : ffff80008066bb30
<4>[  100.368094] x29: ffff80008066bb30 x28: ffffc1ffc3118000 x27: 0000000000000000
<4>[  100.369341] x26: 0000000000000000 x25: 0000ffff9ce00000 x24: ffffc1ffc3118000
<4>[  100.370223] x23: fff00000c47ff000 x22: fff00000c4fff000 x21: ffffc1ffc3138000
<4>[  100.370739] x20: ffffc1ffc3138000 x19: ffffc1ffc311ffc0 x18: ffffffffffffffff
<4>[  100.371285] x17: 0000000000000000 x16: ffffa302fd05bcb0 x15: 0000ffff9d2fdfff
<4>[  100.372778] x14: 0000000000000000 x13: 1ffe00001859f161 x12: fff00000c2cf8b0c
<4>[  100.374124] x11: ffff80008066bd70 x10: ffffa302fe2a20d0 x9 : ffffa302fb438578
<4>[  100.374877] x8 : ffff80008066ba48 x7 : 0000000000000000 x6 : ffffa302fdbdf000
<4>[  100.376152] x5 : 0000000000000000 x4 : fff00000c2f239c0 x3 : fff00000c33e43f0
<4>[  100.376962] x2 : ffffc1ffc3138000 x1 : 00000000000000f4 x0 : 0000000000000000
<4>[  100.377964] Call trace:
<4>[  100.378736]  copy_highpage+0x1d4/0x2d8 (P)
<4>[  100.379422]  copy_highpage+0x78/0x2d8 (L)
<4>[  100.380272]  copy_user_highpage+0x20/0x48
<4>[  100.380805]  copy_user_large_folio+0x1bc/0x268
<4>[  100.381601]  hugetlb_wp+0x190/0x860
<4>[  100.382031]  hugetlb_fault+0xa28/0xc10
<4>[  100.382911]  handle_mm_fault+0x2a0/0x2c0
<4>[  100.383511]  do_page_fault+0x12c/0x578
<4>[  100.384913]  do_mem_abort+0x4c/0xa8
<4>[  100.385397]  el0_da+0x44/0xb0
<4>[  100.385775]  el0t_64_sync_handler+0xc4/0x138
<4>[  100.386243]  el0t_64_sync+0x198/0x1a0
<4>[  100.388759] ---[ end trace 0000000000000000 ]---

-- 
Thanks,
Sasha

Re: [GIT PULL] arm64 updates for 6.13-rc1

[Catalin Marinas]

Thanks Sasha.

Adding Yang Shi (he contributed the support) and David H.

On Mon, Nov 25, 2024 at 10:09:59AM -0500, Sasha Levin wrote:
> On Mon, Nov 18, 2024 at 10:06:23AM +0000, Catalin Marinas wrote:
> >  - MTE: hugetlbfs support and the corresponding kselftests
> 
> Hi Catalin,
> 
> It looks like with the new feature above, LTP manages to trigger the
> following warning on linus-next:
> 
> [  100.133691] hugefork01 (362): drop_caches: 3
> tst_hugepage.c:84: TINFO: 2 hugepage(s) reserved
> tst_tmpdir.c:316: TINFO: Using /scratch/ltp-CckaqgMrC1/LTP_hug5PSMw8 as tmpdir (ext2/ext3/ext4 filesystem)
> tst_test.c:1085: TINFO: Mounting none to /scratch/ltp-CckaqgMrC1/LTP_hug5PSMw8/hugetlbfs fstyp=hugetlbfs flags=0
> tst_test.c:1860: TINFO: LTP version: 20240930
> tst_test.c:1864: TINFO: Tested kernel: 6.12.0 #1 SMP PREEMPT @1732504538 aarch64
> tst_test.c:1703: TINFO: Timeout per run is 0h 02m 30s
> <4>[  100.355230] ------------[ cut here ]------------
> <4>[  100.356888] WARNING: CPU: 0 PID: 363 at arch/arm64/include/asm/mte.h:58 copy_highpage+0x1d4/0x2d8
> <4>[  100.359160] Modules linked in: crct10dif_ce sm3_ce sm3 sha3_ce sha512_ce sha512_arm64 fuse drm backlight ip_tables x_tables
> <4>[  100.363578] CPU: 0 UID: 0 PID: 363 Comm: hugefork01 Not tainted 6.12.0 #1
> <4>[  100.365113] Hardware name: linux,dummy-virt (DT)
> <4>[  100.365966] pstate: 63402009 (nZCv daif +PAN -UAO +TCO +DIT -SSBS BTYPE=--)
> <4>[  100.366468] pc : copy_highpage+0x1d4/0x2d8
> <4>[  100.366780] lr : copy_highpage+0x78/0x2d8
> <4>[  100.367090] sp : ffff80008066bb30
> <4>[  100.368094] x29: ffff80008066bb30 x28: ffffc1ffc3118000 x27: 0000000000000000
> <4>[  100.369341] x26: 0000000000000000 x25: 0000ffff9ce00000 x24: ffffc1ffc3118000
> <4>[  100.370223] x23: fff00000c47ff000 x22: fff00000c4fff000 x21: ffffc1ffc3138000
> <4>[  100.370739] x20: ffffc1ffc3138000 x19: ffffc1ffc311ffc0 x18: ffffffffffffffff
> <4>[  100.371285] x17: 0000000000000000 x16: ffffa302fd05bcb0 x15: 0000ffff9d2fdfff
> <4>[  100.372778] x14: 0000000000000000 x13: 1ffe00001859f161 x12: fff00000c2cf8b0c
> <4>[  100.374124] x11: ffff80008066bd70 x10: ffffa302fe2a20d0 x9 : ffffa302fb438578
> <4>[  100.374877] x8 : ffff80008066ba48 x7 : 0000000000000000 x6 : ffffa302fdbdf000
> <4>[  100.376152] x5 : 0000000000000000 x4 : fff00000c2f239c0 x3 : fff00000c33e43f0
> <4>[  100.376962] x2 : ffffc1ffc3138000 x1 : 00000000000000f4 x0 : 0000000000000000
> <4>[  100.377964] Call trace:
> <4>[  100.378736]  copy_highpage+0x1d4/0x2d8 (P)
> <4>[  100.379422]  copy_highpage+0x78/0x2d8 (L)
> <4>[  100.380272]  copy_user_highpage+0x20/0x48
> <4>[  100.380805]  copy_user_large_folio+0x1bc/0x268
> <4>[  100.381601]  hugetlb_wp+0x190/0x860
> <4>[  100.382031]  hugetlb_fault+0xa28/0xc10
> <4>[  100.382911]  handle_mm_fault+0x2a0/0x2c0
> <4>[  100.383511]  do_page_fault+0x12c/0x578
> <4>[  100.384913]  do_mem_abort+0x4c/0xa8
> <4>[  100.385397]  el0_da+0x44/0xb0
> <4>[  100.385775]  el0t_64_sync_handler+0xc4/0x138
> <4>[  100.386243]  el0t_64_sync+0x198/0x1a0
> <4>[  100.388759] ---[ end trace 0000000000000000 ]---

It looks like this can trigger even if the system does not use MTE. The
warning was introduced in commit 25c17c4b55de ("hugetlb: arm64: add mte
support") and it's supposed to check whether page_mte_tagged() is called
on a large folio inadvertently. But in copy_highpage(), if the source is
a huge page and untagged, it takes the else path with the
page_mte_tagged() check. I think something like below would do but I
haven't tried it yet:

diff --git a/arch/arm64/mm/copypage.c b/arch/arm64/mm/copypage.c
index 87b3f1a25535..ef303a2262c5 100644
--- a/arch/arm64/mm/copypage.c
+++ b/arch/arm64/mm/copypage.c
@@ -30,9 +30,9 @@ void copy_highpage(struct page *to, struct page *from)
 	if (!system_supports_mte())
 		return;
 
-	if (folio_test_hugetlb(src) &&
-	    folio_test_hugetlb_mte_tagged(src)) {
-		if (!folio_try_hugetlb_mte_tagging(dst))
+	if (folio_test_hugetlb(src)) {
+		if (!folio_test_hugetlb_mte_tagged(src) ||
+		    !folio_try_hugetlb_mte_tagging(dst))
 			return;
 
 		/*

-- 
Catalin

Re: [GIT PULL] arm64 updates for 6.13-rc1

[Yang Shi]

On 11/25/24 11:09 AM, Catalin Marinas wrote:
> Thanks Sasha.
>
> Adding Yang Shi (he contributed the support) and David H.
>
> On Mon, Nov 25, 2024 at 10:09:59AM -0500, Sasha Levin wrote:
>> On Mon, Nov 18, 2024 at 10:06:23AM +0000, Catalin Marinas wrote:
>>>   - MTE: hugetlbfs support and the corresponding kselftests
>> Hi Catalin,
>>
>> It looks like with the new feature above, LTP manages to trigger the
>> following warning on linus-next:
>>
>> [  100.133691] hugefork01 (362): drop_caches: 3
>> tst_hugepage.c:84: TINFO: 2 hugepage(s) reserved
>> tst_tmpdir.c:316: TINFO: Using /scratch/ltp-CckaqgMrC1/LTP_hug5PSMw8 as tmpdir (ext2/ext3/ext4 filesystem)
>> tst_test.c:1085: TINFO: Mounting none to /scratch/ltp-CckaqgMrC1/LTP_hug5PSMw8/hugetlbfs fstyp=hugetlbfs flags=0
>> tst_test.c:1860: TINFO: LTP version: 20240930
>> tst_test.c:1864: TINFO: Tested kernel: 6.12.0 #1 SMP PREEMPT @1732504538 aarch64
>> tst_test.c:1703: TINFO: Timeout per run is 0h 02m 30s
>> <4>[  100.355230] ------------[ cut here ]------------
>> <4>[  100.356888] WARNING: CPU: 0 PID: 363 at arch/arm64/include/asm/mte.h:58 copy_highpage+0x1d4/0x2d8
>> <4>[  100.359160] Modules linked in: crct10dif_ce sm3_ce sm3 sha3_ce sha512_ce sha512_arm64 fuse drm backlight ip_tables x_tables
>> <4>[  100.363578] CPU: 0 UID: 0 PID: 363 Comm: hugefork01 Not tainted 6.12.0 #1
>> <4>[  100.365113] Hardware name: linux,dummy-virt (DT)
>> <4>[  100.365966] pstate: 63402009 (nZCv daif +PAN -UAO +TCO +DIT -SSBS BTYPE=--)
>> <4>[  100.366468] pc : copy_highpage+0x1d4/0x2d8
>> <4>[  100.366780] lr : copy_highpage+0x78/0x2d8
>> <4>[  100.367090] sp : ffff80008066bb30
>> <4>[  100.368094] x29: ffff80008066bb30 x28: ffffc1ffc3118000 x27: 0000000000000000
>> <4>[  100.369341] x26: 0000000000000000 x25: 0000ffff9ce00000 x24: ffffc1ffc3118000
>> <4>[  100.370223] x23: fff00000c47ff000 x22: fff00000c4fff000 x21: ffffc1ffc3138000
>> <4>[  100.370739] x20: ffffc1ffc3138000 x19: ffffc1ffc311ffc0 x18: ffffffffffffffff
>> <4>[  100.371285] x17: 0000000000000000 x16: ffffa302fd05bcb0 x15: 0000ffff9d2fdfff
>> <4>[  100.372778] x14: 0000000000000000 x13: 1ffe00001859f161 x12: fff00000c2cf8b0c
>> <4>[  100.374124] x11: ffff80008066bd70 x10: ffffa302fe2a20d0 x9 : ffffa302fb438578
>> <4>[  100.374877] x8 : ffff80008066ba48 x7 : 0000000000000000 x6 : ffffa302fdbdf000
>> <4>[  100.376152] x5 : 0000000000000000 x4 : fff00000c2f239c0 x3 : fff00000c33e43f0
>> <4>[  100.376962] x2 : ffffc1ffc3138000 x1 : 00000000000000f4 x0 : 0000000000000000
>> <4>[  100.377964] Call trace:
>> <4>[  100.378736]  copy_highpage+0x1d4/0x2d8 (P)
>> <4>[  100.379422]  copy_highpage+0x78/0x2d8 (L)
>> <4>[  100.380272]  copy_user_highpage+0x20/0x48
>> <4>[  100.380805]  copy_user_large_folio+0x1bc/0x268
>> <4>[  100.381601]  hugetlb_wp+0x190/0x860
>> <4>[  100.382031]  hugetlb_fault+0xa28/0xc10
>> <4>[  100.382911]  handle_mm_fault+0x2a0/0x2c0
>> <4>[  100.383511]  do_page_fault+0x12c/0x578
>> <4>[  100.384913]  do_mem_abort+0x4c/0xa8
>> <4>[  100.385397]  el0_da+0x44/0xb0
>> <4>[  100.385775]  el0t_64_sync_handler+0xc4/0x138
>> <4>[  100.386243]  el0t_64_sync+0x198/0x1a0
>> <4>[  100.388759] ---[ end trace 0000000000000000 ]---
> It looks like this can trigger even if the system does not use MTE. The
> warning was introduced in commit 25c17c4b55de ("hugetlb: arm64: add mte
> support") and it's supposed to check whether page_mte_tagged() is called
> on a large folio inadvertently. But in copy_highpage(), if the source is
> a huge page and untagged, it takes the else path with the
> page_mte_tagged() check. I think something like below would do but I
> haven't tried it yet:

Hi Catalin,

Thanks for investigating this. Yes, it is. The fix looks correct to me.

>
> diff --git a/arch/arm64/mm/copypage.c b/arch/arm64/mm/copypage.c
> index 87b3f1a25535..ef303a2262c5 100644
> --- a/arch/arm64/mm/copypage.c
> +++ b/arch/arm64/mm/copypage.c
> @@ -30,9 +30,9 @@ void copy_highpage(struct page *to, struct page *from)
>   	if (!system_supports_mte())
>   		return;
>   
> -	if (folio_test_hugetlb(src) &&
> -	    folio_test_hugetlb_mte_tagged(src)) {
> -		if (!folio_try_hugetlb_mte_tagging(dst))
> +	if (folio_test_hugetlb(src)) {
> +		if (!folio_test_hugetlb_mte_tagged(src) ||
> +		    !folio_try_hugetlb_mte_tagging(dst))
>   			return;
>   
>   		/*
>

Re: [GIT PULL] arm64 updates for 6.13-rc1

[Catalin Marinas]

On Tue, Nov 26, 2024 at 09:41:39AM -0800, Yang Shi wrote:
> On 11/25/24 11:09 AM, Catalin Marinas wrote:
> > On Mon, Nov 25, 2024 at 10:09:59AM -0500, Sasha Levin wrote:
> > > On Mon, Nov 18, 2024 at 10:06:23AM +0000, Catalin Marinas wrote:
> > > >   - MTE: hugetlbfs support and the corresponding kselftests
> > > 
> > > It looks like with the new feature above, LTP manages to trigger the
> > > following warning on linus-next:
> > > 
> > > [  100.133691] hugefork01 (362): drop_caches: 3
> > > tst_hugepage.c:84: TINFO: 2 hugepage(s) reserved
> > > tst_tmpdir.c:316: TINFO: Using /scratch/ltp-CckaqgMrC1/LTP_hug5PSMw8 as tmpdir (ext2/ext3/ext4 filesystem)
> > > tst_test.c:1085: TINFO: Mounting none to /scratch/ltp-CckaqgMrC1/LTP_hug5PSMw8/hugetlbfs fstyp=hugetlbfs flags=0
> > > tst_test.c:1860: TINFO: LTP version: 20240930
> > > tst_test.c:1864: TINFO: Tested kernel: 6.12.0 #1 SMP PREEMPT @1732504538 aarch64
> > > tst_test.c:1703: TINFO: Timeout per run is 0h 02m 30s
> > > <4>[  100.355230] ------------[ cut here ]------------
> > > <4>[  100.356888] WARNING: CPU: 0 PID: 363 at arch/arm64/include/asm/mte.h:58 copy_highpage+0x1d4/0x2d8
> > > <4>[  100.359160] Modules linked in: crct10dif_ce sm3_ce sm3 sha3_ce sha512_ce sha512_arm64 fuse drm backlight ip_tables x_tables
> > > <4>[  100.363578] CPU: 0 UID: 0 PID: 363 Comm: hugefork01 Not tainted 6.12.0 #1
> > > <4>[  100.365113] Hardware name: linux,dummy-virt (DT)
> > > <4>[  100.365966] pstate: 63402009 (nZCv daif +PAN -UAO +TCO +DIT -SSBS BTYPE=--)
> > > <4>[  100.366468] pc : copy_highpage+0x1d4/0x2d8
> > > <4>[  100.366780] lr : copy_highpage+0x78/0x2d8
> > > <4>[  100.367090] sp : ffff80008066bb30
> > > <4>[  100.368094] x29: ffff80008066bb30 x28: ffffc1ffc3118000 x27: 0000000000000000
> > > <4>[  100.369341] x26: 0000000000000000 x25: 0000ffff9ce00000 x24: ffffc1ffc3118000
> > > <4>[  100.370223] x23: fff00000c47ff000 x22: fff00000c4fff000 x21: ffffc1ffc3138000
> > > <4>[  100.370739] x20: ffffc1ffc3138000 x19: ffffc1ffc311ffc0 x18: ffffffffffffffff
> > > <4>[  100.371285] x17: 0000000000000000 x16: ffffa302fd05bcb0 x15: 0000ffff9d2fdfff
> > > <4>[  100.372778] x14: 0000000000000000 x13: 1ffe00001859f161 x12: fff00000c2cf8b0c
> > > <4>[  100.374124] x11: ffff80008066bd70 x10: ffffa302fe2a20d0 x9 : ffffa302fb438578
> > > <4>[  100.374877] x8 : ffff80008066ba48 x7 : 0000000000000000 x6 : ffffa302fdbdf000
> > > <4>[  100.376152] x5 : 0000000000000000 x4 : fff00000c2f239c0 x3 : fff00000c33e43f0
> > > <4>[  100.376962] x2 : ffffc1ffc3138000 x1 : 00000000000000f4 x0 : 0000000000000000
> > > <4>[  100.377964] Call trace:
> > > <4>[  100.378736]  copy_highpage+0x1d4/0x2d8 (P)
> > > <4>[  100.379422]  copy_highpage+0x78/0x2d8 (L)
> > > <4>[  100.380272]  copy_user_highpage+0x20/0x48
> > > <4>[  100.380805]  copy_user_large_folio+0x1bc/0x268
> > > <4>[  100.381601]  hugetlb_wp+0x190/0x860
> > > <4>[  100.382031]  hugetlb_fault+0xa28/0xc10
> > > <4>[  100.382911]  handle_mm_fault+0x2a0/0x2c0
> > > <4>[  100.383511]  do_page_fault+0x12c/0x578
> > > <4>[  100.384913]  do_mem_abort+0x4c/0xa8
> > > <4>[  100.385397]  el0_da+0x44/0xb0
> > > <4>[  100.385775]  el0t_64_sync_handler+0xc4/0x138
> > > <4>[  100.386243]  el0t_64_sync+0x198/0x1a0
> > > <4>[  100.388759] ---[ end trace 0000000000000000 ]---
> > 
> > It looks like this can trigger even if the system does not use MTE. The
> > warning was introduced in commit 25c17c4b55de ("hugetlb: arm64: add mte
> > support") and it's supposed to check whether page_mte_tagged() is called
> > on a large folio inadvertently. But in copy_highpage(), if the source is
> > a huge page and untagged, it takes the else path with the
> > page_mte_tagged() check. I think something like below would do but I
> > haven't tried it yet:
> 
> Thanks for investigating this. Yes, it is. The fix looks correct to me.
> 
> > 
> > diff --git a/arch/arm64/mm/copypage.c b/arch/arm64/mm/copypage.c
> > index 87b3f1a25535..ef303a2262c5 100644
> > --- a/arch/arm64/mm/copypage.c
> > +++ b/arch/arm64/mm/copypage.c
> > @@ -30,9 +30,9 @@ void copy_highpage(struct page *to, struct page *from)
> >   	if (!system_supports_mte())
> >   		return;
> > -	if (folio_test_hugetlb(src) &&
> > -	    folio_test_hugetlb_mte_tagged(src)) {
> > -		if (!folio_try_hugetlb_mte_tagging(dst))
> > +	if (folio_test_hugetlb(src)) {
> > +		if (!folio_test_hugetlb_mte_tagged(src) ||
> > +		    !folio_try_hugetlb_mte_tagging(dst))
> >   			return;
> >   		/*

I wonder why we had a 'return' here originally rather than a
WARN_ON_ONCE() as we do further down for the page case. Do you seen any
issue with the hunk below? Destination should be a new folio and not
tagged yet:

diff --git a/arch/arm64/mm/copypage.c b/arch/arm64/mm/copypage.c
index 87b3f1a25535..cc7dfbea1304 100644
--- a/arch/arm64/mm/copypage.c
+++ b/arch/arm64/mm/copypage.c
@@ -30,11 +30,12 @@ void copy_highpage(struct page *to, struct page *from)
 	if (!system_supports_mte())
 		return;

-	if (folio_test_hugetlb(src) &&
-	    folio_test_hugetlb_mte_tagged(src)) {
-		if (!folio_try_hugetlb_mte_tagging(dst))
+	if (folio_test_hugetlb(src)) {
+		if (!folio_test_hugetlb_mte_tagged(src))
 			return;

+		WARN_ON_ONCE(!folio_try_hugetlb_mte_tagging(dst));
+
 		/*
 		 * Populate tags for all subpages.
 		 *

-- 
Catalin

Re: [GIT PULL] arm64 updates for 6.13-rc1

[Yang Shi]

>> diff --git a/arch/arm64/mm/copypage.c b/arch/arm64/mm/copypage.c
>> index 87b3f1a25535..ef303a2262c5 100644
>> --- a/arch/arm64/mm/copypage.c
>> +++ b/arch/arm64/mm/copypage.c
>> @@ -30,9 +30,9 @@ void copy_highpage(struct page *to, struct page *from)
>>    	if (!system_supports_mte())
>>    		return;
>> -	if (folio_test_hugetlb(src) &&
>> -	    folio_test_hugetlb_mte_tagged(src)) {
>> -		if (!folio_try_hugetlb_mte_tagging(dst))
>> +	if (folio_test_hugetlb(src)) {
>> +		if (!folio_test_hugetlb_mte_tagged(src) ||
>> +		    !folio_try_hugetlb_mte_tagging(dst))
>>    			return;
>>    		/*
> I wonder why we had a 'return' here originally rather than a
> WARN_ON_ONCE() as we do further down for the page case. Do you seen any
> issue with the hunk below? Destination should be a new folio and not
> tagged yet:

Yes, I did see problem. Because we copy tags for all sub pages then set 
folio mte tagged when copying the data for the first subpage. The 
warning will be triggered when we copy the second subpage.

>
> diff --git a/arch/arm64/mm/copypage.c b/arch/arm64/mm/copypage.c
> index 87b3f1a25535..cc7dfbea1304 100644
> --- a/arch/arm64/mm/copypage.c
> +++ b/arch/arm64/mm/copypage.c
> @@ -30,11 +30,12 @@ void copy_highpage(struct page *to, struct page *from)
>   	if (!system_supports_mte())
>   		return;
>
> -	if (folio_test_hugetlb(src) &&
> -	    folio_test_hugetlb_mte_tagged(src)) {
> -		if (!folio_try_hugetlb_mte_tagging(dst))
> +	if (folio_test_hugetlb(src)) {
> +		if (!folio_test_hugetlb_mte_tagged(src))
>   			return;
>
> +		WARN_ON_ONCE(!folio_try_hugetlb_mte_tagging(dst));
> +
>   		/*
>   		 * Populate tags for all subpages.
>   		 *
>

Re: [GIT PULL] arm64 updates for 6.13-rc1

[David Hildenbrand]

On 28.11.24 02:21, Yang Shi wrote:
>>> diff --git a/arch/arm64/mm/copypage.c b/arch/arm64/mm/copypage.c
>>> index 87b3f1a25535..ef303a2262c5 100644
>>> --- a/arch/arm64/mm/copypage.c
>>> +++ b/arch/arm64/mm/copypage.c
>>> @@ -30,9 +30,9 @@ void copy_highpage(struct page *to, struct page *from)
>>>     	if (!system_supports_mte())
>>>     		return;
>>> -	if (folio_test_hugetlb(src) &&
>>> -	    folio_test_hugetlb_mte_tagged(src)) {
>>> -		if (!folio_try_hugetlb_mte_tagging(dst))
>>> +	if (folio_test_hugetlb(src)) {
>>> +		if (!folio_test_hugetlb_mte_tagged(src) ||
>>> +		    !folio_try_hugetlb_mte_tagging(dst))
>>>     			return;
>>>     		/*
>> I wonder why we had a 'return' here originally rather than a
>> WARN_ON_ONCE() as we do further down for the page case. Do you seen any
>> issue with the hunk below? Destination should be a new folio and not
>> tagged yet:
> 
> Yes, I did see problem. Because we copy tags for all sub pages then set
> folio mte tagged when copying the data for the first subpage. The
> warning will be triggered when we copy the second subpage.

It's rather weird, though. We're instructed to copy a single page, yet 
copy tags for all pages.

This really only makes sense when called from folio_copy(), where we are 
guaranteed to copy all pages.

I'm starting to wonder if we should be able to hook into / overload 
folio_copy() instead, to just handle the complete hugetlb copy ourselves 
in one shot, and assume that copy_highpage() will never be called for 
hugetlb pages (WARN and don't copy tags).

-- 
Cheers,

David / dhildenb

Re: [GIT PULL] arm64 updates for 6.13-rc1

[Catalin Marinas]

On Wed, Nov 27, 2024 at 05:21:37PM -0800, Yang Shi wrote:
> > > diff --git a/arch/arm64/mm/copypage.c b/arch/arm64/mm/copypage.c
> > > index 87b3f1a25535..ef303a2262c5 100644
> > > --- a/arch/arm64/mm/copypage.c
> > > +++ b/arch/arm64/mm/copypage.c
> > > @@ -30,9 +30,9 @@ void copy_highpage(struct page *to, struct page *from)
> > >    	if (!system_supports_mte())
> > >    		return;
> > > -	if (folio_test_hugetlb(src) &&
> > > -	    folio_test_hugetlb_mte_tagged(src)) {
> > > -		if (!folio_try_hugetlb_mte_tagging(dst))
> > > +	if (folio_test_hugetlb(src)) {
> > > +		if (!folio_test_hugetlb_mte_tagged(src) ||
> > > +		    !folio_try_hugetlb_mte_tagging(dst))
> > >    			return;
> > >    		/*
> > I wonder why we had a 'return' here originally rather than a
> > WARN_ON_ONCE() as we do further down for the page case. Do you seen any
> > issue with the hunk below? Destination should be a new folio and not
> > tagged yet:
> 
> Yes, I did see problem. Because we copy tags for all sub pages then set
> folio mte tagged when copying the data for the first subpage. The warning
> will be triggered when we copy the second subpage.

Ah, good point, copy_highpage() will be called multiple times for each
subpage but we only do the copying once for the folio.

Now, I wonder whether we should actually defer the tag copying until
copy_page() is called on the head page. This way we can keep the warning
for consistency with the non-compound page case:

diff --git a/arch/arm64/mm/copypage.c b/arch/arm64/mm/copypage.c
index 87b3f1a25535..a86c897017df 100644
--- a/arch/arm64/mm/copypage.c
+++ b/arch/arm64/mm/copypage.c
@@ -30,11 +30,13 @@ void copy_highpage(struct page *to, struct page *from)
 	if (!system_supports_mte())
 		return;
 
-	if (folio_test_hugetlb(src) &&
-	    folio_test_hugetlb_mte_tagged(src)) {
-		if (!folio_try_hugetlb_mte_tagging(dst))
+	if (folio_test_hugetlb(src)) {
+		if (!folio_test_hugetlb_mte_tagged(src) ||
+		    from != folio_page(src, 0))
 			return;
 
+		WARN_ON_ONCE(!folio_try_hugetlb_mte_tagging(dst));
+
 		/*
 		 * Populate tags for all subpages.
 		 *

-- 
Catalin

Re: [GIT PULL] arm64 updates for 6.13-rc1

[Yang Shi]

On 11/28/24 6:12 AM, Catalin Marinas wrote:
> On Wed, Nov 27, 2024 at 05:21:37PM -0800, Yang Shi wrote:
>>>> diff --git a/arch/arm64/mm/copypage.c b/arch/arm64/mm/copypage.c
>>>> index 87b3f1a25535..ef303a2262c5 100644
>>>> --- a/arch/arm64/mm/copypage.c
>>>> +++ b/arch/arm64/mm/copypage.c
>>>> @@ -30,9 +30,9 @@ void copy_highpage(struct page *to, struct page *from)
>>>>     	if (!system_supports_mte())
>>>>     		return;
>>>> -	if (folio_test_hugetlb(src) &&
>>>> -	    folio_test_hugetlb_mte_tagged(src)) {
>>>> -		if (!folio_try_hugetlb_mte_tagging(dst))
>>>> +	if (folio_test_hugetlb(src)) {
>>>> +		if (!folio_test_hugetlb_mte_tagged(src) ||
>>>> +		    !folio_try_hugetlb_mte_tagging(dst))
>>>>     			return;
>>>>     		/*
>>> I wonder why we had a 'return' here originally rather than a
>>> WARN_ON_ONCE() as we do further down for the page case. Do you seen any
>>> issue with the hunk below? Destination should be a new folio and not
>>> tagged yet:
>> Yes, I did see problem. Because we copy tags for all sub pages then set
>> folio mte tagged when copying the data for the first subpage. The warning
>> will be triggered when we copy the second subpage.
> Ah, good point, copy_highpage() will be called multiple times for each
> subpage but we only do the copying once for the folio.
>
> Now, I wonder whether we should actually defer the tag copying until
> copy_page() is called on the head page. This way we can keep the warning
> for consistency with the non-compound page case:

Yeah, we can do this. Looks fine to me.

>
> diff --git a/arch/arm64/mm/copypage.c b/arch/arm64/mm/copypage.c
> index 87b3f1a25535..a86c897017df 100644
> --- a/arch/arm64/mm/copypage.c
> +++ b/arch/arm64/mm/copypage.c
> @@ -30,11 +30,13 @@ void copy_highpage(struct page *to, struct page *from)
>   	if (!system_supports_mte())
>   		return;
>   
> -	if (folio_test_hugetlb(src) &&
> -	    folio_test_hugetlb_mte_tagged(src)) {
> -		if (!folio_try_hugetlb_mte_tagging(dst))
> +	if (folio_test_hugetlb(src)) {
> +		if (!folio_test_hugetlb_mte_tagged(src) ||
> +		    from != folio_page(src, 0))
>   			return;
>   
> +		WARN_ON_ONCE(!folio_try_hugetlb_mte_tagging(dst));
> +
>   		/*
>   		 * Populate tags for all subpages.
>   		 *
>

Re: [GIT PULL] arm64 updates for 6.13-rc1

[David Hildenbrand]

On 28.11.24 15:12, Catalin Marinas wrote:
> On Wed, Nov 27, 2024 at 05:21:37PM -0800, Yang Shi wrote:
>>>> diff --git a/arch/arm64/mm/copypage.c b/arch/arm64/mm/copypage.c
>>>> index 87b3f1a25535..ef303a2262c5 100644
>>>> --- a/arch/arm64/mm/copypage.c
>>>> +++ b/arch/arm64/mm/copypage.c
>>>> @@ -30,9 +30,9 @@ void copy_highpage(struct page *to, struct page *from)
>>>>     	if (!system_supports_mte())
>>>>     		return;
>>>> -	if (folio_test_hugetlb(src) &&
>>>> -	    folio_test_hugetlb_mte_tagged(src)) {
>>>> -		if (!folio_try_hugetlb_mte_tagging(dst))
>>>> +	if (folio_test_hugetlb(src)) {
>>>> +		if (!folio_test_hugetlb_mte_tagged(src) ||
>>>> +		    !folio_try_hugetlb_mte_tagging(dst))
>>>>     			return;
>>>>     		/*
>>> I wonder why we had a 'return' here originally rather than a
>>> WARN_ON_ONCE() as we do further down for the page case. Do you seen any
>>> issue with the hunk below? Destination should be a new folio and not
>>> tagged yet:
>>
>> Yes, I did see problem. Because we copy tags for all sub pages then set
>> folio mte tagged when copying the data for the first subpage. The warning
>> will be triggered when we copy the second subpage.
> 
> Ah, good point, copy_highpage() will be called multiple times for each
> subpage but we only do the copying once for the folio.
> 

It makes me still a bit nervous that we assume both the src and the 
destination folio have the same #pages (and in particular, that both are 
hugetlb folios :) ).

Hopefully that's an invariant that will always hold :)

> Now, I wonder whether we should actually defer the tag copying until
> copy_page() is called on the head page. This way we can keep the warning
> for consistency with the non-compound page case:
> 
> diff --git a/arch/arm64/mm/copypage.c b/arch/arm64/mm/copypage.c
> index 87b3f1a25535..a86c897017df 100644
> --- a/arch/arm64/mm/copypage.c
> +++ b/arch/arm64/mm/copypage.c
> @@ -30,11 +30,13 @@ void copy_highpage(struct page *to, struct page *from)
>   	if (!system_supports_mte())
>   		return;
>   
> -	if (folio_test_hugetlb(src) &&
> -	    folio_test_hugetlb_mte_tagged(src)) {
> -		if (!folio_try_hugetlb_mte_tagging(dst))
> +	if (folio_test_hugetlb(src)) {
> +		if (!folio_test_hugetlb_mte_tagged(src) ||
> +		    from != folio_page(src, 0))
>   			return;
>   
> +		WARN_ON_ONCE(!folio_try_hugetlb_mte_tagging(dst));
> +
>   		/*
>   		 * Populate tags for all subpages.
>   		 *
> 

Yes, looks better. A comment describing the oddity of "copy single page 
but copy all tags on head page access" might be reasonable.

-- 
Cheers,

David / dhildenb

Re: [GIT PULL] arm64 updates for 6.13-rc1

[Yang Shi]

On 11/28/24 1:56 AM, David Hildenbrand wrote:
> On 28.11.24 02:21, Yang Shi wrote:
>>>> diff --git a/arch/arm64/mm/copypage.c b/arch/arm64/mm/copypage.c
>>>> index 87b3f1a25535..ef303a2262c5 100644
>>>> --- a/arch/arm64/mm/copypage.c
>>>> +++ b/arch/arm64/mm/copypage.c
>>>> @@ -30,9 +30,9 @@ void copy_highpage(struct page *to, struct page 
>>>> *from)
>>>>         if (!system_supports_mte())
>>>>             return;
>>>> -    if (folio_test_hugetlb(src) &&
>>>> -        folio_test_hugetlb_mte_tagged(src)) {
>>>> -        if (!folio_try_hugetlb_mte_tagging(dst))
>>>> +    if (folio_test_hugetlb(src)) {
>>>> +        if (!folio_test_hugetlb_mte_tagged(src) ||
>>>> +            !folio_try_hugetlb_mte_tagging(dst))
>>>>                 return;
>>>>             /*
>>> I wonder why we had a 'return' here originally rather than a
>>> WARN_ON_ONCE() as we do further down for the page case. Do you seen any
>>> issue with the hunk below? Destination should be a new folio and not
>>> tagged yet:
>>
>> Yes, I did see problem. Because we copy tags for all sub pages then set
>> folio mte tagged when copying the data for the first subpage. The
>> warning will be triggered when we copy the second subpage.
>
> It's rather weird, though. We're instructed to copy a single page, yet 
> copy tags for all pages.
>
> This really only makes sense when called from folio_copy(), where we 
> are guaranteed to copy all pages.
>
> I'm starting to wonder if we should be able to hook into / overload 
> folio_copy() instead, to just handle the complete hugetlb copy 
> ourselves in one shot, and assume that copy_highpage() will never be 
> called for hugetlb pages (WARN and don't copy tags).

Actually folio_copy() is just called by migration. Copy huge page in CoW 
is more complicated and uses copy_user_highpage()->copy_highpage() 
instead of folio_copy(). It may start the page copy from any subpage. 
For example, if the CoW is triggered by accessing to the address in the 
middle of 2M. Kernel may copy the second half first then the first half 
to guarantee the accessed data in cache.

Re: [GIT PULL] arm64 updates for 6.13-rc1

[Catalin Marinas]

On Mon, Dec 02, 2024 at 08:22:57AM -0800, Yang Shi wrote:
> On 11/28/24 1:56 AM, David Hildenbrand wrote:
> > On 28.11.24 02:21, Yang Shi wrote:
> > > > > diff --git a/arch/arm64/mm/copypage.c b/arch/arm64/mm/copypage.c
> > > > > index 87b3f1a25535..ef303a2262c5 100644
> > > > > --- a/arch/arm64/mm/copypage.c
> > > > > +++ b/arch/arm64/mm/copypage.c
> > > > > @@ -30,9 +30,9 @@ void copy_highpage(struct page *to, struct
> > > > > page *from)
> > > > >         if (!system_supports_mte())
> > > > >             return;
> > > > > -    if (folio_test_hugetlb(src) &&
> > > > > -        folio_test_hugetlb_mte_tagged(src)) {
> > > > > -        if (!folio_try_hugetlb_mte_tagging(dst))
> > > > > +    if (folio_test_hugetlb(src)) {
> > > > > +        if (!folio_test_hugetlb_mte_tagged(src) ||
> > > > > +            !folio_try_hugetlb_mte_tagging(dst))
> > > > >                 return;
> > > > >             /*
> > > > I wonder why we had a 'return' here originally rather than a
> > > > WARN_ON_ONCE() as we do further down for the page case. Do you seen any
> > > > issue with the hunk below? Destination should be a new folio and not
> > > > tagged yet:
> > > 
> > > Yes, I did see problem. Because we copy tags for all sub pages then set
> > > folio mte tagged when copying the data for the first subpage. The
> > > warning will be triggered when we copy the second subpage.
> > 
> > It's rather weird, though. We're instructed to copy a single page, yet
> > copy tags for all pages.
> > 
> > This really only makes sense when called from folio_copy(), where we are
> > guaranteed to copy all pages.
> > 
> > I'm starting to wonder if we should be able to hook into / overload
> > folio_copy() instead, to just handle the complete hugetlb copy ourselves
> > in one shot, and assume that copy_highpage() will never be called for
> > hugetlb pages (WARN and don't copy tags).
> 
> Actually folio_copy() is just called by migration. Copy huge page in CoW is
> more complicated and uses copy_user_highpage()->copy_highpage() instead of
> folio_copy(). It may start the page copy from any subpage. For example, if
> the CoW is triggered by accessing to the address in the middle of 2M. Kernel
> may copy the second half first then the first half to guarantee the accessed
> data in cache.

Still trying to understand the possible call paths here. If we get a
write fault on a large folio, does the core code allocate a folio of the
same size for CoW or it starts with smaller ones? wp_page_copy()
allocates order 0 AFAICT, though if it was a pmd fault, it takes a
different path in handle_mm_fault(). But we also have huge pages using
contiguous ptes.

Unless the source and destinations folios are exactly the same size, it
will break many assumptions in the code above. Going the other way
around is also wrong, dst larger than src, we are not initialising the
whole dst folio.

Maybe going back to per-page PG_mte_tagged flag rather than per-folio
would keep things simple, less risk of wrong assumptions.

-- 
Catalin

Re: [GIT PULL] arm64 updates for 6.13-rc1

[David Hildenbrand]

On 04.12.24 16:29, Catalin Marinas wrote:
> On Mon, Dec 02, 2024 at 08:22:57AM -0800, Yang Shi wrote:
>> On 11/28/24 1:56 AM, David Hildenbrand wrote:
>>> On 28.11.24 02:21, Yang Shi wrote:
>>>>>> diff --git a/arch/arm64/mm/copypage.c b/arch/arm64/mm/copypage.c
>>>>>> index 87b3f1a25535..ef303a2262c5 100644
>>>>>> --- a/arch/arm64/mm/copypage.c
>>>>>> +++ b/arch/arm64/mm/copypage.c
>>>>>> @@ -30,9 +30,9 @@ void copy_highpage(struct page *to, struct
>>>>>> page *from)
>>>>>>          if (!system_supports_mte())
>>>>>>              return;
>>>>>> -    if (folio_test_hugetlb(src) &&
>>>>>> -        folio_test_hugetlb_mte_tagged(src)) {
>>>>>> -        if (!folio_try_hugetlb_mte_tagging(dst))
>>>>>> +    if (folio_test_hugetlb(src)) {
>>>>>> +        if (!folio_test_hugetlb_mte_tagged(src) ||
>>>>>> +            !folio_try_hugetlb_mte_tagging(dst))
>>>>>>                  return;
>>>>>>              /*
>>>>> I wonder why we had a 'return' here originally rather than a
>>>>> WARN_ON_ONCE() as we do further down for the page case. Do you seen any
>>>>> issue with the hunk below? Destination should be a new folio and not
>>>>> tagged yet:
>>>>
>>>> Yes, I did see problem. Because we copy tags for all sub pages then set
>>>> folio mte tagged when copying the data for the first subpage. The
>>>> warning will be triggered when we copy the second subpage.
>>>
>>> It's rather weird, though. We're instructed to copy a single page, yet
>>> copy tags for all pages.
>>>
>>> This really only makes sense when called from folio_copy(), where we are
>>> guaranteed to copy all pages.
>>>
>>> I'm starting to wonder if we should be able to hook into / overload
>>> folio_copy() instead, to just handle the complete hugetlb copy ourselves
>>> in one shot, and assume that copy_highpage() will never be called for
>>> hugetlb pages (WARN and don't copy tags).
>>
>> Actually folio_copy() is just called by migration. Copy huge page in CoW is
>> more complicated and uses copy_user_highpage()->copy_highpage() instead of
>> folio_copy(). It may start the page copy from any subpage. For example, if
>> the CoW is triggered by accessing to the address in the middle of 2M. Kernel
>> may copy the second half first then the first half to guarantee the accessed
>> data in cache.
> 
> Still trying to understand the possible call paths here. If we get a
> write fault on a large folio, does the core code allocate a folio of the
> same size for CoW or it starts with smaller ones? wp_page_copy()
> allocates order 0 AFAICT, though if it was a pmd fault, it takes a
> different path in handle_mm_fault(). But we also have huge pages using
> contiguous ptes.
> 
> Unless the source and destinations folios are exactly the same size, it
> will break many assumptions in the code above. Going the other way
> around is also wrong, dst larger than src, we are not initialising the
> whole dst folio.
> 
> Maybe going back to per-page PG_mte_tagged flag rather than per-folio
> would keep things simple, less risk of wrong assumptions.

I think the magic bit here is that for hugetlb, we only get hugetlb 
folios of the same size, and no mixtures.

-- 
Cheers,

David / dhildenb

Re: [GIT PULL] arm64 updates for 6.13-rc1

[Catalin Marinas]

On Wed, Dec 04, 2024 at 04:32:11PM +0100, David Hildenbrand wrote:
> On 04.12.24 16:29, Catalin Marinas wrote:
> > On Mon, Dec 02, 2024 at 08:22:57AM -0800, Yang Shi wrote:
> > > On 11/28/24 1:56 AM, David Hildenbrand wrote:
> > > > On 28.11.24 02:21, Yang Shi wrote:
> > > > > > > diff --git a/arch/arm64/mm/copypage.c b/arch/arm64/mm/copypage.c
> > > > > > > index 87b3f1a25535..ef303a2262c5 100644
> > > > > > > --- a/arch/arm64/mm/copypage.c
> > > > > > > +++ b/arch/arm64/mm/copypage.c
> > > > > > > @@ -30,9 +30,9 @@ void copy_highpage(struct page *to, struct
> > > > > > > page *from)
> > > > > > >          if (!system_supports_mte())
> > > > > > >              return;
> > > > > > > -    if (folio_test_hugetlb(src) &&
> > > > > > > -        folio_test_hugetlb_mte_tagged(src)) {
> > > > > > > -        if (!folio_try_hugetlb_mte_tagging(dst))
> > > > > > > +    if (folio_test_hugetlb(src)) {
> > > > > > > +        if (!folio_test_hugetlb_mte_tagged(src) ||
> > > > > > > +            !folio_try_hugetlb_mte_tagging(dst))
> > > > > > >                  return;
> > > > > > >              /*
> > > > > > I wonder why we had a 'return' here originally rather than a
> > > > > > WARN_ON_ONCE() as we do further down for the page case. Do you seen any
> > > > > > issue with the hunk below? Destination should be a new folio and not
> > > > > > tagged yet:
> > > > > 
> > > > > Yes, I did see problem. Because we copy tags for all sub pages then set
> > > > > folio mte tagged when copying the data for the first subpage. The
> > > > > warning will be triggered when we copy the second subpage.
> > > > 
> > > > It's rather weird, though. We're instructed to copy a single page, yet
> > > > copy tags for all pages.
> > > > 
> > > > This really only makes sense when called from folio_copy(), where we are
> > > > guaranteed to copy all pages.
> > > > 
> > > > I'm starting to wonder if we should be able to hook into / overload
> > > > folio_copy() instead, to just handle the complete hugetlb copy ourselves
> > > > in one shot, and assume that copy_highpage() will never be called for
> > > > hugetlb pages (WARN and don't copy tags).
> > > 
> > > Actually folio_copy() is just called by migration. Copy huge page in CoW is
> > > more complicated and uses copy_user_highpage()->copy_highpage() instead of
> > > folio_copy(). It may start the page copy from any subpage. For example, if
> > > the CoW is triggered by accessing to the address in the middle of 2M. Kernel
> > > may copy the second half first then the first half to guarantee the accessed
> > > data in cache.
> > 
> > Still trying to understand the possible call paths here. If we get a
> > write fault on a large folio, does the core code allocate a folio of the
> > same size for CoW or it starts with smaller ones? wp_page_copy()
> > allocates order 0 AFAICT, though if it was a pmd fault, it takes a
> > different path in handle_mm_fault(). But we also have huge pages using
> > contiguous ptes.
> > 
> > Unless the source and destinations folios are exactly the same size, it
> > will break many assumptions in the code above. Going the other way
> > around is also wrong, dst larger than src, we are not initialising the
> > whole dst folio.
> > 
> > Maybe going back to per-page PG_mte_tagged flag rather than per-folio
> > would keep things simple, less risk of wrong assumptions.
> 
> I think the magic bit here is that for hugetlb, we only get hugetlb folios
> of the same size, and no mixtures.

Ah, ok, we do check for this and only do the advance copy for hugetlb
folios. I'd add a check for folio size just in case, something like
below (I'll add some description and post it properly):

diff --git a/arch/arm64/mm/copypage.c b/arch/arm64/mm/copypage.c
index 87b3f1a25535..c3a83db46ec6 100644
--- a/arch/arm64/mm/copypage.c
+++ b/arch/arm64/mm/copypage.c
@@ -30,11 +30,14 @@ void copy_highpage(struct page *to, struct page *from)
 	if (!system_supports_mte())
 		return;
 
-	if (folio_test_hugetlb(src) &&
-	    folio_test_hugetlb_mte_tagged(src)) {
-		if (!folio_try_hugetlb_mte_tagging(dst))
+	if (folio_test_hugetlb(src)) {
+		if (!folio_test_hugetlb_mte_tagged(src) ||
+		    from != folio_page(src, 0) ||
+		    WARN_ON_ONCE(folio_nr_pages(src) != folio_nr_pages(dst)))
 			return;
 
+		WARN_ON_ONCE(!folio_try_hugetlb_mte_tagging(dst));
+
 		/*
 		 * Populate tags for all subpages.
 		 *

-- 
Catalin

Re: [GIT PULL] arm64 updates for 6.13-rc1

[Yang Shi]

On 12/4/24 7:50 AM, Catalin Marinas wrote:
> On Wed, Dec 04, 2024 at 04:32:11PM +0100, David Hildenbrand wrote:
>> On 04.12.24 16:29, Catalin Marinas wrote:
>>> On Mon, Dec 02, 2024 at 08:22:57AM -0800, Yang Shi wrote:
>>>> On 11/28/24 1:56 AM, David Hildenbrand wrote:
>>>>> On 28.11.24 02:21, Yang Shi wrote:
>>>>>>>> diff --git a/arch/arm64/mm/copypage.c b/arch/arm64/mm/copypage.c
>>>>>>>> index 87b3f1a25535..ef303a2262c5 100644
>>>>>>>> --- a/arch/arm64/mm/copypage.c
>>>>>>>> +++ b/arch/arm64/mm/copypage.c
>>>>>>>> @@ -30,9 +30,9 @@ void copy_highpage(struct page *to, struct
>>>>>>>> page *from)
>>>>>>>>           if (!system_supports_mte())
>>>>>>>>               return;
>>>>>>>> -    if (folio_test_hugetlb(src) &&
>>>>>>>> -        folio_test_hugetlb_mte_tagged(src)) {
>>>>>>>> -        if (!folio_try_hugetlb_mte_tagging(dst))
>>>>>>>> +    if (folio_test_hugetlb(src)) {
>>>>>>>> +        if (!folio_test_hugetlb_mte_tagged(src) ||
>>>>>>>> +            !folio_try_hugetlb_mte_tagging(dst))
>>>>>>>>                   return;
>>>>>>>>               /*
>>>>>>> I wonder why we had a 'return' here originally rather than a
>>>>>>> WARN_ON_ONCE() as we do further down for the page case. Do you seen any
>>>>>>> issue with the hunk below? Destination should be a new folio and not
>>>>>>> tagged yet:
>>>>>> Yes, I did see problem. Because we copy tags for all sub pages then set
>>>>>> folio mte tagged when copying the data for the first subpage. The
>>>>>> warning will be triggered when we copy the second subpage.
>>>>> It's rather weird, though. We're instructed to copy a single page, yet
>>>>> copy tags for all pages.
>>>>>
>>>>> This really only makes sense when called from folio_copy(), where we are
>>>>> guaranteed to copy all pages.
>>>>>
>>>>> I'm starting to wonder if we should be able to hook into / overload
>>>>> folio_copy() instead, to just handle the complete hugetlb copy ourselves
>>>>> in one shot, and assume that copy_highpage() will never be called for
>>>>> hugetlb pages (WARN and don't copy tags).
>>>> Actually folio_copy() is just called by migration. Copy huge page in CoW is
>>>> more complicated and uses copy_user_highpage()->copy_highpage() instead of
>>>> folio_copy(). It may start the page copy from any subpage. For example, if
>>>> the CoW is triggered by accessing to the address in the middle of 2M. Kernel
>>>> may copy the second half first then the first half to guarantee the accessed
>>>> data in cache.
>>> Still trying to understand the possible call paths here. If we get a
>>> write fault on a large folio, does the core code allocate a folio of the
>>> same size for CoW or it starts with smaller ones? wp_page_copy()
>>> allocates order 0 AFAICT, though if it was a pmd fault, it takes a
>>> different path in handle_mm_fault(). But we also have huge pages using
>>> contiguous ptes.
>>>
>>> Unless the source and destinations folios are exactly the same size, it
>>> will break many assumptions in the code above. Going the other way
>>> around is also wrong, dst larger than src, we are not initialising the
>>> whole dst folio.
>>>
>>> Maybe going back to per-page PG_mte_tagged flag rather than per-folio
>>> would keep things simple, less risk of wrong assumptions.
>> I think the magic bit here is that for hugetlb, we only get hugetlb folios
>> of the same size, and no mixtures.

Yes, hugetlb always allocates the same order folio for CoW. And hugetlb 
CoW path is:

handle_mm_fault() ->
   hugetlb_fault() ->
     hugetlb_wp()

> Ah, ok, we do check for this and only do the advance copy for hugetlb
> folios. I'd add a check for folio size just in case, something like
> below (I'll add some description and post it properly):
>
> diff --git a/arch/arm64/mm/copypage.c b/arch/arm64/mm/copypage.c
> index 87b3f1a25535..c3a83db46ec6 100644
> --- a/arch/arm64/mm/copypage.c
> +++ b/arch/arm64/mm/copypage.c
> @@ -30,11 +30,14 @@ void copy_highpage(struct page *to, struct page *from)
>   	if (!system_supports_mte())
>   		return;
>   
> -	if (folio_test_hugetlb(src) &&
> -	    folio_test_hugetlb_mte_tagged(src)) {
> -		if (!folio_try_hugetlb_mte_tagging(dst))
> +	if (folio_test_hugetlb(src)) {
> +		if (!folio_test_hugetlb_mte_tagged(src) ||
> +		    from != folio_page(src, 0) ||
> +		    WARN_ON_ONCE(folio_nr_pages(src) != folio_nr_pages(dst)))

The check is ok, but TBH I don't see too much benefit. The same order is 
guaranteed by hugetlb fault handler. And I don't think we will support 
mixed order for hugetlb in foreseeable future.

>   			return;
>   
> +		WARN_ON_ONCE(!folio_try_hugetlb_mte_tagging(dst));
> +
>   		/*
>   		 * Populate tags for all subpages.
>   		 *
>

Re: [GIT PULL] arm64 updates for 6.13-rc1

[David Hildenbrand]

On 04.12.24 16:50, Catalin Marinas wrote:
> On Wed, Dec 04, 2024 at 04:32:11PM +0100, David Hildenbrand wrote:
>> On 04.12.24 16:29, Catalin Marinas wrote:
>>> On Mon, Dec 02, 2024 at 08:22:57AM -0800, Yang Shi wrote:
>>>> On 11/28/24 1:56 AM, David Hildenbrand wrote:
>>>>> On 28.11.24 02:21, Yang Shi wrote:
>>>>>>>> diff --git a/arch/arm64/mm/copypage.c b/arch/arm64/mm/copypage.c
>>>>>>>> index 87b3f1a25535..ef303a2262c5 100644
>>>>>>>> --- a/arch/arm64/mm/copypage.c
>>>>>>>> +++ b/arch/arm64/mm/copypage.c
>>>>>>>> @@ -30,9 +30,9 @@ void copy_highpage(struct page *to, struct
>>>>>>>> page *from)
>>>>>>>>           if (!system_supports_mte())
>>>>>>>>               return;
>>>>>>>> -    if (folio_test_hugetlb(src) &&
>>>>>>>> -        folio_test_hugetlb_mte_tagged(src)) {
>>>>>>>> -        if (!folio_try_hugetlb_mte_tagging(dst))
>>>>>>>> +    if (folio_test_hugetlb(src)) {
>>>>>>>> +        if (!folio_test_hugetlb_mte_tagged(src) ||
>>>>>>>> +            !folio_try_hugetlb_mte_tagging(dst))
>>>>>>>>                   return;
>>>>>>>>               /*
>>>>>>> I wonder why we had a 'return' here originally rather than a
>>>>>>> WARN_ON_ONCE() as we do further down for the page case. Do you seen any
>>>>>>> issue with the hunk below? Destination should be a new folio and not
>>>>>>> tagged yet:
>>>>>>
>>>>>> Yes, I did see problem. Because we copy tags for all sub pages then set
>>>>>> folio mte tagged when copying the data for the first subpage. The
>>>>>> warning will be triggered when we copy the second subpage.
>>>>>
>>>>> It's rather weird, though. We're instructed to copy a single page, yet
>>>>> copy tags for all pages.
>>>>>
>>>>> This really only makes sense when called from folio_copy(), where we are
>>>>> guaranteed to copy all pages.
>>>>>
>>>>> I'm starting to wonder if we should be able to hook into / overload
>>>>> folio_copy() instead, to just handle the complete hugetlb copy ourselves
>>>>> in one shot, and assume that copy_highpage() will never be called for
>>>>> hugetlb pages (WARN and don't copy tags).
>>>>
>>>> Actually folio_copy() is just called by migration. Copy huge page in CoW is
>>>> more complicated and uses copy_user_highpage()->copy_highpage() instead of
>>>> folio_copy(). It may start the page copy from any subpage. For example, if
>>>> the CoW is triggered by accessing to the address in the middle of 2M. Kernel
>>>> may copy the second half first then the first half to guarantee the accessed
>>>> data in cache.
>>>
>>> Still trying to understand the possible call paths here. If we get a
>>> write fault on a large folio, does the core code allocate a folio of the
>>> same size for CoW or it starts with smaller ones? wp_page_copy()
>>> allocates order 0 AFAICT, though if it was a pmd fault, it takes a
>>> different path in handle_mm_fault(). But we also have huge pages using
>>> contiguous ptes.
>>>
>>> Unless the source and destinations folios are exactly the same size, it
>>> will break many assumptions in the code above. Going the other way
>>> around is also wrong, dst larger than src, we are not initialising the
>>> whole dst folio.
>>>
>>> Maybe going back to per-page PG_mte_tagged flag rather than per-folio
>>> would keep things simple, less risk of wrong assumptions.
>>
>> I think the magic bit here is that for hugetlb, we only get hugetlb folios
>> of the same size, and no mixtures.
> 
> Ah, ok, we do check for this and only do the advance copy for hugetlb
> folios. I'd add a check for folio size just in case, something like
> below (I'll add some description and post it properly):
> 
> diff --git a/arch/arm64/mm/copypage.c b/arch/arm64/mm/copypage.c
> index 87b3f1a25535..c3a83db46ec6 100644
> --- a/arch/arm64/mm/copypage.c
> +++ b/arch/arm64/mm/copypage.c
> @@ -30,11 +30,14 @@ void copy_highpage(struct page *to, struct page *from)
>   	if (!system_supports_mte())
>   		return;
>   
> -	if (folio_test_hugetlb(src) &&
> -	    folio_test_hugetlb_mte_tagged(src)) {
> -		if (!folio_try_hugetlb_mte_tagging(dst))
> +	if (folio_test_hugetlb(src)) {

To be safe, maybe also test that dst is a hugetlb folio? But maybe the 
implicit checks we added in folio_try_hugetlb_mte_tagging() will already 
check that.

> +		if (!folio_test_hugetlb_mte_tagged(src) ||
> +		    from != folio_page(src, 0) ||
> +		    WARN_ON_ONCE(folio_nr_pages(src) != folio_nr_pages(dst)))
>   			return;
>   
> +		WARN_ON_ONCE(!folio_try_hugetlb_mte_tagging(dst));
> +
>   		/*
>   		 * Populate tags for all subpages.
>   		 *
> 


-- 
Cheers,

David / dhildenb

Re: [GIT PULL] arm64 updates for 6.13-rc1

[Yang Shi]

On 12/4/24 8:00 AM, David Hildenbrand wrote:
> On 04.12.24 16:50, Catalin Marinas wrote:
>> On Wed, Dec 04, 2024 at 04:32:11PM +0100, David Hildenbrand wrote:
>>> On 04.12.24 16:29, Catalin Marinas wrote:
>>>> On Mon, Dec 02, 2024 at 08:22:57AM -0800, Yang Shi wrote:
>>>>> On 11/28/24 1:56 AM, David Hildenbrand wrote:
>>>>>> On 28.11.24 02:21, Yang Shi wrote:
>>>>>>>>> diff --git a/arch/arm64/mm/copypage.c b/arch/arm64/mm/copypage.c
>>>>>>>>> index 87b3f1a25535..ef303a2262c5 100644
>>>>>>>>> --- a/arch/arm64/mm/copypage.c
>>>>>>>>> +++ b/arch/arm64/mm/copypage.c
>>>>>>>>> @@ -30,9 +30,9 @@ void copy_highpage(struct page *to, struct
>>>>>>>>> page *from)
>>>>>>>>>           if (!system_supports_mte())
>>>>>>>>>               return;
>>>>>>>>> -    if (folio_test_hugetlb(src) &&
>>>>>>>>> -        folio_test_hugetlb_mte_tagged(src)) {
>>>>>>>>> -        if (!folio_try_hugetlb_mte_tagging(dst))
>>>>>>>>> +    if (folio_test_hugetlb(src)) {
>>>>>>>>> +        if (!folio_test_hugetlb_mte_tagged(src) ||
>>>>>>>>> +            !folio_try_hugetlb_mte_tagging(dst))
>>>>>>>>>                   return;
>>>>>>>>>               /*
>>>>>>>> I wonder why we had a 'return' here originally rather than a
>>>>>>>> WARN_ON_ONCE() as we do further down for the page case. Do you 
>>>>>>>> seen any
>>>>>>>> issue with the hunk below? Destination should be a new folio 
>>>>>>>> and not
>>>>>>>> tagged yet:
>>>>>>>
>>>>>>> Yes, I did see problem. Because we copy tags for all sub pages 
>>>>>>> then set
>>>>>>> folio mte tagged when copying the data for the first subpage. The
>>>>>>> warning will be triggered when we copy the second subpage.
>>>>>>
>>>>>> It's rather weird, though. We're instructed to copy a single 
>>>>>> page, yet
>>>>>> copy tags for all pages.
>>>>>>
>>>>>> This really only makes sense when called from folio_copy(), where 
>>>>>> we are
>>>>>> guaranteed to copy all pages.
>>>>>>
>>>>>> I'm starting to wonder if we should be able to hook into / overload
>>>>>> folio_copy() instead, to just handle the complete hugetlb copy 
>>>>>> ourselves
>>>>>> in one shot, and assume that copy_highpage() will never be called 
>>>>>> for
>>>>>> hugetlb pages (WARN and don't copy tags).
>>>>>
>>>>> Actually folio_copy() is just called by migration. Copy huge page 
>>>>> in CoW is
>>>>> more complicated and uses copy_user_highpage()->copy_highpage() 
>>>>> instead of
>>>>> folio_copy(). It may start the page copy from any subpage. For 
>>>>> example, if
>>>>> the CoW is triggered by accessing to the address in the middle of 
>>>>> 2M. Kernel
>>>>> may copy the second half first then the first half to guarantee 
>>>>> the accessed
>>>>> data in cache.
>>>>
>>>> Still trying to understand the possible call paths here. If we get a
>>>> write fault on a large folio, does the core code allocate a folio 
>>>> of the
>>>> same size for CoW or it starts with smaller ones? wp_page_copy()
>>>> allocates order 0 AFAICT, though if it was a pmd fault, it takes a
>>>> different path in handle_mm_fault(). But we also have huge pages using
>>>> contiguous ptes.
>>>>
>>>> Unless the source and destinations folios are exactly the same 
>>>> size, it
>>>> will break many assumptions in the code above. Going the other way
>>>> around is also wrong, dst larger than src, we are not initialising the
>>>> whole dst folio.
>>>>
>>>> Maybe going back to per-page PG_mte_tagged flag rather than per-folio
>>>> would keep things simple, less risk of wrong assumptions.
>>>
>>> I think the magic bit here is that for hugetlb, we only get hugetlb 
>>> folios
>>> of the same size, and no mixtures.
>>
>> Ah, ok, we do check for this and only do the advance copy for hugetlb
>> folios. I'd add a check for folio size just in case, something like
>> below (I'll add some description and post it properly):
>>
>> diff --git a/arch/arm64/mm/copypage.c b/arch/arm64/mm/copypage.c
>> index 87b3f1a25535..c3a83db46ec6 100644
>> --- a/arch/arm64/mm/copypage.c
>> +++ b/arch/arm64/mm/copypage.c
>> @@ -30,11 +30,14 @@ void copy_highpage(struct page *to, struct page 
>> *from)
>>       if (!system_supports_mte())
>>           return;
>>   -    if (folio_test_hugetlb(src) &&
>> -        folio_test_hugetlb_mte_tagged(src)) {
>> -        if (!folio_try_hugetlb_mte_tagging(dst))
>> +    if (folio_test_hugetlb(src)) {
>
> To be safe, maybe also test that dst is a hugetlb folio? But maybe the 
> implicit checks we added in folio_try_hugetlb_mte_tagging() will 
> already check that.

Yes, we have "VM_WARN_ON_ONCE(!folio_test_hugetlb(folio))" in 
folio_try_hugetlb_mte_tagging(), it should be good enough.

>
>> +        if (!folio_test_hugetlb_mte_tagged(src) ||
>> +            from != folio_page(src, 0) ||
>> +            WARN_ON_ONCE(folio_nr_pages(src) != folio_nr_pages(dst)))
>>               return;
>>   +        WARN_ON_ONCE(!folio_try_hugetlb_mte_tagging(dst));
>> +
>>           /*
>>            * Populate tags for all subpages.
>>            *
>>
>
>