Re: [PATCH 3/3] Revert "MIPS: Loongson64: Handle more memory types passed from firmware"

[Jiaxun Yang <jiaxun.yang@xxxxxxxxxxx>]

在 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;
>   		}
>   	}

--
---
Jiaxun Yang