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

Request to release sources #3

Closed
jerinjoy opened this issue Aug 17, 2024 · 6 comments
Closed

Request to release sources #3

jerinjoy opened this issue Aug 17, 2024 · 6 comments
Assignees

Comments

@jerinjoy
Copy link

Could the sources be released instead?
The binaries can't be edited to provide bug fixes.
The tests can't be run on the public version of Spike because it accesses memory outside the default memory ranges. With the sources the memory ranges used by the test can be made configurable.
The binaries make the repo unnecessarily large.

@darshakk
Copy link
Collaborator

Thanks for opening the ticket, we plan to open source the source files in future releases as we figure out the logistics.
Meanwhile, do you have the test that did not work with public version of the Spike? We are restricting the memory accessed as following in the tests:

IO: 0x0000_0000 - 0x7FFF_FFFF
DRAM: 8000_0000 - 0xFFFF_FFFF

Please share the testname, so we can find the issue and fix it.

@jerinjoy
Copy link
Author

jerinjoy commented Aug 20, 2024

Thanks! Looking forward to trying out the sources.

❯ spike --isa=rv64gcv -l --log-commits  riscv_tests/bare_metal/supervisor/paging_sv39/rv_i/rv64i_0
Access exception occurred while loading payload riscv_tests/bare_metal/supervisor/paging_sv39/rv_i/rv64i_0:
Memory address 0x152335018 is invalid

We are restricting the memory accessed as following in the tests:
IO: 0x0000_0000 - 0x7FFF_FFFF
DRAM: 8000_0000 - 0xFFFF_FFFF

The ELF has load segments outside this range:

❯ riscv64-unknown-elf-readelf -l riscv_tests/bare_metal/supervisor/paging_sv39/rv_i/rv64i_0

Elf file type is EXEC (Executable file)
Entry point 0x80000000
There are 37 program headers, starting at offset 64

Program Headers:
  Type           Offset             VirtAddr           PhysAddr
                 FileSiz            MemSiz              Flags  Align
  RISCV_ATTRIBUT 0x0000000000025160 0x0000000000000000 0x0000000000000000
                 0x00000000000000e3 0x0000000000000000  R      0x1
  LOAD           0x0000000000001000 0x0000000080000000 0x0000000080000000
                 0x0000000000000804 0x0000000000000804  R E    0x1000
  LOAD           0x0000000000002000 0x0000000080010000 0x0000000080010000
                 0x00000000000019f0 0x00000000000019f0  R E    0x1000
  LOAD           0x0000000000004000 0x0000000083bb1000 0x0000000083bb1000
                 0x00000000000002e8 0x00000000000002e8  RW     0x1000
  LOAD           0x0000000000005000 0x00000000b55ca000 0x00000000b55ca000
                 0x0000000000000fb8 0x0000000000000fb8  RW     0x1000
  LOAD           0x0000000000006000 0x0000000152335000 0x0000000152335000
                 0x000000000000001d 0x000000000000001d  R E    0x1000
  LOAD           0x0000000000007000 0x000000021b388000 0x000000021b388000
                 0x0000000000000080 0x0000000000000080  RW     0x1000
  LOAD           0x0000000000008000 0x00000007e727e000 0x00000007e727e000
                 0x0000000000000ba8 0x0000000000000ba8  RW     0x1000
  LOAD           0x0000000000009000 0x0000000818fea000 0x0000000818fea000
                 0x00000000000007f0 0x00000000000007f0  RW     0x1000
..

@darshakk
Copy link
Collaborator

Thanks for pointing this out. This was the bug with addresses that were identity mapped (va=pa) where we generated virtual addresses first and then assigned that address to physical address. Since the VA size is dependent on the paging mode (sv39/48/57) selected, we were erroneously generating a physical address that was not following the provided DRAM range.

This will be fixed in the next release.

@nmatusTT
Copy link
Collaborator

Shooting for a release at the end of the month. This aims to include open source tests (.S) files in the main repo, and a zipped archive using the gitlab "releases" functionality.

@nmatusTT
Copy link
Collaborator

nmatusTT commented Oct 1, 2024

Hey @jerinjoy, we've updated our release flow to include the compiled ELFs in the releases section , and have the source code + linker scripts in the main repo. Both can be downloaded from the releases. This includes some qualification to the tests, and should be respecting the 4GB memory range now.

The linker scripts in v0.1.1 are incorrect and will be corrected shortly. If you notice any other issues, feel to comment or open another issue

@nmatusTT nmatusTT closed this as completed Oct 1, 2024
@jerinjoy
Copy link
Author

jerinjoy commented Oct 1, 2024

Thanks! I will try this out.

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

No branches or pull requests

3 participants