So this is related to my currently very ugly hack at
https://lore.kernel.org/all/CAHk-=whFSz=usMPHHGAQBnJRVAFfuH4gFHtgyLe0YET75zYRzA@mail.gmail.com/
where I'm trying to do "runtime constants". That patch actually works,
but it's flawed in many ways, and one of the ways it is flawed is that
I really want to put the "this is the use for symbol X" in a section
of its own for each X.
Now, creating the sections is trivial, that's not the problem. I'd
just make the asm do
".pushsection .static_const." #sym ",\"a\"\n\t" \
...
".popsection"
and the linker script will just do
KEEP(*(.static_const.*))
and I'm done. Nice individual sections for each of the runtime constant symbols.
However, for the fixup part, I then really want the section start and
end addresses, so that I can iterate over those uses for a particular
named symbol.
And I am not finding any way to do that with a linker script. Sure, I
can trivially just do
. = ALIGN(8);
__static_const_start = . ;
KEEP(*(.static_const.*))
__static_const_end = . ;
and now I have the over-all start and end for those sections, but I
want it per section.
This is actually not even remotely a new thing: We do this manually
for a lot of sections, and we have macros to help do it, eg our
'BOUNDED_SECTION_BY()' macro in <asm/vmlinux.lds.h> does exactly this
for any named section.
But they very much do this on individually named sections, not on the
kind of "do it for this section pattern" that I want. Yes, you can do
it for patterns, and we do:
BOUNDED_SECTION_BY(.note.*, _notes)
but that creates exactly that same "bound the whole set of sections by
symbols", not "bound each individual section" thing.
I tried to find some way to do this at the linker script level, and
decided it's not possible. I may be wrong.
So then I said "we can just make objtool do it".
And I think objtool effectively almost has that already, but I'm not
really familiar enough with the code to start messing around with it
at that level.
It's kind of like elf_create_prefix_symbol(), except at a section
level, and needs to be both prefix and postfix.
Or kind of elf_create_section_symbol(), but making it a real symbol
with a real value..
Hmm? Am I barking up entirely the wrong tree? Or does this seem doable
and reasonable?
Linus
On Thu, Jun 06, 2024 at 11:42:40AM -0700, Linus Torvalds wrote:
> So this is related to my currently very ugly hack at
>
> https://lore.kernel.org/all/CAHk-=whFSz=usMPHHGAQBnJRVAFfuH4gFHtgyLe0YET75zYRzA@mail.gmail.com/
>
> where I'm trying to do "runtime constants". That patch actually works,
> but it's flawed in many ways, and one of the ways it is flawed is that
> I really want to put the "this is the use for symbol X" in a section
> of its own for each X.
>
> Now, creating the sections is trivial, that's not the problem. I'd
> just make the asm do
>
> ".pushsection .static_const." #sym ",\"a\"\n\t" \
> ...
> ".popsection"
>
> and the linker script will just do
>
> KEEP(*(.static_const.*))
>
> and I'm done. Nice individual sections for each of the runtime constant symbols.
>
> However, for the fixup part, I then really want the section start and
> end addresses, so that I can iterate over those uses for a particular
> named symbol.
That should be trivial. But ideally the interface would be less magical
and wouldn't need objtool to sprinkle any pixie dust. Could it be
implemented similar to static keys by making the static const variable
an opaque structure instead of just a "normal" variable?
DEFINE_STATIC_CONST(dentry_hashtable);
That could create something similar to 'struct static_key' which
correlates the const variable with all its use sites.
--
Josh
On Thu, 6 Jun 2024 at 12:45, Josh Poimboeuf <[email protected]> wrote:
>
> That should be trivial. But ideally the interface would be less magical
> and wouldn't need objtool to sprinkle any pixie dust. Could it be
> implemented similar to static keys by making the static const variable
> an opaque structure instead of just a "normal" variable?
So I will absolutely add some wrappers - we'll need them just to make
the "no static constants" case work.
I doubt I'll need any more than a simple wrapper, because there is no
equivalent of a trampoline when it comes to static constants - the
whole point is avoiding any indirection at all, and generating better
code.
So the only "wrapper" needed is the "different architectures will have
to have different ways to generate the runtime constant operations"
and the "with no support, it will just use the variable as before".
But the data itself is fundamentally embedded in the inline asm that
generates the constant op (note that I say "op" - it's not just
loading a constant, it's using a constant as *part* of the
instruction).
IOW, think of this more as an "alternative asm", where the alternative
is about the constants used in order to avoid memory loads and extra
registers.
I did name it after the "static call" model, but it's actually pretty different.
For example, if we ever extend this to modules, then any constants
will be fixed up by the module loader. There will not ever be any
dynamic rewriting like static calls support.
This would be for some very core constants only. Not random code that
wants to avoid an indirect call.
My code did the dentry hash table and size, but I would envision it
would be for pseudo-constants like __VIRTUAL_MASK_SHIFT etc (which
_used_ to be a big pain for put_user() and friends, but we solved it
with a different model).
Things that depend purely on boot time stuff.
> DEFINE_STATIC_CONST(dentry_hashtable);
>
> That could create something similar to 'struct static_key' which
> correlates the const variable with all its use sites.
Oh, honestly, I don't need the disgusting things that static_calls()
do. The patch I already have is better than that.
IOW, already have the list of where the static call data is (the thing
that the .static_call_sites section contains).
Sure, it's not a "single" array of sites, because my sections are
partitioned by size (the static calls use a flag value instead), and
my "key" for the array traversal is just the address of the runtime
constant that they deal with.
It all works fine, although it will need some abstraction to deal with
different architectures.
I'm not at all interested in creating some fancy linked list of those
things like static calls do in __static_call_init() at runtime.,
This is literally for a one-time constant for built-in code, I think
the static call code is completely over-designed and inappropriate for
this.
So what I'm interested in would be to simplify things, and get rid of
the "key" entirely, because with a good symbol for the start and end
of the array of users (for _each_ pseudo-constant symbol), it all
makes the code a lot more straightforward.
In particular, it would make the architecture side much more
straightforward, because instead of traversing some "array of keys and
types" and pick up the ones that match, it would just traverse an
array that is already pre-matched, because each key goes into its own
section.
So I want to *simplify* the code, not make it the horrendous
complicated mess that is static calls. No "build up that list of
sites" at run-time.
Now, I actually suspect that doing this thing would then allow *other*
cases - like possibly static calls - to simplify their code too. I
think the static calls could easily use this to simplify the code.
So I would throw your question back at you and say "No! I'm asking for
this support because it would allow me to *not* do even a simplified
version of the horrible things that static calls do".
Linus
On Thu, Jun 06, 2024 at 02:10:30PM -0700, Linus Torvalds wrote:
> So what I'm interested in would be to simplify things, and get rid of
> the "key" entirely, because with a good symbol for the start and end
> of the array of users (for _each_ pseudo-constant symbol), it all
> makes the code a lot more straightforward.
>
> In particular, it would make the architecture side much more
> straightforward, because instead of traversing some "array of keys and
> types" and pick up the ones that match, it would just traverse an
> array that is already pre-matched, because each key goes into its own
> section.
>
> So I want to *simplify* the code, not make it the horrendous
> complicated mess that is static calls. No "build up that list of
> sites" at run-time.
>
> Now, I actually suspect that doing this thing would then allow *other*
> cases - like possibly static calls - to simplify their code too. I
> think the static calls could easily use this to simplify the code.
>
> So I would throw your question back at you and say "No! I'm asking for
> this support because it would allow me to *not* do even a simplified
> version of the horrible things that static calls do".
The "key" idea was probably misguided. I didn't mean to suggest to make
it all complex like static branches/calls, I was just hoping there's a
simple way to implement "static consts" without needing objtool.
Aside from the idea of avoiding "magic", another small downside of using
objtool is that isn't currently supported on most arches. That said, it
should be easy enough to make it arch-independent by default for simple
features like this, it just needs a little refactoring.
I still get the feeling there's a way to keep static consts simple
without objtool, but I'm too knee-deep in another problem at the moment
to be able to back that feeling up with much technical merit.
So while I'm not yet necessarily conceding that objtool is really needed
here, I could work up a quick objtool patch. It would just be x86-only,
is that ok for the time being?
--
Josh
On Thu, 6 Jun 2024 at 15:19, Josh Poimboeuf <[email protected]> wrote:
>
> The "key" idea was probably misguided. I didn't mean to suggest to make
> it all complex like static branches/calls, I was just hoping there's a
> simple way to implement "static consts" without needing objtool.
I don't think you ever actually looked at the patch I pointed to, did you?
The patch already works. This all works without any objtool magic. See
here again:
https://lore.kernel.org/all/CAHk-=whFSz=usMPHHGAQBnJRVAFfuH4gFHtgyLe0YET75zYRzA@mail.gmail.com/
but in that patch, look at this part:
while (start < end) {
unsigned long where, sym;
where = start[0] + (unsigned long)(start+0);
sym = start[1] + (unsigned long)(start+1);
start += 2;
if (sym != (unsigned long)addr)
continue;
where we traverse all the static constant uses (for the right size),
and where that 'sym' is the key that we then use to say "this is the
one I actually am modifying".
Is it complex? Not really.
But it's *unnecessary*.
It would be so nice to just have a "this is the start/end of the
section dedicated to this symbol".
In fact, the only use of that key is exactly that fixup code. It's
literally only used for matching the address of the "constant pool"
entry that we're replacing. So if we just had better link-time
information, all of this would just GoAway(tm).
Now, the part that really made me want this wasn't actually that
existing patch that has that one extra "match symbol address" thing,
but thinking about making the associated helpers cleaner.
In particular, this part:
static_const_init(1, d_hash_shift);
static_const_init(8, dentry_hashtable);
that initializes the constants has this very ugly hardcoded knowledge
about the particular bucket that those constants are in. And it was
when removing that magic "1" and "8" that I went "wouldn't it be nice
if I could look up the bucket data for this thing directly"
(Note that the 1/8 is *not* "sizeof()" the data - it's the
architecture-specific size, and it's combined using preprocessor '##'
magic into the right address).
And yes, I can fix this with some more wrapper magic when declaring
the variable. It's not going to be even remotely as nasty as static
calls, but some little objtool magic would obviate the need for all of
this.
> Aside from the idea of avoiding "magic", another small downside of using
> objtool is that isn't currently supported on most arches.
Honestly, I don't think that's a problem. If the requirement for
run-time constant support is objtool support, that's more than fine.
> So while I'm not yet necessarily conceding that objtool is really needed
> here, I could work up a quick objtool patch. It would just be x86-only,
> is that ok for the time being?
Absolutely.
Yes, this kind of "section start/end" symbol would be possibly useful
as a generic feature across all architectures, but even as a
x86-specific one there are already other things that could use it and
not just the static constant hack.
We'd be able to simplify the x86 vmlinux.lds.S file - even if we
couldn't necessarily simplify the generic one. There are several cases
of exactly the same kind of "start/end of section symbols" in that
file, eg
__x86_cpu_dev_start = .;
*(.x86_cpu_dev.init)
__x86_cpu_dev_end = .;
__x86_intel_mid_dev_start = .;
*(.x86_intel_mid_dev.init)
__x86_intel_mid_dev_end = .;
__retpoline_sites = .;
*(.retpoline_sites)
__retpoline_sites_end = .;
__return_sites = .;
*(.return_sites)
__return_sites_end = .;
__cfi_sites = .;
*(.cfi_sites)
__cfi_sites_end = .;
__alt_instructions = .;
*(.altinstructions)
__alt_instructions_end = .;
__apicdrivers = .;
*(.apicdrivers);
__apicdrivers_end = .;
just from a quick look. And yes, they could also just use the
BOUNDED_SECTION() macro we have for this pattern.
Imagine if we could just *not* touch that magical vmlinux.lds.S file
at all when we add some new random section for some random new use -
CPU bugs, firmware tables, things like that. And when we want the
beginning of the section, we'd just use a generic section name, like
unsigned char __sec_apicdrivers_start[], __sec_apicdrivers_end[];
instead of those magic hardcoded ones that don't even have a pattern
(it looks like start/end is about a third of the cases, and
nothing/end is two thirds).
So my real point here is that this pattern isn't actually anything
unusual - and the only thing special about my static const patch is
that I don't have one single fixed section name, I have a "section
name pattern", which is why I'd like to automate it rather than list
them individually in the vmlinux.lds.S file.
Because yes, I considered just having another helper macro and then
just a "list each constant name in the vmlinux.lds.h file", and
decided that that would be just too ugly.
It's clearly what our *existing* usage pattern is. But I think we can
improve on it.
Linus
On Thu, Jun 06, 2024 at 03:54:34PM -0700, Linus Torvalds wrote:
> On Thu, 6 Jun 2024 at 15:19, Josh Poimboeuf <[email protected]> wrote:
> > So while I'm not yet necessarily conceding that objtool is really needed
> > here, I could work up a quick objtool patch. It would just be x86-only,
> > is that ok for the time being?
>
> Absolutely.
This adds a new objtool "--bounds" action which creates a
__sec_<secname>_{start,end}
symbol for every section.
The only testing I've done has been staring cross-eyed at the readelf
output on a defconfig kernel.
If you have CONFIG_X86_KERNEL_IBT -- which causes objtool to run on
vmlinux.o -- you can enable bounds symbol creation by adding
OBJTOOL_ARGS="--bounds" to your make cmdline:
make -j$(nproc) -s OBJTOOL_ARGS="--bounds"
Otherwise it would need to be manually added with
objtool --bounds --link vmlinux.o
diff --git a/tools/objtool/builtin-check.c b/tools/objtool/builtin-check.c
index 5e21cfb7661d..e1a86981fd96 100644
--- a/tools/objtool/builtin-check.c
+++ b/tools/objtool/builtin-check.c
@@ -68,6 +68,7 @@ static int parse_hacks(const struct option *opt, const char *str, int unset)
static const struct option check_options[] = {
OPT_GROUP("Actions:"),
OPT_CALLBACK_OPTARG('h', "hacks", NULL, NULL, "jump_label,noinstr,skylake", "patch toolchain bugs/limitations", parse_hacks),
+ OPT_BOOLEAN('b', "bounds", &opts.bounds, "generate section start/end bounds symbols"),
OPT_BOOLEAN('i', "ibt", &opts.ibt, "validate and annotate IBT"),
OPT_BOOLEAN('m', "mcount", &opts.mcount, "annotate mcount/fentry calls for ftrace"),
OPT_BOOLEAN('n', "noinstr", &opts.noinstr, "validate noinstr rules"),
@@ -131,7 +132,8 @@ int cmd_parse_options(int argc, const char **argv, const char * const usage[])
static bool opts_valid(void)
{
- if (opts.hack_jump_label ||
+ if (opts.bounds ||
+ opts.hack_jump_label ||
opts.hack_noinstr ||
opts.ibt ||
opts.mcount ||
@@ -199,6 +201,11 @@ static bool link_opts_valid(struct objtool_file *file)
return false;
}
+ if (opts.bounds) {
+ ERROR("--bounds requires --link");
+ return false;
+ }
+
return true;
}
diff --git a/tools/objtool/check.c b/tools/objtool/check.c
index 0a33d9195b7a..59e6638db83f 100644
--- a/tools/objtool/check.c
+++ b/tools/objtool/check.c
@@ -4213,6 +4213,124 @@ static int add_prefix_symbols(struct objtool_file *file)
return 0;
}
+static bool is_discarded_sec(struct section *sec)
+{
+ static const char * const discards[] = {
+ ".note.gnu.property",
+ ".export_symbol",
+ ".modinfo",
+ "__patchable_function_entries",
+ ".exitcall.exit", // not discarded for modules
+ };
+
+ if (!(sec->sh.sh_flags & SHF_ALLOC) || strstarts(sec->name, ".discard"))
+ return true;
+
+ for (int i = 0; i < ARRAY_SIZE(discards); i++)
+ if (!strcmp(sec->name, discards[i]))
+ return true;
+
+ return false;
+}
+
+#define BOUNDS_SEC_NAME ".rodata.sec_bounds"
+
+static int create_bounds_section(struct objtool_file *file)
+{
+ struct section *bounds_sec, *sec;
+ unsigned int idx;
+
+ sec = find_section_by_name(file->elf, BOUNDS_SEC_NAME);
+ if (sec) {
+ INIT_LIST_HEAD(&file->static_call_list);
+ WARN("file already has .sec_bounds section, skipping");
+ return 0;
+ }
+
+ idx = 0;
+ for_each_sec(file, sec)
+ if (sec->idx && !is_reloc_sec(sec) && !is_discarded_sec(sec))
+ idx++;
+
+ bounds_sec = elf_create_section_pair(file->elf, BOUNDS_SEC_NAME,
+ sizeof(long) * 2, idx, idx * 2);
+ if (!sec)
+ return -1;
+
+ idx = 0;
+ for_each_sec(file, sec) {
+ if (sec->idx && !is_reloc_sec(sec) && !is_discarded_sec(sec)) {
+
+ char sanitized_name[116];
+ char start_name[128];
+ char end_name[128];
+
+ if (!strcmp(sec->name, BOUNDS_SEC_NAME))
+ continue;
+
+ if (!sec->sym && !elf_create_section_symbol(file->elf, sec))
+ return -1;
+
+ strncpy(sanitized_name, sec->name, sizeof(sanitized_name) - 1);
+ for (char *s = sanitized_name; *s; s++)
+ if (*s == '.')
+ *s = '_';
+
+ snprintf(start_name, 256, "__sec%s_start", sanitized_name);
+ snprintf(end_name, 256, "__sec%s_end", sanitized_name);
+
+ if (find_symbol_by_name(file->elf, start_name) ||
+ find_symbol_by_name(file->elf, end_name))
+ continue;
+
+ /* 'start' symbol */
+ if (!elf_create_symbol(file->elf,
+ start_name,
+ bounds_sec,
+ STB_GLOBAL,
+ STT_OBJECT,
+ idx * 2 * sizeof(long),
+ sizeof(long)))
+ return -1;
+
+ if (!elf_create_symbol(file->elf,
+ end_name,
+ bounds_sec,
+ STB_GLOBAL,
+ STT_OBJECT,
+ (idx * 2 * sizeof(long)) + sizeof(long),
+ sizeof(long)))
+ return -1;
+
+ /* 'start' reloc */
+ if (!elf_init_reloc(file->elf,
+ bounds_sec->rsec,
+ idx * 2,
+ idx * 2 * sizeof(long),
+ sec->sym,
+ 0,
+ R_ABS64))
+ return -1;
+
+ /* 'end' reloc */
+ if (!elf_init_reloc(file->elf,
+ bounds_sec->rsec,
+ (idx * 2) + 1,
+ (idx * 2 * sizeof(long)) + sizeof(long),
+ sec->sym,
+ sec->sh.sh_size,
+ R_ABS64))
+ return -1;
+
+
+ idx++;
+ }
+ }
+
+
+ return 0;
+}
+
static int validate_symbol(struct objtool_file *file, struct section *sec,
struct symbol *sym, struct insn_state *state)
{
@@ -4826,6 +4944,13 @@ int check(struct objtool_file *file)
warnings += ret;
}
+ if (opts.bounds) {
+ ret = create_bounds_section(file);
+ if (ret < 0)
+ goto out;
+ warnings += ret;
+ }
+
if (opts.ibt) {
ret = create_ibt_endbr_seal_sections(file);
if (ret < 0)
diff --git a/tools/objtool/elf.c b/tools/objtool/elf.c
index 3d27983dc908..b596e992ca80 100644
--- a/tools/objtool/elf.c
+++ b/tools/objtool/elf.c
@@ -788,8 +788,7 @@ __elf_create_symbol(struct elf *elf, struct symbol *sym)
return sym;
}
-static struct symbol *
-elf_create_section_symbol(struct elf *elf, struct section *sec)
+struct symbol *elf_create_section_symbol(struct elf *elf, struct section *sec)
{
struct symbol *sym = calloc(1, sizeof(*sym));
@@ -811,6 +810,8 @@ elf_create_section_symbol(struct elf *elf, struct section *sec)
if (sym)
elf_add_symbol(elf, sym);
+ sec->sym = sym;
+
return sym;
}
@@ -845,10 +846,37 @@ elf_create_prefix_symbol(struct elf *elf, struct symbol *orig, long size)
return sym;
}
-static struct reloc *elf_init_reloc(struct elf *elf, struct section *rsec,
- unsigned int reloc_idx,
- unsigned long offset, struct symbol *sym,
- s64 addend, unsigned int type)
+struct symbol *elf_create_symbol(struct elf *elf, char *name,
+ struct section *sec, unsigned int bind,
+ unsigned int type, unsigned long offset,
+ size_t size)
+{
+ struct symbol *sym = calloc(1, sizeof(*sym));
+
+ if (!sym) {
+ perror("calloc");
+ return NULL;
+ }
+
+ sym->name = strdup(name);
+ sym->sec = sec;
+
+ sym->sym.st_name = elf_add_string(elf, NULL, sym->name);
+ sym->sym.st_info = GELF_ST_INFO(bind, type);
+ sym->sym.st_value = offset;
+ sym->sym.st_size = size;
+
+ sym = __elf_create_symbol(elf, sym);
+ if (sym)
+ elf_add_symbol(elf, sym);
+
+ return sym;
+}
+
+struct reloc *elf_init_reloc(struct elf *elf, struct section *rsec,
+ unsigned int reloc_idx,
+ unsigned long offset, struct symbol *sym,
+ s64 addend, unsigned int type)
{
struct reloc *reloc, empty = { 0 };
@@ -906,8 +934,6 @@ struct reloc *elf_init_reloc_text_sym(struct elf *elf, struct section *sec,
sym = elf_create_section_symbol(elf, insn_sec);
if (!sym)
return NULL;
-
- insn_sec->sym = sym;
}
return elf_init_reloc(elf, sec->rsec, reloc_idx, offset, sym, addend,
diff --git a/tools/objtool/include/objtool/builtin.h b/tools/objtool/include/objtool/builtin.h
index fcca6662c8b4..c0a73e247722 100644
--- a/tools/objtool/include/objtool/builtin.h
+++ b/tools/objtool/include/objtool/builtin.h
@@ -30,6 +30,7 @@ struct opts {
/* options: */
bool backtrace;
bool backup;
+ bool bounds;
bool dryrun;
bool link;
bool mnop;
diff --git a/tools/objtool/include/objtool/elf.h b/tools/objtool/include/objtool/elf.h
index 2b8a69de4db8..a2f9c1b4ca88 100644
--- a/tools/objtool/include/objtool/elf.h
+++ b/tools/objtool/include/objtool/elf.h
@@ -113,8 +113,20 @@ struct section *elf_create_section_pair(struct elf *elf, const char *name,
size_t entsize, unsigned int nr,
unsigned int reloc_nr);
+struct symbol *elf_create_symbol(struct elf *elf, char *name,
+ struct section *sec, unsigned int bind,
+ unsigned int type, unsigned long offset,
+ size_t size);
+
+struct symbol *elf_create_section_symbol(struct elf *elf, struct section *sec);
+
struct symbol *elf_create_prefix_symbol(struct elf *elf, struct symbol *orig, long size);
+struct reloc *elf_init_reloc(struct elf *elf, struct section *rsec,
+ unsigned int reloc_idx,
+ unsigned long offset, struct symbol *sym,
+ s64 addend, unsigned int type);
+
struct reloc *elf_init_reloc_text_sym(struct elf *elf, struct section *sec,
unsigned long offset,
unsigned int reloc_idx,
On 06/06/2024 20.42, Linus Torvalds wrote:
> So this is related to my currently very ugly hack at
>
> https://lore.kernel.org/all/CAHk-=whFSz=usMPHHGAQBnJRVAFfuH4gFHtgyLe0YET75zYRzA@mail.gmail.com/
>
> where I'm trying to do "runtime constants". That patch actually works,
> but it's flawed in many ways, and one of the ways it is flawed is that
> I really want to put the "this is the use for symbol X" in a section
> of its own for each X.
>
> Now, creating the sections is trivial, that's not the problem. I'd
> just make the asm do
>
> ".pushsection .static_const." #sym ",\"a\"\n\t" \
> ...
> ".popsection"
>
> and the linker script will just do
>
> KEEP(*(.static_const.*))
>
> and I'm done. Nice individual sections for each of the runtime constant symbols.
I'm probably missing something, but isn't this exactly what you get for
free if you avoid using dots and other non-identifier symbols in the
section names, i.e. make it "__static_const__" #sym or whatnot:
https://sourceware.org/binutils/docs/ld/Input-Section-Example.html
If an output section's name is the same as the input section's name
and is representable as a C identifier, then the linker will
automatically *note PROVIDE:: two symbols: __start_SECNAME and
__stop_SECNAME, where SECNAME is the name of the section. These
indicate the start address and end address of the output section
respectively. Note: most section names are not representable as C
identifiers because they contain a '.' character.
And this tiny test program suggests that works as advertised, though I
don't know if we use some linker flag or the fact that we use our own
linker script then somehow suppresses that behaviour:
#include <stdio.h>
#define x(name, val) asm(".pushsection __test__" #name ",\"a\"\n\t" \
".long " #val "\n\t" \
".popsection\n")
extern char __start___test__foo[];
extern char __stop___test__foo[];
extern char __start___test__bar[];
extern char __stop___test__bar[];
int main(int argc, char *argv[])
{
int *p;
x(foo, 10);
x(bar, 12);
x(foo, 47);
printf("Contents of foo section:\n");
for (p = (int*)__start___test__foo; p < (int*)__stop___test__foo; ++p)
printf("%d\n", *p);
printf("Contents of bar section:\n");
for (p = (int*)__start___test__bar; p < (int*)__stop___test__bar; ++p)
printf("%d\n", *p);
return 0;
}
Rasmus
On 06/06/2024 20.42, Linus Torvalds wrote:
> So this is related to my currently very ugly hack at
>
> https://lore.kernel.org/all/CAHk-=whFSz=usMPHHGAQBnJRVAFfuH4gFHtgyLe0YET75zYRzA@mail.gmail.com/
>
> where I'm trying to do "runtime constants".
FWIW, I did a POC some years ago but either never managed to send it, or
never got a response. It did boot in virtme and I managed to get gdb to
do disassembly to show that the dentry hash lookup did become a 'shift
immediate'.
https://github.com/Villemoes/linux/tree/rai
IIRC, I didn't bother with updating users when the boottime value was
settled, but just had a single at-the-end-of-init call to update all
sites; in the meantime, use sites were implemented as a jump to a piece
of code that did the dynamic load of the constants.
Rasmus
On Fri, 7 Jun 2024 at 02:39, Rasmus Villemoes <[email protected]> wrote:
>
> I'm probably missing something, but isn't this exactly what you get for
> free if you avoid using dots and other non-identifier symbols in the
> section names, i.e. make it "__static_const__" #sym or whatnot:
LOL. You're not missing anything - I am. I clearly missed this linker
rule entirely when I was looking for some explicit way to set these
start/end symbols, because that rule - which is almost exactly what I
wanted - is implicit.
That said, I say "almost exactly", because I can't get it to work.
Why? You need to match the output section name with the input section,
and since the whole point was that the input section was a *pattern*,
I can't do that.
I can hardcode the section names, which fixes it, but that is what I
wanted to avoid (once I hardcode the section names I could have just
added the start/end symbols by hand).
That said, clearly there's a way to just do it, since your
test-program - using the built-in linker script can do it.
I do worry that I also need that "AT()" logic to subtract the LOAD_OFFSET.
I really don't know linker scripts very well. I spent more time than I
want to admit to just because of missing whitespace.
Linus
On Fri, 7 Jun 2024 at 02:52, Rasmus Villemoes <[email protected]> wrote:
>
> FWIW, I did a POC some years ago but either never managed to send it, or
> never got a response. It did boot in virtme and I managed to get gdb to
> do disassembly to show that the dentry hash lookup did become a 'shift
> immediate'.
>
> https://github.com/Villemoes/linux/tree/rai
Looks conceptually very similar to what I do, except I literally
_only_ rewrite the constant itself in the instruction stream.
You end up using these replacement sequences, which certainly works,
but limits your instruction scheduling a bit (ie the minimal size ends
up being a full branch to the thunk.
I started out wanting to literally replace a single 8-bit constant in
a shift-instruction that might be smaller than the jump.
That said, you then made your approach work just fine by just
combining the shift with the address load, so it's not a single small
instruction that gets replaced any more.
And honestly, I think your approach may be better than mine.
The "replace constant in one instruction" approach works fine in my
tests, and gcc in particular seems to actually take advantage of the
instruction scheduling freedom (clang less so).
But your thunking approach would probably be much easier on
architectures like arm64 where the "load a constant" thing can be a
lot less convenient than one single contiguous value in memory.
Would you be willing to resurrect your thing for a modern kernel? I'll
certainly try it out next to mine?
Linus