-
Notifications
You must be signed in to change notification settings - Fork 13
ot_rstmgr: Add option to treat unexpected resets as Power-on-Reset
#293
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
base: ot-9.2.0
Are you sure you want to change the base?
Conversation
ot_rstmgr, ot_darjeeling, ot_earlgrey: Reflect Power-on Reset (POR) on system resetot_rstmgr: Reflect Power-on Reset (POR) on system reset
|
I've not thought deeply about it, but can't this feature achieve with a ot_rstmgr property rather than being handled in the machine, since it's does not seem to be a HW feature but more a hack to use the qemu monitor? |
Yes, thinking about it I think you are correct, this should be on the rstmgr. I put this here because in reality the POR signal exists as a direct manual pad on the SoC-level that is then passed through, but all the logic that makes the distinction between the POR and the HW reset is actually internal to the rstmgr from looking at the RTL so I think that is the more correct place. I'll change that.
That is the motivation but I am not clear why you think this is not a HW feature - the rstmgr RTL always resets the POR reset info on a power-on-reset and otherwise assigns the reset info according to the reset type. The current QEMU rstmgr implementation effectively signals that POR should only be set in the first reset after realization and never again. It does not distinguish between the reset source. So you would need to recreate the entire machine to be able to emulate a POR signal. But in actual HW you would send Power-on Resets to the rstmgr at any point via the pad.
From my understanding I don't think the current monitor feature is wrong - calling a |
0854b5f to
43073ee
Compare
|
I've pushed the change to handle the logic internal to the rstmgr now - it turns out a lot simpler and I think it is more clear that the change should match the rstmgr HW. |
I think I start to understand your point, but QEMU is not really designed to support multiple kind of resets. Although the ResettableAPI is supposed to support several kinds of reset, they are not actually implemented for now. The OT ResetManager tracks different kind of resets (PoR, SW, Watchdog, etc.) The way it has been designed IIRC is that you need to connect the PoR pad to the reset manager ( I do not think it is possible to achieve a true PoR with system-reset, or at least each OT component needs to be checked, as several of them rely on the VM to be initialized as the true power up sequence. It might work .. or not. Trying to map QEMU reset onto OT reset is likely to break something somewhere. If there is a way to preserve the existing behavior, that should be fine. |
I see - I understand what you are saying now. I guess based on the docs there is also a mismatch here between QEMU / OpenTitan. So for example on a rstmgr HW reset request we call a "Cold" reset on the SoC (and later assert that all EG/DJ machine reset reasons are cold resets), but the rstmgr technically does not follow QEMU's definition of a cold reset as it does not get reset to the same initial state as at the start of QEMU. And I guess there is no nice mapping of this reset to the current reset types. I personally expected a monitor system reset to be a POR, but I guess it is actually a cold reset where cold ≠ POR.
I think this makes sense, I am trying to think about what is the best way to expose this functionality. It doesn't seem correct to me add OT-specific functionality to the QEMU monitor (hence I was avoiding that), but I am not sure what the best way to expose this POR pad is - a CharDev on the SoC / rstmgr? The goal is to implement a method of emulating using reset strappings to perform a POR without needing to kill and recreate the QEMU process, as a lot of OpenTitan SW is reliant on this rstmgr reset cause.
That makes sense, I understand better now why you say the QEMU/OT resets are so different to each other... For now I'll change this PR to retain the existing behaviour and gate this new functionality behind a property, and try and document it a bit better - does that sound okay to you? Otherwise, if you have any ideas of nice ways to expose the |
Adds an optional property (defautl disabled) to the rstmgr to treat any reset of unknown origin (i.e. not triggered by an `ot_rstmgr_reset_req`) as Power-on-Resets (PoR) rather than using the currently latched cause. This will enable resets through e.g. the QEMU monitor to be treated as a PoR, rather than just assuming that PoR can only occur when the system is created (in the first reset after realization). In the future, it might be that there should instead be some external mechanism for exposing a POR pad that will be used to signal `ot_rstmgr_reset_req(..., OT_RSTMGR_RESET_POR)` externally. Signed-off-by: Alex Jones <alex.jones@lowrisc.org>
43073ee to
daf5166
Compare
ot_rstmgr: Reflect Power-on Reset (POR) on system resetot_rstmgr: Add option to treat unexpected resets as Power-on-Reset
Why would it be? We already have an OT-specific monitor command, or rather Ibex-specific command: "
I think you do not need to add a new device & protocol if you use already use the monitor: you can add device properties and use I think this boils down to a recurring task for EerlGrey, is that it needs a padring device, where you can connect those kinds of signals, GPIOs, etc. We have an implementation for our chip for example, and this is how we trigger such reset or other I/O signals; to sum up the involved components look like the following:
I think it is far better than to add a new CharDev w/ a custom protocol: this implementation only uses existing QEMU components to trigger the reset manager PoR line from an external pad. From the monitor, that would look something like
|
I see - I thought generally the preference was to avoid changes to QEMU that would make it more difficult to rebase on upstream unless they are necessary, but I see there are other device-specific commands there as well so I guess this is fine to add to the monitor?
Yes probably this should be added to the open padring issue, though unfortunately I don't know if I have the capacity to look into properly resolving that at the moment. The solution you describe with QOM setting properties on the padring sounds a lot better than using a CharDev or adding a new monitor command. I could look into stubbing a basic padring device that just supports this property for now and then link that up to the rstmgr, do you think that sounds like a better fix? I wonder if the rstmgr property would be useful anyway for any other cases of resets coming from sources external to the rstmgr? |
I'd say this would generate some easy-to-fix conflicts.
I've created a fully untested skeleton, should you want to use it as for bootstrapping a padring: 011f00e I do not think you are using Python to drive the monitor; if you are, let me know. |
Yes, we are trying to address different needs with different time sequence with a single, very limited and yet-to-be-completed I think this is the topic I've refactored the most for the last nearly 3 years (with many try-and-error sessions :-)). It does not mean it has reached the best solution yet. However this version has not bumped into show stoppers and is much simpler and more generic that the previous iterations and scales well with several OT instances within the same machine. |
Ok great I will look into this, thanks for the link to the skeleton.
I am going through our host-side Rust code (OpenTitanLib) but I'd prefer if any solution I implement is compatible with the current Python scripting, so it'd be useful to know if there's anything extra I'd need to add there to support it.
I think the only issue that I have run into so far is with handling the Debug Module's HALTREQ for an unresponsive (resetting) hart, where ideally I need a way to have the vCPU call into the RV_DM to be halted when it begins executing instructions, which I would expect to map to a |
Fortunately, there is nothing specific to Python; the issue is when using Python: QEMU provides a QMP wrapper module, which is nice; however it is an
I think we would need a reference from the CPU to the DM module, which is not supposed to exist in QEMU. There may be a way through the RISCV CPU class, but this is for sure a high probability of upstream merge conflics, as RISCV target keeps being heavily modified on each QEMU major version. I do agree there is a missing piece of code here (and there are other bugs in the RISVC "HW" debug support...)
😄 |
See the relevant commit messages and comments for more details. This PR introduces the logic for both Earlgrey and Darjeeling to consider system resets that are not requested by the rstmgr hardware (e.g. aThis PR introduces the logic for the rstmgr to consider any reset not triggered from its internal request to be a POR, rather than just the first reset after initialization. This PR hence allows SW to correctly determine the reset reason after a system reset.system-resetcommand sent over the QEMU monitor) as Power-on Resets (PORs). This information is then forwarded to the rstmgr which it uses during its own reset to determine theRESET_INFOthat it should display.