linux-mips.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: "Maciej W. Rozycki" <macro@imgtec.com>
To: James Hogan <james.hogan@imgtec.com>
Cc: Bhushan Attarde <bhushan.attarde@imgtec.com>,
	gdb-patches@sourceware.org,
	Matthew Fortune <Matthew.Fortune@imgtec.com>,
	Andrew Bennett <Andrew.Bennett@imgtec.com>,
	Jaydeep Patil <Jaydeep.Patil@imgtec.com>,
	linux-mips@linux-mips.org
Subject: Re: [PATCH 02/24]     Add MIPS32 FPU64 GDB target descriptions
Date: Wed, 12 Oct 2016 17:29:53 +0100	[thread overview]
Message-ID: <alpine.DEB.2.00.1610121701180.31859@tp.orcam.me.uk> (raw)
Message-ID: <20161012162953.6IETpn3BGIpniAg6zW5oeb1eGiG2IK5F8VXm7AkwkXQ@z> (raw)
In-Reply-To: <20161012135803.GT19354@jhogan-linux.le.imgtec.org>

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

 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

       reply	other threads:[~2016-10-12 16:30 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <1467038991-6600-1-git-send-email-bhushan.attarde@imgtec.com>
     [not found] ` <1467038991-6600-2-git-send-email-bhushan.attarde@imgtec.com>
     [not found]   ` <alpine.DEB.2.00.1607221827040.4076@tp.orcam.me.uk>
     [not found]     ` <20161012135803.GT19354@jhogan-linux.le.imgtec.org>
2016-10-12 16:29       ` Maciej W. Rozycki [this message]
2016-10-12 16:29         ` [PATCH 02/24] Add MIPS32 FPU64 GDB target descriptions Maciej W. Rozycki
2016-10-12 18:05         ` James Hogan
2016-10-12 18:05           ` James Hogan
2016-10-12 22:04           ` Maciej W. Rozycki
2016-10-12 22:04             ` Maciej W. Rozycki
2016-10-12 22:26             ` James Hogan
2016-10-12 22:26               ` James Hogan
2016-10-13 10:09             ` Matthew Fortune
2016-10-21 19:16               ` Maciej W. Rozycki
2016-10-21 19:24                 ` Maciej W. Rozycki

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=alpine.DEB.2.00.1610121701180.31859@tp.orcam.me.uk \
    --to=macro@imgtec.com \
    --cc=Andrew.Bennett@imgtec.com \
    --cc=Jaydeep.Patil@imgtec.com \
    --cc=Matthew.Fortune@imgtec.com \
    --cc=bhushan.attarde@imgtec.com \
    --cc=gdb-patches@sourceware.org \
    --cc=james.hogan@imgtec.com \
    --cc=linux-mips@linux-mips.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).