On 10/14/22 04:22, Julien Grall wrote:
Hi,
On 14/10/2022 08:16, Jan Beulich wrote:
On 14.10.2022 04:53, Stewart Hildebrand wrote:
Signed-off-by: Stewart Hildebrand <stewart.hildebr...@amd.com>
I guess a non-empty description and a Fixes: tag would be nice.
Okay, I will send a v2 with the following description:
All functions in domain_build.c should be marked __init. This was
spotted when building the hypervisor with -Og.
Fixes: 1050a7b91c xen/arm: add pci-domain for disabled devices
Signed-off-by: Stewart Hildebrand <stewart.hildebr...@amd.com>
+1. I am actually quite interested to understand how this was spotted.
The build system should check that any function/data in domain_build.c
are part of the __init section. So I guess the compiler you are using
doesn't inline the function?
If so, I am actually surprised you are the first one spotted this... We
are building on various distribution without any issues (?). I would be
interested to know the compiler version and maybe we could add it in the
CI.
I added -Og to the make command line so it takes precedence over the
default -O1/-O2:
$ make EXTRA_CFLAGS_XEN_CORE="-Og" XEN_TARGET_ARCH=arm64
CROSS_COMPILE=aarch64-none-linux-gnu- dist-xen -j $(nproc)
Indeed, I did observe the build error:
Error: size of arch/arm/domain_build.o:.text is 0x00000008
I used this rune to reveal the culprit:
$ aarch64-none-linux-gnu-objdump -d xen/arch/arm/domain_build.o | head
xen/arch/arm/domain_build.o: file format elf64-littleaarch64
Disassembly of section .text:
0000000000000000 <handle_linux_pci_domain>:
0: 52800000 mov w0, #0x0 // #0
4: d65f03c0 ret
I am using this toolchain:
https://developer.arm.com/-/media/Files/downloads/gnu/11.3.rel1/binrel/arm-gnu-toolchain-11.3.rel1-x86_64-aarch64-none-linux-gnu.tar.xz
Further, there were two more build errors observed when building with -Og:
arch/arm/domain_build.c: In function ‘make_cpus_node’:
arch/arm/domain_build.c:2013:12: error: ‘clock_valid’ may be used
uninitialized in this function [-Werror=maybe-uninitialized]
2013 | if ( clock_valid )
| ^
arch/arm/efi/boot.c: In function ‘efi_start’:
arch/arm/efi/boot.c:1464:9: error: ‘argc’ may be used uninitialized in
this function [-Werror=maybe-uninitialized]
1464 | efi_arch_handle_cmdline(argc ? *argv : NULL, options,
name.s);
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
I assume these uninitialized use errors can simply be fixed by
initializing the respective variables to false/0, but a second opinion
would certainly be helpful.
