Skip to contentSkip to conversation

arm64: fix kernel crash with 48-bit VA and 64KB granule

PATCH New Dennis Chen 4 participants 18 messages Jan 5, 2016 – Jan 6, 2016 arm64

lore
Share & tools
lore

Dennis Chen

author
#0 lore raw

PATCH 1/1 arm64: fix kernel crash with 48-bit VA and 64KB granule

The commit 3400749b5a22 ("arm64/efi: refactor EFI init and runtime code for reuse by 32-bit ARM") uses pgd_alloc() to allocate space for efi_mm.pgd while not the static efi_pgd[], since this function will be called with early_initcall, which results in the pgd_cache used by pgd_alloc() has not been initialized yet, kernel will hang in this case. This patch is trying to make the pgd_cache_init() called before arm_enable_runtime_services() by changing its core_initcall to early_initcall.

Signed-off-by Dennis Chen <[email protected]>
Tested-by Sudeep Holla <[email protected]>
Cc Will Deacon <[email protected]>
Cc Catalin Marinas <[email protected]>
Cc Ard Biesheuvel <[email protected]>
Cc Sudeep Holla <[email protected]>
---
 arch/arm64/mm/pgd.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
arch/arm64/mm/pgd.c +1 -1
index cb3ba1b..859a788 100644
@@ -56,4 +56,4 @@ static int __init pgd_cache_init(void)
                                              SLAB_PANIC, NULL);
        return 0;
 }
-core_initcall(pgd_cache_init);
+early_initcall(pgd_cache_init);

--
1.9.1

IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.

#1 lore raw

PATCH 1/1 arm64: fix kernel crash with 48-bit VA and 64KB granule

Quoted 25 lines · Dennis Chen

The commit 3400749b5a22 ("arm64/efi: refactor EFI init and runtime code for reuse by 32-bit ARM") uses pgd_alloc() to allocate space for efi_mm.pgd while not the static efi_pgd[], since this function will be called with early_initcall, which results in the pgd_cache used by pgd_alloc() has not been initialized yet, kernel will hang in this case. This patch is trying to make the pgd_cache_init() called before arm_enable_runtime_services() by changing its core_initcall to early_initcall.

Signed-off-by Dennis Chen <[email protected]>
Tested-by Sudeep Holla <[email protected]>
Cc Will Deacon <[email protected]>
Cc Catalin Marinas <[email protected]>
Cc Ard Biesheuvel <[email protected]>
Cc Sudeep Holla <[email protected]>
---
 arch/arm64/mm/pgd.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
arch/arm64/mm/pgd.c +0 -0
index cb3ba1b..859a788 100644

arch/arm64/mm/pgd.c static int __init pgd_cache_init(void) · line 59

Quoted 5 lines
arch/arm64/mm/pgd.c +1 -1
@@ -56,4 +56,4 @@ static int __init pgd_cache_init(void)
                                              SLAB_PANIC, NULL);
        return 0;
 }
-core_initcall(pgd_cache_init);
+early_initcall(pgd_cache_init);

Hello all,

Apologies for not spotting this before sending it out.

The issue only occurs when PGD_SIZE != PAGE_SIZE, which is why I did not see it myself. I only tested with 4k/39-bits and 64k/42-bits IIRC, since the series this is part of primarily concerns ARM not arm64.

Anyway, shuffling init ordering is my least favorite way of fixing bugs. Since only ARM requires pgd_alloc() (for the kernel mappings), we could also simply factor out pgd_alloc() into efi_pgd_alloc(), and define it to mean '__get_free_page(PGALLOC_GFP)' on arm64.

Signature · 1 line
Ard.
author
#2 lore raw

PATCH 1/1 arm64: fix kernel crash with 48-bit VA and 64KB granule

Quoted 37 lines · Ard Biesheuvel
Quoted 25 lines · Dennis Chen

The commit 3400749b5a22 ("arm64/efi: refactor EFI init and runtime code for reuse by 32-bit ARM") uses pgd_alloc() to allocate space for efi_mm.pgd while not the static efi_pgd[], since this function will be called with early_initcall, which results in the pgd_cache used by pgd_alloc() has not been initialized yet, kernel will hang in this case. This patch is trying to make the pgd_cache_init() called before arm_enable_runtime_services() by changing its core_initcall to early_initcall.

Signed-off-by Dennis Chen <[email protected]>
Tested-by Sudeep Holla <[email protected]>
Cc Will Deacon <[email protected]>
Cc Catalin Marinas <[email protected]>
Cc Ard Biesheuvel <[email protected]>
Cc Sudeep Holla <[email protected]>
---
 arch/arm64/mm/pgd.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
arch/arm64/mm/pgd.c +0 -0
index cb3ba1b..859a788 100644

arch/arm64/mm/pgd.c static int __init pgd_cache_init(void) · line 59

Quoted 5 lines
arch/arm64/mm/pgd.c +1 -1
@@ -56,4 +56,4 @@ static int __init pgd_cache_init(void)
                                              SLAB_PANIC, NULL);
        return 0;
 }
-core_initcall(pgd_cache_init);
+early_initcall(pgd_cache_init);

Hello all,

Apologies for not spotting this before sending it out.

Quoted 10 lines

The issue only occurs when PGD_SIZE != PAGE_SIZE, which is why I did not see it myself. I only tested with 4k/39-bits and 64k/42-bits IIRC, since the series this is part of primarily concerns ARM not arm64.

Anyway, shuffling init ordering is my least favorite way of fixing bugs. Since only ARM requires pgd_alloc() (for the kernel mappings), we could also simply factor out pgd_alloc() into efi_pgd_alloc(), and define it to mean '__get_free_page(PGALLOC_GFP)' on arm64.

Ard,

Both arm and arm64 provide pgd_alloc(), I don't think it's a good idea to create another efi_pgd_alloc() used by efi only, which looks more like a temp workaround. Plus, for 64KB page granule when VA = 48bit, the PGD_SIZE = 0x200, it doesn't make sense to get an entire page as the space for the efi.pgd. Changing the init order sequence is a reasonable way to fix it unless you can see some potential issues there.

Quoted 3 lines

--
Ard.

IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.

#3 lore raw

PATCH 1/1 arm64: fix kernel crash with 48-bit VA and 64KB granule

Quoted 48 lines · Dennis Chen
Quoted 37 lines · Ard Biesheuvel
Quoted 25 lines · Dennis Chen

The commit 3400749b5a22 ("arm64/efi: refactor EFI init and runtime code for reuse by 32-bit ARM") uses pgd_alloc() to allocate space for efi_mm.pgd while not the static efi_pgd[], since this function will be called with early_initcall, which results in the pgd_cache used by pgd_alloc() has not been initialized yet, kernel will hang in this case. This patch is trying to make the pgd_cache_init() called before arm_enable_runtime_services() by changing its core_initcall to early_initcall.

Signed-off-by Dennis Chen <[email protected]>
Tested-by Sudeep Holla <[email protected]>
Cc Will Deacon <[email protected]>
Cc Catalin Marinas <[email protected]>
Cc Ard Biesheuvel <[email protected]>
Cc Sudeep Holla <[email protected]>
---
 arch/arm64/mm/pgd.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
arch/arm64/mm/pgd.c +0 -0
index cb3ba1b..859a788 100644

arch/arm64/mm/pgd.c static int __init pgd_cache_init(void) · line 59

Quoted 5 lines
arch/arm64/mm/pgd.c +1 -1
@@ -56,4 +56,4 @@ static int __init pgd_cache_init(void)
                                              SLAB_PANIC, NULL);
        return 0;
 }
-core_initcall(pgd_cache_init);
+early_initcall(pgd_cache_init);

Hello all,

Apologies for not spotting this before sending it out.

Quoted 10 lines

The issue only occurs when PGD_SIZE != PAGE_SIZE, which is why I did not see it myself. I only tested with 4k/39-bits and 64k/42-bits IIRC, since the series this is part of primarily concerns ARM not arm64.

Anyway, shuffling init ordering is my least favorite way of fixing bugs. Since only ARM requires pgd_alloc() (for the kernel mappings), we could also simply factor out pgd_alloc() into efi_pgd_alloc(), and define it to mean '__get_free_page(PGALLOC_GFP)' on arm64.

Quoted 8 lines

Ard,

Both arm and arm64 provide pgd_alloc(), I don't think it's a good idea to create another efi_pgd_alloc() used by efi only, which looks more like a temp workaround. Plus, for 64KB page granule when VA = 48bit, the PGD_SIZE = 0x200, it doesn't make sense to get an entire page as the space for the efi.pgd. Changing the init order sequence is a reasonable way to fix it unless you can see some potential issues there.

Well, since arm_enable_runtime_services() is an early_initcall() itself, how are you guaranteeing the ordering between the two? Link order?

author
#4 lore raw

PATCH 1/1 arm64: fix kernel crash with 48-bit VA and 64KB granule

Quoted 58 lines · Ard Biesheuvel
Quoted 48 lines · Dennis Chen
Quoted 37 lines · Ard Biesheuvel
Quoted 25 lines · Dennis Chen

The commit 3400749b5a22 ("arm64/efi: refactor EFI init and runtime code for reuse by 32-bit ARM") uses pgd_alloc() to allocate space for efi_mm.pgd while not the static efi_pgd[], since this function will be called with early_initcall, which results in the pgd_cache used by pgd_alloc() has not been initialized yet, kernel will hang in this case. This patch is trying to make the pgd_cache_init() called before arm_enable_runtime_services() by changing its core_initcall to early_initcall.

Signed-off-by Dennis Chen <[email protected]>
Tested-by Sudeep Holla <[email protected]>
Cc Will Deacon <[email protected]>
Cc Catalin Marinas <[email protected]>
Cc Ard Biesheuvel <[email protected]>
Cc Sudeep Holla <[email protected]>
---
 arch/arm64/mm/pgd.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
arch/arm64/mm/pgd.c +0 -0
index cb3ba1b..859a788 100644

arch/arm64/mm/pgd.c static int __init pgd_cache_init(void) · line 59

Quoted 5 lines
arch/arm64/mm/pgd.c +1 -1
@@ -56,4 +56,4 @@ static int __init pgd_cache_init(void)
                                              SLAB_PANIC, NULL);
        return 0;
 }
-core_initcall(pgd_cache_init);
+early_initcall(pgd_cache_init);

Hello all,

Apologies for not spotting this before sending it out.

Quoted 10 lines

The issue only occurs when PGD_SIZE != PAGE_SIZE, which is why I did not see it myself. I only tested with 4k/39-bits and 64k/42-bits IIRC, since the series this is part of primarily concerns ARM not arm64.

Anyway, shuffling init ordering is my least favorite way of fixing bugs. Since only ARM requires pgd_alloc() (for the kernel mappings), we could also simply factor out pgd_alloc() into efi_pgd_alloc(), and define it to mean '__get_free_page(PGALLOC_GFP)' on arm64.

Quoted 8 lines

Ard,

Both arm and arm64 provide pgd_alloc(), I don't think it's a good idea to create another efi_pgd_alloc() used by efi only, which looks more like a temp workaround. Plus, for 64KB page granule when VA = 48bit, the PGD_SIZE = 0x200, it doesn't make sense to get an entire page as the space for the efi.pgd. Changing the init order sequence is a reasonable way to fix it unless you can see some potential issues there.

Quoted 4 lines

Well, since arm_enable_runtime_services() is an early_initcall() itself, how are you guaranteeing the ordering between the two? Link order?

Link order.
IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.

maintainer
#5 lore raw

PATCH 1/1 arm64: fix kernel crash with 48-bit VA and 64KB granule

Quoted 32 lines · Dennis Chen
Quoted 31 lines · Ard Biesheuvel
Quoted 31 lines
Quoted 31 lines · Dennis Chen
Quoted 30 lines

The commit 3400749b5a22 ("arm64/efi: refactor EFI init and runtime code for reuse by 32-bit ARM") uses pgd_alloc() to allocate space for efi_mm.pgd while not the static efi_pgd[], since this function will be called with early_initcall, which results in the pgd_cache used by pgd_alloc() has not been initialized yet, kernel will hang in this case. This patch is trying to make the pgd_cache_init() called before arm_enable_runtime_services() by changing its core_initcall to early_initcall.

