-
Notifications
You must be signed in to change notification settings - Fork 4
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
Interaction with trace #11
Comments
Conclusion is to use new signal (or a vector) rather than halted, @AoteJin to work on the details |
Revisit the e-trace and debug spec, trace-on/trace-off trigger are defined as sideband signal to trace encoder and their priority are UNDEFINED (debug spec says it should be defined by trace spec, while e-trace spec doesn't define it, hmmm... ). Thus, it cannot be a candidate for trace control. Based on discussion on TG meeting, adding another bit alongside halted signal to bundle a vector is friendly to future extension.
It should be general to accommodate both e-trace and n-trace. |
@bcstrongx @sifiverobert @IainCRobertson could you help review the above solution. If the idea is acceptable, the external debug security spec will follow this direction. |
This looks good to me. |
Looks good to me as well. |
Apologies for the delay - I've been on vacation. This looks good to me too. We will need an issue opening against the E-Trace spec. At some point the interface will be pulled out of the E-trace spec into a separate standalone doc, but either way this change will lead to a minor-version change (if still within E-Trace it would be 2.1.0). You should also add guidance about interoperability with trace encoders compatible with the current spec (OR these 2 bits together and connect to the TE's 'halted' input). Regarding your comment about trace-on/off priority, can you clarify what you mean exactly? The E-Trace spec is clear that tracing starts from the cycle in which trace-on is asserted, and stops after the cycle in which trace-off is asserted. By implication, if both are asserted together, only instructions retired in that cycle will be traced. I don't see an ambiguity about priority here. Or is your concern something else? |
Sure! I will open an issue for e-trace regarding the new signal.
Thanks for reminding! It is a really good point.
Actually, I mean the following case might be hazardous with trace-on/trace-off signals:
|
Case 1: assuming code is executing linearly and is active initially, trace will stop after 0x4, and then start at 0x8. If 0x4 is a 16-bit instruction then the instruction at 0x6 will not be traced (though I think you're assuming 32-bit instructions and 0x8 is the instruction after 0x4). Case 2: xtrcen if a level, not a pulse (same as halted signal). So if was low after 0x4, nothing will be traced again until it goes high. However, all signals on the interface are ignored when halted is high (or xtrcen is low), and this includes the triggers. So when xtrcen eventually goes high again, tracing will not start until there is a trace-on trigger. In other words trace on/off triggers are only effective if the code they are associated with is permitted to be traced. This could definitely be clarified. |
Change the trace signal that will be driven for security control to address issue #11
Yes, I am assuming 32-bits instruction only. |
Okay, so in that case, tracing should continue. The trace-off at 0x4 says "this is the last instruction traced", but the trace-on at 0x8 says "this is the 1st instruction traced". The end result is that the trace enable never actually goes low. However, I accept that this is not stated explicitly and could be clarified. |
The external debug security spec use the signal vector to inform the trace module that it should stop output per commit #19. |
Use existing trace-on/trace-off triggers
Potential race condition when trigger is set to those action.
The signal has to be in-band to be synchronized with trace output transaction.
With a new xtrcen signal in the interface
Same behavior as halted signal. Avoid confusion with ”halt” in Debug spec
The text was updated successfully, but these errors were encountered: