All of lore.kernel.org
 help / color / mirror / Atom feed
From: Nathan Chancellor <nathan@kernel.org>
To: Li Zetao <lizetao1@huawei.com>
Cc: tglx@linutronix.de, mingo@redhat.com, bp@alien8.de,
	dave.hansen@linux.intel.com, x86@kernel.org, hpa@zytor.com,
	kirill.shutemov@linux.intel.com, akpm@linux-foundation.org,
	ndesaulniers@google.com, masahiroy@kernel.org,
	sathyanarayanan.kuppuswamy@linux.intel.com, michael.roth@amd.com,
	brijesh.singh@amd.com, venu.busireddy@oracle.com,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH -next v2 0/2] Remove unused variables in x86/boot
Date: Tue, 27 Sep 2022 14:50:19 -0700	[thread overview]
Message-ID: <YzNwG3iWz+qfNCC9@dev-arch.thelio-3990X> (raw)
In-Reply-To: <20220927081512.2456624-1-lizetao1@huawei.com>

Hi Li,

On Tue, Sep 27, 2022 at 08:15:10AM +0000, Li Zetao wrote:
> This patch set removes some unused variables in x86/boot, and add the
> "-Wall" flag to Makefile, which is the old problem of x86 not sharing
> makefiles.
> 
> Changes since v1:
> - Add "-Wall" flag to x86/boot/compressed/Makefile
> - Remove unused variables "et" in efi_get_system_table() and "ret" in
>   efi_get_conf_table()
> - Remove unused variables "ret" in __efi_get_rsdp_addr() and 
>   "nr_tables" in efi_get_rsdp_addr()
> 
> v1 at:
> https://lore.kernel.org/all/20220923113209.3046960-1-lizetao1@huawei.com/
> 
> Li Zetao (2):
>   x86/boot/compressed: Add "-Wall" flag to Makefile
>   x86/boot: Remove unused variables
> 
>  arch/x86/boot/compressed/Makefile | 2 +-
>  arch/x86/boot/compressed/acpi.c   | 2 --
>  arch/x86/boot/compressed/efi.c    | 2 --
>  arch/x86/boot/compressed/sev.c    | 1 -
>  4 files changed, 1 insertion(+), 6 deletions(-)

I took this series for a spin with clang and found a few extra warnings.

1.

  In file included from arch/x86/boot/compressed/misc.c:15:
  In file included from arch/x86/boot/compressed/misc.h:24:
  In file included from ./include/linux/elf.h:6:
  In file included from ./arch/x86/include/asm/elf.h:8:
  In file included from ./include/linux/thread_info.h:60:
  ./arch/x86/include/asm/thread_info.h:175:13: warning: calling '__builtin_frame_address' with a nonzero argument is unsafe [-Wframe-address]
          oldframe = __builtin_frame_address(1);
                     ^~~~~~~~~~~~~~~~~~~~~~~~~~
  ./arch/x86/include/asm/thread_info.h:177:11: warning: calling '__builtin_frame_address' with a nonzero argument is unsafe [-Wframe-address]
                  frame = __builtin_frame_address(2);
                          ^~~~~~~~~~~~~~~~~~~~~~~~~~

This warning is disabled in the main Makefile for this reason so we
should just be able to disable it:

diff --git a/arch/x86/boot/compressed/Makefile b/arch/x86/boot/compressed/Makefile
index 10abb7c45d04..3f004567f3d5 100644
--- a/arch/x86/boot/compressed/Makefile
+++ b/arch/x86/boot/compressed/Makefile
@@ -43,6 +43,7 @@ KBUILD_CFLAGS += -mno-mmx -mno-sse
 KBUILD_CFLAGS += -ffreestanding -fshort-wchar
 KBUILD_CFLAGS += -fno-stack-protector
 KBUILD_CFLAGS += $(call cc-disable-warning, address-of-packed-member)
+KBUILD_CFLAGS += $(call cc-disable-warning, frame-address)
 KBUILD_CFLAGS += $(call cc-disable-warning, gnu)
 KBUILD_CFLAGS += -Wno-pointer-sign
 KBUILD_CFLAGS += $(call cc-option,-fmacro-prefix-map=$(srctree)/=)

2.

  arch/x86/boot/compressed/kaslr.c:627:6: warning: unused variable 'i' [-Wunused-variable]
          int i;
              ^

This happens when CONFIG_MEMORY_HOTREMOVE or CONFIG_ACPI are 'n'. I
think it can just be fixed by aligning arch/x86/boot/compressed with the
rest of the kernel and explicitly compiling with '-std=gnu11', which
will allow us to declare the variable within the for loop, like so.

diff --git a/arch/x86/boot/compressed/Makefile b/arch/x86/boot/compressed/Makefile
index 3f004567f3d5..6c7e366a437b 100644
--- a/arch/x86/boot/compressed/Makefile
+++ b/arch/x86/boot/compressed/Makefile
@@ -34,7 +34,7 @@ targets := vmlinux vmlinux.bin vmlinux.bin.gz vmlinux.bin.bz2 vmlinux.bin.lzma \
 # be valid.
 KBUILD_CFLAGS := -m$(BITS) -O2 $(CLANG_FLAGS)
 KBUILD_CFLAGS += -fno-strict-aliasing -fPIE
-KBUILD_CFLAGS += -Wundef -Wall
+KBUILD_CFLAGS += -Wundef -Wall -std=gnu11
 KBUILD_CFLAGS += -DDISABLE_BRANCH_PROFILING
 cflags-$(CONFIG_X86_32) := -march=i386
 cflags-$(CONFIG_X86_64) := -mcmodel=small -mno-red-zone
diff --git a/arch/x86/boot/compressed/kaslr.c b/arch/x86/boot/compressed/kaslr.c
index 4a3f223973f4..be859c7e7f6b 100644
--- a/arch/x86/boot/compressed/kaslr.c
+++ b/arch/x86/boot/compressed/kaslr.c
@@ -624,7 +624,6 @@ static bool process_mem_region(struct mem_vector *region,
 			       unsigned long minimum,
 			       unsigned long image_size)
 {
-	int i;
 	/*
 	 * If no immovable memory found, or MEMORY_HOTREMOVE disabled,
 	 * use @region directly.
@@ -644,7 +643,7 @@ static bool process_mem_region(struct mem_vector *region,
 	 * If immovable memory found, filter the intersection between
 	 * immovable memory and @region.
 	 */
-	for (i = 0; i < num_immovable_mem; i++) {
+	for (int i = 0; i < num_immovable_mem; i++) {
 		u64 start, end, entry_end, region_end;
 		struct mem_vector entry;
 

Additionally, I think these two patches should be reordered so that the
warnings are fixed before they are enabled.

With those comments addressed, consider the series:

Reviewed-by: Nathan Chancellor <nathan@kernel.org>

Cheers,
Nathan

  parent reply	other threads:[~2022-09-27 21:50 UTC|newest]

Thread overview: 19+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-09-27  8:15 [PATCH -next v2 0/2] Remove unused variables in x86/boot Li Zetao
2022-09-27  8:15 ` [PATCH -next v2 1/2] x86/boot/compressed: Add "-Wall" flag to Makefile Li Zetao
2022-09-27  8:15 ` [PATCH -next v2 2/2] x86/boot: Remove unused variables Li Zetao
2022-09-27 21:50 ` Nathan Chancellor [this message]
2022-09-27 21:58   ` [PATCH -next v2 0/2] Remove unused variables in x86/boot Nathan Chancellor
2022-09-29  2:16   ` Li Zetao
2022-09-29 16:59     ` Nathan Chancellor
2022-09-30  3:27 ` [PATCH -next v3 " Li Zetao
2022-09-30  3:27   ` [PATCH -next v3 1/2] x86/boot/compressed: Add "-Wall" flag to Makefile Li Zetao
2022-09-30  3:27   ` [PATCH -next v3 2/2] x86/boot: Remove unused variables Li Zetao
2022-10-08 13:41   ` Ping: [PATCH -next v3 0/2] Remove unused variables in x86/boot Li Zetao
2022-10-09 15:17     ` Kees Cook
2022-10-11  1:29       ` [PATCH -next v4 " Li Zetao
2022-10-11  1:29         ` [PATCH -next v4 1/2] x86/boot: Remove unused variables Li Zetao
2022-10-11 21:15           ` Nick Desaulniers
2022-10-11 21:22           ` Nick Desaulniers
2022-10-11  1:29         ` [PATCH -next v4 2/2] x86/boot/compressed: Add "-Wall" flag to Makefile Li Zetao
2022-10-11 21:20           ` Nick Desaulniers
2022-10-17 17:01           ` Borislav Petkov

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=YzNwG3iWz+qfNCC9@dev-arch.thelio-3990X \
    --to=nathan@kernel.org \
    --cc=akpm@linux-foundation.org \
    --cc=bp@alien8.de \
    --cc=brijesh.singh@amd.com \
    --cc=dave.hansen@linux.intel.com \
    --cc=hpa@zytor.com \
    --cc=kirill.shutemov@linux.intel.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=lizetao1@huawei.com \
    --cc=masahiroy@kernel.org \
    --cc=michael.roth@amd.com \
    --cc=mingo@redhat.com \
    --cc=ndesaulniers@google.com \
    --cc=sathyanarayanan.kuppuswamy@linux.intel.com \
    --cc=tglx@linutronix.de \
    --cc=venu.busireddy@oracle.com \
    --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.