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

[Yao Qi <qiyaoltc at gmail dot com>]

On Thu, Nov 10, 2016 at 12:42:53PM +0000, Maciej W. Rozycki 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.

Yeah, there are different variants.  Let me be specific, with this patch
applied, on MIPS64, FR=0, how many bits needed for each FPR in 'g' packet?
I ask this question for this sentence "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." in Bhushan's mail.
IIUC, with this patch applied, on MIPS64, FR=0, FPR is 32-bit, but 64-bit
space is used in 'g' packet.  No?

> 
>  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).

Oh, it must be this patch
https://sourceware.org/ml/gdb-patches/2016-06/msg00511.html

> 
> > "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.

Why stubs doesn't transfer the upper bits of PFRs in case of MIPS32 FR=1?
because of keeping backward compatibility?  Does
register_remote_g_packet_guess help?

> 
> > 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).

If we have $fp_mode, that is better.

> 
>  The rest looks like a good plan to me, although I think I like your #1 
> proposal a bit better.
> 

Thanks for your feedback, Maciej.  I personally like #1 better too :)

-- 
Yao (齐尧)