Message-ID: <alpine.DEB.2.21.2102101444140.35623@angie.orcam.me.uk>
Date:   Wed, 10 Feb 2021 15:15:10 +0100 (CET)
From:   "Maciej W. Rozycki" <macro@...am.me.uk>
To:     Daniel Thompson <daniel.thompson@...aro.org>
cc:     Arnd Bergmann <arnd@...nel.org>,
        Thomas Bogendoerfer <tsbogend@...ha.franken.de>,
        Arnd Bergmann <arnd@...db.de>,
        kernel test robot <lkp@...el.com>,
        Jiaxun Yang <jiaxun.yang@...goat.com>,
        Paul Cercueil <paul@...pouillou.net>,
        Paul Burton <paulburton@...nel.org>,
        linux-mips@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH 2/2] MIPS: make kgdb depend on FPU support

On Wed, 10 Feb 2021, Daniel Thompson wrote:

> >  NB if GDB sees a register padded out (FAOD it means all-x's rather than a 
> > hex string placed throughout the respective slot) in a `g' packet, then it 
> > will mark the register internally as "unavailable" and present it to the 
> > receiver of the information as such rather than giving any specific value.  
> > I don't remember offhand what the syntax for the `G' packet is in that 
> > case; possibly GDB just sends all-zeros, and in any case you can't make 
> > GDB write any specific value to such a register via any user
> > interface.
> 
> kgdb doesn't track register validity and adding would be a fairly big
> change. Everything internally (including some of the interactions with
> arch code) is based on updating a binary shadow of register state which
> is only bin2hex'ed just before transmitting a packet.

 I've had a peek and it doesn't appear to me it would be a big deal.

 We have `gdb_regs' defined as an array of longs.  We'd just need a second 
array for a register validity bitmap, which could for simplicity just have 
a single bit per each byte of `gdb_regs'.  It would then be updated in 
`pt_regs_to_gdb_regs' according to the result of `dbg_get_reg' across the 
number of bits given by `dbg_reg_def[i].size'.  And then `kgdb_mem2hex' 
would interpret the bitmap given as an extra argument accordingly.

 It looks to me like a couple of lines of extra code really.

> It will simply default them to zero and update them on a 'G' packet.

 Ack.

> >  The way the unavailability is shown depends on the interface used, i.e. 
> > it will be different between the `info all-registers'/`info register $reg' 
> > commands, and the `p $reg' command (or any expression involving `$reg'), 
> > and the MI interface.  But in any case it will be unambiguous.
> 
> I guess this probably does create a technical protocol violation since
> kgdb will reject per-register read/write for register that its report
> says are zero rather then invalid.

 Not a violation, as GDB won't ever issue a `p'/`P' packet for a register 
that is in the range covered by `g'/`G'.  This is by design.  I'd have to 
track down the justification, but this is the right thing really.

 Also there is no issue with returning a rubbish value written with `G', 
as the same already happens with any RSP debug stub (or for that matter 
native GDB target) that deals with read-only registers.  If you attempt to 
write one, then all the caches will keep the new value, and you will often 
have to make the target resume execution before the value reported is 
reset to the hardwired one.

 Debug stubs often cache registers for performance reasons, and may not 
even write them out unless execution is to be resumed, which often has 
serious consequences if a write to a hardware registers has side effects.  
For example I had that with an Intel Atom CPU switching between the real 
and the protected mode with a CR0 register write issued via a debug probe 
wired through the JTAG inteface.

 Caching is surely what Linux `gdbserver' does, as is what all JTAG debug 
interfaces do that I have come across, as JTAG access is usually painfully 
slow.  Therefore in many cases GDB's `flushregs' command won't help as the 
stub will happily resend what it has previously cached with any updates 
applied locally only.

 FWIW,

  Maciej

Powered by blists - more mailing lists