linux-mm.kvack.org archive mirror
 help / color / mirror / Atom feed
* [PATCH V3 0/2] mm/ioremap: Check virtual address alignment
@ 2019-05-09  4:46 Anshuman Khandual
  2019-05-09  4:46 ` [PATCH V3 1/2] mm/ioremap: Check virtual address alignment while creating huge mappings Anshuman Khandual
  2019-05-09  4:46 ` [PATCH V3 2/2] arm64/mm: Change offset base address in [pud|pmd]_free_[pmd|pte]_page() Anshuman Khandual
  0 siblings, 2 replies; 14+ messages in thread
From: Anshuman Khandual @ 2019-05-09  4:46 UTC (permalink / raw)
  To: linux-mm, linux-arm-kernel
  Cc: Anshuman Khandual, Andrew Morton, Will Deacon, Toshi Kani,
	Thomas Gleixner, Catalin Marinas, Mark Rutland, James Morse,
	Chintan Pandya, Robin Murphy, Laura Abbott

This series makes sure that ioremap_page_range()'s input virtual address
alignment is checked along with physical address before creating huge page
kernel mappings to avoid problems related to random freeing of PMD or PTE
pgtable pages potentially with existing valid entries. It also cleans up
arm64 pgtable page address offset in [pud|pmd]_free_[pmd|pte]_page().

Changes in V3:

- Added virtual address alignment check in ioremap_page_range()
- Dropped VM_WARN_ONCE() as input virtual addresses are aligned for sure

Changes in V2: (https://patchwork.kernel.org/patch/10922795/)

- Replaced WARN_ON_ONCE() with VM_WARN_ONCE() as per Catalin

Changes in V1: (https://patchwork.kernel.org/patch/10921135/)

Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Will Deacon <will.deacon@arm.com>
Cc: Toshi Kani <toshi.kani@hpe.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: Mark Rutland <mark.rutland@arm.com>
Cc: James Morse <james.morse@arm.com>
Cc: Chintan Pandya <cpandya@codeaurora.org>
Cc: Robin Murphy <robin.murphy@arm.com>
Cc: Laura Abbott <labbott@redhat.com>

Anshuman Khandual (2):
  mm/ioremap: Check virtual address alignment while creating huge mappings
  arm64/mm: Change offset base address in [pud|pmd]_free_[pmd|pte]_page()

 arch/arm64/mm/mmu.c | 6 +++---
 lib/ioremap.c       | 6 ++++++
 2 files changed, 9 insertions(+), 3 deletions(-)

-- 
2.20.1


^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH V3 1/2] mm/ioremap: Check virtual address alignment while creating huge mappings
  2019-05-09  4:46 [PATCH V3 0/2] mm/ioremap: Check virtual address alignment Anshuman Khandual
@ 2019-05-09  4:46 ` Anshuman Khandual
  2019-05-13 22:26   ` Kani, Toshi
  2019-05-09  4:46 ` [PATCH V3 2/2] arm64/mm: Change offset base address in [pud|pmd]_free_[pmd|pte]_page() Anshuman Khandual
  1 sibling, 1 reply; 14+ messages in thread
From: Anshuman Khandual @ 2019-05-09  4:46 UTC (permalink / raw)
  To: linux-mm, linux-arm-kernel
  Cc: Anshuman Khandual, Toshi Kani, Andrew Morton, Will Deacon,
	Chintan Pandya, Thomas Gleixner, Catalin Marinas

Virtual address alignment is essential in ensuring correct clearing for all
intermediate level pgtable entries and freeing associated pgtable pages. An
unaligned address can end up randomly freeing pgtable page that potentially
still contains valid mappings. Hence also check it's alignment along with
existing phys_addr check.

Signed-off-by: Anshuman Khandual <anshuman.khandual@arm.com>
Cc: Toshi Kani <toshi.kani@hpe.com>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Will Deacon <will.deacon@arm.com>
Cc: Chintan Pandya <cpandya@codeaurora.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Catalin Marinas <catalin.marinas@arm.com>
---
 lib/ioremap.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/lib/ioremap.c b/lib/ioremap.c
index 063213685563..8b5c8dda857d 100644
--- a/lib/ioremap.c
+++ b/lib/ioremap.c
@@ -86,6 +86,9 @@ static int ioremap_try_huge_pmd(pmd_t *pmd, unsigned long addr,
 	if ((end - addr) != PMD_SIZE)
 		return 0;
 
+	if (!IS_ALIGNED(addr, PMD_SIZE))
+		return 0;
+
 	if (!IS_ALIGNED(phys_addr, PMD_SIZE))
 		return 0;
 
@@ -126,6 +129,9 @@ static int ioremap_try_huge_pud(pud_t *pud, unsigned long addr,
 	if ((end - addr) != PUD_SIZE)
 		return 0;
 
+	if (!IS_ALIGNED(addr, PUD_SIZE))
+		return 0;
+
 	if (!IS_ALIGNED(phys_addr, PUD_SIZE))
 		return 0;
 
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH V3 2/2] arm64/mm: Change offset base address in [pud|pmd]_free_[pmd|pte]_page()
  2019-05-09  4:46 [PATCH V3 0/2] mm/ioremap: Check virtual address alignment Anshuman Khandual
  2019-05-09  4:46 ` [PATCH V3 1/2] mm/ioremap: Check virtual address alignment while creating huge mappings Anshuman Khandual
@ 2019-05-09  4:46 ` Anshuman Khandual
  2019-06-03 15:36   ` Catalin Marinas
  2019-06-04 14:24   ` Catalin Marinas
  1 sibling, 2 replies; 14+ messages in thread
From: Anshuman Khandual @ 2019-05-09  4:46 UTC (permalink / raw)
  To: linux-mm, linux-arm-kernel
  Cc: Anshuman Khandual, Catalin Marinas, Will Deacon, Mark Rutland,
	James Morse, Robin Murphy

Pgtable page address can be fetched with [pmd|pte]_offset_[kernel] if input
address is PMD_SIZE or PTE_SIZE aligned. Input address is now guaranteed to
be aligned, hence fetched pgtable page address is always correct. But using
0UL as offset base address has been a standard practice across platforms.
It also makes more sense as it isolates pgtable page address computation
from input virtual address alignment. This does not change functionality.

Signed-off-by: Anshuman Khandual <anshuman.khandual@arm.com>
Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: Will Deacon <will.deacon@arm.com>
Cc: Mark Rutland <mark.rutland@arm.com>
Cc: James Morse <james.morse@arm.com>
Cc: Robin Murphy <robin.murphy@arm.com>
---
 arch/arm64/mm/mmu.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/arch/arm64/mm/mmu.c b/arch/arm64/mm/mmu.c
index e97f018ff740..71bcb783aace 100644
--- a/arch/arm64/mm/mmu.c
+++ b/arch/arm64/mm/mmu.c
@@ -1005,7 +1005,7 @@ int pmd_free_pte_page(pmd_t *pmdp, unsigned long addr)
 		return 1;
 	}
 
-	table = pte_offset_kernel(pmdp, addr);
+	table = pte_offset_kernel(pmdp, 0UL);
 	pmd_clear(pmdp);
 	__flush_tlb_kernel_pgtable(addr);
 	pte_free_kernel(NULL, table);
@@ -1026,8 +1026,8 @@ int pud_free_pmd_page(pud_t *pudp, unsigned long addr)
 		return 1;
 	}
 
-	table = pmd_offset(pudp, addr);
-	pmdp = table;
+	table = pmd_offset(pudp, 0UL);
+	pmdp = pmd_offset(pudp, addr);
 	next = addr;
 	end = addr + PUD_SIZE;
 	do {
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* Re: [PATCH V3 1/2] mm/ioremap: Check virtual address alignment while creating huge mappings
  2019-05-09  4:46 ` [PATCH V3 1/2] mm/ioremap: Check virtual address alignment while creating huge mappings Anshuman Khandual
@ 2019-05-13 22:26   ` Kani, Toshi
  2019-05-14  5:55     ` Anshuman Khandual
  0 siblings, 1 reply; 14+ messages in thread
From: Kani, Toshi @ 2019-05-13 22:26 UTC (permalink / raw)
  To: linux-arm-kernel, linux-mm, anshuman.khandual
  Cc: tglx, cpandya, catalin.marinas, akpm, will.deacon

On Thu, 2019-05-09 at 10:16 +0530, Anshuman Khandual wrote:
> Virtual address alignment is essential in ensuring correct clearing for all
> intermediate level pgtable entries and freeing associated pgtable pages. An
> unaligned address can end up randomly freeing pgtable page that potentially
> still contains valid mappings. Hence also check it's alignment along with
> existing phys_addr check.
> 
> Signed-off-by: Anshuman Khandual <anshuman.khandual@arm.com>
> Cc: Toshi Kani <toshi.kani@hpe.com>
> Cc: Andrew Morton <akpm@linux-foundation.org>
> Cc: Will Deacon <will.deacon@arm.com>
> Cc: Chintan Pandya <cpandya@codeaurora.org>
> Cc: Thomas Gleixner <tglx@linutronix.de>
> Cc: Catalin Marinas <catalin.marinas@arm.com>
> ---
>  lib/ioremap.c | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/lib/ioremap.c b/lib/ioremap.c
> index 063213685563..8b5c8dda857d 100644
> --- a/lib/ioremap.c
> +++ b/lib/ioremap.c
> @@ -86,6 +86,9 @@ static int ioremap_try_huge_pmd(pmd_t *pmd, unsigned long addr,
>  	if ((end - addr) != PMD_SIZE)
>  		return 0;
>  
> +	if (!IS_ALIGNED(addr, PMD_SIZE))
> +		return 0;
> +
>  	if (!IS_ALIGNED(phys_addr, PMD_SIZE))
>  		return 0;
>  
> @@ -126,6 +129,9 @@ static int ioremap_try_huge_pud(pud_t *pud, unsigned long addr,
>  	if ((end - addr) != PUD_SIZE)
>  		return 0;
>  
> +	if (!IS_ALIGNED(addr, PUD_SIZE))
> +		return 0;
> +
>  	if (!IS_ALIGNED(phys_addr, PUD_SIZE))
>  		return 0;

Not sure if we have such case today, but I agree that it is prudent to
have such checks.  Is there any reason not to add this check to p4d for
consistency?

Thanks,
-Toshi


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH V3 1/2] mm/ioremap: Check virtual address alignment while creating huge mappings
  2019-05-13 22:26   ` Kani, Toshi
@ 2019-05-14  5:55     ` Anshuman Khandual
  2019-05-15  2:35       ` [PATCH V4] " Anshuman Khandual
  0 siblings, 1 reply; 14+ messages in thread
From: Anshuman Khandual @ 2019-05-14  5:55 UTC (permalink / raw)
  To: Kani, Toshi, linux-arm-kernel, linux-mm
  Cc: tglx, cpandya, catalin.marinas, akpm, will.deacon



On 05/14/2019 03:56 AM, Kani, Toshi wrote:
> On Thu, 2019-05-09 at 10:16 +0530, Anshuman Khandual wrote:
>> Virtual address alignment is essential in ensuring correct clearing for all
>> intermediate level pgtable entries and freeing associated pgtable pages. An
>> unaligned address can end up randomly freeing pgtable page that potentially
>> still contains valid mappings. Hence also check it's alignment along with
>> existing phys_addr check.
>>
>> Signed-off-by: Anshuman Khandual <anshuman.khandual@arm.com>
>> Cc: Toshi Kani <toshi.kani@hpe.com>
>> Cc: Andrew Morton <akpm@linux-foundation.org>
>> Cc: Will Deacon <will.deacon@arm.com>
>> Cc: Chintan Pandya <cpandya@codeaurora.org>
>> Cc: Thomas Gleixner <tglx@linutronix.de>
>> Cc: Catalin Marinas <catalin.marinas@arm.com>
>> ---
>>  lib/ioremap.c | 6 ++++++
>>  1 file changed, 6 insertions(+)
>>
>> diff --git a/lib/ioremap.c b/lib/ioremap.c
>> index 063213685563..8b5c8dda857d 100644
>> --- a/lib/ioremap.c
>> +++ b/lib/ioremap.c
>> @@ -86,6 +86,9 @@ static int ioremap_try_huge_pmd(pmd_t *pmd, unsigned long addr,
>>  	if ((end - addr) != PMD_SIZE)
>>  		return 0;
>>  
>> +	if (!IS_ALIGNED(addr, PMD_SIZE))
>> +		return 0;
>> +
>>  	if (!IS_ALIGNED(phys_addr, PMD_SIZE))
>>  		return 0;
>>  
>> @@ -126,6 +129,9 @@ static int ioremap_try_huge_pud(pud_t *pud, unsigned long addr,
>>  	if ((end - addr) != PUD_SIZE)
>>  		return 0;
>>  
>> +	if (!IS_ALIGNED(addr, PUD_SIZE))
>> +		return 0;
>> +
>>  	if (!IS_ALIGNED(phys_addr, PUD_SIZE))
>>  		return 0;
> 
> Not sure if we have such case today, but I agree that it is prudent to
> have such checks.  Is there any reason not to add this check to p4d for
> consistency?

No, will add it.


^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH V4] mm/ioremap: Check virtual address alignment while creating huge mappings
  2019-05-14  5:55     ` Anshuman Khandual
@ 2019-05-15  2:35       ` Anshuman Khandual
  2019-05-15  9:46         ` Will Deacon
                           ` (2 more replies)
  0 siblings, 3 replies; 14+ messages in thread
From: Anshuman Khandual @ 2019-05-15  2:35 UTC (permalink / raw)
  To: linux-arm-kernel, linux-mm
  Cc: Anshuman Khandual, Toshi Kani, Andrew Morton, Will Deacon,
	Chintan Pandya, Thomas Gleixner, Catalin Marinas

Virtual address alignment is essential in ensuring correct clearing for all
intermediate level pgtable entries and freeing associated pgtable pages. An
unaligned address can end up randomly freeing pgtable page that potentially
still contains valid mappings. Hence also check it's alignment along with
existing phys_addr check.

Signed-off-by: Anshuman Khandual <anshuman.khandual@arm.com>
Cc: Toshi Kani <toshi.kani@hpe.com>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Will Deacon <will.deacon@arm.com>
Cc: Chintan Pandya <cpandya@codeaurora.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Catalin Marinas <catalin.marinas@arm.com>
---
Changes in V4:

- Added similar check for ioremap_try_huge_p4d() as per Toshi Kani

 lib/ioremap.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/lib/ioremap.c b/lib/ioremap.c
index 063213685563..a95161d9c883 100644
--- a/lib/ioremap.c
+++ b/lib/ioremap.c
@@ -86,6 +86,9 @@ static int ioremap_try_huge_pmd(pmd_t *pmd, unsigned long addr,
 	if ((end - addr) != PMD_SIZE)
 		return 0;
 
+	if (!IS_ALIGNED(addr, PMD_SIZE))
+		return 0;
+
 	if (!IS_ALIGNED(phys_addr, PMD_SIZE))
 		return 0;
 
@@ -126,6 +129,9 @@ static int ioremap_try_huge_pud(pud_t *pud, unsigned long addr,
 	if ((end - addr) != PUD_SIZE)
 		return 0;
 
+	if (!IS_ALIGNED(addr, PUD_SIZE))
+		return 0;
+
 	if (!IS_ALIGNED(phys_addr, PUD_SIZE))
 		return 0;
 
@@ -166,6 +172,9 @@ static int ioremap_try_huge_p4d(p4d_t *p4d, unsigned long addr,
 	if ((end - addr) != P4D_SIZE)
 		return 0;
 
+	if (!IS_ALIGNED(addr, P4D_SIZE))
+		return 0;
+
 	if (!IS_ALIGNED(phys_addr, P4D_SIZE))
 		return 0;
 
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* Re: [PATCH V4] mm/ioremap: Check virtual address alignment while creating huge mappings
  2019-05-15  2:35       ` [PATCH V4] " Anshuman Khandual
@ 2019-05-15  9:46         ` Will Deacon
  2019-05-15 10:10           ` Anshuman Khandual
  2019-05-16 18:38         ` Kani, Toshi
  2019-06-06 13:51         ` Catalin Marinas
  2 siblings, 1 reply; 14+ messages in thread
From: Will Deacon @ 2019-05-15  9:46 UTC (permalink / raw)
  To: Anshuman Khandual
  Cc: linux-arm-kernel, linux-mm, Toshi Kani, Andrew Morton,
	Chintan Pandya, Thomas Gleixner, Catalin Marinas

On Wed, May 15, 2019 at 08:05:16AM +0530, Anshuman Khandual wrote:
> Virtual address alignment is essential in ensuring correct clearing for all
> intermediate level pgtable entries and freeing associated pgtable pages. An
> unaligned address can end up randomly freeing pgtable page that potentially
> still contains valid mappings. Hence also check it's alignment along with
> existing phys_addr check.
> 
> Signed-off-by: Anshuman Khandual <anshuman.khandual@arm.com>
> Cc: Toshi Kani <toshi.kani@hpe.com>
> Cc: Andrew Morton <akpm@linux-foundation.org>
> Cc: Will Deacon <will.deacon@arm.com>
> Cc: Chintan Pandya <cpandya@codeaurora.org>
> Cc: Thomas Gleixner <tglx@linutronix.de>
> Cc: Catalin Marinas <catalin.marinas@arm.com>
> ---
> Changes in V4:
> 
> - Added similar check for ioremap_try_huge_p4d() as per Toshi Kani

Sorry to be a pain, but in future please can you just resend the entire
series as a v4 (after giving it a few days for any other comments to come
in) if you make an update? It's a bit fiddly tracking which replies to which
individual patches need to be picked up, although I'm sure this varies
between maintainers.

No need to do anything this time, but just a small ask for future patches.

Will


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH V4] mm/ioremap: Check virtual address alignment while creating huge mappings
  2019-05-15  9:46         ` Will Deacon
@ 2019-05-15 10:10           ` Anshuman Khandual
  0 siblings, 0 replies; 14+ messages in thread
From: Anshuman Khandual @ 2019-05-15 10:10 UTC (permalink / raw)
  To: Will Deacon
  Cc: linux-arm-kernel, linux-mm, Toshi Kani, Andrew Morton,
	Chintan Pandya, Thomas Gleixner, Catalin Marinas



On 05/15/2019 03:16 PM, Will Deacon wrote:
> On Wed, May 15, 2019 at 08:05:16AM +0530, Anshuman Khandual wrote:
>> Virtual address alignment is essential in ensuring correct clearing for all
>> intermediate level pgtable entries and freeing associated pgtable pages. An
>> unaligned address can end up randomly freeing pgtable page that potentially
>> still contains valid mappings. Hence also check it's alignment along with
>> existing phys_addr check.
>>
>> Signed-off-by: Anshuman Khandual <anshuman.khandual@arm.com>
>> Cc: Toshi Kani <toshi.kani@hpe.com>
>> Cc: Andrew Morton <akpm@linux-foundation.org>
>> Cc: Will Deacon <will.deacon@arm.com>
>> Cc: Chintan Pandya <cpandya@codeaurora.org>
>> Cc: Thomas Gleixner <tglx@linutronix.de>
>> Cc: Catalin Marinas <catalin.marinas@arm.com>
>> ---
>> Changes in V4:
>>
>> - Added similar check for ioremap_try_huge_p4d() as per Toshi Kani
> 
> Sorry to be a pain, but in future please can you just resend the entire
> series as a v4 (after giving it a few days for any other comments to come
> in) if you make an update? It's a bit fiddly tracking which replies to which
> individual patches need to be picked up, although I'm sure this varies
> between maintainers.

I wondered for some time about both the ways before landing on this side as it was
pretty minor change. I understand the concern and will follow the suggestion next
time around. If this one requires further update, will make it V5 and carry the
change logs from here.

> 
> No need to do anything this time, but just a small ask for future patches.

Sure will do.


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH V4] mm/ioremap: Check virtual address alignment while creating huge mappings
  2019-05-15  2:35       ` [PATCH V4] " Anshuman Khandual
  2019-05-15  9:46         ` Will Deacon
@ 2019-05-16 18:38         ` Kani, Toshi
  2019-06-06 13:51         ` Catalin Marinas
  2 siblings, 0 replies; 14+ messages in thread
From: Kani, Toshi @ 2019-05-16 18:38 UTC (permalink / raw)
  To: linux-arm-kernel, linux-mm, anshuman.khandual
  Cc: tglx, cpandya, catalin.marinas, akpm, will.deacon

On Wed, 2019-05-15 at 08:05 +0530, Anshuman Khandual wrote:
> Virtual address alignment is essential in ensuring correct clearing for all
> intermediate level pgtable entries and freeing associated pgtable pages. An
> unaligned address can end up randomly freeing pgtable page that potentially
> still contains valid mappings. Hence also check it's alignment along with
> existing phys_addr check.
> 
> Signed-off-by: Anshuman Khandual <anshuman.khandual@arm.com>
> Cc: Toshi Kani <toshi.kani@hpe.com>
> Cc: Andrew Morton <akpm@linux-foundation.org>
> Cc: Will Deacon <will.deacon@arm.com>
> Cc: Chintan Pandya <cpandya@codeaurora.org>
> Cc: Thomas Gleixner <tglx@linutronix.de>
> Cc: Catalin Marinas <catalin.marinas@arm.com>
> ---
> Changes in V4:
> 
> - Added similar check for ioremap_try_huge_p4d() as per Toshi Kani

Thanks for the update. It looks good to me.

Reviewed-by: Toshi Kani <toshi.kani@hpe.com>

-Toshi

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH V3 2/2] arm64/mm: Change offset base address in [pud|pmd]_free_[pmd|pte]_page()
  2019-05-09  4:46 ` [PATCH V3 2/2] arm64/mm: Change offset base address in [pud|pmd]_free_[pmd|pte]_page() Anshuman Khandual
@ 2019-06-03 15:36   ` Catalin Marinas
  2019-06-06  8:08     ` Anshuman Khandual
  2019-06-04 14:24   ` Catalin Marinas
  1 sibling, 1 reply; 14+ messages in thread
From: Catalin Marinas @ 2019-06-03 15:36 UTC (permalink / raw)
  To: Anshuman Khandual
  Cc: linux-mm, linux-arm-kernel, Will Deacon, Mark Rutland,
	James Morse, Robin Murphy

Hi Anshuman,


On Thu, May 09, 2019 at 10:16:17AM +0530, Anshuman Khandual wrote:
> Pgtable page address can be fetched with [pmd|pte]_offset_[kernel] if input
> address is PMD_SIZE or PTE_SIZE aligned. Input address is now guaranteed to
> be aligned, hence fetched pgtable page address is always correct. But using
> 0UL as offset base address has been a standard practice across platforms.
> It also makes more sense as it isolates pgtable page address computation
> from input virtual address alignment. This does not change functionality.
> 
> Signed-off-by: Anshuman Khandual <anshuman.khandual@arm.com>
> Cc: Catalin Marinas <catalin.marinas@arm.com>
> Cc: Will Deacon <will.deacon@arm.com>
> Cc: Mark Rutland <mark.rutland@arm.com>
> Cc: James Morse <james.morse@arm.com>
> Cc: Robin Murphy <robin.murphy@arm.com>

