[PATCH v1 0/5] arm64/mm: uffd write-protect and soft-dirty tracking

[PATCH v1 0/5] arm64/mm: uffd write-protect and soft-dirty tracking

[Ryan Roberts]

Hi All,

This series adds uffd write-protect and soft-dirty tracking support for arm64. I
consider the soft-dirty support (patches 3 and 4) as RFC - see rationale below.

Previous attempts to add these features have failed because of a perceived lack
of available PTE SW bits. However it actually turns out that there are 2
available but they are hidden. PTE_PROT_NONE was previously occupying a SW bit,
but it only applies when PTE_VALID is clear, so this is moved to overlay PTE_UXN
in patch 1, freeing up the SW bit. Bit 63 is marked as "IGNORED" in the Arm ARM,
but it does not currently indicate "reserved for SW use" like it does for the
other SW bits. I've confirmed with the spec owner that this is an oversight; the
bit is intended to be reserved for SW use and the spec will clarify this in a
future update.

So we have our two bits; patch 2 enables uffd-wp, patch 3 enables soft-dirty and
patches 4 and 5 sort out the selftests so that the soft-dirty tests are compiled
for, and run on arm64.

That said, these are the last 2 SW bits and we may want to keep 1 bit in reserve
for future use. soft-dirty is only used for CRIU to my knowledge, and it is
thought that their use case could be solved with the more generic uffd-wp. So
unless somebody makes a clear case for the inclusion of soft-dirty support, we
are probably better off dropping patches 3 and 4 and keeping bit 63 for future
use. Although note that the most recent attempt to add soft-dirty for arm64 was
last month [1] so I'd like to give Shivansh Vij the opportunity to make the
case.

---8<---
As an appendix, I've also experimented with adding an "extended SW bits" region
linked by the `struct ptdesc` (which you can always find from the `pte_t *`). If
demonstrated to work, this would act as an insurance policy in case we ever need
more SW bits in future, giving us confidence to merge soft-dirty now.
Unfortunately this approach suffers from 2 problems; 1) its slow; my fork()
microbenchmark takes 40% longer in the worst case. 2) it is not possible to read
the HW pte and the extended SW bits atomically so it is impossible to implement
ptep_get_lockess() in its current form. So I've abandoned this experiment. (I
can provide more details if there is interest).
---8<---

[1] https://lore.kernel.org/linux-arm-kernel/MW4PR12MB687563EFB56373E8D55DDEABB92B2@MW4PR12MB6875.namprd12.prod.outlook.com/

Thanks,
Ryan


Ryan Roberts (5):
  arm64/mm: Move PTE_PROT_NONE and PMD_PRESENT_INVALID
  arm64/mm: Add uffd write-protect support
  arm64/mm: Add soft-dirty page tracking support
  selftests/mm: Enable soft-dirty tests on arm64
  selftests/mm: soft-dirty should fail if a testcase fails

 arch/arm64/Kconfig                         |   2 +
 arch/arm64/include/asm/pgtable-prot.h      |  20 +++-
 arch/arm64/include/asm/pgtable.h           | 118 +++++++++++++++++++--
 arch/arm64/mm/contpte.c                    |   6 +-
 arch/arm64/mm/fault.c                      |   3 +-
 arch/arm64/mm/hugetlbpage.c                |   6 +-
 tools/testing/selftests/mm/Makefile        |   5 +-
 tools/testing/selftests/mm/madv_populate.c |  26 +----
 tools/testing/selftests/mm/run_vmtests.sh  |   5 +-
 tools/testing/selftests/mm/soft-dirty.c    |   2 +-
 10 files changed, 141 insertions(+), 52 deletions(-)

--
2.25.1

[PATCH v1 1/5] arm64/mm: Move PTE_PROT_NONE and PMD_PRESENT_INVALID

[Ryan Roberts]

Previously PTE_PROT_NONE was occupying bit 58, one of the bits reserved
for SW use when the PTE is valid. This is a waste of those precious SW
bits since PTE_PROT_NONE can only ever be set when valid is clear.
Instead let's overlay it on what would be a HW bit if valid was set.

We need to be careful about which HW bit to choose since some of them
must be preserved; when pte_present() is true (as it is for a
PTE_PROT_NONE pte), it is legitimate for the core to call various
accessors, e.g. pte_dirty(), pte_write() etc. There are also some
accessors that are private to the arch which must continue to be
honoured, e.g. pte_user(), pte_user_exec() etc.

So we choose to overlay PTE_UXN; This effectively means that whenever a
pte has PTE_PROT_NONE set, it will always report pte_user_exec() ==
false, which is obviously always correct.

As a result of this change, we must shuffle the layout of the
arch-specific swap pte so that PTE_PROT_NONE is always zero and not
overlapping with any other field. As a result of this, there is no way
to keep the `type` field contiguous without conflicting with
PMD_PRESENT_INVALID (bit 59), which must also be 0 for a swap pte. So
let's move PMD_PRESENT_INVALID to bit 60.

In the end, this frees up bit 58 for future use as a proper SW bit (e.g.
soft-dirty or uffd-wp).

Signed-off-by: Ryan Roberts <ryan.roberts@arm.com>
---
 arch/arm64/include/asm/pgtable-prot.h |  4 ++--
 arch/arm64/include/asm/pgtable.h      | 16 +++++++++-------
 2 files changed, 11 insertions(+), 9 deletions(-)

diff --git a/arch/arm64/include/asm/pgtable-prot.h b/arch/arm64/include/asm/pgtable-prot.h
index dd9ee67d1d87..ef952d69fd04 100644
--- a/arch/arm64/include/asm/pgtable-prot.h
+++ b/arch/arm64/include/asm/pgtable-prot.h
@@ -18,14 +18,14 @@
 #define PTE_DIRTY		(_AT(pteval_t, 1) << 55)
 #define PTE_SPECIAL		(_AT(pteval_t, 1) << 56)
 #define PTE_DEVMAP		(_AT(pteval_t, 1) << 57)
-#define PTE_PROT_NONE		(_AT(pteval_t, 1) << 58) /* only when !PTE_VALID */
+#define PTE_PROT_NONE		(PTE_UXN)		 /* Reuse PTE_UXN; only when !PTE_VALID */

 /*
  * This bit indicates that the entry is present i.e. pmd_page()
  * still points to a valid huge page in memory even if the pmd
  * has been invalidated.
  */
