On Wed, Oct 12, 2016 at 05:29:53PM +0100, Maciej W. Rozycki wrote: > Hi James, > > Thanks for your input! > > Cc-ing linux-mips for the discussion about a ptrace(2) kernel API update; > anyone interested in previous talk about this change please have a look > at: <https://sourceware.org/ml/gdb-patches/2016-06/msg00441.html> and > <https://sourceware.org/ml/gdb-patches/2016-10/msg00311.html> for the > earlier messages. > > > > Hmm, has Linux kernel support for CP0.Config5 accesses gone upstream > > > already? Can you give me an upstream commit ID and/or reference to the > > > discussion where it has been approved if so? > > > > I don't think it did go upstream yet. > > Good! > > > > More importantly, what do we need CP0.Config5 access for in the first > > > place? It looks to me like this bit is irrelevant to GDB as it does not > > > affect the native (raw) register format. So the only use would be to let > > > the user running a debugging session switch between the FRE and NFRE modes > > > without the need to poke at CP1C.FRE or CP1C.NFRE registers with a CTC1 > > > instruction, which by itself makes sense to me, but needs a further > > > consideration. > > > > It allows the FRE bit to be read (I seem to remember this was the only > > bit actually exposed through ptrace by the patch). > > Then I think it makes sense even more not to create this artificial API > and use the CP1C.FRE/CP1C.NFRE registers instead which do correspond to > what hardware presents to user software. well, barely. Linux at least doesn't enable Config5.UFE or Config5.UFR, since FP mode changes must be done for all threads in the process, so userland can't actually directly access those FCRs either. > Also with CP1C.UFR/CP1C.UNFR vs > CP0.Status; while we want to retain the latter register in the view for > historical reasons, it has always been read-only and I think it ought to > remain such, with any writes to CP0.Status.FR executed via the former CP1C > registers only. > > > FRE simply causes certain instructions (all single precision FP > > arithmetic instructions and FP word loads/stores) to trap to the kernel > > so that it can emulate a variation/subset of FR=0, so the debugger would > > use it to decide how to decode the single precision FP registers based > > on the double precision FP registers (iirc). > > I don't think there is any value in it for GDB, I think all 64-bit FP > registers ought to remain being presented as doubles and pairs of singles > regardless of the mode selected (and also possibly fixed-point longs and > pairs of fixed-point words). We don't know what's emulated and what's not > after all, and then the contents of FPRs are not interpreted by GDB itself > anyhow except in user-supplied expressions or assignment requests, which > for users' convenience I think should retain the maximum flexibility > possible. Well it technically depends on test_tsk_thread_flag(target, TIF_HYBRID_FPREGS) So it allows gdb to detect Linux's Hybrid FPU mode, and present the fp regs (e.g. f0 or f1) more like below to the user depending on the fp mode, which is surely much more intuitive from an assembly debugging point of view. Its also worth noting that "When software changes the value of this bit [Status.FR], the contents of the floating-point registers are UNPREDICTABLE". I.e. there is no architectural unifying register layout between FR=0 / FR=1, only convention. Linux will I presume intentionally save in old mode and restore in new mode on an fp mode change according to its own convention (such that the valid mode changes don't clobber register state). (disclaimer: I haven't looked at this gdb patchset in detail as to whether any of below has changed since I worked on it). (1) Even singles and doubles always overlap one another, as do odd singles and doubles when FR=1 (and FRE=0): /* (little endian) */ union __gdb_builtin_mips_fp64 { float f32; double f64; int32 i32; int64 i64; }; (2) Odd singles when FR=0 (there are no odd doubles): union __gdb_builtin_mips_fp32 { float f32; int32_t i32; }; (3) Odd singles and doubles when FR=1, FRE=1 don't overlap at all: struct __gdb_builtin_mips_fp96 { union { double f64; int64 i64; }; union { float f32; int32 i32; }; }; i.e. FR=0: (1) even double: FEDCBA9876543210 single: 76543210 (2) odd single: FEDCBA98 FR=1, FRE=0: (1) even double: FEDCBA9876543210 single: 76543210 (1) odd double: 0123456789ABCDEF single: 89ABCDEF FR=1, FRE=1: (1) even double: FEDCBA9876543210 single: 76543210 (Hybrid FPR emulation) (3) odd double: 0123456789ABCDEF single: FEDCBA98 (Hybrid FPR emulation) ) Cheers James > > So as I say it looks to me like the only, though obviously valid and > wholeheartedly supported, use for CP1C.FRE/CP1C.NFRE would be for user's > control of the execution environment. > > > > Additionally exposing CP0.Config5 may have security implications, > > > especially as parts of the register have not been defined yet in the > > > architectures and we'd have to force architecture maintainers somehow to > > > ask us every time they intend to add a bit to this register to check if > > > this has security implications and has to be avoided and/or explicitly > > > handled in software. > > > > yes, as above it explicity only shows certain bits. I'm fine with the > > api changing if necessary though since it isn't upstream. > > It sounds like a plan to me then -- any further questions or comments > about the kernel API part, anyone? > > Maciej
Attachment:
signature.asc
Description: Digital signature
