Messages in this thread Patch in this message | | | Date | Tue, 27 Sep 2022 14:58:25 -0700 | From | Nathan Chancellor <> | Subject | Re: [PATCH -next v2 0/2] Remove unused variables in x86/boot |
| |
On Tue, Sep 27, 2022 at 02:50:19PM -0700, Nathan Chancellor wrote: > 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>
Oops, missed another one on second glance of my build logs:
3.
arch/x86/boot/compressed/acpi.c:23:1: warning: unused function '__efi_get_rsdp_addr' [-Wunused-function] __efi_get_rsdp_addr(unsigned long cfg_tbl_pa, unsigned int cfg_tbl_len) ^ 1 warning generated.
This happens when CONFIG_EFI is disabled. This will resolve it:
diff --git a/arch/x86/boot/compressed/acpi.c b/arch/x86/boot/compressed/acpi.c index 21febd9f21ab..5f2b8966e723 100644 --- a/arch/x86/boot/compressed/acpi.c +++ b/arch/x86/boot/compressed/acpi.c @@ -19,10 +19,10 @@ */ struct mem_vector immovable_mem[MAX_NUMNODES*2]; +#ifdef CONFIG_EFI static acpi_physical_address __efi_get_rsdp_addr(unsigned long cfg_tbl_pa, unsigned int cfg_tbl_len) { -#ifdef CONFIG_EFI unsigned long rsdp_addr; /* @@ -41,9 +41,9 @@ __efi_get_rsdp_addr(unsigned long cfg_tbl_pa, unsigned int cfg_tbl_len) return (acpi_physical_address)rsdp_addr; debug_putstr("Error getting RSDP address.\n"); -#endif return 0; } +#endif static acpi_physical_address efi_get_rsdp_addr(void) {
| |