| Message ID | 20240113095509.178697-4-huangpei@loongson.cn (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [1/3] MIPS: adjust exception vector space revervation | expand |
在 2024/1/13 09:55, Huang Pei 写道: > This reverts commit 264927e3538169fe2973a5732f03ea01b0f9f9e8. > > The SMBIOS memory reserved region(0xfffe000-0xfffe7ff) is not OWNED > by kenel, here OWNED means the region is within the physical memory > given to kernel by firmware as SYSTEM_RAM_{LOW,HIGH}. > > Since commit 61167ad5fecd("mm: pass nid to reserve_bootmem_region()), > loongson64 booting failed with CONFIG_DEFERRED_STRUCT_PAGE_INIT like > this: > ---------------------------------------------------------------------- > Call Trace: > [<ffffffff8235d088>] reserve_bootmem_region+0xa8/0x184 > [<ffffffff82333940>] memblock_free_all+0x104/0x2a8 > [<ffffffff8231d8e4>] mem_init+0x84/0x94 > [<ffffffff82330958>] mm_core_init+0xf8/0x308 > [<ffffffff82318c38>] start_kernel+0x43c/0x86c > > Code: 10400028 2402fff0 de420000 <dc432880> 0203182b 14600022 > 64420070 00003025 24040003 > > ---[ end trace 0000000000000000 ]--- > Kernel panic - not syncing: Attempted to kill the idle task! > ---[ end Kernel panic - not syncing: Attempted to kill the idle task! ]--- > ---------------------------------------------------------------------- > > This is another case of inappropriate usage of memblock_reserve In my test system with kunlun firmware they are actually covered by SYSRAM type. IMO, better to add those memory to memblock as well in your case. Thanks - Jiaxun > --- > .../include/asm/mach-loongson64/boot_param.h | 6 +-- > arch/mips/loongson64/init.c | 42 +++++++------------ > 2 files changed, 17 insertions(+), 31 deletions(-) > > diff --git a/arch/mips/include/asm/mach-loongson64/boot_param.h b/arch/mips/include/asm/mach-loongson64/boot_param.h > index e007edd6b60a..c454ef734c45 100644 > --- a/arch/mips/include/asm/mach-loongson64/boot_param.h > +++ b/arch/mips/include/asm/mach-loongson64/boot_param.h > @@ -14,11 +14,7 @@ > #define ADAPTER_ROM 8 > #define ACPI_TABLE 9 > #define SMBIOS_TABLE 10 > -#define UMA_VIDEO_RAM 11 > -#define VUMA_VIDEO_RAM 12 > -#define MAX_MEMORY_TYPE 13 > - > -#define MEM_SIZE_IS_IN_BYTES (1 << 31) > +#define MAX_MEMORY_TYPE 11 > > #define LOONGSON3_BOOT_MEM_MAP_MAX 128 > struct efi_memory_map_loongson { > diff --git a/arch/mips/loongson64/init.c b/arch/mips/loongson64/init.c > index f25caa6aa9d3..d62262f93069 100644 > --- a/arch/mips/loongson64/init.c > +++ b/arch/mips/loongson64/init.c > @@ -49,7 +49,8 @@ void virtual_early_config(void) > void __init szmem(unsigned int node) > { > u32 i, mem_type; > - phys_addr_t node_id, mem_start, mem_size; > + static unsigned long num_physpages; > + u64 node_id, node_psize, start_pfn, end_pfn, mem_start, mem_size; > > /* Otherwise come from DTB */ > if (loongson_sysconf.fw_interface != LOONGSON_LEFI) > @@ -63,38 +64,27 @@ void __init szmem(unsigned int node) > > mem_type = loongson_memmap->map[i].mem_type; > mem_size = loongson_memmap->map[i].mem_size; > - > - /* Memory size comes in MB if MEM_SIZE_IS_IN_BYTES not set */ > - if (mem_size & MEM_SIZE_IS_IN_BYTES) > - mem_size &= ~MEM_SIZE_IS_IN_BYTES; > - else > - mem_size = mem_size << 20; > - > - mem_start = (node_id << 44) | loongson_memmap->map[i].mem_start; > + mem_start = loongson_memmap->map[i].mem_start; > > switch (mem_type) { > case SYSTEM_RAM_LOW: > case SYSTEM_RAM_HIGH: > - case UMA_VIDEO_RAM: > - pr_info("Node %d, mem_type:%d\t[%pa], %pa bytes usable\n", > - (u32)node_id, mem_type, &mem_start, &mem_size); > - memblock_add_node(mem_start, mem_size, node, > + start_pfn = ((node_id << 44) + mem_start) >> PAGE_SHIFT; > + node_psize = (mem_size << 20) >> PAGE_SHIFT; > + end_pfn = start_pfn + node_psize; > + num_physpages += node_psize; > + pr_info("Node%d: mem_type:%d, mem_start:0x%llx, mem_size:0x%llx MB\n", > + (u32)node_id, mem_type, mem_start, mem_size); > + pr_info(" start_pfn:0x%llx, end_pfn:0x%llx, num_physpages:0x%lx\n", > + start_pfn, end_pfn, num_physpages); > + memblock_add_node(PFN_PHYS(start_pfn), > + PFN_PHYS(node_psize), node, > MEMBLOCK_NONE); > break; > case SYSTEM_RAM_RESERVED: > - case VIDEO_ROM: > - case ADAPTER_ROM: > - case ACPI_TABLE: > - case SMBIOS_TABLE: > - pr_info("Node %d, mem_type:%d\t[%pa], %pa bytes reserved\n", > - (u32)node_id, mem_type, &mem_start, &mem_size); > - memblock_reserve(mem_start, mem_size); > - break; > - /* We should not reserve VUMA_VIDEO_RAM as it overlaps with MMIO */ > - case VUMA_VIDEO_RAM: > - default: > - pr_info("Node %d, mem_type:%d\t[%pa], %pa bytes unhandled\n", > - (u32)node_id, mem_type, &mem_start, &mem_size); > + pr_info("Node%d: mem_type:%d, mem_start:0x%llx, mem_size:0x%llx MB\n", > + (u32)node_id, mem_type, mem_start, mem_size); > + memblock_reserve(((node_id << 44) + mem_start), mem_size << 20); > break; > } > }
On Sat, Jan 13, 2024 at 11:59:11AM +0000, Jiaxun Yang wrote: > > > 在 2024/1/13 09:55, Huang Pei 写道: > > This reverts commit 264927e3538169fe2973a5732f03ea01b0f9f9e8. > > > > The SMBIOS memory reserved region(0xfffe000-0xfffe7ff) is not OWNED > > by kenel, here OWNED means the region is within the physical memory > > given to kernel by firmware as SYSTEM_RAM_{LOW,HIGH}. > > > > Since commit 61167ad5fecd("mm: pass nid to reserve_bootmem_region()), > > loongson64 booting failed with CONFIG_DEFERRED_STRUCT_PAGE_INIT like > > this: > > ---------------------------------------------------------------------- > > Call Trace: > > [<ffffffff8235d088>] reserve_bootmem_region+0xa8/0x184 > > [<ffffffff82333940>] memblock_free_all+0x104/0x2a8 > > [<ffffffff8231d8e4>] mem_init+0x84/0x94 > > [<ffffffff82330958>] mm_core_init+0xf8/0x308 > > [<ffffffff82318c38>] start_kernel+0x43c/0x86c > > > > Code: 10400028 2402fff0 de420000 <dc432880> 0203182b 14600022 > > 64420070 00003025 24040003 > > > > ---[ end trace 0000000000000000 ]--- > > Kernel panic - not syncing: Attempted to kill the idle task! > > ---[ end Kernel panic - not syncing: Attempted to kill the idle task! ]--- > > ---------------------------------------------------------------------- > > > > This is another case of inappropriate usage of memblock_reserve > > In my test system with kunlun firmware they are actually covered by SYSRAM > type. > IMO, better to add those memory to memblock as well in your case. > My test machine is PMON-based 3B1500, the SMBIOS_TABLE located in 0xfffe000-0xfffe7ff, which is not included in SYSTEM_RAM_LOW(0x200000- 0xeffffff), I think we need a check like, ---------------------------------------------------------------------- if(memblock_is_region_memory(addr, size)) memblock_reserve(addr, size); ---------------------------------------------------------------------- to support both cases; > Thanks > - Jiaxun > > --- > > .../include/asm/mach-loongson64/boot_param.h | 6 +-- > > arch/mips/loongson64/init.c | 42 +++++++------------ > > 2 files changed, 17 insertions(+), 31 deletions(-) > > > > diff --git a/arch/mips/include/asm/mach-loongson64/boot_param.h b/arch/mips/include/asm/mach-loongson64/boot_param.h > > index e007edd6b60a..c454ef734c45 100644 > > --- a/arch/mips/include/asm/mach-loongson64/boot_param.h > > +++ b/arch/mips/include/asm/mach-loongson64/boot_param.h > > @@ -14,11 +14,7 @@ > > #define ADAPTER_ROM 8 > > #define ACPI_TABLE 9 > > #define SMBIOS_TABLE 10 > > -#define UMA_VIDEO_RAM 11 > > -#define VUMA_VIDEO_RAM 12 > > -#define MAX_MEMORY_TYPE 13 > > - > > -#define MEM_SIZE_IS_IN_BYTES (1 << 31) > > +#define MAX_MEMORY_TYPE 11 > > #define LOONGSON3_BOOT_MEM_MAP_MAX 128 > > struct efi_memory_map_loongson { > > diff --git a/arch/mips/loongson64/init.c b/arch/mips/loongson64/init.c > > index f25caa6aa9d3..d62262f93069 100644 > > --- a/arch/mips/loongson64/init.c > > +++ b/arch/mips/loongson64/init.c > > @@ -49,7 +49,8 @@ void virtual_early_config(void) > > void __init szmem(unsigned int node) > > { > > u32 i, mem_type; > > - phys_addr_t node_id, mem_start, mem_size; > > + static unsigned long num_physpages; > > + u64 node_id, node_psize, start_pfn, end_pfn, mem_start, mem_size; > > /* Otherwise come from DTB */ > > if (loongson_sysconf.fw_interface != LOONGSON_LEFI) > > @@ -63,38 +64,27 @@ void __init szmem(unsigned int node) > > mem_type = loongson_memmap->map[i].mem_type; > > mem_size = loongson_memmap->map[i].mem_size; > > - > > - /* Memory size comes in MB if MEM_SIZE_IS_IN_BYTES not set */ > > - if (mem_size & MEM_SIZE_IS_IN_BYTES) > > - mem_size &= ~MEM_SIZE_IS_IN_BYTES; > > - else > > - mem_size = mem_size << 20; > > - > > - mem_start = (node_id << 44) | loongson_memmap->map[i].mem_start; > > + mem_start = loongson_memmap->map[i].mem_start; > > switch (mem_type) { > > case SYSTEM_RAM_LOW: > > case SYSTEM_RAM_HIGH: > > - case UMA_VIDEO_RAM: > > - pr_info("Node %d, mem_type:%d\t[%pa], %pa bytes usable\n", > > - (u32)node_id, mem_type, &mem_start, &mem_size); > > - memblock_add_node(mem_start, mem_size, node, > > + start_pfn = ((node_id << 44) + mem_start) >> PAGE_SHIFT; > > + node_psize = (mem_size << 20) >> PAGE_SHIFT; > > + end_pfn = start_pfn + node_psize; > > + num_physpages += node_psize; > > + pr_info("Node%d: mem_type:%d, mem_start:0x%llx, mem_size:0x%llx MB\n", > > + (u32)node_id, mem_type, mem_start, mem_size); > > + pr_info(" start_pfn:0x%llx, end_pfn:0x%llx, num_physpages:0x%lx\n", > > + start_pfn, end_pfn, num_physpages); > > + memblock_add_node(PFN_PHYS(start_pfn), > > + PFN_PHYS(node_psize), node, > > MEMBLOCK_NONE); > > break; > > case SYSTEM_RAM_RESERVED: > > - case VIDEO_ROM: > > - case ADAPTER_ROM: > > - case ACPI_TABLE: > > - case SMBIOS_TABLE: > > - pr_info("Node %d, mem_type:%d\t[%pa], %pa bytes reserved\n", > > - (u32)node_id, mem_type, &mem_start, &mem_size); > > - memblock_reserve(mem_start, mem_size); > > - break; > > - /* We should not reserve VUMA_VIDEO_RAM as it overlaps with MMIO */ > > - case VUMA_VIDEO_RAM: > > - default: > > - pr_info("Node %d, mem_type:%d\t[%pa], %pa bytes unhandled\n", > > - (u32)node_id, mem_type, &mem_start, &mem_size); > > + pr_info("Node%d: mem_type:%d, mem_start:0x%llx, mem_size:0x%llx MB\n", > > + (u32)node_id, mem_type, mem_start, mem_size); > > + memblock_reserve(((node_id << 44) + mem_start), mem_size << 20); > > break; > > } > > } > > -- > --- > Jiaxun Yang
在 2024/1/14 08:53, Huang Pei 写道: > On Sat, Jan 13, 2024 at 11:59:11AM +0000, Jiaxun Yang wrote: [...] > > In my test system with kunlun firmware they are actually covered by SYSRAM > type. > IMO, better to add those memory to memblock as well in your case. > > My test machine is PMON-based 3B1500, the SMBIOS_TABLE located in > 0xfffe000-0xfffe7ff, which is not included in SYSTEM_RAM_LOW(0x200000- > 0xeffffff), I think we need a check like, > ---------------------------------------------------------------------- > if(memblock_is_region_memory(addr, size)) > memblock_reserve(addr, size); > ---------------------------------------------------------------------- > to support both cases; Then we are running into ordering issue. memblock_add of SYSRAM may later then reservation. What about unconditionally add those ranges to memblock? As it's certain that those regions will be RAM. Thanks - Jiaxun >> Thanks >> - Jiaxun >>> --- >>> .../include/asm/mach-loongson64/boot_param.h | 6 +-- >>> arch/mips/loongson64/init.c | 42 +++++++------------ >>> 2 files changed, 17 insertions(+), 31 deletions(-) >>> >>> diff --git a/arch/mips/include/asm/mach-loongson64/boot_param.h b/arch/mips/include/asm/mach-loongson64/boot_param.h >>> index e007edd6b60a..c454ef734c45 100644 >>> --- a/arch/mips/include/asm/mach-loongson64/boot_param.h >>> +++ b/arch/mips/include/asm/mach-loongson64/boot_param.h >>> @@ -14,11 +14,7 @@ >>> #define ADAPTER_ROM 8 >>> #define ACPI_TABLE 9 >>> #define SMBIOS_TABLE 10 >>> -#define UMA_VIDEO_RAM 11 >>> -#define VUMA_VIDEO_RAM 12 >>> -#define MAX_MEMORY_TYPE 13 >>> - >>> -#define MEM_SIZE_IS_IN_BYTES (1 << 31) >>> +#define MAX_MEMORY_TYPE 11 >>> #define LOONGSON3_BOOT_MEM_MAP_MAX 128 >>> struct efi_memory_map_loongson { >>> diff --git a/arch/mips/loongson64/init.c b/arch/mips/loongson64/init.c >>> index f25caa6aa9d3..d62262f93069 100644 >>> --- a/arch/mips/loongson64/init.c >>> +++ b/arch/mips/loongson64/init.c >>> @@ -49,7 +49,8 @@ void virtual_early_config(void) >>> void __init szmem(unsigned int node) >>> { >>> u32 i, mem_type; >>> - phys_addr_t node_id, mem_start, mem_size; >>> + static unsigned long num_physpages; >>> + u64 node_id, node_psize, start_pfn, end_pfn, mem_start, mem_size; >>> /* Otherwise come from DTB */ >>> if (loongson_sysconf.fw_interface != LOONGSON_LEFI) >>> @@ -63,38 +64,27 @@ void __init szmem(unsigned int node) >>> mem_type = loongson_memmap->map[i].mem_type; >>> mem_size = loongson_memmap->map[i].mem_size; >>> - >>> - /* Memory size comes in MB if MEM_SIZE_IS_IN_BYTES not set */ >>> - if (mem_size & MEM_SIZE_IS_IN_BYTES) >>> - mem_size &= ~MEM_SIZE_IS_IN_BYTES; >>> - else >>> - mem_size = mem_size << 20; >>> - >>> - mem_start = (node_id << 44) | loongson_memmap->map[i].mem_start; >>> + mem_start = loongson_memmap->map[i].mem_start; >>> switch (mem_type) { >>> case SYSTEM_RAM_LOW: >>> case SYSTEM_RAM_HIGH: >>> - case UMA_VIDEO_RAM: >>> - pr_info("Node %d, mem_type:%d\t[%pa], %pa bytes usable\n", >>> - (u32)node_id, mem_type, &mem_start, &mem_size); >>> - memblock_add_node(mem_start, mem_size, node, >>> + start_pfn = ((node_id << 44) + mem_start) >> PAGE_SHIFT; >>> + node_psize = (mem_size << 20) >> PAGE_SHIFT; >>> + end_pfn = start_pfn + node_psize; >>> + num_physpages += node_psize; >>> + pr_info("Node%d: mem_type:%d, mem_start:0x%llx, mem_size:0x%llx MB\n", >>> + (u32)node_id, mem_type, mem_start, mem_size); >>> + pr_info(" start_pfn:0x%llx, end_pfn:0x%llx, num_physpages:0x%lx\n", >>> + start_pfn, end_pfn, num_physpages); >>> + memblock_add_node(PFN_PHYS(start_pfn), >>> + PFN_PHYS(node_psize), node, >>> MEMBLOCK_NONE); >>> break; >>> case SYSTEM_RAM_RESERVED: >>> - case VIDEO_ROM: >>> - case ADAPTER_ROM: >>> - case ACPI_TABLE: >>> - case SMBIOS_TABLE: >>> - pr_info("Node %d, mem_type:%d\t[%pa], %pa bytes reserved\n", >>> - (u32)node_id, mem_type, &mem_start, &mem_size); >>> - memblock_reserve(mem_start, mem_size); >>> - break; >>> - /* We should not reserve VUMA_VIDEO_RAM as it overlaps with MMIO */ >>> - case VUMA_VIDEO_RAM: >>> - default: >>> - pr_info("Node %d, mem_type:%d\t[%pa], %pa bytes unhandled\n", >>> - (u32)node_id, mem_type, &mem_start, &mem_size); >>> + pr_info("Node%d: mem_type:%d, mem_start:0x%llx, mem_size:0x%llx MB\n", >>> + (u32)node_id, mem_type, mem_start, mem_size); >>> + memblock_reserve(((node_id << 44) + mem_start), mem_size << 20); >>> break; >>> } >>> } >> -- >> --- >> Jiaxun Yang >
On Sun, Jan 14, 2024 at 11:58:25AM +0000, Jiaxun Yang wrote: > > > 在 2024/1/14 08:53, Huang Pei 写道: > > On Sat, Jan 13, 2024 at 11:59:11AM +0000, Jiaxun Yang wrote: > [...] > > > > In my test system with kunlun firmware they are actually covered by SYSRAM > > type. > > IMO, better to add those memory to memblock as well in your case. > > > > My test machine is PMON-based 3B1500, the SMBIOS_TABLE located in > > 0xfffe000-0xfffe7ff, which is not included in SYSTEM_RAM_LOW(0x200000- > > 0xeffffff), I think we need a check like, > > ---------------------------------------------------------------------- > > if(memblock_is_region_memory(addr, size)) > > memblock_reserve(addr, size); > > ---------------------------------------------------------------------- > > to support both cases; > > Then we are running into ordering issue. memblock_add of SYSRAM may later > then reservation. > What about unconditionally add those ranges to memblock? As it's certain > that those regions will > be RAM. > I think we can do szmem twice, one for memblock.memory, the other for memblock_reserve. > Thanks > - Jiaxun > > > > Thanks > > > - Jiaxun > > > > --- > > > > .../include/asm/mach-loongson64/boot_param.h | 6 +-- > > > > arch/mips/loongson64/init.c | 42 +++++++------------ > > > > 2 files changed, 17 insertions(+), 31 deletions(-) > > > > > > > > diff --git a/arch/mips/include/asm/mach-loongson64/boot_param.h b/arch/mips/include/asm/mach-loongson64/boot_param.h > > > > index e007edd6b60a..c454ef734c45 100644 > > > > --- a/arch/mips/include/asm/mach-loongson64/boot_param.h > > > > +++ b/arch/mips/include/asm/mach-loongson64/boot_param.h > > > > @@ -14,11 +14,7 @@ > > > > #define ADAPTER_ROM 8 > > > > #define ACPI_TABLE 9 > > > > #define SMBIOS_TABLE 10 > > > > -#define UMA_VIDEO_RAM 11 > > > > -#define VUMA_VIDEO_RAM 12 > > > > -#define MAX_MEMORY_TYPE 13 > > > > - > > > > -#define MEM_SIZE_IS_IN_BYTES (1 << 31) > > > > +#define MAX_MEMORY_TYPE 11 > > > > #define LOONGSON3_BOOT_MEM_MAP_MAX 128 > > > > struct efi_memory_map_loongson { > > > > diff --git a/arch/mips/loongson64/init.c b/arch/mips/loongson64/init.c > > > > index f25caa6aa9d3..d62262f93069 100644 > > > > --- a/arch/mips/loongson64/init.c > > > > +++ b/arch/mips/loongson64/init.c > > > > @@ -49,7 +49,8 @@ void virtual_early_config(void) > > > > void __init szmem(unsigned int node) > > > > { > > > > u32 i, mem_type; > > > > - phys_addr_t node_id, mem_start, mem_size; > > > > + static unsigned long num_physpages; > > > > + u64 node_id, node_psize, start_pfn, end_pfn, mem_start, mem_size; > > > > /* Otherwise come from DTB */ > > > > if (loongson_sysconf.fw_interface != LOONGSON_LEFI) > > > > @@ -63,38 +64,27 @@ void __init szmem(unsigned int node) > > > > mem_type = loongson_memmap->map[i].mem_type; > > > > mem_size = loongson_memmap->map[i].mem_size; > > > > - > > > > - /* Memory size comes in MB if MEM_SIZE_IS_IN_BYTES not set */ > > > > - if (mem_size & MEM_SIZE_IS_IN_BYTES) > > > > - mem_size &= ~MEM_SIZE_IS_IN_BYTES; > > > > - else > > > > - mem_size = mem_size << 20; > > > > - > > > > - mem_start = (node_id << 44) | loongson_memmap->map[i].mem_start; > > > > + mem_start = loongson_memmap->map[i].mem_start; > > > > switch (mem_type) { > > > > case SYSTEM_RAM_LOW: > > > > case SYSTEM_RAM_HIGH: > > > > - case UMA_VIDEO_RAM: > > > > - pr_info("Node %d, mem_type:%d\t[%pa], %pa bytes usable\n", > > > > - (u32)node_id, mem_type, &mem_start, &mem_size); > > > > - memblock_add_node(mem_start, mem_size, node, > > > > + start_pfn = ((node_id << 44) + mem_start) >> PAGE_SHIFT; > > > > + node_psize = (mem_size << 20) >> PAGE_SHIFT; > > > > + end_pfn = start_pfn + node_psize; > > > > + num_physpages += node_psize; > > > > + pr_info("Node%d: mem_type:%d, mem_start:0x%llx, mem_size:0x%llx MB\n", > > > > + (u32)node_id, mem_type, mem_start, mem_size); > > > > + pr_info(" start_pfn:0x%llx, end_pfn:0x%llx, num_physpages:0x%lx\n", > > > > + start_pfn, end_pfn, num_physpages); > > > > + memblock_add_node(PFN_PHYS(start_pfn), > > > > + PFN_PHYS(node_psize), node, > > > > MEMBLOCK_NONE); > > > > break; > > > > case SYSTEM_RAM_RESERVED: > > > > - case VIDEO_ROM: > > > > - case ADAPTER_ROM: > > > > - case ACPI_TABLE: > > > > - case SMBIOS_TABLE: > > > > - pr_info("Node %d, mem_type:%d\t[%pa], %pa bytes reserved\n", > > > > - (u32)node_id, mem_type, &mem_start, &mem_size); > > > > - memblock_reserve(mem_start, mem_size); > > > > - break; > > > > - /* We should not reserve VUMA_VIDEO_RAM as it overlaps with MMIO */ > > > > - case VUMA_VIDEO_RAM: > > > > - default: > > > > - pr_info("Node %d, mem_type:%d\t[%pa], %pa bytes unhandled\n", > > > > - (u32)node_id, mem_type, &mem_start, &mem_size); > > > > + pr_info("Node%d: mem_type:%d, mem_start:0x%llx, mem_size:0x%llx MB\n", > > > > + (u32)node_id, mem_type, mem_start, mem_size); > > > > + memblock_reserve(((node_id << 44) + mem_start), mem_size << 20); > > > > break; > > > > } > > > > } > > > -- > > > --- > > > Jiaxun Yang > > > > -- > --- > Jiaxun Yang
在 2024/1/15 01:25, Huang Pei 写道: > On Sun, Jan 14, 2024 at 11:58:25AM +0000, Jiaxun Yang wrote: >> >> 在 2024/1/14 08:53, Huang Pei 写道: >>> On Sat, Jan 13, 2024 at 11:59:11AM +0000, Jiaxun Yang wrote: >> [...] >>> In my test system with kunlun firmware they are actually covered by SYSRAM >>> type. >>> IMO, better to add those memory to memblock as well in your case. >>> >>> My test machine is PMON-based 3B1500, the SMBIOS_TABLE located in >>> 0xfffe000-0xfffe7ff, which is not included in SYSTEM_RAM_LOW(0x200000- >>> 0xeffffff), I think we need a check like, >>> ---------------------------------------------------------------------- >>> if(memblock_is_region_memory(addr, size)) >>> memblock_reserve(addr, size); >>> ---------------------------------------------------------------------- >>> to support both cases; >> Then we are running into ordering issue. memblock_add of SYSRAM may later >> then reservation. >> What about unconditionally add those ranges to memblock? As it's certain >> that those regions will >> be RAM. >> > I think we can do szmem twice, one for memblock.memory, the other for > memblock_reserve. IMO this is a little bit insane. LoongArch made a workaround to set all reserved memory to node zero [1]. [1]: https://lore.kernel.org/all/20230911092810.3108092-1-chenhuacai@loongson.cn/ Thanks
On Mon, Jan 15, 2024 at 02:14:14PM +0000, Jiaxun Yang wrote: > > > 在 2024/1/15 01:25, Huang Pei 写道: > > On Sun, Jan 14, 2024 at 11:58:25AM +0000, Jiaxun Yang wrote: > > > > > > 在 2024/1/14 08:53, Huang Pei 写道: > > > > On Sat, Jan 13, 2024 at 11:59:11AM +0000, Jiaxun Yang wrote: > > > [...] > > > > In my test system with kunlun firmware they are actually covered by SYSRAM > > > > type. > > > > IMO, better to add those memory to memblock as well in your case. > > > > > > > > My test machine is PMON-based 3B1500, the SMBIOS_TABLE located in > > > > 0xfffe000-0xfffe7ff, which is not included in SYSTEM_RAM_LOW(0x200000- > > > > 0xeffffff), I think we need a check like, > > > > ---------------------------------------------------------------------- > > > > if(memblock_is_region_memory(addr, size)) > > > > memblock_reserve(addr, size); > > > > ---------------------------------------------------------------------- > > > > to support both cases; > > > Then we are running into ordering issue. memblock_add of SYSRAM may later > > > then reservation. > > > What about unconditionally add those ranges to memblock? As it's certain > > > that those regions will > > > be RAM. > > > > > I think we can do szmem twice, one for memblock.memory, the other for > > memblock_reserve. > IMO this is a little bit insane. > LoongArch made a workaround to set all reserved memory to node zero [1]. > > [1]: > https://lore.kernel.org/all/20230911092810.3108092-1-chenhuacai@loongson.cn/ > I think they forgot MIPS totally. Their fix is right, because all the reserved memory is located on Node 0 by now. If we keep reserved memory within range of the memoryblock.memory and keep this conresponcy, then there would not be this bug. My fix is just another way to keep this conresponcy and make the memory ownership more clear between kernel and firmware. > Thanks > > -- > --- > Jiaxun Yang
diff --git a/arch/mips/include/asm/mach-loongson64/boot_param.h b/arch/mips/include/asm/mach-loongson64/boot_param.h index e007edd6b60a..c454ef734c45 100644 --- a/arch/mips/include/asm/mach-loongson64/boot_param.h +++ b/arch/mips/include/asm/mach-loongson64/boot_param.h @@ -14,11 +14,7 @@ #define ADAPTER_ROM 8 #define ACPI_TABLE 9 #define SMBIOS_TABLE 10 -#define UMA_VIDEO_RAM 11 -#define VUMA_VIDEO_RAM 12 -#define MAX_MEMORY_TYPE 13 - -#define MEM_SIZE_IS_IN_BYTES (1 << 31) +#define MAX_MEMORY_TYPE 11 #define LOONGSON3_BOOT_MEM_MAP_MAX 128 struct efi_memory_map_loongson { diff --git a/arch/mips/loongson64/init.c b/arch/mips/loongson64/init.c index f25caa6aa9d3..d62262f93069 100644 --- a/arch/mips/loongson64/init.c +++ b/arch/mips/loongson64/init.c @@ -49,7 +49,8 @@ void virtual_early_config(void) void __init szmem(unsigned int node) { u32 i, mem_type; - phys_addr_t node_id, mem_start, mem_size; + static unsigned long num_physpages; + u64 node_id, node_psize, start_pfn, end_pfn, mem_start, mem_size; /* Otherwise come from DTB */ if (loongson_sysconf.fw_interface != LOONGSON_LEFI) @@ -63,38 +64,27 @@ void __init szmem(unsigned int node) mem_type = loongson_memmap->map[i].mem_type; mem_size = loongson_memmap->map[i].mem_size; - - /* Memory size comes in MB if MEM_SIZE_IS_IN_BYTES not set */ - if (mem_size & MEM_SIZE_IS_IN_BYTES) - mem_size &= ~MEM_SIZE_IS_IN_BYTES; - else - mem_size = mem_size << 20; - - mem_start = (node_id << 44) | loongson_memmap->map[i].mem_start; + mem_start = loongson_memmap->map[i].mem_start; switch (mem_type) { case SYSTEM_RAM_LOW: case SYSTEM_RAM_HIGH: - case UMA_VIDEO_RAM: - pr_info("Node %d, mem_type:%d\t[%pa], %pa bytes usable\n", - (u32)node_id, mem_type, &mem_start, &mem_size); - memblock_add_node(mem_start, mem_size, node, + start_pfn = ((node_id << 44) + mem_start) >> PAGE_SHIFT; + node_psize = (mem_size << 20) >> PAGE_SHIFT; + end_pfn = start_pfn + node_psize; + num_physpages += node_psize; + pr_info("Node%d: mem_type:%d, mem_start:0x%llx, mem_size:0x%llx MB\n", + (u32)node_id, mem_type, mem_start, mem_size); + pr_info(" start_pfn:0x%llx, end_pfn:0x%llx, num_physpages:0x%lx\n", + start_pfn, end_pfn, num_physpages); + memblock_add_node(PFN_PHYS(start_pfn), + PFN_PHYS(node_psize), node, MEMBLOCK_NONE); break; case SYSTEM_RAM_RESERVED: - case VIDEO_ROM: - case ADAPTER_ROM: - case ACPI_TABLE: - case SMBIOS_TABLE: - pr_info("Node %d, mem_type:%d\t[%pa], %pa bytes reserved\n", - (u32)node_id, mem_type, &mem_start, &mem_size); - memblock_reserve(mem_start, mem_size); - break; - /* We should not reserve VUMA_VIDEO_RAM as it overlaps with MMIO */ - case VUMA_VIDEO_RAM: - default: - pr_info("Node %d, mem_type:%d\t[%pa], %pa bytes unhandled\n", - (u32)node_id, mem_type, &mem_start, &mem_size); + pr_info("Node%d: mem_type:%d, mem_start:0x%llx, mem_size:0x%llx MB\n", + (u32)node_id, mem_type, mem_start, mem_size); + memblock_reserve(((node_id << 44) + mem_start), mem_size << 20); break; } }
This reverts commit 264927e3538169fe2973a5732f03ea01b0f9f9e8. The SMBIOS memory reserved region(0xfffe000-0xfffe7ff) is not OWNED by kenel, here OWNED means the region is within the physical memory given to kernel by firmware as SYSTEM_RAM_{LOW,HIGH}. Since commit 61167ad5fecd("mm: pass nid to reserve_bootmem_region()), loongson64 booting failed with CONFIG_DEFERRED_STRUCT_PAGE_INIT like this: ---------------------------------------------------------------------- Call Trace: [<ffffffff8235d088>] reserve_bootmem_region+0xa8/0x184 [<ffffffff82333940>] memblock_free_all+0x104/0x2a8 [<ffffffff8231d8e4>] mem_init+0x84/0x94 [<ffffffff82330958>] mm_core_init+0xf8/0x308 [<ffffffff82318c38>] start_kernel+0x43c/0x86c Code: 10400028 2402fff0 de420000 <dc432880> 0203182b 14600022 64420070 00003025 24040003 ---[ end trace 0000000000000000 ]--- Kernel panic - not syncing: Attempted to kill the idle task! ---[ end Kernel panic - not syncing: Attempted to kill the idle task! ]--- ---------------------------------------------------------------------- This is another case of inappropriate usage of memblock_reserve --- .../include/asm/mach-loongson64/boot_param.h | 6 +-- arch/mips/loongson64/init.c | 42 +++++++------------ 2 files changed, 17 insertions(+), 31 deletions(-)