This is the mail archive of the gdb-patches@sourceware.org mailing list for the GDB project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH 01/24] MIPS: Handle run-time reconfigurable FPR size


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


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]