Skip to content

Conversation

jwnrt
Copy link

@jwnrt jwnrt commented Sep 12, 2025

Fixes a bug where QEMU will fail to find the OTP drive on reset if the -daemonize flag is used. This flag detaches the QEMU subprocess from the current parent and changes its CWD to the root. It runs after realization and once all IOs are connected to chardevs.

If a relative path is given to -drive for the raw OTP image, it will become invalid when blk_set_perms tries to re-open the OTP image file again post-daemonization.

@jwnrt jwnrt force-pushed the jw/otp-perms-realize branch from b2c32af to 5272a41 Compare September 12, 2025 15:18
If we leave this until reset time, the QEMU process may have
`-daemonize`d and changed its directory to `/`. This will cause any
relative paths given to `-drive` arguments to be invalid.

Signed-off-by: James Wainwright <james.wainwright@lowrisc.org>
@jwnrt jwnrt force-pushed the jw/otp-perms-realize branch from 5272a41 to 5daef1e Compare September 12, 2025 15:21
Copy link

@AlexJones0 AlexJones0 left a comment

Choose a reason for hiding this comment

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

This LGTM after my comments are addressed. I've walked through the issue with @jwnrt and this seems like the correct solution as far as I understand, but I'd definitely wait until @rivos-eblot or @loiclefort can check first since maybe I'm missing some understanding about the different QEMU stages.

Edit: also maybe should this be extracted to its own function to avoid cluttering _realize?

@rivos-eblot
Copy link

I need to check what if there could be any issue with multiple resets and OTP/LC update. We never used/have to use -daemonize.

I guess that this issue might also affect other devices that use the same API (flash device for example)

@jwnrt
Copy link
Author

jwnrt commented Sep 12, 2025

Yes, flash_ctrl is also running this function and re-opening the file, but it seems to be happening pre-daemonization so relative paths are okay

@rivos-eblot
Copy link

I need to check what if there could be any issue with multiple resets and OTP/LC update. We never used/have to use -daemonize.

No regression found.

@jwnrt
Copy link
Author

jwnrt commented Sep 15, 2025

Thank you for checking

@jwnrt jwnrt merged commit 4b273eb into lowRISC:ot-earlgrey-9.2.0 Sep 15, 2025
8 checks passed
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