-
Notifications
You must be signed in to change notification settings - Fork 1.2k
pta: stm32mp: add debug access PTA #7673
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: master
Are you sure you want to change the base?
Conversation
20c97e9 to
deeaa34
Compare
|
Reworked to not expose BSEC registers to the PTA. Comments addressed, thank you. |
| { 0xdd05bc8b, 0x9f3b, 0x49f0, \ | ||
| { 0xb6, 0x49, 0x01, 0xaa, 0x10, 0xc1, 0xc2, 0x10 } } | ||
|
|
||
| enum stm32_pta_dbg_profile { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Suggestion:
/* Debug support from external debug port */
PTA_STM32_DEBUG_PERIPHERAL_DBG_PROFILE = 0,
/* Hardware Debug Port (HDP) support (internal signals observation) */
PTA_STM32_DEBUG_HDP_DBG_PROFILE = 1,There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll rephrase but ok
| bool stm32_bsec_hdp_is_enabled(void); | ||
|
|
||
| /* | ||
| * Return true if coresight peripheral can be used. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
IIUC:
| * Return true if coresight peripheral can be used. | |
| * Return true if coresight is accessible from the external debug port. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nope, it's related the their use (ETM/ETR being able to trace etc...). External debug port is enabled with less.
core/pta/stm32mp/debug_access_pta.c
Outdated
| case PTA_STM32_DEBUG_CMD_GRANT_DBG_ACCESS: | ||
| return pta_dbg_grant_dbg_access(param_types, params); | ||
| default: | ||
| break; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| break; | |
| return TEE_ERROR_NOT_IMPLEMENTED; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It breaks to the error return but I'm not against this.
|
Comments addressed |
|
Comment addressed |
etienne-lms
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewed-by: Etienne Carriere <[email protected]> with remain review comment addressed.
Add the debug access PTA that is responsible of validating whether a given debug profile is configured or not. This basically means that the debug configuration should allow (at least!) access to the debug peripherals requiring the debug profile being checked. For now, as it is specific to BSEC, only embed the PTA if the BSEC support is embedded as well. Signed-off-by: Gatien Chevallier <[email protected]> Reviewed-by: Etienne Carriere <[email protected]>
In order to handle request on the debug configuration, default enable CFG_STM32_DEBUG_ACCESS_PTA to embed the debug access PTA. Signed-off-by: Gatien Chevallier <[email protected]> Reviewed-by: Etienne Carriere <[email protected]>
|
Comment addressed and tag applied, thank you |
tbourgoi
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
some comments on missing new lines at the end of the new files
| */ | ||
| #define PTA_STM32_DEBUG_CMD_GRANT_DBG_ACCESS 0x0 | ||
|
|
||
| #endif /* __PTA_STM32MP_DEBUG_ACCESS_H */ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please add an empty line at the end of the file
| .flags = PTA_DEFAULT_FLAGS | TA_FLAG_CONCURRENT | | ||
| TA_FLAG_DEVICE_ENUM, | ||
| .open_session_entry_point = pta_dbg_access_open_session, | ||
| .invoke_command_entry_point = pta_dbg_access_invoke_cmd); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ditto, new line is appreciated
|
Hello @tbourgoi, I do have new lines at the end of the files. They just don't show up on the Github interface |
This P-R adds a stm32 debug access PTA that allows the REE to check the debug configuration that is inaccessible to it. This will allow to use debug peripherals in the REE without unexpected errors or firewall exceptions.
Edit:
Link to kernel patchset