[PATCH 1/3] arm64: mmu: update paging_init comments

[PATCH 1/3] arm64: mmu: update paging_init comments

[Peng Fan]

The comments has not been changed since the function introduced, but the
function has been changed dramatically, so update the comments to
reflect the code.

Signed-off-by: Peng Fan <peng.fan@nxp.com>
---
 arch/arm64/mm/mmu.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/arm64/mm/mmu.c b/arch/arm64/mm/mmu.c
index d6b6f1b169bb..065202da3ad2 100644
--- a/arch/arm64/mm/mmu.c
+++ b/arch/arm64/mm/mmu.c
@@ -655,8 +655,8 @@ static void __init map_kernel(pgd_t *pgdp)
 }
 
 /*
- * paging_init() sets up the page tables, initialises the zone memory
- * maps and sets up the zero page.
+ * paging_init() sets up the page tables, switch ttbr1 from init_pg_dir
+ * to swapper_pg_dir, free init_pg_dir memblock and permit memblock resizing.
  */
 void __init paging_init(void)
 {
-- 
2.16.4

[PATCH 1/3] arm64: mmu: update paging_init comments

[Peng Fan]

The comments has not been changed since the function introduced, but the
function has been changed dramatically, so update the comments to
reflect the code.

Signed-off-by: Peng Fan <peng.fan@nxp.com>
---
 arch/arm64/mm/mmu.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/arm64/mm/mmu.c b/arch/arm64/mm/mmu.c
index d6b6f1b169bb..065202da3ad2 100644
--- a/arch/arm64/mm/mmu.c
+++ b/arch/arm64/mm/mmu.c
@@ -655,8 +655,8 @@ static void __init map_kernel(pgd_t *pgdp)
 }
 
 /*
- * paging_init() sets up the page tables, initialises the zone memory
- * maps and sets up the zero page.
+ * paging_init() sets up the page tables, switch ttbr1 from init_pg_dir
+ * to swapper_pg_dir, free init_pg_dir memblock and permit memblock resizing.
  */
 void __init paging_init(void)
 {
-- 
2.16.4


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 2/3] arm64: mmu: early_pgtable_alloc: remove unnecessary comments

[Peng Fan]

The empty zero page has been moved to bss area by
commit 5227cfa71f9e ("arm64: mm: place empty_zero_page in bss"),
and it alreay added "dsb     ishst" in head.S to make sure the empty
zero page visible to PTW.

There is no code to reflect the comment, so remove it.

Signed-off-by: Peng Fan <peng.fan@nxp.com>
---
 arch/arm64/mm/mmu.c | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/arch/arm64/mm/mmu.c b/arch/arm64/mm/mmu.c
index 065202da3ad2..f61fa7c8fd2e 100644
--- a/arch/arm64/mm/mmu.c
+++ b/arch/arm64/mm/mmu.c
@@ -113,10 +113,6 @@ static phys_addr_t __init early_pgtable_alloc(void)
 
 	memset(ptr, 0, PAGE_SIZE);
 
-	/*
-	 * Implicit barriers also ensure the zeroed page is visible to the page
-	 * table walker
-	 */
 	pte_clear_fixmap();
 
 	return phys;
-- 
2.16.4

[PATCH 2/3] arm64: mmu: early_pgtable_alloc: remove unnecessary comments

[Peng Fan]

The empty zero page has been moved to bss area by
commit 5227cfa71f9e ("arm64: mm: place empty_zero_page in bss"),
and it alreay added "dsb     ishst" in head.S to make sure the empty
zero page visible to PTW.

There is no code to reflect the comment, so remove it.

Signed-off-by: Peng Fan <peng.fan@nxp.com>
---
 arch/arm64/mm/mmu.c | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/arch/arm64/mm/mmu.c b/arch/arm64/mm/mmu.c
index 065202da3ad2..f61fa7c8fd2e 100644
--- a/arch/arm64/mm/mmu.c
+++ b/arch/arm64/mm/mmu.c
@@ -113,10 +113,6 @@ static phys_addr_t __init early_pgtable_alloc(void)
 
 	memset(ptr, 0, PAGE_SIZE);
 
-	/*
-	 * Implicit barriers also ensure the zeroed page is visible to the page
-	 * table walker
-	 */
 	pte_clear_fixmap();
 
 	return phys;
