All of lore.kernel.org
 help / color / mirror / Atom feed
From: kbuild test robot <lkp@intel.com>
Cc: kbuild-all@01.org, tglx@linutronix.de, mingo@redhat.com,
	hpa@zytor.com, x86@kernel.org, linux-kernel@vger.kernel.org,
	keescook@chromium.org, bhe@redhat.com,
	indou.takao@jp.fujitsu.com, caoj.fnst@cn.fujitsu.com,
	douly.fnst@cn.fujitsu.com, Chao Fan <fanc.fnst@cn.fujitsu.com>,
	"Rafael J. Wysocki" <rjw@rjwysocki.net>,
	Len Brown <lenb@kernel.org>,
	Ard Biesheuvel <ard.biesheuvel@linaro.org>,
	linux-efi@vger.kernel.org, linux-acpi@vger.kernel.org
Subject: Re: [PATCH v6 1/3] x86/boot: Add acpitb.c to parse acpi tables
Date: Tue, 11 Sep 2018 16:52:26 +0800	[thread overview]
Message-ID: <201809111628.yudoAWlu%fengguang.wu@intel.com> (raw)
In-Reply-To: <20180910124015.18073-2-fanc.fnst@cn.fujitsu.com>

[-- Attachment #1: Type: text/plain, Size: 6851 bytes --]

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 #2: .config.gz --]
[-- Type: application/gzip, Size: 65704 bytes --]

WARNING: multiple messages have this Message-ID (diff)
From: kbuild test robot <lkp@intel.com>
To: Chao Fan <fanc.fnst@cn.fujitsu.com>
Cc: kbuild-all@01.org, tglx@linutronix.de, mingo@redhat.com,
	hpa@zytor.com, x86@kernel.org, linux-kernel@vger.kernel.org,
	keescook@chromium.org, bhe@redhat.com,
	indou.takao@jp.fujitsu.com, caoj.fnst@cn.fujitsu.com,
	douly.fnst@cn.fujitsu.com, Chao Fan <fanc.fnst@cn.fujitsu.com>,
	"Rafael J. Wysocki" <rjw@rjwysocki.net>,
	Len Brown <lenb@kernel.org>,
	Ard Biesheuvel <ard.biesheuvel@linaro.org>,
	linux-efi@vger.kernel.org, linux-acpi@vger.kernel.org
Subject: Re: [PATCH v6 1/3] x86/boot: Add acpitb.c to parse acpi tables
Date: Tue, 11 Sep 2018 16:52:26 +0800	[thread overview]
Message-ID: <201809111628.yudoAWlu%fengguang.wu@intel.com> (raw)
In-Reply-To: <20180910124015.18073-2-fanc.fnst@cn.fujitsu.com>

[-- Attachment #1: Type: text/plain, Size: 6851 bytes --]

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 #2: .config.gz --]
[-- Type: application/gzip, Size: 65704 bytes --]

  parent reply	other threads:[~2018-09-11  8:52 UTC|newest]

Thread overview: 22+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-09-10 12:40 [PATCH v6 0/3] x86/boot/KASLR: Parse ACPI table and limit kaslr in immovable memory Chao Fan
2018-09-10 12:40 ` [PATCH v6 1/3] x86/boot: Add acpitb.c to parse acpi tables Chao Fan
2018-09-10 12:40   ` Chao Fan
2018-09-10 20:13   ` Rafael J. Wysocki
2018-09-11  1:27     ` Chao Fan
2018-09-11  1:27       ` Chao Fan
2018-09-11  7:41       ` Rafael J. Wysocki
2018-09-11  7:52         ` Chao Fan
2018-09-11  7:52           ` Chao Fan
2018-09-11  8:52   ` [RFC PATCH] x86/boot: get_acpi_srat_table() can be static kbuild test robot
2018-09-11  8:52     ` kbuild test robot
2018-09-11  8:55     ` Chao Fan
2018-09-11  8:55       ` Chao Fan
2018-09-11  8:52   ` kbuild test robot [this message]
2018-09-11  8:52     ` [PATCH v6 1/3] x86/boot: Add acpitb.c to parse acpi tables kbuild test robot
2018-09-12  7:08   ` kbuild test robot
2018-09-12  7:08     ` kbuild test robot
2018-09-10 12:40 ` [PATCH v6 2/3] x86/boot/KASLR: Walk srat tables to filter immovable memory Chao Fan
2018-09-10 12:40 ` [PATCH v6 3/3] x86/boot/KASLR: Limit kaslr to choosing the " Chao Fan
2018-09-11  1:13 ` [PATCH v6 0/3] x86/boot/KASLR: Parse ACPI table and limit kaslr in " Chao Fan
2018-09-11  7:28   ` Rafael J. Wysocki
2018-09-11  7:53     ` Chao Fan

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=201809111628.yudoAWlu%fengguang.wu@intel.com \
    --to=lkp@intel.com \
    --cc=ard.biesheuvel@linaro.org \
    --cc=bhe@redhat.com \
    --cc=caoj.fnst@cn.fujitsu.com \
    --cc=douly.fnst@cn.fujitsu.com \
    --cc=fanc.fnst@cn.fujitsu.com \
    --cc=hpa@zytor.com \
    --cc=indou.takao@jp.fujitsu.com \
    --cc=kbuild-all@01.org \
    --cc=keescook@chromium.org \
    --cc=lenb@kernel.org \
    --cc=linux-acpi@vger.kernel.org \
    --cc=linux-efi@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@redhat.com \
    --cc=rjw@rjwysocki.net \
    --cc=tglx@linutronix.de \
    --cc=x86@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.