2023-07-03 14:27:44

by Nico Pache

[permalink] [raw]
Subject: [RFC] arm64: properly define SOFT_DIRTY for arm64

ARM64 has a soft-dirty bit (software dirty) but never properly defines
CONFIG_ARCH_HAS_SOFT_DIRTY or its necessary functions. This patch
introduces the ability to set/clear the soft dirty bit in a similar
manner as the other arches that utilize it.

However, we must be careful... there are cases where the DBM bit is not
available and the software dirty bit plays a essential role in determining
whether or not a page is dirty. In these cases we must not allow the
user to clear the software dirty bit. We can test for these cases by
utilizing the arch_faults_on_old_pte() function which test the availability
of DBM.

Cc: Andrew Morton <[email protected]>
Cc: Anshuman Khandual <[email protected]>
Cc: Catalin Marinas <[email protected]>
Cc: David Hildenbrand <[email protected]>
Cc: Gerald Schaefer <[email protected]>
Cc: Liu Shixin <[email protected]>
Cc: Will Deacon <[email protected]>
Cc: Yu Zhao <[email protected]>
Signed-off-by: Nico Pache <[email protected]>
---
arch/arm64/Kconfig | 1 +
arch/arm64/include/asm/pgtable.h | 77 +++++++++++++++++++++++++++++++-
2 files changed, 76 insertions(+), 2 deletions(-)

diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
index 891ab530a665..4de491627f49 100644
--- a/arch/arm64/Kconfig
+++ b/arch/arm64/Kconfig
@@ -173,6 +173,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.h b/arch/arm64/include/asm/pgtable.h
index 0bd18de9fd97..a0a15ffa2417 100644
--- a/arch/arm64/include/asm/pgtable.h
+++ b/arch/arm64/include/asm/pgtable.h
@@ -121,8 +121,9 @@ static inline pteval_t __phys_to_pte_val(phys_addr_t phys)
})

#define pte_hw_dirty(pte) (pte_write(pte) && !(pte_val(pte) & PTE_RDONLY))
-#define pte_sw_dirty(pte) (!!(pte_val(pte) & PTE_DIRTY))
-#define pte_dirty(pte) (pte_sw_dirty(pte) || pte_hw_dirty(pte))
+#define pte_soft_dirty(pte) (!!(pte_val(pte) & PTE_DIRTY))
+#define pte_dirty(pte) (pte_soft_dirty(pte) || pte_hw_dirty(pte))
+#define pte_swp_soft_dirty(pte) pte_soft_dirty(pte)

#define pte_valid(pte) (!!(pte_val(pte) & PTE_VALID))
/*
@@ -1096,6 +1097,78 @@ static inline bool pud_sect_supported(void)
return PAGE_SIZE == SZ_4K;
}

+#ifdef CONFIG_ARM64_HW_AFDBM
+/*
+ * if we have the DBM bit we can utilize the software dirty bit as
+ * a mechanism to introduce the soft_dirty functionality; however, without
+ * it this bit is crucial to determining if a entry is dirty and we cannot
+ * clear it via software. DBM can also be disabled or broken on some early
+ * armv8 devices, so check its availability before modifying it.
+ */
+static inline pte_t pte_clear_soft_dirty(pte_t pte)
+{
+ if (arch_faults_on_old_pte())
+ return pte;
+
+ return clear_pte_bit(pte, __pgprot(PTE_DIRTY));
+}
+
+static inline pte_t pte_mksoft_dirty(pte_t pte)
+{
+ if (arch_faults_on_old_pte())
+ return pte;
+
+ return set_pte_bit(pte, __pgprot(PTE_DIRTY));
+}
+
+static inline pte_t pte_swp_clear_soft_dirty(pte_t pte)
+{
+ if (arch_faults_on_old_pte())
+ return pte;
+
+ return clear_pte_bit(pte, __pgprot(PTE_DIRTY));
+}
+
+static inline pte_t pte_swp_mksoft_dirty(pte_t pte)
+{
+ if (arch_faults_on_old_pte())
+ return pte;
+
+ return set_pte_bit(pte, __pgprot(PTE_DIRTY));
+}
+
+static inline int pmd_soft_dirty(pmd_t pmd)
+{
+ return pte_soft_dirty(pmd_pte(pmd));
+}
+
+static inline pmd_t pmd_clear_soft_dirty(pmd_t pmd)
+{
+ return pte_pmd(pte_clear_soft_dirty(pmd_pte(pmd)));
+}
+
+static inline pmd_t pmd_mksoft_dirty(pmd_t pmd)
+{
+ return pte_pmd(pte_mksoft_dirty(pmd_pte(pmd)));
+}
+
+#ifdef CONFIG_ARCH_ENABLE_THP_MIGRATION
+static inline int pmd_swp_soft_dirty(pmd_t pmd)
+{
+ return pmd_soft_dirty(pmd);
+}
+
+static inline pmd_t pmd_swp_clear_soft_dirty(pmd_t pmd)
+{
+ return pmd_clear_soft_dirty(pmd);
+}
+
+static inline pmd_t pmd_swp_mksoft_dirty(pmd_t pmd)
+{
+ return pmd_mksoft_dirty(pmd);
+}
+#endif /* CONFIG_ARCH_ENABLE_THP_MIGRATION */
+#endif /* CONFIG_ARM64_HW_AFDBM */