-- 
2.16.4


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 3/3] arm64: mmu: pgd_pgtable_alloc: drop barrier

[Peng Fan]

The barriers are added for empty_zero_page, however the
empty zero page has been moved to bss area by
commit 5227cfa71f9e ("arm64: mm: place empty_zero_page in bss"),
and it alreay added "dsb     ishst" in head.S to make sure
the empty zero page visible to PTW.

pgd_pgtable_alloc is only called by __create_pgd_mapping, and
there are implicit barriers in __create_pgd_mapping, so we
could remove the barrier pgd_pgtable_alloc

Signed-off-by: Peng Fan <peng.fan@nxp.com>
---

Note: this patch tested on qemu-system-aarch64, not real hardware.

 arch/arm64/mm/mmu.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/arch/arm64/mm/mmu.c b/arch/arm64/mm/mmu.c
index f61fa7c8fd2e..04e3d4d070ce 100644
--- a/arch/arm64/mm/mmu.c
+++ b/arch/arm64/mm/mmu.c
@@ -371,8 +371,6 @@ static phys_addr_t pgd_pgtable_alloc(void)
 	if (!ptr || !pgtable_page_ctor(virt_to_page(ptr)))
 		BUG();
 
-	/* Ensure the zeroed page is visible to the page table walker */
-	dsb(ishst);
 	return __pa(ptr);
 }
 
-- 
2.16.4

[PATCH 3/3] arm64: mmu: pgd_pgtable_alloc: drop barrier

[Peng Fan]

The barriers are added for empty_zero_page, however the
empty zero page has been moved to bss area by
commit 5227cfa71f9e ("arm64: mm: place empty_zero_page in bss"),
and it alreay added "dsb     ishst" in head.S to make sure
the empty zero page visible to PTW.

pgd_pgtable_alloc is only called by __create_pgd_mapping, and
there are implicit barriers in __create_pgd_mapping, so we
could remove the barrier pgd_pgtable_alloc

Signed-off-by: Peng Fan <peng.fan@nxp.com>
---

Note: this patch tested on qemu-system-aarch64, not real hardware.

 arch/arm64/mm/mmu.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/arch/arm64/mm/mmu.c b/arch/arm64/mm/mmu.c
index f61fa7c8fd2e..04e3d4d070ce 100644
--- a/arch/arm64/mm/mmu.c
+++ b/arch/arm64/mm/mmu.c
@@ -371,8 +371,6 @@ static phys_addr_t pgd_pgtable_alloc(void)
 	if (!ptr || !pgtable_page_ctor(virt_to_page(ptr)))
 		BUG();
 
-	/* Ensure the zeroed page is visible to the page table walker */
-	dsb(ishst);
 	return __pa(ptr);
 }
 
-- 
2.16.4


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 1/3] arm64: mmu: update paging_init comments

[Mark Rutland]

On Wed, Feb 13, 2019 at 09:37:24AM +0000, Peng Fan wrote:
> The comments has not been changed since the function introduced, but the
> function has been changed dramatically, so update the comments to
> reflect the code.
> 
> Signed-off-by: Peng Fan <peng.fan@nxp.com>
> ---
>  arch/arm64/mm/mmu.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/arm64/mm/mmu.c b/arch/arm64/mm/mmu.c
> index d6b6f1b169bb..065202da3ad2 100644
> --- a/arch/arm64/mm/mmu.c
> +++ b/arch/arm64/mm/mmu.c
> @@ -655,8 +655,8 @@ static void __init map_kernel(pgd_t *pgdp)
>  }
>  
>  /*
> - * paging_init() sets up the page tables, initialises the zone memory
> - * maps and sets up the zero page.
> + * paging_init() sets up the page tables, switch ttbr1 from init_pg_dir
> + * to swapper_pg_dir, free init_pg_dir memblock and permit memblock resizing.
>   */

I think that it would be better to remove the comment entirely. This
doesn't explain the intent, or rationale, and all this information is
obvious from a straight-line reading of the code.

If you remove the comment instead:

Acked-by: Mark Rutland <mark.rutland@arm.com>

Mark.