What's the plan with this small series? I didn't find a v5 (unless I
deleted it by mistake). I can queue this patch through the arm64 tree or
they can both go in via the mm tree.

-- 
Catalin


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH V3 2/2] arm64/mm: Change offset base address in [pud|pmd]_free_[pmd|pte]_page()
  2019-05-09  4:46 ` [PATCH V3 2/2] arm64/mm: Change offset base address in [pud|pmd]_free_[pmd|pte]_page() Anshuman Khandual
  2019-06-03 15:36   ` Catalin Marinas
@ 2019-06-04 14:24   ` Catalin Marinas
  2019-06-06  4:44     ` Anshuman Khandual
  1 sibling, 1 reply; 14+ messages in thread
From: Catalin Marinas @ 2019-06-04 14:24 UTC (permalink / raw)
  To: Anshuman Khandual
  Cc: linux-mm, linux-arm-kernel, Will Deacon, Mark Rutland,
	James Morse, Robin Murphy

On Thu, May 09, 2019 at 10:16:17AM +0530, Anshuman Khandual wrote:
> Pgtable page address can be fetched with [pmd|pte]_offset_[kernel] if input
> address is PMD_SIZE or PTE_SIZE aligned. Input address is now guaranteed to
> be aligned, hence fetched pgtable page address is always correct. But using
> 0UL as offset base address has been a standard practice across platforms.
> It also makes more sense as it isolates pgtable page address computation
> from input virtual address alignment. This does not change functionality.
> 
> Signed-off-by: Anshuman Khandual <anshuman.khandual@arm.com>
> Cc: Catalin Marinas <catalin.marinas@arm.com>
> Cc: Will Deacon <will.deacon@arm.com>
> Cc: Mark Rutland <mark.rutland@arm.com>
> Cc: James Morse <james.morse@arm.com>
> Cc: Robin Murphy <robin.murphy@arm.com>
> ---
>  arch/arm64/mm/mmu.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/arch/arm64/mm/mmu.c b/arch/arm64/mm/mmu.c
> index e97f018ff740..71bcb783aace 100644
> --- a/arch/arm64/mm/mmu.c
> +++ b/arch/arm64/mm/mmu.c
> @@ -1005,7 +1005,7 @@ int pmd_free_pte_page(pmd_t *pmdp, unsigned long addr)
>  		return 1;
>  	}
>  
> -	table = pte_offset_kernel(pmdp, addr);
> +	table = pte_offset_kernel(pmdp, 0UL);
>  	pmd_clear(pmdp);
>  	__flush_tlb_kernel_pgtable(addr);
>  	pte_free_kernel(NULL, table);
> @@ -1026,8 +1026,8 @@ int pud_free_pmd_page(pud_t *pudp, unsigned long addr)
>  		return 1;
>  	}
>  
> -	table = pmd_offset(pudp, addr);
> -	pmdp = table;
> +	table = pmd_offset(pudp, 0UL);
> +	pmdp = pmd_offset(pudp, addr);
>  	next = addr;
>  	end = addr + PUD_SIZE;
>  	do {

I have the same comment as last time:

https://lore.kernel.org/linux-arm-kernel/20190430161759.GI29799@arrakis.emea.arm.com/

I don't see why pmdp needs to be different from table. We get the
pointer to a pmd page and we want to iterate over it to free the pte
entries it contains. You can add a VM_WARN on addr alignment as in the
previous version of the patch but pmdp is just an iterator over table.

-- 
Catalin


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH V3 2/2] arm64/mm: Change offset base address in [pud|pmd]_free_[pmd|pte]_page()
  2019-06-04 14:24   ` Catalin Marinas
@ 2019-06-06  4:44     ` Anshuman Khandual
  0 siblings, 0 replies; 14+ messages in thread
From: Anshuman Khandual @ 2019-06-06  4:44 UTC (permalink / raw)
  To: Catalin Marinas
  Cc: linux-mm, linux-arm-kernel, Will Deacon, Mark Rutland,
	James Morse, Robin Murphy



On 06/04/2019 07:54 PM, Catalin Marinas wrote:
> On Thu, May 09, 2019 at 10:16:17AM +0530, Anshuman Khandual wrote:
>> Pgtable page address can be fetched with [pmd|pte]_offset_[kernel] if input
>> address is PMD_SIZE or PTE_SIZE aligned. Input address is now guaranteed to
>> be aligned, hence fetched pgtable page address is always correct. But using
>> 0UL as offset base address has been a standard practice across platforms.
>> It also makes more sense as it isolates pgtable page address computation
>> from input virtual address alignment. This does not change functionality.
>>
>> Signed-off-by: Anshuman Khandual <anshuman.khandual@arm.com>
>> Cc: Catalin Marinas <catalin.marinas@arm.com>
>> Cc: Will Deacon <will.deacon@arm.com>
>> Cc: Mark Rutland <mark.rutland@arm.com>
>> Cc: James Morse <james.morse@arm.com>
>> Cc: Robin Murphy <robin.murphy@arm.com>
>> ---
>>  arch/arm64/mm/mmu.c | 6 +++---
>>  1 file changed, 3 insertions(+), 3 deletions(-)
>>
>> diff --git a/arch/arm64/mm/mmu.c b/arch/arm64/mm/mmu.c
>> index e97f018ff740..71bcb783aace 100644
>> --- a/arch/arm64/mm/mmu.c
>> +++ b/arch/arm64/mm/mmu.c
>> @@ -1005,7 +1005,7 @@ int pmd_free_pte_page(pmd_t *pmdp, unsigned long addr)
>>  		return 1;
>>  	}
>>  
>> -	table = pte_offset_kernel(pmdp, addr);
>> +	table = pte_offset_kernel(pmdp, 0UL);
>>  	pmd_clear(pmdp);
>>  	__flush_tlb_kernel_pgtable(addr);
>>  	pte_free_kernel(NULL, table);
>> @@ -1026,8 +1026,8 @@ int pud_free_pmd_page(pud_t *pudp, unsigned long addr)
>>  		return 1;
>>  	}
>>  
>> -	table = pmd_offset(pudp, addr);
>> -	pmdp = table;
>> +	table = pmd_offset(pudp, 0UL);
>> +	pmdp = pmd_offset(pudp, addr);
>>  	next = addr;
>>  	end = addr + PUD_SIZE;
>>  	do {
> 
> I have the same comment as last time:
> 
> https://lore.kernel.org/linux-arm-kernel/20190430161759.GI29799@arrakis.emea.arm.com/
> 
> I don't see why pmdp needs to be different from table. We get the
> pointer to a pmd page and we want to iterate over it to free the pte
> entries it contains. You can add a VM_WARN on addr alignment as in the
> previous version of the patch but pmdp is just an iterator over table.

Fair enough. I believe VM_WARN() is needed to check address alignment because
they are now guaranteed to be aligned because of the previous patch. I guess
we should probably drop this patch and consider only the previous one ?


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH V3 2/2] arm64/mm: Change offset base address in [pud|pmd]_free_[pmd|pte]_page()
  2019-06-03 15:36   ` Catalin Marinas
@ 2019-06-06  8:08     ` Anshuman Khandual
  0 siblings, 0 replies; 14+ messages in thread