#define __HAVE_ARCH_PTEP_MODIFY_PROT_TRANSACTION
#define ptep_modify_prot_start ptep_modify_prot_start
--
2.41.0



2023-07-04 10:25:52

by Anshuman Khandual

[permalink] [raw]
Subject: Re: [RFC] arm64: properly define SOFT_DIRTY for arm64

Hi Nico,

On 7/3/23 19:25, Nico Pache wrote:
> ARM64 has a soft-dirty bit (software dirty) but never properly defines
> CONFIG_ARCH_HAS_SOFT_DIRTY or its necessary functions. This patch
> introduces the ability to set/clear the soft dirty bit in a similar
> manner as the other arches that utilize it.
>
> However, we must be careful... there are cases where the DBM bit is not
> available and the software dirty bit plays a essential role in determining
> whether or not a page is dirty. In these cases we must not allow the
> user to clear the software dirty bit. We can test for these cases by
> utilizing the arch_faults_on_old_pte() function which test the availability
> of DBM.

The current soft-dirty bit is a SW PTE bit i.e PTE_DIRTY, tracking PTE
dirtiness in absence of HW DBM support, although both these tracking
methods are very much intertwined. Current pte helpers like pte_dirty(),
pte_mkdirty(), pte_mkclean(), and pte_wrrotect() etc operate both on
HW and SW dirty tracking bits irrespective of whether DBM is supported
or not.

For soft dirty to work, we need a software PTE bit which sticks around
on the PTE entry until user space (only) clears it and above PTE_DIRTY
bit cannot be used for that purpose as it could be cleared in the kernel.

static inline pte_t pte_mkclean(pte_t pte)
{
pte = clear_pte_bit(pte, __pgprot(PTE_DIRTY));
pte = set_pte_bit(pte, __pgprot(PTE_RDONLY));

return pte;
}

BTW arch_faults_on_old_pte() is no longer available mainline.

- Anshuman