>  void __init paging_init(void)
>  {
> -- 
> 2.16.4
> 

Re: [PATCH 1/3] arm64: mmu: update paging_init comments

[Mark Rutland]

On Wed, Feb 13, 2019 at 09:37:24AM +0000, Peng Fan wrote:
> The comments has not been changed since the function introduced, but the
> function has been changed dramatically, so update the comments to
> reflect the code.
> 
> Signed-off-by: Peng Fan <peng.fan@nxp.com>
> ---
>  arch/arm64/mm/mmu.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/arm64/mm/mmu.c b/arch/arm64/mm/mmu.c
> index d6b6f1b169bb..065202da3ad2 100644
> --- a/arch/arm64/mm/mmu.c
> +++ b/arch/arm64/mm/mmu.c
> @@ -655,8 +655,8 @@ static void __init map_kernel(pgd_t *pgdp)
>  }
>  
>  /*
> - * paging_init() sets up the page tables, initialises the zone memory
> - * maps and sets up the zero page.
> + * paging_init() sets up the page tables, switch ttbr1 from init_pg_dir
> + * to swapper_pg_dir, free init_pg_dir memblock and permit memblock resizing.
>   */

I think that it would be better to remove the comment entirely. This
doesn't explain the intent, or rationale, and all this information is
obvious from a straight-line reading of the code.

If you remove the comment instead:

Acked-by: Mark Rutland <mark.rutland@arm.com>

Mark.

>  void __init paging_init(void)
>  {
> -- 
> 2.16.4
> 

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 2/3] arm64: mmu: early_pgtable_alloc: remove unnecessary comments

[Mark Rutland]

On Wed, Feb 13, 2019 at 09:37:29AM +0000, Peng Fan wrote:
> The empty zero page has been moved to bss area by
> commit 5227cfa71f9e ("arm64: mm: place empty_zero_page in bss"),
> and it alreay added "dsb     ishst" in head.S to make sure the empty
> zero page visible to PTW.
> 
> There is no code to reflect the comment, so remove it.

The comment below refers to the memory pointed to by ptr, which is
zeroed by the memset. The comment does not refer to the generic
empty_zero_page.

This comment is correct, and should stay. Please drop this patch.

Thanks,
Mark.

> 
> Signed-off-by: Peng Fan <peng.fan@nxp.com>
> ---
>  arch/arm64/mm/mmu.c | 4 ----
>  1 file changed, 4 deletions(-)
> 
> diff --git a/arch/arm64/mm/mmu.c b/arch/arm64/mm/mmu.c
> index 065202da3ad2..f61fa7c8fd2e 100644
> --- a/arch/arm64/mm/mmu.c
> +++ b/arch/arm64/mm/mmu.c
> @@ -113,10 +113,6 @@ static phys_addr_t __init early_pgtable_alloc(void)
>  
>  	memset(ptr, 0, PAGE_SIZE);
>  
> -	/*
> -	 * Implicit barriers also ensure the zeroed page is visible to the page
> -	 * table walker
> -	 */
>  	pte_clear_fixmap();
>  
>  	return phys;
> -- 
> 2.16.4
> 

Re: [PATCH 2/3] arm64: mmu: early_pgtable_alloc: remove unnecessary comments

[Mark Rutland]

On Wed, Feb 13, 2019 at 09:37:29AM +0000, Peng Fan wrote:
> The empty zero page has been moved to bss area by
> commit 5227cfa71f9e ("arm64: mm: place empty_zero_page in bss"),
> and it alreay added "dsb     ishst" in head.S to make sure the empty
> zero page visible to PTW.
> 
> There is no code to reflect the comment, so remove it.

The comment below refers to the memory pointed to by ptr, which is
zeroed by the memset. The comment does not refer to the generic
empty_zero_page.

This comment is correct, and should stay. Please drop this patch.

Thanks,
Mark.

> 
> Signed-off-by: Peng Fan <peng.fan@nxp.com>
> ---
>  arch/arm64/mm/mmu.c | 4 ----
>  1 file changed, 4 deletions(-)
> 
> diff --git a/arch/arm64/mm/mmu.c b/arch/arm64/mm/mmu.c
> index 065202da3ad2..f61fa7c8fd2e 100644
> --- a/arch/arm64/mm/mmu.c
> +++ b/arch/arm64/mm/mmu.c
> @@ -113,10 +113,6 @@ static phys_addr_t __init early_pgtable_alloc(void)
>  
>  	memset(ptr, 0, PAGE_SIZE);
>  
> -	/*
> -	 * Implicit barriers also ensure the zeroed page is visible to the page
> -	 * table walker
> -	 */
>  	pte_clear_fixmap();
>  
>  	return phys;
> -- 
> 2.16.4
> 

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 3/3] arm64: mmu: pgd_pgtable_alloc: drop barrier

