xen/Kconfig.debug | 12 ++++++++++++ xen/Rules.mk | 10 ++++++++-- xen/arch/arm/setup.c | 2 ++ xen/arch/x86/setup.c | 4 ++++ xen/common/libelf/Makefile | 1 - xen/common/libfdt/Makefile | 1 - 6 files changed, 26 insertions(+), 4 deletions(-)
From: Grygorii Strashko <grygorii_strashko@epam.com>
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 <grygorii_strashko@epam.com>
---
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
v1: https://patchwork.kernel.org/project/xen-devel/patch/20251203222436.660044-1-grygorii_strashko@epam.com/
xen/Kconfig.debug | 12 ++++++++++++
xen/Rules.mk | 10 ++++++++--
xen/arch/arm/setup.c | 2 ++
xen/arch/x86/setup.c | 4 ++++
xen/common/libelf/Makefile | 1 -
xen/common/libfdt/Makefile | 1 -
6 files changed, 26 insertions(+), 4 deletions(-)
diff --git a/xen/Kconfig.debug b/xen/Kconfig.debug
index d900d926c555..8fc201d12c2c 100644
--- a/xen/Kconfig.debug
+++ b/xen/Kconfig.debug
@@ -39,11 +39,23 @@ config COVERAGE
bool "Code coverage support"
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.
If unsure, say N here.
+config RELAX_INIT_CHECK
+ bool
+ help
+ Relax strict check for .init sections only in %.init.o files.
+
+config DO_NOT_FREE_INIT_MEMORY
+ bool
+ help
+ Prevent freeing of .init sections at the end of Xen boot.
+
config CONDITION_COVERAGE
bool "Condition coverage support"
depends on COVERAGE && CC_HAS_MCDC
diff --git a/xen/Rules.mk b/xen/Rules.mk
index 24f447b95734..c884a4199dc2 100644
--- a/xen/Rules.mk
+++ b/xen/Rules.mk
@@ -145,10 +145,10 @@ endif
$(call cc-option-add,cov-cflags-$(CONFIG_COVERAGE),CC,-fprofile-update=atomic)
# Reset cov-cflags-y in cases where an objects has another one as prerequisite
-$(nocov-y) $(filter %.init.o, $(obj-y) $(obj-bin-y) $(extra-y)): \
+$(nocov-y) $(extra-y): \
cov-cflags-y :=
-$(non-init-objects): _c_flags += $(cov-cflags-y)
+$(obj-y) $(obj-bin-y) $(extra-y) $(lib-y): _c_flags += $(cov-cflags-y)
ifeq ($(CONFIG_UBSAN),y)
# Any -fno-sanitize= options need to come after any -fsanitize= options
@@ -259,6 +259,7 @@ $(obj)/%.o: $(src)/%.S FORCE
quiet_cmd_obj_init_o = INIT_O $@
+ifneq ($(CONFIG_RELAX_INIT_CHECK),y)
define cmd_obj_init_o
$(OBJDUMP) -h $< | while read idx name sz rest; do \
case "$$name" in \
@@ -271,6 +272,11 @@ define cmd_obj_init_o
done || exit $$?; \
$(OBJCOPY) $(foreach s,$(SPECIAL_DATA_SECTIONS),--rename-section .$(s)=.init.$(s)) $< $@
endef
+else
+define cmd_obj_init_o
+ $(OBJCOPY) $(foreach s,$(SPECIAL_DATA_SECTIONS),--rename-section .$(s)=.init.$(s)) $< $@
+endef
+endif
$(filter %.init.o,$(obj-y) $(obj-bin-y) $(extra-y)): $(obj)/%.init.o: $(obj)/%.o FORCE
$(call if_changed,obj_init_o)
diff --git a/xen/arch/arm/setup.c b/xen/arch/arm/setup.c
index 22e929aac778..2a0b322445cd 100644
--- a/xen/arch/arm/setup.c
+++ b/xen/arch/arm/setup.c
@@ -69,10 +69,12 @@ static __used void noreturn init_done(void)
{
int rc;
+#ifndef CONFIG_DO_NOT_FREE_INIT_MEMORY
/* Must be done past setting system_state. */
unregister_init_virtual_region();
free_init_memory();
+#endif
/*
* We have finished booting. Mark the section .data.ro_after_init
diff --git a/xen/arch/x86/setup.c b/xen/arch/x86/setup.c
index 7bbba76a92f8..280085c206a7 100644
--- a/xen/arch/x86/setup.c
+++ b/xen/arch/x86/setup.c
@@ -815,6 +815,7 @@ static void noreturn init_done(void)
for ( unsigned int i = 0; i < bi->nr_domains; i++ )
domain_unpause_by_systemcontroller(bi->domains[i].d);
+#ifndef CONFIG_DO_NOT_FREE_INIT_MEMORY
/* MUST be done prior to removing .init data. */
unregister_init_virtual_region();
@@ -837,6 +838,9 @@ static void noreturn init_done(void)
destroy_xen_mappings(start, end);
init_xenheap_pages(__pa(start), __pa(end));
printk("Freed %lukB init memory\n", (end - start) >> 10);
+#else
+ (void) end, (void) start, (void)va;
+#endif
/* Mark .rodata/ro_after_init as RO. Maybe reform the superpage. */
modify_xen_mappings((unsigned long)&__2M_rodata_start,
diff --git a/xen/common/libelf/Makefile b/xen/common/libelf/Makefile
index 917d12b006f7..60b3ae40728f 100644
--- a/xen/common/libelf/Makefile
+++ b/xen/common/libelf/Makefile
@@ -1,5 +1,4 @@
obj-bin-y := libelf.o
-nocov-y += libelf.o
libelf-objs := libelf-tools.o libelf-loader.o libelf-dominfo.o
SECTIONS := text data $(SPECIAL_DATA_SECTIONS)
diff --git a/xen/common/libfdt/Makefile b/xen/common/libfdt/Makefile
index 6ce679f98f47..ae0f69c01373 100644
--- a/xen/common/libfdt/Makefile
+++ b/xen/common/libfdt/Makefile
@@ -5,7 +5,6 @@ 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))
-nocov-y += libfdt.o
endif
obj-y += libfdt.o
--
2.34.1On 05.12.2025 20:34, Grygorii Strashko wrote: > --- a/xen/Kconfig.debug > +++ b/xen/Kconfig.debug > @@ -39,11 +39,23 @@ config COVERAGE > bool "Code coverage support" > 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. > > If unsure, say N here. > > +config RELAX_INIT_CHECK > + bool > + help > + Relax strict check for .init sections only in %.init.o files. > + > +config DO_NOT_FREE_INIT_MEMORY > + bool > + help > + Prevent freeing of .init sections at the end of Xen boot. > + > config CONDITION_COVERAGE > bool "Condition coverage support" > depends on COVERAGE && CC_HAS_MCDC Please obey to the somewhat special indentation rules for Kconfig files. > @@ -259,6 +259,7 @@ $(obj)/%.o: $(src)/%.S FORCE > > > quiet_cmd_obj_init_o = INIT_O $@ > +ifneq ($(CONFIG_RELAX_INIT_CHECK),y) > define cmd_obj_init_o > $(OBJDUMP) -h $< | while read idx name sz rest; do \ > case "$$name" in \ > @@ -271,6 +272,11 @@ define cmd_obj_init_o > done || exit $$?; \ > $(OBJCOPY) $(foreach s,$(SPECIAL_DATA_SECTIONS),--rename-section .$(s)=.init.$(s)) $< $@ > endef > +else > +define cmd_obj_init_o > + $(OBJCOPY) $(foreach s,$(SPECIAL_DATA_SECTIONS),--rename-section .$(s)=.init.$(s)) $< $@ > +endef > +endif If the objcopy indeed needs suppressing altogether (as Andrew suggests), the unwanted redundancy here would go away anyway. Otherwise my (recurring) request to avoid such duplication. > --- a/xen/common/libfdt/Makefile > +++ b/xen/common/libfdt/Makefile > @@ -5,7 +5,6 @@ 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)) > -nocov-y += libfdt.o > endif > > obj-y += libfdt.o Is this sufficient? Don't you first need to replace the custom objcopy use? Jan
On 08.12.25 10:35, Jan Beulich wrote: > On 05.12.2025 20:34, Grygorii Strashko wrote: >> --- a/xen/Kconfig.debug >> +++ b/xen/Kconfig.debug >> @@ -39,11 +39,23 @@ config COVERAGE >> bool "Code coverage support" >> 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. >> >> If unsure, say N here. >> >> +config RELAX_INIT_CHECK >> + bool >> + help >> + Relax strict check for .init sections only in %.init.o files. >> + >> +config DO_NOT_FREE_INIT_MEMORY >> + bool >> + help >> + Prevent freeing of .init sections at the end of Xen boot. >> + >> config CONDITION_COVERAGE >> bool "Condition coverage support" >> depends on COVERAGE && CC_HAS_MCDC > > Please obey to the somewhat special indentation rules for Kconfig files. ok. > >> @@ -259,6 +259,7 @@ $(obj)/%.o: $(src)/%.S FORCE >> >> >> quiet_cmd_obj_init_o = INIT_O $@ >> +ifneq ($(CONFIG_RELAX_INIT_CHECK),y) >> define cmd_obj_init_o >> $(OBJDUMP) -h $< | while read idx name sz rest; do \ >> case "$$name" in \ >> @@ -271,6 +272,11 @@ define cmd_obj_init_o >> done || exit $$?; \ >> $(OBJCOPY) $(foreach s,$(SPECIAL_DATA_SECTIONS),--rename-section .$(s)=.init.$(s)) $< $@ >> endef >> +else >> +define cmd_obj_init_o >> + $(OBJCOPY) $(foreach s,$(SPECIAL_DATA_SECTIONS),--rename-section .$(s)=.init.$(s)) $< $@ >> +endef >> +endif > > If the objcopy indeed needs suppressing altogether (as Andrew suggests), the > unwanted redundancy here would go away anyway. Otherwise my (recurring) > request to avoid such duplication. Could you suggest the best way to avoid duplication, please? if/else/endif is not working inside "Custom commands" make file commands. May be split it on two - cmd_obj_init_check and obj_init_objcopy? >> --- a/xen/common/libfdt/Makefile >> +++ b/xen/common/libfdt/Makefile >> @@ -5,7 +5,6 @@ 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)) >> -nocov-y += libfdt.o >> endif >> >> obj-y += libfdt.o > > Is this sufficient? Don't you first need to replace the custom objcopy use? It works just fine if .init memory is not freed, as it doesn't matter where sections are placed. -- Best regards, -grygorii
On 08.12.2025 19:54, Grygorii Strashko wrote: > On 08.12.25 10:35, Jan Beulich wrote: >> On 05.12.2025 20:34, Grygorii Strashko wrote: >>> @@ -259,6 +259,7 @@ $(obj)/%.o: $(src)/%.S FORCE >>> >>> >>> quiet_cmd_obj_init_o = INIT_O $@ >>> +ifneq ($(CONFIG_RELAX_INIT_CHECK),y) >>> define cmd_obj_init_o >>> $(OBJDUMP) -h $< | while read idx name sz rest; do \ >>> case "$$name" in \ >>> @@ -271,6 +272,11 @@ define cmd_obj_init_o >>> done || exit $$?; \ >>> $(OBJCOPY) $(foreach s,$(SPECIAL_DATA_SECTIONS),--rename-section .$(s)=.init.$(s)) $< $@ >>> endef >>> +else >>> +define cmd_obj_init_o >>> + $(OBJCOPY) $(foreach s,$(SPECIAL_DATA_SECTIONS),--rename-section .$(s)=.init.$(s)) $< $@ >>> +endef >>> +endif >> >> If the objcopy indeed needs suppressing altogether (as Andrew suggests), the >> unwanted redundancy here would go away anyway. Otherwise my (recurring) >> request to avoid such duplication. > > Could you suggest the best way to avoid duplication, please? > if/else/endif is not working inside "Custom commands" make file commands. > May be split it on two - cmd_obj_init_check and obj_init_objcopy? That's one option. Another is to use something like $(if $(filter y,$(CONFIG_RELAX_INIT_CHECK)), ...). Jan
On 05/12/2025 7:34 pm, Grygorii Strashko wrote:
> From: Grygorii Strashko <grygorii_strashko@epam.com>
>
> 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 <grygorii_strashko@epam.com>
> ---
> 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?
> diff --git a/xen/Rules.mk b/xen/Rules.mk
> index 24f447b95734..c884a4199dc2 100644
> --- a/xen/Rules.mk
> +++ b/xen/Rules.mk
> @@ -145,10 +145,10 @@ endif
> $(call cc-option-add,cov-cflags-$(CONFIG_COVERAGE),CC,-fprofile-update=atomic)
>
> # Reset cov-cflags-y in cases where an objects has another one as prerequisite
> -$(nocov-y) $(filter %.init.o, $(obj-y) $(obj-bin-y) $(extra-y)): \
> +$(nocov-y) $(extra-y): \
> cov-cflags-y :=
This could become a single line now.
> diff --git a/xen/arch/arm/setup.c b/xen/arch/arm/setup.c
> index 22e929aac778..2a0b322445cd 100644
> --- a/xen/arch/arm/setup.c
> +++ b/xen/arch/arm/setup.c
> @@ -69,10 +69,12 @@ static __used void noreturn init_done(void)
> {
> int rc;
>
> +#ifndef CONFIG_DO_NOT_FREE_INIT_MEMORY
> /* Must be done past setting system_state. */
> unregister_init_virtual_region();
>
> free_init_memory();
> +#endif
>
> /*
> * We have finished booting. Mark the section .data.ro_after_init
> diff --git a/xen/arch/x86/setup.c b/xen/arch/x86/setup.c
> index 7bbba76a92f8..280085c206a7 100644
> --- a/xen/arch/x86/setup.c
> +++ b/xen/arch/x86/setup.c
> @@ -815,6 +815,7 @@ static void noreturn init_done(void)
> for ( unsigned int i = 0; i < bi->nr_domains; i++ )
> domain_unpause_by_systemcontroller(bi->domains[i].d);
>
> +#ifndef CONFIG_DO_NOT_FREE_INIT_MEMORY
> /* MUST be done prior to removing .init data. */
> unregister_init_virtual_region();
>
> @@ -837,6 +838,9 @@ static void noreturn init_done(void)
> destroy_xen_mappings(start, end);
> init_xenheap_pages(__pa(start), __pa(end));
> printk("Freed %lukB init memory\n", (end - start) >> 10);
> +#else
> + (void) end, (void) start, (void)va;
> +#endif
For both of these, the preferred way would be to use if (
IS_ENABLED(CONFIG_DO_NOT_FREE_INIT_MEMORY) ), which removes the need for
the else clause in x86.
Also, you make free_init_memory() un-called on ARM with this change.
Depending on how the freeing-init question lands, you either want some
extra ifdefary, or the --gc-sections option that we discussed before.
~Andrew
On 05.12.25 22:00, Andrew Cooper wrote: > On 05/12/2025 7:34 pm, Grygorii Strashko wrote: >> From: Grygorii Strashko <grygorii_strashko@epam.com> >> >> 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 <grygorii_strashko@epam.com> >> --- >> 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 -- Best regards, -grygorii
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 <grygorii_strashko@epam.com> >>> >>> 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 <grygorii_strashko@epam.com> >>> --- >>> 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. ~Andrew
On 06.12.2025 15:15, 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 <grygorii_strashko@epam.com> >>>> >>>> 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 <grygorii_strashko@epam.com> >>>> --- >>>> 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. Maybe in that context. In the patch submission this isn't obvious at all, I would say. Please add some explanation in such a case. >> 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. Yet then do we still need to suppress the freeing of .init.*? Jan
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 <grygorii_strashko@epam.com> >>>> >>>> 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 <grygorii_strashko@epam.com> >>>> --- >>>> 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. ~Andrew
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 <grygorii_strashko@epam.com>
>>>>>
>>>>> 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 <grygorii_strashko@epam.com>
>>>>> ---
>>>>> 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
obj-y += libfdt.o
--
Best regards,
-grygorii
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 <grygorii_strashko@epam.com> >>>>>> >>>>>> 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 <grygorii_strashko@epam.com> >>>>>> --- >>>>>> 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. For libelf this means we need regular __init annotations, but #undef'd outside of __XEN__ (when we're doing the userspace build). 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. ~Andrew
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 <grygorii_strashko@epam.com> >>>>>>> >>>>>>> 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 <grygorii_strashko@epam.com> >>>>>>> --- >>>>>>> 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. :( 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. -- Best regards, -grygorii
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 <grygorii_strashko@epam.com> >>>>>>>> >>>>>>>> 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 <grygorii_strashko@epam.com> >>>>>>>> --- >>>>>>>> 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. > >
On 09/12/2025 2:28 pm, Jan Beulich wrote: > 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 <grygorii_strashko@epam.com> >>>>>>>>> >>>>>>>>> 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 <grygorii_strashko@epam.com> >>>>>>>>> --- >>>>>>>>> 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. Agreed. libelf was already modified for Xen, and then XSA-55 made it entirely unrecognisable. It's very much our code now. If we really don't want to modify libfdt's source, we should make a standard way of init-ing code like this, so we can move the custom logic out of libfdt's Makefile. The problem really is custom local logic; that's what we need to fix. ~Andrew
© 2016 - 2025 Red Hat, Inc.