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

litex_json2renode: fix --bios-binary and add --opensbi-binary #1786

Merged
merged 1 commit into from
Sep 30, 2023

Conversation

AndrewD
Copy link
Collaborator

@AndrewD AndrewD commented Sep 23, 2023

--bios-binary is reverted to loading the bios into rom as per the documentation and litex defaults --opensbi-binary is added to load the openSBI binary into the opensbi memory region

The examples documented in the wiki now work again

Fixes #1785

The Generate Renode simulation wiki page should also have one more section:

Finally, you can run load the simulation by executing the command:

renode litex.resc

Then in the renode terminal window execute the start (or s) command to boot the bios

(litex_vexriscv) start

@MateuszKarlic I think this change makes sense but as you have done more significant changes in the past maybe you can confirm this approach.

@MateuszKarlic
Copy link
Contributor

@AndrewD I confirm that both changes make sense. It was my mistake to change behavior of this switch.

In your commit, I'd suggest setting PC for all cores when loading Bios, not just cpu0, unless this somehow breaks the examples:

for cpu_id in range(0, number_of_cores):
    result += f"cpu{cpu_id} PC {hex(bios_base)}\n"

Since the options are not mutually exclusive, it might be helpful to print message to user when loading both, that the PC will be set to rom_base not opensbi_base.

--bios-binary is reverted to loading the bios into _rom_ as per the documentation and litex defaults
--opensbi-binary is added to load the openSBI binary into the opensbi memory region

The examples documented in the wiki now work again
@AndrewD
Copy link
Collaborator Author

AndrewD commented Sep 25, 2023

@AndrewD I confirm that both changes make sense. It was my mistake to change behavior of this switch.

In your commit, I'd suggest setting PC for all cores when loading Bios, not just cpu0, unless this somehow breaks the examples:

for cpu_id in range(0, number_of_cores):
    result += f"cpu{cpu_id} PC {hex(bios_base)}\n"

I wasn't sure it made sense to boot the bios on all cores, but I'm only using single core ATM so either is fine with me.

Since the options are not mutually exclusive, it might be helpful to print message to user when loading both, that the PC will be set to rom_base not opensbi_base.

I wondered about making it mutually exclusive or adding a warning. As you've also raised this I'll make it mutually exclusive as I could not think of a reason to use both together.

@AndrewD AndrewD merged commit e8689eb into enjoy-digital:master Sep 30, 2023
1 check passed
@AndrewD
Copy link
Collaborator Author

AndrewD commented Sep 30, 2023

@MateuszKarlic thanks for the review!

@MateuszKarlic
Copy link
Contributor

Happy to help :)

@AndrewD AndrewD deleted the json2renode branch October 13, 2023 02:46
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.

litex_json2renode.py --bios-binary doesn't load bios to rom - wiki example doesn't work
2 participants