Signed-off-by Dennis Chen <[email protected]>
Tested-by Sudeep Holla <[email protected]>
Cc Will Deacon <[email protected]>
Cc Catalin Marinas <[email protected]>
Cc Ard Biesheuvel <[email protected]>
Cc Sudeep Holla <[email protected]>
---
 arch/arm64/mm/pgd.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
arch/arm64/mm/pgd.c +1 -1
index cb3ba1b..859a788 100644
@@ -56,4 +56,4 @@ static int __init pgd_cache_init(void)
                                              SLAB_PANIC, NULL);
        return 0;
 }
-core_initcall(pgd_cache_init);
+early_initcall(pgd_cache_init);

[...]

Quoted 5 lines
Quoted 3 lines

Well, since arm_enable_runtime_services() is an early_initcall() itself, how are you guaranteeing the ordering between the two? Link order?

Link order.

And can you explain how this works, what guarantees it gives?

Quoted 5 lines

IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.

BTW, please sort out the legal disclaimer (raise an IT ticket).

Signature · 1 line
Catalin
author
#6 lore raw

PATCH 1/1 arm64: fix kernel crash with 48-bit VA and 64KB granule

Quoted 34 lines · Catalin Marinas
Quoted 32 lines · Dennis Chen
Quoted 31 lines · Ard Biesheuvel
Quoted 31 lines
Quoted 31 lines · Dennis Chen
Quoted 30 lines

The commit 3400749b5a22 ("arm64/efi: refactor EFI init and runtime code for reuse by 32-bit ARM") uses pgd_alloc() to allocate space for efi_mm.pgd while not the static efi_pgd[], since this function will be called with early_initcall, which results in the pgd_cache used by pgd_alloc() has not been initialized yet, kernel will hang in this case. This patch is trying to make the pgd_cache_init() called before arm_enable_runtime_services() by changing its core_initcall to early_initcall.

Signed-off-by Dennis Chen <[email protected]>
Tested-by Sudeep Holla <[email protected]>
Cc Will Deacon <[email protected]>
Cc Catalin Marinas <[email protected]>
Cc Ard Biesheuvel <[email protected]>
Cc Sudeep Holla <[email protected]>
---
 arch/arm64/mm/pgd.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
arch/arm64/mm/pgd.c +1 -1
index cb3ba1b..859a788 100644
@@ -56,4 +56,4 @@ static int __init pgd_cache_init(void)
                                              SLAB_PANIC, NULL);
        return 0;
 }
-core_initcall(pgd_cache_init);
+early_initcall(pgd_cache_init);

[...]

Quoted 8 lines
Quoted 5 lines
Quoted 3 lines

Well, since arm_enable_runtime_services() is an early_initcall() itself, how are you guaranteeing the ordering between the two? Link order?

Link order.

And can you explain how this works, what guarantees it gives?

You can take a look at include/asm-generic/vmlinux.ldx.h: INIT_CALLS macro, for the init call section, early_initcall is the first chuck in the section, followed by LEVEL[0-7]. For the same level, the layout order is determined by the link order, ARCH is always precedence over the drivers. Catalin, are you giving me a kernel examination? :)

Quoted 9 lines
Quoted 6 lines

IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.

BTW, please sort out the legal disclaimer (raise an IT ticket).

Hmm, the ticket has been raised.

Quoted 8 lines

--
Catalin

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

IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.

#7 lore raw

PATCH 1/1 arm64: fix kernel crash with 48-bit VA and 64KB granule

Quoted 41 lines · Dennis Chen
Quoted 40 lines · Catalin Marinas
Quoted 32 lines · Dennis Chen
Quoted 31 lines · Ard Biesheuvel
Quoted 31 lines
Quoted 31 lines · Dennis Chen
Quoted 30 lines

The commit 3400749b5a22 ("arm64/efi: refactor EFI init and runtime code for reuse by 32-bit ARM") uses pgd_alloc() to allocate space for efi_mm.pgd while not the static efi_pgd[], since this function will be called with early_initcall, which results in the pgd_cache used by pgd_alloc() has not been initialized yet, kernel will hang in this case. This patch is trying to make the pgd_cache_init() called before arm_enable_runtime_services() by changing its core_initcall to early_initcall.

Signed-off-by Dennis Chen <[email protected]>
Tested-by Sudeep Holla <[email protected]>
Cc Will Deacon <[email protected]>
Cc Catalin Marinas <[email protected]>
Cc Ard Biesheuvel <[email protected]>
Cc Sudeep Holla <[email protected]>
---
 arch/arm64/mm/pgd.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
arch/arm64/mm/pgd.c +1 -1
index cb3ba1b..859a788 100644
@@ -56,4 +56,4 @@ static int __init pgd_cache_init(void)
                                              SLAB_PANIC, NULL);
        return 0;
 }
-core_initcall(pgd_cache_init);
+early_initcall(pgd_cache_init);

[...]

Quoted 5 lines
Quoted 3 lines

Well, since arm_enable_runtime_services() is an early_initcall() itself, how are you guaranteeing the ordering between the two? Link order?

Link order.

Quoted 8 lines
Quoted 2 lines

And can you explain how this works, what guarantees it gives?

You can take a look at include/asm-generic/vmlinux.ldx.h: INIT_CALLS macro, for the init call section, early_initcall is the first chuck in the section, followed by LEVEL[0-7]. For the same level, the layout order is determined by the link order, ARCH is always precedence over the drivers. Catalin, are you giving me a kernel examination? :)

We all know how initcalls are implemented. The question is how you are going to ensure that the early_initcall() that initializes the PGD cache is always invoked before the early_initcall() that creates the UEFI runtime page tables.

And saying that the currently observed link order happens to be correct is not good enough. We need to be sure that, even if we change the link order, or switch to LTO at some point, things don't suddenly break again.

Signature · 1 line
Ard.
author
#8 lore raw

PATCH 1/1 arm64: fix kernel crash with 48-bit VA and 64KB granule

Quoted 56 lines · Ard Biesheuvel
Quoted 41 lines · Dennis Chen
Quoted 40 lines · Catalin Marinas
Quoted 32 lines · Dennis Chen
Quoted 31 lines · Ard Biesheuvel
Quoted 31 lines
Quoted 31 lines · Dennis Chen
Quoted 30 lines

