load_elf: change maximum memory address to SYSTEM.start() #878
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
load_elf takes a max_mem parameter before this change, MEM_SIZE was used as the parameter. Any memory region about `SYSTEM.start()` is intended for use by the zkVM and is not to be used by the ELF. Rather than have maximum memory as a parameter, use a hard-coded constant, GUEST_MAX_MEM as the upper bound for memory during elf load.
|
I'd prefer to use a parameter, so that we can use this same code for newer circuit versions that have a different max memory bound. |
Ah, that's a good point. Will fix tomorrow |
This reverts commit d03ce85.
…MEM_SIZE in risc0-zkvm MEM_SIZE indicates the size of the total memory in the zkVM. This information is not very useful because what users are actually concerned about are the maximum memory that guest programs can use. As of this writing anything between 0x400 and SYSTEM.start can be used for the guest. Anything between SYSTEM.start to MEM_SIZE should not be used by guests or it could result in unintended behavior in the guest program. Users call the load_elf function to create an instances of valid `Program` structs. Using MEM_SIZE as a parameter to this function allows load_elf to write between invalid regions for guest programs but it also enables this function to write between SYSTEM.start and MEM_SIZE which results in undesirable behavior. Prevent this by passing GUEST_MAX_MEM instead of MEM_SIZE to load_elf. Thinking about the usecase of MEM_SIZE, I didn't find that this is a useful value for the zkVM to export becasue users are actually interested in GUEST_MAX_MEM rather than MEM_SIZE. I've removed the re-export for MEM_SIZE from risc0-zkvm.
Benchmark for Linux-cuda 76556c2Click to hide benchmark
Benchmark for Linux-defaultBenchmark for macOS-default 76556c2Click to hide benchmark
Benchmark for macOS-metal 76556c2Click to hide benchmark
|
Benchmark for Linux-cudaBenchmark for Linux-default 359b16dClick to hide benchmark
Benchmark for macOS-default 359b16dClick to hide benchmark
Benchmark for macOS-metal 359b16dClick to hide benchmark
|
flaub
approved these changes
Sep 15, 2023
Benchmark for Linux-cuda 3e39786Click to hide benchmark
Benchmark for Linux-defaultBenchmark for macOS-default 3e39786Click to hide benchmark
Benchmark for macOS-metal 3e39786Click to hide benchmark
|
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
load_elf takes a max_mem parameter before this change, MEM_SIZE was used as the
parameter. Any memory region about
SYSTEM.start()is intended for use by thezkVM and is not to be used by the ELF. Rather than have maximum memory as a
parameter, use a hard-coded constant, GUEST_MAX_MEM as the upper bound for
memory during elf load. This PR also restricts the loader to no load above GUEST_MAX_MEM