>
> Cc: Andrew Morton <[email protected]>
> Cc: Anshuman Khandual <[email protected]>
> Cc: Catalin Marinas <[email protected]>
> Cc: David Hildenbrand <[email protected]>
> Cc: Gerald Schaefer <[email protected]>
> Cc: Liu Shixin <[email protected]>
> Cc: Will Deacon <[email protected]>
> Cc: Yu Zhao <[email protected]>
> Signed-off-by: Nico Pache <[email protected]>
> ---
> arch/arm64/Kconfig | 1 +
> arch/arm64/include/asm/pgtable.h | 77 +++++++++++++++++++++++++++++++-
> 2 files changed, 76 insertions(+), 2 deletions(-)
>
> diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
> index 891ab530a665..4de491627f49 100644
> --- a/arch/arm64/Kconfig
> +++ b/arch/arm64/Kconfig
> @@ -173,6 +173,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.h b/arch/arm64/include/asm/pgtable.h
> index 0bd18de9fd97..a0a15ffa2417 100644
> --- a/arch/arm64/include/asm/pgtable.h
> +++ b/arch/arm64/include/asm/pgtable.h
> @@ -121,8 +121,9 @@ static inline pteval_t __phys_to_pte_val(phys_addr_t phys)
> })
>
> #define pte_hw_dirty(pte) (pte_write(pte) && !(pte_val(pte) & PTE_RDONLY))
> -#define pte_sw_dirty(pte) (!!(pte_val(pte) & PTE_DIRTY))
> -#define pte_dirty(pte) (pte_sw_dirty(pte) || pte_hw_dirty(pte))
> +#define pte_soft_dirty(pte) (!!(pte_val(pte) & PTE_DIRTY))
> +#define pte_dirty(pte) (pte_soft_dirty(pte) || pte_hw_dirty(pte))
> +#define pte_swp_soft_dirty(pte) pte_soft_dirty(pte)
>
> #define pte_valid(pte) (!!(pte_val(pte) & PTE_VALID))
> /*
> @@ -1096,6 +1097,78 @@ static inline bool pud_sect_supported(void)
> return PAGE_SIZE == SZ_4K;
> }
>
> +#ifdef CONFIG_ARM64_HW_AFDBM
> +/*
> + * if we have the DBM bit we can utilize the software dirty bit as
> + * a mechanism to introduce the soft_dirty functionality; however, without
> + * it this bit is crucial to determining if a entry is dirty and we cannot
> + * clear it via software. DBM can also be disabled or broken on some early
> + * armv8 devices, so check its availability before modifying it.
> + */
> +static inline pte_t pte_clear_soft_dirty(pte_t pte)
> +{
> + if (arch_faults_on_old_pte())
> + return pte;
> +
> + return clear_pte_bit(pte, __pgprot(PTE_DIRTY));
> +}
> +
> +static inline pte_t pte_mksoft_dirty(pte_t pte)
> +{
> + if (arch_faults_on_old_pte())
> + return pte;
> +
> + return set_pte_bit(pte, __pgprot(PTE_DIRTY));
> +}
> +
> +static inline pte_t pte_swp_clear_soft_dirty(pte_t pte)
> +{
> + if (arch_faults_on_old_pte())
> + return pte;
> +
> + return clear_pte_bit(pte, __pgprot(PTE_DIRTY));
> +}
> +
> +static inline pte_t pte_swp_mksoft_dirty(pte_t pte)
> +{
> + if (arch_faults_on_old_pte())
> + return pte;
> +
> + return set_pte_bit(pte, __pgprot(PTE_DIRTY));
> +}
> +
> +static inline int pmd_soft_dirty(pmd_t pmd)
> +{
> + return pte_soft_dirty(pmd_pte(pmd));
> +}
> +
> +static inline pmd_t pmd_clear_soft_dirty(pmd_t pmd)
> +{
> + return pte_pmd(pte_clear_soft_dirty(pmd_pte(pmd)));
> +}
> +
> +static inline pmd_t pmd_mksoft_dirty(pmd_t pmd)
> +{
> + return pte_pmd(pte_mksoft_dirty(pmd_pte(pmd)));
> +}
> +
> +#ifdef CONFIG_ARCH_ENABLE_THP_MIGRATION
> +static inline int pmd_swp_soft_dirty(pmd_t pmd)
> +{
> + return pmd_soft_dirty(pmd);
> +}
> +
> +static inline pmd_t pmd_swp_clear_soft_dirty(pmd_t pmd)
> +{
> + return pmd_clear_soft_dirty(pmd);
> +}
> +
> +static inline pmd_t pmd_swp_mksoft_dirty(pmd_t pmd)
> +{
> + return pmd_mksoft_dirty(pmd);
> +}
> +#endif /* CONFIG_ARCH_ENABLE_THP_MIGRATION */
> +#endif /* CONFIG_ARM64_HW_AFDBM */
>
> #define __HAVE_ARCH_PTEP_MODIFY_PROT_TRANSACTION
> #define ptep_modify_prot_start ptep_modify_prot_start

2023-07-04 10:26:30

by Nico Pache

[permalink] [raw]
Subject: Re: [RFC] arm64: properly define SOFT_DIRTY for arm64

Whoops I pulled but never actually rebased... commit e1fd09e3d1dd
("mm: x86, arm64: add arch_has_hw_pte_young()") changed
arch_faults_on_old_pte() into !arch_has_hw_pte_young(). Following up
with a V2 shortly.

-- Nico

On Mon, Jul 3, 2023 at 10:06 AM Nico Pache <[email protected]> wrote:
>
> ARM64 has a soft-dirty bit (software dirty) but never properly defines
> CONFIG_ARCH_HAS_SOFT_DIRTY or its necessary functions. This patch
> introduces the ability to set/clear the soft dirty bit in a similar
> manner as the other arches that utilize it.
>
> However, we must be careful... there are cases where the DBM bit is not
> available and the software dirty bit plays a essential role in determining
> whether or not a page is dirty. In these cases we must not allow the
> user to clear the software dirty bit. We can test for these cases by
> utilizing the arch_faults_on_old_pte() function which test the availability
> of DBM.
>
> Cc: Andrew Morton <[email protected]>
> Cc: Anshuman Khandual <[email protected]>
> Cc: Catalin Marinas <[email protected]>
> Cc: David Hildenbrand <[email protected]>
> Cc: Gerald Schaefer <[email protected]>
> Cc: Liu Shixin <[email protected]>
> Cc: Will Deacon <[email protected]>
> Cc: Yu Zhao <[email protected]>
> Signed-off-by: Nico Pache <[email protected]>
> ---
> arch/arm64/Kconfig | 1 +
> arch/arm64/include/asm/pgtable.h | 77 +++++++++++++++++++++++++++++++-
> 2 files changed, 76 insertions(+), 2 deletions(-)
>
> diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
> index 891ab530a665..4de491627f49 100644
> --- a/arch/arm64/Kconfig
> +++ b/arch/arm64/Kconfig
> @@ -173,6 +173,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.h b/arch/arm64/include/asm/pgtable.h
> index 0bd18de9fd97..a0a15ffa2417 100644
> --- a/arch/arm64/include/asm/pgtable.h
> +++ b/arch/arm64/include/asm/pgtable.h
> @@ -121,8 +121,9 @@ static inline pteval_t __phys_to_pte_val(phys_addr_t phys)
> })
>
> #define pte_hw_dirty(pte) (pte_write(pte) && !(pte_val(pte) & PTE_RDONLY))
> -#define pte_sw_dirty(pte) (!!(pte_val(pte) & PTE_DIRTY))
> -#define pte_dirty(pte) (pte_sw_dirty(pte) || pte_hw_dirty(pte))
> +#define pte_soft_dirty(pte) (!!(pte_val(pte) & PTE_DIRTY))
> +#define pte_dirty(pte) (pte_soft_dirty(pte) || pte_hw_dirty(pte))
> +#define pte_swp_soft_dirty(pte) pte_soft_dirty(pte)
>
> #define pte_valid(pte) (!!(pte_val(pte) & PTE_VALID))
> /*
> @@ -1096,6 +1097,78 @@ static inline bool pud_sect_supported(void)
> return PAGE_SIZE == SZ_4K;
> }
>
> +#ifdef CONFIG_ARM64_HW_AFDBM
> +/*
> + * if we have the DBM bit we can utilize the software dirty bit as
> + * a mechanism to introduce the soft_dirty functionality; however, without
> + * it this bit is crucial to determining if a entry is dirty and we cannot
> + * clear it via software. DBM can also be disabled or broken on some early
> + * armv8 devices, so check its availability before modifying it.
> + */
> +static inline pte_t pte_clear_soft_dirty(pte_t pte)
> +{
> + if (arch_faults_on_old_pte())
> + return pte;
> +
> + return clear_pte_bit(pte, __pgprot(PTE_DIRTY));
> +}
> +
> +static inline pte_t pte_mksoft_dirty(pte_t pte)
> +{
> + if (arch_faults_on_old_pte())
> + return pte;
> +
> + return set_pte_bit(pte, __pgprot(PTE_DIRTY));
> +}
> +
> +static inline pte_t pte_swp_clear_soft_dirty(pte_t pte)
> +{
> + if (arch_faults_on_old_pte())
> + return pte;
> +
> + return clear_pte_bit(pte, __pgprot(PTE_DIRTY));
> +}
> +
> +static inline pte_t pte_swp_mksoft_dirty(pte_t pte)
> +{
> + if (arch_faults_on_old_pte())
> + return pte;
> +
> + return set_pte_bit(pte, __pgprot(PTE_DIRTY));
> +}
> +
> +static inline int pmd_soft_dirty(pmd_t pmd)
> +{
> + return pte_soft_dirty(pmd_pte(pmd));
> +}
> +
> +static inline pmd_t pmd_clear_soft_dirty(pmd_t pmd)
> +{
> + return pte_pmd(pte_clear_soft_dirty(pmd_pte(pmd)));
> +}
> +
> +static inline pmd_t pmd_mksoft_dirty(pmd_t pmd)
> +{
> + return pte_pmd(pte_mksoft_dirty(pmd_pte(pmd)));
> +}
> +
> +#ifdef CONFIG_ARCH_ENABLE_THP_MIGRATION
> +static inline int pmd_swp_soft_dirty(pmd_t pmd)
> +{
> + return pmd_soft_dirty(pmd);
> +}
> +
> +static inline pmd_t pmd_swp_clear_soft_dirty(pmd_t pmd)
> +{
> + return pmd_clear_soft_dirty(pmd);
> +}
> +
> +static inline pmd_t pmd_swp_mksoft_dirty(pmd_t pmd)
> +{
> + return pmd_mksoft_dirty(pmd);
> +}
> +#endif /* CONFIG_ARCH_ENABLE_THP_MIGRATION */
> +#endif /* CONFIG_ARM64_HW_AFDBM */
>
> #define __HAVE_ARCH_PTEP_MODIFY_PROT_TRANSACTION
> #define ptep_modify_prot_start ptep_modify_prot_start
> --
> 2.41.0
>


2023-07-04 10:33:53

by Nico Pache

[permalink] [raw]
Subject: Re: [RFC] arm64: properly define SOFT_DIRTY for arm64

Hi Anshuman,

Thanks for the explanation!

Is it possible to add the same DBM check I'm using
(!arch_has_hw_pte_young) in these pte helper functions to only clear
it when DBM is not present?

Cheers,
-- Nico

On Tue, Jul 4, 2023 at 6:01 AM Anshuman Khandual
<[email protected]> wrote:
>
> Hi Nico,
>
> On 7/3/23 19:25, Nico Pache wrote:
> > ARM64 has a soft-dirty bit (software dirty) but never properly defines
> > CONFIG_ARCH_HAS_SOFT_DIRTY or its necessary functions. This patch
> > introduces the ability to set/clear the soft dirty bit in a similar
> > manner as the other arches that utilize it.
> >
> > However, we must be careful... there are cases where the DBM bit is not
> > available and the software dirty bit plays a essential role in determining
> > whether or not a page is dirty. In these cases we must not allow the
> > user to clear the software dirty bit. We can test for these cases by
> > utilizing the arch_faults_on_old_pte() function which test the availability
> > of DBM.
>
> The current soft-dirty bit is a SW PTE bit i.e PTE_DIRTY, tracking PTE
> dirtiness in absence of HW DBM support, although both these tracking
> methods are very much intertwined. Current pte helpers like pte_dirty(),
> pte_mkdirty(), pte_mkclean(), and pte_wrrotect() etc operate both on
> HW and SW dirty tracking bits irrespective of whether DBM is supported
> or not.
>
> For soft dirty to work, we need a software PTE bit which sticks around
> on the PTE entry until user space (only) clears it and above PTE_DIRTY
> bit cannot be used for that purpose as it could be cleared in the kernel.
>
> static inline pte_t pte_mkclean(pte_t pte)
> {
> pte = clear_pte_bit(pte, __pgprot(PTE_DIRTY));
> pte = set_pte_bit(pte, __pgprot(PTE_RDONLY));
>
> return pte;
> }
>
> BTW arch_faults_on_old_pte() is no longer available mainline.
>
> - Anshuman
>
> >
> > Cc: Andrew Morton <[email protected]>
> > Cc: Anshuman Khandual <[email protected]>
> > Cc: Catalin Marinas <[email protected]>
> > Cc: David Hildenbrand <[email protected]>
> > Cc: Gerald Schaefer <[email protected]>
> > Cc: Liu Shixin <[email protected]>
> > Cc: Will Deacon <[email protected]>
> > Cc: Yu Zhao <[email protected]>
> > Signed-off-by: Nico Pache <[email protected]>
> > ---
> > arch/arm64/Kconfig | 1 +
> > arch/arm64/include/asm/pgtable.h | 77 +++++++++++++++++++++++++++++++-
> > 2 files changed, 76 insertions(+), 2 deletions(-)
> >
> > diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
> > index 891ab530a665..4de491627f49 100644
> > --- a/arch/arm64/Kconfig
> > +++ b/arch/arm64/Kconfig
> > @@ -173,6 +173,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.h b/arch/arm64/include/asm/pgtable.h
> > index 0bd18de9fd97..a0a15ffa2417 100644
> > --- a/arch/arm64/include/asm/pgtable.h
> > +++ b/arch/arm64/include/asm/pgtable.h
> > @@ -121,8 +121,9 @@ static inline pteval_t __phys_to_pte_val(phys_addr_t phys)
> > })
> >
> > #define pte_hw_dirty(pte) (pte_write(pte) && !(pte_val(pte) & PTE_RDONLY))
> > -#define pte_sw_dirty(pte) (!!(pte_val(pte) & PTE_DIRTY))
> > -#define pte_dirty(pte) (pte_sw_dirty(pte) || pte_hw_dirty(pte))
> > +#define pte_soft_dirty(pte) (!!(pte_val(pte) & PTE_DIRTY))
> > +#define pte_dirty(pte) (pte_soft_dirty(pte) || pte_hw_dirty(pte))
> > +#define pte_swp_soft_dirty(pte) pte_soft_dirty(pte)
> >
> > #define pte_valid(pte) (!!(pte_val(pte) & PTE_VALID))
> > /*
> > @@ -1096,6 +1097,78 @@ static inline bool pud_sect_supported(void)
> > return PAGE_SIZE == SZ_4K;
> > }
> >
> > +#ifdef CONFIG_ARM64_HW_AFDBM
> > +/*
> > + * if we have the DBM bit we can utilize the software dirty bit as
> > + * a mechanism to introduce the soft_dirty functionality; however, without
> > + * it this bit is crucial to determining if a entry is dirty and we cannot
> > + * clear it via software. DBM can also be disabled or broken on some early
> > + * armv8 devices, so check its availability before modifying it.
> > + */
> > +static inline pte_t pte_clear_soft_dirty(pte_t pte)
> > +{
> > + if (arch_faults_on_old_pte())
> > + return pte;
> > +
> > + return clear_pte_bit(pte, __pgprot(PTE_DIRTY));
> > +}
> > +
> > +static inline pte_t pte_mksoft_dirty(pte_t pte)
> > +{
> > + if (arch_faults_on_old_pte())
> > + return pte;
> > +
> > + return set_pte_bit(pte, __pgprot(PTE_DIRTY));
> > +}
> > +
> > +static inline pte_t pte_swp_clear_soft_dirty(pte_t pte)
> > +{
> > + if (arch_faults_on_old_pte())
> > + return pte;
> > +
> > + return clear_pte_bit(pte, __pgprot(PTE_DIRTY));
> > +}
> > +
> > +static inline pte_t pte_swp_mksoft_dirty(pte_t pte)
> > +{
> > + if (arch_faults_on_old_pte())
> > + return pte;
> > +
> > + return set_pte_bit(pte, __pgprot(PTE_DIRTY));
> > +}
> > +
> > +static inline int pmd_soft_dirty(pmd_t pmd)
> > +{
> > + return pte_soft_dirty(pmd_pte(pmd));
> > +}
> > +
> > +static inline pmd_t pmd_clear_soft_dirty(pmd_t pmd)
> > +{
> > + return pte_pmd(pte_clear_soft_dirty(pmd_pte(pmd)));
> > +}
> > +
> > +static inline pmd_t pmd_mksoft_dirty(pmd_t pmd)
> > +{
> > + return pte_pmd(pte_mksoft_dirty(pmd_pte(pmd)));
> > +}
> > +
> > +#ifdef CONFIG_ARCH_ENABLE_THP_MIGRATION
> > +static inline int pmd_swp_soft_dirty(pmd_t pmd)
> > +{
> > + return pmd_soft_dirty(pmd);
> > +}
> > +
> > +static inline pmd_t pmd_swp_clear_soft_dirty(pmd_t pmd)
> > +{
> > + return pmd_clear_soft_dirty(pmd);
> > +}
> > +
> > +static inline pmd_t pmd_swp_mksoft_dirty(pmd_t pmd)
> > +{
> > + return pmd_mksoft_dirty(pmd);
> > +}
> > +#endif /* CONFIG_ARCH_ENABLE_THP_MIGRATION */
> > +#endif /* CONFIG_ARM64_HW_AFDBM */
> >
> > #define __HAVE_ARCH_PTEP_MODIFY_PROT_TRANSACTION
> > #define ptep_modify_prot_start ptep_modify_prot_start
>


2023-07-16 15:16:58

by Catalin Marinas

[permalink] [raw]
Subject: Re: [RFC] arm64: properly define SOFT_DIRTY for arm64

(I noticed Mark already replied in another thread along the same lines)

On Tue, Jul 04, 2023 at 06:08:59AM -0400, Nico Pache wrote:
> Is it possible to add the same DBM check I'm using
> (!arch_has_hw_pte_young) in these pte helper functions to only clear
> it when DBM is not present?

It's not possible since we don't have a way to encode a read-only +
dirty PTE (e.g. after ptep_set_wrprotect()). The PTE_WRITE/PTE_DBM bit
in the architecture only tells that the hardware is allowed to clear the
PTE_RDONLY bit on a write access and that's what we consider hw-dirty.
When a dirty/writeable PTE is made read-only, we clear PTE_WRITE, set
PTE_RDONLY _and_ the software PTE_DIRTY bit.

With the permission indirection extensions (PIE, see patches from Joey),
PTE_RDONLY can be treated as a true !PTE_DIRTY bit but there's no
hardware around yet.

So if you need software dirty, it can only be done with another software
PTE bit. The problem is that we are short of such bits (only one left if
we move PTE_PROT_NONE to a different location). The userfaultfd people
also want such bit.

Personally I'd reuse the four PBHA bits but I keep hearing that they may
be used with some out of tree patches.

--
Catalin

2023-07-25 21:50:52

by Nico Pache

[permalink] [raw]
Subject: Re: [RFC] arm64: properly define SOFT_DIRTY for arm64

Hi Catalin,

Thanks for your reply.

From my understanding only two of the PBHA bits [60:59] are being used
for *_TABLE_<UXN|PXN>.

If that's the case is it safe to assume bits [63:61] are usable
(without considering OOT patches) and are treated similarly to the
software bits [58:55]? or do more considerations need to be made with
regards to using these bits?

There isnt much info in the codebase about PBHA, but from some
external research it seems the PBHA bits are intended to be used by
future SoC hardware but are currently not exposed to allow users to
use them for their intended purpose.

If you think this is a viable solution, I will go ahead and use bit 61
to implement a SOFTWARE_DIRTY bit. But if the goal is to inevitably
expose these bits to the hardware and allow them to use it, then
perhaps introducing this feature would be short lived.

Thanks,
-- Nico

On Sun, Jul 16, 2023 at 9:19 AM Catalin Marinas <[email protected]> wrote:
>
> (I noticed Mark already replied in another thread along the same lines)
>
> On Tue, Jul 04, 2023 at 06:08:59AM -0400, Nico Pache wrote:
> > Is it possible to add the same DBM check I'm using
> > (!arch_has_hw_pte_young) in these pte helper functions to only clear
> > it when DBM is not present?
>
> It's not possible since we don't have a way to encode a read-only +
> dirty PTE (e.g. after ptep_set_wrprotect()). The PTE_WRITE/PTE_DBM bit
> in the architecture only tells that the hardware is allowed to clear the
> PTE_RDONLY bit on a write access and that's what we consider hw-dirty.
> When a dirty/writeable PTE is made read-only, we clear PTE_WRITE, set
> PTE_RDONLY _and_ the software PTE_DIRTY bit.
>
> With the permission indirection extensions (PIE, see patches from Joey),
> PTE_RDONLY can be treated as a true !PTE_DIRTY bit but there's no
> hardware around yet.
>
> So if you need software dirty, it can only be done with another software
> PTE bit. The problem is that we are short of such bits (only one left if
> we move PTE_PROT_NONE to a different location). The userfaultfd people
> also want such bit.
>
> Personally I'd reuse the four PBHA bits but I keep hearing that they may
> be used with some out of tree patches.
>
> --
> Catalin
>