On Fri, Jan 08, 2021 at 12:00:55PM +0800, Tingwei Zhang wrote: > On Wed, Jan 06, 2021 at 08:23:56PM +0800, Mark Rutland wrote: > > On Wed, Jan 06, 2021 at 06:29:53PM +0800, ting...@codeaurora.org wrote: > > > Hi Will and Mark, > > > > Hi Tingwei, > > > > > In recent implementation of save/restore ARM debug registers in > > > EL2/EL3, we found it's necessary to know whether self-host debug is > > > enabled so EL2/EL3 can avoid saving/restoring debug registers but no > > > one is using debug. > > > > In what situation are you considering? I assume you mean idle sequences > > using CPU_SUSPEND? > > > > Generally our expectation for CPU_SUSPEND is: > > > > * Where StateType==0, the debug state is preserved with all other > > PE state. > > > > * Where StateType==1 and the PE returns "warm" without having entered a > > powerdown state, the debug state is preserved along with all other PE > > state. > > > > * Where StateType==1, and the PE returns "cold" after having entered a > > powerdown state (i.e. we return via the entry point address), the > > debug state is not preserved. > > > > ... and I'm missing where you could avoid saving the state. What > > situation(s) did you have in mind? > > > In StateType==1 case, EL2/EL3 can save debug registers before PE suspend > and restore them after PE resume. If EL2/EL3 doesn't know whether external > debugger or self-host debugger is using debug registers, it can save and > restore bindly everytime. However, if EL2/EL3 can get the information from > DBGCLAIM, it can only save/restore debug registers when debug is ongoing > which means DGBCLAIM[0] is set by external debugger or DGBCLAIM[1] is set > by self-host debugger.
I think I'm missing something here. Are you tring to enable self-hosted debug over powerdown events, or are you trying to optimize the save/restore of debug state? Linux doesn't support self-hosted debug over powerdown events, since we lose context anyway, and we need to get through a reasonable portion of setup code before it's safe to take debug exceptions again. Linux saves and restores the debug state in this case (e.g. we call hw_breakpoint_restore(cpu) in __cpu_suspend_exit()). So AFAICT, Linux never needs to set DBGCLAIM[1]. For "cold" returns we already have the relevant save/restore in Linux, and for "warm" returns the debug state must be preserved by the firmware. > > I was under the impression that this was solely for the benefit of an > > external debugger, and should have no functional impact on the PSCI > > implementation from the kernel's PoV, so as above I think we need a > > better description of the case you're trying to address. > > If self-host debugger like gdb/kgdb is used for debug, Kernel can set > DBGCLAIM[1] to inform EL2/EL3. I think there's additional complexity there. KGDB relies on other CPU state (e.g. MMU, VBARs, TPIDRs, SP_EL0, DAIF.D) being configured, so there will always be a blackout period over a power-down events where this is restored. The kernel itself can save/restore the debug state during this blackout period. What benefit do you see there being if FW saves/restores this? I'm very concerned that there are FW implementations out there which do not save/restore when DBGCLAIM[1] is set, and the spec isn't entirely clear as to what "debug context" consists of, so I suspect we don't have reliable behaviour in this area. Thanks, Mark.