-#define PMD_PRESENT_INVALID	(_AT(pteval_t, 1) << 59) /* only when !PMD_SECT_VALID */
+#define PMD_PRESENT_INVALID	(_AT(pteval_t, 1) << 60) /* only when !PMD_SECT_VALID */

 #define _PROT_DEFAULT		(PTE_TYPE_PAGE | PTE_AF | PTE_SHARED)
 #define _PROT_SECT_DEFAULT	(PMD_TYPE_SECT | PMD_SECT_AF | PMD_SECT_S)
diff --git a/arch/arm64/include/asm/pgtable.h b/arch/arm64/include/asm/pgtable.h
index afdd56d26ad7..23aabff4fa6f 100644
--- a/arch/arm64/include/asm/pgtable.h
+++ b/arch/arm64/include/asm/pgtable.h
@@ -1248,20 +1248,22 @@ static inline pmd_t pmdp_establish(struct vm_area_struct *vma,
  * Encode and decode a swap entry:
  *	bits 0-1:	present (must be zero)
  *	bits 2:		remember PG_anon_exclusive
- *	bits 3-7:	swap type
- *	bits 8-57:	swap offset
- *	bit  58:	PTE_PROT_NONE (must be zero)
+ *	bits 4-53:	swap offset
+ *	bit  54:	PTE_PROT_NONE (overlays PTE_UXN) (must be zero)
+ *	bits 55-59:	swap type
+ *	bit  60:	PMD_PRESENT_INVALID (must be zero)
  */
-#define __SWP_TYPE_SHIFT	3
+#define __SWP_TYPE_SHIFT	55
 #define __SWP_TYPE_BITS		5
-#define __SWP_OFFSET_BITS	50
 #define __SWP_TYPE_MASK		((1 << __SWP_TYPE_BITS) - 1)
-#define __SWP_OFFSET_SHIFT	(__SWP_TYPE_BITS + __SWP_TYPE_SHIFT)
+#define __SWP_OFFSET_SHIFT	4
+#define __SWP_OFFSET_BITS	50
 #define __SWP_OFFSET_MASK	((1UL << __SWP_OFFSET_BITS) - 1)

 #define __swp_type(x)		(((x).val >> __SWP_TYPE_SHIFT) & __SWP_TYPE_MASK)
 #define __swp_offset(x)		(((x).val >> __SWP_OFFSET_SHIFT) & __SWP_OFFSET_MASK)
-#define __swp_entry(type,offset) ((swp_entry_t) { ((type) << __SWP_TYPE_SHIFT) | ((offset) << __SWP_OFFSET_SHIFT) })
+#define __swp_entry(type, offset) ((swp_entry_t) { ((unsigned long)(type) << __SWP_TYPE_SHIFT) | \
+						   ((unsigned long)(offset) << __SWP_OFFSET_SHIFT) })

 #define __pte_to_swp_entry(pte)	((swp_entry_t) { pte_val(pte) })
 #define __swp_entry_to_pte(swp)	((pte_t) { (swp).val })
--
2.25.1

[PATCH v1 2/5] arm64/mm: Add uffd write-protect support

[Ryan Roberts]

Let's use the newly-free PTE SW bit (58) to add support for uffd-wp.

The standard handlers are implemented for set/test/clear for both pte
and pmd. Additionally we must also track the uffd-wp state as a pte swp
bit, so use a free swap entry pte bit (3).

Signed-off-by: Ryan Roberts <ryan.roberts@arm.com>
---
 arch/arm64/Kconfig                    |  1 +
 arch/arm64/include/asm/pgtable-prot.h |  8 ++++
 arch/arm64/include/asm/pgtable.h      | 55 +++++++++++++++++++++++++++
 3 files changed, 64 insertions(+)

diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
index 7b11c98b3e84..763e221f2169 100644
--- a/arch/arm64/Kconfig
+++ b/arch/arm64/Kconfig
@@ -255,6 +255,7 @@ config ARM64
 	select SYSCTL_EXCEPTION_TRACE
 	select THREAD_INFO_IN_TASK
 	select HAVE_ARCH_USERFAULTFD_MINOR if USERFAULTFD
+	select HAVE_ARCH_USERFAULTFD_WP if USERFAULTFD
 	select TRACE_IRQFLAGS_SUPPORT
 	select TRACE_IRQFLAGS_NMI_SUPPORT
 	select HAVE_SOFTIRQ_ON_OWN_STACK
diff --git a/arch/arm64/include/asm/pgtable-prot.h b/arch/arm64/include/asm/pgtable-prot.h
index ef952d69fd04..f1e1f6306e03 100644
--- a/arch/arm64/include/asm/pgtable-prot.h
+++ b/arch/arm64/include/asm/pgtable-prot.h
@@ -20,6 +20,14 @@
 #define PTE_DEVMAP		(_AT(pteval_t, 1) << 57)
 #define PTE_PROT_NONE		(PTE_UXN)		 /* Reuse PTE_UXN; only when !PTE_VALID */