[Mark Rutland]

On Wed, Feb 13, 2019 at 09:37:38AM +0000, Peng Fan wrote:
> The barriers are added for empty_zero_page, however the
> empty zero page has been moved to bss area by
> commit 5227cfa71f9e ("arm64: mm: place empty_zero_page in bss"),
> and it alreay added "dsb     ishst" in head.S to make sure
> the empty zero page visible to PTW.

The "zeroed page" the comment refers to is not empty_zero_page.

The page
the comment refers to is an arbitrary page returned by
__get_free_page(), which has been filled with zeroed at some point
thanks to PGALLOC_GFP containing __GFP_ZERO.

> pgd_pgtable_alloc is only called by __create_pgd_mapping, and
> there are implicit barriers in __create_pgd_mapping, so we
> could remove the barrier pgd_pgtable_alloc

I don't think this is true.

Consider:

  create_pgd_mapping()
  -> __create_pgd_mapping()
     -> alloc_init_pud()
        -> pgtable_alloc() // pgd_pgtable_alloc()
	-> __pgd_populate()

... where AFAICT there is no barrier between pgtable_alloc() and
__pgd_populate().

Where is the barrier between the page zeroing and the update of the pgd
entry?

Thanks,
Mark.

> 
> Signed-off-by: Peng Fan <peng.fan@nxp.com>
> ---
> 
> Note: this patch tested on qemu-system-aarch64, not real hardware.
> 
>  arch/arm64/mm/mmu.c | 2 --
>  1 file changed, 2 deletions(-)
> 
> diff --git a/arch/arm64/mm/mmu.c b/arch/arm64/mm/mmu.c
> index f61fa7c8fd2e..04e3d4d070ce 100644
> --- a/arch/arm64/mm/mmu.c
> +++ b/arch/arm64/mm/mmu.c
> @@ -371,8 +371,6 @@ static phys_addr_t pgd_pgtable_alloc(void)
>  	if (!ptr || !pgtable_page_ctor(virt_to_page(ptr)))
>  		BUG();
>  
> -	/* Ensure the zeroed page is visible to the page table walker */
> -	dsb(ishst);
>  	return __pa(ptr);
>  }
>  
> -- 
> 2.16.4
> 

Re: [PATCH 3/3] arm64: mmu: pgd_pgtable_alloc: drop barrier

[Mark Rutland]

On Wed, Feb 13, 2019 at 09:37:38AM +0000, Peng Fan wrote:
> The barriers are added for empty_zero_page, however the
> empty zero page has been moved to bss area by
> commit 5227cfa71f9e ("arm64: mm: place empty_zero_page in bss"),
> and it alreay added "dsb     ishst" in head.S to make sure
> the empty zero page visible to PTW.

The "zeroed page" the comment refers to is not empty_zero_page.

The page
the comment refers to is an arbitrary page returned by
__get_free_page(), which has been filled with zeroed at some point
thanks to PGALLOC_GFP containing __GFP_ZERO.

> pgd_pgtable_alloc is only called by __create_pgd_mapping, and
> there are implicit barriers in __create_pgd_mapping, so we
> could remove the barrier pgd_pgtable_alloc

I don't think this is true.

Consider:

  create_pgd_mapping()
  -> __create_pgd_mapping()
     -> alloc_init_pud()
        -> pgtable_alloc() // pgd_pgtable_alloc()
	-> __pgd_populate()

... where AFAICT there is no barrier between pgtable_alloc() and
__pgd_populate().

Where is the barrier between the page zeroing and the update of the pgd
entry?

Thanks,
Mark.

