Hi Mike,
On Wed, Oct 01, 2025 at 09:48:06AM +0100, Mike Leach wrote:
Hi Leo,
Overall I like the idea, but there are some minor issues that need addressing.
Thanks for review!
[...]
Claim Tag Handling
The claim tag is acquired and released in normal flow, on the other hand, the claim protocol is not applied in CPU idle flow - it simply restores the saved value.
The claim bits serve two purpose:
Exclusive access between the kernel driver and an external debugger. During CPU idle, the kernel driver locks the OS Lock, ensuring that the external debugger cannot access the trace unit. Therefore, it is safe to release the claim tag during idle.
Notification to supervisory software to save/restore context for external debuggers. The kernel driver does not touch the external debugger's claim bit (ETMCLAIM[0]).
Based on this, it is safe to acquire and release claim tag in the idle sequence.
Not sure I agree - in the original save restore the claim tag state was retained - so if an external claim was asserted it would not be lost. here if we do not retain the claim tag value, then an external state can be lost.
As described in PSCI (DEN0022F.b 1.3), section 6.8.1.3 "Saving and restoring trace context", when ETMCLAIM[0] is set to 1 by the external debugger, "The supervisory software will perform a save and restore sequence of the trace context". So the supervisory software (TF-a or EL2 hypervisor) should save and restore context (including the claim bit) for the external debugger.
It does not make much sense to use the ETM driver to save and restore only the claim bit while ignoring other registers. For a robust solution, the supervisory software would be better place for the job.
The debugger cannot access the device during idle/oslok, but it can detect that the device is powered down/oslocked and this potential is a change of behaviour from the external debug perspective, where a race to claim the resource now exists over powerdown were one did not before.
If we consider the supervisory software, then no race condition occurs. As the supervisory software must run prior to the Linux kernel during resume, it will safely restore claim bit for the external debugger.
Thoughts?
[...]
if (drvdata->nrseqstate) {state->trcseqrstevr = etm4x_read32(csa, TRCSEQRSTEVR);state->trcseqstr = etm4x_read32(csa, TRCSEQSTR);The TRCSEQSTR changes in use, but is not read back be the standard enable/disable flow. This does not matter in a normal enable/disable case, but in a save/restore we need to retain the current state. Probably should be added to the normal disable flow to read back this as we do with counters etc.
Good point. I will update in next spin.
Thanks, Leo