ARM-software / sdm-api

Secure Debug Manager API
BSD 3-Clause "New" or "Revised" License
6 stars 0 forks source link

Add reset type parameter to reset callbacks. #3

Closed flit closed 2 years ago

flit commented 2 years ago

This PR defines an SDMResetType enumeration and type to be used as parameters to the reset callbacks. (In earlier versions of the SDM API there used to be an SDMResetType as well, but it had a different purpose and was passed to the SDM plugin rather than to the host.)


(Update 03-Feb-2022: Removed reset notification enums, so the text below is for historical reference only.)

Of special interest are the SDM_InstantaneousResetNotification and SDM_GatedResetNotification reset types. To copy the doc comment:

The notification reset types have special semantics. These reset types are intended to be used to inform the SDM host that the SDM plugin will be performing a system reset using its own implementation. For instance, it may be an IP-specific method of triggering reset used to establish a communications channel.

SDM_InstantaneousResetNotification should be used for cases where the reset is triggered immediately and the target cannot be held in reset. In this case, the actual reset must only be performed after the call to the resetStart() callback has completed.

SDM_GatedResetNotification is for cases where reset can be held asserted between the calls to resetStart() and resetFinish().

All of this PR is just a proposal. Let's discuss whether the changes are really necessary and how reset would be used by SDM plugins, hopefully with solid use cases, before committing these changes. I expect some changes to this PR to be required after discussion.

jamiebird-arm commented 2 years ago

Looks good - a couple of questions.

How does the SDM know which reset type to request/perform? This used to a be a user selection option in the old API and Arm DS. We discussed last year that ADAC has the ability to discover the execution context of the SDA, but the SDA is allowed to not respond to this command. And I also think the SDM would need to know the reset type before it can query the exec context - e.g. SDA runs in boot ROM so will need a reset to even open the SDM<->SDA comms channel?

Is it expected the the reset notifications will be sent via both resetStart() and resetEnd()? E.g. the following sequence:

... and:

... if so, I'm not sure both type are needed other than telling the debugger "held in reset during this period, don't do anything", is that the intention?

nadnoo01 commented 2 years ago

Looks good - a couple of questions.

How does the SDM know which reset type to request/perform? This used to a be a user selection option in the old API and Arm DS. We discussed last year that ADAC has the ability to discover the execution context of the SDA, but the SDA is allowed to not respond to this command. And I also think the SDM would need to know the reset type before it can query the exec context - e.g. SDA runs in boot ROM so will need a reset to even open the SDM<->SDA comms channel?

Is it expected the the reset notifications will be sent via both resetStart() and resetEnd()? E.g. the following sequence:

  • resetStart(SDM_InstantaneousResetNotification);
  • SDM performs instantaneous reset
  • resetEnd(SDM_InstantaneousResetNotification);

... and:

  • resetStart(SDM_GatedResetNotification);
  • SDM asserts reset
  • ... other stuff while reset asserted ...
  • SDM deasserts reset
  • resetEnd(SDM_GatedResetNotification);

... if so, I'm not sure both type are needed other than telling the debugger "held in reset during this period, don't do anything", is that the intention?

I second the point mentioned about how does the SDM know about the reset type to request/perform, as currently we have a workaround in place using preprocessor directives.

flit commented 2 years ago

How the plugin knows the reset type

The SDM_DefaultReset type constant is exists basically so the plugin doesn't have to know the reset type. If it does care, like if the plugin knows that the target needs to be reset in a particular way, then it can use the explicit reset types (or perform the reset itself and send notifications, though that's not possible for hw reset).

If there's a case for the plugin needing to know the default/selected reset type, then we can certainly add that to the open parameters struct. I just hadn't come up with a case for it.

Your comment about execution contexts reminds me that we probably need to pass the "connect mode" via open parameters, as we had talked about on Slack. I'll create an issue to track it.

Notifications

Yes, I was thinking the notifications should bracket the actual reset.

Instantaneous is easiest to think about.

Gated:

Looking at that now, I'm not entirely sure there's any difference from the debugger's perspective between instantaneous and gated. 😄 Maybe they should just be merged into a single type of notification.

flit commented 2 years ago

Re: #11

flit commented 2 years ago

The obvious difference between the gated and instantaneous notifications is that it would be possible for the debugger to perform some task while the target is held in reset.

Notifications may not be directly needed for the plugins being built to use v1.0, but I'm pretty certain they will be needed at some point, so it's less work overall to get this sorted now.

henriksandin commented 2 years ago

The way I've been thinking about this so far is that any call(s) to resetStart and resetEnd will be on behalf of the SDM plugin in order to complete the authentication process and end up in an authenticated state. The Embedded Workbench debugger does a whole lot of stuff before the debug session is up and running (from a user perspective), including multiple resets of potentially different types for different purposes. So far I've been thinking in terms of performing the authentication early and then, once authenticated, deal with this fact in the rest of the debug session start sequence/phases making sure (as far as possible) that the authenticated state is not lost. At present, thinking in terms of the above, it's a bit unsure whether some part of the debug session startup sequence after authentication would "clash" with the authentication, i.e. there is some action towards the hardware which would "break" the authenticated state. The debugger startup in the Embedded Workbench is highly configurable, possibly involving fairly arbitrary scripts which can be hooked in at different points in the session startup sequence. From this perspective, I'm not sure that there is anything in particular that the Embedded Workbench debugger would need to do in conjunction with calls to resetStart and resetEnd during authentication. Unless the authentication process would somehow "wrap" the entire debugger startup sequence, but I can't see how that would work out at the time being given the complexity in said sequence. Or is that (i.e. "wrapping" the entire startup in the authentication process) an approach which is necessary to take?

flit commented 2 years ago

That makes sense. And I think most professional debuggers are similarly configurable in terms of the startup sequence. Your comments mesh with Jamie's comment that the notifications probably aren't needed because the those resets would be performed only during the authentication process.

I'm leaning towards just removing the notifications for the 1.0 API. I'll update the PR.

It sounds like something that might be useful is a way for the debugger to know what kind of reset (if any) will cause the authentication to be lost. This probably can just be part of the manifest XML.

henriksandin commented 2 years ago

Approved by IAR.