On 09.12.2025 15:21, Grygorii Strashko wrote: > > > On 09.12.25 15:19, Andrew Cooper wrote: >> On 08/12/2025 6:49 pm, Grygorii Strashko wrote: >>> Hi Andrew, >>> >>> On 06.12.25 16:21, Andrew Cooper wrote: >>>> On 06/12/2025 2:15 pm, Andrew Cooper wrote: >>>>> On 06/12/2025 9:10 am, Grygorii Strashko wrote: >>>>>> >>>>>> On 05.12.25 22:00, Andrew Cooper wrote: >>>>>>> On 05/12/2025 7:34 pm, Grygorii Strashko wrote: >>>>>>>> From: Grygorii Strashko <[email protected]> >>>>>>>> >>>>>>>> Extend coverage support on .init and lib code. >>>>>>>> Add two hidden Kconfig options: >>>>>>>> - RELAX_INIT_CHECK "Relax strict check for .init sections only in >>>>>>>> %.init.o >>>>>>>> files" >>>>>>>> - DO_NOT_FREE_INIT_MEMORY "Prevent freeing of .init sections at the >>>>>>>> end of >>>>>>>> Xen boot." >>>>>>>> >>>>>>>> Both selected selected when COVERAGE=y, as getting coverage >>>>>>>> report for >>>>>>>> ".init" code is required: >>>>>>>> - to bypass strict check for .init sections only in %.init.o files; >>>>>>>> - the .init code stay in memory after Xen boot. >>>>>>>> >>>>>>>> RELAX_INIT_CHECK/DO_NOT_FREE_INIT_MEMORY could be used by other >>>>>>>> debug >>>>>>>> features in the future. >>>>>>>> >>>>>>>> Signed-off-by: Grygorii Strashko <[email protected]> >>>>>>>> --- >>>>>>>> changes in v2: >>>>>>>> - add RELAX_INIT_CHECK and DO_NOT_FREE_INIT_MEMORY, those are two >>>>>>>> different things, >>>>>>>> both potentially reusable >>>>>>>> - enable coverage for libfdt/libelf always >>>>>>>> - enable colverage for .init always >>>>>>> This is a lot nicer (i.e. more simple). >>>>>>> >>>>>>> But, I still don't know why we need to avoid freeing init memory >>>>>>> to make >>>>>>> this work. What explodes if we dont? >>>>>>> >>>>>> It will just crash when coverage data is collected. >>>>>> >>>>>> First I made changes in make file to get .init covered >>>>>> then I hit a crash >>>>>> then I checked %.init.o >>>>>> conclusion was obvious. >>>>>> >>>>>> For example: >>>>>> objdump -x bzimage.init.o | grep gcov >>>>>> >>>>>> 0000000000000010 l O .bss 0000000000000028 >>>>>> __gcov0.bzimage_check >>>>>> 0000000000000040 l O .bss 0000000000000040 >>>>>> __gcov0.bzimage_headroom >>>>>> 0000000000000000 l O .bss 0000000000000008 >>>>>> __gcov0.output_length >>>>>> 0000000000000080 l O .bss 0000000000000060 >>>>>> __gcov0.bzimage_parse >>>>>> 0000000000000098 l O .init.data.rel.local 0000000000000028 >>>>>> __gcov_.bzimage_parse >>>>>> 0000000000000070 l O .init.data.rel.local 0000000000000028 >>>>>> __gcov_.bzimage_headroom >>>>>> 0000000000000048 l O .init.data.rel.local 0000000000000028 >>>>>> __gcov_.bzimage_check >>>>>> 0000000000000020 l O .init.data.rel.local 0000000000000028 >>>>>> __gcov_.output_length >>>>>> 0000000000000000 *UND* 0000000000000000 __gcov_init >>>>>> 0000000000000000 *UND* 0000000000000000 __gcov_exit >>>>>> 0000000000000000 *UND* 0000000000000000 __gcov_merge_add >>>>>> 0000000000000008 R_X86_64_PLT32 __gcov_init-0x0000000000000004 >>>>>> 0000000000000012 R_X86_64_PLT32 __gcov_exit-0x0000000000000004 >>>>>> 0000000000000020 R_X86_64_64 __gcov_merge_add >>>>>> >>>>> Aah, we should exclude the OJBCOPY too. That's what's moving >>>>> .data.rel.local amongst other sections we target with attributes >>>>> directly. >>>> >>>> we can't target. >>> >>> I've come up with below diff - seems it's working without >>> DO_NOT_FREE_INIT_MEMORY. >>> Is this what you have in mind? >>> >>> diff --git a/xen/Kconfig.debug b/xen/Kconfig.debug >>> index 8fc201d12c2c..16b1a82db46e 100644 >>> --- a/xen/Kconfig.debug >>> +++ b/xen/Kconfig.debug >>> @@ -40,7 +40,6 @@ config COVERAGE >>> depends on SYSCTL && !LIVEPATCH >>> select SUPPRESS_DUPLICATE_SYMBOL_WARNINGS if >>> !ENFORCE_UNIQUE_SYMBOLS >>> select RELAX_INIT_CHECK >>> - select DO_NOT_FREE_INIT_MEMORY >>> help >>> Enable code coverage support. >>> >>> diff --git a/xen/Rules.mk b/xen/Rules.mk >>> index 8c4861a427e6..47fdcc1d23b5 100644 >>> --- a/xen/Rules.mk >>> +++ b/xen/Rules.mk >>> @@ -33,11 +33,15 @@ cov-cflags-y := >>> nocov-y := >>> noubsan-y := >>> >>> +# when coverage is enabled the gcc internal section should stay in >>> memory >>> +# after Xen boot >>> +ifneq ($(CONFIG_COVERAGE),y) >>> SPECIAL_DATA_SECTIONS := rodata $(foreach a,1 2 4 8 16, \ >>> $(foreach w,1 2 4, \ >>> >>> rodata.str$(w).$(a)) \ >>> rodata.cst$(a)) \ >>> $(foreach r,rel rel.ro,data.$(r).local) >>> +endif >>> >>> # The filename build.mk has precedence over Makefile >>> include $(firstword $(wildcard $(srcdir)/build.mk) $(srcdir)/Makefile) >>> diff --git a/xen/common/libelf/Makefile b/xen/common/libelf/Makefile >>> index 60b3ae40728f..8180c78f1510 100644 >>> --- a/xen/common/libelf/Makefile >>> +++ b/xen/common/libelf/Makefile >>> @@ -1,8 +1,10 @@ >>> obj-bin-y := libelf.o >>> libelf-objs := libelf-tools.o libelf-loader.o libelf-dominfo.o >>> >>> +ifneq ($(CONFIG_COVERAGE),y) >>> SECTIONS := text data $(SPECIAL_DATA_SECTIONS) >>> OBJCOPYFLAGS := $(foreach s,$(SECTIONS),--rename-section >>> .$(s)=.init.$(s)) >>> +endif >>> >>> CFLAGS-y += -Wno-pointer-sign >>> >>> diff --git a/xen/common/libfdt/Makefile b/xen/common/libfdt/Makefile >>> index ae0f69c01373..fb26e5bff0fd 100644 >>> --- a/xen/common/libfdt/Makefile >>> +++ b/xen/common/libfdt/Makefile >>> @@ -4,7 +4,9 @@ SECTIONS := text data $(SPECIAL_DATA_SECTIONS) >>> >>> # For CONFIG_OVERLAY_DTB, libfdt functionalities will be needed >>> during runtime. >>> ifneq ($(CONFIG_OVERLAY_DTB),y) >>> -OBJCOPYFLAGS := $(foreach s,$(SECTIONS),--rename-section >>> .$(s)=.init.$(s)) >>> + ifneq ($(CONFIG_COVERAGE),y) >>> + OBJCOPYFLAGS := $(foreach >>> s,$(SECTIONS),--rename-section .$(s)=.init.$(s)) >>> + endif >>> endif >> >> This is the (aforementioned) non-standard way of doing .init.o, which is >> why it doesn't play nicely. >> >> I suggest that we first convert libelf and libfdt to the standard way of >> doing .init. > > I assume the rest is ok. > >> >> For libelf this means we need regular __init annotations, but #undef'd >> outside of __XEN__ (when we're doing the userspace build). >> > > Need clarification here - this are imported libraries and changing their code > directly was not welcome before. Therefore there is Xen specific magic in > Makefiles. > :(
I can't and won't speak for libfdt, but for libelf I think we should really consider this ours (not imported) the latest as of the re-work for XSA-55. Jan > Just an idea1, may be ".init" handling can be just dropped from libelf and > libfdt > Makefiles with comment added instead (kinda "TODO") - they will be built-in. > It doesn't work with CONFIG_CC_SPLIT_SECTIONS any way now. > > Just an idea2, drop libelf and libfdt changes from this patch. > - they will be not in coverage report (nocov-y += *.obj) > - will be resolved in the future. > > Trying to avoid blocking on external dependencies :( > >> For libfdt, this will need some init_or_$FOO things (matching >> init_or_livepatch). >> >> Once the custom init has been made standard, this code becomes easier to >> move into lib, and we no longer have special cases when trying to extend >> coverage. > >
