Feature - Adjustments of SIMD-0189 #92

Lichtso · 2025-11-06T11:09:26Z

Adjusts the e_machine to EM_BPF
Ignores the e_type
Removes the stack and heap program headers
Reorders the bytecode and rodata program headers

LucasSte · 2025-11-06T20:18:13Z

src/elf.rs

-                || program_header.p_paddr != *p_vaddr
-                || program_header.p_filesz != p_filesz
+                || program_header.p_vaddr != p_vaddr
+                || program_header.p_paddr != p_vaddr


Is this relevant for us? p_addr is the physical address, isn't it?

I did not change anything about p_addr in this PR, just removed the deref.

I'm just wondering if we shouldn't disregard this field.

Unless there is tooling which likes to emit this value differently I would say we pin as much as we can.

I don't think there is any problem there. Those fields are controlled by the linker script. If they are working now, they will continue working with the new layout, I believe.

LucasSte · 2025-11-06T20:20:41Z

tests/elfs/elf.ld

    . = ALIGN(8);
  } :rodata
+  .text 0x100000000 : {
+     *(.text*)


No need to do it here, but if you want, you can remove the .bss.stack and .bss.heap from the linker script.

LucasSte · 2025-11-06T20:22:24Z

tests/execution.rs

        (),
        TestContextObject::new(3),
-        ProgramResult::Ok(ebpf::MM_BYTECODE_START),
+        ProgramResult::Ok(ebpf::MM_RODATA_START),


The address of the entrypoint is MM_BYTECODE_START, isn't it?

Jup, see answer for the other test.

LucasSte · 2025-11-06T20:23:00Z

tests/execution.rs

-        TestContextObject::new(4),
-        ProgramResult::Ok(ebpf::MM_RODATA_START),
+        TestContextObject::new(3),
+        ProgramResult::Ok(ebpf::MM_BYTECODE_START),


All these tests are returning the inverted addresses, aren't they?

That is because I did not change the constants naming in this PR. And that again has to do with the AlignedMemory mapping rejecting a region at address 0. We can fix that once we enable account data direct mapping any only use the UnalignedMemory mapping in production.

If you want I can put the numeric values here to avoid confusion in the meantime.

To make tests comprehensive, you could just compare with the number itself and add a comment next to it explaining what section that number refers to in the v3 organization.

I rebased the PR ontop of the changes which allow us to now swap / rename these two constants.

LucasSte · 2025-11-19T13:45:37Z

src/elf.rs

-            text_section_vaddr: if sbpf_version.enable_lower_bytecode_vaddr() {
-                ebpf::MM_BYTECODE_START
-            } else {
+            ro_section: Section::Borrowed(ebpf::MM_BYTECODE_START as usize, 0..text_bytes.len()),


Shouldn't this be MM_RODATA_START for v3?

LucasSte · 2025-11-19T13:48:24Z

src/elf.rs

+            text_section_vaddr: if sbpf_version.enable_lower_rodata_vaddr() {
                ebpf::MM_RODATA_START
+            } else {
+                ebpf::MM_BYTECODE_START


So, enable_lower_rodata_vaddr returns true for a version greater than v3, but you are saying that the text section vaddr is MM_RODATA_START. Shouldn't this if be inverted?

buffalojoec · 2025-11-25T04:28:03Z

src/memory_region.rs

+            let region_name = match vm_addr & (!ebpf::MM_BYTECODE_START.saturating_sub(1)) {
+                ebpf::MM_BYTECODE_START => "program",


Should we include another match arm in here for ebpf::MM_RODATA_START?

We could treat both as "program", like we do in < SBPF v3, or we could have "bytecode" and "rodata" messages. Seems probably useful to distinguish between the two, right?

https://github.com/anza-xyz/sbpf/pull/45/files

We didn't merge that PR yet, because we have not yet finalized the ABIv2 design.

Ah okay, thanks. Does it hurt to add the "rodata" arm here though?

LucasSte · 2025-11-26T21:09:21Z

src/elf.rs

            if program_header.p_type != PT_LOAD
                || program_header.p_flags != *p_flags
-                || program_header.p_offset < program_header_table_range.end as u64
+                || program_header.p_offset != expected_offset


This restriction is not guaranteed to work. I tested it here and LLD does not output it automatically. In one of the cases, I could only get it once I called llvm-objcopy --strip-all. I suggest reverting it to the original restriction.

llvm-objcopy --strip-all does not work with enable_symbol_and_section_labels set to true.

Lichtso force-pushed the feature/adjustments_of_simd_0189 branch 2 times, most recently from f72af30 to f8a44d5 Compare November 6, 2025 12:45

LucasSte self-requested a review November 6, 2025 19:59

LucasSte reviewed Nov 6, 2025

View reviewed changes

Lichtso force-pushed the feature/adjustments_of_simd_0189 branch from f8a44d5 to 526b717 Compare November 18, 2025 16:12

LucasSte reviewed Nov 19, 2025

View reviewed changes

Lichtso force-pushed the feature/adjustments_of_simd_0189 branch 2 times, most recently from e277dab to a898615 Compare November 24, 2025 09:45

Lichtso added 2 commits November 24, 2025 15:21

Swaps MM_BYTECODE_START and MM_RODATA_START.

e1f83b0

Adjusts the e_type, e_machine and program headers.

8e313a0

Lichtso force-pushed the feature/adjustments_of_simd_0189 branch from a898615 to 8e313a0 Compare November 24, 2025 15:22

buffalojoec reviewed Nov 25, 2025

View reviewed changes

Lichtso requested a review from LucasSte November 26, 2025 17:58

LucasSte reviewed Nov 26, 2025

View reviewed changes

		let region_name = match vm_addr & (!ebpf::MM_BYTECODE_START.saturating_sub(1)) {
		ebpf::MM_BYTECODE_START => "program",

Feature - Adjustments of SIMD-0189 #92

Are you sure you want to change the base?

Feature - Adjustments of SIMD-0189 #92

Conversation

Lichtso commented Nov 6, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants