Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feature - lduw (load upper word immediate) #486

Merged
merged 4 commits into from
Jul 26, 2023

Conversation

Lichtso
Copy link

@Lichtso Lichtso commented Jul 20, 2023

The current lddw (load double word immediate) instruction requires many special cases:

  • It is the only instruction which takes two 64 bit slots instead of one, and thus extra counting is required as one can not just divide the text section length by 8 to get the number of instructions.
  • It only consumes one CU instead of two, so the instruction meter requires extra logic to prevent it from counting twice.
  • The verifier has a special check to prevent functions (even those which are never called) starting in the middle of a lddw instruction.
  • The verifier has a special check to prevent the program from ending in a lddw instruction.
  • The verifier has an explicit check to prevent the first half of lddw occurring on its own an implicit check to prevent the second half of lddw occurring on its own.
  • The control-flow can not be redirected into the middle of a lddw instruction: conditional jumps, unconditional jumps, call and callx all require extra handling.

To end this madness this proposal introduces the lduw (load upper word immediate) instruction as a replacement for lddw in SBPFv2. It is like the or64 reg, imm instruction but the imm is implicitly shifted by 32 bit upwards (towards MSBs). And it is used like the ARM "move top"-instruction, except that it does not mask out the previous content of the upper half. That however does not matter because the typically preceding mov32 will already do so.

For example lddw r1, 0x1122334455667788 would be replaced by:

mov32 r1, 0x55667788
lduw r1, 0x11223344

@codecov-commenter
Copy link

codecov-commenter commented Jul 20, 2023

Codecov Report

Merging #486 (1794133) into main (4c68373) will increase coverage by 0.00%.
The diff coverage is 96.15%.

@@           Coverage Diff           @@
##             main     #486   +/-   ##
=======================================
  Coverage   89.67%   89.68%           
=======================================
  Files          23       23           
  Lines       10105    10117   +12     
=======================================
+ Hits         9062     9073   +11     
- Misses       1043     1044    +1     
Impacted Files Coverage Δ
src/disassembler.rs 92.92% <0.00%> (-0.45%) ⬇️
src/ebpf.rs 79.59% <ø> (ø)
src/assembler.rs 99.60% <100.00%> (+<0.01%) ⬆️
src/elf.rs 87.31% <100.00%> (+0.02%) ⬆️
src/interpreter.rs 97.94% <100.00%> (+0.01%) ⬆️
src/jit.rs 92.83% <100.00%> (+0.03%) ⬆️
src/static_analysis.rs 66.51% <100.00%> (ø)
src/verifier.rs 97.48% <100.00%> (-0.04%) ⬇️

@Lichtso Lichtso force-pushed the feature/load_upper_word_immediate branch from a66ca4c to 1794133 Compare July 20, 2023 21:37
@Lichtso Lichtso requested a review from alessandrod July 20, 2023 21:50
@Lichtso Lichtso mentioned this pull request Jul 21, 2023
18 tasks
_ => 1,
};
if executable.get_sbpf_version().disable_lddw() {
pc = program.len() / ebpf::INSN_SIZE;

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Imagine that 😍😂

@Lichtso Lichtso merged commit 5debf64 into main Jul 26, 2023
12 checks passed
@Lichtso Lichtso deleted the feature/load_upper_word_immediate branch July 26, 2023 10:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants