loadable modules: add support for loading modules on Xtensa

[lyakh]

This is based on an earlier version of #57086 . The latter has a few more commits now, but the final state isn't very different from my snapshot - @teburd has fixed a couple of bugs that I also found and fixed in this PR, added support for some ARM specific features, and started work on Xtensa relocation code, which isn't used here yet. So far this code can load shared objects, built as PIC, which don't need relocation. This is still WiP, I expect comments, fixes, and a couple more iterations of this.

[teburd]

I've worked on incorporating the EXPORT_SYMBOL work from this PR, which was great. I've reused some existing macros/linker scripts that are useful beyond just ace to do that. Iterable sections and common-ram.ld for now. So same general idea but slight variation on how its implemented.

Ideally in the base image the symbol table would be const and placed in rom for most systems, and in loadable modules perhaps a custom linker section could be constructed and then imported to determine which symbols are actually intentionally exported as opposed to the ones marked extern as I've done so far.

For loadable modules we likely want the symbol table to be still placed in RAM so that it can be updated on relocation.

I don't think generally we can rely on module_peek though. When memory is allocated for a loadable module, on MMU systems especially though also for MPU systems, we likely need to setup some memory permissions (e.g. RO, RW, RX) for the various sections and I don't think its enough to simply point at an arbitrary address in the ELF stream to do that. So module_peek is unlikely to make it in without maybe some pre-defined page aligned sections making the elf stream potentially much bigger than it needs to be.

I could be wrong though, and please correct me if so!

[lyakh]

I don't think generally we can rely on module_peek though.

@teburd I thought it would be a bit controversial. But so far I see only one implementation of module operations in your PR - one for buffers. And in it your module_buf_read() does just a memcpy(), so it does assume direct access to that memory. And as you see .module_peek() is extremely convenient - it eliminates the need to copy large memory areas of unknown size, both saving memory and time. I'd very much like to find a way to cleanly integrate it.

[teburd]

I don't think generally we can rely on module_peek though.

@teburd I thought it would be a bit controversial. But so far I see only one implementation of module operations in your PR - one for buffers. And in it your module_buf_read() does just a memcpy(), so it does assume direct access to that memory. And as you see .module_peek() is extremely convenient - it eliminates the need to copy large memory areas of unknown size, both saving memory and time. I'd very much like to find a way to cleanly integrate it.

So just to clarify my thinking here... and put into github our conversation a bit

Not all streams will come from memory that can be mapped into an addressable space, its very likely some elfs will come from off-chip flash that can be read from and copied out but not mapped.

At the same time I completely agree that if you have memory that can be directly mapped, and you happen to have an elf file that is padded and aligned appropriately... as is I believe your case, it should be entirely supported to directly map the elf.

In the buffer case, its pretty lame that there's allocations for things like elf headers and section headers when its... right there and can be pointed at.

So to support all of these cases I think a little bit of rethinking on the module stream interface is needed to support it.

Basic idea would be to modify module_stream_read to take... the stream, memory kind (rx, ro, rw, metadata), and length to read as parameters, while returning a void *.

e.g.

void* module_read(struct module_stream *ms, enum module_mem_kind *mk, size_t sz);

The stream itself can be the allocator then when it needs to be, or it can simply pass back a pointer into the elf. And since the stream API is overrideable it means any of the above scenarios is easily dealt with, leaving the load and link functions to do what the need to without worrying about it or having branching all over the place.

This is likely worth doing after the initial PR though as the initial PR is already quite close and mostly needs one other architecture to work imo.

[lyakh]

void* module_read(struct module_stream *ms, enum module_mem_kind *mk, size_t sz);

mk doesn't have to be a pointer, does it? Otherwise - yes! One could also simplify the API a bit more by removing module_seek() and adding an offset to module_read() but that's optional - I'm fine either way.

The stream itself can be the allocator then when it needs to be, or it can simply pass back a pointer into the elf. And since the stream API is overrideable it means any of the above scenarios is easily dealt with, leaving the load and link functions to do what the need to without worrying about it or having branching all over the place.
This is likely worth doing after the initial PR though as the initial PR is already quite close and mostly needs one other architecture to work imo.

As also just discussed offline, internal allocation is difficult because it's unknown when to free that memory. I think we need an additional memory pointer for the call. The caller will provide a buffer - be it allocated, on stack or static. And if the caller knows, that they can peek at the image, they'll supply NULL for the buffer

Otherwise this version now works both with twister with the added here Xtensa ELF object, and with SOF.

[lyakh]

This now temporarily includes #64381 until it's merged

[lyakh]

This is an interesting failure "Illegal use of the EPSR" https://github.com/zephyrproject-rtos/zephyr/actions/runs/6665109962/job/18114283475?pr=62433 @teburd any ideas?

[lyakh]

This is an interesting failure "Illegal use of the EPSR" https://github.com/zephyrproject-rtos/zephyr/actions/runs/6665109962/job/18114283475?pr=62433 @teburd any ideas?

This failure only happens on Cortex M3 and it's introduced by dbd17fa i.e. by the commit, that switches from copying ELF sections to using them in their existing buffers. Other tested ARM architectures work with that configuration, only Cortex M3 has a problem with it.

[teburd]

This is an interesting failure "Illegal use of the EPSR" https://github.com/zephyrproject-rtos/zephyr/actions/runs/6665109962/job/18114283475?pr=62433 @teburd any ideas?

No ideas from me no, I can try and take a look at this closer today. There's another issue on cortex-m7 as well with dcache involved.

Maybe alignment related?

It might be worth trying to break this PR up a bit into some small PRs that get merged incrementally. PLT/GOT support with -fPIC, xtensa support, and maybe lastly the peek + copy-free usage perhaps?

[lyakh]

It might be worth trying to break this PR up a bit into some small PRs that get merged incrementally. PLT/GOT support with -fPIC, xtensa support, and maybe lastly the peek + copy-free usage perhaps?

@teburd can we do it in a different order - first add llext_peek() and then add PLT? I'd prefer to add Xtensa support on top of a peek-enabled implementation directly?

[cfriedt]

None on my end, but there may be a couple of still relevant compliance issues. Likely the EXPORT_SYMBOL() warnings can be ignored.

[lyakh]

None on my end, but there may be a couple of still relevant compliance issues. Likely the EXPORT_SYMBOL() warnings can be ignored.

@cfriedt I cleaned up a couple of checkpatch / compliance failures, the rest are false positives: EXPORT_SYMBOL() in a separate file unlike Linux kernel, and a const array that has no need to be static. If you have no further objections, maybe you could drop your request for change as fulfilled.

I'm not blocking anymore

[lyakh]

@nashif @teburd @pillo79 status update: only EXPORT_SYMBOL() checkpatch complaints remaining, CI clean otherwise

[teburd]

We might need a few CI checks fixed, let me look, but approved

[nashif]

overriding checkpatch as false negatives.