The commit 3400749b5a22 ("arm64/efi: refactor EFI init and runtime code for reuse by 32-bit ARM") uses pgd_alloc() to allocate space for efi_mm.pgd while not the static efi_pgd[], since this function will be called with early_initcall, which results in the pgd_cache used by pgd_alloc() has not been initialized yet, kernel will hang in this case. This patch is trying to make the pgd_cache_init() called before arm_enable_runtime_services() by changing its core_initcall to early_initcall.

Signed-off-by Dennis Chen <[email protected]>
Tested-by Sudeep Holla <[email protected]>
Cc Will Deacon <[email protected]>
Cc Catalin Marinas <[email protected]>
Cc Ard Biesheuvel <[email protected]>
Cc Sudeep Holla <[email protected]>
---
 arch/arm64/mm/pgd.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
arch/arm64/mm/pgd.c +1 -1
index cb3ba1b..859a788 100644
@@ -56,4 +56,4 @@ static int __init pgd_cache_init(void)
                                              SLAB_PANIC, NULL);
        return 0;
 }
-core_initcall(pgd_cache_init);
+early_initcall(pgd_cache_init);

[...]

Quoted 5 lines
Quoted 3 lines

Well, since arm_enable_runtime_services() is an early_initcall() itself, how are you guaranteeing the ordering between the two? Link order?

Link order.

Quoted 8 lines
Quoted 2 lines

And can you explain how this works, what guarantees it gives?

You can take a look at include/asm-generic/vmlinux.ldx.h: INIT_CALLS macro, for the init call section, early_initcall is the first chuck in the section, followed by LEVEL[0-7]. For the same level, the layout order is determined by the link order, ARCH is always precedence over the drivers. Catalin, are you giving me a kernel examination? :)

We all know how initcalls are implemented. The question is how you are going to ensure that the early_initcall() that initializes the PGD cache is always invoked before the early_initcall() that creates the UEFI runtime page tables.

Quoted 5 lines

And saying that the currently observed link order happens to be correct is not good enough. We need to be sure that, even if we change the link order, or switch to LTO at some point, things don't suddenly break again.

Well, if the build system changes the link order, it can't make sure it will break something unexpectedly, needless to say the pgd_cache_init here at all. Do you think the kernel will change its currently link order policy? If the answer is yes, what's the benefit?

Quoted 3 lines

--
Ard.

IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.

#9 lore raw

PATCH 1/1 arm64: fix kernel crash with 48-bit VA and 64KB granule

Quoted 57 lines · Dennis Chen
Quoted 56 lines · Ard Biesheuvel
Quoted 41 lines · Dennis Chen
Quoted 40 lines · Catalin Marinas
Quoted 32 lines · Dennis Chen
Quoted 31 lines · Ard Biesheuvel
Quoted 31 lines
Quoted 31 lines · Dennis Chen
Quoted 30 lines

The commit 3400749b5a22 ("arm64/efi: refactor EFI init and runtime code for reuse by 32-bit ARM") uses pgd_alloc() to allocate space for efi_mm.pgd while not the static efi_pgd[], since this function will be called with early_initcall, which results in the pgd_cache used by pgd_alloc() has not been initialized yet, kernel will hang in this case. This patch is trying to make the pgd_cache_init() called before arm_enable_runtime_services() by changing its core_initcall to early_initcall.

Signed-off-by Dennis Chen <[email protected]>
Tested-by Sudeep Holla <[email protected]>
Cc Will Deacon <[email protected]>
Cc Catalin Marinas <[email protected]>
Cc Ard Biesheuvel <[email protected]>
Cc Sudeep Holla <[email protected]>
---
 arch/arm64/mm/pgd.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
arch/arm64/mm/pgd.c +1 -1
index cb3ba1b..859a788 100644
@@ -56,4 +56,4 @@ static int __init pgd_cache_init(void)
                                              SLAB_PANIC, NULL);
        return 0;
 }
-core_initcall(pgd_cache_init);
+early_initcall(pgd_cache_init);

[...]

Quoted 5 lines
Quoted 3 lines

Well, since arm_enable_runtime_services() is an early_initcall() itself, how are you guaranteeing the ordering between the two? Link order?

Link order.

Quoted 8 lines
Quoted 2 lines

And can you explain how this works, what guarantees it gives?

You can take a look at include/asm-generic/vmlinux.ldx.h: INIT_CALLS macro, for the init call section, early_initcall is the first chuck in the section, followed by LEVEL[0-7]. For the same level, the layout order is determined by the link order, ARCH is always precedence over the drivers. Catalin, are you giving me a kernel examination? :)

We all know how initcalls are implemented. The question is how you are going to ensure that the early_initcall() that initializes the PGD cache is always invoked before the early_initcall() that creates the UEFI runtime page tables.

Quoted 7 lines
Quoted 5 lines

And saying that the currently observed link order happens to be correct is not good enough. We need to be sure that, even if we change the link order, or switch to LTO at some point, things don't suddenly break again.

Well, if the build system changes the link order, it can't make sure it will break something unexpectedly, needless to say the pgd_cache_init here at all.

That is no excuse to introduce yet another failure mode.

Quoted 3 lines

Do you think the kernel
will change its currently link order policy? If the answer is yes, what's the benefit?

It does not matter what I think. You seem to think that the link order is set in stone, so it is you who should argue why that is a reasonable assumption.

Signature · 1 line
Ard.
author
#10 lore raw

PATCH 1/1 arm64: fix kernel crash with 48-bit VA and 64KB granule

Quoted 68 lines · Ard Biesheuvel
Quoted 57 lines · Dennis Chen
Quoted 56 lines · Ard Biesheuvel
Quoted 41 lines · Dennis Chen
Quoted 40 lines · Catalin Marinas
Quoted 32 lines · Dennis Chen
Quoted 31 lines · Ard Biesheuvel
Quoted 31 lines
Quoted 31 lines · Dennis Chen
Quoted 30 lines

The commit 3400749b5a22 ("arm64/efi: refactor EFI init and runtime code for reuse by 32-bit ARM") uses pgd_alloc() to allocate space for efi_mm.pgd while not the static efi_pgd[], since this function will be called with early_initcall, which results in the pgd_cache used by pgd_alloc() has not been initialized yet, kernel will hang in this case. This patch is trying to make the pgd_cache_init() called before arm_enable_runtime_services() by changing its core_initcall to early_initcall.

Signed-off-by Dennis Chen <[email protected]>
Tested-by Sudeep Holla <[email protected]>
Cc Will Deacon <[email protected]>
Cc Catalin Marinas <[email protected]>
Cc Ard Biesheuvel <[email protected]>
Cc Sudeep Holla <[email protected]>
---
 arch/arm64/mm/pgd.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
arch/arm64/mm/pgd.c +1 -1
index cb3ba1b..859a788 100644
@@ -56,4 +56,4 @@ static int __init pgd_cache_init(void)
                                              SLAB_PANIC, NULL);
        return 0;
 }
-core_initcall(pgd_cache_init);
+early_initcall(pgd_cache_init);

[...]

Quoted 5 lines
Quoted 3 lines

Well, since arm_enable_runtime_services() is an early_initcall() itself, how are you guaranteeing the ordering between the two? Link order?

Link order.

Quoted 8 lines
Quoted 2 lines

And can you explain how this works, what guarantees it gives?

You can take a look at include/asm-generic/vmlinux.ldx.h: INIT_CALLS macro, for the init call section, early_initcall is the first chuck in the section, followed by LEVEL[0-7]. For the same level, the layout order is determined by the link order, ARCH is always precedence over the drivers. Catalin, are you giving me a kernel examination? :)

We all know how initcalls are implemented. The question is how you are going to ensure that the early_initcall() that initializes the PGD cache is always invoked before the early_initcall() that creates the UEFI runtime page tables.

Quoted 7 lines
Quoted 5 lines

And saying that the currently observed link order happens to be correct is not good enough. We need to be sure that, even if we change the link order, or switch to LTO at some point, things don't suddenly break again.

Well, if the build system changes the link order, it can't make sure it will break something unexpectedly, needless to say the pgd_cache_init here at all.

That is no excuse to introduce yet another failure mode.

Quoted 8 lines
Quoted 3 lines

Do you think the kernel
will change its currently link order policy? If the answer is yes, what's the benefit?

It does not matter what I think. You seem to think that the link order is set in stone, so it is you who should argue why that is a reasonable assumption.

OK, If you think the link order is volatile, how can you guarantee the arm_enable_runtime_services with early_initcall always work in a volatile link order environment?

Quoted 3 lines

--
Ard.

IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.

#11 lore raw

PATCH 1/1 arm64: fix kernel crash with 48-bit VA and 64KB granule

Quoted 73 lines · Dennis Chen
Quoted 72 lines · Ard Biesheuvel
Quoted 57 lines · Dennis Chen
Quoted 56 lines · Ard Biesheuvel
Quoted 41 lines · Dennis Chen
Quoted 40 lines · Catalin Marinas
Quoted 32 lines · Dennis Chen
Quoted 31 lines · Ard Biesheuvel
Quoted 31 lines
Quoted 31 lines · Dennis Chen
Quoted 30 lines

The commit 3400749b5a22 ("arm64/efi: refactor EFI init and runtime code for reuse by 32-bit ARM") uses pgd_alloc() to allocate space for efi_mm.pgd while not the static efi_pgd[], since this function will be called with early_initcall, which results in the pgd_cache used by pgd_alloc() has not been initialized yet, kernel will hang in this case. This patch is trying to make the pgd_cache_init() called before arm_enable_runtime_services() by changing its core_initcall to early_initcall.

Signed-off-by Dennis Chen <[email protected]>
Tested-by Sudeep Holla <[email protected]>
Cc Will Deacon <[email protected]>
Cc Catalin Marinas <[email protected]>
Cc Ard Biesheuvel <[email protected]>
Cc Sudeep Holla <[email protected]>
---
 arch/arm64/mm/pgd.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
arch/arm64/mm/pgd.c +1 -1
index cb3ba1b..859a788 100644
@@ -56,4 +56,4 @@ static int __init pgd_cache_init(void)
                                              SLAB_PANIC, NULL);
        return 0;
 }
-core_initcall(pgd_cache_init);
+early_initcall(pgd_cache_init);

[...]

Quoted 5 lines
Quoted 3 lines

Well, since arm_enable_runtime_services() is an early_initcall() itself, how are you guaranteeing the ordering between the two? Link order?

Link order.

Quoted 8 lines
Quoted 2 lines

And can you explain how this works, what guarantees it gives?

You can take a look at include/asm-generic/vmlinux.ldx.h: INIT_CALLS macro, for the init call section, early_initcall is the first chuck in the section, followed by LEVEL[0-7]. For the same level, the layout order is determined by the link order, ARCH is always precedence over the drivers. Catalin, are you giving me a kernel examination? :)

We all know how initcalls are implemented. The question is how you are going to ensure that the early_initcall() that initializes the PGD cache is always invoked before the early_initcall() that creates the UEFI runtime page tables.

Quoted 7 lines
Quoted 5 lines

And saying that the currently observed link order happens to be correct is not good enough. We need to be sure that, even if we change the link order, or switch to LTO at some point, things don't suddenly break again.

Well, if the build system changes the link order, it can't make sure it will break something unexpectedly, needless to say the pgd_cache_init here at all.

That is no excuse to introduce yet another failure mode.

Quoted 3 lines

Do you think the kernel
will change its currently link order policy? If the answer is yes, what's the benefit?

Quoted 7 lines
Quoted 4 lines

It does not matter what I think. You seem to think that the link order is set in stone, so it is you who should argue why that is a reasonable assumption.

OK, If you think the link order is volatile, how can you guarantee the arm_enable_runtime_services with early_initcall always work in a volatile link order environment?

By not relying on other early_initcalls

author
#12 lore raw

PATCH 1/1 arm64: fix kernel crash with 48-bit VA and 64KB granule

