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

Updates 2.0.4 issue 153 #164

Open
wants to merge 6 commits into
base: updates-2.0.4
Choose a base branch
from

Conversation

IainCRobertson
Copy link
Collaborator

Awaiting confirmation from Paul and Greg that this addresses their concerns

payload.adoc Show resolved Hide resolved
payload.adoc Outdated
* Before the first sync packet, in order to ensure the decoder is aware of how the encoder is configured. It is recommended this be sent as soon as the encoder is enabled (*trTeEnable* changes from 0 to 1). This reduces the likelihood of having to generate two packets (support and sync-start) at the point tracing actually starts;
* When tracing ceases for any reason (*trTeEnable* set to 0, trace-off trigger, halt, reset, loss of filter qualification, etc.), in order to inform the decoder that the preceding packet reported the address of the final traced instruction;
* If one or more trace packets cannot be sent (for example, due back-pressure from the packet transport infrastructure);
* If the operating mode of the encoder changes.
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm afraid that a reader might confuse "operating mode of the encoder" with the hart privilege mode. It would help to give an example such as trTeInstNoAddrDiff.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@pdonahue-ventana that's a fair point. I have reworded.

* The operating mode changes;
* One or more trace packets cannot be sent (for example, due
back-pressure from the packet transport infrastructure).
* Before the first sync packet, in order to ensure the decoder is aware of how the encoder is configured. It is recommended this be sent as soon as the encoder is enabled (*trTeEnable* changes from 0 to 1). This reduces the likelihood of having to generate two packets (support and sync-start) at the point tracing actually starts;
Copy link
Collaborator

Choose a reason for hiding this comment

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

This seems to change the specification. It was previously required to generate Support on trace enable but now its a recommendation. Its unclear why the clause for generating Support on operating mode is dropped.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I don't believe it changes the spec, and it is certainly not my intention to do so. Bear in mind that when the spec was ratified, the common control spec didn't exist, and there was no trTeEnable bit defined. It was understood that there was some means of enabling the encoder but the precise mechanism was not defined. There are now several aspects to enabling: trTeEnable is the overall enable, but trTeInstTracing must also be set in order to emit any decodable trace packets (e.g. a sync-start). In the context of this section, the intended original meaning was "before any decodable trace is output" but this was not spelled out beyond "when enabled". But now that the control is defined the original wording is not sufficiently precise. What I'm attempting to do here is clarify that.
It's still mandatory to send a support packet before the first decodable trace packet (i.e. a sync-start). I'm simply recommending that it be sent as early as possible after the encoder is enabled.

@IainCRobertson IainCRobertson marked this pull request as ready for review November 12, 2024 17:33
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