This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [PATCH 01/24] MIPS: Handle run-time reconfigurable FPR size
- From: "Maciej W. Rozycki" <macro at imgtec dot com>
- To: Yao Qi <qiyaoltc at gmail dot com>
- Cc: Bhushan Attarde <bhushan dot attarde at imgtec dot com>, "gdb-patches at sourceware dot org" <gdb-patches at sourceware dot org>, Matthew Fortune <Matthew dot Fortune at imgtec dot com>, James Hogan <James dot Hogan at imgtec dot com>, Andrew Bennett <Andrew dot Bennett at imgtec dot com>, Jaydeep Patil <Jaydeep dot Patil at imgtec dot com>
- Date: Thu, 10 Nov 2016 12:42:53 +0000
- Subject: Re: [PATCH 01/24] MIPS: Handle run-time reconfigurable FPR size
- Authentication-results: sourceware.org; auth=none
- References: <1467038991-6600-1-git-send-email-bhushan.attarde@imgtec.com> <CAH=s-POALmism689z+D2R63=WvYkkT7mdwiEpzWuo2PAmQtt5g@mail.gmail.com>
Yao,
I'll chime in in Bhushan's stead a bit as I've beaten the oddities of the
MIPS FPU to death throughout my dealings with the architecture (Bhushan,
you're of course welcome to reply too if you have anything to add).
On Tue, 8 Nov 2016, Yao Qi wrote:
> > This is implemented by retaining the raw register size for the FPRs at its
> > native size (64-bits; this is required for remote packet offsets to work
> > out correctly) and then truncating the cooked register size or not as
> > required. I have skipped `maintenance print registers' dumps here for
> > brevity, the types flip between "double" and "float" as expected.
> >
>
> If I understand you correctly, 64-bits are still transferred in remote protocol
> if FPRs are 32-bit.
Nope. In the FR=0 mode existing XML-described stubs present a file of 32
32-bit FPRs; this applies to MIPS32 targets only as so far we've had an
assumption that MIPS32 <=> FR=0 and MIPS64 <=> FR=1. I think switching
the view at this time would be an undesireable complication, as it would
be yet another variant we'd have to handle forever along with the new one.
FWIW I think the FR=0 mode should have been chosen to present the FPR
file as 16 64-bit FPRs instead, with the cooked register view used to
access register halves if required, but then I think a further
complication would arise with non-XML-described stubs.
> > The change also supports both XML and non-XML targets, but currently
> > bare-iron targets for which this update has any significant meaning do not
> > really support XML. Any XML target is supposed to always provide an FPU
> > description that matches the current setting of CP0.Status.FR, the new
> > code verifies this is always the case and rejects the description as
> > invalid otherwise.
>
> How do I understand "Any XML target is supposed to always provide
> an FPU description that matches the current setting of CP0.Status.FR"?
> I don't see how it is done in current GDB/GDBserver.
I do hope the last patch which adds `gdbserver' features en masse, which
I didn't get to to looking through yet, has it (although I think the
individual features should be included along with corresponding base
target and native support instead).
> "reconfigure FPRs" is a target description change to me, instead of a
> regcache change.
The regcache change is partly historic, because parts of the series
(dating back to 2005) were implemented even before we had XML
descriptions, and an FR transition is also supposed to work with
non-XML-described MIPS64 bare-metal targets (where the FPR file is
presented as 32 64-bit FPRs regardless of the FR setting). I do agree a
target description should take precedence where available, and certainly
with Linux targets where you cannot get a MIPS64 FR=0 configuration or a
non-XML-described MIPS32 FR=1 configuration).
What do you propose if we don't have a target description though? We
can't handle MIPS32 FR=1, because stubs won't transfer the upper halves of
FPRs, but we can handle MIPS64 FR=0, because we can simply ignore the
irrelevant upper halves.
> There are two different ways to support handling target description
> change in general. Other thoughts are welcome too.
>
> 1)
> - Implement target hook to_thread_architecture in ARCH-linux-nat.c,
> in which we can use ptrace to read the register A which indicates
> the target description is changed or not. In MIPS, register A is
> CP0.Status. Create target info and return the right gdbarch.
> [In MIPS, we have one gdbarch for 32-bit FPR and one gdbarch for
> 64-bit FPR]
> - Add register A to expedited registers, which is included in the stop
> reply from GDBserver.
I think I'd prefer this to be the $fp_mode register if available (as
it'll include the FRE setting as well), and the $status register
otherwise. Linux kernels which support the FRE setting will always make
$fp_mode available, as it'll be a part of the same change (unless someone
sneaks a broken patch through behind my back, as I still yet have to see
the kernel parts of this framework, let alone review them).
> - Add new gdbarch method target_description_changed_p, and its
> parameter is a vector of expedited registers got in
> remote.c:process_stop_reply. In default, it returns false. In
> MIPS, we can tell whether target description is changed by the
> value of and current gdbarch.
> - In remote.c:process_stop_reply, if
> gdbarch_target_description_changed_p returns true, invalidate all
> regcaches, request target description from gdbserver again, and get
> the updated target description.
>
> 2)
> - Add enum TARGET_WAITKIND_ARCH_CHANGED,
> - In ARCH-linux-nat.c, interpret/override linux_nat_wait. If
> linux_nat_wait returns, and there is a real event of stop, call
> ptrace to get the register A value, if it is different from what we
> remember in current gdbarch, mark the event pending and return
> TARGET_WAITKIND_ARCH_CHANGED,
> - In remote, add a new stop reply, T00arch, for example,
> https://sourceware.org/gdb/onlinedocs/gdb/Stop-Reply-Packets.html
> If remote.c:process_stop_reply sees T00arch, return
> TARGET_WAITKIND_ARCH_CHANGED.
> - In gdb core, if TARGET_WAITKIND_ARCH_CHANGED is reported by
> target_wait, call target_find_description, to update target
> description.
> - In gdbserver, add target method target_stopped_by_changed_arch,
> and use it in remote-utils.c:prepare_resume_reply. Get register A
> value by ptrace too, and return true if it is different from the
> knowledge in process_info.tdesc.
The rest looks like a good plan to me, although I think I like your #1
proposal a bit better.
Maciej