[PATCH -next v2 0/2] Remove unused variables in x86/boot

[PATCH -next v2 0/2] Remove unused variables in x86/boot

[Li Zetao]

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(-)

-- 
2.34.1

[PATCH -next v2 1/2] x86/boot/compressed: Add "-Wall" flag to Makefile

[Li Zetao]

Compressed/Makefile does not have "-Wall" flag, this is the old problem of
x86 not sharing makefiles. Fix by adding "-Wall" flag to Makefile.

Signed-off-by: Li Zetao <lizetao1@huawei.com>
---
v1 -> v2: patch is new

 arch/x86/boot/compressed/Makefile | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/x86/boot/compressed/Makefile b/arch/x86/boot/compressed/Makefile
index 3a261abb6d15..bcef0554b560 100644
--- a/arch/x86/boot/compressed/Makefile
+++ b/arch/x86/boot/compressed/Makefile
@@ -35,7 +35,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
+KBUILD_CFLAGS += -Wundef -Wall
 KBUILD_CFLAGS += -DDISABLE_BRANCH_PROFILING
 cflags-$(CONFIG_X86_32) := -march=i386
 cflags-$(CONFIG_X86_64) := -mcmodel=small -mno-red-zone
-- 
2.34.1

[PATCH -next v2 2/2] x86/boot: Remove unused variables

[Li Zetao]

Gcc report warning as follows:

arch/x86/boot/compressed/efi.c: In function ‘efi_get_system_table’:
arch/x86/boot/compressed/efi.c:62:23: warning: unused variable ‘et’
  [-Wunused-variable]

arch/x86/boot/compressed/efi.c: In function ‘efi_get_conf_table’:
arch/x86/boot/compressed/efi.c:134:13: warning: unused variable
  ‘ret’ [-Wunused-variable]

arch/x86/boot/compressed/acpi.c: In function ‘__efi_get_rsdp_addr’:
arch/x86/boot/compressed/acpi.c:27:13: warning: unused variable
  ‘ret’ [-Wunused-variable]

arch/x86/boot/compressed/acpi.c: In function ‘efi_get_rsdp_addr’:
arch/x86/boot/compressed/acpi.c:55:22: warning: unused variable
  ‘nr_tables’ [-Wunused-variable]

arch/x86/boot/compressed/sev.c: In function ‘enforce_vmpl0’:
arch/x86/boot/compressed/sev.c:256:13: error: unused variable ‘err’
  [-Werror=unused-variable]

Fix these warnings by removing unused variables.

Fixes: 58f3e6b71f42 ("x86/compressed/acpi: Move EFI system table lookup to helper")
Fixes: 61c14ceda840 ("x86/compressed/acpi: Move EFI config table lookup to helper")
Fixes: dee602dd5d14 ("x86/compressed/acpi: Move EFI vendor table lookup to helper")
Fixes: f9d230e893e8 ("x86/boot: Correct RSDP parsing with 32-bit EFI")
Fixes: 81cc3df9a90e ("x86/sev: Check the VMPL level")
Signed-off-by: Li Zetao <lizetao1@huawei.com>
---
v1 -> v2: Remove unused variables "et" in efi_get_system_table(), "ret" in 
efi_get_conf_table(), "ret" in __efi_get_rsdp_addr() and  "nr_tables" in 
efi_get_rsdp_addr().

 arch/x86/boot/compressed/acpi.c | 2 --
 arch/x86/boot/compressed/efi.c  | 2 --
 arch/x86/boot/compressed/sev.c  | 1 -
 3 files changed, 5 deletions(-)

diff --git a/arch/x86/boot/compressed/acpi.c b/arch/x86/boot/compressed/acpi.c
index 9caf89063e77..21febd9f21ab 100644
--- a/arch/x86/boot/compressed/acpi.c
+++ b/arch/x86/boot/compressed/acpi.c
@@ -24,7 +24,6 @@ __efi_get_rsdp_addr(unsigned long cfg_tbl_pa, unsigned int cfg_tbl_len)
 {
 #ifdef CONFIG_EFI
 	unsigned long rsdp_addr;
-	int ret;
 
 	/*
 	 * Search EFI system tables for RSDP. Preferred is ACPI_20_TABLE_GUID to
@@ -52,7 +51,6 @@ static acpi_physical_address efi_get_rsdp_addr(void)
 	unsigned long cfg_tbl_pa = 0;
 	unsigned int cfg_tbl_len;
 	unsigned long systab_pa;
-	unsigned int nr_tables;
 	enum efi_type et;
 	int ret;
 
diff --git a/arch/x86/boot/compressed/efi.c b/arch/x86/boot/compressed/efi.c
index 6edd034b0b30..6ffd22710ed2 100644
--- a/arch/x86/boot/compressed/efi.c
+++ b/arch/x86/boot/compressed/efi.c
@@ -59,7 +59,6 @@ unsigned long efi_get_system_table(struct boot_params *bp)
 {
 	unsigned long sys_tbl_pa;
 	struct efi_info *ei;
-	enum efi_type et;
 
 	/* Get systab from boot params. */
 	ei = &bp->efi_info;
@@ -131,7 +130,6 @@ int efi_get_conf_table(struct boot_params *bp, unsigned long *cfg_tbl_pa,
 {
 	unsigned long sys_tbl_pa;
 	enum efi_type et;
-	int ret;
 
 	if (!cfg_tbl_pa || !cfg_tbl_len)
 		return -EINVAL;
diff --git a/arch/x86/boot/compressed/sev.c b/arch/x86/boot/compressed/sev.c
index c93930d5ccbd..b9451761a69a 100644
--- a/arch/x86/boot/compressed/sev.c
+++ b/arch/x86/boot/compressed/sev.c
@@ -253,7 +253,6 @@ void do_boot_stage2_vc(struct pt_regs *regs, unsigned long exit_code)
 static void enforce_vmpl0(void)
 {
 	u64 attrs;
-	int err;
 
 	/*
 	 * RMPADJUST modifies RMP permissions of a lesser-privileged (numerically
-- 
2.34.1

Re: [PATCH -next v2 0/2] Remove unused variables in x86/boot

[Nathan Chancellor]

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

Re: [PATCH -next v2 0/2] Remove unused variables in x86/boot

[Nathan Chancellor]

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)
 {

Re: [PATCH -next v2 0/2] Remove unused variables in x86/boot

[Li Zetao]

Hi Nathan,

On 2022/9/28 5:50, 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>
>
> Cheers,
> Nathan

What is your clang compiler version and .config? My clang compiler is 
14.0.0-1ubuntu1, and I can't reproduce

problem 1("calling '__builtin_frame_address' with a nonzero argument is 
unsafe"). Can you provide more

information.


Thx.


Best regards,

Li Zetao

Re: [PATCH -next v2 0/2] Remove unused variables in x86/boot

[Nathan Chancellor]

On Thu, Sep 29, 2022 at 10:16:11AM +0800, Li Zetao wrote:
> Hi Nathan,
> 
> On 2022/9/28 5:50, 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>
> > 
> > Cheers,
> > Nathan
> 
> What is your clang compiler version and .config? My clang compiler is
> 14.0.0-1ubuntu1, and I can't reproduce
> 
> problem 1("calling '__builtin_frame_address' with a nonzero argument is
> unsafe"). Can you provide more
> 
> information.

Sure thing! My compiler is tip of tree LLVM, so clang 16.0.0. That
should not matter for this particular issue though. I can reproduce it
with my distribution's clang (14.0.6) with a configuration that has
CONFIG_FRAME_POINTER enabled; allnoconfig works for me here.

  $ clang --version
  clang version 14.0.6
  Target: x86_64-pc-linux-gnu
  Thread model: posix
  InstalledDir: /usr/bin

  $ make -skj"$(nproc)" ARCH=i386 LLVM=1 clean allnoconfig all
  ...
  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);
                          ^~~~~~~~~~~~~~~~~~~~~~~~~~
  ...

  $ grep CONFIG_FRAME_POINTER .config
  CONFIG_FRAME_POINTER=y

If there is any other information I can provide, please let me know!

Cheers,
Nathan

[PATCH -next v3 0/2] Remove unused variables in x86/boot

[Li Zetao]

This series 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 v2:
- Add "frame-address" flag and "-std=gnu11" to 
  x86/boot/compressed/Makefile to fix warnings when "-Wall" flag added.
- Declare the variable "i" within the for loop to reslove unused 
  variable warning.
- Delete __efi_get_rsdp_addr function when CONFIG_EFI is disabled to 
  resolve unused function warning.

v2 at:
https://lore.kernel.org/all/20220927081512.2456624-1-lizetao1@huawei.com/

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 | 3 ++-
 arch/x86/boot/compressed/acpi.c   | 7 +++----
 arch/x86/boot/compressed/efi.c    | 2 --
 arch/x86/boot/compressed/kaslr.c  | 3 +--
 arch/x86/boot/compressed/sev.c    | 1 -
 5 files changed, 6 insertions(+), 10 deletions(-)

-- 
2.34.1

[PATCH -next v3 1/2] x86/boot/compressed: Add "-Wall" flag to Makefile

[Li Zetao]

Compressed/Makefile does not have "-Wall" flag, this is the old problem of
x86 not sharing makefiles. Fix by adding "-Wall" flag to Makefile. But when
"-Wall" flag added to Makefile, a few extra warnings were found.

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, adding "frame-address" flag to
Makefile.

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".
Fix by adding "-std=gnu11" flag to Makefile, and we should put
the variable "i" within the for loop.

3.
arch/x86/boot/compressed/acpi.c:23:1: warning: unused function
  "__efi_get_rsdp_addr" [-Wunused-function]

This happens when CONFIG_EFI is disabled for the reason that
function "__efi_get_rsdp_addr" is only called in efi_get_rsdp_addr
when CONFIG_EFI enable. So function "__efi_get_rsdp_addr" should
not be defined when CONFIG_EFI is disabled.

Signed-off-by: Li Zetao <lizetao1@huawei.com>
Reviewed-by: Nathan Chancellor <nathan@kernel.org>
---
v1 -> v2: patch is new
v2 -> v3: resolve extra warnings after "-Wall" flag added.

 arch/x86/boot/compressed/Makefile | 3 ++-
 arch/x86/boot/compressed/acpi.c   | 5 +++--
 arch/x86/boot/compressed/kaslr.c  | 3 +--
 3 files changed, 6 insertions(+), 5 deletions(-)

diff --git a/arch/x86/boot/compressed/Makefile b/arch/x86/boot/compressed/Makefile
index 3a261abb6d15..8918a8306dff 100644
--- a/arch/x86/boot/compressed/Makefile
+++ b/arch/x86/boot/compressed/Makefile
@@ -35,7 +35,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
+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
@@ -44,6 +44,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)/=)
diff --git a/arch/x86/boot/compressed/acpi.c b/arch/x86/boot/compressed/acpi.c
index 9caf89063e77..79742ab34e3f 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;
 	int ret;
 
@@ -42,9 +42,10 @@ __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)
 {
diff --git a/arch/x86/boot/compressed/kaslr.c b/arch/x86/boot/compressed/kaslr.c
index e476bcbd9b42..4abc9c42cf4d 100644
--- a/arch/x86/boot/compressed/kaslr.c
+++ b/arch/x86/boot/compressed/kaslr.c
@@ -625,7 +625,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.
@@ -645,7 +644,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;
 
-- 
2.34.1

[PATCH -next v3 2/2] x86/boot: Remove unused variables

[Li Zetao]

Gcc report warning as follows:

arch/x86/boot/compressed/efi.c: In function ‘efi_get_system_table’:
arch/x86/boot/compressed/efi.c:62:23: warning: unused variable ‘et’
  [-Wunused-variable]

arch/x86/boot/compressed/efi.c: In function ‘efi_get_conf_table’:
arch/x86/boot/compressed/efi.c:134:13: warning: unused variable
  ‘ret’ [-Wunused-variable]

arch/x86/boot/compressed/acpi.c: In function ‘__efi_get_rsdp_addr’:
arch/x86/boot/compressed/acpi.c:27:13: warning: unused variable
  ‘ret’ [-Wunused-variable]

arch/x86/boot/compressed/acpi.c: In function ‘efi_get_rsdp_addr’:
arch/x86/boot/compressed/acpi.c:55:22: warning: unused variable
  ‘nr_tables’ [-Wunused-variable]

arch/x86/boot/compressed/sev.c: In function ‘enforce_vmpl0’:
arch/x86/boot/compressed/sev.c:256:13: error: unused variable ‘err’
  [-Werror=unused-variable]

Fix these warnings by removing unused variables.

Fixes: 58f3e6b71f42 ("x86/compressed/acpi: Move EFI system table lookup to helper")
Fixes: 61c14ceda840 ("x86/compressed/acpi: Move EFI config table lookup to helper")
Fixes: dee602dd5d14 ("x86/compressed/acpi: Move EFI vendor table lookup to helper")
Fixes: f9d230e893e8 ("x86/boot: Correct RSDP parsing with 32-bit EFI")
Fixes: 81cc3df9a90e ("x86/sev: Check the VMPL level")
Signed-off-by: Li Zetao <lizetao1@huawei.com>
Reviewed-by: Nathan Chancellor <nathan@kernel.org>
---
v1 -> v2: Remove unused variables "et" in efi_get_system_table(), "ret" in 
efi_get_conf_table(), "ret" in __efi_get_rsdp_addr() and  "nr_tables" in 
efi_get_rsdp_addr().
v2 -> v3: none

 arch/x86/boot/compressed/acpi.c | 2 --
 arch/x86/boot/compressed/efi.c  | 2 --
 arch/x86/boot/compressed/sev.c  | 1 -
 3 files changed, 5 deletions(-)

diff --git a/arch/x86/boot/compressed/acpi.c b/arch/x86/boot/compressed/acpi.c
index 79742ab34e3f..ee7e689d4e0f 100644
--- a/arch/x86/boot/compressed/acpi.c
+++ b/arch/x86/boot/compressed/acpi.c
@@ -24,7 +24,6 @@ static acpi_physical_address
 __efi_get_rsdp_addr(unsigned long cfg_tbl_pa, unsigned int cfg_tbl_len)
 {
 	unsigned long rsdp_addr;
-	int ret;
 
 	/*
 	 * Search EFI system tables for RSDP. Preferred is ACPI_20_TABLE_GUID to
@@ -53,7 +52,6 @@ static acpi_physical_address efi_get_rsdp_addr(void)
 	unsigned long cfg_tbl_pa = 0;
 	unsigned int cfg_tbl_len;
 	unsigned long systab_pa;
-	unsigned int nr_tables;
 	enum efi_type et;
 	int ret;
 
diff --git a/arch/x86/boot/compressed/efi.c b/arch/x86/boot/compressed/efi.c
index 6edd034b0b30..6ffd22710ed2 100644
--- a/arch/x86/boot/compressed/efi.c
+++ b/arch/x86/boot/compressed/efi.c
@@ -59,7 +59,6 @@ unsigned long efi_get_system_table(struct boot_params *bp)
 {
 	unsigned long sys_tbl_pa;
 	struct efi_info *ei;
-	enum efi_type et;
 
 	/* Get systab from boot params. */
 	ei = &bp->efi_info;
@@ -131,7 +130,6 @@ int efi_get_conf_table(struct boot_params *bp, unsigned long *cfg_tbl_pa,
 {
 	unsigned long sys_tbl_pa;
 	enum efi_type et;
-	int ret;
 
 	if (!cfg_tbl_pa || !cfg_tbl_len)
 		return -EINVAL;
diff --git a/arch/x86/boot/compressed/sev.c b/arch/x86/boot/compressed/sev.c
index c93930d5ccbd..b9451761a69a 100644
--- a/arch/x86/boot/compressed/sev.c
+++ b/arch/x86/boot/compressed/sev.c
@@ -253,7 +253,6 @@ void do_boot_stage2_vc(struct pt_regs *regs, unsigned long exit_code)
 static void enforce_vmpl0(void)
 {
 	u64 attrs;
-	int err;
 
 	/*
 	 * RMPADJUST modifies RMP permissions of a lesser-privileged (numerically
-- 
2.34.1

Ping: [PATCH -next v3 0/2] Remove unused variables in x86/boot

[Li Zetao]

On 2022/9/30 11:27, Li Zetao wrote:
> This series 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 v2:
> - Add "frame-address" flag and "-std=gnu11" to
>    x86/boot/compressed/Makefile to fix warnings when "-Wall" flag added.
> - Declare the variable "i" within the for loop to reslove unused
>    variable warning.
> - Delete __efi_get_rsdp_addr function when CONFIG_EFI is disabled to
>    resolve unused function warning.
>
> v2 at:
> https://lore.kernel.org/all/20220927081512.2456624-1-lizetao1@huawei.com/
>
> 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 | 3 ++-
>   arch/x86/boot/compressed/acpi.c   | 7 +++----
>   arch/x86/boot/compressed/efi.c    | 2 --
>   arch/x86/boot/compressed/kaslr.c  | 3 +--
>   arch/x86/boot/compressed/sev.c    | 1 -
>   5 files changed, 6 insertions(+), 10 deletions(-)

PING.


Best regards,

Li Zetao

Re: Ping: [PATCH -next v3 0/2] Remove unused variables in x86/boot

[Kees Cook]

On Sat, Oct 08, 2022 at 09:41:59PM +0800, Li Zetao wrote:
> On 2022/9/30 11:27, Li Zetao wrote:
> > This series 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 v2:
> > - Add "frame-address" flag and "-std=gnu11" to
> >    x86/boot/compressed/Makefile to fix warnings when "-Wall" flag added.
> > - Declare the variable "i" within the for loop to reslove unused
> >    variable warning.
> > - Delete __efi_get_rsdp_addr function when CONFIG_EFI is disabled to
> >    resolve unused function warning.

Nathan suggested earlier (and I agree): please re-order these patches so
the fixes for the new warnings are first, and then turn on the compiler
flags in a final patch. This will keep the build "warning free" at all
steps.

-- 
Kees Cook

[PATCH -next v4 0/2] Remove unused variables in x86/boot

[Li Zetao]

This series 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 v3:
- Re-order the patches, so the fixes for the new warnings are first,
  and then turn on the compiler flags in a final patch

v3 at:
https://lore.kernel.org/all/20220930032727.3451619-1-lizetao1@huawei.com/

Changes since v2:
- Add "frame-address" flag and "-std=gnu11" to
  x86/boot/compressed/Makefile to fix warnings when "-Wall" flag added.
- Declare the variable "i" within the for loop to reslove unused
  variable warning.
- Delete __efi_get_rsdp_addr function when CONFIG_EFI is disabled to
  resolve unused function warning.

v2 at:
https://lore.kernel.org/all/20220927081512.2456624-1-lizetao1@huawei.com/

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: Remove unused variables
  x86/boot/compressed: Add "-Wall" flag to Makefile

 arch/x86/boot/compressed/Makefile | 3 ++-
 arch/x86/boot/compressed/acpi.c   | 7 +++----
 arch/x86/boot/compressed/efi.c    | 2 --
 arch/x86/boot/compressed/kaslr.c  | 3 +--
 arch/x86/boot/compressed/sev.c    | 1 -
 5 files changed, 6 insertions(+), 10 deletions(-)

-- 
2.34.1

[PATCH -next v4 1/2] x86/boot: Remove unused variables

[Li Zetao]

Gcc report warning as follows:

arch/x86/boot/compressed/efi.c: In function ‘efi_get_system_table’:
arch/x86/boot/compressed/efi.c:62:23: warning: unused variable ‘et’
  [-Wunused-variable]

arch/x86/boot/compressed/efi.c: In function ‘efi_get_conf_table’:
arch/x86/boot/compressed/efi.c:134:13: warning: unused variable
  ‘ret’ [-Wunused-variable]

arch/x86/boot/compressed/acpi.c: In function ‘__efi_get_rsdp_addr’:
arch/x86/boot/compressed/acpi.c:27:13: warning: unused variable
  ‘ret’ [-Wunused-variable]

arch/x86/boot/compressed/acpi.c: In function ‘efi_get_rsdp_addr’:
arch/x86/boot/compressed/acpi.c:55:22: warning: unused variable
  ‘nr_tables’ [-Wunused-variable]

arch/x86/boot/compressed/sev.c: In function ‘enforce_vmpl0’:
arch/x86/boot/compressed/sev.c:256:13: error: unused variable ‘err’
  [-Werror=unused-variable]

Fix these warnings by removing unused variables.

Fixes: 58f3e6b71f42 ("x86/compressed/acpi: Move EFI system table lookup to helper")
Fixes: 61c14ceda840 ("x86/compressed/acpi: Move EFI config table lookup to helper")
Fixes: dee602dd5d14 ("x86/compressed/acpi: Move EFI vendor table lookup to helper")
Fixes: f9d230e893e8 ("x86/boot: Correct RSDP parsing with 32-bit EFI")
Fixes: 81cc3df9a90e ("x86/sev: Check the VMPL level")
Signed-off-by: Li Zetao <lizetao1@huawei.com>
---
v1 -> v2: Remove unused variables "et" in efi_get_system_table(), "ret" in
efi_get_conf_table(), "ret" in __efi_get_rsdp_addr() and  "nr_tables" in
efi_get_rsdp_addr()
v2 -> v3: None
v3 -> v4: Put this patch in front

 arch/x86/boot/compressed/acpi.c | 2 --
 arch/x86/boot/compressed/efi.c  | 2 --
 arch/x86/boot/compressed/sev.c  | 1 -
 3 files changed, 5 deletions(-)

diff --git a/arch/x86/boot/compressed/acpi.c b/arch/x86/boot/compressed/acpi.c
index 9caf89063e77..21febd9f21ab 100644
--- a/arch/x86/boot/compressed/acpi.c
+++ b/arch/x86/boot/compressed/acpi.c
@@ -24,7 +24,6 @@ __efi_get_rsdp_addr(unsigned long cfg_tbl_pa, unsigned int cfg_tbl_len)
 {
 #ifdef CONFIG_EFI
 	unsigned long rsdp_addr;
-	int ret;
 
 	/*
 	 * Search EFI system tables for RSDP. Preferred is ACPI_20_TABLE_GUID to
@@ -52,7 +51,6 @@ static acpi_physical_address efi_get_rsdp_addr(void)
 	unsigned long cfg_tbl_pa = 0;
 	unsigned int cfg_tbl_len;
 	unsigned long systab_pa;
-	unsigned int nr_tables;
 	enum efi_type et;
 	int ret;
 
diff --git a/arch/x86/boot/compressed/efi.c b/arch/x86/boot/compressed/efi.c
index 6edd034b0b30..6ffd22710ed2 100644
--- a/arch/x86/boot/compressed/efi.c
+++ b/arch/x86/boot/compressed/efi.c
@@ -59,7 +59,6 @@ unsigned long efi_get_system_table(struct boot_params *bp)
 {
 	unsigned long sys_tbl_pa;
 	struct efi_info *ei;
-	enum efi_type et;
 
 	/* Get systab from boot params. */
 	ei = &bp->efi_info;
@@ -131,7 +130,6 @@ int efi_get_conf_table(struct boot_params *bp, unsigned long *cfg_tbl_pa,
 {
 	unsigned long sys_tbl_pa;
 	enum efi_type et;
-	int ret;
 
 	if (!cfg_tbl_pa || !cfg_tbl_len)
 		return -EINVAL;
diff --git a/arch/x86/boot/compressed/sev.c b/arch/x86/boot/compressed/sev.c
index c93930d5ccbd..b9451761a69a 100644
--- a/arch/x86/boot/compressed/sev.c
+++ b/arch/x86/boot/compressed/sev.c
@@ -253,7 +253,6 @@ void do_boot_stage2_vc(struct pt_regs *regs, unsigned long exit_code)
 static void enforce_vmpl0(void)
 {
 	u64 attrs;
-	int err;
 
 	/*
 	 * RMPADJUST modifies RMP permissions of a lesser-privileged (numerically
-- 
2.34.1

[PATCH -next v4 2/2] x86/boot/compressed: Add "-Wall" flag to Makefile

[Li Zetao]

Compressed/Makefile does not have "-Wall" flag, this is the old problem of
x86 not sharing makefiles. Fix by adding "-Wall" flag to Makefile. But when
"-Wall" flag added to Makefile, a few extra warnings were found.

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, adding "frame-address" flag to
Makefile.

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".
Fix by adding "-std=gnu11" flag to Makefile, and we should put
the variable "i" within the for loop.

3.
arch/x86/boot/compressed/acpi.c:23:1: warning: unused function
  "__efi_get_rsdp_addr" [-Wunused-function]

This happens when CONFIG_EFI is disabled for the reason that
function "__efi_get_rsdp_addr" is only called in efi_get_rsdp_addr
when CONFIG_EFI enable. So function "__efi_get_rsdp_addr" should
not be defined when CONFIG_EFI is disabled.

Signed-off-by: Li Zetao <lizetao1@huawei.com>
---
v1 -> v2: Patch is new
v2 -> v3: Resolve extra warnings after "-Wall" flag added
v3 -> v4: Put this patch at the end

 arch/x86/boot/compressed/Makefile | 3 ++-
 arch/x86/boot/compressed/acpi.c   | 5 +++--
 arch/x86/boot/compressed/kaslr.c  | 3 +--
 3 files changed, 6 insertions(+), 5 deletions(-)

diff --git a/arch/x86/boot/compressed/Makefile b/arch/x86/boot/compressed/Makefile
index 3a261abb6d15..8918a8306dff 100644
--- a/arch/x86/boot/compressed/Makefile
+++ b/arch/x86/boot/compressed/Makefile
@@ -35,7 +35,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
+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
@@ -44,6 +44,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)/=)
diff --git a/arch/x86/boot/compressed/acpi.c b/arch/x86/boot/compressed/acpi.c
index 21febd9f21ab..c062a8230e9c 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,10 @@ __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)
 {
diff --git a/arch/x86/boot/compressed/kaslr.c b/arch/x86/boot/compressed/kaslr.c
index e476bcbd9b42..4abc9c42cf4d 100644
--- a/arch/x86/boot/compressed/kaslr.c
+++ b/arch/x86/boot/compressed/kaslr.c
@@ -625,7 +625,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.
@@ -645,7 +644,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;
 
-- 
2.34.1

Re: [PATCH -next v4 1/2] x86/boot: Remove unused variables

[Nick Desaulniers]

On Mon, Oct 10, 2022 at 6:32 PM Li Zetao <lizetao1@huawei.com> wrote:
>
> Gcc report warning as follows:
>
> arch/x86/boot/compressed/efi.c: In function ‘efi_get_system_table’:
> arch/x86/boot/compressed/efi.c:62:23: warning: unused variable ‘et’
>   [-Wunused-variable]
>
> arch/x86/boot/compressed/efi.c: In function ‘efi_get_conf_table’:
> arch/x86/boot/compressed/efi.c:134:13: warning: unused variable
>   ‘ret’ [-Wunused-variable]
>
> arch/x86/boot/compressed/acpi.c: In function ‘__efi_get_rsdp_addr’:
> arch/x86/boot/compressed/acpi.c:27:13: warning: unused variable
>   ‘ret’ [-Wunused-variable]
>
> arch/x86/boot/compressed/acpi.c: In function ‘efi_get_rsdp_addr’:
> arch/x86/boot/compressed/acpi.c:55:22: warning: unused variable
>   ‘nr_tables’ [-Wunused-variable]
>
> arch/x86/boot/compressed/sev.c: In function ‘enforce_vmpl0’:
> arch/x86/boot/compressed/sev.c:256:13: error: unused variable ‘err’
>   [-Werror=unused-variable]
>
> Fix these warnings by removing unused variables.
>
> Fixes: 58f3e6b71f42 ("x86/compressed/acpi: Move EFI system table lookup to helper")
> Fixes: 61c14ceda840 ("x86/compressed/acpi: Move EFI config table lookup to helper")
> Fixes: dee602dd5d14 ("x86/compressed/acpi: Move EFI vendor table lookup to helper")
> Fixes: f9d230e893e8 ("x86/boot: Correct RSDP parsing with 32-bit EFI")
> Fixes: 81cc3df9a90e ("x86/sev: Check the VMPL level")
> Signed-off-by: Li Zetao <lizetao1@huawei.com>

Thanks for the patch!
Reviewed-by: Nick Desaulniers <ndesaulniers@google.com>

> ---
> v1 -> v2: Remove unused variables "et" in efi_get_system_table(), "ret" in
> efi_get_conf_table(), "ret" in __efi_get_rsdp_addr() and  "nr_tables" in
> efi_get_rsdp_addr()
> v2 -> v3: None
> v3 -> v4: Put this patch in front
>
>  arch/x86/boot/compressed/acpi.c | 2 --
>  arch/x86/boot/compressed/efi.c  | 2 --
>  arch/x86/boot/compressed/sev.c  | 1 -
>  3 files changed, 5 deletions(-)
>
> diff --git a/arch/x86/boot/compressed/acpi.c b/arch/x86/boot/compressed/acpi.c
> index 9caf89063e77..21febd9f21ab 100644
> --- a/arch/x86/boot/compressed/acpi.c
> +++ b/arch/x86/boot/compressed/acpi.c
> @@ -24,7 +24,6 @@ __efi_get_rsdp_addr(unsigned long cfg_tbl_pa, unsigned int cfg_tbl_len)
>  {
>  #ifdef CONFIG_EFI
>         unsigned long rsdp_addr;
> -       int ret;
>
>         /*
>          * Search EFI system tables for RSDP. Preferred is ACPI_20_TABLE_GUID to
> @@ -52,7 +51,6 @@ static acpi_physical_address efi_get_rsdp_addr(void)
>         unsigned long cfg_tbl_pa = 0;
>         unsigned int cfg_tbl_len;
>         unsigned long systab_pa;
> -       unsigned int nr_tables;
>         enum efi_type et;
>         int ret;
>
> diff --git a/arch/x86/boot/compressed/efi.c b/arch/x86/boot/compressed/efi.c
> index 6edd034b0b30..6ffd22710ed2 100644
> --- a/arch/x86/boot/compressed/efi.c
> +++ b/arch/x86/boot/compressed/efi.c
> @@ -59,7 +59,6 @@ unsigned long efi_get_system_table(struct boot_params *bp)
>  {
>         unsigned long sys_tbl_pa;
>         struct efi_info *ei;
> -       enum efi_type et;
>
>         /* Get systab from boot params. */
>         ei = &bp->efi_info;
> @@ -131,7 +130,6 @@ int efi_get_conf_table(struct boot_params *bp, unsigned long *cfg_tbl_pa,
>  {
>         unsigned long sys_tbl_pa;
>         enum efi_type et;
> -       int ret;
>
>         if (!cfg_tbl_pa || !cfg_tbl_len)
>                 return -EINVAL;
> diff --git a/arch/x86/boot/compressed/sev.c b/arch/x86/boot/compressed/sev.c
> index c93930d5ccbd..b9451761a69a 100644
> --- a/arch/x86/boot/compressed/sev.c
> +++ b/arch/x86/boot/compressed/sev.c
> @@ -253,7 +253,6 @@ void do_boot_stage2_vc(struct pt_regs *regs, unsigned long exit_code)
>  static void enforce_vmpl0(void)
>  {
>         u64 attrs;
> -       int err;
>
>         /*
>          * RMPADJUST modifies RMP permissions of a lesser-privileged (numerically
> --
> 2.34.1
>


-- 
Thanks,
~Nick Desaulniers

Re: [PATCH -next v4 2/2] x86/boot/compressed: Add "-Wall" flag to Makefile

[Nick Desaulniers]

On Mon, Oct 10, 2022 at 6:32 PM Li Zetao <lizetao1@huawei.com> wrote:
>
> Compressed/Makefile does not have "-Wall" flag, this is the old problem of
> x86 not sharing makefiles. Fix by adding "-Wall" flag to Makefile. But when
> "-Wall" flag added to Makefile, a few extra warnings were found.
>
> 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, adding "frame-address" flag to
> Makefile.
>
> 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".
> Fix by adding "-std=gnu11" flag to Makefile, and we should put
> the variable "i" within the for loop.
>
> 3.
> arch/x86/boot/compressed/acpi.c:23:1: warning: unused function
>   "__efi_get_rsdp_addr" [-Wunused-function]
>
> This happens when CONFIG_EFI is disabled for the reason that
> function "__efi_get_rsdp_addr" is only called in efi_get_rsdp_addr
> when CONFIG_EFI enable. So function "__efi_get_rsdp_addr" should
> not be defined when CONFIG_EFI is disabled.
>
> Signed-off-by: Li Zetao <lizetao1@huawei.com>

I gave this a quick spin with clang to ensure that I didn't observe
new warnings produced via -Wall:

$ make LLVM=1 -j128 -s defconfig all
$ make LLVM=1 ARCH=i386 -j128 -s defconfig all

Reviewed-by: Nick Desaulniers <ndesaulniers@google.com>
Tested-by: Nick Desaulniers <ndesaulniers@google.com>

> ---
> v1 -> v2: Patch is new
> v2 -> v3: Resolve extra warnings after "-Wall" flag added
> v3 -> v4: Put this patch at the end
>
>  arch/x86/boot/compressed/Makefile | 3 ++-
>  arch/x86/boot/compressed/acpi.c   | 5 +++--
>  arch/x86/boot/compressed/kaslr.c  | 3 +--
>  3 files changed, 6 insertions(+), 5 deletions(-)
>
> diff --git a/arch/x86/boot/compressed/Makefile b/arch/x86/boot/compressed/Makefile
> index 3a261abb6d15..8918a8306dff 100644
> --- a/arch/x86/boot/compressed/Makefile
> +++ b/arch/x86/boot/compressed/Makefile
> @@ -35,7 +35,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
> +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
> @@ -44,6 +44,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)/=)
> diff --git a/arch/x86/boot/compressed/acpi.c b/arch/x86/boot/compressed/acpi.c
> index 21febd9f21ab..c062a8230e9c 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,10 @@ __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)
>  {
> diff --git a/arch/x86/boot/compressed/kaslr.c b/arch/x86/boot/compressed/kaslr.c
> index e476bcbd9b42..4abc9c42cf4d 100644
> --- a/arch/x86/boot/compressed/kaslr.c
> +++ b/arch/x86/boot/compressed/kaslr.c
> @@ -625,7 +625,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.
> @@ -645,7 +644,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;
>
> --
> 2.34.1
>


-- 
Thanks,
~Nick Desaulniers

Re: [PATCH -next v4 1/2] x86/boot: Remove unused variables

[Nick Desaulniers]

On Mon, Oct 10, 2022 at 6:32 PM Li Zetao <lizetao1@huawei.com> wrote:
>
> diff --git a/arch/x86/boot/compressed/sev.c b/arch/x86/boot/compressed/sev.c
> index c93930d5ccbd..b9451761a69a 100644
> --- a/arch/x86/boot/compressed/sev.c
> +++ b/arch/x86/boot/compressed/sev.c
> @@ -253,7 +253,6 @@ void do_boot_stage2_vc(struct pt_regs *regs, unsigned long exit_code)
>  static void enforce_vmpl0(void)
>  {
>         u64 attrs;
> -       int err;

Just a thought but you could initialize attrs now on the same line.

I also ended up testing the whole series, so:

Tested-by: Nick Desaulniers <ndesaulniers@google.com>
-- 
Thanks,
~Nick Desaulniers

Re: [PATCH -next v4 2/2] x86/boot/compressed: Add "-Wall" flag to Makefile

[Borislav Petkov]

On Tue, Oct 11, 2022 at 01:29:04AM +0000, Li Zetao wrote:
> This warning is disabled in the main Makefile for this reason so we

Who's "we"?

Please use passive voice in your commit message: no "we" or "I", etc,
and describe your changes in imperative mood.

Also, pls read section "2) Describe your changes" in
Documentation/process/submitting-patches.rst for more details.

Also, see section "Changelog" in
Documentation/process/maintainer-tip.rst

Bottom line is: personal pronouns are ambiguous in text, especially with
so many parties/companies/etc developing the kernel so let's avoid them
please.

In any case, it is getting there but this is not how you structure a
patchset like this. The proper order is:

1st patch: Remove unused variables

2nd patch: Fix -Wframe-address warning and explain why

3rd patch: Fix arch/x86/boot/compressed/acpi.c:23:1: warning: unused function "__efi_get_rsdp_addr" [-Wunused-function]

Btw, I also get with -Wall

In function ‘find_trampoline_placement’,
    inlined from ‘paging_prepare’ at arch/x86/boot/compressed/pgtable_64.c:135:35:
arch/x86/boot/compressed/pgtable_64.c:60:30: warning: array subscript 0 is outside array bounds of ‘short unsigned int[0]’ [-Warray-bounds]
   60 |                 ebda_start = *(unsigned short *)0x40e << 4;
      |                              ^~~~~~~~~~~~~~~~~~~~~~~~
arch/x86/boot/compressed/pgtable_64.c:61:30: warning: array subscript 0 is outside array bounds of ‘short unsigned int[0]’ [-Warray-bounds]
   61 |                 bios_start = *(unsigned short *)0x413 << 10;
      |                              ^~~~~~~~~~~~~~~~~~~~~~~~
In function ‘bios_get_rsdp_addr’,
    inlined from ‘get_rsdp_addr’ at arch/x86/boot/compressed/acpi.c:163:8:
arch/x86/boot/compressed/acpi.c:130:19: warning: array subscript 0 is outside array bounds of ‘u16[0]’ {aka ‘short unsigned int[]’} [-Warray-bounds]
  130 |         address = *(u16 *)ACPI_EBDA_PTR_LOCATION;

That should be a separate patch too.

Nth patch(es): Fix more warnings after building at least

	"allnoconfig" "defconfig" "allmodconfig" "allyesconfig"

for both 32-bit and 64-bit. A bunch of randconfigs on a big machine
would help a lot.

N + kth patch: Add -std=gnu11

X. Final patch: Add -Wall to compressed/Makefile.

In that order so that no warnings are seen when doing random builds.

Thx.

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette