2022-03-25 18:54:02

by Kefeng Wang

[permalink] [raw]
Subject: [PATCH 1/2] arm64: mm: Do not defer reserve_crashkernel() if only ZONE_DMA32

The kernel could be benifit due to BLOCK_MAPPINGS, see commit
031495635b46 ("arm64: Do not defer reserve_crashkernel() for
platforms with no DMA memory zones"), if there is only with
ZONE_DMA32, we could set arm64_dma_phys_limit to max_zone_phys(32)
earlier in arm64_memblock_init(), then we will benifit too.

Cc: Vijay Balakrishna <[email protected]>
Cc: Pasha Tatashin <[email protected]>
Cc: Will Deacon <[email protected]>
Signed-off-by: Kefeng Wang <[email protected]>
---
arch/arm64/mm/init.c | 18 ++++++++++--------
arch/arm64/mm/mmu.c | 6 ++----
2 files changed, 12 insertions(+), 12 deletions(-)

diff --git a/arch/arm64/mm/init.c b/arch/arm64/mm/init.c
index 8ac25f19084e..9dded8779d72 100644
--- a/arch/arm64/mm/init.c
+++ b/arch/arm64/mm/init.c
@@ -157,14 +157,14 @@ static phys_addr_t __init max_zone_phys(unsigned int zone_bits)
return min(zone_mask, memblock_end_of_DRAM() - 1) + 1;
}

+phys_addr_t __ro_after_init dma32_phys_limit;
static void __init zone_sizes_init(unsigned long min, unsigned long max)
{
unsigned long max_zone_pfns[MAX_NR_ZONES] = {0};
- unsigned int __maybe_unused acpi_zone_dma_bits;
- unsigned int __maybe_unused dt_zone_dma_bits;
- phys_addr_t __maybe_unused dma32_phys_limit = max_zone_phys(32);
-
#ifdef CONFIG_ZONE_DMA
+ unsigned int acpi_zone_dma_bits;
+ unsigned int dt_zone_dma_bits;
+
acpi_zone_dma_bits = fls64(acpi_iort_dma_get_max_cpu_address());
dt_zone_dma_bits = fls64(of_dma_get_max_cpu_address(NULL));
zone_dma_bits = min3(32U, dt_zone_dma_bits, acpi_zone_dma_bits);
@@ -173,8 +173,6 @@ static void __init zone_sizes_init(unsigned long min, unsigned long max)
#endif
#ifdef CONFIG_ZONE_DMA32
max_zone_pfns[ZONE_DMA32] = PFN_DOWN(dma32_phys_limit);
- if (!arm64_dma_phys_limit)
- arm64_dma_phys_limit = dma32_phys_limit;
#endif
max_zone_pfns[ZONE_NORMAL] = max;

@@ -336,8 +334,12 @@ void __init arm64_memblock_init(void)

early_init_fdt_scan_reserved_mem();

- if (!IS_ENABLED(CONFIG_ZONE_DMA) && !IS_ENABLED(CONFIG_ZONE_DMA32))
+ dma32_phys_limit = max_zone_phys(32);
+ if (!IS_ENABLED(CONFIG_ZONE_DMA)) {
+ if (IS_ENABLED(CONFIG_ZONE_DMA32))
+ arm64_dma_phys_limit = dma32_phys_limit;
reserve_crashkernel();
+ }

high_memory = __va(memblock_end_of_DRAM() - 1) + 1;
}
@@ -385,7 +387,7 @@ void __init bootmem_init(void)
* request_standard_resources() depends on crashkernel's memory being
* reserved, so do it here.
*/
- if (IS_ENABLED(CONFIG_ZONE_DMA) || IS_ENABLED(CONFIG_ZONE_DMA32))
+ if (IS_ENABLED(CONFIG_ZONE_DMA))
reserve_crashkernel();

memblock_dump_all();
diff --git a/arch/arm64/mm/mmu.c b/arch/arm64/mm/mmu.c
index 626ec32873c6..23734481318a 100644
--- a/arch/arm64/mm/mmu.c
+++ b/arch/arm64/mm/mmu.c
@@ -529,8 +529,7 @@ static void __init map_mem(pgd_t *pgdp)

#ifdef CONFIG_KEXEC_CORE
if (crash_mem_map) {
- if (IS_ENABLED(CONFIG_ZONE_DMA) ||
- IS_ENABLED(CONFIG_ZONE_DMA32))
+ if (IS_ENABLED(CONFIG_ZONE_DMA))
flags |= NO_BLOCK_MAPPINGS | NO_CONT_MAPPINGS;
else if (crashk_res.end)
memblock_mark_nomap(crashk_res.start,
@@ -571,8 +570,7 @@ static void __init map_mem(pgd_t *pgdp)
* through /sys/kernel/kexec_crash_size interface.
*/
#ifdef CONFIG_KEXEC_CORE
- if (crash_mem_map &&
- !IS_ENABLED(CONFIG_ZONE_DMA) && !IS_ENABLED(CONFIG_ZONE_DMA32)) {
+ if (crash_mem_map && !IS_ENABLED(CONFIG_ZONE_DMA)) {
if (crashk_res.end) {
__map_memblock(pgdp, crashk_res.start,
crashk_res.end + 1,
--
2.26.2


2022-03-25 19:26:21

by Kefeng Wang

[permalink] [raw]
Subject: [PATCH 2/2] arm64: mm: Cleanup useless parameters in zone_sizes_init()

Directly use max_pfn for max and no one use min, kill them.

Signed-off-by: Kefeng Wang <[email protected]>
---
arch/arm64/mm/init.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/arch/arm64/mm/init.c b/arch/arm64/mm/init.c
index 9dded8779d72..eacb38c2d225 100644
--- a/arch/arm64/mm/init.c
+++ b/arch/arm64/mm/init.c
@@ -158,7 +158,7 @@ static phys_addr_t __init max_zone_phys(unsigned int zone_bits)
}

phys_addr_t __ro_after_init dma32_phys_limit;
-static void __init zone_sizes_init(unsigned long min, unsigned long max)
+static void __init zone_sizes_init(void)
{
unsigned long max_zone_pfns[MAX_NR_ZONES] = {0};
#ifdef CONFIG_ZONE_DMA
@@ -174,7 +174,7 @@ static void __init zone_sizes_init(unsigned long min, unsigned long max)
#ifdef CONFIG_ZONE_DMA32
max_zone_pfns[ZONE_DMA32] = PFN_DOWN(dma32_phys_limit);
#endif
- max_zone_pfns[ZONE_NORMAL] = max;
+ max_zone_pfns[ZONE_NORMAL] = max_pfn;

free_area_init(max_zone_pfns);
}
@@ -376,7 +376,7 @@ void __init bootmem_init(void)
* done after the fixed reservations
*/
sparse_init();
- zone_sizes_init(min, max);
+ zone_sizes_init();

/*
* Reserve the CMA area after arm64_dma_phys_limit was initialised.
--
2.26.2

2022-03-30 09:44:31

by Florian Fainelli

[permalink] [raw]
Subject: Re: [PATCH 1/2] arm64: mm: Do not defer reserve_crashkernel() if only ZONE_DMA32



On 3/24/2022 10:53 PM, Kefeng Wang wrote:
> The kernel could be benifit due to BLOCK_MAPPINGS, see commit
> 031495635b46 ("arm64: Do not defer reserve_crashkernel() for
> platforms with no DMA memory zones"), if there is only with
> ZONE_DMA32, we could set arm64_dma_phys_limit to max_zone_phys(32)
> earlier in arm64_memblock_init(), then we will benifit too.
>
> Cc: Vijay Balakrishna <[email protected]>
> Cc: Pasha Tatashin <[email protected]>
> Cc: Will Deacon <[email protected]>
> Signed-off-by: Kefeng Wang <[email protected]>
> ---
> arch/arm64/mm/init.c | 18 ++++++++++--------
> arch/arm64/mm/mmu.c | 6 ++----
> 2 files changed, 12 insertions(+), 12 deletions(-)
>
> diff --git a/arch/arm64/mm/init.c b/arch/arm64/mm/init.c
> index 8ac25f19084e..9dded8779d72 100644
> --- a/arch/arm64/mm/init.c
> +++ b/arch/arm64/mm/init.c
> @@ -157,14 +157,14 @@ static phys_addr_t __init max_zone_phys(unsigned int zone_bits)
> return min(zone_mask, memblock_end_of_DRAM() - 1) + 1;
> }
>
> +phys_addr_t __ro_after_init dma32_phys_limit;

This variable should still be scoped to this file, it is not used
outside of arch/arm64/mm/init.c.
--
Florian

2022-03-30 15:29:42

by Kefeng Wang

[permalink] [raw]
Subject: Re: [PATCH 1/2] arm64: mm: Do not defer reserve_crashkernel() if only ZONE_DMA32


On 2022/3/30 6:29, Vijay Balakrishna wrote:
>
>
> On 3/24/2022 10:53 PM, Kefeng Wang wrote:
>> The kernel could be benifit due to BLOCK_MAPPINGS, see commit
>> 031495635b46 ("arm64: Do not defer reserve_crashkernel() for
>> platforms with no DMA memory zones"), if there is only with
>> ZONE_DMA32, we could set arm64_dma_phys_limit to max_zone_phys(32)
>> earlier in arm64_memblock_init(), then we will benifit too.
>
> Thanks for noticing platforms with just ZONE_DMA32 config can also
> benefit BLOCK_MAPPINGS.  I assume you have access to one where you
> notice the difference with proposed changes and able to test.  I did
> test proposed changes on SoC we use with IOMMU (no ZONE_DMA configs
> enabled).
>
> Nits --
> - benifit -> benefit
> - consider making commit message clear, "then we will.." seems you are
> referring to platforms with just ZONE_DMA32 config enabled
> - to reflect new change consider updating comment added in commit
> 031495635b46
>
One more question, could we add new cmdline(eg, dma_force_32bit) to let
the arm64 using

32bit dma  by default?  I think most platforms are not with small DMA
bit(only found 20bit in Raspberry Pi 4?),

Here is a draft(based on this patch) to do it, if the kernel boot with
this cmdline and even ZONE_DMA

is enabled, the kernel allows linear creation of block mapping, not sure
this is the right way, any suggestion? Thanks.

diff --git a/arch/arm64/include/asm/processor.h
b/arch/arm64/include/asm/processor.h
index 73e38d9a540c..8febed26aeed 100644
--- a/arch/arm64/include/asm/processor.h
+++ b/arch/arm64/include/asm/processor.h
@@ -100,6 +100,7 @@
                                        base)
 #endif /* CONFIG_ARM64_FORCE_52BIT */

+extern bool arm64_dma_force_32bit;
 extern phys_addr_t arm64_dma_phys_limit;
 #define ARCH_LOW_ADDRESS_LIMIT (arm64_dma_phys_limit - 1)

diff --git a/arch/arm64/mm/init.c b/arch/arm64/mm/init.c
index d6f64819ef89..86aab5db735b 100644
--- a/arch/arm64/mm/init.c
+++ b/arch/arm64/mm/init.c
@@ -92,6 +92,16 @@ phys_addr_t __ro_after_init arm64_dma_phys_limit;
 phys_addr_t __ro_after_init arm64_dma_phys_limit = PHYS_MASK + 1;
 #endif

+bool __ro_after_init arm64_dma_force_32bit;
+static int __init arm64_dma_force_32bit_setup(char *p)
+{
+       arm64_dma_force_32bit = true;
+       zone_dma_bits = 32;
+
+       return 0;
+}
+early_param("dma_force_32bit", arm64_dma_force_32bit_setup);
+
 /*
  * reserve_crashkernel() - reserves memory for crash kernel
  *
@@ -165,10 +175,11 @@ static void __init zone_sizes_init(void)
 #ifdef CONFIG_ZONE_DMA
        unsigned int acpi_zone_dma_bits;
        unsigned int dt_zone_dma_bits;
-
-       acpi_zone_dma_bits = fls64(acpi_iort_dma_get_max_cpu_address());
-       dt_zone_dma_bits = fls64(of_dma_get_max_cpu_address(NULL));
-       zone_dma_bits = min3(32U, dt_zone_dma_bits, acpi_zone_dma_bits);
+       if (!arm64_dma_force_32bit) {
+               acpi_zone_dma_bits =
fls64(acpi_iort_dma_get_max_cpu_address());
+               dt_zone_dma_bits = fls64(of_dma_get_max_cpu_address(NULL));
+               zone_dma_bits = min3(32U, dt_zone_dma_bits,
acpi_zone_dma_bits);
+       }
        arm64_dma_phys_limit = max_zone_phys(zone_dma_bits);
        max_zone_pfns[ZONE_DMA] = PFN_DOWN(arm64_dma_phys_limit);
 #endif
@@ -336,7 +347,7 @@ void __init arm64_memblock_init(void)
        early_init_fdt_scan_reserved_mem();

        dma32_phys_limit = max_zone_phys(32);
-       if (!IS_ENABLED(CONFIG_ZONE_DMA)) {
+       if (!IS_ENABLED(CONFIG_ZONE_DMA) || arm64_dma_force_32bit) {
                if (IS_ENABLED(CONFIG_ZONE_DMA32))
                        arm64_dma_phys_limit = dma32_phys_limit;
                reserve_crashkernel();
@@ -388,7 +399,7 @@ void __init bootmem_init(void)
         * request_standard_resources() depends on crashkernel's memory
being
         * reserved, so do it here.
         */
-       if (IS_ENABLED(CONFIG_ZONE_DMA))
+       if (IS_ENABLED(CONFIG_ZONE_DMA) && !arm64_dma_force_32bit)
                reserve_crashkernel();

        memblock_dump_all();
diff --git a/arch/arm64/mm/mmu.c b/arch/arm64/mm/mmu.c
index 23734481318a..9431ad581927 100644
--- a/arch/arm64/mm/mmu.c
+++ b/arch/arm64/mm/mmu.c
@@ -529,7 +529,7 @@ static void __init map_mem(pgd_t *pgdp)

 #ifdef CONFIG_KEXEC_CORE
        if (crash_mem_map) {
-               if (IS_ENABLED(CONFIG_ZONE_DMA))
+               if (IS_ENABLED(CONFIG_ZONE_DMA) && !arm64_dma_force_32bit)
                        flags |= NO_BLOCK_MAPPINGS | NO_CONT_MAPPINGS;
                else if (crashk_res.end)
                        memblock_mark_nomap(crashk_res.start,
@@ -570,7 +570,7 @@ static void __init map_mem(pgd_t *pgdp)
         * through /sys/kernel/kexec_crash_size interface.
         */
 #ifdef CONFIG_KEXEC_CORE
-       if (crash_mem_map && !IS_ENABLED(CONFIG_ZONE_DMA)) {
+       if (crash_mem_map && (!IS_ENABLED(CONFIG_ZONE_DMA) ||
arm64_dma_force_32bit)) {
                if (crashk_res.end) {
                        __map_memblock(pgdp, crashk_res.start,
                                       crashk_res.end + 1,


2022-03-31 04:50:22

by Vijay Balakrishna

[permalink] [raw]
Subject: Re: [PATCH 1/2] arm64: mm: Do not defer reserve_crashkernel() if only ZONE_DMA32



On 3/24/2022 10:53 PM, Kefeng Wang wrote:
> The kernel could be benifit due to BLOCK_MAPPINGS, see commit
> 031495635b46 ("arm64: Do not defer reserve_crashkernel() for
> platforms with no DMA memory zones"), if there is only with
> ZONE_DMA32, we could set arm64_dma_phys_limit to max_zone_phys(32)
> earlier in arm64_memblock_init(), then we will benifit too.

Thanks for noticing platforms with just ZONE_DMA32 config can also
benefit BLOCK_MAPPINGS. I assume you have access to one where you
notice the difference with proposed changes and able to test. I did
test proposed changes on SoC we use with IOMMU (no ZONE_DMA configs
enabled).

Nits --
- benifit -> benefit
- consider making commit message clear, "then we will.." seems you are
referring to platforms with just ZONE_DMA32 config enabled
- to reflect new change consider updating comment added in commit
031495635b46

>
> Cc: Vijay Balakrishna <[email protected]>
> Cc: Pasha Tatashin <[email protected]>
> Cc: Will Deacon <[email protected]>
> Signed-off-by: Kefeng Wang <[email protected]>

Reviewed-by: Vijay Balakrishna <[email protected]>

> ---
> arch/arm64/mm/init.c | 18 ++++++++++--------
> arch/arm64/mm/mmu.c | 6 ++----
> 2 files changed, 12 insertions(+), 12 deletions(-)
>
> diff --git a/arch/arm64/mm/init.c b/arch/arm64/mm/init.c
> index 8ac25f19084e..9dded8779d72 100644
> --- a/arch/arm64/mm/init.c
> +++ b/arch/arm64/mm/init.c
> @@ -157,14 +157,14 @@ static phys_addr_t __init max_zone_phys(unsigned int zone_bits)
> return min(zone_mask, memblock_end_of_DRAM() - 1) + 1;
> }
>
> +phys_addr_t __ro_after_init dma32_phys_limit;
> static void __init zone_sizes_init(unsigned long min, unsigned long max)
> {
> unsigned long max_zone_pfns[MAX_NR_ZONES] = {0};
> - unsigned int __maybe_unused acpi_zone_dma_bits;
> - unsigned int __maybe_unused dt_zone_dma_bits;
> - phys_addr_t __maybe_unused dma32_phys_limit = max_zone_phys(32);
> -
> #ifdef CONFIG_ZONE_DMA
> + unsigned int acpi_zone_dma_bits;
> + unsigned int dt_zone_dma_bits;
> +
> acpi_zone_dma_bits = fls64(acpi_iort_dma_get_max_cpu_address());
> dt_zone_dma_bits = fls64(of_dma_get_max_cpu_address(NULL));
> zone_dma_bits = min3(32U, dt_zone_dma_bits, acpi_zone_dma_bits);
> @@ -173,8 +173,6 @@ static void __init zone_sizes_init(unsigned long min, unsigned long max)
> #endif
> #ifdef CONFIG_ZONE_DMA32
> max_zone_pfns[ZONE_DMA32] = PFN_DOWN(dma32_phys_limit);
> - if (!arm64_dma_phys_limit)
> - arm64_dma_phys_limit = dma32_phys_limit;
> #endif
> max_zone_pfns[ZONE_NORMAL] = max;
>
> @@ -336,8 +334,12 @@ void __init arm64_memblock_init(void)
>
> early_init_fdt_scan_reserved_mem();
>
> - if (!IS_ENABLED(CONFIG_ZONE_DMA) && !IS_ENABLED(CONFIG_ZONE_DMA32))
> + dma32_phys_limit = max_zone_phys(32);
> + if (!IS_ENABLED(CONFIG_ZONE_DMA)) {
> + if (IS_ENABLED(CONFIG_ZONE_DMA32))
> + arm64_dma_phys_limit = dma32_phys_limit;
> reserve_crashkernel();
> + }
>
> high_memory = __va(memblock_end_of_DRAM() - 1) + 1;
> }
> @@ -385,7 +387,7 @@ void __init bootmem_init(void)
> * request_standard_resources() depends on crashkernel's memory being
> * reserved, so do it here.
> */
> - if (IS_ENABLED(CONFIG_ZONE_DMA) || IS_ENABLED(CONFIG_ZONE_DMA32))
> + if (IS_ENABLED(CONFIG_ZONE_DMA))
> reserve_crashkernel();
>
> memblock_dump_all();
> diff --git a/arch/arm64/mm/mmu.c b/arch/arm64/mm/mmu.c
> index 626ec32873c6..23734481318a 100644
> --- a/arch/arm64/mm/mmu.c
> +++ b/arch/arm64/mm/mmu.c
> @@ -529,8 +529,7 @@ static void __init map_mem(pgd_t *pgdp)
>
> #ifdef CONFIG_KEXEC_CORE
> if (crash_mem_map) {
> - if (IS_ENABLED(CONFIG_ZONE_DMA) ||
> - IS_ENABLED(CONFIG_ZONE_DMA32))
> + if (IS_ENABLED(CONFIG_ZONE_DMA))
> flags |= NO_BLOCK_MAPPINGS | NO_CONT_MAPPINGS;
> else if (crashk_res.end)
> memblock_mark_nomap(crashk_res.start,
> @@ -571,8 +570,7 @@ static void __init map_mem(pgd_t *pgdp)
> * through /sys/kernel/kexec_crash_size interface.
> */
> #ifdef CONFIG_KEXEC_CORE
> - if (crash_mem_map &&
> - !IS_ENABLED(CONFIG_ZONE_DMA) && !IS_ENABLED(CONFIG_ZONE_DMA32)) {
> + if (crash_mem_map && !IS_ENABLED(CONFIG_ZONE_DMA)) {
> if (crashk_res.end) {
> __map_memblock(pgdp, crashk_res.start,
> crashk_res.end + 1,