Re: [Xen-devel] [RFC v3 10/13] jump_label: port __jump_table to linker tables

[Josh Poimboeuf <jpoimboe@xxxxxxxxxx>]

On Sat, Jul 23, 2016 at 12:26:54AM +0200, Luis R. Rodriguez wrote:
> On Fri, Jul 22, 2016 at 04:49:45PM -0500, Josh Poimboeuf wrote:
> > On Fri, Jul 22, 2016 at 02:24:44PM -0700, Luis R. Rodriguez wrote:
> > > diff --git a/tools/objtool/special.c b/tools/objtool/special.c
> > > index bff8abb3a4aa..f0ad369f994b 100644
> > > --- a/tools/objtool/special.c
> > > +++ b/tools/objtool/special.c
> > > @@ -26,6 +26,10 @@
> > >  #include "special.h"
> > >  #include "warn.h"
> > >  
> > > +#include "../../include/asm-generic/sections.h"
> > > +#include "../../include/asm-generic/tables.h"
> > > +#include "../../include/linux/stringify.h"
> > > +
> > >  #define EX_ENTRY_SIZE            12
> > >  #define EX_ORIG_OFFSET           0
> > >  #define EX_NEW_OFFSET            4
> > > @@ -63,7 +67,9 @@ struct special_entry entries[] = {
> > >           .feature = ALT_FEATURE_OFFSET,
> > >   },
> > >   {
> > > -         .sec = "__jump_table",
> > > +         .sec = __stringify(SECTION_TBL(SECTION_DATA,
> > > +                                        __jump_table,
> > > +                                        SECTION_ORDER_ANY)),
> > >           .jump_or_nop = true,
> > >           .size = JUMP_ENTRY_SIZE,
> > >           .orig = JUMP_ORIG_OFFSET,
> > 
> > (continuing our discussion from another thread...)
> > 
> > I think tools code isn't allowed to include kernel files because the
> > tools subdirectory is supposed to be completely independent.
> 
> That was also the case for userpsace tools in scripts/ -- for instance
> scripts/mod/modpost.c made an exception. What I've proposed here to
> keep things simple was to ensure -UKERNEL is passed and then only
> include files we know will work.
> 
> Refer to the patch "kprobes: port .kprobes.text to section range"
> in this series for an extension of the scripts/mod/modpost.c kernel
> headers use.

I think the rule of separating code is stricter for tools/ than it is
for scripts/.  The scripts are generally kernel-specific whereas the
tools are independent.  I believe the goal is to be able to copy them
out of the kernel tree and still be able to compile them.

> > As far as I can tell, the section name will always be
> > ".data.tbl.__jump_table.any".  Is that true?  If so, any reason why we
> > can't just hard-code the string here?
> 
> Its a fair strategy, however if upstream changes the order name we'd
> have to hand code this as well, by using a macro we keep it all in one
> place.

Hm, do you expect the section name to change often?

> > As you saw, if the string
> > changes, objtool will complain and 0-day will report it.
> 
> Indeed, which is why I was hoping to instead stick to a standard
> sections set of header files that lets us get these in on place.

Actually, I meant that obtool reporting the change would be a good
thing, in favor of just hard-coding the string.  It lets objtool do its
job of letting us know when something changes, like it did today.

> The only place I hand coded in this series was in the perl
> file scripts/recordmcount.pl but I suppose if we wanted to get
> creative we could even generate a header for it too.
> 
> If we want to avoid all this hacker include stuff another option
> is to *generate* each respective sections.h for the kernel, and
> also, one for tools, and one for perl. What do you think?

If the generated files aren't checked into git, tools/ will rely on
kernel files and will no longer be independent.  If they *are* checked
in, then we have to keep the files in sync.  Either way it sounds like
overkill, just to avoid hard-coding a string for which objtool will
already warn if it changes.

-- 
Josh

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel