Message-ID: <Zr0MvswCE3VJBKhp@oracle.com>
Date: Wed, 14 Aug 2024 15:59:58 -0400
From: Kris Van Hees <kris.van.hees@...cle.com>
To: Steven Rostedt <rostedt@...dmis.org>
Cc: Kris Van Hees <kris.van.hees@...cle.com>, linux-kernel@...r.kernel.org,
        linux-kbuild@...r.kernel.org, linux-modules@...r.kernel.org,
        linux-trace-kernel@...r.kernel.org,
        Nick Alcock <nick.alcock@...cle.com>,
        Alan Maguire <alan.maguire@...cle.com>,
        Masahiro Yamada <masahiroy@...nel.org>,
        Luis Chamberlain <mcgrof@...nel.org>,
        Masami Hiramatsu <mhiramat@...nel.org>,
        Nick Desaulniers <ndesaulniers@...gle.com>,
        Jiri Olsa <olsajiri@...il.com>,
        Elena Zannoni <elena.zannoni@...cle.com>
Subject: Re: [PATCH v5 3/4] scripts: add verifier script for builtin module
 range data

On Wed, Aug 14, 2024 at 03:19:45PM -0400, Steven Rostedt wrote:
> 
> Hmm, does this handle my concern from the last patch. That is, if the
> previous script is broken by some change, this will catch it?
> If so, should there be a way to run this always? As it looks to be only
> used for manual tests.

It is meant to address detecting things going wrong, yes.  I hesitate to make
this validation be something that is always executed because I wouldn't want
to disrupt people's kernel builds with failure that are not critical to the
operation of the kernel itself.  I could make it a config option so it can
nbe enabled for those who might want to, e.g. for release building?

Does that make sense?

> On Mon, 15 Jul 2024 23:10:44 -0400
> Kris Van Hees <kris.van.hees@...cle.com> wrote:
> 
> > The modules.builtin.ranges offset range data for builtin modules is
> > generated at compile time based on the list of built-in modules and
> > the vmlinux.map and vmlinux.o.map linker maps.  This data can be used
> 						^^
> As my daughter keeps reminding me, nobody uses double spaces after a period
> anymore ;-)

I am old-fashion :)

> > to determine whether a symbol at a particular address belongs to
> > module code that was configured to be compiled into the kernel proper
> > as a built-in module (rather than as a standalone module).
> > 
> > This patch adds a script that uses the generated modules.builtin.ranges
> > data to annotate the symbols in the System.map with module names if
> > their address falls within a range that belongs to one or mre built-in
> 							   "more" ?

Oops, yes, thanks.

> > modules.
> > 
> > It then processes the vmlinux.map (and if needed, vmlinux.o.map) to
> > verify the annotation:
> > 
> >   - For each top-level section:
> >      - For each object in the section:
> >         - Determine whether the object is part of a built-in module
> >           (using modules.builtin and the .*.cmd file used to compile
> >            the object as suggested in [0])
> >         - For each symbol in that object, verify that the built-in
> >           module association (or lack thereof) matches the annotation
> >           given to the symbol.
> > 
> > Signed-off-by: Kris Van Hees <kris.van.hees@...cle.com>
> > Reviewed-by: Nick Alcock <nick.alcock@...cle.com>
> > Reviewed-by: Alan Maguire <alan.maguire@...cle.com>
> 
> After running this, I do get a lot of messages:
> 
> uncore_pmu_event_start in intel_uncore (should NOT be)
> uncore_pcibus_to_dieid in intel_uncore (should NOT be)
> uncore_die_to_segment in intel_uncore (should NOT be)
> uncore_device_to_die in intel_uncore (should NOT be)
> __find_pci2phy_map in intel_uncore (should NOT be)
> uncore_event_show in intel_uncore (should NOT be)
> uncore_pmu_to_box in intel_uncore (should NOT be)
> uncore_msr_read_counter in intel_uncore (should NOT be)
> uncore_mmio_exit_box in intel_uncore (should NOT be)
> uncore_mmio_read_counter in intel_uncore (should NOT be)
> uncore_get_constraint in intel_uncore (should NOT be)
> uncore_put_constraint in intel_uncore (should NOT be)
> uncore_shared_reg_config in intel_uncore (should NOT be)
> uncore_perf_event_update in intel_uncore (should NOT be)
> uncore_pmu_event_read in intel_uncore (should NOT be)
> uncore_pmu_event_stop in intel_uncore (should NOT be)
> uncore_pmu_event_add in intel_uncore (should NOT be)
> [..]
> usb_debug_root in usb_common (should NOT be)
> usb_hcds_loaded in usbcore (should NOT be)
> iTCO_vendorsupport in iTCO_vendor_support (should NOT be)
> snd_ecards_limit in snd (should NOT be)
> snd_major in snd (should NOT be)
> snd_oss_root in snd (should NOT be)
> snd_seq_root in snd (should NOT be)
> ip6_min_hopcount in ipv6 (should NOT be)
> ip6_ra_chain in ipv6 (should NOT be)
> raw_v6_hashinfo in ipv6 (should NOT be)
> Verification of /work/build/nobackup/debiantesting-x86-64/modules.builtin.ranges:
>   Correct matches:   24962 (75% of total)
>     Module matches:      0 (0% of matches)
>   Mismatches:         8262 (24% of total)
>   Missing:               0 (0% of total)
> 
> 
> What does this mean?

Hm, this is certainly why the validation script exists.  I am surprised, though
not entirely because kernel changes toward the 6.10 branching and such came
after I create this version.  Would you be willing to send me a copy of your
.config for this kernel build so I can investigate?  This output is typical
of a case where the script was not able to determine offse ranges correctly.

	Kris

> > ---
> > 
> > Notes:
> >     Changes since v4:
> >      - New patch in the series
> > 
> >  scripts/verify_builtin_ranges.awk | 348 ++++++++++++++++++++++++++++++
> >  1 file changed, 348 insertions(+)
> >  create mode 100755 scripts/verify_builtin_ranges.awk
> > 
> > diff --git a/scripts/verify_builtin_ranges.awk b/scripts/verify_builtin_ranges.awk
> > new file mode 100755
> > index 000000000000..a2475a38ba50
> > --- /dev/null
> > +++ b/scripts/verify_builtin_ranges.awk
> > @@ -0,0 +1,348 @@
> > +#!/usr/bin/gawk -f
> > +# SPDX-License-Identifier: GPL-2.0
> > +# verify_builtin_ranges.awk: Verify address range data for builtin modules
> > +# Written by Kris Van Hees <kris.van.hees@...cle.com>
> > +#
> > +# Usage: verify_builtin_ranges.awk modules.builtin.ranges System.map \
> > +#				   modules.builtin vmlinux.map vmlinux.o.map
> > +#
> > +
> > +# Return the module name(s) (if any) associated with the given object.
> > +#
> > +# If we have seen this object before, return information from the cache.
> > +# Otherwise, retrieve it from the corresponding .cmd file.
> > +#

Powered by blists - more mailing lists