> 
> Signed-off-by: Peng Fan <peng.fan@nxp.com>
> ---
> 
> Note: this patch tested on qemu-system-aarch64, not real hardware.
> 
>  arch/arm64/mm/mmu.c | 2 --
>  1 file changed, 2 deletions(-)
> 
> diff --git a/arch/arm64/mm/mmu.c b/arch/arm64/mm/mmu.c
> index f61fa7c8fd2e..04e3d4d070ce 100644
> --- a/arch/arm64/mm/mmu.c
> +++ b/arch/arm64/mm/mmu.c
> @@ -371,8 +371,6 @@ static phys_addr_t pgd_pgtable_alloc(void)
>  	if (!ptr || !pgtable_page_ctor(virt_to_page(ptr)))
>  		BUG();
>  
> -	/* Ensure the zeroed page is visible to the page table walker */
> -	dsb(ishst);
>  	return __pa(ptr);
>  }
>  
> -- 
> 2.16.4
> 

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

RE: [PATCH 1/3] arm64: mmu: update paging_init comments

[Peng Fan]

Hi Mark,

> -----Original Message-----
> From: Mark Rutland [mailto:mark.rutland@arm.com]
> Sent: 2019年2月13日 19:19
> To: Peng Fan <peng.fan@nxp.com>
> Cc: catalin.marinas@arm.com; will.deacon@arm.com;
> ard.biesheuvel@linaro.org; yaojun8558363@gmail.com;
> cpandya@codeaurora.org; robin.murphy@arm.com;
> linux-arm-kernel@lists.infradead.org; linux-kernel@vger.kernel.org;
> van.freenix@gmail.com
> Subject: Re: [PATCH 1/3] arm64: mmu: update paging_init comments
> 
> On Wed, Feb 13, 2019 at 09:37:24AM +0000, Peng Fan wrote:
> > The comments has not been changed since the function introduced, but
> > the function has been changed dramatically, so update the comments to
> > reflect the code.
> >
> > Signed-off-by: Peng Fan <peng.fan@nxp.com>
> > ---
> >  arch/arm64/mm/mmu.c | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/arch/arm64/mm/mmu.c b/arch/arm64/mm/mmu.c index
> > d6b6f1b169bb..065202da3ad2 100644
> > --- a/arch/arm64/mm/mmu.c
> > +++ b/arch/arm64/mm/mmu.c
> > @@ -655,8 +655,8 @@ static void __init map_kernel(pgd_t *pgdp)  }
> >
> >  /*
> > - * paging_init() sets up the page tables, initialises the zone memory
> > - * maps and sets up the zero page.
> > + * paging_init() sets up the page tables, switch ttbr1 from
> > + init_pg_dir
> > + * to swapper_pg_dir, free init_pg_dir memblock and permit memblock
> resizing.
> >   */
> 
> I think that it would be better to remove the comment entirely. This doesn't
> explain the intent, or rationale, and all this information is obvious from a
> straight-line reading of the code.
> 
> If you remove the comment instead:
> 
> Acked-by: Mark Rutland <mark.rutland@arm.com>

I'll remove the comment with you a-b added.

Thanks,
Peng.
> 
> Mark.
> 
> >  void __init paging_init(void)
> >  {
> > --
> > 2.16.4
> >

RE: [PATCH 1/3] arm64: mmu: update paging_init comments

[Peng Fan]

Hi Mark,

> -----Original Message-----
> From: Mark Rutland [mailto:mark.rutland@arm.com]
> Sent: 2019年2月13日 19:19
> To: Peng Fan <peng.fan@nxp.com>
> Cc: catalin.marinas@arm.com; will.deacon@arm.com;
> ard.biesheuvel@linaro.org; yaojun8558363@gmail.com;
> cpandya@codeaurora.org; robin.murphy@arm.com;
> linux-arm-kernel@lists.infradead.org; linux-kernel@vger.kernel.org;
> van.freenix@gmail.com
> Subject: Re: [PATCH 1/3] arm64: mmu: update paging_init comments
> 
> On Wed, Feb 13, 2019 at 09:37:24AM +0000, Peng Fan wrote:
> > The comments has not been changed since the function introduced, but
> > the function has been changed dramatically, so update the comments to
> > reflect the code.
> >
> > Signed-off-by: Peng Fan <peng.fan@nxp.com>
> > ---
> >  arch/arm64/mm/mmu.c | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/arch/arm64/mm/mmu.c b/arch/arm64/mm/mmu.c index
> > d6b6f1b169bb..065202da3ad2 100644
> > --- a/arch/arm64/mm/mmu.c
> > +++ b/arch/arm64/mm/mmu.c
> > @@ -655,8 +655,8 @@ static void __init map_kernel(pgd_t *pgdp)  }
> >
> >  /*
> > - * paging_init() sets up the page tables, initialises the zone memory
> > - * maps and sets up the zero page.
> > + * paging_init() sets up the page tables, switch ttbr1 from
> > + init_pg_dir
> > + * to swapper_pg_dir, free init_pg_dir memblock and permit memblock
> resizing.
> >   */
> 
> I think that it would be better to remove the comment entirely. This doesn't
> explain the intent, or rationale, and all this information is obvious from a
> straight-line reading of the code.
> 
> If you remove the comment instead:
> 
> Acked-by: Mark Rutland <mark.rutland@arm.com>

