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

[Jiaxun Yang <jiaxun.yang@xxxxxxxxxxx>]

在 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

--
---
Jiaxun Yang