Quoted 78 lines · Ard Biesheuvel
Quoted 77 lines · Dennis Chen
Quoted 76 lines · Ard Biesheuvel
Quoted 57 lines · Dennis Chen
Quoted 56 lines · Ard Biesheuvel
Quoted 41 lines · Dennis Chen
Quoted 40 lines · Catalin Marinas
Quoted 32 lines · Dennis Chen
Quoted 31 lines · Ard Biesheuvel
Quoted 31 lines
Quoted 31 lines · Dennis Chen
Quoted 30 lines

The commit 3400749b5a22 ("arm64/efi: refactor EFI init and runtime code for reuse by 32-bit ARM") uses pgd_alloc() to allocate space for efi_mm.pgd while not the static efi_pgd[], since this function will be called with early_initcall, which results in the pgd_cache used by pgd_alloc() has not been initialized yet, kernel will hang in this case. This patch is trying to make the pgd_cache_init() called before arm_enable_runtime_services() by changing its core_initcall to early_initcall.

Signed-off-by Dennis Chen <[email protected]>
Tested-by Sudeep Holla <[email protected]>
Cc Will Deacon <[email protected]>
Cc Catalin Marinas <[email protected]>
Cc Ard Biesheuvel <[email protected]>
Cc Sudeep Holla <[email protected]>
---
 arch/arm64/mm/pgd.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
arch/arm64/mm/pgd.c +1 -1
index cb3ba1b..859a788 100644
@@ -56,4 +56,4 @@ static int __init pgd_cache_init(void)
                                              SLAB_PANIC, NULL);
        return 0;
 }
-core_initcall(pgd_cache_init);
+early_initcall(pgd_cache_init);

[...]

Quoted 5 lines
Quoted 3 lines

Well, since arm_enable_runtime_services() is an early_initcall() itself, how are you guaranteeing the ordering between the two? Link order?

Link order.

Quoted 8 lines
Quoted 2 lines

And can you explain how this works, what guarantees it gives?

You can take a look at include/asm-generic/vmlinux.ldx.h: INIT_CALLS macro, for the init call section, early_initcall is the first chuck in the section, followed by LEVEL[0-7]. For the same level, the layout order is determined by the link order, ARCH is always precedence over the drivers. Catalin, are you giving me a kernel examination? :)

We all know how initcalls are implemented. The question is how you are going to ensure that the early_initcall() that initializes the PGD cache is always invoked before the early_initcall() that creates the UEFI runtime page tables.

Quoted 7 lines
Quoted 5 lines

And saying that the currently observed link order happens to be correct is not good enough. We need to be sure that, even if we change the link order, or switch to LTO at some point, things don't suddenly break again.

Well, if the build system changes the link order, it can't make sure it will break something unexpectedly, needless to say the pgd_cache_init here at all.

That is no excuse to introduce yet another failure mode.

Quoted 3 lines

Do you think the kernel
will change its currently link order policy? If the answer is yes, what's the benefit?

It does not matter what I think. You seem to think that the link order is set in stone, so it is you who should argue why that is a reasonable assumption.

Quoted 6 lines
Quoted 3 lines

OK, If you think the link order is volatile, how can you guarantee the arm_enable_runtime_services with early_initcall always work in a volatile link order environment?

By not relying on other early_initcalls

You're limiting the scope of the link order
IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.

#13 lore raw

PATCH 1/1 arm64: fix kernel crash with 48-bit VA and 64KB granule

Dennis,

Quoted 17 lines · Dennis Chen
Quoted 16 lines · Ard Biesheuvel
Quoted 15 lines · Dennis Chen
Quoted 14 lines · Ard Biesheuvel
Quoted 2 lines · Dennis Chen

Well, if the build system changes the link order, it can't make sure it will break something unexpectedly, needless to say the pgd_cache_init here at all.

That is no excuse to introduce yet another failure mode.

Quoted 3 lines

Do you think the kernel
will change its currently link order policy? If the answer is yes, what's the benefit?

It does not matter what I think. You seem to think that the link order is set in stone, so it is you who should argue why that is a reasonable assumption.

Quoted 7 lines
Quoted 6 lines
Quoted 3 lines

OK, If you think the link order is volatile, how can you guarantee the arm_enable_runtime_services with early_initcall always work in a volatile link order environment?

By not relying on other early_initcalls

You're limiting the scope of the link order

Please stop this mindless bickering. It's a waste of everybody's time and we have a kernel to test.

Will

#14 lore raw

PATCH 1/1 arm64: fix kernel crash with 48-bit VA and 64KB granule

In reply to Ard Biesheuvel
Quoted 37 lines · Ard Biesheuvel
Quoted 25 lines · Dennis Chen

The commit 3400749b5a22 ("arm64/efi: refactor EFI init and runtime code for reuse by 32-bit ARM") uses pgd_alloc() to allocate space for efi_mm.pgd while not the static efi_pgd[], since this function will be called with early_initcall, which results in the pgd_cache used by pgd_alloc() has not been initialized yet, kernel will hang in this case. This patch is trying to make the pgd_cache_init() called before arm_enable_runtime_services() by changing its core_initcall to early_initcall.

Signed-off-by Dennis Chen <[email protected]>
Tested-by Sudeep Holla <[email protected]>
Cc Will Deacon <[email protected]>
Cc Catalin Marinas <[email protected]>
Cc Ard Biesheuvel <[email protected]>
Cc Sudeep Holla <[email protected]>
---
 arch/arm64/mm/pgd.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
arch/arm64/mm/pgd.c +0 -0
index cb3ba1b..859a788 100644

arch/arm64/mm/pgd.c static int __init pgd_cache_init(void) · line 59

Quoted 5 lines
arch/arm64/mm/pgd.c +1 -1
@@ -56,4 +56,4 @@ static int __init pgd_cache_init(void)
                                              SLAB_PANIC, NULL);
        return 0;
 }
-core_initcall(pgd_cache_init);
+early_initcall(pgd_cache_init);

Hello all,

Apologies for not spotting this before sending it out.

Quoted 8 lines

The issue only occurs when PGD_SIZE != PAGE_SIZE, which is why I did not see it myself. I only tested with 4k/39-bits and 64k/42-bits IIRC, since the series this is part of primarily concerns ARM not arm64.