From: Anshuman Khandual @ 2019-06-06  8:08 UTC (permalink / raw)
  To: Catalin Marinas
  Cc: linux-mm, linux-arm-kernel, Will Deacon, Mark Rutland,
	James Morse, Robin Murphy



On 06/03/2019 09:06 PM, Catalin Marinas wrote:
> Hi Anshuman,
> 
> 
> On Thu, May 09, 2019 at 10:16:17AM +0530, Anshuman Khandual wrote:
>> Pgtable page address can be fetched with [pmd|pte]_offset_[kernel] if input
>> address is PMD_SIZE or PTE_SIZE aligned. Input address is now guaranteed to
>> be aligned, hence fetched pgtable page address is always correct. But using
>> 0UL as offset base address has been a standard practice across platforms.
>> It also makes more sense as it isolates pgtable page address computation
>> from input virtual address alignment. This does not change functionality.
>>
>> Signed-off-by: Anshuman Khandual <anshuman.khandual@arm.com>
>> Cc: Catalin Marinas <catalin.marinas@arm.com>
>> Cc: Will Deacon <will.deacon@arm.com>
>> Cc: Mark Rutland <mark.rutland@arm.com>
>> Cc: James Morse <james.morse@arm.com>
>> Cc: Robin Murphy <robin.murphy@arm.com>
> 
> What's the plan with this small series? I didn't find a v5 (unless I
> deleted it by mistake). I can queue this patch through the arm64 tree or
> they can both go in via the mm tree.

As mentioned earlier [1] I believe the second patch is not needed anymore. Hence
only V4 of the first patch (https://patchwork.kernel.org/patch/10944191/) which
has a Reviewed-by tag should be considered.

[1] https://patchwork.kernel.org/patch/10936625/


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH V4] mm/ioremap: Check virtual address alignment while creating huge mappings
  2019-05-15  2:35       ` [PATCH V4] " Anshuman Khandual
  2019-05-15  9:46         ` Will Deacon
  2019-05-16 18:38         ` Kani, Toshi
@ 2019-06-06 13:51         ` Catalin Marinas
  2 siblings, 0 replies; 14+ messages in thread
From: Catalin Marinas @ 2019-06-06 13:51 UTC (permalink / raw)
  To: Anshuman Khandual
  Cc: linux-arm-kernel, linux-mm, Toshi Kani, Andrew Morton,
	Will Deacon, Chintan Pandya, Thomas Gleixner

On Wed, May 15, 2019 at 08:05:16AM +0530, Anshuman Khandual wrote:
> Virtual address alignment is essential in ensuring correct clearing for all
> intermediate level pgtable entries and freeing associated pgtable pages. An
> unaligned address can end up randomly freeing pgtable page that potentially
> still contains valid mappings. Hence also check it's alignment along with
> existing phys_addr check.
> 
> Signed-off-by: Anshuman Khandual <anshuman.khandual@arm.com>
> Cc: Toshi Kani <toshi.kani@hpe.com>
> Cc: Andrew Morton <akpm@linux-foundation.org>
> Cc: Will Deacon <will.deacon@arm.com>
> Cc: Chintan Pandya <cpandya@codeaurora.org>
> Cc: Thomas Gleixner <tglx@linutronix.de>
> Cc: Catalin Marinas <catalin.marinas@arm.com>

Reviewed-by: Catalin Marinas <catalin.marinas@arm.com>

I guess Andrew can pick this up, otherwise I can queue it through arm64
(there are no arm64 dependencies on this).

Thanks.

-- 
Catalin


^ permalink raw reply	[flat|nested] 14+ messages in thread

end of thread, other threads:[~2019-06-06 13:51 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-05-09  4:46 [PATCH V3 0/2] mm/ioremap: Check virtual address alignment Anshuman Khandual
2019-05-09  4:46 ` [PATCH V3 1/2] mm/ioremap: Check virtual address alignment while creating huge mappings Anshuman Khandual
2019-05-13 22:26   ` Kani, Toshi
2019-05-14  5:55     ` Anshuman Khandual
2019-05-15  2:35       ` [PATCH V4] " Anshuman Khandual
2019-05-15  9:46         ` Will Deacon
2019-05-15 10:10           ` Anshuman Khandual
2019-05-16 18:38         ` Kani, Toshi
2019-06-06 13:51         ` Catalin Marinas
2019-05-09  4:46 ` [PATCH V3 2/2] arm64/mm: Change offset base address in [pud|pmd]_free_[pmd|pte]_page() Anshuman Khandual
2019-06-03 15:36   ` Catalin Marinas
2019-06-06  8:08     ` Anshuman Khandual
2019-06-04 14:24   ` Catalin Marinas
2019-06-06  4:44     ` Anshuman Khandual

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).