Re: [PATCH v6 1/3] x86/boot: Add acpitb.c to parse acpi tables

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Chao,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on tip/x86/core]
[also build test WARNING on v4.19-rc3 next-20180910]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Chao-Fan/x86-boot-KASLR-Parse-ACPI-table-and-limit-kaslr-in-immovable-memory/20180911-043350
config: x86_64-allmodconfig (attached as .config)
compiler: gcc-7 (Debian 7.3.0-1) 7.3.0
reproduce:
        # save the attached .config to linux build tree
        make ARCH=x86_64 

All warnings (new ones prefixed by >>):

   arch/x86/boot/compressed/misc.h:121:5: sparse: symbol 'num_immovable_mem' was not declared. Should it be static?
   arch/x86/boot/compressed/acpitb.c:21:19: sparse: symbol 'immovable_mem' was not declared. Should it be static?
   arch/x86/boot/compressed/acpitb.c:225:17: sparse: undefined identifier 'error'
   arch/x86/boot/compressed/acpitb.c:237:25: sparse: undefined identifier 'warn'
   arch/x86/boot/compressed/acpitb.c:282:26: sparse: symbol 'get_acpi_srat_table' was not declared. Should it be static?
   arch/x86/boot/compressed/acpitb.c:225:22: sparse: call with no type!
   arch/x86/boot/compressed/acpitb.c:237:29: sparse: call with no type!
   arch/x86/boot/compressed/acpitb.c: In function 'get_acpi_rsdp':
>> arch/x86/boot/compressed/acpitb.c:225:3: warning: implicit declaration of function 'error' [-Wimplicit-function-declaration]
      error("Failed to allocate space for tmp_cmdline");
      ^~~~~
>> arch/x86/boot/compressed/acpitb.c:237:4: warning: implicit declaration of function 'warn'; did you mean '__warn'? [-Wimplicit-function-declaration]
       warn("Only '--' specified in cmdline");
       ^~~~
       __warn

sparse warnings: (new ones prefixed by >>)

   arch/x86/boot/compressed/misc.h:121:5: sparse: symbol 'num_immovable_mem' was not declared. Should it be static?
   arch/x86/boot/compressed/acpitb.c:225:17: sparse: undefined identifier 'error'
   arch/x86/boot/compressed/acpitb.c:237:25: sparse: undefined identifier 'warn'
>> arch/x86/boot/compressed/acpitb.c:282:26: sparse: symbol 'get_acpi_srat_table' was not declared. Should it be static?
>> arch/x86/boot/compressed/acpitb.c:225:22: sparse: call with no type!
   arch/x86/boot/compressed/acpitb.c:237:29: sparse: call with no type!
   arch/x86/boot/compressed/acpitb.c: In function 'get_acpi_rsdp':
   arch/x86/boot/compressed/acpitb.c:225:3: warning: implicit declaration of function 'error' [-Wimplicit-function-declaration]
      error("Failed to allocate space for tmp_cmdline");
      ^~~~~
   arch/x86/boot/compressed/acpitb.c:237:4: warning: implicit declaration of function 'warn'; did you mean '__warn'? [-Wimplicit-function-declaration]
       warn("Only '--' specified in cmdline");
       ^~~~
       __warn

Please review and possibly fold the followup patch.

vim +/error +225 arch/x86/boot/compressed/acpitb.c

   210	
   211	#ifdef CONFIG_KEXEC
   212	static bool get_acpi_rsdp(acpi_physical_address *rsdp_addr)
   213	{
   214		char *args = (char *)get_cmd_line_ptr();
   215		size_t len = strlen((char *)args);
   216		char *tmp_cmdline, *param, *val;
   217		unsigned long long addr = 0;
   218		char *endptr;
   219	
   220		if (!strstr(args, "acpi_rsdp="))
   221			return false;
   222	
   223		tmp_cmdline = malloc(len+1);
   224		if (!tmp_cmdline)
 > 225			error("Failed to allocate space for tmp_cmdline");
   226	
   227		memcpy(tmp_cmdline, args, len);
   228		tmp_cmdline[len] = 0;
   229		args = tmp_cmdline;
   230	
   231		args = skip_spaces(args);
   232	
   233		while (*args) {
   234			args = next_arg(args, &param, &val);
   235	
   236			if (!val && strcmp(param, "--") == 0) {
 > 237				warn("Only '--' specified in cmdline");
   238				free(tmp_cmdline);
   239				return false;
   240			}
   241	
   242			if (!strcmp(param, "acpi_rsdp")) {
   243				addr = simple_strtoull(val, &endptr, 0);
   244	
   245				if (addr == 0)
   246					return false;
   247	
   248				*rsdp_addr = (acpi_physical_address)addr;
   249				return true;
   250			}
   251		}
   252		return false;
   253	}
   254	#else
   255	static bool get_acpi_rsdp(acpi_physical_address *rsdp_addr)
   256	{
   257		return false;
   258	}
   259	#endif
   260	
   261	/*
   262	 * Used to dig rsdp table from EFI table or BIOS.
   263	 * If rsdp table found in EFI table, use it. Or search BIOS.
   264	 * Based on acpi_os_get_root_pointer().
   265	 */
   266	static acpi_physical_address get_rsdp_addr(void)
   267	{
   268		acpi_physical_address pa = 0;
   269		bool status = false;
   270	
   271		status = get_acpi_rsdp(&pa);
   272	
   273		if (!status || pa == 0)
   274			status = efi_get_rsdp_addr(&pa);
   275	
   276		if (!status || pa == 0)
   277			bios_get_rsdp_addr(&pa);
   278	
   279		return pa;
   280	}
   281	
 > 282	struct acpi_table_header *get_acpi_srat_table(void)
   283	{
   284		char *args = (char *)get_cmd_line_ptr();
   285		acpi_physical_address acpi_table;
   286		acpi_physical_address root_table;
   287		struct acpi_table_header *header;
   288		struct acpi_table_rsdp *rsdp;
   289		char *signature;
   290		u8 *entry;
   291		u32 count;
   292		u32 size;
   293		int i, j;
   294		u32 len;
   295	
   296		rsdp = (struct acpi_table_rsdp *)get_rsdp_addr();
   297		if (!rsdp)
   298			return NULL;
   299	
   300		/* Get rsdt or xsdt from rsdp. */
   301		if (!strstr(args, "acpi=rsdt") &&
   302		    rsdp->xsdt_physical_address && rsdp->revision > 1) {
   303			root_table = rsdp->xsdt_physical_address;
   304			size = ACPI_XSDT_ENTRY_SIZE;
   305		} else {
   306			root_table = rsdp->rsdt_physical_address;
   307			size = ACPI_RSDT_ENTRY_SIZE;
   308		}
   309	
   310		/* Get ACPI root table from rsdt or xsdt.*/
   311		header = (struct acpi_table_header *)root_table;
   312		len = header->length;
   313		count = (u32)((len - sizeof(struct acpi_table_header)) / size);
   314		entry = ACPI_ADD_PTR(u8, header, sizeof(struct acpi_table_header));
   315	
   316		for (i = 0; i < count; i++) {
   317			u64 address64;
   318	
   319			if (size == ACPI_RSDT_ENTRY_SIZE)
   320				acpi_table = ((acpi_physical_address)
   321					      (*ACPI_CAST_PTR(u32, entry)));
   322			else {
   323				*(u64 *)(void *)&address64 = *(u64 *)(void *)entry;
   324				acpi_table = (acpi_physical_address) address64;
   325			}
   326	
   327			if (acpi_table) {
   328				header = (struct acpi_table_header *)acpi_table;
   329				signature = header->signature;
   330	
   331				if (!strncmp(signature, "SRAT", 4))
   332					return header;
   333			}
   334			entry += size;
   335		}
   336		return NULL;
   337	}
   338	

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

Attachment: .config.gz
Description: application/gzip


[Index of Archives]     [Linux IBM ACPI]     [Linux Power Management]     [Linux Kernel]     [Linux Laptop]     [Kernel Newbies]     [Share Photos]     [Security]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Samba]     [Video 4 Linux]     [Device Mapper]     [Linux Resources]

  Powered by Linux