Anyway, shuffling init ordering is my least favorite way of fixing bugs. Since only ARM requires pgd_alloc() (for the kernel mappings), we could also simply factor out pgd_alloc() into efi_pgd_alloc(), and define it to mean '__get_free_page(PGALLOC_GFP)' on arm64.

Or we could put the pgd_cache initialisation in pgtable_cache_init, which sounds like the right place for it anyway.

Curious, but why are our EFI runtime services initialised off the back of initcalls, whereas on x86 the initialisation is driven directly from init/main.c? I'm not saying it should be changed, it just looks odd having the two paths.

Will

#15 lore raw

PATCH 1/1 arm64: fix kernel crash with 48-bit VA and 64KB granule

Quoted 42 lines · Will Deacon
Quoted 41 lines · Ard Biesheuvel
Quoted 25 lines · Dennis Chen

The commit 3400749b5a22 ("arm64/efi: refactor EFI init and runtime code for reuse by 32-bit ARM") uses pgd_alloc() to allocate space for efi_mm.pgd while not the static efi_pgd[], since this function will be called with early_initcall, which results in the pgd_cache used by pgd_alloc() has not been initialized yet, kernel will hang in this case. This patch is trying to make the pgd_cache_init() called before arm_enable_runtime_services() by changing its core_initcall to early_initcall.

Signed-off-by Dennis Chen <[email protected]>
Tested-by Sudeep Holla <[email protected]>
Cc Will Deacon <[email protected]>
Cc Catalin Marinas <[email protected]>
Cc Ard Biesheuvel <[email protected]>
Cc Sudeep Holla <[email protected]>
---
 arch/arm64/mm/pgd.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
arch/arm64/mm/pgd.c +0 -0
index cb3ba1b..859a788 100644

arch/arm64/mm/pgd.c static int __init pgd_cache_init(void) · line 59

Quoted 5 lines
arch/arm64/mm/pgd.c +1 -1
@@ -56,4 +56,4 @@ static int __init pgd_cache_init(void)
                                              SLAB_PANIC, NULL);
        return 0;
 }
-core_initcall(pgd_cache_init);
+early_initcall(pgd_cache_init);

Hello all,

Apologies for not spotting this before sending it out.

The issue only occurs when PGD_SIZE != PAGE_SIZE, which is why I did not see it myself. I only tested with 4k/39-bits and 64k/42-bits IIRC, since the series this is part of primarily concerns ARM not arm64.

Quoted 8 lines
Quoted 4 lines

Anyway, shuffling init ordering is my least favorite way of fixing bugs. Since only ARM requires pgd_alloc() (for the kernel mappings), we could also simply factor out pgd_alloc() into efi_pgd_alloc(), and define it to mean '__get_free_page(PGALLOC_GFP)' on arm64.

Or we could put the pgd_cache initialisation in pgtable_cache_init, which sounds like the right place for it anyway.

Indeed.

Quoted 5 lines

Curious, but why are our EFI runtime services initialised off the back of initcalls, whereas on x86 the initialisation is driven directly from init/main.c? I'm not saying it should be changed, it just looks odd having the two paths.

Simply because we tried not to pollute all core files with explicit EFI calls. The EFI init bits are called in line by setup_arch(), because they are needed so early, so there we had no choice. The runtime services bits are only needed much later, so there we used an initcall because we could.

Signature · 1 line
Ard.
#16 lore raw

PATCH 1/1 arm64: fix kernel crash with 48-bit VA and 64KB granule

Quoted 11 lines · Ard Biesheuvel
Quoted 10 lines · Will Deacon
Quoted 8 lines · Ard Biesheuvel

The issue only occurs when PGD_SIZE != PAGE_SIZE, which is why I did not see it myself. I only tested with 4k/39-bits and 64k/42-bits IIRC, since the series this is part of primarily concerns ARM not arm64.

Anyway, shuffling init ordering is my least favorite way of fixing bugs. Since only ARM requires pgd_alloc() (for the kernel mappings), we could also simply factor out pgd_alloc() into efi_pgd_alloc(), and define it to mean '__get_free_page(PGALLOC_GFP)' on arm64.

Quoted 5 lines
Quoted 3 lines

Or we could put the pgd_cache initialisation in pgtable_cache_init, which sounds like the right place for it anyway.

Indeed.

Patch below. Seems to do the job on my Juno.

Will

--->8

Quoted 1 line

From 39b5be9b4233a9f212b98242bddf008f379b5122 Mon Sep 17 00:00:00 2001

From: Will Deacon <[email protected]>
Date: Tue, 5 Jan 2016 15:36:59 +0000
Subject: [PATCH] arm64: mm: move pgd_cache initialisation to
pgtable_cache_init

Initialising the suppport for EFI runtime services requires us to allocate a pgd off the back of an early_initcall. On systems where the PGD_SIZE is smaller than PAGE_SIZE (e.g. 64k pages and 48-bit VA), the pgd_cache isn't initialised at this stage, and we panic with a NULL dereference during boot:

  Unable to handle kernel NULL pointer dereference at virtual address 00000000

  __create_mapping.isra.5+0x84/0x350
  create_pgd_mapping+0x20/0x28
  efi_create_mapping+0x5c/0x6c
  arm_enable_runtime_services+0x154/0x1e4
  do_one_initcall+0x8c/0x190
  kernel_init_freeable+0x84/0x1ec
  kernel_init+0x10/0xe0
  ret_from_fork+0x10/0x50

This patch fixes the problem by initialising the pgd_cache earlier, in the pgtable_cache_init callback, which sounds suspiciously like what it was intended for.

Reported-by Dennis Chen <[email protected]>
Signed-off-by Will Deacon <[email protected]>
---
 arch/arm64/include/asm/pgtable.h |  3 ++-
 arch/arm64/mm/pgd.c              | 12 ++++++------
 2 files changed, 8 insertions(+), 7 deletions(-)
arch/arm64/include/asm/pgtable.h +2 -1
index 35a318c2fd87..a87e964d2791 100644
@@ -676,7 +676,8 @@ extern int kern_addr_valid(unsigned long addr);
 
 #include <asm-generic/pgtable.h>
 
-#define pgtable_cache_init() do { } while (0)
+void pgd_cache_init(void);
+#define pgtable_cache_init	pgd_cache_init
 
 /*
  * On AArch64, the cache coherency is handled via the set_pte_at() function.
arch/arm64/mm/pgd.c +6 -6
index cb3ba1b812e7..ae11d4e03d0e 100644
@@ -46,14 +46,14 @@ void pgd_free(struct mm_struct *mm, pgd_t *pgd)
 		kmem_cache_free(pgd_cache, pgd);
 }
 
-static int __init pgd_cache_init(void)
+void __init pgd_cache_init(void)
 {
+	if (PGD_SIZE == PAGE_SIZE)
+		return;
+
 	/*
 	 * Naturally aligned pgds required by the architecture.
 	 */
-	if (PGD_SIZE != PAGE_SIZE)
-		pgd_cache = kmem_cache_create("pgd_cache", PGD_SIZE, PGD_SIZE,
-					      SLAB_PANIC, NULL);
-	return 0;
+	pgd_cache = kmem_cache_create("pgd_cache", PGD_SIZE, PGD_SIZE,
+				      SLAB_PANIC, NULL);
 }
-core_initcall(pgd_cache_init);
Signature · 1 line
2.1.4
author
#17 lore raw

PATCH 1/1 arm64: fix kernel crash with 48-bit VA and 64KB granule

Quoted 77 lines · Will Deacon
Quoted 11 lines · Ard Biesheuvel
Quoted 10 lines · Will Deacon
Quoted 8 lines · Ard Biesheuvel

The issue only occurs when PGD_SIZE != PAGE_SIZE, which is why I did not see it myself. I only tested with 4k/39-bits and 64k/42-bits IIRC, since the series this is part of primarily concerns ARM not arm64.

Anyway, shuffling init ordering is my least favorite way of fixing bugs. Since only ARM requires pgd_alloc() (for the kernel mappings), we could also simply factor out pgd_alloc() into efi_pgd_alloc(), and define it to mean '__get_free_page(PGALLOC_GFP)' on arm64.

Quoted 5 lines
Quoted 3 lines

Or we could put the pgd_cache initialisation in pgtable_cache_init, which sounds like the right place for it anyway.

Indeed.

Patch below. Seems to do the job on my Juno.

Will

--->8

From 39b5be9b4233a9f212b98242bddf008f379b5122 Mon Sep 17 00:00:00 2001
From: Will Deacon <[email protected]>
Date: Tue, 5 Jan 2016 15:36:59 +0000
Subject: [PATCH] arm64: mm: move pgd_cache initialisation to
pgtable_cache_init

Initialising the suppport for EFI runtime services requires us to allocate a pgd off the back of an early_initcall. On systems where the PGD_SIZE is smaller than PAGE_SIZE (e.g. 64k pages and 48-bit VA), the pgd_cache isn't initialised at this stage, and we panic with a NULL dereference during boot:

  Unable to handle kernel NULL pointer dereference at virtual address 00000000

  __create_mapping.isra.5+0x84/0x350
  create_pgd_mapping+0x20/0x28
  efi_create_mapping+0x5c/0x6c
  arm_enable_runtime_services+0x154/0x1e4
  do_one_initcall+0x8c/0x190
  kernel_init_freeable+0x84/0x1ec
  kernel_init+0x10/0xe0
  ret_from_fork+0x10/0x50

This patch fixes the problem by initialising the pgd_cache earlier, in the pgtable_cache_init callback, which sounds suspiciously like what it was intended for.

Reported-by Dennis Chen <[email protected]>
Signed-off-by Will Deacon <[email protected]>
---
 arch/arm64/include/asm/pgtable.h |  3 ++-
 arch/arm64/mm/pgd.c              | 12 ++++++------
 2 files changed, 8 insertions(+), 7 deletions(-)
arch/arm64/include/asm/pgtable.h +2 -1
index 35a318c2fd87..a87e964d2791 100644
@@ -676,7 +676,8 @@ extern int kern_addr_valid(unsigned long addr);
 
 #include <asm-generic/pgtable.h>
 
-#define pgtable_cache_init() do { } while (0)
+void pgd_cache_init(void);
+#define pgtable_cache_init   pgd_cache_init
 
 /*
  * On AArch64, the cache coherency is handled via the set_pte_at() function.
arch/arm64/mm/pgd.c +0 -0
index cb3ba1b812e7..ae11d4e03d0e 100644

arch/arm64/mm/pgd.c void pgd_free(struct mm_struct *mm, pgd_t *pgd) · line 53

Quoted 9 lines
arch/arm64/mm/pgd.c +4 -1
@@ -46,14 +46,14 @@ void pgd_free(struct mm_struct *mm, pgd_t *pgd)
              kmem_cache_free(pgd_cache, pgd);
 }
 
-static int __init pgd_cache_init(void)
+void __init pgd_cache_init(void)
 {
+     if (PGD_SIZE == PAGE_SIZE)
+             return;
+

It doesn't need to add a new if statement here.

arch/arm64/mm/pgd.c void pgd_free(struct mm_struct *mm, pgd_t *pgd) · line 56

Quoted 6 lines
arch/arm64/mm/pgd.c +0 -3
      /*
       * Naturally aligned pgds required by the architecture.
       */
-     if (PGD_SIZE != PAGE_SIZE)
-             pgd_cache = kmem_cache_create("pgd_cache", PGD_SIZE, PGD_SIZE,
-                                           SLAB_PANIC, NULL);

The deleted code here is more better, seems we don't need to change it here

arch/arm64/mm/pgd.c void pgd_free(struct mm_struct *mm, pgd_t *pgd) · line 57

Quoted 1 line
arch/arm64/mm/pgd.c +0 -1
-     return 0;

Nice.

arch/arm64/mm/pgd.c void pgd_free(struct mm_struct *mm, pgd_t *pgd) · line 59

Quoted 5 lines
arch/arm64/mm/pgd.c +2 -1
+     pgd_cache = kmem_cache_create("pgd_cache", PGD_SIZE, PGD_SIZE,
+                                   SLAB_PANIC, NULL);
 
 }
-core_initcall(pgd_cache_init);

The new patch is reasonable for me.

Quoted 3 lines

--
2.1.4

IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.