I'll remove the comment with you a-b added.

Thanks,
Peng.
> 
> Mark.
> 
> >  void __init paging_init(void)
> >  {
> > --
> > 2.16.4
> >
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

RE: [PATCH 2/3] arm64: mmu: early_pgtable_alloc: remove unnecessary comments

[Peng Fan]

Hi Mark,

> -----Original Message-----
> From: Mark Rutland [mailto:mark.rutland@arm.com]
> Sent: 2019年2月13日 19:22
> To: Peng Fan <peng.fan@nxp.com>
> Cc: catalin.marinas@arm.com; will.deacon@arm.com;
> ard.biesheuvel@linaro.org; yaojun8558363@gmail.com;
> cpandya@codeaurora.org; robin.murphy@arm.com;
> linux-arm-kernel@lists.infradead.org; linux-kernel@vger.kernel.org;
> van.freenix@gmail.com
> Subject: Re: [PATCH 2/3] arm64: mmu: early_pgtable_alloc: remove
> unnecessary comments
> 
> On Wed, Feb 13, 2019 at 09:37:29AM +0000, Peng Fan wrote:
> > The empty zero page has been moved to bss area by commit 5227cfa71f9e
> > ("arm64: mm: place empty_zero_page in bss"),
> > and it alreay added "dsb     ishst" in head.S to make sure the empty
> > zero page visible to PTW.
> >
> > There is no code to reflect the comment, so remove it.
> 
> The comment below refers to the memory pointed to by ptr, which is zeroed
> by the memset. The comment does not refer to the generic
> empty_zero_page.
> 
> This comment is correct, and should stay. Please drop this patch.

I understand wrong. You are right.

Thanks,
Peng.

> 
> Thanks,
> Mark.
> 
> >
> > Signed-off-by: Peng Fan <peng.fan@nxp.com>
> > ---
> >  arch/arm64/mm/mmu.c | 4 ----
> >  1 file changed, 4 deletions(-)
> >
> > diff --git a/arch/arm64/mm/mmu.c b/arch/arm64/mm/mmu.c index
> > 065202da3ad2..f61fa7c8fd2e 100644
> > --- a/arch/arm64/mm/mmu.c
> > +++ b/arch/arm64/mm/mmu.c
> > @@ -113,10 +113,6 @@ static phys_addr_t __init
> > early_pgtable_alloc(void)
> >
> >  	memset(ptr, 0, PAGE_SIZE);
> >
> > -	/*
> > -	 * Implicit barriers also ensure the zeroed page is visible to the page
> > -	 * table walker
> > -	 */
> >  	pte_clear_fixmap();
> >
> >  	return phys;
> > --
> > 2.16.4
> >

RE: [PATCH 2/3] arm64: mmu: early_pgtable_alloc: remove unnecessary comments

[Peng Fan]

Hi Mark,

> -----Original Message-----
> From: Mark Rutland [mailto:mark.rutland@arm.com]
> Sent: 2019年2月13日 19:22
> To: Peng Fan <peng.fan@nxp.com>
> Cc: catalin.marinas@arm.com; will.deacon@arm.com;
> ard.biesheuvel@linaro.org; yaojun8558363@gmail.com;
> cpandya@codeaurora.org; robin.murphy@arm.com;
> linux-arm-kernel@lists.infradead.org; linux-kernel@vger.kernel.org;
> van.freenix@gmail.com
> Subject: Re: [PATCH 2/3] arm64: mmu: early_pgtable_alloc: remove
> unnecessary comments
> 
> On Wed, Feb 13, 2019 at 09:37:29AM +0000, Peng Fan wrote:
> > The empty zero page has been moved to bss area by commit 5227cfa71f9e
> > ("arm64: mm: place empty_zero_page in bss"),
> > and it alreay added "dsb     ishst" in head.S to make sure the empty
> > zero page visible to PTW.
> >
> > There is no code to reflect the comment, so remove it.
> 
> The comment below refers to the memory pointed to by ptr, which is zeroed
> by the memset. The comment does not refer to the generic
> empty_zero_page.
> 
> This comment is correct, and should stay. Please drop this patch.

I understand wrong. You are right.

Thanks,
Peng.

> 
> Thanks,
> Mark.
> 
> >
> > Signed-off-by: Peng Fan <peng.fan@nxp.com>
> > ---
> >  arch/arm64/mm/mmu.c | 4 ----
> >  1 file changed, 4 deletions(-)
> >
> > diff --git a/arch/arm64/mm/mmu.c b/arch/arm64/mm/mmu.c index
> > 065202da3ad2..f61fa7c8fd2e 100644
> > --- a/arch/arm64/mm/mmu.c
> > +++ b/arch/arm64/mm/mmu.c
> > @@ -113,10 +113,6 @@ static phys_addr_t __init
> > early_pgtable_alloc(void)
> >
> >  	memset(ptr, 0, PAGE_SIZE);
> >
> > -	/*
> > -	 * Implicit barriers also ensure the zeroed page is visible to the page
> > -	 * table walker
> > -	 */
> >  	pte_clear_fixmap();
> >
> >  	return phys;
> > --
> > 2.16.4
> >
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

RE: [PATCH 3/3] arm64: mmu: pgd_pgtable_alloc: drop barrier

[Peng Fan]

Hi Mark,

> -----Original Message-----
> From: Mark Rutland [mailto:mark.rutland@arm.com]
> Sent: 2019年2月13日 19:31
> To: Peng Fan <peng.fan@nxp.com>
> Cc: catalin.marinas@arm.com; will.deacon@arm.com;
> ard.biesheuvel@linaro.org; yaojun8558363@gmail.com;
> cpandya@codeaurora.org; robin.murphy@arm.com;
> linux-arm-kernel@lists.infradead.org; linux-kernel@vger.kernel.org;
> van.freenix@gmail.com
> Subject: Re: [PATCH 3/3] arm64: mmu: pgd_pgtable_alloc: drop barrier
> 
> On Wed, Feb 13, 2019 at 09:37:38AM +0000, Peng Fan wrote:
> > The barriers are added for empty_zero_page, however the empty zero
> > page has been moved to bss area by commit 5227cfa71f9e ("arm64: mm:
> > place empty_zero_page in bss"),
> > and it alreay added "dsb     ishst" in head.S to make sure
> > the empty zero page visible to PTW.
> 
> The "zeroed page" the comment refers to is not empty_zero_page.

Thanks, I understand wrong.

> 
> The page
> the comment refers to is an arbitrary page returned by __get_free_page(),
> which has been filled with zeroed at some point thanks to PGALLOC_GFP
> containing __GFP_ZERO.
> 
> > pgd_pgtable_alloc is only called by __create_pgd_mapping, and there
> > are implicit barriers in __create_pgd_mapping, so we could remove the
> > barrier pgd_pgtable_alloc
> 
> I don't think this is true.
> 
> Consider:
> 
>   create_pgd_mapping()
>   -> __create_pgd_mapping()
>      -> alloc_init_pud()
>         -> pgtable_alloc() // pgd_pgtable_alloc()
> 	-> __pgd_populate()
> 
> ... where AFAICT there is no barrier between pgtable_alloc() and
> __pgd_populate().
> 
> Where is the barrier between the page zeroing and the update of the pgd
> entry?

I though the barrier in __pgd_populate could be enough, but this is not
enough, we still need barrier before write pgd/pud/pmd/pte entries.

So drop this patch.

Thanks,
Peng.

> 
> Thanks,
> Mark.
> 
> >
> > Signed-off-by: Peng Fan <peng.fan@nxp.com>
> > ---
> >
> > Note: this patch tested on qemu-system-aarch64, not real hardware.
> >
> >  arch/arm64/mm/mmu.c | 2 --
> >  1 file changed, 2 deletions(-)
> >
> > diff --git a/arch/arm64/mm/mmu.c b/arch/arm64/mm/mmu.c index
> > f61fa7c8fd2e..04e3d4d070ce 100644
> > --- a/arch/arm64/mm/mmu.c
> > +++ b/arch/arm64/mm/mmu.c
> > @@ -371,8 +371,6 @@ static phys_addr_t pgd_pgtable_alloc(void)
> >  	if (!ptr || !pgtable_page_ctor(virt_to_page(ptr)))
> >  		BUG();
> >
> > -	/* Ensure the zeroed page is visible to the page table walker */
> > -	dsb(ishst);
> >  	return __pa(ptr);
> >  }
> >
> > --
> > 2.16.4
> >

RE: [PATCH 3/3] arm64: mmu: pgd_pgtable_alloc: drop barrier

[Peng Fan]

Hi Mark,

> -----Original Message-----
> From: Mark Rutland [mailto:mark.rutland@arm.com]
> Sent: 2019年2月13日 19:31
> To: Peng Fan <peng.fan@nxp.com>
> Cc: catalin.marinas@arm.com; will.deacon@arm.com;
> ard.biesheuvel@linaro.org; yaojun8558363@gmail.com;
> cpandya@codeaurora.org; robin.murphy@arm.com;
> linux-arm-kernel@lists.infradead.org; linux-kernel@vger.kernel.org;
> van.freenix@gmail.com
> Subject: Re: [PATCH 3/3] arm64: mmu: pgd_pgtable_alloc: drop barrier
> 
> On Wed, Feb 13, 2019 at 09:37:38AM +0000, Peng Fan wrote:
> > The barriers are added for empty_zero_page, however the empty zero
> > page has been moved to bss area by commit 5227cfa71f9e ("arm64: mm:
> > place empty_zero_page in bss"),
> > and it alreay added "dsb     ishst" in head.S to make sure
> > the empty zero page visible to PTW.
> 
> The "zeroed page" the comment refers to is not empty_zero_page.

Thanks, I understand wrong.

> 
> The page
> the comment refers to is an arbitrary page returned by __get_free_page(),
> which has been filled with zeroed at some point thanks to PGALLOC_GFP
> containing __GFP_ZERO.
> 
> > pgd_pgtable_alloc is only called by __create_pgd_mapping, and there
> > are implicit barriers in __create_pgd_mapping, so we could remove the
> > barrier pgd_pgtable_alloc
> 
> I don't think this is true.
> 
> Consider:
> 
>   create_pgd_mapping()
>   -> __create_pgd_mapping()
>      -> alloc_init_pud()
>         -> pgtable_alloc() // pgd_pgtable_alloc()
> 	-> __pgd_populate()
> 
> ... where AFAICT there is no barrier between pgtable_alloc() and
> __pgd_populate().
> 
> Where is the barrier between the page zeroing and the update of the pgd
> entry?

I though the barrier in __pgd_populate could be enough, but this is not
enough, we still need barrier before write pgd/pud/pmd/pte entries.

So drop this patch.

Thanks,
Peng.

> 
> Thanks,
> Mark.
> 
> >
> > Signed-off-by: Peng Fan <peng.fan@nxp.com>
> > ---
> >
> > Note: this patch tested on qemu-system-aarch64, not real hardware.
> >
> >  arch/arm64/mm/mmu.c | 2 --
> >  1 file changed, 2 deletions(-)
> >
> > diff --git a/arch/arm64/mm/mmu.c b/arch/arm64/mm/mmu.c index
> > f61fa7c8fd2e..04e3d4d070ce 100644
> > --- a/arch/arm64/mm/mmu.c
> > +++ b/arch/arm64/mm/mmu.c
> > @@ -371,8 +371,6 @@ static phys_addr_t pgd_pgtable_alloc(void)
> >  	if (!ptr || !pgtable_page_ctor(virt_to_page(ptr)))
> >  		BUG();
> >
> > -	/* Ensure the zeroed page is visible to the page table walker */
> > -	dsb(ishst);
> >  	return __pa(ptr);
> >  }
> >
> > --
> > 2.16.4
> >
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel