Re: Set DBGCLAIM when self-host debug is enabled

[Mark Rutland]

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, tingwei@xxxxxxxxxxxxxx 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.