[v4] riscv: mm: Add soft-dirty and uffd-wp support

[RESEND PATCH V4 1/3] riscv: mm: Prepare for reusing PTE RSW bit(9)

Posted by Chunyan Zhang 1 year, 5 months ago

The PTE bit(9) on RISC-V is reserved for software, it is used by DEVMAP
now which has to be disabled if we want to use bit(9) for other features,
since there's no more free PTE bit on RISC-V now.

So to make ARCH_HAS_PTE_DEVMAP selectable, this patch uses it as
the build condition of devmap definitions.

Signed-off-by: Chunyan Zhang <zhangchunyan@iscas.ac.cn>
---
 arch/riscv/include/asm/pgtable-64.h   | 2 +-
 arch/riscv/include/asm/pgtable-bits.h | 6 ++++++
 2 files changed, 7 insertions(+), 1 deletion(-)

diff --git a/arch/riscv/include/asm/pgtable-64.h b/arch/riscv/include/asm/pgtable-64.h
index 0897dd99ab8d..babb8d2b0f0b 100644
--- a/arch/riscv/include/asm/pgtable-64.h
+++ b/arch/riscv/include/asm/pgtable-64.h
@@ -398,7 +398,7 @@ static inline struct page *pgd_page(pgd_t pgd)
 #define p4d_offset p4d_offset
 p4d_t *p4d_offset(pgd_t *pgd, unsigned long address);
 
-#ifdef CONFIG_TRANSPARENT_HUGEPAGE
+#if defined(CONFIG_TRANSPARENT_HUGEPAGE) && defined(CONFIG_ARCH_HAS_PTE_DEVMAP)
 static inline int pte_devmap(pte_t pte);
 static inline pte_t pmd_pte(pmd_t pmd);
 
diff --git a/arch/riscv/include/asm/pgtable-bits.h b/arch/riscv/include/asm/pgtable-bits.h
index a8f5205cea54..5bcc73430829 100644
--- a/arch/riscv/include/asm/pgtable-bits.h
+++ b/arch/riscv/include/asm/pgtable-bits.h
@@ -19,7 +19,13 @@
 #define _PAGE_SOFT      (3 << 8)    /* Reserved for software */
 
 #define _PAGE_SPECIAL   (1 << 8)    /* RSW: 0x1 */
+
+#ifdef CONFIG_ARCH_HAS_PTE_DEVMAP
 #define _PAGE_DEVMAP    (1 << 9)    /* RSW, devmap */
+#else
+#define _PAGE_DEVMAP	0
+#endif /* CONFIG_ARCH_HAS_PTE_DEVMAP */
+
 #define _PAGE_TABLE     _PAGE_PRESENT
 
 /*
-- 
2.34.1

Re: [RESEND PATCH V4 1/3] riscv: mm: Prepare for reusing PTE RSW bit(9)

Posted by Alexandre Ghiti 1 year, 3 months ago

Hi Chunyan,

On 30/08/2024 03:10, Chunyan Zhang wrote:
> The PTE bit(9) on RISC-V is reserved for software, it is used by DEVMAP
> now which has to be disabled if we want to use bit(9) for other features,
> since there's no more free PTE bit on RISC-V now.
>
> So to make ARCH_HAS_PTE_DEVMAP selectable, this patch uses it as
> the build condition of devmap definitions.
>
> Signed-off-by: Chunyan Zhang <zhangchunyan@iscas.ac.cn>
> ---
>   arch/riscv/include/asm/pgtable-64.h   | 2 +-
>   arch/riscv/include/asm/pgtable-bits.h | 6 ++++++
>   2 files changed, 7 insertions(+), 1 deletion(-)
>
> diff --git a/arch/riscv/include/asm/pgtable-64.h b/arch/riscv/include/asm/pgtable-64.h
> index 0897dd99ab8d..babb8d2b0f0b 100644
> --- a/arch/riscv/include/asm/pgtable-64.h
> +++ b/arch/riscv/include/asm/pgtable-64.h
> @@ -398,7 +398,7 @@ static inline struct page *pgd_page(pgd_t pgd)
>   #define p4d_offset p4d_offset
>   p4d_t *p4d_offset(pgd_t *pgd, unsigned long address);
>   
> -#ifdef CONFIG_TRANSPARENT_HUGEPAGE
> +#if defined(CONFIG_TRANSPARENT_HUGEPAGE) && defined(CONFIG_ARCH_HAS_PTE_DEVMAP)
>   static inline int pte_devmap(pte_t pte);
>   static inline pte_t pmd_pte(pmd_t pmd);
>   
> diff --git a/arch/riscv/include/asm/pgtable-bits.h b/arch/riscv/include/asm/pgtable-bits.h
> index a8f5205cea54..5bcc73430829 100644
> --- a/arch/riscv/include/asm/pgtable-bits.h
> +++ b/arch/riscv/include/asm/pgtable-bits.h
> @@ -19,7 +19,13 @@
>   #define _PAGE_SOFT      (3 << 8)    /* Reserved for software */
>   
>   #define _PAGE_SPECIAL   (1 << 8)    /* RSW: 0x1 */
> +
> +#ifdef CONFIG_ARCH_HAS_PTE_DEVMAP
>   #define _PAGE_DEVMAP    (1 << 9)    /* RSW, devmap */
> +#else
> +#define _PAGE_DEVMAP	0
> +#endif /* CONFIG_ARCH_HAS_PTE_DEVMAP */
> +
>   #define _PAGE_TABLE     _PAGE_PRESENT
>   
>   /*


There is a small inconsistency in our code: pte_devmap() is protected by 
a #ifdef but pte_mkdevmap() is not, I guess that's why you had to define 
_PAGE_DEVMAP if !CONFIG_ARCH_HAS_PTE_DEVMAP.

I'd be in favor of removing the #ifdef around pte_devmap() too.

Thanks,

Alex

Re: [RESEND PATCH V4 1/3] riscv: mm: Prepare for reusing PTE RSW bit(9)

Posted by Chunyan Zhang 1 year, 3 months ago

Hi Alex,

On Tue, 5 Nov 2024 at 20:45, Alexandre Ghiti <alex@ghiti.fr> wrote:
>
> Hi Chunyan,
>
> On 30/08/2024 03:10, Chunyan Zhang wrote:
> > The PTE bit(9) on RISC-V is reserved for software, it is used by DEVMAP
> > now which has to be disabled if we want to use bit(9) for other features,
> > since there's no more free PTE bit on RISC-V now.
> >
> > So to make ARCH_HAS_PTE_DEVMAP selectable, this patch uses it as
> > the build condition of devmap definitions.
> >
> > Signed-off-by: Chunyan Zhang <zhangchunyan@iscas.ac.cn>
> > ---
> >   arch/riscv/include/asm/pgtable-64.h   | 2 +-
> >   arch/riscv/include/asm/pgtable-bits.h | 6 ++++++
> >   2 files changed, 7 insertions(+), 1 deletion(-)
> >
> > diff --git a/arch/riscv/include/asm/pgtable-64.h b/arch/riscv/include/asm/pgtable-64.h
> > index 0897dd99ab8d..babb8d2b0f0b 100644
> > --- a/arch/riscv/include/asm/pgtable-64.h
> > +++ b/arch/riscv/include/asm/pgtable-64.h
> > @@ -398,7 +398,7 @@ static inline struct page *pgd_page(pgd_t pgd)
> >   #define p4d_offset p4d_offset
> >   p4d_t *p4d_offset(pgd_t *pgd, unsigned long address);
> >
> > -#ifdef CONFIG_TRANSPARENT_HUGEPAGE
> > +#if defined(CONFIG_TRANSPARENT_HUGEPAGE) && defined(CONFIG_ARCH_HAS_PTE_DEVMAP)
> >   static inline int pte_devmap(pte_t pte);
> >   static inline pte_t pmd_pte(pmd_t pmd);
> >
> > diff --git a/arch/riscv/include/asm/pgtable-bits.h b/arch/riscv/include/asm/pgtable-bits.h
> > index a8f5205cea54..5bcc73430829 100644
> > --- a/arch/riscv/include/asm/pgtable-bits.h
> > +++ b/arch/riscv/include/asm/pgtable-bits.h
> > @@ -19,7 +19,13 @@
> >   #define _PAGE_SOFT      (3 << 8)    /* Reserved for software */
> >
> >   #define _PAGE_SPECIAL   (1 << 8)    /* RSW: 0x1 */
> > +
> > +#ifdef CONFIG_ARCH_HAS_PTE_DEVMAP
> >   #define _PAGE_DEVMAP    (1 << 9)    /* RSW, devmap */
> > +#else
> > +#define _PAGE_DEVMAP 0
> > +#endif /* CONFIG_ARCH_HAS_PTE_DEVMAP */
> > +
> >   #define _PAGE_TABLE     _PAGE_PRESENT
> >
> >   /*
>
>
> There is a small inconsistency in our code: pte_devmap() is protected by
> a #ifdef but pte_mkdevmap() is not, I guess that's why you had to define

pte_devmap() is defined in include/linux/mm.h as well, we have to use
#ifdef to avoid compile error since we will change the
CONFIG_ARCH_HAS_PTE_DEVMAP to be selectable on riscv.

> _PAGE_DEVMAP if !CONFIG_ARCH_HAS_PTE_DEVMAP.
>
> I'd be in favor of removing the #ifdef around pte_devmap() too.

So we cannot remove the #ifdef here, the compiler would report
redefinition errors if CONFIG_ARCH_HAS_PTE_DEVMAP is not selected.

Thanks for the review,
Chunyan