+#ifdef CONFIG_HAVE_ARCH_USERFAULTFD_WP
+#define PTE_UFFD_WP		(_AT(pteval_t, 1) << 58) /* uffd-wp tracking */
+#define PTE_SWP_UFFD_WP		(_AT(pteval_t, 1) << 3)	 /* only for swp ptes */
+#else
+#define PTE_UFFD_WP		(_AT(pteval_t, 0))
+#define PTE_SWP_UFFD_WP		(_AT(pteval_t, 0))
+#endif /* CONFIG_HAVE_ARCH_USERFAULTFD_WP */
+
 /*
  * This bit indicates that the entry is present i.e. pmd_page()
  * still points to a valid huge page in memory even if the pmd
diff --git a/arch/arm64/include/asm/pgtable.h b/arch/arm64/include/asm/pgtable.h
index 23aabff4fa6f..3f4748741fdb 100644
--- a/arch/arm64/include/asm/pgtable.h
+++ b/arch/arm64/include/asm/pgtable.h
@@ -271,6 +271,34 @@ static inline pte_t pte_mkdevmap(pte_t pte)
 	return set_pte_bit(pte, __pgprot(PTE_DEVMAP | PTE_SPECIAL));
 }

+#ifdef CONFIG_HAVE_ARCH_USERFAULTFD_WP
+static inline int pte_uffd_wp(pte_t pte)
+{
+	bool wp = !!(pte_val(pte) & PTE_UFFD_WP);
+
+#ifdef CONFIG_DEBUG_VM
+	/*
+	 * Having write bit for wr-protect-marked present ptes is fatal, because
+	 * it means the uffd-wp bit will be ignored and write will just go
+	 * through. See comment in x86 implementation.
+	 */
+	WARN_ON_ONCE(wp && pte_write(pte));
+#endif
+
+	return wp;
+}
+
+static inline pte_t pte_mkuffd_wp(pte_t pte)
+{
+	return pte_wrprotect(set_pte_bit(pte, __pgprot(PTE_UFFD_WP)));
+}
+
+static inline pte_t pte_clear_uffd_wp(pte_t pte)
+{
+	return clear_pte_bit(pte, __pgprot(PTE_UFFD_WP));
+}
+#endif /* CONFIG_HAVE_ARCH_USERFAULTFD_WP */
+
 static inline void __set_pte(pte_t *ptep, pte_t pte)
 {
 	WRITE_ONCE(*ptep, pte);
@@ -463,6 +491,23 @@ static inline pte_t pte_swp_clear_exclusive(pte_t pte)
 	return clear_pte_bit(pte, __pgprot(PTE_SWP_EXCLUSIVE));
 }

+#ifdef CONFIG_HAVE_ARCH_USERFAULTFD_WP
+static inline pte_t pte_swp_mkuffd_wp(pte_t pte)
+{
+	return set_pte_bit(pte, __pgprot(PTE_SWP_UFFD_WP));
+}
+
+static inline int pte_swp_uffd_wp(pte_t pte)
+{
+	return !!(pte_val(pte) & PTE_SWP_UFFD_WP);
+}
+
+static inline pte_t pte_swp_clear_uffd_wp(pte_t pte)
+{
+	return clear_pte_bit(pte, __pgprot(PTE_SWP_UFFD_WP));
+}
+#endif /* CONFIG_HAVE_ARCH_USERFAULTFD_WP */
+
 #ifdef CONFIG_NUMA_BALANCING
 /*
  * See the comment in include/linux/pgtable.h
@@ -508,6 +553,15 @@ static inline int pmd_trans_huge(pmd_t pmd)
 #define pmd_mkclean(pmd)	pte_pmd(pte_mkclean(pmd_pte(pmd)))
 #define pmd_mkdirty(pmd)	pte_pmd(pte_mkdirty(pmd_pte(pmd)))
 #define pmd_mkyoung(pmd)	pte_pmd(pte_mkyoung(pmd_pte(pmd)))
+#ifdef CONFIG_HAVE_ARCH_USERFAULTFD_WP
+#define pmd_uffd_wp(pmd)	pte_uffd_wp(pmd_pte(pmd))
+#define pmd_mkuffd_wp(pmd)	pte_pmd(pte_mkuffd_wp(pmd_pte(pmd)))
+#define pmd_clear_uffd_wp(pmd)	pte_pmd(pte_clear_uffd_wp(pmd_pte(pmd)))
+#define pmd_swp_uffd_wp(pmd)	pte_swp_uffd_wp(pmd_pte(pmd))
+#define pmd_swp_mkuffd_wp(pmd)	pte_pmd(pte_swp_mkuffd_wp(pmd_pte(pmd)))
+#define pmd_swp_clear_uffd_wp(pmd) \
+				pte_pmd(pte_swp_clear_uffd_wp(pmd_pte(pmd)))
+#endif /* CONFIG_HAVE_ARCH_USERFAULTFD_WP */

 static inline pmd_t pmd_mkinvalid(pmd_t pmd)
 {
@@ -1248,6 +1302,7 @@ static inline pmd_t pmdp_establish(struct vm_area_struct *vma,
  * Encode and decode a swap entry:
  *	bits 0-1:	present (must be zero)
  *	bits 2:		remember PG_anon_exclusive
+ *	bit  3:		remember uffd-wp state
  *	bits 4-53:	swap offset
  *	bit  54:	PTE_PROT_NONE (overlays PTE_UXN) (must be zero)
  *	bits 55-59:	swap type
--
2.25.1

[RFC PATCH v1 3/5] arm64/mm: Add soft-dirty page tracking support

[Ryan Roberts]

Use the final remaining PTE SW bit (63) for soft-dirty tracking. The
standard handlers are implemented for set/test/clear for both pte and
pmd. Additionally we must also track the soft-dirty state as a pte swp
bit, so use a free swap entry pte bit (61).

There are a few complexities worth calling out:

  - The semantic of soft-dirty calls for having it auto-set by
    pte_mkdirty(). But the arch code would previously call pte_mkdirty()
    for various house-keeping operations such as gathering dirty bits
    into a pte across a contpte block. These operations must not cause
    soft-dirty to be set. So an internal version, __pte_mkdirty(), has
    been created that does not manipulate soft-dirty, and pte_mkdirty()
    is now a wrapper around that, which also sets the soft-dirty bit.

  - For a region with soft-dirty tracking enabled, it works by
    wrprotecting the ptes, causing a write to fault, where the handler
    calls pte_mkdirty(ptep_get()) (which causes soft-dirty to be set),
    then the resulting pte is written back with ptep_set_access_flags().
    So the arm64 version of ptep_set_access_flags() now needs to
    explicitly also set the soft-dirty bit to prevent loss.

The patch is very loosely based on a similar patch posted by Shivansh
Vij <shivanshvij@outlook.com>, at the below link.

Primary motivation for adding soft-dirty support is to allow
Checkpoint-Restore in Userspace (CRIU) to be able to track a memory
page's changes if we want to enable pre-dumping, which is important for
live migration.

Link: https://lore.kernel.org/linux-arm-kernel/MW4PR12MB687563EFB56373E8D55DDEABB92B2@MW4PR12MB6875.namprd12.prod.outlook.com/
Signed-off-by: Ryan Roberts <ryan.roberts@arm.com>
---
 arch/arm64/Kconfig                    |  1 +
 arch/arm64/include/asm/pgtable-prot.h |  8 +++++
 arch/arm64/include/asm/pgtable.h      | 47 +++++++++++++++++++++++++--
 arch/arm64/mm/contpte.c               |  6 ++--
 arch/arm64/mm/fault.c                 |  3 +-
 arch/arm64/mm/hugetlbpage.c           |  6 ++--
 6 files changed, 61 insertions(+), 10 deletions(-)

diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
index 763e221f2169..3a5e22208e38 100644
--- a/arch/arm64/Kconfig
+++ b/arch/arm64/Kconfig
@@ -178,6 +178,7 @@ config ARM64
 	select HAVE_ARCH_PREL32_RELOCATIONS
 	select HAVE_ARCH_RANDOMIZE_KSTACK_OFFSET
 	select HAVE_ARCH_SECCOMP_FILTER
+	select HAVE_ARCH_SOFT_DIRTY
 	select HAVE_ARCH_STACKLEAK
 	select HAVE_ARCH_THREAD_STRUCT_WHITELIST
 	select HAVE_ARCH_TRACEHOOK
diff --git a/arch/arm64/include/asm/pgtable-prot.h b/arch/arm64/include/asm/pgtable-prot.h
index f1e1f6306e03..7fce22ed3fda 100644
--- a/arch/arm64/include/asm/pgtable-prot.h
+++ b/arch/arm64/include/asm/pgtable-prot.h
@@ -28,6 +28,14 @@
 #define PTE_SWP_UFFD_WP		(_AT(pteval_t, 0))
 #endif /* CONFIG_HAVE_ARCH_USERFAULTFD_WP */

+#ifdef CONFIG_MEM_SOFT_DIRTY
+#define PTE_SOFT_DIRTY		(_AT(pteval_t, 1) << 63) /* soft-dirty tracking */
+#define PTE_SWP_SOFT_DIRTY	(_AT(pteval_t, 1) << 61) /* only for swp ptes */
+#else
+#define PTE_SOFT_DIRTY		(_AT(pteval_t, 0))
+#define PTE_SWP_SOFT_DIRTY	(_AT(pteval_t, 0))
+#endif /* CONFIG_MEM_SOFT_DIRTY */
+
 /*
  * This bit indicates that the entry is present i.e. pmd_page()
  * still points to a valid huge page in memory even if the pmd
diff --git a/arch/arm64/include/asm/pgtable.h b/arch/arm64/include/asm/pgtable.h
index 3f4748741fdb..0118e6e0adde 100644
--- a/arch/arm64/include/asm/pgtable.h
+++ b/arch/arm64/include/asm/pgtable.h
@@ -114,6 +114,7 @@ static inline pteval_t __phys_to_pte_val(phys_addr_t phys)
 #define pte_user_exec(pte)	(!(pte_val(pte) & PTE_UXN))
 #define pte_cont(pte)		(!!(pte_val(pte) & PTE_CONT))
 #define pte_devmap(pte)		(!!(pte_val(pte) & PTE_DEVMAP))
+#define pte_soft_dirty(pte)	(!!(pte_val(pte) & PTE_SOFT_DIRTY))
 #define pte_tagged(pte)		((pte_val(pte) & PTE_ATTRINDX_MASK) == \
 				 PTE_ATTRINDX(MT_NORMAL_TAGGED))

@@ -206,7 +207,7 @@ static inline pte_t pte_mkclean(pte_t pte)
 	return pte;
 }

-static inline pte_t pte_mkdirty(pte_t pte)
+static inline pte_t __pte_mkdirty(pte_t pte)
 {
 	pte = set_pte_bit(pte, __pgprot(PTE_DIRTY));

@@ -216,6 +217,11 @@ static inline pte_t pte_mkdirty(pte_t pte)
 	return pte;
 }

+static inline pte_t pte_mkdirty(pte_t pte)
+{
+	return __pte_mkdirty(set_pte_bit(pte, __pgprot(PTE_SOFT_DIRTY)));
+}
+
 static inline pte_t pte_wrprotect(pte_t pte)
 {
 	/*
@@ -299,6 +305,16 @@ static inline pte_t pte_clear_uffd_wp(pte_t pte)
 }
 #endif /* CONFIG_HAVE_ARCH_USERFAULTFD_WP */

+static inline pte_t pte_mksoft_dirty(pte_t pte)
+{
+	return set_pte_bit(pte, __pgprot(PTE_SOFT_DIRTY));
+}
+
+static inline pte_t pte_clear_soft_dirty(pte_t pte)
+{
+	return clear_pte_bit(pte, __pgprot(PTE_SOFT_DIRTY));
+}
+
 static inline void __set_pte(pte_t *ptep, pte_t pte)
 {
 	WRITE_ONCE(*ptep, pte);
@@ -508,6 +524,21 @@ static inline pte_t pte_swp_clear_uffd_wp(pte_t pte)
 }
 #endif /* CONFIG_HAVE_ARCH_USERFAULTFD_WP */

+static inline pte_t pte_swp_mksoft_dirty(pte_t pte)
+{
+	return set_pte_bit(pte, __pgprot(PTE_SWP_SOFT_DIRTY));
+}
+
+static inline bool pte_swp_soft_dirty(pte_t pte)
+{
+	return !!(pte_val(pte) & PTE_SWP_SOFT_DIRTY);
+}
+
+static inline pte_t pte_swp_clear_soft_dirty(pte_t pte)
+{
+	return clear_pte_bit(pte, __pgprot(PTE_SWP_SOFT_DIRTY));
+}
+
 #ifdef CONFIG_NUMA_BALANCING
 /*
  * See the comment in include/linux/pgtable.h
@@ -562,6 +593,15 @@ static inline int pmd_trans_huge(pmd_t pmd)
 #define pmd_swp_clear_uffd_wp(pmd) \
 				pte_pmd(pte_swp_clear_uffd_wp(pmd_pte(pmd)))
 #endif /* CONFIG_HAVE_ARCH_USERFAULTFD_WP */
+#define pmd_soft_dirty(pmd)	pte_soft_dirty(pmd_pte(pmd))
+#define pmd_mksoft_dirty(pmd)	pte_pmd(pte_mksoft_dirty(pmd_pte(pmd)))
+#define pmd_clear_soft_dirty(pmd) \
+				pte_pmd(pte_clear_soft_dirty(pmd_pte(pmd)))
+#define pmd_swp_soft_dirty(pmd)	pte_swp_soft_dirty(pmd_pte(pmd))
+#define pmd_swp_mksoft_dirty(pmd) \
+				pte_pmd(pte_swp_mksoft_dirty(pmd_pte(pmd)))
+#define pmd_swp_clear_soft_dirty(pmd) \
+				pte_pmd(pte_swp_clear_soft_dirty(pmd_pte(pmd)))

 static inline pmd_t pmd_mkinvalid(pmd_t pmd)
 {
@@ -1093,7 +1133,7 @@ static inline pte_t pte_modify(pte_t pte, pgprot_t newprot)
 	 * dirtiness again.
 	 */
 	if (pte_sw_dirty(pte))
-		pte = pte_mkdirty(pte);
+		pte = __pte_mkdirty(pte);
 	return pte;
 }

@@ -1228,7 +1268,7 @@ static inline pte_t __get_and_clear_full_ptes(struct mm_struct *mm,
 		addr += PAGE_SIZE;
 		tmp_pte = __ptep_get_and_clear(mm, addr, ptep);
 		if (pte_dirty(tmp_pte))
-			pte = pte_mkdirty(pte);
+			pte = __pte_mkdirty(pte);
 		if (pte_young(tmp_pte))
 			pte = pte_mkyoung(pte);
 	}
@@ -1307,6 +1347,7 @@ static inline pmd_t pmdp_establish(struct vm_area_struct *vma,
  *	bit  54:	PTE_PROT_NONE (overlays PTE_UXN) (must be zero)
  *	bits 55-59:	swap type
  *	bit  60:	PMD_PRESENT_INVALID (must be zero)
+ *	bit  61:	remember soft-dirty state
  */
 #define __SWP_TYPE_SHIFT	55
 #define __SWP_TYPE_BITS		5
diff --git a/arch/arm64/mm/contpte.c b/arch/arm64/mm/contpte.c
index 1b64b4c3f8bf..c6f52fcf5d9a 100644
--- a/arch/arm64/mm/contpte.c
+++ b/arch/arm64/mm/contpte.c
@@ -62,7 +62,7 @@ static void contpte_convert(struct mm_struct *mm, unsigned long addr,
 		pte_t ptent = __ptep_get_and_clear(mm, addr, ptep);

 		if (pte_dirty(ptent))
-			pte = pte_mkdirty(pte);
+			pte = __pte_mkdirty(pte);

 		if (pte_young(ptent))
 			pte = pte_mkyoung(pte);
@@ -170,7 +170,7 @@ pte_t contpte_ptep_get(pte_t *ptep, pte_t orig_pte)
 		pte = __ptep_get(ptep);

 		if (pte_dirty(pte))
-			orig_pte = pte_mkdirty(orig_pte);
+			orig_pte = __pte_mkdirty(orig_pte);

 		if (pte_young(pte))
 			orig_pte = pte_mkyoung(orig_pte);
@@ -227,7 +227,7 @@ pte_t contpte_ptep_get_lockless(pte_t *orig_ptep)
 			goto retry;

 		if (pte_dirty(pte))
-			orig_pte = pte_mkdirty(orig_pte);
+			orig_pte = __pte_mkdirty(orig_pte);

 		if (pte_young(pte))
 			orig_pte = pte_mkyoung(orig_pte);
diff --git a/arch/arm64/mm/fault.c b/arch/arm64/mm/fault.c
index 8251e2fea9c7..678171fd88bd 100644
--- a/arch/arm64/mm/fault.c
+++ b/arch/arm64/mm/fault.c
@@ -220,7 +220,8 @@ int __ptep_set_access_flags(struct vm_area_struct *vma,
 		return 0;

 	/* only preserve the access flags and write permission */
-	pte_val(entry) &= PTE_RDONLY | PTE_AF | PTE_WRITE | PTE_DIRTY;
+	pte_val(entry) &= PTE_RDONLY | PTE_AF | PTE_WRITE |
+			  PTE_DIRTY | PTE_SOFT_DIRTY;

 	/*
 	 * Setting the flags must be done atomically to avoid racing with the
diff --git a/arch/arm64/mm/hugetlbpage.c b/arch/arm64/mm/hugetlbpage.c
index 0f0e10bb0a95..4605eb146a2f 100644
--- a/arch/arm64/mm/hugetlbpage.c
+++ b/arch/arm64/mm/hugetlbpage.c
@@ -155,7 +155,7 @@ pte_t huge_ptep_get(pte_t *ptep)
 		pte_t pte = __ptep_get(ptep);

 		if (pte_dirty(pte))
-			orig_pte = pte_mkdirty(orig_pte);
+			orig_pte = __pte_mkdirty(orig_pte);

 		if (pte_young(pte))
 			orig_pte = pte_mkyoung(orig_pte);
@@ -189,7 +189,7 @@ static pte_t get_clear_contig(struct mm_struct *mm,
 		 * so check them all.
 		 */
 		if (pte_dirty(pte))
-			orig_pte = pte_mkdirty(orig_pte);
+			orig_pte = __pte_mkdirty(orig_pte);

 		if (pte_young(pte))
 			orig_pte = pte_mkyoung(orig_pte);
@@ -464,7 +464,7 @@ int huge_ptep_set_access_flags(struct vm_area_struct *vma,

 	/* Make sure we don't lose the dirty or young state */
 	if (pte_dirty(orig_pte))
-		pte = pte_mkdirty(pte);
+		pte = __pte_mkdirty(pte);

 	if (pte_young(orig_pte))
 		pte = pte_mkyoung(pte);
--
2.25.1

[RFC PATCH v1 4/5] selftests/mm: Enable soft-dirty tests on arm64

[Ryan Roberts]

Now that arm64 supports soft-dirty tracking lets enable the tests, which
were previously disabled for arm64 to reduce noise.

This reverts commit f6dd4e223d87 ("selftests/mm: skip soft-dirty tests
on arm64").

Signed-off-by: Ryan Roberts <ryan.roberts@arm.com>
---
 tools/testing/selftests/mm/Makefile        |  5 +----
 tools/testing/selftests/mm/madv_populate.c | 26 ++--------------------
 tools/testing/selftests/mm/run_vmtests.sh  |  5 +----
 3 files changed, 4 insertions(+), 32 deletions(-)

diff --git a/tools/testing/selftests/mm/Makefile b/tools/testing/selftests/mm/Makefile
index eb5f39a2668b..7f1a6ad09534 100644
--- a/tools/testing/selftests/mm/Makefile
+++ b/tools/testing/selftests/mm/Makefile
@@ -65,6 +65,7 @@ TEST_GEN_FILES += thuge-gen
 TEST_GEN_FILES += transhuge-stress
 TEST_GEN_FILES += uffd-stress
 TEST_GEN_FILES += uffd-unit-tests
+TEST_GEN_FILES += soft-dirty
 TEST_GEN_FILES += split_huge_page_test
 TEST_GEN_FILES += ksm_tests
 TEST_GEN_FILES += ksm_functional_tests
@@ -72,10 +73,6 @@ TEST_GEN_FILES += mdwe_test
 TEST_GEN_FILES += hugetlb_fault_after_madv
 TEST_GEN_FILES += hugetlb_madv_vs_map

-ifneq ($(ARCH),arm64)
-TEST_GEN_FILES += soft-dirty
-endif
-
 ifeq ($(ARCH),x86_64)
 CAN_BUILD_I386 := $(shell ./../x86/check_cc.sh "$(CC)" ../x86/trivial_32bit_program.c -m32)
 CAN_BUILD_X86_64 := $(shell ./../x86/check_cc.sh "$(CC)" ../x86/trivial_64bit_program.c)
diff --git a/tools/testing/selftests/mm/madv_populate.c b/tools/testing/selftests/mm/madv_populate.c
index 17bcb07f19f3..60547245e479 100644
--- a/tools/testing/selftests/mm/madv_populate.c
+++ b/tools/testing/selftests/mm/madv_populate.c
@@ -264,35 +264,14 @@ static void test_softdirty(void)
 	munmap(addr, SIZE);
 }

-static int system_has_softdirty(void)
-{
-	/*
-	 * There is no way to check if the kernel supports soft-dirty, other
-	 * than by writing to a page and seeing if the bit was set. But the
-	 * tests are intended to check that the bit gets set when it should, so
-	 * doing that check would turn a potentially legitimate fail into a
-	 * skip. Fortunately, we know for sure that arm64 does not support
-	 * soft-dirty. So for now, let's just use the arch as a corse guide.
-	 */
-#if defined(__aarch64__)
-	return 0;
-#else
-	return 1;
-#endif
-}
-
 int main(int argc, char **argv)
 {
-	int nr_tests = 16;
 	int err;

 	pagesize = getpagesize();

-	if (system_has_softdirty())
-		nr_tests += 5;
-
 	ksft_print_header();
-	ksft_set_plan(nr_tests);
+	ksft_set_plan(21);

 	sense_support();
 	test_prot_read();
@@ -300,8 +279,7 @@ int main(int argc, char **argv)
 	test_holes();
 	test_populate_read();
 	test_populate_write();
-	if (system_has_softdirty())
-		test_softdirty();
+	test_softdirty();

 	err = ksft_get_fail_cnt();
 	if (err)
diff --git a/tools/testing/selftests/mm/run_vmtests.sh b/tools/testing/selftests/mm/run_vmtests.sh
index c2c542fe7b17..29806d352c73 100755
--- a/tools/testing/selftests/mm/run_vmtests.sh
+++ b/tools/testing/selftests/mm/run_vmtests.sh
@@ -395,10 +395,7 @@ then
 	CATEGORY="pkey" run_test ./protection_keys_64
 fi

-if [ -x ./soft-dirty ]
-then
-	CATEGORY="soft_dirty" run_test ./soft-dirty
-fi
+CATEGORY="soft_dirty" run_test ./soft-dirty

 CATEGORY="pagemap" run_test ./pagemap_ioctl

--
2.25.1

[PATCH v1 5/5] selftests/mm: soft-dirty should fail if a testcase fails

[Ryan Roberts]

Previously soft-dirty was unconditionally exiting with success, even if
one of it's testcases failed. Let's fix that so that failure can be
reported to automated systems properly.

Signed-off-by: Ryan Roberts <ryan.roberts@arm.com>
---
 tools/testing/selftests/mm/soft-dirty.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/testing/selftests/mm/soft-dirty.c b/tools/testing/selftests/mm/soft-dirty.c
index 7dbfa53d93a0..bdfa5d085f00 100644
--- a/tools/testing/selftests/mm/soft-dirty.c
+++ b/tools/testing/selftests/mm/soft-dirty.c
@@ -209,5 +209,5 @@ int main(int argc, char **argv)

 	close(pagemap_fd);

-	return ksft_exit_pass();
+	ksft_finished();
 }
--
2.25.1

Re: [PATCH v1 0/5] arm64/mm: uffd write-protect and soft-dirty tracking

[Ryan Roberts]

On 19/04/2024 08:43, Ryan Roberts wrote:
> Hi All,
> 
> This series adds uffd write-protect and soft-dirty tracking support for arm64. I
> consider the soft-dirty support (patches 3 and 4) as RFC - see rationale below.
> 
> Previous attempts to add these features have failed because of a perceived lack
> of available PTE SW bits. However it actually turns out that there are 2
> available but they are hidden. PTE_PROT_NONE was previously occupying a SW bit,
> but it only applies when PTE_VALID is clear, so this is moved to overlay PTE_UXN
> in patch 1, freeing up the SW bit. Bit 63 is marked as "IGNORED" in the Arm ARM,
> but it does not currently indicate "reserved for SW use" like it does for the
> other SW bits. I've confirmed with the spec owner that this is an oversight; the
> bit is intended to be reserved for SW use and the spec will clarify this in a
> future update.
> 
> So we have our two bits; patch 2 enables uffd-wp, patch 3 enables soft-dirty and
> patches 4 and 5 sort out the selftests so that the soft-dirty tests are compiled
> for, and run on arm64.
> 
> That said, these are the last 2 SW bits and we may want to keep 1 bit in reserve
> for future use. soft-dirty is only used for CRIU to my knowledge, and it is
> thought that their use case could be solved with the more generic uffd-wp. So
> unless somebody makes a clear case for the inclusion of soft-dirty support, we
> are probably better off dropping patches 3 and 4 and keeping bit 63 for future
> use. Although note that the most recent attempt to add soft-dirty for arm64 was
> last month [1] so I'd like to give Shivansh Vij the opportunity to make the
> case.

Ugh, forgot to mention that this applies on top of v6.9-rc3, and all the uffd-wp
and soft-dirty tests in the mm selftests suite run and pass. And no regressions
are observed in any of the other selftests.


> 
> ---8<---
> As an appendix, I've also experimented with adding an "extended SW bits" region
> linked by the `struct ptdesc` (which you can always find from the `pte_t *`). If
> demonstrated to work, this would act as an insurance policy in case we ever need
> more SW bits in future, giving us confidence to merge soft-dirty now.
> Unfortunately this approach suffers from 2 problems; 1) its slow; my fork()
> microbenchmark takes 40% longer in the worst case. 2) it is not possible to read
> the HW pte and the extended SW bits atomically so it is impossible to implement
> ptep_get_lockess() in its current form. So I've abandoned this experiment. (I
> can provide more details if there is interest).
> ---8<---
> 
> [1] https://lore.kernel.org/linux-arm-kernel/MW4PR12MB687563EFB56373E8D55DDEABB92B2@MW4PR12MB6875.namprd12.prod.outlook.com/
> 
> Thanks,
> Ryan
> 
> 
> Ryan Roberts (5):
>   arm64/mm: Move PTE_PROT_NONE and PMD_PRESENT_INVALID
>   arm64/mm: Add uffd write-protect support
>   arm64/mm: Add soft-dirty page tracking support
>   selftests/mm: Enable soft-dirty tests on arm64
>   selftests/mm: soft-dirty should fail if a testcase fails
> 
>  arch/arm64/Kconfig                         |   2 +
>  arch/arm64/include/asm/pgtable-prot.h      |  20 +++-
>  arch/arm64/include/asm/pgtable.h           | 118 +++++++++++++++++++--
>  arch/arm64/mm/contpte.c                    |   6 +-
>  arch/arm64/mm/fault.c                      |   3 +-
>  arch/arm64/mm/hugetlbpage.c                |   6 +-
>  tools/testing/selftests/mm/Makefile        |   5 +-
>  tools/testing/selftests/mm/madv_populate.c |  26 +----
>  tools/testing/selftests/mm/run_vmtests.sh  |   5 +-
>  tools/testing/selftests/mm/soft-dirty.c    |   2 +-
>  10 files changed, 141 insertions(+), 52 deletions(-)
> 
> --
> 2.25.1
> 

Re: [PATCH v1 0/5] arm64/mm: uffd write-protect and soft-dirty tracking

[Shivansh Vij]

[-- Attachment #1: Type: text/plain, Size: 3247 bytes --]

Hey All,

On Fri, Apr 19, 2024 at 3:47 AM, Ryan Roberts <ryan.roberts@arm.com<mailto:ryan.roberts@arm.com>> wrote:

On 19/04/2024 08:43, Ryan Roberts wrote:

Hi All,

This series adds uffd write-protect and soft-dirty tracking support for arm64. I consider the soft-dirty support (patches 3 and 4) as RFC - see rationale below.

Previous attempts to add these features have failed because of a perceived lack of available PTE SW bits. However it actually turns out that there are 2 available but they are hidden. PTE_PROT_NONE was previously occupying a SW bit, but it only applies when PTE_VALID is clear, so this is moved to overlay PTE_UXN in patch 1, freeing up the SW bit. Bit 63 is marked as "IGNORED" in the Arm ARM, but it does not currently indicate "reserved for SW use" like it does for the other SW bits. I've confirmed with the spec owner that this is an oversight; the bit is intended to be reserved for SW use and the spec will clarify this in a future update.

So we have our two bits; patch 2 enables uffd-wp, patch 3 enables soft-dirty and patches 4 and 5 sort out the selftests so that the soft-dirty tests are compiled for, and run on arm64.

That said, these are the last 2 SW bits and we may want to keep 1 bit in reserve for future use. soft-dirty is only used for CRIU to my knowledge, and it is thought that their use case could be solved with the more generic uffd-wp. So unless somebody makes a clear case for the inclusion of soft-dirty support, we are probably better off dropping patches 3 and 4 and keeping bit 63 for future use. Although note that the most recent attempt to add soft-dirty for arm64 was last month [1] so I'd like to give Shivansh Vij the opportunity to make the case.

Ugh, forgot to mention that this applies on top of v6.9-rc3, and all the uffd-wp and soft-dirty tests in the mm selftests suite run and pass. And no regressions are observed in any of the other selftests.

Appreciate the opportunity to provide input here.

I personally don't know of any applications other than CRIU that make heavy use of soft-dirty, and my use case is specifically focused on adding live-migration support to CRIU on ARM.

Cloud providers like AWS have pretty massive discounts for ARM-based spot instances (90% last time I checked), and having live-migration in CRIU would allow more applications to take advantage of that.

As Ryan mentioned, there are two ways to achieve this - add dirty tracking to ARM (Patch 3/4), or tear out the existing dirty tracking code in CRIU and replace it with uffd-wp.

I picked option one (dirty tracking in arm) because it seems to be the simplest way to move forward, whereas it would be a relatively heavy effort to add uffd-wp support to CRIU.

From a performance perspective I am also a little worried that uffd will be slower than just tracking the dirty bits asynchronously with sw dirty, but maybe that's not as much of a concern with the addition of uffd-wp async.

With all this being said, I'll defer to the wisdom of the crowd about which approach makes more sense - after all, with this patch we should get uffd-wp support on arm so at least there will be _a_ way forward for CRIU (albeit one requiring slightly more work).




[-- Attachment #2: Type: text/html, Size: 8365 bytes --]

Re: [PATCH v1 5/5] selftests/mm: soft-dirty should fail if a testcase fails

[David Hildenbrand]

On 19.04.24 09:43, Ryan Roberts wrote:
> Previously soft-dirty was unconditionally exiting with success, even if
> one of it's testcases failed. Let's fix that so that failure can be
> reported to automated systems properly.
> 
> Signed-off-by: Ryan Roberts <ryan.roberts@arm.com>
> ---
>   tools/testing/selftests/mm/soft-dirty.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/tools/testing/selftests/mm/soft-dirty.c b/tools/testing/selftests/mm/soft-dirty.c
> index 7dbfa53d93a0..bdfa5d085f00 100644
> --- a/tools/testing/selftests/mm/soft-dirty.c
> +++ b/tools/testing/selftests/mm/soft-dirty.c
> @@ -209,5 +209,5 @@ int main(int argc, char **argv)
> 
>   	close(pagemap_fd);
> 
> -	return ksft_exit_pass();
> +	ksft_finished();
>   }
> --
> 2.25.1
> 

Guess that makes sense independent of all the other stuff?

Reviewed-by: David Hildenbrand <david@redhat.com>

-- 
Cheers,

David / dhildenb

Re: [PATCH v1 5/5] selftests/mm: soft-dirty should fail if a testcase fails

[Ryan Roberts]

On 22/04/2024 10:33, David Hildenbrand wrote:
> On 19.04.24 09:43, Ryan Roberts wrote:
>> Previously soft-dirty was unconditionally exiting with success, even if
>> one of it's testcases failed. Let's fix that so that failure can be
>> reported to automated systems properly.
>>
>> Signed-off-by: Ryan Roberts <ryan.roberts@arm.com>
>> ---
>>   tools/testing/selftests/mm/soft-dirty.c | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/tools/testing/selftests/mm/soft-dirty.c
>> b/tools/testing/selftests/mm/soft-dirty.c
>> index 7dbfa53d93a0..bdfa5d085f00 100644
>> --- a/tools/testing/selftests/mm/soft-dirty.c
>> +++ b/tools/testing/selftests/mm/soft-dirty.c
>> @@ -209,5 +209,5 @@ int main(int argc, char **argv)
>>
>>       close(pagemap_fd);
>>
>> -    return ksft_exit_pass();
>> +    ksft_finished();
>>   }
>> -- 
>> 2.25.1
>>
> 
> Guess that makes sense independent of all the other stuff?

Yes definitely. What's the process here? Do I need to re-post as a stand-alone
patch? Or perhaps, Shuah, you could take this into your tree as is?

> 
> Reviewed-by: David Hildenbrand <david@redhat.com>

Thanks!

Re: [PATCH v1 5/5] selftests/mm: soft-dirty should fail if a testcase fails

[Muhammad Usama Anjum]

On 4/23/24 1:24 PM, Ryan Roberts wrote:
> On 22/04/2024 10:33, David Hildenbrand wrote:
>> On 19.04.24 09:43, Ryan Roberts wrote:
>>> Previously soft-dirty was unconditionally exiting with success, even if
>>> one of it's testcases failed. Let's fix that so that failure can be
>>> reported to automated systems properly.
>>>
>>> Signed-off-by: Ryan Roberts <ryan.roberts@arm.com>
Reviewed-by: Muhammad Usama Anjum <usama.anjum@collabora.com>

>>> ---
>>>   tools/testing/selftests/mm/soft-dirty.c | 2 +-
>>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/tools/testing/selftests/mm/soft-dirty.c
>>> b/tools/testing/selftests/mm/soft-dirty.c
>>> index 7dbfa53d93a0..bdfa5d085f00 100644
>>> --- a/tools/testing/selftests/mm/soft-dirty.c
>>> +++ b/tools/testing/selftests/mm/soft-dirty.c
>>> @@ -209,5 +209,5 @@ int main(int argc, char **argv)
>>>
>>>       close(pagemap_fd);
>>>
>>> -    return ksft_exit_pass();
>>> +    ksft_finished();
>>>   }
>>> -- 
>>> 2.25.1
>>>
>>
>> Guess that makes sense independent of all the other stuff?
> 
> Yes definitely. What's the process here? Do I need to re-post as a stand-alone
> patch? Or perhaps, Shuah, you could take this into your tree as is?
She can. But if she misses it or you want to post v2 of this current
series, you can just send this one separately. Usually I try to send
separate patches for trivial and discussion required patches so that there
isn't confusion of this kind.

> 
>>
>> Reviewed-by: David Hildenbrand <david@redhat.com>
> 
> Thanks!
> 
> 
> 

-- 
BR,
Muhammad Usama Anjum

Re: [PATCH v1 5/5] selftests/mm: soft-dirty should fail if a testcase fails

[Ryan Roberts]

On 23/04/2024 09:44, Muhammad Usama Anjum wrote:
> On 4/23/24 1:24 PM, Ryan Roberts wrote:
>> On 22/04/2024 10:33, David Hildenbrand wrote:
>>> On 19.04.24 09:43, Ryan Roberts wrote:
>>>> Previously soft-dirty was unconditionally exiting with success, even if
>>>> one of it's testcases failed. Let's fix that so that failure can be
>>>> reported to automated systems properly.
>>>>
>>>> Signed-off-by: Ryan Roberts <ryan.roberts@arm.com>
> Reviewed-by: Muhammad Usama Anjum <usama.anjum@collabora.com>

Thanks!

> 
>>>> ---
>>>>   tools/testing/selftests/mm/soft-dirty.c | 2 +-
>>>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>>>
>>>> diff --git a/tools/testing/selftests/mm/soft-dirty.c
>>>> b/tools/testing/selftests/mm/soft-dirty.c
>>>> index 7dbfa53d93a0..bdfa5d085f00 100644
>>>> --- a/tools/testing/selftests/mm/soft-dirty.c
>>>> +++ b/tools/testing/selftests/mm/soft-dirty.c
>>>> @@ -209,5 +209,5 @@ int main(int argc, char **argv)
>>>>
>>>>       close(pagemap_fd);
>>>>
>>>> -    return ksft_exit_pass();
>>>> +    ksft_finished();
>>>>   }
>>>> -- 
>>>> 2.25.1
>>>>
>>>
>>> Guess that makes sense independent of all the other stuff?
>>
>> Yes definitely. What's the process here? Do I need to re-post as a stand-alone
>> patch? Or perhaps, Shuah, you could take this into your tree as is?
> She can. But if she misses it or you want to post v2 of this current
> series, you can just send this one separately. Usually I try to send
> separate patches for trivial and discussion required patches so that there
> isn't confusion of this kind.

Thanks - I'll do that.

> 
>>
>>>
>>> Reviewed-by: David Hildenbrand <david@redhat.com>
>>
>> Thanks!
>>
>>
>>
>